diff --git a/apps/bot/src/ipc/pair-handler.ts b/apps/bot/src/ipc/pair-handler.ts index 1f08bb8..48240e7 100644 --- a/apps/bot/src/ipc/pair-handler.ts +++ b/apps/bot/src/ipc/pair-handler.ts @@ -10,6 +10,7 @@ import { renderQrPng } from "../whatsapp/qr-renderer.js"; import { syncGroupsForAccount } from "../whatsapp/group-sync.js"; import { writeAuditLog } from "../audit.js"; import { pgNotifyWeb } from "./notify.js"; +import { decidePairListenerOnClose } from "./pair-state.js"; const PAIR_TIMEOUT_MS = 5 * 60 * 1000; const offByAccount = new Map void>(); @@ -150,40 +151,33 @@ export async function handleStartPairing(accountId: string): Promise { count: synced, }); off(); - } else if (event.type === "close" && event.restartRequired) { - // After the user scans, WhatsApp tells Baileys to "restart" - // the connection. The socket closes with status 515 and the - // session-manager will reopen it with the new credentials — - // the next `open` event is what completes the pairing. - // This is NOT a failure: keep the listener attached so we see - // that subsequent `open` event, and don't surface a timeout - // to the UI. The DB row stays in `pending` until `open`. - logger.info( - { accountId: id }, - "pair: restart-required close (post-pair reconnect) — keeping listener alive", - ); - // The session-manager handles the actual reconnect; nothing to - // do here other than NOT tear our listener / DB state down. } else if (event.type === "close") { - // Swallow the leaked close from the *previous* pairing attempt - // being torn down inside this handler's own - // `await sessionManager.stop()`. The old session's close is - // broadcast asynchronously and lands here AFTER we've attached - // the new listener; treating it as a real timeout would flash - // "Pairing timed out" before any QR even arrives. - if (pairingWarmingUp.has(id)) { + const decision = decidePairListenerOnClose({ + warmingUp: pairingWarmingUp.has(id), + restartRequired: event.restartRequired, + }); + if (decision === "ignore-leaked-close") { logger.info( { accountId: id }, "pair: ignoring close from previous attempt while warming up", ); return; } - // During the pairing window, any other close means the QR window - // ended without a successful link — Baileys' default is to - // close after exhausting QR refs (~2.5 min). Surface this to - // the UI so the user gets a "pairing timed out" screen, and - // park the row in a stable state so it shows up cleanly on - // the accounts list with a "Re-pair" affordance. + if (decision === "post-pair-restart") { + // After the user scans, WhatsApp tells Baileys to "restart" + // the connection. The socket closes with status 515 and the + // session-manager will reopen it with the new credentials — + // the next `open` event finishes the pairing. Keep the + // listener attached and don't surface a timeout to the UI. + logger.info( + { accountId: id }, + "pair: restart-required close (post-pair reconnect) — keeping listener alive", + ); + return; + } + // decision === "treat-as-timeout": ephemeral close on a live + // attempt. Park the row as `unpaired` and push session.timeout + // so the operator sees the "Re-pair" affordance. const t = pairTimeouts.get(id); if (t) { clearTimeout(t); diff --git a/apps/bot/src/ipc/pair-state.test.ts b/apps/bot/src/ipc/pair-state.test.ts index 52c8205..6134ccc 100644 --- a/apps/bot/src/ipc/pair-state.test.ts +++ b/apps/bot/src/ipc/pair-state.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest"; import { decideOnPairClose, decideOnPairTimeout, + decidePairListenerOnClose, shouldAutoReconnect, } from "./pair-state.js"; @@ -82,3 +83,45 @@ describe("shouldAutoReconnect", () => { expect(shouldAutoReconnect({ loggedOut: false, hasEverConnected: false })).toBe(false); }); }); + +describe("decidePairListenerOnClose (back→re-pair flicker regression)", () => { + it("ignores a close while warming up — even if also restartRequired", () => { + // The exact bug: stop() was awaited, listener attached, then the OLD + // session's close arrives and races our new listener. Warming-up + // wins over every other branch so the UI never sees a spurious + // session.timeout before the new QR is rendered. + expect( + decidePairListenerOnClose({ warmingUp: true, restartRequired: false }), + ).toBe("ignore-leaked-close"); + expect( + decidePairListenerOnClose({ warmingUp: true, restartRequired: true }), + ).toBe("ignore-leaked-close"); + }); + + it("treats a close on a live attempt (warmingUp=false) as a real timeout", () => { + // Refs exhausted, network blip, etc. — operator gets the + // "Pairing timed out" screen and a Re-pair affordance. + expect( + decidePairListenerOnClose({ warmingUp: false, restartRequired: false }), + ).toBe("treat-as-timeout"); + expect(decidePairListenerOnClose({ warmingUp: false })).toBe("treat-as-timeout"); + }); + + it("preserves the restart-required (post-pair-success) branch when not warming up", () => { + // Status 515 close: the session-manager will reconnect and the next + // `open` finishes the pair. We must NOT push session.timeout here. + expect( + decidePairListenerOnClose({ warmingUp: false, restartRequired: true }), + ).toBe("post-pair-restart"); + }); + + it("warming-up overrides restartRequired so 515 from a stale session is also swallowed", () => { + // Defense-in-depth: if Baileys' restart-required close from the OLD + // session somehow leaks through, treating it as a real 515 would + // KEEP the listener attached forever (no reconnect comes from a + // session we just stopped). Ignore it entirely until a fresh qr/open. + expect( + decidePairListenerOnClose({ warmingUp: true, restartRequired: true }), + ).toBe("ignore-leaked-close"); + }); +}); diff --git a/apps/bot/src/ipc/pair-state.ts b/apps/bot/src/ipc/pair-state.ts index df76e31..7901c3d 100644 --- a/apps/bot/src/ipc/pair-state.ts +++ b/apps/bot/src/ipc/pair-state.ts @@ -80,3 +80,37 @@ export function decideOnPairTimeout({ current }: { current: AccountStatus }): St if (current !== "pending") return null; return { next: "unpaired", clearQrPng: true }; } + +/** + * Decide how the pair-handler should react to a `close` event delivered + * to its listener. Three outcomes: + * + * - "ignore-leaked-close": the new attempt is still warming up and + * we're seeing the OLD session's tail close. Do nothing — don't + * emit timeout to the UI, don't touch the DB row. + * - "post-pair-restart": status-515 close from a successful scan. + * The session-manager will reconnect; we keep the listener alive + * and wait for the subsequent `open` event. + * - "treat-as-timeout": a real ephemeral close on a live attempt + * (refs exhausted, etc.). Park the row as `unpaired` and push + * `session.timeout` to the UI. + * + * Captures the regression where, after the user pulled up a QR and + * navigated back, clicking Pair again would instantly flash "Pairing + * timed out" because the await on stop() returned before + * sessionManager.handleEvent finished broadcasting the old session's + * close — and the new listener was already attached. + */ +export type PairListenerCloseDecision = + | "ignore-leaked-close" + | "post-pair-restart" + | "treat-as-timeout"; + +export function decidePairListenerOnClose(input: { + warmingUp: boolean; + restartRequired?: boolean; +}): PairListenerCloseDecision { + if (input.warmingUp) return "ignore-leaked-close"; + if (input.restartRequired) return "post-pair-restart"; + return "treat-as-timeout"; +}