fix(discord): add reconcileUnknownSend adapter for durable delivery recovery (#100979)#100997
fix(discord): add reconcileUnknownSend adapter for durable delivery recovery (#100979)#100997ZOOWH wants to merge 5 commits into
Conversation
503ef9e to
74f704c
Compare
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close this PR as superseded: it is a valid-looking Discord-specific recovery attempt, but the same linked message-loss bug now has a cleaner proof-positive shared outbound recovery PR that owns the central fix without adding a new plugin SDK context field. Canonical path: Use #101024 as the canonical fix path, then open a narrower follow-up only if maintainers still want Discord nonce-based reconciliation or recovery for pre-existing stuck rows. So I’m closing this here and keeping the remaining discussion on #101024. Review detailsBest possible solution: Use #101024 as the canonical fix path, then open a narrower follow-up only if maintainers still want Discord nonce-based reconciliation or recovery for pre-existing stuck rows. Do we have a high-confidence way to reproduce the issue? Yes. Current main marks sends as Is this the best way to solve the issue? No. The latest patch fixed the timeout allowlist issue, but the shared core recovery PR is the cleaner canonical fix for the central pre-dispatch outage problem and avoids adding a parallel Discord-specific API/context path. Security review: Security review cleared: No concrete security or supply-chain concern was found; the diff changes TypeScript recovery logic, tests, and snapshots without new dependencies, workflows, secrets, or install-time execution. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against d5e9b4d1a32f. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…ecovery (openclaw#100979) When a Discord send fails during a network outage, the delivery queue entry is marked send_attempt_started. Without reconcileUnknownSend the reconnect drain refuses blind replay and the message is permanently dropped. Add a minimal reconcileUnknownSend adapter that returns not_sent, allowing the drain to safely retry. The common case is a network error where the message never reached Discord. Slack already has an equivalent adapter (extensions/slack/src/channel.ts:582-589).
… failure Only return not_sent for connection-phase errors (UND_ERR_CONNECT_TIMEOUT, ECONNREFUSED, ENOTFOUND, EAI_AGAIN, ENETUNREACH) that prove the HTTP request never reached Discord. Falls back to unresolved for unknown failures to preserve the duplicate-send safety guard. (openclaw#100979)
21e58cb to
c3f6c13
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
ETIMEDOUT can occur both before and after the HTTP request reaches Discord (response timeout after acceptance), so returning not_sent would risk duplicate messages. Keep it unresolved to preserve duplicate-send safety. (openclaw#100979)
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞✅ Reason: structured ClawSweeper close marker: close-required (sha=5699be8a4d3c7ce749c7e488c0428af2c257e4c6) Closed:
|
Fixes #100979
What Problem This Solves
When a Discord send fails during a full network outage, the delivery queue entry is marked
send_attempt_started. On gateway reconnect, the drain finds this entry but Discord has noreconcileUnknownSendadapter — so it refuses blind replay and permanently drops the message.Slack already has this adapter (
extensions/slack/src/channel.ts:582-589). Discord was missing it.Why This Change Was Made
Added
durableFinal.reconcileUnknownSendto Discord's message adapter with a safety gate — only returnsnot_sentwhen the last recorded error is a connection-phase failure that proves the HTTP request never reached Discord:PRE_DISPATCH_ERROR_REmatches:UND_ERR_CONNECT_TIMEOUT,ECONNREFUSED,ENOTFOUND,EAI_AGAIN,ENETUNREACH,ERR_CONNECT_TIMEOUT,EADDRNOTAVAIL,ETIMEDOUTunresolvedwhen the error is ambiguous (e.g. HTTP 500, or no error at all after a process crash) to preserve the duplicate-send guardlastErrortoChannelMessageUnknownSendContextso adapters can make informed decisionsThe
not_sentverdict for connection-phase errors is correct because the HTTP request never reached Discord.Evidence
Proof — Live outage/recovery on production gateway:
Tests:
declares reconcileUnknownSend adapter for reconnect drain recoveryreturns not_sent when lastError is a connection-phase failurereturns unresolved when lastError is not a connection-phase errorreturns unresolved when lastError is missing and retryCount > 0Pre-submit: