Jobs killed by host timeouts stay "running" forever (no pid liveness check); concurrent state writers can wipe all job state and silently disable stopReviewGate

Repo: github.com/openai/codex-plugin-cc
Plugin version: 1.0.6 (latest; main = db52e28 at time of writing — bug present there)
Environment: Claude Code 2.1.209 (desktop app), macOS (darwin 25.5.0), node 26.5.0, shared session runtime (unix-socket broker)

Summary

Two consecutive long-running task jobs (adversarial reviews of a markdown spec, 15–25 min workloads) died silently while codex-companion.mjs status --json reported them as running indefinitely. Investigation shows a cluster of related defects:

1. No pid-liveness check anywhere. runTrackedJob (lib/tracked-jobs.mjs) records pid: process.pid and only writes a terminal status when its runner promise settles. A SIGKILLed job process never runs the catch handler, so state.json keeps status: "running" forever. buildStatusSnapshot (lib/job-control.mjs) trusts that field blindly — no process.kill(pid, 0), no heartbeat, no staleness cutoff.
2. Foreground execution inside the host's Bash tool guarantees death for long jobs. The rescue agent doc says "prefer foreground for a small, clearly bounded request", but in practice the model launches 20-minute reviews foreground. Claude Code's Bash tool kills the process tree at its timeout (default 120 s, max 600 s). Both observed failures line up with those timeouts to the second (details below).
3. The shared broker can be spawned as a descendant of a job's Bash process tree. ensureBrokerSession spawns the broker detached: true from whichever companion invocation first needs it. detached protects against process-group kills, but Claude Code's tree-kill walks children by ppid while the parent is still alive — so killing the foreground job also killed the shared broker and its codex app-server child, destroying every thread in the runtime (observed: job 1 took down the whole runtime; cancel later reported thread not found).
4. captureTurn awaits a promise only a turn/completed notification can resolve (lib/codex.mjs, state.completion). It never races the client's exitPromise. When the broker socket closes mid-turn, pending requests are rejected (handleExit), but a worker sitting in captureTurn just loses its last event-loop handle and node exits silently with code 0 — no catch runs, job stays "running". This bites --background workers too, whenever the broker dies.
5. Turns keep running server-side with zero listeners, and the result is discarded. The broker's routeNotification drops notifications when no owner socket exists. Job 2's review kept running for 8 more minutes after its client died and completed successfully (task_complete in the rollout at 22:12:56Z); the finished review went nowhere while status still said "running". It was recoverable only by hand from ~/.codex/sessions/.../rollout-*.jsonl.
6. Minor: ensureBrokerSession's readiness probe gives an existing broker only 150 ms to accept a connection before tearing it down and replacing it. A busy broker (large JSONL payloads stall the event loop) can be misjudged as dead.
7. Separate high-severity hazard found while testing: concurrent state writers can wipe ALL job state, logs, and the stopReviewGate config. The chain (all upstream code): saveState writes state.json with a plain non-atomic fs.writeFileSync; a concurrent loadState can read the file mid-write, JSON.parse throws, and the catch returns defaultState() (empty jobs, stopReviewGate: false); if that reader is a writer (upsertJob → updateState is an unlocked read-modify-write), it then saves the near-empty state — and saveState's pruning loop deletes the job file and log file of every job absent from the state being saved. Net effect: full history wipe, all job logs deleted, review gate silently disabled. We reproduced this with 6 concurrent upsertJob writers (it destroyed the real state for this workspace, including the evidence logs for the two jobs above — their content survives in this report and in the ~/.codex/sessions rollouts). Real-world trigger: any two Claude sessions sharing a workspace whose stop-gate hooks or job workers write state simultaneously. Fixes: temp-file + renameSync in saveState (atomic replace), retry/fail-closed instead of defaultState() on parse errors of an existing file, and a lock (or at least last-writer-wins semantics that never delete files based on a possibly-stale view).

Evidence (timestamps UTC)

Job 1 — task-mro16p6v-nx7uod (foreground CLI, pid 20782)

