From 1352e0b891093ec350d60df7bad6475d8e2cfcf2 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Mon, 17 Aug 2026 13:16:47 +1000 Subject: [PATCH 1/9] fix(swift-ios): show cached source control status --- apps/swift-ios/App/NativeFeatureClient.swift | 42 +++++ .../swift-ios/App/NativeWorkspaceMapper.swift | 59 +++++++ .../Features/Shared/FeatureClient.swift | 15 ++ .../FeatureSourceControlView.swift | 15 +- .../FeatureTests/FeatureToolStateTests.swift | 165 ++++++++++++++++++ 5 files changed, 292 insertions(+), 4 deletions(-) diff --git a/apps/swift-ios/App/NativeFeatureClient.swift b/apps/swift-ios/App/NativeFeatureClient.swift index d7a8d4645baa..26ee7dfb9543 100644 --- a/apps/swift-ios/App/NativeFeatureClient.swift +++ b/apps/swift-ios/App/NativeFeatureClient.swift @@ -1883,6 +1883,48 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, ) } + func sourceControlStatuses( + threadID: String + ) async throws -> AsyncThrowingStream { + let route = try threadRoute(for: threadID) + let context = try workspaceContext(route: route) + let events = await route.client.vcsStatusEvents(cwd: context.cwd) + let (statuses, continuation) = AsyncThrowingStream.makeStream( + of: FeatureSourceControlStatus.self, + bufferingPolicy: .bufferingNewest(2) + ) + let task = Task { + var accumulator = NativeSourceControlStatusAccumulator() + do { + for try await event in events { + if let status = accumulator.consume(event) { + continuation.yield(status) + } + if accumulator.isComplete { + continuation.finish() + return + } + } + if Task.isCancelled { + continuation.finish() + } else { + try accumulator.validateEnd() + continuation.finish() + } + } catch is CancellationError { + continuation.finish() + } catch { + if Task.isCancelled { + continuation.finish() + } else { + continuation.finish(throwing: error) + } + } + } + continuation.onTermination = { @Sendable _ in task.cancel() } + return statuses + } + func performSourceControlAction( threadID: String, action: FeatureSourceControlAction, diff --git a/apps/swift-ios/App/NativeWorkspaceMapper.swift b/apps/swift-ios/App/NativeWorkspaceMapper.swift index 8a7ea4ff616f..5095e3c510b7 100644 --- a/apps/swift-ios/App/NativeWorkspaceMapper.swift +++ b/apps/swift-ios/App/NativeWorkspaceMapper.swift @@ -90,6 +90,33 @@ enum NativeWorkspaceMapper { ) } + static func sourceControl( + local: VCSLocalStatus, + remote: VCSRemoteStatus? + ) -> FeatureSourceControlStatus { + FeatureSourceControlStatus( + isRepository: local.isRepo, + branch: local.refName, + aheadCount: remote?.aheadCount ?? 0, + behindCount: remote?.behindCount ?? 0, + files: local.workingTree.files.map { + FeatureSourceControlFile( + path: $0.path, + state: .modified, + isStaged: false + ) + }, + pullRequest: remote?.pr.map { + FeaturePullRequest( + number: $0.number, + title: $0.title, + state: $0.state, + url: URL(string: $0.url) + ) + } + ) + } + static func gitAction(_ action: FeatureSourceControlAction) -> GitStackedAction { switch action { case .commit: .commit @@ -342,3 +369,35 @@ enum NativeWorkspaceMapper { ) } } + +struct NativeSourceControlStatusAccumulator { + private var local: VCSLocalStatus? + private(set) var isComplete = false + + mutating func consume(_ event: VCSStatusEvent) -> FeatureSourceControlStatus? { + switch event { + case let .snapshot(nextLocal, remote): + local = nextLocal + isComplete = remote != nil || !nextLocal.isRepo || !nextLocal.hasPrimaryRemote + return NativeWorkspaceMapper.sourceControl(local: nextLocal, remote: remote) + case let .localUpdated(nextLocal): + local = nextLocal + isComplete = !nextLocal.isRepo || !nextLocal.hasPrimaryRemote + return NativeWorkspaceMapper.sourceControl(local: nextLocal, remote: nil) + case let .remoteUpdated(remote): + guard let local else { return nil } + isComplete = true + return NativeWorkspaceMapper.sourceControl(local: local, remote: remote) + } + } + + var prematureEndError: RPCError { + RPCError.protocolViolation( + "The source-control status stream ended before completion." + ) + } + + func validateEnd() throws { + guard isComplete else { throw prematureEndError } + } +} diff --git a/apps/swift-ios/Features/Shared/FeatureClient.swift b/apps/swift-ios/Features/Shared/FeatureClient.swift index 67df5e51d16b..631456c71c26 100644 --- a/apps/swift-ios/Features/Shared/FeatureClient.swift +++ b/apps/swift-ios/Features/Shared/FeatureClient.swift @@ -117,6 +117,9 @@ public protocol FeatureClient: AnyObject { ) async throws -> FeatureReviewFileContents? func sourceControlStatus(threadID: String) async throws -> FeatureSourceControlStatus + func sourceControlStatuses( + threadID: String + ) async throws -> AsyncThrowingStream func performSourceControlAction( threadID: String, action: FeatureSourceControlAction, @@ -335,6 +338,18 @@ public extension FeatureClient { throw FeatureCapabilityUnavailable("Source control") } + func sourceControlStatuses( + threadID: String + ) async throws -> AsyncThrowingStream { + let status = try await sourceControlStatus(threadID: threadID) + let (stream, continuation) = AsyncThrowingStream.makeStream( + of: FeatureSourceControlStatus.self + ) + continuation.yield(status) + continuation.finish() + return stream + } + func performSourceControlAction( threadID: String, action: FeatureSourceControlAction, diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index ea26b9fcd97b..f91b59f00405 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -100,7 +100,7 @@ public struct FeatureSourceControlView: View { Label(action.title, systemImage: action.icon) .frame(maxWidth: .infinity, alignment: .leading) } - .disabled(isRunningAction) + .disabled(isLoading || isRunningAction) } } @@ -131,7 +131,9 @@ public struct FeatureSourceControlView: View { } .listStyle(.insetGrouped) .scrollContentBackground(.hidden) - .refreshable { await load() } + .refreshable { + if !isLoading { await load() } + } .overlay { if isRunningAction { ProgressView() @@ -154,8 +156,13 @@ public struct FeatureSourceControlView: View { isLoading = true defer { isLoading = false } do { - status = try await client.sourceControlStatus(threadID: threadID) - errorMessage = nil + let statuses = try await client.sourceControlStatuses(threadID: threadID) + for try await nextStatus in statuses { + status = nextStatus + errorMessage = nil + } + } catch is CancellationError { + return } catch { errorMessage = error.localizedDescription } diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift index 871f08d926b8..4653a849a72a 100644 --- a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift @@ -3,6 +3,171 @@ import Testing @Suite("Thread tool state") struct FeatureToolStateTests { + private static func vcsLocal( + isRepo: Bool = true, + hasPrimaryRemote: Bool = true, + refName: String? = "feature/cached", + files: [String] = [] + ) -> VCSLocalStatus { + VCSLocalStatus( + isRepo: isRepo, + sourceControlProvider: nil, + hasPrimaryRemote: hasPrimaryRemote, + isDefaultRef: false, + refName: refName, + hasWorkingTreeChanges: files.isEmpty == false, + workingTree: VCSWorkingTree( + files: files.map { + VCSWorkingTreeFile(path: $0, insertions: 1, deletions: 0) + }, + insertions: files.count, + deletions: 0 + ) + ) + } + + private static func vcsRemote( + aheadCount: Int, + behindCount: Int = 0, + pullRequest: VCSChangeRequest? = nil + ) -> VCSRemoteStatus { + VCSRemoteStatus( + hasUpstream: true, + aheadCount: aheadCount, + behindCount: behindCount, + aheadOfDefaultCount: nil, + pr: pullRequest + ) + } + + @Test( + "Cached local status is available before remote status", + .bug("https://github.com/saphid/t3code-personal/issues/107") + ) + func cachedLocalStatusArrivesFirst() throws { + var accumulator = NativeSourceControlStatusAccumulator() + + let consumedLocal = accumulator.consume( + .snapshot( + local: Self.vcsLocal(files: ["App/NativeFeatureClient.swift"]), + remote: nil + ) + ) + let localOnly = try #require(consumedLocal) + + #expect(localOnly.branch == "feature/cached") + #expect(localOnly.files.map(\.path) == ["App/NativeFeatureClient.swift"]) + #expect(localOnly.aheadCount == 0) + #expect(localOnly.pullRequest == nil) + #expect(accumulator.isComplete == false) + } + + @Test( + "Remote status combines with the latest local status", + .bug("https://github.com/saphid/t3code-personal/issues/107") + ) + func remoteStatusUsesLatestLocalState() throws { + var accumulator = NativeSourceControlStatusAccumulator() + _ = accumulator.consume( + .snapshot(local: Self.vcsLocal(refName: "feature/old"), remote: nil) + ) + _ = accumulator.consume( + .localUpdated(Self.vcsLocal(refName: "feature/new", files: ["new.swift"])) + ) + let pullRequest = VCSChangeRequest( + number: 107, + title: "Cached status", + url: "https://github.com/saphid/t3code-personal/pull/107", + baseRef: "main", + headRef: "feature/new", + state: "OPEN" + ) + + let consumedRemote = accumulator.consume( + .remoteUpdated( + Self.vcsRemote( + aheadCount: 2, + behindCount: 1, + pullRequest: pullRequest + ) + ) + ) + let combined = try #require(consumedRemote) + + #expect(combined.branch == "feature/new") + #expect(combined.files.map(\.path) == ["new.swift"]) + #expect(combined.aheadCount == 2) + #expect(combined.behindCount == 1) + #expect(combined.pullRequest?.number == 107) + #expect(accumulator.isComplete) + } + + @Test( + "Remote-before-local ordering is ignored safely", + .bug("https://github.com/saphid/t3code-personal/issues/107") + ) + func remoteBeforeLocalIsSafe() throws { + var accumulator = NativeSourceControlStatusAccumulator() + + let remoteBeforeLocal = accumulator.consume( + .remoteUpdated(Self.vcsRemote(aheadCount: 9)) + ) + #expect(remoteBeforeLocal == nil) + let consumedLocal = accumulator.consume( + .localUpdated(Self.vcsLocal(refName: "feature/local")) + ) + let localOnly = try #require(consumedLocal) + let consumedRemote = accumulator.consume( + .remoteUpdated(Self.vcsRemote(aheadCount: 3)) + ) + let combined = try #require(consumedRemote) + + #expect(localOnly.aheadCount == 0) + #expect(combined.branch == "feature/local") + #expect(combined.aheadCount == 3) + #expect(accumulator.isComplete) + } + + @Test( + "Terminal local states do not wait for remote status", + .bug("https://github.com/saphid/t3code-personal/issues/107"), + arguments: [ + Self.vcsLocal(isRepo: false, hasPrimaryRemote: false, refName: nil), + Self.vcsLocal(hasPrimaryRemote: false, refName: "local-only"), + ] + ) + func terminalLocalStatesAreExplicit(local: VCSLocalStatus) throws { + var accumulator = NativeSourceControlStatusAccumulator() + + let consumed = accumulator.consume(.snapshot(local: local, remote: nil)) + let status = try #require(consumed) + + #expect(status.isRepository == local.isRepo) + #expect(status.branch == local.refName) + #expect(status.pullRequest == nil) + #expect(accumulator.isComplete) + } + + @Test( + "Premature stream end has an explicit protocol error", + .bug("https://github.com/saphid/t3code-personal/issues/107") + ) + func prematureStreamEndIsExplicit() { + let accumulator = NativeSourceControlStatusAccumulator() + + do { + try accumulator.validateEnd() + Issue.record("Expected the incomplete stream to report a protocol error.") + } catch let error as RPCError { + #expect( + error.errorDescription + == "The source-control status stream ended before completion." + ) + } catch { + Issue.record("Unexpected stream error: \(error)") + } + } + @Test func fileFilteringKeepsDirectoriesFirstAndHonorsHiddenFiles() { let entries = [ From 785cad07deffffb664c59ab75375632d600138f4 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Mon, 17 Aug 2026 23:12:18 +1000 Subject: [PATCH 2/9] fix(swift-ios): keep source control refresh authoritative and bounded Follow-up to the cached-status change, from an independent review of it. Showing the cached status first is right, but the streaming path had replaced the forced refresh outright and made the screen depend on an event the server does not guarantee to send. - Explicit refresh affordances (toolbar reload, pull-to-refresh) go back through `vcs.refreshStatus`. `streamStatus` is cache-first over a cache with no TTL, so a reload would otherwise only replay what the server already had, and working-tree changes made outside the app would never appear on the one screen meant to show them. Entering the screen still streams the cached status first. - Bound the stream. `updateCachedRemoteStatus` publishes only when the remote fingerprint changes, and a failed remote refresh backs off silently, so the remote half may never arrive; subscriptions carry no deadline. Without a bound the screen could sit loading forever. - Stop gating actions on `isLoading`. Combined with the above, a stalled stream left a fully populated screen where every action and the reload button were disabled, pull-to-refresh was a silent no-op, and nothing explained why. - Surface mid-stream failures. Once a status renders, the unavailable state is unreachable, so an error after the first status was stored and never shown. - Distinguish a pending remote from zero. Ahead/behind and the pull request now read as "not yet known" instead of "0 ahead, 0 behind, no PR", which had offered Create Pull Request for a branch that may already have one. - Carry the last known remote across local-only updates and retain a remote that arrives before the first local, mirroring `applyGitStatusStreamEvent`; latch completion so a later local-only event cannot reopen a finished stream. - Adopt the surrounding streaming conventions in NativeFeatureClient: weak self plus the environment-generation guard used by sibling subscriptions. --- apps/swift-ios/App/NativeFeatureClient.swift | 33 +++++- .../swift-ios/App/NativeWorkspaceMapper.swift | 38 ++++--- .../Features/Shared/FeatureToolModels.swift | 11 +- .../FeatureSourceControlView.swift | 70 +++++++++--- .../FeatureTests/FeatureToolStateTests.swift | 102 +++++++++++------- 5 files changed, 182 insertions(+), 72 deletions(-) diff --git a/apps/swift-ios/App/NativeFeatureClient.swift b/apps/swift-ios/App/NativeFeatureClient.swift index 26ee7dfb9543..7207959e83fe 100644 --- a/apps/swift-ios/App/NativeFeatureClient.swift +++ b/apps/swift-ios/App/NativeFeatureClient.swift @@ -35,6 +35,7 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, private static let olderThreadPageUserTurnLimit = 20 private static let projectFaviconRefreshInterval: TimeInterval = 15 * 60 private static let projectFaviconFallbackMarker = "project-favicon-missing" + private static let sourceControlStatusStreamTimeoutSeconds: TimeInterval = 10 private let runtime: EnvironmentRuntime let t3ConnectController: T3ConnectController @@ -1888,15 +1889,41 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, ) async throws -> AsyncThrowingStream { let route = try threadRoute(for: threadID) let context = try workspaceContext(route: route) - let events = await route.client.vcsStatusEvents(cwd: context.cwd) + let client = route.client + let environmentID = route.environmentID + let generation = environmentGeneration + let events = await client.vcsStatusEvents(cwd: context.cwd) + // Each element is a whole status, so only the newest one is ever useful. let (statuses, continuation) = AsyncThrowingStream.makeStream( of: FeatureSourceControlStatus.self, - bufferingPolicy: .bufferingNewest(2) + bufferingPolicy: .bufferingNewest(1) ) - let task = Task { + let task = Task { [weak self] in + // The server publishes `remoteUpdated` only when the remote + // fingerprint changes, and backs off silently when a remote refresh + // fails, so the remote half may never arrive. Bound the wait rather + // than leaving the screen loading forever. + let deadline = Task { + try? await Task.sleep( + for: .seconds(Self.sourceControlStatusStreamTimeoutSeconds) + ) + guard !Task.isCancelled else { return } + continuation.finish() + } + defer { deadline.cancel() } + var accumulator = NativeSourceControlStatusAccumulator() do { for try await event in events { + guard !Task.isCancelled else { break } + guard let self else { break } + guard self.isKnownClient( + client, + environmentID: environmentID, + generation: generation + ) else { + break + } if let status = accumulator.consume(event) { continuation.yield(status) } diff --git a/apps/swift-ios/App/NativeWorkspaceMapper.swift b/apps/swift-ios/App/NativeWorkspaceMapper.swift index 5095e3c510b7..c9c7a303e2db 100644 --- a/apps/swift-ios/App/NativeWorkspaceMapper.swift +++ b/apps/swift-ios/App/NativeWorkspaceMapper.swift @@ -99,6 +99,9 @@ enum NativeWorkspaceMapper { branch: local.refName, aheadCount: remote?.aheadCount ?? 0, behindCount: remote?.behindCount ?? 0, + // A workspace with no repository or no primary remote will never + // receive a remote half, so nothing is pending in those cases. + isRemoteKnown: remote != nil || !local.isRepo || !local.hasPrimaryRemote, files: local.workingTree.files.map { FeatureSourceControlFile( path: $0.path, @@ -370,34 +373,39 @@ enum NativeWorkspaceMapper { } } +/// Folds `vcs.subscribeStatus` events into successive UI statuses, mirroring +/// `applyGitStatusStreamEvent` in packages/shared: the last known remote half is +/// carried across local-only updates instead of being dropped. struct NativeSourceControlStatusAccumulator { private var local: VCSLocalStatus? + private var remote: VCSRemoteStatus? private(set) var isComplete = false mutating func consume(_ event: VCSStatusEvent) -> FeatureSourceControlStatus? { switch event { - case let .snapshot(nextLocal, remote): + case let .snapshot(nextLocal, nextRemote): local = nextLocal - isComplete = remote != nil || !nextLocal.isRepo || !nextLocal.hasPrimaryRemote - return NativeWorkspaceMapper.sourceControl(local: nextLocal, remote: remote) + if let nextRemote { remote = nextRemote } case let .localUpdated(nextLocal): local = nextLocal - isComplete = !nextLocal.isRepo || !nextLocal.hasPrimaryRemote - return NativeWorkspaceMapper.sourceControl(local: nextLocal, remote: nil) - case let .remoteUpdated(remote): - guard let local else { return nil } - isComplete = true - return NativeWorkspaceMapper.sourceControl(local: local, remote: remote) + case let .remoteUpdated(nextRemote): + remote = nextRemote } - } - var prematureEndError: RPCError { - RPCError.protocolViolation( - "The source-control status stream ended before completion." - ) + guard let local else { return nil } + // Latches: a later local-only update must not reopen a stream whose + // remote half already arrived. + if remote != nil || !local.isRepo || !local.hasPrimaryRemote { + isComplete = true + } + return NativeWorkspaceMapper.sourceControl(local: local, remote: remote) } func validateEnd() throws { - guard isComplete else { throw prematureEndError } + guard isComplete else { + throw RPCError.protocolViolation( + "The source-control status stream ended before completion." + ) + } } } diff --git a/apps/swift-ios/Features/Shared/FeatureToolModels.swift b/apps/swift-ios/Features/Shared/FeatureToolModels.swift index fc03f6f00b48..960eb432e902 100644 --- a/apps/swift-ios/Features/Shared/FeatureToolModels.swift +++ b/apps/swift-ios/Features/Shared/FeatureToolModels.swift @@ -842,6 +842,9 @@ public struct FeatureSourceControlStatus: Sendable, Equatable, Codable { public var upstream: String? public var aheadCount: Int public var behindCount: Int + /// `false` while the remote half of a streamed status is still pending, so + /// ahead/behind/pull-request fields are "not yet known" rather than zero. + public var isRemoteKnown: Bool public var files: [FeatureSourceControlFile] public var pullRequest: FeaturePullRequest? public var isBusy: Bool @@ -852,6 +855,7 @@ public struct FeatureSourceControlStatus: Sendable, Equatable, Codable { upstream: String? = nil, aheadCount: Int = 0, behindCount: Int = 0, + isRemoteKnown: Bool = true, files: [FeatureSourceControlFile] = [], pullRequest: FeaturePullRequest? = nil, isBusy: Bool = false @@ -861,6 +865,7 @@ public struct FeatureSourceControlStatus: Sendable, Equatable, Codable { self.upstream = upstream self.aheadCount = aheadCount self.behindCount = behindCount + self.isRemoteKnown = isRemoteKnown self.files = files self.pullRequest = pullRequest self.isBusy = isBusy @@ -872,13 +877,15 @@ public struct FeatureSourceControlStatus: Sendable, Equatable, Codable { if !files.isEmpty { actions.append(.commit) actions.append(.commitAndPush) - if pullRequest == nil { + if isRemoteKnown, pullRequest == nil { actions.append(.commitPushAndCreatePullRequest) } } if aheadCount > 0 { actions.append(.push) } if behindCount > 0 { actions.append(.pull) } - if pullRequest == nil { actions.append(.createPullRequest) } + // Withheld until the remote half lands: offering it against an unknown + // remote can propose a second PR for a branch that already has one. + if isRemoteKnown, pullRequest == nil { actions.append(.createPullRequest) } return actions } } diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index f91b59f00405..142001c6bde9 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -10,6 +10,7 @@ public struct FeatureSourceControlView: View { @State private var errorMessage: String? @State private var commitMessage = "" @State private var pendingCommitAction: FeatureSourceControlAction? + @State private var loadGeneration = 0 public init(client: any FeatureClient, threadID: String) { self.client = client @@ -41,8 +42,8 @@ public struct FeatureSourceControlView: View { .navigationBarTitleDisplayMode(.inline) .toolbar { ToolbarItem(placement: .topBarTrailing) { - Button { Task { await load() } } label: { Image(systemName: "arrow.clockwise") } - .disabled(isLoading || isRunningAction) + Button { Task { await reload() } } label: { Image(systemName: "arrow.clockwise") } + .disabled(isRunningAction) .accessibilityLabel("Reload source control") } } @@ -65,18 +66,34 @@ public struct FeatureSourceControlView: View { private func statusList(_ status: FeatureSourceControlStatus) -> some View { List { + // Once a status is on screen the unavailable-state view is + // unreachable, so a later failure needs its own inline surface. + if let errorMessage { + Section { + Label(errorMessage, systemImage: "exclamationmark.triangle") + .font(T3Typography.supporting) + .foregroundStyle(.orange) + } + } + Section("Repository") { LabeledContent("Branch", value: status.branch ?? "Detached HEAD") if let upstream = status.upstream { LabeledContent("Upstream", value: upstream) } - HStack { - Label("\(status.aheadCount) ahead", systemImage: "arrow.up") - Spacer() - Label("\(status.behindCount) behind", systemImage: "arrow.down") + if status.isRemoteKnown { + HStack { + Label("\(status.aheadCount) ahead", systemImage: "arrow.up") + Spacer() + Label("\(status.behindCount) behind", systemImage: "arrow.down") + } + .font(T3Typography.supporting) + .foregroundStyle(T3Colors.textSecondary) + } else { + Label("Checking remote…", systemImage: "arrow.triangle.2.circlepath") + .font(T3Typography.supporting) + .foregroundStyle(T3Colors.textSecondary) } - .font(T3Typography.supporting) - .foregroundStyle(T3Colors.textSecondary) if let pullRequest = status.pullRequest { if let url = pullRequest.url { Link(destination: url) { @@ -100,7 +117,7 @@ public struct FeatureSourceControlView: View { Label(action.title, systemImage: action.icon) .frame(maxWidth: .infinity, alignment: .leading) } - .disabled(isLoading || isRunningAction) + .disabled(isRunningAction) } } @@ -131,9 +148,7 @@ public struct FeatureSourceControlView: View { } .listStyle(.insetGrouped) .scrollContentBackground(.hidden) - .refreshable { - if !isLoading { await load() } - } + .refreshable { await reload() } .overlay { if isRunningAction { ProgressView() @@ -152,18 +167,41 @@ public struct FeatureSourceControlView: View { } } + /// Streams the cached status first so the screen fills immediately. private func load() async { + await load(force: false) + } + + /// Explicit refresh affordances bypass the server-side status cache: the + /// streaming path is cache-first, so without this a reload would only + /// replay what the server already had. + private func reload() async { + await load(force: true) + } + + private func load(force: Bool) async { + loadGeneration += 1 + let generation = loadGeneration isLoading = true - defer { isLoading = false } + defer { if generation == loadGeneration { isLoading = false } } do { - let statuses = try await client.sourceControlStatuses(threadID: threadID) - for try await nextStatus in statuses { - status = nextStatus + if force { + let refreshed = try await client.sourceControlStatus(threadID: threadID) + guard generation == loadGeneration else { return } + status = refreshed errorMessage = nil + } else { + let statuses = try await client.sourceControlStatuses(threadID: threadID) + for try await nextStatus in statuses { + guard generation == loadGeneration else { return } + status = nextStatus + errorMessage = nil + } } } catch is CancellationError { return } catch { + guard generation == loadGeneration else { return } errorMessage = error.localizedDescription } } diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift index 4653a849a72a..c129c7eca8f4 100644 --- a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift @@ -40,10 +40,7 @@ struct FeatureToolStateTests { ) } - @Test( - "Cached local status is available before remote status", - .bug("https://github.com/saphid/t3code-personal/issues/107") - ) + @Test("Cached local status is available before remote status") func cachedLocalStatusArrivesFirst() throws { var accumulator = NativeSourceControlStatusAccumulator() @@ -60,12 +57,15 @@ struct FeatureToolStateTests { #expect(localOnly.aheadCount == 0) #expect(localOnly.pullRequest == nil) #expect(accumulator.isComplete == false) + // Ahead/behind are unknown rather than zero, so remote-dependent + // actions stay withheld until the remote half lands. + #expect(localOnly.isRemoteKnown == false) + #expect(localOnly.availableActions.contains(.createPullRequest) == false) + #expect(localOnly.availableActions.contains(.commitPushAndCreatePullRequest) == false) + #expect(localOnly.availableActions.contains(.commit)) } - @Test( - "Remote status combines with the latest local status", - .bug("https://github.com/saphid/t3code-personal/issues/107") - ) + @Test("Remote status combines with the latest local status") func remoteStatusUsesLatestLocalState() throws { var accumulator = NativeSourceControlStatusAccumulator() _ = accumulator.consume( @@ -75,9 +75,9 @@ struct FeatureToolStateTests { .localUpdated(Self.vcsLocal(refName: "feature/new", files: ["new.swift"])) ) let pullRequest = VCSChangeRequest( - number: 107, + number: 42, title: "Cached status", - url: "https://github.com/saphid/t3code-personal/pull/107", + url: "https://example.com/pull/42", baseRef: "main", headRef: "feature/new", state: "OPEN" @@ -98,31 +98,56 @@ struct FeatureToolStateTests { #expect(combined.files.map(\.path) == ["new.swift"]) #expect(combined.aheadCount == 2) #expect(combined.behindCount == 1) - #expect(combined.pullRequest?.number == 107) + #expect(combined.pullRequest?.number == 42) + #expect(combined.isRemoteKnown) #expect(accumulator.isComplete) } - @Test( - "Remote-before-local ordering is ignored safely", - .bug("https://github.com/saphid/t3code-personal/issues/107") - ) - func remoteBeforeLocalIsSafe() throws { + @Test("Remote status is retained across a later local-only update") + func localUpdateKeepsKnownRemoteStatus() throws { + var accumulator = NativeSourceControlStatusAccumulator() + _ = accumulator.consume( + .snapshot( + local: Self.vcsLocal(refName: "feature/one"), + remote: Self.vcsRemote(aheadCount: 4, behindCount: 2) + ) + ) + #expect(accumulator.isComplete) + + let consumed = accumulator.consume( + .localUpdated(Self.vcsLocal(refName: "feature/one", files: ["later.swift"])) + ) + let updated = try #require(consumed) + + #expect(updated.files.map(\.path) == ["later.swift"]) + #expect(updated.aheadCount == 4) + #expect(updated.behindCount == 2) + #expect(updated.isRemoteKnown) + // Completion latches: a local-only update must not reopen the stream. + #expect(accumulator.isComplete) + } + + @Test("Remote-before-local ordering retains the remote status") + func remoteBeforeLocalIsRetained() throws { var accumulator = NativeSourceControlStatusAccumulator() let remoteBeforeLocal = accumulator.consume( .remoteUpdated(Self.vcsRemote(aheadCount: 9)) ) #expect(remoteBeforeLocal == nil) + #expect(accumulator.isComplete == false) + let consumedLocal = accumulator.consume( .localUpdated(Self.vcsLocal(refName: "feature/local")) ) - let localOnly = try #require(consumedLocal) + let withRetainedRemote = try #require(consumedLocal) let consumedRemote = accumulator.consume( .remoteUpdated(Self.vcsRemote(aheadCount: 3)) ) let combined = try #require(consumedRemote) - #expect(localOnly.aheadCount == 0) + #expect(withRetainedRemote.aheadCount == 9) + #expect(withRetainedRemote.isRemoteKnown) #expect(combined.branch == "feature/local") #expect(combined.aheadCount == 3) #expect(accumulator.isComplete) @@ -130,7 +155,6 @@ struct FeatureToolStateTests { @Test( "Terminal local states do not wait for remote status", - .bug("https://github.com/saphid/t3code-personal/issues/107"), arguments: [ Self.vcsLocal(isRepo: false, hasPrimaryRemote: false, refName: nil), Self.vcsLocal(hasPrimaryRemote: false, refName: "local-only"), @@ -145,27 +169,33 @@ struct FeatureToolStateTests { #expect(status.isRepository == local.isRepo) #expect(status.branch == local.refName) #expect(status.pullRequest == nil) + // No remote will ever arrive, so nothing is left pending. + #expect(status.isRemoteKnown) #expect(accumulator.isComplete) } - @Test( - "Premature stream end has an explicit protocol error", - .bug("https://github.com/saphid/t3code-personal/issues/107") - ) - func prematureStreamEndIsExplicit() { - let accumulator = NativeSourceControlStatusAccumulator() - - do { - try accumulator.validateEnd() - Issue.record("Expected the incomplete stream to report a protocol error.") - } catch let error as RPCError { - #expect( - error.errorDescription - == "The source-control status stream ended before completion." + @Test("A stream ending after only a cached local status is a protocol error") + func prematureStreamEndIsExplicit() throws { + var accumulator = NativeSourceControlStatusAccumulator() + _ = accumulator.consume( + .snapshot(local: Self.vcsLocal(files: ["pending.swift"]), remote: nil) + ) + #expect(accumulator.isComplete == false) + + #expect(throws: RPCError.self) { try accumulator.validateEnd() } + } + + @Test("A completed stream ends without error") + func completedStreamEndIsAccepted() throws { + var accumulator = NativeSourceControlStatusAccumulator() + _ = accumulator.consume( + .snapshot( + local: Self.vcsLocal(), + remote: Self.vcsRemote(aheadCount: 1) ) - } catch { - Issue.record("Unexpected stream error: \(error)") - } + ) + + try accumulator.validateEnd() } @Test From 389ea1eac03860f8df9c4cd3bcbb077772dc1f18 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Tue, 18 Aug 2026 02:19:46 +1000 Subject: [PATCH 3/9] fix(swift-ios): resolve the remote half of a streamed status explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second follow-up, from an independent review of the previous commit. - A superseded stream is no longer reported as a protocol violation. Breaking out of the event loop on cancellation or an environment switch fell through to the end-of-stream validation, so a normal client replacement surfaced "the stream ended before completion" to the user. Sibling subscriptions in this file all finish plainly in that case. - Track whether the remote half has resolved, separately from its value. `remoteUpdated` carries an optional payload and the server does publish null for a workspace without a repository, which previously read as "still pending": the status stayed remote-unknown on an already-closed stream, and a leading null made the client wait out the whole bound for a half the server had said was absent. - A snapshot now replaces the remote half rather than merging into it, matching `applyGitStatusStreamEvent`. On resubscribe after a reconnect the server prepends a snapshot carrying whatever its cache holds, so merging kept a stale pull request and stale ahead/behind counts and reported them as known. - Say when the bounded wait gave up. Expiry finished the stream silently, leaving "Checking remote…" on screen forever with the pull-request actions withheld and nothing to act on. It now reports that the remote status is unavailable and points at pull-to-refresh. The bound goes to 30s: the first refresh on a cold cache is a fetch plus a pull-request lookup, and 10s expired routinely on a slow network. - Only the first streamed status clears the error message. The screen is interactive for the rest of the stream, so clearing on every element wiped the failure message of an action run meanwhile. - Give the toolbar reload a spinner. It is no longer disabled while loading and the full-screen indicator only covers the empty state, so on a populated screen it looked like nothing happened. - Share the file and pull-request mapping between the two status mappers instead of duplicating them. --- apps/swift-ios/App/NativeFeatureClient.swift | 26 ++++-- .../swift-ios/App/NativeWorkspaceMapper.swift | 87 ++++++++++--------- .../FeatureSourceControlView.swift | 36 ++++++-- .../FeatureTests/FeatureToolStateTests.swift | 66 ++++++++++++++ 4 files changed, 163 insertions(+), 52 deletions(-) diff --git a/apps/swift-ios/App/NativeFeatureClient.swift b/apps/swift-ios/App/NativeFeatureClient.swift index 7207959e83fe..7e46760621a6 100644 --- a/apps/swift-ios/App/NativeFeatureClient.swift +++ b/apps/swift-ios/App/NativeFeatureClient.swift @@ -35,7 +35,7 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, private static let olderThreadPageUserTurnLimit = 20 private static let projectFaviconRefreshInterval: TimeInterval = 15 * 60 private static let projectFaviconFallbackMarker = "project-favicon-missing" - private static let sourceControlStatusStreamTimeoutSeconds: TimeInterval = 10 + private static let sourceControlStatusStreamTimeoutSeconds: TimeInterval = 30 private let runtime: EnvironmentRuntime let t3ConnectController: T3ConnectController @@ -1902,27 +1902,38 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, // The server publishes `remoteUpdated` only when the remote // fingerprint changes, and backs off silently when a remote refresh // fails, so the remote half may never arrive. Bound the wait rather - // than leaving the screen loading forever. + // than leaving the screen loading forever, and say so instead of + // leaving the status quietly half-known. let deadline = Task { try? await Task.sleep( for: .seconds(Self.sourceControlStatusStreamTimeoutSeconds) ) guard !Task.isCancelled else { return } - continuation.finish() + continuation.finish(throwing: NativeFeatureClientError.remoteStatusUnavailable) } defer { deadline.cancel() } var accumulator = NativeSourceControlStatusAccumulator() do { for try await event in events { - guard !Task.isCancelled else { break } - guard let self else { break } + // Superseded by cancellation or an environment switch: the + // stream is over, but nothing about it was malformed, so it + // must not run the end-of-stream validation below. + guard !Task.isCancelled else { + continuation.finish() + return + } + guard let self else { + continuation.finish() + return + } guard self.isKnownClient( client, environmentID: environmentID, generation: generation ) else { - break + continuation.finish() + return } if let status = accumulator.consume(event) { continuation.yield(status) @@ -6010,6 +6021,7 @@ private enum NativeFeatureClientError: LocalizedError { case currentDeviceUnknown case missingScope(String) case tooManyAttachments + case remoteStatusUnavailable var errorDescription: String? { switch self { @@ -6026,6 +6038,8 @@ private enum NativeFeatureClientError: LocalizedError { case .currentDeviceUnknown: "This installation has not registered for device access yet." case .missingScope: "This connection does not have permission to manage devices." case .tooManyAttachments: "You can attach up to 8 images per message." + case .remoteStatusUnavailable: + "Couldn't check the remote status. Pull to refresh." } } } diff --git a/apps/swift-ios/App/NativeWorkspaceMapper.swift b/apps/swift-ios/App/NativeWorkspaceMapper.swift index c9c7a303e2db..452dcc18e540 100644 --- a/apps/swift-ios/App/NativeWorkspaceMapper.swift +++ b/apps/swift-ios/App/NativeWorkspaceMapper.swift @@ -72,54 +72,50 @@ enum NativeWorkspaceMapper { branch: status.refName, aheadCount: status.aheadCount, behindCount: status.behindCount, - files: status.workingTree.files.map { - FeatureSourceControlFile( - path: $0.path, - state: .modified, - isStaged: false - ) - }, - pullRequest: status.pr.map { - FeaturePullRequest( - number: $0.number, - title: $0.title, - state: $0.state, - url: URL(string: $0.url) - ) - } + files: sourceControlFiles(status.workingTree), + pullRequest: pullRequest(status.pr) ) } + /// The streamed counterpart, where the remote half arrives separately and + /// may still be pending. static func sourceControl( local: VCSLocalStatus, - remote: VCSRemoteStatus? + remote: VCSRemoteStatus?, + isRemoteKnown: Bool ) -> FeatureSourceControlStatus { FeatureSourceControlStatus( isRepository: local.isRepo, branch: local.refName, aheadCount: remote?.aheadCount ?? 0, behindCount: remote?.behindCount ?? 0, - // A workspace with no repository or no primary remote will never - // receive a remote half, so nothing is pending in those cases. - isRemoteKnown: remote != nil || !local.isRepo || !local.hasPrimaryRemote, - files: local.workingTree.files.map { - FeatureSourceControlFile( - path: $0.path, - state: .modified, - isStaged: false - ) - }, - pullRequest: remote?.pr.map { - FeaturePullRequest( - number: $0.number, - title: $0.title, - state: $0.state, - url: URL(string: $0.url) - ) - } + isRemoteKnown: isRemoteKnown, + files: sourceControlFiles(local.workingTree), + pullRequest: pullRequest(remote?.pr) ) } + private static func sourceControlFiles( + _ workingTree: VCSWorkingTree + ) -> [FeatureSourceControlFile] { + // The RPC carries only path/insertions/deletions, so there is no staged + // or added/deleted information to preserve here. + workingTree.files.map { + FeatureSourceControlFile(path: $0.path, state: .modified, isStaged: false) + } + } + + private static func pullRequest(_ request: VCSChangeRequest?) -> FeaturePullRequest? { + request.map { + FeaturePullRequest( + number: $0.number, + title: $0.title, + state: $0.state, + url: URL(string: $0.url) + ) + } + } + static func gitAction(_ action: FeatureSourceControlAction) -> GitStackedAction { switch action { case .commit: .commit @@ -374,31 +370,44 @@ enum NativeWorkspaceMapper { } /// Folds `vcs.subscribeStatus` events into successive UI statuses, mirroring -/// `applyGitStatusStreamEvent` in packages/shared: the last known remote half is -/// carried across local-only updates instead of being dropped. +/// `applyGitStatusStreamEvent` in packages/shared: a snapshot replaces both +/// halves, while the last known remote half is carried across local-only +/// updates instead of being dropped. struct NativeSourceControlStatusAccumulator { private var local: VCSLocalStatus? private var remote: VCSRemoteStatus? + /// Tracked separately from `remote` because the remote half can legitimately + /// resolve to nil — "known to be absent" is not the same as "still pending". + private var isRemoteResolved = false private(set) var isComplete = false mutating func consume(_ event: VCSStatusEvent) -> FeatureSourceControlStatus? { switch event { case let .snapshot(nextLocal, nextRemote): local = nextLocal - if let nextRemote { remote = nextRemote } + remote = nextRemote + isRemoteResolved = nextRemote != nil case let .localUpdated(nextLocal): local = nextLocal case let .remoteUpdated(nextRemote): remote = nextRemote + isRemoteResolved = true } guard let local else { return nil } + // A workspace with no repository or no primary remote never receives a + // remote half, so nothing is pending in those cases. + let isRemoteKnown = isRemoteResolved || !local.isRepo || !local.hasPrimaryRemote // Latches: a later local-only update must not reopen a stream whose // remote half already arrived. - if remote != nil || !local.isRepo || !local.hasPrimaryRemote { + if isRemoteKnown { isComplete = true } - return NativeWorkspaceMapper.sourceControl(local: local, remote: remote) + return NativeWorkspaceMapper.sourceControl( + local: local, + remote: remote, + isRemoteKnown: isRemoteKnown + ) } func validateEnd() throws { diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index 142001c6bde9..d258a03a146c 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -42,9 +42,17 @@ public struct FeatureSourceControlView: View { .navigationBarTitleDisplayMode(.inline) .toolbar { ToolbarItem(placement: .topBarTrailing) { - Button { Task { await reload() } } label: { Image(systemName: "arrow.clockwise") } - .disabled(isRunningAction) - .accessibilityLabel("Reload source control") + Button { Task { await reload() } } label: { + // Never disabled on a populated screen, so the spinner is + // the only signal that a reload is under way. + if isLoading { + ProgressView() + } else { + Image(systemName: "arrow.clockwise") + } + } + .disabled(isRunningAction) + .accessibilityLabel("Reload source control") } } .alert("Commit changes", isPresented: Binding( @@ -90,9 +98,15 @@ public struct FeatureSourceControlView: View { .font(T3Typography.supporting) .foregroundStyle(T3Colors.textSecondary) } else { - Label("Checking remote…", systemImage: "arrow.triangle.2.circlepath") - .font(T3Typography.supporting) - .foregroundStyle(T3Colors.textSecondary) + // Only claim to be checking while something actually is. + Label( + isLoading ? "Checking remote…" : "Remote status unavailable", + systemImage: isLoading + ? "arrow.triangle.2.circlepath" + : "exclamationmark.triangle" + ) + .font(T3Typography.supporting) + .foregroundStyle(T3Colors.textSecondary) } if let pullRequest = status.pullRequest { if let url = pullRequest.url { @@ -192,10 +206,18 @@ public struct FeatureSourceControlView: View { errorMessage = nil } else { let statuses = try await client.sourceControlStatuses(threadID: threadID) + var didClearError = false for try await nextStatus in statuses { guard generation == loadGeneration else { return } status = nextStatus - errorMessage = nil + // Only the first status clears the error. The screen stays + // interactive for the rest of the stream, and clearing on + // every element would wipe a failure message raised by an + // action the user ran meanwhile. + if !didClearError { + errorMessage = nil + didClearError = true + } } } } catch is CancellationError { diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift index c129c7eca8f4..9745bb3a9d66 100644 --- a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift @@ -185,6 +185,72 @@ struct FeatureToolStateTests { #expect(throws: RPCError.self) { try accumulator.validateEnd() } } + @Test("An absent remote half resolves the status instead of leaving it pending") + func nilRemotePayloadResolvesTheRemoteHalf() throws { + var accumulator = NativeSourceControlStatusAccumulator() + _ = accumulator.consume(.snapshot(local: Self.vcsLocal(), remote: nil)) + #expect(accumulator.isComplete == false) + + let consumed = accumulator.consume(.remoteUpdated(nil)) + let resolved = try #require(consumed) + + // "Known to be absent" is not "still pending": the stream is finished + // and the screen must stop claiming it is checking. + #expect(resolved.isRemoteKnown) + #expect(resolved.aheadCount == 0) + #expect(resolved.behindCount == 0) + #expect(resolved.pullRequest == nil) + #expect(accumulator.isComplete) + } + + @Test("A later snapshot replaces the remote half rather than merging into it") + func snapshotReplacesKnownRemoteStatus() throws { + var accumulator = NativeSourceControlStatusAccumulator() + _ = accumulator.consume( + .snapshot( + local: Self.vcsLocal(refName: "feature/one"), + remote: Self.vcsRemote(aheadCount: 7) + ) + ) + + // Resubscribe after a reconnect: the server prepends a fresh snapshot + // carrying whatever its cache holds, so a stale ahead count must not + // survive and must not be reported as known. + let consumed = accumulator.consume( + .snapshot(local: Self.vcsLocal(refName: "feature/one"), remote: nil) + ) + let replaced = try #require(consumed) + + #expect(replaced.aheadCount == 0) + #expect(replaced.isRemoteKnown == false) + } + + @Test("Completion never regresses across a long mixed sequence") + func completionLatchesAcrossMixedSequence() { + var accumulator = NativeSourceControlStatusAccumulator() + let events: [VCSStatusEvent] = [ + .snapshot(local: Self.vcsLocal(refName: "feature/seq"), remote: nil), + .localUpdated(Self.vcsLocal(refName: "feature/seq", files: ["a.swift"])), + .remoteUpdated(Self.vcsRemote(aheadCount: 1)), + .localUpdated(Self.vcsLocal(refName: "feature/seq", files: ["a.swift", "b.swift"])), + .remoteUpdated(nil), + .localUpdated(Self.vcsLocal(refName: "feature/seq")), + ] + + var completedAt: Int? + for (index, event) in events.enumerated() { + _ = accumulator.consume(event) + if accumulator.isComplete, completedAt == nil { + completedAt = index + } + if completedAt != nil { + #expect(accumulator.isComplete) + } + } + + #expect(completedAt == 2) + } + @Test("A completed stream ends without error") func completedStreamEndIsAccepted() throws { var accumulator = NativeSourceControlStatusAccumulator() From bdf796486f2cdd8102734546bbbb8d9a817755b3 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Tue, 18 Aug 2026 02:31:58 +1000 Subject: [PATCH 4/9] test(swift-ios): cover the source control completion latch The mixed-sequence and snapshot-replacement tests asserted the latch without ever driving the one input that can break it, so both still passed with the latch removed. Drive a cache-empty snapshot after the remote half has resolved, which is the resubscribe-after-reconnect case, and assert completion holds across it. Also reword the expiry message, which recommended pull-to-refresh in a state that has no pull-to-refresh, and soften the accumulator's doc comment: it is modelled on `applyGitStatusStreamEvent` rather than mirroring it, since the pending state has no counterpart there. --- apps/swift-ios/App/NativeFeatureClient.swift | 2 +- apps/swift-ios/App/NativeWorkspaceMapper.swift | 7 ++++--- .../Tests/FeatureTests/FeatureToolStateTests.swift | 6 ++++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/swift-ios/App/NativeFeatureClient.swift b/apps/swift-ios/App/NativeFeatureClient.swift index 7e46760621a6..ac283fdb1fd3 100644 --- a/apps/swift-ios/App/NativeFeatureClient.swift +++ b/apps/swift-ios/App/NativeFeatureClient.swift @@ -6039,7 +6039,7 @@ private enum NativeFeatureClientError: LocalizedError { case .missingScope: "This connection does not have permission to manage devices." case .tooManyAttachments: "You can attach up to 8 images per message." case .remoteStatusUnavailable: - "Couldn't check the remote status. Pull to refresh." + "Couldn't check the remote status. Try reloading." } } } diff --git a/apps/swift-ios/App/NativeWorkspaceMapper.swift b/apps/swift-ios/App/NativeWorkspaceMapper.swift index 452dcc18e540..69a0b16e1183 100644 --- a/apps/swift-ios/App/NativeWorkspaceMapper.swift +++ b/apps/swift-ios/App/NativeWorkspaceMapper.swift @@ -369,10 +369,11 @@ enum NativeWorkspaceMapper { } } -/// Folds `vcs.subscribeStatus` events into successive UI statuses, mirroring -/// `applyGitStatusStreamEvent` in packages/shared: a snapshot replaces both +/// Folds `vcs.subscribeStatus` events into successive UI statuses. Modelled on +/// `applyGitStatusStreamEvent` in packages/shared — a snapshot replaces both /// halves, while the last known remote half is carried across local-only -/// updates instead of being dropped. +/// updates instead of being dropped — but with an explicit pending state, which +/// the reference has no need for because it never reports a partial status. struct NativeSourceControlStatusAccumulator { private var local: VCSLocalStatus? private var remote: VCSRemoteStatus? diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift index 9745bb3a9d66..f418070effbc 100644 --- a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift @@ -223,6 +223,9 @@ struct FeatureToolStateTests { #expect(replaced.aheadCount == 0) #expect(replaced.isRemoteKnown == false) + // The latch holds even though this event alone would not set it: + // without it the stream would report a spurious protocol violation. + #expect(accumulator.isComplete) } @Test("Completion never regresses across a long mixed sequence") @@ -234,6 +237,9 @@ struct FeatureToolStateTests { .remoteUpdated(Self.vcsRemote(aheadCount: 1)), .localUpdated(Self.vcsLocal(refName: "feature/seq", files: ["a.swift", "b.swift"])), .remoteUpdated(nil), + // Drives the latch: on its own this snapshot leaves the remote half + // unresolved again, so completion would regress without it. + .snapshot(local: Self.vcsLocal(refName: "feature/seq"), remote: nil), .localUpdated(Self.vcsLocal(refName: "feature/seq")), ] From 9c5fb6d801b33f2cae1eba587146992711a4b612 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Tue, 18 Aug 2026 02:50:25 +1000 Subject: [PATCH 5/9] fix(swift-ios): stop an open status stream from reverting an action An action runs while a cached-status stream may still be open: the screen stays interactive throughout, and the stream's accumulator still holds the local half from before the action. A late event folds that stale half into a status that overwrites the action's result, so a successful commit could show the old dirty working tree until a manual reload; a late stream error, including the bounded-wait expiry, could likewise mask the action's own failure message. Split the two roles the load token was serving. `loadGeneration` still owns the loading indicator and is bumped only by a load, so the indicator is always cleared by the load that set it. `statusGeneration` invalidates status and error writes and is bumped by an action as well, so an action supersedes an open stream without stranding the indicator. --- .../FeatureSourceControlView.swift | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index d258a03a146c..6d6538637f58 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -10,7 +10,10 @@ public struct FeatureSourceControlView: View { @State private var errorMessage: String? @State private var commitMessage = "" @State private var pendingCommitAction: FeatureSourceControlAction? + /// Owns the loading indicator; only a newer load supersedes it. @State private var loadGeneration = 0 + /// Invalidates status writes; a running action supersedes them too. + @State private var statusGeneration = 0 public init(client: any FeatureClient, threadID: String) { self.client = client @@ -195,25 +198,28 @@ public struct FeatureSourceControlView: View { private func load(force: Bool) async { loadGeneration += 1 - let generation = loadGeneration + statusGeneration += 1 + let loadID = loadGeneration + let statusID = statusGeneration isLoading = true - defer { if generation == loadGeneration { isLoading = false } } + // Only a newer *load* takes over the indicator. An action supersedes + // this load's writes without taking ownership of the indicator, so + // this must still be the one to clear it. + defer { if loadID == loadGeneration { isLoading = false } } do { if force { let refreshed = try await client.sourceControlStatus(threadID: threadID) - guard generation == loadGeneration else { return } + guard statusID == statusGeneration else { return } status = refreshed errorMessage = nil } else { let statuses = try await client.sourceControlStatuses(threadID: threadID) var didClearError = false for try await nextStatus in statuses { - guard generation == loadGeneration else { return } + guard statusID == statusGeneration else { return } status = nextStatus - // Only the first status clears the error. The screen stays - // interactive for the rest of the stream, and clearing on - // every element would wipe a failure message raised by an - // action the user ran meanwhile. + // Only the first status clears the error: the screen stays + // interactive for the rest of the stream. if !didClearError { errorMessage = nil didClearError = true @@ -223,12 +229,17 @@ public struct FeatureSourceControlView: View { } catch is CancellationError { return } catch { - guard generation == loadGeneration else { return } + guard statusID == statusGeneration else { return } errorMessage = error.localizedDescription } } private func perform(_ action: FeatureSourceControlAction, message: String?) async { + // Supersede any open stream. Its accumulator still holds the local half + // from before this action, so a late event would fold that stale half + // into a status that overwrites this action's result — and a late + // stream error would mask this action's failure message. + statusGeneration += 1 isRunningAction = true defer { isRunningAction = false } do { From dd917ed0dcfffbf01c2a1301fe256b8563c97782 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Tue, 18 Aug 2026 02:59:52 +1000 Subject: [PATCH 6/9] fix(swift-ios): make the status supersede protocol symmetric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull-to-refresh is not gated on a running action, so a refresh started after an action could still land last and wipe that action's failure message — the mirror image of the race the previous commit closed. Guard the action's own writes with the same token so whichever side started later wins, rather than whichever happens to finish later. Drop the first-status-only error latch from the streamed load: any writer that could have set an error since the stream started has already bumped the token and invalidated it, so the latch defends against nothing now. --- .../FeatureSourceControlView.swift | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index 6d6538637f58..2babd07a7db2 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -204,7 +204,8 @@ public struct FeatureSourceControlView: View { isLoading = true // Only a newer *load* takes over the indicator. An action supersedes // this load's writes without taking ownership of the indicator, so - // this must still be the one to clear it. + // this must still be the one to clear it — at the cost of a superseded + // stream holding the toolbar spinner until its bound expires. defer { if loadID == loadGeneration { isLoading = false } } do { if force { @@ -214,16 +215,10 @@ public struct FeatureSourceControlView: View { errorMessage = nil } else { let statuses = try await client.sourceControlStatuses(threadID: threadID) - var didClearError = false for try await nextStatus in statuses { guard statusID == statusGeneration else { return } status = nextStatus - // Only the first status clears the error: the screen stays - // interactive for the rest of the stream. - if !didClearError { - errorMessage = nil - didClearError = true - } + errorMessage = nil } } } catch is CancellationError { @@ -240,16 +235,22 @@ public struct FeatureSourceControlView: View { // into a status that overwrites this action's result — and a late // stream error would mask this action's failure message. statusGeneration += 1 + let statusID = statusGeneration isRunningAction = true defer { isRunningAction = false } do { - status = try await client.performSourceControlAction( + let result = try await client.performSourceControlAction( threadID: threadID, action: action, message: message?.trimmingCharacters(in: .whitespacesAndNewlines) ) + // Guarded like a load's: pull-to-refresh is not gated on a running + // action, so a refresh started after this one must win. + guard statusID == statusGeneration else { return } + status = result errorMessage = nil } catch { + guard statusID == statusGeneration else { return } errorMessage = error.localizedDescription } } From 2bb15dbfd6d6a88197f0bfa2ff5e162b5a5d0187 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Tue, 18 Aug 2026 03:18:07 +1000 Subject: [PATCH 7/9] fix(swift-ios): recover the remote half after a failed action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An action supersedes any stream that is still open, which is what keeps a stale local half from reverting its result. But when the action itself fails it writes no status, so on a cold-cache entry the screen was left holding the pending status the stream had produced so far — reporting the remote as unavailable and withholding the pull-request actions until the user reloaded by hand. Re-run the cached-status stream after a failed action, without clearing the message that says why it failed. --- .../SourceControl/FeatureSourceControlView.swift | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index 2babd07a7db2..96a4cb774600 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -196,7 +196,9 @@ public struct FeatureSourceControlView: View { await load(force: true) } - private func load(force: Bool) async { + /// `clearsError` is false when recovering after a failed action, so the + /// reason that action failed survives the refresh it triggers. + private func load(force: Bool, clearsError: Bool = true) async { loadGeneration += 1 statusGeneration += 1 let loadID = loadGeneration @@ -212,19 +214,19 @@ public struct FeatureSourceControlView: View { let refreshed = try await client.sourceControlStatus(threadID: threadID) guard statusID == statusGeneration else { return } status = refreshed - errorMessage = nil + if clearsError { errorMessage = nil } } else { let statuses = try await client.sourceControlStatuses(threadID: threadID) for try await nextStatus in statuses { guard statusID == statusGeneration else { return } status = nextStatus - errorMessage = nil + if clearsError { errorMessage = nil } } } } catch is CancellationError { return } catch { - guard statusID == statusGeneration else { return } + guard statusID == statusGeneration, clearsError else { return } errorMessage = error.localizedDescription } } @@ -252,6 +254,11 @@ public struct FeatureSourceControlView: View { } catch { guard statusID == statusGeneration else { return } errorMessage = error.localizedDescription + // This action superseded an in-flight stream, so the remote half it + // was still waiting on would otherwise never arrive: the screen + // would keep withholding the pull-request actions until the user + // reloaded by hand. Recover, without losing the reason above. + await load(force: false, clearsError: false) } } } From 83ed7fef309a324f8c6d8895836c9bc6870aa40b Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Fri, 21 Aug 2026 11:42:45 +1000 Subject: [PATCH 8/9] fix(swift-ios): stop action progress before recovery --- .../FeatureSourceControlView.swift | 29 +++++++++++++++---- .../FeatureTests/FeatureToolStateTests.swift | 18 ++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index 96a4cb774600..39784acda389 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -1,5 +1,20 @@ import SwiftUI +@MainActor +func runFeatureSourceControlAction( + setRunning: (Bool) -> Void, + operation: () async throws -> Value +) async -> Result { + setRunning(true) + defer { setRunning(false) } + + do { + return .success(try await operation()) + } catch { + return .failure(error) + } +} + public struct FeatureSourceControlView: View { let client: any FeatureClient let threadID: String @@ -238,20 +253,24 @@ public struct FeatureSourceControlView: View { // stream error would mask this action's failure message. statusGeneration += 1 let statusID = statusGeneration - isRunningAction = true - defer { isRunningAction = false } - do { - let result = try await client.performSourceControlAction( + let actionResult = await runFeatureSourceControlAction( + setRunning: { isRunningAction = $0 } + ) { + try await client.performSourceControlAction( threadID: threadID, action: action, message: message?.trimmingCharacters(in: .whitespacesAndNewlines) ) + } + + switch actionResult { + case let .success(result): // Guarded like a load's: pull-to-refresh is not gated on a running // action, so a refresh started after this one must win. guard statusID == statusGeneration else { return } status = result errorMessage = nil - } catch { + case let .failure(error): guard statusID == statusGeneration else { return } errorMessage = error.localizedDescription // This action superseded an in-flight stream, so the remote half it diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift index f418070effbc..4734d448d224 100644 --- a/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/FeatureToolStateTests.swift @@ -3,6 +3,24 @@ import Testing @Suite("Thread tool state") struct FeatureToolStateTests { + @MainActor + @Test("A failed source-control action stops progress before recovery") + func failedSourceControlActionStopsProgressBeforeRecovery() async { + var phases: [String] = [] + + let result: Result = await runFeatureSourceControlAction( + setRunning: { phases.append($0 ? "running" : "stopped") } + ) { + throw FeatureCapabilityUnavailable("Source control") + } + + if case .failure = result { + phases.append("recovery") + } + + #expect(phases == ["running", "stopped", "recovery"]) + } + private static func vcsLocal( isRepo: Bool = true, hasPrimaryRemote: Bool = true, From ede5862ee5c811ebc802338c2d82d0c7fa63aa3d Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Sat, 29 Aug 2026 23:15:59 +1000 Subject: [PATCH 9/9] fix(swift-ios): clear stale source control errors --- .../Features/SourceControl/FeatureSourceControlView.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift index 28802ffa3346..205dfd77843a 100644 --- a/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift +++ b/apps/swift-ios/Features/SourceControl/FeatureSourceControlView.swift @@ -221,8 +221,8 @@ public struct FeatureSourceControlView: View { await load(force: true) } - /// `clearsError` is false when recovering after a failed action, so the - /// reason that action failed survives the refresh it triggers. + /// Action failures use `actionErrorMessage`; a successful recovery should + /// clear any stale load error from the inline status banner. private func load(force: Bool, clearsError: Bool = true) async { loadGeneration += 1 statusGeneration += 1 @@ -305,7 +305,7 @@ public struct FeatureSourceControlView: View { // was still waiting on would otherwise never arrive: the screen // would keep withholding the pull-request actions until the user // reloaded by hand. Recover, without losing the reason above. - await load(force: false, clearsError: false) + await load(force: false) } } }