fix(exec): clean approval-pending chat prompts by vincentkoc · Pull Request #78184 · openclaw/openclaw

vincentkoc · 2026-05-06T01:34:12Z

Summary

This PR fixes the Telegram/Zollie approval loop symptom from the live test instance by separating two things that were being collapsed into one confusing chat prompt:

approval-pending gateway/node execs, where no command is running yet and the result should just ask for approval
real foreground fallback execs, where background execution is unavailable and the command actually does run synchronously

It also adds doctor preview/fix coverage for the older restricted-profile tool config shape that allowed exec/write but omitted their companion process/edit groups.

RCA

The live logs did not point at Telegram delivery loss. Telegram sends were making it through. The bad behavior was command approval churn: the gateway had pending exec approvals waiting for user action, including one exec.approval.waitDecision span that sat for roughly 1,799,938 ms before the follow-up path handled it.

The screenshot made the confusing part visible. The bot first said the check was pending approval, then rendered a second card that said Warning: background execution is disabled; running synchronously. That warning was wrong for this approval state. For approval-gated gateway/node execs, the command has not started yet. The tool registers an approval request and returns the pending card immediately; if approved, the command continues later through the approval follow-up path.

The bug was in createExecTool. It added the background-disabled warning to the shared warning list before gateway approval routing. The approval renderer reuses that warning list, so the pending approval prompt inherited a warning that only makes sense after OpenClaw has decided to run the command in the foreground. That made Telegram look like it was running lost synchronous exec loops when it was really waiting on approval.

The second contributing issue was config drift. The instance had a restricted messaging tools profile with explicit alsoAllow entries for exec and write, plus configured tools.exec/tools.fs, but it did not also allow the companion capabilities process and edit. Current policy intentionally no longer widens those implicitly. That is good security behavior, but older configs can end up half-migrated: the user is allowed to start an exec/write path but not the companion management/edit tool that makes the experience coherent. The logs were warning about exactly that.

What Changed

The exec tool now keeps the background-disabled warning staged until after gateway approval routing. If the command returns an approval-pending result, the warning is not shown and is not included in the approval request warning text. If OpenClaw really does run a foreground command because background execution is disabled, the existing warning is still shown.

Doctor now detects and repairs the older partial restricted-profile shape. It only repairs explicit partial grants: if a restricted messaging or minimal profile already explicitly allows exec while tools.exec is configured, doctor adds process; if it already explicitly allows write while tools.fs is configured, doctor adds edit. It does not touch complete grouped configs such as group:runtime/group:fs, coding/full profiles, or implicit-only configs.

I also documented the approval-gated exec exception in the exec docs and updated the gateway tool config docs to steer operators toward grouped capabilities and doctor repair.

Why I’m Confident

The tests lock both sides of the behavior. There is a regression test proving approval-pending gateway exec does not include the misleading background-disabled warning, and a foreground failure test proving the real synchronous fallback warning still appears when it should. The doctor helper tests cover preview warnings, global repair, inherited agent repair, and no-op cases for complete groups, implicit configs, and coding profiles.

The fresh Testbox run synced only this branch’s changed files and passed the focused tests plus the changed gate on the expected core, coreTests, and docs lanes.

Verification

pnpm docs:list
git diff --check origin/main...HEAD
pnpm changed:lanes --json showed core, coreTests, and docs
Testbox tbx_01kqxehk7wevjj9memh7kdhy75: pnpm test:serial src/agents/bash-tools.exec.approval-id.test.ts src/agents/bash-tools.exec-foreground-failures.test.ts src/commands/doctor/shared/tool-companion-allowlist-repair.test.ts passed, 35 tests
Testbox tbx_01kqxehk7wevjj9memh7kdhy75: pnpm check:changed passed on core, coreTests, and docs

Docs:

clawsweeper · 2026-05-06T01:37:17Z

Codex review: found issues before merge. Reviewed July 4, 2026, 12:37 PM ET / 16:37 UTC.

Summary
The branch stages the exec background-disabled warning until after approval routing, adds doctor preview/fix repair for partial restricted tool allowlists, and updates exec/gateway docs, tests, and CHANGELOG.

PR surface: Source +128, Tests +162, Docs +9. Total +299 across 10 files.

Reproducibility: yes. Current main adds the background-disabled warning before gateway approval registration and forwards that warnings array into approval warningText, so the prompt bug is source-reproducible without running a live Telegram flow.

Review metrics: 1 noteworthy metric.

Doctor repair grants: 2 added: process, edit. Doctor --fix would mutate existing restricted-profile allowlists, so maintainers should review compatibility and authorization impact before merge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: CHANGELOG.md, migration/backfill/repair: docs/gateway/config-tools.md, migration/backfill/repair: src/commands/doctor/repair-sequencing.ts, migration/backfill/repair: src/commands/doctor/shared/preview-warnings.ts, migration/backfill/repair: src/commands/doctor/shared/tool-companion-allowlist-repair.test.ts, migration/backfill/repair: src/commands/doctor/shared/tool-companion-allowlist-repair.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
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:

[P2] Resolve the companion-grant repair by splitting it or recording explicit maintainer security/compatibility approval with upgrade proof.
Narrow the exec docs to only the approval paths that still return approval-pending.
Capture Telegram-visible proof once the prompt behavior is ready for maintainer review.

Mantis proof suggestion
Native Telegram proof would directly show whether the approval-pending prompt omits the misleading warning while true foreground fallback behavior remains intact. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify approval-pending gateway exec omits the background execution disabled warning while a true foreground fallback still shows it.

Risk before merge

[P1] Doctor --fix would add process/edit to existing restricted-profile allowlists that explicitly allowed exec/write, broadening the repaired authorization boundary for operator configs.
[P1] The added exec docs say gateway/node approval prompts return immediately, but current native chat and webchat approval paths wait inline when no explicit follow-up mode is set.
[P1] The PR is a maintainer-labeled draft touching security-sensitive repair behavior, so cleanup should stay behind explicit maintainer review rather than auto-close or auto-merge.

Maintainer options:

Approve Or Split The Repair Boundary (recommended)
Refresh the branch and either record explicit maintainer security/compatibility approval for adding process/edit during doctor repair or split that repair into its own review.
Land Prompt Cleanup Separately
A narrow PR can keep only the warning-order fix and its regression tests while leaving companion-grant repair for a security-focused follow-up.
Accept The Grant Expansion
Maintainers may intentionally accept the process/edit repair if they own the operator-facing upgrade and authorization impact.

Next step before merge

[P2] Manual review is needed because the remaining blocker is a security/compatibility decision about repair-time tool grants plus author cleanup of the docs wording.

Security
Needs attention: Needs attention because the doctor repair can broaden restricted allowlists by adding process/edit during --fix; no supply-chain changes were found.

Review findings

[P1] Get approval before granting companion tools — src/commands/doctor/shared/tool-companion-allowlist-repair.ts:38-42
[P2] Narrow the approval docs to async paths — docs/tools/exec.md:12-13

Review details

Best possible solution:

Keep the warning-staging fix, narrow the docs to the approval paths that still return pending, and split or explicitly approve the doctor companion-grant repair with upgrade/security proof before merge.

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

Yes. Current main adds the background-disabled warning before gateway approval registration and forwards that warnings array into approval warningText, so the prompt bug is source-reproducible without running a live Telegram flow.

Is this the best way to solve the issue?

Partly. Staging the warning until after approval routing is the narrow fix, but the doctor grant repair and broad docs sentence need maintainer approval or cleanup before this is the best merge shape.

Full review comments:

[P1] Get approval before granting companion tools — src/commands/doctor/shared/tool-companion-allowlist-repair.ts:38-42
This repair adds process and/or edit to existing restricted-profile allowlists during doctor --fix. That changes the effective authorization boundary for operators who granted only exec/write, so please split it or land it only with explicit maintainer security/compatibility approval and upgrade proof.
Confidence: 0.88
[P2] Narrow the approval docs to async paths — docs/tools/exec.md:12-13
Current main waits inline for native chat gateway approvals when no explicit follow-up mode is set, so this blanket statement is false for Telegram, Discord, Slack, and similar native approval channels. Please document only the node, non-native, explicit follow-up, or unavailable paths that still return approval-pending.
Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P1: The PR targets a real exec approval chat workflow and changes repair-time authorization behavior that needs urgent maintainer review before merge.
merge-risk: 🚨 compatibility: Doctor repair would rewrite existing restricted-profile configs by adding companion tool grants during an operator repair flow.
merge-risk: 🚨 security-boundary: Adding process/edit to repaired allowlists changes the effective tool authorization boundary for existing configs.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this member-authored, maintainer-labeled draft PR; the PR body includes Testbox validation, while refreshed Telegram-visible proof would still help review.
mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes a user-visible Telegram approval-pending exec prompt, which a short Telegram Desktop proof can demonstrate.

Evidence reviewed

PR surface:

Source +128, Tests +162, Docs +9. Total +299 across 10 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	4	131	3	+128
Tests	3	162	0	+162
Docs	3	9	0	+9
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	10	302	3	+299

Security concerns:

[medium] Review companion tool grants before merge — src/commands/doctor/shared/tool-companion-allowlist-repair.ts:38
The repair adds process and/or edit to explicit restricted-profile allowlists when exec/write and their tool sections are present, changing the effective repaired authorization boundary for older configs.
Confidence: 0.88

What I checked:

Repository policy read and applied: Root AGENTS.md and scoped src/agents and docs guides were read fully; their compatibility/security guidance applies because this PR touches exec approval routing, doctor repair, and user-visible docs. (AGENTS.md:31, cf36cb915dd6)
Current main still has the warning-order bug: Current main appends the background-disabled warning before gateway approval routing, so the narrow prompt cleanup is still source-reproducible. (src/agents/bash-tools.exec.ts:1586, cf36cb915dd6)
Gateway approval receives the shared warning list: Current main passes the same warnings array into gateway approval warningText, making pre-approval warnings visible in pending approval prompts. (src/agents/bash-tools.exec-host-gateway.ts:676, cf36cb915dd6)
PR head stages the warning after gateway routing: The PR stores the background-disabled warning separately and appends it only after gateway approval routing returns without a pending result, which is the narrow fix for the prompt symptom. (src/agents/bash-tools.exec.ts:1269, a99e520bec25)
Doctor repair changes authorization grants: The new repair adds process and edit when restricted profiles already explicitly allow exec/write and have corresponding tool sections, changing repaired authorization for existing configs. (src/commands/doctor/shared/tool-companion-allowlist-repair.ts:38, a99e520bec25)
Current migration does not auto-add process: Existing migration coverage preserves an explicit alsoAllow exec grant without automatically adding process, so the PR's doctor --fix behavior is a new compatibility/security decision. (src/commands/doctor/shared/legacy-config-migrate.test.ts:826, cf36cb915dd6)

Likely related people:

vincentkoc: History shows prior merged work across remote exec approval and doctor sequencing, beyond authorship of this PR. (role: recent area contributor; confidence: high; commits: 2d53ffdec1da, 6f2e80418207, b2380b3ab1ad; files: src/agents/bash-tools.exec.ts, src/agents/bash-tools.exec-host-gateway.ts, src/agents/bash-tools.exec-host-node.ts)
NianJiuZst: Authored the merged native chat inline approval fix that changed the same gateway approval semantics this branch must document correctly. (role: adjacent approval-flow contributor; confidence: high; commits: 1bfa2787b503; files: src/agents/bash-tools.exec-host-gateway.ts, src/agents/bash-tools.exec-host-gateway.test.ts, src/utils/message-channel-constants.ts)
steipete: Shortlog and recent history show substantial work across the affected exec, agent-test, and doctor/config repair surfaces. (role: heavy adjacent owner; confidence: medium; commits: b71c91022b58, e7d33b4870f7, 310b5e4f6a22; files: src/agents/bash-tools.exec.ts, src/agents/bash-tools.exec-host-gateway.ts, src/commands/doctor/shared/preview-warnings.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.

Review history (1 earlier review cycle)

reviewed 2026-07-03T21:02:08.260Z sha a99e520 :: found issues before merge. :: [P1] Get approval before granting companion tools | [P2] Narrow the approval docs to async paths

steipete · 2026-07-07T10:31:15Z

The valid part of this work has landed via #101561 in 942b4496610756818e90e12d95327520e85ce2bb, with contributor credit preserved. Thank you, @vincentkoc.

That replay keeps foreground-only fallback warnings out of pending gateway and node approval prompts while retaining them when execution actually happens, and it corrects the approval-mode documentation. It deliberately does not carry over this branch's Doctor repair that granted process and edit to restricted profiles: that would silently widen the authorization boundary. The original branch also could not be updated by maintainers and had drifted substantially, so I am closing it in favor of the focused landed replacement. Enabling Allow edits by maintainers on future contributor branches makes in-place repair possible.

fix(exec): clean approval pending warnings

a99e520

vincentkoc self-assigned this May 6, 2026

openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime commands Command implementations agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels May 6, 2026

Zac-W mentioned this pull request May 15, 2026

fix(exec): return approved WebChat gateway exec output inline #82019

Closed

clawsweeper Bot had a problem deploying to qa-live-shared June 14, 2026 19:13 Failure

This was referenced Jun 23, 2026

fix(agents): add channelData marker to exec-approval-unavailable payloads #60445

Closed

Exec approval requests should pause the agent turn until resolved #70953

Open

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 3, 2026

clawsweeper Bot mentioned this pull request Jul 6, 2026

ClawSec security banner wording is inaccurate and not actionable #100935

Open

4 tasks

steipete mentioned this pull request Jul 7, 2026

fix(exec): keep pending approval warnings truthful #101561

Merged

steipete closed this Jul 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(exec): clean approval-pending chat prompts#78184

fix(exec): clean approval-pending chat prompts#78184
vincentkoc wants to merge 1 commit into
mainfrom
fix-telegram-exec-policy-doctor

vincentkoc commented May 6, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 6, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

vincentkoc commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

RCA

What Changed

Why I’m Confident

Verification

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vincentkoc commented May 6, 2026 •

edited

Loading

clawsweeper Bot commented May 6, 2026 •

edited

Loading