fix(exec-approval): distinguish non-persistable command from ask=always policy in approval prompt by ianalloway · Pull Request #98215 · openclaw/openclaw

ianalloway · 2026-06-30T16:27:22Z

What Problem This Solves

When `allowAlwaysPersistence.kind === "one-shot"` (for example, a command that uses shell redirection), `allow-always` is correctly excluded from the allowed decisions. The approval prompt still showed the same explanation used for an `ask=always` policy:

"The effective approval policy requires approval every time, so Allow Always is unavailable."

That is misleading because the command can be approved; it just cannot be saved as a reusable approval pattern.

Closes #97069

What Changed

exec-approval-reply.ts — added optional nonPersistableCommand field to ExecApprovalPendingReplyParams. When true, buildExecApprovalPendingReplyPayload emits a distinct message explaining the command cannot be saved as a reusable pattern.
bash-tools.exec-runtime.ts — same branch added to buildApprovalPendingMessage (foreground/CLI approval message path).
bash-tools.exec-types.ts — added nonPersistableCommand?: boolean to the approval-pending variant of ExecToolDetails, so the flag survives the gateway→tool-result→embedded-handler hop.
bash-tools.exec-host-shared.ts — buildExecApprovalPendingToolResult accepts and passes nonPersistableCommand through to both the message builder and the details output.
bash-tools.exec-host-gateway.ts — derives and passes nonPersistableCommand: effectiveAllowAlwaysPersistence.kind === "one-shot" && hostAsk !== "always" to the tool-result builder.
bash-tools.exec-host-node.ts — same derivation using allowAlwaysPersistence and approvalDecisionAsk.
embedded-agent-subscribe.handlers.tools.ts — readExecApprovalPendingDetails return type extended with nonPersistableCommand?: boolean; value read from details and forwarded to buildExecApprovalPendingReplyPayload.
exec-approval-forwarding.ts (Telegram) — derives nonPersistableCommand from allowedDecisions and request.request.ask and passes it through.

Evidence

188/188 tests pass across 5 test files after the change, including two tests that directly assert the distinct copy for non-persistable vs. ask=always:

$ pnpm exec vitest run \
    src/infra/exec-approval-reply.test.ts \
    src/agents/bash-tools.exec-host-shared.test.ts \
    src/agents/bash-tools.exec-host-gateway.test.ts

 Test Files  5 passed (5)
      Tests  188 passed (188)
   Duration  ~14s

Specific assertions in exec-approval-reply.test.ts (34/34):

shows non-persistable message when allow-always is excluded by command type — asserts output contains "cannot be saved as a reusable pattern", not "policy requires approval every time"
shows policy message when allow-always is excluded by ask=always policy — asserts output contains "policy requires approval every time", not "cannot be saved as a reusable pattern"

Gateway test confirms wiring at the buildExecApprovalPendingToolResult call site: src/agents/bash-tools.exec-host-gateway.test.ts 106/106 passing.

Production Paths Now Wired

Path	Before	After
Gateway foreground (`buildApprovalPendingMessage`)	always policy copy	✅ correct copy
Gateway tool result details (`ExecToolDetails`)	no flag	✅ flag stored
Embedded handler → reply payload	no flag	✅ flag forwarded
Telegram forwarder → reply payload	no flag	✅ flag derived + forwarded
Node foreground	always policy copy	✅ correct copy

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

clawsweeper · 2026-06-30T16:28:34Z

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Current main already solves the reported approval-copy problem through the broader merged implementation from #97740, so this competing branch is no longer necessary.

Root-cause cluster
Relationship: superseded
Canonical: #97069
Summary: The canonical bug report was fixed by the merged #97740; this PR and the remaining competing branches are superseded.

Members:

