fix(exec): dedupe Discord approval delivery by scoootscooob · Pull Request #58002 · openclaw/openclaw

scoootscooob · 2026-03-31T00:13:48Z

Summary

Problem: Discord DM-origin approvals could post two approval cards when channels.discord.execApprovals.target="both" was enabled.
Why it matters: approvers saw duplicate approval cards in the same DM even though only one approval request existed.
What changed: dedupe Discord approval sends by the resolved Discord channel id, and ignore session-store fallback origin targets for true discord:dm:* sessions.
What did NOT change (scope boundary): no Telegram changes, no approval auth changes, and no exec approval policy/runtime changes outside Discord delivery.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related refactor(exec): unify channel approvals and restore routing/auth #57838
This PR fixes a bug or regression

Root Cause / Regression History (if applicable)

Root cause: the shared approval planner deduped logical targets before Discord resolved approver DMs into concrete channel ids. When the origin route and approver DM later collapsed onto the same Discord DM channel, DiscordExecApprovalHandler still posted twice.
Missing detection / guardrail: there was no send-time dedupe keyed by the resolved discordChannelId, and the discord:dm:* session-store fallback could still reintroduce an origin target.
Prior context (git blame, prior PR, issue, or refactor if known): this regressed in the shared channel approval unification from refactor(exec): unify channel approvals and restore routing/auth #57838, which moved Discord native approval delivery onto the shared planner/runtime.
Why this regressed now: target="both" intentionally schedules origin + approver delivery, but Discord DM surfaces can converge after DM creation rather than at plan time.
If unknown, what was ruled out: ruled out multiple approval requests and multiple gateway instances; the live repro was one request delivered twice to the same DM surface.

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: extensions/discord/src/monitor/exec-approvals.test.ts
Scenario the test should lock in: origin delivery and approver DM delivery resolve to the same concrete Discord channel id under target="both", and only one card is posted.
Why this is the smallest reliable guardrail: the bug appears after Discord resolves Routes.userChannels() into a DM channel id, so it needs the delivery seam rather than a pure planner unit test.
Existing test that already covers this (if any): extensions/discord/src/approval-native.test.ts now also covers the narrower discord:dm:* session-store fallback path.
If no new test is added, why not: N/A

User-visible / Behavior Changes

Discord DM-origin approvals no longer send duplicate approval cards when origin delivery and approver DM delivery collapse onto the same DM surface.

Security Impact (required)

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

Repro + Verification

Environment

OS: macOS
Runtime/container: local gateway + Discord plugin runtime on latest main
Model/provider: N/A
Integration/channel (if any): Discord
Relevant config (redacted): channels.discord.execApprovals.enabled=true, channels.discord.execApprovals.target="both", single approver configured

Steps

Start latest main with Discord exec approvals enabled and target="both".
Trigger an exec approval from a Discord DM with an approver who also receives DM approvals.
Observe delivery in the approver DM.

Expected

One approval card in the DM.

Actual

Two approval cards in the same DM.

Evidence

Attach at least one:

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

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: reproduced the duplicate-card behavior on latest main, patched the clean smoke runtime, and confirmed the same Discord DM-origin approval flow produced only one card after the fix.
Edge cases checked: focused Discord regression tests for the concrete channel-id collision and the discord:dm:* session-store fallback; pnpm build passed.
What you did not verify: Telegram flows and non-Discord approval surfaces.

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.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

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

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert this PR, or temporarily switch Discord exec approval target from both to dm or channel.
Files/config to restore: extensions/discord/src/monitor/exec-approvals.ts, extensions/discord/src/approval-native.ts
Known bad symptoms reviewers should watch for: duplicate Discord approval cards in a DM, or missing approval delivery for true discord:dm:* sessions.

Risks and Mitigations

Risk: deduping by resolved Discord channel id could accidentally suppress a legitimate second post if two logical targets are intentionally meant to land in the same Discord channel.
- Mitigation: the dedupe key is scoped per approval request, and the only legitimate collision here is the duplicate DM/origin convergence we want to suppress.

greptile-apps · 2026-03-31T00:17:36Z

Greptile Summary

This PR fixes a duplicate approval-card delivery bug that surfaced under channels.discord.execApprovals.target="both" when the request originated from a Discord DM: the origin route and the approver's DM channel resolved to the same concrete Discord channel id, causing DiscordExecApprovalHandler to post two identical approval cards to the same DM surface.

The fix is applied at two complementary levels:

approval-native.ts — resolveDiscordOriginTarget now returns null immediately when sessionKind === "dm", preventing the session-store / legacy-channel-id fallback paths from supplying a DM channel id as an origin target. Since turnSourceTarget can never be set for DM sessions (the existing sessionKind !== "dm" guard on line 105 prevents it), this guard is effectively "for all DM sessions, origin target = null," which is the correct semantic: the DM surface should only be reached via the approver-DM path, not duplicated through an origin delivery.
exec-approvals.ts — A per-request deliveredChannelIds Set is created before the delivery loop. Successful origin-surface sends populate the set; before each DM-surface send the resolved dmChannel.id is checked against the set and skipped if already present. This is the correct place for the second layer of defence because it fires after Discord resolves Routes.userChannels() into a concrete channel id, the point in time where the collision can first be detected.

Both regression test scenarios (the DM-session fallback in approval-native.test.ts and the converging channel-id scenario in exec-approvals.test.ts) correctly fail on the pre-fix code and pass after, and the test mocking faithfully represents the live API topology.

Key observations:

The dedupe set is scoped to a single sendApprovalCards invocation, so it cannot incorrectly suppress approval delivery across separate requests.
If origin delivery fails (catch path), the channel id is not added to the set, preserving resilience: the DM path will still attempt delivery to the same channel rather than being silently skipped.
The notifyOriginWhenDmOnly redirect-notice path intentionally does not interact with deliveredChannelIds; it sends a different message type and its delivery correctly remains independent of the approval-card dedupe.
Minor style nit: createApproverRestrictedNativeApprovalAdapter and resolveExecApprovalSessionTarget are now two separate import statements from the same "openclaw/plugin-sdk/approval-runtime" module; they can be merged into one.

Confidence Score: 5/5

Safe to merge — the fix is logically sound, correctly layered, and covered by targeted regression tests; no P0 or P1 issues found.
Both the approval-native.ts early-return guard and the exec-approvals.ts send-time dedupe are correct. The only finding is a cosmetic duplicate-import style nit (P2). The dedupe Set is correctly scoped per request, error paths intentionally leave the set unpopulated for resilience, and the interaction with notifyOriginWhenDmOnly is clean.
No files require special attention; all four changed files are in good shape.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/discord/src/approval-native.ts
Line: 1-2

Comment:
**Duplicate import from same module**

`createApproverRestrictedNativeApprovalAdapter` and `resolveExecApprovalSessionTarget` are both imported from `"openclaw/plugin-sdk/approval-runtime"` as two separate statements. They should be merged into one import to keep the file consistent and avoid redundant module references.

```suggestion
import { createApproverRestrictedNativeApprovalAdapter, resolveExecApprovalSessionTarget } from "openclaw/plugin-sdk/approval-runtime";
```

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

_{Reviews (1): Last reviewed commit: "fix(exec): dedupe Discord approval deliv..." | Re-trigger Greptile}

scoootscooob · 2026-03-31T00:18:10Z

@codex review

chatgpt-codex-connector · 2026-03-31T00:23:36Z

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* fix(exec): dedupe Discord approval delivery * Update extensions/discord/src/approval-native.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

fix(exec): dedupe Discord approval delivery

b8cad67

openclaw-barnacle Bot added channel: discord Channel integration: discord size: S maintainer Maintainer-authored PR labels Mar 31, 2026

greptile-apps Bot reviewed Mar 31, 2026

View reviewed changes

Comment thread extensions/discord/src/approval-native.ts Outdated

Update extensions/discord/src/approval-native.ts

eb9ec15

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

scoootscooob merged commit eba41da into main Mar 31, 2026
9 checks passed

scoootscooob deleted the codex/discord-approval-dedupe branch March 31, 2026 00:27

github-actions Bot mentioned this pull request Mar 31, 2026

📡 Upstream Digest — 2026-03-31 01:45 UTC curtismercier/openclaw-mods#416

Open

clawsweeper Bot mentioned this pull request May 14, 2026

Discord exec approval card/buttons not delivered in v2026.4.26 (regression after #71804) #73802

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(exec): dedupe Discord approval delivery#58002

fix(exec): dedupe Discord approval delivery#58002
scoootscooob merged 2 commits into
mainfrom
codex/discord-approval-dedupe

scoootscooob commented Mar 31, 2026

Uh oh!

greptile-apps Bot commented Mar 31, 2026

Uh oh!

Uh oh!

scoootscooob commented Mar 31, 2026

Uh oh!

chatgpt-codex-connector Bot commented Mar 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant