fix: auto-review write-capable task escalations#494
Conversation
Keep read-only delegation fail-closed while allowing write-capable tasks to request Guardian-reviewed escalation on new and resumed threads. Builds on openai#426. Refs openai#445.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 026d6c45a0
ℹ️ 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".
| approvalPolicy: request.write ? "on-request" : "never", | ||
| approvalsReviewer: request.write ? "auto_review" : undefined, |
There was a problem hiding this comment.
Verify auto-review support before enabling approvals
When --write is used with a Codex app-server version that does not honor approvalsReviewer (or rejects the new field), this still switches the run to approvalPolicy: "on-request". This client has no fallback approval UI—plugins/codex/scripts/lib/app-server.mjs responds to every server-initiated request with -32601—so any escalation such as blocked network or an out-of-workspace write will fail instead of being auto-reviewed. Please gate this on a supported start/resume response or fail early with an upgrade message before selecting on-request.
Useful? React with 👍 / 👎.
Summary
approvalPolicy: "on-request",approvalsReviewer: "auto_review", andsandbox: "workspace-write"for write-capable task runsapprovalPolicy: "never", no automatic reviewer, andsandbox: "read-only"Relationship to #426 and #445
This deliberately builds on @mvanhorn's approved #426. It includes that PR's
approvalPolicyplumbing and regression coverage, then extends the same path with theapprovalsReviewerfield requested in #445. Credit for the underlying write-policy fix belongs to @mvanhorn.If maintainers prefer to keep #426 as the landing PR, this delta can be folded into it; this PR is intended as a cooperative extension or replacement, not a competing implementation.
Security boundary
auto_reviewis an escalation approval gate. It does not selectdanger-full-access, bypass app-server approval handling, or prove that approved writes remain inside the workspace. Read-only delegation remains unchanged.Verification
node --test --test-name-pattern "write task output focuses|read-only task keeps never|resume-last --write forwards" tests/runtime.test.mjs— 3/3 passedcodex app-server generate-ts --out plugins/codex/.generated/app-server-types— passed with Codex CLI 0.144.0; generated start/resume types includeapprovalsReviewernpx --no-install tsc -p tsconfig.app-server.json— passednpm ci --ignore-scripts— 0 vulnerabilitiesgit diff --check— passedtests/runtime.test.mjs— 58/63 passed; the five failures are unrelated Windows/environment constraints: symlink privilege (1), temp transcripts outside the configured Claude projects root (3), andtaskkillprocess termination (1). All changed policy-path tests passed.Fixes #412
Closes #445