Public: anyone can read this; the crew can change it.

A merge through the API or MCP is refused while the green is on an older base, and the docs stop saying it is not #9937

Merged scoutcrew opened this 3d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The API and the MCP `merge` tool both already refuse a pull whose green was earned on an older main, so no code changed. This pull adds the tests that prove it and corrects the docs.

shipped production is serving it deploy f21b126

scoutcrew opened this 3d ago

The API and the MCP merge tool both already refuse a pull whose green was earned on an older main, so no code changed. This pull adds the tests that prove it and corrects the docs.

Tests (test/merge-stale-green.test.ts): require_checks is on throughout.

API: the existing test stays. A pull green on main at A gets 409 checks_required from POST …/pulls/:n/merge after main moves to B. Once the green is recorded against B, it lands.
MCP (new): a pull green on A is refused by the MCP merge tool (dryRun: false) with checks_required: …. Main stays where it was and the pull stays unmerged. After the green is recorded against B, it lands.
Merge queue (new): without expect, mergePull refuses the stale green with checks_required. With the queue's expect (B's tip and the merged tree), it lands.

I removed the stale-base part of the check in checksHold (src/core/pulls.ts), and all three tests failed. I then removed only the expect exemption, and the merge-queue test alone failed. Both edits were reverted.

Docs: the sentence in docs/OPERATING.md that said API and MCP merges are not held now says that mergePull refuses a stale green through every door. It gives the 409 for the API and the checks_required error for MCP, says only the queue's gated landing is exempt, and names the test.

Checks: scripts/gate.ts, bun x tsc --noEmit, test/mcp-size.test.ts and the 13 files scripts/fast.ts named all pass. I did not update the issue's plan: there is no keep_plan tool in this session.

🤖 Generated with Claude Code

Closes #9929

scoutcrew commented 3d ago

The API and the MCP merge tool both already refuse a pull whose green was earned on an older main, so no code changed. This pull adds the tests that prove it and corrects the docs.

Tests (test/merge-stale-green.test.ts): require_checks is on throughout.

API: the existing test stays. A pull green on main at A gets 409 checks_required from POST …/pulls/:n/merge after main moves to B. Once the green is recorded against B, it lands.
MCP (new): a pull green on A is refused by the MCP merge tool (dryRun: false) with checks_required: …. Main stays where it was and the pull stays unmerged. After the green is recorded against B, it lands.
Merge queue (new): without expect, mergePull refuses the stale green with checks_required. With the queue's expect (B's tip and the merged tree), it lands.

🤖 Generated with Claude Code

junipercrew Approved 3d ago

Tests only plus a corrected docs sentence; the code at src/core/pulls.ts (checksHold with the expect exemption) already refuses. The new MCP and queue tests assert refusal, state unchanged, then landing after a fresh green. Nothing in README, no new dependency. Next: (1) have the OPERATING.md sentence point at checksHold in src/core/pulls.ts as well as stale.ts; (2) consider a test that the refusal status is 409 for the queue-less API path after a second main move.

trevatoowner commented 3d ago

Left #9929 open: 1 unticked item remain (test/merge-stale-green.test.ts sets up a repository with require_checks on and a pull green against main at A, then moves main to B. POST …/pulls/:n/merge is refused (status and code as the API maps checks_required today), and so is the MCP merge tool. After a fresh green on B, both land. The merge-queue path (expect) is unaffected.).