fix(codex): rename destructive approval mode to ask by kevinslin · Pull Request #98501 · openclaw/openclaw

kevinslin · 2026-07-01T08:24:29Z

Related: #97123
Related: #97327

What Problem This Solves

Fixes an issue where users configuring Codex plugin write/destructive approvals had to use the value "always" even though the behavior asks the user for every action. The contradictory name makes the approval contract easy to misread and can lead operators to make incorrect permission assumptions.

Why This Change Was Made

The Codex plugin contract now uses "ask" across the manifest, public types, runtime policy, persisted binding shape, migration contracts, examples, tests, and documentation. This is an intentional hard cut: "always" fails validation and is neither aliased nor migrated.

"ask" preserves the existing app-scoped approval behavior. Codex receives approvals_reviewer: "user" only on the affected apps.<id> entry; the thread-level reviewer remains unchanged for unrelated shell, file, permission, and MCP approvals. Per-app policy context remains authoritative in the elicitation bridge, so mixed true, false, "auto", and "ask" configurations do not broaden sibling app permissions.

AI-assisted: yes. Codex implemented the change and completed a clean current-main repository autoreview.

User Impact

Operators can express the behavior directly with allow_destructive_actions: "ask": destructive plugin actions remain available but require user approval every time. Existing boolean and "auto" policies are unchanged. Old configurations using "always" must be updated and will now fail validation instead of being silently accepted or migrated.

Evidence

Focused validation passed: 23 selected tests passed across the config schema, plugin thread config, lifecycle, run-attempt, side-question, and elicitation surfaces; 386 unrelated tests were skipped by the targeted filter.
After main advanced, the branch was rebased through eb417fa206. Two newly merged base commits exposed stale test fixtures: a daemon provider fixture lacked required baseUrl, a raw Telegram SecretRef needed the established config-boundary cast, and two cron assertions still described pre-detachment state. The three test-only corrections now pass 105 focused tests plus the core test typecheck.
The full supported build passed from the rebased worktree, including tsdown, CLI import guards, SDK declarations/exports, runtime postbuild, and Control UI build.
Extension oxlint, docs MDX validation, docs listing, task-file formatting, and git diff --check passed.
Fresh current-main autoreview completed cleanly with no accepted or actionable findings (0.98 confidence).
Direct inspection of the pinned Codex rust-v0.142.4 source confirmed that apps.<id>.approvals_reviewer is the supported app-scoped reviewer contract and is resolved for codex_apps without changing the thread reviewer.
Blacksmith Testbox was unavailable because its Crabbox image is 0.16.0 while this repository requires at least 0.22.0; the fallback Azure coordinator rejected the request with HTTP 401. GitHub CI is the broad remote proof for this head.
Exact-head GitHub CI run 28548825324 passed all required build, type, lint, test, security, and docs jobs.

Commands:

OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-vitest.mjs extensions/codex/src/app-server/plugin-thread-config.test.ts extensions/codex/src/app-server/thread-lifecycle.binding.test.ts extensions/codex/src/app-server/thread-lifecycle.test.ts extensions/codex/src/app-server/run-attempt.test.ts extensions/codex/src/app-server/side-question.test.ts extensions/codex/src/app-server/elicitation-bridge.test.ts src/config/config.plugin-validation.test.ts -t 'ask|always|reviewer|plugin policy'

OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-vitest.mjs src/commands/daemon-install-helpers.test.ts src/cron/isolated-agent/run-session-state.test.ts src/cron/isolated-agent/run.message-tool-policy.test.ts

OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile /tmp/openclaw-codex-ask-core-test.tsbuildinfo

OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/codex/src/app-server/plugin-thread-config.ts extensions/codex/src/app-server/thread-lifecycle.ts extensions/codex/src/app-server/run-attempt.ts extensions/codex/src/app-server/side-question.ts

node scripts/check-docs-mdx.mjs docs/gateway/configuration-reference.md docs/plugins/codex-native-plugins.md

OPENCLAW_BUILD_ALL_NO_PNPM=1 OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/build-all.mjs

.agents/skills/autoreview/scripts/autoreview --mode local --prompt '<task-scoped current-main review>' --stream-engine-output

Real behavior proof

Behavior or issue addressed: Replace the misleading Codex plugin destructive-action value "always" with "ask", reject the retired value, and retain per-app user approval without changing unrelated reviewers.
Real environment tested: Existing local Kevin OpenClaw gateway and persistent devbox, using image openclaw-debug:codex-destructive-ask-41e41eec-20260701t2138z built from exact PR head 41e41eec010fcf39fc75210beee8ff4dbfaa88c8. The existing devbox was preserved and the prior gateway container was retained as rollback.
Exact steps run: Sent a Slack request requiring Linear.save_issue; observed the generated plugin approval; confirmed the pending gateway record exposed only allow-once and deny; resolved that exact approval once through the authenticated local gateway; then read the Slack thread response.
Evidence after fix: The running app policy snapshot reported Linear as destructiveApprovalMode: "ask", while Google Calendar remained "auto" and Google Drive, Notion, and Slack remained "allow". The Linear write did not execute before approval. After the one-time decision, Slack reported Plugin approval: Allowed once, returned the required completion marker, and included the created issue URL.
Observed result: Passed. The uniquely titled Linear issue was created only after approval and independently verified from the returned issue URL. Internal Slack and Linear artifact links are intentionally omitted from this public PR.
Additional failure-mode proof: An earlier exact-head attempt received the same approval prompt but its Slack text approval was queued behind the active reply lane; the approval expired and no Linear write occurred. This independently confirms fail-closed behavior when approval is not delivered through the resolver in time.
What was not changed: No devbox recreation, remote gateway deployment/restart, publish, or live-service mutation was performed. The existing remote Kevin gateway remained out of the local test path.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8a7e95685

ℹ️ 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".

clawsweeper · 2026-07-01T09:03:17Z

Codex review: found issues before merge. Reviewed July 1, 2026, 6:10 PM ET / 22:10 UTC.

Summary
The PR renames the Codex plugin destructive-action policy value from always to ask across schema, runtime policy, persisted binding parsing, migration planning, docs, and tests.

PR surface: Source 0, Tests +149, Docs +9. Total +158 across 20 files.

Reproducibility: yes. at source level. PR head forks side conversations with the base reviewer while Codex thread/fork loads fresh cwd/request config, so the app-scoped ask config from the parent binding is not replayed.

Review metrics: 2 noteworthy metrics.

Config Policy Value: 1 added, 1 removed. The PR adds "ask" and removes current-main "always", so operator config validation and upgrade behavior need maintainer-visible review.
Persisted Binding Mode: 1 serialized value changed. The app-server binding enum changes from "always" to "ask", so resume and replay behavior for existing mainline bindings matters before merge.

Stored data model
Persistent data-model change detected: database schema: extensions/codex/src/app-server/session-binding.test.ts, migration/backfill/repair: extensions/codex/src/migration/plan.ts, migration/backfill/repair: extensions/codex/src/migration/provider.test.ts, serialized state: extensions/codex/src/app-server/session-binding.test.ts, unknown-data-model-change: extensions/codex/src/app-server/session-binding.test.ts, unknown-data-model-change: extensions/codex/src/migration/plan.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #98499
Summary: This PR is the open implementation candidate for the linked feature issue requesting the same Codex destructive approval policy rename; the earlier merged PRs are feature-history context, not replacements.

Members:

canonical: [Feature]: Rename Codex destructive approval policy to ask #98499 - The PR body uses closing syntax for the open issue and implements its requested rename.
partial_overlap: feat(codex): add always plugin approval mode #97123 - That merged PR introduced the current-main "always" destructive approval policy value this PR renames.
partial_overlap: fix(codex): preserve per-app approval reviewer policy #97327 - That merged PR repaired reviewer policy on the same Codex plugin approval surface and informs this PR's app-scoped reviewer behavior.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦞 diamond lobster
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Replay the persisted plugin app config into side-thread forks.
[P2] Add or restore focused side-question coverage for an ask plugin policy.
Have maintainers explicitly accept or revise the current-main-only always hard cut.

Mantis proof suggestion
After the side-thread fix, a Slack smoke would materially show the real destructive plugin approval prompt and side-conversation behavior. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis slack desktop smoke: verify Codex allow_destructive_actions "ask" prompts for a side-thread destructive plugin write and preserves sibling policy.

Risk before merge

[P1] Side-thread forks from an ask parent do not replay the app-scoped Codex plugin config, so the fork can lose apps.<id>.approvals_reviewer = "user" and the associated destructive-approval boundary.
[P1] The PR intentionally rejects current-main "always" configs and drops old schema-v2 binding policy context without an alias or doctor migration; that appears unshipped in v2026.6.11, but maintainers still need to accept the mainline cut.
[P1] The changed setting controls human approval routing for destructive Codex plugin actions, so green CI does not settle the security-boundary/product decision.

Maintainer options:

Fix Side-Fork App Replay (recommended)
Replay binding.pluginAppPolicyContext into the side-thread fork config and restore focused coverage for ask side conversations before merge.
Accept The Mainline Hard Cut
After the side-fork fix, maintainers can explicitly accept that unshipped "always" configs and bindings should fail or drop context rather than be migrated.
Pause Rename Direction
If the hard-cut naming contract is not settled, pause this PR and keep the linked issue as the product-decision thread.

