diff --git a/bin/gh-prs-merge.ts b/bin/gh-prs-merge.ts index a9d46ee..f4b78eb 100755 --- a/bin/gh-prs-merge.ts +++ b/bin/gh-prs-merge.ts @@ -48,6 +48,15 @@ Eligibility: - the head commit has not changed when the merge is submitted +Fork CI awaiting approval: + GitHub holds workflow runs on a first-time contributor's fork PR at + "action_required" until a maintainer approves them, and until then the PR + reports no checks at all. Such a PR is not a repo without CI. With --apply + its held pull_request runs are approved (from a fork they get a read-only + token and no secrets) and the PR is parked until they finish, within the + --fix-wait budget; a dry run only names them. Held runs on any other event + are reported as FIXME and left for a human. + Fixing (--fix): A skip is not always a verdict on the PR. Some are this tool arriving at the wrong moment. Requires --apply, since every repair writes. diff --git a/package.json b/package.json index c62b1d9..3d3a916 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/cli-tools", - "version": "0.66.0", + "version": "0.66.1", "private": true, "description": "Local command-line tools, in TypeScript, exposed on PATH.", "type": "module", diff --git a/src/gh.ts b/src/gh.ts index a9864e1..c6c7552 100644 --- a/src/gh.ts +++ b/src/gh.ts @@ -124,6 +124,35 @@ export function parseChecks(raw: unknown, where = 'gh pr checks'): Check[] { }); } +export interface WorkflowRun { + id: number; + name: string; + /** `pull_request` runs from a fork get a read-only token and no secrets. */ + event: string; +} + +export function parseRunsAwaitingApproval( + raw: unknown, + where = 'gh api actions/runs', +): WorkflowRun[] { + const record = (typeof raw === 'object' && raw !== null ? raw : {}) as Record; + if (!Array.isArray(record.workflow_runs)) { + throw new GhError(`${where}: expected workflow_runs, got ${JSON.stringify(raw).slice(0, 200)}`); + } + + return record.workflow_runs + .map((entry) => entry as Record) + .filter((run) => run.conclusion === 'action_required') + .map((run) => { + if (typeof run.id !== 'number') fail('id', run.id, where); + return { + id: run.id, + name: asString(run.name, 'name', where), + event: asString(run.event, 'event', where), + }; + }); +} + /** * The refusal GitHub returns from the GraphQL merge mutation for a pull * request that belongs to a stack. Matched as a substring because the rest of @@ -326,6 +355,42 @@ export class Gh { }); } + /** + * Workflow runs on this PR's head that GitHub is holding for a maintainer. + * + * A fork PR from a first-time contributor gets its runs created and then + * parked at conclusion `action_required` until someone clicks "Approve and + * run". Until then `gh pr checks` reports nothing at all, so the PR looks + * exactly like one in a repo with no CI — which is how a CVE bump with a + * perfectly good CI workflow sat skipped as "no CI checks found". + * + * Filtered here rather than with the API's `status=` parameter, because the + * one thing this method must never do is report "none" for a query GitHub + * read differently than we meant. + */ + async runsAwaitingApproval(url: string, headSha: string): Promise { + const target = parsePullRequestUrl(url); + if (!target) throw new GhError(`cannot read owner/repo from ${url}`); + + return this.json( + ['api', `repos/${target.slug}/actions/runs?head_sha=${headSha}&per_page=100`], + (raw) => parseRunsAwaitingApproval(raw), + ); + } + + async approveRun(url: string, runId: number): Promise { + const target = parsePullRequestUrl(url); + if (!target) { + return { code: 1, stdout: '', stderr: `cannot read owner/repo from ${url}` }; + } + return this.call([ + 'api', + '--method', + 'POST', + `repos/${target.slug}/actions/runs/${runId}/approve`, + ]); + } + async ready(url: string): Promise { return this.call(['pr', 'ready', url]); } diff --git a/src/prs-merge.ts b/src/prs-merge.ts index 8065703..7dff78e 100644 --- a/src/prs-merge.ts +++ b/src/prs-merge.ts @@ -1,4 +1,4 @@ -import { Gh, type Check, type MergeState, type PullRequest } from './gh.ts'; +import { Gh, type Check, type MergeState, type PullRequest, type WorkflowRun } from './gh.ts'; import { sleep } from './exec.ts'; export interface MergeOptions { @@ -136,6 +136,12 @@ interface Parked { title: string; /** Already counted in `fixed` — a branch update repaired it before parking. */ repaired: boolean; + /** + * Its fork CI was just approved. Runs take a moment to report a check, and + * until they do the PR still has none — which here means "not started yet", + * not "this repo has no CI". + */ + awaitingChecks?: boolean; } export async function sweep( @@ -284,6 +290,23 @@ export async function sweep( } const checks = await gh.checks(url); + + // No checks can mean no CI, or CI that GitHub is holding for a maintainer + // because the PR comes from a first-time contributor's fork. Only the + // second is a blocker this tool can clear. + if (checks.length === 0 && pr.state === 'OPEN' && !pr.isDraft) { + const outcome = await approveForkRuns(pr, options, gh, emit); + if (outcome === 'parked') { + summary.fixed += 1; + parked.push({ url, title: pr.title, repaired: true, awaitingChecks: true }); + continue; + } + if (outcome === 'skipped') { + summary.skipped += 1; + continue; + } + } + const reason = reasonNotMergeable(pr, checks, options.allowNoChecks); // Repair is for open PRs. A PR that was merged or closed between the search @@ -349,6 +372,86 @@ export async function sweep( return summary; } +/** + * Approve the fork workflow runs GitHub is holding on this PR. + * + * Only `pull_request` runs are approved. From a fork those get a read-only + * token and no secrets, so approving one spends runner minutes and nothing + * else; the merge that follows is still gated on what the runs report. Any + * other event is left for a human, since approving it is a trust decision. + * + * `none` — nothing held, judge the PR as usual. `parked` — approved, wait for + * the checks. `skipped` — held runs exist that this call did not approve. + */ +async function approveForkRuns( + pr: PullRequest, + options: MergeOptions, + gh: Gh, + emit: (line: Line) => void, +): Promise<'none' | 'parked' | 'skipped'> { + let held: WorkflowRun[]; + try { + held = await gh.runsAwaitingApproval(pr.url, pr.headRefOid); + } catch (error) { + emit({ + kind: 'warn', + text: `could not list workflow runs for ${pr.url} — ${ + error instanceof Error ? error.message : String(error) + }`, + }); + return 'none'; + } + + if (held.length === 0) return 'none'; + + const names = held.map((run) => run.name).join(', '); + const approvable = held.filter((run) => run.event === 'pull_request'); + const other = held.filter((run) => run.event !== 'pull_request'); + + if (other.length > 0) { + emit({ + kind: 'fixme', + url: pr.url, + text: `fork CI awaiting approval (${other + .map((run) => `${run.name} on ${run.event}`) + .join(', ')}); not a pull_request run, so approve it by hand`, + title: pr.title, + }); + return 'skipped'; + } + + if (!options.apply) { + emit({ + kind: 'skip', + url: pr.url, + reason: `fork CI awaiting maintainer approval (${names}); --apply approves and waits for it`, + title: pr.title, + }); + return 'skipped'; + } + + for (const run of approvable) { + const approved = await gh.approveRun(pr.url, run.id); + if (approved.code !== 0) { + const message = (approved.stderr.trim() || approved.stdout.trim()).replace(/\s+/g, ' '); + emit({ + kind: 'fixme', + url: pr.url, + text: `could not approve fork CI run ${run.name} (${run.id}): ${message}`, + title: pr.title, + }); + return 'skipped'; + } + } + + emit({ + kind: 'fixing', + url: pr.url, + text: `approved fork CI awaiting maintainer approval (${names}); parked for the second pass`, + }); + return 'parked'; +} + /** * Second pass: wait out the PRs whose checks were still running. * @@ -385,7 +488,7 @@ async function drainParked( continue; } - if (checks.some(isPending)) { + if (checks.some(isPending) || (item.awaitingChecks && checks.length === 0)) { stillRunning.push(item); continue; } diff --git a/test/prs-merge.test.ts b/test/prs-merge.test.ts index 71b0637..338f30d 100644 --- a/test/prs-merge.test.ts +++ b/test/prs-merge.test.ts @@ -4,6 +4,7 @@ import { parseChecks, parseMergeAsync, parsePullRequest, + parseRunsAwaitingApproval, parsePullRequestUrl, GhError, } from '../src/gh.ts'; @@ -44,6 +45,9 @@ function stubGh(options: { stacked?: boolean; /** How many polls the asynchronous merge takes before it reports merged. */ asyncPolls?: number; + /** Workflow runs GitHub is holding for maintainer approval on the head. */ + heldRuns?: { id: number; name: string; event: string }[]; + approveFails?: boolean; }) { const calls: string[] = []; let viewIndex = 0; @@ -121,6 +125,25 @@ function stubGh(options: { if (args[0] === 'pr' && args[1] === 'ready') return ok(''); + if (args[0] === 'api' && args.some((a) => a.includes('/actions/runs?head_sha='))) { + const approved = calls.some((c) => c.includes('/approve')); + const runs = approved ? [] : (options.heldRuns ?? []); + return ok( + JSON.stringify({ + workflow_runs: [ + { id: 1, name: 'already ran', event: 'push', conclusion: 'success' }, + ...runs.map((r) => ({ ...r, conclusion: 'action_required' })), + ], + }), + ); + } + + if (args[0] === 'api' && args.some((a) => a.endsWith('/approve'))) { + return options.approveFails + ? { code: 1, stdout: '', stderr: 'HTTP 403: Must have admin rights' } + : ok('{}'); + } + return ok(''); }; @@ -190,6 +213,9 @@ function stubFleet(prs: { url: string; createdAt: string; steps: StubStep[] }[]) if (args[0] === 'pr' && args[1] === 'update-branch') return ok('Updated branch'); if (args[0] === 'pr' && args[1] === 'merge') return ok('Merged'); if (args[0] === 'pr' && args[1] === 'ready') return ok(''); + if (args[0] === 'api' && args.some((a) => a.includes('/actions/runs?head_sha='))) { + return ok(JSON.stringify({ workflow_runs: [] })); + } return ok(''); }; @@ -728,3 +754,93 @@ describe('parseMergeAsync', () => { expect(parseMergeAsync('not json')).toBeUndefined(); }); }); + +describe('fork CI held for maintainer approval', () => { + const lines = () => { + const out: Line[] = []; + return { out, emit: (line: Line) => out.push(line) }; + }; + const held = [ + { id: 41, name: 'CI', event: 'pull_request' }, + { id: 42, name: 'scan', event: 'pull_request' }, + ]; + + it('keeps only runs concluded action_required', () => { + expect( + parseRunsAwaitingApproval({ + workflow_runs: [ + { id: 1, name: 'CI', event: 'pull_request', conclusion: 'action_required' }, + { id: 2, name: 'CI', event: 'push', conclusion: 'success' }, + { id: 3, name: 'CI', event: 'pull_request', conclusion: null }, + ], + }), + ).toEqual([{ id: 1, name: 'CI', event: 'pull_request' }]); + }); + + it('refuses a response with no workflow_runs rather than reading it as none', () => { + expect(() => parseRunsAwaitingApproval({ message: 'Not Found' })).toThrow(GhError); + }); + + it('approves the runs, waits for their checks, then merges', async () => { + const { gh, calls } = stubGh({ + heldRuns: held, + steps: [ + { checks: [], mergeStateStatus: 'UNSTABLE' }, + { checks: [], mergeStateStatus: 'UNSTABLE' }, + { checks: [{ name: 'lint', bucket: 'pending' }] }, + { checks: [{ name: 'lint', bucket: 'pass' }] }, + ], + }); + const { out, emit } = lines(); + const summary = await sweep(baseOptions(), gh, emit); + + expect(calls.filter((c) => c.endsWith('/approve'))).toEqual([ + 'api --method POST repos/acme/repo/actions/runs/41/approve', + 'api --method POST repos/acme/repo/actions/runs/42/approve', + ]); + expect(out.some((l) => l.kind === 'fixing' && /approved fork CI/.test(l.text))).toBe(true); + expect(summary).toMatchObject({ merged: 1, fixed: 1, skipped: 0 }); + }); + + it('only reports in a dry run', async () => { + const { gh, calls } = stubGh({ heldRuns: held, steps: [{ checks: [] }] }); + const { out, emit } = lines(); + const summary = await sweep(baseOptions({ apply: false }), gh, emit); + + expect(calls.some((c) => c.endsWith('/approve'))).toBe(false); + expect(render(out.find((l) => l.kind === 'skip')!)).toMatch( + /fork CI awaiting maintainer approval \(CI, scan\)/, + ); + expect(summary.skipped).toBe(1); + }); + + it('leaves non-pull_request runs to a human', async () => { + const { gh, calls } = stubGh({ + heldRuns: [{ id: 7, name: 'deploy', event: 'pull_request_target' }], + steps: [{ checks: [] }], + }); + const { out, emit } = lines(); + const summary = await sweep(baseOptions(), gh, emit); + + expect(calls.some((c) => c.endsWith('/approve'))).toBe(false); + expect(out.some((l) => l.kind === 'fixme' && /approve it by hand/.test(l.text))).toBe(true); + expect(summary).toMatchObject({ merged: 0, skipped: 1 }); + }); + + it('reports a refused approval instead of waiting on it', async () => { + const { gh } = stubGh({ heldRuns: held, approveFails: true, steps: [{ checks: [] }] }); + const { out, emit } = lines(); + const summary = await sweep(baseOptions(), gh, emit); + + expect(out.some((l) => l.kind === 'fixme' && /Must have admin rights/.test(l.text))).toBe(true); + expect(summary).toMatchObject({ merged: 0, skipped: 1 }); + }); + + it('still skips a PR in a repo with no CI at all', async () => { + const { gh } = stubGh({ steps: [{ checks: [] }] }); + const { out, emit } = lines(); + await sweep(baseOptions(), gh, emit); + + expect(out.find((l) => l.kind === 'skip')).toMatchObject({ reason: 'no CI checks found' }); + }); +});