Skip to content

fix(signal): retry inbound flushes on reply session init conflict#103218

Merged
steipete merged 2 commits into
openclaw:mainfrom
cxbAsDev:fix/signal-reply-session-conflict-retry
Jul 10, 2026
Merged

fix(signal): retry inbound flushes on reply session init conflict#103218
steipete merged 2 commits into
openclaw:mainfrom
cxbAsDev:fix/signal-reply-session-conflict-retry

Conversation

@cxbAsDev

@cxbAsDev cxbAsDev commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #100944. Signal could silently drop an already-authorized inbound message when reply-session initialization lost its final optimistic concurrency race. The generic inbound debouncer logged the failed flush but did not retain the Signal payload needed for another delivery attempt.

Why This Change Was Made

The shared reply-session initializer already retries once under its writer lock, so another unbounded session-layer loop would hide sustained contention. Signal is the narrow owner that still has the original inbound payload, making a bounded channel retry the appropriate recovery boundary.

This revision:

  • recognizes the exact conflict through nested cause and error wrappers;
  • retries the original ordered batch after 1s, 2s, and 4s backoffs;
  • keeps the debouncer's same-conversation execution slot reserved during backoff, preventing newer buffered text from overtaking the failed batch;
  • binds retry delays to the Signal monitor abort signal and tracks retries in the monitor's drain set; and
  • leaves non-conflict failures and exhausted retries terminal and logged.

User Impact

Signal follow-up messages that hit this transient session race are retried instead of being silently lost. No configuration or migration is required. Monitor shutdown cancels pending backoff and does not dispatch stale retries afterward.

Evidence

  • corepack pnpm test extensions/signal/src/monitor/event-handler.reply-session-conflict.test.ts extensions/signal/src/monitor.tool-result.pairs-uuid-only-senders-uuid-allowlist-entry.test.ts on Blacksmith Testbox tbx_01kx591b6gg472mwcn1mhhhcqt: 11/11 passed across 2 files (run 29072172902).
  • corepack pnpm check:changed on the same Testbox passed all guards, all four TypeScript lanes, and core lint. Extension lint found three local fixup issues; all were corrected and targeted extension oxlint is clean.
  • Exact pushed-head GitHub CI run 29073336485 completed successfully with zero failed jobs.
  • Focused regressions cover nested error graphs, bounded exhaustion, non-conflict failures, ordered batch retry, newer-message ordering during backoff, abort cancellation, and retry task draining.
  • The production monitor boundary test emits an inbound Signal event through monitorSignalProvider, aborts the monitor during backoff, and proves no post-stop dispatch.
  • Node and Bun both verified node:timers/promises abortable unref delay behavior (AbortError).
  • oxfmt and git diff --check: clean.
  • Fresh reviews accepted and fixed two lifecycle/ordering findings. Final full-branch Codex review on exact pushed head ef26709dcd74d632d07e6654b4f5870d7f555bc4 reported no actionable findings (confidence 0.82).

Live Signal account proof was not available in this checkout; the monitor-level test is the closest credential-free production-path proof.

@openclaw-barnacle openclaw-barnacle Bot added channel: signal Channel integration: signal size: M labels Jul 10, 2026
@cxbAsDev

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 10, 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.

@steipete
steipete force-pushed the fix/signal-reply-session-conflict-retry branch from 708ef3a to ef26709 Compare July 10, 2026 06:14
@steipete

Copy link
Copy Markdown
Contributor

Land-ready proof for the refreshed Signal fix:

  • Exact prepared head: ef26709dcd74d632d07e6654b4f5870d7f555bc4
  • Focused Blacksmith Testbox tbx_01kx591b6gg472mwcn1mhhhcqt: 11/11 passed across 2 files (run 29072172902)
  • Changed gate on the same Testbox: guards, all four TypeScript lanes, and core lint passed; three extension lint fixups were applied and targeted extension oxlint is clean
  • Exact-head GitHub CI: run 29073336485, successful with zero failed jobs
  • Fresh full-branch Codex autoreview: clean, no actionable findings (confidence 0.82)

The fix now retries the original batch inline while retaining the same-key debounce execution slot, so newer buffered Signal messages cannot overtake it. Retry backoff is abortable and unref'ed; the monitor tracks and drains retries, and the production monitor boundary regression proves abort prevents post-stop dispatch. No live linked Signal account was available, so live send/receive remains the explicit proof gap.

@steipete
steipete merged commit 158d646 into openclaw:main Jul 10, 2026
110 of 112 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
…enclaw#103218)

* fix(signal): retry inbound flushes on reply session init conflict

* fix(signal): bind conflict retries to monitor lifecycle

---------

Co-authored-by: Peter Steinberger <[email protected]>
@cxbAsDev
cxbAsDev deleted the fix/signal-reply-session-conflict-retry branch July 15, 2026 03:33
chenyangjun-xy added a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 17, 2026
…ct (openclaw#108320)

When reply dispatch fails with 'reply session initialization conflicted',
the Feishu channel now retries with exponential backoff (250ms to 2s, up to
4 retries) instead of silently dropping the inbound message. Matches the
retry pattern already used by Slack (openclaw#99647) and Signal (openclaw#100944, openclaw#103218).

- Fixes off-by-one in retry/attempt counting: first attempt is outside
  the retry loop, and MAX_RETRIES = 4 consistently with the delay array
- Fixes eslint no-promise-executor-return lint error at bot.ts:121
- Adds 6 focused unit tests covering schedule, exhaustion, and shutdown
- Exports runFeishuDispatchWithSessionInitRetry for testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: signal Channel integration: signal size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Signal DM silently dropped on reply session initialization conflict (retry fix ported to Slack/Telegram but not Signal)

2 participants