canonical: [Bug]: Approval prompt wrongly says effective policy requires approval every time for non-persistable redirected command #97069 - This is the concrete production report and was closed by the current-main landing commit.
fixed_by_candidate: fix(exec-approval): stop misattributing Allow Always unavailability to policy #97740 - This merged PR produced the broader current-main implementation.
superseded: fix(approval): distinguish policy vs non-persistable reason when Allow Always unavailable #97145 - This open conflicting candidate targets behavior now implemented on main.
superseded: fix(infra): clarify Allow Always unavailable message for one-shot commands #98108 - This open conflicting candidate targets behavior now implemented on main.
superseded: fix(exec-approvals): clarify why Allow Always is unavailable for one-shot commands (#97069) #98359 - This earlier candidate was closed unmerged.

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

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the merged reason-neutral wording across all approval surfaces; it fixes the false policy attribution without adding a parallel unavailable-reason field or retaining this obsolete branch.

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

No on current main. The shared reply, generic forwarder, foreground agent, reaction, gateway validation, and Control UI paths now use truthful command-specific unavailable copy.

Is this the best way to solve the issue?

Yes. The merged neutral-copy solution fixes every known renderer with less state plumbing and compatibility surface than this PR’s optional flag.

Security review:

Security review cleared: No concrete security or supply-chain concern remains relevant because this branch should close rather than merge.

AGENTS.md: found and applied where relevant.

What I checked:

Current-main shared reply: Exec approval replies now use the neutral command-specific explanation whenever Allow Always is unavailable. (src/infra/exec-approval-reply.ts:386, cbc84767bd7e)
Current-main approval surfaces: The merged fix covers the generic forwarder, foreground agent message, reaction instructions, gateway validation, Control UI, localized copy, and focused tests rather than only the paths changed by this PR. (src/infra/exec-approval-forwarder.ts:286, cbc84767bd7e)
Fix provenance: GitHub file history at the exact current main SHA identifies cbc84767bd7ed9400a94e51466b71fbb7b168b10, merged through fix(exec-approval): stop misattributing Allow Always unavailability to policy #97740, as the landing commit for the reason-neutral copy. (src/plugin-sdk/approval-reaction-runtime.ts:247, cbc84767bd7e)
Canonical issue closure: The landing commit formally closes the canonical report at [Bug]: Approval prompt wrongly says effective policy requires approval every time for non-persistable redirected command #97069, which closed at the same July 11, 2026 timestamp. (cbc84767bd7e)
Current-main-only provenance: The latest stable release, v2026.6.11, predates the fix. The newer v2026.7.1-beta.5 tag targets commit b6387afd6d2e0f43c2ae98d2d124dbc277f03cca at 2026-07-11T10:07:22Z, before the fix landed at 2026-07-11T10:09:51Z; no inspected release contains the fix. (cbc84767bd7e)

Likely related people:

saju01: Authored the merged fix that provides the current-main neutral approval copy across all affected surfaces. (role: introduced current behavior; confidence: high; commits: cbc84767bd7e; files: src/agents/bash-tools.exec-runtime.ts, src/gateway/server-methods/exec-approval.ts, src/infra/exec-approval-forwarder.ts)
vincentkoc: Co-authored the landing commit, closed the canonical issue, and has recent merged history in approval reaction and forwarding code. (role: merger and adjacent owner; confidence: high; commits: cbc84767bd7e, b3fbe5325e0b, b261e9e6ddaa; files: src/plugin-sdk/approval-reaction-runtime.ts, src/infra/exec-approval-forwarder.ts)
jesse-merhi: The one-shot persistence behavior exposing the misleading copy appears to date to the command-planner work in c9707ab635b9. (role: introduced related behavior; confidence: medium; commits: c9707ab635b9; files: src/gateway/server-methods/exec-approval.ts)

Codex review notes: model internal, reasoning high; reviewed against bc239a851618; fix evidence: commit cbc84767bd7e, main fix timestamp 2026-07-11T10:09:51Z.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9dd6623c7a

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

ianalloway · 2026-07-09T19:19:42Z

@jesse-merhi all CI is green — production wiring is in place for gateway, node, embedded, and Telegram paths. Ready for merge when you have a moment.

* Update Claude CLI billing docs * Sync Claude live testing billing note

…#97289) * fix(shared): truncate assistant error text on UTF-16 boundary Use truncateUtf16Safe from the existing utf16-slice utility instead of String.prototype.slice(0, 600), which can split a surrogate pair and produce malformed Unicode when the error message contains characters outside the Basic Multilingual Plane (emoji, rare CJK, etc.). * test(shared): add regression test for UTF-16 safe error truncation Verify formatRawAssistantErrorForUi does not produce dangling surrogates when truncating a fallback raw error message with a non-BMP character (emoji) straddling the 600-code-unit boundary. * fix(agents): truncate assistant error text fallback on UTF-16 boundary Use truncateUtf16Safe instead of String.prototype.slice(0,600) in formatAssistantErrorText and its companion comparison in isRawAssistantErrorPassthrough, preventing dangling surrogates when non-BMP characters straddle the 600-code-unit truncation boundary.

truncateReplyBody used a raw value.slice(0, MAX_REPLY_BODY_LENGTH - 3) to shorten long reply bodies. When the cut index fell between the two UTF-16 code units of a surrogate pair (e.g. an emoji), the slice left a lone high surrogate before the ellipsis, which renders as a broken glyph in the reply context shown to the agent. Replace the raw slice with sliceUtf16Safe from the plugin SDK so the truncation never cuts inside a surrogate pair. A normal (non-astral) body is unaffected. Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

sanitizeForConsole filtered control characters code-point-aware but then truncated with sanitized.slice(0, maxChars), which cuts on UTF-16 code units. When the cap landed between the two code units of an astral character (emoji, CJK extension, etc.) the output ended in a lone high surrogate before the ellipsis. Cap on code points instead. Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

* fix(gateway): surface idempotency key in history metadata * test(gateway): avoid unsafe optional chain * fix(gateway): preserve oversized transcript idempotency keys * fix(gateway): preserve oversized idempotency keys * fix(gateway): reuse transcript json helpers --------- Co-authored-by: 吴杨帆 <[email protected]>

* ci(docker): publish releases to Docker Hub * ci(docker): clarify beta image tags

github-actions · 2026-07-11T16:26:30Z

Dependency Guard

This PR changes dependency-related files. Maintainers should confirm these changes are intentional.

Changed files:

extensions/qa-lab/package.json
package.json
pnpm-lock.yaml
pnpm-workspace.yaml

Maintainer follow-up:

Review whether the dependency changes are intentional.
Inspect resolved package deltas when lockfile, shrinkwrap, or workspace dependency policy changes are present.
Treat package-lock.json and npm-shrinkwrap.json diffs as security-review surfaces.
Run pnpm deps:changes:report -- --base-ref origin/main --markdown /tmp/dependency-changes.md --json /tmp/dependency-changes.json locally for detailed release-style evidence.

github-actions · 2026-07-11T16:26:31Z

Dependency graph changes are blocked

OpenClaw does not accept dependency graph changes through PRs unless a repository admin or security explicitly authorizes the current head SHA. Dependency updates are generated internally by maintainers so external PRs cannot change the resolved graph.

Detected dependency graph changes:

pnpm-lock.yaml changed.
extensions/qa-lab/package.json changed dependencies, devDependencies, peerDependencies, version.
package.json changed dependencies, devDependencies, version, engines.

Auto-scrub was not attempted because this PR changes package manifest dependency graph fields:

extensions/qa-lab/package.json changed dependencies, devDependencies, peerDependencies, version.
package.json changed dependencies, devDependencies, version, engines.

Dependency graph changes must be reviewed by security or handled by maintainers internally. Please remove lockfile changes manually if they are not needed.

To remove lockfile changes, restore them from the target branch:

git fetch origin
git checkout 'origin/main' -- 'pnpm-lock.yaml'
git commit -m 'chore: remove dependency lockfile change'
git push

If this PR intentionally needs a dependency graph change, ask a repository admin or member of @openclaw/openclaw-secops to comment:

/allow-dependencies-change

The action will approve the current head SHA (e09f6486f7008a4d45b791dfec804b878e291df5) when it reruns. A later push requires a fresh approval.

steipete · 2026-07-12T00:17:42Z

Thanks for the careful analysis and focused tests here.

Current main already carries the same user-facing fix through #97740, merged as cbc8476. That implementation uses command-specific neutral copy across the shared approval renderers without adding parallel optional state through each execution path.

This branch also includes a large amount of unrelated history, including an iOS change already present on main, so rebasing or salvaging it would not improve the canonical fix. Closing as superseded. If the wording is still wrong on current main, a new focused reproduction against the landed path would change this decision.

Copilot AI review requested due to automatic review settings June 30, 2026 16:27

Copilot AI reviewed Jun 30, 2026

openclaw-barnacle Bot added size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 30, 2026

chatgpt-codex-connector Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/infra/exec-approval-reply.ts

clawsweeper Bot temporarily deployed to qa-live-shared June 30, 2026 16:36 Inactive

tirion-p and others added 7 commits July 11, 2026 12:23

[codex] Update Claude CLI billing docs (#96848)

3039c32

* Update Claude CLI billing docs * Sync Claude live testing billing note

fix path to Discord Developer Mode in setup guide (#97336)

4f1d2a6

ci(docker): publish releases to Docker Hub (#97122)

4e158f8

* ci(docker): publish releases to Docker Hub * ci(docker): clarify beta image tags

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(exec-approval): distinguish non-persistable command from ask=always policy in approval prompt#98215

fix(exec-approval): distinguish non-persistable command from ask=always policy in approval prompt#98215
ianalloway wants to merge 330 commits into
openclaw:mainfrom
ianalloway:fix/approval-nonpersistable-message

ianalloway commented Jun 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

ianalloway commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 11, 2026

Uh oh!

github-actions Bot commented Jul 11, 2026

Uh oh!

steipete commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Uh oh!

Uh oh!

Conversation

ianalloway commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

What Changed

Evidence

Production Paths Now Wired

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

ianalloway commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 11, 2026

Dependency Guard

Uh oh!

github-actions Bot commented Jul 11, 2026

Dependency graph changes are blocked

Uh oh!

steipete commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

ianalloway commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading