diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 36e4a33d..a5705bbb 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -743,10 +743,14 @@ extension RepositoriesFeature { state.prRefreshResultsByRepositoryID.removeValue( forKey: repositoryID ) ?? [:] - let confirmedNoPrBranches = + let hadFailedBatch = state.prRefreshFailedBatchRepositoryIDs.remove(repositoryID) != nil + let accumulatedConfirmedNoPrBranches = state.prRefreshNoPrBranchesByID.removeValue( forKey: repositoryID ) ?? [] + // A failed host batch means branch status on that host is unknown, even when + // it arrived before this final refreshed outcome — suppress confirmed clears. + let confirmedNoPrBranches = hadFailedBatch ? [] : accumulatedConfirmedNoPrBranches state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) let prsByWorktreeID = pullRequestsByWorktreeID( repository: repository, @@ -766,6 +770,7 @@ extension RepositoriesFeature { .send(.githubIntegration(.repositoryPullRequestRefreshCompleted(repositoryID))) ) case .failed(let repositoryID, let worktreeIDs, _): + state.prRefreshFailedBatchRepositoryIDs.insert(repositoryID) guard consumePullRequestRefreshBatch(repositoryID: repositoryID, state: &state) else { return .none } @@ -773,6 +778,7 @@ extension RepositoriesFeature { state.prRefreshResultsByRepositoryID.removeValue( forKey: repositoryID ) ?? [:] + state.prRefreshFailedBatchRepositoryIDs.remove(repositoryID) _ = state.prRefreshNoPrBranchesByID.removeValue(forKey: repositoryID) state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) guard !mergedPRsByBranch.isEmpty, @@ -847,6 +853,7 @@ extension RepositoriesFeature { state.prRefreshBatchCountsByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshResultsByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshNoPrBranchesByID.removeValue(forKey: repositoryID) + state.prRefreshFailedBatchRepositoryIDs.remove(repositoryID) state.prRefreshRemotePrioritiesByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) } @@ -855,6 +862,7 @@ extension RepositoriesFeature { state.prRefreshBatchCountsByRepositoryID.removeAll() state.prRefreshResultsByRepositoryID.removeAll() state.prRefreshNoPrBranchesByID.removeAll() + state.prRefreshFailedBatchRepositoryIDs.removeAll() state.prRefreshRemotePrioritiesByRepositoryID.removeAll() state.prRefreshResultPrioritiesByRepositoryID.removeAll() } @@ -884,8 +892,10 @@ extension RepositoriesFeature { if let pullRequest = prsByBranch[worktree.name] { prsByWorktreeID[worktreeID] = pullRequest } else if confirmedNoPrBranches.contains(worktree.name) { - // All repos confirmed no PR for this branch — explicitly clear. - prsByWorktreeID[worktreeID] = nil + // All repos confirmed no PR for this branch — explicitly clear. A nil + // literal through the subscript would remove the key instead of storing + // an explicit nil, so downstream would never see the clear. + prsByWorktreeID.updateValue(nil, forKey: worktreeID) } // Otherwise: unknown status (partial failure) — omit to preserve existing. } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index 8680e915..b913f591 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -340,6 +340,10 @@ struct RepositoriesFeature { /// Branches confirmed as having no PR (all repos succeeded, none returned a PR). /// Used to clear stale PR state without flashing when only some repos succeed. var prRefreshNoPrBranchesByID: [Repository.ID: Set] = [:] + /// Repositories with at least one failed host batch in the current refresh cycle. + /// A failure means branch status on that host is unknown, so confirmed-no-PR + /// clears from the healthy hosts must be suppressed regardless of arrival order. + var prRefreshFailedBatchRepositoryIDs: Set = [] /// Cross-host PR refresh batches complete independently; keep the intended remote /// order so same-branch collisions are resolved by priority, not arrival time. var prRefreshRemotePrioritiesByRepositoryID: [Repository.ID: [String: Int]] = [:] diff --git a/supacodeTests/BatchedPullRequestRefreshReducerTests.swift b/supacodeTests/BatchedPullRequestRefreshReducerTests.swift index 8e52d73e..c957fc82 100644 --- a/supacodeTests/BatchedPullRequestRefreshReducerTests.swift +++ b/supacodeTests/BatchedPullRequestRefreshReducerTests.swift @@ -387,6 +387,242 @@ struct BatchedPullRequestRefreshReducerTests { await store.finish() } + @Test func coordinatorOutcomeConfirmedNoPrClearsStalePullRequest() async { + let context = makeContext() + let stalePullRequest = makePullRequestFixture(url: "https://github.com/khoi/alpha/pull/7") + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + var staleEntry = WorktreeInfoEntry() + staleEntry.pullRequest = stalePullRequest + initialState.worktreeInfoByID[context.featureWorktree.id] = staleEntry + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.pullRequestRefreshCoordinator = .unimplemented + } + + let outcome = PullRequestRefreshCoordinator.Outcome.refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: [:], + confirmedNoPrBranches: ["feature"] + ) + + await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) + await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { + $0.worktreeInfoByID.removeValue(forKey: context.featureWorktree.id) + } + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) { + $0.inFlightPullRequestRefreshRepositoryIDs = [] + } + await store.finish() + } + + @Test func coordinatorOutcomeUnknownBranchStatusPreservesPullRequest() async { + let context = makeContext() + let stalePullRequest = makePullRequestFixture(url: "https://github.com/khoi/alpha/pull/7") + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + var staleEntry = WorktreeInfoEntry() + staleEntry.pullRequest = stalePullRequest + initialState.worktreeInfoByID[context.featureWorktree.id] = staleEntry + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.pullRequestRefreshCoordinator = .unimplemented + } + + let outcome = PullRequestRefreshCoordinator.Outcome.refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: [:], + confirmedNoPrBranches: [] + ) + + await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) + await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) { + $0.inFlightPullRequestRefreshRepositoryIDs = [] + } + await store.finish() + + #expect(store.state.worktreeInfoByID[context.featureWorktree.id]?.pullRequest == stalePullRequest) + } + + @Test func confirmedNoPrClearIsSuppressedWhenAnotherHostBatchFailedFirst() async { + let context = makeContext() + let stalePullRequest = makePullRequestFixture(url: "https://ghe.example/khoi/alpha/pull/9") + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + initialState.prRefreshBatchCountsByRepositoryID[context.repository.id] = 2 + var staleEntry = WorktreeInfoEntry() + staleEntry.pullRequest = stalePullRequest + initialState.worktreeInfoByID[context.featureWorktree.id] = staleEntry + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.pullRequestRefreshCoordinator = .unimplemented + } + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .failed( + repositoryID: context.repository.id, + worktreeIDs: context.worktreeIDs, + message: "enterprise host down" + ) + )) + ) { + $0.prRefreshFailedBatchRepositoryIDs = [context.repository.id] + $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 + } + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: [:], + confirmedNoPrBranches: ["feature"] + ) + )) + ) { + $0.prRefreshBatchCountsByRepositoryID = [:] + $0.prRefreshFailedBatchRepositoryIDs = [] + } + await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) { + $0.inFlightPullRequestRefreshRepositoryIDs = [] + } + await store.finish() + + #expect(store.state.worktreeInfoByID[context.featureWorktree.id]?.pullRequest == stalePullRequest) + } + + @Test func confirmedNoPrClearIsDiscardedWhenFinalHostBatchFails() async { + let context = makeContext() + let stalePullRequest = makePullRequestFixture(url: "https://ghe.example/khoi/alpha/pull/9") + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + initialState.prRefreshBatchCountsByRepositoryID[context.repository.id] = 2 + var staleEntry = WorktreeInfoEntry() + staleEntry.pullRequest = stalePullRequest + initialState.worktreeInfoByID[context.featureWorktree.id] = staleEntry + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.pullRequestRefreshCoordinator = .unimplemented + } + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: [:], + confirmedNoPrBranches: ["feature"] + ) + )) + ) { + $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 + $0.prRefreshResultsByRepositoryID[context.repository.id] = [:] + $0.prRefreshNoPrBranchesByID[context.repository.id] = ["feature"] + $0.prRefreshResultPrioritiesByRepositoryID[context.repository.id] = [:] + } + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .failed( + repositoryID: context.repository.id, + worktreeIDs: context.worktreeIDs, + message: "enterprise host down" + ) + )) + ) { + $0.prRefreshBatchCountsByRepositoryID = [:] + $0.prRefreshResultsByRepositoryID = [:] + $0.prRefreshNoPrBranchesByID = [:] + $0.prRefreshResultPrioritiesByRepositoryID = [:] + } + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) { + $0.inFlightPullRequestRefreshRepositoryIDs = [] + } + await store.finish() + + #expect(store.state.worktreeInfoByID[context.featureWorktree.id]?.pullRequest == stalePullRequest) + } + + @Test func pullRequestFromLaterHostBatchOverridesEarlierConfirmedNoPr() async { + let context = makeContext() + let pullRequest = makePullRequestFixture() + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + initialState.prRefreshBatchCountsByRepositoryID[context.repository.id] = 2 + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.pullRequestRefreshCoordinator = .unimplemented + } + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: [:], + confirmedNoPrBranches: ["feature"] + ) + )) + ) { + $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 + $0.prRefreshResultsByRepositoryID[context.repository.id] = [:] + $0.prRefreshNoPrBranchesByID[context.repository.id] = ["feature"] + $0.prRefreshResultPrioritiesByRepositoryID[context.repository.id] = [:] + } + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: ["feature": pullRequest], + confirmedNoPrBranches: [] + ) + )) + ) { + $0.prRefreshBatchCountsByRepositoryID = [:] + $0.prRefreshResultsByRepositoryID = [:] + $0.prRefreshNoPrBranchesByID = [:] + $0.prRefreshResultPrioritiesByRepositoryID = [:] + } + await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { + var entry = WorktreeInfoEntry() + entry.pullRequest = pullRequest + $0.worktreeInfoByID[context.featureWorktree.id] = entry + } + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) { + $0.inFlightPullRequestRefreshRepositoryIDs = [] + } + await store.finish() + } + @Test(.dependencies) func refreshSkippedWhenPullRequestStateFetchDisabled() async { let context = makeContext() let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) diff --git a/supacodeTests/PullRequestRefreshCoordinatorTests.swift b/supacodeTests/PullRequestRefreshCoordinatorTests.swift index ef13cd6b..3a786186 100644 --- a/supacodeTests/PullRequestRefreshCoordinatorTests.swift +++ b/supacodeTests/PullRequestRefreshCoordinatorTests.swift @@ -345,6 +345,90 @@ struct PullRequestRefreshCoordinatorTests { #expect(refreshed.first?["feat-1"]?.title == "PR-upstream") } + @Test func allCandidateReposSucceedingConfirmsBranchesWithoutPullRequests() async throws { + let clock = TestClock() + let probe = CoordinatorProbe() + let outcomes = OutcomeCollector() + let coordinator = makeCoordinator( + probe: probe, + clock: clock, + outcomes: outcomes, + batched: { _, requests in + var dict: [RepoKey: [String: GithubPullRequest]] = [:] + for request in requests { + if request.repo == "upstream" { + dict[request.key] = ["feat-1": makeFixturePullRequest(repo: "upstream")] + } else { + dict[request.key] = [:] + } + } + return CrossRepoPullRequestResult(successByRepo: dict) + } + ) + + coordinator.enqueue(request(repo: "fork", repositoryID: "local", branches: ["feat-1", "feat-2"])) + coordinator.enqueue(request(repo: "upstream", repositoryID: "local", branches: ["feat-1", "feat-2"])) + await advanceCoordinatorClock(clock, by: .milliseconds(250)) + await Task.yield() + await Task.yield() + + let refreshed = await outcomes.snapshot().compactMap { + outcome -> ([String: GithubPullRequest], Set)? in + if case .refreshed("local", _, _, let prsByBranch, let confirmedNoPrBranches) = outcome { + return (prsByBranch, confirmedNoPrBranches) + } + return nil + } + let result = try #require(refreshed.first) + #expect(refreshed.count == 1) + #expect(result.0["feat-1"]?.title == "PR-upstream") + #expect(result.1 == ["feat-2"]) + } + + @Test func partialCandidateRepoFailureLeavesBranchesUnconfirmed() async throws { + let clock = TestClock() + let probe = CoordinatorProbe() + let outcomes = OutcomeCollector() + let coordinator = makeCoordinator( + probe: probe, + clock: clock, + outcomes: outcomes, + batched: { _, requests in + var success: [RepoKey: [String: GithubPullRequest]] = [:] + var failed: [RepoKey: GithubCLIError] = [:] + for request in requests { + if request.repo == "upstream" { + failed[request.key] = .commandFailed("boom") + } else { + success[request.key] = [:] + } + } + return CrossRepoPullRequestResult(successByRepo: success, failedRepos: failed) + }, + legacy: { _, _, _, _ in + throw GithubCLIError.commandFailed("fallback down too") + } + ) + + coordinator.enqueue(request(repo: "fork", repositoryID: "local", branches: ["feat-1", "feat-2"])) + coordinator.enqueue(request(repo: "upstream", repositoryID: "local", branches: ["feat-1", "feat-2"])) + await advanceCoordinatorClock(clock, by: .milliseconds(250)) + await Task.yield() + await Task.yield() + + let refreshed = await outcomes.snapshot().compactMap { + outcome -> ([String: GithubPullRequest], Set)? in + if case .refreshed("local", _, _, let prsByBranch, let confirmedNoPrBranches) = outcome { + return (prsByBranch, confirmedNoPrBranches) + } + return nil + } + let result = try #require(refreshed.first) + #expect(refreshed.count == 1) + #expect(result.0.isEmpty) + #expect(result.1.isEmpty) + } + @Test func duplicateRepoKeysFallbackOnceAndFanOutToEachRepository() async throws { let clock = TestClock() let probe = CoordinatorProbe()