Skip to content

Commit 9e57cd4

Browse files
ralyodioclaude
andauthored
gh-prs-merge: merge stacked PRs through the asynchronous merge endpoint (0.45.2) (#87)
`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) <noreply@anthropic.com>
1 parent b2dd07b commit 9e57cd4

3 files changed

Lines changed: 298 additions & 4 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@profullstack/cli-tools",
3-
"version": "0.45.1",
3+
"version": "0.45.2",
44
"private": true,
55
"description": "Local command-line tools, in TypeScript, exposed on PATH.",
66
"type": "module",

‎src/gh.ts‎

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,52 @@ export function parseChecks(raw: unknown, where = 'gh pr checks'): Check[] {
124124
});
125125
}
126126

127+
/**
128+
* The refusal GitHub returns from the GraphQL merge mutation for a pull
129+
* request that belongs to a stack. Matched as a substring because the rest of
130+
* the sentence carries a docs URL that is not ours to depend on.
131+
*/
132+
export const STACK_REFUSAL = 'asynchronous merge REST API';
133+
134+
/** `https://github.com/owner/repo/pull/7` → `{ slug: 'owner/repo', number: 7 }`. */
135+
export function parsePullRequestUrl(
136+
url: string,
137+
): { slug: string; number: string } | undefined {
138+
const match = /github\.com\/([^/]+\/[^/]+)\/pull\/(\d+)/.exec(url);
139+
return match ? { slug: match[1]!, number: match[2]! } : undefined;
140+
}
141+
142+
export interface MergeAsyncState {
143+
status?: string | undefined;
144+
uuid?: string | undefined;
145+
message?: string | undefined;
146+
}
147+
148+
/**
149+
* Read one asynchronous-merge reply. Unparseable output is not an error here:
150+
* the caller treats a missing uuid as "stop polling" and reports that the
151+
* merge never settled, which is truer than inventing a status.
152+
*/
153+
export function parseMergeAsync(stdout: string): MergeAsyncState | undefined {
154+
let raw: unknown;
155+
try {
156+
raw = JSON.parse(stdout);
157+
} catch {
158+
return undefined;
159+
}
160+
161+
if (typeof raw !== 'object' || raw === null) return undefined;
162+
163+
const record = raw as Record<string, unknown>;
164+
const details = (record.details ?? {}) as Record<string, unknown>;
165+
166+
return {
167+
status: typeof record.status === 'string' ? record.status : undefined,
168+
uuid: typeof details.uuid === 'string' ? details.uuid : undefined,
169+
message: typeof details.message === 'string' ? details.message : undefined,
170+
};
171+
}
172+
127173
export interface GhOptions {
128174
/** Swap in a fake for tests. */
129175
exec?: typeof run;
@@ -291,8 +337,89 @@ export class Gh {
291337
* lands on a commit nothing verified.
292338
*
293339
* Deliberately no `--admin`. Branch protections stay enforced.
340+
*
341+
* `gh pr merge` calls the GraphQL mergePullRequest mutation, and GitHub
342+
* refuses that mutation outright for a PR that belongs to a stack, naming
343+
* the asynchronous merge REST endpoint instead. Nothing is wrong with such
344+
* a PR — it reports MERGEABLE and CLEAN — so there is no repair to attempt
345+
* and no honest way to call it a refusal. Fall back to that endpoint.
294346
*/
295-
async squashMerge(url: string, headSha: string): Promise<RunResult> {
296-
return this.call(['pr', 'merge', url, '--squash', '--match-head-commit', headSha]);
347+
async squashMerge(
348+
url: string,
349+
headSha: string,
350+
{ pollMs = 2_000, attempts = 30 }: { pollMs?: number; attempts?: number } = {},
351+
): Promise<RunResult> {
352+
const direct = await this.call([
353+
'pr',
354+
'merge',
355+
url,
356+
'--squash',
357+
'--match-head-commit',
358+
headSha,
359+
]);
360+
361+
if (direct.code === 0) return direct;
362+
363+
// Keyed on the message rather than on the base branch: GitHub still calls
364+
// a PR stacked after it has retargeted it onto the default branch.
365+
if (!`${direct.stderr}${direct.stdout}`.includes(STACK_REFUSAL)) return direct;
366+
367+
return this.mergeAsync(url, headSha, { pollMs, attempts });
368+
}
369+
370+
/**
371+
* The asynchronous merge endpoint enqueues the squash and hands back a uuid
372+
* to poll. The head stays pinned and no `--admin` is passed, so the PR is
373+
* held to exactly the rules it would have been held to above.
374+
*/
375+
private async mergeAsync(
376+
url: string,
377+
headSha: string,
378+
{ pollMs, attempts }: { pollMs: number; attempts: number },
379+
): Promise<RunResult> {
380+
const target = parsePullRequestUrl(url);
381+
if (!target) {
382+
return { code: 1, stdout: '', stderr: `cannot read owner/repo from ${url}` };
383+
}
384+
385+
const endpoint = `repos/${target.slug}/pulls/${target.number}/merge-async`;
386+
387+
let response = await this.call([
388+
'api',
389+
'--method',
390+
'PUT',
391+
endpoint,
392+
'-f',
393+
'merge_method=squash',
394+
'-f',
395+
`sha=${headSha}`,
396+
]);
397+
398+
for (let attempt = 0; attempt < attempts; attempt += 1) {
399+
if (response.code !== 0) return response;
400+
401+
const state = parseMergeAsync(response.stdout);
402+
403+
// `enqueued` means a merge queue owns it from here, which is as merged
404+
// as this tool can make it.
405+
if (state?.status === 'merged' || state?.status === 'enqueued') {
406+
return {
407+
code: 0,
408+
stdout: `Merged ${url} via the asynchronous merge API`,
409+
stderr: '',
410+
};
411+
}
412+
413+
if (state?.status === 'failed') {
414+
return { code: 1, stdout: '', stderr: state.message ?? 'async merge failed' };
415+
}
416+
417+
if (!state?.uuid) break;
418+
419+
await sleep(pollMs);
420+
response = await this.call(['api', `${endpoint}/${state.uuid}`]);
421+
}
422+
423+
return { code: 1, stdout: '', stderr: `async merge never settled for ${url}` };
297424
}
298425
}

‎test/prs-merge.test.ts‎

Lines changed: 168 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import { describe, expect, it } from 'vitest';
2-
import { Gh, parseChecks, parsePullRequest, GhError } from '../src/gh.ts';
2+
import {
3+
Gh,
4+
parseChecks,
5+
parseMergeAsync,
6+
parsePullRequest,
7+
parsePullRequestUrl,
8+
GhError,
9+
} from '../src/gh.ts';
310
import type { RunResult } from '../src/exec.ts';
411
import {
512
defaults,
@@ -32,10 +39,15 @@ function stubGh(options: {
3239
steps: StubStep[];
3340
updateBranchFails?: boolean;
3441
mergeFails?: boolean;
42+
/** Refuse `pr merge` the way GitHub refuses a PR that is part of a stack. */
43+
stacked?: boolean;
44+
/** How many polls the asynchronous merge takes before it reports merged. */
45+
asyncPolls?: number;
3546
}) {
3647
const calls: string[] = [];
3748
let viewIndex = 0;
3849
let checkIndex = 0;
50+
let asyncPolls = options.asyncPolls ?? 0;
3951

4052
const step = (index: number): StubStep =>
4153
options.steps[Math.min(index, options.steps.length - 1)]!;
@@ -83,11 +95,29 @@ function stubGh(options: {
8395
}
8496

8597
if (args[0] === 'pr' && args[1] === 'merge') {
98+
if (options.stacked) {
99+
return {
100+
code: 1,
101+
stdout: '',
102+
stderr:
103+
'GraphQL: This pull request is part of a stack and must be merged ' +
104+
'using the asynchronous merge REST API. (mergePullRequest)',
105+
};
106+
}
86107
return options.mergeFails
87108
? { code: 1, stdout: '', stderr: 'refused' }
88109
: ok('Merged');
89110
}
90111

112+
// The asynchronous merge endpoint: the PUT enqueues, each GET polls.
113+
if (args[0] === 'api' && args.some((a) => a.includes('merge-async'))) {
114+
if (asyncPolls > 0) {
115+
asyncPolls -= 1;
116+
return ok(JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } }));
117+
}
118+
return ok(JSON.stringify({ status: 'merged', details: { sha: 'cafe' } }));
119+
}
120+
91121
if (args[0] === 'pr' && args[1] === 'ready') return ok('');
92122

93123
return ok('');
@@ -282,6 +312,26 @@ describe('sweep', () => {
282312
expect(summary.merged).toBe(0);
283313
});
284314

315+
it('merges a stacked PR through the asynchronous merge endpoint', async () => {
316+
const { gh, calls } = stubGh({ steps: [{}], stacked: true });
317+
const { summary } = await runSweep(gh, baseOptions());
318+
319+
expect(summary.merged).toBe(1);
320+
expect(summary.failed).toBe(0);
321+
expect(calls.some((c) => c.includes('merge-async'))).toBe(true);
322+
});
323+
324+
it('pins the head commit on the asynchronous merge too', async () => {
325+
const { gh, calls } = stubGh({ steps: [{}], stacked: true });
326+
await runSweep(gh, baseOptions());
327+
328+
const put = calls.find((c) => c.startsWith('api --method PUT'));
329+
expect(put).toContain('sha=deadbeef');
330+
expect(put).toContain('merge_method=squash');
331+
// Still no admin override on this path.
332+
expect(calls.some((c) => c.includes('--admin'))).toBe(false);
333+
});
334+
285335
it('marks a draft ready, then judges it normally', async () => {
286336
const { gh, calls } = stubGh({ steps: [{ isDraft: true }, { isDraft: false }] });
287337
const { summary } = await runSweep(gh, baseOptions());
@@ -320,3 +370,120 @@ describe('defaults', () => {
320370
expect(defaults.pollMs).toBe(20_000);
321371
});
322372
});
373+
374+
describe('asynchronous merge', () => {
375+
const PR = 'https://github.com/acme/repo/pull/7';
376+
377+
/** A `gh` that refuses the mutation, then answers the REST endpoint. */
378+
function stubAsync(replies: string[], { refuse = true } = {}) {
379+
const calls: string[] = [];
380+
let index = 0;
381+
382+
const exec = async (_f: string, args: readonly string[]): Promise<RunResult> => {
383+
calls.push(args.join(' '));
384+
385+
if (args[0] === 'pr' && args[1] === 'merge') {
386+
return refuse
387+
? {
388+
code: 1,
389+
stdout: '',
390+
stderr: 'must be merged using the asynchronous merge REST API. (mergePullRequest)',
391+
}
392+
: { code: 0, stdout: 'Merged', stderr: '' };
393+
}
394+
395+
const reply = replies[Math.min(index, replies.length - 1)]!;
396+
index += 1;
397+
return { code: 0, stdout: reply, stderr: '' };
398+
};
399+
400+
return { gh: new Gh({ exec }), calls };
401+
}
402+
403+
it('polls the uuid until the merge settles', async () => {
404+
const { gh, calls } = stubAsync([
405+
JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } }),
406+
JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } }),
407+
JSON.stringify({ status: 'merged', details: { sha: 'cafe' } }),
408+
]);
409+
410+
const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 });
411+
412+
expect(result.code).toBe(0);
413+
expect(calls.filter((c) => c.startsWith('api repos/acme/repo/pulls/7/merge-async/u-1')))
414+
.toHaveLength(2);
415+
});
416+
417+
it('treats enqueued as merged, because a merge queue owns it from there', async () => {
418+
const { gh } = stubAsync([JSON.stringify({ status: 'enqueued', details: { uuid: 'u-1' } })]);
419+
const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 });
420+
421+
expect(result.code).toBe(0);
422+
});
423+
424+
it('reports a failed asynchronous merge with the reason GitHub gave', async () => {
425+
const { gh } = stubAsync([
426+
JSON.stringify({ status: 'failed', details: { message: 'head moved' } }),
427+
]);
428+
const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 });
429+
430+
expect(result.code).toBe(1);
431+
expect(result.stderr).toContain('head moved');
432+
});
433+
434+
it('gives up rather than polling forever', async () => {
435+
const { gh } = stubAsync([JSON.stringify({ status: 'pending', details: { uuid: 'u-1' } })]);
436+
const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1, attempts: 3 });
437+
438+
expect(result.code).toBe(1);
439+
expect(result.stderr).toContain('never settled');
440+
});
441+
442+
it('does not reach for the endpoint when the refusal is a different one', async () => {
443+
const calls: string[] = [];
444+
const gh = new Gh({
445+
exec: async (_f, args) => {
446+
calls.push(args.join(' '));
447+
return { code: 1, stdout: '', stderr: 'Pull request is not mergeable' };
448+
},
449+
});
450+
451+
const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 });
452+
453+
expect(result.code).toBe(1);
454+
expect(calls.some((c) => c.includes('merge-async'))).toBe(false);
455+
});
456+
457+
it('never merges twice when the mutation already worked', async () => {
458+
const { gh, calls } = stubAsync([], { refuse: false });
459+
const result = await gh.squashMerge(PR, 'deadbeef', { pollMs: 1 });
460+
461+
expect(result.code).toBe(0);
462+
expect(calls.some((c) => c.includes('merge-async'))).toBe(false);
463+
});
464+
});
465+
466+
describe('parsePullRequestUrl', () => {
467+
it('reads owner, repo and number', () => {
468+
expect(parsePullRequestUrl('https://github.com/acme/repo/pull/7')).toEqual({
469+
slug: 'acme/repo',
470+
number: '7',
471+
});
472+
});
473+
474+
it('returns nothing for something that is not a PR url', () => {
475+
expect(parsePullRequestUrl('https://example.com/acme/repo')).toBeUndefined();
476+
});
477+
});
478+
479+
describe('parseMergeAsync', () => {
480+
it('lifts status, uuid and message out of the reply', () => {
481+
expect(
482+
parseMergeAsync(JSON.stringify({ status: 'failed', details: { message: 'no' } })),
483+
).toEqual({ status: 'failed', uuid: undefined, message: 'no' });
484+
});
485+
486+
it('returns nothing rather than inventing a status', () => {
487+
expect(parseMergeAsync('not json')).toBeUndefined();
488+
});
489+
});

0 commit comments

Comments
 (0)