fix(test): fake codex app-server exits when its parent dies (leak fix)

[Wibias]

Why

The fake-codex-server test helper (deterministic fake of the codex app-server,
spawned as a real child process by the Fabric CI tests) only exited via a stdin-EOF
handler. When the test worker that spawned it is killed hard (which
bun test --isolate does at file end), that EOF is not reliably delivered on
Windows — the child outlives its worker.

What was found

Diagnosis from a 30-minute macOS CI freeze (PR #1005) and a local Windows hang:

1. macOS: The suite stopped right after the last file worker had started — no
   test output for 30 minutes, then the job timeout killed it. At job end the
   runner cleaned up exactly one orphaned bun process (pid 4812). Cause: a
   leaked child keeps the finished file worker's stdout pipe open. bun test
   waits for pipe EOF, so it waits until the timeout.
2. Windows: On a hard worker kill, stdin EOF is not delivered reliably. On the
   local machine, 19 orphaned test processes from several aborted suite runs
   (including fake-codex-server children) held test ports; scripts/test.ts
   waited up to 45 minutes for competing runners and the suite appeared hung.
3. Exclusion: The test code path itself cannot hang — every real call is
   mocked or capped with a timeout. The hanging element was always the surviving
   child process, never the test.

Consequences before

• macOS CI: suite runs into the 30-minute job timeout, PRs stay red (blocked feat(analytics): add route reliability and latency analytics #1005
  until a manual rerun).
• Windows: orphan pileup across runs — port holders plus CPU load strangle the
  machine, suites crawl from ~10 min to 45+ min or hang completely.

After the fix

The file now contains a parent-death watchdog: every 500 ms the spawner PID
(process.ppid) is checked via process.kill(pid, 0); once it is gone, the child
exits immediately — even without stdin EOF and even in the hangOnShutdown script
(the explicit force-kill test still wins first). The timer is unref'd and does
not keep the child artificially alive.

Verified:

• Fabric transport tests (fabric-codex-jsonrpc + fabric-codex-adapter):
  39/39 green
• Parent-death simulation (real Bun path, as used by the suite): child exits
  < 3 s after the parent dies
• A full suite run leaves no fake-codex-server orphans behind

Scope note

tests/fake-codex-server.ts currently only exists in the unmerged Fabric branch
(fab-03/codex-runtime). This PR introduces the file on dev independently —
including the watchdog. The later Fabric rebase will adopt this version (the
branch is 178 commits behind dev anyway).

Summary by CodeRabbit

• Tests
  • Expanded automated coverage for app-server communication, including lifecycle events, approvals, interruptions, failures, malformed input, and shutdown scenarios.
  • Added deterministic testing for varied response sequences and process behavior to improve confidence in reliability and error handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c874e0f3-7302-48be-8810-b783088a99f6

📥 Commits

Reviewing files that changed from the base of the PR and between d68679c and 0a3c127.

📒 Files selected for processing (1)

• tests/fake-codex-server.ts

---

📝 Walkthrough

Walkthrough

The pull request adds a deterministic fake codex app-server child process for integration tests. It implements newline-delimited JSON-RPC handling, thread operations, scripted turns, approvals, interruptions, failures, shutdown behavior, and parent-process cleanup.

Changes

Fake Codex app-server

Layer / File(s)	Summary
Protocol contracts and object builders tests/fake-codex-server.ts:1-53, tests/fake-codex-server.ts:55-119	Defines configurable server scripts and helpers for JSON-RPC messages, thread objects, turn objects, counters, and approval-response tracking.
Initialization and process lifecycle tests/fake-codex-server.ts:121-177, tests/fake-codex-server.ts:179-207, tests/fake-codex-server.ts:310-317, tests/fake-codex-server.ts:319-333	Parses protocol input, handles initialization and thread lifecycle methods, supports startup and shutdown scenarios, exits on EOF, and terminates when the parent process disappears.
Scripted turns and interruptions tests/fake-codex-server.ts:208-279, tests/fake-codex-server.ts:280-297, tests/fake-codex-server.ts:299-308	Selects turn scripts by input text and emits configurable progress, approval, terminal, failure, duplicate, late, and interruption events. Unsupported methods receive JSON-RPC errors.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested labels: chore

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: the fake Codex app-server exits when its parent process dies to prevent leaks.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/fake-codex-server.ts`:
- Around line 51-53: Update notify to include the JSON-RPC 2.0 version in every
notification, and remove the fake-exit-ack stdout emission from the shutdown
path around shutdownRequested. Keep stdout limited to valid protocol messages;
tests should detect graceful shutdown via the child process exit rather than an
out-of-band marker.
- Around line 140-143: Move the startupDelayMs wait from the general request
path into the method === "initialize" branch in the fake server request handler,
so only initialization is delayed while thread and turn operations proceed
without the startup delay.
- Around line 206-270: Update the terminal-event flow in the fake server’s
scripted turn handler to construct one terminal payload containing the shared
status and failure details, including the error information for failed turns.
Emit that payload once by default, suppressing the later emission when
emitTerminalFirst is enabled, and emit a second identical payload only when
duplicateTerminal is explicitly enabled; preserve the existing notification,
approval, and heldUntilInterrupt behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2733d23e-cb96-4d11-b2fe-a5d722feb2c9

📥 Commits

Reviewing files that changed from the base of the PR and between a594938 and d68679c.

📒 Files selected for processing (1)

• tests/fake-codex-server.ts

[Wibias]

[GD] Verdict: approve-comment

TLDR

• PR: fix(test): fake codex app-server exits when its parent dies (leak fix) #1035 — fix(test): fake codex app-server exits when its parent dies (leak fix)
• Head: d68679cc5 (base dev, MERGEABLE, CLEAN)
• Decision: approve-comment
• Usefulness: pass — watchdog ends the macOS 30-min pipe-EOF CI hang and the Windows orphan pileup
• Bugs: none confirmed (watchdog verified: child exits < 3 s after parent death; EOF + SIGKILL paths unchanged)
• Security: pass — test-only helper, no new surface; privacy scan green
• Spec/Standards: pass — leak fix as described; file unused on dev until the Fabric branch lands (documented in the PR body)
• Reviews: 3 CodeRabbit threads triaged with [GD] replies and resolved (wire-fidelity nits, Fabric-branch scope)
• Base/CI: all required checks green (test 1-4/4, macos, gates, keyring, ci)
• Gate: ship-gate ready
• Owner actions: none — merge when ready (recommend squash)
• Bottom line: ship it; Fabric rebase will adopt this file version

Full review

Usefulness

The PR introduces tests/fake-codex-server.ts (a deterministic fake of the codex app-server, previously only present on the unmerged Fabric branch) together with
a parent-death watchdog. The watchdog polls process.ppid every 500 ms and exits
when the spawner is gone — covering the case where bun test --isolate kills a
worker without delivering stdin EOF (Windows). This directly fixes two observed
failures: the macOS CI run that sat for 30 minutes waiting for a leaked child's
stdout-pipe EOF (PR #1005, job timeout), and the Windows orphan pileup that held
test ports across runs and wedged the local machine.

Bugs

None confirmed. Reviewed against the required lenses:

• resource_lifecycle / resource_leaks: the fix is the leak fix; both exit
  paths (20 ms stdin-EOF handler, 500 ms watchdog) are idempotent
• boundary_conditions: hangOnShutdown still survives EOF while the parent
  is alive (forced-shutdown test passes); it now also dies when the parent dies —
  intended, matches the comment
• edge_cases: PID-reuse within the 500 ms poll window could in theory keep a
  child alive; non-blocking nit, not observable in the test harness. The catch
  also exits on EPERM, but the child and spawner share a user, so kill(pid, 0)
  never yields EPERM here
• concurrency_races / error_propagation / time_clocks: watchdog interval is
  unref'd (does not keep the child alive); no shared state; no timers changed
• api_compatibility / parsing_serialization / state_consistency: wire
  protocol and script semantics untouched; only startup/exit behavior appended
• network_cancellation / ui_async_state / silent_failures: n/a — test helper

Security

Pass. Scope surfaces authn, authz, secrets_config, injection: n/a with
evidence — test-only file, no credentials, no network, no user-controlled shell
interpolation; stdin is parsed as JSON by the test driver; privacy:scan green in
CI gates. No new dependency.

Spec and Standards

Spec source: PR body + originating leak-fix request. Implemented as specified and
verified (fabric transport tests 39/39 on the source branch, parent-death
simulation exits < 3 s, no orphans after a full suite run). Standards: gates green
(typecheck, privacy, lint, CLI smoke); placement in tests/ matches other
spawnable helpers. One non-blocking note, documented in the PR body: the file has
no consumer on dev until the Fabric branch lands; the Fabric rebase adopts this
version.

Semantic propagation

• Concept: an app-server child process must never outlive its spawner; graceful
  shutdown = stdin EOF, forced = SIGKILL
• Authoritative source: the transport shutdown contract
  (src/fabric/codex-jsonrpc.ts — closes stdin, expects exit; force fallback)
• Producers/consumers: only on the Fabric branch (fabric transport tests);
  no consumers on dev yet
• Derived/public representations: none — no persistence, API, or UI surface
• Variants: hangOnShutdown, exitAtStartup, crashOnTurnStart,
  neverRespondToInit — all unaffected while the parent lives; parent death now
  exits every variant (intentional)
• Negative assertion checked: no test asserts the fake survives its parent —
  every fabric test shuts down or expects death
• Result: matched

Reviews triaged

• 3 CodeRabbit threads (JSON-RPC wire fidelity, startupDelayMs scope, terminal
  mode separation): all non-blocking nits on the inherited Fabric-authored file,
  replied in-thread with [GD] and resolved; fixes belong to the Fabric PR where
  the consumers live

Fixes this session

• none (no in-scope defects found)

[Wibias]

[GD] Verdict: approve-comment

TLDR

• PR: fix(test): fake codex app-server exits when its parent dies (leak fix) #1035 — fix(test): fake codex app-server exits when its parent dies (leak fix)
• Head: 0a3c127d (base dev, MERGEABLE) — review fixes applied after the first round
• Decision: approve-comment
• Usefulness: pass — watchdog ends the macOS 30-min pipe-EOF CI hang and the Windows orphan pileup
• Bugs: none confirmed (watchdog verified: child exits < 3 s after parent death; EOF + SIGKILL paths unchanged)
• Security: pass — test-only helper, no new surface; privacy scan green
• Spec/Standards: pass — leak fix as described; CodeRabbit round applied (wire purity, initialize-only delay, terminal modes); file unused on dev until the Fabric branch lands (documented in the PR body)
• Reviews: 3 CodeRabbit threads — all fixed in 0a3c127d (replies posted, threads resolved)
• Base/CI: required checks green on 0a3c127d (test 1-4/4, macos, gates, keyring, ci)
• Gate: ship-gate ready
• Owner actions: none — merge when ready (recommend squash)
• Bottom line: ship it; Fabric rebase will adopt this file version

Full review

Usefulness

The PR introduces tests/fake-codex-server.ts (a deterministic fake of the codex app-server, previously only present on the unmerged Fabric branch) together with
a parent-death watchdog. The watchdog polls process.ppid every 500 ms and exits
when the spawner is gone — covering the case where bun test --isolate kills a
worker without delivering stdin EOF (Windows). This directly fixes two observed
failures: the macOS CI run that sat for 30 minutes waiting for a leaked child's
stdout-pipe EOF (PR #1005, job timeout), and the Windows orphan pileup that held
test ports across runs and wedged the local machine.

Bugs

None confirmed. Reviewed against the required lenses:

• resource_lifecycle / resource_leaks: the fix is the leak fix; both exit
  paths (20 ms stdin-EOF handler, 500 ms watchdog) are idempotent
• boundary_conditions: hangOnShutdown still survives EOF while the parent
  is alive (forced-shutdown test passes); it now also dies when the parent dies —
  intended, matches the comment
• edge_cases: PID-reuse within the 500 ms poll window could in theory keep a
  child alive; non-blocking nit, not observable in the test harness. The catch
  also exits on EPERM, but the child and spawner share a user, so kill(pid, 0)
  never yields EPERM here
• concurrency_races / error_propagation / time_clocks: watchdog interval is
  unref'd (does not keep the child alive); no shared state; no timers changed
• api_compatibility / parsing_serialization / state_consistency: wire
  protocol unchanged apart from the CodeRabbit fixes below
• network_cancellation / ui_async_state / silent_failures: n/a — test helper

Security

Pass. Scope surfaces authn, authz, secrets_config, injection: n/a with
evidence — test-only file, no credentials, no network, no user-controlled shell
interpolation; stdin is parsed as JSON by the test driver; privacy:scan green in
CI gates. No new dependency.

Spec and Standards

Spec source: PR body + originating leak-fix request. Implemented as specified and
verified (fabric transport tests 39/39 against the fixed fake, parent-death
simulation exits < 3 s, no orphans after a full suite run). Standards: gates green
(typecheck, privacy, lint, CLI smoke); placement in tests/ matches other
spawnable helpers. One non-blocking note, documented in the PR body: the file has
no consumer on dev until the Fabric branch lands; the Fabric rebase adopts this
version.

CodeRabbit round (fixed in 0a3c127d)

• JSON-RPC wire purity: notifications now carry jsonrpc:"2.0"; the
  shutdownRequested ack is a JSON-RPC notification (shutdownAck) instead of a
  bare fake-exit-ack line
• startupDelayMs: now applies to initialize only, not every id-bearing
  message
• Terminal modes: emitTerminalFirst no longer emits a trailing terminal
  (modes stay separate); all terminal payloads are built by one shared
  completedTurn helper so failed duplicates carry the same error payload

Verified: fabric-codex-jsonrpc + fabric-codex-adapter still pass 39/39
against the fixed fake (real deps in the Fabric checkout).

Semantic propagation

• Concept: an app-server child process must never outlive its spawner; graceful
  shutdown = stdin EOF, forced = SIGKILL
• Authoritative source: the transport shutdown contract
  (src/fabric/codex-jsonrpc.ts — closes stdin, expects exit; force fallback)
• Producers/consumers: only on the Fabric branch (fabric transport tests);
  no consumers on dev yet
• Derived/public representations: none — no persistence, API, or UI surface
• Variants: hangOnShutdown, exitAtStartup, crashOnTurnStart,
  neverRespondToInit — all unaffected while the parent lives; parent death now
  exits every variant (intentional)
• Negative assertion checked: no test asserts the fake survives its parent —
  every fabric test shuts down or expects death
• Result: matched

Reviews triaged

• 3 CodeRabbit threads: all fixed in 0a3c127d, [GD] update replies posted,
  threads resolved

Fixes this session

• 0a3c127d — CodeRabbit round (wire purity, initialize-only delay, terminal
  modes) on top of d68679cc (parent-death watchdog)

[Wibias]

Merged — the fake codex app-server now exits when its parent dies, ending the macOS pipe-EOF CI hangs and the Windows orphan pileup. The Fabric rebase will adopt this file version.