fix(rescue): forbid node-based prompt writes and pgrep wait loops - #692
fix(rescue): forbid node-based prompt writes and pgrep wait loops#692tyoon10 wants to merge 5 commits into
Conversation
6326c38 to
a793db6
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6326c385dd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - Leave model unset by default. Add `--model` only when the user explicitly asks for one. | ||
| - Map `spark` to `--model gpt-5.3-codex-spark`. | ||
| - Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. | ||
| - Default to a write-capable Codex run in `codex:codex-rescue` unless the user explicitly asks for read-only behavior. |
There was a problem hiding this comment.
For rescue requests that ask only for review, diagnosis, or research without edits, this change now instructs the runtime skill to use a write-capable run unless the user explicitly says “read-only,” even though agents/codex-rescue.md still directs the same subagent to omit --write for those request types. Because the companion selects workspace-write solely from that flag, the contradictory contracts can grant edit capability to a review-only request; restore the non-editing exemptions here or update both contracts consistently if the permission expansion is intentional.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Restored the non-editing exemptions in the runtime skill so both agents/codex-rescue.md and skills/codex-cli-runtime/SKILL.md now keep --write off for review/diagnosis/research-only requests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93e7e8d486
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| Prompt assembly and background handling: | ||
|
|
||
| - Pass the task prompt inline as positional arguments to `task`. Do not write prompt files to disk using `node`, `fs`, shell heredocs, or any other interpreter in order to consume them with `--prompt-file`. |
There was a problem hiding this comment.
-- before inline prompt arguments
For a read-only request whose task text mentions a supported option, such as “diagnose why --write changes the sandbox,” following this instruction with task "<prompt>" can silently enable writes and remove that token from the prompt. normalizeArgv splits a sole quoted argument and parseArgs recognizes options anywhere before a -- delimiter, so the inline-prompt contract must require task [runtime options] -- "<prompt>" to keep task text from becoming runtime control flags.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in f9862c5 — the contract now requires task [runtime options] -- "<prompt>" in both agents/codex-rescue.md and skills/codex-cli-runtime/SKILL.md.
Confirmed the mechanism: with a sole quoted argument normalizeArgv runs it through splitRawArgumentString, and parseArgs treats any --write / --model token before a -- delimiter as a runtime option, so it both flips the flag and drops the token from the positionals that readTaskPrompt joins into the prompt. With the delimiter, parseArgs sets passthrough and every remaining token is preserved verbatim as prompt text.
| Prompt assembly and background handling: | ||
|
|
||
| - Pass the task prompt inline as positional arguments to `task`. Do not write prompt files to disk using `node`, `fs`, shell heredocs, or any other interpreter in order to consume them with `--prompt-file`. | ||
| - Do not poll, `pgrep`, `watch`, `tail` logs, or run wait loops for a background task. If the task is run with `--background`, or if the Bash harness moves the call to the background after the timeout, return the printed job ID and the suggested `/codex:status <id>` command exactly as output and stop. |
There was a problem hiding this comment.
When the Bash harness backgrounds a foreground task call after its timeout, the companion never executes its explicit options.background branch, which is the only path that renders the task-* job ID and /codex:status suggestion. The harness ID is not a companion job ID, so this instruction either asks the subagent to return output that does not exist or produces a status command that cannot resolve the job; this fallback should explicitly launch task --background or avoid promising a companion ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right — fixed in f9862c5. Only the options.background branch in handleTask reaches renderQueuedTaskLaunch, so a foreground call that the Bash harness backgrounds after its timeout never prints a task-* ID, and the harness shell ID does not resolve in /codex:status.
The job-ID/status instruction is now scoped to --background runs only, and a separate rule states that a harness-backgrounded foreground call has no companion job ID: return the harness output as-is, do not invent an ID or suggest a /codex:status command for it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22fc0b4ba5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| Prompt assembly and background handling: | ||
|
|
||
| - Pass the task prompt inline, as the last arguments and after a `--` delimiter: `task [runtime options] -- "<prompt>"`. Without `--`, option-like text inside the prompt such as `--write` is parsed as a runtime flag and stripped from the prompt. |
There was a problem hiding this comment.
For rescue text containing shell syntax such as $(command), backticks, $VAR, or embedded double quotes, inserting the raw prompt into the shown "<prompt>" Bash argument allows expansion—or even quote termination—before the companion receives it; command substitutions can therefore execute locally and the forwarded prompt is corrupted. Since the following rule now forbids the prompt-file transport, specify a robust shell-escaping strategy or use a non-shell argument channel rather than directing the forwarder to interpolate arbitrary task text into double quotes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a2eec3f. The contract now mandates single-quoting instead of double-quoting, in both agents/codex-rescue.md and skills/codex-cli-runtime/SKILL.md:
- The shown form is
task [runtime options] -- '<prompt>'. - Embedded single quotes must be escaped as
'\''(don't stop→'don'\''t stop'), which is the standard closed-quote/escaped-quote/reopen-quote idiom and needs no other escaping. - Double quotes are explicitly forbidden, with the reason spelled out:
$(...), backticks,$VAR, and a bare"would expand or terminate the argument locally before the companion receives the text.
Single quotes suppress all Bash expansion, so with that escape rule the prompt reaches codex-companion byte-for-byte and no command substitution can run locally — without reintroducing the prompt-file transport that the following rule forbids.
Test coverage in tests/commands.test.mjs was updated to assert the single-quoted form and the escaping/no-double-quote rule for both files. node --test tests/commands.test.mjs (9/9) and npx tsc -p tsconfig.app-server.json pass.
a2eec3f to
36ff14a
Compare
1 participant
Closes #553 and #686.
Summary
codex:codex-rescueis defined as a thin forwarder, but the current prompt leaves two failure modes unaddressed that show up in subagent transcripts:Node-based prompt-file writes (codex-rescue: node-based file writes for prompt assembly trip the auto-mode bypass classifier #553). The rescue subagent has assembled prompts by shelling out to
node -e "... fs.writeFileSync ..."and then passing--prompt-file. That pattern is indistinguishable from shell indirection used to route around hooks and has triggered false-positive auto-mode bypass warnings. This change explicitly forbids writing prompt files to disk withnode,fs, shell heredocs, or any other interpreter; prompts must be passed inline as positional arguments.pgrepwait loops for background tasks (codex-rescuecan generate duplicatepgrep -f "codex-companion.mjs"wait loops that keep each other alive on macOS (stuck background tasks) #686). When a foregroundtaskcall is auto-backgrounded by the Bash harness, the model has improvisedwhile pgrep -f "codex-companion.mjs" ...wait loops. These loops match each other and stay alive indefinitely on macOS. This change explicitly forbids polling,pgrep,watch,tail, or any wait loop, and tells the subagent to return the printed job ID and the suggested/codex:status <id>command exactly as output.Changes are in
agents/codex-rescue.mdandskills/codex-cli-runtime/SKILL.md.tests/commands.test.mjsnow asserts both prohibitions and the expected fallback behavior for each.