SessionEnd cleanup fails when review cwd ≠ session cwd — broker.json looked up by cwd-hash, leaving orphan brokers even on graceful /quit

[hongsu]

Summary

handleSessionEnd cleans up the broker by calling loadBrokerSession(input.cwd), but broker.json is stored under a cwd-derived hash (resolveStateDir(cwd)). When the broker was spawned for a different cwd than the session's cwd, the lookup misses, endpoint is null, no broker/shutdown is sent, and the broker is orphaned — even when the session exits gracefully (/quit, SIGTERM).

This is distinct from #108 (which attributes orphans to the hook not firing on crash/kill). Here the hook fires normally (exit 0) but cannot locate the broker.

Why this happens in normal use

The broker cwd is decided by resolveCommandCwd (codex-companion.mjs:149):

options.cwd ? path.resolve(process.cwd(), options.cwd) : process.cwd()

→ flows to connect(cwd) → ensureBrokerSession(cwd) → broker --cwd and broker.json location.

A very common workflow breaks this:

• Session starts in the main tree (session cwd = /repo).
• A review runs against a git worktree — either cd worktree && node companion … or node companion … --cwd worktree → broker cwd = worktree.
• /quit → SessionEnd input.cwd = /repo → loadBrokerSession(/repo) looks at the /repo hash, never finds the worktree broker.json → broker survives.

Reviewing a worktree's uncommitted working tree requires broker cwd = worktree, so for a main-tree session this mismatch is structural, not accidental.

Reproduction (verified)

# broker registered for cwd=A
ensureBrokerSession("/tmp/probe")            # broker --cwd=/tmp/probe, alive

# SessionEnd with matching cwd  -> broker is killed (OK)
echo '{"cwd":"/tmp/probe","session_id":"s1","hook_event_name":"SessionEnd"}' \
  | node session-lifecycle-hook.mjs SessionEnd   # exit 0, broker DEAD

# SessionEnd with mismatched cwd -> hook exits 0 but broker SURVIVES
echo '{"cwd":"/tmp","session_id":"s2","hook_event_name":"SessionEnd"}' \
  | node session-lifecycle-hook.mjs SessionEnd   # exit 0, broker ALIVE  <-- bug

Observed in the wild: every live session had cwd in the main tree, while every orphan broker's --cwd pointed at a worktree — a perfect mismatch pattern.

Root-cause fix (preferred)

Decouple broker cleanup from cwd:

• A. Track brokers by session, not cwd-hash. Record the owning session_id in broker.json (and/or a global broker index). handleSessionEnd resolves brokers by input.session_id and tears down all of them regardless of cwd. This fixes the graceful-exit mismatch directly.
• B. Parent-session liveness watchdog in the broker. The broker records the owning session PID and periodically checks process.kill(pid, 0); if the owner is gone it self-terminates. This additionally covers SIGKILL / crash / OOM, where SessionEnd never fires at all.

A handles graceful exits; B handles abnormal exits. Together they remove the cwd dependency entirely.

Fallback (defense-in-depth, not a root fix)

An idle timeout (#108) as a last-resort net if both A and B fail. Useful but secondary — it lets brokers linger for the timeout window and masks the real ownership/lookup defect.

Related

• Broker process not cleaned up on session exit — no idle timeout #108 — broker not cleaned up on session exit / no idle timeout (overlapping symptom, different attributed root cause; the idle-timeout there is the fallback above)
• Same-cwd parallel /codex:* races on jobs.json + broker.json: data loss + orphan brokers #286 — same-cwd parallel races on broker.json (opposite case: same cwd)
• Background task hangs forever in "queued" when --cwd is a git worktree (detached worker dies silently; no timeout) #367 — background task hangs in a worktree cwd (worktree-specific, different symptom)

Environment

• Plugin openai-codex/codex v1.0.4
• Linux; reproduced directly against session-lifecycle-hook.mjs / broker-lifecycle.mjs

[MountainsCalling-me]

Confirming #380 in the wild, with a git worktree remove amplifier plus scale/symptom data that may help prioritize.

Setup: a repo that cuts a fresh git worktree per task and removes it right after merge. Codex reviews run against the worktree working tree, so the broker's --cwd is the worktree path — exactly the cwd ≠ session-cwd mismatch described here.

The amplifier: after merge, git worktree remove deletes the broker's --cwd. So beyond the SessionEnd lookup missing (this issue), the broker is now keyed to a path that no longer exists — it can never be located for shutdown and never be reused (ensureBrokerSession only reuses a reachable endpoint for the same cwd-hash, and that cwd is gone). Every merged worktree leaves a permanent orphan, recurring on every task.

Scale observed (macOS, plugin v1.0.4, ~26 days): 98 orphaned app-server-broker.mjs serve + 375 codex app-server processes, all holding ~/.codex/logs_2.sqlite open → the logs DB had grown to 384 MB with an active WAL.

Downstream symptom: once enough orphans accumulate, adversarial-review began intermittently dying at startup with:

[codex] Starting Codex task thread.
failed to load configuration: No such file or directory (os error 2)

So the accumulation doesn't just waste resources — it eventually breaks the review path (looks like contention on shared per-session state / the logs DB). Killing the orphans and rotating the logs DB cleared it immediately; a fresh run rebuilt a ~2 MB DB.

This reinforces the #108 idle-timeout ask as the right fix: in the deleted-worktree case, neither a corrected cwd-hash lookup nor session-liveness (#450) can help — the owning path is gone — so a broker-side idle self-terminate is the only thing that can reap it.

[sufhayes7]

Corroborating on Linux (plugin v1.0.4, worktree-per-task workflow, task --cwd <worktree> background jobs rather than reviews): 69 orphaned app-server-broker.mjs processes accumulated over a few weeks, every one keyed to a since-removed worktree path — same mismatch pattern as above. Two additions to the thread:

1. The deleted-worktree case is still reachable for graceful shutdown — idle timeout is not the only remedy.

in the deleted-worktree case, neither a corrected cwd-hash lookup nor session-liveness (#450) can help — the owning path is gone — so a broker-side idle self-terminate is the only thing that can reap it.

This turns out not to be so: resolveStateDir falls back to hashing the raw path string when the directory no longer resolves (realpathSync/git-repo resolution fail), so loadBrokerSession(<deleted-path>) still finds broker.json and the broker can be shut down cleanly. Verified directly: spawned a broker via ensureBrokerSession(dir), rm -rf dir, then ran loadBrokerSession(dir) → sendBrokerShutdown(endpoint) → teardownBrokerSession({..., killProcess: terminateProcessTree}) → clearBrokerSession(dir) — the broker exits and state is cleaned, directory long gone. (That sequence, targeted at the workdir instead of the session cwd, is exactly what handleSessionEnd already does; it's how we're working around this today, run right before git worktree remove.)

So fix A (session-scoped broker registry) would cover deleted worktrees on graceful exit, and an idle timeout is defense-in-depth for crash paths rather than the only option.

2. Feature ask: a supported caller-facing shutdown verb.

The companion CLI currently has no way to stop a broker (cancel interrupts the turn and kills the job worker's process tree, but the broker is spawned detached + unref() and isn't in it). Callers that manage worktrees know exactly when a workdir's broker becomes garbage — at git worktree remove — but the only way to act on that today is importing lib/broker-lifecycle.mjs internals as above. A supported

codex-companion.mjs shutdown --cwd <dir>

(or broker stop) would make deterministic cleanup possible for any worktree-based workflow, independent of — and complementary to — the SessionEnd-side fixes proposed here.

[axisrow]

+1. Corroborating the mismatch pattern in normal use on macOS v1.0.6 — among the ~260 orphaned app-server-broker.mjs (all PPID=1) I just cleaned up, the survivors consistently pointed at socket/cwd dirs (cxc-*) different from the session cwd, exactly the "hook fires but can't locate the broker" class described here rather than the never-fires case in #108. Decoupling cleanup from the cwd-hash (option A) + a parent-session liveness watchdog (option B) would cover both. The idle-timeout in #490 is a useful net for the survivors, but as you note it only masks the lookup defect.

[axisrow]

Correction on the "PPID=1" qualifier I used above — it's the wrong discriminator for the broker process, as @GeoffreyDjof lays out in #108 (and I've since confirmed on my own box): because spawnBrokerProcess uses detached: true + unref() and the spawner (codex-companion.mjs) is a one-shot that exits immediately, a broker is reparented to launchd at birth. A perfectly healthy broker serving a live session reads PPID=1. So "PPID=1" alone does not separate leaked from active — SIGTERM'ing that cohort would kill in-flight sessions.

The ~260 count in my earlier comment is still real, but the proof has to come from elsewhere: the survivors pointed at cxc-* socket/cwd dirs whose --cwd no longer exists on disk (a deleted worktree can't be under review by anyone — false positives impossible, and it reads --cwd off the process cmdline so it doesn't depend on PID liveness at all). That's the same sound signal @GeoffreyDjof converges on. The mismatch/lookup defect this issue is about is unchanged; just flagging the ppid framing in my prior note as unreliable.