fix(approvals): make exec approval fallback guidance channel-specific by gumadeiras · Pull Request #61424 · openclaw/openclaw

gumadeiras · 2026-04-05T16:23:10Z

Summary

Problem: unavailable exec-approval replies used stale shared fallback guidance and Matrix-flavored setup copy, so Discord/Slack/Telegram/Matrix users could be told to configure the wrong knobs.
Why it matters: users get blocked on approval setup even when native approvals exist for their channel/account.
What changed: native exec-approval fallback messaging now discovers approval-capable channels generically, each channel/plugin owns its own setup guidance, and the setup-guidance seam is preserved through both canonical capability wiring and legacy adapter wiring.
What did NOT change (scope boundary): no approval transport/runtime behavior changed; this is reply UX plus approval-capability plumbing only.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: the unavailable-approval reply builder in core used stale hard-coded fallback copy instead of channel/plugin-owned approval capability metadata.
Root cause extension: setup guidance initially lived in a shared helper and then, after the first fix, still was not preserved through the legacy approval-adapter path.
Missing detection / guardrail: reply tests did not cover non-Matrix disabled guidance, and helper/adapter tests did not assert the new setup-guidance seam survived capability splitting/rebuild.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/infra/exec-approval-reply.test.ts, src/plugin-sdk/approval-delivery-helpers.test.ts, src/channels/plugins/approvals.test.ts, extensions/discord/src/approval-native.test.ts, extensions/slack/src/approval-native.test.ts, extensions/telegram/src/approval-native.test.ts, extensions/matrix/src/approval-native.test.ts
Scenario the test should lock in: disabled native approval guidance is channel-specific, generic fallback comes from discovered approval-capable channels, and setup guidance survives both capability and legacy adapter paths.
Why this is the smallest reliable guardrail: the bug lives in reply composition plus plugin/capability plumbing, so focused unit + seam coverage pins the real boundary without needing live chat runtimes.
Existing test that already covers this (if any): none specific enough before this change.
If no new test is added, why not: N/A

User-visible / Behavior Changes

Disabled native exec-approval replies now use channel-specific setup guidance instead of shared Matrix-style copy.
Generic fallback guidance now lists native approval clients from channel capability discovery instead of a stale hard-coded list.
Legacy approval-adapter integrations now preserve channel-specific setup guidance instead of silently falling back to generic copy.

Diagram (if applicable)

Before:
[exec request needs approval] -> [fallback reply] -> [shared stale copy]
                                       |
                                       +-> Matrix-specific setup text could leak into other channels
                                       +-> legacy adapter path could drop setup guidance entirely

After:
[exec request needs approval] -> [capability discovery + plugin-owned setup seam] -> [channel-specific guidance]
                                                                   |
                                                                   +-> canonical capability path preserved
                                                                   +-> legacy adapter path preserved

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS
Runtime/container: local Node/pnpm dev environment
Model/provider: N/A
Integration/channel (if any): Discord, Slack, Telegram, Matrix exec approvals
Relevant config (redacted): native approval-capable channel with approvals disabled/unconfigured

Steps

Trigger an exec approval request from a native approval-capable channel without native approval routing configured.
Inspect the unavailable approval reply.
Compare disabled-path guidance and generic fallback guidance.
Exercise capability and legacy adapter paths in tests.

Expected

The initiating channel is described as supported but unconfigured.
Setup guidance references the correct channel-owned config knobs.
Generic fallback guidance references currently approval-capable native clients.

Actual

Old copy could claim the wrong setup knobs, use Matrix-flavored guidance across channels, or lose setup guidance entirely through the legacy adapter path.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios: targeted approval reply tests, channel-native approval tests, helper/adapter seam tests, and scoped build.
Edge cases checked: Discord/Slack/Telegram/Matrix disabled guidance, generic no-route guidance, helper split/rebuild compatibility.
What you did not verify: pnpm plugin-sdk:api:gen is still blocked by unrelated pre-existing elevenlabs export drift; full repo-wide pnpm check remains blocked by unrelated pre-existing failures on main; no live chat/manual repro in this PR worktree.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Risks and Mitigations

