From f92651eadbc83bf74fc3729dd89128cac43e7e86 Mon Sep 17 00:00:00 2001 From: Alex Date: Sat, 4 Jul 2026 10:30:05 +0800 Subject: [PATCH] Fix PR state flicker with Set-based tri-state semantics Revert the [String: GithubPullRequest?] approach (Swift dict[key]=nil deletes the key, not stores .some(nil)) and implement tri-state using a separate Set for "confirmed no PR" branches. Three-way distinction: - prsByBranch contains branches with a PR -> update - confirmedNoPrBranches contains branches all repos confirmed as no PR -> clear - neither -> unknown (partial failure) -> preserve existing state Key changes: - Outcome.refreshed gains confirmedNoPrBranches: Set - emitOutcomes computes it only when ALL candidate repos succeeded - pullRequestsByWorktreeID only clears worktrees in confirmedNoPrBranches - mergePullRequestRefreshResults accumulates Set without overwriting existing PRs - Keep no-remote path fix (remoteInfos.isEmpty no longer clears PRs) - Rename refreshClearsStalePullRequestsWhenGithubRemotesDisappear to refreshPreservesPullRequestsWhenGithubRemotesUnavailable Signed-off-by: Alex --- supacode/Clients/Github/GithubCLIClient.swift | 37 ++++++------ supacode/Clients/Github/GithubCLIModels.swift | 4 +- .../GithubGraphQLPullRequestResponse.swift | 16 +++-- .../PullRequestRefreshCoordinator.swift | 33 ++++++---- ...epositoriesFeature+GithubIntegration.swift | 60 ++++++++++++------- ...epositoriesFeature+WorkspaceChildren.swift | 2 +- .../Reducer/RepositoriesFeature.swift | 5 +- ...atchedPullRequestRefreshReducerTests.swift | 21 +++++-- .../GithubBatchPullRequestsTests.swift | 20 +++---- supacodeTests/GithubCLIClientTests.swift | 6 +- .../PullRequestRefreshCoordinatorTests.swift | 28 ++++----- 11 files changed, 133 insertions(+), 99 deletions(-) diff --git a/supacode/Clients/Github/GithubCLIClient.swift b/supacode/Clients/Github/GithubCLIClient.swift index 972d61c4..94f4dd5d 100644 --- a/supacode/Clients/Github/GithubCLIClient.swift +++ b/supacode/Clients/Github/GithubCLIClient.swift @@ -177,7 +177,7 @@ struct GithubCLIClient: Sendable { var resolveRemoteInfo: @Sendable (URL) async -> GithubRemoteInfo? var latestRun: @Sendable (URL, String, GithubAccountOverride?) async throws -> GithubWorkflowRun? var batchPullRequests: - @Sendable (String, String, String, [String], GithubAccountOverride?) async throws -> [String: GithubPullRequest?] + @Sendable (String, String, String, [String], GithubAccountOverride?) async throws -> [String: GithubPullRequest] var batchPullRequestsAcrossRepositories: @Sendable (String, [CrossRepoPullRequestRequest], GithubAccountOverride?) async throws -> CrossRepoPullRequestResult var mergePullRequest: @@ -330,7 +330,7 @@ nonisolated private func latestRunFetcher( nonisolated private func batchPullRequestsFetcher( shell: ShellClient, resolver: GithubCLIExecutableResolver -) -> @Sendable (String, String, String, [String], GithubAccountOverride?) async throws -> [String: GithubPullRequest?] { +) -> @Sendable (String, String, String, [String], GithubAccountOverride?) async throws -> [String: GithubPullRequest] { { host, owner, repo, branches, accountOverride in try await withExpectedGithubAccount( shell: shell, @@ -365,7 +365,7 @@ nonisolated private let crossRepoBatchAliasLimit = 15 nonisolated private let crossRepoBatchMaxConcurrentRequests = 3 nonisolated private struct CrossRepoChunkOutcome: Sendable { - let successByRepo: [RepoKey: [String: GithubPullRequest?]] + let successByRepo: [RepoKey: [String: GithubPullRequest]] let failedRepos: [RepoKey: GithubCLIError] } @@ -492,7 +492,7 @@ nonisolated private func loadCrossRepoChunks( nonisolated private func mergeCrossRepoChunkResults( _ outcomes: [CrossRepoChunkOutcome] ) -> CrossRepoPullRequestResult { - var success: [RepoKey: [String: GithubPullRequest?]] = [:] + var success: [RepoKey: [String: GithubPullRequest]] = [:] var failed: [RepoKey: GithubCLIError] = [:] for outcome in outcomes { for (key, prs) in outcome.successByRepo { @@ -537,7 +537,7 @@ nonisolated private func fetchCrossRepoChunk( let response = try GithubCLIOutput.decode(CrossRepoPullRequestResponse.self, from: output, decoder: decoder) let errorMessagesByAlias = response.errorMessagesByAlias() - var success: [RepoKey: [String: GithubPullRequest?]] = [:] + var success: [RepoKey: [String: GithubPullRequest]] = [:] var failed: [RepoKey: GithubCLIError] = [:] for (alias, key) in plan.repoAliases { if let detail = errorMessagesByAlias[alias] { @@ -669,14 +669,15 @@ nonisolated private func rankCrossRepoPullRequests( pullRequestsByAlias: [String: GithubGraphQLPullRequestResponse.PullRequestConnection], aliasMap: [String: String], allowedHeadRepositories: Set -) -> [String: GithubPullRequest?] { - var results: [String: GithubPullRequest?] = [:] - for (alias, branch) in aliasMap { - guard let connection = pullRequestsByAlias[alias] else { - results[branch] = nil +) -> [String: GithubPullRequest] { + var results: [String: GithubPullRequest] = [:] + for (alias, connection) in pullRequestsByAlias { + guard let branch = aliasMap[alias] else { continue } - results[branch] = connection.bestMatchingPullRequest(allowedHeadRepositories: allowedHeadRepositories)?.pullRequest + if let node = connection.bestMatchingPullRequest(allowedHeadRepositories: allowedHeadRepositories) { + results[branch] = node.pullRequest + } } return results } @@ -1054,9 +1055,9 @@ nonisolated private func loadPullRequestChunks( resolver: GithubCLIExecutableResolver, request: GithubPullRequestsRequest, chunks: [[String]] -) async throws -> [Int: [String: GithubPullRequest?]] { +) async throws -> [Int: [String: GithubPullRequest]] { try await withThrowingTaskGroup( - of: (Int, [String: GithubPullRequest?]).self + of: (Int, [String: GithubPullRequest]).self ) { group in var nextChunkIndex = 0 let initialCount = min(batchPullRequestsMaxConcurrentRequests, chunks.count) @@ -1075,7 +1076,7 @@ nonisolated private func loadPullRequestChunks( nextChunkIndex += 1 } - var resultsByChunkIndex: [Int: [String: GithubPullRequest?]] = [:] + var resultsByChunkIndex: [Int: [String: GithubPullRequest]] = [:] while let (chunkIndex, prsByBranch) = try await group.next() { resultsByChunkIndex[chunkIndex] = prsByBranch if nextChunkIndex < chunks.count { @@ -1099,10 +1100,10 @@ nonisolated private func loadPullRequestChunks( } nonisolated private func mergePullRequestChunkResults( - _ chunkResults: [Int: [String: GithubPullRequest?]], + _ chunkResults: [Int: [String: GithubPullRequest]], chunkCount: Int -) -> [String: GithubPullRequest?] { - var results: [String: GithubPullRequest?] = [:] +) -> [String: GithubPullRequest] { + var results: [String: GithubPullRequest] = [:] for chunkIndex in 0.. (Int, [String: GithubPullRequest?]) { +) async throws -> (Int, [String: GithubPullRequest]) { let (query, aliasMap) = makeBatchPullRequestsQuery(branches: chunk) let output = try await runGh( shell: shell, diff --git a/supacode/Clients/Github/GithubCLIModels.swift b/supacode/Clients/Github/GithubCLIModels.swift index 37e198e0..2e8a122d 100644 --- a/supacode/Clients/Github/GithubCLIModels.swift +++ b/supacode/Clients/Github/GithubCLIModels.swift @@ -167,11 +167,11 @@ nonisolated struct CrossRepoPullRequestRequest: Sendable, Hashable { } nonisolated struct CrossRepoPullRequestResult: Sendable { - let successByRepo: [RepoKey: [String: GithubPullRequest?]] + let successByRepo: [RepoKey: [String: GithubPullRequest]] let failedRepos: [RepoKey: GithubCLIError] init( - successByRepo: [RepoKey: [String: GithubPullRequest?]] = [:], + successByRepo: [RepoKey: [String: GithubPullRequest]] = [:], failedRepos: [RepoKey: GithubCLIError] = [:] ) { self.successByRepo = successByRepo diff --git a/supacode/Clients/Github/GithubGraphQLPullRequestResponse.swift b/supacode/Clients/Github/GithubGraphQLPullRequestResponse.swift index 41651d8d..4e6ceb0a 100644 --- a/supacode/Clients/Github/GithubGraphQLPullRequestResponse.swift +++ b/supacode/Clients/Github/GithubGraphQLPullRequestResponse.swift @@ -7,17 +7,15 @@ nonisolated struct GithubGraphQLPullRequestResponse: Decodable { aliasMap: [String: String], owner: String, repo: String - ) -> [String: GithubPullRequest?] { - var results: [String: GithubPullRequest?] = [:] - // Iterate aliasMap (not pullRequestsByAlias) so every queried branch appears - // in the result — nil means "queried but no PR found", which is semantically - // different from "branch not queried at all". - for (alias, branch) in aliasMap { - guard let connection = data.repository.pullRequestsByAlias[alias] else { - results[branch] = nil + ) -> [String: GithubPullRequest] { + var results: [String: GithubPullRequest] = [:] + for (alias, connection) in data.repository.pullRequestsByAlias { + guard let branch = aliasMap[alias] else { continue } - results[branch] = connection.bestMatchingPullRequest(owner: owner, repo: repo)?.pullRequest + if let node = connection.bestMatchingPullRequest(owner: owner, repo: repo) { + results[branch] = node.pullRequest + } } return results } diff --git a/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift b/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift index 739fb140..6446388f 100644 --- a/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift +++ b/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift @@ -70,7 +70,8 @@ final class PullRequestRefreshCoordinator { repositoryID: Repository.ID, repositoryRootURL: URL, worktreeIDs: [Worktree.ID], - prsByBranch: [String: GithubPullRequest?] + prsByBranch: [String: GithubPullRequest], + confirmedNoPrBranches: Set ) case failed( repositoryID: Repository.ID, @@ -337,7 +338,7 @@ final class PullRequestRefreshCoordinator { private func emitOutcomes( _ requests: [Request], - prsByRepo: [RepoKey: [String: GithubPullRequest?]], + prsByRepo: [RepoKey: [String: GithubPullRequest]], failedMessagesByRepo: [RepoKey: String] ) { for request in requests { @@ -355,12 +356,21 @@ final class PullRequestRefreshCoordinator { ) ) } else { + // Only mark branches as "confirmed no PR" when all candidate repos + // succeeded — if any repo failed, the branch status is unknown and + // the reducer should preserve existing PR state. + let allCandidatesSucceeded = + !candidateKeys.isEmpty + && candidateKeys.allSatisfy { prsByRepo[$0] != nil && failedMessagesByRepo[$0] == nil } + let confirmedNoPrBranches: Set = + allCandidatesSucceeded ? Set(request.branches).subtracting(prsByBranch.keys) : [] resultHandler( .refreshed( repositoryID: request.repositoryID, repositoryRootURL: request.repositoryRootURL, worktreeIDs: request.worktreeIDs, - prsByBranch: prsByBranch + prsByBranch: prsByBranch, + confirmedNoPrBranches: confirmedNoPrBranches ) ) } @@ -369,16 +379,13 @@ final class PullRequestRefreshCoordinator { private func mergedPullRequests( for request: Request, - prsByRepo: [RepoKey: [String: GithubPullRequest?]] - ) -> [String: GithubPullRequest?] { - var prsByBranch: [String: GithubPullRequest?] = [:] + prsByRepo: [RepoKey: [String: GithubPullRequest]] + ) -> [String: GithubPullRequest] { + var prsByBranch: [String: GithubPullRequest] = [:] for branch in request.branches { for repository in request.repositories { - let repoResult = prsByRepo[repository.key] - // Check if this repo even knows about this branch — if the key exists, - // use its value (PR or nil) and stop searching. - if let prs = repoResult, prs.keys.contains(branch) { - prsByBranch[branch] = prs[branch] + if let pullRequest = prsByRepo[repository.key]?[branch] { + prsByBranch[branch] = pullRequest break } } @@ -440,12 +447,12 @@ final class PullRequestRefreshCoordinator { } private struct RepoFetchResults: Sendable { - var successByRepo: [RepoKey: [String: GithubPullRequest?]] = [:] + var successByRepo: [RepoKey: [String: GithubPullRequest]] = [:] var failedMessagesByRepo: [RepoKey: String] = [:] } private enum RepoFetchOutcome: Sendable { - case success(RepoKey, [String: GithubPullRequest?]) + case success(RepoKey, [String: GithubPullRequest]) case failed(RepoKey, String) } } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 7f4c79d7..3e306257 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -724,7 +724,7 @@ extension RepositoriesFeature { outcome: PullRequestRefreshCoordinator.Outcome ) -> Effect { switch outcome { - case .refreshed(let repositoryID, _, let worktreeIDs, let prsByBranch): + case .refreshed(let repositoryID, _, let worktreeIDs, let prsByBranch, let confirmedNoPrBranches): guard let repository = state.repositories[id: repositoryID] else { state.inFlightPullRequestRefreshRepositoryIDs.remove(repositoryID) clearPullRequestRefreshTracking(repositoryID: repositoryID, state: &state) @@ -733,6 +733,7 @@ extension RepositoriesFeature { mergePullRequestRefreshResults( repositoryID: repositoryID, prsByBranch: prsByBranch, + confirmedNoPrBranches: confirmedNoPrBranches, state: &state ) guard consumePullRequestRefreshBatch(repositoryID: repositoryID, state: &state) else { @@ -742,11 +743,16 @@ extension RepositoriesFeature { state.prRefreshResultsByRepositoryID.removeValue( forKey: repositoryID ) ?? [:] + let confirmedNoPrBranches = + state.prRefreshConfirmedNoPrBranchesByRepositoryID.removeValue( + forKey: repositoryID + ) ?? [] state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) let prsByWorktreeID = pullRequestsByWorktreeID( repository: repository, worktreeIDs: worktreeIDs, - prsByBranch: mergedPRsByBranch + prsByBranch: mergedPRsByBranch, + confirmedNoPrBranches: confirmedNoPrBranches ) return .merge( .send( @@ -767,6 +773,7 @@ extension RepositoriesFeature { state.prRefreshResultsByRepositoryID.removeValue( forKey: repositoryID ) ?? [:] + let _ = state.prRefreshConfirmedNoPrBranchesByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) guard !mergedPRsByBranch.isEmpty, let repository = state.repositories[id: repositoryID] @@ -781,7 +788,8 @@ extension RepositoriesFeature { pullRequestsByWorktreeID: pullRequestsByWorktreeID( repository: repository, worktreeIDs: worktreeIDs, - prsByBranch: mergedPRsByBranch + prsByBranch: mergedPRsByBranch, + confirmedNoPrBranches: [] ) ) ) @@ -793,31 +801,31 @@ extension RepositoriesFeature { private func mergePullRequestRefreshResults( repositoryID: Repository.ID, - prsByBranch: [String: GithubPullRequest?], + prsByBranch: [String: GithubPullRequest], + confirmedNoPrBranches: Set, state: inout State ) { - guard !prsByBranch.isEmpty else { - return - } var merged = state.prRefreshResultsByRepositoryID[repositoryID] ?? [:] var resultPriorities = state.prRefreshResultPrioritiesByRepositoryID[repositoryID] ?? [:] let remotePriorities = state.prRefreshRemotePrioritiesByRepositoryID[repositoryID] ?? [:] // Host batches race independently. Use the returned PR URL to recover its // source repo, then compare against the original remote order before replacing. for (branch, pullRequest) in prsByBranch { - if let pullRequest { - let priority = remotePriority(for: pullRequest, remotePriorities: remotePriorities) - if merged[branch] == nil || priority < (resultPriorities[branch] ?? .max) { - merged[branch] = pullRequest - resultPriorities[branch] = priority - } - } else if merged[branch] == nil { - // Explicitly no PR found for this branch, and we haven't seen a PR from - // another host yet — record nil so downstream consumers can clear stale state. - // If merged already has a PR, don't overwrite it with nil. - merged[branch] = nil + let priority = remotePriority(for: pullRequest, remotePriorities: remotePriorities) + if merged[branch] == nil || priority < (resultPriorities[branch] ?? .max) { + merged[branch] = pullRequest + resultPriorities[branch] = priority } } + // Accumulate confirmed-no-PR branches. Only clear when all repos for a + // branch succeeded and none returned a PR — partial failures leave the + // branch out of confirmedNoPrBranches so existing state is preserved. + var existingConfirmed = state.prRefreshConfirmedNoPrBranchesByRepositoryID[repositoryID] ?? [] + existingConfirmed.formUnion(confirmedNoPrBranches) + // Remove any confirmed-no-PR entries that now have a PR (priority-based + // merge may have resolved a later host's PR over an earlier "no PR"). + existingConfirmed.subtract(merged.keys) + state.prRefreshConfirmedNoPrBranchesByRepositoryID[repositoryID] = existingConfirmed state.prRefreshResultsByRepositoryID[repositoryID] = merged state.prRefreshResultPrioritiesByRepositoryID[repositoryID] = resultPriorities } @@ -838,6 +846,7 @@ extension RepositoriesFeature { ) { state.prRefreshBatchCountsByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshResultsByRepositoryID.removeValue(forKey: repositoryID) + state.prRefreshConfirmedNoPrBranchesByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshRemotePrioritiesByRepositoryID.removeValue(forKey: repositoryID) state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) } @@ -845,6 +854,7 @@ extension RepositoriesFeature { private func clearAllPullRequestRefreshTracking(state: inout State) { state.prRefreshBatchCountsByRepositoryID.removeAll() state.prRefreshResultsByRepositoryID.removeAll() + state.prRefreshConfirmedNoPrBranchesByRepositoryID.removeAll() state.prRefreshRemotePrioritiesByRepositoryID.removeAll() state.prRefreshResultPrioritiesByRepositoryID.removeAll() } @@ -865,13 +875,19 @@ extension RepositoriesFeature { private func pullRequestsByWorktreeID( repository: Repository, worktreeIDs: [Worktree.ID], - prsByBranch: [String: GithubPullRequest?] + prsByBranch: [String: GithubPullRequest], + confirmedNoPrBranches: Set ) -> [Worktree.ID: GithubPullRequest?] { var prsByWorktreeID: [Worktree.ID: GithubPullRequest?] = [:] for worktreeID in worktreeIDs { - if let worktree = repository.worktrees[id: worktreeID] { - prsByWorktreeID[worktreeID] = prsByBranch[worktree.name] - } + guard let worktree = repository.worktrees[id: worktreeID] else { continue } + if let pr = prsByBranch[worktree.name] { + prsByWorktreeID[worktreeID] = pr + } else if confirmedNoPrBranches.contains(worktree.name) { + // All repos confirmed no PR for this branch — explicitly clear. + prsByWorktreeID[worktreeID] = nil + } + // Otherwise: unknown status (partial failure) — omit to preserve existing. } return prsByWorktreeID } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift index 094ad3e5..c9202e2b 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorkspaceChildren.swift @@ -89,7 +89,7 @@ extension RepositoriesFeature { [branch], nil ) - return pullRequestsByBranch?[branch] ?? nil + return pullRequestsByBranch?[branch] } } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index f8039dac..78beaec3 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -336,7 +336,10 @@ struct RepositoriesFeature { var pendingPullRequestRefreshByRepositoryID: [Repository.ID: PendingPullRequestRefresh] = [:] var inFlightPullRequestRefreshRepositoryIDs: Set = [] var prRefreshBatchCountsByRepositoryID: [Repository.ID: Int] = [:] - var prRefreshResultsByRepositoryID: [Repository.ID: [String: GithubPullRequest?]] = [:] + var prRefreshResultsByRepositoryID: [Repository.ID: [String: GithubPullRequest]] = [:] + /// 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 prRefreshConfirmedNoPrBranchesByRepositoryID: [Repository.ID: 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 4ed78293..2ac7dde5 100644 --- a/supacodeTests/BatchedPullRequestRefreshReducerTests.swift +++ b/supacodeTests/BatchedPullRequestRefreshReducerTests.swift @@ -109,12 +109,14 @@ struct BatchedPullRequestRefreshReducerTests { repositoryID: context.repository.id, repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs, - prsByBranch: ["feature": githubPullRequest] + prsByBranch: ["feature": githubPullRequest], + confirmedNoPrBranches: [] ) )) ) { $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = ["feature": githubPullRequest] + $0.prRefreshConfirmedNoPrBranchesByRepositoryID[context.repository.id] = [] $0.prRefreshResultPrioritiesByRepositoryID[context.repository.id] = ["feature": .max] } @@ -125,12 +127,14 @@ struct BatchedPullRequestRefreshReducerTests { repositoryID: context.repository.id, repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs, - prsByBranch: [:] + prsByBranch: [:], + confirmedNoPrBranches: [] ) )) ) { $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] + $0.prRefreshConfirmedNoPrBranchesByRepositoryID = [:] $0.prRefreshResultPrioritiesByRepositoryID = [:] } await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { @@ -200,12 +204,14 @@ struct BatchedPullRequestRefreshReducerTests { repositoryID: context.repository.id, repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs, - prsByBranch: ["feature": enterprisePullRequest] + prsByBranch: ["feature": enterprisePullRequest], + confirmedNoPrBranches: [] ) )) ) { $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = ["feature": enterprisePullRequest] + $0.prRefreshConfirmedNoPrBranchesByRepositoryID[context.repository.id] = [] $0.prRefreshResultPrioritiesByRepositoryID[context.repository.id] = ["feature": 1] } @@ -216,12 +222,14 @@ struct BatchedPullRequestRefreshReducerTests { repositoryID: context.repository.id, repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs, - prsByBranch: ["feature": originPullRequest] + prsByBranch: ["feature": originPullRequest], + confirmedNoPrBranches: [] ) )) ) { $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] + $0.prRefreshConfirmedNoPrBranchesByRepositoryID = [:] $0.prRefreshResultPrioritiesByRepositoryID = [:] } await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { @@ -281,7 +289,7 @@ struct BatchedPullRequestRefreshReducerTests { #expect(enqueued.value.count == 1) } - @Test func refreshClearsStalePullRequestsWhenGithubRemotesDisappear() async { + @Test func refreshPreservesPullRequestsWhenGithubRemotesUnavailable() async { let context = makeContext() let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) let stalePullRequest = makePullRequestFixture(url: "https://github.com/khoi/alpha/pull/7") @@ -339,7 +347,8 @@ struct BatchedPullRequestRefreshReducerTests { repositoryID: context.repository.id, repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs, - prsByBranch: ["feature": pullRequest] + prsByBranch: ["feature": pullRequest], + confirmedNoPrBranches: [] ) await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) diff --git a/supacodeTests/GithubBatchPullRequestsTests.swift b/supacodeTests/GithubBatchPullRequestsTests.swift index dfea93b2..a66ebfec 100644 --- a/supacodeTests/GithubBatchPullRequestsTests.swift +++ b/supacodeTests/GithubBatchPullRequestsTests.swift @@ -61,9 +61,9 @@ struct GithubBatchPullRequestsTests { owner: "octo", repo: "repo" ) - #expect((prs["feature-a"] ?? nil)?.number == 2) - #expect((prs["feature-a"] ?? nil)?.title == "Primary PR") - #expect((prs["feature-b"] ?? nil) == nil) + #expect(prs["feature-a"]?.number == 2) + #expect(prs["feature-a"]?.title == "Primary PR") + #expect(prs["feature-b"] == nil) } @Test func ignoresForkOnlyMatches() throws { @@ -104,7 +104,7 @@ struct GithubBatchPullRequestsTests { owner: "octo", repo: "repo" ) - #expect((prs["feature-a"] ?? nil) == nil) + #expect(prs["feature-a"] == nil) } @Test func ignoresPullRequestWithUnknownHeadRepository() throws { @@ -142,7 +142,7 @@ struct GithubBatchPullRequestsTests { owner: "octo", repo: "repo" ) - #expect((prs["feature-a"] ?? nil) == nil) + #expect(prs["feature-a"] == nil) } @Test func ignoresForkEvenWhenBaseBranchDiffers() throws { @@ -201,7 +201,7 @@ struct GithubBatchPullRequestsTests { owner: "octo", repo: "repo" ) - #expect((prs["feature-a"] ?? nil) == nil) + #expect(prs["feature-a"] == nil) } @Test func prefersOpenOverMergedEvenIfOlder() throws { @@ -258,8 +258,8 @@ struct GithubBatchPullRequestsTests { owner: "octo", repo: "repo" ) - #expect((prs["feature-a"] ?? nil)?.number == 11) - #expect((prs["feature-a"] ?? nil)?.title == "Open PR") + #expect(prs["feature-a"]?.number == 11) + #expect(prs["feature-a"]?.title == "Open PR") } @Test func fallsBackToLatestMerged() throws { @@ -316,7 +316,7 @@ struct GithubBatchPullRequestsTests { owner: "octo", repo: "repo" ) - #expect((prs["feature-a"] ?? nil)?.number == 21) - #expect((prs["feature-a"] ?? nil)?.title == "Merged Newer") + #expect(prs["feature-a"]?.number == 21) + #expect(prs["feature-a"]?.title == "Merged Newer") } } diff --git a/supacodeTests/GithubCLIClientTests.swift b/supacodeTests/GithubCLIClientTests.swift index 9cb05236..00e3bbe5 100644 --- a/supacodeTests/GithubCLIClientTests.swift +++ b/supacodeTests/GithubCLIClientTests.swift @@ -662,7 +662,7 @@ struct GithubCLIClientTests { let result = try await client.batchPullRequestsAcrossRepositories("github.com", requests, nil) let alphaPRs = try #require(result.successByRepo[RepoKey(owner: "khoi", repo: "alpha")]) - let pullRequest = try #require(alphaPRs["feat-1"] ?? nil) + let pullRequest = try #require(alphaPRs["feat-1"]) #expect(pullRequest.number == 42) } @@ -683,7 +683,7 @@ struct GithubCLIClientTests { let result = try await client.batchPullRequestsAcrossRepositories("github.com", requests, nil) let kingfisherPRs = try #require(result.successByRepo[RepoKey(owner: "onevcat", repo: "Kingfisher")]) - #expect((kingfisherPRs["master"] ?? nil) == nil) + #expect(kingfisherPRs["master"] == nil) } @Test func batchAcrossRepositoriesAllowsPullRequestFromConfiguredHeadRemote() async throws { @@ -724,7 +724,7 @@ struct GithubCLIClientTests { let result = try await client.batchPullRequestsAcrossRepositories("github.com", requests, nil) let upstreamPRs = try #require(result.successByRepo[RepoKey(owner: "supabitapp", repo: "supacode")]) - #expect((upstreamPRs["feature"] ?? nil)?.number == 42) + #expect(upstreamPRs["feature"]?.number == 42) } @Test func executableResolutionIsSingleFlightAndReused() async { diff --git a/supacodeTests/PullRequestRefreshCoordinatorTests.swift b/supacodeTests/PullRequestRefreshCoordinatorTests.swift index 29f64678..ef13cd6b 100644 --- a/supacodeTests/PullRequestRefreshCoordinatorTests.swift +++ b/supacodeTests/PullRequestRefreshCoordinatorTests.swift @@ -100,7 +100,7 @@ struct PullRequestRefreshCoordinatorTests { clock: clock, outcomes: outcomes, batched: { _, requests in - var success: [RepoKey: [String: GithubPullRequest?]] = [:] + var success: [RepoKey: [String: GithubPullRequest]] = [:] var failed: [RepoKey: GithubCLIError] = [:] for request in requests { let key = RepoKey(owner: request.owner, repo: request.repo) @@ -258,7 +258,7 @@ struct PullRequestRefreshCoordinatorTests { clock: clock, outcomes: outcomes, batched: { _, requests in - var dict: [RepoKey: [String: GithubPullRequest?]] = [:] + var dict: [RepoKey: [String: GithubPullRequest]] = [:] for request in requests { dict[request.key] = [ "feat-1": makeFixturePullRequest(repo: request.repo), @@ -289,7 +289,7 @@ struct PullRequestRefreshCoordinatorTests { let snapshots = await outcomes.snapshot() let refreshed = snapshots.compactMap { outcome -> (Repository.ID, [String])? in - if case .refreshed(let id, _, _, let prs) = outcome { + if case .refreshed(let id, _, _, let prs, _) = outcome { return (id, Array(prs.keys)) } return nil @@ -308,7 +308,7 @@ struct PullRequestRefreshCoordinatorTests { clock: clock, outcomes: outcomes, batched: { _, requests in - var dict: [RepoKey: [String: GithubPullRequest?]] = [:] + var dict: [RepoKey: [String: GithubPullRequest]] = [:] for request in requests { if request.repo == "upstream" { dict[request.key] = ["feat-1": makeFixturePullRequest(repo: "upstream")] @@ -335,14 +335,14 @@ struct PullRequestRefreshCoordinatorTests { ] #expect(calls.first?.requests.allSatisfy { $0.allowedHeadRepositories == expectedAllowedHeadRepositories } == true) - let refreshed = await outcomes.snapshot().compactMap { outcome -> [String: GithubPullRequest?]? in - if case .refreshed("local", _, _, let prsByBranch) = outcome { + let refreshed = await outcomes.snapshot().compactMap { outcome -> [String: GithubPullRequest]? in + if case .refreshed("local", _, _, let prsByBranch, _) = outcome { return prsByBranch } return nil } #expect(refreshed.count == 1) - #expect((refreshed.first?["feat-1"] ?? nil)?.title == "PR-upstream") + #expect(refreshed.first?["feat-1"]?.title == "PR-upstream") } @Test func duplicateRepoKeysFallbackOnceAndFanOutToEachRepository() async throws { @@ -459,7 +459,7 @@ struct PullRequestRefreshCoordinatorTests { clock: clock, outcomes: outcomes, batched: { _, requests in - var dict: [RepoKey: [String: GithubPullRequest?]] = [:] + var dict: [RepoKey: [String: GithubPullRequest]] = [:] for request in requests { let pullRequest = makeFixturePullRequest(repo: request.repo) dict[RepoKey(owner: request.owner, repo: request.repo)] = ["feat-1": pullRequest] @@ -475,8 +475,8 @@ struct PullRequestRefreshCoordinatorTests { let snapshots = await outcomes.snapshot() let refresh = try #require( - snapshots.compactMap { snapshot -> (String, [String: GithubPullRequest?])? in - if case .refreshed(let id, _, _, let prs) = snapshot { + snapshots.compactMap { snapshot -> (String, [String: GithubPullRequest])? in + if case .refreshed(let id, _, _, let prs, _) = snapshot { return (id, prs) } return nil @@ -484,7 +484,7 @@ struct PullRequestRefreshCoordinatorTests { .first ) #expect(refresh.0 == "alpha") - #expect((refresh.1["feat-1"] ?? nil)?.title == "PR-alpha") + #expect(refresh.1["feat-1"]?.title == "PR-alpha") } @Test func enqueueAfterFlushStartsNewDebounceWindow() async throws { @@ -560,7 +560,7 @@ private func makeCoordinator( CrossRepoPullRequestResult, legacy: @escaping @Sendable (String, String, String, [String]) async throws -> - [String: GithubPullRequest?] = { _, _, _, _ in [:] } + [String: GithubPullRequest] = { _, _, _, _ in [:] } ) -> PullRequestRefreshCoordinator { var client = GithubCLIClient.testValue client.batchPullRequestsAcrossRepositories = { host, requests, accountOverride in @@ -629,7 +629,7 @@ nonisolated private func request( nonisolated private func successResult( for requests: [CrossRepoPullRequestRequest] ) -> CrossRepoPullRequestResult { - var dict: [RepoKey: [String: GithubPullRequest?]] = [:] + var dict: [RepoKey: [String: GithubPullRequest]] = [:] for request in requests { dict[RepoKey(owner: request.owner, repo: request.repo)] = [:] } @@ -708,7 +708,7 @@ actor OutcomeCollector { func refreshedRepositories() -> [String] { outcomes.compactMap { - if case .refreshed(let id, _, _, _) = $0 { + if case .refreshed(let id, _, _, _, _) = $0 { return id } return nil -- 2.51.2