Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:19547qkh3k is past its budget: trevato/forge: "a walk that blows up half-way (outside any step) still removes its forge" failed on 3 pulls in the last… #12458

The flaky test "a walk that blows up half-way (outside any step) still removes its forge" is fixed. The only change is in test/release-journey.test.ts, and no production code changed.

Cause: the test runs the child forge in-process, with a stand-in crew that keeps writing into the child's data directory. Removal deletes that directory while the child is still live, so under load the child sometimes wrote its files back. That is why existsSync(dataDir) came back true on the three failed checks (#12227, #12278, #12345).

Fix: a real removal ends the child's process first, and the test now does the same. Just before the parent receives the /forges/<name>/remove POST, the test stops that child's crew, server and app. A flag, stopBeforeRemove, limits this to the one test. afterAll skips children that are already stopped.

What I left alone: the assertions are unchanged, so the test still checks that the forge's ledger entry and data directory are gone.

Verification: I ran release-journey.test.ts three times, and all 11 tests passed each time. tsc --noEmit printed nothing, so it is clean. I did not run scripts/gate.ts or the rest of the suite. The forge's check runs the full suite on this push.

To try: bun test test/release-journey.test.ts

Closes #12457

The flaky release-journey test is fixed, and the gate failure on this pull is fixed too. The change is only in test/release-journey.test.ts. I pushed it, but I haven't seen the forge's check result on the new push yet.

What was wrong: "a walk that blows up half-way (outside any step) still removes its forge" runs the throwaway child forge in the test's own process. That child's stand-in crew keeps writing into its data directory. Removal deleted the directory while the child was still live, and under load the child sometimes wrote its files back. That is why existsSync(dataDir) came back true on pulls #12227, #12278 and #12345.

What changed:

• Stop before removal: the test now stops the child's crew, server and app just before the parent receives the /forges/<name>/remove POST. A real removal ends the process first, so this matches what happens in production. A flag, stopBeforeRemove, limits it to this one test, and afterAll skips children that are already stopped.
• Gate fix: the comment I added for that flag was 187 columns wide, and the gate's 100-column rule rejected it. I rewrapped it.

No production code changed and the assertions are untouched, so the test still checks that the forge's ledger entry and data directory are gone.

Checks I ran:

• release-journey.test.ts passed 3 times out of 3, 11 tests each, on the first commit.
• On the rewrap commit I re-ran it once, 11 pass, and then bun scripts/gate.ts and bun x tsc --noEmit came back clean.
• The gate only reports clean after the commit, so it may read the committed file, not the working tree.
• I didn't run the rest of the suite; the forge's check does that.

To try it: bun test test/release-journey.test.ts

This fixes the flaky test "a walk that blows up half-way (outside any step) still removes its forge" in test/release-journey.test.ts. The cause was in the test, not in removeForge.

Cause. The test runs the throwaway child forge inside its own process, with a stand-in crew that keeps writing into the child's data directory every 100 ms. In production, removeForge puts the child to sleep and then deletes its directory. In the test, nothing stopped that stand-in, so on a loaded machine it sometimes wrote files back after the delete. That is why existsSync(dataDir) came back true on #12227, #12278 and #12345.

Fix. A flag, stopBeforeRemove, is set only in that test. When it is set, the routed fetch stops the child's crew, server and app just before the parent receives POST /forges/<name>/remove. That matches production, where the process is gone before its directory is deleted. afterAll skips children that are already stopped. The assertions are unchanged, so the test still fails if removal leaves the ledger file or the data directory behind. The second commit only rewraps the new comment to the gate's 100-column limit.

Not done. I left src/spawn/manage.ts alone: in production the child is already asleep before its data is removed, so there is no race there to fix.

Checks.

• release-journey.test.ts passed 3 runs out of 3 (11 tests each). After the rewrap I ran it once more, and it passed again.
• bun scripts/gate.ts and bun x tsc --noEmit are clean.
• I didn't run the rest of the suite here; the forge's check runs it on this push.

The new message on the issue is the automatic triage filing it under P0 Trust. It needed no code change.

To try it: bun test test/release-journey.test.ts

🤖 Generated with Claude Code