Fix GitHub refresh review follow-ups

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-05-14 02:49:38 -04:00
co-authored by Orca
parent e8b6ccf23b
commit 169ab144fb
7 changed files with 212 additions and 110 deletions
+37
View File
@@ -102,6 +102,43 @@ describe('getPRForBranch', () => {
expect(pr?.mergeable).toBe('MERGEABLE')
})
it('uses linked PR number as the source of truth when provided', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 77,
title: 'Linked PR lookup',
state: 'OPEN',
url: 'https://github.com/acme/widgets/pull/77',
statusCheckRollup: [],
updatedAt: '2026-03-28T00:00:00Z',
isDraft: false,
mergeable: 'MERGEABLE',
baseRefName: 'main',
headRefName: 'contributor/original',
baseRefOid: 'base-oid',
headRefOid: 'head-oid'
})
})
const pr = await getPRForBranch('/repo-root', 'refs/heads/local-created-from-pr', 77)
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1)
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
[
'pr',
'view',
'77',
'--repo',
'acme/widgets',
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
],
{ cwd: '/repo-root' }
)
expect(pr?.number).toBe(77)
})
it('falls back to gh pr view when the remote cannot be resolved to GitHub', async () => {
getOwnerRepoMock.mockResolvedValueOnce(null)
ghExecFileAsyncMock.mockResolvedValueOnce({
+74 -47
View File
@@ -51,9 +51,25 @@ import {
deriveCheckStatus
} from './mappers'
import { mapGraphQLReactionGroups, type GitHubGraphQLReactionGroup } from './comment-reactions'
import {
getRateLimit,
noteRateLimitSpend,
rateLimitGuard,
type RateLimitBucketKind
} from './rate-limit'
const ORCA_REPO = 'stablyai/orca'
async function assertRateLimitBudget(bucket: RateLimitBucketKind): Promise<void> {
await getRateLimit()
const guard = rateLimitGuard(bucket)
if (guard.blocked) {
throw new Error(
`GitHub ${bucket} rate limit is low; retry after ${new Date(guard.resetAt * 1000).toLocaleTimeString()}`
)
}
}
type PRBranchData = {
number: number
title: string
@@ -984,11 +1000,10 @@ export async function getWorkItemByOwnerRepo(
* Get PR info for a given branch using gh CLI.
* Returns null if gh is not installed, or no PR exists for the branch.
*
* When `linkedPRNumber` is provided and the branch lookup yields nothing,
* falls back to looking up the PR by number. This handles "create from PR"
* worktrees, whose branch is a fresh local branch (not the PR's head ref) —
* the branch-keyed lookup misses, but the user still expects the linked PR
* to surface on the worktree card.
* When `linkedPRNumber` is provided, it is the source of truth. This handles
* "create from PR" worktrees whose local branch differs from the PR head ref,
* and prevents a coalesced linked-PR refresh from fanning out an unrelated
* branch lookup result to sibling aliases.
*/
export async function getPRForBranch(
repoPath: string,
@@ -1012,10 +1027,45 @@ export async function getPRForBranchOutcome(
const ownerRepo = await getOwnerRepo(repoPath)
let data: PRBranchData | null = null
// During a rebase the worktree is in detached HEAD and branch is empty.
// An empty --head filter causes gh to return an arbitrary PR — skip the
// branch lookup and rely on the linkedPR fallback below if available.
if (branchName) {
if (typeof linkedPRNumber === 'number') {
const args = ownerRepo
? [
'pr',
'view',
String(linkedPRNumber),
'--repo',
`${ownerRepo.owner}/${ownerRepo.repo}`,
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
]
: [
'pr',
'view',
String(linkedPRNumber),
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
]
try {
const { stdout } = await ghExecFileAsync(args, { cwd: repoPath })
data = JSON.parse(stdout)
} catch (err) {
if (!isNoPullRequestError(err)) {
return {
kind: 'upstream-error',
errorType: classifyPRRefreshError(err),
message: err instanceof Error ? err.message : String(err),
fetchedAt: Date.now()
}
}
// Why: a stale linkedPRNumber (PR deleted, wrong repo, …) makes
// `gh pr view <number>` reject. Treat that as the no-PR case so
// callers see the historical `null` semantics instead of a thrown
// error every poll cycle.
data = null
}
} else if (branchName) {
// During a rebase the worktree is in detached HEAD and branch is empty.
// An empty --head filter causes gh to return an arbitrary PR.
if (ownerRepo) {
const { stdout } = await ghExecFileAsync(
[
@@ -1059,44 +1109,6 @@ export async function getPRForBranchOutcome(
}
}
if (!data && typeof linkedPRNumber === 'number') {
const args = ownerRepo
? [
'pr',
'view',
String(linkedPRNumber),
'--repo',
`${ownerRepo.owner}/${ownerRepo.repo}`,
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
]
: [
'pr',
'view',
String(linkedPRNumber),
'--json',
'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid'
]
try {
const { stdout } = await ghExecFileAsync(args, { cwd: repoPath })
data = JSON.parse(stdout)
} catch (err) {
if (!isNoPullRequestError(err)) {
return {
kind: 'upstream-error',
errorType: classifyPRRefreshError(err),
message: err instanceof Error ? err.message : String(err),
fetchedAt: Date.now()
}
}
// Why: a stale linkedPRNumber (PR deleted, wrong repo, …) makes
// `gh pr view <number>` reject. Treat that as the no-PR case so
// callers see the historical `null` semantics instead of a thrown
// error every poll cycle.
data = null
}
}
if (!data) {
return { kind: 'no-pr', fetchedAt: Date.now() }
}
@@ -1145,6 +1157,12 @@ export async function getPRChecks(
options?: { noCache?: boolean }
): Promise<PRCheckDetail[]> {
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) {
@@ -1160,6 +1178,7 @@ export async function getPRChecks(
],
{ cwd: repoPath }
)
noteRateLimitSpend('core')
const data = JSON.parse(stdout) as {
check_runs: {
name: string
@@ -1187,6 +1206,7 @@ export async function getPRChecks(
['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,
@@ -1265,6 +1285,10 @@ export async function getPRComments(
options?: { noCache?: boolean }
): Promise<PRComment[]> {
const ownerRepo = await getOwnerRepo(repoPath)
if (ownerRepo) {
await assertRateLimitBudget('core')
}
await assertRateLimitBudget('graphql')
await acquire()
try {
if (ownerRepo) {
@@ -1309,6 +1333,8 @@ export async function getPRComments(
{ cwd: repoPath, encoding: 'utf-8' }
)
])
noteRateLimitSpend('core', 2)
noteRateLimitSpend('graphql')
// Parse issue comments (REST)
type RESTComment = {
@@ -1459,6 +1485,7 @@ export async function getPRComments(
['pr', 'view', String(prNumber), '--json', 'comments'],
{ cwd: repoPath, encoding: 'utf-8' }
)
noteRateLimitSpend('graphql')
const data = JSON.parse(stdout) as {
comments: {
author: { login: string }
+15 -5
View File
@@ -118,6 +118,9 @@ function validateCandidate(
if (candidate.isArchived) {
return 'archived'
}
if (candidate.connectionId && candidate.connectionState === 'connected') {
return 'remote'
}
if (candidate.connectionId && candidate.connectionState === 'disconnected') {
return 'disconnected'
}
@@ -468,6 +471,12 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
branch: candidate.branch,
worktreeId: candidate.worktreeId
}
const key = refreshKey(candidate)
const existing = queue.get(key)
const aliases = existing ? Array.from(existing.aliases.values()) : [alias]
if (!aliases.some((entry) => entry.cacheKey === alias.cacheKey)) {
aliases.push(alias)
}
const skippedReason = validateCandidate(candidate)
if (skippedReason) {
const outcome: PRRefreshOutcome = {
@@ -476,19 +485,20 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
message: `Cannot refresh PR for this worktree: ${skippedReason}`,
fetchedAt: Date.now()
}
broadcast({ aliases: [alias], reason: 'manual', status: 'skipped', skippedReason })
broadcast({ aliases, reason: 'manual', status: 'skipped', skippedReason })
return outcome
}
queue.delete(refreshKey(candidate))
queue.delete(key)
const requestSequence = nextSequence()
broadcast({ aliases: [alias], reason: 'manual', status: 'in-flight' }, requestSequence)
broadcast({ aliases, reason: 'manual', status: 'in-flight' }, requestSequence)
const outcome = await getPRForBranchOutcome(
candidate.repoPath,
candidate.branch,
candidate.linkedPRNumber ?? null
)
broadcast({ aliases: [alias], reason: 'manual', outcome }, requestSequence)
scheduleVisibleFollowUp(refreshKey(candidate), candidate, outcome, 40, [alias])
outcomeObserver?.(candidate, outcome)
broadcast({ aliases, reason: 'manual', outcome }, requestSequence)
scheduleVisibleFollowUp(key, candidate, outcome, 40, aliases)
return outcome
}
@@ -76,6 +76,9 @@ export default function ChecksPanel(): React.JSX.Element {
const isFolder = repo ? isFolderRepo(repo) : false
const prCacheKey = repo && branch ? `${repo.path}::${branch}` : ''
const pr: PRInfo | null = prCacheKey ? (prCache[prCacheKey]?.data ?? null) : null
const prRefreshState = useAppStore((s) =>
prCacheKey ? s.prRefreshStates[prCacheKey] : undefined
)
const prNumber = pr?.number ?? null
const conflictOperation = activeWorktreeId
? (gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown')
@@ -311,42 +314,25 @@ export default function ChecksPanel(): React.JSX.Element {
}
}, [repo, branch, linkedPR, fetchPRForBranch, fetchPRChecks, fetchComments])
const handleEntryRefresh = useCallback(async () => {
if (!repo || !branch || !activeWorktreeId) {
return
}
// Why: entering the Checks tab is automatic UI behavior, not an explicit
// user refresh. Route PR refresh through the coordinator so rate-limit
// guards still apply, while checks/comments can refresh from cached PR data.
enqueueGitHubPRRefresh(activeWorktreeId, 'active', 80)
if (!pr) {
setChecks([])
setComments([])
return
}
const refreshedChecks = fetchPRChecks(repo.path, pr.number, branch, pr.headSha, {
force: true
}).then(
(result) => {
setChecks(result)
const signature = JSON.stringify(result.map((c) => `${c.name}:${c.status}:${c.conclusion}`))
pollIntervalRef.current =
signature === prevChecksRef.current
? Math.min(pollIntervalRef.current * 2, 120_000)
: 30_000
prevChecksRef.current = signature
},
(err) => {
console.warn('Failed to fetch PR checks:', err)
setChecks([])
const handleEntryRefresh = useCallback(
(options: { refreshChecks: boolean; refreshComments: boolean }) => {
if (!repo || !branch || !activeWorktreeId) {
return
}
)
setChecksLoading(true)
const refreshedComments = fetchComments({ force: true, prNumberOverride: pr.number })
await Promise.all([refreshedChecks.finally(() => setChecksLoading(false)), refreshedComments])
}, [repo, branch, activeWorktreeId, enqueueGitHubPRRefresh, pr, fetchPRChecks, fetchComments])
// Why: entering the Checks tab is automatic UI behavior, not an explicit
// user refresh. Route PR refresh through the coordinator so rate-limit
// guards still apply; only force detail panes that the entry freshness rule
// already proved stale, so tab entry stays fresh without broad fan-out.
enqueueGitHubPRRefresh(activeWorktreeId, 'active', 80)
if (options.refreshChecks) {
void fetchChecks({ force: true })
}
if (options.refreshComments) {
void fetchComments({ force: true })
}
},
[repo, branch, activeWorktreeId, enqueueGitHubPRRefresh, fetchChecks, fetchComments]
)
// Why: force a freshness check on each "entry" into the Checks tab so PRs
// opened outside Orca, externally force-pushed heads, and stale checks/comments
@@ -371,23 +357,29 @@ export default function ChecksPanel(): React.JSX.Element {
}
lastEntryKeyRef.current = entryKey
const now = Date.now()
const stale = shouldEntryRefresh({
prFetchedAt,
checksFetchedAt,
commentsFetchedAt,
prNumber,
now: Date.now(),
now,
graceMs: ENTRY_REFRESH_GRACE_MS
})
if (!stale) {
return
}
const cutoff = now - ENTRY_REFRESH_GRACE_MS
const refreshChecks =
prNumber !== null && (checksFetchedAt === undefined || checksFetchedAt < cutoff)
const refreshComments =
prNumber !== null && (commentsFetchedAt === undefined || commentsFetchedAt < cutoff)
// Reset polling attention state so the forced fetch's signature establishes
// a fresh baseline rather than colliding with the previous PR's backoff.
pollIntervalRef.current = 30_000
prevChecksRef.current = ''
void handleEntryRefresh()
handleEntryRefresh({ refreshChecks, refreshComments })
}, [entryKey, prFetchedAt, checksFetchedAt, commentsFetchedAt, prNumber, handleEntryRefresh])
const handleStartEdit = useCallback(() => {
@@ -505,21 +497,37 @@ export default function ChecksPanel(): React.JSX.Element {
: conflictOperation === 'cherry-pick'
? 'Cherry-pick'
: null
const isQueuedPRRefresh = prRefreshState?.status === 'queued'
const isInFlightPRRefresh = prRefreshState?.status === 'in-flight'
const isCheckingPR = isQueuedPRRefresh || isInFlightPRRefresh
const isPausedPRRefresh = prRefreshState?.status === 'paused'
return (
<div className="px-4 py-6">
<div className="text-sm font-medium text-foreground">
{operationInProgress ? `${operationLabel} in progress` : 'No pull request found'}
{operationInProgress
? `${operationLabel} in progress`
: isQueuedPRRefresh
? 'Refresh queued'
: isInFlightPRRefresh
? 'Checking for pull request'
: 'No pull request found'}
</div>
<div className="mt-1 text-xs text-muted-foreground">
{operationInProgress
? 'PR checks will be available after the operation completes'
: 'Push your branch and open a PR to see checks here'}
: isQueuedPRRefresh
? 'GitHub status will refresh shortly'
: 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'}
</div>
{!operationInProgress && (
<button
className="mt-3 px-3 py-1 text-xs font-medium rounded-md border border-border bg-accent/50 text-foreground hover:bg-accent transition-colors disabled:opacity-50"
disabled={emptyRefreshing}
disabled={emptyRefreshing || isCheckingPR || isPausedPRRefresh}
onClick={() => {
if (!activeWorktreeId) {
return
@@ -530,7 +538,13 @@ export default function ChecksPanel(): React.JSX.Element {
})
}}
>
{emptyRefreshing ? 'Refreshing…' : 'Refresh'}
{isQueuedPRRefresh
? 'Queued'
: emptyRefreshing || isInFlightPRRefresh
? 'Refreshing...'
: isPausedPRRefresh
? 'Paused'
: 'Refresh'}
</button>
)}
</div>
@@ -616,7 +630,7 @@ export default function ChecksPanel(): React.JSX.Element {
{/* Updated at */}
{pr.updatedAt && (
<div className="text-[10px] text-muted-foreground/60">
Updated {new Date(pr.updatedAt).toLocaleString()}
PR updated {new Date(pr.updatedAt).toLocaleString()}
</div>
)}
@@ -7,9 +7,9 @@ import { Tooltip, TooltipTrigger, TooltipContent } from '@/components/ui/tooltip
import {
AlertTriangle,
Bell,
Clock3,
GitMerge,
LoaderCircle,
PauseCircle,
CircleCheck,
CircleX,
Server,
@@ -286,6 +286,7 @@ const WorktreeCard = React.memo(function WorktreeCard({
)
const showIssue = cardProps.includes('issue')
const showPRMeta = cardProps.includes('pr') && Boolean(pr)
// Same rationale for issues: once that section is hidden, polling only burns
// GitHub calls and keeps stale-but-invisible data warm for no user benefit.
@@ -522,21 +523,24 @@ const WorktreeCard = React.memo(function WorktreeCard({
<Tooltip>
<TooltipTrigger asChild>
<span className="inline-flex items-center opacity-80 hover:opacity-100 transition-opacity">
{prRefreshState?.status === 'in-flight' && (
{!showPRMeta && prRefreshState?.status === 'queued' && (
<Clock3 className="size-3.5 text-muted-foreground" />
)}
{!showPRMeta && prRefreshState?.status === 'in-flight' && (
<LoaderCircle className="size-3.5 text-muted-foreground animate-spin" />
)}
{prRefreshState?.status === 'paused' && (
<PauseCircle className="size-3.5 text-muted-foreground" />
)}
{!['in-flight', 'paused'].includes(prRefreshState?.status ?? '') &&
{(showPRMeta ||
!['queued', 'in-flight'].includes(prRefreshState?.status ?? '')) &&
pr.checksStatus === 'success' && (
<CircleCheck className="size-3.5 text-emerald-500" />
)}
{!['in-flight', 'paused'].includes(prRefreshState?.status ?? '') &&
{(showPRMeta ||
!['queued', 'in-flight'].includes(prRefreshState?.status ?? '')) &&
pr.checksStatus === 'failure' && (
<CircleX className="size-3.5 text-rose-500" />
)}
{!['in-flight', 'paused'].includes(prRefreshState?.status ?? '') &&
{(showPRMeta ||
!['queued', 'in-flight'].includes(prRefreshState?.status ?? '')) &&
pr.checksStatus === 'pending' && (
<LoaderCircle className="size-3.5 text-amber-500 animate-spin" />
)}
@@ -544,11 +548,13 @@ const WorktreeCard = React.memo(function WorktreeCard({
</TooltipTrigger>
<TooltipContent side="right" sideOffset={8}>
<span>
{prRefreshState?.status === 'in-flight'
? 'Refreshing CI checks'
: prRefreshState?.status === 'paused'
? 'CI refresh paused'
: `CI checks ${checksLabel(pr.checksStatus).toLowerCase()}`}
{!showPRMeta && prRefreshState?.status === 'queued'
? 'CI refresh queued'
: !showPRMeta && prRefreshState?.status === 'in-flight'
? 'Refreshing CI checks'
: prRefreshState?.status === 'paused'
? `CI checks ${checksLabel(pr.checksStatus).toLowerCase()} (refresh paused)`
: `CI checks ${checksLabel(pr.checksStatus).toLowerCase()}`}
</span>
</TooltipContent>
</Tooltip>
+3 -2
View File
@@ -1283,7 +1283,8 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
linkedPRNumber,
cachedFetchedAt: cached?.fetchedAt ?? null
}
const outcome = window.api.gh.refreshPRNow
const usesRefreshCoordinator = Boolean(window.api.gh.refreshPRNow)
const outcome = usesRefreshCoordinator
? await window.api.gh.refreshPRNow({ candidate })
: await window.api.gh
.prForBranch({ repoPath, branch, linkedPRNumber })
@@ -1294,7 +1295,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
)
const pr: PRInfo | null =
outcome.kind === 'found' ? outcome.pr : outcome.kind === 'no-pr' ? null : null
if (prRequestGenerations.get(cacheKey) === generation) {
if (!usesRefreshCoordinator && prRequestGenerations.get(cacheKey) === generation) {
if (outcome.kind !== 'upstream-error') {
set((s) => ({
prCache: { ...s.prCache, [cacheKey]: { data: pr, fetchedAt: outcome.fetchedAt } }
+8 -1
View File
@@ -537,7 +537,14 @@ export type GitHubPRRefreshEvent = {
outcome?: PRRefreshOutcome
status?: 'queued' | 'in-flight' | 'paused' | 'skipped'
pausedUntil?: number
skippedReason?: 'fresh' | 'not-git' | 'bare' | 'archived' | 'disconnected' | 'rate-limit'
skippedReason?:
| 'fresh'
| 'not-git'
| 'bare'
| 'archived'
| 'disconnected'
| 'remote'
| 'rate-limit'
}
export type PRCheckDetail = {