Skip to content

fix(codex): require human approval for app-server commands#103457

Merged
steipete merged 2 commits into
mainfrom
codex/disable-codex-internal-auto-approval
Jul 10, 2026
Merged

fix(codex): require human approval for app-server commands#103457
steipete merged 2 commits into
mainfrom
codex/disable-codex-internal-auto-approval

Conversation

@steipete

Copy link
Copy Markdown
Contributor

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

  • Direct upstream contract inspection: Codex command approval requests carry command/cwd/actions but no enforceable resolved executable, and acceptance executes the original command.
  • Blacksmith Testbox crimson-hermit: pnpm test extensions/codex/src/app-server/approval-bridge.test.ts — 78/78 passed on final rebased head.
  • Blacksmith Testbox 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.
  • Regression proves a configured low-risk reviewer is not invoked and cannot override a human denial.
  • Fresh branch autoreview: no accepted/actionable findings.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: codex size: M maintainer Maintainer-authored PR labels Jul 10, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines 2421 to 2424
threadId: thread.threadId,
turnId,
nativeHookRelay,
execPolicy,
execReviewerAgentId: sessionAgentId,
internalExecAutoReview: appServer.approvalsReviewer === "user",
autoApprove: shouldAutoApproveCodexAppServerApprovals(appServer),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@steipete
steipete merged commit a97633a into main Jul 10, 2026
117 of 119 checks passed
@steipete
steipete deleted the codex/disable-codex-internal-auto-approval branch July 10, 2026 07:07
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
…103457)

* fix(codex): require human approval for app-server commands

* docs(exec): clarify Codex approval fallback
wheakerd pushed a commit to wheakerd/clawdbot that referenced this pull request Jul 15, 2026
…103457)

* fix(codex): require human approval for app-server commands

* docs(exec): clarify Codex approval fallback

(cherry picked from commit a97633a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation extensions: codex maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Exec auto-review can lose exact approval binding

1 participant