Repository navigation
ensureBrokerSession() deletes a live broker's state without killing it — the only production caller passes no killProcess #753
Description
Summary
teardownBrokerSession() guards the kill behind killProcess:
// scripts/lib/broker-lifecycle.mjs:174
if (Number.isFinite(pid) && killProcess) {ensureBrokerSession() passes options.killProcess ?? null into both of its teardown
branches — but the only production caller passes no killProcess at all:
// scripts/lib/app-server.mjs:344
const brokerSession = await ensureBrokerSession(cwd, { env: options.env });(grep -rn "ensureBrokerSession(" --include=*.mjs returns exactly this one call site
besides the definition.)
So killProcess is null, the guard is false, and both teardown branches proceed to
unlink pidFile, logFile and sessionDir without terminating anything. The broker
survives, and deleting pidFile destroys the last handle anyone had on it.
This is a plain wiring gap, independent of the state-root split in #636 and of the missing
idle timeout in #108 / #543 — but it produces the same end state, and unlike those it can
fire on a perfectly healthy session.
The path that actually bites: the readiness timeout
ensureBrokerSession() spawns the broker, then waits 2 s for its endpoint:
// broker-lifecycle.mjs:149
const ready = await waitForBrokerEndpoint(endpoint, options.timeoutMs ?? 2000);
if (!ready) {
teardownBrokerSession({ endpoint, pidFile, logFile, sessionDir, pid: child.pid ?? null,
killProcess: options.killProcess ?? null }); // null -> no kill
return null;
}If the broker is merely slow, not broken, this deletes its session directory and
returns null. The process then finishes starting a moment later and listens on a pipe
whose name no longer exists anywhere on disk. Nothing can address it, and
ensureBrokerSession() will spawn a fresh one on the next call.
The same applies to the earlier branch (:119-128), where an existing broker that does
not answer within 150 ms has its state removed but is likewise never killed.
Reproduction
Verified on Windows 11 / Node 22.16 / plugin 1.0.6. timeoutMs: 1 stands in for a
loaded machine; killProcess is omitted exactly as app-server.mjs:344 omits it:
const { ensureBrokerSession } = await import("./lib/broker-lifecycle.mjs");
const before = brokerPids();
const s = await ensureBrokerSession(cwd, { env: process.env, timeoutMs: 1 });
await new Promise(r => setTimeout(r, 3000)); // let the broker finish starting
console.log(s, brokerPids().filter(p => !before.includes(p)));ensureBrokerSession returned: null
new broker processes still alive: 130224
broker.json written: false
=> live process, recorded nowhere
How much headroom does the 2 s actually have?
Measured five broker starts on an otherwise busy machine (14 concurrent Claude Code
sessions):
1413 ms · 1315 ms · 1405 ms · 1366 ms · 1334 ms
median 1366 ms limit 2000 ms
runs over the limit: 0 of 5
So the timeout did not trip during this measurement — I am not claiming it is the
dominant trigger in practice, only that the margin is about 630 ms on a machine that is
already loaded, and that when it does trip the result is a permanently unreachable
process rather than a retry. Cold start, antivirus on node.exe, or heavier load would
consume that margin.
Impact
On this machine the various leak paths together accumulated 705 orphaned node.exe
processes / 17.8 GB RSS over roughly two days, oldest 31.1 h. Each broker carries a
codex app-server plus its children, so one leaked broker costs about 7 processes.
Of 29 live brokers at one measurement, 7 had their broker.pid already deleted while the
process kept running — the signature this bug produces.
Suggested fix
Pass a real terminator at the call site, and do not delete the pid file before a kill has
been attempted:
// app-server.mjs
const brokerSession = await ensureBrokerSession(cwd, {
env: options.env,
killProcess: terminateProcessTree,
});Better still, default it inside ensureBrokerSession() rather than relying on every
caller to remember — teardownBrokerSession() already ignores a missing process, so a
default is safe. terminateProcessTree() is right there in lib/process.mjs and is what
session-lifecycle-hook.mjs already hands in.
Two smaller hardening notes:
teardownBrokerSession()could recover the PID frompidFilewhen the caller has none,
and should unlinkpidFileonly after the kill attempt.- A silent no-op is hard to notice here; having
ensureBrokerSession()report that it
abandoned a live child would have surfaced this much sooner.
Related
- SessionEnd cannot find the broker when CLAUDE_PLUGIN_DATA differs between spawn and teardown — same cwd, same hash, different state root #636 — state root split via
CLAUDE_PLUGIN_DATA. Confirmed on Windows too: all 4
workspaces present in%CLAUDE_PLUGIN_DATA%\statealso existed under
%TEMP%\codex-companion, each with a different, also-running PID. I reproduced the
duplicate-broker sequence directly rather than inferring it. - Broker process not cleaned up on session exit — no idle timeout #108, app-server brokers never self-terminate → orphaned process/RAM leak (34 chains, 272 procs, ~2.2GB) #543 — no idle timeout. An idle self-exit would also reclaim the brokers this bug
strands. - SessionEnd cleanup fails when review cwd ≠ session cwd — broker.json looked up by cwd-hash, leaving orphan brokers even on graceful /quit #380 — cwd mismatch (different hash); distinct from both of the above.
Note on spawnDetachedTaskWorker
detached: true + unref() on the task workers looks like the obvious suspect but is not
implicated: of 29 orphaned processes, 0 were task workers — they are tracked as jobs
and cleanupSessionJobs() terminates them. The brokers are the leak.
Activity
CBEPX commentedon Sep 27, 2026
This is addressed in the CBEPX fork release v1.3.0; see the changelog for the full list of changes. A live but wedged broker is now killed before
ensureBrokerSessionreplaces it, instead of just deleting its state and leaving the old process running — thanks @Soumya95 for the PR, which we used as reference. Same root cause as #782; the identity-checked kill this relies on is posix-only until process identity support lands on Windows in v1.4.0.This is a fork release, not an upstream fix, so I'm leaving this issue open for the maintainers here.
hibinoyuki commentedon Oct 6, 2026
Adding a real-world data point from Windows, since this wiring gap turned into a machine-level outage for us.
Environment: Windows 11 (64 GB RAM), Node v24.13.0, codex-cli 0.153.4, plugin 1.0.3. I checked that
app-server.mjs:344andbroker-lifecycle.mjs:102-126are unchanged in v1.0.6 and onmain, so I believe this still applies.What we observed (2026-10-06, 21 days of uptime, ~10 Claude Code sessions per day):
app-server-broker.mjs serveprocesses were still alive. The oldest was 21 days old. Only one of them was referenced by a currentbroker.json; we had 38broker.jsonfiles across workspaces.codex.js app-server → codex.exe → MCP serverschains: 559 processes and about 21 GB of private memory. Each chain carries the full MCP set configured for Codex (playwright, analytics, notebooklm, etc.), so a single orphan cost 1–7 GB.Possible feedback loop (hypothesis, not measured): the reuse check gives an existing broker only 150 ms (
isBrokerEndpointReady→waitForBrokerEndpoint(endpoint, 150)). Under memory pressure a healthy broker can miss that window. Because nokillProcessis passed, the old broker is then orphaned instead of terminated. More orphans mean more memory pressure, which makes the next 150 ms check more likely to fail.A note for anyone writing an external reaper:
broker.logis not an activity signal. It is the broker's own stdout/stderr, and on our machine it only ever contained the startup DEP0190 warning. Its mtime equals the start time even during long turns. A tracked job instate.json(runningwith a live pid) was the only reliable "in use" signal we found.Workaround we deployed: a scheduled reaper that stops a broker together with its whole subtree only when all of the following hold:
~/.claude/plugins/data/codex-*/state/*,$CLAUDE_PLUGIN_DATA/state/*and the%TEMP%\codex-companionfallback, failing closed on unreadable state);ensureBrokerSessionthen recreates the broker on the next/codexcall as expected. It works, but passingkillProcess(and a more generous readiness timeout) at the single call site would remove the need for it.skaffen commentedon Oct 20, 2026
Still present in 1.0.6:
CodexAppServerClient.connect(lib/app-server.mjs) callsensureBrokerSession(cwd, { env: options.env })with nokillProcess, so the "existing broker not ready" branch removes the old broker's state and starts a new one without stopping the old process.The readiness probe is
waitForBrokerEndpoint(endpoint, 150). A 150 ms connect timeout fails easily on a heavily loaded machine (my load average was 150–500 while this was happening), so the leak gets faster exactly when the machine can least afford it. Each orphan keeps itscodex app-serverand MCP servers. This added to a memory exhaustion that ended in a watchdog panic; there are details and a repro for the related cwd leak in #380.Passing
killProcess: terminateProcessTreethere, and/or raising the probe timeout (or retrying it once), would close this.abooyaya commentedon Oct 24, 2026
Another data point, this time from macOS (Apple Silicon, plugin 1.0.6), plus a note on why the 150 ms probe makes this self-reinforcing.
What we saw: after ~2 days of normal use with background
taskjobs across a few workspaces/worktrees, there were 83 unreachable brokers. Each one kept its whole tree alive: 2× nativecodex(app-server), plus whatever MCP servers that codex loads. That added up to about 19 GB and swap was nearly full (30.7 / 31.7 GB). The 84 leftovercxc-*dirs in$TMPDIRmatched the leak count almost exactly.Why it gets worse over time:
isBrokerEndpointReady()gives an existing broker only 150 ms (waitForBrokerEndpoint(endpoint, 150)). Under memory pressure a perfectly healthy broker misses that window. When it does,ensureBrokerSession()tears down its state without killing it (thekillProcesswiring gap described above) and starts another broker. That adds more memory pressure, so the next probe is even more likely to time out.Local fix we're running:
On top of that, a periodic sweeper. Since brokers are spawned
detached, every broker has PPID=1, so PPID can't tell you whether a broker is orphaned. What does work is checking whether the broker's own--endpoint unix:<path>socket still exists, and whether its pid is still listed in anybroker.json. Each of those signals has been reliable for us on its own.Verified on 1.0.6 after applying both changes: three consecutive
taskruns in the same workspace reused a single broker, and a broker whose socket we deleted (simulating the stale-teardown case) was detected by the sweeper and its whole tree, broker →codex app-server→ nativecodex, was terminated. Brokers that were still registered and working were left alone.One caveat for anyone writing a similar sweeper: leave brokers younger than about 2 minutes alone. During startup the socket isn't listening yet and
broker.jsonhasn't been written, so a brand-new broker passes both orphan checks.