fix: outbound recovery can replay already sent replies by zhangguiping-xydt · Pull Request #99600 · openclaw/openclaw

zhangguiping-xydt · 2026-07-03T16:39:57Z

What Problem This Solves

Outbound recovery could successfully send one or more platform messages, then fail during later persistence, acknowledgement, or adapter work while leaving the canonical queue row eligible for replay. After a restart, users could receive duplicate replies or attachments that had already reached the channel.

Why This Change Was Made

Recovery now owns one canonical queue row through the full send lifecycle:

persist send_attempt_started before crossing the platform boundary;
record every concrete sub-send result before another send or hook can fail;
advance sent evidence to unknown_after_send before acknowledgement;
acknowledge before running fallible commit hooks;
reconcile partial batches by receipt-part multiplicity, including platforms such as LINE that reuse a constant message ID.

The maintainer rewrite removes the original parallel failed-diagnostic path. The same typed result callback is carried through core message delivery and every bundled multi-send adapter, so recovery no longer has to infer whether a rejected adapter sent nothing or sent part of a batch.

User Impact

Recovered outbound replies no longer replay after the platform accepted them. Partial multi-message deliveries retain exact progress across failure or restart, while genuine pre-send failures remain retryable. The behavior applies consistently across bundled channels, including Discord, Feishu, LINE, Matrix, Microsoft Teams, Signal, Slack, Telegram, Twitch, WhatsApp, Zalo, and Zalouser.

Evidence

Changed-surface tests: 805 tests across 22 files and 14 Vitest shards passed.
Focused final receipt reconciliation: 125 tests passed across core outbound delivery and LINE payload delivery.
Exact-head heartbeat + core + LINE regression set: 137 tests passed, including the CI-discovered heartbeat expectation for the propagated delivery-result callback.
Broad Testbox changed gate: tbx_01kwny7am5ze6ef3m5v1gy5mwm, 10m04, passed.
Real Gateway restart/recovery QA: tbx_01kwnxyzfmqjg0pe8d5xpm31pf, passed with the interrupted marker emitted at most once and the recovery marker emitted exactly once.
Full-branch Codex autoreview: clean, 0 findings, 0.94 confidence after fixing two repeated-platform-ID findings. Final exact-head fixture autoreview: clean, 0 findings, 0.99 confidence.
Exact-head hosted gates after the final mainline rebase: CI 44/44 passed, with CodeQL, critical-quality, OpenGrep, security/dependency guards, and real-behavior proof green. Native scripts/pr prepare-run 99600 accepted the exact head in hosted_exact_head mode.
Additional standalone Testbox/Azure reruns were blocked before command start by Blacksmith shutdown/rate-limit errors and an Azure lease timeout; the successful broad Testbox run, real-Gateway scenario, and exact-head hosted gates cover the landed behavior.

No before/after screenshot applies: this is internal queue/recovery behavior with no UI surface.

clawsweeper · 2026-07-03T16:41:12Z

Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 4:02 AM ET / 08:02 UTC.

Summary
Threads per-send delivery progress through outbound queue recovery, message adapters, Plugin SDK contexts, and bundled channel senders so partial platform sends are persisted before replayable failures.

PR surface: Source +926, Tests +1359. Total +2285 across 56 files.

Reproducibility: yes. Current main records successful platform sends only after the adapter returns, and the PR's focused tests exercise later chunk/commit failures that would otherwise leave replayable queue state.

Review metrics: 1 noteworthy metric.

Plugin send context API: 2 optional callback fields added. The new fields flow through plugin-facing message and outbound contexts, so maintainers should notice the additive SDK compatibility surface before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
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 changes message-delivery accounting across core recovery, Plugin SDK contexts, and many bundled channels; a missed edge case could still duplicate, suppress, or mis-account recovered messages.
[P1] The optional onDeliveryResult field is passed through plugin-facing send contexts, so maintainers should consider third-party plugins that forward whole context objects to stricter transport helpers.
[P1] At review time, live head 5b764f0 still had some ordinary CI jobs in progress even though the real-behavior proof check and several focused gates were green.

Maintainer options:

Wait For Exact-Head Gates (recommended)
Merge after the remaining head 5b764f0 CI jobs finish green and the real Gateway recovery proof remains attached to the same effective change.
Review Plugin Context Compatibility
If maintainers are concerned about third-party plugins forwarding context objects, document or constrain the optional callback before merge.
Accept Additive SDK Risk
Maintainers can accept the optional callback as an internal additive field because bundled channels were updated and recovery tests cover the intended path.

Next step before merge

No ClawSweeper repair job is needed; the remaining action is normal maintainer review and exact-head CI completion.

Security
Cleared: The diff does not change workflows, dependencies, credentials, permissions, or secret handling, and no concrete security or supply-chain concern was found.

Review details

Best possible solution:

Land the shared queue-state and per-send progress approach after exact-head checks finish, keeping the callback additive/internal and preserving the bundled-channel regression coverage.

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

Yes. Current main records successful platform sends only after the adapter returns, and the PR's focused tests exercise later chunk/commit failures that would otherwise leave replayable queue state.

Is this the best way to solve the issue?

Yes. The shared write-ahead queue transition plus per-send progress callback is the maintainable fix because it avoids per-channel inference and carries concrete results through the core recovery path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add merge-risk: 🚨 compatibility: The PR adds optional callback fields to plugin-facing send context types, which is additive but still compatibility-sensitive for plugin authors.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body/comment provide after-fix Testbox and real Gateway restart/recovery proof, and the live head shows the Real behavior proof check succeeded; no UI screenshot is relevant for this internal queue path.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR body/comment provide after-fix Testbox and real Gateway restart/recovery proof, and the live head shows the Real behavior proof check succeeded; no UI screenshot is relevant for this internal queue path.

Label justifications:

P2: This is a normal-priority correctness fix for duplicate outbound replies with broad but bounded channel-delivery blast radius.
merge-risk: 🚨 message-delivery: The diff changes queue replay, recovery acknowledgement, and per-channel send progress paths where mistakes can duplicate or suppress delivered messages.
merge-risk: 🚨 compatibility: The PR adds optional callback fields to plugin-facing send context types, which is additive but still compatibility-sensitive for plugin authors.
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 (linked_artifact): The PR body/comment provide after-fix Testbox and real Gateway restart/recovery proof, and the live head shows the Real behavior proof check succeeded; no UI screenshot is relevant for this internal queue path.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body/comment provide after-fix Testbox and real Gateway restart/recovery proof, and the live head shows the Real behavior proof check succeeded; no UI screenshot is relevant for this internal queue path.

Evidence reviewed

PR surface:

Source +926, Tests +1359. Total +2285 across 56 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	33	1220	294	+926
Tests	23	1420	61	+1359
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	56	2640	355	+2285

What I checked:

Repository policy read and applied: Root and scoped AGENTS guidance required deep review across channel delivery, plugin SDK, outbound recovery, and merge-risk visibility for message delivery and plugin API surfaces. (AGENTS.md:1, 1e20f15581f9)
Current main lacks per-send progress persistence: Current main only pushes identified delivery results after send helpers return, so a platform send followed by an adapter failure can leave the queued row without concrete send evidence. (src/infra/outbound/deliver.ts:863, 1e20f15581f9)
PR records progress before later fallible work: The PR records identified delivery results through the new callback before pinning, hooks, mirroring, or later sends can fail. (src/infra/outbound/deliver.ts:1580, 5b764f08c225)
Recovery preserves post-send state: Recovery now marks unknown-after-send or falls back to ack/fail-after-platform-send once any delivery result proves a platform send completed. (src/infra/outbound/delivery-queue-recovery.ts:349, 5b764f08c225)
Additive plugin-facing callback surface: The PR adds optional internal onDeliveryResult callbacks to channel message and outbound contexts, which is additive but still plugin-contract adjacent. (src/channels/message/types.ts:179, 5b764f08c225)
Bundled channel coverage sampled: The PR propagates progress through representative bundled senders including Telegram, Discord components, WhatsApp voice-caption sends, LINE payloads, Slack, Matrix, Signal, and Zalouser. (extensions/telegram/src/send.ts:720, 5b764f08c225)

Likely related people:

steipete: Git history shows broad recent work on outbound delivery, delivery queue recovery, plugin SDK seams, and channel sender refactors; this PR also contains maintainer rewrite commits and proof from the same person. (role: recent area contributor and PR rewrite/proof owner; confidence: high; commits: 562e4a1791fb, 62ddc9d9e0d9, 62edfdffbdd0; files: src/infra/outbound/deliver.ts, src/infra/outbound/delivery-queue-recovery.ts, src/plugin-sdk/reply-payload.ts)
vincentkoc: Recent history includes release and WhatsApp runtime-seam work near the bundled channel surfaces affected by this PR. (role: adjacent channel/runtime contributor; confidence: medium; commits: 489d0f7cd940, e085fa1a3ffd; files: extensions/whatsapp/src/send.ts, extensions/whatsapp/src/outbound-base.ts)
Ayaan Zaidi: Recent history shows work in Telegram send behavior, one of the bundled channel send paths modified by this PR. (role: adjacent Telegram send-path contributor; confidence: low; commits: f48ff25b3be3; files: extensions/telegram/src/send.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-04T01:45:29.692Z sha e9b4df6 :: needs maintainer review before merge. :: none

steipete · 2026-07-04T07:40:59Z

Maintainer proof for exact head 5b764f08c2250ae0f391b3708153e4ea6a84a42e:

Rewrote recovery around the canonical queue row: pre-send state, per-sub-send evidence, acknowledgement before commit hooks, and no parallel failed-diagnostic row.
Threaded concrete delivery results through core plus every bundled multi-send adapter.
Fixed autoreview findings for repeated LINE-style platform IDs by matching one progress result per receipt part. Non-aggregate duplicate IDs remain separate; repeated-ID aggregate receipts collapse exactly once.
node scripts/run-vitest.mjs <22 changed test files>: 805 tests passed across 14 shards.
Focused final core + LINE reconciliation: 125 tests passed.
Exact-head heartbeat + core + LINE regression set: 137 tests passed. This includes the CI-discovered heartbeat expectation for the newly propagated delivery-result callback.
Broad Testbox changed gate tbx_01kwny7am5ze6ef3m5v1gy5mwm: passed in 10m04 (run).
Real Gateway restart/recovery scenario tbx_01kwnxyzfmqjg0pe8d5xpm31pf: passed; interrupted marker at most once, recovery marker exactly once (run). A provider key is not needed for this deterministic queue/restart boundary; the scenario uses a real Gateway and qa-channel with the controlled mock provider.
Full-branch Codex autoreview: clean, 0 findings, 0.94 confidence. Final exact-head fixture autoreview: clean, 0 findings, 0.99 confidence.
Exact-head hosted gates after the final mainline rebase: CI 44/44 passed, with CodeQL, critical-quality, OpenGrep, security/dependency guards, and real-behavior proof green (run). Native scripts/pr prepare-run 99600 accepted the exact head in hosted_exact_head mode.
git diff --check: passed.

Additional standalone Testbox/Azure rerun attempts were blocked before command execution by runner shutdown/rate-limit errors and an Azure lease timeout; the successful broad Testbox run, real-Gateway scenario, and exact-head hosted gates above cover the landed behavior. No screenshot applies because this has no UI surface.

Co-authored-by: zhang-guiping <[email protected]>

steipete · 2026-07-04T08:09:17Z

Merged via squash.

Prepared head SHA: 5b764f08c2250ae0f391b3708153e4ea6a84a42e
Landed commit: 741d9524bec2ef9a4748bf8b27f1fd6324dc74e7

* fix(outbound): avoid replaying sent recovery deliveries * fix(outbound): persist recovery send state before ack Co-authored-by: zhang-guiping <[email protected]> * fix(outbound): preserve partial send evidence across adapters * test(line): cover multi-send delivery progress * fix(outbound): preserve repeated receipt result multiplicity * test(heartbeat): expect delivery progress callback --------- Co-authored-by: Peter Steinberger <[email protected]>

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

steipete self-assigned this Jul 3, 2026

steipete force-pushed the fix/delivery-queue-recovery-commit-hook branch 2 times, most recently from e9b4df6 to 6780532 Compare July 4, 2026 07:40

steipete force-pushed the fix/delivery-queue-recovery-commit-hook branch 2 times, most recently from 529ce2e to e659f40 Compare July 4, 2026 07:47

zhangguiping-xydt and others added 6 commits July 4, 2026 08:57

fix(outbound): avoid replaying sent recovery deliveries

5f7bbe6

fix(outbound): persist recovery send state before ack

43921d7

Co-authored-by: zhang-guiping <[email protected]>

fix(outbound): preserve partial send evidence across adapters

abe8fa9

test(line): cover multi-send delivery progress

9f1a064

fix(outbound): preserve repeated receipt result multiplicity

5b36867

test(heartbeat): expect delivery progress callback

5b764f0

steipete force-pushed the fix/delivery-queue-recovery-commit-hook branch from e659f40 to 5b764f0 Compare July 4, 2026 07:57

steipete merged commit 741d952 into openclaw:main Jul 4, 2026
132 of 138 checks passed

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

📡 Upstream Digest — 2026-07-04 10:01 UTC curtismercier/openclaw-mods#1203

Open

clawsweeper Bot mentioned this pull request Jul 5, 2026

fix(slack): reconcile ambiguous sends before replay #97480

Merged

SunnyShu0925 mentioned this pull request Jul 6, 2026

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

Merged

solavrc mentioned this pull request Jul 9, 2026

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

Open

This was referenced Jul 9, 2026

[codex] Verify Telegram topic proof for cron delivery #97361

Open

[AI] fix(agents): recover from non-resumable assistant tail by re-presenting last user message #95899

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: outbound recovery can replay already sent replies#99600

fix: outbound recovery can replay already sent replies#99600
steipete merged 6 commits into
openclaw:mainfrom
zhangguiping-xydt:fix/delivery-queue-recovery-commit-hook

zhangguiping-xydt commented Jul 3, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zhangguiping-xydt commented Jul 3, 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 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zhangguiping-xydt commented Jul 3, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

steipete commented Jul 4, 2026 •

edited

Loading