Import three high-value upstream PRs, pin what they guarantee, and describe the fork

[Edo771977]

Second round of imports from openai/codex-plugin-cc, one merge commit each so any of them can be reverted on its own — plus the defects an adversarial review found in those merges, and a README that finally says what this fork does. The fourth candidate, openai#733, is deliberately not here (see below).

Imported

Commit	Upstream PR	What it fixes	Conflicts
b4f788e	#707	the broker never released its app-server thread subscriptions, so a client that disconnected left its threads subscribed for the broker's lifetime and their notifications reached whoever connected next	9
b59bdff	#659	state written under one CLAUDE_PLUGIN_DATA root was invisible to an invocation that resolved to another, orphaning brokers and hiding jobs	9
c8aa8a8	#728	a job whose worker died kept reading as "running" forever, because nothing ever rewrote the record	9

Each merge commit records its own resolution. Three are worth calling out here.

openai#707 required rebuilding the message loop, not merging it. The PR rewrote the loop into handleLine() on top of upstream main, which has none of this fork's hardening (instance-token auth on broker/shutdown, the busy check, JSON-RPC shape validation, responseSocket, abandoned turns, completions racing a handoff) — and the auto-merge kept that copy as dead code beside the fork's loop. It is removed. Its serialization is kept and matters: with the fork's async (chunk) handler a second data event interleaves with an await inside the first, and the per-request ownership claims stop describing who holds which thread. broker serializes subscription requests from one downstream client fails exactly that way. So the fork's loop body is lifted into handleLine() and driven through the PR's processing chain.

It also surfaced a latent bug in tests/fake-codex-fixture.mjs: merging the two nextThread() signatures left four call sites passing the options object positionally, which made a forked or subagent thread silently read-only instead of inheriting its source's sandbox.

openai#659 keeps this fork's atomic writes. The PR's cross-root pruning shipped with a plain writeFileSync; here it goes through writeJsonFileAtomic() like every other state write. The fork's terminal-claim files get the same candidate-root treatment as job files through a new resolveJobClaimFileCandidates(), which the PR had no reason to know about.

openai#728 is grafted, not adopted. Where its session-end and cancel rewrites met this fork's, the fork's are kept (terminal claims, interrupt budget, straggler handling, record-first cancellation) and only its distinct insight is taken: a dead worker still gets its terminal record — that is what stops /codex:status answering "No job found" for the session that just ended — except when it exited after turn/start with a thread but no turn id, where nothing can interrupt that turn and recording it cancelled would claim an outcome that did not happen.

Not taken from openai#728: session end fully cleans up jobs for the ending session, which asserts the ending session's job files are deleted. This fork retains a terminal record so a later status query shows a cause, and has its own test pinning that.

openai#733 is not imported

Its central behavioral test, cancelled queued task is never reclaimed by a late-starting worker, already passes on this fork: the cancelled job is not reclaimed, Codex is never invoked, the record stays cancelled. The only difference is the worker's exit code (1 with Job … was cancelled before it started. rather than 0). Its other "behavioral" test asserts the shape of the source (trackedBody.indexOf("try {") < indexOf("writeJobFile")), so it fails on any fork whose code is arranged differently.

What would remain is a second cancellation mechanism — .cancelled and .removed marker files — beside the terminal claims this fork already uses, i.e. two sources of truth for the same decision, and .removed serves a session-end semantics this fork deliberately does not have.

Review: five rounds

An independent review was run over the merges and re-run after each round of fixes. The full account is in a comment; the fifth round's verdict is "nothing blocking remains — the PR is mergeable."

The first pass found eight defects, four of them in the conflict resolutions made here, including two that quietly undid the protections they were part of: the cancel refusing after taking the job's unreleasable terminal claim (so the next cancel turned that claim into a bogus cancelled record for the turn being protected), and the session-end guard reading the stale state.json snapshot instead of the job file the code re-reads ten lines below for that very purpose. The other four were in imported code: a cross-root prune running under the wrong lock, a stranded stopReviewGate: true outvoting an explicit disable forever, CLAUDE_ENV_FILE rewritten rather than appended (dropping other plugins' exports and the file's mode), and a staging-lease race that let one process delete a file another was reading.

Rounds two through four were the fixes' own defects — most of them around the record retained for a turn that cannot be interrupted, which in turn became reapable, then unbounded, then a permanent broker leak under CODEX_BROKER_IDLE_SHUTDOWN_MS=0. That last one was called blocking and is fixed; the window now falls back to the staleness bound, and an unreadable timestamp counts as expired.

Every fix carries a regression test that fails with only its own fix reverted, except the staging-lease race, which needs an interleaving between two processes and rests on the reasoning in its commit.

Tests asked for by the upstream reviews

Both reviews on the upstream PRs this fork had already patched ask for coverage; it belongs here too.

• From #731 (184df24): the durable review-gate config is 0600 in a 0700 directory (reverting to the PR's plain writeFileSync fails it), and a replacement that fails mid-write leaves the previous config intact with no temporary file beside it.
• From #737 (ed2d434), with every manager root populated at once: a managed toolchain beats a system install (restoring the pre-fix ordering fails it, along with the four upstream tests that caught the bug), and CODEX_COMPANION_NODE overrides every manager. Which manager wins is deliberately not pinned — the launcher reads no .nvmrc, .tool-versions or mise config, so asserting one would freeze an arbitrary order as a guarantee.

