fix(discord): messages sent during gateway reconnect are silently dropped by tiffanychum · Pull Request #100896 · openclaw/openclaw

tiffanychum · 2026-07-06T13:10:47Z

What Problem This Solves

Discord REST requests can fail with statusless transport errors while the Gateway websocket is reconnecting. The normal request retry budget can expire before the reconnect settles, dropping a reply even though the outage is brief.

The earlier version retried whole deliveries and drained durable delivery rows on reconnect. That boundary was unsafe: one delivery can contain several REST writes, so replaying it can duplicate chunks that Discord already accepted. Durable rows in send_attempt_started also cannot be replayed blindly without Discord reconciliation; #100979 remains the owner for that separate full-outage problem.

Why This Change Was Made

This revision keeps one canonical retry boundary: the individual Discord REST request.

The existing request runner receives the account's live Gateway connectivity state.
A configured multi-attempt policy gets two bounded extra attempts only after a disconnect was observed and only for statusless transient transport failures.
HTTP and rate-limit failures do not gain reconnect-only attempts.
An explicit single-attempt policy remains single-attempt.
All delivery-level replay wrappers and reconnect-drain lifecycle changes are removed.

No new config, protocol, or core surface is added.

User Impact

Short Discord reconnects get additional time to finish the current REST operation without replaying earlier writes from a multi-chunk or multi-media delivery. Partial delivery remains explicit when a later platform write fails. Full-outage durable reconciliation remains tracked by #100979.

Evidence

Focused regression run after rebase: node scripts/run-vitest.mjs extensions/discord/src/durable-delivery.test.ts extensions/discord/src/retry.test.ts extensions/discord/src/client.test.ts extensions/discord/src/outbound-adapter.test.ts extensions/discord/src/outbound-adapter.interactive-order.test.ts extensions/discord/src/outbound-payload.contract.test.ts — 6 files, 79 tests passed.
Coverage proves the request runner extends statusless transport retries only after an observed disconnect, remembers recovery during the request, preserves HTTP/configured attempt bounds, honors one attempt, and never replays an injected delivery or an already-sent earlier chunk.
Fresh branch autoreview after rebase: no accepted or actionable findings; patch judged correct at 0.90 confidence.
Live reconnect proof on behavior-equivalent pre-rebase head 882eb2fe138023f478e7be774af9c0686769fbca on the dedicated Discord QA host: blocked all resolved Discord REST/Gateway IPs until both bot gateways reported disconnected; the iptables chain rejected 77 TCP packets. The agent produced nonce PR100896-REQUEST-RETRY-1783371460 at 20:58:05.733 UTC, the block was removed 2.3 seconds later (after the configured three-attempt window), and the fourth request attempt delivered at 20:58:09.508 UTC. CLI delivery status was sent; an independent Mantis-account read found message 1523795212927897640 exactly once. The test chain was removed and both accounts reconnected. The subsequent rebase only incorporated unrelated main changes.

clawsweeper · 2026-07-06T13:28:26Z

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 5:46 PM ET / 21:46 UTC.

Summary
The branch replaces Discord delivery-level retry/replay with bounded Gateway-aware per-REST-request retry, removes the delivery retry helper, updates Discord outbound tests, and adds a changelog line.

PR surface: Source -68, Tests +45, Docs +1. Total -22 across 11 files.

Reproducibility: yes. Current main lacks Gateway-disconnected retry input for Discord REST requests, and the PR discussion includes a live reconnect proof showing the after-fix nonce delivered exactly once; I did not run a new live Discord repro in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #56610
Summary: This PR is the active candidate fix for the short Discord Gateway reconnect message-loss problem, while the full-outage durable recovery gap is a related but separate remaining issue.

Members:

