From 13647a42252eaa6fe170d845c67af7e3506f4771 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Thu, 24 Sep 2026 07:56:50 +0000 Subject: [PATCH] gh-prs-merge: merge stacked PRs through the asynchronous merge endpoint (0.45.2) `gh pr merge` calls the GraphQL mergePullRequest mutation, and GitHub refuses that mutation outright for a pull request that belongs to a stack, naming the asynchronous merge REST API instead. Nothing is wrong with such a PR: it reads MERGEABLE and CLEAN with every check green, so no repair applies and the sweep just reported FAILED and merged nothing. `Gh.squashMerge` now falls back to PUT /repos/{owner}/{repo}/pulls/{n}/merge-async and polls the uuid it returns until the merge settles. The fallback is narrow on purpose: it fires only on that one refusal message, still pins the head commit, still asks for a squash, and still passes no --admin, so a PR is held to exactly the rules it was held to before. A PR the endpoint hands to a merge queue reports enqueued, which counts as merged because nothing further is ours to do. The refusal is matched on the message rather than on the base branch: GitHub still calls a PR stacked after it has retargeted it onto the default branch, which is how profullstack/hqtui#96 behaved in practice. The shell original in profullstack/scripts got the same fix (v0.3.2) and is being retired in favour of this one. Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 2 +- src/gh.ts | 131 +++++++++++++++++++++++++++++++- test/prs-merge.test.ts | 169 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 298 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 0965cfb..67b28b0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/cli-tools", - "version": "0.45.1", + "version": "0.45.2", "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 9611b39..94c301b 100644 --- a/src/gh.ts +++ b/src/gh.ts @@ -124,6 +124,52 @@ export function parseChecks(raw: unknown, where = 'gh pr checks'): Check[] { }); } +/** + * 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 + * the sentence carries a docs URL that is not ours to depend on. + */ +export const STACK_REFUSAL = 'asynchronous merge REST API'; + +/** `https://github.com/owner/repo/pull/7` → `{ slug: 'owner/repo', number: 7 }`. */ +export function parsePullRequestUrl( + url: string, +): { slug: string; number: string } | undefined { + const match = /github\.com\/([^/]+\/[^/]+)\/pull\/(\d+)/.exec(url); + return match ? { slug: match[1]!, number: match[2]! } : undefined; +} + +export interface MergeAsyncState { + status?: string | undefined; + uuid?: string | undefined; + message?: string | undefined; +} + +/** + * Read one asynchronous-merge reply. Unparseable output is not an error here: + * the caller treats a missing uuid as "stop polling" and reports that the + * merge never settled, which is truer than inventing a status. + */ +export function parseMergeAsync(stdout: string): MergeAsyncState | undefined { + let raw: unknown; + try { + raw = JSON.parse(stdout); + } catch { + return undefined; + } + + if (typeof raw !== 'object' || raw === null) return undefined; + + const record = raw as Record; + const details = (record.details ?? {}) as Record; + + return { + status: typeof record.status === 'string' ? record.status : undefined, + uuid: typeof details.uuid === 'string' ? details.uuid : undefined, + message: typeof details.message === 'string' ? details.message : undefined, + }; +} + export interface GhOptions { /** Swap in a fake for tests. */ exec?: typeof run; @@ -291,8 +337,89 @@ export class Gh { * lands on a commit nothing verified. * * Deliberately no `--admin`. Branch protections stay enforced. + * + * `gh pr merge` calls the GraphQL mergePullRequest mutation, and GitHub + * refuses that mutation outright for a PR that belongs to a stack, naming + * the asynchronous merge REST endpoint instead. Nothing is wrong with such + * a PR — it reports MERGEABLE and CLEAN — so there is no repair to attempt + * and no honest way to call it a refusal. Fall back to that endpoint. */ - async squashMerge(url: string, headSha: string): Promise { - return this.call(['pr', 'merge', url, '--squash', '--match-head-commit', headSha]); + async squashMerge( + url: string, + headSha: string, + { pollMs = 2_000, attempts = 30 }: { pollMs?: number; attempts?: number } = {}, + ): Promise { + const direct = await this.call([ + 'pr', + 'merge', + url, + '--squash', + '--match-head-commit', + headSha, + ]); + + if (direct.code === 0) return direct; + + // Keyed on the message rather than on the base branch: GitHub still calls + // a PR stacked after it has retargeted it onto the default branch. + if (!`${direct.stderr}${direct.stdout}`.includes(STACK_REFUSAL)) return direct; + + return this.mergeAsync(url, headSha, { pollMs, attempts }); + } + + /** + * The asynchronous merge endpoint enqueues the squash and hands back a uuid + * to poll. The head stays pinned and no `--admin` is passed, so the PR is + * held to exactly the rules it would have been held to above. + */ + private async mergeAsync( + url: string, + headSha: string, + { pollMs, attempts }: { pollMs: number; attempts: number }, + ): Promise { + const target = parsePullRequestUrl(url); + if (!target) { + return { code: 1, stdout: '', stderr: `cannot read owner/repo from ${url}` }; + } + + const endpoint = `repos/${target.slug}/pulls/${target.number}/merge-async`; + + let response = await this.call([ + 'api', + '--method', + 'PUT', + endpoint, + '-f', + 'merge_method=squash', + '-f', + `sha=${headSha}`, + ]); + + for (let attempt = 0; attempt < attempts; attempt += 1) { + if (response.code !== 0) return response; + + const state = parseMergeAsync(response.stdout); + + // `enqueued` means a merge queue owns it from here, which is as merged + // as this tool can make it. + if (state?.status === 'merged' || state?.status === 'enqueued') { + return { + code: 0, + stdout: `Merged ${url} via the asynchronous merge API`, + stderr: '', + }; + } + + if (state?.status === 'failed') { + return { code: 1, stdout: '', stderr: state.message ?? 'async merge failed' }; + } + + if (!state?.uuid) break; + + await sleep(pollMs); + response = await this.call(['api', `${endpoint}/${state.uuid}`]); + } + + return { code: 1, stdout: '', stderr: `async merge never settled for ${url}` }; } } diff --git a/test/prs-merge.test.ts b/test/prs-merge.test.ts index cbd7c70..0274ed0 100644 --- a/test/prs-merge.test.ts +++ b/test/prs-merge.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { Gh, parseChecks, parsePullRequest, GhError } from '../src/gh.ts'; +import { + Gh, + parseChecks, + parseMergeAsync, + parsePullRequest, + parsePullRequestUrl, + GhError, +} from '../src/gh.ts'; import type { RunResult } from '../src/exec.ts'; import { defaults, @@ -32,10 +39,15 @@ function stubGh(options: { steps: StubStep[]; updateBranchFails?: boolean; mergeFails?: boolean; + /** Refuse `pr merge` the way GitHub refuses a PR that is part of a stack. */ + stacked?: boolean; + /** How many polls the asynchronous merge takes before it reports merged. */ + asyncPolls?: number; }) { const calls: string[] = []; let viewIndex = 0; let checkIndex = 0; + let asyncPolls = options.asyncPolls ?? 0; const step = (index: number): StubStep => options.steps[Math.min(index, options.steps.length - 1)]!; @@ -83,11 +95,29 @@ function stubGh(options: { } if (args[0] === 'pr' && args[1] === 'merge') { + if (options.stacked) { + return { + code: 1, + stdout: '', + stderr: + 'GraphQL: This pull request is part of a stack and must be merged ' + + 'using the asynchronous merge REST API. (mergePullRequest)', + }; + } return options.mergeFails ? { code: 1, stdout: '', stderr: 'refused' } : ok('Merged'); } + // The asynchronous merge endpoint: the PUT enqueues, each GET polls. + if (args[0] === 'api' && args.some((a) => a.includes('merge-async'))) { + if (asyncPolls > 0) { + asyncPolls -= 1; + return ok(JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } })); + } + return ok(JSON.stringify({ status: 'merged', details: { sha: 'cafe' } })); + } + if (args[0] === 'pr' && args[1] === 'ready') return ok(''); return ok(''); @@ -282,6 +312,26 @@ describe('sweep', () => { expect(summary.merged).toBe(0); }); + it('merges a stacked PR through the asynchronous merge endpoint', async () => { + const { gh, calls } = stubGh({ steps: [{}], stacked: true }); + const { summary } = await runSweep(gh, baseOptions()); + + expect(summary.merged).toBe(1); + expect(summary.failed).toBe(0); + expect(calls.some((c) => c.includes('merge-async'))).toBe(true); + }); + + it('pins the head commit on the asynchronous merge too', async () => { + const { gh, calls } = stubGh({ steps: [{}], stacked: true }); + await runSweep(gh, baseOptions()); + + const put = calls.find((c) => c.startsWith('api --method PUT')); + expect(put).toContain('sha=deadbeef'); + expect(put).toContain('merge_method=squash'); + // Still no admin override on this path. + expect(calls.some((c) => c.includes('--admin'))).toBe(false); + }); + it('marks a draft ready, then judges it normally', async () => { const { gh, calls } = stubGh({ steps: [{ isDraft: true }, { isDraft: false }] }); const { summary } = await runSweep(gh, baseOptions()); @@ -320,3 +370,120 @@ describe('defaults', () => { expect(defaults.pollMs).toBe(20_000); }); }); + +describe('asynchronous merge', () => { + const PR = 'https://github.com/acme/repo/pull/7'; + + /** A `gh` that refuses the mutation, then answers the REST endpoint. */ + function stubAsync(replies: string[], { refuse = true } = {}) { + const calls: string[] = []; + let index = 0; + + const exec = async (_f: string, args: readonly string[]): Promise => { + calls.push(args.join(' ')); + + if (args[0] === 'pr' && args[1] === 'merge') { + return refuse + ? { + code: 1, + stdout: '', + stderr: 'must be merged using the asynchronous merge REST API. (mergePullRequest)', + } + : { code: 0, stdout: 'Merged', stderr: '' }; + } + + const reply = replies[Math.min(index, replies.length - 1)]!; + index += 1; + return { code: 0, stdout: reply, stderr: '' }; + }; + + return { gh: new Gh({ exec }), calls }; + } + + it('polls the uuid until the merge settles', async () => { + const { gh, calls } = stubAsync([ + JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } }), + JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } }), + JSON.stringify({ status: 'merged', details: { sha: 'cafe' } }), + ]); + + const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 }); + + expect(result.code).toBe(0); + expect(calls.filter((c) => c.startsWith('api repos/acme/repo/pulls/7/merge-async/u-1'))) + .toHaveLength(2); + }); + + it('treats enqueued as merged, because a merge queue owns it from there', async () => { + const { gh } = stubAsync([JSON.stringify({ status: 'enqueued', details: { uuid: 'u-1' } })]); + const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 }); + + expect(result.code).toBe(0); + }); + + it('reports a failed asynchronous merge with the reason GitHub gave', async () => { + const { gh } = stubAsync([ + JSON.stringify({ status: 'failed', details: { message: 'head moved' } }), + ]); + const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 }); + + expect(result.code).toBe(1); + expect(result.stderr).toContain('head moved'); + }); + + it('gives up rather than polling forever', async () => { + const { gh } = stubAsync([JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } })]); + const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1, attempts: 3 }); + + expect(result.code).toBe(1); + expect(result.stderr).toContain('never settled'); + }); + + it('does not reach for the endpoint when the refusal is a different one', async () => { + const calls: string[] = []; + const gh = new Gh({ + exec: async (_f, args) => { + calls.push(args.join(' ')); + return { code: 1, stdout: '', stderr: 'Pull request is not mergeable' }; + }, + }); + + const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 }); + + expect(result.code).toBe(1); + expect(calls.some((c) => c.includes('merge-async'))).toBe(false); + }); + + it('never merges twice when the mutation already worked', async () => { + const { gh, calls } = stubAsync([], { refuse: false }); + const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 }); + + expect(result.code).toBe(0); + expect(calls.some((c) => c.includes('merge-async'))).toBe(false); + }); +}); + +describe('parsePullRequestUrl', () => { + it('reads owner, repo and number', () => { + expect(parsePullRequestUrl('https://github.com/acme/repo/pull/7')).toEqual({ + slug: 'acme/repo', + number: '7', + }); + }); + + it('returns nothing for something that is not a PR url', () => { + expect(parsePullRequestUrl('https://example.com/acme/repo')).toBeUndefined(); + }); +}); + +describe('parseMergeAsync', () => { + it('lifts status, uuid and message out of the reply', () => { + expect( + parseMergeAsync(JSON.stringify({ status: 'failed', details: { message: 'no' } })), + ).toEqual({ status: 'failed', uuid: undefined, message: 'no' }); + }); + + it('returns nothing rather than inventing a status', () => { + expect(parseMergeAsync('not json')).toBeUndefined(); + }); +});