Close GitHub refresh review gaps

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-05-12 17:49:11 -04:00
co-authored by Orca
parent 70857de542
commit 150f0ce45a
5 changed files with 118 additions and 18 deletions
@@ -125,4 +125,51 @@ describe('pr-refresh-coordinator', () => {
expect(inFlightEvents.map((event) => event.reason)).toEqual(['visible', 'active'])
expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2)
})
it('preserves coalesced aliases across visible follow-up refreshes', async () => {
const { reportVisiblePRRefreshCandidates } = await import('./pr-refresh-coordinator')
getPRForBranchOutcomeMock
.mockResolvedValueOnce({
kind: 'found',
pr: makePR({ checksStatus: 'pending' }),
fetchedAt: Date.now()
})
.mockResolvedValueOnce({
kind: 'found',
pr: makePR({ checksStatus: 'success' }),
fetchedAt: Date.now()
})
reportVisiblePRRefreshCandidates(
[
makeCandidate({
cacheKey: '/repo::feature/a',
branch: 'feature/a',
linkedPRNumber: 12,
worktreeId: 'wt-a'
}),
makeCandidate({
cacheKey: '/repo::feature/b',
branch: 'feature/b',
linkedPRNumber: 12,
worktreeId: 'wt-b'
})
],
1,
1
)
await vi.runOnlyPendingTimersAsync()
await vi.advanceTimersByTimeAsync(90_000)
const outcomeEvents = sendMock.mock.calls
.map(([, event]) => event)
.filter((event) => event.outcome)
expect(outcomeEvents).toHaveLength(2)
expect(outcomeEvents[1].aliases.map((alias) => alias.cacheKey).sort()).toEqual([
'/repo::feature/a',
'/repo::feature/b'
])
expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2)
})
})
+26 -1
View File
@@ -22,6 +22,11 @@ type QueueEntry = {
windowId?: number
}
type PRRefreshOutcomeObserver = (
candidate: GitHubPRRefreshCandidate,
outcome: PRRefreshOutcome
) => void
const MIN_BACKGROUND_REFRESH_AGE_MS = 60_000
const BACKGROUND_BUDGET_WINDOW_MS = 5 * 60_000
const MIN_BACKGROUND_SPACING_MS = 10_000
@@ -38,6 +43,11 @@ const backgroundStarts: number[] = []
const errorBackoff = new Map<string, { failures: number; retryAt: number }>()
let lastBackgroundStartAt = 0
const visibleByWindow = new Map<number, { generation: number; keys: Set<string> }>()
let outcomeObserver: PRRefreshOutcomeObserver | null = null
export function setPRRefreshOutcomeObserver(observer: PRRefreshOutcomeObserver | null): void {
outcomeObserver = observer
}
function nextSequence(): number {
sequence += 1
@@ -187,7 +197,21 @@ function scheduleVisibleFollowUp(
return
}
errorBackoff.delete(key)
enqueuePRRefresh(visibleCandidateAfterOutcome(candidate, outcome), 'visible', priority, windowId)
const followUpCandidate = visibleCandidateAfterOutcome(candidate, outcome)
const dueAt = freshRetryAt(followUpCandidate) ?? Date.now()
// Why: coalesced linked-PR refreshes may represent several local branches.
// Preserve every alias for the next visible follow-up so all cache entries
// keep receiving periodic updates.
queue.set(key, {
key,
candidate: followUpCandidate,
aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])),
reason: 'visible',
priority,
dueAt,
windowId
})
scheduleDrain(Math.max(0, dueAt - Date.now()))
}
function refreshIntervalForCandidate(candidate: GitHubPRRefreshCandidate): number {
@@ -340,6 +364,7 @@ async function drainQueue(): Promise<void> {
next.candidate.branch,
next.candidate.linkedPRNumber ?? null
)
outcomeObserver?.(next.candidate, outcome)
broadcast({ aliases, reason: next.reason, outcome }, requestSequence)
scheduleVisibleFollowUp(
next.key,
+17 -3
View File
@@ -41,7 +41,8 @@ import {
import {
enqueuePRRefresh,
refreshPRNow,
reportVisiblePRRefreshCandidates
reportVisiblePRRefreshCandidates,
setPRRefreshOutcomeObserver
} from '../github/pr-refresh-coordinator'
import { getWorkItemDetails, getPRFileContents } from '../github/work-item-details'
import { getRateLimit } from '../github/rate-limit'
@@ -132,6 +133,15 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
}
}
setPRRefreshOutcomeObserver((candidate, outcome) => {
const repo =
store.getRepos().find((r) => r.id === candidate.repoId) ??
store.getRepos().find((r) => resolve(r.path) === resolve(candidate.repoPath))
if (repo) {
recordPRIfNeeded(repo, outcome)
}
})
ipcMain.handle(
'gh:prForBranch',
async (_event, args: { repoPath: string; branch: string; linkedPRNumber?: number | null }) => {
@@ -173,7 +183,11 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
}
) => {
const repo = assertRegisteredRepo(args.candidate.repoPath, store)
enqueuePRRefresh({ ...args.candidate, repoPath: repo.path }, args.reason, args.priority ?? 0)
enqueuePRRefresh(
{ ...args.candidate, repoPath: repo.path, repoId: repo.id },
args.reason,
args.priority ?? 0
)
return true
}
)
@@ -183,7 +197,7 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi
(event, args: { candidates: GitHubPRRefreshCandidate[]; generation: number }) => {
const candidates = args.candidates.map((candidate) => {
const repo = assertRegisteredRepo(candidate.repoPath, store)
return { ...candidate, repoPath: repo.path }
return { ...candidate, repoPath: repo.path, repoId: repo.id }
})
reportVisiblePRRefreshCandidates(candidates, args.generation, event.sender.id)
return true
@@ -101,13 +101,22 @@ export default function ChecksPanel(): React.JSX.Element {
// differs from the PR's head ref) resolve via the number-based fallback.
const linkedPR = activeWorktree?.linkedPR ?? null
useEffect(() => {
if (repo && !isFolder && branch) {
void fetchPRForBranch(repo.path, branch, { linkedPRNumber: linkedPR })
if (isPanelVisible && repo && !isFolder && branch) {
if (activeWorktreeId) {
enqueueGitHubPRRefresh(activeWorktreeId, 'swr', 30)
}
}
}, [repo, isFolder, branch, linkedPR, fetchPRForBranch])
}, [repo, isFolder, branch, activeWorktreeId, enqueueGitHubPRRefresh, isPanelVisible])
useEffect(() => {
if (!repo || isFolder || !branch || !pr || pr.mergeable !== 'CONFLICTING') {
if (
!repo ||
isFolder ||
!branch ||
!pr ||
pr.mergeable !== 'CONFLICTING' ||
!activeWorktreeId
) {
conflictSummaryRefreshKeyRef.current = null
return
}
@@ -122,8 +131,8 @@ export default function ChecksPanel(): React.JSX.Element {
// them so we don't keep rendering cached branch summaries or empty file
// lists from an older payload.
conflictSummaryRefreshKeyRef.current = refreshKey
void fetchPRForBranch(repo.path, branch, { force: true, linkedPRNumber: linkedPR })
}, [repo, isFolder, branch, pr, linkedPR, fetchPRForBranch])
void enqueueGitHubPRRefresh(activeWorktreeId, 'active', 80)
}, [repo, isFolder, branch, pr, activeWorktreeId, enqueueGitHubPRRefresh])
// Fetch checks via cached store method
const fetchChecks = useCallback(
@@ -199,7 +199,7 @@ function SourceControlInner(): React.JSX.Element {
const isRemoteOperationActive = useAppStore((s) => s.isRemoteOperationActive)
const inFlightRemoteOpKind = useAppStore((s) => s.inFlightRemoteOpKind)
const prCache = useAppStore((s) => s.prCache)
const fetchPRForBranch = useAppStore((s) => s.fetchPRForBranch)
const enqueueGitHubPRRefresh = useAppStore((s) => s.enqueueGitHubPRRefresh)
const updateRepo = useAppStore((s) => s.updateRepo)
const beginGitBranchCompareRequest = useAppStore((s) => s.beginGitBranchCompareRequest)
const setGitBranchCompareResult = useAppStore((s) => s.setGitBranchCompareResult)
@@ -366,19 +366,24 @@ function SourceControlInner(): React.JSX.Element {
const prCacheKey = activeRepo && branchName ? `${activeRepo.path}::${branchName}` : null
const prInfo: PRInfo | null = prCacheKey ? (prCache[prCacheKey]?.data ?? null) : null
const linkedPR = activeWorktree?.linkedPR ?? null
useEffect(() => {
if (!isBranchVisible || !activeRepo || isFolder || !branchName || branchName === 'HEAD') {
if (
!isBranchVisible ||
!activeRepo ||
isFolder ||
!branchName ||
branchName === 'HEAD' ||
!activeWorktreeId
) {
return
}
// Why: the Source Control panel renders the branch's PR badge directly.
// When a terminal checkout moves this worktree onto a new branch, we need
// to fetch that branch's PR immediately instead of waiting for the user to
// reselect the worktree or open the separate Checks panel. Pass linkedPR
// so create-from-PR worktrees resolve via the number-based fallback.
void fetchPRForBranch(activeRepo.path, branchName, { linkedPRNumber: linkedPR })
}, [activeRepo, branchName, fetchPRForBranch, isBranchVisible, isFolder, linkedPR])
// to refresh that branch's PR without bypassing the coordinator's
// rate-limit guard.
enqueueGitHubPRRefresh(activeWorktreeId, 'swr', 30)
}, [activeRepo, activeWorktreeId, branchName, enqueueGitHubPRRefresh, isBranchVisible, isFolder])
const grouped = useMemo(() => {
const groups = {