Next step before merge

[P2] The PR needs a side-thread approval-boundary fix plus maintainer acceptance of the hard config/binding cut, and the protected maintainer label keeps the remaining action human-owned.

Security
Needs attention: The diff changes a destructive-action approval boundary, and side-thread forks currently lose the app-scoped human reviewer config.

Review findings

[P1] Replay ask app policy into side-thread forks — extensions/codex/src/app-server/side-question.ts:439

Review details

Best possible solution:

Replay the persisted plugin app policy config into side-thread forks, keep the app-scoped reviewer shape, then have maintainers explicitly accept or revise the current-main-only always to ask hard cut.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. PR head forks side conversations with the base reviewer while Codex thread/fork loads fresh cwd/request config, so the app-scoped ask config from the parent binding is not replayed.

Is this the best way to solve the issue?

No, not yet. The rename and app-scoped reviewer shape are the right owner-boundary direction, but the side-thread fork path must preserve the same app config before this is the best complete fix.

Full review comments:

[P1] Replay ask app policy into side-thread forks — extensions/codex/src/app-server/side-question.ts:439
This now forks with only the base modelScopedAppServer.approvalsReviewer, but the fork config above never replays the parent binding's plugin app policy as an apps patch. Codex thread/fork loads fresh config from cwd/request overrides, so an ask parent side conversation loses apps.<id>.approvals_reviewer = "user" and the destructive-approval boundary. Merge buildCodexPluginAppsConfigPatchFromPolicyContext(binding.pluginAppPolicyContext) into the fork config and restore the ask side-thread test.
Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against eb417fa206e6.

Label changes

Label changes:

add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦞 diamond lobster and patch quality is 🧂 unranked krab.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body and latest comment provide exact-head live Slack-to-Linear proof for the main destructive approval path, though that proof does not cover the side-thread replay finding.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.
remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.

Label justifications:

P2: This is a bounded Codex plugin approval config change with compatibility and approval-boundary impact but no evidence of an urgent shipped outage.
merge-risk: 🚨 compatibility: The diff intentionally rejects current-main "always" config values without an alias or doctor migration.
merge-risk: 🚨 session-state: The diff changes persisted Codex app-server binding interpretation by dropping old schema-v2 "always" policy context.
merge-risk: 🚨 security-boundary: The changed config controls whether destructive Codex plugin actions are routed to human approval, and the side-thread path currently loses that app-scoped setting.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦞 diamond lobster and patch quality is 🧂 unranked krab.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body and latest comment provide exact-head live Slack-to-Linear proof for the main destructive approval path, though that proof does not cover the side-thread replay finding.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and latest comment provide exact-head live Slack-to-Linear proof for the main destructive approval path, though that proof does not cover the side-thread replay finding.

Evidence reviewed

PR surface:

Source 0, Tests +149, Docs +9. Total +158 across 20 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	7	27	27	0
Tests	11	204	55	+149
Docs	2	16	7	+9
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	20	247	89	+158

Security concerns:

[high] Side-thread approval boundary is not replayed — extensions/codex/src/app-server/side-question.ts:439
The side-thread fork path keeps the base reviewer and does not replay the plugin app apps config, so ask destructive plugin approvals in side conversations may no longer be routed through the intended human approval boundary.
Confidence: 0.88

What I checked:

PR head changes the public config value: The PR head changes CodexPluginDestructivePolicy and the zod schema to accept "ask" instead of "always". (extensions/codex/src/app-server/config.ts:77, 41e41eec010f)
PR head uses app-scoped reviewer for ask apps: The PR head sets apps.<id>.approvals_reviewer = "user" only for plugin apps whose resolved destructive approval mode is "ask". (extensions/codex/src/app-server/plugin-thread-config.ts:275, 41e41eec010f)
Side-thread fork omits plugin app config replay: The side-question fork config is built from runtime and network-proxy config only, then sends the base app-server reviewer; it does not merge the persisted plugin app policy config from the parent binding. (extensions/codex/src/app-server/side-question.ts:423, 41e41eec010f)
Codex fork loads fresh request config: Pinned Codex thread/fork loads config from cwd plus request overrides before creating the fork, so OpenClaw must replay ephemeral app config in the fork request if it wants it preserved. (../codex/codex-rs/app-server/src/request_processors/thread_processor.rs:3332, d0fd96663e19)
Codex app config supports app-scoped reviewer: Pinned Codex rust-v0.142.4 defines AppConfig.approvals_reviewer as the reviewer override for approval prompts from a single app. (../codex/codex-rs/config/src/types.rs:456, d0fd96663e19)
Codex approval routing checks app reviewer first: Pinned Codex resolves mcp_approvals_reviewer for codex_apps by connector id before _default and global reviewer. (../codex/codex-rs/core/src/connectors.rs:548, d0fd96663e19)

