Skip to content

fix(whatsapp): retry QR login 408 timeouts#88183

Merged
mcaxtr merged 1 commit into
mainfrom
fix/whatsapp-qr-408-retry
May 30, 2026
Merged

fix(whatsapp): retry QR login 408 timeouts#88183
mcaxtr merged 1 commit into
mainfrom
fix/whatsapp-qr-408-retry

Conversation

@mcaxtr

@mcaxtr mcaxtr commented May 30, 2026

Copy link
Copy Markdown
Member

Summary

  • retry WhatsApp login sockets once when Baileys reports DisconnectReason.timedOut / 408 before login completes
  • keep the existing post-pairing 515 restart as a separate bounded restart so 408 recovery does not consume it
  • wake the QR start flow from replacement-socket QR updates so a 408 before the first QR can still return a scannable QR

Refs #47367.
Supersedes #47710.

Verification

Behavior addressed: WhatsApp QR-link login no longer treats the first pre-QR 408 opening-handshake timeout as an immediate terminal failure when a replacement socket can emit a QR.

Real environment tested: Local mocked WhatsApp/Baileys unit coverage in a fresh branch worktree.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/whatsapp/src/connection-controller.test.ts extensions/whatsapp/src/login-qr.test.ts
pnpm tsgo:extensions:test
pnpm exec oxfmt --check --threads=1 extensions/whatsapp/src/connection-controller.ts extensions/whatsapp/src/connection-controller.test.ts extensions/whatsapp/src/login-qr.ts extensions/whatsapp/src/login-qr.test.ts
git diff --check HEAD~1..HEAD
.agents/skills/autoreview/scripts/autoreview --mode local

Evidence after fix: Regression coverage proves one 408 timeout restart, persistent 408 exhaustion without socket churn, 408 followed by the existing 515 post-pairing restart, and returning a replacement QR when the first socket times out before emitting QR.

Observed result after fix: All commands above passed; autoreview reported no accepted/actionable findings.

What was not tested: No live WhatsApp device pairing run was performed.

@openclaw-barnacle openclaw-barnacle Bot added channel: whatsapp-web Channel integration: whatsapp-web size: M maintainer Maintainer-authored PR labels May 30, 2026
@mcaxtr mcaxtr mentioned this pull request May 30, 2026
Closed
@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 29, 2026, 9:12 PM ET / 01:12 UTC.

Summary
The PR adds a bounded one-time WhatsApp QR-login retry for Baileys 408 timeouts, preserves the separate 515 pairing restart, and adds mocked regression coverage for the replacement-QR flow.

PR surface: Source +53, Tests +150. Total +203 across 4 files.

Reproducibility: yes. at source level: current main retries the WhatsApp login socket only for 515, so a pre-QR 408 from waitForWaConnection falls through as a failed login. I did not run a live WhatsApp pairing flow in this read-only review.

Review metrics: 1 noteworthy metric.

  • Login restart budgets: 1 timeout retry added, 1 post-pairing retry preserved. The PR intentionally changes login fallback behavior, so maintainers should verify both the new 408 recovery path and the existing 515 restart before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦞 diamond lobster
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:

  • [P2] Add live WhatsApp QR pairing proof for the replacement-socket path, or have maintainers explicitly accept mocked-only validation for this bounded retry.

Risk before merge

  • [P2] No live WhatsApp device pairing run was performed, so mocked tests prove the intended control flow but not the real Baileys/WebSocket lifecycle under an actual pre-QR 408 timeout.

Maintainer options:

  1. Add live QR pairing proof (recommended)
    Run a real WhatsApp QR login, or the closest available device proof, and show that the replacement socket returns a scannable QR without hanging after the pre-QR 408 path.
  2. Accept the mocked coverage
    Maintainers can explicitly accept the risk because the retry is capped at one timeout restart and the existing 515 restart budget remains separate.
  3. Pause if live behavior diverges
    If the real Baileys lifecycle cannot demonstrate this path or shows socket churn, pause the PR until the failure mode is narrowed further.

Next step before merge

  • [P2] This maintainer-labeled MEMBER PR has no narrow automation repair; the next action is human acceptance of mocked coverage or live WhatsApp proof before merge.

Security
Cleared: The diff only changes WhatsApp login control flow and tests; it does not add dependencies, workflows, package scripts, secret handling, or broader supply-chain surface.

Review details

Best possible solution:

Land the bounded 408 retry after maintainers either accept the mocked coverage for this channel bug or add live WhatsApp QR pairing proof for the replacement-socket path.

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

Yes, at source level: current main retries the WhatsApp login socket only for 515, so a pre-QR 408 from waitForWaConnection falls through as a failed login. I did not run a live WhatsApp pairing flow in this read-only review.

Is this the best way to solve the issue?

Yes, with one validation caveat: the PR localizes the retry in the shared login wait helper, keeps 408 and 515 restart budgets separate, and wakes the QR flow from replacement sockets. Live Baileys proof would settle the remaining runtime risk.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against deb48a96fb8d.

Label changes

