Skip to content

refactor(dispatch): gate reasoning delivery on a channel capability (follow-up to #95051)#95380

Closed
Marvinthebored wants to merge 1 commit into
openclaw:qa-sre-telegram-reasoning-delivery-20260619from
Marvinthebored:pr-on-pr-capgate-95051
Closed

refactor(dispatch): gate reasoning delivery on a channel capability (follow-up to #95051)#95380
Marvinthebored wants to merge 1 commit into
openclaw:qa-sre-telegram-reasoning-delivery-20260619from
Marvinthebored:pr-on-pr-capgate-95051

Conversation

@Marvinthebored

Copy link
Copy Markdown
Contributor

Small follow-up on top of this PR's branch (qa-sre-telegram-reasoning-delivery-20260619) — feel free to squash/cherry-pick into #95051 or ignore.

Summary

#95051 gates reasoning-payload suppression on deliveryChannel !== "telegram". This swaps that hardcoded channel name for a declared reasoningPayloads channel capability, mirroring the existing resolveChannelTtsVoiceDelivery precedent (getChannelPlugin(id)?.capabilities.tts?.voice). Channels with a reasoning lane opt in by declaring the capability instead of being named in core dispatch, so the next reasoning-capable channel (Slack, Matrix, …) needs no core edit.

  • ChannelCapabilities gains reasoningPayloads?: boolean (next to blockStreaming).
  • New channelSupportsReasoningPayloads() resolver — uses getLoadedChannelPlugin (loaded-only, no bundled cold-load) since it runs on every dispatch; fails closed for unloaded/unknown channels.
  • Telegram declares reasoningPayloads: true; the suppression gate becomes !channelSupportsReasoningPayloads(deliveryChannel).

Behaviour-preserving: Telegram still delivers durable reasoning, generic channels (WhatsApp/web) still suppress — the existing dispatch tests assert both, unchanged.

Verification

  • node scripts/run-tsgo.mjs -p tsconfig.core.json → 0
  • vitest run dispatch-from-config.test.ts -t "isReasoning" + the new reasoning-capabilities.test.ts → green (telegram delivers via capability; generic suppressed)
  • Reviewed with a Codex red-team pass (no blocking findings; verified the getLoadedChannelPlugin choice avoids a hot-path cold-load).

AI assistance

Prepared with AI assistance (Claude). Behaviour verified locally on OpenClaw 2026.6.9-beta.1.

cc @Peetiegonzalez

…bility

Replace the hardcoded `deliveryChannel !== "telegram"` reasoning-suppression
check with a `reasoningPayloads` channel capability, resolved via the existing
process-stable channel registry (mirrors resolveChannelTtsVoiceDelivery). New
reasoning-lane channels opt in by declaring the capability instead of being
named in core dispatch. Telegram declares it; generic channels (WhatsApp/web)
stay suppressed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 20, 2026
@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 21, 2026, 1:43 PM ET / 17:43 UTC.

Summary
The PR replaces the Telegram-name reasoning delivery gate on the stacked Telegram fix branch with a reasoningPayloads channel capability, declares it for Telegram, and adds resolver coverage.

PR surface: Source +31, Tests +57. Total +88 across 6 files.

Reproducibility: yes. The linked Telegram bug is source-reproducible because current main suppresses isReasoning payloads before channel delivery, and the issue provides concrete /reasoning on production steps.

Review metrics: 1 noteworthy metric.

  • Channel Capability Surface: 1 optional capability added. The new reasoningPayloads field is part of the public channel plugin capability type, so maintainers should notice the API contract before merge.

Root-cause cluster
Relationship: partial_overlap
Canonical: #95051
Summary: This PR is a stacked capability-refactor follow-up to the open canonical Telegram durable-reasoning fix, not an independent replacement.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit ✨ media proof bonus
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Add maintainer-approved docs or contract coverage for reasoningPayloads, or keep the capability internal to the canonical Telegram fix.
  • [P2] Coordinate the final landing branch with fix(telegram): deliver durable reasoning replies #95051 so reviewers validate one Telegram durable-reasoning path.

Mantis proof suggestion
Native Telegram proof would show the final capability-gated branch persists the Thinking message and final answer without duplicate output. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify the final folded capability-gated /reasoning on branch persists Thinking and final answer without duplicate output.

Risk before merge

  • [P1] The PR adds reasoningPayloads to the public channel capability type without accompanying SDK documentation or contract coverage, so external channel plugins could rely on an underspecified delivery contract after release.
  • [P1] Reasoning delivery now fails closed when the loaded channel plugin does not advertise the capability, so plugin registration drift can silently suppress durable reasoning again.
  • [P1] This PR is stacked on fix(telegram): deliver durable reasoning replies #95051, which still owns the Telegram durable-reasoning fix and should remain the final review and proof target.

Maintainer options:

  1. Document And Fold Into Canonical Branch (recommended)
    Add maintainer-approved docs or contract coverage for reasoningPayloads, then squash or cherry-pick this follow-up into fix(telegram): deliver durable reasoning replies #95051.
  2. Keep The Telegram-Specific Gate
    Maintainers can decline the new capability surface for now and land the narrower Telegram-specific fix first.
  3. Accept The New API Surface
    Maintainers may intentionally accept the undocumented capability, but external channel authors could copy it before the semantics are written down.

Next step before merge

  • [P2] Maintainer review should decide whether the new public channel capability belongs in the canonical Telegram fix and require docs or contract coverage before merge.

Security
Cleared: The diff changes TypeScript source and tests only, with no dependency, workflow, secret, install, or package-publishing changes.

Review findings

  • [P1] Document the reasoning channel capability before exposing it — src/channels/plugins/types.core.ts:319-322
Review details

Best possible solution:

Fold the capability gate into #95051 only after the capability contract is documented or explicitly maintainer-approved and Telegram proof is retained on the final branch.

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

Yes. The linked Telegram bug is source-reproducible because current main suppresses isReasoning payloads before channel delivery, and the issue provides concrete /reasoning on production steps.

Is this the best way to solve the issue?

No, not yet as a merge-ready PR. The capability gate is a cleaner owner boundary than a hardcoded Telegram check, but the public plugin API contract needs maintainer approval or documentation before this is the best final shape.

Full review comments:

  • [P1] Document the reasoning channel capability before exposing it — src/channels/plugins/types.core.ts:319-322
    This adds reasoningPayloads to ChannelCapabilities, which is re-exported through types.public.ts and plugin-sdk/channel-contract.ts. External channel plugins can now compile against an undocumented dispatch contract, so add public docs/contract coverage for the capability semantics or keep the gate internal until maintainers approve that API.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded Telegram reasoning-delivery follow-up with a plugin API contract blocker, but no crash, data-loss, or security-bypass signal.
  • merge-risk: 🚨 compatibility: The diff adds public channel capability surface that external plugins could depend on after release.
  • merge-risk: 🚨 message-delivery: The diff changes the condition that decides whether durable reasoning payloads are delivered or suppressed for channel replies.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): The contributor posted inspectable Telegram Desktop screenshots showing durable Thinking messages and final answers after /reasoning on; they support visible behavior, though the final folded branch should keep equivalent proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor posted inspectable Telegram Desktop screenshots showing durable Thinking messages and final answers after /reasoning on; they support visible behavior, though the final folded branch should keep equivalent proof.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The contributor posted inspectable Telegram Desktop screenshots showing durable Thinking messages and final answers after /reasoning on; they support visible behavior, though the final folded branch should keep equivalent proof.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR affects Telegram-visible reasoning delivery, so a short Telegram Desktop proof remains useful for the final folded branch.
Evidence reviewed

PR surface:

Source +31, Tests +57. Total +88 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 4 36 5 +31
Tests 2 58 1 +57
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 94 6 +88

What I checked:

Likely related people:

  • vincentkoc: Current-main blame and history put recent dispatch-from-config.ts, types.core.ts, and Telegram capability-adjacent work under Vincent Koc, and fix(telegram): deliver durable reasoning replies #95051 is the canonical stacked base for this follow-up. (role: recent central dispatch and canonical PR contributor; confidence: high; commits: 9f888d95e082, 54ab69d23bbb, a88fbf0f6476; files: src/auto-reply/reply/dispatch-from-config.ts, src/channels/plugins/types.core.ts, extensions/telegram/src/shared.ts)
  • steipete: Git history shows repeated work on plugin SDK/channel capability boundaries and prior channel registry/capability refactors that this PR extends. (role: plugin SDK and channel capability history contributor; confidence: high; commits: 63cbc097b5ca, 41e39eb46f19, 05279539a805; files: src/channels/plugins/types.core.ts, src/plugin-sdk/channel-contract.ts, docs/plugins/sdk-channel-plugins.md)
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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 20, 2026
@Marvinthebored

Copy link
Copy Markdown
Contributor Author

Live Telegram proof that this capability-gate refactor is behaviour-preserving.

This PR replaces the hardcoded deliveryChannel !== "telegram" check with a reasoningPayloads channel-capability lookup. By construction it produces the same delivery decision for Telegram — the dispatch tests assert telegram-delivers / generic-suppresses, unchanged — so the proof captured on the base branch (#95051) applies directly here.

Captured on prod, deepseek /reasoning on, Telegram Desktop — durable thinking persists and the final answer delivers, no duplication:

turn 1

turn 2

These are the same stills posted on the base PR (#95051, issuecomment-4759234640) — output is identical with the capability gate in place. cc @Peetiegonzalez

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 21, 2026
@Marvinthebored

Copy link
Copy Markdown
Contributor Author

Closing — superseded by #97875, which lands the canonical reasoningPayloadsEnabled seam as a shared per-call reply-option. That achieves this PR's goal (a channel opts into durable reasoning-payload delivery without core naming each one) more cleanly, and correctly excludes reasoning from TTS. Thanks @fuller-stack-dev for extracting it into a shared foundation. 🙏

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

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. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants