From 93bf77f271cfbd6670524866699e248305676d0e Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 13 May 2026 05:04:27 -0400 Subject: [PATCH] Optimize PR refresh queue usage Co-authored-by: Orca --- src/main/github/client.ts | 110 ++++++----- src/main/github/pr-refresh-coordinator.ts | 2 +- src/main/ipc/github.ts | 24 ++- .../components/right-sidebar/ChecksPanel.tsx | 29 +-- .../src/components/sidebar/WorktreeList.tsx | 4 + src/renderer/src/store/slices/github.test.ts | 183 ++++++++++++++++++ src/renderer/src/store/slices/github.ts | 36 +++- 7 files changed, 315 insertions(+), 73 deletions(-) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index b714c675744..6717cf5451f 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -1159,66 +1159,72 @@ export async function getPRChecks( const ownerRepo = headSha ? await getOwnerRepo(repoPath) : null if (ownerRepo && headSha) { await assertRateLimitBudget('core') - } - // Why: even the REST check-runs path can fall back to `gh pr checks`, so - // guard GraphQL before acquiring the global gh lock. - await assertRateLimitBudget('graphql') - await acquire() - try { - if (ownerRepo && headSha) { + await acquire() + try { // Why: --cache 60s saves rate-limit budget during polling, but when the // user explicitly clicks refresh we must skip it so gh fetches fresh data. const cacheArgs = options?.noCache ? [] : ['--cache', '60s'] - try { - const { stdout } = await ghExecFileAsync( - [ - 'api', - ...cacheArgs, - `repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodeURIComponent(headSha)}/check-runs?per_page=100` - ], - { cwd: repoPath } - ) - noteRateLimitSpend('core') - const data = JSON.parse(stdout) as { - check_runs: { - name: string - status: string - conclusion: string | null - html_url: string - details_url: string | null - }[] - } - return data.check_runs.map((d) => ({ - name: d.name, - status: mapCheckRunRESTStatus(d.status), - conclusion: mapCheckRunRESTConclusion(d.status, d.conclusion), - url: d.details_url || d.html_url || null - })) - } catch (err) { - // Why: a PR can outlive the cached head SHA after force-pushes or remote - // rewrites. Falling back to `gh pr checks` keeps the panel populated - // instead of rendering a false "no checks" state from a stale commit. - console.warn('getPRChecks via head SHA failed, falling back to gh pr checks:', err) + const { stdout } = await ghExecFileAsync( + [ + 'api', + ...cacheArgs, + `repos/${ownerRepo.owner}/${ownerRepo.repo}/commits/${encodeURIComponent(headSha)}/check-runs?per_page=100` + ], + { cwd: repoPath } + ) + noteRateLimitSpend('core') + const data = JSON.parse(stdout) as { + check_runs: { + name: string + status: string + conclusion: string | null + html_url: string + details_url: string | null + }[] } + return data.check_runs.map((d) => ({ + name: d.name, + status: mapCheckRunRESTStatus(d.status), + conclusion: mapCheckRunRESTConclusion(d.status, d.conclusion), + url: d.details_url || d.html_url || null + })) + } catch (err) { + // Why: a PR can outlive the cached head SHA after force-pushes or remote + // rewrites. Falling back to `gh pr checks` keeps the panel populated + // instead of rendering a false "no checks" state from a stale commit. + console.warn('getPRChecks via head SHA failed, falling back to gh pr checks:', err) + } finally { + release() + } + } + + // Fallback: no branch provided or non-GitHub remote + // Why: the REST check-runs path spends core only. Guard GraphQL only when + // we actually fall back to `gh pr checks`, so a low GraphQL bucket does not + // block fresh REST check data. Keep this outside the gh lock because the + // guard may need its own `gh api rate_limit` call. + try { + await assertRateLimitBudget('graphql') + await acquire() + try { + const { stdout } = await ghExecFileAsync( + ['pr', 'checks', String(prNumber), '--json', 'name,state,link'], + { cwd: repoPath } + ) + noteRateLimitSpend('graphql') + const data = JSON.parse(stdout) as { name: string; state: string; link: string }[] + return data.map((d) => ({ + name: d.name, + status: mapCheckStatus(d.state), + conclusion: mapCheckConclusion(d.state), + url: d.link || null + })) + } finally { + release() } - // Fallback: no branch provided or non-GitHub remote - const { stdout } = await ghExecFileAsync( - ['pr', 'checks', String(prNumber), '--json', 'name,state,link'], - { cwd: repoPath } - ) - noteRateLimitSpend('graphql') - const data = JSON.parse(stdout) as { name: string; state: string; link: string }[] - return data.map((d) => ({ - name: d.name, - status: mapCheckStatus(d.state), - conclusion: mapCheckConclusion(d.state), - url: d.link || null - })) } catch (err) { console.warn('getPRChecks failed:', err) return [] - } finally { - release() } } diff --git a/src/main/github/pr-refresh-coordinator.ts b/src/main/github/pr-refresh-coordinator.ts index e23303981ee..0b7f0b25532 100644 --- a/src/main/github/pr-refresh-coordinator.ts +++ b/src/main/github/pr-refresh-coordinator.ts @@ -547,7 +547,7 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise message: `Cannot refresh PR for this worktree: ${skippedReason}`, fetchedAt: Date.now() } - broadcast({ aliases, reason: 'manual', status: 'skipped', skippedReason }) + broadcast({ aliases: [alias], reason: 'manual', status: 'skipped', skippedReason }) return outcome } diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index a148aa2d6f2..689a00e051c 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -166,7 +166,13 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi 'gh:refreshPRNow', async (_event, args: { candidate: GitHubPRRefreshCandidate }) => { const repo = assertRegisteredRepo(args.candidate.repoPath, store) - const outcome = await refreshPRNow({ ...args.candidate, repoPath: repo.path }) + const outcome = await refreshPRNow({ + ...args.candidate, + repoPath: repo.path, + repoId: repo.id, + connectionId: repo.connectionId ?? args.candidate.connectionId, + connectionState: repo.connectionId ? 'connected' : args.candidate.connectionState + }) recordPRIfNeeded(repo, outcome) return outcome } @@ -184,7 +190,13 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi ) => { const repo = assertRegisteredRepo(args.candidate.repoPath, store) enqueuePRRefresh( - { ...args.candidate, repoPath: repo.path, repoId: repo.id }, + { + ...args.candidate, + repoPath: repo.path, + repoId: repo.id, + connectionId: repo.connectionId ?? args.candidate.connectionId, + connectionState: repo.connectionId ? 'connected' : args.candidate.connectionState + }, args.reason, args.priority ?? 0 ) @@ -197,7 +209,13 @@ 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, repoId: repo.id } + return { + ...candidate, + repoPath: repo.path, + repoId: repo.id, + connectionId: repo.connectionId ?? candidate.connectionId, + connectionState: repo.connectionId ? 'connected' : candidate.connectionState + } }) reportVisiblePRRefreshCandidates(candidates, args.generation, event.sender.id) return true diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index af7bb77c176..91bdde3c32e 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -77,6 +77,7 @@ export default function ChecksPanel(): React.JSX.Element { // Find active worktree and repo const branch = activeWorktree ? activeWorktree.branch.replace(/^refs\/heads\//, '') : '' const isFolder = repo ? isFolderRepo(repo) : false + const isRemoteRepo = Boolean(repo?.connectionId) const prCacheKey = repo && branch ? `${repo.path}::${branch}` : '' const refreshContextKey = `${activeWorktreeId ?? ''}::${repo?.path ?? ''}::${branch}` if (refreshContextKey !== refreshContextKeyRef.current) { @@ -549,24 +550,28 @@ export default function ChecksPanel(): React.JSX.Element {
{operationInProgress ? `${operationLabel} in progress` - : isQueuedPRRefresh - ? 'Checking for pull request' - : isInFlightPRRefresh + : isRemoteRepo + ? 'Checks unavailable' + : isQueuedPRRefresh ? 'Checking for pull request' - : 'No pull request found'} + : isInFlightPRRefresh + ? 'Checking for pull request' + : 'No pull request found'}
{operationInProgress ? 'PR checks will be available after the operation completes' - : isQueuedPRRefresh - ? 'Waiting to refresh GitHub status for this branch' - : isInFlightPRRefresh - ? 'Refreshing GitHub status for this branch' - : isPausedPRRefresh - ? 'GitHub refresh is paused by the current rate-limit budget' - : 'Push your branch and open a PR to see checks here'} + : isRemoteRepo + ? 'PR refresh is not supported for remote worktrees yet' + : isQueuedPRRefresh + ? 'Waiting to refresh GitHub status for this branch' + : isInFlightPRRefresh + ? 'Refreshing GitHub status for this branch' + : isPausedPRRefresh + ? 'GitHub refresh is paused by the current rate-limit budget' + : 'Push your branch and open a PR to see checks here'}
- {!operationInProgress && ( + {!operationInProgress && !isRemoteRepo && (