Likely related people:

kevinslin: Authored and merged the related always approval mode PR, the per-app reviewer follow-up, and this rename candidate on the same Codex plugin approval surface. (role: feature-history owner and recent area contributor; confidence: high; commits: c5d34c8376f8, 389c355bcf3f, 41e41eec010f; files: extensions/codex/src/app-server/config.ts, extensions/codex/src/app-server/plugin-thread-config.ts, extensions/codex/src/app-server/session-binding.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

kevinslin · 2026-07-01T21:35:23Z

@clawsweeper re-review

clawsweeper · 2026-07-01T21:35:26Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

kevinlin-openai · 2026-07-01T21:57:52Z

Exact-head live proof is now complete on 41e41eec010fcf39fc75210beee8ff4dbfaa88c8.

Built and ran openclaw-debug:codex-destructive-ask-41e41eec-20260701t2138z against the existing local Kevin devbox without recreating it.
Runtime policy snapshot: Linear=ask; Calendar=auto; Google Drive, Notion, and Slack=allow.
Slack requested approval before Linear.save_issue; the pending approval allowed only allow-once or deny.
Resolving that exact request once produced Plugin approval: Allowed once, created the uniquely titled Linear issue, and returned the required completion marker with its issue URL.
A preceding attempt was allowed to expire and produced no Linear write, confirming the fail-closed path.

The PR body now contains the exact steps, image/head, adjacent-policy snapshot, result, and caveats. Internal Slack and Linear artifact links are intentionally omitted from this public PR.

@clawsweeper re-review

kevinlin-openai · 2026-07-01T22:02:29Z

@clawsweeper re-review

kevinlin-openai · 2026-07-01T22:15:00Z

Merged via squash.

Prepared head SHA: 41e41eec010fcf39fc75210beee8ff4dbfaa88c8
Landed commit: d9b5afad180b1276be788706ecee5733e5c42ec1

* fix(codex): rename destructive approval mode to ask * fix(codex): keep ask reviewer app-scoped * test: fix current-main CI regressions

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

chatgpt-codex-connector Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread extensions/codex/src/app-server/plugin-thread-config.ts Outdated

clawsweeper Bot mentioned this pull request Jul 1, 2026

[Feature]: Rename Codex destructive approval policy to ask #98499

Closed

kevinslin force-pushed the dev/kevinlin/codex-plugin-destructive-ask branch from b8a7e95 to 8819678 Compare July 1, 2026 20:45

kevinslin added 3 commits July 1, 2026 14:25

fix(codex): rename destructive approval mode to ask

cef6d20

fix(codex): keep ask reviewer app-scoped

d85c2c6

test: fix current-main CI regressions

41e41ee

kevinslin force-pushed the dev/kevinlin/codex-plugin-destructive-ask branch from ca58a1d to 41e41ee Compare July 1, 2026 21:26

openclaw-barnacle Bot added the commands Command implementations label Jul 1, 2026

clawsweeper Bot added the status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. label Jul 1, 2026

kevinlin-openai self-assigned this Jul 1, 2026

kevinlin-openai merged commit d9b5afa into main Jul 1, 2026
157 of 175 checks passed

kevinlin-openai deleted the dev/kevinlin/codex-plugin-destructive-ask branch July 1, 2026 22:14

github-actions Bot mentioned this pull request Jul 1, 2026

📡 Upstream Digest — 2026-07-01 23:04 UTC curtismercier/openclaw-mods#1185

Open

This was referenced Jul 1, 2026

fix: restore daemon install test typecheck #98770

Closed

fix(codex): preserve plugin app approvals in side conversations #98812

Merged

This was referenced Jul 2, 2026

fix(ios): chat composer stays a single line while typing multiline messages #98851

Closed

test: cover string-backed non-streaming assistant replies #65198

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): rename destructive approval mode to ask#98501

fix(codex): rename destructive approval mode to ask#98501
kevinlin-openai merged 3 commits into
mainfrom
dev/kevinlin/codex-plugin-destructive-ask

kevinslin commented Jul 1, 2026 •

edited by kevinlin-openai

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

kevinslin commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

kevinlin-openai commented Jul 1, 2026

Uh oh!

kevinlin-openai commented Jul 1, 2026

Uh oh!

Uh oh!

kevinlin-openai commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

kevinslin commented Jul 1, 2026 • edited by kevinlin-openai Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Real behavior proof

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinslin commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinlin-openai commented Jul 1, 2026

Uh oh!

kevinlin-openai commented Jul 1, 2026

Uh oh!

Uh oh!

kevinlin-openai commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevinslin commented Jul 1, 2026 •

edited by kevinlin-openai

Loading

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading