Skip to content

Commit 2cebedd

Browse files
yaacovcorcosclaude
andcommitted
fix(server): skip merged queue removals and explain GitHub's removal reasons
GitHub also records a RemovedFromMergeQueueEvent with reason "merged" as the queue lands a pull request, up to a second before the pull request reads as merged, so a read in that second woke the agent with "removed from the merge queue without merging: merged". GitHub's real reasons (failed_checks, checks_timed_out, merge_conflict, manual) now read as sentences, and a failed_checks or checks_timed_out wake says the checks ran on the queue's own commit, not on the head. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 7c63ff1 commit 2cebedd

2 files changed

Lines changed: 70 additions & 1 deletion

File tree

‎apps/server/src/orchestration-v2/pullRequestWatch.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,57 @@ describe("evaluatePullRequestWatch", () => {
453453
);
454454
});
455455

456+
it("stays quiet when the queue lets go of a pull request by merging it", () => {
457+
const watching = evaluatePullRequestWatch(watch(), detail(), noRemarks).next;
458+
// GitHub stamps this removal up to a second before the pull request reads as merged.
459+
const merging = detail({
460+
mergeQueueRemoval: { id: "landed", removedAt: "2026-10-02T12:10:00Z", reason: "merged" },
461+
});
462+
const read = evaluatePullRequestWatch(watching, merging, noRemarks);
463+
assert.deepEqual(read.changes, []);
464+
assert.equal(read.next.mergeQueueRemovalId, "landed");
465+
});
466+
467+
it("explains GitHub's removal reasons in the wake", () => {
468+
const watching = evaluatePullRequestWatch(watch(), detail(), noRemarks).next;
469+
const wakeLine = (reason: string | null) => {
470+
const report = evaluatePullRequestWatch(
471+
watching,
472+
detail({
473+
mergeQueueRemoval: { id: "dropped", removedAt: "2026-10-02T12:10:00Z", reason },
474+
}),
475+
noRemarks,
476+
);
477+
return pullRequestWatchMessage({
478+
number: 1,
479+
url: "https://github.com/o/r/pull/1",
480+
baseBranch: "main",
481+
headSha: report.next.headSha,
482+
report,
483+
})
484+
.text.split("\n")
485+
.find((line) => line.startsWith("- "));
486+
};
487+
assert.equal(
488+
wakeLine("failed_checks"),
489+
"- The merge queue removed it because its checks failed. They ran on the queue's own commit, not on this head, so look for the failures in the queue's runs.",
490+
);
491+
assert.equal(
492+
wakeLine("checks_timed_out"),
493+
"- The merge queue removed it because its checks did not finish in time. They ran on the queue's own commit, not on this head, so look in the queue's runs for the check that did not finish.",
494+
);
495+
assert.equal(
496+
wakeLine("merge_conflict"),
497+
"- The merge queue removed it because it conflicts with the queue's base.",
498+
);
499+
assert.equal(wakeLine("manual"), "- It was taken out of the merge queue by hand.");
500+
assert.equal(
501+
wakeLine("some_new_reason"),
502+
"- It was removed from the merge queue without merging: some_new_reason.",
503+
);
504+
assert.equal(wakeLine(null), "- It was removed from the merge queue without merging.");
505+
});
506+
456507
it("does not spend the comment wake limit on check results", () => {
457508
const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 });
458509
const result = evaluatePullRequestWatch(

‎apps/server/src/orchestration-v2/pullRequestWatch.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ export function evaluatePullRequestWatch(
149149
const removal = detail.mergeQueueRemoval;
150150
if (
151151
removal !== undefined &&
152+
// Merging also records a removal, up to a second before the pull request reads as merged.
153+
removal.reason !== "merged" &&
152154
removal.id !== watch.mergeQueueRemovalId &&
153155
(watch.mergeQueueRemovalId !== undefined ||
154156
Date.parse(removal.removedAt) >= Math.floor(Date.parse(watch.startedAt) / 1000) * 1000)
@@ -235,11 +237,27 @@ function changeLines(
235237
return [`- The branch now conflicts with ${context.baseBranch}.`];
236238
case "merge-queue-removed":
237239
return [
238-
`- It was removed from the merge queue without merging${change.reason === null ? "" : `: ${change.reason}`}.`,
240+
`- ${
241+
(change.reason === null ? undefined : MERGE_QUEUE_REMOVALS.get(change.reason)) ??
242+
`It was removed from the merge queue without merging${change.reason === null ? "" : `: ${change.reason}`}.`
243+
}`,
239244
];
240245
}
241246
}
242247

248+
const MERGE_QUEUE_REMOVALS = new Map([
249+
[
250+
"failed_checks",
251+
"The merge queue removed it because its checks failed. They ran on the queue's own commit, not on this head, so look for the failures in the queue's runs.",
252+
],
253+
[
254+
"checks_timed_out",
255+
"The merge queue removed it because its checks did not finish in time. They ran on the queue's own commit, not on this head, so look in the queue's runs for the check that did not finish.",
256+
],
257+
["merge_conflict", "The merge queue removed it because it conflicts with the queue's base."],
258+
["manual", "It was taken out of the merge queue by hand."],
259+
]);
260+
243261
const SUMMARY: Record<PullRequestWatchChange["kind"], string> = {
244262
"checks-failed": "checks failed",
245263
"checks-passed": "checks passed",

0 commit comments

Comments
 (0)