fix: keep rescue Bash attached to Codex - #738
Conversation
![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: 19810b45ba
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Use the single `Bash` call in the foreground. Never set `run_in_background` on the Bash call. | ||
| - Call `task` without `--background` so the Bash call stays attached until Codex returns its final stdout, even when this subagent itself is running in the background. |
There was a problem hiding this comment.
When a Codex rescue exceeds Claude Code's foreground Bash limit, this mandatory foreground call is terminated even if the user selected --background: the Bash tool defaults to a 120-second timeout and caps an explicit timeout at 600,000 ms. The runtime's task --background path is the mechanism that detaches a durable worker, but these lines now prohibit it; because this agent also returns nothing on Bash failure, long tasks—the exact case README.md recommends backgrounding—can silently lose their final result. Keep a detached runtime path for background rescues and retrieve its result instead of holding one foreground Bash call open indefinitely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76f876af2d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Never set `run_in_background` on any Bash call. | ||
| - For a forwarded `--background` request, use the durable companion path: first run a foreground Bash call with `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task --background --json ...` and read `jobId` from its JSON. | ||
| - While that job is `queued` or `running`, use foreground Bash calls to `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status "$jobId" --wait --timeout-ms 60000 --json`. Each wait is bounded so no Bash call stays attached for the whole Codex run. | ||
| - When the background job is terminal, run `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result "$jobId"` in a foreground Bash call and return that stdout exactly as-is. |
There was a problem hiding this comment.
When a completed background job has a thread ID, this plain result call does not return Codex's output verbatim: renderStoredJobResult appends Codex session ID and Resume in Codex lines to the stored raw output (plugins/codex/scripts/lib/render.mjs:405-410). This makes --background violate the command's explicit verbatim-output contract and behave differently from the foreground path; use a raw/JSON result mode and emit only the stored Codex response.
Useful? React with 👍 / 👎.
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
The updated ownership model makes sense now that background rescue has to survive the host Agent lifecycle: durability belongs to the companion job, while the Agent becomes a waiter/reader instead of trying to keep the Bash process alive. The PDEATHSIG regression is much stronger than a mocked child-process assertion.
One lifecycle edge I would still pin explicitly is cancellation or interruption of the waiting Agent after the detached job has been created. Does that intentionally leave the Codex job running for a later status / result, or should it cancel it? Either behavior can be valid, but it matters because this design deliberately decouples worker lifetime from the Agent.
2 participants
Summary
Fixes #432 and #324.
The current
/codex:rescuecommand already owns execution mode at the Agent layer:--backgroundbackgrounds the rescue subagent, while--wait/ default runs it in the foreground. However, the rescue subagent still contained an independent heuristic telling itself to prefer background execution for complicated work.That leaves the single inner Bash call free to use Claude Code's
run_in_backgroundbehavior. In that mode the Bash-owned companion/process tree can be reaped independently, and the thin forwarding subagent can return only a background-job acknowledgement instead of Codex's final output.This patch makes the ownership boundary explicit:
/codex:rescuecommand is the only layer that decides background vs foregroundtaskcall never receives--backgroundNo runtime implementation changes are needed:
taskwithout--backgroundalready executes synchronously throughrunForegroundCommand.Current-vs-historical behavior
Part of #324 predates the current rescue routing. The present command already backgrounds the Agent itself rather than requiring the task runtime to own that choice. The remaining defect on current
mainis the contradictory subagent guidance that can background the Bash call inside that Agent. This PR fixes that current, still-reproducible boundary rather than reintroducing the historical--waitdesign.Validation
Against current
main(db52e28):prefer background executionheuristictests/commands.test.mjs: 8 passed, 0 failedprefer background executionguidancegit diff --check: passFresh collision checks immediately before publication found no open PR for #432 or #324 implementing this execution-boundary fix.
Scope
Four files only:
plugins/codex/agents/codex-rescue.mdplugins/codex/commands/rescue.mdplugins/codex/skills/codex-cli-runtime/SKILL.mdtests/commands.test.mjsThis intentionally does not add polling, a second process supervisor, or a new task execution mode.