diff --git a/.bumpy/ci-comment-fork-pr-resolution.md b/.bumpy/ci-comment-fork-pr-resolution.md new file mode 100644 index 0000000..b2e756b --- /dev/null +++ b/.bumpy/ci-comment-fork-pr-resolution.md @@ -0,0 +1,5 @@ +--- +'@varlock/bumpy': patch +--- + +Fix `bumpy ci comment` failing to resolve the target PR for fork PRs. Under `workflow_run`, the PR was looked up via `GET commits/{head_sha}/pulls`, which only knows about commits in the base repo's own branches — for a fork PR it returns nothing, so the command exited with "Could not resolve a target PR" (defeating the whole point of the fork-safe `pull_request` + `workflow_run` split). When that lookup is empty, bumpy now scans the repo's open PRs (paginated) and matches `head.sha` against the trusted `workflow_run.head_sha`. The target still derives only from the trusted event, never from the artifact or from `workflow_run.pull_requests[]` (which GitHub leaves empty for forks). diff --git a/packages/bumpy/src/commands/ci.ts b/packages/bumpy/src/commands/ci.ts index 7fe35b3..7beb8d8 100644 --- a/packages/bumpy/src/commands/ci.ts +++ b/packages/bumpy/src/commands/ci.ts @@ -349,22 +349,54 @@ export function resolveTargetPrNumber(rootDir: string): string | null { const repo = process.env.GITHUB_REPOSITORY; // Sanitize both before they reach the gh api path: a 40-hex SHA and an owner/repo slug. if (headSha && /^[0-9a-f]{40}$/i.test(headSha) && repo && /^[\w.-]+\/[\w.-]+$/.test(repo)) { - const out = tryRunArgs( - ['gh', 'api', `repos/${repo}/commits/${headSha}/pulls`, '--jq', '.[] | select(.state == "open") | .number'], - { cwd: rootDir }, - ); - return ( - out - ?.split('\n') - .map((l) => l.trim()) - .find((l) => /^\d+$/.test(l)) ?? null - ); + return findOpenPrByHeadSha(repo, headSha, rootDir); } return null; } return detectPrNumber(); } +/** + * Find the open PR whose head commit is `headSha`. + * + * `GET commits/{sha}/pulls` is the cheap lookup, but it only knows about commits that + * live in the base repo's own branches — for a PR opened from a fork the head commit + * isn't there, so it returns `[]`. Fall back to scanning the repo's open PRs and matching + * on `head.sha`. Both derive the target purely from the trusted SHA, so the security + * model (never trust the artifact, never trust `workflow_run.pull_requests[]`, which + * GitHub leaves empty for forks) is preserved. + */ +function findOpenPrByHeadSha(repo: string, headSha: string, rootDir: string): string | null { + const firstNumber = (out: string | null): string | null => + out + ?.split('\n') + .map((l) => l.trim()) + .find((l) => /^\d+$/.test(l)) ?? null; + + const fromCommit = firstNumber( + tryRunArgs( + ['gh', 'api', `repos/${repo}/commits/${headSha}/pulls`, '--jq', '.[] | select(.state == "open") | .number'], + { cwd: rootDir }, + ), + ); + if (fromCommit) return fromCommit; + + // `--paginate` follows Link headers, so repos with >100 open PRs are still covered. + return firstNumber( + tryRunArgs( + [ + 'gh', + 'api', + '--paginate', + `repos/${repo}/pulls?state=open&per_page=100`, + '--jq', + `.[] | select(.head.sha == "${headSha}") | .number`, + ], + { cwd: rootDir }, + ), + ); +} + // ---- ci plan ---- /** Path (relative to rootDir) where ci plan caches its output for ci release to reuse */ diff --git a/packages/bumpy/test/core/ci-comment.test.ts b/packages/bumpy/test/core/ci-comment.test.ts index 82f1a28..e1a742a 100644 --- a/packages/bumpy/test/core/ci-comment.test.ts +++ b/packages/bumpy/test/core/ci-comment.test.ts @@ -59,6 +59,40 @@ describe('resolveTargetPrNumber — workflow_run', () => { // It must look up the PR by the event's head_sha — that's the trusted derivation. const apiCalls = getCallsMatching(`commits/${HEAD_SHA}/pulls`); expect(apiCalls).toHaveLength(1); + // Same-repo PR: the commits endpoint answers, so no open-PR scan is needed. + expect(getCallsMatching('pulls?state=open')).toHaveLength(0); + }); + + test('fork PR: falls back to scanning open PRs by head.sha when the commits endpoint is empty', () => { + process.env.GITHUB_EVENT_NAME = 'workflow_run'; + process.env.GITHUB_REPOSITORY = 'owner/repo'; + process.env.GITHUB_EVENT_PATH = writeEvent({ + // GitHub leaves pull_requests[] empty for fork PRs — it must not be relied on. + workflow_run: { head_sha: HEAD_SHA, pull_requests: [] }, + }); + // The fork's head commit isn't in the base repo, so commits/{sha}/pulls yields nothing. + addMockRule({ match: /commits\/[0-9a-f]+\/pulls/, response: '' }); + addMockRule({ match: 'pulls?state=open', response: '1075\n' }); + + const pr = resolveTargetPrNumber(tmp); + + expect(pr).toBe('1075'); + expect(getCallsMatching(`commits/${HEAD_SHA}/pulls`)).toHaveLength(1); + const scans = getCallsMatching('pulls?state=open'); + expect(scans).toHaveLength(1); + // The scan matches on the trusted head_sha and paginates past 100 open PRs. + expect(scans[0]!.args).toContain('--paginate'); + expect(scans[0]!.command).toContain(`select(.head.sha == "${HEAD_SHA}")`); + }); + + test('returns null when neither lookup finds an open PR for the head_sha', () => { + process.env.GITHUB_EVENT_NAME = 'workflow_run'; + process.env.GITHUB_REPOSITORY = 'owner/repo'; + process.env.GITHUB_EVENT_PATH = writeEvent({ workflow_run: { head_sha: HEAD_SHA } }); + addMockRule({ match: /commits\/[0-9a-f]+\/pulls/, response: '' }); + addMockRule({ match: 'pulls?state=open', response: '' }); + + expect(resolveTargetPrNumber(tmp)).toBeNull(); }); test('returns null when the event has no usable head_sha', () => {