Label justifications:

  • P1: The PR targets a broken WhatsApp QR login workflow where users cannot link the channel after a pre-QR 408 timeout.
  • merge-risk: 🚨 availability: The diff changes live WhatsApp login socket retry behavior, and mocked tests cannot fully prove that real QR login will not stall or churn sockets under Baileys 408 timeouts.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this MEMBER maintainer-labeled PR, though the PR body explicitly notes mocked coverage only and no live WhatsApp device pairing run.
Evidence reviewed

PR surface:

Source +53, Tests +150. Total +203 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 59 6 +53
Tests 2 151 1 +150
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 210 7 +203

What I checked:

  • Repository policy read: Read the full root AGENTS.md and the scoped extensions/AGENTS.md; their plugin-boundary, dependency-contract, validation, protected-label, and maintainer-review guidance applies to this WhatsApp plugin PR. (AGENTS.md:1, deb48a96fb8d)
  • Current main lacks the requested 408 retry: Current main only restarts the login socket for status 515 and otherwise returns a failed login result, so a Baileys 408 from waitForWaConnection still falls through as terminal failure. (extensions/whatsapp/src/connection-controller.ts:216, deb48a96fb8d)
  • Current QR-start race does not wake on replacement-socket QR updates: Current main races the original QR promise against the login waiter; replacement-socket QR updates from the login waiter do not directly wake startWebLoginWithQr. (extensions/whatsapp/src/login-qr.ts:273, deb48a96fb8d)
  • PR adds separate bounded restart budgets: The PR maps 515 and timedOut/408 to separate login restart kinds and recreates the socket at most once for each kind while preserving authDir, socket timing, and onQr options. (extensions/whatsapp/src/connection-controller.ts:223, dc655e02e860)
  • PR wakes QR start from replacement sockets: The PR notifies QR updates from the login waiter and adds qrUpdatePromise to the startWebLoginWithQr race so a replacement socket can return a scannable QR after the first socket times out. (extensions/whatsapp/src/login-qr.ts:187, dc655e02e860)
  • Regression coverage added: The diff adds tests for one 408 retry, preserving the 515 restart after 408 recovery, persistent 408 exhaustion, and returning a replacement QR after a pre-QR 408. (extensions/whatsapp/src/connection-controller.test.ts:132, dc655e02e860)

Likely related people:

  • mcaxtr: GitHub commit history shows mcaxtr authored the central WhatsApp account connection lifecycle refactor and later auth stabilization work that the current login wait path builds on; this PR also comes from the same domain owner. (role: feature-history owner and recent area contributor; confidence: high; commits: aa023e428306, aa76cf43f011, dc655e02e860; files: extensions/whatsapp/src/connection-controller.ts, extensions/whatsapp/src/login-qr.ts, extensions/whatsapp/src/session.ts)
  • BunsDev: GitHub history shows BunsDev authored the recent QR login state synchronization fix that preserved QR data URL/version behavior across login polling flows touched by this PR. (role: adjacent QR-login contributor; confidence: medium; commits: 245451b6a9c5; files: extensions/whatsapp/src/login-qr.ts)
  • steipete: Local blame and GitHub commit history show recent current-main touches to WhatsApp login/runtime files, socket teardown, and plugin import surfaces near the reviewed path. (role: recent area contributor; confidence: medium; commits: 37ccec0dc7f1, 69225003820a, d647ba1c6fb6; files: extensions/whatsapp/src/connection-controller.ts, extensions/whatsapp/src/login-qr.ts, extensions/whatsapp/src/session.ts)
  • velvet-shark: GitHub history ties velvet-shark to the Baileys socket timing work whose options this PR intentionally preserves when creating the replacement socket. (role: adjacent socket-timing contributor/reviewer; confidence: medium; commits: dd643c82b55e; files: extensions/whatsapp/src/connection-controller.ts, extensions/whatsapp/src/login-qr.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.

@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. P1 High-priority user-facing bug, regression, or broken workflow. labels May 30, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. label May 30, 2026
@mcaxtr
mcaxtr force-pushed the fix/whatsapp-qr-408-retry branch from c65c308 to dc655e0 Compare May 30, 2026 01:06
@mcaxtr
mcaxtr merged commit f613f32 into main May 30, 2026
96 of 97 checks passed
@mcaxtr
mcaxtr deleted the fix/whatsapp-qr-408-retry branch May 30, 2026 03:59
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 30, 2026
@mcaxtr
fix(whatsapp): retry QR login 408 timeouts (openclaw#88183)
7786f68
steipete pushed a commit that referenced this pull request May 30, 2026
@mcaxtr @steipete
fix(whatsapp): retry QR login 408 timeouts (#88183)
139b52a
@Jason-Vaughan Jason-Vaughan mentioned this pull request Jun 1, 2026
Closed
SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026
@mcaxtr
fix(whatsapp): retry QR login 408 timeouts (openclaw#88183)
ca6721f
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
@mcaxtr
fix(whatsapp): retry QR login 408 timeouts (openclaw#88183)
02b1bfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: whatsapp-web Channel integration: whatsapp-web maintainer Maintainer-authored PR merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mcaxtr