fix: flush stdout/stderr before every CLI process.exit() (#1596)

[thymikee]

Summary

Part of #1596 (hardening; the reported replace-path failure itself was not reproduced — see review discussion, issue stays open) — the field reports of the driving process going silent (zero further tool calls) right after:

Replacing daemon (pid N, vX) in <state-dir>: unreachable
Error (SESSION_NOT_FOUND): iOS snapshot requires an active app session on the target device.

Investigation

I read through the daemon replace/takeover path (src/daemon/client/daemon-client-lifecycle.ts) and the SESSION_NOT_FOUND emission path (src/daemon/snapshot-runtime.ts, src/daemon/handlers/*). The takeover mechanics themselves look sound:

• the detached daemon child is spawned with detached: true (its own process group/session) and its stdout/stderr are redirected to explicit file descriptors, never inherited from the CLI's own stdio — so a replaced daemon can't hold the CLI's own stdout pipe open;
• stopDaemonProcessForTakeover/isAgentDeviceDaemonProcess verify a live process by command pattern + processStartTime before signaling it, and isProcessAlive explicitly rejects pid <= 0 — so a PID-reuse or pid: 0→kill(0, …) group-signal footgun isn't reachable;
• the daemon-startup exited promise (runCmdDetachedMonitored) only ever resolves, never rejects — so there's no unhandled-rejection path there;
• SESSION_NOT_FOUND from a fresh daemon is a normal structured ok:false response and already carries a default hint ("Run open first…") via normalizeError/defaultHintForCode.

I could not find or prove a hang/crash/process-group-leak mechanism in the replace/takeover code itself. What I did find and prove is a real, general Node.js correctness bug on the CLI's own exit path: process.exit() called immediately after a process.stdout/process.stderr write can silently drop that write — Node flushes those streams synchronously only when they're a file or TTY; on a pipe (this CLI's normal condition when driven as a subprocess by an agent harness) writes are queued asynchronously, and process.exit() tears the process down before a queued write reaches the pipe.

This is directly reachable from the exact code path that renders the reported error: handleRunCliFailure in src/cli.ts calls printHumanError and then, under --debug, printDaemonLogTailOnError — which dumps up to 200 unbounded lines of the daemon's log — immediately before process.exit(1).

Evidence (red without the fix, green with it), against the real CLI, not just an isolated repro:

1. Started a real local daemon, killed it with SIGKILL (stale daemon.json, live-then-dead pid — the same shape as "unreachable").
2. Seeded its daemon.log past 64KB (macOS/Linux's default pipe buffer size) and re-ran close --debug --state-dir <dir> through a real piped spawnSync, matching how an agent harness captures subprocess output.
3. Pre-fix: stderr length: 66672, trailing marker missing — truncated mid-write.
4. Post-fix (same daemon, same seeded log, re-seeded to 65151 bytes): trailing marker present — fully delivered.

I want to be upfront about the limit of this evidence: a freshly started daemon truncates its own daemon.log to empty at startup (src/daemon/server/server-lifecycle.ts:29, fs.writeFileSync(logPath, '')), so in a plain sandbox (no simulator/device backend) the log-tail dump right after a replace is only a few dozen bytes — nowhere near 64KB. I cannot prove this exact log-tail dump is what corrupted the 3 AppControlBench transcripts; that would need a host where the fresh daemon's own startup logging (device enumeration, xcrun/adb calls, etc.) is chatty enough to approach the pipe-buffer threshold on its own, which I can't reproduce here. What I can say confidently: the mechanism is real, it lives in the CLI's shared failure-rendering path, it is reachable from this exact SESSION_NOT_FOUND-after-replace scenario, and it was previously completely unguarded — this PR closes that gap regardless of whether it's proven to be the exact historical trigger.

Fix

• Added src/utils/process-exit.ts (exitAfterFlush): drains process.stdout/stderr (bounded by a 2s timeout so a stalled/broken pipe still can't hang the process) before calling process.exit().
• Routed every process.exit() call site in src/cli.ts and src/bin.ts through it (the failure-rendering path, the parse/help/version fast-exits, and the react-devtools/web/cdp subcommand exit-code wrappers) — all share the same footgun.
• Bounded printDaemonLogTailOnError's dump to 64,000 bytes on top of its existing 200-line cap, as defense in depth.

Tests

test/integration/daemon-replace-exit-flush.test.ts (new, node --test, follows the existing smoke-daemon-*.test.ts harness pattern):

• End-to-end: starts a real daemon, SIGKILLs it, reruns a command against the stale state dir, asserts the CLI prints the "Replacing daemon … unreachable" notice, exits with code 1, and returns parseable --json with error.code === 'SESSION_NOT_FOUND' and an open-mentioning hint.
• Mechanism, isolated and deterministic: two small fixture scripts (test/integration/support/exit-naive.ts / exit-after-flush.ts) each write an oversized payload to stderr then exit the old way vs. the fixed way, run as real piped child processes. Confirms the naive path truncates and exitAfterFlush doesn't, independent of any daemon/device setup.

Verification

• pnpm check:quick (lint + typecheck): clean.
• node --test test/integration/daemon-replace-exit-flush.test.ts test/integration/smoke-daemon-clean.test.ts test/integration/smoke-daemon-http.test.ts: 5/5 pass.
• pnpm test:unit: 603/603 files pass (5290/5290 tests). One initial run showed transient timeouts from CPU contention caused by two concurrent full suite runs, and one ENOTEMPTY temp-dir cleanup race in an unrelated test file — both reproduced as flaky-under-contention and confirmed unrelated to this change by re-running each failing file in isolation (all green).

Test plan

• pnpm check:quick
• node --test test/integration/daemon-replace-exit-flush.test.ts
• pnpm test:unit (clean run, 603/603 files)

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.96 MB	1.96 MB	+500 B
JS gzip	627.1 kB	627.4 kB	+325 B
npm tarball	748.7 kB	749.5 kB	+756 B
npm unpacked	2.62 MB	2.62 MB	+3.3 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.3 ms	27.2 ms	-0.1 ms
CLI --help	64.1 ms	64.3 ms	+0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/cli.js	+67 B	+54 B

[thymikee]

P2: This PR proves and fixes a real generic Node process.exit() pipe-truncation hazard, but it does not prove the reported #1596 daemon-replace failure. The real replace-path test asserts a small SESSION_NOT_FOUND response that would also pass before this change; the only counterfactual red→green proof is the synthetic oversized fixture, and the body acknowledges a fresh replacement daemon truncates its log so the historical path was not reproduced. Keep #1596 open/remove Fixes #1596 unless the reported replace path itself can be made red pre-fix and green post-fix. The hardening may still land on its own merits after exact-head owner-action CI is fixed: format drift, Fallow-unreachable fixture files, and changed-line coverage 59.26% below the 70% gate.

[thymikee]

P2 addressed: the PR no longer claims Fixes #1596 — the body now says Part of #1596 and the issue stays open, exactly per the review's reasoning: the red→green proof covers the generic pipe-truncation hazard (synthetic oversized fixture), while the reported replace-path failure was not itself reproduced (a fresh replacement daemon truncates its own log, so the historical log-tail size can't be reconstructed). The hardening lands on its own merits; #1596 remains open for the root cause. CI gate fixes were pushed separately in dfb257f.

🤖 Addressed by Claude Code

[thymikee]

Non-blocking nit for the record: drainStream resolves on the 'drain' event, which only fires when a write exceeded the high-water mark — a small queued write (writableLength > 0 but below HWM) never emits it, so that rare path pays the full 2s cap instead of exiting promptly. Correctness is unaffected (the >64KB truncation case always crosses HWM); a writableLength poll would tighten it if the latency ever matters.

🤖 Addressed by Claude Code

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-04 19:57 UTC