fix(outbound): retry proven pre-connect failures by SunnyShu0925 · Pull Request #101024 · openclaw/openclaw

SunnyShu0925 · 2026-07-06T16:54:36Z

What Problem This Solves

Fixes #100979. A queued Discord or other channel send that fails before connecting to the platform can retain send_attempt_started; recovery then treats it as an unknown send and refuses replay even though no request left the host.

Why This Change Was Made

The original patch cleared queue evidence in a separate mutation and classified only the terminal error. That left split-state failure windows, omitted best-effort and recovery branches, and could erase evidence when an adapter's final safe-looking retry hid an earlier ambiguous attempt.

This maintainer rewrite:

adds one atomic queue transition that records failure metadata while clearing both platform-send evidence fields;
shares one structured classifier across live delivery and recovery;
retains all exhausted internal retry errors without replacing the public terminal error;
requires every retry attempt and every aggregate transport leaf to prove DNS/connect failure;
preserves the duplicate-send guard for resets, read/write timeouts, mixed batches, opaque wrappers, and any known/possible platform result.

The classifier follows actual Node/Undici and Slack SDK wrapper shapes. It does not use error-message inference, add config, or special-case Discord policy in core.

User Impact

Messages queued during a full network outage can replay after connectivity returns when the runtime can prove the platform connection was never established. Truly ambiguous failures remain guarded against blind replay and duplicate messages.

Evidence

Blacksmith Testbox tbx_01kwxh9xadqgh8a3yrdafdy52b: 210 focused core tests passed across retry, live delivery, SQLite storage, recovery, and queue-to-replay integration.
Same Testbox: 21 Discord retry tests passed.
Same Testbox: final pnpm check:changed passed for core, core-test, and docs lanes.
Live Node 24 probe reproduced the production dual-stack shape: TypeError -> AggregateError with summary ECONNREFUSED -> two ECONNREFUSED/connect children.
Fresh autoreview: clean; no accepted or actionable findings.
git diff --check: passed.

The linked issue contains the credentialed real Discord Wi-Fi-cut reproduction. This maintainer pass did not repeat that bot send; it validated the exact runtime error graph, persisted queue transition, and eventual replay path on Testbox.

Co-authored-by: 0668000539 [email protected]

clawsweeper · 2026-07-06T17:08:38Z

🦞🧹
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.

SunnyShu0925 · 2026-07-06T17:36:08Z

@clawsweeper re-review

clawsweeper · 2026-07-06T17:40:09Z

Codex review: needs changes before merge. Reviewed July 7, 2026, 2:07 AM ET / 06:07 UTC.

Summary
The PR adds shared outbound recovery classification for provable DNS/connect failures, a queue failure helper that clears stale send evidence, retry-attempt error metadata, focused recovery tests, and a release-note entry.

PR surface: Source +140, Tests +559, Docs +1. Total +700 across 12 files.

Reproducibility: yes. Current main marks send_attempt_started before platform I/O, generic failure preserves that marker, and recovery refuses replay without reconciliation; the linked issue also includes a live Discord Wi-Fi-cut repro.

Review metrics: 2 noteworthy metrics.

Persisted Queue Transition: 1 added. The PR adds a new failure transition that clears platform-send evidence, which is the central message-delivery safety decision.
Release-Owned Changelog Edit: 1 added entry. The diff touches CHANGELOG.md, which repository policy reserves for release generation rather than normal PRs.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100979
Summary: This PR is the current shared-core fix candidate for the canonical Discord full-outage outbound recovery issue.

Members:

canonical: [Bug]: Discord sends failed during a full network outage are dropped on reconnect (send_attempt_started refuses replay, no reconcileUnknownSend) #100979 - The PR body uses closing syntax for the issue and implements the issue's first candidate fix: clear send evidence for proven pre-dispatch network failures.
superseded: fix(discord): add reconcileUnknownSend adapter for durable delivery recovery (#100979) #100997 - That Discord-specific reconciliation PR targeted the same issue but is closed unmerged, so it is context rather than the safe canonical landing path.
partial_overlap: fix(discord): messages sent during gateway reconnect are silently dropped #100896 - The merged Discord reconnect PR handles short reconnect windows and explicitly leaves the full-outage send_attempt_started gap to the canonical issue.
partial_overlap: fix: outbound recovery can replay already sent replies #99600 - The merged outbound recovery rewrite fixed duplicate replay after accepted sends and carried forward the generic failure state that this PR narrows.
same_root_cause: Signal final replies can be stranded by gateway restart after send_attempt_started #90233 - The Signal report shares the send_attempt_started plus no safe reconciliation root cause, though it has channel-specific restart and surfacing requirements.

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:

Remove the CHANGELOG.md entry and keep the release-note context in the PR body or squash message.

Risk before merge

[P1] The pre-connect classifier sits on the duplicate-send safety boundary: if it is broadened incorrectly, recovery could replay a message the platform already accepted; if it is too narrow, the linked message-loss path remains.
[P1] This PR fixes newly observed provable pre-connect failures but does not migrate already poisoned send_attempt_started rows or solve true crash/unknown-after-send reconciliation.
[P1] The PR currently edits release-owned CHANGELOG.md, which should be left to release generation before merge.

Maintainer options:

Remove Changelog, Then Land Shared Boundary (recommended)
Remove the release-owned changelog edit and merge if maintainers accept the narrow pre-connect proof rules as sufficient for safe replay.
Ask For Exact-Head Outage Proof
Require a redacted Discord or Gateway outage/reconnect transcript on this head before accepting the message-delivery risk.
Pause For Adapter-Owned Reconciliation
Close or pause this PR if maintainers decide Discord-specific reconciliation should own this class instead of shared core classification.

Next step before merge

[P2] A repair worker can remove the release-owned changelog entry; maintainer review still needs to accept the message-delivery replay boundary before landing.

Maintainer decision needed

Question: Should OpenClaw land this shared pre-connect queue cleanup as the canonical fix path for the linked full-outage delivery drop, with Discord unknown-send reconciliation left as follow-up?
Rationale: The code path is a message-delivery safety boundary: shared replay unlocks dropped messages across channels, while an overly broad classification could duplicate platform sends and the competing Discord-specific reconciliation PR was closed unmerged.
Likely owner: steipete — He is the current assignee and the strongest history signal for the shared recovery rewrite and Discord delivery boundary.
Options:
- Land Shared Cleanup After Changelog Removal (recommended): Accept the narrow shared classifier and queue transition after removing the release-owned changelog entry, leaving true unknown-send reconciliation as follow-up work.
- Require Exact Live Discord Proof: Hold the PR until a redacted exact-head Discord or Gateway outage run shows one queued message delivered and no duplicate send.
- Prefer Adapter Reconciliation: Pause this PR and revive a Discord-owned reconcileUnknownSend approach if maintainers want provider evidence before any replay.

Security
Cleared: The diff changes core outbound queue handling and tests without new dependencies, workflows, permissions, secrets, package metadata, or install/build execution surfaces.

Review findings

[P3] Remove the release-owned changelog entry — CHANGELOG.md:31

Review details

Best possible solution:

Remove the changelog entry, then land the shared pre-connect cleanup if maintainers accept the narrow classifier boundary; keep Discord idempotent reconciliation and existing stuck-row recovery as separate follow-up work.

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

Yes. Current main marks send_attempt_started before platform I/O, generic failure preserves that marker, and recovery refuses replay without reconciliation; the linked issue also includes a live Discord Wi-Fi-cut repro.

Is this the best way to solve the issue?

Yes, with one release-flow cleanup. A shared classifier for provable DNS/connect failures is the narrow maintainable fix for observed pre-dispatch failures, while Discord reconciliation remains the safer separate path for true unknown-send cases.

Full review comments:

[P3] Remove the release-owned changelog entry — CHANGELOG.md:31
CHANGELOG.md is release-owned in this repo, and AGENTS.md says normal PRs should not edit it because release generation derives entries from merged PRs and commits. Please drop this entry and keep the release-note context in the PR body or squash message instead, so the release flow remains the single source of truth.
Confidence: 0.93

Overall correctness: patch is correct
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P1: The PR targets a source-reproduced outbound message-loss path during channel recovery after a full network outage.
merge-risk: 🚨 message-delivery: Changing when send_attempt_started is cleared determines whether recovery drops, retries, or could duplicate outbound 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 (terminal): The PR body includes after-fix Testbox results, a live Node 24 error-graph probe, and linked live Discord outage reproduction context; that is sufficient real behavior proof for this internal queue recovery change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix Testbox results, a live Node 24 error-graph probe, and linked live Discord outage reproduction context; that is sufficient real behavior proof for this internal queue recovery change.

Evidence reviewed

PR surface:

Source +140, Tests +559, Docs +1. Total +700 across 12 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	6	152	12	+140
Tests	5	560	1	+559
Docs	1	1	0	+1
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	12	713	13	+700

Acceptance criteria:

[P1] git diff --check.

What I checked:

Current-main failure path: Current main marks platform send start before adapter I/O, generic failure preserves the queue row state, and recovery refuses send_attempt_started/unknown_after_send rows without reconciliation. (src/infra/outbound/deliver.ts:1448, b66220948882)
PR shared classifier: The PR head adds isPreConnectNetworkError, requiring recognized pre-connect codes or nested aggregate children while rejecting ambiguous transport leaves. (src/infra/delivery-recovery.shared.ts:46, 87ee2e8ed914)
PR queue transition: The PR head adds failDeliveryBeforePlatformSend, which records retry metadata while clearing both platformSendStartedAt and recoveryState together. (src/infra/outbound/delivery-queue-storage.ts:156, 87ee2e8ed914)
Regression coverage: The PR covers replay after cleared send evidence, repeated pre-connect recovery failures, best-effort recovery failures, retry-attempt preservation, Slack wrapper shape, Undici connect-timeout shape, and ambiguous reset cases. (src/infra/outbound/deliver.test.ts:1505, 87ee2e8ed914)
Release-owned changelog policy: Repository policy says CHANGELOG.md is release-only and normal PRs should not edit it; this PR adds one changelog entry. (AGENTS.md:238, b66220948882)
Dependency contract check: Undici v8.5.0 defines ConnectTimeoutError with code UND_ERR_CONNECT_TIMEOUT, and Slack Web API 7.18.0 request errors carry code slack_webapi_request_error plus the original error.

Likely related people:

steipete: Current blame in the central recovery wrapper points to Peter Steinberger, he authored the current PR head rewrite, and he co-authored/merged recent outbound recovery and Discord reconnect work touching the same delivery boundary. (role: recent area contributor and likely follow-up owner; confidence: high; commits: 87ee2e8ed914, 741d9524bec2, 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 recent merged outbound recovery PR that introduced the same durable queue state and replay-safety boundary this PR adjusts. (role: recent area contributor; confidence: medium; commits: 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 canonical full-outage issue that this PR references, making them relevant for Discord proof and acceptance context. (role: adjacent Discord reporter and contributor; confidence: medium; commits: da0103f02544; files: extensions/discord/src/retry.ts, extensions/discord/src/client.ts, extensions/discord/src/outbound-adapter.ts)
Joey Frasier (Boothe): Authored the recent Slack unknown-send reconciliation implementation, which is the sibling adapter-owned alternative discussed for the same recovery class. (role: adjacent reconciliation contributor; confidence: medium; commits: 7e0d530e07d6; files: extensions/slack/src/channel.ts, extensions/slack/src/send.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 (1 earlier review cycle)

reviewed 2026-07-06T17:47:52.762Z sha ab09620 :: needs maintainer review before merge. :: none

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

steipete · 2026-07-07T06:23:38Z

Land-ready maintainer rewrite complete at signed head 364173b5d4149f05bc23e50b38ff9b8b7bfe85b9.

Replaced the split queue mutation with one atomic pre-connect failure transition that records retry metadata while clearing stale platformSendStartedAt and recoveryState evidence.
One structured classifier now covers live delivery, recovery, best-effort batches, Slack wrappers, Node/Undici aggregate errors, and exhausted internal retries. Replay remains fail-closed unless every attempt and every transport leaf proves DNS/connect failure.
Retry attempt history stays private infra metadata and preserves the terminal error object's identity; no public plugin SDK surface was added.
Blacksmith Testbox tbx_01kwxh9xadqgh8a3yrdafdy52b: 210 focused core tests, 21 Discord retry tests, and changed gates passed. A Node 24 live probe reproduced TypeError -> AggregateError(ECONNREFUSED) -> connect leaves.
Blacksmith Testbox tbx_01kwxk1btxca2wew7vf9sym2en: 206 focused retry/delivery/storage/recovery tests passed after the private-metadata refactor. Proof run: https://github.com/openclaw/openclaw/actions/runs/28845427463
node scripts/plugin-sdk-surface-report.mjs --check passed with the unchanged public SDK budgets; git diff --check passed.
Fresh autoreview: clean, no accepted/actionable findings (0.96).
Exact-head hosted CI: https://github.com/openclaw/openclaw/actions/runs/28845557146

Known proof gap: I did not repeat the credentialed real Discord Wi-Fi-cut send. The linked issue contains that live reproduction; this verification targets the shared duplicate-send safety invariant and the exact transport error shapes that decide replay.

steipete · 2026-07-07T06:23:59Z

Merged via squash.

Prepared head SHA: 364173b5d4149f05bc23e50b38ff9b8b7bfe85b9
Landed commit: 13f8dfae864ff1d44eae2d3884e68f9176a1d430

* fix(outbound): retry proven pre-connect failures Co-authored-by: 0668000539 <[email protected]> * refactor(infra): keep retry attempt metadata internal --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added the size: S label Jul 6, 2026

This was referenced Jul 6, 2026

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

Closed

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

Merged

steipete self-assigned this Jul 7, 2026

steipete force-pushed the fix/network-outage-recovery-100979 branch from ab09620 to 87ee2e8 Compare July 7, 2026 05:53

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

steipete changed the title ~~fix(outbound): clear recoveryState on connect-phase errors so drain can retry (#100979)~~ fix(outbound): retry proven pre-connect failures Jul 7, 2026

steipete and others added 2 commits July 7, 2026 07:10

fix(outbound): retry proven pre-connect failures

54d2dd1

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

refactor(infra): keep retry attempt metadata internal

364173b

steipete force-pushed the fix/network-outage-recovery-100979 branch from 87ee2e8 to 364173b Compare July 7, 2026 06:10

steipete merged commit 13f8dfa into openclaw:main Jul 7, 2026
101 checks passed

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

📡 Upstream Digest — 2026-07-07 07:31 UTC curtismercier/openclaw-mods#1228

Open

This was referenced Jul 7, 2026

fix(agents): remove abort listener on early reject paths in ls tool #101514

Closed

[codex] Keep ambiguous delivery attempts recoverable #87552

Open

solavrc mentioned this pull request Jul 9, 2026

Ensure transcript-to-client delivery consistency for channel-scoped sessions #87544

Open

Kailigithub mentioned this pull request Jul 9, 2026

fix(telegram): make pre-connect network failures recoverable in send context #80529

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

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

fix(outbound): retry proven pre-connect failures#101024
steipete merged 2 commits into
openclaw:mainfrom
SunnyShu0925:fix/network-outage-recovery-100979

SunnyShu0925 commented Jul 6, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

SunnyShu0925 commented Jul 6, 2026

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

SunnyShu0925 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!

SunnyShu0925 commented Jul 6, 2026

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!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SunnyShu0925 commented Jul 6, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading