From e13558b4d42827203080c93e25f22cf212d71d46 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 2 Oct 2026 02:00:51 +0200 Subject: [PATCH 1/7] Ask GitHub only about pull requests whose state can change soon Every periodic refresh asked about every branch of the repository, so the query grew with the worktree count even when nearly all of their pull requests were merged or settled and could not have changed. A periodic refresh now asks about the selected worktree, any worktree never answered, and open pull requests with checks running, mergeability still being computed, or a merge-queue entry. Settled open pull requests follow a 3-minute interval, branches without one 5 minutes, and merged or closed ones 30 minutes; a refresh with nothing due sends no query. Only a branch GitHub answered for records the time, and a remote change clears a repository's recorded times. --- .../PullRequestRefreshCadence.swift | 63 +++++++ .../RepositoriesFeature+CoreReducer.swift | 4 + ...epositoriesFeature+GithubIntegration.swift | 36 +++- .../Reducer/RepositoriesFeature.swift | 3 + ...atchedPullRequestRefreshReducerTests.swift | 160 +++++++++++++++++- .../PullRequestRefreshCadenceTests.swift | 111 ++++++++++++ App/Tests/RepositoriesFeatureTests.swift | 33 ++++ docs-ai/028-pr-status-tracking/000-plan.md | 3 + .../008-cadence-by-state.md | 50 ++++++ docs/components/github-pull-requests.md | 16 ++ 10 files changed, 470 insertions(+), 9 deletions(-) create mode 100644 App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift create mode 100644 App/Tests/PullRequestRefreshCadenceTests.swift create mode 100644 docs-ai/028-pr-status-tracking/008-cadence-by-state.md diff --git a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift new file mode 100644 index 00000000..87cb839b --- /dev/null +++ b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift @@ -0,0 +1,63 @@ +import Foundation + +// Decides which worktrees a periodic refresh asks GitHub about. A pull request whose state can +// change soon is refreshed on every sweep; one that has settled is asked about less often, so the +// batched query stays small on an account other tools share. +nonisolated enum PullRequestRefreshCadence { + /// An open pull request with nothing in flight: reviews and new pushes still change it. + static let settledOpenInterval: Duration = .seconds(180) + /// No pull request yet: one appears when someone opens it, from Prowl or elsewhere. + static let noPullRequestInterval: Duration = .seconds(300) + /// Merged or closed: only a reopened or new pull request for the same branch changes it. + static let finishedInterval: Duration = .seconds(1_800) + + static func isDue( + pullRequest: GithubPullRequest?, + lastCheckedAt: Date?, + now: Date, + isSelected: Bool + ) -> Bool { + if isSelected { + return true + } + guard let lastCheckedAt else { + return true + } + guard let interval = interval(for: pullRequest) else { + return true + } + return now.timeIntervalSince(lastCheckedAt) >= interval.seconds + } + + /// nil means every sweep. + static func interval(for pullRequest: GithubPullRequest?) -> Duration? { + guard let pullRequest else { + return noPullRequestInterval + } + switch pullRequest.state.uppercased() { + case "MERGED", "CLOSED": + return finishedInterval + default: + return isActive(pullRequest) ? nil : settledOpenInterval + } + } + + // Checks still running, GitHub still computing mergeability, or a place in the merge queue. + private static func isActive(_ pullRequest: GithubPullRequest) -> Bool { + let checks = PullRequestCheckBreakdown(checks: pullRequest.statusCheckRollup?.checks ?? []) + if checks.inProgress + checks.expected > 0 { + return true + } + if pullRequest.mergeable?.uppercased() == "UNKNOWN" || pullRequest.mergeStateStatus?.uppercased() == "UNKNOWN" { + return true + } + return pullRequest.mergeQueueEntry != nil + } +} + +extension Duration { + nonisolated fileprivate var seconds: TimeInterval { + let parts = components + return TimeInterval(parts.seconds) + TimeInterval(parts.attoseconds) / 1e18 + } +} diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift index 2ea9a458..1f38c80a 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift @@ -1009,6 +1009,10 @@ extension RepositoriesFeature { let repositories = IdentifiedArrayOf(uniqueElements: [repository]) var effects: [Effect] = [] let worktreeIDs = repository.worktrees.map(\.id) + // A changed remote can point every branch at different pull requests. + for worktreeID in worktreeIDs { + state.pullRequestCheckedAtByWorktreeID.removeValue(forKey: worktreeID) + } if repository.capabilities.supportsPullRequests, !worktreeIDs.isEmpty { effects.append( .send( diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index b47e187a..65c4862f 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -92,12 +92,26 @@ extension RepositoriesFeature { guard repositorySettings.fetchesPullRequestState else { return .none } - let worktrees = worktreeIDs.compactMap { state.worktree(for: $0) } + let selectedWorktreeID = state.selectedWorktreeID + let checkedAt = state.pullRequestCheckedAtByWorktreeID + let currentDate = worktreeIDs.contains { checkedAt[$0] != nil } ? now : nil + let worktrees = worktreeIDs.compactMap { state.worktree(for: $0) }.filter { worktree in + guard let currentDate else { + return true + } + return PullRequestRefreshCadence.isDue( + pullRequest: state.worktreeInfo(for: worktree.id)?.pullRequest, + lastCheckedAt: checkedAt[worktree.id], + now: currentDate, + isSelected: worktree.id == selectedWorktreeID + ) + } guard let firstWorktree = worktrees.first, let repositoryID = state.repositoryID(containing: firstWorktree.id) else { return .none } + let dueWorktreeIDs = worktrees.map(\.id) var seen = Set() let branches = worktrees @@ -112,7 +126,7 @@ extension RepositoriesFeature { queuePullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, - worktreeIDs: worktreeIDs, + worktreeIDs: dueWorktreeIDs, refreshesByRepositoryID: &state.queuedPullRequestRefreshByRepositoryID ) return .none @@ -128,7 +142,7 @@ extension RepositoriesFeature { queuePullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, - worktreeIDs: worktreeIDs, + worktreeIDs: dueWorktreeIDs, refreshesByRepositoryID: &state.pendingPullRequestRefreshByRepositoryID ) return .send(.githubIntegration(.refreshGithubIntegrationAvailability)) @@ -136,7 +150,7 @@ extension RepositoriesFeature { queuePullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, - worktreeIDs: worktreeIDs, + worktreeIDs: dueWorktreeIDs, refreshesByRepositoryID: &state.pendingPullRequestRefreshByRepositoryID ) return .none @@ -144,7 +158,7 @@ extension RepositoriesFeature { queuePullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, - worktreeIDs: worktreeIDs, + worktreeIDs: dueWorktreeIDs, refreshesByRepositoryID: &state.pendingPullRequestRefreshByRepositoryID ) return .none @@ -745,6 +759,18 @@ extension RepositoriesFeature { ) -> Effect { switch outcome { case .refreshed(let repositoryID, _, let worktreeIDs, let prsByBranch, let confirmedNoPrBranches): + // Only a branch GitHub answered for, with a pull request or a confirmed absence of one, has + // known state; a branch left unknown by a partial failure stays due. + let answeredBranches = Set(prsByBranch.keys).union(confirmedNoPrBranches) + if !answeredBranches.isEmpty { + let checkedAt = now + for worktreeID in worktreeIDs { + guard let name = state.worktree(for: worktreeID)?.name, answeredBranches.contains(name) else { + continue + } + state.pullRequestCheckedAtByWorktreeID[worktreeID] = checkedAt + } + } guard let repository = state.repositories[id: repositoryID] else { state.inFlightPullRequestRefreshRepositoryIDs.remove(repositoryID) clearPullRequestRefreshTracking(repositoryID: repositoryID, state: &state) diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift index 14dde166..3331bdf3 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -348,6 +348,9 @@ struct RepositoriesFeature { var statusToast: StatusToast? var snapshotPersistencePhase: SnapshotPersistencePhase = .idle var githubIntegrationAvailability: GithubIntegrationAvailability = .unknown + /// When GitHub last answered a pull request refresh for each worktree; a periodic refresh skips + /// a worktree whose pull request has settled until its cadence comes round. + var pullRequestCheckedAtByWorktreeID: [Worktree.ID: Date] = [:] var pendingPullRequestRefreshByRepositoryID: [Repository.ID: PendingPullRequestRefresh] = [:] var inFlightPullRequestRefreshRepositoryIDs: Set = [] var prRefreshBatchCountsByRepositoryID: [Repository.ID: Int] = [:] diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index a3828dd8..07fda5fd 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -62,6 +62,139 @@ struct BatchedPullRequestRefreshReducerTests { #expect(request.branches == ["main", "feature"]) } + @Test func periodicRefreshAsksOnlyForDueWorktrees() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + var merged = WorktreeInfoEntry() + merged.pullRequest = makePullRequestFixture(state: "MERGED") + initialState.worktreeInfoByID[context.featureWorktree.id] = merged + // The merged pull request was answered a minute ago; the branch without one ten minutes ago. + initialState.pullRequestCheckedAtByWorktreeID = [ + context.featureWorktree.id: now.addingTimeInterval(-60), + context.mainWorktree.id: now.addingTimeInterval(-600), + ] + let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo] } + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { request in enqueued.withValue { $0.append(request) } }, + cancelHost: { _ in }, + reset: {} + ) + } + store.exhaustivity = .off + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.finish() + + #expect(enqueued.value.map(\.branches) == [["main"]]) + #expect(enqueued.value.map(\.worktreeIDs) == [[context.mainWorktree.id]]) + } + + @Test func periodicRefreshWithNothingDueSendsNoQuery() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + initialState.pullRequestCheckedAtByWorktreeID = [ + context.featureWorktree.id: now.addingTimeInterval(-60), + context.mainWorktree.id: now.addingTimeInterval(-60), + ] + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { _ in Issue.record("Nothing is due, so nothing should be enqueued") }, + cancelHost: { _ in }, + reset: {} + ) + } + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshRequested) + await store.finish() + } + + @Test func selectedWorktreeIsAskedForEvenWhenRecentlyAnswered() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + initialState.selection = .worktree(context.featureWorktree.id) + initialState.pullRequestCheckedAtByWorktreeID = [ + context.featureWorktree.id: now.addingTimeInterval(-5), + context.mainWorktree.id: now.addingTimeInterval(-5), + ] + let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo] } + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { request in enqueued.withValue { $0.append(request) } }, + cancelHost: { _ in }, + reset: {} + ) + } + store.exhaustivity = .off + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.finish() + + #expect(enqueued.value.map(\.branches) == [["feature"]]) + } + + @Test func answeredRefreshRecordsWhenEachBranchWasAnswered() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.pullRequestRefreshCoordinator = .unimplemented + } + store.exhaustivity = .off + + // GitHub answered for "feature"; "main" stayed unknown after a partial failure. + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: ["feature": makePullRequestFixture()], + confirmedNoPrBranches: [] + ) + )) + ) { + $0.pullRequestCheckedAtByWorktreeID = [context.featureWorktree.id: now] + } + await store.finish() + } + @Test func refreshWaitsForAllHostBatchesBeforeCompleting() async { let context = makeContext() let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) @@ -71,6 +204,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: context.state) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo, enterpriseInfo] } $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( enqueue: { request in @@ -114,6 +248,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = ["feature": githubPullRequest] $0.prRefreshNoPrBranchesByID[context.repository.id] = [] @@ -166,6 +301,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: context.state) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo, enterpriseInfo] } $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( enqueue: { request in @@ -209,6 +345,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = ["feature": enterprisePullRequest] $0.prRefreshNoPrBranchesByID[context.repository.id] = [] @@ -339,6 +476,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: initialState) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.pullRequestRefreshCoordinator = .unimplemented } @@ -351,7 +489,9 @@ struct BatchedPullRequestRefreshReducerTests { confirmedNoPrBranches: [] ) - await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) + await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + } await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { var entry = WorktreeInfoEntry() entry.pullRequest = pullRequest @@ -399,6 +539,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: initialState) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.pullRequestRefreshCoordinator = .unimplemented } @@ -410,7 +551,9 @@ struct BatchedPullRequestRefreshReducerTests { confirmedNoPrBranches: ["feature"] ) - await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) + await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + } await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { $0.worktreeInfoByID.removeValue(forKey: context.featureWorktree.id) } @@ -466,6 +609,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: initialState) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.pullRequestRefreshCoordinator = .unimplemented } @@ -495,6 +639,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshFailedBatchRepositoryIDs = [] } @@ -520,6 +665,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: initialState) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.pullRequestRefreshCoordinator = .unimplemented } @@ -535,6 +681,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = [:] $0.prRefreshNoPrBranchesByID[context.repository.id] = ["feature"] @@ -574,6 +721,7 @@ struct BatchedPullRequestRefreshReducerTests { let store = TestStore(initialState: initialState) { RepositoriesFeature() } withDependencies: { + $0.date.now = refreshDate $0.pullRequestRefreshCoordinator = .unimplemented } @@ -589,6 +737,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = [:] $0.prRefreshNoPrBranchesByID[context.repository.id] = ["feature"] @@ -696,6 +845,8 @@ private func makeContext() -> RefreshTestContext { ) } +nonisolated private let refreshDate = Date(timeIntervalSince1970: 1_700_000_000) + @MainActor private struct RefreshTestContext { let repoRoot: String @@ -711,12 +862,13 @@ private struct RefreshTestContext { nonisolated private func makePullRequestFixture( title: String = "Coord PR", - url: String = "https://example.com/coord-pr/7" + url: String = "https://example.com/coord-pr/7", + state: String = "OPEN" ) -> GithubPullRequest { GithubPullRequest( number: 7, title: title, - state: "OPEN", + state: state, additions: 0, deletions: 0, isDraft: false, diff --git a/App/Tests/PullRequestRefreshCadenceTests.swift b/App/Tests/PullRequestRefreshCadenceTests.swift new file mode 100644 index 00000000..4813fc2d --- /dev/null +++ b/App/Tests/PullRequestRefreshCadenceTests.swift @@ -0,0 +1,111 @@ +import Foundation +import Testing + +@testable import Prowl + +struct PullRequestRefreshCadenceTests { + nonisolated static let checkedAt = Date(timeIntervalSince1970: 1_000_000) + + struct Case: Sendable, CustomTestStringConvertible { + let pullRequest: GithubPullRequest? + let secondsSinceAnswer: Int + let isDue: Bool + + var testDescription: String { + "\(pullRequest?.state ?? "no PR") after \(secondsSinceAnswer) s: \(isDue ? "due" : "not due")" + } + } + + @Test( + arguments: [ + Case(pullRequest: nil, secondsSinceAnswer: 299, isDue: false), + Case(pullRequest: nil, secondsSinceAnswer: 300, isDue: true), + Case(pullRequest: pullRequest(state: "MERGED"), secondsSinceAnswer: 1_799, isDue: false), + Case(pullRequest: pullRequest(state: "MERGED"), secondsSinceAnswer: 1_800, isDue: true), + Case(pullRequest: pullRequest(state: "CLOSED"), secondsSinceAnswer: 1_800, isDue: true), + Case(pullRequest: pullRequest(state: "OPEN"), secondsSinceAnswer: 179, isDue: false), + Case(pullRequest: pullRequest(state: "OPEN"), secondsSinceAnswer: 180, isDue: true), + Case( + pullRequest: pullRequest(state: "OPEN", checks: [GithubPullRequestStatusCheck(status: "IN_PROGRESS")]), + secondsSinceAnswer: 1, + isDue: true + ), + Case( + pullRequest: pullRequest(state: "OPEN", checks: [GithubPullRequestStatusCheck(state: "EXPECTED")]), + secondsSinceAnswer: 1, + isDue: true + ), + Case(pullRequest: pullRequest(state: "OPEN", mergeable: "UNKNOWN"), secondsSinceAnswer: 1, isDue: true), + Case(pullRequest: pullRequest(state: "OPEN", mergeStateStatus: "UNKNOWN"), secondsSinceAnswer: 1, isDue: true), + Case(pullRequest: pullRequest(state: "OPEN", queued: true), secondsSinceAnswer: 1, isDue: true), + Case( + pullRequest: pullRequest( + state: "OPEN", + checks: [GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: "SUCCESS")] + ), + secondsSinceAnswer: 179, + isDue: false + ), + ] + ) + func dueByPullRequestState(_ testCase: Case) { + #expect( + PullRequestRefreshCadence.isDue( + pullRequest: testCase.pullRequest, + lastCheckedAt: Self.checkedAt, + now: Self.checkedAt.addingTimeInterval(TimeInterval(testCase.secondsSinceAnswer)), + isSelected: false + ) == testCase.isDue + ) + } + + @Test func selectedWorktreeIsAlwaysDue() { + #expect( + PullRequestRefreshCadence.isDue( + pullRequest: pullRequest(state: "MERGED"), + lastCheckedAt: Self.checkedAt, + now: Self.checkedAt.addingTimeInterval(1), + isSelected: true + ) + ) + } + + @Test func neverAnsweredWorktreeIsDue() { + #expect( + PullRequestRefreshCadence.isDue( + pullRequest: pullRequest(state: "MERGED"), + lastCheckedAt: nil, + now: Self.checkedAt, + isSelected: false + ) + ) + } +} + +nonisolated private func pullRequest( + state: String, + checks: [GithubPullRequestStatusCheck] = [], + mergeable: String? = "MERGEABLE", + mergeStateStatus: String? = "CLEAN", + queued: Bool = false +) -> GithubPullRequest { + GithubPullRequest( + number: 7, + title: "PR", + state: state, + additions: 0, + deletions: 0, + isDraft: false, + reviewDecision: nil, + mergeable: mergeable, + mergeStateStatus: mergeStateStatus, + updatedAt: nil, + url: "https://example.com/pull/7", + headRefName: "feature", + baseRefName: "main", + commitsCount: 1, + authorLogin: "khoi", + statusCheckRollup: checks.isEmpty ? nil : GithubPullRequestStatusCheckRollup(checks: checks), + mergeQueueEntry: queued ? GithubMergeQueueEntry(position: 1, estimatedTimeToMerge: nil, state: "QUEUED") : nil + ) +} diff --git a/App/Tests/RepositoriesFeatureTests.swift b/App/Tests/RepositoriesFeatureTests.swift index ce258367..80c9f231 100644 --- a/App/Tests/RepositoriesFeatureTests.swift +++ b/App/Tests/RepositoriesFeatureTests.swift @@ -502,6 +502,39 @@ struct RepositoriesFeatureTests { } } + @Test func repositoryRemoteConfigurationChangedForgetsWhenBranchesWereAnswered() async { + let repoRoot = "/tmp/repo" + let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) + let featureWorktree = makeWorktree(id: "\(repoRoot)/feature", name: "feature", repoRoot: repoRoot) + let repository = makeRepository(id: repoRoot, worktrees: [mainWorktree, featureWorktree]) + var initialState = makeState(repositories: [repository]) + initialState.githubIntegrationAvailability = .unavailable + initialState.codeHostByRepositoryID[repository.id] = .github + let answeredAt = Date(timeIntervalSince1970: 1_000_000) + initialState.pullRequestCheckedAtByWorktreeID = [mainWorktree.id: answeredAt, featureWorktree.id: answeredAt] + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.repositoryWebURL = { _ in nil } + } + store.exhaustivity = .off + + // A changed remote can point every branch at other pull requests, so all of them are due. + await store.send( + .worktreeInfoEvent( + .repositoryRemoteConfigurationChanged(repositoryRootURL: repository.rootURL) + ) + ) { + $0.pullRequestCheckedAtByWorktreeID = [:] + } + await store.receive(\.githubIntegration.repositoryPullRequestRefreshRequested) { + $0.pendingPullRequestRefreshByRepositoryID[repository.id] = RepositoriesFeature.PendingPullRequestRefresh( + repositoryRootURL: repository.rootURL, + worktreeIDs: [mainWorktree.id, featureWorktree.id] + ) + } + } + @Test func repositoriesLoadedEmitsChangedDelegateWhenTransitioningFromRestoring() async { let worktree = makeWorktree(id: "/tmp/repo/main", name: "main") let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) diff --git a/docs-ai/028-pr-status-tracking/000-plan.md b/docs-ai/028-pr-status-tracking/000-plan.md index 49a46b47..8c856eb3 100644 --- a/docs-ai/028-pr-status-tracking/000-plan.md +++ b/docs-ai/028-pr-status-tracking/000-plan.md @@ -99,3 +99,6 @@ As of the anchor wave (reconstructed from #256/#305/#366/#379 PR descriptions): [003-status-fidelity-and-refresh-cadence.md](003-status-fidelity-and-refresh-cadence.md) - Updated 2026-07-08: flicker elimination and explicit no-PR semantics (#533 → #538 → #539) — see [004-flicker-and-no-pr-semantics.md](004-flicker-and-no-pr-semantics.md) +- Updated 2026-10-02: periodic refreshes ask only about pull requests whose state can + change soon; settled, absent, and finished ones follow slower intervals — see + [008-cadence-by-state.md](008-cadence-by-state.md) diff --git a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md new file mode 100644 index 00000000..fa31801d --- /dev/null +++ b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md @@ -0,0 +1,50 @@ +# 028.008 — Refresh Cadence by Pull Request State + +## Context + +PR polling tripped GitHub's GraphQL secondary rate limit on a shared account twice +(2026-09-29 and 2026-09-30). Every periodic refresh asked about every branch of the +repository, so the batched query grew with the number of worktrees even when nearly all +of their pull requests were merged or settled and could not have changed. + +The incident report suggested skipping branches whose head SHA had not changed. That +would freeze check results and review state, which change without a push, so the +cadence here follows the pull request's state instead. + +## Change + +`PullRequestRefreshCadence.isDue` decides, per worktree, whether a periodic refresh +asks GitHub about it: + +| Worktree | Asked | +| --- | --- | +| Selected | every refresh | +| Never answered | every refresh | +| Open, checks in progress or expected, mergeability `UNKNOWN`, or in the merge queue | every refresh | +| Open and settled | every 180 s | +| No pull request | every 300 s | +| Merged or closed | every 1,800 s | + +`RepositoriesFeature.State.pullRequestCheckedAtByWorktreeID` records when GitHub last +answered for each worktree. A `.refreshed` outcome records the time only for worktrees +whose branch is in `prsByBranch` or `confirmedNoPrBranches`, so a branch left unknown by a +partial failure stays due. `repositoryPullRequestRefreshRequested` filters the requested +worktrees through `isDue` and sends no query when none is due; it reads the date only +when some worktree has a recorded time. A remote configuration change clears the +repository's recorded times. + +The HEAD watcher sees branch switches, not pushes or new commits, so a pull request +opened outside Prowl appears within the no-pull-request interval, or immediately when +its worktree is selected. + +`isActive` reads checks from `statusCheckRollup.checks`. Combined with the check-count +change (amendment 007, a separate change), it reads `GithubPullRequest.checkBreakdown` +instead, since background refreshes then carry counts without a list. + +## Refs + +Tests: `App/Tests/PullRequestRefreshCadenceTests.swift` (the interval for each state, +the selected and never-answered cases), `BatchedPullRequestRefreshReducerTests.swift` +(only due worktrees are asked; nothing due sends nothing; the selected worktree is always +asked; only answered branches record a time), `RepositoriesFeatureTests.swift` (a remote +change clears recorded times). diff --git a/docs/components/github-pull-requests.md b/docs/components/github-pull-requests.md index fdf0759f..1484c94e 100644 --- a/docs/components/github-pull-requests.md +++ b/docs/components/github-pull-requests.md @@ -84,6 +84,22 @@ that repository, then switches the host back to the previously active account. This uses `gh`'s stored authentication state; Prowl still never reads or stores GitHub tokens. +## Which pull requests a refresh asks about + +Background refreshes skip pull requests that are unlikely to have changed, so +each query to the shared `gh` account stays small: + +- The selected worktree is always refreshed. +- An open PR with checks still running, mergeability still being computed, or a + place in the merge queue is refreshed every time. +- A settled open PR is refreshed every 3 minutes, a branch without a PR every 5 + minutes, and a merged or closed PR every 30 minutes. +- A new worktree is refreshed right away, and so is every worktree of a + repository whose remotes change. + +A PR opened outside Prowl therefore appears within 5 minutes, or at once when you +select its worktree. + ## Gotchas for agents - No `gh` / not authenticated → no PR features. If a human expects PR actions and -- 2.51.2 From fb3fdcd8e028686e55cdd5b1eb241dceeabfb44f Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 2 Oct 2026 02:13:19 +0200 Subject: [PATCH 2/7] Never let an older answer or a cadence hold back a needed refresh Three cases let the cadence skip a refresh that had to happen. An answer from the previous remote, still in flight when the remote changed, recorded a fresh time; a refresh after a pull request action waited for the cadence; and a worktree created again at a removed one's path inherited its recorded time. Worktrees changed by an action, and every worktree of a repository whose remote changed, are now marked for the next refresh, and a mark clears only when a refresh that asks about them is sent. Removing a worktree forgets its refresh history. --- .../RepositoriesFeature+CoreReducer.swift | 4 +- ...epositoriesFeature+GithubIntegration.swift | 8 ++- ...epositoriesFeature+RepositoryLoading.swift | 1 + ...epositoriesFeature+WorktreeLifecycle.swift | 1 + .../RepositoriesFeature+WorktreeState.swift | 15 +++++ .../Reducer/RepositoriesFeature.swift | 5 ++ ...atchedPullRequestRefreshReducerTests.swift | 56 +++++++++++++++++++ App/Tests/RepositoriesFeatureTests.swift | 33 +++++++++-- .../008-cadence-by-state.md | 19 +++++-- 9 files changed, 128 insertions(+), 14 deletions(-) diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift index 1f38c80a..3f9947c1 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift @@ -1010,9 +1010,7 @@ extension RepositoriesFeature { var effects: [Effect] = [] let worktreeIDs = repository.worktrees.map(\.id) // A changed remote can point every branch at different pull requests. - for worktreeID in worktreeIDs { - state.pullRequestCheckedAtByWorktreeID.removeValue(forKey: worktreeID) - } + state.pullRequestRefreshForcedWorktreeIDs.formUnion(worktreeIDs) if repository.capabilities.supportsPullRequests, !worktreeIDs.isEmpty { effects.append( .send( diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 65c4862f..9fb262e9 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -72,6 +72,8 @@ extension RepositoriesFeature { else { return .none } + // An action just changed this pull request; its refresh must not wait for the cadence. + state.pullRequestRefreshForcedWorktreeIDs.insert(worktreeID) let repositoryRootURL = worktree.repositoryRootURL let worktreeIDs = repository.worktrees.map(\.id) return .run { send in @@ -94,9 +96,10 @@ extension RepositoriesFeature { } let selectedWorktreeID = state.selectedWorktreeID let checkedAt = state.pullRequestCheckedAtByWorktreeID - let currentDate = worktreeIDs.contains { checkedAt[$0] != nil } ? now : nil + let forced = state.pullRequestRefreshForcedWorktreeIDs + let currentDate = worktreeIDs.contains { checkedAt[$0] != nil && !forced.contains($0) } ? now : nil let worktrees = worktreeIDs.compactMap { state.worktree(for: $0) }.filter { worktree in - guard let currentDate else { + guard let currentDate, !forced.contains(worktree.id) else { return true } return PullRequestRefreshCadence.isDue( @@ -132,6 +135,7 @@ extension RepositoriesFeature { return .none } state.inFlightPullRequestRefreshRepositoryIDs.insert(repositoryID) + state.pullRequestRefreshForcedWorktreeIDs.subtract(dueWorktreeIDs) return enqueueBatchedPullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+RepositoryLoading.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+RepositoryLoading.swift index 03a9edef..e11ce450 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+RepositoryLoading.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+RepositoryLoading.swift @@ -281,6 +281,7 @@ extension RepositoriesFeature { let filteredWorktreeInfo = state.worktreeInfoByID.filter { availableWorktreeIDs.contains($0.key) } + state.forgetPullRequestRefreshHistory(keeping: availableWorktreeIDs) state.$prowlCreatedWorktreeIDs.withLock { $0.removeAll { !availableWorktreeIDs.contains($0) } } diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift index 7a02c1aa..dcc15082 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift @@ -440,6 +440,7 @@ extension RepositoriesFeature { state.pendingTerminalFocusWorktreeIDs.remove(worktreeID) state.archiveScriptProgressByWorktreeID.removeValue(forKey: worktreeID) state.worktreeInfoByID.removeValue(forKey: worktreeID) + state.forgetPullRequestRefreshHistory(of: worktreeID) state.pinnedWorktreeIDs.removeAll { $0 == worktreeID } state.archivedWorktrees.removeAll { $0.id == worktreeID } state.$prowlCreatedWorktreeIDs.withLock { diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift index e6858987..87ec464c 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift @@ -150,6 +150,7 @@ func cleanupWorktreeState( state.archiveScriptProgressByWorktreeID.removeValue(forKey: worktreeID) state.deletingWorktreeIDs.remove(worktreeID) state.worktreeInfoByID.removeValue(forKey: worktreeID) + state.forgetPullRequestRefreshHistory(of: worktreeID) let didUpdatePinned = state.pinnedWorktreeIDs.contains(worktreeID) if didUpdatePinned { state.pinnedWorktreeIDs.removeAll { $0 == worktreeID } @@ -307,3 +308,17 @@ nonisolated func normalizedLineChanges(_ entry: WorktreeInfoEntry?) -> GitLineCh nonisolated func normalizedLineChanges(_ changes: GitLineChanges) -> GitLineChanges? { changes.isEmpty ? nil : changes } + +extension RepositoriesFeature.State { + // A worktree ID is its path, so a worktree created again at a removed one's path must start + // without the removed worktree's refresh history. + mutating func forgetPullRequestRefreshHistory(of worktreeID: Worktree.ID) { + pullRequestCheckedAtByWorktreeID.removeValue(forKey: worktreeID) + pullRequestRefreshForcedWorktreeIDs.remove(worktreeID) + } + + mutating func forgetPullRequestRefreshHistory(keeping worktreeIDs: Set) { + pullRequestCheckedAtByWorktreeID = pullRequestCheckedAtByWorktreeID.filter { worktreeIDs.contains($0.key) } + pullRequestRefreshForcedWorktreeIDs.formIntersection(worktreeIDs) + } +} diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift index 3331bdf3..02330c10 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -351,6 +351,11 @@ struct RepositoriesFeature { /// When GitHub last answered a pull request refresh for each worktree; a periodic refresh skips /// a worktree whose pull request has settled until its cadence comes round. var pullRequestCheckedAtByWorktreeID: [Worktree.ID: Date] = [:] + /// Worktrees the next refresh must ask about whatever their cadence says: after a pull + /// request action, or a remote change that can point a branch at other pull requests. A mark + /// clears only when a refresh that asks about the worktree is sent, so an answer to an older + /// request cannot undo it. + var pullRequestRefreshForcedWorktreeIDs: Set = [] var pendingPullRequestRefreshByRepositoryID: [Repository.ID: PendingPullRequestRefresh] = [:] var inFlightPullRequestRefreshRepositoryIDs: Set = [] var prRefreshBatchCountsByRepositoryID: [Repository.ID: Int] = [:] diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index 07fda5fd..3e9c97e7 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -163,6 +163,62 @@ struct BatchedPullRequestRefreshReducerTests { #expect(enqueued.value.map(\.branches) == [["feature"]]) } + @Test func markedWorktreeIsAskedDespiteARecentAnswerAndThenUnmarked() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + var merged = WorktreeInfoEntry() + merged.pullRequest = makePullRequestFixture(state: "MERGED") + initialState.worktreeInfoByID[context.featureWorktree.id] = merged + // An answer from the old remote, or before an action, recorded a fresh time. + initialState.pullRequestCheckedAtByWorktreeID = [ + context.featureWorktree.id: now.addingTimeInterval(-10), + context.mainWorktree.id: now.addingTimeInterval(-10), + ] + initialState.pullRequestRefreshForcedWorktreeIDs = [context.featureWorktree.id] + let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo] } + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { request in enqueued.withValue { $0.append(request) } }, + cancelHost: { _ in }, + reset: {} + ) + } + store.exhaustivity = .off + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshRequested) { + $0.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + $0.pullRequestRefreshForcedWorktreeIDs = [] + } + await store.finish() + + #expect(enqueued.value.map(\.branches) == [["feature"]]) + } + + @Test func pullRequestActionMarksItsWorktreeForTheNextRefresh() async { + let context = makeContext() + let store = TestStore(initialState: context.state) { + RepositoriesFeature() + } + store.exhaustivity = .off + + await store.send(.githubIntegration(.delayedPullRequestRefresh(context.featureWorktree.id))) { + $0.pullRequestRefreshForcedWorktreeIDs = [context.featureWorktree.id] + } + // The action's delayed refresh runs on the wall clock; under load it may already be done. + await store.skipInFlightEffects(strict: false) + } + @Test func answeredRefreshRecordsWhenEachBranchWasAnswered() async { let context = makeContext() let now = Date(timeIntervalSince1970: 1_000_000) diff --git a/App/Tests/RepositoriesFeatureTests.swift b/App/Tests/RepositoriesFeatureTests.swift index 80c9f231..050106f5 100644 --- a/App/Tests/RepositoriesFeatureTests.swift +++ b/App/Tests/RepositoriesFeatureTests.swift @@ -490,7 +490,9 @@ struct RepositoriesFeatureTests { .worktreeInfoEvent( .repositoryRemoteConfigurationChanged(repositoryRootURL: repository.rootURL) ) - ) + ) { + $0.pullRequestRefreshForcedWorktreeIDs = [mainWorktree.id, featureWorktree.id] + } await store.receive(\.githubIntegration.repositoryPullRequestRefreshRequested) { $0.pendingPullRequestRefreshByRepositoryID[repository.id] = RepositoriesFeature.PendingPullRequestRefresh( repositoryRootURL: repository.rootURL, @@ -502,7 +504,7 @@ struct RepositoriesFeatureTests { } } - @Test func repositoryRemoteConfigurationChangedForgetsWhenBranchesWereAnswered() async { + @Test func repositoryRemoteConfigurationChangedMarksEveryBranchDue() async { let repoRoot = "/tmp/repo" let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) let featureWorktree = makeWorktree(id: "\(repoRoot)/feature", name: "feature", repoRoot: repoRoot) @@ -519,13 +521,14 @@ struct RepositoriesFeatureTests { } store.exhaustivity = .off - // A changed remote can point every branch at other pull requests, so all of them are due. + // A changed remote can point every branch at other pull requests, so all of them are due, + // whatever an answer from the old remote records meanwhile. await store.send( .worktreeInfoEvent( .repositoryRemoteConfigurationChanged(repositoryRootURL: repository.rootURL) ) ) { - $0.pullRequestCheckedAtByWorktreeID = [:] + $0.pullRequestRefreshForcedWorktreeIDs = [mainWorktree.id, featureWorktree.id] } await store.receive(\.githubIntegration.repositoryPullRequestRefreshRequested) { $0.pendingPullRequestRefreshByRepositoryID[repository.id] = RepositoriesFeature.PendingPullRequestRefresh( @@ -535,6 +538,28 @@ struct RepositoriesFeatureTests { } } + @Test func repositoriesLoadedForgetsRefreshHistoryOfRemovedWorktrees() async { + let worktree = makeWorktree(id: "/tmp/repo/main", name: "main") + let removed = makeWorktree(id: "/tmp/repo/removed", name: "removed") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var initialState = makeState(repositories: [repository]) + let answeredAt = Date(timeIntervalSince1970: 1_000_000) + initialState.pullRequestCheckedAtByWorktreeID = [worktree.id: answeredAt, removed.id: answeredAt] + initialState.pullRequestRefreshForcedWorktreeIDs = [removed.id] + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } + store.exhaustivity = .off + + await store.send( + .repositoriesLoaded([repository], failures: [], roots: [repository.rootURL], animated: false) + ) { + $0.pullRequestCheckedAtByWorktreeID = [worktree.id: answeredAt] + $0.pullRequestRefreshForcedWorktreeIDs = [] + } + } + @Test func repositoriesLoadedEmitsChangedDelegateWhenTransitioningFromRestoring() async { let worktree = makeWorktree(id: "/tmp/repo/main", name: "main") let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) diff --git a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md index fa31801d..e4789969 100644 --- a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md +++ b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md @@ -19,7 +19,7 @@ asks GitHub about it: | Worktree | Asked | | --- | --- | | Selected | every refresh | -| Never answered | every refresh | +| Never answered, or marked after an action or remote change | every refresh | | Open, checks in progress or expected, mergeability `UNKNOWN`, or in the merge queue | every refresh | | Open and settled | every 180 s | | No pull request | every 300 s | @@ -30,8 +30,15 @@ answered for each worktree. A `.refreshed` outcome records the time only for wor whose branch is in `prsByBranch` or `confirmedNoPrBranches`, so a branch left unknown by a partial failure stays due. `repositoryPullRequestRefreshRequested` filters the requested worktrees through `isDue` and sends no query when none is due; it reads the date only -when some worktree has a recorded time. A remote configuration change clears the -repository's recorded times. +when some worktree has a recorded time. + +`pullRequestRefreshForcedWorktreeIDs` marks worktrees the next refresh must ask about +whatever their cadence says: the worktree a pull request action just changed +(`delayedPullRequestRefresh`), and every worktree of a repository whose remote +configuration changed. A mark clears only when a refresh that asks about the worktree is +sent, so an answer to an older request, such as one to the previous remote still in +flight, cannot undo it. A worktree ID is its path, so removing a worktree forgets its +recorded time and mark; a worktree created again at that path starts as never answered. The HEAD watcher sees branch switches, not pushes or new commits, so a pull request opened outside Prowl appears within the no-pull-request interval, or immediately when @@ -46,5 +53,7 @@ instead, since background refreshes then carry counts without a list. Tests: `App/Tests/PullRequestRefreshCadenceTests.swift` (the interval for each state, the selected and never-answered cases), `BatchedPullRequestRefreshReducerTests.swift` (only due worktrees are asked; nothing due sends nothing; the selected worktree is always -asked; only answered branches record a time), `RepositoriesFeatureTests.swift` (a remote -change clears recorded times). +asked; only answered branches record a time; a marked worktree is asked despite a recent +answer and then unmarked; a pull request action marks its worktree), +`RepositoriesFeatureTests.swift` (a remote change marks every branch; loading forgets the +history of removed worktrees). -- 2.51.2 From fedf5a6241b81213e3a74cc3a9953da4159e2ed4 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 2 Oct 2026 02:58:48 +0200 Subject: [PATCH 3/7] Keep a refresh mark until GitHub answers for its branch A mark cleared when its refresh was sent, so a refresh that never reached GitHub, for want of a remote or because the query failed, lost it while the old recorded time stayed, and the worktree waited for its cadence. Sending a refresh now moves its marks aside; GitHub's answer for a branch drops that branch's mark, and whatever is left when the refresh completes is marked again. --- ...epositoriesFeature+GithubIntegration.swift | 11 ++- .../RepositoriesFeature+WorktreeState.swift | 21 ++++++ .../Reducer/RepositoriesFeature.swift | 3 + ...atchedPullRequestRefreshReducerTests.swift | 70 +++++++++++++++++++ .../008-cadence-by-state.md | 11 +-- 5 files changed, 111 insertions(+), 5 deletions(-) diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 9fb262e9..51393875 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -135,7 +135,11 @@ extension RepositoriesFeature { return .none } state.inFlightPullRequestRefreshRepositoryIDs.insert(repositoryID) - state.pullRequestRefreshForcedWorktreeIDs.subtract(dueWorktreeIDs) + let sentMarks = state.pullRequestRefreshForcedWorktreeIDs.intersection(dueWorktreeIDs) + state.pullRequestRefreshForcedWorktreeIDs.subtract(sentMarks) + if !sentMarks.isEmpty { + state.sentPullRequestRefreshMarks[repositoryID, default: []].formUnion(sentMarks) + } return enqueueBatchedPullRequestRefresh( repositoryID: repositoryID, repositoryRootURL: repositoryRootURL, @@ -200,6 +204,7 @@ extension RepositoriesFeature { } state.queuedPullRequestRefreshByRepositoryID.removeAll() state.inFlightPullRequestRefreshRepositoryIDs.removeAll() + state.restoreUnansweredPullRequestRefreshMarks() clearAllPullRequestRefreshTracking(state: &state) return .run { send in while !Task.isCancelled { @@ -234,6 +239,7 @@ extension RepositoriesFeature { ) case .repositoryPullRequestRefreshCompleted(let repositoryID): + state.restoreUnansweredPullRequestRefreshMarks(of: repositoryID) state.inFlightPullRequestRefreshRepositoryIDs.remove(repositoryID) clearPullRequestRefreshTracking(repositoryID: repositoryID, state: &state) guard state.githubIntegrationAvailability == .available, @@ -724,6 +730,7 @@ extension RepositoriesFeature { state.pendingPullRequestRefreshByRepositoryID.removeAll() state.queuedPullRequestRefreshByRepositoryID.removeAll() state.inFlightPullRequestRefreshRepositoryIDs.removeAll() + state.restoreUnansweredPullRequestRefreshMarks() clearAllPullRequestRefreshTracking(state: &state) return .merge( .cancel(id: CancelID.githubIntegrationRecovery), @@ -734,6 +741,7 @@ extension RepositoriesFeature { state.pendingPullRequestRefreshByRepositoryID.removeAll() state.queuedPullRequestRefreshByRepositoryID.removeAll() state.inFlightPullRequestRefreshRepositoryIDs.removeAll() + state.restoreUnansweredPullRequestRefreshMarks() clearAllPullRequestRefreshTracking(state: &state) let worktreeIDs = Array(state.worktreeInfoByID.keys) for worktreeID in worktreeIDs { @@ -773,6 +781,7 @@ extension RepositoriesFeature { continue } state.pullRequestCheckedAtByWorktreeID[worktreeID] = checkedAt + state.sentPullRequestRefreshMarks[repositoryID]?.remove(worktreeID) } } guard let repository = state.repositories[id: repositoryID] else { diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift index 87ec464c..fdc53de7 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift @@ -315,10 +315,31 @@ extension RepositoriesFeature.State { mutating func forgetPullRequestRefreshHistory(of worktreeID: Worktree.ID) { pullRequestCheckedAtByWorktreeID.removeValue(forKey: worktreeID) pullRequestRefreshForcedWorktreeIDs.remove(worktreeID) + for repositoryID in Array(sentPullRequestRefreshMarks.keys) { + sentPullRequestRefreshMarks[repositoryID]?.remove(worktreeID) + } + } + + /// Marks the refresh in flight for `repositoryID`, or for every repository, still carries go + /// back on the next refresh: GitHub did not answer for those branches. + mutating func restoreUnansweredPullRequestRefreshMarks(of repositoryID: Repository.ID? = nil) { + if let repositoryID { + if let unanswered = sentPullRequestRefreshMarks.removeValue(forKey: repositoryID) { + pullRequestRefreshForcedWorktreeIDs.formUnion(unanswered) + } + return + } + for unanswered in sentPullRequestRefreshMarks.values { + pullRequestRefreshForcedWorktreeIDs.formUnion(unanswered) + } + sentPullRequestRefreshMarks.removeAll() } mutating func forgetPullRequestRefreshHistory(keeping worktreeIDs: Set) { pullRequestCheckedAtByWorktreeID = pullRequestCheckedAtByWorktreeID.filter { worktreeIDs.contains($0.key) } pullRequestRefreshForcedWorktreeIDs.formIntersection(worktreeIDs) + sentPullRequestRefreshMarks = sentPullRequestRefreshMarks.mapValues { + $0.intersection(worktreeIDs) + } } } diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift index 02330c10..e8895876 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -356,6 +356,9 @@ struct RepositoriesFeature { /// clears only when a refresh that asks about the worktree is sent, so an answer to an older /// request cannot undo it. var pullRequestRefreshForcedWorktreeIDs: Set = [] + /// Marks a sent refresh carries until GitHub answers for their branches. Whatever is left when + /// the refresh completes, because it failed or never reached GitHub, is marked again. + var sentPullRequestRefreshMarks: [Repository.ID: Set] = [:] var pendingPullRequestRefreshByRepositoryID: [Repository.ID: PendingPullRequestRefresh] = [:] var inFlightPullRequestRefreshRepositoryIDs: Set = [] var prRefreshBatchCountsByRepositoryID: [Repository.ID: Int] = [:] diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index 3e9c97e7..99274cdd 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -199,12 +199,82 @@ struct BatchedPullRequestRefreshReducerTests { await store.receive(\.githubIntegration.repositoryPullRequestRefreshRequested) { $0.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] $0.pullRequestRefreshForcedWorktreeIDs = [] + $0.sentPullRequestRefreshMarks = [context.repository.id: [context.featureWorktree.id]] } await store.finish() #expect(enqueued.value.map(\.branches) == [["feature"]]) } + @Test func markSurvivesARefreshThatNeverReachesGitHub() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + initialState.pullRequestCheckedAtByWorktreeID = [context.featureWorktree.id: now.addingTimeInterval(-10)] + initialState.pullRequestRefreshForcedWorktreeIDs = [context.featureWorktree.id] + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + // No GitHub remote resolves, so the refresh completes without a query. + $0.gitClient.githubRemoteInfos = { _ in [] } + $0.githubCLI.resolveRemoteInfo = { _ in nil } + $0.pullRequestRefreshCoordinator = .unimplemented + } + store.exhaustivity = .off + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.finish() + await store.skipReceivedActions() + + #expect(store.state.pullRequestRefreshForcedWorktreeIDs == [context.featureWorktree.id]) + #expect(store.state.sentPullRequestRefreshMarks.isEmpty) + } + + @Test func answeredBranchDropsItsMarkAndAnUnansweredOneKeepsIt() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + initialState.sentPullRequestRefreshMarks = [ + context.repository.id: [context.featureWorktree.id, context.mainWorktree.id] + ] + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.pullRequestRefreshCoordinator = .unimplemented + } + store.exhaustivity = .off + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: ["feature": makePullRequestFixture()], + confirmedNoPrBranches: [] + ) + )) + ) + #expect( + store.state.sentPullRequestRefreshMarks == [context.repository.id: [context.mainWorktree.id]] + ) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) + await store.finish() + + #expect(store.state.pullRequestRefreshForcedWorktreeIDs == [context.mainWorktree.id]) + #expect(store.state.sentPullRequestRefreshMarks.isEmpty) + } + @Test func pullRequestActionMarksItsWorktreeForTheNextRefresh() async { let context = makeContext() let store = TestStore(initialState: context.state) { diff --git a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md index e4789969..1d08fac7 100644 --- a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md +++ b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md @@ -35,9 +35,11 @@ when some worktree has a recorded time. `pullRequestRefreshForcedWorktreeIDs` marks worktrees the next refresh must ask about whatever their cadence says: the worktree a pull request action just changed (`delayedPullRequestRefresh`), and every worktree of a repository whose remote -configuration changed. A mark clears only when a refresh that asks about the worktree is -sent, so an answer to an older request, such as one to the previous remote still in -flight, cannot undo it. A worktree ID is its path, so removing a worktree forgets its +configuration changed. Sending a refresh moves its marks into +`sentPullRequestRefreshMarks`; a mark drops when GitHub answers for its branch, and any +still there when the refresh completes, because it failed or never reached GitHub, is +marked again. An answer to an older request, such as one to the previous remote still in +flight, never carried the new marks and cannot clear them. A worktree ID is its path, so removing a worktree forgets its recorded time and mark; a worktree created again at that path starts as never answered. The HEAD watcher sees branch switches, not pushes or new commits, so a pull request @@ -54,6 +56,7 @@ Tests: `App/Tests/PullRequestRefreshCadenceTests.swift` (the interval for each s the selected and never-answered cases), `BatchedPullRequestRefreshReducerTests.swift` (only due worktrees are asked; nothing due sends nothing; the selected worktree is always asked; only answered branches record a time; a marked worktree is asked despite a recent -answer and then unmarked; a pull request action marks its worktree), +answer; a mark survives a refresh that never reaches GitHub; an answered branch drops its +mark and an unanswered one keeps it; a pull request action marks its worktree), `RepositoriesFeatureTests.swift` (a remote change marks every branch; loading forgets the history of removed worktrees). -- 2.51.2 From 7fe72dd073eba538a0522d17f5a64f1d73dd105a Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Fri, 2 Oct 2026 03:10:00 +0200 Subject: [PATCH 4/7] Settle a branch only when every host of its refresh answered When one host confirmed a branch had no pull request and another host failed, the first answer recorded the time and dropped the branch's mark, though the failed host left its status unknown. The time and the mark now settle when the last host batch comes back, and only when no batch failed, matching how confirmed absences are already suppressed. --- ...epositoriesFeature+GithubIntegration.swift | 43 +++++++++++----- ...atchedPullRequestRefreshReducerTests.swift | 49 +++++++++++++++++-- .../008-cadence-by-state.md | 9 ++-- 3 files changed, 79 insertions(+), 22 deletions(-) diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 51393875..3fa8139e 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -771,19 +771,6 @@ extension RepositoriesFeature { ) -> Effect { switch outcome { case .refreshed(let repositoryID, _, let worktreeIDs, let prsByBranch, let confirmedNoPrBranches): - // Only a branch GitHub answered for, with a pull request or a confirmed absence of one, has - // known state; a branch left unknown by a partial failure stays due. - let answeredBranches = Set(prsByBranch.keys).union(confirmedNoPrBranches) - if !answeredBranches.isEmpty { - let checkedAt = now - for worktreeID in worktreeIDs { - guard let name = state.worktree(for: worktreeID)?.name, answeredBranches.contains(name) else { - continue - } - state.pullRequestCheckedAtByWorktreeID[worktreeID] = checkedAt - state.sentPullRequestRefreshMarks[repositoryID]?.remove(worktreeID) - } - } guard let repository = state.repositories[id: repositoryID] else { state.inFlightPullRequestRefreshRepositoryIDs.remove(repositoryID) clearPullRequestRefreshTracking(repositoryID: repositoryID, state: &state) @@ -811,6 +798,17 @@ extension RepositoriesFeature { // it arrived before this final refreshed outcome — suppress confirmed clears. let confirmedNoPrBranches = hadFailedBatch ? [] : accumulatedConfirmedNoPrBranches state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) + // A branch's status is settled only when every host batch came back and none failed: a found + // pull request or a confirmed absence then records when GitHub answered and drops its mark. + // Otherwise the branch stays due, and a sent mark goes back on completion. + if !hadFailedBatch { + recordAnsweredPullRequestRefresh( + repositoryID: repositoryID, + worktreeIDs: worktreeIDs, + answeredBranches: Set(mergedPRsByBranch.keys).union(confirmedNoPrBranches), + state: &state + ) + } let prsByWorktreeID = pullRequestsByWorktreeID( repository: repository, worktreeIDs: worktreeIDs, @@ -926,6 +924,25 @@ extension RepositoriesFeature { state.prRefreshResultPrioritiesByRepositoryID.removeAll() } + private func recordAnsweredPullRequestRefresh( + repositoryID: Repository.ID, + worktreeIDs: [Worktree.ID], + answeredBranches: Set, + state: inout State + ) { + guard !answeredBranches.isEmpty else { + return + } + let checkedAt = now + for worktreeID in worktreeIDs { + guard let name = state.worktree(for: worktreeID)?.name, answeredBranches.contains(name) else { + continue + } + state.pullRequestCheckedAtByWorktreeID[worktreeID] = checkedAt + state.sentPullRequestRefreshMarks[repositoryID]?.remove(worktreeID) + } + } + private func consumePullRequestRefreshBatch( repositoryID: Repository.ID, state: inout State diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index 99274cdd..bc98d0bf 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -275,6 +275,47 @@ struct BatchedPullRequestRefreshReducerTests { #expect(store.state.sentPullRequestRefreshMarks.isEmpty) } + @Test func markSurvivesWhenOneHostConfirmsAndAnotherFails() async { + let context = makeContext() + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + initialState.prRefreshBatchCountsByRepositoryID[context.repository.id] = 2 + initialState.sentPullRequestRefreshMarks = [context.repository.id: [context.featureWorktree.id]] + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = refreshDate + $0.pullRequestRefreshCoordinator = .unimplemented + } + store.exhaustivity = .off + + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: [:], + confirmedNoPrBranches: ["feature"] + ) + )) + ) + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .failed(repositoryID: context.repository.id, worktreeIDs: context.worktreeIDs, message: "host down") + )) + ) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) + await store.finish() + + // The failed host leaves the branch's status unknown: no answer is recorded and the mark is back. + #expect(store.state.pullRequestCheckedAtByWorktreeID.isEmpty) + #expect(store.state.pullRequestRefreshForcedWorktreeIDs == [context.featureWorktree.id]) + } + @Test func pullRequestActionMarksItsWorktreeForTheNextRefresh() async { let context = makeContext() let store = TestStore(initialState: context.state) { @@ -374,7 +415,6 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = ["feature": githubPullRequest] $0.prRefreshNoPrBranchesByID[context.repository.id] = [] @@ -393,6 +433,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] $0.prRefreshNoPrBranchesByID = [:] @@ -471,7 +512,6 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = ["feature": enterprisePullRequest] $0.prRefreshNoPrBranchesByID[context.repository.id] = [] @@ -490,6 +530,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] $0.prRefreshNoPrBranchesByID = [:] @@ -765,7 +806,6 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshFailedBatchRepositoryIDs = [] } @@ -807,7 +847,6 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = [:] $0.prRefreshNoPrBranchesByID[context.repository.id] = ["feature"] @@ -863,7 +902,6 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID[context.repository.id] = 1 $0.prRefreshResultsByRepositoryID[context.repository.id] = [:] $0.prRefreshNoPrBranchesByID[context.repository.id] = ["feature"] @@ -882,6 +920,7 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { + $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] $0.prRefreshNoPrBranchesByID = [:] diff --git a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md index 1d08fac7..60af3d96 100644 --- a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md +++ b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md @@ -26,9 +26,9 @@ asks GitHub about it: | Merged or closed | every 1,800 s | `RepositoriesFeature.State.pullRequestCheckedAtByWorktreeID` records when GitHub last -answered for each worktree. A `.refreshed` outcome records the time only for worktrees -whose branch is in `prsByBranch` or `confirmedNoPrBranches`, so a branch left unknown by a -partial failure stays due. `repositoryPullRequestRefreshRequested` filters the requested +answered for each worktree. The time is recorded when the last host batch of a refresh +comes back and none failed, and only for worktrees whose branch has a pull request or a +confirmed absence of one, so a branch left unknown by a partial failure stays due. `repositoryPullRequestRefreshRequested` filters the requested worktrees through `isDue` and sends no query when none is due; it reads the date only when some worktree has a recorded time. @@ -56,7 +56,8 @@ Tests: `App/Tests/PullRequestRefreshCadenceTests.swift` (the interval for each s the selected and never-answered cases), `BatchedPullRequestRefreshReducerTests.swift` (only due worktrees are asked; nothing due sends nothing; the selected worktree is always asked; only answered branches record a time; a marked worktree is asked despite a recent -answer; a mark survives a refresh that never reaches GitHub; an answered branch drops its +answer; a mark survives a refresh that never reaches GitHub, and one where a host fails after +another confirmed no pull request; an answered branch drops its mark and an unanswered one keeps it; a pull request action marks its worktree), `RepositoriesFeatureTests.swift` (a remote change marks every branch; loading forgets the history of removed worktrees). -- 2.51.2 From 1cfce69ba0f4e6df8930282e81246dd20faec127 Mon Sep 17 00:00:00 2001 From: Simon Heimlicher Date: Sat, 3 Oct 2026 16:38:24 +0200 Subject: [PATCH 5/7] Record the refresh cadence in the PR status action log --- docs-ai/028-pr-status-tracking/001-action.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs-ai/028-pr-status-tracking/001-action.md b/docs-ai/028-pr-status-tracking/001-action.md index 3c2e2ab0..ba7131d1 100644 --- a/docs-ai/028-pr-status-tracking/001-action.md +++ b/docs-ai/028-pr-status-tracking/001-action.md @@ -42,6 +42,12 @@ Grouped by problem wave; rows chronological within each wave. | 2026-07-05 | Supersedes #533: fix the nil-literal no-op clear (`updateValue(nil, forKey:)`); order-independent cross-host suppression via `prRefreshFailedBatchRepositoryIDs`; the missing non-empty-set test coverage | PR #538 → [004](004-flicker-and-no-pr-semantics.md) | | 2026-07-08 | Preserve last-known `mergeable`/`mergeStateStatus` when GitHub returns transient `UNKNOWN` (kills the "Blocked" flash) | PR #539 → [004](004-flicker-and-no-pr-semantics.md) | +### Wave 5 — Rate limits & refresh load (October) + +| Date | Change | Ref | +| --- | --- | --- | +| 2026-10-02 | Refresh cadence by pull request state: active pull requests on every sweep, settled open ones every 3 min, branches without one every 5 min, merged or closed ones every 30 min; a pull request action or remote change forces the next refresh | [008](008-cadence-by-state.md) | + ## Outcome & current state (as of 2026-07-12) Client layer, `supacode/Clients/Github/`: -- 2.51.2 From 3f130f7eabc5b358af4f59ada7d2d4e24437d362 Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 9 Oct 2026 01:37:32 +0900 Subject: [PATCH 6/7] Key the refresh cadence on a per-branch checkpoint and treat partial answers as incomplete The cadence by pull request state kept one timestamp per worktree and read the pull request shown on screen. Three cases went wrong: - A worktree that switched branches inherited the previous branch's interval, so a merged branch's 30 minutes delayed the new branch. - The display keeps the previous mergeability while GitHub reports UNKNOWN, so a pull request GitHub was still computing looked settled. - A host whose repositories answered only in part still produced a complete `.refreshed` outcome, so the reducer recorded the time and dropped the forced mark. `PullRequestRefreshCadence.Checkpoint` now records the branch the answer was for, when it arrived, and the interval read from the raw answer; a worktree on another branch is due at once. `Outcome.refreshed` carries `isPartial`, and the reducer treats a partial host like a failed one: its pull requests show, nothing is confirmed absent, no checkpoint is recorded, and the sent mark goes back on completion. --- .../PullRequestRefreshCadence.swift | 28 ++- .../PullRequestRefreshCoordinator.swift | 8 +- ...epositoriesFeature+GithubIntegration.swift | 50 +++-- .../RepositoriesFeature+WorktreeState.swift | 6 +- .../Reducer/RepositoriesFeature.swift | 7 +- ...atchedPullRequestRefreshReducerTests.swift | 207 ++++++++++++++++-- .../PullRequestRefreshCadenceTests.swift | 43 +++- .../PullRequestRefreshCoordinatorTests.swift | 51 +++-- App/Tests/RepositoriesFeatureTests.swift | 15 +- .../008-cadence-by-state.md | 47 ++-- docs/components/github-pull-requests.md | 5 +- 11 files changed, 364 insertions(+), 103 deletions(-) diff --git a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift index 87cb839b..2a0aaa86 100644 --- a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift +++ b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCadence.swift @@ -11,22 +11,40 @@ nonisolated enum PullRequestRefreshCadence { /// Merged or closed: only a reopened or new pull request for the same branch changes it. static let finishedInterval: Duration = .seconds(1_800) + /// What GitHub's last complete answer tells a periodic refresh about one worktree: the branch it + /// answered for, when, and how long the answer stays good. The interval comes from the answer as + /// GitHub gave it, not from the pull request on screen, which keeps the previous mergeability + /// while GitHub still computes the new one. + nonisolated struct Checkpoint: Equatable, Sendable { + let branch: String + let answeredAt: Date + /// nil means every sweep. + let interval: Duration? + + init(branch: String, pullRequest: GithubPullRequest?, answeredAt: Date) { + self.branch = branch + self.answeredAt = answeredAt + self.interval = PullRequestRefreshCadence.interval(for: pullRequest) + } + } + static func isDue( - pullRequest: GithubPullRequest?, - lastCheckedAt: Date?, + branch: String, + checkpoint: Checkpoint?, now: Date, isSelected: Bool ) -> Bool { if isSelected { return true } - guard let lastCheckedAt else { + // A checkpoint answers for the branch it was recorded for; a switched branch starts over. + guard let checkpoint, checkpoint.branch == branch else { return true } - guard let interval = interval(for: pullRequest) else { + guard let interval = checkpoint.interval else { return true } - return now.timeIntervalSince(lastCheckedAt) >= interval.seconds + return now.timeIntervalSince(checkpoint.answeredAt) >= interval.seconds } /// nil means every sweep. diff --git a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift index 2c1db9a1..b0cd38c7 100644 --- a/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift +++ b/App/Sources/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift @@ -66,12 +66,15 @@ final class PullRequestRefreshCoordinator { } nonisolated enum Outcome: Sendable, Equatable { + // isPartial: a candidate repository of the request did not answer, even after the fallback. The + // found pull requests are real, but nothing is confirmed absent and the answer is not complete. case refreshed( repositoryID: Repository.ID, repositoryRootURL: URL, worktreeIDs: [Worktree.ID], prsByBranch: [String: GithubPullRequest], - confirmedNoPrBranches: Set + confirmedNoPrBranches: Set, + isPartial: Bool = false ) case failed( repositoryID: Repository.ID, @@ -357,7 +360,8 @@ final class PullRequestRefreshCoordinator { repositoryRootURL: request.repositoryRootURL, worktreeIDs: request.worktreeIDs, prsByBranch: prsByBranch, - confirmedNoPrBranches: confirmedNoPrBranches + confirmedNoPrBranches: confirmedNoPrBranches, + isPartial: !allCandidatesSucceeded ) ) } diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 3fa8139e..78412597 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -95,16 +95,16 @@ extension RepositoriesFeature { return .none } let selectedWorktreeID = state.selectedWorktreeID - let checkedAt = state.pullRequestCheckedAtByWorktreeID + let checkpoints = state.pullRequestRefreshCheckpointByWorktreeID let forced = state.pullRequestRefreshForcedWorktreeIDs - let currentDate = worktreeIDs.contains { checkedAt[$0] != nil && !forced.contains($0) } ? now : nil + let currentDate = worktreeIDs.contains { checkpoints[$0] != nil && !forced.contains($0) } ? now : nil let worktrees = worktreeIDs.compactMap { state.worktree(for: $0) }.filter { worktree in guard let currentDate, !forced.contains(worktree.id) else { return true } return PullRequestRefreshCadence.isDue( - pullRequest: state.worktreeInfo(for: worktree.id)?.pullRequest, - lastCheckedAt: checkedAt[worktree.id], + branch: worktree.name, + checkpoint: checkpoints[worktree.id], now: currentDate, isSelected: worktree.id == selectedWorktreeID ) @@ -770,12 +770,18 @@ extension RepositoriesFeature { outcome: PullRequestRefreshCoordinator.Outcome ) -> Effect { switch outcome { - case .refreshed(let repositoryID, _, let worktreeIDs, let prsByBranch, let confirmedNoPrBranches): + case .refreshed(let repositoryID, _, let worktreeIDs, let prsByBranch, let confirmedNoPrBranches, let isPartial): guard let repository = state.repositories[id: repositoryID] else { state.inFlightPullRequestRefreshRepositoryIDs.remove(repositoryID) clearPullRequestRefreshTracking(repositoryID: repositoryID, state: &state) return .none } + // A host whose repositories did not all answer leaves the branch status on that host unknown, + // like a host whose batch failed: its found pull requests show, but nothing is confirmed absent + // and no checkpoint is recorded. + if isPartial { + state.prRefreshFailedBatchRepositoryIDs.insert(repositoryID) + } mergePullRequestRefreshResults( repositoryID: repositoryID, prsByBranch: prsByBranch, @@ -798,14 +804,15 @@ extension RepositoriesFeature { // it arrived before this final refreshed outcome — suppress confirmed clears. let confirmedNoPrBranches = hadFailedBatch ? [] : accumulatedConfirmedNoPrBranches state.prRefreshResultPrioritiesByRepositoryID.removeValue(forKey: repositoryID) - // A branch's status is settled only when every host batch came back and none failed: a found - // pull request or a confirmed absence then records when GitHub answered and drops its mark. + // A branch's status is settled only when every host batch came back complete: a found pull + // request or a confirmed absence then records a checkpoint from the answer and drops its mark. // Otherwise the branch stays due, and a sent mark goes back on completion. if !hadFailedBatch { recordAnsweredPullRequestRefresh( repositoryID: repositoryID, worktreeIDs: worktreeIDs, - answeredBranches: Set(mergedPRsByBranch.keys).union(confirmedNoPrBranches), + prsByBranch: mergedPRsByBranch, + confirmedNoPrBranches: confirmedNoPrBranches, state: &state ) } @@ -927,18 +934,29 @@ extension RepositoriesFeature { private func recordAnsweredPullRequestRefresh( repositoryID: Repository.ID, worktreeIDs: [Worktree.ID], - answeredBranches: Set, + prsByBranch: [String: GithubPullRequest], + confirmedNoPrBranches: Set, state: inout State ) { - guard !answeredBranches.isEmpty else { + let answered = worktreeIDs.compactMap { worktreeID -> (id: Worktree.ID, branch: String)? in + guard let branch = state.worktree(for: worktreeID)?.name, + prsByBranch[branch] != nil || confirmedNoPrBranches.contains(branch) + else { + return nil + } + return (worktreeID, branch) + } + guard !answered.isEmpty else { return } - let checkedAt = now - for worktreeID in worktreeIDs { - guard let name = state.worktree(for: worktreeID)?.name, answeredBranches.contains(name) else { - continue - } - state.pullRequestCheckedAtByWorktreeID[worktreeID] = checkedAt + // The checkpoint reads the answer as GitHub gave it; the display may keep an earlier mergeability. + let answeredAt = now + for (worktreeID, branch) in answered { + state.pullRequestRefreshCheckpointByWorktreeID[worktreeID] = PullRequestRefreshCadence.Checkpoint( + branch: branch, + pullRequest: prsByBranch[branch], + answeredAt: answeredAt + ) state.sentPullRequestRefreshMarks[repositoryID]?.remove(worktreeID) } } diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift index fdc53de7..3eafe151 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature+WorktreeState.swift @@ -313,7 +313,7 @@ extension RepositoriesFeature.State { // A worktree ID is its path, so a worktree created again at a removed one's path must start // without the removed worktree's refresh history. mutating func forgetPullRequestRefreshHistory(of worktreeID: Worktree.ID) { - pullRequestCheckedAtByWorktreeID.removeValue(forKey: worktreeID) + pullRequestRefreshCheckpointByWorktreeID.removeValue(forKey: worktreeID) pullRequestRefreshForcedWorktreeIDs.remove(worktreeID) for repositoryID in Array(sentPullRequestRefreshMarks.keys) { sentPullRequestRefreshMarks[repositoryID]?.remove(worktreeID) @@ -336,7 +336,9 @@ extension RepositoriesFeature.State { } mutating func forgetPullRequestRefreshHistory(keeping worktreeIDs: Set) { - pullRequestCheckedAtByWorktreeID = pullRequestCheckedAtByWorktreeID.filter { worktreeIDs.contains($0.key) } + pullRequestRefreshCheckpointByWorktreeID = pullRequestRefreshCheckpointByWorktreeID.filter { + worktreeIDs.contains($0.key) + } pullRequestRefreshForcedWorktreeIDs.formIntersection(worktreeIDs) sentPullRequestRefreshMarks = sentPullRequestRefreshMarks.mapValues { $0.intersection(worktreeIDs) diff --git a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift index e8895876..7acb3874 100644 --- a/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/App/Sources/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -348,9 +348,10 @@ struct RepositoriesFeature { var statusToast: StatusToast? var snapshotPersistencePhase: SnapshotPersistencePhase = .idle var githubIntegrationAvailability: GithubIntegrationAvailability = .unknown - /// When GitHub last answered a pull request refresh for each worktree; a periodic refresh skips - /// a worktree whose pull request has settled until its cadence comes round. - var pullRequestCheckedAtByWorktreeID: [Worktree.ID: Date] = [:] + /// GitHub's last complete answer for each worktree: the branch it was for, when it arrived, and + /// how long it stays good. A periodic refresh skips a worktree whose pull request has settled + /// until its cadence comes round, and asks at once about one that moved to another branch. + var pullRequestRefreshCheckpointByWorktreeID: [Worktree.ID: PullRequestRefreshCadence.Checkpoint] = [:] /// Worktrees the next refresh must ask about whatever their cadence says: after a pull /// request action, or a remote change that can point a branch at other pull requests. A mark /// clears only when a refresh that asks about the worktree is sent, so an answer to an older diff --git a/App/Tests/BatchedPullRequestRefreshReducerTests.swift b/App/Tests/BatchedPullRequestRefreshReducerTests.swift index bc98d0bf..c0350979 100644 --- a/App/Tests/BatchedPullRequestRefreshReducerTests.swift +++ b/App/Tests/BatchedPullRequestRefreshReducerTests.swift @@ -70,9 +70,11 @@ struct BatchedPullRequestRefreshReducerTests { merged.pullRequest = makePullRequestFixture(state: "MERGED") initialState.worktreeInfoByID[context.featureWorktree.id] = merged // The merged pull request was answered a minute ago; the branch without one ten minutes ago. - initialState.pullRequestCheckedAtByWorktreeID = [ - context.featureWorktree.id: now.addingTimeInterval(-60), - context.mainWorktree.id: now.addingTimeInterval(-600), + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint( + branch: "feature", pullRequest: merged.pullRequest, answeredAt: now.addingTimeInterval(-60) + ), + context.mainWorktree.id: makeCheckpoint(branch: "main", answeredAt: now.addingTimeInterval(-600)), ] let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) @@ -104,9 +106,9 @@ struct BatchedPullRequestRefreshReducerTests { let context = makeContext() let now = Date(timeIntervalSince1970: 1_000_000) var initialState = context.state - initialState.pullRequestCheckedAtByWorktreeID = [ - context.featureWorktree.id: now.addingTimeInterval(-60), - context.mainWorktree.id: now.addingTimeInterval(-60), + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint(branch: "feature", answeredAt: now.addingTimeInterval(-60)), + context.mainWorktree.id: makeCheckpoint(branch: "main", answeredAt: now.addingTimeInterval(-60)), ] let store = TestStore(initialState: initialState) { @@ -134,9 +136,9 @@ struct BatchedPullRequestRefreshReducerTests { let now = Date(timeIntervalSince1970: 1_000_000) var initialState = context.state initialState.selection = .worktree(context.featureWorktree.id) - initialState.pullRequestCheckedAtByWorktreeID = [ - context.featureWorktree.id: now.addingTimeInterval(-5), - context.mainWorktree.id: now.addingTimeInterval(-5), + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint(branch: "feature", answeredAt: now.addingTimeInterval(-5)), + context.mainWorktree.id: makeCheckpoint(branch: "main", answeredAt: now.addingTimeInterval(-5)), ] let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) @@ -171,9 +173,11 @@ struct BatchedPullRequestRefreshReducerTests { merged.pullRequest = makePullRequestFixture(state: "MERGED") initialState.worktreeInfoByID[context.featureWorktree.id] = merged // An answer from the old remote, or before an action, recorded a fresh time. - initialState.pullRequestCheckedAtByWorktreeID = [ - context.featureWorktree.id: now.addingTimeInterval(-10), - context.mainWorktree.id: now.addingTimeInterval(-10), + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint( + branch: "feature", pullRequest: merged.pullRequest, answeredAt: now.addingTimeInterval(-10) + ), + context.mainWorktree.id: makeCheckpoint(branch: "main", answeredAt: now.addingTimeInterval(-10)), ] initialState.pullRequestRefreshForcedWorktreeIDs = [context.featureWorktree.id] let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) @@ -210,7 +214,9 @@ struct BatchedPullRequestRefreshReducerTests { let context = makeContext() let now = Date(timeIntervalSince1970: 1_000_000) var initialState = context.state - initialState.pullRequestCheckedAtByWorktreeID = [context.featureWorktree.id: now.addingTimeInterval(-10)] + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint(branch: "feature", answeredAt: now.addingTimeInterval(-10)) + ] initialState.pullRequestRefreshForcedWorktreeIDs = [context.featureWorktree.id] let store = TestStore(initialState: initialState) { @@ -312,7 +318,7 @@ struct BatchedPullRequestRefreshReducerTests { await store.finish() // The failed host leaves the branch's status unknown: no answer is recorded and the mark is back. - #expect(store.state.pullRequestCheckedAtByWorktreeID.isEmpty) + #expect(store.state.pullRequestRefreshCheckpointByWorktreeID.isEmpty) #expect(store.state.pullRequestRefreshForcedWorktreeIDs == [context.featureWorktree.id]) } @@ -357,11 +363,146 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID = [context.featureWorktree.id: now] + $0.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint( + branch: "feature", pullRequest: makePullRequestFixture(), answeredAt: now + ) + ] } await store.finish() } + @Test func periodicRefreshAsksAboutASwitchedBranchAtOnce() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + var merged = WorktreeInfoEntry() + merged.pullRequest = makePullRequestFixture(state: "MERGED") + initialState.worktreeInfoByID[context.featureWorktree.id] = merged + // GitHub answered a minute ago for the merged branch this worktree was on; it is on "feature" now. + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + context.featureWorktree.id: makeCheckpoint( + branch: "merged-feature", pullRequest: merged.pullRequest, answeredAt: now.addingTimeInterval(-60) + ), + context.mainWorktree.id: makeCheckpoint(branch: "main", answeredAt: now.addingTimeInterval(-60)), + ] + let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo] } + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { request in enqueued.withValue { $0.append(request) } }, + cancelHost: { _ in }, + reset: {} + ) + } + store.exhaustivity = .off + + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.finish() + + #expect(enqueued.value.map(\.branches) == [["feature"]]) + } + + @Test func unknownMergeabilityKeepsThePullRequestActiveWhileTheDisplayKeepsTheKnownState() async { + let context = makeContext() + let now = Date(timeIntervalSince1970: 1_000_000) + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + var known = WorktreeInfoEntry() + known.pullRequest = makePullRequestFixture(mergeable: "MERGEABLE", mergeStateStatus: "CLEAN") + initialState.worktreeInfoByID[context.featureWorktree.id] = known + let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = now + $0.gitClient.githubRemoteInfos = { _ in [context.remoteInfo] } + $0.pullRequestRefreshCoordinator = PullRequestRefreshCoordinatorClient( + enqueue: { request in enqueued.withValue { $0.append(request) } }, + cancelHost: { _ in }, + reset: {} + ) + } + store.exhaustivity = .off + + // GitHub is computing mergeability again; the display keeps the known state meanwhile. + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: ["feature": makePullRequestFixture(mergeable: "UNKNOWN", mergeStateStatus: "UNKNOWN")], + confirmedNoPrBranches: ["main"] + ) + )) + ) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) + await store.finish() + + #expect(store.state.worktreeInfoByID[context.featureWorktree.id]?.pullRequest?.mergeable == "MERGEABLE") + #expect(store.state.pullRequestRefreshCheckpointByWorktreeID[context.featureWorktree.id]?.interval == nil) + + // A minute later only the pull request GitHub is still computing is asked about. + store.dependencies.date.now = now.addingTimeInterval(60) + await store.send( + .worktreeInfoEvent( + .repositoryPullRequestRefresh(repositoryRootURL: context.repoRootURL, worktreeIDs: context.worktreeIDs) + ) + ) + await store.finish() + + #expect(enqueued.value.map(\.branches) == [["feature"]]) + } + + @Test func partialAnswerShowsItsPullRequestsButRecordsNoCheckpointAndKeepsTheMark() async { + let context = makeContext() + var initialState = context.state + initialState.inFlightPullRequestRefreshRepositoryIDs = [context.repository.id] + initialState.sentPullRequestRefreshMarks = [context.repository.id: [context.featureWorktree.id]] + let merged = makePullRequestFixture(state: "MERGED") + + let store = TestStore(initialState: initialState) { + RepositoriesFeature() + } withDependencies: { + $0.date.now = refreshDate + $0.pullRequestRefreshCoordinator = .unimplemented + } + store.exhaustivity = .off + + // The preferred remote and its fallback failed; a secondary remote answered with a merged PR. + await store.send( + .githubIntegration( + .pullRequestRefreshBatchOutcome( + .refreshed( + repositoryID: context.repository.id, + repositoryRootURL: context.repoRootURL, + worktreeIDs: context.worktreeIDs, + prsByBranch: ["feature": merged], + confirmedNoPrBranches: [], + isPartial: true + ) + )) + ) + await store.receive(\.githubIntegration.repositoryPullRequestRefreshCompleted) + await store.finish() + + #expect(store.state.worktreeInfoByID[context.featureWorktree.id]?.pullRequest == merged) + #expect(store.state.pullRequestRefreshCheckpointByWorktreeID.isEmpty) + #expect(store.state.pullRequestRefreshForcedWorktreeIDs == [context.featureWorktree.id]) + #expect(store.state.sentPullRequestRefreshMarks.isEmpty) + } + @Test func refreshWaitsForAllHostBatchesBeforeCompleting() async { let context = makeContext() let enqueued = LockIsolated<[PullRequestRefreshCoordinator.Request]>([]) @@ -433,7 +574,9 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + $0.pullRequestRefreshCheckpointByWorktreeID[context.featureWorktree.id] = makeCheckpoint( + branch: "feature", pullRequest: githubPullRequest, answeredAt: refreshDate + ) $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] $0.prRefreshNoPrBranchesByID = [:] @@ -530,7 +673,9 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + $0.pullRequestRefreshCheckpointByWorktreeID[context.featureWorktree.id] = makeCheckpoint( + branch: "feature", pullRequest: originPullRequest, answeredAt: refreshDate + ) $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] $0.prRefreshNoPrBranchesByID = [:] @@ -657,7 +802,9 @@ struct BatchedPullRequestRefreshReducerTests { ) await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + $0.pullRequestRefreshCheckpointByWorktreeID[context.featureWorktree.id] = makeCheckpoint( + branch: "feature", pullRequest: pullRequest, answeredAt: refreshDate + ) } await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { var entry = WorktreeInfoEntry() @@ -719,7 +866,9 @@ struct BatchedPullRequestRefreshReducerTests { ) await store.send(.githubIntegration(.pullRequestRefreshBatchOutcome(outcome))) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + $0.pullRequestRefreshCheckpointByWorktreeID[context.featureWorktree.id] = makeCheckpoint( + branch: "feature", answeredAt: refreshDate + ) } await store.receive(\.githubIntegration.repositoryPullRequestsLoaded) { $0.worktreeInfoByID.removeValue(forKey: context.featureWorktree.id) @@ -920,7 +1069,9 @@ struct BatchedPullRequestRefreshReducerTests { ) )) ) { - $0.pullRequestCheckedAtByWorktreeID[context.featureWorktree.id] = refreshDate + $0.pullRequestRefreshCheckpointByWorktreeID[context.featureWorktree.id] = makeCheckpoint( + branch: "feature", pullRequest: pullRequest, answeredAt: refreshDate + ) $0.prRefreshBatchCountsByRepositoryID = [:] $0.prRefreshResultsByRepositoryID = [:] $0.prRefreshNoPrBranchesByID = [:] @@ -1025,10 +1176,20 @@ private struct RefreshTestContext { var worktreeIDs: [Worktree.ID] { [mainWorktree.id, featureWorktree.id] } } +nonisolated private func makeCheckpoint( + branch: String, + pullRequest: GithubPullRequest? = nil, + answeredAt: Date +) -> PullRequestRefreshCadence.Checkpoint { + PullRequestRefreshCadence.Checkpoint(branch: branch, pullRequest: pullRequest, answeredAt: answeredAt) +} + nonisolated private func makePullRequestFixture( title: String = "Coord PR", url: String = "https://example.com/coord-pr/7", - state: String = "OPEN" + state: String = "OPEN", + mergeable: String? = nil, + mergeStateStatus: String? = nil ) -> GithubPullRequest { GithubPullRequest( number: 7, @@ -1038,8 +1199,8 @@ nonisolated private func makePullRequestFixture( deletions: 0, isDraft: false, reviewDecision: nil, - mergeable: nil, - mergeStateStatus: nil, + mergeable: mergeable, + mergeStateStatus: mergeStateStatus, updatedAt: nil, url: url, headRefName: "feature", diff --git a/App/Tests/PullRequestRefreshCadenceTests.swift b/App/Tests/PullRequestRefreshCadenceTests.swift index 4813fc2d..ab6e9b9d 100644 --- a/App/Tests/PullRequestRefreshCadenceTests.swift +++ b/App/Tests/PullRequestRefreshCadenceTests.swift @@ -4,7 +4,7 @@ import Testing @testable import Prowl struct PullRequestRefreshCadenceTests { - nonisolated static let checkedAt = Date(timeIntervalSince1970: 1_000_000) + nonisolated static let answeredAt = Date(timeIntervalSince1970: 1_000_000) struct Case: Sendable, CustomTestStringConvertible { let pullRequest: GithubPullRequest? @@ -51,9 +51,9 @@ struct PullRequestRefreshCadenceTests { func dueByPullRequestState(_ testCase: Case) { #expect( PullRequestRefreshCadence.isDue( - pullRequest: testCase.pullRequest, - lastCheckedAt: Self.checkedAt, - now: Self.checkedAt.addingTimeInterval(TimeInterval(testCase.secondsSinceAnswer)), + branch: "feature", + checkpoint: checkpoint(for: testCase.pullRequest), + now: Self.answeredAt.addingTimeInterval(TimeInterval(testCase.secondsSinceAnswer)), isSelected: false ) == testCase.isDue ) @@ -62,24 +62,47 @@ struct PullRequestRefreshCadenceTests { @Test func selectedWorktreeIsAlwaysDue() { #expect( PullRequestRefreshCadence.isDue( - pullRequest: pullRequest(state: "MERGED"), - lastCheckedAt: Self.checkedAt, - now: Self.checkedAt.addingTimeInterval(1), + branch: "feature", + checkpoint: checkpoint(for: pullRequest(state: "MERGED")), + now: Self.answeredAt.addingTimeInterval(1), isSelected: true ) ) } @Test func neverAnsweredWorktreeIsDue() { + #expect( + PullRequestRefreshCadence.isDue(branch: "feature", checkpoint: nil, now: Self.answeredAt, isSelected: false) + ) + } + + @Test func switchedBranchIsDue() { + // The checkpoint answered for the merged branch; the worktree is on another branch now. #expect( PullRequestRefreshCadence.isDue( - pullRequest: pullRequest(state: "MERGED"), - lastCheckedAt: nil, - now: Self.checkedAt, + branch: "new-feature", + checkpoint: checkpoint(for: pullRequest(state: "MERGED")), + now: Self.answeredAt.addingTimeInterval(1), isSelected: false ) ) } + + @Test func checkpointReadsItsIntervalFromTheAnswer() { + let computing = pullRequest(state: "OPEN", mergeable: "UNKNOWN", mergeStateStatus: "UNKNOWN") + #expect(checkpoint(for: computing).interval == nil) + #expect(checkpoint(for: pullRequest(state: "OPEN")).interval == PullRequestRefreshCadence.settledOpenInterval) + #expect(checkpoint(for: nil).interval == PullRequestRefreshCadence.noPullRequestInterval) + #expect(checkpoint(for: pullRequest(state: "MERGED")).interval == PullRequestRefreshCadence.finishedInterval) + } +} + +nonisolated private func checkpoint(for pullRequest: GithubPullRequest?) -> PullRequestRefreshCadence.Checkpoint { + PullRequestRefreshCadence.Checkpoint( + branch: "feature", + pullRequest: pullRequest, + answeredAt: PullRequestRefreshCadenceTests.answeredAt + ) } nonisolated private func pullRequest( diff --git a/App/Tests/PullRequestRefreshCoordinatorTests.swift b/App/Tests/PullRequestRefreshCoordinatorTests.swift index 52d9babc..7fcf2d3e 100644 --- a/App/Tests/PullRequestRefreshCoordinatorTests.swift +++ b/App/Tests/PullRequestRefreshCoordinatorTests.swift @@ -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 @@ -336,7 +336,7 @@ 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 { + if case .refreshed("local", _, _, let prsByBranch, _, _) = outcome { return prsByBranch } return nil @@ -371,17 +371,12 @@ struct PullRequestRefreshCoordinatorTests { await advanceCoordinatorClock(clock, by: .milliseconds(250)) await waitUntil { await outcomes.refreshedRepositories().count == 1 } - 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 refreshed = await outcomes.snapshot().compactMap { RefreshedAnswer(outcome: $0, repositoryID: "local") } let result = try #require(refreshed.first) #expect(refreshed.count == 1) - #expect(result.0["feat-1"]?.title == "PR-upstream") - #expect(result.1 == ["feat-2"]) + #expect(result.prsByBranch["feat-1"]?.title == "PR-upstream") + #expect(result.confirmedNoPrBranches == ["feat-2"]) + #expect(!result.isPartial) } @Test func partialCandidateRepoFailureLeavesBranchesUnconfirmed() async throws { @@ -414,17 +409,13 @@ struct PullRequestRefreshCoordinatorTests { await advanceCoordinatorClock(clock, by: .milliseconds(250)) await waitUntil { await outcomes.refreshedRepositories().count == 1 } - 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 refreshed = await outcomes.snapshot().compactMap { RefreshedAnswer(outcome: $0, repositoryID: "local") } let result = try #require(refreshed.first) #expect(refreshed.count == 1) - #expect(result.0.isEmpty) - #expect(result.1.isEmpty) + #expect(result.prsByBranch.isEmpty) + #expect(result.confirmedNoPrBranches.isEmpty) + // The reducer must not record this answer as complete. + #expect(result.isPartial) } @Test func duplicateRepoKeysFallbackOnceAndFanOutToEachRepository() async throws { @@ -556,7 +547,7 @@ 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 { + if case .refreshed(let id, _, _, let prs, _, _) = snapshot { return (id, prs) } return nil @@ -772,6 +763,22 @@ actor CoordinatorProbe { } } +nonisolated private struct RefreshedAnswer: Sendable { + let prsByBranch: [String: GithubPullRequest] + let confirmedNoPrBranches: Set + let isPartial: Bool + + init?(outcome: PullRequestRefreshCoordinator.Outcome, repositoryID: Repository.ID) { + guard case .refreshed(repositoryID, _, _, let prsByBranch, let confirmedNoPrBranches, let isPartial) = outcome + else { + return nil + } + self.prsByBranch = prsByBranch + self.confirmedNoPrBranches = confirmedNoPrBranches + self.isPartial = isPartial + } +} + actor OutcomeCollector { private var outcomes: [PullRequestRefreshCoordinator.Outcome] = [] @@ -785,7 +792,7 @@ actor OutcomeCollector { func refreshedRepositories() -> [String] { outcomes.compactMap { - if case .refreshed(let id, _, _, _, _) = $0 { + if case .refreshed(let id, _, _, _, _, _) = $0 { return id } return nil diff --git a/App/Tests/RepositoriesFeatureTests.swift b/App/Tests/RepositoriesFeatureTests.swift index 050106f5..fc72e592 100644 --- a/App/Tests/RepositoriesFeatureTests.swift +++ b/App/Tests/RepositoriesFeatureTests.swift @@ -513,7 +513,12 @@ struct RepositoriesFeatureTests { initialState.githubIntegrationAvailability = .unavailable initialState.codeHostByRepositoryID[repository.id] = .github let answeredAt = Date(timeIntervalSince1970: 1_000_000) - initialState.pullRequestCheckedAtByWorktreeID = [mainWorktree.id: answeredAt, featureWorktree.id: answeredAt] + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + mainWorktree.id: PullRequestRefreshCadence.Checkpoint(branch: "main", pullRequest: nil, answeredAt: answeredAt), + featureWorktree.id: PullRequestRefreshCadence.Checkpoint( + branch: "feature", pullRequest: nil, answeredAt: answeredAt + ), + ] let store = TestStore(initialState: initialState) { RepositoriesFeature() } withDependencies: { @@ -544,7 +549,11 @@ struct RepositoriesFeatureTests { let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) var initialState = makeState(repositories: [repository]) let answeredAt = Date(timeIntervalSince1970: 1_000_000) - initialState.pullRequestCheckedAtByWorktreeID = [worktree.id: answeredAt, removed.id: answeredAt] + let kept = PullRequestRefreshCadence.Checkpoint(branch: "main", pullRequest: nil, answeredAt: answeredAt) + initialState.pullRequestRefreshCheckpointByWorktreeID = [ + worktree.id: kept, + removed.id: PullRequestRefreshCadence.Checkpoint(branch: "removed", pullRequest: nil, answeredAt: answeredAt), + ] initialState.pullRequestRefreshForcedWorktreeIDs = [removed.id] let store = TestStore(initialState: initialState) { @@ -555,7 +564,7 @@ struct RepositoriesFeatureTests { await store.send( .repositoriesLoaded([repository], failures: [], roots: [repository.rootURL], animated: false) ) { - $0.pullRequestCheckedAtByWorktreeID = [worktree.id: answeredAt] + $0.pullRequestRefreshCheckpointByWorktreeID = [worktree.id: kept] $0.pullRequestRefreshForcedWorktreeIDs = [] } } diff --git a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md index 60af3d96..c422ca61 100644 --- a/docs-ai/028-pr-status-tracking/008-cadence-by-state.md +++ b/docs-ai/028-pr-status-tracking/008-cadence-by-state.md @@ -19,18 +19,31 @@ asks GitHub about it: | Worktree | Asked | | --- | --- | | Selected | every refresh | -| Never answered, or marked after an action or remote change | every refresh | +| Never answered, on another branch than its checkpoint, or marked after an action or remote change | every refresh | | Open, checks in progress or expected, mergeability `UNKNOWN`, or in the merge queue | every refresh | | Open and settled | every 180 s | | No pull request | every 300 s | | Merged or closed | every 1,800 s | -`RepositoriesFeature.State.pullRequestCheckedAtByWorktreeID` records when GitHub last -answered for each worktree. The time is recorded when the last host batch of a refresh -comes back and none failed, and only for worktrees whose branch has a pull request or a -confirmed absence of one, so a branch left unknown by a partial failure stays due. `repositoryPullRequestRefreshRequested` filters the requested -worktrees through `isDue` and sends no query when none is due; it reads the date only -when some worktree has a recorded time. +`RepositoriesFeature.State.pullRequestRefreshCheckpointByWorktreeID` records GitHub's last +complete answer for each worktree as a `PullRequestRefreshCadence.Checkpoint`: the branch +the answer was for, when it arrived, and the interval the answer earns. A checkpoint is +recorded when the last host batch of a refresh comes back and every host answered in full, +and only for worktrees whose branch has a pull request or a confirmed absence of one. A +host whose batch failed, or whose repositories did not all answer even after the +per-repository fallback (`Outcome.refreshed(isPartial: true)`), leaves the branch status +on that host unknown: its found pull requests still show, but nothing is confirmed absent, +no checkpoint is recorded, and a sent mark goes back on completion, so the branch stays due. + +The checkpoint reads its interval from the answer as GitHub gave it, not from the pull +request on screen: `repositoryPullRequestsLoaded` keeps the previous mergeability while +GitHub reports `UNKNOWN` (amendment 004), which would otherwise turn a pull request GitHub +is still computing into a settled one for the cadence. A checkpoint answers only for its +branch, so a worktree whose HEAD moved to another branch, through the HEAD watcher's branch +update or a repository reload, is due on the next sweep instead of inheriting the previous +branch's interval. `repositoryPullRequestRefreshRequested` filters the requested worktrees +through `isDue` and sends no query when none is due; it reads the date only when some +worktree has a checkpoint. `pullRequestRefreshForcedWorktreeIDs` marks worktrees the next refresh must ask about whatever their cadence says: the worktree a pull request action just changed @@ -53,11 +66,15 @@ instead, since background refreshes then carry counts without a list. ## Refs Tests: `App/Tests/PullRequestRefreshCadenceTests.swift` (the interval for each state, -the selected and never-answered cases), `BatchedPullRequestRefreshReducerTests.swift` -(only due worktrees are asked; nothing due sends nothing; the selected worktree is always -asked; only answered branches record a time; a marked worktree is asked despite a recent -answer; a mark survives a refresh that never reaches GitHub, and one where a host fails after -another confirmed no pull request; an answered branch drops its -mark and an unanswered one keeps it; a pull request action marks its worktree), -`RepositoriesFeatureTests.swift` (a remote change marks every branch; loading forgets the -history of removed worktrees). +the selected, never-answered and switched-branch cases, the interval a checkpoint reads +from the answer), `BatchedPullRequestRefreshReducerTests.swift` (only due worktrees are +asked; nothing due sends nothing; the selected worktree is always asked; only answered +branches record a checkpoint; a switched branch is asked at once; an `UNKNOWN` answer keeps +the pull request on every sweep while the display keeps the known state; a partial host +answer shows its pull requests but records no checkpoint and keeps the mark; a marked +worktree is asked despite a recent answer; a mark survives a refresh that never reaches +GitHub, and one where a host fails after another confirmed no pull request; an answered +branch drops its mark and an unanswered one keeps it; a pull request action marks its +worktree), `PullRequestRefreshCoordinatorTests.swift` (a complete answer is not partial, a +candidate repository that failed makes it partial), `RepositoriesFeatureTests.swift` (a +remote change marks every branch; loading forgets the history of removed worktrees). diff --git a/docs/components/github-pull-requests.md b/docs/components/github-pull-requests.md index 1484c94e..e66c1262 100644 --- a/docs/components/github-pull-requests.md +++ b/docs/components/github-pull-requests.md @@ -94,8 +94,9 @@ each query to the shared `gh` account stays small: place in the merge queue is refreshed every time. - A settled open PR is refreshed every 3 minutes, a branch without a PR every 5 minutes, and a merged or closed PR every 30 minutes. -- A new worktree is refreshed right away, and so is every worktree of a - repository whose remotes change. +- A new worktree is refreshed right away, and so is a worktree that switched to + another branch and every worktree of a repository whose remotes change. +- A refresh that GitHub answered only in part is repeated on the next sweep. A PR opened outside Prowl therefore appears within 5 minutes, or at once when you select its worktree. -- 2.51.2 From bcab0b08b18154f941e08ea3814d3a724350ac85 Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 9 Oct 2026 09:28:06 +0900 Subject: [PATCH 7/7] Separate the refresh and rate-limit sections in the pull request docs --- docs/components/github-pull-requests.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/components/github-pull-requests.md b/docs/components/github-pull-requests.md index 55a6f3c8..a6090955 100644 --- a/docs/components/github-pull-requests.md +++ b/docs/components/github-pull-requests.md @@ -100,6 +100,7 @@ each query to the shared `gh` account stays small: A PR opened outside Prowl therefore appears within 5 minutes, or at once when you select its worktree. + ## Rate limits The `gh` account Prowl uses is usually shared with other tools and agents, and -- 2.51.2