fix(discord): limit implicit reply fanout#100784
Conversation
openclaw-barnacle
Bot
added
channel: discord
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 970adb5856
ℹ️ 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".
| replyToFirstChunkOnly: shouldReplyToFirstDiscordChunkOnly({ | ||
| replyToIdSource, | ||
| replyToMode, | ||
| }), |
There was a problem hiding this comment.
When replyToMode is absent or the reply is not single-use, this expression still forwards replyToFirstChunkOnly: false to the injected Discord send dependency. Existing ordinary-send adapter proofs assert the exact option bag for non-threaded/non-first-mode sends (extensions/discord/src/channel.message-adapter.test.ts:94, :118, :196), so those paths now fail solely because an unrelated optional flag became observable; only include this property when it is true.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed July 6, 2026, 6:04 AM ET / 10:04 UTC. Summary PR surface: Source +105, Tests +185. Total +290 across 12 files. Reproducibility: yes. by source inspection: current main passes the same Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Keep this as the canonical Discord fix, omit Do we have a high-confidence way to reproduce the issue? Yes by source inspection: current main passes the same Is this the best way to solve the issue? No, not yet. The owner-boundary fix shape is right, but the PR must omit the new optional flag when it is not enabled so ordinary send contracts and tests remain stable. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3221d43d891c. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +105, Tests +185. Total +290 across 12 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
proof: sufficient
|
Land-ready maintainer pass complete at What changed:
Proof:
Per the maintainer request for this batch, I did not block landing on a new exact-head hosted CI completion; the focused, broad Testbox, type/lint, autoreview, and real Discord proofs above cover the changed surface. |
|
Merged via squash.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01e97e0d48
ℹ️ 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".
| const messageReference = params.opts.reply | ||
| ? { message_id: params.opts.reply.messageId, fail_if_not_exists: false } |
There was a problem hiding this comment.
When a caller uses the deprecated public openclaw/plugin-sdk/discord facade to edit a component message with replyTo, the facade type still exposes replyTo and forwards the options unchanged (src/plugin-sdk/discord.ts:63), but this path now only looks at opts.reply. Those calls silently stop adding message_reference and stop recording reply metadata; either translate the facade's replyTo to a DiscordReplyReference or keep accepting the legacy field here.
Useful? React with 👍 / 👎.
| fallbackChannelId: string; | ||
| kind: MessageReceiptPartKind; | ||
| threadId?: string | number; | ||
| replyToId?: string; | ||
| reply?: DiscordReplyReference; | ||
| }): DiscordSendResult { |
There was a problem hiding this comment.
After this parameter rename, the webhook send path was not migrated: sendWebhookMessageDiscord still calls createDiscordSendResult with the legacy replyToId field (extensions/discord/src/send.webhook.ts:151). In bound-thread webhook replies the Discord request still includes message_reference, but the returned receipt now drops replyToId, so durable metadata no longer matches the native reply that was sent.
Useful? React with 👍 / 👎.
* fix(discord): limit implicit reply fanout Co-authored-by: qingminlong <[email protected]> * fix(discord): preserve oversized fallback fanout * refactor(discord): make reply metadata per-message * fix(discord): keep reply receipts serializable * docs(changelog): defer Discord entry to release * refactor(discord): model native reply fanout --------- Co-authored-by: qingminlong <[email protected]>
* fix(discord): limit implicit reply fanout Co-authored-by: qingminlong <[email protected]> * fix(discord): preserve oversized fallback fanout * refactor(discord): make reply metadata per-message * fix(discord): keep reply receipts serializable * docs(changelog): defer Discord entry to release * refactor(discord): model native reply fanout --------- Co-authored-by: qingminlong <[email protected]>
1 participant
What Problem This Solves
Closes #99068. Supersedes #99150.
Discord
replyToMode: "first"and"batched"could attach the same nativemessage_referenceto every physical chunk of one logical reply. Long text, media captions, video caption/file pairs, voice payload fallbacks, and component downgrades could therefore notify the referenced user multiple times.Why This Change Was Made
The fix carries one source-aware
replyToFirstChunkOnlydecision from the Discord outbound policy boundary into every Discord-owned physical send path. Implicit single-use replies attach the reference once;all, direct sends without a single-use mode, and explicit reply tags remain reusable.Maintainer review added the missing split-video matrix, aligned omitted
replyToIdSourcewith the existingcreateReplyToFanoutcontract, and removed unused voice plumbing. It also replaced the 14-20 argument text/media helpers with named options, carried the actual per-message reply target into delivery progress, and made mixed adapter receipt metadata authoritative during final core aggregation. This maintainer-owned replacement is necessary because GitHub's verified fork-edit mutation for the old contributor branch exceeded the 45 MB request limit after rebasing; the original contributor remains co-author and is credited here.User Impact
One logical implicit Discord reply now produces one native reply notification even when Discord requires several physical messages. Explicit replies and
replyToMode: "all"continue to attach native reply references to every intended message.Evidence
tbx_01kwv79p9t30gce0ekr460d14h: 164 focused assertions across Discord outbound adapter, physical send, components, shared message planning, and Plugin SDK reply policy; a later exact patch run passed another 100 focused Discord/reply-policy assertions including upload fallback and per-message receipt coverage.src/channels/message/receipt.test.ts, 5/5 assertions, covering preservation of mixed nested reply metadata through final route aggregation.[source, null, null]and all-mode references[source, source, source]through real Discord sends.git diff --checkand targeted oxfmt passed.Known proof boundary: the real Discord send was performed on the contributor implementation before the bounded maintainer follow-up fixes; exact HTTP-boundary, receipt, type, and lint checks cover the final branch, with the native hosted gate required before merge.