canonical: [Feature]: Discord delivery queue / retry on WebSocket reconnect (parity with WhatsApp) #56610 - The PR body uses closing syntax for this issue and the code changes target its short Gateway reconnect message-loss path.
partial_overlap: [Bug]: Discord sends failed during a full network outage are dropped on reconnect (send_attempt_started refuses replay, no reconcileUnknownSend) #100979 - This issue documents the full-network-outage send_attempt_started recovery gap that the PR explicitly leaves for separate work.
partial_overlap: fix(outbound): retry proven pre-connect failures #101024 - This open PR targets the separate full-outage recovery issue, not the request-level short reconnect retry fixed here.
partial_overlap: fix(discord): add reconcileUnknownSend adapter for durable delivery recovery (#100979) #100997 - This open PR is an alternate candidate for the separate Discord reconciliation gap and does not replace this short reconnect fix.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Risk before merge

[P1] This PR intentionally changes Discord message-delivery retry boundaries; if the statusless transport classification is wrong, Discord sends could still be dropped or retried in a way that affects duplicate protection.
[P0] The full-network-outage send_attempt_started recovery path remains separate at [Bug]: Discord sends failed during a full network outage are dropped on reconnect (send_attempt_started refuses replay, no reconcileUnknownSend) #100979, so merging this PR should not be treated as closing that remaining gap.
[P1] The branch edits CHANGELOG.md even though release generation owns changelog text; this is a release-policy cleanup concern rather than a correctness blocker.

Maintainer options:

Land scoped request retry (recommended)
Merge after exact-head checks if maintainers accept the bounded per-request reconnect retry and keep full-outage recovery separate.
Pause for combined recovery
Hold this PR only if maintainers want one change to also solve the full-outage send_attempt_started recovery path.
Trim release-owned changelog
Drop the CHANGELOG.md line before merge if maintainers want release generation to remain the only changelog writer.

Next step before merge

[P2] No repair lane is needed; the remaining action is maintainer merge review of the scoped message-delivery risk, exact-head checks, and optional release-owned changelog cleanup.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes Discord plugin retry/outbound code, tests, and changelog text without new dependencies, workflows, permissions, or secret handling.

Review details

Best possible solution:

Land the scoped request-level retry if maintainers accept the message-delivery risk, keep full-outage reconciliation under #100979, and avoid restoring delivery-level replay.

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

Yes. Current main lacks Gateway-disconnected retry input for Discord REST requests, and the PR discussion includes a live reconnect proof showing the after-fix nonce delivered exactly once; I did not run a new live Discord repro in this read-only review.

Is this the best way to solve the issue?

Yes. Per-REST-request retry is the narrowest maintainable fix for short reconnect windows because a delivery can contain multiple platform writes, while the full-outage durable recovery case belongs to the separate reconciliation work.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR fixes bounded but real Discord outbound message loss during short reconnect windows without evidence of emergency runtime-wide impact.
merge-risk: 🚨 message-delivery: The patch changes Discord retry boundaries for outbound sends, where an incorrect retry decision could drop or duplicate messages.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The PR includes after-fix live Discord reconnect proof with a unique nonce delivered exactly once, plus focused regression output and a passing Real behavior proof check on the current head.
proof: sufficient: Contributor real behavior proof is sufficient. The PR includes after-fix live Discord reconnect proof with a unique nonce delivered exactly once, plus focused regression output and a passing Real behavior proof check on the current head.

Evidence reviewed

PR surface:

Source -68, Tests +45, Docs +1. Total -22 across 11 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	6	173	241	-68
Tests	4	121	76	+45
Docs	1	1	0	+1
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	11	295	317	-22

What I checked:

Repository policy applied: Root policy was read fully and affected the review by requiring whole-path review, best-fix judgment, scoped extension rules, and explicit message-delivery merge-risk handling. (AGENTS.md:29, e80e8a2b6730)
Repository policy final chunk read: The final AGENTS.md chunk was read, completing the full 287-line policy review before verdict. (AGENTS.md:261, e80e8a2b6730)
Scoped extension policy applied: The Discord plugin change stays inside the bundled plugin boundary and uses SDK imports, matching the scoped extensions guidance. (extensions/AGENTS.md:27, e80e8a2b6730)
Current main lacks the requested retry signal: Current main creates the Discord retry runner without any Gateway connectivity callback, so it cannot extend request retries based on an observed reconnect window. (extensions/discord/src/client.ts:147, e80e8a2b6730)
PR adds the request-boundary retry: The PR head passes live Gateway connectivity into the Discord retry runner and adds two extra attempts only for multi-attempt policies after a Gateway disconnect is observed. (extensions/discord/src/retry.ts:95, 331ba3d2486c)
PR avoids delivery-level replay: The PR head removes the outer Discord delivery retry wrapper from outbound sends, so an already-sent chunk is not replayed when a later chunk fails. (extensions/discord/src/outbound-adapter.ts:198, 331ba3d2486c)

Likely related people:

steipete: Recent GitHub history shows repeated commits in Discord retry, client, outbound adapter, outbound payload, and shared delivery recovery paths, and the live PR is assigned to this account. (role: recent Discord retry/outbound contributor and likely follow-up owner; confidence: high; commits: 2644f26a3514, a6ccb5f698df, bfb89d3ea653; files: extensions/discord/src/retry.ts, extensions/discord/src/client.ts, extensions/discord/src/outbound-adapter.ts)
zhangguiping-xydt: Recent main history includes outbound recovery work affecting replay of already-sent replies in the same durable delivery and Discord outbound boundary. (role: recent shared outbound recovery contributor; confidence: medium; commits: 741d9524bec2; files: extensions/discord/src/outbound-adapter.ts, extensions/discord/src/outbound-payload.ts, src/infra/outbound/delivery-queue-recovery.ts)
vincentkoc: Recent commits touched shared recovery primitives and Discord outbound payload option sharing near the reviewed retry and delivery boundary. (role: adjacent delivery and Discord outbound contributor; confidence: medium; commits: 94df665cdcc9, c782fa98aa88; files: extensions/discord/src/outbound-payload.ts, src/infra/outbound/delivery-queue-recovery.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 (4 earlier review cycles)

reviewed 2026-07-06T13:32:58.184Z sha 4820a3a :: needs real behavior proof before merge. :: none
reviewed 2026-07-06T16:13:36.719Z sha 4820a3a :: needs changes before merge. :: [P2] Prevent initial READY from firing the reconnect drain
reviewed 2026-07-06T20:58:21.138Z sha 882eb2f :: needs real behavior proof before merge. :: none
reviewed 2026-07-06T21:08:22.865Z sha 882eb2f :: needs maintainer review before merge. :: none

tiffanychum · 2026-07-06T15:31:10Z

Live behavior proof added (the one gap the earlier review flagged).

Setup: real Discord bot + test guild, gateway from this branch on macOS with an isolated state dir, real network cut via Wi-Fi toggle (no mocks, no harness).

Scenario (issue #56610 headline path — send attempted while the websocket is down):

23:19:25 wifi OFF
23:21:03 discord gateway: Gateway websocket closed: 1006     <- reconnect window begins
23:21:05 openclaw message send started (gateway disconnected)
23:21:15 wifi ON (send still in flight)
23:21:38 send finished exit=0 — in-line retry rode out the window
23:21:41 Discord REST API: nonce-tagged message present in channel exactly once

The send took 33s versus ~2s for a baseline connected send, i.e. the first attempt(s) failed as statusless transport errors and the new disconnected-gateway retry carried it across the reconnect. Delivered messageId 1523710525924708412, verified exactly-once by polling channel messages for the unique nonce. After READY, the Discord reconnect drain hook fired (also observed firing for real in a separate run, log below).

Second, harsher run (full outage until in-line retries exhaust) exposed a pre-existing gap outside this PR's scope: the queued entry is marked send_attempt_started at platform-send start, and the drain — which did fire — is correctly stopped by the delivery queue's blind-replay guard because Discord has no reconcileUnknownSend adapter:

22:50:06 discord gateway: Gateway websocket opened
22:50:07 [discord] Discord reconnect drain: retry failed for entry 155a9da3-…: delivery state is send_attempt_started; refusing blind replay without adapter reconciliation

Filed as #100979 with full logs and two candidate fixes (connect-phase error classification in core, or a Discord reconcileUnknownSend built on nonce/enforce_nonce). PR body Evidence section updated with both runs.

steipete · 2026-07-06T21:00:07Z

Maintainer refactor and exact-head proof complete.

Replaced whole-delivery replay with one bounded retry policy at the Discord REST-request boundary. This prevents earlier chunks/media writes from being duplicated when a later write fails.
Extra reconnect attempts apply only after an observed Gateway disconnect and only to statusless transient transport failures. HTTP/rate-limit bounds and explicit attempts: 1 remain unchanged.
Focused post-rebase run: 6 files, 79 tests passed (durable-delivery, retry, client, outbound adapter/order, and outbound payload contract).
Fresh branch autoreview: no accepted/actionable findings; correct at 0.90 confidence.
Exact-head live Discord proof: with both bot gateways disconnected and 77 TCP packets rejected, nonce PR100896-REQUEST-RETRY-1783371460 entered delivery at 20:58:05.733 UTC. The block remained through the configured three-attempt window, was removed 2.3 seconds later, and the fourth request attempt delivered at 20:58:09.508 UTC. CLI status: sent. Independent Mantis read: message 1523795212927897640, exactly one occurrence. Test firewall chain removed; both accounts reconnected.

The longer full-outage send_attempt_started reconciliation problem is intentionally not replayed here and remains tracked by #100979.

openclaw#56610) Discord sends that failed while the gateway websocket was reconnecting (close codes 1005/1006) stayed stuck in the outbound delivery queue and were silently dropped. Classify sends that fail while the registered gateway is disconnected as retryable, and drain pending discord queue entries when the gateway reconnects, matching the WhatsApp/Telegram reconnect drains.

Co-authored-by: tiffanychum <[email protected]>

steipete · 2026-07-06T22:56:21Z

Merged via squash.

Prepared head SHA: cc8daea215d248ae7282d831131860f26f43b3bd
Landed commit: da0103f025445d4e796966274f3ec2b835b92596

…pped (openclaw#100896) * fix(discord): drain queued outbound deliveries after gateway reconnect (openclaw#56610) Discord sends that failed while the gateway websocket was reconnecting (close codes 1005/1006) stayed stuck in the outbound delivery queue and were silently dropped. Classify sends that fail while the registered gateway is disconnected as retryable, and drain pending discord queue entries when the gateway reconnects, matching the WhatsApp/Telegram reconnect drains. * fix(discord): harden reconnect recovery Co-authored-by: tiffanychum <[email protected]> * refactor(discord): remove unsafe reconnect replay * fix(discord): bound reconnect retry per request Co-authored-by: tiffanychum <[email protected]> * chore(changelog): leave reconnect fix to release flow --------- Co-authored-by: Peter Steinberger <[email protected]>

…pped (openclaw#100896) * fix(discord): drain queued outbound deliveries after gateway reconnect (openclaw#56610) Discord sends that failed while the gateway websocket was reconnecting (close codes 1005/1006) stayed stuck in the outbound delivery queue and were silently dropped. Classify sends that fail while the registered gateway is disconnected as retryable, and drain pending discord queue entries when the gateway reconnects, matching the WhatsApp/Telegram reconnect drains. * fix(discord): harden reconnect recovery Co-authored-by: tiffanychum <[email protected]> * refactor(discord): remove unsafe reconnect replay * fix(discord): bound reconnect retry per request Co-authored-by: tiffanychum <[email protected]> * chore(changelog): leave reconnect fix to release flow --------- Co-authored-by: Peter Steinberger <[email protected]> (cherry picked from commit da0103f)

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

tiffanychum mentioned this pull request Jul 6, 2026

[Bug]: Discord sends failed during a full network outage are dropped on reconnect (send_attempt_started refuses replay, no reconcileUnknownSend) #100979

Closed

This was referenced Jul 6, 2026

fix(discord): add reconcileUnknownSend adapter for durable delivery recovery (#100979) #100997

Closed

fix(outbound): retry proven pre-connect failures #101024

Merged

steipete self-assigned this Jul 6, 2026

openclaw-barnacle Bot added size: M and removed size: S labels Jul 6, 2026

steipete force-pushed the feat/56610-discord-reconnect-delivery-drain branch from 1a00be4 to d5b1fcb Compare July 6, 2026 20:09

openclaw-barnacle Bot added size: S and removed size: M labels Jul 6, 2026

steipete force-pushed the feat/56610-discord-reconnect-delivery-drain branch from d5b1fcb to 882eb2f Compare July 6, 2026 20:51

openclaw-barnacle Bot added size: L and removed size: S labels Jul 6, 2026

tiffanychum and others added 4 commits July 6, 2026 22:15

fix(discord): harden reconnect recovery

bb98b25

Co-authored-by: tiffanychum <[email protected]>

refactor(discord): remove unsafe reconnect replay

06422ba

fix(discord): bound reconnect retry per request

331ba3d

Co-authored-by: tiffanychum <[email protected]>

steipete force-pushed the feat/56610-discord-reconnect-delivery-drain branch from 882eb2f to 331ba3d Compare July 6, 2026 21:15

chore(changelog): leave reconnect fix to release flow

cc8daea

steipete merged commit da0103f into openclaw:main Jul 6, 2026
201 checks passed

github-actions Bot mentioned this pull request Jul 6, 2026

📡 Upstream Digest — 2026-07-06 22:59 UTC curtismercier/openclaw-mods#1226

Open

This was referenced Jul 10, 2026

fix(discord): stop retrying non-idempotent sends on post-connect-ambiguous errors #103859

Closed

fix(discord): stop retrying non-idempotent sends on post-connect-ambiguous errors #103867

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(discord): messages sent during gateway reconnect are silently dropped#100896

fix(discord): messages sent during gateway reconnect are silently dropped#100896
steipete merged 5 commits into
openclaw:mainfrom
tiffanychum:feat/56610-discord-reconnect-delivery-drain

tiffanychum commented Jul 6, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

tiffanychum commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

tiffanychum commented Jul 6, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiffanychum commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tiffanychum commented Jul 6, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading