Skip to content

fix: suppress duplicate Telegram plugin approvals 🤖#89688

Closed
cyphercodes wants to merge 2 commits into
openclaw:mainfrom
cyphercodes:fix/plugin-approval-native-fallback-75749
Closed

fix: suppress duplicate Telegram plugin approvals 🤖#89688
cyphercodes wants to merge 2 commits into
openclaw:mainfrom
cyphercodes:fix/plugin-approval-native-fallback-75749

Conversation

@cyphercodes

@cyphercodes cyphercodes commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • Treat a missing approval turnSourceChannel as unknown instead of a channel mismatch when checking native-delivery fallback suppression.
  • This lets Telegram suppress the generic forwarding fallback when native delivery is enabled for the resolved approval account, avoiding duplicate plugin approval prompts.
  • AI-assisted contribution; I reviewed the diff and targeted test results.

Fixes #75749

Real behavior proof

  • Reproduced the broken decision path at the shared delivery helper level: with turnSourceChannel: null, Telegram target channel, and native delivery enabled for the resolved account, the helper now returns true to suppress forwarding fallback.
  • Verified by adding a regression to src/plugin-sdk/approval-delivery-helpers.test.ts that exercises the null-channel case and confirms the resolved account is still checked.
  • I did not run a live Telegram bot/DM session in this environment.

Validation

  • pnpm exec vitest run src/plugin-sdk/approval-delivery-helpers.test.ts --reporter=dot
  • pnpm exec oxlint src/plugin-sdk/approval-delivery-helpers.ts src/plugin-sdk/approval-delivery-helpers.test.ts
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 3, 2026
@clawsweeper

clawsweeper Bot commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 11:04 PM ET / 03:04 UTC.

Summary
The PR adds a Telegram-only opt-in so missing turn-source channel metadata can still suppress generic approval forwarding when native Telegram delivery can handle the request.

PR surface: Source +47, Tests +48. Total +95 across 3 files.

Reproducibility: yes. at source level: current main exits fallback suppression on null turnSourceChannel, while Telegram native matching can still accept a session/account-bound request. I did not run a live Telegram DM reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Plugin SDK helper option: 1 added. The new optional parameter controls when generic approval fallback messages are suppressed, so maintainers should notice the SDK fallback contract before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #75749
Summary: This PR is the current direct fix candidate for the duplicate Telegram plugin approval prompts tracked by the canonical issue; the merged turn-source forwarding PR fixes one null-source trigger but not this fallback-suppression behavior.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Attach redacted Telegram Desktop, terminal, log, recording, or linked-artifact proof showing exactly one approval prompt after the fix.
  • Resolve the conflicting branch so the current-main merge result can be reviewed.
  • Have maintainers accept the SDK helper option or choose a Telegram-local suppression path.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body and author comment provide helper/unit-test validation and explicitly say no live Telegram bot or DM proof was added; contributor proof should be redacted and the PR body updated for re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A native Telegram proof can show whether duplicate approval prompts disappear without losing the approval prompt. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify a plugin approval from a Telegram session with missing turnSourceChannel delivers exactly one native approval prompt and no forwarding duplicate.

Risk before merge

  • [P1] No live Telegram proof is attached, so the intended single-prompt behavior and absence of lost Telegram approval prompts remain unproven in the real transport.
  • [P1] GitHub reports the branch as DIRTY/CONFLICTING, so the actual merge result against current main still needs a rebase and review refresh.
  • [P2] The PR adds a Plugin SDK helper option that changes fallback suppression semantics for opt-in channels; maintainers should explicitly accept that contract or choose a Telegram-local/session-aware suppression path.

Maintainer options:

  1. Rebase And Prove Telegram Delivery (recommended)
    Resolve the conflict, keep the Telegram-only opt-in if maintainers accept it, and attach redacted live Telegram proof showing one native approval prompt with no forwarding duplicate.
  2. Maintainer-Owned Live Proof
    A maintainer can run Mantis or an equivalent live Telegram setup and record the proof if contributor-side live proof is not available.
  3. Replace The SDK Knob
    If the new helper option is not the desired plugin SDK contract, pause this branch and move the behavior into a Telegram-owned or session-aware suppression path.

Next step before merge

  • [P1] The remaining blockers are live Telegram proof, a dirty/conflicting branch, and maintainer judgment on the SDK helper contract rather than a narrow automated code repair.

Security
Cleared: No dependency, workflow, secret, package, or supply-chain surface is changed; the security-sensitive approval routing behavior is covered by functional merge risk.

Review details

Best possible solution:

Land a rebased Telegram-only fallback-suppression fix after proof shows exactly one Telegram approval prompt; if maintainers do not want the SDK helper option, replace it with a Telegram-local or session-aware suppression path.

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

Yes, at source level: current main exits fallback suppression on null turnSourceChannel, while Telegram native matching can still accept a session/account-bound request. I did not run a live Telegram DM reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, conditionally: the Telegram-only opt-in is narrower than changing all channels and preserves sibling defaults. It is not merge-ready until maintainers accept the helper contract, the branch is refreshed, and live Telegram proof is supplied.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR targets a focused Telegram approval delivery bug with limited workflow scope and no emergency core outage.
  • merge-risk: 🚨 compatibility: The diff adds an opt-in Plugin SDK helper behavior that can change fallback suppression semantics for channels using the public native approval helper.
  • merge-risk: 🚨 message-delivery: The change decides whether generic approval fallback messages are suppressed, which can drop or duplicate approval prompts if the native route is wrong.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and author comment provide helper/unit-test validation and explicitly say no live Telegram bot or DM proof was added; contributor proof should be redacted and the PR body updated for re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram approval prompt delivery and can be demonstrated in a short Telegram Desktop proof.
Evidence reviewed

PR surface:

Source +47, Tests +48. Total +95 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 48 1 +47
Tests 1 48 0 +48
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 96 1 +95

Acceptance criteria:

  • [P1] Rebase the branch, then run the focused approval helper tests through the repo test wrapper.
  • [P1] Capture redacted live Telegram proof or Mantis proof showing a null-source plugin approval produces exactly one prompt.

What I checked:

  • Current main still short-circuits null source channel: shouldSuppressForwardingFallback returns false when requireMatchingTurnSourceChannel is set and the normalized turnSourceChannel is not Telegram; null normalizes to no channel, so current main keeps the forwarding fallback enabled. (src/plugin-sdk/approval-delivery-helpers.ts:171, 54b09580f61b)
  • Telegram opts into the matching-source guard: Telegram's native approval capability sets requireMatchingTurnSourceChannel: true, so it exercises the current null-source short-circuit in the shared helper. (extensions/telegram/src/approval-native.ts:106, 54b09580f61b)
  • Telegram native matching can still handle missing source metadata: The Telegram request-account matcher only rejects a foreign source when a source channel is present and then falls through to account/session matching, which is the native path that can produce the duplicate prompt alongside forwarding fallback. (extensions/telegram/src/exec-approvals.ts:166, 54b09580f61b)
  • Forwarder uses this suppression hook before generic delivery: The generic approval forwarder calls shouldSuppressForwardingFallback before delivering a fallback target, so the helper decides whether the second prompt remains deliverable. (src/infra/exec-approval-forwarder.ts:219, 54b09580f61b)
  • PR diff scopes the behavior to Telegram opt-in: The PR adds allowMissingTurnSourceChannelForSuppression to the shared helper and sets it only in the Telegram native approval capability; sibling helper behavior remains defaulted off by a regression test. (extensions/telegram/src/approval-native.ts:107, 89f520eb70e8)
  • Latest release still has the current behavior: The v2026.6.10 tag has the same turnSourceChannel !== params.channel return path and no Telegram opt-in, so the behavior is not shipped as fixed. (src/plugin-sdk/approval-delivery-helpers.ts:171, aa69b12d0086)

Likely related people:

  • gumadeiras: File history shows repeated work centralizing channel approval seams, native approval lifecycle assembly, and Telegram foreign-channel account routing around the touched helper and Telegram matching paths. (role: feature/refactor owner; confidence: high; commits: c87c8e66bf7d, d78512b09d1d, dc306013e167; files: src/plugin-sdk/approval-delivery-helpers.ts, extensions/telegram/src/approval-native.ts, extensions/telegram/src/exec-approvals.ts)
  • scoootscooob: The unified channel approvals commit added the shared approval runtime and Telegram approval routing behavior that this PR extends. (role: introduced related unified approval routing behavior; confidence: medium; commits: 9ff57ac47902; files: src/plugin-sdk/approval-delivery-helpers.ts, extensions/telegram/src/approval-native.ts, src/plugin-sdk/approval-delivery-helpers.test.ts)
  • steipete: History shows the shared native approval delivery helper foundation and later Plugin SDK/runtime surface refactors on the files involved in this PR. (role: shared helper foundation and recent SDK area contributor; confidence: medium; commits: f8dc4305a599, 827b0de0ce74, e27c32b9b0d9; files: src/plugin-sdk/approval-delivery-helpers.ts, extensions/telegram/src/approval-native.ts, extensions/telegram/src/exec-approvals.ts)
  • pashpashpash: Recent Telegram exec approval ownership work touched the same Telegram native approval capability and account-routing area, making this a useful secondary routing candidate. (role: adjacent Telegram approval contributor; confidence: low; commits: 43fa40a35dc3; files: extensions/telegram/src/approval-native.ts, extensions/telegram/src/exec-approvals.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 3, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 15, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 17, 2026
Open
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jun 19, 2026
@openclaw-barnacle openclaw-barnacle Bot added the channel: telegram Channel integration: telegram label Jun 19, 2026
@cyphercodes

cyphercodes commented Jun 19, 2026

Copy link
Copy Markdown
Author

Updated the branch to address the ClawSweeper P1 shared-helper concern.

Changes:

  • restored the shared helper default so missing turnSourceChannel keeps forwarding fallback delivery
  • made null-source fallback suppression an explicit opt-in for Telegram only
  • added regression coverage proving other channels keep fallback when source metadata is missing

Local verification:

  • pnpm install --frozen-lockfile ✅
  • pnpm test src/plugin-sdk/approval-delivery-helpers.test.ts ✅
  • pnpm exec oxfmt --check --threads=1 src/plugin-sdk/approval-delivery-helpers.ts src/plugin-sdk/approval-delivery-helpers.test.ts extensions/telegram/src/approval-native.ts ✅
  • git diff --check ✅

Note: the real Telegram behavior proof gate still needs live/redacted proof from a real setup or maintainer override; I did not add proof data.

@cyphercodes
cyphercodes force-pushed the fix/plugin-approval-native-fallback-75749 branch from 3349a12 to 89f520e Compare June 19, 2026 10:35
This was referenced Jun 19, 2026
Merged
Merged
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 16, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added stale Marked as stale due to inactivity triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 16, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 23, 2026

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: telegram Channel integration: telegram mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Plugin approval delivers duplicate messages on Telegram — forwarder fallback and native runtime both send when turnSourceChannel is null

1 participant

@cyphercodes