spawn: launch codex through the bundled monitor shim, and refuse a bare fallback - #1064
Merged
Merged
Conversation
…re fallback
A codex seat spawned by spawn.sh never received a message: the bridge retried
against its thread every few seconds ("already has an active writer"). The
seat's argv was the real binary with no --remote. Only the monitor shim adds
--remote, and the shim is installed on purpose as an interactive-shell
function (#193), which is not exported and does not exist in the
non-interactive shell that runs a boot script — so `command -v codex` there
resolves the real binary, on every spawn, not only the first.
A type manifest may now declare `spawn_wrapper=<script>`, relative to the
type's driver directory; spawn resolves it through agmsg_type_dir to a bundled
absolute path and makes it the executable of the boot script's CLI line, with
any fixed cli suffix tokens kept. codex declares codex-shim.sh. A declared
wrapper that is missing or not executable is a refusal, never a fallback to
the bare cli: the symptom this closes is a seat that looks spawned and never
says a word, and any silent bare launch would reproduce it. The `cli=`
existence check stays, because the wrapper needs the real binary too.
codex-monitor.sh's fail-open (a broken app-server hands off to a plain codex)
now fails closed when AGMSG_SPAWNED=1, the marker every spawn boot script
exports. Fail-open is for a person at the keyboard; a spawned seat has nobody
at its pane, and a plain codex there is the same silent seat. The person's
path is unchanged and its test is the control.
Tests read the evidence rather than the intent: the boot script's executable
token is the shim's bundled path; chmod -x on the shim turns spawn into a
refusal with no boot script handed to the terminal; claude-code still launches
bare claude; and running the boot script itself against a fake codex that
records argv and serves a fake app-server reaches the real binary exactly once,
with --remote ws://127.0.0.1:<port> and the actas prompt.
Refs #1063
…wrapper runs Review found a path around the fail-closed guard: codex-shim.sh execs the real binary at once when AGMSG_CODEX_BRIDGE=1 or AGMSG_CODEX_SHIM_DISABLE=1 is in its environment — a contract for nested codex invocations inside a bridged session — and codex-monitor exports AGMSG_CODEX_BRIDGE=1 right before it execs the bridged TUI. So a seat spawned FROM a bridged codex seat inherited that variable, the shim handed the boot straight to the real binary, and the guard inside codex-monitor was never reached. The guard sat after the act it was meant to prevent. The boot script now clears that state before the wrapper line, through the manifest's existing spawn_unset_env list (#294's mechanism): CODEX_THREAD_ID, AGMSG_CODEX_BRIDGE, AGMSG_CODEX_BRIDGE_APP_SERVER, AGMSG_CODEX_BRIDGE_LAUNCHER and AGMSG_CODEX_SHIM_DISABLE. Control flow of the generated script, in order: cd into the project; export AGMSG_SPAWNED=1; unset those variables; run the wrapper. The shim reads its environment only after that unset, so neither early exit can fire for a spawned seat; the AGMSG_SPAWNED guard in codex-monitor stays as the second line. Tests seed the parent environment and read the argv that reaches the real binary: with the bridge variables set, and separately with the opt-out set, the spawned seat still arrives as codex --remote ws://127.0.0.1:<port> … actas …, and the unset line is asserted to precede the wrapper line. Removing the spawn_unset_env entry turns both seeded tests red while the clean-environment launch stays green, so each variable is caught on its own. Refs #1063
…from its readers
The second review round found one more inherited control the deny-list had
missed: the PATH wrapper that codex-shim-install.sh generates exports
AGMSG_CODEX_SHIM_WRAPPER=1 and AGMSG_CODEX_SHIM_SCRIPT_DIR before starting the
bundled shim, and a codex started through it passes both to anything it
spawns. Inherited, the bundled shim takes the parent's install directory as
its own — a stale or different path has no delivery.sh, which the shim reads
as "not a monitor project" — and hands the boot to the real binary, again
before any guard in codex-monitor runs. One name per round is the deny-list
failing in its own way, so the rule is now a namespace.
spawn_unset_env entries may end in `*`: the boot script enumerates every
exported variable with that prefix from `env` at boot time and unsets each,
so a control the stack adds later is cleared without anyone listing it. codex
declares `AGMSG_CODEX_*` plus the two names outside it, AGMSG_REAL_CODEX and
CODEX_THREAD_ID. The emitted loop uses `sed -e … -e …` rather than `sed -n`
so the script carries no bare `-n`, which is also claude-code's name flag and
is asserted absent from a codex boot.
The set was derived from both sides and the verdict per name is recorded in
type.conf: readers are every $VAR / ${VAR in codex-shim.sh, codex-monitor.sh,
_app-server.sh, codex-bridge-launcher.sh, codex-record-session.sh,
_session-start.sh, codex-shim-install.sh and _delivery.sh, and every
process.env.VAR in codex-bridge.js, for AGMSG_* and CODEX_*; writers are the
stack's own export lines, the command-local AGMSG_REAL_CODEX the shim hands to
codex-monitor, and the generated wrapper. Kept, each with its reason: the
seat's own AGMSG_SPAWNED marker, agmsg-wide configuration (AGMSG_BASH,
AGMSG_WATCH_ONCE_*), bats-only hooks, and names that are script-local
variables rather than environment inputs. A full allow-list (env -i) was
rejected: the CLI needs the user's own auth, PATH, HOME, locale and terminal,
which spawn cannot enumerate per CLI version, so a miss there breaks every
spawn; a miss here — a control named outside the namespace — is what the
reader-inventory test now catches.
Tests: the inventory test derives that reader set the same way and fails on
any name not covered by the manifest or the reasoned keep list; a seat spawned
with AGMSG_CODEX_SHIM_WRAPPER=1 and a stale AGMSG_CODEX_SHIM_SCRIPT_DIR still
arrives bridged; inherited resolution overrides pointing at nonexistent paths
do not steer it; and a control that does not exist yet
(AGMSG_CODEX_FUTURE_KNOB) is absent from the environment the real binary
receives while an ordinary variable and AGMSG_SPAWNED survive — that last test
goes red if the namespace entry is replaced by today's explicit list. The
bridge-launcher stand-in for the end-to-end tests is now a file in the test
copy of the skill rather than an environment hook, since the boot clears those
too.
Refs #1063
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs #1063.
spawn.shlaunches a Codex seat through the bundled monitor shim instead of the barecodexbinary. The shim is installed on purpose as an interactive-shell function (Prefer shell function setup for Codex monitor shim #193), which a non-interactive boot script cannot see, socommand -v codexthere resolved the real binary and the seat started without--remote; the bridge then retried against a thread it could not own, every few seconds, and the seat never received a message.spawn_wrapper=codex-shim.shincodex/type.conf, relative to the type's driver directory — and spawn resolves it throughagmsg_type_dirto a bundled absolute path. No PATH or function lookup is involved, which is how the bypass happened. Thecli=existence check is kept: the shim needs the real binary too.spawn_unset_envlist (Same-typeagmsg spawn(claude-code → claude-code) inherits the parent's session-identity env vars, breaks auth #294's mechanism), which now accepts a namespace entry:AGMSG_CODEX_*is enumerated fromenvat boot time and every match is unset, plus the two names outside it,AGMSG_REAL_CODEXandCODEX_THREAD_ID. Two of those controls are exported by the stack itself in normal operation, so inheritance is the ordinary case: codex-monitor exportsAGMSG_CODEX_BRIDGE=1just before exec'ing the bridged TUI (inherited, the shim execs the real binary at once), and the installed PATH wrapper exportsAGMSG_CODEX_SHIM_WRAPPER=1/AGMSG_CODEX_SHIM_SCRIPT_DIR(inherited, the bundled shim takes the parent's install dir, and a stale one reads as "not a monitor project"). Both fire ahead of any guard inside codex-monitor — found in review, one name per round, which is why the rule is a namespace rather than a list. Control flow of the generated script, in order:cdinto the project;export AGMSG_SPAWNED=1; the namespace loop and the literalunset; run the wrapper.codex/type.conf: readers are every$VAR/${VARin the stack's bash files (shim, monitor,_app-server.sh, bridge launcher,codex-record-session.sh,_session-start.sh,codex-shim-install.sh,_delivery.sh) and everyprocess.env.VARincodex-bridge.js, forAGMSG_*andCODEX_*; writers are the stack'sexportlines, the command-localAGMSG_REAL_CODEXthe shim hands to codex-monitor, and the generated wrapper. Cleared: the 19AGMSG_CODEX_*names in use today,AGMSG_REAL_CODEX,CODEX_THREAD_ID. Kept, each with its reason:AGMSG_SPAWNED(the seat's own marker), agmsg-wide configuration (AGMSG_BASH,AGMSG_WATCH_ONCE_*), bats-only hooks, and names that are script-local variables rather than environment inputs.env -i) was considered and rejected: the CLI needs the user's own auth, PATH, HOME, locale and terminal, which spawn cannot enumerate per CLI version, so a miss there breaks every spawn. A miss in the namespace rule — a control variable added outsideAGMSG_CODEX_*— is caught at review time by the reader-inventory test below; if the rule were bypassed at runtime the symptom would be the original one, a seat that looks spawned and never speaks.codex-monitor.sh's fail-open (exec_plain_codex, the internal fallback when the app-server cannot start) now fails closed whenAGMSG_SPAWNED=1— the marker every spawn boot script already exports. Fail-open exists for a person at the keyboard who reads the banner and keeps a working Codex; a spawned seat has nobody at its pane, and a plain Codex there is the same silent seat. The person's path is unchanged (its test is the control).Not changed
monitor/bothis a configured state, not the bypass, and is left as is.exec … --remotelines that fix(codex): open the TUI on the role's recorded thread, and let the bridge attach to it #960 (thread affinity) rewrites are not touched; this change lives inexec_plain_codexand in spawn.Verification
tests/test_spawn.bats: the boot script's CLI line starts with<skill>/scripts/drivers/types/codex/codex-shim.sh(read from the script, first non-assignment token) and contains no barecodexlaunch; making the shim non-executable turns spawn into a refusal that names the wrapper, and no boot script reaches the terminal;claude-codestill launches bareclaude; and end to end, running the boot script spawn wrote — with a fakecodexon PATH that records its argv and serves a fake app-server, the project inmonitormode, and a stub bridge launcher — reaches the real binary exactly once, ascodex --remote ws://127.0.0.1:<port> … actas ….tests/test_spawn.bats, inherited state — each seeded into the parent environment and the argv that reaches the real binary read back: the bridge family (AGMSG_CODEX_BRIDGE=1+ app-server/launcher markers + a parentCODEX_THREAD_ID);AGMSG_CODEX_SHIM_DISABLE=1alone; the installed-wrapper pair with a staleAGMSG_CODEX_SHIM_SCRIPT_DIR(nodelivery.shthere); and the resolution overrides (AGMSG_REAL_CODEX,AGMSG_CODEX_MONITOR_CMD, launcher/bridge commands) pointing at nonexistent paths. The namespace loop and the literalunsetare asserted to precede the wrapper line.tests/test_spawn.bats, unknown names: a control that does not exist yet (AGMSG_CODEX_FUTURE_KNOB=1) is absent from the environment the real binary receives, while an ordinary variable andAGMSG_SPAWNEDsurvive; replacing the namespace entry with today's explicit list turns exactly this test red. A reader-inventory test derives the stack'sAGMSG_*/CODEX_*reads the same way as above and fails on any name not covered by the manifest or the reasoned keep list.tests/test_codex_monitor.bats: withAGMSG_SPAWNED=1and an app-server that will not start, codex-monitor exits non-zero, starts no plain Codex, and says why; the identical failure without the marker still fails open (control).codex-shim.sh resume <uuid>,codex-shim.sh -m gpt-5).bats tests/test_codex_monitor.bats12/12,tests/test_codex_shim.bats25/25,tests/test_spawn.bats99/99 (full suite at the final head, run detached on a loaded host);.github/scripts/check-enforced-assertions.shandcheck-errexit-status-reads.shat baseline;bash -non the touched scripts.