fix(codex): require human approval for app-server commands#103457
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afc01d916c
ℹ️ 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".
| threadId: thread.threadId, | ||
| turnId, | ||
| nativeHookRelay, | ||
| execPolicy, | ||
| execReviewerAgentId: sessionAgentId, | ||
| internalExecAutoReview: appServer.approvalsReviewer === "user", | ||
| autoApprove: shouldAutoApproveCodexAppServerApprovals(appServer), |
There was a problem hiding this comment.
Force Codex reviewer to user for auto mode
When tools.exec.mode=auto uses a trusted model, resolveCodexAppServerRuntimeOptions still sets approvalsReviewer: "auto_review" and buildTurnStartParams forwards that to Codex, while Codex's app-server README documents auto_review as routing approvals to a prompted subagent before the client reviews them. In that configuration, low-risk command approvals can be decided by Codex before this handler is invoked, so removing OpenClaw's internal reviewer here does not actually require the human approval route for those app-server commands; force the app-server reviewer to user (or otherwise disable native auto-review) for this path if the approval must be human-bound.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…103457) * fix(codex): require human approval for app-server commands * docs(exec): clarify Codex approval fallback
Closes #103427
What Problem This Solves
Fixes an issue where Codex app-server command approvals could be accepted by the configured exec reviewer even though Codex does not expose an enforceable resolved executable. A low-risk review decision therefore was not bound to the executable that Codex would run.
Why This Change Was Made
Removes the Codex app-server-only internal exec reviewer and its caller plumbing. Explicit runtime/native policy decisions remain intact; otherwise the existing plugin approval route asks a human. The gateway and node exec reviewers remain unchanged because those paths enforce resolved execution plans.
User Impact
Codex app-server commands that previously could receive a low-risk automatic review now wait for human approval unless explicit runtime or native policy already decided the request. This trades a small amount of convenience for an enforceable approval boundary.
Evidence
crimson-hermit:pnpm test extensions/codex/src/app-server/approval-bridge.test.ts— 78/78 passed on final rebased head.crimson-hermit:pnpm check:changed— extension production/tests, docs, typechecks, lint, guards, and import-cycle checks passed on the same functional patch before the final documentation-only clarification/rebase.