fix: pass on-request approval policy for write-capable task runs#426
fix: pass on-request approval policy for write-capable task runs#426mvanhorn wants to merge 1 commit into
Conversation
rajpratham1
left a comment
There was a problem hiding this comment.
This is a focused bug fix with good test coverage.
What's good
Fixes the root cause by forwarding approvalPolicy to both:
startThread()
resumeThread()
Keeps behavior unchanged for read-only tasks (never).
Enables the intended behavior for write tasks (on-request).
Adds regression tests for:
new write thread
read-only thread
resumed write thread
Updates the fake Codex fixture so tests validate the actual parameters being sent.
Code quality
The implementation is small and consistent:
approvalPolicy: options.approvalPolicy,
sandbox: options.sandbox,
is now passed in both thread creation and resume paths, eliminating the inconsistency.
The companion also correctly maps:
request.write
to
approvalPolicy = on-request
sandbox = workspace-write
which matches expected behavior.
Tests
The new tests verify exactly what the fix intends:
✅ write task → on-request
✅ read-only task → never
✅ resume write task → on-request
This provides good regression coverage.
Summary
Write-capable task runs now send
approvalPolicy: "on-request"to the app server, so--writeactually gets a writable sandbox. Previously every run sentnever, and the Codex CLI silently downgrades aworkspace-writesandbox to effectively read-only underapproval_policy=never.Why this matters
The reporter of #412 isolated this with a plugin-free repro (Windows 11, Codex CLI 0.142.4, plugin 1.0.5):
The plugin only toggled
sandboxbetweenread-onlyandworkspace-writeand hardcodedapprovalPolicy: "never"everywhere, so even a trivial "create test.txt" rescue task failed with a read-only sandbox error. Two threading gaps caused it:runAppServerTurninplugins/codex/scripts/lib/codex.mjsbuilt itsstartThread/resumeThreadoptions withoutapprovalPolicy(sobuildThreadParams' existingoptions.approvalPolicy ?? "never"could never see a value), andexecuteTaskRunincodex-companion.mjsnever passed one.Changes
approvalPolicyis forwarded through bothrunAppServerTurnbranches (start and resume), andexecuteTaskRunsets it from the run mode:on-requestfor--write(including--resume-lastresumes),neverfor read-only runs - reviews and other read-only paths are behaviorally unchanged.Testing
node --test tests/runtime.test.mjs: new assertions verify the fake app-server recordsapprovalPolicy: "on-request"+sandbox: "workspace-write"on write runs,never+read-onlyon read-only runs, andon-requeston the--resume-last --writeresume path. Full suite matches the baseline commit exactly in a clean checkout (the only failures in my environment are three pre-existing status/result tests that also fail on the base commit due to a live shared app-server on this machine).Fixes #412