From f70e405e6f5a448387228c69de9ac67a0f58e41b Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Thu, 4 Jun 2026 16:39:39 -0700 Subject: [PATCH 1/2] test: cover coalesced git metadata probes --- Sources/TabManager.swift | 23 +++- cmuxTests/TabManagerUnitTests.swift | 159 ++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 4 deletions(-) diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index a9a642e00c99..475fe275cef6 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -17,6 +17,12 @@ typealias Tab = Workspace private let tabManagerLogger = Logger(subsystem: "com.cmuxterm.app", category: "TabManager") +protocol WorkspaceGitMetadataReading: Sendable { + func workspaceMetadata(for directory: String) async -> GitWorkspaceMetadata +} + +extension GitMetadataService: WorkspaceGitMetadataReading {} + enum NewWorkspacePlacement: String, CaseIterable, Identifiable { case top case afterCurrent @@ -1200,6 +1206,7 @@ class TabManager: ObservableObject { // slugs) off the main actor. Stateless; the reads are pure functions of the // directory argument. private let gitMetadataService: GitMetadataService + private let workspaceGitMetadataReader: any WorkspaceGitMetadataReading // Resolves GitHub PR badges (slug resolution, REST fetch, candidate // matching). Stateless; the repo cache stays here in @@ -1217,10 +1224,12 @@ class TabManager: ObservableObject { autoWelcomeIfNeeded: Bool = true, commandRunner: any CommandRunning = CommandRunner(), gitMetadataService: GitMetadataService = GitMetadataService(), + workspaceGitMetadataReader: (any WorkspaceGitMetadataReading)? = nil, gitPollClock: any GitPollClock = SystemGitPollClock() ) { self.commandRunner = commandRunner self.gitMetadataService = gitMetadataService + self.workspaceGitMetadataReader = workspaceGitMetadataReader ?? gitMetadataService self.gitPollClock = gitPollClock #if DEBUG self.pullRequestProbeService = PullRequestProbeService( @@ -2715,8 +2724,13 @@ class TabManager: ObservableObject { return } + let reader = workspaceGitMetadataReader Task.detached(priority: .utility) { [weak self] in - guard let snapshot = await self?.initialWorkspaceGitMetadataSnapshot(for: expectedDirectory) else { + let snapshot = await Self.initialWorkspaceGitMetadataSnapshot( + for: expectedDirectory, + reader: reader + ) + guard self != nil else { return } guard !Task.isCancelled else { return } @@ -3002,10 +3016,11 @@ class TabManager: ObservableObject { } } - private nonisolated func initialWorkspaceGitMetadataSnapshot( - for directory: String + private nonisolated static func initialWorkspaceGitMetadataSnapshot( + for directory: String, + reader: any WorkspaceGitMetadataReading ) async -> InitialWorkspaceGitMetadataSnapshot { - let metadata = await gitMetadataService.workspaceMetadata(for: directory) + let metadata = await reader.workspaceMetadata(for: directory) guard metadata.isRepository else { return InitialWorkspaceGitMetadataSnapshot( isRepository: false, diff --git a/cmuxTests/TabManagerUnitTests.swift b/cmuxTests/TabManagerUnitTests.swift index 3503692ba3c2..35b603dde0e2 100644 --- a/cmuxTests/TabManagerUnitTests.swift +++ b/cmuxTests/TabManagerUnitTests.swift @@ -6,6 +6,7 @@ import WebKit import ObjectiveC.runtime import Bonsplit import UserNotifications +import CmuxGit #if canImport(cmux_DEV) @testable import cmux_DEV @@ -61,6 +62,75 @@ private func waitForCondition( return true } +private func restoreUserDefaultForTabManagerTests(_ value: Any?, key: String) { + let defaults = UserDefaults.standard + if let value { + defaults.set(value, forKey: key) + } else { + defaults.removeObject(forKey: key) + } +} + +private actor BlockingWorkspaceGitMetadataReader: WorkspaceGitMetadataReading { + private let metadata: GitWorkspaceMetadata + private var callCount = 0 + private var maxActiveCallCount = 0 + private var activeCallCount = 0 + private var callCountWaiters: [(Int, CheckedContinuation)] = [] + private var releaseContinuations: [CheckedContinuation] = [] + + init(metadata: GitWorkspaceMetadata) { + self.metadata = metadata + } + + func workspaceMetadata(for directory: String) async -> GitWorkspaceMetadata { + callCount += 1 + activeCallCount += 1 + maxActiveCallCount = max(maxActiveCallCount, activeCallCount) + resumeSatisfiedCallCountWaiters() + await withCheckedContinuation { continuation in + releaseContinuations.append(continuation) + } + activeCallCount -= 1 + return metadata + } + + func waitForCallCount(_ expected: Int) async { + guard callCount < expected else { return } + await withCheckedContinuation { continuation in + callCountWaiters.append((expected, continuation)) + } + } + + func releaseAll() { + let continuations = releaseContinuations + releaseContinuations.removeAll() + for continuation in continuations { + continuation.resume() + } + } + + var observedCallCount: Int { + callCount + } + + var observedMaxActiveCallCount: Int { + maxActiveCallCount + } + + private func resumeSatisfiedCallCountWaiters() { + var remaining: [(Int, CheckedContinuation)] = [] + for waiter in callCountWaiters { + if callCount >= waiter.0 { + waiter.1.resume() + } else { + remaining.append(waiter) + } + } + callCountWaiters = remaining + } +} + private struct ProcessRunResult { let status: Int32 let stdout: String @@ -673,6 +743,95 @@ final class TabManagerPullRequestProbeTests: XCTestCase { ) } + func testSameDirectoryInitialGitMetadataProbesShareOneSnapshotRead() async throws { + let defaults = UserDefaults.standard + let previousWatchGitStatus = defaults.object(forKey: SidebarWorkspaceDetailDefaults.watchGitStatusKey) + defaults.set(false, forKey: SidebarWorkspaceDetailDefaults.watchGitStatusKey) + defer { + restoreUserDefaultForTabManagerTests( + previousWatchGitStatus, + key: SidebarWorkspaceDetailDefaults.watchGitStatusKey + ) + } + + let directoryURL = FileManager.default.temporaryDirectory.appendingPathComponent( + "cmux-git-coalesced-probes-\(UUID().uuidString)", + isDirectory: true + ) + try FileManager.default.createDirectory(at: directoryURL, withIntermediateDirectories: true) + defer { + try? FileManager.default.removeItem(at: directoryURL) + } + + let reader = BlockingWorkspaceGitMetadataReader( + metadata: GitWorkspaceMetadata( + isRepository: true, + branch: "main", + isDirty: false, + indexSignature: "index", + indexContentSignature: "content", + headSignature: "head" + ) + ) + defer { + Task { + await reader.releaseAll() + } + } + + let manager = TabManager(workspaceGitMetadataReader: reader) + guard let workspace = manager.selectedWorkspace, + let mainPanelId = workspace.focusedPanelId, + let paneId = workspace.bonsplitController.focusedPaneId, + let splitPanel = workspace.newTerminalSplit(from: mainPanelId, orientation: .horizontal, focus: false), + let tabPanel = workspace.newTerminalSurface(inPane: paneId) else { + XCTFail("Expected selected workspace with three terminal panels") + return + } + + let panelIds = [mainPanelId, splitPanel.id, tabPanel.id] + for panelId in panelIds { + manager.updateSurfaceDirectory( + tabId: workspace.id, + surfaceId: panelId, + directory: directoryURL.path + ) + } + + defaults.set(true, forKey: SidebarWorkspaceDetailDefaults.watchGitStatusKey) + manager.sidebarGitMetadataWatchSettingsDidChangeForTesting() + + let firstRead = expectation(description: "first git snapshot read started") + Task { + await reader.waitForCallCount(1) + firstRead.fulfill() + } + await fulfillment(of: [firstRead], timeout: 1.0) + + let uncoalescedSecondRead = expectation(description: "uncoalesced second git snapshot read") + uncoalescedSecondRead.isInverted = true + Task { + await reader.waitForCallCount(2) + uncoalescedSecondRead.fulfill() + } + await fulfillment(of: [uncoalescedSecondRead], timeout: 0.2) + + let observedCallCount = await reader.observedCallCount + let observedMaxActiveCallCount = await reader.observedMaxActiveCallCount + XCTAssertEqual(observedCallCount, 1) + XCTAssertEqual(observedMaxActiveCallCount, 1) + + await reader.releaseAll() + XCTAssertTrue( + waitForCondition { + panelIds.allSatisfy { workspace.panelGitBranches[$0]?.branch == "main" } + }, + "One same-directory snapshot should update every queued panel." + ) + let finalObservedCallCount = await reader.observedCallCount + XCTAssertEqual(finalObservedCallCount, 1) + } + func testTrackedWorkspaceGitMetadataPollCandidatesExcludeDirectoriesWithoutResolvedGitMetadata() throws { let fileManager = FileManager.default let directoryURL = fileManager.temporaryDirectory.appendingPathComponent( From a23bf2cb7a8d0a739faa9caf962bc5a2b16e9cfd Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Thu, 4 Jun 2026 16:40:40 -0700 Subject: [PATCH 2/2] fix: coalesce sidebar git metadata probes --- .../Parsing/GitMetadataService+Index.swift | 32 +++- .../GitMetadataServiceTests.swift | 19 ++ Sources/TabManager.swift | 176 +++++++++++++++++- 3 files changed, 213 insertions(+), 14 deletions(-) diff --git a/Packages/CmuxGit/Sources/CmuxGit/Parsing/GitMetadataService+Index.swift b/Packages/CmuxGit/Sources/CmuxGit/Parsing/GitMetadataService+Index.swift index ebcac2296010..75dfae2473ab 100644 --- a/Packages/CmuxGit/Sources/CmuxGit/Parsing/GitMetadataService+Index.swift +++ b/Packages/CmuxGit/Sources/CmuxGit/Parsing/GitMetadataService+Index.swift @@ -1,6 +1,8 @@ import Foundation extension GitMetadataService { + private nonisolated static let gitIndexHexAlphabet = Array("0123456789abcdef".utf8) + /// Compares the working tree against the parsed index to decide dirtiness, /// returning the index signatures alongside. /// @@ -86,9 +88,7 @@ extension GitMetadataService { let mtimeNanoseconds = readBigEndianUInt32(bytes, at: offset + 12) let mode = readBigEndianUInt32(bytes, at: offset + 24) let size = readBigEndianUInt32(bytes, at: offset + 36) - let objectID = bytes[(offset + 40)..<(offset + 60)].map { - String(format: "%02x", $0) - }.joined() + let objectID = gitIndexHexString(bytes[(offset + 40)..<(offset + 60)]) let flags = readBigEndianUInt16(bytes, at: offset + 60) let pathLength = Int(flags & 0x0fff) let hasExtendedFlags = version >= 3 && (flags & 0x4000) != 0 @@ -157,7 +157,7 @@ extension GitMetadataService { } } - let checksum = bytes[(bytes.count - 20)..(_ bytes: S) -> String where S.Element == UInt8 { + var encoded: [UInt8] = [] + encoded.reserveCapacity(bytes.underestimatedCount * 2) + for byte in bytes { + encoded.append(gitIndexHexAlphabet[Int(byte >> 4)]) + encoded.append(gitIndexHexAlphabet[Int(byte & 0x0f)]) + } + return String(decoding: encoded, as: UTF8.self) + } + + private nonisolated static func gitIndexFixedWidthHexString(_ value: UInt64) -> String { + var encoded = Array(repeating: UInt8(ascii: "0"), count: 16) + var remaining = value + for index in stride(from: 15, through: 0, by: -1) { + encoded[index] = gitIndexHexAlphabet[Int(remaining & 0x0f)] + remaining >>= 4 + } + return String(decoding: encoded, as: UTF8.self) } /// The submodule's currently checked-out commit for a parent gitlink entry, @@ -251,7 +271,7 @@ extension GitMetadataService { guard let data = try? Data(contentsOf: indexURL), data.count >= 20 else { return nil } - return data.suffix(20).map { String(format: "%02x", $0) }.joined() + return gitIndexHexString(data.suffix(20)) } /// Decodes a git index v4 path strip-length varint, advancing `offset`. diff --git a/Packages/CmuxGit/Tests/CmuxGitTests/GitMetadataServiceTests.swift b/Packages/CmuxGit/Tests/CmuxGitTests/GitMetadataServiceTests.swift index 4c7897bc3a4f..c8a545d7d49f 100644 --- a/Packages/CmuxGit/Tests/CmuxGitTests/GitMetadataServiceTests.swift +++ b/Packages/CmuxGit/Tests/CmuxGitTests/GitMetadataServiceTests.swift @@ -180,6 +180,25 @@ import Testing #expect(snapshot.entries.map(\.path) == [longPrefix + "first.txt", longPrefix + "second.txt"]) } + @Test func indexHexFieldsDecodeWithoutChangingObjectAndSignatureText() throws { + let fixture = try GitRepositoryFixture() + try fixture.writeBranch("main") + let objectID = "000102030405060708090a0b0c0d0e0f10111213" + let trailer = Array(UInt8(0x14)...UInt8(0x27)) + try fixture.writeIndex(GitIndexFixture( + version: 2, + entries: [ + GitIndexFixture.Entry(path: "tracked.txt", objectID: objectID), + ], + trailer: trailer + )) + + let indexURL = fixture.gitDirectory.appendingPathComponent("index") + let snapshot = try #require(GitMetadataService.gitIndexSnapshot(indexURL: indexURL)) + #expect(snapshot.entries.map(\.objectID) == [objectID]) + #expect(snapshot.signature == "1415161718191a1b1c1d1e1f2021222324252627") + } + // MARK: Content signature stability @Test func contentSignatureIgnoresStatOnlyChanges() { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 475fe275cef6..22368410221a 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -23,6 +23,70 @@ protocol WorkspaceGitMetadataReading: Sendable { extension GitMetadataService: WorkspaceGitMetadataReading {} +private struct WorkspaceGitMetadataProbeWaiter { + let id: UUID + let continuation: CheckedContinuation +} + +actor WorkspaceGitMetadataProbeLimiter { + static let shared = WorkspaceGitMetadataProbeLimiter(limit: 2) + + private let limit: Int + private var activeCount = 0 + private var waiters: [WorkspaceGitMetadataProbeWaiter] = [] + private var cancelledWaiterIds: Set = [] + + init(limit: Int) { + self.limit = max(1, limit) + } + + func acquire() async -> Bool { + let id = UUID() + guard !Task.isCancelled else { return false } + if activeCount < limit { + activeCount += 1 + return true + } + + return await withTaskCancellationHandler { + await withCheckedContinuation { continuation in + if cancelledWaiterIds.remove(id) != nil { + continuation.resume(returning: false) + } else { + waiters.append(WorkspaceGitMetadataProbeWaiter(id: id, continuation: continuation)) + } + } + } onCancel: { + Task { + await self.cancelWaiter(id: id) + } + } + } + + func release() { + guard activeCount > 0 else { return } + while !waiters.isEmpty { + let waiter = waiters.removeFirst() + if cancelledWaiterIds.remove(waiter.id) != nil { + waiter.continuation.resume(returning: false) + continue + } + waiter.continuation.resume(returning: true) + return + } + activeCount -= 1 + } + + private func cancelWaiter(id: UUID) { + if let index = waiters.firstIndex(where: { $0.id == id }) { + let waiter = waiters.remove(at: index) + waiter.continuation.resume(returning: false) + } else { + cancelledWaiterIds.insert(id) + } + } +} + enum NewWorkspacePlacement: String, CaseIterable, Identifiable { case top case afterCurrent @@ -986,6 +1050,11 @@ class TabManager: ObservableObject { let panelId: UUID } + private struct WorkspaceGitSnapshotProbeRequest: Sendable { + let probeKey: WorkspaceGitProbeKey + let isLastAttempt: Bool + } + private enum WorkspaceGitProbeState: Equatable { case idle case inFlight(rerunPending: Bool) @@ -1145,6 +1214,9 @@ class TabManager: ObservableObject { private var workspaceGitMetadataWatcherSourceDirectoryByKey: [WorkspaceGitProbeKey: String] = [:] private var workspaceGitMetadataWatcherDescriptorRequestsByKey: [WorkspaceGitProbeKey: WorkspaceGitMetadataWatcherDescriptorRequest] = [:] private var workspaceGitMetadataWatcherDescriptorGeneration: UInt64 = 0 + private var workspaceGitSnapshotRequestsByDirectory: [String: [WorkspaceGitSnapshotProbeRequest]] = [:] + private var workspaceGitSnapshotTasksByDirectory: [String: Task] = [:] + private var workspaceGitSnapshotDirectoryByProbeKey: [WorkspaceGitProbeKey: String] = [:] private var workspaceGitMetadataFallbackTask: Task? private var lastSidebarGitMetadataWatchEnabled = SidebarWorkspaceDetailDefaults.watchGitStatusValue(defaults: .standard) private var lastSidebarPullRequestPollingEnabled = SidebarWorkspaceDetailDefaults.pullRequestPollingEnabled(defaults: .standard) @@ -1310,6 +1382,9 @@ class TabManager: ObservableObject { for task in workspaceGitProbeTasksByKey.values { task.cancel() } + for task in workspaceGitSnapshotTasksByDirectory.values { + task.cancel() + } workspacePullRequestRefreshTask?.cancel() } @@ -1432,6 +1507,7 @@ class TabManager: ObservableObject { task.cancel() } workspaceGitProbeTasksByKey.removeAll() + cancelAllWorkspaceGitSnapshotTasks() workspaceGitTrackedDirectoryByKey.removeAll() workspaceGitCleanIndexSignatureByKey.removeAll() workspaceGitCleanIndexContentSignatureByKey.removeAll() @@ -2724,32 +2800,116 @@ class TabManager: ObservableObject { return } + enqueueWorkspaceGitMetadataSnapshotRequest( + probeKey: probeKey, + expectedDirectory: expectedDirectory, + isLastAttempt: isLastAttempt + ) + } + + private func enqueueWorkspaceGitMetadataSnapshotRequest( + probeKey: WorkspaceGitProbeKey, + expectedDirectory: String, + isLastAttempt: Bool + ) { + let request = WorkspaceGitSnapshotProbeRequest( + probeKey: probeKey, + isLastAttempt: isLastAttempt + ) + if let currentDirectory = workspaceGitSnapshotDirectoryByProbeKey[probeKey], + currentDirectory != expectedDirectory { + removeWorkspaceGitSnapshotRequest(for: probeKey) + } + workspaceGitSnapshotDirectoryByProbeKey[probeKey] = expectedDirectory + if var requests = workspaceGitSnapshotRequestsByDirectory[expectedDirectory], + let existingRequestIndex = requests.firstIndex(where: { $0.probeKey == probeKey }) { + requests[existingRequestIndex] = request + workspaceGitSnapshotRequestsByDirectory[expectedDirectory] = requests + } else { + workspaceGitSnapshotRequestsByDirectory[expectedDirectory, default: []].append(request) + } + guard workspaceGitSnapshotTasksByDirectory[expectedDirectory] == nil else { +#if DEBUG + cmuxDebugLog( + "workspace.gitProbe.joinSnapshot dir=\(expectedDirectory) " + + "queued=\(workspaceGitSnapshotRequestsByDirectory[expectedDirectory]?.count ?? 0)" + ) +#endif + return + } + let reader = workspaceGitMetadataReader - Task.detached(priority: .utility) { [weak self] in + workspaceGitSnapshotTasksByDirectory[expectedDirectory] = Task.detached(priority: .utility) { [weak self] in + let didAcquirePermit = await WorkspaceGitMetadataProbeLimiter.shared.acquire() + guard didAcquirePermit else { return } + defer { + Task { + await WorkspaceGitMetadataProbeLimiter.shared.release() + } + } + + guard !Task.isCancelled else { return } let snapshot = await Self.initialWorkspaceGitMetadataSnapshot( for: expectedDirectory, reader: reader ) - guard self != nil else { - return - } guard !Task.isCancelled else { return } await MainActor.run { [weak self] in - self?.applyWorkspaceGitMetadataSnapshot( + guard !Task.isCancelled else { return } + self?.applyWorkspaceGitMetadataSnapshotBatch( snapshot, - probeKey: probeKey, - expectedDirectory: expectedDirectory, - isLastAttempt: isLastAttempt + expectedDirectory: expectedDirectory ) } } } + private func applyWorkspaceGitMetadataSnapshotBatch( + _ snapshot: InitialWorkspaceGitMetadataSnapshot, + expectedDirectory: String + ) { + workspaceGitSnapshotTasksByDirectory.removeValue(forKey: expectedDirectory) + let requests = workspaceGitSnapshotRequestsByDirectory.removeValue(forKey: expectedDirectory) ?? [] + for request in requests { + workspaceGitSnapshotDirectoryByProbeKey.removeValue(forKey: request.probeKey) + applyWorkspaceGitMetadataSnapshot( + snapshot, + probeKey: request.probeKey, + expectedDirectory: expectedDirectory, + isLastAttempt: request.isLastAttempt + ) + } + } + + private func removeWorkspaceGitSnapshotRequest(for key: WorkspaceGitProbeKey) { + guard let directory = workspaceGitSnapshotDirectoryByProbeKey.removeValue(forKey: key), + var requests = workspaceGitSnapshotRequestsByDirectory[directory] else { + return + } + requests.removeAll { $0.probeKey == key } + if requests.isEmpty { + workspaceGitSnapshotRequestsByDirectory.removeValue(forKey: directory) + workspaceGitSnapshotTasksByDirectory.removeValue(forKey: directory)?.cancel() + } else { + workspaceGitSnapshotRequestsByDirectory[directory] = requests + } + } + + private func cancelAllWorkspaceGitSnapshotTasks() { + for task in workspaceGitSnapshotTasksByDirectory.values { + task.cancel() + } + workspaceGitSnapshotTasksByDirectory.removeAll() + workspaceGitSnapshotRequestsByDirectory.removeAll() + workspaceGitSnapshotDirectoryByProbeKey.removeAll() + } + private func cancelWorkspaceGitProbeTask(for key: WorkspaceGitProbeKey) { workspaceGitProbeTasksByKey.removeValue(forKey: key)?.cancel() } private func clearWorkspaceGitProbe(_ key: WorkspaceGitProbeKey) { + removeWorkspaceGitSnapshotRequest(for: key) workspaceGitProbeStateByKey.removeValue(forKey: key) workspaceGitCleanIndexSignatureByKey.removeValue(forKey: key) workspaceGitCleanIndexContentSignatureByKey.removeValue(forKey: key)