diff --git a/bin/gh-prs-merge.ts b/bin/gh-prs-merge.ts index 350eb8f..a9d46ee 100755 --- a/bin/gh-prs-merge.ts +++ b/bin/gh-prs-merge.ts @@ -36,9 +36,16 @@ Options: Eligibility: - PR is open, and not a draft (or was successfully marked ready) - - mergeable is MERGEABLE and mergeStateStatus is CLEAN + - mergeable is MERGEABLE - at least one CI check exists, unless --allow-no-checks - every check is pass or skipping + - mergeStateStatus is CLEAN, HAS_HOOKS or UNSTABLE + + UNSTABLE means mergeable with a non-required context that is not green. + GitHub takes the merge, and the check rule above is the stricter test, so + refusing UNSTABLE only ever skipped PRs that were ready. With no checks to + read there is no second opinion and UNSTABLE is not accepted. + - the head commit has not changed when the merge is submitted Fixing (--fix): @@ -46,15 +53,22 @@ Fixing (--fix): wrong moment. Requires --apply, since every repair writes. Repaired: - checks still running waits for them to settle, up to --fix-wait - mergeStateStatus=BEHIND asks GitHub to merge the base branch in - mergeable=CONFLICTING same request; it succeeds when the base merely - moved underneath the branch + checks still running parks the PR and judges it in a second pass, so + the rest of the sweep merges now instead of + queueing behind one unfinished test suite + mergeStateStatus=BEHIND asks GitHub to merge the base branch in, then + parks it too, since the new head re-runs every check + + --fix-wait is the budget for the second pass as a whole, not per PR: the + parked PRs are polled together and they are all running their suites at the + same time anyway. Never repaired: - A conflict GitHub declines to merge is left alone and its message printed. - Resolving one means choosing between two authors' intent. A check that ran - and failed is a result, not an obstacle. + mergeStateStatus=DIRTY is GitHub's settled verdict that the branch + conflicts with its base. update-branch cannot merge over a conflict, so it + is not attempted; the PR is reported once as FIXME and left alone. + Resolving a conflict means choosing between two authors' intent. A check + that ran and failed is a result, not an obstacle. `; async function main(argv: string[]): Promise { diff --git a/node_modules b/node_modules new file mode 120000 index 0000000..3efdaf0 --- /dev/null +++ b/node_modules @@ -0,0 +1 @@ +/home/anthony/src/profullstack/cli-tools/node_modules \ No newline at end of file diff --git a/src/prs-merge.ts b/src/prs-merge.ts index 95fa8f4..52bc6db 100644 --- a/src/prs-merge.ts +++ b/src/prs-merge.ts @@ -1,4 +1,4 @@ -import { Gh, type Check, type PullRequest } from './gh.ts'; +import { Gh, type Check, type MergeState, type PullRequest } from './gh.ts'; import { sleep } from './exec.ts'; export interface MergeOptions { @@ -30,8 +30,8 @@ export type Line = | { kind: 'readied'; url: string; title: string } | { kind: 'would-ready'; url: string; title: string } | { kind: 'fixing'; url: string; text: string } - | { kind: 'waiting'; url: string; pending: number } - | { kind: 'fixme'; url: string; text: string } + | { kind: 'waiting'; parked: number; secondsLeft: number } + | { kind: 'fixme'; url: string; text: string; title: string } | { kind: 'skip'; url: string; reason: string; title: string } | { kind: 'failed'; url: string; reason: string } | { kind: 'warn'; text: string }; @@ -45,12 +45,33 @@ export const defaults = { const isBad = (check: Check): boolean => check.bucket !== 'pass' && check.bucket !== 'skipping'; +const isPending = (check: Check): boolean => check.bucket === 'pending'; + +/** + * Aggregate states in which GitHub will accept a squash merge. + * + * CLEAN is the obvious one. HAS_HOOKS is CLEAN with pre-receive hooks. UNSTABLE + * means mergeable with a non-required context that is not green — GitHub takes + * the merge, and the check gate below is the stricter test anyway, so treating + * UNSTABLE as a blocker only skipped PRs that were ready. That was the bug: a + * manifest refresh sat unmerged with two green checks and nothing wrong with it. + */ +const MERGE_STATE_OK: ReadonlySet = new Set([ + 'CLEAN', + 'HAS_HOOKS', + 'UNSTABLE', +]); + /** * Why this PR cannot be merged, or empty when it can. * * One function so `--fix` re-judges with the same rules rather than a copy of * them. In the bash version this logic was inline in the loop, which is why * adding a re-check meant duplicating it. + * + * Checks are judged before the aggregate state deliberately: when both are + * unhappy, the name of the red check is the useful sentence and + * `mergeStateStatus=UNSTABLE` is the useless one. */ export function reasonNotMergeable( pr: PullRequest, @@ -60,7 +81,6 @@ export function reasonNotMergeable( if (pr.state !== 'OPEN') return `state=${pr.state}`; if (pr.isDraft) return 'draft'; if (pr.mergeable !== 'MERGEABLE') return `mergeable=${pr.mergeable}`; - if (pr.mergeStateStatus !== 'CLEAN') return `mergeStateStatus=${pr.mergeStateStatus}`; if (checks.length === 0 && !allowNoChecks) return 'no CI checks found'; const bad = checks.filter(isBad); @@ -68,19 +88,51 @@ export function reasonNotMergeable( return `checks not green: ${bad.map((c) => `${c.name}=${c.bucket}`).join(', ')}`; } + // UNSTABLE is only trustworthy because the checks above were read and were + // green. With nothing to read there is no second opinion, so hold out for a + // state GitHub itself calls clean. + if (checks.length === 0 && pr.mergeStateStatus === 'UNSTABLE') { + return 'mergeStateStatus=UNSTABLE with no checks to confirm it'; + } + + if (!MERGE_STATE_OK.has(pr.mergeStateStatus)) { + return `mergeStateStatus=${pr.mergeStateStatus}`; + } + return ''; } -/** A blocker `--fix` is willing to act on. */ -export function isRepairable(pr: PullRequest, checks: Check[]): boolean { - if (checks.some((check) => check.bucket === 'pending')) return true; +/** + * Can `gh pr update-branch` help? + * + * It merges the base into the head, which is possible only when the two do not + * conflict. DIRTY is GitHub's settled verdict that they do, so asking anyway + * spends a round trip to be told "Cannot update PR branch due to conflicts" and + * then prints the refusal as though it were news. + */ +export function isUpdatable(pr: PullRequest): boolean { + if (pr.mergeStateStatus === 'DIRTY') return false; return ( pr.mergeStateStatus === 'BEHIND' || - pr.mergeStateStatus === 'DIRTY' || + pr.mergeStateStatus === 'UNKNOWN' || pr.mergeable === 'CONFLICTING' ); } +/** A blocker `--fix` is willing to act on. */ +export function isRepairable(pr: PullRequest, checks: Check[]): boolean { + if (checks.some(isPending)) return true; + return isUpdatable(pr); +} + +/** A PR set aside until its checks finish, so it does not hold up the sweep. */ +interface Parked { + url: string; + title: string; + /** Already counted in `fixed` — a branch update repaired it before parking. */ + repaired: boolean; +} + export async function sweep( options: MergeOptions, gh: Gh, @@ -135,13 +187,45 @@ export async function sweep( if (options.fix) { emit({ kind: 'mode', - text: 'MODE: FIX — repairable blockers will be repaired once, then re-judged.', + text: 'MODE: FIX — repairable blockers are repaired once; PRs whose checks ' + + 'are still running are parked and judged at the end.', }); } } else { emit({ kind: 'mode', text: 'MODE: DRY RUN — nothing will be merged. Add --apply to merge.' }); } + /** Judge a PR believed to be settled, and merge it when it is eligible. */ + const judgeAndMerge = async (pr: PullRequest, checks: Check[]): Promise => { + const reason = reasonNotMergeable(pr, checks, options.allowNoChecks); + + if (reason) { + emit({ kind: 'skip', url: pr.url, reason, title: pr.title }); + summary.skipped += 1; + return; + } + + emit({ kind: 'ready', url: pr.url, title: pr.title, checks: checks.length }); + summary.ready += 1; + + if (!options.apply) return; + + const merged = await gh.squashMerge(pr.url, pr.headRefOid); + if (merged.code === 0) { + emit({ kind: 'merged', url: pr.url }); + summary.merged += 1; + } else { + emit({ + kind: 'failed', + url: pr.url, + reason: `GitHub refused the merge: ${merged.stderr.trim() || merged.stdout.trim()}`, + }); + summary.failed += 1; + } + }; + + const parked: Parked[] = []; + for (const url of urls) { let pr: PullRequest; @@ -194,124 +278,154 @@ export async function sweep( } } - let checks = await gh.checks(url); - let reason = reasonNotMergeable(pr, checks, options.allowNoChecks); - - if (reason && options.fix && isRepairable(pr, checks)) { - const repaired = await repair({ url, checks, reason, options, gh, emit }); - - if (repaired) { - summary.fixed += 1; - try { - pr = await gh.pullRequest(url); - checks = await gh.checks(url); - reason = reasonNotMergeable(pr, checks, options.allowNoChecks); - } catch (error) { - emit({ - kind: 'warn', - text: `could not re-read ${url} after repair — ${ - error instanceof Error ? error.message : String(error) - }`, - }); + const checks = await gh.checks(url); + const reason = reasonNotMergeable(pr, checks, options.allowNoChecks); + + if (reason && options.fix) { + // Checks still running. The PR is not blocked, it is unfinished — the + // only defect is that we looked too early. Set it aside rather than + // standing here: every PR behind it in the sweep is merge-ready now and + // should not wait out someone else's test suite. + if (checks.some(isPending)) { + emit({ kind: 'fixing', url, text: 'checks still running; parked for the second pass' }); + parked.push({ url, title: pr.title, repaired: false }); + continue; + } + + if (isUpdatable(pr)) { + // Base branch moved. GitHub merges it in without a local checkout, and + // only when the result needs no human judgement. + emit({ kind: 'fixing', url, text: `${reason}; asking GitHub to merge the base branch in` }); + const updated = await gh.updateBranch(url); + + if (updated.code === 0) { + summary.fixed += 1; + // New head, so every check re-runs. Park it for the same reason. + parked.push({ url, title: pr.title, repaired: true }); + continue; } + + const message = (updated.stderr.trim() || updated.stdout.trim()).replace(/\s+/g, ' '); + emit({ + kind: 'fixme', + url, + text: `GitHub could not merge the base in: ${message}`, + title: pr.title, + }); + summary.skipped += 1; + continue; } - } - if (reason) { - emit({ kind: 'skip', url, reason, title: pr.title }); - summary.skipped += 1; - continue; + if (pr.mergeStateStatus === 'DIRTY') { + // A real conflict. Say so once — as FIXME, not as a second SKIP line + // repeating it — and leave it alone: choosing between two authors' + // intent is not a batch operation. + emit({ + kind: 'fixme', + url, + text: `${reason}; the branch conflicts with its base and needs resolving by hand`, + title: pr.title, + }); + summary.skipped += 1; + continue; + } } - emit({ kind: 'ready', url, title: pr.title, checks: checks.length }); - summary.ready += 1; - - if (!options.apply) continue; + await judgeAndMerge(pr, checks); + } - const merged = await gh.squashMerge(url, pr.headRefOid); - if (merged.code === 0) { - emit({ kind: 'merged', url }); - summary.merged += 1; - } else { - emit({ - kind: 'failed', - url, - reason: `GitHub refused the merge: ${merged.stderr.trim() || merged.stdout.trim()}`, - }); - summary.failed += 1; - } + if (parked.length > 0) { + await drainParked(parked, options, gh, emit, summary, judgeAndMerge); } return summary; } /** - * One repair attempt. Returns true when something was done. + * Second pass: wait out the PRs whose checks were still running. * - * Deliberately not a loop: if a repair did not make the PR mergeable, doing it - * again will not either, and a tool that keeps trying turns a sweep into an - * afternoon of API calls. + * The deadline is shared by the whole queue rather than spent per PR. Ten + * minutes each turns a sweep of five unfinished PRs into fifty minutes of + * sleeping, and they are all running their suites at the same time anyway. */ -async function repair(args: { - url: string; - checks: Check[]; - reason: string; - options: MergeOptions; - gh: Gh; - emit: (line: Line) => void; -}): Promise { - const { url, checks, reason, options, gh, emit } = args; - - // Checks still running. The PR is not blocked, it is unfinished — the only - // defect is that we looked too early. This is the common false skip. - if (checks.some((check) => check.bucket === 'pending')) { - emit({ - kind: 'fixing', - url, - text: `checks still running; waiting up to ${Math.round(options.fixWaitMs / 1000)}s`, - }); - await waitForChecks(url, options, gh, emit); - return true; - } - - // Base branch moved. GitHub merges it in without a local checkout, and only - // when the result needs no human judgement. - emit({ kind: 'fixing', url, text: `${reason}; asking GitHub to merge the base branch in` }); - - const updated = await gh.updateBranch(url); - - if (updated.code !== 0) { - // A real conflict. Print what GitHub said and leave it alone: choosing - // between two authors' intent is not a batch operation. - const message = (updated.stderr.trim() || updated.stdout.trim()).replace(/\s+/g, ' '); - emit({ kind: 'fixme', url, text: `GitHub could not merge the base in: ${message}` }); - return false; - } - - // New head, so every check re-runs. Waiting here is what makes the repair - // worth anything — otherwise the re-judge sees a pending suite and skips for - // the very reason we just set in motion. - await waitForChecks(url, options, gh, emit); - return true; -} - -async function waitForChecks( - url: string, +async function drainParked( + queue: Parked[], options: MergeOptions, gh: Gh, emit: (line: Line) => void, + summary: Summary, + judgeAndMerge: (pr: PullRequest, checks: Check[]) => Promise, ): Promise { const deadline = Date.now() + options.fixWaitMs; + let remaining = queue; for (;;) { - const checks = await gh.checks(url); - const pending = checks.filter((check) => check.bucket === 'pending').length; + const stillRunning: Parked[] = []; - if (pending === 0) return; - if (Date.now() >= deadline) return; + for (const item of remaining) { + let checks: Check[]; + try { + checks = await gh.checks(item.url); + } catch (error) { + emit({ + kind: 'warn', + text: `could not re-read checks for ${item.url} — ${ + error instanceof Error ? error.message : String(error) + }`, + }); + stillRunning.push(item); + continue; + } + + if (checks.some(isPending)) { + stillRunning.push(item); + continue; + } + + // Settled. Re-read the PR, because the run that just finished may have + // moved the aggregate state as well as the checks. A PR whose branch was + // updated first is already counted; waiting it out is the same repair + // seen through, not a second one. + if (!item.repaired) summary.fixed += 1; + try { + const pr = await gh.pullRequest(item.url); + await judgeAndMerge(pr, checks); + } catch (error) { + emit({ + kind: 'skip', + url: item.url, + reason: `could not re-read PR metadata after its checks finished — ${ + error instanceof Error ? error.message : String(error) + }`, + title: item.title, + }); + summary.skipped += 1; + } + } + + remaining = stillRunning; + if (remaining.length === 0) return; - emit({ kind: 'waiting', url, pending }); - await sleep(options.pollMs); + const msLeft = deadline - Date.now(); + if (msLeft <= 0) { + for (const item of remaining) { + emit({ + kind: 'skip', + url: item.url, + reason: `checks still running after ${Math.round(options.fixWaitMs / 1000)}s`, + title: item.title, + }); + summary.skipped += 1; + } + return; + } + + emit({ + kind: 'waiting', + parked: remaining.length, + secondsLeft: Math.round(msLeft / 1000), + }); + await sleep(Math.min(options.pollMs, msLeft)); } } @@ -330,9 +444,9 @@ export function render(line: Line): string { case 'fixing': return `FIXING ${line.url} — ${line.text}`; case 'waiting': - return ` … ${line.pending} check(s) still running on ${line.url}; waiting`; + return ` … ${line.parked} PR(s) still running checks; ${line.secondsLeft}s left`; case 'fixme': - return `FIXME ${line.url} — ${line.text}`; + return `FIXME ${line.url} — ${line.text} — ${line.title}`; case 'skip': return `SKIP ${line.url} — ${line.reason} — ${line.title}`; case 'failed': diff --git a/test/prs-merge.test.ts b/test/prs-merge.test.ts index 0274ed0..0051a5f 100644 --- a/test/prs-merge.test.ts +++ b/test/prs-merge.test.ts @@ -11,6 +11,7 @@ import type { RunResult } from '../src/exec.ts'; import { defaults, isRepairable, + isUpdatable, reasonNotMergeable, render, sweep, @@ -126,6 +127,76 @@ function stubGh(options: { return { gh: new Gh({ exec }), calls }; } +/** + * A scripted `gh` serving several PRs at once. + * + * `stubGh` answers for exactly one URL, which cannot express the thing the + * second pass exists for: whether a PR whose checks are still running holds up + * the PRs behind it. + */ +function stubFleet(prs: { url: string; createdAt: string; steps: StubStep[] }[]) { + const calls: string[] = []; + const cursors = new Map(); + + const find = (args: readonly string[]) => { + const url = args.find((a) => a.includes('/pull/')); + const entry = prs.find((p) => p.url === url); + if (!entry) throw new Error(`stubFleet: no PR for ${String(url)}`); + const cursor = cursors.get(entry.url) ?? { view: 0, check: 0 }; + cursors.set(entry.url, cursor); + return { entry, cursor }; + }; + + const at = (entry: { steps: StubStep[] }, index: number): StubStep => + entry.steps[Math.min(index, entry.steps.length - 1)]!; + + const exec = async (_file: string, args: readonly string[]): Promise => { + calls.push(args.join(' ')); + const ok = (stdout: string): RunResult => ({ code: 0, stdout, stderr: '' }); + + if (args[0] === 'search') { + return ok(JSON.stringify(prs.map((p) => ({ url: p.url, createdAt: p.createdAt })))); + } + + if (args[0] === 'pr' && args[1] === 'view') { + const { entry, cursor } = find(args); + const current = at(entry, cursor.view); + cursor.view += 1; + return ok( + JSON.stringify({ + url: entry.url, + title: `stub ${entry.url}`, + state: current.state ?? 'OPEN', + isDraft: current.isDraft ?? false, + mergeable: current.mergeable ?? 'MERGEABLE', + mergeStateStatus: current.mergeStateStatus ?? 'CLEAN', + headRefOid: 'deadbeef', + }), + ); + } + + if (args[0] === 'pr' && args[1] === 'checks') { + const { entry, cursor } = find(args); + const current = at(entry, cursor.check); + cursor.check += 1; + const checks = current.checks ?? [{ name: 'test', bucket: 'pass' }]; + return { + code: checks.some((c) => c.bucket !== 'pass') ? 1 : 0, + stdout: JSON.stringify(checks), + stderr: '', + }; + } + + 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(''); + + return ok(''); + }; + + return { gh: new Gh({ exec }), calls }; +} + function baseOptions(overrides: Partial = {}): MergeOptions { return { orgs: ['acme'], @@ -168,8 +239,8 @@ describe('eligibility rules', () => { expect(reasonNotMergeable({ ...pr, mergeable: 'CONFLICTING' }, [], false)).toBe( 'mergeable=CONFLICTING', ); - expect(reasonNotMergeable({ ...pr, mergeStateStatus: 'UNSTABLE' }, [], false)).toBe( - 'mergeStateStatus=UNSTABLE', + expect(reasonNotMergeable({ ...pr, mergeStateStatus: 'BLOCKED' }, [], true)).toBe( + 'mergeStateStatus=BLOCKED', ); expect(reasonNotMergeable(pr, [], false)).toBe('no CI checks found'); expect(reasonNotMergeable(pr, [{ name: 'test', bucket: 'fail' }], false)).toBe( @@ -191,6 +262,39 @@ describe('eligibility rules', () => { // A check that ran and failed is a result, not an obstacle. expect(isRepairable(pr, [{ name: 'a', bucket: 'fail' }])).toBe(false); }); + + it('merges UNSTABLE when the checks it can read are green', () => { + // UNSTABLE is "mergeable, some non-required context is not green". GitHub + // takes the merge; demanding CLEAN skipped ready PRs for nothing. + const unstable = { ...pr, mergeStateStatus: 'UNSTABLE' } as const; + expect(reasonNotMergeable(unstable, [{ name: 'test', bucket: 'pass' }], false)).toBe(''); + expect(reasonNotMergeable({ ...pr, mergeStateStatus: 'HAS_HOOKS' }, [ + { name: 'test', bucket: 'pass' }, + ], false)).toBe(''); + + // But it is only trustworthy because the checks were read. With none to + // read, nothing corroborates it. + expect(reasonNotMergeable(unstable, [], true)).toContain('no checks to confirm it'); + }); + + it('names the red check rather than the aggregate it explains', () => { + // Both are unhappy at once; "socket=fail" is the sentence worth printing. + expect( + reasonNotMergeable({ ...pr, mergeStateStatus: 'UNSTABLE' }, [ + { name: 'socket', bucket: 'fail' }, + ], false), + ).toBe('checks not green: socket=fail'); + }); + + it('will not ask update-branch to merge over a settled conflict', () => { + // DIRTY is GitHub having already decided the two sides conflict. The call + // can only come back "Cannot update PR branch due to conflicts". + expect(isUpdatable({ ...pr, mergeable: 'CONFLICTING', mergeStateStatus: 'DIRTY' })).toBe(false); + expect(isRepairable({ ...pr, mergeable: 'CONFLICTING', mergeStateStatus: 'DIRTY' }, [])).toBe( + false, + ); + expect(isUpdatable({ ...pr, mergeStateStatus: 'BEHIND' })).toBe(true); + }); }); describe('gh response validation', () => { @@ -263,20 +367,48 @@ describe('sweep', () => { expect(calls.some((c) => c.startsWith('pr update-branch'))).toBe(true); }); - it('refuses to guess at a conflict GitHub will not merge', async () => { + it('reports a settled conflict once, without asking GitHub to merge over it', async () => { const { gh, calls } = stubGh({ steps: [{ mergeable: 'CONFLICTING', mergeStateStatus: 'DIRTY' }], updateBranchFails: true, }); - const { summary, text } = await runSweep(gh, baseOptions({ fix: true })); + const { summary, lines, text } = await runSweep(gh, baseOptions({ fix: true })); expect(text).toContain('FIXME'); - expect(text).toContain('Cannot update PR branch due to conflicts'); + expect(text).toContain('mergeable=CONFLICTING'); + expect(text).toContain('needs resolving by hand'); expect(summary.merged).toBe(0); expect(summary.skipped).toBe(1); // Repair did not succeed, so it is not counted as one. expect(summary.fixed).toBe(0); expect(calls.some((c) => c.startsWith('pr merge'))).toBe(false); + + // The wasted round trip is gone, and so is the SKIP line that used to + // repeat the FIXME immediately underneath it. + expect(calls.some((c) => c.startsWith('pr update-branch'))).toBe(false); + expect(lines.filter((l) => l.kind === 'skip')).toHaveLength(0); + }); + + it('merges a PR GitHub calls UNSTABLE when every check is green', async () => { + // The false skip this sweep kept producing: MERGEABLE, two green checks, + // and an aggregate state that only meant a non-required context was not. + const { gh, calls } = stubGh({ + steps: [ + { + mergeStateStatus: 'UNSTABLE', + checks: [ + { name: 'test', bucket: 'pass' }, + { name: 'lint', bucket: 'pass' }, + ], + }, + ], + }); + const { summary, text } = await runSweep(gh, baseOptions()); + + expect(summary.merged).toBe(1); + expect(summary.skipped).toBe(0); + expect(text).not.toContain('mergeStateStatus=UNSTABLE'); + expect(calls.some((c) => c.startsWith('pr merge'))).toBe(true); }); it('waits for a running check, then merges once it goes green', async () => { @@ -294,6 +426,63 @@ describe('sweep', () => { expect(summary.merged).toBe(1); }); + it('merges the rest of the sweep instead of queueing behind a running suite', async () => { + // The sweep used to stand in front of the first unfinished PR for the + // whole of --fix-wait. Every PR after it was ready and waited anyway. + const slow = 'https://github.com/acme/repo/pull/1'; + const quick = 'https://github.com/acme/repo/pull/2'; + + const { gh, calls } = stubFleet([ + { + url: slow, + createdAt: '2026-01-01T00:00:00Z', + steps: [ + { mergeStateStatus: 'UNSTABLE', checks: [{ name: 'test', bucket: 'pending' }] }, + { mergeStateStatus: 'CLEAN', checks: [{ name: 'test', bucket: 'pass' }] }, + ], + }, + { url: quick, createdAt: '2026-01-02T00:00:00Z', steps: [{}] }, + ]); + + const { summary, text } = await runSweep(gh, baseOptions({ fix: true })); + + expect(summary.merged).toBe(2); + expect(text).toContain('parked for the second pass'); + + const mergeOrder = calls.filter((c) => c.startsWith('pr merge')); + expect(mergeOrder).toHaveLength(2); + // The ready PR goes first even though the slow one was swept first. + expect(mergeOrder[0]).toContain(quick); + expect(mergeOrder[1]).toContain(slow); + }); + + it('spends --fix-wait on the parked queue as a whole, not per PR', async () => { + // Two PRs, both stuck pending, one second of budget between them. Serially + // this was two full waits; the deadline is shared, so it is one. + const { gh } = stubFleet([ + { + url: 'https://github.com/acme/repo/pull/1', + createdAt: '2026-01-01T00:00:00Z', + steps: [{ mergeStateStatus: 'UNSTABLE', checks: [{ name: 'a', bucket: 'pending' }] }], + }, + { + url: 'https://github.com/acme/repo/pull/2', + createdAt: '2026-01-02T00:00:00Z', + steps: [{ mergeStateStatus: 'UNSTABLE', checks: [{ name: 'b', bucket: 'pending' }] }], + }, + ]); + + const started = Date.now(); + const { summary, text } = await runSweep( + gh, + baseOptions({ fix: true, fixWaitMs: 60, pollMs: 10 }), + ); + + expect(Date.now() - started).toBeLessThan(500); + expect(summary.skipped).toBe(2); + expect(text).toContain('checks still running after'); + }); + it('gives up waiting at --fix-wait rather than hanging', async () => { const { gh } = stubGh({ steps: [{ mergeStateStatus: 'UNSTABLE', checks: [{ name: 'x', bucket: 'pending' }] }],