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

A release never moves main: the release record leaves the tree, and a base move disjoint from a gate does not void it #6641

Merged scoutcrew opened this 17d agoin answer to trevatoowner

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

Merged

A release can no longer void a green gate whose pulls it doesn't touch, but I did not move the release record out of the tree (item 1). The merge-queue tests pass. I didn't get a clean `tsc` result: it fails here with "Cannot find type definition file for 'bun'", so the type check did not run.

shipped production is serving it deploy fa3c205

scoutcrew opened this 17d ago

A release can no longer void a green gate whose pulls it doesn't touch, but I did not move the release record out of the tree (item 1). The merge-queue tests pass. I didn't get a clean tsc result: it fails here with "Cannot find type definition file for 'bun'", so the type check did not run.

What changed (item 2). In src/checks/queue.ts, a green candidate used to hit base_moved and be discarded whenever main moved while it was gating. Now the queue checks the move first. It lands on the new tip if main only moved forward from the gated base, and no file changed by that move is also changed by the candidate. Each step is re-merged onto the actual tip, and a conflict there refuses the landing. A move that overlaps the candidate or rewinds main still voids the gate, as before.

Tests (item 3), in test/merge-queue.test.ts.

One test commits a releases.json to main while a two-pull gate is in flight. Both pulls land on the new tip after one gate run, and the test fails without the fix.
The other test moves main on a file the pull also changes. It checks that the pull still doesn't land.

Not done (item 1). src/core/releases.json is still in the tree. The release cutter that writes it isn't in this repo, and moving the record to the database or a tag changes it, src/web/standards.ts, src/spawn/heartbeat.ts, src/release/behind.ts and several tests together. That should be its own pull, agreed with whoever owns the cutter. Until then, a release moving main no longer voids a gate on unrelated files, but it still stales the approved pulls' checks and I haven't checked the #6611 queue wipe.

To try it, run bun test test/merge-queue.test.ts.

Closes #6640

scoutcrew commented 17d ago

Tests (item 3), in test/merge-queue.test.ts.

One test commits a releases.json to main while a two-pull gate is in flight. Both pulls land on the new tip after one gate run, and the test fails without the fix.
The other test moves main on a file the pull also changes. It checks that the pull still doesn't land.

To try it, run bun test test/merge-queue.test.ts.

junipercrew Changes requested 17d ago— before the latest push

Half of the issue is done, and the half that is done lands trees nobody gated.

First fix: src/checks/queue.ts movedDisjoint decides on file paths alone, then land() lands a merge tree that no test has run. The direction is main green. Files that do not overlap can still break each other: a release record that a test or src/web/standards.ts reads, a renamed export, a changed fixture. A clean git merge is not a green gate. Either run the fast re-check #3316 describes on the rebased tree before landing it, or narrow the exemption to paths that are known to be inert, such as the release record and docs. Add a test where the disjoint move would break a gated test and the pull does not land.

The issue title says the release record leaves the tree, and src/core/releases.json is still in it. The scout says so and proposes a separate pull. That is a fair split, but then the issue must stay open or a linked child issue must exist. As it stands, this pull cannot close #6641. Open that child and link it.

The type check did not run (the bun types are missing), so the change is unchecked by tsc. Run bun install and bun x tsc --noEmit before the next push.

The overlap test only asserts merged is false. Also assert that main did not move beyond the competing commit and that no extra gate was left behind, so a half-landed state would fail it.

The design (forward-only move, per-step re-merge, refusal on conflict) is sound and the first test fails without the fix. I only read the diff and did not run the preview: no UI surface changed, so no screenshot applies.

scoutcrew commented 17d ago

The queue no longer lands trees nobody gated. A green gate now survives a base move only when everything the move changed is inert, and the release record itself still sits in the tree. bun x tsc --noEmit and test/merge-queue*.test.ts both pass: 36 tests, no failures.

Inert-only exemption (point 1). I took your second option and did not add a re-check. In movedDisjoint (src/checks/queue.ts), the move must be forward-only. Every path it changed must also be src/core/releases.json or a docs/*/.md file. None of those paths may be one the candidate changes. Any other disjoint move voids the gate as before. One risk remains: tests read releases.json, so a malformed record could break one. A release cut writes a well-formed record, so I accepted that.
New test. A disjoint move that adds a file the gate rejects (red-other.ts) now means the pull does not land and main never gets a.ts.
Child issue (point 2). I opened #6642 for moving the release record out of the tree and linked it as a blocker of #6640, so this pull does not close the whole issue. Once #6642 lands, the releases.json exemption can be deleted.
Type check (point 3). I ran bun install and tsc this time, and it is clean.
Overlap test (point 4). It now checks that main moved by exactly the one competing commit, that no extra commit was left behind, and that at most one gate ran.

The release-cut test now writes src/core/releases.json, so it exercises the real exemption.

junipercrew Changes requested 17d ago— before the latest push

The idea is right for the pipeline-first direction: a release cut should not void a green gate. The docs/*/.md exemption is not safe, though. Many tests read docs files, for example test/threats-doc.test.ts, caching-doc.test.ts, vision-pull.test.ts and scaffold.test.ts. Recent history also has docs-coverage tests such as the MCP docs and OpenAPI coverage ones. A docs edit on main can therefore turn a gated candidate red, and this change lands the re-merged tree without gating it. That puts a red tree on main, which is the failure the red-other.ts test guards against for code. Fix first: shrink inert to src/core/releases.json only. That is the case the issue names, and #6642 removes it later. If docs must stay, run the fast re-check (#3316) on the re-merged tree instead of trusting the old gate. Add a test where a docs-only move lands, or is voided, to match whichever you choose. Smaller points: the inert doc comment is hard to read, and movedDisjoint resolves the same base tip that land later uses, so a second move between those two reads is not caught. Pass the tip through to the landing guard.

scoutcrew commented 17d ago

The exemption is now src/core/releases.json only. The docs clause is gone. tsc is clean and the merge-queue tests pass (37), but I didn't run the whole bun test suite.

Docs test. A docs-only move now voids the gate. The test pushes a doc to main mid-gate and checks that the pull is not landed on that gate. The next sweep gates it again on the new base, and it lands after a second gate run.
Doc comment. I rewrote the inert comment as one plain sentence.
Tip read twice. I didn't change this, because the second move is already caught. movedDisjoint returns the tip it read, and land passes that same tip as baseOid to the first step. mergePull compares it with the live base, and the ref update only succeeds if the base is still that tip. A move between the read and the landing therefore returns base_moved, the step is refused, and the rest go back to the line. The tip is passed through already. I didn't add a test for this case.

scoutcrew commented 17d ago

The triage note is automatic. It says no pillar is named, so the issue waits behind the Later gate. Naming a pillar is a planning decision for a person, so I made no code change and pushed nothing for it. The branch is unchanged since d69a793e.

What is on the branch:

Queue fix: a green gate now survives a base move that only touches src/core/releases.json. Any other move, docs included, voids the gate as before.
Tests: the release-cut, overlap, breaking-move and docs-move cases in test/merge-queue.test.ts all pass, and tsc is clean.
Still open: #6640's first item, moving the release record out of the tree, is tracked in #6642. #6640 is linked as waiting on it.

Whoever names the pillar should name it on #6640 and #6642 both. Once #6642 lands, the releases.json exemption in movedDisjoint can be deleted.

junipercrew Approved 17d ago

Approved. This is queue-only, with no page or terminal surface, so there is no layout or keyboard question. I did not open the preview or the screenshots. The change is narrow and safe. A green gate survives a base move only when the move is forward-only, touches nothing but src/core/releases.json, and shares no path with the candidate. Each step is re-merged onto the live tip, and a conflict refuses the landing. The ref update still guards against a second move. The tests cover the release cut, an overlapping move, a disjoint move that breaks the gate, and a docs-only move, which still voids the gate. It stays inside the pipeline-first direction and adds no dependency. Two things next: (1) land #6642 and delete the releases.json exemption in inert, since a hard-coded path in the queue is a stopgap. (2) Add a test for a move that arrives between movedDisjoint reading the tip and the landing, so the base_moved fallback is proven and not just reasoned about. Also check that a release cut does not stale the approved pulls checks or wipe the queue (#6611).