Skip to content

Commit e2b528f

Browse files
committed
fix: merge stacked PRs via GitHub's async merge endpoint
The synchronous merge endpoint rejects stacked PRs. Route stacked PRs through the async merge endpoint and poll until the background merge completes, so the merge status updates reliably (even for slow merges).
1 parent 1a32ddd commit e2b528f

4 files changed

Lines changed: 262 additions & 0 deletions

File tree

src/api/types.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ import { components } from "@octokit/openapi-types";
44
// Extended types include body_html from GitHub's HTML media type (application/vnd.github.html+json)
55
export type PullRequest = components["schemas"]["pull-request"] & {
66
body_html?: string;
7+
// Stacked PRs (newer than openapi-types@27; null for non-stacked PRs)
8+
stack?: {
9+
id: number;
10+
number: number;
11+
size: number;
12+
position: number;
13+
base: { ref: string; sha: string };
14+
} | null;
715
};
816
export type PullRequestFile = components["schemas"]["diff-entry"];
917
export type ReviewComment =

src/browser/contexts/github.tsx

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,19 @@ export type UserProfile = components["schemas"]["public-user"];
7474
// Types
7575
// ============================================================================
7676

77+
/**
78+
* Result of an asynchronous merge (POST /pulls/{pull_number}/merge-async).
79+
* While pending, `details.uuid` can be polled until a terminal status.
80+
*/
81+
export interface AsyncMergeResult {
82+
status: "pending" | "merged" | "enqueued" | "failed";
83+
details: {
84+
message: string;
85+
uuid?: string;
86+
sha?: string;
87+
};
88+
}
89+
7790
export interface PRSearchResult {
7891
id: number;
7992
number: number;
@@ -1120,6 +1133,117 @@ function createGitHubStore() {
11201133
return data;
11211134
}
11221135

1136+
/**
1137+
* Merge a PR asynchronously (required for stacked PRs). Submits the merge
1138+
* request and polls the returned UUID until the merge reaches a terminal
1139+
* state. Throws if the merge fails or times out.
1140+
*/
1141+
async function mergePRAsync(
1142+
owner: string,
1143+
repo: string,
1144+
number: number,
1145+
options?: {
1146+
merge_method?: "merge" | "squash" | "rebase";
1147+
sha?: string;
1148+
}
1149+
): Promise<AsyncMergeResult> {
1150+
if (!octokit) throw new Error("Not initialized");
1151+
1152+
let result: AsyncMergeResult;
1153+
try {
1154+
const { data } = await octokit.request<AsyncMergeResult>({
1155+
method: "PUT",
1156+
url: "/repos/{owner}/{repo}/pulls/{pull_number}/merge-async",
1157+
owner,
1158+
repo,
1159+
pull_number: number,
1160+
merge_method: options?.merge_method ?? "squash",
1161+
merge_action: "direct_merge",
1162+
sha: options?.sha,
1163+
});
1164+
result = data;
1165+
} catch (error) {
1166+
// 409: an async merge is already in flight for this PR — the response
1167+
// carries that request's UUID, which we can poll instead.
1168+
const err = error as {
1169+
status?: number;
1170+
response?: { data?: AsyncMergeResult };
1171+
};
1172+
if (err.status === 409 && err.response?.data) {
1173+
result = err.response.data;
1174+
} else {
1175+
throw error;
1176+
}
1177+
}
1178+
1179+
const { uuid } = result.details ?? {};
1180+
if (!uuid) return result; // already terminal (merged/enqueued/failed)
1181+
1182+
// The merge runs as a background job on GitHub (stacked merges can take
1183+
// several minutes), so poll patiently instead of giving up quickly.
1184+
const startedAt = Date.now();
1185+
const MAX_POLL_MS = 15 * 60_000;
1186+
let intervalMs = 2000;
1187+
let attempt = 0;
1188+
while (
1189+
result.status === "pending" &&
1190+
Date.now() - startedAt < MAX_POLL_MS
1191+
) {
1192+
await new Promise((resolve) => setTimeout(resolve, intervalMs));
1193+
if (Date.now() - startedAt > 2 * 60_000) {
1194+
intervalMs = Math.min(intervalMs + 2000, 10_000);
1195+
}
1196+
attempt++;
1197+
1198+
try {
1199+
const { data } = await octokit.request<AsyncMergeResult>({
1200+
method: "GET",
1201+
url: "/repos/{owner}/{repo}/pulls/{pull_number}/merge-async/{uuid}",
1202+
owner,
1203+
repo,
1204+
pull_number: number,
1205+
uuid,
1206+
});
1207+
result = data;
1208+
if (result.status !== "pending") break;
1209+
} catch {
1210+
// Transient error — keep polling; the PR-state check below still runs.
1211+
}
1212+
1213+
// Accelerator: if the PR itself has flipped to merged, we're done even
1214+
// if the UUID endpoint misbehaves.
1215+
if (attempt % 5 === 0) {
1216+
try {
1217+
const pr = await queryClient.fetchQuery({
1218+
...queries.pullRequest(owner, repo, number),
1219+
staleTime: 0,
1220+
});
1221+
if (pr.merged) {
1222+
return {
1223+
status: "merged",
1224+
details: {
1225+
message: "Merged",
1226+
sha: pr.merge_commit_sha ?? undefined,
1227+
},
1228+
};
1229+
}
1230+
} catch {
1231+
// Ignore — rely on UUID polling
1232+
}
1233+
}
1234+
}
1235+
1236+
if (result.status === "pending") {
1237+
throw new Error(
1238+
"Merge is still running in the background on GitHub. It will complete there and the PR status will update."
1239+
);
1240+
}
1241+
if (result.status === "failed") {
1242+
throw new Error(result.details?.message ?? "Failed to merge");
1243+
}
1244+
return result;
1245+
}
1246+
11231247
async function dequeuePullRequest(
11241248
owner: string,
11251249
repo: string,
@@ -2462,6 +2586,7 @@ function createGitHubStore() {
24622586
getWorkflowRuns: getWorkflowRunsForSha,
24632587
approveWorkflowRun,
24642588
mergePR,
2589+
mergePRAsync,
24652590
dequeuePullRequest,
24662591
enqueuePullRequest,
24672592
getPRCommits,

src/browser/contexts/pr-review/index.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ function createMockGitHubStore(): GitHubStore {
4242
invalidatePR: () => {},
4343
getPR: async () => createMockPR(),
4444
mergePR: async () => ({ merged: true }),
45+
mergePRAsync: async () => ({
46+
status: "merged",
47+
details: { message: "merged" },
48+
}),
4549
closePR: async () => {},
4650
reopenPR: async () => {},
4751
deleteBranch: async () => {},
@@ -763,6 +767,10 @@ function createMockGitHubStoreWithVersions(
763767
invalidatePR: () => {},
764768
getPR: async () => createMockPR(),
765769
mergePR: async () => ({ merged: true }),
770+
mergePRAsync: async () => ({
771+
status: "merged",
772+
details: { message: "merged" },
773+
}),
766774
closePR: async () => {},
767775
reopenPR: async () => {},
768776
deleteBranch: async () => {},
@@ -987,6 +995,108 @@ test("mergePR sets mergeError and clears merging on failure", async () => {
987995
expect(state.pr.merged).toBe(false);
988996
});
989997

998+
// ============================================================================
999+
// mergePR (stacked PRs - async merge endpoint)
1000+
// ============================================================================
1001+
1002+
function createStackedMockPR(): PullRequest {
1003+
return createMockPR({
1004+
stack: {
1005+
id: 1,
1006+
number: 2,
1007+
size: 2,
1008+
position: 1,
1009+
base: { ref: "main", sha: "def456" },
1010+
},
1011+
});
1012+
}
1013+
1014+
test("mergePR uses async merge endpoint for stacked PRs and sets merged=true", async () => {
1015+
let syncCalled = false;
1016+
let asyncCalled = false;
1017+
const github = {
1018+
...createMockGitHubStore(),
1019+
mergePR: async () => {
1020+
syncCalled = true;
1021+
return { merged: true };
1022+
},
1023+
mergePRAsync: async () => {
1024+
asyncCalled = true;
1025+
return { status: "merged", details: { message: "merged" } };
1026+
},
1027+
} as unknown as GitHubStore;
1028+
const store = new PRReviewStore(github, {
1029+
pr: createStackedMockPR(),
1030+
files: [],
1031+
comments: [],
1032+
owner: "test",
1033+
repo: "repo",
1034+
viewerPermission: "WRITE",
1035+
});
1036+
1037+
const result = await store.mergePR();
1038+
1039+
expect(result).toBe(true);
1040+
expect(asyncCalled).toBe(true);
1041+
expect(syncCalled).toBe(false);
1042+
const state = store.getSnapshot();
1043+
expect(state.pr.merged).toBe(true);
1044+
expect(state.pr.state).toBe("closed");
1045+
expect(state.merging).toBe(false);
1046+
});
1047+
1048+
test("mergePR sets prInMergeQueue when async merge is enqueued", async () => {
1049+
const github = {
1050+
...createMockGitHubStore(),
1051+
mergePRAsync: async () => ({
1052+
status: "enqueued",
1053+
details: { message: "enqueued" },
1054+
}),
1055+
} as unknown as GitHubStore;
1056+
const store = new PRReviewStore(github, {
1057+
pr: createStackedMockPR(),
1058+
files: [],
1059+
comments: [],
1060+
owner: "test",
1061+
repo: "repo",
1062+
viewerPermission: "WRITE",
1063+
});
1064+
1065+
const result = await store.mergePR();
1066+
1067+
expect(result).toBe(true);
1068+
const state = store.getSnapshot();
1069+
expect(state.prInMergeQueue).toBe(true);
1070+
expect(state.pr.merged).toBe(false);
1071+
expect(state.merging).toBe(false);
1072+
});
1073+
1074+
test("mergePR sets mergeError when async merge fails", async () => {
1075+
const github = {
1076+
...createMockGitHubStore(),
1077+
mergePRAsync: async () => {
1078+
throw new Error("Merge failed");
1079+
},
1080+
} as unknown as GitHubStore;
1081+
const store = new PRReviewStore(github, {
1082+
pr: createStackedMockPR(),
1083+
files: [],
1084+
comments: [],
1085+
owner: "test",
1086+
repo: "repo",
1087+
viewerPermission: "WRITE",
1088+
});
1089+
1090+
const result = await store.mergePR();
1091+
1092+
expect(result).toBe(false);
1093+
const state = store.getSnapshot();
1094+
expect(state.merging).toBe(false);
1095+
expect(state.mergeError).toBe("Merge failed");
1096+
expect(state.pr.merged).toBe(false);
1097+
expect(state.prInMergeQueue).toBe(false);
1098+
});
1099+
9901100
// ============================================================================
9911101
// Conversation / Timeline events
9921102
// ============================================================================

src/browser/contexts/pr-review/index.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3769,6 +3769,25 @@ export class PRReviewStore {
37693769
this.invalidatePRCaches(owner, repo, pr.number);
37703770

37713771
this.set({ prInMergeQueue: true, merging: false });
3772+
} else if (pr.stack) {
3773+
// Stacked PRs must be merged via the asynchronous merge endpoint.
3774+
const result = await this.github.mergePRAsync(owner, repo, pr.number, {
3775+
merge_method: mergeMethod,
3776+
sha: pr.head.sha,
3777+
});
3778+
3779+
this.invalidatePRCaches(owner, repo, pr.number);
3780+
3781+
if (result.status === "merged") {
3782+
this.set({
3783+
pr: { ...this.state.pr, merged: true, state: "closed" as const },
3784+
merging: false,
3785+
});
3786+
} else if (result.status === "enqueued") {
3787+
this.set({ prInMergeQueue: true, merging: false });
3788+
} else {
3789+
throw new Error(result.details?.message ?? "Failed to merge");
3790+
}
37723791
} else {
37733792
await this.github.mergePR(owner, repo, pr.number, {
37743793
merge_method: mergeMethod,

0 commit comments

Comments
 (0)