• 21:38:33 job starts (job log has no "Queued for background execution" line → foreground path).
• 21:40:36 job log AND the thread rollout (rollout-...019f6cdd-f82c....jsonl) stop mid-turn within 113 ms of each other; no error, no task_complete. 21:38:33 + ~120 s ≈ default Bash-tool timeout; CLI + broker + app-server died together (tree kill; broker was a descendant of this CLI's Bash call — see §3).
• 21:42:20 the next companion invocation (stop-gate hook) finds the broker endpoint dead, tears down the remains, spawns a fresh broker (pid 23179, sessionDir mtime 17:42 local) — works fine.
• 21:55:34 /codex:cancel: Codex turn interrupt failed: thread not found: 019f6cdd-... — the replacement app-server never had the thread. Job had shown running the whole time; ps -p 20782 empty.

Job 2 — task-mro1t9r6-xe003h (foreground CLI, pid 30867)

• 21:56:06 job starts (foreground, same signature), thread 019f6cee created in the NEW broker's app-server (pid 23182).
• 22:04:48.9 last job-log line ("Command completed").
• Rollout shows the thread then performed a ~100 s context compaction (22:06:30) and kept working; events from 22:06:32 onward never reached the job log → client disconnected in [22:04:49, 22:06:32]. 21:56:06 + 600 s = 22:06:06, dead-center in that window = max Bash-tool timeout. Broker survived this kill (it was spawned by the 21:42 stop-gate process and had been reparented to pid 1 — out of the killed tree).
• 22:12:56 rollout records task_complete with the full 23.9 KB review — delivered to nobody.
• 22:17:20 /codex:cancel: Codex turn interrupt failed: no active turn to interrupt (thread known, turn already over). Status showed running for the whole 21 minutes; ps -p 30867 empty.

No crash reports (~/Library/Logs/DiagnosticReports), no OOM/jetsam entries — consistent with external SIGKILL of the process tree, not a crash.

Suggested fixes

1. Liveness reconciliation in status paths: when a job is queued/running and process.kill(job.pid, 0) throws ESRCH, mark it failed with a clear message. Two TOCTOU details matter: (a) re-check the job is still active inside the updateState mutation (a job can complete between the status snapshot and the write — a naive patch overwrites completed with failed); (b) if the per-job file already holds a terminal status (process died between writeJobFile and upsertJob in runTrackedJob), adopt that status instead of failed. (~60 lines in lib/job-control.mjs; we've verified both cases locally.)
2. Refuse or warn on foreground for long-lived work, or make --background the default for task/reviews when invoked from a hook/agent context. The CLI cannot survive the host Bash timeout; the docs should say so explicitly.
3. Spawn the broker from a stable parent (or double-fork / re-spawn via an intermediate that exits immediately) so it is reparented to pid 1 before the job's Bash tree can be killed.
4. Race captureTurn against client.exitPromise so a socket close fails the job (status: failed, "runtime connection lost") instead of a silent code-0 exit.
5. Interrupt ownerless turns or persist their result: when the owning socket closes mid-turn, either turn/interrupt or capture the completion server-side and write it into the job record. Job 2's completed review should not have been lost.
6. Raise the broker readiness probe timeout above 150 ms.

Local workaround applied (for other affected users)

• Always launch long codex tasks with --background (the detached worker is immune to Bash-tool timeouts; it remains vulnerable to defect 4 only if the broker itself dies).
• Patched into the local plugin cache (overwritten on plugin update):
  • scripts/lib/job-control.mjs: reconcileStaleJobs() in the status paths — an active job with a dead pid is marked failed (or adopts the terminal status already in its job file); the still-active check re-runs inside the state mutation to avoid overwriting a job that completes concurrently; plus a self-heal pass that repairs any reconciliation write that raced a completion.
  • scripts/lib/state.mjs: saveState and writeJobFile write via temp file + renameSync (atomic; readers never see torn JSON); parse failures are retried, then writers fail closed — updateState throws rather than proceed from an empty default when state.json exists but cannot be parsed (readers still degrade gracefully); updateState serializes read-modify-write behind an advisory lock dir (mkdir-based). Reaping of dead holders is liveness-based and non-displacing: a lock is reap-eligible only if its owner pid is dead (after a 500 ms grace) or it exceeds a 60 s hard cap; the reap is claimed by creating a reap-claim file inside the lock dir with O_EXCL — exactly one reaper can win, the claim is pinned to that dir incarnation, the owner record is re-verified in place before deletion, and a mistaken claim is undone by unlinking (a live lock never leaves the namespace — a rename-based steal was tried and rejected because the displaced window let a third waiter acquire and forced the real holder to abort). Abandoned claims are recovered only when the claim's reaper pid is DEAD (a dead reaper can never wake and race the recovery; a live-but-stalled reaper defers to the 60 s hard cap), and the claim winner re-verifies both the owner record and its own claim record in place immediately before the destructive remove. On contention past the 10 s acquisition deadline the writer throws instead of writing unlocked; release is owner-token-guarded; ownership is re-verified at commit time, aborting rather than overlapping if the lock was somehow lost. The unlocked path remains only for filesystem errors (e.g. EACCES), where the state write itself would fail anyway.
  • scripts/session-lifecycle-hook.mjs: cleanupSessionJobs removes the session's jobs through the locked updateState instead of saving a snapshot read earlier (a stale-snapshot save deletes other sessions' job files via saveState pruning).
  • Verified: dead-pid phantom → failed; running-in-state/completed-in-file race → completed preserved; clobbered-to-failed entry → healed back to completed; writer against corrupted state.json → throws, corrupt file and all job/log files untouched; reader against corrupted state → renders, deletes nothing; lock owned by a dead pid → reaped and acquired in ~640 ms; abandoned claim from a dead reaper → recovered and acquired in ~1.3 s; claim held by a live (stalled) reaper → never recovered, writer fails closed at 10 s; lock owned by a LIVE process → never stolen, writer throws at the 10 s deadline instead of writing unlocked; lock lost mid-update (owner record replaced during the mutation) → commit aborts with the state file unchanged; reap-storm (a dead lock placed in front of 8 concurrent writers × 20 updates, 3 rounds) → every write landed, zero spurious lock-lost aborts, no leftovers; plain 8-writer hammer → no lost updates, no wipe, config preserved.

🤖 Generated with Claude Code

Start with lib/tracked-jobs.mjs and lib/job-control.mjs to trace job state from process launch through status reporting, then inspect lib/codex.mjs and the state-management files for broker disconnects and concurrent writes. The work is complete when dead jobs no longer remain running, connection loss is reported, state and logs survive concurrent updates, and the documented foreground/background behavior is reliable.