Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:39d89j4f7b is past its budget: trevato/forge: "a pull whose review loop is spent is the planner's (#1655) > the planner is asked for it and its brief … #3993
Check · Review · Land · Live done — Landed and live in production.
This fixes a race in `test/crew-parks-planner.test.ts`, which I believe caused the flaky "the planner is asked for it and its brief says three reviews asked for changes" test. I couldn't reproduce the failure here, so the fix is untested against the real cause.
shipped production is serving it deploy 9105665
This fixes a race in test/crew-parks-planner.test.ts, which I believe caused the flaky "the planner is asked for it and its brief says three reviews asked for changes" test. I couldn't reproduce the failure here, so the fix is untested against the real cause.
Cause: the test starts a live crew with a 50 ms tick, then calls driveRepo itself and expects "planned". The crew's own tick can ask the planner first. On a loaded check machine that happens often enough that the test's call returns "idle" ("a planning run is in flight"), and the test fails even though the planner was asked correctly. I found this by reading the log, which shows that "planner not asked … in flight" line. The sibling "second park, next tick" test in the same file has the same race.
Change: both tests now accept "planned" or "idle" from their own call. They then wait up to 45 s for directionIssueId to appear before looking for the planner run. That run, and the brief the stand-in Claude Code received, are still asserted exactly as before. The verdict now depends on the code under test, not on who wins the tick.
Not done: I didn't change the production tick or the planner logic, and I didn't touch the other crew tests that may share this pattern. I ran bun test test/crew-parks-planner.test.ts three times and it passed each time. bun x tsc --noEmit couldn't run in the sandbox (read-only tempdir), so the forge's check will be its first run.
To verify: bun test test/crew-parks-planner.test.ts. The sentinel cause shared:…39d89j4f7b should clear once main's check is green again.
Test-only change that removes a real race: the live crew tick and the test's own driveRepo call can both ask the planner, and either outcome (planned or idle) still leaves the planner run and brief asserted exactly. No production code, security surface, UI or dependency is touched, so the preview is unaffected. Two things I would do next: (1) the shared 45s deadline now also bounds the later settled() wait, which fits inside the 60s test timeout but leaves little margin; give the settled wait its own deadline. (2) Accepting idle also hides the case where the planner was never asked for another reason; if directionIssueId is still null after the wait, fail with an explicit message instead of the non-null assertion, and check the other crew tests for the same tick race.
This fixes a race in
test/crew-parks-planner.test.ts, which I believe caused the flaky "the planner is asked for it and its brief says three reviews asked for changes" test. I couldn't reproduce the failure here, so the fix is untested against the real cause.Cause: the test starts a live crew with a 50 ms tick, then calls
driveRepoitself and expects"planned". The crew's own tick can ask the planner first. On a loaded check machine that happens often enough that the test's call returns"idle"("a planning run is in flight"), and the test fails even though the planner was asked correctly. I found this by reading the log, which shows that "planner not asked … in flight" line. The sibling "second park, next tick" test in the same file has the same race.Change: both tests now accept
"planned"or"idle"from their own call. They then wait up to 45 s fordirectionIssueIdto appear before looking for the planner run. That run, and the brief the stand-in Claude Code received, are still asserted exactly as before. The verdict now depends on the code under test, not on who wins the tick.Not done: I didn't change the production tick or the planner logic, and I didn't touch the other crew tests that may share this pattern. I ran
bun test test/crew-parks-planner.test.tsthree times and it passed each time.bun x tsc --noEmitcouldn't run in the sandbox (read-only tempdir), so the forge's check will be its first run.To verify:
bun test test/crew-parks-planner.test.ts. The sentinel causeshared:…39d89j4f7bshould clear once main's check is green again.Closes #3965