Skip to content

Commit 4e691ab

Browse files
committed
fix(pr-header): surface gh errors, forward head SHA, dedupe disable-auto
Addresses PR review feedback on the write layer: - write() now reads the server's { error } body on failure so toasts show the actionable gh stderr instead of a generic "POST … failed: 500". - Forward expectedHeadOid through the enqueue/auto-merge path into gh pr merge --auto --match-head-commit, guarding auto-merge against a stale head (parity with the direct-merge path). - The merge-queue off-toggle no longer fires both dequeue and disable-auto (which map to the same gh --disable-auto), avoiding a duplicate request.
1 parent 745d47c commit 4e691ab

5 files changed

Lines changed: 61 additions & 17 deletions

File tree

packages/cli/src/__tests__/pull-request-mutations.routes.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,18 @@ describe("pull-request mutation API", () => {
155155
expect(await ghArgs()).toEqual(["pr merge 7 --auto --merge", "pr merge 7 --disable-auto"]);
156156
});
157157

158+
it("forwards expectedHeadOid to gh as --match-head-commit when enabling auto-merge", async () => {
159+
const runId = insertRun();
160+
const res = await send(await start(), "POST", `/api/runs/${runId}/pull-request/auto-merge`, {
161+
number: 7,
162+
enabled: true,
163+
mergeMethod: "SQUASH",
164+
expectedHeadOid: SHA,
165+
});
166+
expect(res.status).toBe(200);
167+
expect(await ghArgs()).toEqual([`pr merge 7 --auto --squash --match-head-commit ${SHA}`]);
168+
});
169+
158170
it("adds and removes reviewers via gh pr edit", async () => {
159171
const runId = insertRun();
160172
const port = await start();

packages/cli/src/github/mutations.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,13 @@ export function setAutoMerge(
7070
number: number,
7171
enabled: boolean,
7272
mergeMethod?: PullRequestMergeMethod,
73+
expectedHeadOid?: string,
7374
): Promise<void> {
7475
if (!enabled) return ghWrite(["pr", "merge", String(number), "--disable-auto"], repoRoot);
7576
const args = ["pr", "merge", String(number), "--auto"];
7677
if (mergeMethod) args.push(MERGE_METHOD_FLAG[mergeMethod]);
78+
// Guard against enabling auto-merge for a stale head the user hasn't seen.
79+
if (expectedHeadOid) args.push("--match-head-commit", expectedHeadOid);
7780
return ghWrite(args, repoRoot);
7881
}
7982

packages/cli/src/routes/pull-request-mutations.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const autoMergeInput = z.object({
3737
number: numberField,
3838
enabled: z.boolean(),
3939
mergeMethod: mergeMethod.optional(),
40+
expectedHeadOid: z.string().optional(),
4041
});
4142
const reviewersInput = z.object({ number: numberField, reviewers: z.array(z.string()).min(1) });
4243

@@ -127,7 +128,13 @@ export function pullRequestMutationRoutes(db: StageDb): Route[] {
127128
const input = await parseBody(req, res, autoMergeInput);
128129
if (!input) return;
129130
await runMutation(res, () =>
130-
setAutoMerge(run.repoRoot, input.number, input.enabled, input.mergeMethod),
131+
setAutoMerge(
132+
run.repoRoot,
133+
input.number,
134+
input.enabled,
135+
input.mergeMethod,
136+
input.expectedHeadOid,
137+
),
131138
);
132139
},
133140
},

packages/web/src/components/pull-request/merge-status.tsx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -174,18 +174,18 @@ function MergeActions({
174174
} else {
175175
autoMergeMutation.mutate({ owner, repo, number, enabled: true, mergeMethod });
176176
}
177-
} else {
178-
if (mergeInfo.isInMergeQueue && mergeInfo.entry) {
179-
dequeueMutation.mutate({
180-
owner,
181-
repo,
182-
number,
183-
mergeQueueEntryId: mergeInfo.entry.id,
184-
});
185-
}
186-
if (mergeInfo.autoMergeEnabled) {
187-
autoMergeMutation.mutate({ owner, repo, number, enabled: false });
188-
}
177+
} else if (mergeInfo.isInMergeQueue && mergeInfo.entry) {
178+
dequeueMutation.mutate({
179+
owner,
180+
repo,
181+
number,
182+
mergeQueueEntryId: mergeInfo.entry.id,
183+
});
184+
} else if (mergeInfo.autoMergeEnabled) {
185+
// `else if`: both dequeue and disabling auto-merge map to the same
186+
// `gh pr merge --disable-auto` here, so fire only one to avoid a
187+
// duplicate request (and a spurious error toast on the second).
188+
autoMergeMutation.mutate({ owner, repo, number, enabled: false });
189189
}
190190
}}
191191
/>

packages/web/src/lib/pull-request-mutations.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,39 @@
11
import type { PullRequestMergeMethod } from "@stagereview/types/pull-request";
22
import { useQueryClient } from "@tanstack/react-query";
3-
import { jsonFetch } from "@/lib/use-view-state";
43

54
function prPath(runId: string, suffix: string): string {
65
return `/api/runs/${encodeURIComponent(runId)}/pull-request${suffix}`;
76
}
87

9-
function write(
8+
async function write(
109
runId: string,
1110
suffix: string,
1211
method: "POST" | "PATCH" | "DELETE",
1312
body: Record<string, unknown>,
1413
): Promise<unknown> {
15-
return jsonFetch(prPath(runId, suffix), {
14+
const path = prPath(runId, suffix);
15+
const res = await fetch(path, {
1616
method,
1717
headers: { "Content-Type": "application/json" },
1818
body: JSON.stringify(body),
1919
});
20+
const text = await res.text();
21+
if (!res.ok) {
22+
// The server returns `{ error: <gh stderr> }` on failure — surface it so the
23+
// toast shows the actionable gh message, not a generic "POST … failed: 500".
24+
let message = `${method} ${path} failed: ${res.status}`;
25+
try {
26+
const parsed: unknown = text ? JSON.parse(text) : null;
27+
if (parsed && typeof parsed === "object" && "error" in parsed) {
28+
const { error } = parsed as { error: unknown };
29+
if (typeof error === "string" && error) message = error;
30+
}
31+
} catch {
32+
// non-JSON body — keep the generic message
33+
}
34+
throw new Error(message);
35+
}
36+
return text ? JSON.parse(text) : {};
2037
}
2138

2239
/** Invalidate every PR-derived query for a run after a mutation. */
@@ -83,10 +100,15 @@ export function mergeMutationOptions(runId: string) {
83100
}
84101

85102
// Merge-queue enqueue maps to "enable auto-merge" — gh enqueues when ready.
103+
// Forward the head SHA so the server can guard against a stale head (--match-head-commit).
86104
export function enqueueMutationOptions(runId: string) {
87105
return {
88106
mutationFn: (v: RepoVars & { number: number; expectedHeadOid?: string }) =>
89-
write(runId, "/auto-merge", "POST", { number: v.number, enabled: true }),
107+
write(runId, "/auto-merge", "POST", {
108+
number: v.number,
109+
enabled: true,
110+
expectedHeadOid: v.expectedHeadOid,
111+
}),
90112
};
91113
}
92114

0 commit comments

Comments
 (0)