From 03a74fe26ad06d37fa4b8458479e2b9fa78f4b6a Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 15 May 2026 12:47:51 -0700 Subject: [PATCH] Fix PR lookup during GitHub GraphQL limits --- src/main/github/client.test.ts | 75 ++++++++++++++ src/main/github/client.ts | 180 ++++++++++++++++++++++++--------- 2 files changed, 205 insertions(+), 50 deletions(-) diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 5880c738ffc..37b8241d565 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -42,6 +42,10 @@ vi.mock('./gh-utils', () => ({ gitExecFileAsync: gitExecFileAsyncMock, ghRepoExecOptions: ghRepoExecOptionsMock, githubRepoContext: githubRepoContextMock, + classifyGhError: (stderr: string) => + stderr.toLowerCase().includes('not found') || stderr.includes('HTTP 404') + ? { type: 'not_found', message: stderr } + : { type: 'unknown', message: stderr }, parseGitHubOwnerRepo: (remoteUrl: string) => { const match = remoteUrl.trim().match(/github\.com[:/]([^/]+)\/([^/]+?)(?:\.git)?$/) return match ? { owner: match[1], repo: match[2] } : null @@ -120,6 +124,44 @@ describe('getPRForBranch', () => { expect(pr?.mergeable).toBe('MERGEABLE') }) + it('falls back to REST branch lookup when gh pr list is GraphQL rate limited', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockRejectedValueOnce(new Error('GraphQL: API rate limit already exceeded')) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 43, + title: 'REST branch lookup', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/43', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + head: { ref: 'feature/test', sha: 'rest-head-oid' }, + base: { ref: 'main', sha: 'rest-base-oid' } + } + ]) + }) + + const pr = await getPRForBranch('/repo-root', 'feature/test') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, + ['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=1'], + { cwd: '/repo-root' } + ) + expect(pr).toMatchObject({ + number: 43, + title: 'REST branch lookup', + state: 'open', + url: 'https://github.com/acme/widgets/pull/43', + checksStatus: 'neutral', + mergeable: 'MERGEABLE', + headSha: 'rest-head-oid' + }) + }) + it('falls back to gh pr view when the remote cannot be resolved to GitHub', async () => { getOwnerRepoMock.mockResolvedValueOnce(null) ghExecFileAsyncMock.mockResolvedValueOnce({ @@ -288,6 +330,39 @@ describe('getPRForBranch', () => { expect(pr).toBeNull() }) + it('falls back to REST number lookup when linked PR GraphQL lookup is rate limited', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockRejectedValueOnce(new Error('GraphQL: API rate limit already exceeded')) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 99, + title: 'REST linked PR lookup', + state: 'closed', + merged_at: '2026-03-28T00:00:00Z', + html_url: 'https://github.com/acme/widgets/pull/99', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable_state: 'clean', + head: { ref: 'someone/fix', sha: 'linked-head-oid' }, + base: { ref: 'main', sha: 'linked-base-oid' } + }) + }) + + const pr = await getPRForBranch('/repo-root', 'feature/test', 99) + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(3, ['api', 'repos/acme/widgets/pulls/99'], { + cwd: '/repo-root' + }) + expect(pr).toMatchObject({ + number: 99, + state: 'merged', + mergeable: 'MERGEABLE', + headSha: 'linked-head-oid' + }) + }) + it('resolves fork PR push target using the origin URL protocol', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 4f0bb72c0ee..6787553a2e7 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -990,6 +990,99 @@ export async function getWorkItemByOwnerRepo( } } +type PullRequestLookupData = { + number: number + title: string + state: string + url: string + statusCheckRollup: unknown[] + updatedAt: string + isDraft?: boolean + mergeable: string + baseRefName?: string + headRefName?: string + baseRefOid?: string + headRefOid?: string +} + +type RestPullRequest = { + number: number + title: string + state: string + html_url?: string + url?: string + updated_at?: string + draft?: boolean + merged_at?: string | null + mergeable?: boolean | null + mergeable_state?: string | null + base?: { ref?: string; sha?: string } + head?: { ref?: string; sha?: string } +} + +const PR_LOOKUP_JSON_FIELDS = + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + +function mapRestPRMergeable(pr: RestPullRequest): PRMergeableState { + const mergeableState = pr.mergeable_state?.toLowerCase() + if (mergeableState === 'dirty') { + return 'CONFLICTING' + } + if (mergeableState === 'clean' || pr.mergeable === true) { + return 'MERGEABLE' + } + return 'UNKNOWN' +} + +function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData { + return { + number: pr.number, + title: pr.title, + state: pr.merged_at ? 'MERGED' : pr.state, + url: pr.html_url ?? pr.url ?? '', + statusCheckRollup: [], + updatedAt: pr.updated_at ?? '', + isDraft: pr.draft, + mergeable: mapRestPRMergeable(pr), + baseRefName: pr.base?.ref, + headRefName: pr.head?.ref, + baseRefOid: pr.base?.sha, + headRefOid: pr.head?.sha + } +} + +async function getRestPRForBranch( + ownerRepo: OwnerRepo, + branchName: string, + ghOptions: ReturnType +): Promise { + const head = encodeURIComponent(`${ownerRepo.owner}:${branchName}`) + const { stdout } = await ghExecFileAsync( + ['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls?head=${head}&state=all&per_page=1`], + ghOptions + ) + const list = JSON.parse(stdout) as RestPullRequest[] + const pr = list[0] + return pr ? mapRestPullRequest(pr) : null +} + +async function getRestPRByNumber( + ownerRepo: OwnerRepo, + number: number, + ghOptions: ReturnType +): Promise { + const { stdout } = await ghExecFileAsync( + ['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`], + ghOptions + ) + return mapRestPullRequest(JSON.parse(stdout) as RestPullRequest) +} + +function isNotFoundGhError(err: unknown): boolean { + const stderr = err instanceof Error ? err.message : String(err) + return classifyGhError(stderr).type === 'not_found' +} + /** * Get PR info for a given branch using gh CLI. * Returns null if gh is not installed, or no PR exists for the branch. @@ -1014,54 +1107,44 @@ export async function getPRForBranch( await acquire() try { const ownerRepo = await getOwnerRepo(repoPath, connectionId) - let data: { - number: number - title: string - state: string - url: string - statusCheckRollup: unknown[] - updatedAt: string - isDraft?: boolean - mergeable: string - baseRefName?: string - headRefName?: string - baseRefOid?: string - headRefOid?: string - } | null = null + let data: PullRequestLookupData | 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 (ownerRepo) { - const { stdout } = await ghExecFileAsync( - [ - 'pr', - 'list', - '--repo', - `${ownerRepo.owner}/${ownerRepo.repo}`, - '--head', - branchName, - '--state', - 'all', - '--limit', - '1', - '--json', - 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' - ], - ghOptions - ) - const list = JSON.parse(stdout) as NonNullable[] - data = list[0] ?? null + try { + const { stdout } = await ghExecFileAsync( + [ + 'pr', + 'list', + '--repo', + `${ownerRepo.owner}/${ownerRepo.repo}`, + '--head', + branchName, + '--state', + 'all', + '--limit', + '1', + '--json', + PR_LOOKUP_JSON_FIELDS + ], + ghOptions + ) + const list = JSON.parse(stdout) as PullRequestLookupData[] + data = list[0] ?? null + } catch (err) { + // Why: `gh pr list/view` uses GraphQL and can hit that quota while + // REST is still available. Falling back prevents a real PR from + // rendering as "No pull request found" during GraphQL outages. + if (!isNotFoundGhError(err)) { + data = await getRestPRForBranch(ownerRepo, branchName, ghOptions) + } + } } else { const { stdout } = await ghExecFileAsync( - [ - 'pr', - 'view', - branchName, - '--json', - 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' - ], + ['pr', 'view', branchName, '--json', PR_LOOKUP_JSON_FIELDS], ghOptions ) data = JSON.parse(stdout) @@ -1077,24 +1160,21 @@ export async function getPRForBranch( '--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' + PR_LOOKUP_JSON_FIELDS ] + : ['pr', 'view', String(linkedPRNumber), '--json', PR_LOOKUP_JSON_FIELDS] try { const { stdout } = await ghExecFileAsync(args, ghOptions) data = JSON.parse(stdout) - } catch { + } catch (err) { // Why: a stale linkedPRNumber (PR deleted, wrong repo, …) makes // `gh pr view ` 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 + data = + ownerRepo && !isNotFoundGhError(err) + ? await getRestPRByNumber(ownerRepo, linkedPRNumber, ghOptions) + : null } }