feat: add --sandbox <mode> to task and /codex:rescue - #742
Conversation
`task` always mapped `--write` to `workspace-write` and everything else to `read-only`, so a rescue run could never ask for `danger-full-access` even when the work needs it (test tooling that writes outside the repository, or network access). Add an explicit `--sandbox <read-only|workspace-write| danger-full-access>` flag: - the flag wins over `--write`; `--write --sandbox read-only` is rejected - the resolved mode is stored in the job request, so the detached worker and `--resume-last` reuse exactly what the caller asked for - jobs started with a writable sandbox keep the review hints that `--write` jobs already get - older stored requests without a sandbox field keep the old mapping The fake app-server fixture now records the `thread/start` and `thread/resume` params so tests can assert what reached Codex. The rescue command, agent and runtime skill forward `--sandbox` as a runtime flag and never add `danger-full-access` on their own. Closes openai#145 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… a stored request without a sandbox field Measured on Codex CLI 0.153.2: the app-server keeps the sandbox a thread was started with when it is resumed, whichever mode the new request names on thread/resume. Say so in the README instead of the opposite, and keep the resume test to what the plugin forwards. Add a task-worker test that replays a job record written before this change, with no sandbox field in the request, and checks it still starts the thread with the --write mapping. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the pair A rescue request such as `--sandbox read-only fix the failing test` reads as write intent, and the agent that forwards it may still add --write; a hard error there returns nothing to the user. The explicit flag now wins: `--write --sandbox read-only` starts a read-only thread. Say so in the README and the runtime skill, and scope `--sandbox` to /codex:rescue in the README, since the review commands stay read-only. The rescue command carries the same "never add danger-full-access yourself" rule as the agent and the skill. The fake app-server records only the thread params the tests assert. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Measured on Codex CLI 0.153.2: the mode named on thread/resume is applied when the app-server loads the thread again from disk, and ignored while the thread is still live in the app-server. The plugin keeps one shared app-server per session, so inside a session a resumed rescue keeps the sandbox it started with. The README says exactly that. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 3d7b5247c8
ℹ️ 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".
…hat asks for another sandbox Two findings from the Codex review of this branch. The argument parser recognises flags anywhere in the request, so a task whose text merely mentioned `--sandbox danger-full-access` ran without a sandbox and with the words stripped from the prompt. `task` now takes `--sandbox` only from the leading option block; once the task text has begun the same words stay part of the prompt. Other options keep their existing behaviour. A thread the shared app-server still holds keeps its sandbox on resume, so forwarding a different mode on `thread/resume` produced a turn with broader or narrower access than requested, silently. The job record now carries the sandbox a thread was started with, and `--resume-last` refuses a request whose sandbox differs from it, naming `--fresh` and the matching `--sandbox` as the ways out. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91cc73bb30
ℹ️ 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".
…ext --sandbox in the prompt, validate the whole inline value Three findings from the second Codex review of this branch. The shared app-server answers thread/resume with the sandbox the thread actually has (measured on Codex CLI 0.153.2: a live danger-full-access thread answers dangerFullAccess to a read-only request). runAppServerTurn now compares that answer with the requested mode before turn/start and refuses a mismatch, so a resume is checked twice: against the job record before connecting, and against the server's own answer when the record is missing or predates the sandbox field. The fake app-server keeps a sandbox per thread and echoes it the same way. The rescue command, agent and skill told the wrapper to treat any --sandbox as a runtime flag, which would have hoisted one out of the task text and defeated the leading-only parser. They now say: forward the request in the user's order; a leading --sandbox is a flag, one inside the task text stays there as prompt text. extractLeadingSandbox split the inline value at the second "=", so --sandbox=danger-full-access=false passed as danger-full-access. It now splits at the first "=" and validates the whole remainder. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3803d2935
ℹ️ 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".
… as the first token The runtime skill said "starts with --sandbox", which would have made the forwarder miss `--model spark --sandbox danger-full-access run tests` or a request with --resume prepended, leaving the tokens in the prompt while it added its default --write. It now matches the command, the agent and the parser: any --sandbox before the task text is the control. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89792b541c
ℹ️ 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".
`--sandbox=` and a trailing `--sandbox` produced an empty value that normalized to null and silently fell back to the --write mapping. The flag now needs one of the documented modes whenever it is present. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3e03cc77c
ℹ️ 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".
extractLeadingSandbox already normalizes the argv; parseCommandInput
normalized it again, and when the leading options left one positional
token the second pass re-split it and reinterpreted its quotes and
backslashes. handleTask now hands the already tokenized argv to parseArgs
directly, so `task "--sandbox read-only '{\"key\":\"value\"}'"` sends
the JSON as typed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c462cedab4
ℹ️ 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".
A job record written before this change has no sandbox field, and its write flag is not evidence of the thread's sandbox: the shared app-server ignored sandbox changes on live resumes, so a legacy --write resume of a read-only thread was recorded as write-capable while the thread stayed read-only. When only such a record remained, the job-record check rejected the correct read-only request before the response-based check could run, and the thread could not be resumed at all. threadStartSandbox now reads only records with an explicit sandbox field, which every run since this change writes and which the checks guarantee matched the thread. Without one it returns null and the thread/resume response decides. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…b-record check After the read-only resume succeeded it wrote a record with an explicit sandbox, so the following --write resume is refused before connecting, not by the thread/resume answer. The test asserted the wrong message. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r and a job-record pre-flight Six review rounds had each added a patch on top of the last. This commit removes what they accumulated and keeps the behaviour: - parseArgs gains `leadingOnlyOptions`: an option in that set is recognised only until the first positional, and afterwards stays in the positionals. `task` lists `--sandbox` there, so the hand-rolled extractLeadingSandbox pre-parser, its option tables and the second tokenization pass go. Inline values are now split at the first "=" for every option. - normalizeSandboxMode treats a present-but-empty value as an error. - The resume check is the app-server's own answer on thread/resume, and only that: the job-record pre-flight, threadStartSandbox and the sandbox field on job records go, together with the edge cases they had (legacy records, pruned records, refused runs). A cold-loaded thread that takes the new mode is no longer refused. - The rescue command, agent, skill and README each say the rule once. Tests cover the same behaviour with two fewer cases; 98 pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d6c7d9e04
ℹ️ 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".
…ot trusted thread/resume can report a policy outside the three modes the flag names (externalSandbox today, anything added later). The check treated that as verified; it now refuses with the reported type and names --fresh, since the requested boundary cannot be shown to hold. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 959b0ec4e8
ℹ️ 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".
Splitting inline values at the first "=" left `--write=false=x` with the value `false=x`, which the boolean branch read as true. A boolean option now accepts no value, `true` or `false`, and anything else is an error, instead of either silently enabling or silently disabling the flag. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
Why
taskdecides the Codex sandbox from one boolean:--writemeansworkspace-write, anything else meansread-only. There is no way to ask fordanger-full-access, which #145 requests and which a rescue run needs whenever the work has to write outside the repository or reach the network (test tooling, package installs, pushing to a remote as in #304). The same hard-coded mapping is what #482, #240 and #505 run into from the configuration side.This PR adds the explicit flag. It composes with the config-driven proposals (#646, #613, #575): an explicit flag should win over configuration, so nothing here has to change if those land.
What changes
task --sandbox <read-only|workspace-write|danger-full-access>--write;--writestays shorthand forworkspace-write. Unknown or empty values are rejected with the accepted list, matching--effort.parseArgsgainsleadingOnlyOptions: an option in that set is recognised until the first positional and afterwards stays in the positionals, so a request whose text merely mentions--sandbox danger-full-accesskeeps those words in the prompt and runs with the default sandbox. Inline values are now split at the first=for every option (--sandbox=danger-full-access=falseis rejected), and a boolean option accepts onlytrueorfalseinline (--write=false=xis an error rather than silently true or false).sandboxfield and keeps the old--writemapping.writeflag, which drives the "Review changes" hints, now means "the sandbox allows writes", so--sandbox workspace-writeanddanger-full-accessget the hints--writealready gets.thread/resumeapplies only when the app-server loads the thread from disk; a thread still live in the plugin's shared app-server keeps its sandbox, in both directions (thread/resumesandbox is ignored while the thread is still live in the shared app-server, sotask --resume-last --writecannot write after a read-only run #740, with the response echoes and the turn results). The app-server reports the sandbox the thread actually has in thethread/resumeresponse (a live full-access thread answersdangerFullAccessto a read-only request), sorunAppServerTurncompares that answer with the requested mode and refuses beforeturn/start, naming the matching--sandboxand--freshas the ways out; a policy it cannot map to a mode (externalSandbox, or a missing field) is refused the same way. Before this change the plugin forwarded the new mode and ran with whatever the live thread had.--sandbox, the plugin still pinsread-onlyorworkspace-writerather than deferring toconfig.toml. Hardcoded sandbox values always override config.toml sandbox_mode #482, Plugin overrides Codex sandbox config and can trigger bwrap failures #240, Plugin runtime ignores sandbox_mode from config.toml; unusable on kernels with apparmor_restrict_unprivileged_userns=1 (bwrap loopback failure) while direct codex exec works #505 and Resolve the task sandbox from config.toml and add task --read-only #646 are about that default, and this flag is the "alternative shape" Hardcoded sandbox values always override config.toml sandbox_mode #482 names; it does not close them./codex:rescuecommands/rescue.md:--sandboxin the argument hint; forward the request in the user's order, never add a--sandbox, a--sandboxinside the task text is prompt text.agents/codex-rescue.mdandskills/codex-cli-runtime/SKILL.md: the same rule, and do not add--writewhen the user passed--sandbox.danger-full-accessmeans, and the resume rule.Tests
tests/fake-codex-fixture.mjskeeps a sandbox per thread, records thethread/startandthread/resumeparams (lastThreadStart,lastThreadResume), and echoes the thread's sandbox in thethread/resumeresponse the way the real app-server does.tests/runtime.test.mjs, eight tests: default and--writemapping;--sandboxforwarding (including case normalisation);--sandboxread only before the task text (the single raw-string shape the rescue skill uses, split tokens, a quoted JSON prompt kept verbatim); unknown, suffixed and empty values rejected with no thread started, and--sandbox read-onlywinning over--write; a resume refused on the app-server's answer in both directions, and on a policy the plugin cannot compare; background worker persistence via the stored job request; a pre-change stored request replayed bytask-workerwith the old mapping (mutation-tested: removing the fallback fails it).tests/commands.test.mjs: the new doc rules, in the existing style.Not in scope
reviewandadversarial-reviewstay read-only; the docs promise that. feat #167: add --sandbox flag and CODEX_SANDBOX env var for review and task commands #226 covers them if wanted.CODEX_SANDBOXenvironment variable (feat #167: add --sandbox flag and CODEX_SANDBOX env var for review and task commands #226) and noconfig.tomlresolution (Resolve the task sandbox from config.toml and add task --read-only #646). Both are compatible with this flag./codex:statuscannot yet show which run was unsandboxed; Capture resolved model/effort/sandbox in job records #645 captures the resolved sandbox from the app-server response and composes with this.Verification
npm test: 99 tests, 99 pass, 0 fail (node 22.17.0);npm run build: exit 0.--sandbox read-onlycould not create a file in the workspace;--sandbox danger-full-accesswrote a file under$HOME, outside the workspace;--resume --sandbox danger-full-accessappended to it on the resumed thread. While that thread stayed live in the shared app-server,--resume --sandbox read-onlystill wrote and thethread/resumeresponse echoeddangerFullAccess; after the app-server was restarted, the same thread resumed with--sandbox read-onlywas denied the write and the response echoedreadOnly(thread/resumesandbox is ignored while the thread is still live in the shared app-server, sotask --resume-last --writecannot write after a read-only run #740).Note for anyone running the suite from inside a Claude Code session: the tests inherit the environment. With
CLAUDE_PLUGIN_DATAset, four pre-existingresolveStateDirtests fail, and withCODEX_COMPANION_SESSION_IDset, three status/result tests fail because session filtering hides their fixture jobs. Runenv -u CLAUDE_PLUGIN_DATA -u CODEX_COMPANION_SESSION_ID npm test. Neither is related to this change.Closes #145. Related: #304, #482, #240, #505, #740. Earlier attempts at the same flag: #226 (with env var, review included), #147 (
--full-access, where tests were requested; this PR carries them). #739 adds a separate--networkflag on the same lines ofhandleTask; the two compose and whichever lands second needs a small rebase.🤖 Generated with Claude Code