* fix: route /codex:rescue through the Agent tool to stop Skill recursion (#234)
`/codex:rescue` previously combined two things that together caused a hang:
- `context: fork` in the frontmatter, which spawns a `general-purpose`
subagent for the command body.
- Body prose "Route this request to the `codex:codex-rescue` subagent."
without naming the transport.
When the main agent called `Skill(codex:rescue)` programmatically, the
fork resolved the ambiguous prose by trying `Skill(codex:codex-rescue)`
(unknown skill) and then falling back to `Skill(codex:rescue)`, which
re-entered this command and hung the session until the user cancelled.
No Codex job was ever created.
Naming the transport as `Agent(codex:codex-rescue)` alone is not enough:
forked general-purpose subagents do not expose the `Agent` tool, so the
forked runner cannot reach the subagent that way either. The minimal fix
is therefore two coordinated changes:
- Drop `context: fork` so the command body runs inline in the calling
agent's context, where `Agent` is in scope.
- Say explicitly "use the `Agent` tool with `subagent_type:
"codex:codex-rescue"`", and call out that `Skill(codex:codex-rescue)`
and `Skill(codex:rescue)` are not valid routing paths. Add `Agent`
to `allowed-tools` so the call does not prompt for permission.
Everything else in rescue.md (resume-candidate check, flag handling,
background/foreground semantics, operating rules) is unchanged. The
`codex:codex-rescue` subagent itself is unchanged.
Tests pin the new allow-list, the explicit `subagent_type`, the ban on
`Skill(codex:codex-rescue)`, and the absence of `context: fork`. The
existing "run the `codex:codex-rescue` subagent in the background"
assertion continues to hold since that sentence still reads correctly
with the Agent-tool transport.
Fixesopenai/codex-plugin-cc#234
* test: match quoted result and cancel command arguments
---------
Co-authored-by: Dominik Kundel <dkundel@openai.com>
Unquoted \$ARGUMENTS in the ! shell commands allowed shell metacharacters
in user-supplied job IDs to be expanded before Node received them (e.g.,
`task-123; malicious-cmd` would execute the trailing command). This is
inconsistent with review.md and adversarial-review.md, which both wrap
"$ARGUMENTS" in double quotes.
Co-authored-by: claude[bot] <claude-bot@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
* fix: declare model in codex-rescue agent frontmatter
The codex-rescue agent had no model field, leaving Claude Code to assign
whatever default it chooses. As a thin forwarding wrapper that issues a
single Bash call, this agent is well-suited to the haiku tier; declaring
it explicitly ensures a predictable cost profile and tier guarantee.
Co-Authored-By: Claude Code <noreply@anthropic.com>
* Update codex-rescue.md
---------
Co-authored-by: claude[bot] <claude-bot@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Dominik Kundel <dkundel@openai.com>
* fix: gracefully handle unsupported thread/name/set on older Codex CLI
Codex CLI v0.118.0 does not recognize the thread/name/set JSON-RPC method,
causing startThread() to throw. Thread naming is cosmetic (for job log
labels) and should not block thread creation. Wraps the call in try/catch
so it fails silently on older CLI versions.
Fixes#119
* refactor: only suppress unsupported-method errors for thread/name/set
Address Codex review feedback: the bare catch swallowed all errors
including auth, network, and server failures. Now only suppresses
errors containing 'unknown variant' or 'unknown method' (the specific
error older CLI versions return) and rethrows everything else.
`SpawnedCodexAppServerClient.initialize()` passes `this.options.env` to
`spawn()`, but no caller ever sets `env` in options. In Node.js, passing
`undefined` for `env` gives the child process **no** environment variables,
breaking any model provider that relies on env vars (e.g. DATABRICKS_TOKEN).
Fall back to `process.env` when `this.options.env` is not set, matching the
existing pattern in `broker-lifecycle.mjs` and `codex-companion.mjs`.
Co-authored-by: Isaac
Co-authored-by: Bhuvanesh Sridharan <bhuvanesh.sridharan@databricks.com>
* fix: make test suite portable across platforms
Replace hardcoded macOS path with fileURLToPath for cross-platform
compatibility. Add .cmd wrapper creation and platform-aware PATH
separator in test fixtures so the fake codex binary is discoverable
on Windows. Add shell and windowsHide options to the test helper
run() function to match production behavior.
Test results on Windows improve from 12/64 pass to 59/64 pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: skip shell for absolute-path commands to avoid Windows space-in-path breakage
When `process.execPath` resolves to a path with spaces (e.g.,
`C:\Program Files\nodejs\node.exe`), `shell: true` causes cmd.exe
to split the path at the space. Guard with `path.isAbsolute()` so
only bare command names (which need `.cmd` shim resolution) use the
shell.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Dominik Kundel <dkundel@openai.com>
* fix: add shell and windowsHide options for Windows spawn in app-server
On Windows, spawn("codex", ["app-server"]) fails with ENOENT because
Node.js cannot resolve .cmd shims without shell: true. This adds
platform-gated shell and windowsHide options to the app-server spawn
call, and uses terminateProcessTree for proper process tree cleanup
since shell: true wraps the child in cmd.exe.
Without terminateProcessTree, plain SIGTERM only kills cmd.exe and
leaves the actual codex node process orphaned — verified with 274+
zombie node.exe processes accumulating on Windows.
Fixes#32Fixes#46
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: guard terminateProcessTree against PID reuse after process exit
ChildProcess.killed only reflects whether .kill() was called by this
process — it stays false when the child exits on its own. On Windows,
where PIDs are recycled quickly, the 50 ms timer could fire after
cmd.exe has exited and its PID has been reassigned, causing taskkill
to terminate an unrelated process.
Add an exitCode === null check so the tree-kill path is skipped once
the child has already exited.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On Windows, Node.js `spawnSync` without `shell: true` uses
`CreateProcess`, which only resolves `.exe` files. npm installs global
tools (like `codex`) as `.cmd` shims, so `spawnSync("codex", ...)`
returns ENOENT even when codex is correctly installed and on PATH.
Adding `shell: process.platform === "win32"` routes through `cmd.exe`
on Windows, which properly resolves `.cmd`, `.bat`, and PATHEXT entries.
No behavior change on macOS/Linux since the condition evaluates to false.