CI

916fcfc fixes the only failure CI reported, in tests/locking.test.mjs — a test neither this branch nor the imports touch. The assertion was the flaky part, not the behavior: acquireLock() checks its deadline only after a failed attempt, so on a loaded runner one slow attempt still returns successfully past the 500ms the test measured. Nothing is weakened by dropping that measurement — acquireLock() throws when its deadline passes, so a successful return under a 500ms budget with staleMs at 30s already proves the lock was reclaimed because its owner is gone. Verified by commenting out reclaimAbandonedLock(): the test fails again.

Docs and metadata

625c770 and 877e58b rewrite the landing page around what the plugin now guarantees rather than which PRs it carries:

• a "What The Background Runtime Guarantees" section up front — state checked rather than trusted, session end never reporting an outcome that did not happen, one broker per workspace retired by the last session out, a finished job keeping its cause, nothing world-readable or half-written, Node found where it is installed;
• /codex:status documents the two reconciled states a dead worker produces, /codex:result that a non-crashing failure keeps its error text, /codex:cancel its order of operations and the three outcomes that are not a plain cancellation;
• a "Where State Lives" table (the two plugin-data roots, the durable config under CODEX_HOME, and that threads and auth stay with the Codex CLI), and the note that the broker idle window also bounds a retained orphan;
• "Differences From Upstream" gains this round plus every defect the review found, and what is deliberately not imported.

The plugin, marketplace and package.json descriptions now say this is a fork of openai/codex-plugin-cc carrying open upstream fixes — until now /plugin listed it with upstream's own description. The version stays 1.0.6 (npm run check-version passes): these merges do not cut a release.

Testing

npm test: 312/312, green on repeated full runs. npx tsc -p tsconfig.app-server.json clean, CI green on the head.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UXfvnjSC72HsM6EEPVt2Tg

[Edo771977]

Review: five rounds, and the last one comes back clean

An independent review was run over the whole day's work (this PR plus the already-merged round), then re-run after each round of fixes. The fifth round found nothing blocking: "this round closes all three findings correctly — the PR is mergeable." Suite is 312/312, tsc clean, CI green.

It was worth running. The first pass found eight defects, four of them in the conflict resolutions made here, and each fix round surfaced more — which is the point of doing it adversarially rather than once.

Round 1 — eight findings

Four in this branch's grafts, fixed in a5f7959:

• The cancel refused after taking the terminal claim. The claim is never released, so the leaked cancel-intent claim made the next cancel — or SessionEnd — adopt it and write a cancelled record for the turn the refusal exists to protect. Exactly the outcome the guard was added to prevent.
• The session-end guard read the stale state.json snapshot instead of the job file the code re-reads ten lines below for that very reason. With pid: null in the snapshot the guard was skipped and the job recorded cancelled while its turn ran on.
• The retained orphan was written back with pid: null, and reconcileJobLiveness() needs a pid: the record could never be judged again.
• assertResumedSandbox() asserted the requested mode on scoped resumes, although buildThreadAccessParams() deliberately sends a permission profile and no sandbox with --read-root. Every scoped resume was refused, and the error's own advice silently dropped the write grant. Reproduced end to end by the reviewer.

Four in imported code, fixed in 2a2bd77: the cross-root prune running under the wrong lock; a stranded stopReviewGate: true outvoting an explicit disable forever; CLAUDE_ENV_FILE rewritten rather than appended, dropping other plugins' exports and the file's mode; and a staging-lease race that let one process delete a staged file another was reading.

Rounds 2–4 — the fixes' own defects

• The pid: null + workerExited record made the orphan reapable: another session's SessionEnd failed it and it stopped pinning the broker — tearing the runtime down under the protected turn (af0cdd7).
• That same record then had no exit short of a 24h reap, and belonged to the ending session, so the very hook that retained it shut the broker down anyway (0c5ddd8).
• Bounding it to the broker's idle window then leaked permanently under CODEX_BROKER_IDLE_SHUTDOWN_MS=0 — a documented opt-out. The reviewer called this blocking; it now falls back to the staleness bound, and an unreadable timestamp counts as expired rather than as protected forever (0fdea5f).
• Also fixed along the way: the escalation check dropped by the scoped-resume fix (restored, and extended to fresh starts with the right remedy named), per-root deletion of a dropped job's artifacts, the env-file newline separator, release() masking the import error with a lock timeout, and the other-root prune judging ids from a later re-read instead of the caller's snapshot.

Round 5

Three low findings, two addressed in 84625b1: both new expiry branches were dead in the suite (the existing test seeds updatedAt in 2099) and are now driven end to end, and two comments that overstated the guarantees are corrected. The third — isStaleJobRecord() reading an unparseable timestamp conservatively for non-orphan pid-less records — is pre-existing behavior outside this PR's scope and is left alone.

Every fix carries a regression test that fails with only its own fix reverted, except the staging-lease race, which needs an interleaving between two processes and rests on the reasoning in its commit.

---

Generated by Claude Code