Skip to content

fix(discord): limit implicit reply references to first chunk#99171

Closed
LiLan0125 wants to merge 1 commit into
openclaw:mainfrom
LiLan0125:fix/99068-discord-reply-first-chunks
Closed

fix(discord): limit implicit reply references to first chunk#99171
LiLan0125 wants to merge 1 commit into
openclaw:mainfrom
LiLan0125:fix/99068-discord-reply-first-chunks

Conversation

@LiLan0125

Copy link
Copy Markdown
Contributor

Summary

  • Limit native Discord message_reference reuse for implicit replyToMode: "first" and "batched" replies to the first physical Discord message chunk.
  • Preserve reusable reply references for replyToMode: "all", explicit/direct replies, and audioAsVoice follow-up sends that reuse a captured reply id.
  • Add regression coverage for text chunks, media caption chunks, explicit replies, and the audioAsVoice payload forwarding path.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security
  • Chore

Scope

  • Gateway
  • Skills
  • Auth
  • Memory
  • Integrations
  • API
  • UI/UX
  • CI

Verification

  • node scripts/run-vitest.mjs extensions/discord/src/send.sends-basic-channel-messages.test.ts src/infra/outbound/message-plan.test.ts extensions/discord/src/outbound-adapter.test.ts -> [test] passed 2 Vitest shards in 52.29s
  • pnpm tsgo:extensions:test -> passed with exit code 0
  • git diff --check -> passed
  • Production-path proof below calls the actual sendMessageDiscord implementation from this branch.

Real behavior proof

Behavior addressed: Chunked Discord replies with implicit replyToMode: "first" now attach the native Discord reply reference only to the first physical message, while reusable explicit/all-mode replies still keep the reference on later chunks.

Environment tested: Linux workspace on Node v22.22.0, branch fix/99068-discord-reply-first-chunks.

Steps run after the patch: Called the actual sendMessageDiscord function from extensions/discord/src/send.js with node --import tsx; captured the request bodies sent through a local RequestClient implementation.

Evidence after fix:

node --import tsx --no-warnings -e '
import { sendMessageDiscord } from "./extensions/discord/src/send.js";

const cfg = { channels: { discord: { token: "t" } } };

function createCaptureRest() {
  const sends = [];
  return {
    rest: {
      async post(route, options) {
        if (String(route).includes("/messages")) {
          sends.push({
            send: sends.length + 1,
            route,
            contentLength: typeof options?.body?.content === "string" ? options.body.content.length : 0,
            fileCount: Array.isArray(options?.body?.files) ? options.body.files.length : 0,
            hasMessageReference: Boolean(options?.body?.message_reference),
            messageReferenceId: options?.body?.message_reference?.message_id ?? null,
          });
          return { id: `msg-${sends.length}`, channel_id: "789" };
        }
        return { id: "789" };
      },
      async get() {
        return { id: "789", type: 0 };
      },
    },
    sends,
  };
}

async function collect(label, text, opts = {}) {
  const capture = createCaptureRest();
  await sendMessageDiscord("channel:789", text, {
    rest: capture.rest,
    token: "t",
    cfg,
    replyTo: "orig-123",
    ...opts,
  });
  return [label, capture.sends];
}

const cases = Object.fromEntries(await Promise.all([
  collect("implicitFirstText", "a".repeat(2001), { replyToIdSource: "implicit", replyToMode: "first" }),
  collect("implicitFirstMedia", "b".repeat(2500), { replyToIdSource: "implicit", replyToMode: "first", mediaUrl: "file:///tmp/openclaw-99068-proof.png", mediaLocalRoots: ["/tmp"] }),
  collect("implicitAllText", "c".repeat(2001), { replyToIdSource: "implicit", replyToMode: "all" }),
  collect("explicitFirstText", "d".repeat(2001), { replyToIdSource: "explicit", replyToMode: "first" }),
]));

console.log(JSON.stringify(cases, null, 2));
'

Output:

{
  "implicitFirstText": [
    {
      "send": 1,
      "route": "/channels/789/messages",
      "contentLength": 2000,
      "fileCount": 0,
      "hasMessageReference": true,
      "messageReferenceId": "orig-123"
    },
    {
      "send": 2,
      "route": "/channels/789/messages",
      "contentLength": 1,
      "fileCount": 0,
      "hasMessageReference": false,
      "messageReferenceId": null
    }
  ],
  "implicitFirstMedia": [
    {
      "send": 1,
      "route": "/channels/789/messages",
      "contentLength": 2000,
      "fileCount": 1,
      "hasMessageReference": true,
      "messageReferenceId": "orig-123"
    },
    {
      "send": 2,
      "route": "/channels/789/messages",
      "contentLength": 500,
      "fileCount": 0,
      "hasMessageReference": false,
      "messageReferenceId": null
    }
  ],
  "implicitAllText": [
    {
      "send": 1,
      "route": "/channels/789/messages",
      "contentLength": 2000,
      "fileCount": 0,
      "hasMessageReference": true,
      "messageReferenceId": "orig-123"
    },
    {
      "send": 2,
      "route": "/channels/789/messages",
      "contentLength": 1,
      "fileCount": 0,
      "hasMessageReference": true,
      "messageReferenceId": "orig-123"
    }
  ],
  "explicitFirstText": [
    {
      "send": 1,
      "route": "/channels/789/messages",
      "contentLength": 2000,
      "fileCount": 0,
      "hasMessageReference": true,
      "messageReferenceId": "orig-123"
    },
    {
      "send": 2,
      "route": "/channels/789/messages",
      "contentLength": 1,
      "fileCount": 0,
      "hasMessageReference": true,
      "messageReferenceId": "orig-123"
    }
  ]
}

Observed result: Implicit first-mode text and media sends only include message_reference on send 1, while all-mode and explicit first-mode sends include message_reference on both chunks.

Not tested: Live Discord API delivery with real credentials and real voice-message media conversion were not exercised locally.

Root Cause

  • The Discord send path treated every replyTo passed into chunked sends as reusable, so an implicit replyToMode: "first" logical reply could become multiple native Discord replies after text/media splitting.
  • The audioAsVoice payload path intentionally captures one reply id for its internal voice/text/media sends, so that captured id must be treated as explicit when passed to chunked follow-up sends.

Regression Test Plan

  • send.sends-basic-channel-messages.test.ts covers implicit first-mode text chunks, implicit first-mode media caption chunks, all-mode chunks, explicit first-mode chunks, and direct chunked replies.
  • outbound-adapter.test.ts covers audioAsVoice forwarding so captured reply ids are marked explicit before reaching chunked message sends.
  • message-plan.test.ts remains in the targeted run to cover upstream implicit reply consumption behavior.

User-visible / Behavior Changes

  • Discord replies using replyToMode: "first" or "batched" no longer create reply-thread references on every split chunk.
  • Explicit/direct reply sends and replyToMode: "all" keep the previous reusable reference behavior.

Impact Assessment

  • Scope: Discord outbound message delivery only; the change is applied at the shared Discord send layer and the Discord payload forwarding layer.
  • Downstream: text sends, media caption sends, component/media payloads, and audioAsVoice payloads were traced and covered by targeted tests.
  • Edge cases: empty replyTo, direct sends without policy metadata, explicit reply ids, all-mode replies, and media caption follow-ups were considered.
  • Hard-coded values: no new runtime constants or configuration defaults were introduced.
  • Existing fixes: no open competing PR for [Bug]: Discord: replyToMode "first" — chunked reply sends the native reply reference on every physical chunk (reply-notification spam) #99068 was found before implementation.
  • Import paths: existing project imports are used.

Security Impact

  • New permissions: No
  • Secret handling changed: No
  • New network calls: No

Closes #99068

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: S labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 1:03 PM ET / 17:03 UTC.

Summary
The PR forwards Discord reply source/mode into send serialization, limits implicit single-use reply references to the first text/media chunk, preserves reusable reply paths, and adds focused tests.

PR surface: Source +67, Tests +44. Total +111 across 6 files.

Reproducibility: yes. Current-main source repeats replyTo through Discord text/media chunks and the linked issue includes shipped-version instrumentation showing repeated message_reference values; I did not run a live Discord repro in this read-only review.

Review metrics: 1 noteworthy metric.

  • Discord Reply Policy Fields: 2 forwarded. The PR carries reply source and mode into send serialization, which is the state needed to limit implicit chunks without suppressing explicit replies.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99068
Summary: This PR is a candidate fix for the open Discord chunked native-reply fan-out bug, with multiple same-root-cause PR candidates still open.

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: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted live Discord proof showing implicit first/batched long replies quote only the first physical chunk and explicit/all replies keep expected native references.
  • Have maintainers choose one same-root-cause candidate PR before landing to avoid duplicate fixes.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body includes terminal output from actual sendMessageDiscord calls with a local RequestClient capture, but no live Discord delivery or API readback proof; contributors should add redacted screenshots, recordings, terminal/live output, linked artifacts, or logs and update the PR body for re-review.

Mantis proof suggestion
A real Discord visual or API-readback proof would materially confirm native reply references and notification behavior across long-message chunks. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify Discord replyToMode first quotes only the first long-reply chunk while explicit first/all chunk replies keep expected native references.

Risk before merge

Maintainer options:

  1. Prove Native Discord Delivery (recommended)
    Before merge, add redacted Discord screenshot, recording, terminal/live output, API readback, or logs showing first-chunk-only implicit replies and preserved explicit/all replies.
  2. Accept Serialization-Only Proof
    Maintainers can intentionally merge on request-body serialization tests and CI alone, but that accepts unverified native Discord notification behavior.
  3. Pick One Candidate PR
    Choose this PR or one same-root-cause candidate as the landing path, then close or supersede the others after the selected fix lands.

Next step before merge

  • [P1] This needs contributor or maintainer live-proof follow-up and a maintainer choice among same-root-cause PRs, not an automated code repair.

Security
Cleared: The diff changes Discord TypeScript send logic and tests only; no dependency, workflow, permission, secret, package, or code-download surface changed.

Review details

Best possible solution:

Land one source-aware Discord fix after redacted live Discord proof confirms implicit first/batched chunks quote only the first physical message while explicit/all and the existing audioAsVoice threading behavior remain intact.

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

Yes. Current-main source repeats replyTo through Discord text/media chunks and the linked issue includes shipped-version instrumentation showing repeated message_reference values; I did not run a live Discord repro in this read-only review.

Is this the best way to solve the issue?

Yes for the code path. The PR keeps the fix in the Discord plugin owner boundary and uses source plus mode to preserve explicit/all behavior; the remaining blockers are live proof and selecting one same-root-cause PR to land.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR targets a deterministic Discord delivery bug with user-visible notification impact, but the blast radius is channel-specific.
  • merge-risk: 🚨 message-delivery: The diff changes native Discord reply references across visible message chunks, and green CI does not prove real Discord notification behavior.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
  • 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 includes terminal output from actual sendMessageDiscord calls with a local RequestClient capture, but no live Discord delivery or API readback proof; contributors should add redacted screenshots, recordings, terminal/live output, linked artifacts, or logs and update the PR body for re-review.
Evidence reviewed

PR surface:

Source +67, Tests +44. Total +111 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 4 74 7 +67
Tests 2 48 4 +44
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 122 11 +111

What I checked:

  • Repository policy read: Root AGENTS.md and the scoped extensions policy were read; their whole-path review, plugin-boundary, and real-behavior-proof guidance affected this review. (AGENTS.md:7)
  • Current-main bug surface: On current main, sendDiscordText passes the closed-over replyTo into every text chunk request, and sendDiscordMedia passes the same replyTo into trailing text chunks. (extensions/discord/src/send.shared.ts:326, 3f289fdb4042)
  • Current request builder: buildDiscordMessageRequest emits Discord message_reference whenever params.replyTo is present, so repeated lower-level replyTo values become repeated native Discord replies. (extensions/discord/src/send.message-request.ts:90, 3f289fdb4042)
  • Reply contract: The Discord docs say explicit reply tags remain honored and first mode attaches the implicit native reply reference to the first outbound Discord message for the turn. Public docs: docs/channels/discord.md. (docs/channels/discord.md:659, 3f289fdb4042)
  • Shared reply policy: The shared reply policy distinguishes explicit from implicit reply ids and consumes single-use implicit ids only once. (src/infra/outbound/reply-policy.ts:55, 3f289fdb4042)
  • PR source-aware chunk gate: At PR head, Discord chunks keep replyTo on the first chunk or when the policy is not an implicit single-use reply; media follow-up chunks drop replyTo only for that same implicit single-use policy. (extensions/discord/src/send.shared.ts:302, 7545a66a7155)

Likely related people:

  • steipete: GitHub path history shows repeated Discord send and outbound reply policy work, including preserved outbound reply threading and shared reply fanout policy. (role: feature-history owner; confidence: high; commits: 5e640b93dae6, 7ef4ecf499ce, f550aa7622ce; files: extensions/discord/src/send.shared.ts, extensions/discord/src/outbound-payload.ts, src/infra/outbound/reply-policy.ts)
  • nxmxbbd: Merged PR history for Discord audioAsVoice reply threading intentionally introduced the captured-reply behavior that this PR preserves. (role: audioAsVoice behavior owner; confidence: high; commits: 99f56cd548c0, f02406c9dcba; files: extensions/discord/src/outbound-payload.ts, extensions/discord/src/outbound-adapter.test.ts)
  • vincentkoc: Recent GitHub path history includes Discord outbound payload option refactoring near the reviewed adapter/payload surface. (role: recent adjacent contributor; confidence: medium; commits: 94df665cdcc9; files: extensions/discord/src/outbound-payload.ts, extensions/discord/src/outbound-adapter.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: 🧂 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 2, 2026
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Superseded by #100784, landed as bfb89d3ea6531c27c6d91402c0a4a7a8462b7f3f.

The landed fix uses one source-aware { messageId, scope } model across text, media, components, voice, retries, fallbacks, and receipts. It passed focused Discord tests, extension type/lint gates, autoreview, and a live Discord API proof showing implicit-first references [true, false] versus reusable-all [true, true].

Thanks @LiLan0125 for the implementation and tests on this bug.

@steipete steipete closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: discord Channel integration: discord 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 status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Discord: replyToMode "first" — chunked reply sends the native reply reference on every physical chunk (reply-notification spam)

2 participants