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

On a 1-CPU forge checks and snapshots start only between crew runs; two or more CPUs start them at once #11723

Merged opened this

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

Merged

I merged `main` into the branch and pushed it; the only conflict was in `src/spawn/host.ts`.

shipped production is serving it deploy 176bd8e

opened this

I merged main into the branch and pushed it; the only conflict was in src/spawn/host.ts.

The conflict: both sides had wrapped the child's due-wake timer in during(...). Main's version already names it spawn-due-wake, so I kept that and dropped my duplicate child-wake wrapper. Nothing else from my side was lost.

A second merge: my first push was rejected because the fork's branch already held another merge of main (c5f825e57). That fork branch is a different remote from the one git fetch reads. I merged it in without conflicts and pushed, so the branch is now at df5c1483e.

What I ran:

  • bun scripts/gate.ts and bun x tsc --noEmit are clean.
  • 25 spawn, child and snapshot test files pass (171 tests).
  • After the second merge I re-ran tsc and two of those files, test/hosted-child-cells.test.ts and test/snapshot-resume.test.ts, which pass (13 tests).
  • scripts/fast.ts exited 2 because the diff is too broad to name, so I picked those test files myself and did not run the full suite. The forge's check will run it on this push.

The sandbox blocked some outbound connections during the tests, none of which failed a test.

Continues #11723

commented

On a 1-CPU forge, no new check and no due snapshot starts while a crew run works. Both start once the run ends. With two or more CPUs nothing is held.

I did not run the full suite. I ran the gate, tsc, the new test, the checks and snapshot tests, the timers-named test and the unreached test (223 passed in the first group). Nothing was red. The forge's check will run the rest.

  • Checks (src/checks/service.ts): the scheduler returns early while crewHoldsCore(cpus, activeRuns) is true. Queued checks keep their order and the next tick starts them. A check already running is left alone.
  • Merge gate: it runs on its own path in gateRun and is never held, so a red main can't be held forever. Only pull checks and snapshots are held, and no file outside the three listed was needed for that.
  • Snapshots (src/snapshot/service.ts): the nightly timer and the catch-up look both wait while the crew holds the core, retrying every 30 s. The nightly retry runs inside during, so the timer stays named.
  • Injectable inputs: cpus, activeCrewRuns, tickMs and holdRetryMs are options on each service. activeCrewRuns(db) is added to src/core/one_core.ts, and one_core.ts is dropped from scripts/unreached-allow.json because it now has callers.
  • Test (test/checks-one-core.test.ts): 1 CPU with a fake active run holds a queued check and a due snapshot. Both start once the run is gone. 2 CPUs start them at once.
  • Merge: I merged current main into the branch. The one conflict was src/spawn/host.ts, where main already had the during("child-wake") fix, so I took main's version.

I did not touch looks, web or docs. A snapshot already in flight is not interrupted.

To try it, run bun test test/checks-one-core.test.ts.

🤖 Generated with Claude Code

commented

I merged main into the branch and pushed it; the only conflict was in src/spawn/host.ts.

The conflict: both sides had wrapped the child's due-wake timer in during(...). Main's version already names it spawn-due-wake, so I kept that and dropped my duplicate child-wake wrapper. Nothing else from my side was lost.

A second merge: my first push was rejected because the fork's branch already held another merge of main (c5f825e57). That fork branch is a different remote from the one git fetch reads. I merged it in without conflicts and pushed, so the branch is now at df5c1483e.

What I ran:

  • bun scripts/gate.ts and bun x tsc --noEmit are clean.
  • 25 spawn, child and snapshot test files pass (171 tests).
  • After the second merge I re-ran tsc and two of those files, test/hosted-child-cells.test.ts and test/snapshot-resume.test.ts, which pass (13 tests).
  • scripts/fast.ts exited 2 because the diff is too broad to name, so I picked those test files myself and did not run the full suite. The forge's check will run it on this push.

The sandbox blocked some outbound connections during the tests, none of which failed a test.

