Skip to content

fix(whatsapp): harden bridge reconnection supervision - #75700

Open
Dannyzen wants to merge 1 commit into
NousResearch:mainfrom
Dannyzen:fix/whatsapp-reconnect-supervision
Open

fix(whatsapp): harden bridge reconnection supervision#75700
Dannyzen wants to merge 1 commit into
NousResearch:mainfrom
Dannyzen:fix/whatsapp-reconnect-supervision

Conversation

@Dannyzen

Copy link
Copy Markdown
Contributor

Summary

  • Let a managed WhatsApp bridge exit hand off to GatewayRunner without the poll task cancelling itself during teardown.
  • Bound and cache Baileys version discovery, with packaged-version fallback when discovery stalls or fails.
  • Add a 45-second socket settle watchdog and fail back to the process supervisor when startup/reconnect fails.
  • Fence stale socket events and coalesce reconnect requests so only one timer, one in-flight start, and one deferred retry can exist.

Problem

When the managed Node bridge exits inside the adapter poll task, the fatal-error handler tears the adapter down from a child task. disconnect() then cancels the still-recorded poll task, aborting the parent before GatewayRunner can queue background reconnection.

The Node bridge also had unbounded version discovery and raw setTimeout(startSocket, ...) calls. A stalled discovery or repeated/early close event could wedge startup, overlap sockets, or silently drop the next reconnect.

Behavior

  • The current poll task clears its ownership marker before invoking the fatal callback.
  • Version discovery has a 10-second ceiling and one cached result.
  • A socket that neither opens nor closes within 45 seconds exits for supervisor recovery.
  • Duplicate close events share one reconnect timer.
  • A close emitted while a reconnect start is in flight becomes one deferred retry, rather than starting concurrently or being discarded.
  • A successful open cancels pending and deferred retries.
  • Events from stale socket generations cannot overwrite current state.

Verification

  • ./scripts/run_tests.sh tests/gateway/test_whatsapp_connect.py -q (14 passed)
  • node --test reconnect.test.js allowlist.test.mjs bridge.sendqueue.test.mjs outbound_ids.test.mjs owner_message_gate.test.mjs (28 passed)
  • node bridge.native.test.mjs
  • node --check scripts/whatsapp-bridge/bridge.js
  • node --check scripts/whatsapp-bridge/reconnect.js
  • Ruff check on changed Python files
  • git diff --check
  • Added-line secret/unsafe-execution scan
  • Independent fail-closed review of exact diff SHA-256 14d789fcc3394ea53403e2b2f61c36cb881faf1c50073a27d19d9b4e27a661f2: no security or logic findings

Related work

#73795 changes which WhatsApp Web version resolver is preferred. This PR is complementary: its cache/timeout wrapper can bound whichever resolver wins. If #73795 lands first, the overlap is limited to the version-discovery call site.

No package manifest or lockfile changes are included.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins comp/gateway Gateway runner, session dispatch, delivery platform/whatsapp WhatsApp Business adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 31, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for hardening the bridge supervision. The timeout/cache and reconnect-scheduling work addresses behavior still present in current main (scripts/whatsapp-bridge/bridge.js:398,452).

Problems

  • The Python poll-task handoff is already covered centrally: current gateway/run.py:6853-6879 detaches and shields fatal handling, and queues retryable adapters at gateway/run.py:6970-6986 (introduced by 2ab153218ba4). The WhatsApp-specific hunk and its regression test should be removed or re-scoped.
  • The new generation fence does not cover all events. scripts/whatsapp-bridge/bridge.js:446 still accepts stale creds.update; QR handling at lines 451-459 runs before the generation checks; and the later messages.update/messages.upsert listeners remain unguarded. A stale socket can therefore still affect credentials, pairing output, or inbound handling after replacement.

Suggested changes

  • Guard every socket callback at entry with the captured generation, bind listeners to currentSock, and add a two-socket regression test covering stale credentials, QR, and inbound events.
  • Remove the adapter-specific handoff portion now supplied by the shared runner.

Automated hermes-sweeper review.

}, 45_000);

sock.ev.on('creds.update', () => { saveCreds(); lidToPhone = buildLidMap(); });
currentSock.ev.on('creds.update', () => { saveCreds(); lidToPhone = buildLidMap(); });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This listener still accepts events from a replaced socket. Add if (sock !== currentSock) return; before saving credentials (and apply the same captured-generation guard before QR and all message callbacks); otherwise the stated stale-event fence does not cover credential or inbound-event state.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/whatsapp WhatsApp Business adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants