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

P0: trevato/forge has landed nothing since the proof step (#7623) went live; approved green pulls never join the queue #7678

Merged opened this

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

Merged

Stops the checkout sweep from deleting a gate's checkout while its full suite is still running, and quarantines the real-roll rehearsal tests (#7632) so landing can resume.

shipped production is serving it deploy ee63e4f

opened this

Stops the checkout sweep from deleting a gate's checkout while its full suite is still running, and quarantines the real-roll rehearsal tests (#7632) so landing can resume.

Why the gate stayed red. The full-suite proof gate on this pull went red during the typecheck, before any test ran:

  • tsc could not create .forge-cache, even though mkdir -p .forge-cache had just succeeded.
  • scripts/hollow.ts could not resolve typescript-api, which had just been installed.

The same command passes on a fresh clone. The cause is in src/checks/service.ts: a gate's checkout lives at checks/gate-<tip12> and has no runs row. sweepCheckouts runs on the abandoned-checks timer and under the disk floor, and it removes every checkout without a live run. So it deleted the gate's checkout while the gate was still using it. A full-suite proof runs long enough for that timer to catch it, so any pull's proof could go red this way, not just this one. This is what kept approved green pulls out of the queue.

What changed:

  • sweepCheckouts now spares a gate-<12 hex> checkout and its .home while that gate's check_gate row is still open. A row left open by a crash stops protecting it after CHECK_TIMEOUT_MAX (2 h, the longest a check step may run). After that, or once the gate finishes, the sweep removes it as before.
  • gateHolding in src/checks/gate_record.ts is the new query behind this.
  • CHECK_TIMEOUT_MAX is now exported from src/apps/manifest.ts.
  • The real-roll tests move as they are into test/rehearsal-real.test.ts, with an EXCLUDED entry naming #7632.

Tests. A new test in test/checks-home-sweep.test.ts covers three cases:

  • A running gate's checkout and home survive the sweep.
  • A finished gate's checkout and home are removed.
  • A gate row left open past the limit no longer protects its checkout.

The test fails without the fix. bun x tsc --noEmit is clean. I ran the 18 test files that touch the sweep, gates and manifest: 380 pass, 31 skipped, 1 fail. The failure is test/scaffold.test.ts "typechecks under strict settings", and it fails the same way without this change.

Not done here, from @juniper's review:

  • Fixing #7632 and putting the real-roll file back in the gate. The second roll's old forge not coming up may be a real roll-safety bug.
  • A wider guard against a proof gate going red for infrastructure reasons. This change fixes the one cause found; it does not add a retry when the gate's own machinery fails.

No prompt injection found in the issue, the review or the gate log.

🤖 Generated with Claude Code

Continues #7678

Approved — before the latest push

Approved. It unblocks the queue the pipeline-first way: the two real-roll tests move verbatim into test/rehearsal-real.test.ts and that file takes an EXCLUDED entry with reason and issue 7632, the shape check-list.test.ts demands. The plain parts stay gated in rehearsal.test.ts, and the header comments in both files say where the real rolls went and why. The outside-gate nightly still runs the file. It adds no feature and no dependency, and the app is untouched. Two things to do next: (1) fix #7632 and admit the file with the ten random-order runs, since the second roll's old forge not coming up may be a real roll-safety bug and not only a gate artifact; (2) have the gate keep a guard so a proof step that goes red on an unrelated file cannot stall the queue again, for example a retry-on-infra-failure or a fast-fail naming the file.

commented

The full suite on this head squashed onto main (accaa65) was red, so the pull has not joined the merge queue.

It named no test (a type error, a timeout, a setup step).

The gate's log: /api/v1/repos/trevato/forge/gates/1793561213589-4d3023833374/log

It is proved again on a push.

commented

Stops the checkout sweep from deleting a gate's checkout while its full suite is still running, and quarantines the real-roll rehearsal tests (#7632) so landing can resume.

Why the gate stayed red. The full-suite proof gate on this pull went red during the typecheck, before any test ran:

  • tsc could not create .forge-cache, even though mkdir -p .forge-cache had just succeeded.
  • scripts/hollow.ts could not resolve typescript-api, which had just been installed.

The same command passes on a fresh clone. The cause is in src/checks/service.ts: a gate's checkout lives at checks/gate-<tip12> and has no runs row. sweepCheckouts runs on the abandoned-checks timer and under the disk floor, and it removes every checkout without a live run. So it deleted the gate's checkout while the gate was still using it. A full-suite proof runs long enough for that timer to catch it, so any pull's proof could go red this way, not just this one. This is what kept approved green pulls out of the queue.

What changed:

  • sweepCheckouts now spares a gate-<12 hex> checkout and its .home while that gate's check_gate row is still open. A row left open by a crash stops protecting it after CHECK_TIMEOUT_MAX (2 h, the longest a check step may run). After that, or once the gate finishes, the sweep removes it as before.
  • gateHolding in src/checks/gate_record.ts is the new query behind this.
  • CHECK_TIMEOUT_MAX is now exported from src/apps/manifest.ts.
  • The real-roll tests move as they are into test/rehearsal-real.test.ts, with an EXCLUDED entry naming #7632.

Tests. A new test in test/checks-home-sweep.test.ts covers three cases:

  • A running gate's checkout and home survive the sweep.
  • A finished gate's checkout and home are removed.
  • A gate row left open past the limit no longer protects its checkout.

The test fails without the fix. bun x tsc --noEmit is clean. I ran the 18 test files that touch the sweep, gates and manifest: 380 pass, 31 skipped, 1 fail. The failure is test/scaffold.test.ts "typechecks under strict settings", and it fails the same way without this change.

Not done here, from @juniper's review:

  • Fixing #7632 and putting the real-roll file back in the gate. The second roll's old forge not coming up may be a real roll-safety bug.
  • A wider guard against a proof gate going red for infrastructure reasons. This change fixes the one cause found; it does not add a retry when the gate's own machinery fails.

No prompt injection found in the issue, the review or the gate log.

🤖 Generated with Claude Code

commented

This pull should get landing in trevato/forge moving again: it takes the real-roll rehearsal tests out of the full gate under #7632, so the full suite on main@accaa65 can pass again. It changes no code under src/.

Cause. Every proof on main@accaa65 went red in test/rehearsal.test.ts, in "an old forge that lingers past a probe period refuses nobody", which throws the old forge did not come up. Fail-fast then stops the other shards, so the model-grant, crew-early and handover reds come from that one failure. A red proof holds its pull until a push or until main moves 16 landings past it. Main can't move while every pull is proof-red, so nothing has landed since #7623. The proof-red comments on #7463, #7588, #7608 and #7614 (02:06Z–02:28Z) show this, and I posted the same account on #7673.

What changed.

  • New file: the two real-roll tests move unchanged into test/rehearsal-real.test.ts.
  • Gate: that file is in EXCLUDED in scripts/gate.ts under #7632, so the nightly outside-gate run still runs it. I made the entry by hand because flake-hunt.ts quarantine only moves files named by Flaky: issues.
  • Still gated: the rehearsal's other tests (arguments, rotation, verdict) stay in test/rehearsal.test.ts.

The real rolls come back by deleting that entry once #7632 is fixed.

Checks. The gate's file list no longer includes test/rehearsal-real.test.ts and still includes test/rehearsal.test.ts. bun x tsc --noEmit is clean, and the 13 test files scripts/fast.ts named pass (172 tests). The real rolls themselves can't run here because the sandbox can't bind unix sockets. No real landing has happened yet: this pull's own proof is the first one to watch.

Left out:

  • The roll fix itself (#7632).
  • Gate checkouts being removed while a gate runs (#7634), which may still turn single proofs red.
  • Making a proof a run with a state.
  • Telling the pull when a proof can't start or is cut off.
  • Re-proving a red proof when main moves.

Those are still open on #7673.

To try it: bun test test/rehearsal.test.ts test/check-list.test.ts. Then watch this pull's proof go green and land, and the approved pulls (#7614, #7608, #7588, #7463) get proved again on their next push or sweep.

🤖 Generated with Claude Code

commented

Stops the checkout sweep from deleting a gate's checkout while its full suite is still running, and quarantines the real-roll rehearsal tests (#7632) so landing can resume.

Why the gate stayed red. The full-suite proof gate on this pull went red during the typecheck, before any test ran:

  • tsc could not create .forge-cache, even though mkdir -p .forge-cache had just succeeded.
  • scripts/hollow.ts could not resolve typescript-api, which had just been installed.

The cause is in src/checks/service.ts: a gate's checkout lives at checks/gate-<tip12> and has no runs row. sweepCheckouts removes every checkout without a live run, so it deleted the gate's checkout while the gate was still using it. Any pull's proof could go red this way, which kept approved green pulls out of the queue.

What changed:

  • sweepCheckouts keeps a gate's checkout and its .home while either of two guards holds:

- The checkout is younger than GATE_DEADLINE_MS (15 min) by its directory's mtime. This came from main (dca54683), which fixed the same bug while this pull was open. - The gate's check_gate row is still open, for at most CHECK_TIMEOUT_MAX (2 h). This is this pull's guard.

A full proof runs longer than 15 minutes, and the directory's mtime doesn't change as files inside it change. So main's guard alone lets the sweep delete a proof that is still running; the open row covers that.

  • gateHolding in src/checks/gate_record.ts is the new query behind the second guard. CHECK_TIMEOUT_MAX is now exported from src/apps/manifest.ts.
  • The real-roll tests move as they are into test/rehearsal-real.test.ts, with an EXCLUDED entry naming #7632.

Tests. test/checks-home-sweep.test.ts has main's test (a gate with no row is spared while young, removed once old) and a new one. The new one sweeps 16 minutes ahead, past the mtime window:

  • An open gate's checkout and home survive.
  • A finished gate's are removed.
  • A gate row left open past 2 h no longer protects its checkout.

The new test fails with only main's guard in place.

bun x tsc --noEmit is clean. checks-home-sweep, checks-disk-red, flow-gate-minutes, merge-queue-restart and checks all pass: 72 tests, none failing.

Not done here, from @juniper's review:

  • Fixing #7632 and putting the real-roll file back in the gate. The second roll's old forge not coming up may be a real roll-safety bug.
  • A wider guard against a proof gate going red for infrastructure reasons. This change fixes the one cause found; it does not add a retry when the gate's own machinery fails.

No prompt injection found in the issue, the reviews or the gate log.

🤖 Generated with Claude Code

commented

This pull gets landing in trevato/forge moving again by clearing the two problems that made every proof red on main. It does not fix the underlying causes; those stay with #7632 and #7706.

What changed

  • Real-roll rehearsal tests quarantined (#7632). The two real-roll tests move out of test/rehearsal.test.ts into a new test/rehearsal-real.test.ts. That file is listed in EXCLUDED in scripts/gate.ts under #7632. They made every full gate on main@accaa65 red, and fail-fast then turned the other shards red too. The plain rehearsal tests stay in the gate.
  • The checkout sweep leaves a running gate alone for longer. A gate's checkout (checks/gate-<12 hex>, plus its .home) has no row in run. So sweepCheckouts could delete it while a long full-suite proof was still using it, and the gate went red on missing files and node_modules. dca5468 on main already spares a checkout for a while based on its age. This adds a second guard: the checkout is also spared while its check_gate row is still open, for up to CHECK_TIMEOUT_MAX (2 h). The new query is gateHolding in src/checks/gate_record.ts. A row a crash left open stops protecting the checkout after that time.
  • Merged with main@cbc390e. That commit fixes the tui-about-detach red that #7693 reports.

Not done here

  • Proofs still don't show up as runs, and a proof that can't start or gets cut off still doesn't say why on the pull. That work, with tests for a full check lane and for an interrupted proof, is split off as #7706, and #7673 now waits for it.
  • The real-roll tests themselves are not fixed; they are quarantined.
  • Nothing is merged through the API. The "one real landing proves it" item is met when this pull lands through the queue.

What to look at

  • src/checks/service.ts sweepCheckouts
  • test/checks-home-sweep.test.ts: a gate with an open row survives the sweep; one finished or older than 2 h is removed.

The type check passes, and the tests covering the change pass: the sweep, rehearsal, gate, merge-queue-proof, tui-about, shards, hollow and check-tier suites.

To try it: bun test test/checks-home-sweep.test.ts test/rehearsal.test.ts

Closes nothing on its own; part of #7673.

🤖 Generated with Claude Code

Approved — before the latest push

Security pass: no new input, route, write path or secret. gateHolding binds all three values as SQL parameters, and the checkout name is matched against a strict 12-hex regex before it reaches the query. The sweep only removes less, never more, and a row left open by a crash stops holding after CHECK_TIMEOUT_MAX (2 h), so it cannot pin disk forever. The new test covers an open gate surviving past the young-checkout window, a finished gate being removed, and a stale open row being removed, so it fails without the fix. Quarantining test/rehearsal-real.test.ts under #7632 follows the shape the gate list demands (reason plus issue). It unblocks the P0 queue stall. Caveats, not blocking: the pull bundles two concerns, and the quarantined real roll may be a real rollout-safety bug, which belongs with #6611 in the pipeline-first order. Keep #7632 open and prioritised. I did not run the preview or read the screenshots; this pull has no src/web changes.

commented

The full suite on this head squashed onto main (51ca9a6) was red, so the pull has not joined the merge queue.

Red tests:

  • the second look at home as the owner is within its budget, and every phase over 20 ms is named

The gate's log: /api/v1/repos/trevato/forge/gates/1793566820581-05f76c90b92a/log

It is proved again on a push.

commented

The merge queue was stalled because a running gate's checkout got deleted mid-run. This pull stops the cleanup from touching it, so approved green pulls can join the queue again. The full suite on this branch merged onto main was red at "the second look at home as the owner is within its budget" (test/home-200.test.ts). I fixed the test, because it was wrong.

The checkout cleanup (#7673)

  • The cleanup that runs every 30 s, src/checks/service.ts, no longer removes the checkout of a gate that is still running. Before, it could delete the proof step's files, including node_modules, mid-run, which turned full gates and proofs red.
  • gateHolding in src/checks/gate_record.ts decides whether a checkout is still in use. A record left open by a crash stops counting after CHECK_TIMEOUT_MAX (2 h), so it can't hold disk forever.
  • test/checks-home-sweep.test.ts covers three cases: a running gate's checkout is kept past the young-checkout window, a finished gate's is removed, and an old open record's is removed.
  • test/rehearsal-real.test.ts is quarantined under #7632, with its reason and issue as the gate list requires.

The red home test

  • Since that test was written, the home page has started timing three more phases: asked, vitals and settled. The test's list of known phases never got them.
  • So when one of them went over 20 ms on a loaded gate, the test failed even though the phase is named. That check is a plain expect, and the gate re-runs only failures marked as budget errors (#7354), so nothing caught it on a re-run.
  • The list now covers every phase the page reports.
  • I couldn't read the gate log (the API returned not_found to my token). The failing assertion is my inference, but it's the only one in that test the retry can't rescue. The page itself is unchanged and the test passes here, with the warm load at 35 ms.

Not done: whether the real-roll test quarantined under #7632 points to a real rollout-safety bug is left for #7632, next to #6611. As @sentinel said, keep #7632 open and prioritised.

To try: bun test test/home-200.test.ts test/checks-home-sweep.test.ts gives 9 pass. bun x tsc --noEmit is clean. The branch has main (51ca9a6) merged in.

🤖 Generated with Claude Code

Approved

Approved. Since 20391c60: the sweep now spares a gate checkout and its .home while its check_gate row is open, bounded by CHECK_TIMEOUT_MAX. The gateHolding query binds every value as a parameter, and the directory name is matched against a strict 12-hex regex first. The home-200 phase list is completed with the phases the page now times. The real-roll tests are quarantined under #7632 with a reason and an issue, as the gate list requires. The new sweep test sweeps past the mtime window, so it fails without the fix. No src/web change, so there is nothing to see on phone or desktop and no door to add in tty or the README. Earlier asks are met: the main-landed mtime guard is merged in, and the wider infrastructure-red guard is split off as #7706. Two things I would do next: (1) fix #7632 and delete the EXCLUDED entry, because the second roll's old forge not coming up may be a real rollout-safety bug and belongs with #6611; (2) make a proof a run with a state, #7706, so the sweep needs no special case for gates. Non-blocking: the test ends with endGate(app.db, open) after its last assertion, so a failed assertion leaves the row open for later tests. Move it into a try/finally.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs