fix(cli): configurable dev readiness timeout; de-flake child-disposal test

[blove]

Summary

The dev-command disposal test ("disposes a newly spawned child when startup readiness fails") flaked repeatedly in CI (Timed out waiting for path /tmp/dawn-dev-child-pid-*.txt), blocking merges on #259/#263/#275.

• New knob: DAWN_DEV_READY_TIMEOUT_MS — the parent's /healthz readiness window (was hard-coded 5s in waitForDevServerReady's sole call site). Mirrors the existing DAWN_DEV_SHUTDOWN_TIMEOUT_MS pattern; default unchanged.
• Test hardening: the flaky test now sets a 15s readiness window (its child must fully boot under CI load before the parent gives up), and the test helper waitForPath default goes 2s → 10s.

Honest caveat: the precise flake mechanism isn't 100% pinned — the child writes its PID before reporting ready, and the parent has no pre-ready timeout, so the timing of the observed 7s failure doesn't fully close. This PR widens both timing windows around the observed failure and adds the knob; if the flake recurs, next step is instrumenting the child's pre-ready exit stderr (DevChildStartupError).

Test Plan

• test/dev-command.test.ts run 4× sequentially: 16/16 each, no flakes
• typecheck + lint clean
• Changeset (@dawn-ai/cli patch)

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
dawnai	Ready	Preview, Comment	Jul 6, 2026 2:52am

[github-actions]

Automated approval: this PR received an intelligent (AI) code review. See the review comments on this PR.

[github-actions]

Claude finished @blove's task in 2m 7s —— View job

---

Code Review

• Gather context (PR diff, changed files)
• Review dev-session.ts changes
• Review dev-command.test.ts changes
• Review changeset
• Post findings

---

The fix is clean and the implementation is correct. A few observations:

readReadyTimeoutMs() (dev-session.ts:403–412)

Exact mirror of readShutdownTimeoutMs() — same structure, same guard (Number.isFinite), sensible default. No issues. One minor note: Number("0") is 0 and isFinite(0) is true, so DAWN_DEV_READY_TIMEOUT_MS=0 is accepted and causes an immediate-timeout behaviour. That's probably intentional (mirrors the shutdown function), just worth knowing.

Test timing (dev-command.test.ts:208–236)

The test now sets a 15 s health window and already carries a timeout: 30_000. The math holds (15 s floor + spawn overhead < 30 s ceiling), so no race against the Vitest timeout.

One ordering observation: the test does

const exitCode = await dev.waitForExit()   // blocks until parent exits (~15 s)
await waitForPath(pidPath)                 // then polls for PID file

The child writes its PID before signalling ready, and the parent only exits after the health-check window expires, so by the time waitForPath is called the file should already be there. The 10 s poll window is therefore irrelevant for this specific call site — but it's harmless, and the wider window is a sensible defensive buffer if the root-cause story turns out to be subtler than assumed.

If you want to make the intent clearer (and potentially catch a future flake earlier), swapping the order would be more direct:

await waitForPath(pidPath)                 // confirm child actually started
const exitCode = await dev.waitForExit()   // then wait for parent to clean up

Not a blocking issue — either order works once the PID file exists.

waitForPath default (dev-command.test.ts:743)

The 2 s → 10 s change affects three call sites (lines 229, 402, 574). Lines 402 and 574 wait for marker files in tests with active running servers. Widening the default means those tests would tolerate a 10 s delay silently before failing. Under normal conditions marker files appear in < 100 ms, so this won't hide real bugs in practice, but it's worth being aware that the sentinel change is global, not scoped to just the flaky test.

Overall

The implementation is correct, follows the established readShutdownTimeoutMs pattern, and the changeset is properly scoped as a patch. The PR description is admirably honest about the uncertainty. If the flake does recur, instrumenting the child's DevChildStartupError path (as noted) or moving to a named pipe/signal rather than a polled file for PID handoff would give a more deterministic signal.

Looks good to merge.

[github-actions]

Claude finished @blove's task in 1m 2s —— View job

---

Code Review

• Gather context (PR diff, changed files)
• Review dev-session.ts changes
• Review dev-command.test.ts changes
• Review changeset
• Post findings

---

The implementation is correct and follows the established readShutdownTimeoutMs pattern cleanly. A few observations:

readReadyTimeoutMs() (dev-session.ts:403–412)

Exact mirror of readShutdownTimeoutMs() — same structure, same Number.isFinite guard. One minor note: health.ts:26 also defaults timeoutMs to 5_000. Now that dev-session.ts always passes an explicit value, health.ts's default is effectively dead for this call site. If someone changes health.ts's default in the future, dev-session.ts won't benefit — but that's a reasonable and intentional trade-off for having an explicit, env-configurable value here.

DAWN_DEV_READY_TIMEOUT_MS=0 is accepted (Number.isFinite(0) === true), causing an immediate-timeout behaviour. Same as the shutdown function — probably intentional but worth documenting if there's a usage guide.

Test ordering (dev-command.test.ts:228–229)

const exitCode = await dev.waitForExit()  // blocks ~15s
await waitForPath(pidPath)                // polls for PID file

The child writes its PID before reporting ready, so the file exists well before waitForExit() returns. The waitForPath call here is effectively a no-op under normal conditions. Swapping the order would more explicitly express intent and could catch a future regression where the child never writes its PID:

await waitForPath(pidPath)                // confirm child started
const exitCode = await dev.waitForExit()  // then wait for parent cleanup

Not a blocking issue — either order works today. Fix this →

waitForPath default (dev-command.test.ts:743)

The 2s → 10s change is global and affects all three call sites (lines 229, 402, 574). For the flaky test, this is meaningful. For the other two sites (active running servers), marker files arrive in <100ms in practice, so the wider window won't mask real bugs — just adds a longer silent wait on genuine failures. Acceptable trade-off for the defensive buffer.

Changeset

Correctly scoped as @dawn-ai/cli patch.

Overall: The PR is correct and the approach is sound. The honest caveat in the description (flake mechanism not fully pinned) is appreciated — the DAWN_DEV_READY_TIMEOUT_MS=15000 in the test and waitForPath widening give sufficient headroom for CI. Looks good to merge.