fix(test): remove the registration race behind the fleet.test.ts 5s timeout (#442)

[khaliqgant]

Fixes#442.

The 5s timeout was the messenger, not the cause

Running the test with a generous timeout instead of the 5s default is what exposed the real behaviour:

npx vitest run src/cli/fleet.test.ts -t 'keeps relay dispatch ownership…' --testTimeout=120000
→ 32237ms, then:
AssertionError: expected 1 to be +0 // expect(code).toBe(0)

It does not hang. It runs ~32s and then fails an assertion, with this on the CLI's stderr:

[factory] error {"name":"RemoteAgentRegistrationTimeoutError",
"message":"Remote agent ar-77-impl-pear did not register with the fleet before the startup deadline"}

This matters for the fix#442 proposed: raising testTimeout would not have made this test pass. It only converts the timeout into a hard assertion failure. The 5s default was hiding a genuinely broken run, not creating one.

Mechanism

CompletingRemoteFleetClient extends CompletingRemoteFleetBase, which sets placementLocality = 'remote'. For a remote placement, Factory waits for the spawned agent to become roster-visible before owning it:

• factory.ts:11383 → #awaitRemoteAgentRegistration(...)
• polls roster() every 500ms against REMOTE_AGENT_REGISTRATION_TIMEOUT_MS = 30_000 (factory.ts:459) — a real wall clock; no fake timers here. The 30s constant is exactly the ~32s observed.

The fake emitted the implementer's exit straight out of spawn:

if(input.name.includes('-impl-')){setTimeout(()=>this.emitAgentExit(input.name,'exited'),0)}

and FakeFleetClient.emitAgentExit (src/testing/fakes.ts:418) deletes the agent from the roster before notifying listeners:

emitAgentExit(name: string,reason?: string): void{this.#agents.delete(name)// ← roster entry gonethis.#tracked.delete(name)for(constlistenerofthis.#exitListeners)listener(name,reason)}

Between spawn returning and the first registration poll there are real awaits (#dispatchLifecycleStillOwned, file I/O), so the setTimeout(…, 0) macrotask and the registration probe raced the same roster entry:

who wins	outcome
probe first	registration observed → dispatch proceeds → green
timer first	agent already deleted → never observable → dispatch burns the full 30s deadline, exits 1 → "timed out in 5000ms"

The scheduler picked the winner. That is why an idle runner passed, a loaded one failed, and it reproduced on bare main.

What changed

1. dcb827f — the fix. Arm the exit in spawn, release it from an isAgentRegistered probe, which #awaitRemoteAgentRegistration prefers over a roster read (factory.ts:11422). The exit still arrives asynchronously, without the test body saying when — the ordering this class exists to provide — but now lands causally after registration instead of racing it. Test-only; no production code touched, no assertion weakened, skipped or removed.

2. b578b6c — a MITIGATION, explicitly not a fix. File-scoped vi.setConfig({ testTimeout: 40_000 }) in fleet.test.ts. This covers a second, independent problem that the race fix does not explain: these CLI tests each drive a whole dispatch through real filesystem I/O and cost 2–10s apiece, so at the 5s default a loaded runner fails a rotating cast of them — #442's own "each rerun fails a different test" observation. Measured on 8 cores at load average ~155, one run failed 10 distinct tests, every one between 4.3s and 10.4s.

It is scoped to this file, not global, because the repo deliberately configures no global testTimeout (documented in sweep-counters.test.ts) and uses per-test budgets — 99 of them. It is set above the 30s production deadline on purpose: a smaller budget would truncate a regressed race into a bare "Test timed out", which is the exact masking that hid this bug. It buys headroom and makes no test cheaper.

3. 963e801 — comment-only. The class comment described the race as intended behaviour, which invited its restoration.

Evidence

Determinism — 20 sequential runs of the whole file, on 8 cores at load average 26–155:

ITERATIONS=20 FAILED_ITERATIONS=2 LOOP_EXIT=2
named test ("keeps relay dispatch ownership…"): passed 20 / failed 0

The loop exit code is 2, not 0, and that is reported rather than smoothed over. Both failing iterations were different tests, each failing once, both as assertion failures, and neither uses the class this PR changes (CompletingRemoteFleetClient is used by exactly one test — the named one):

1 × serializes terminal completion that starts after the ready read against the dispatch claim
1 × does not attribute a third-party GitHub park from an in-flight 'legacy void' receipt
AssertionError: expected false to be true

These are pre-existing ordering flakes of the #342 shape, not timeouts, and not introduced here. Raising the budget above 5s makes them surface as honest assertion failures instead of being masked as timeouts. They deserve their own issue rather than being absorbed into this one.

For the named test:

	duration	result
before	32237ms	fail (5s timeout; assertion code===1 at 120s)
after	329–1821ms	pass, 170/170 in the file

Mutation testing — the test is not vacuous. Two independent defects reintroduced into factory.ts, both caught:

1. skip exit-path PR publication (#tryPublishImplementerPr → early return {}) → RED; dispatch never sees a published PR.
2. wrong release reason (#finishDurableRelease → always 'issue-done') → RED in 3911ms:
   expected [ 'issue-done', 'issue-done' ] to deeply equal [ 'issue-human-review', … ]

Both reverted; the diff touches only src/cli/fleet.test.ts.

Relationship to #342 / #353

• flake: shared timing substrate in src/orchestrator/factory.test.ts fails 3 different tests nondeterministically #342 — same root shape. A scheduler-decided ordering race in the same placement/ownership machinery, where both outcomes are real code paths. The two residual flakes above are further instances. Worth treating as one class rather than three patches.
• flake: tailscale-preview 'bounds a slow readiness probe by one wall-clock deadline' fails under CI load #353 — unrelated. A genuine wall-clock assertion (expected 492 to be less than 250); no ordering fix addresses it.

Residual

The 30s cliff is gone, which is what made this fail hard rather than slowly. What remains is that this file does seconds of real I/O per test and still harbours #342-shaped assertion races. The real cure is making these tests stop doing real I/O; the timeout only buys room.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gv6gy7NUDmJs5SUcDvQNK2

[coderabbitai]

Warning

Review limit reached

Next included review available in 17 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e85326c5-7ff4-40d7-8acd-610eb9b48b74

📥 Commits

Reviewing files that changed from the base of the PR and between 23e97ca and 963e801.

📒 Files selected for processing (1)

• src/cli/fleet.test.ts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed2026-09-04T12:19:11.859071Z	dcb827f	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

@coderabbitai review

Requested for exact head dcb827f0a085f14ac86b8ac86110af1c8abdc8f5.

[github-actions]

@coderabbitai review

Requested for exact head 53154772b6b233e1353d4099c66d43b1b71744ab.

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[github-actions]

@coderabbitai review

Requested for exact head b578b6cb4f26415eb266d8de1326b278cfa0786b.

[github-actions]

@coderabbitai review

Requested for exact head 963e8016233a6e22e0dc5f3b7b99a5b277ac4818.