Import eleven open upstream PRs, fix what they broke, and bring the README up to date

[Edo771977]

Imports eleven pull requests that are still open in openai/codex-plugin-cc, one merge commit each so any of them can be reverted on its own, fixes three defects this surfaced, and updates the README to describe what the fork actually does.

Upstream main has not moved since this fork's base (db52e28, 7 Jul), so there is nothing merged to catch up on — only open PRs.

Imported

Commit	Upstream PR	What it brings	Conflicts
8c30e32	#565	the stop-review gate honors stop_hook_active instead of re-blocking every continuation turn until the retry cap	none
1f6d4ad	#748	CLAUDE_ENV_FILE keeps one export per key instead of growing on every session	none
66c9b24	#746	--model/--effort on the review commands, and a warning instead of silence for an unrecognised option	none
196276b	#742	--sandbox <mode> on task and /codex:rescue	1
4a2ef41	#727	--resume-thread <id> to continue one specific Codex thread	4
9e6b38b	#729	/codex:transfer works with a relocated CLAUDE_CONFIG_DIR	none
4cda989	#724	--read-root <directory> for OS-enforced scoped reads	14
c4a4c4f	#747	explicit 256 MiB maxBuffer, so a large git diff stops being truncated at Node's 1 MiB default	2
fce431e	#763	a turn that fails without throwing persists its error text, so /codex:result says why	2
ed667e1	#731	the review-gate flag moves out of the transient state dir into a durable file under CODEX_HOME	2
62c5832	#737	hooks resolve Node through scripts/run-node.sh (nvm, fnm, asdf, mise, Volta, Homebrew)	1

Every conflict came from the imported PRs colliding with each other, not with the fork. Each merge commit records its own resolution.

Not imported: #761 (max/ultra reasoning efforts) — the same proposal from another author, #648, was closed upstream, so those values look unsupported rather than merely undocumented.

Resolutions worth reviewing

#724 vs #742: who owns the sandbox mode. #724 derived it from --write; #742 makes --sandbox the input and derives write from it. The resolution keeps #742's direction and layers read roots on top. Three things follow from the combination and exist in neither PR alone:

• --read-root together with --sandbox danger-full-access is now rejected — that mode turns the sandbox off, so no read scope would be enforced.
• In runAppServerTurn(), #724's try/catch around thread start/resume wraps #742's assertResumedSandbox() rather than replacing it. The plain auto-merge would have dropped the resume check.
• tests/fake-codex-fixture.mjs recorded each thread start twice after the auto-merge (#724's raw params, then #742's structured record silently overwriting them). Both handlers now store {...message.params, threadId}, which satisfies both PRs' assertions.

Not taken from #724: its version bump to 1.0.11 and the five CHANGELOG sections that go with it. Those releases do not exist in this fork, so the plugin stays at 1.0.6 (npm run check-version passes) and the entries are folded into an Unreleased section.

In hooks.json the SessionEnd timeout stays at this fork's 30s rather than reverting to upstream's 5s, since the bounded teardown imported earlier needs it.

Defects fixed along the way

dd70d87 — a busy broker refusing shutdown is not an identity rejection. shutdownBrokerSessionLocked() treated every error reply to broker/shutdown as an identity mismatch, the BUSY refusal included, so when a client connected between the session-end guard check and the shutdown request — the normal race — SessionEnd exited 1 blaming the instance token for a broker that was simply still in use. It now returns { refused: true, exited: false } and leaves the process and the record alone. This also retires the two tests that had been failing since the earlier imports: one predated #623's busy check and now parks its lingering socket as a peer shutdown requester (what the broker itself calls "not work"); the other expected null from sendBrokerShutdown() and now asserts the outcome shape that matters.

62c5832 — #737 shipped with its precedence inverted. find_managed_node() listed /home/linuxbrew, /opt/homebrew, /usr/local/bin and /opt/local/bin before the version-manager roots, so on any machine carrying /usr/local/bin/node the system install shadowed every nvm/fnm/asdf/mise toolchain — the precise failure the PR set out to fix. Four of its own tests catch it. The managed roots now come first; HOMEBREW_PREFIX keeps its place ahead of them because it is configured rather than guessed.

ed667e1 — #731 wrote its new config file unprotected. writeDurableConfig() used fs.writeFileSync and a plain mkdirSync, while every other artifact state.mjs creates goes through ensurePrivateDir()/writeJsonFileAtomic(). It now does too: 0600 like the rest, and a torn write can no longer make readDurableConfig() fall back and silently disable the review gate.

4f5c23e — the typecheck was already red on main. CI's npm run build step rejected six TS2339 errors that reproduce identically on the base commit (7c86c7f); they arrived with the earlier lifecycle imports, merged while Actions was not running on this fork. Three annotations with no runtime effect: proc declared on the client base class, a socket error typed as NodeJS.ErrnoException, and a @param for terminateProcessTreeAndExit's options.

README

The landing page still described upstream's command surface, so several flags this fork supports were undocumented:

• a "Commands At A Glance" table listing every slash command with its real flags, the accepted --effort values, and the new unrecognised-option warning;
• the install snippet points at this fork, and says how to get upstream instead;
• --model/--effort on the review commands, --resume-thread and the resume flags' exclusivity, CLAUDE_CONFIG_DIR on /codex:transfer, the review gate's continuation-turn skip, and a note that Node no longer has to be on the system PATH;
• a "Differences From Upstream" section listing every imported PR with a link, including the earlier rounds (Fix test broker leaks, state races, and signal-masked command failures openai/codex-plugin-cc#541, bug fix: stop session end from tearing down the shared broker under other sessions' jobs openai/codex-plugin-cc#623, fix: bound the lifetime of detached brokers and task workers openai/codex-plugin-cc#652).

Both review commands' argument-hint now name --model/--effort, which the script already accepted. tests/commands.test.mjs asserted the adversarial hint as one adjacent string ([--scope ...] [focus ...]); it now asserts the scope, effort and focus fragments separately, so inserting a flag between them no longer fails a documentation test.

Testing

npm test: 254/254, green on repeated full runs, and CI is green on the head. npx tsc -p tsconfig.app-server.json is clean and npm run check-version passes.

Authorship

The commits carried in by the merges keep their original authors and hashes, so they still match the upstream PRs and can be compared against them later. GitHub marks them Unverified, as it does for any third-party work merged into a fork.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UXfvnjSC72HsM6EEPVt2Tg

[Edo771977]

Pushed dd70d87: the two pre-existing failures this PR's description listed as not addressed are now fixed, so the suite is 233/233 (green on two consecutive full runs).

One of them turned out to be a real defect rather than a stale test. shutdownBrokerSessionLocked() treated every error reply to broker/shutdown as an identity mismatch, the BUSY refusal included — so when a client connected between the session-end guard check and the shutdown request (the normal race), SessionEnd exited 1 with "broker rejected shutdown identity" about a broker that had recognized it perfectly well and was simply still in use. It now returns { refused: true, exited: false } and leaves the process and the persisted record alone, which is exactly what the hook's own pre-check already did with a refusal.

On the tests:

• shutdown closes idle half-open broker clients (from #541) predates the busy check #623 added later, which refuses shutdown while any other client is connected — deliberately, since a worker between requests has no in-flight request to detect. Rather than weaken that rule, the test now parks its lingering socket as a peer shutdown requester, which the broker itself defines as "not work": same subject (shutdown destroys a half-open socket instead of waiting on it), current contract. It also awaits end instead of close — an allowHalfOpen client never closes itself, so the old expectation could not have been met either way.
• shutdown request always uses a finite deadline expected null from sendBrokerShutdown(), which has returned an outcome object since the lifecycle merges. It now asserts what matters: a broker that accepts the connection and then says nothing is not delivered, not refused, and deliberately not unreachable, so teardown cannot reap a broker that may still be serving someone.

Added a busy broker refuses shutdown instead of failing it to cover the fix directly.

---

Generated by Claude Code

[Edo771977]

Pushed the second batch (c4a4c4f…4783ed1): four more upstream PRs, same one-merge-commit-each shape. Suite is 254/254, green on two consecutive full runs.

Commit	Upstream PR	What it brings	Conflicts
c4a4c4f	#747	explicit 256 MiB maxBuffer, so a large git diff stops being truncated at Node's 1 MiB default	2
fce431e	#763	a turn that fails without throwing persists its error text, so /codex:result says why	2
ed667e1	#731	the review-gate flag moves out of the transient state dir into a durable file under CODEX_HOME	2
62c5832	#737	hooks resolve Node through scripts/run-node.sh (nvm, fnm, asdf, mise, Volta, Homebrew)	1

Two places needed more than a merge:

openai#737 shipped a defect its own tests caught here. find_managed_node() listed /home/linuxbrew, /opt/homebrew, /usr/local/bin and /opt/local/bin before the version-manager roots, so on any machine that has /usr/local/bin/node — Debian and Ubuntu packages, CI images, this repo's own container — the system install shadowed every nvm/fnm/asdf/mise toolchain. That is precisely the failure the PR set out to fix, and four of its tests (discovers Node from a custom NVM_DIR, custom FNM_DIR, custom ASDF_DATA_DIR and MISE_DATA_DIR, skips unsupported Node versions) fail unless you happen to have no system node. The managed roots now come first; HOMEBREW_PREFIX keeps its place ahead of them because it is configured rather than guessed. All 14 launcher tests pass.

openai#731 wrote its new config file unprotected. writeDurableConfig() used fs.writeFileSync and a plain mkdirSync, while every other artifact state.mjs creates goes through ensurePrivateDir()/writeJsonFileAtomic(). It now does too — 0600 like the rest, and a torn write can no longer silently disable the review gate on the next read.

Also worth recording: in hooks.json the SessionEnd timeout stays at this fork's 30s rather than reverting to upstream's 5s, since the bounded teardown imported earlier needs it.

Not imported: #761 (max/ultra reasoning efforts). The same proposal from another author, #648, was closed upstream, so those values look unsupported rather than merely undocumented.

---

Generated by Claude Code

[Edo771977]

CI ran for the first time on this fork and failed on dd70d87 — not on the tests (233/233 at that commit), but on the npm run build typecheck, with six TS2339 errors.

They are not this branch's. All six reproduce identically on main (7c86c7f): I ran npx tsc -p tsconfig.app-server.json against a worktree of the base commit and got the same list, same files, same positions. They arrived with the earlier lifecycle imports, which were merged while Actions was not running on this fork, so nothing ever flagged them.

Since no fix for them exists anywhere and the PR cannot reach green while the base is red, 4f5c23e fixes them here. All three are annotations with no runtime effect:

• app-server.mjs:441 read client.proc off the client union, which only the spawned transport has. The base class now declares proc as a nullable ChildProcess, so the shared cleanup path is honest about both transports; the spawned client still assigns the real child in its own constructor, after super().
• broker-lifecycle.mjs:74 read error.code off a socket error typed as plain Error — the callback parameter is annotated NodeJS.ErrnoException.
• process.mjs terminateProcessTreeAndExit() took beforeKill with no default, so it was absent from the options type inferred from = {}. A @param states the shape.

npx tsc -p tsconfig.app-server.json is now clean, and the suite is 254/254.

If you would rather keep this branch strictly to the imports, the commit reverts cleanly on its own — but then main stays red on the build step until the same fix lands there.

---

Generated by Claude Code