Risk: a channel plugin that never exposes setup guidance will still fall back to generic native-client guidance.
- Mitigation: the change reuses the existing capability seam, keeps guidance plugin-owned, and adds regression coverage for both canonical and legacy adapter paths.

greptile-apps · 2026-04-05T16:26:24Z

Greptile Summary

This PR fixes Matrix exec-approval fallback UX by routing the initiating-platform-disabled path through supportsNativeExecApprovalClient for Matrix-specific copy, and replacing a stale hard-coded channel list with listNativeExecApprovalClientLabels for generic fallback guidance. The capability-discovery approach is clean and the new tests pin both the Matrix-specific and generic paths correctly.

Confidence Score: 5/5

Safe to merge; all remaining findings are P2 style/process issues with no impact on runtime behavior.

No P0 or P1 issues. The two P2 findings are a cosmetic text duplication in the no-approval-route fallback and a CHANGELOG placement order preference — neither affects correctness or user safety.

src/infra/exec-approval-reply.ts (minor text duplication in no-approval-route branch); CHANGELOG.md (entry placement order).

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/infra/exec-approval-reply.ts
Line: 409-414

Comment:
**Duplicate allowFrom guidance in `no-approval-route` fallback**

When native clients are available, `buildGenericNativeExecApprovalFallbackText()` already closes with "If those accounts already know your owner ID via allowFrom or owner config, OpenClaw can often infer approvers automatically." The caller then appends "Then retry the command. If those accounts already know your owner ID via allowFrom or owner config, you can usually leave execApprovals.approvers unset." — the condition phrase appears twice in the same paragraph. Consider folding the extra hint into the helper or rewriting the appended suffix so it does not re-state the `allowFrom` condition.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: CHANGELOG.md
Line: 24

Comment:
**CHANGELOG entry inserted mid-section instead of appended**

Per `CLAUDE.md`, new entries should be appended to the **end** of the target section. This entry appears at line 24 just after the related Matrix entry, but the `### Changes` section continues to line 45. The new entry should be placed after the last existing `### Changes` line.

**Context Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8))

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Matrix: clarify exec approval fallback U..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5602945aaf

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

Copilot

Pull request overview

This PR fixes misleading Matrix exec-approval fallback messaging by switching from hard-coded “supported clients” copy to capability-based discovery, and by adding Matrix-specific “supported but unconfigured” guidance when the initiating channel can do native approvals.

Changes:

Add capability-based helpers to detect/list native exec-approval-capable channels.
Improve exec-approval unavailable reply copy (generic + Matrix-unconfigured path) and plumb initiating channel through tool result details.
Update unit/seam tests and document the UX fix in the changelog.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/infra/exec-approval-surface.ts	Adds helpers to detect native exec-approval capability and list approval-capable channel labels.
src/infra/exec-approval-reply.ts	Updates unavailable-approval reply composition to use capability discovery and Matrix “supported but unconfigured” guidance.
src/infra/exec-approval-reply.test.ts	Mocks capability discovery and asserts new fallback/setup wording (including Matrix).
src/agents/pi-embedded-subscribe.handlers.tools.ts	Parses and forwards `channel` in approval-unavailable tool result details.
src/agents/pi-embedded-subscribe.handlers.tools.test.ts	Extends seam test fixtures/assertions for new wording + channel plumbing.
src/agents/bash-tools.exec-types.ts	Extends exec tool details contract with optional `channel`.
src/agents/bash-tools.exec-host-shared.ts	Includes initiating surface `channel` in approval-unavailable tool result + inline text.
CHANGELOG.md	Notes the Matrix exec-approval unavailable-reply UX clarification.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6fd619acd2

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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 45eb93654e

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

gumadeiras · 2026-04-05T18:26:50Z

Merged via squash.

Prepared head SHA: cb5d3c249cad92919ae99296ca8545d1c76e7776
Merge commit: 0ef9383487a222020c20d51550eadcbb26882132

Thanks @gumadeiras!

@gumadeiras

…openclaw#61424) Merged via squash. Prepared head SHA: cb5d3c2 Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

…openclaw#61424) Merged via squash. Prepared head SHA: cb5d3c2 Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

…openclaw#61424) Merged via squash. Prepared head SHA: cb5d3c2 Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

…openclaw#61424) Merged via squash. Prepared head SHA: cb5d3c2 Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

Copilot AI review requested due to automatic review settings April 5, 2026 16:23

openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels Apr 5, 2026

Copilot started reviewing on behalf of gumadeiras April 5, 2026 16:23 View session

greptile-apps Bot reviewed Apr 5, 2026

View reviewed changes

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

Comment thread CHANGELOG.md Outdated

chatgpt-codex-connector Bot reviewed Apr 5, 2026

View reviewed changes

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

gumadeiras self-assigned this Apr 5, 2026

Copilot AI reviewed Apr 5, 2026

View reviewed changes

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

openclaw-barnacle Bot added channel: discord Channel integration: discord channel: matrix Channel integration: matrix channel: slack Channel integration: slack channel: telegram Channel integration: telegram size: M and removed size: S labels Apr 5, 2026

chatgpt-codex-connector Bot reviewed Apr 5, 2026

View reviewed changes

Comment thread src/plugin-sdk/approval-delivery-helpers.ts

gumadeiras changed the title ~~fix(matrix): clarify exec approval fallback UX~~ fix(approvals): make exec approval fallback guidance channel-specific Apr 5, 2026

chatgpt-codex-connector Bot reviewed Apr 5, 2026

View reviewed changes

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

openclaw-barnacle Bot added the docs Improvements or additions to documentation label Apr 5, 2026

gumadeiras added 7 commits April 5, 2026 14:24

Matrix: clarify exec approval fallback UX

af31211

Exec approvals: make setup guidance channel-specific

2dfdf8e

Plugin SDK: lock exec approval setup seam

7fb89ca

Exec approvals: dedupe no-route fallback guidance

a927dcd

Plugin SDK: preserve exec setup helper in legacy adapters

a8adbae

Exec approvals: add account-aware setup guidance

1c8b55d

Docs: clarify native approval capability hooks

cb5d3c2

gumadeiras force-pushed the codex/matrix-approval-ux branch from 6220f65 to cb5d3c2 Compare April 5, 2026 18:26

gumadeiras merged commit 0ef9383 into main Apr 5, 2026
9 checks passed

gumadeiras deleted the codex/matrix-approval-ux branch April 5, 2026 18:26

github-actions Bot mentioned this pull request Apr 5, 2026

📡 Upstream Digest — 2026-04-05 18:32 UTC curtismercier/openclaw-mods#479

Open

clawsweeper Bot mentioned this pull request Apr 29, 2026

Approval UX on Feishu needs clearer fallback guidance when interactive buttons are unavailable or approval degrades to text #69754

Closed

clawsweeper Bot mentioned this pull request May 11, 2026

[UX] Exec approval timeout message should include Control UI link and setup hints #53250

Closed

clawsweeper Bot mentioned this pull request Jun 27, 2026

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

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(approvals): make exec approval fallback guidance channel-specific#61424

fix(approvals): make exec approval fallback guidance channel-specific#61424
gumadeiras merged 7 commits into
mainfrom
codex/matrix-approval-ux

gumadeiras commented Apr 5, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 5, 2026

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gumadeiras commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

gumadeiras commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 5, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

gumadeiras commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gumadeiras commented Apr 5, 2026 •

edited

Loading