From 3111be170087e0b7e202c50598f3f0ea7dea8c90 Mon Sep 17 00:00:00 2001 From: Heewon Oh Date: Mon, 5 Oct 2026 13:35:17 +0900 Subject: [PATCH] fix(pr): preserve GitHub lookup failures during branch PR resolution (AGT-3474) --- src/cli/prResolve.test.ts | 54 +++++++++++++++++++++++- src/cli/prResolve.ts | 87 ++++++++++++++++++++++++++++++++------- 2 files changed, 124 insertions(+), 17 deletions(-) diff --git a/src/cli/prResolve.test.ts b/src/cli/prResolve.test.ts index d0a67606..89f1a8b1 100644 --- a/src/cli/prResolve.test.ts +++ b/src/cli/prResolve.test.ts @@ -14,7 +14,7 @@ vi.mock('node:child_process', () => { return { execFile }; }); -const { parsePRRef, resolveRepoName, resolveOriginRepo, resolvePR, toPRInfo } = await import('./prResolve.js'); +const { parsePRRef, resolveRepoName, resolveOriginRepo, resolveCurrentBranchPR, resolvePR, toPRInfo, PRProviderLookupError, isNoPullRequestsError } = await import('./prResolve.js'); beforeEach(() => { execImpl.mockReset(); @@ -118,6 +118,58 @@ describe('resolvePR (INT-3282)', () => { }); }); +describe('resolveCurrentBranchPR (AGT-3474)', () => { + const ghView = { number: 9, title: 'Ship it', headRefName: 'feat/x', url: 'https://example/pr/9', author: { login: 'someone' } }; + + it('resolves the open PR when one exists', async () => { + execImpl.mockResolvedValueOnce({ stdout: JSON.stringify(ghView), stderr: '' }); + const result = await resolveCurrentBranchPR('/tmp/proj', 'o/r'); + expect(result).toEqual({ repo: 'o/r', number: 9, title: 'Ship it', branch: 'feat/x', url: 'https://example/pr/9', author: 'someone' }); + }); + + it('resolves to null when gh reports no PR for the branch', async () => { + execImpl.mockRejectedValueOnce(new Error('no pull requests found for branch "feat/x"')); + await expect(resolveCurrentBranchPR('/tmp/proj', 'o/r')).resolves.toBeNull(); + }); + + it('classifies the no-open-pr phrasing and stderr payloads as no-PR', () => { + expect(isNoPullRequestsError(new Error('no open pull requests in this repo'))).toBe(true); + expect(isNoPullRequestsError({ message: 'Command failed: gh pr view', stderr: 'no pull requests found for branch "x"' })).toBe(true); + expect(isNoPullRequestsError(new Error('Could not resolve host: api.github.com'))).toBe(false); + expect(isNoPullRequestsError('gh auth expired')).toBe(false); + }); + + it('surfaces a provider lookup failure instead of reading it as no-PR', async () => { + execImpl.mockRejectedValueOnce(new Error('Could not resolve host: api.github.com')); + const promise = resolveCurrentBranchPR('/tmp/proj', 'o/r'); + await expect(promise).rejects.toBeInstanceOf(PRProviderLookupError); + await expect(promise).rejects.toThrow(/Could not look up the open PR for the current branch in o\/r/); + await expect(promise).rejects.toThrow(/Could not resolve host/); + }); + + it('keeps the underlying gh failure as the cause', async () => { + const cause = new Error('HTTP 401: Bad credentials'); + execImpl.mockRejectedValueOnce(cause); + try { + await resolveCurrentBranchPR('/tmp/proj', 'o/r'); + expect.unreachable('resolveCurrentBranchPR should have thrown'); + } catch (error) { + expect(error).toBeInstanceOf(PRProviderLookupError); + expect((error as InstanceType).cause).toBe(cause); + } + }); + + it('propagates through resolvePR, never masked as "no open PR"', async () => { + execImpl + .mockResolvedValueOnce({ stdout: 'o/r\n', stderr: '' }) + .mockRejectedValueOnce(new Error('HTTP 401: Bad credentials')); // gh pr view + const promise = resolvePR({ path: '/tmp/proj' }); + await expect(promise).rejects.toBeInstanceOf(PRProviderLookupError); + const message = await promise.catch((error: unknown) => (error as Error).message); + expect(message).not.toMatch(/No open PR for branch/); + }); +}); + describe('toPRInfo (INT-3282)', () => { it('carries repo/number/title/branch/url/author through, stamping createdAt', () => { const info = toPRInfo({ repo: 'o/r', number: 9, title: 'Ship it', branch: 'feat/x', url: 'https://example/pr/9', author: 'someone' }); diff --git a/src/cli/prResolve.ts b/src/cli/prResolve.ts index 9c165df7..2ef89ccf 100644 --- a/src/cli/prResolve.ts +++ b/src/cli/prResolve.ts @@ -44,6 +44,42 @@ export interface ResolvePROptions { repo?: string; } +/** + * A provider lookup failure while finding the current branch's PR — network + * outage, expired auth, permission failure. Distinct from "the branch simply + * has no PR" (which is normal flow) so a caller never reads an actionable + * GitHub error as "open a new PR" (AGT-3474). + */ +export class PRProviderLookupError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = 'PRProviderLookupError'; + } +} + +/** + * gh reports "this branch has no open PR" by exiting non-zero with that phrase + * in its output (usually `no pull requests found for branch "X"`). Match + * conservatively against message + stderr so formatting differences between + * gh versions still classify, while every other failure stays a lookup + * failure instead of being silently read as no-PR. + */ +export function isNoPullRequestsError(error: unknown): boolean { + const err = error as { message?: unknown; stderr?: unknown } | null; + const text = [ + err?.message, + typeof err?.stderr === 'string' ? err.stderr : '', + ] + .filter((part) => typeof part === 'string' && part.length > 0) + .join('\n'); + return /no (?:open )?pull requests/i.test(text); +} + +function describeCause(error: unknown): string { + if (error instanceof Error) return error.message; + return String(error); +} + /** * Parse `owner/repo#123`, `#123`, or bare `123` into parts. * Pure — no I/O. @@ -94,17 +130,16 @@ export async function resolveOriginRepo(cwd: string): Promise { } /** - * Resolve the PR to operate on: explicit number → current branch's PR → error. + * Look up the open PR for the current branch, if any. + * Returns null when gh reports no PR on the branch. A provider lookup failure + * (network, auth, permissions) throws `PRProviderLookupError` so callers can + * surface the actionable error rather than treat it as "no PR" (AGT-3474). */ -export async function resolvePR(opts: ResolvePROptions = {}): Promise { - const cwd = opts.path ?? process.cwd(); - const repo = await resolveRepoName(cwd, opts.repo); - - if (opts.number != null) { +export async function resolveCurrentBranchPR(cwd: string, repo: string): Promise { + try { const stdout = await gh( cwd, - 'pr', 'view', String(opts.number), - '-R', repo, + 'pr', 'view', '--json', 'number,title,headRefName,url,author', ); const view = JSON.parse(stdout) as { @@ -122,13 +157,30 @@ export async function resolvePR(opts: ResolvePROptions = {}): Promise { + const cwd = opts.path ?? process.cwd(); + const repo = await resolveRepoName(cwd, opts.repo); + + if (opts.number != null) { const stdout = await gh( cwd, - 'pr', 'view', + 'pr', 'view', String(opts.number), + '-R', repo, '--json', 'number,title,headRefName,url,author', ); const view = JSON.parse(stdout) as { @@ -146,12 +198,15 @@ export async function resolvePR(opts: ResolvePROptions = {}): Promise or create one with: openswarm pr create`, - ); } + + const current = await resolveCurrentBranchPR(cwd, repo); + if (current) return current; + + const branch = (await git(cwd, 'rev-parse', '--abbrev-ref', 'HEAD')).trim(); + throw new Error( + `No open PR for branch "${branch}" in ${repo}. Pass --number or create one with: openswarm pr create`, + ); } /** Convert ResolvedPR → github.PRInfo. */