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

test/week-pump.test.ts never takes its shard down: it passes or names its failing test #9983

Merged opened this

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

Merged

The crew's tick no longer takes the process down when it throws, and `crew.stop()` now waits for the sweeps a tick started. In `test/week-pump.test.ts`, every wait now fits inside the test's own timeout. What the test asserts (#8873) is unchanged.

shipped production is serving it deploy a5a2512

opened this

The crew's tick no longer takes the process down when it throws, and crew.stop() now waits for the sweeps a tick started. In test/week-pump.test.ts, every wait now fits inside the test's own timeout. What the test asserts (#8873) is unchanged.

Why the shard exited without a (fail) line. I couldn't confirm the exact error: the check log the API returns for #9964 is only the tail and doesn't include shard 19's output, and the failure didn't reproduce locally. Of the three suspects in the issue, the cause I found in the code is a timer left running. The stand-in crew's 50 ms tick (src/crew/dispatch.ts) called requeueInterrupted, recheckForDisk and pump directly inside setInterval, with no guard. Any throw there, such as a refused or busy database, became an uncaught error. While a test is running, bun puts that error on the test. Between tests, during a hook or after a file's last test, it is an error outside any test: the shard goes red with no (fail) line, which is what shard 19 showed. Nothing in the forge outside the TUI catches uncaught errors, so the same throw would also stop production.

Also, stop() waited for runs in flight but not for the settleAll/autonomyTick sweeps a tick had started, so one could still read app.db after afterAll closed it.

until's 20 s deadline was shorter than the 30 s test timeout on its own. But the first test waits twice and the second three times, so the deadlines could add up past the timeout (40 s and 60 s). A slow tick would then hit bun's timeout instead of the test's own timed out waiting for … message. It didn't cause this exit (the shard ended at 15 s), but it's fixed below.

Changes

  • src/crew/dispatch.ts: a tick that throws is logged as crew tick failed and the next tick runs normally. The tick tracks the two sweeps it starts, and stop() waits for them for at most 10 s (SWEEPS_STOP_MS) before it requeues interrupted runs, so a sweep stuck on the network can't hold up a restart. That cap's timer is cleared once the sweeps settle.
  • test/week-pump.test.ts: each wait gets 8 s (WAIT_MS); three waits are 24 s, inside the 30 s TEST_MS. afterAll gets TEST_MS as its timeout for the stop.
  • test/crew-tick.test.ts (new): a run left running with a lapsed lease, and the database set to query_only, so the tick's first write throws. The test checks that the crew keeps ticking and cancels and requeues the run once writes are allowed again. With the old dispatch.ts it fails with SQLiteError: attempt to write a readonly database thrown from the timer.

test/helpers/crew-standin.ts is unchanged; every suite that uses it gets the fix through crew.stop().

Not done: the wait for sweeps has no direct test. Testing it would mean a way to inject a slow autonomyTick or settleAll, which I didn't add. I also didn't track the startSweep that autonomyTick starts without awaiting; its errors are already caught and logged.

Try it

(for j in $(seq 40); do bun test test/crew-interrupted.test.ts test/crew-held.test.ts >/dev/null 2>&1; done) &
for i in $(seq 20); do bun test test/week-pump.test.ts || break; done

Result: passed 20 of 20 in a row with the other bun test running alongside, each run 2 pass, 0 fail in about 1.9 s.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit pass. scripts/fast.ts named 519 files (about 46 minutes), so instead I ran the 55 files that drive startCrew or the stand-in crew in one process: 353 pass, 2 skip, 1 fail. The failure is test/protection.test.ts › a fully protected forge has no notice, which picked up the "no sandbox here" notice from crew files earlier in the same process. It passes on its own (4 pass), and this change doesn't touch it.

🤖 Generated with Claude Code

Closes #9977

commented

The crew's tick no longer takes the process down when it throws, and crew.stop() now waits for the sweeps a tick started. In test/week-pump.test.ts, every wait now fits inside the test's own timeout. What the test asserts (#8873) is unchanged.

Why the shard exited without a (fail) line. I couldn't confirm the exact error: the check log the API returns for #9964 is only the tail and doesn't include shard 19's output, and the failure didn't reproduce locally. Of the three suspects in the issue, the cause I found in the code is a timer left running. The stand-in crew's 50 ms tick (src/crew/dispatch.ts) called requeueInterrupted, recheckForDisk and pump directly inside setInterval, with no guard. Any throw there, such as a refused or busy database, became an uncaught error. While a test is running, bun puts that error on the test. Between tests, during a hook or after a file's last test, it is an error outside any test: the shard goes red with no (fail) line, which is what shard 19 showed. Nothing in the forge outside the TUI catches uncaught errors, so the same throw would also stop production.

Also, stop() waited for runs in flight but not for the settleAll/autonomyTick sweeps a tick had started, so one could still read app.db after afterAll closed it.

until's 20 s deadline was shorter than the 30 s test timeout on its own. But the first test waits twice and the second three times, so the deadlines could add up past the timeout (40 s and 60 s). A slow tick would then hit bun's timeout instead of the test's own timed out waiting for … message. It didn't cause this exit (the shard ended at 15 s), but it's fixed below.

Changes

  • src/crew/dispatch.ts: a tick that throws is logged as crew tick failed and the next tick runs normally. The tick tracks the two sweeps it starts, and stop() waits for them for at most 10 s (SWEEPS_STOP_MS) before it requeues interrupted runs, so a sweep stuck on the network can't hold up a restart. That cap's timer is cleared once the sweeps settle.
  • test/week-pump.test.ts: each wait gets 8 s (WAIT_MS); three waits are 24 s, inside the 30 s TEST_MS. afterAll gets TEST_MS as its timeout for the stop.
  • test/crew-tick.test.ts (new): a run left running with a lapsed lease, and the database set to query_only, so the tick's first write throws. The test checks that the crew keeps ticking and cancels and requeues the run once writes are allowed again. With the old dispatch.ts it fails with SQLiteError: attempt to write a readonly database thrown from the timer.

test/helpers/crew-standin.ts is unchanged; every suite that uses it gets the fix through crew.stop().

Not done: the wait for sweeps has no direct test. Testing it would mean a way to inject a slow autonomyTick or settleAll, which I didn't add. I also didn't track the startSweep that autonomyTick starts without awaiting; its errors are already caught and logged.

Try it

(for j in $(seq 40); do bun test test/crew-interrupted.test.ts test/crew-held.test.ts >/dev/null 2>&1; done) &
for i in $(seq 20); do bun test test/week-pump.test.ts || break; done

Result: passed 20 of 20 in a row with the other bun test running alongside, each run 2 pass, 0 fail in about 1.9 s.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit pass. scripts/fast.ts named 519 files (about 46 minutes), so instead I ran the 55 files that drive startCrew or the stand-in crew in one process: 353 pass, 2 skip, 1 fail. The failure is test/protection.test.ts › a fully protected forge has no notice, which picked up the "no sandbox here" notice from crew files earlier in the same process. It passes on its own (4 pass), and this change doesn't touch it.

🤖 Generated with Claude Code

commented

When the crew's tick throws, the error is now logged and the next tick tries again, where before it took the process down. stop() also waits for the sweeps a tick started, so none of them reads app.db after its caller has closed it (#9977).

Tick (src/crew/dispatch.ts). The tick body is wrapped in a try/catch. Before, a refused or closed database thrown from the setInterval was an uncaught error, and a test shard would exit without printing a (fail) line. stop() now waits for the converge and autonomy sweeps the tick started, for at most 10 s, so one stuck on the network can't hold up a restart. test/crew-tick.test.ts covers this.

test/week-pump.test.ts. Each of the test's three waits now has an 8 s deadline, so together they fit inside the 30 s test timeout. A slow tick then fails as that test, by name, instead of killing the shard. afterAll has room for the stop.

Flaky stagger tests (test/crew-week.test.ts). Last push, the check failed on "the stagger reads yesterday's spend once for one instant". This wasn't caused by the changes above; the test itself was wrong, which is why I changed the test and not the code. The three stagger tests asked staggeredUntil at 00:05. Each repository's restart offset comes from a hash of its id, and test ids are random, so about 1 in 70 times a repository's offset fell before 00:05. That repository was already free, and the test saw null where it expected a hold. The tests now ask at exactly 00:00, which is earlier than every offset except one of exactly 0 ms.

bun scripts/gate.ts and bun x tsc --noEmit pass. test/crew-week.test.ts passed three runs in a row, and test/crew-tick.test.ts and test/week-pump.test.ts pass.

🤖 Generated with Claude Code

commented

When the crew's tick throws, the error is now logged and the next tick tries again; before, it took the process down. stop() also waits for the sweeps a tick started, so none of them reads app.db after its caller has closed it. This push also fixes two flaky tests that turned the check red. I couldn't reproduce either test failure locally, and both edits change the tests, not the code they test.

Tick (src/crew/dispatch.ts). The tick body is wrapped in a try/catch. Before, a refused or closed database thrown from the setInterval was an uncaught error, and a test shard would exit without printing a (fail) line. stop() now waits for the converge and autonomy sweeps the tick started, for at most 10 s, so one stuck on the network can't hold up a restart. test/crew-tick.test.ts covers this.

test/week-pump.test.ts. Each of the test's three waits now has an 8 s deadline, so together they fit inside the 30 s test timeout. A slow tick then fails as that test, by name, instead of killing the shard.

test/crew-week.test.ts (a test failure on the first push). The three stagger tests asked staggeredUntil at 00:05. Each repository's restart offset comes from a hash of its id, and test ids are random, so about 1 in 70 times a repository's offset came before 00:05. That repository was already free, and the test saw null where it expected a hold. The tests now ask at exactly 00:00.

test/features.test.ts (the red shard on the second push). The check reported "shard 24 killed, running test/week-pump.test.ts", but week-pump's own tests passed. "Killed" is just how the shard runner names a red shard that names no failing test. The full check log shows RangeError: Cannot use a closed database from finishRun (src/apps/runtime.ts:1225). It was raised as an unhandled error between tests during test/gate-interrupted.test.ts, and it happened in both the first run and the re-run. It came from an earlier file, test/features.test.ts:

  • That test constructs an AppsRuntime, which registers itself as the app's runtime, and never stops it.
  • absorbApp and unabsorbApp then ask it to redeploy (reconcileApp(…, "redeploy")), and nothing waited for those deploys.
  • One of them failed after afterAll had closed the database.

The test now runs await runtime.stop() in its finally; stop() already waits for in-flight deploy chains.

bun scripts/gate.ts and bun x tsc --noEmit pass. Run together in one process (as the shard does), the seven files involved pass twice in a row. Those are features, fleet-archive, flow-rebase-first, gate-interrupted, week-pump, crew-week and crew-tick.

🤖 Generated with Claude Code

Approved

Approved. The tick no longer throws from the timer, stop() waits for the sweeps, week-pump waits fit inside the test timeout, and the new crew-tick test covers the throw. Two things I would do next: (1) put the 20x loop command and its result back in the pull description; the latest description dropped it. (2) Test the bounded wait for sweeps in stop() by injecting a slow sweep. Scope note: the crew-week and features.test edits fix real red shards and are justified in the pull.

commented

Left #9977 open: 1 unticked item remain (for i in $(seq 20); do bun test test/week-pump.test.ts || break; done passes 20 times in a row with another bun test running alongside it. Put the command and its result in the pull.).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs