fix: keep rescue Bash attached to Codex - #738
Conversation
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".
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".
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e5d8e786b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82279bbe5b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8574b097fb
ℹ️ 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".
|
Lifecycle contract clarification requested in review: once a background rescue has created a durable |
|
@codex review Please review the current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1c63b00fe
ℹ️ 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".
|
@codex review Please review current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54cd28d96d
ℹ️ 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".
|
@codex review Please review current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35de31d58d
ℹ️ 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".
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@openai/developer-experience — contributor-side work on this PR is complete at exact head |
Summary
Fixes #432 and #324.
This PR makes background
/codex:rescuedurable without requiring one Bash invocation to remain attached for the whole Codex run. The outer rescue layer owns the user-facing execution choice; a background rescue delegates to the companion's detached task runtime and then acts as a bounded waiter/reader.Current behavior:
task --background --jsonand captures a durablejobId;status <jobId> --wait --timeout-ms 60000 --json;result <jobId> --raw, preserving Codex stdout verbatim;task-workerruns only if the stored job is stillqueued, preventing a pre-start cancellation from being revived;jobIdexists, interruption/cancellation of the waiting Agent does not implicitly cancel the detached Codex job; it remains recoverable through status/result until completion or an explicit/codex:cancel;spawnDetachedTaskWorker, the parent performs no mutable job-file,state.json, or PID-sidecar read/write. This avoids racing a worker that may truncate/rewrite its job JSON while starting;process.pidbefore its first queued-state read;/codex:cancelclaims terminalcancelledstate before PID discovery, then terminates/removes the worker PID and reasserts the same terminal record afterward, closing the startup interleavings where cancellation could otherwise report success while a worker continued;finally, cancellation removes it, and state pruning removes orphaned sidecars for dropped jobs.This incorporates the lifecycle/race issues found during review rather than preserving the original foreground-only inner-Bash design.
Validation
Fresh deterministic validation at head
69484a7:node --check plugins/codex/scripts/codex-companion.mjs: PASS;git diff --check: PASS.An earlier combined affected-set run on the previous head hit the E2E's 15-second status window under host load; later isolated and full deterministic runs passed. That failed timing run is not represented as PASS.
The upstream Pull Request CI for this branch is checked per head. If the current head reports
action_requiredwith 0 jobs created, that is treated as an external workflow-approval/action gate, not a CI test result.Current scope
GitHub currently reports 8 changed files. The changes cover rescue routing/contracts, durable task runtime/state handling, result rendering behavior, documentation, and regression coverage.