Skip to content

fix(discord): preserve text after oversized uploads#99577

Merged
steipete merged 1 commit into
openclaw:mainfrom
lin-hongkuan:lhk/fix-discord-oversize-attachment-fallback-99021
Jul 6, 2026
Merged

fix(discord): preserve text after oversized uploads#99577
steipete merged 1 commit into
openclaw:mainfrom
lin-hongkuan:lhk/fix-discord-oversize-attachment-fallback-99021

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Closes #99021

What Problem This Solves

Discord sends captions and attachments in one multipart request. When Discord rejects an oversized upload, the caption is rejected with it, leaving a completed OpenClaw delivery with no visible Discord message.

Why This Change Was Made

Discord upload limits vary by guild/account, so the fix stays at the last-mile send boundary instead of hard-coding a size cap. It recognizes Discord API error 40005 and HTTP 413, then routes the original text through the canonical text sender with a visible skipped-attachment note. The fallback deliberately omits attachment-coupled embeds and components; unrelated failures still propagate.

User Impact

Text remains visible when Discord rejects an oversized attachment. Media-only sends receive an explicit skipped-attachment notice instead of silently disappearing. Reply references, text chunking, silent delivery, embed suppression, delivery receipts, and callbacks continue through the existing text path.

Evidence

  • Blacksmith Testbox run 28777362690: focused Discord send suite, 48/48 tests passed; targeted oxfmt passed.
  • Blacksmith Testbox run 28777473595: pnpm check:changed passed, including extension typechecks, lint, import-cycle checks, and repository guards.
  • Blacksmith Testbox run 28777770371: live local HTTP-boundary proof sent a real multipart request through the Discord REST client, returned API code 40005, and observed exactly one text fallback with the original text, reply reference, no files, and the fallback message ID.
  • Fresh Codex autoreview: no accepted/actionable findings.
  • git diff --check: passed.

No external Discord channel was mutated by the live proof; the target was an isolated HTTP boundary using Discord's documented error contract.

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

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 6:21 PM ET / 22:21 UTC.

Summary
The branch catches Discord oversized media upload errors in the send path, retries a text-only fallback with an upload-skipped note, and adds mocked REST-boundary tests.

PR surface: Source +52, Tests +45. Total +97 across 2 files.

Reproducibility: yes. at source level: current main sends caption text and the file in one multipart Discord request, and the linked issue has production HTTP 413 logs for that failure class. I did not run a live Discord upload in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99021
Summary: This PR is one candidate fix for the canonical Discord oversized-attachment HTTP 413 message-loss issue; sibling PRs propose overlapping fallback and accounting shapes.

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: 🦪 silver shellfish
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:

  • [P2] Add redacted live Discord proof showing an oversized media reply posts the text fallback and upload-skipped note without exposing private IDs, tokens, endpoints, phone numbers, or other private details.
  • [P2] Fix fallback result accounting so stripped-media text delivery is recorded as text or an explicit stripped-attachment state.
  • Coordinate with the sibling candidate PRs so only one canonical implementation lands.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists mocked REST-boundary tests and says no live Discord upload was attempted, so the contributor should add redacted live Discord proof and update the PR body for re-review.

Mantis proof suggestion
A visible Discord transcript or desktop recording would materially prove the transport-visible fallback that the PR currently exercises only with mocks. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis discord thread attachment: verify an oversized Discord attachment reply sends the text fallback and upload-skipped note without the file.

Risk before merge

  • [P1] The PR body still has only mocked REST-boundary proof and explicitly says no live Discord upload was attempted.
  • [P2] The fallback can deliver text after a 413 while the final send receipt still classifies the logical send as media.
  • [P1] Several candidate PRs target the same Discord oversized-upload message-loss bug, so maintainers still need one canonical implementation before landing or closing siblings.

Maintainer options:

  1. Fix Receipt Semantics And Add Live Proof (recommended)
    Update the fallback so the delivered text-only message is recorded as text or an explicit stripped-attachment state, then attach redacted live Discord proof of a real oversized upload falling back to text.
  2. Select Another Candidate Path
    Pause this PR if maintainers prefer an overlapping implementation, then close duplicate candidates only after one canonical path is selected or merged.
  3. Accept Mock-Only Transport Proof
    Maintainers could intentionally merge with mocked REST-boundary coverage only, accepting that the real Discord upload rejection path remains unproven here.

Next step before merge

  • [P1] Contributor proof and maintainer canonical-selection are still needed; automation cannot supply the author's real Discord setup proof or choose among overlapping PRs.

Maintainer decision needed

  • Question: Should this PR become the canonical fix for the linked Discord oversized-upload message-loss issue after fixing receipt semantics and proof, or should maintainers adopt a sibling/replacement implementation instead?
  • Rationale: Multiple open candidate PRs use different fallback and receipt-accounting shapes; code review can identify the remaining blocker but cannot choose the maintainer-owned canonical landing path.
  • Likely owner: steipete — The history points most strongly to this owner for Discord send-path and media-cap decisions.
  • Options:
    • Keep This PR After Fixes (recommended): Require this PR to return text or explicit stripped-media receipts and add redacted live Discord proof before treating it as canonical.
    • Adopt A Sibling Implementation: Use an overlapping PR such as fix(discord): strip attachments >10MB before send to prevent HTTP 413 silent message loss (fixes #99021) #99053 if maintainers prefer its broader receipt/proof approach despite its current conflict state.
    • Ask For A Clean Replacement: Pause the duplicate candidates and request one narrow replacement that combines the preferred fallback, receipt, and live-proof shape.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes Discord send-path TypeScript and focused tests only.

Review findings

  • [P2] Return a text receipt for oversized-upload fallbacks — extensions/discord/src/send.shared.ts:463
Review details

Best possible solution:

Land one canonical Discord-owned 413 fallback that preserves visible text, records text-only stripped-media delivery accurately, includes redacted live Discord proof, and closes duplicate candidates after merge.

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

Yes, at source level: current main sends caption text and the file in one multipart Discord request, and the linked issue has production HTTP 413 logs for that failure class. I did not run a live Discord upload in this read-only review.

Is this the best way to solve the issue?

No, not yet. The Discord send-boundary fallback is the right owner area, but the PR needs accurate text or stripped-media receipt semantics and real Discord proof before it is the best merge path.

Full review comments:

  • [P2] Return a text receipt for oversized-upload fallbacks — extensions/discord/src/send.shared.ts:463
    This branch returns sendDiscordText after a 413, but sendMessageDiscord still classifies any mediaUrl send as kind: "media". The visible fallback would reach Discord as text while receipts and diagnostics can claim media delivery, so return an explicit text/stripped-media signal or handle the fallback where the receipt kind can be set correctly. This prior finding remains unresolved at the new head.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a linked Discord channel bug where oversized media can make an entire agent reply disappear for users.
  • merge-risk: 🚨 message-delivery: The diff changes Discord last-mile delivery behavior for failed media uploads and still has unsettled real-transport proof and receipt semantics.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • 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 lists mocked REST-boundary tests and says no live Discord upload was attempted, so the contributor should add redacted live Discord proof and update the PR body for re-review.
Evidence reviewed

PR surface:

Source +52, Tests +45. Total +97 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 56 4 +52
Tests 1 45 0 +45
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 101 4 +97

What I checked:

  • Repository policy read: Root AGENTS.md and extensions/AGENTS.md were read fully; the plugin-boundary, whole-path review, fallback-risk, and real-behavior-proof guidance apply to this Discord plugin send-path PR. (AGENTS.md:1, c40f5e068387)
  • Current main all-or-nothing media request: Current main builds one Discord message body with caption text plus files and posts it as a single channel message, so a 413 can reject the text and file together before any text-only retry exists. (extensions/discord/src/send.shared.ts:410, c40f5e068387)
  • PR fallback implementation: At PR head, the 413 catch returns sendDiscordText with the original text plus the upload-skipped note, so the visible fallback request itself is text-only. (extensions/discord/src/send.shared.ts:463, 4c1df049a242)
  • Receipt classification gap: At PR head, sendMessageDiscord still wraps the final result as kind media whenever opts.mediaUrl is set, even when sendDiscordMedia returned a text-only fallback result. (extensions/discord/src/send.outbound.ts:406, 4c1df049a242)
  • Mock-only PR tests: The added tests simulate a 413 and assert the fallback request body, but they do not assert the returned receipt kind or exercise a real Discord upload. (extensions/discord/src/send.sends-basic-channel-messages.test.ts:597, 4c1df049a242)
  • PR proof statement: The PR body explicitly says no live Discord upload was attempted, so real behavior proof is still missing for an external contributor PR. (4c1df049a242)

Likely related people:

  • steipete: History shows repeated Discord send-path refactors and the current split send files were added under Peter Steinberger's authorship; the default media-cap history also points to this area. (role: recent area contributor; confidence: high; commits: 1afb61f7b70d, 134d3095716e, 9832559f4505; files: extensions/discord/src/send.shared.ts, extensions/discord/src/send.outbound.ts, extensions/discord/src/outbound-adapter.ts)
  • vincentkoc: The Discord history includes prior Telegram/Discord mediaMaxMb upload handling and recent adjacent release stabilization touching Discord send history. (role: adjacent media/send contributor; confidence: medium; commits: 9c1786bdd6cc, e085fa1a3ffd; files: extensions/discord/src/send.shared.ts, extensions/discord/src/send.outbound.ts)
  • scoootscooob: History shows this contributor moved the Discord channel implementation into extensions and fixed generated image delivery to Discord, shaping the current plugin boundary. (role: feature migration contributor; confidence: medium; commits: 5682ec37fada, 24032dcc0ef7; files: extensions/discord/src/send.shared.ts, extensions/discord/src/send.outbound.ts)
  • Josh Avant: History shows a focused outbound configuration threading fix in the Discord send path, making this person relevant to send-result plumbing even though ownership is shared. (role: adjacent send-path contributor; confidence: medium; commits: 646817dd808b; files: extensions/discord/src/send.shared.ts, extensions/discord/src/send.outbound.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.
Review history (3 earlier review cycles)
  • reviewed 2026-07-03T14:39:02.149Z sha 46e22f6 :: needs real behavior proof before merge. :: [P2] Mark stripped-media fallbacks as text receipts
  • reviewed 2026-07-05T02:19:03.974Z sha bb1375b :: needs real behavior proof before merge. :: [P2] Return a text receipt for oversized-upload fallbacks
  • reviewed 2026-07-05T12:43:59.573Z sha 053da77 :: needs real behavior proof before merge. :: [P2] Return a text receipt for oversized-upload fallbacks

@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 3, 2026
@lin-hongkuan lin-hongkuan force-pushed the lhk/fix-discord-oversize-attachment-fallback-99021 branch from 46e22f6 to bb1375b Compare July 5, 2026 02:11
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 5, 2026
lin-hongkuan added a commit to lin-hongkuan/openclaw that referenced this pull request Jul 5, 2026
@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. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 5, 2026
lin-hongkuan added a commit to lin-hongkuan/openclaw that referenced this pull request Jul 5, 2026
@steipete steipete self-assigned this Jul 6, 2026
Co-authored-by: lin-hongkuan <lin-hongkuan@users.noreply.github.com>
@steipete steipete force-pushed the lhk/fix-discord-oversize-attachment-fallback-99021 branch from 4c1df04 to 5c4d645 Compare July 6, 2026 08:23
@steipete steipete merged commit 9607c3a into openclaw:main Jul 6, 2026
92 of 93 checks passed
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: lin-hongkuan <lin-hongkuan@users.noreply.github.com>
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: lin-hongkuan <lin-hongkuan@users.noreply.github.com>
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. P1 High-priority user-facing bug, regression, or broken workflow. 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 reply with >10MB attachment fails 413 and is silently lost — text discarded too, dispatch still reports outcome=completed

2 participants