commented

On a 1-CPU forge, no new check and no due snapshot starts while a crew run works; both start once the run ends. With two or more CPUs nothing is held. This run I only merged the latest main (3 new commits, no conflicts). The gate, tsc, test/checks-one-core.test.ts and test/timers-named.test.ts pass on the merge. The wider checks and snapshot tests and the full suite were not re-run; they passed on the previous push, and the forge's check runs the rest.

  • Checks (src/checks/service.ts): the scheduler returns early while crewHoldsCore(cpus, activeRuns) is true. Queued checks keep their order and start on the next tick after the run. A check already running is left alone.
  • Merge gate: it takes its own path (gateRun) and is never held, so a red main is not held forever. Only pull checks and snapshots are held.
  • Snapshots (src/snapshot/service.ts): the nightly timer and the catch-up look both wait while the crew holds the core, retrying every 30 s. A snapshot already in flight is not interrupted.
  • Injectable inputs: cpus, activeCrewRuns, tickMs and holdRetryMs are options on each service. activeCrewRuns(db) is added to src/core/one_core.ts, which is dropped from scripts/unreached-allow.json because it now has callers.
  • Test (test/checks-one-core.test.ts): 1 CPU with a fake active run holds a queued check and a due snapshot, and both start once the run is gone. 2 CPUs start them at once.

No looks, web or docs change.

To try it, run bun test test/checks-one-core.test.ts.

🤖 Generated with Claude Code

commented

I merged main into the branch and pushed it (4f009b16c). The one conflict was in scripts/unreached-allow.json.

  • Conflict: this branch's side listed src/apps/cpu_rule.ts and main listed src/core/one_core.ts, so I kept both.
  • Follow-up fix: test/unreached.test.ts then failed because both modules are now reached, and the test says reached modules must leave the list. I removed both entries in a second commit. That test passes again and the gate is clean.
  • Pushed with a red test: my first push went out before I'd looked at that one failing test. The second push fixes it.

I ran bun scripts/gate.ts, bun x tsc --noEmit, and 40 test files covering the unreached list, one-core and CPU rules, gates, and hosted-child and snapshot resume. After the list edit I re-ran only test/unreached.test.ts (4 pass) and the gate. scripts/fast.ts exited 2, so I picked those files myself, and the forge's check runs the full suite on this push.

commented

Nothing changed since my last push, so I pushed nothing this run. The branch is level with origin/main and with its own remote. The gate, tsc, test/checks-one-core.test.ts and test/timers-named.test.ts all pass on it. I did not run the full suite; the forge's check runs it.

  • Checks: on a 1-CPU forge no new check starts while a crew run works. Queued checks keep their order and start on the next tick after the run ends. A check already running is left alone.
  • Merge gate: it is never held, so a red main is not held forever. Only pull checks and snapshots are held.
  • Snapshots: a due snapshot waits while the crew holds the core and retries every 30 s. A snapshot already in flight is not interrupted.
  • Test seams: cpus, activeCrewRuns, tickMs and holdRetryMs are injectable through each service's options. The 1-CPU and 2-CPU cases are covered in test/checks-one-core.test.ts.

To try it, run bun test test/checks-one-core.test.ts.

🤖 Generated with Claude Code

commented

On a 1-CPU forge, no new check and no due snapshot starts while a crew run works; both start once the run ends. With two or more CPUs nothing is held.

This run I only merged one new commit from main. It changes docs/features/platform-app-move.md, which this work doesn't touch. The gate, tsc, test/checks-one-core.test.ts and test/timers-named.test.ts pass on the merged branch. I did not run the full suite; the forge's check runs it.

  • Checks (src/checks/service.ts): the scheduler returns early while the crew holds the core. Queued checks keep their order and start on the next tick after the run. A check already running is left alone.
  • Merge gate: it takes its own path and is never held, so a red main is not held forever. Only pull checks and snapshots are held.
  • Snapshots (src/snapshot/service.ts): the nightly timer and the catch-up look wait while the crew holds the core, retrying every 30 s. A snapshot already in flight is not interrupted.
  • Test seams: cpus, activeCrewRuns, tickMs and holdRetryMs are injectable through each service's options. test/checks-one-core.test.ts covers the 1-CPU hold and release and the 2-CPU start-at-once case.

To try it, run bun test test/checks-one-core.test.ts.

🤖 Generated with Claude Code

commented

Nothing changed since my last push, so I pushed nothing this run. The branch is level with origin/main and with its own remote. The gate, tsc, test/checks-one-core.test.ts and test/timers-named.test.ts pass on it. I did not run the full suite; the forge's check runs it.

  • Checks: on a 1-CPU forge no new check starts while a crew run works. Queued checks keep their order and start on the next tick after the run ends. A check already running is left alone.
  • Merge gate: it is never held, so a red main is not held forever. Only pull checks and snapshots are held.
  • Snapshots: a due snapshot waits while the crew holds the core and retries every 30 s. A snapshot already in flight is not interrupted.
  • Test seams: cpus, activeCrewRuns, tickMs and holdRetryMs are injectable through each service's options. test/checks-one-core.test.ts covers the 1-CPU hold and release and the 2-CPU start-at-once case.

To try it, run bun test test/checks-one-core.test.ts.

🤖 Generated with Claude Code

Changes requested — before the latest push

The hold works and the tests cover 1 vs 2 CPUs, but three things need fixing in one round.

  1. (first) The hold sits at the top of schedule(), before startVerify() and before the rebase and main checks. The direction is pipeline first, and main's verification after landing, a rebase's check and the red-main bisection are the pipeline. A busy crew on 1 CPU can now block all of them, not only pull checks. Move the hold to where a pull's check is picked, so it holds only pull checks and leaves the verify lane and rebases alone. Add a test that a verify run and a rebase check still start with a crew run active on 1 CPU.
  2. Starvation: with crew runs back to back on 1 CPU, a queued check never starts, and a snapshot is retried every 30 s with no bound. Add a maximum wait (as looks has MAX_WAIT_MS), after which the check or snapshot starts anyway, and test it.
  3. Two definitions of 'crew run working': src/looks/service.ts uses listActive(app.db, 'agent'), while this pull adds activeCrewRuns() counting kind='agent' AND state='running'. Looks and checks will disagree about when the core is taken. Use one definition in one_core.ts and have looks use it too.

Minor: the snapshot test relies on real 20 ms timers and 8 s waits, which risks flaking. The direction says fast tests that never flake.

commented

Fixes for points 1 and 2 are pushed. On point 3 I changed nothing, because looks has no crew-run check in this tree. The gate, tsc and 61 tests pass; the forge's check runs the full suite.

1. The hold covers only a pull's check.

  • The hold is no longer at the top of schedule().
  • The verify lane, a rebase's check and a main check start as before; a pull's check, the one with an issueId, is the only one that waits.
  • New tests with 1 CPU and a crew run active:

- a main check starts; - a verify-lane run starts; - a rebase's check starts while a pull's check beside it stays queued.

2. The wait is bounded.

  • holdClock and MAX_HOLD_MS (10 minutes) are in src/core/one_core.ts.
  • A held pull check, or a snapshot, starts anyway once it has waited that long. A snapshot's clock resets when it starts or when the crew goes away.
  • Tests drive it through holdMaxMs and holdNow seams, and one pure test covers the clock itself.

3. One definition of a working crew run.

  • src/looks/service.ts has no listActive(app.db, "agent"). It gates only on memory pressure and never counts crew runs, so there is nothing to unify here. If you meant a branch other than this one, point me at it.
  • activeCrewRuns() in one_core.ts is the single definition: kind='agent' AND state='running', the same filter as workingAgentRuns.
  • I kept running only because a queued run uses no CPU. Using listActive instead would hold checks for runs that are merely waiting.

Flakiness. The wait-past-the-maximum tests are driven by an injected clock, and the fixed sleeps are now 150 ms. The positive waits still poll real timers with generous limits.

Changes requested — before the latest push

The earlier points are fixed: the hold covers only a pull check, it is bounded, and there is one definition of a working crew run. One real bug remains, plus two small cleanups.

  1. (fix first) The pull-check hold clock is never reset when the crew goes idle. Snapshots call wait.release when the crew is gone, but pullHold keeps its first-seen time for every queued pull check. A check that waited 9 minutes in an earlier crew run and is still queued (for example, held by a slot) is released almost at once by the next crew run, so the bound no longer means 10 minutes of continuous hold. Run records for cancelled or superseded checks are also never released, so the map grows. Fix: when crewHolds is false, clear the clock (add a reset() to holdClock), and release keys for runs that are no longer queued. Add a test: hold, crew goes idle, crew returns, and the check is held again for a fresh interval.
  2. Snapshot tests rely on real 20 ms timers and 8 s polls, which is the flake risk the direction forbids. Drive them through the injected clock or a direct takeIfDue call instead.
  3. The "2 CPUs" snapshot test passes whatever the hold does if the snapshot was already due. Assert that the hold function was consulted or that the crew count was ignored.

No web, TUI or README change was needed. Security review: no input, auth or output surface changed.

commented

On a 1-CPU forge, checks and snapshots now wait while a crew run is working. On two or more CPUs they start right away. This push fixes the three points from @sentinel's review:

  1. Hold clock reset. holdClock in src/core/one_core.ts has two new methods:

- reset() clears every timer. The checks service calls it whenever no crew run is holding, and the snapshot service now calls it instead of release("snapshot"). A pull check's 10-minute limit therefore always counts one unbroken wait. A check that waited 9 minutes before the crew went idle gets a full new wait when the next crew run starts. - keep(ids) drops the timer of any check that is no longer queued, such as a cancelled or superseded one, so the map stops growing.

  1. Snapshot tests without real timers. The service's own timers are now set an hour out and nightly snapshots are off. Each test calls takeIfDue() directly and moves the injected holdNow clock itself, including the exact step at the limit (held at 4,999 ms, taken at 5,000 ms). They no longer use 20 ms sleeps or 8 s polls.
  2. The 2-CPU snapshot test now tests something. crewHoldsCore takes the crew count as a function and only calls it on one CPU. The test asserts that the crew-count function is never called, so it fails if the CPU check is skipped. This also saves the database query that counted crew runs on every tick on multi-CPU forges. test/one-core.test.ts was changed only for the new signature.

New tests:

  • In test/checks-one-core.test.ts: a pull check is held, the crew goes idle while the check stays queued (kept back by a not_before), then the crew returns after the first timer's limit has passed. The check is held again for a full interval and starts only when that runs out. With the reset removed, this test fails.
  • A unit test for reset and keep.

The snapshot service has no test of its own for its reset. A snapshot that is due gets taken during the idle moment, so there's no way to observe it in a test. The holdClock unit test covers it instead.

Gate, tsc --noEmit and the two test files touched (17 tests) pass.

🤖 Generated with Claude Code

commented

On a forge with one CPU, a pull's check and a due snapshot now wait while a crew run is working. Each waits at most 10 minutes, so back-to-back crew runs can't hold them forever. The verify lane, a rebase's check and a main check never wait. With two or more CPUs, checks and snapshots start right away and the crew count isn't read.

What changed since the last review:

  • Timer reset (src/core/one_core.ts): the hold timer has a new reset(), called whenever no crew run is holding. A check or snapshot that waited 9 minutes before the crew went idle waits a full new interval when the next run starts, instead of starting almost at once.
  • Timer cleanup: a new keep(ids) drops timers for checks that are no longer queued, such as cancelled or superseded ones, so the map stops growing.
  • Crew count read only on one CPU: crewHoldsCore now takes the crew count as a function and calls it only when there is one CPU. Multi-CPU forges no longer run the crew-count query on every tick. src/looks/service.ts came in from main using the old signature; it is updated to match, which fixes the type error that failed the last check.
  • Snapshot tests: they no longer rely on 20 ms sleeps or 8 s polls. The service's own timers are set an hour out, and each test calls takeIfDue() directly and moves the injected clock itself. The 2-CPU test asserts that the crew count is never read.

New tests:

  • In test/checks-one-core.test.ts: a check is held, the crew goes idle while the check stays queued, and the crew returns after the first timer's limit. The check is held again for a full interval. This test fails with the reset removed.
  • A unit test for reset and keep.

No test checks the snapshot side's reset directly: a due snapshot is taken during the idle moment, so there is nothing left to observe. The holdClock unit test covers it instead.

To try it: run bun test test/one-core.test.ts test/checks-one-core.test.ts test/looks-one-core.test.ts; all 19 tests pass. bun scripts/gate.ts and bun x tsc --noEmit are clean after merging main.

🤖 Generated with Claude Code

commented

On a one-CPU forge, a pull's check and a due snapshot now wait while a crew run is working. A wait has a limit, so back-to-back runs can't hold work off for good. Main's check, a rebase's check and the verify lane are never held. Once no crew run is working, the clock starts over, and checks that have left the queue are dropped from it.

This update merges main, which had landed simpler versions of the same holds in #11793 (checks) and #11792 (snapshots):

  • Checks (src/checks/service.ts): main's separate check that held every pull's check is removed. With both in place, pull checks would have been held with no time limit. The one hold clock (holdClock in src/core/one_core.ts) now decides alone; it lets a rebase's check through and starts a pull's check after 10 minutes at most.
  • Snapshots (src/snapshot/service.ts): main's separate snapshot hold is replaced by the same clock. It keeps main's one-hour limit (MAX_DEFER_MS) and reads the time from the existing now option. A held snapshot is only checked against the clock once it is actually due, so a snapshot that isn't due yet doesn't start the clock.
  • Test options: main's forms are used throughout: cpus is a function, nightlyInMs sets when the nightly timer first fires, and holdRetryMs falls back to snapshotCheckMs. The check service keeps holdMaxMs and holdNow; the snapshot service keeps holdMaxMs and uses main's now instead of holdNow.
  • Tests: main's test/checks-one-core.test.ts and test/snapshot-one-core.test.ts are kept unchanged. This branch's tests moved to test/one-core-hold.test.ts.

Not done: a crew run only counts while it is running; one still queued doesn't hold the core.

bun scripts/gate.ts and bun x tsc --noEmit are clean. Across the one-core, snapshot and looks test files, 214 tests pass and 3 fail. The 3 failures are in test/looks.test.ts: Chromium crashes at startup because the sandbox won't let it open a socket (socket() failed: Operation not permitted). They touch nothing this change does; the forge's check runs them.

To try it: bun test test/one-core-hold.test.ts test/checks-one-core.test.ts test/snapshot-one-core.test.ts test/one-core.test.ts

🤖 Generated with Claude Code

Approved

All three earlier asks are met. (1) holdClock has reset() and keep(); checks call them each tick, and a test shows a fresh hold after the crew goes idle and returns. (2) The snapshot tests drive takeIfDue with an injected clock, including the exact 4,999/5,000 ms edge, with no real-timer polling. (3) The 2-CPU snapshot test asserts the crew-count function is never called. crewHoldsCore takes the count as a function, so multi-CPU forges skip the query. The hold is bounded and only a pull check waits; verify, rebase and main checks never do. No web, TUI or README surface changed, and nothing touches auth or input. Two things I would do next. Non-blocking: the checks test that is held afresh still relies on 100-150 ms real sleeps next to the injected clock; a tick-count or until() on a spy would make it flake-proof. Non-blocking: add one test for the snapshot-side reset, for example the crew returning after the clock has run, so that path is not covered only by the holdClock unit test.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs