app-server client never marks itself unusable after child exit — two unbounded hangs in the broker dispatch path (1.0.4)

[90stm]

Summary

In plugin version 1.0.4, when the spawned codex app-server child dies, the client in scripts/lib/app-server.mjs never marks itself unusable — handleExit() rejects the requests already in pending and clears the map, but nothing anchors future or non-RPC waits to the connection's death. This produces two distinct unbounded hangs in the broker dispatch path (scripts/app-server-broker.mjs), which on our machine silently lost 3 of 5 dispatches over two days while codex exec (which does not use the broker) worked perfectly, masking the failure.

Defect 1 — requests issued after child death hang forever

After the child exits, handleExit() has already cleared pending. A subsequent request() writes to a dead pipe and registers a pending entry no code path can ever reject or resolve. The caller hangs unboundedly (we observed dispatch workers parked 24+ minutes at 0.0% CPU, no children).

Compounding it: the broker keeps its Unix socket listening with a dead child behind it, and endpoint readiness is judged by socket-connect success alone — so every subsequent client reuses the broken broker indefinitely.

Repro: start the broker, let it spawn its app-server child, kill the child, then issue any request (e.g. thread/start) through the broker. It never resolves, never rejects, no timeout, no log.

Defect 2 — child death during the notification wait hangs forever

captureTurn in scripts/lib/codex.mjs awaits state.completion — a promise whose rejectCompletion is created (~line 302 in 1.0.4) but never called anywhere in the codebase. The RPC for review/start / turn/start resolves when the turn starts; the real work (8–9 minutes for a review) is awaited on the notification stream via state.completion, settled only by turn/completed (or the inferred-completion timer, which arms only after a final answer is seen). If the app-server dies in that window, handleExit() rejects only the — by then empty — pending map, and nothing rejects state.completion: the dispatcher hangs forever. Since nearly all of a review's wall-clock lives in this window, this is the dominant hang in practice.

Repro: start a review, wait until the review/start RPC has returned and notifications are flowing, then kill -9 the app-server child. The dispatcher never settles.

Root cause, generalized

handleExit marks the connection finished, but no consumer wait is anchored to it. Any wait that only an expected message can settle becomes a permanent hang when the peer dies.

Suggested fix (what we patched locally; verified over 24/24 dispatches after 2/5 before)

1. Derive a single rejecting promise from exitPromise (e.g. connectionLoss, with a no-op .catch attached so it can't become an unhandled rejection), and race every unbounded consumer wait against it — the promise returned by request(), and captureTurn's state.completion. This adds no timeout to the notification stream, so long-running reviews are unaffected.
2. Guard request()/notify() admission on closed || exitResolved.
   Caveat: do not implement this by setting closed = true inside handleExit — handleExit is also reachable with the child still alive (a JSONL parse failure at ~line 126), and close()'s early return on this.closed then skips readline.close(), stdin.end(), and the SIGTERM timer, leaking the child process. We hit this on our first attempt; it is reproducible. Keep two flags: closed = "close() ran" (gates teardown), exitResolved = "connection finished" (gates admission).
3. In the broker: exit the process when the app-server client dies, so the socket disappears and ensureBrokerSession respawns a clean broker. Without this, fix 2 turns the infinite hang into infinite errors, because endpoint readiness passes on socket connect alone.

Happy to provide our full local patches or test harness details if useful.

Environment

• macOS (darwin 25.x), Node 23.11 (note: Node's default unhandled-rejection behavior is fatal, relevant to any timer/race added around these paths)
• codex-plugin-cc 1.0.4 (Claude Code plugin)
• Codex CLI current as of 2026-08-21

[90stm]

Verified this reproduces on the latest release (v1.0.6) by inspection of the tagged source, not only on 1.0.4:

• plugins/codex/scripts/lib/app-server.mjs (v1.0.6): request() guards only this.closed (line 87), and handleExit() sets exitResolved (line 168) without any admission check reading it — so requests issued after child death still register into a cleared pending and hang (Defect 1).
• plugins/codex/scripts/lib/codex.mjs (v1.0.6): rejectCompletion occurs exactly four times — typedef, declaration, assignment, struct field — and is still never invoked, so state.completion has no rejection path (Defect 2).
• plugins/codex/scripts/app-server-broker.mjs (v1.0.6): no observer on the app-server client's exit, so a broker with a dead child keeps its socket listening and is reused by every subsequent client.

Both repro sketches in the issue body apply to v1.0.6 unchanged.

[90stm]

One more instance of the same root cause, found while hardening around it: handleLine treats a JSONL parse error as a transport exit and resolves exitPromise while the child process is still alive — conflating "transport failed" with "child process ended". Any teardown or escalation logic keyed on exitPromise is therefore defeated on exactly the live-child path where it would be needed (we built and then removed a SIGKILL escalation timer that this conflation silently disarmed). The two concepts want separate promises: exitPromise (transport-level, drives request/wait settlement) and a childExited promise keyed on the child's actual exit event (drives teardown and any escalation).

Related detail from measurement, in case it helps triage: the codex app-server binary itself reliably exits within ~20-40ms of stdin EOF in every state we probed (idle, mid-turn, parent death), so stdin.end() in close() is an effective reaper — the one genuinely uncovered path we found is connect() throwing during initialize (e.g. malformed JSONL during handshake), where the client is never returned to the caller and close() can never run, leaving the child alive for the host process's lifetime. A try/catch around initialize() that closes the client before rethrowing covers it.

[90stm]

Third instance from the same review effort, this one in lib/broker-lifecycle.mjs: spawnBrokerProcess() launches the shared broker detached: true + unref(), readiness has a 2000ms deadline, and on a miss teardownBrokerSession() removes the endpoint/session artifacts but kills nothing — its killProcess parameter is omitted at the connect() call site. A broker that takes longer than 2s to become ready is therefore abandoned alive and untracked: it finishes starting, spawns and retains its own codex app-server child, and listens on a socket no longer recorded anywhere. The caller meanwhile falls back to a direct client, so the failure is invisible — the cost is a leaked detached process pair per slow bootstrap, surviving the original host's exit (detached). Suggested fix is one line at the call site: supply killProcess (or make teardown kill by default when the spawn was its own).