diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 03de78343b4..d0d001764ed 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -106,6 +106,7 @@ import { checkOrcaStarred, getPRComments, getPRForBranch, + getPRForBranchOutcome, getRepoUpstream, getWorkItem, getPullRequestPushTarget, @@ -835,6 +836,215 @@ describe('getPRForBranch', () => { expect(pr).toMatchObject({ number: 42, title: 'Fallback PR lookup' }) }) + it('reports upstream error when fallback branch discovery fails transiently then retry misses', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'stablyai', repo: 'orca' }], + headRepo: null + }) + ghExecFileAsyncMock + .mockRejectedValueOnce(new Error('HTTP 429: API rate limit exceeded')) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'feature/test') + + expect(outcome).toMatchObject({ + kind: 'upstream-error', + errorType: 'rate_limited' + }) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 1, + [ + 'pr', + 'list', + '--repo', + 'stablyai/orca', + '--head', + 'feature/test', + '--state', + 'all', + '--limit', + '1', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: '/repo-root' } + ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, + ['api', 'repos/stablyai/orca/pulls?head=stablyai%3Afeature%2Ftest&state=all&per_page=1'], + { cwd: '/repo-root' } + ) + }) + + it('reports no PR when fallback branch discovery cleanly misses', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'stablyai', repo: 'orca' }], + headRepo: null + }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'feature/test') + + expect(outcome.kind).toBe('no-pr') + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('returns found when fallback branch discovery retry finds the PR', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'stablyai', repo: 'orca' }], + headRepo: null + }) + ghExecFileAsyncMock + .mockRejectedValueOnce(new Error('HTTP 429: API rate limit exceeded')) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 42, + title: 'Retry branch PR', + state: 'open', + html_url: 'https://github.com/stablyai/orca/pull/42', + updated_at: '2026-03-28T00:00:00Z', + draft: false, + mergeable: true, + base: { ref: 'main', sha: 'base-oid' }, + head: { ref: 'feature/test', sha: 'retry-head-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Hydrated retry branch PR', + state: 'OPEN', + url: 'https://github.com/stablyai/orca/pull/42', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'feature/test', + baseRefOid: 'base-oid', + headRefOid: 'retry-head-oid' + }) + }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'feature/test') + + expect(outcome).toMatchObject({ + kind: 'found', + pr: { + number: 42, + title: 'Hydrated retry branch PR', + prRepo: { owner: 'stablyai', repo: 'orca' } + } + }) + }) + + it('lets fallback PR number recovery win after fallback branch queries throw', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'stablyai', repo: 'orca' }], + headRepo: null + }) + ghExecFileAsyncMock + .mockRejectedValueOnce(new Error('HTTP 429: API rate limit exceeded')) + .mockRejectedValueOnce(new Error('HTTP 502: Bad Gateway')) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Fallback number recovered PR', + state: 'OPEN', + url: 'https://github.com/stablyai/orca/pull/42', + statusCheckRollup: [], + updatedAt: '2026-03-28T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'feature/test', + baseRefOid: 'base-oid', + headRefOid: 'fallback-head-oid' + }) + }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'feature/test', null, null, 42) + + expect(outcome).toMatchObject({ + kind: 'found', + pr: { + number: 42, + title: 'Fallback number recovered PR' + } + }) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 3, + [ + 'pr', + 'view', + '42', + '--repo', + 'stablyai/orca', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,reviewDecision,mergeStateStatus,autoMergeRequest,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: '/repo-root' } + ) + }) + + it('reports upstream error when fallback branch discovery has a network failure', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'stablyai', repo: 'orca' }], + headRepo: null + }) + ghExecFileAsyncMock + .mockRejectedValueOnce(new Error('could not resolve host: api.github.com')) + .mockRejectedValueOnce(new Error('could not resolve host: api.github.com')) + + const outcome = await getPRForBranchOutcome('/repo-root', 'feature/test') + + expect(outcome).toMatchObject({ + kind: 'upstream-error', + errorType: 'network' + }) + }) + + it('keeps a pending fallback branch error when a later candidate cleanly misses', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [ + { owner: 'stablyai', repo: 'orca' }, + { owner: 'fork', repo: 'orca' } + ], + headRepo: null + }) + ghExecFileAsyncMock + .mockRejectedValueOnce(new Error('HTTP 429: API rate limit exceeded')) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([]) }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'feature/test') + + expect(outcome).toMatchObject({ + kind: 'upstream-error', + errorType: 'rate_limited' + }) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 3, + [ + 'pr', + 'list', + '--repo', + 'fork/orca', + '--head', + 'feature/test', + '--state', + 'all', + '--limit', + '1', + '--json', + 'number,title,state,url,statusCheckRollup,updatedAt,isDraft,mergeable,baseRefName,headRefName,baseRefOid,headRefOid' + ], + { cwd: '/repo-root' } + ) + }) + it('treats a merged branch lookup as a miss before using a fallback PR number', async () => { getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 377c5f1312e..7285f9b1202 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -2554,8 +2554,14 @@ async function lookupPRByBranchName(args: { headRepo: OwnerRepo | null branchName: string ghOptions: GhExecOptions -}): Promise<{ data: PullRequestLookupData | null; dataRepo: OwnerRepo | null }> { +}): Promise<{ + data: PullRequestLookupData | null + dataRepo: OwnerRepo | null + pendingError?: unknown +}> { if (args.candidates.length > 0) { + let pendingError: unknown + let hasPendingError = false for (const candidate of args.candidates) { try { const branchData = args.headRepo @@ -2576,19 +2582,34 @@ async function lookupPRByBranchName(args: { if (args.headRepo) { throw err } - const branchData = await getRestPRForBranch( - candidate, - candidate.owner, - args.branchName, - args.ghOptions - ) - const data = await hydrateBranchLookupWithExactPR(candidate, branchData, args.ghOptions) - if (data) { - return { data, dataRepo: candidate } + if (!hasPendingError) { + pendingError = err + hasPendingError = true + } + try { + const branchData = await getRestPRForBranch( + candidate, + candidate.owner, + args.branchName, + args.ghOptions + ) + const data = await hydrateBranchLookupWithExactPR(candidate, branchData, args.ghOptions) + if (data) { + return { data, dataRepo: candidate } + } + } catch (retryErr) { + if (!hasPendingError) { + pendingError = retryErr + hasPendingError = true + } } } } - return { data: null, dataRepo: null } + // Why: branch-list failures are ambiguous for fork discovery, but exact + // fallback-number recovery should still get a chance before surfacing error. + return hasPendingError + ? { data: null, dataRepo: null, pendingError } + : { data: null, dataRepo: null } } try { @@ -2778,6 +2799,8 @@ export async function getPRForBranchOutcome( let data: PullRequestLookupData | null = null let dataRepo: OwnerRepo | null = null let dataHeadRepo: OwnerRepo | null = headRepo + let pendingBranchLookupError: unknown + let hasPendingBranchLookupError = false let currentHeadOidForMergedImplicit: string | null | undefined const explicitCurrentHeadOid = @@ -2817,6 +2840,10 @@ export async function getPRForBranchOutcome( }) data = branchLookup.data dataRepo = branchLookup.dataRepo + if ('pendingError' in branchLookup) { + pendingBranchLookupError = branchLookup.pendingError + hasPendingBranchLookupError = true + } if (!data) { // Why: the tracked upstream can identify the real PR head by branch // name or by fork owner even when branch names match locally. @@ -2846,6 +2873,10 @@ export async function getPRForBranchOutcome( }) data = upstreamLookup.data dataRepo = upstreamLookup.dataRepo + if (!hasPendingBranchLookupError && 'pendingError' in upstreamLookup) { + pendingBranchLookupError = upstreamLookup.pendingError + hasPendingBranchLookupError = true + } if (data) { dataHeadRepo = upstreamHeadRepo } @@ -2870,6 +2901,9 @@ export async function getPRForBranchOutcome( dataRepo = fallbackLookup.dataRepo } if (!data) { + if (hasPendingBranchLookupError) { + return prRefreshUpstreamError(pendingBranchLookupError) + } return { kind: 'no-pr', fetchedAt: Date.now() } } const fallbackConfirmedMergedBranch =