Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ final class PullRequestRefreshCoordinator {
repositoryID: Repository.ID,
repositoryRootURL: URL,
worktreeIDs: [Worktree.ID],
prsByBranch: [String: GithubPullRequest]
prsByBranch: [String: GithubPullRequest],
confirmedNoPrBranches: Set<String>
)
case failed(
repositoryID: Repository.ID,
Expand Down Expand Up @@ -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<String> =
allCandidatesSucceeded ? Set(request.branches).subtracting(prsByBranch.keys) : []
resultHandler(
.refreshed(
repositoryID: request.repositoryID,
repositoryRootURL: request.repositoryRootURL,
worktreeIDs: request.worktreeIDs,
prsByBranch: prsByBranch
prsByBranch: prsByBranch,
confirmedNoPrBranches: confirmedNoPrBranches
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ extension RepositoriesFeature {
outcome: PullRequestRefreshCoordinator.Outcome
) -> Effect<Action> {
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)
Expand All @@ -733,6 +733,7 @@ extension RepositoriesFeature {
mergePullRequestRefreshResults(
repositoryID: repositoryID,
prsByBranch: prsByBranch,
confirmedNoPrBranches: confirmedNoPrBranches,
state: &state
)
guard consumePullRequestRefreshBatch(repositoryID: repositoryID, state: &state) else {
Expand All @@ -742,11 +743,20 @@ extension RepositoriesFeature {
state.prRefreshResultsByRepositoryID.removeValue(
forKey: repositoryID
) ?? [:]
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,
worktreeIDs: worktreeIDs,
prsByBranch: mergedPRsByBranch
prsByBranch: mergedPRsByBranch,
confirmedNoPrBranches: confirmedNoPrBranches
)
return .merge(
.send(
Expand All @@ -760,13 +770,16 @@ 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
}
let mergedPRsByBranch =
state.prRefreshResultsByRepositoryID.removeValue(
forKey: repositoryID
) ?? [:]
state.prRefreshFailedBatchRepositoryIDs.remove(repositoryID)
_ = state.prRefreshNoPrBranchesByID.removeValue(forKey: repositoryID)
state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID)
guard !mergedPRsByBranch.isEmpty,
let repository = state.repositories[id: repositoryID]
Expand All @@ -781,7 +794,8 @@ extension RepositoriesFeature {
pullRequestsByWorktreeID: pullRequestsByWorktreeID(
repository: repository,
worktreeIDs: worktreeIDs,
prsByBranch: mergedPRsByBranch
prsByBranch: mergedPRsByBranch,
confirmedNoPrBranches: []
)
)
)
Expand All @@ -794,11 +808,9 @@ extension RepositoriesFeature {
private func mergePullRequestRefreshResults(
repositoryID: Repository.ID,
prsByBranch: [String: GithubPullRequest],
confirmedNoPrBranches: Set<String>,
state: inout State
) {
guard !prsByBranch.isEmpty else {
return
}
var merged = state.prRefreshResultsByRepositoryID[repositoryID] ?? [:]
var resultPriorities = state.prRefreshResultPrioritiesByRepositoryID[repositoryID] ?? [:]
let remotePriorities = state.prRefreshRemotePrioritiesByRepositoryID[repositoryID] ?? [:]
Expand All @@ -811,6 +823,15 @@ extension RepositoriesFeature {
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.prRefreshNoPrBranchesByID[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.prRefreshNoPrBranchesByID[repositoryID] = existingConfirmed
state.prRefreshResultsByRepositoryID[repositoryID] = merged
state.prRefreshResultPrioritiesByRepositoryID[repositoryID] = resultPriorities
}
Expand All @@ -831,13 +852,17 @@ 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)
}

private func clearAllPullRequestRefreshTracking(state: inout State) {
state.prRefreshBatchCountsByRepositoryID.removeAll()
state.prRefreshResultsByRepositoryID.removeAll()
state.prRefreshNoPrBranchesByID.removeAll()
state.prRefreshFailedBatchRepositoryIDs.removeAll()
state.prRefreshRemotePrioritiesByRepositoryID.removeAll()
state.prRefreshResultPrioritiesByRepositoryID.removeAll()
}
Expand All @@ -858,13 +883,21 @@ extension RepositoriesFeature {
private func pullRequestsByWorktreeID(
repository: Repository,
worktreeIDs: [Worktree.ID],
prsByBranch: [String: GithubPullRequest]
prsByBranch: [String: GithubPullRequest],
confirmedNoPrBranches: Set<String>
) -> [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 pullRequest = prsByBranch[worktree.name] {
prsByWorktreeID[worktreeID] = pullRequest
} else if confirmedNoPrBranches.contains(worktree.name) {
// 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.
}
return prsByWorktreeID
}
Expand All @@ -886,18 +919,9 @@ extension RepositoriesFeature {
gitClient: gitClient
)
guard !remoteInfos.isEmpty else {
let clearedPullRequestsByWorktreeID = Dictionary(
worktreeIDs.map { ($0, Optional<GithubPullRequest>.none) },
uniquingKeysWith: { first, _ in first }
)
await send(
.githubIntegration(
.repositoryPullRequestsLoaded(
repositoryID: repositoryID,
pullRequestsByWorktreeID: clearedPullRequestsByWorktreeID
)
)
)
// No GitHub remote configured for this repository — preserve existing PR
// values rather than clearing them, which would cause a flicker during
// refresh cycles.
await send(.githubIntegration(.repositoryPullRequestRefreshCompleted(repositoryID)))
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,13 @@ struct RepositoriesFeature {
var inFlightPullRequestRefreshRepositoryIDs: Set<Repository.ID> = []
var prRefreshBatchCountsByRepositoryID: [Repository.ID: Int] = [:]
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 prRefreshNoPrBranchesByID: [Repository.ID: Set<String>] = [:]
/// 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<Repository.ID> = []
/// 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]] = [:]
Expand Down
Loading
Loading