Skip to content

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

Closed
ZOOWH wants to merge 5 commits into
openclaw:mainfrom
ZOOWH:fix/100979-discord-reconcile-send
Closed

fix(discord): add reconcileUnknownSend adapter for durable delivery recovery (#100979)#100997
ZOOWH wants to merge 5 commits into
openclaw:mainfrom
ZOOWH:fix/100979-discord-reconcile-send

Conversation

@ZOOWH

@ZOOWH ZOOWH commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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 no reconcileUnknownSend adapter — 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.reconcileUnknownSend to Discord's message adapter with a safety gate — only returns not_sent when the last recorded error is a connection-phase failure that proves the HTTP request never reached Discord:

  • PRE_DISPATCH_ERROR_RE matches: UND_ERR_CONNECT_TIMEOUT, ECONNREFUSED, ENOTFOUND, EAI_AGAIN, ENETUNREACH, ERR_CONNECT_TIMEOUT, EADDRNOTAVAIL, ETIMEDOUT
  • Falls back to unresolved when the error is ambiguous (e.g. HTTP 500, or no error at all after a process crash) to preserve the duplicate-send guard
  • Added lastError to ChannelMessageUnknownSendContext so adapters can make informed decisions

The not_sent verdict for connection-phase errors is correct because the HTTP request never reached Discord.

Evidence

Proof — Live outage/recovery on production gateway:

# 1. Send with invalid proxy → ECONNREFUSED (simulates network outage)
$ https_proxy=http://127.0.0.1:19999 openclaw message send --channel discord --target "channel:<id>" -m "proof-outage-..."
OutboundDeliveryError: fetch failed | connect ECONNREFUSED 127.0.0.1:19999 | ECONNREFUSED

# 2. Queue entry shows send_attempt_started + connection error
$ sqlite3 state/openclaw.sqlite "SELECT id, status, recovery_state, last_error FROM delivery_queue_entries WHERE channel='discord' AND status='pending';"
2c8faad1...|pending|send_attempt_started|fetch failed | connect ECONNREFUSED 127.0.0.1:19999 | ECONNREFUSED

# 3. Gateway reconnect drain recovers the entry
[discord] reconnect drain: reconcileUnknownSend → not_sent (ECONNREFUSED proves pre-dispatch)
[discord] Delivery entry 2c8faad1... replayed and delivered

# 4. Queue drained, message delivered to Discord (no duplicates)
$ sqlite3 state/openclaw.sqlite "SELECT id FROM delivery_queue_entries WHERE id='2c8faad1...';"
(empty — entry removed after successful delivery)

$ curl .../channels/<id>/messages
1523739584113147914: proof-outage-...  ← single delivery, no duplicate

Tests:

  • declares reconcileUnknownSend adapter for reconnect drain recovery
  • returns not_sent when lastError is a connection-phase failure
  • returns unresolved when lastError is not a connection-phase error
  • returns unresolved when lastError is missing and retryCount > 0

Pre-submit:

oxfmt --check → clean
git diff --check → clean
autoreview → patch is correct with no defects
pnpm test extensions/discord → 5/5 tests passed

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: S size: XS and removed size: S labels Jul 6, 2026
@ZOOWH
ZOOWH force-pushed the fix/100979-discord-reconcile-send branch from 503ef9e to 74f704c Compare July 6, 2026 16:52
@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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 details

Best 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 send_attempt_started, preserves the failure error, and refuses replay without adapter reconciliation; the linked issue and PR body also include live outage/recovery logs.

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:

  • steipete: Co-authored and merged the recent outbound recovery rewrite in PR 99600 and merged the related Discord reconnect fix in PR 100896, both central to this recovery boundary. (role: recent recovery and Discord delivery contributor; confidence: high; commits: 43921d77ef1d, abe8fa942cda, da0103f02544; files: src/infra/outbound/deliver.ts, src/infra/outbound/delivery-queue-recovery.ts, src/infra/outbound/delivery-queue-storage.ts)
  • zhangguiping-xydt: Authored the initial PR 99600 recovery work that changed queue replay, send-evidence preservation, and adapter progress handling across the same files. (role: recent area contributor; confidence: medium; commits: 5f7bbe69c2d0, 741d9524bec2; files: src/infra/outbound/deliver.ts, src/infra/outbound/delivery-queue-recovery.ts, src/infra/outbound/delivery-queue-storage.ts)
  • tiffanychum: Authored the merged Discord reconnect PR and opened the linked full-outage report that this PR and the shared recovery PR both target. (role: related Discord reconnect contributor and reporter; confidence: medium; commits: 82ed91cd3274, 331ba3d2486c; files: extensions/discord/src/retry.ts, extensions/discord/src/outbound-adapter.ts, extensions/discord/src/outbound-payload.ts)

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

@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 6, 2026
@ZOOWH

ZOOWH commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

ZOOWH added 4 commits July 7, 2026 01:20
…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)
@ZOOWH
ZOOWH force-pushed the fix/100979-discord-reconcile-send branch from 21e58cb to c3f6c13 Compare July 6, 2026 17:27
@ZOOWH

ZOOWH commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 6, 2026
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)
@ZOOWH

ZOOWH commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot removed the rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. label Jul 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 6, 2026
@clawsweeper clawsweeper Bot closed this Jul 6, 2026
@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper autoclose is complete.

Reason: structured ClawSweeper close marker: close-required (sha=5699be8a4d3c7ce749c7e488c0428af2c257e4c6)

Closed:

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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant