fix(whatsapp): preserve replies across reconnects#62892
Conversation
Greptile SummaryThis PR fixes a socket-staleness bug in the WhatsApp inbound path: Confidence Score: 5/5Safe to merge — the fix is well-scoped, backward-compatible, and covered by focused regression tests. All remaining findings are P2 or informational. The core logic in No files require special attention.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7d60d7219
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Addressed the review feedback in follow-up commit Changes made:
Validation run:
@greptile review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eac0982b17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
eac0982 to
2524abb
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
🔒 Aisle Security AnalysisWe found 3 potential security issue(s) in this PR:
1. 🟡 PII (WhatsApp JID) and error details logged during send retry backoff
DescriptionThe
Vulnerable code: logVerbose(
`Waiting ${delayMs}ms for WhatsApp reconnect before retrying send to ${jid}: ${formatError(lastErr)}`,
);RecommendationAvoid logging raw identifiers and full error strings in retry loops.
Example: const redactedJid = jid.replace(/^\d+(?=@)/, (m) => `***${m.slice(-4)}`);
const reason = lastErr instanceof Error ? lastErr.name : "send_failed";
logVerbose(`Waiting ${delayMs}ms for reconnect before retrying send to ${redactedJid}: ${reason}`);If you need full error detail, emit it only to secured traces with access controls, not general logs. 2. 🟡 Unbounded concurrent send retry loops can exhaust resources during WhatsApp reconnect
Description
Because this function is called per outbound operation (reply/media/etc.) and there is no global throttling/queueing, a burst of inbound messages (or a single message that triggers many outbound sends) during a disconnect/reconnect window can create many concurrent retry loops. Each loop can:
If Vulnerable code: for (let attempt = 1; ; attempt++) {
const currentSock = getCurrentSock();
...
const delayMs = computeBackoff(disconnectRetryPolicy, attempt);
await sleepWithAbort(delayMs, options.disconnectRetryAbortSignal);
}RecommendationAdd global backpressure for outbound sends during reconnect, so many concurrent sends do not each start their own retry loop. Options (any one is fine):
Example sketch (shared barrier): let reconnectWait: Promise<void> | null = null;
async function waitForReconnect(signal?: AbortSignal) {
if (!reconnectWait) {
reconnectWait = (async () => {
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
if (getCurrentSock()) return;
await sleepWithAbort(computeBackoff(policy, attempt), signal);
}
throw new Error("reconnect timeout");
})().finally(() => { reconnectWait = null; });
}
return reconnectWait;
}
async function sendTrackedMessage(jid: string, content: AnyMessageContent) {
if (!getCurrentSock()) await waitForReconnect(options.disconnectRetryAbortSignal);
return getCurrentSock()!.sendMessage(jid, content);
}Also ensure all call sites provide 3. 🟡 Potential duplicate WhatsApp sends due to retrying on broad regex-matched errors (includes timeouts)
Description
Vulnerable logic: function isRetryableSendDisconnectError(err: unknown): boolean {
return /closed|reset|timed\s*out|disconnect|no active socket/i.test(formatError(err));
}
...
const result = await currentSock.sendMessage(jid, content);
...
} catch (err) {
if (!shouldRetryDisconnect() || !isRetryableSendDisconnectError(err)) {
throw err;
}
// retry loop continues
}Impact:
RecommendationMake retries idempotent and/or substantially narrow the retry conditions. Suggested changes:
function isRetryableSendDisconnectError(err: unknown): boolean {
const msg = formatError(err);
// Only retry on clear transport/socket replacement conditions
return /no active socket|EPIPE|ECONNRESET|socket closed|disconnect/i.test(msg);
}
Analyzed PR: #62892 at commit Last updated on: 2026-04-09T03:06:21Z |
9ebcfc1 to
13454a9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13454a9aca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| disconnectRetryPolicy.maxAttempts > 0 | ||
| ? disconnectRetryPolicy.maxAttempts | ||
| : DEFAULT_RECONNECT_POLICY.maxAttempts; |
There was a problem hiding this comment.
Allow at least one reconnect wait at maxAttempts=1
sendRetryMaxAttempts is taken directly from disconnectRetryPolicy.maxAttempts when it is positive, so a config of web.reconnect.maxAttempts: 1 causes sendTrackedMessage to fail on the first reconnect-gap iteration before any backoff sleep occurs. In that case, a reply that lands while socketRef.current is temporarily null throws no active socket - reconnection in progress immediately, even though the single reconnect attempt may still complete moments later; this undermines the new “preserve replies across reconnects” behavior for a valid finite-retry configuration.
Useful? React with 👍 / 👎.
13454a9 to
baefbfe
Compare
Summary
Describe the problem and fix in 2–5 bullets:
socketRef.current; only confirmed disconnect-style failures clear the ref.deliver-reply.ts, config, or user-facing defaults.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.monitorWebInbox()builtreply/sendMedia/sendComposingclosures against a specific Baileys socket, whilemonitorWebChannel()replaces that socket during reconnect.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.extensions/whatsapp/src/monitor-inbox.streams-inbound-messages.test.tsUser-visible / Behavior Changes
Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation:Repro + Verification
Environment
Steps
:443for the gateway user to create a reconnect gap.Expected
Actual
main, the real repro hitWhatsApp Web connected.and later still loggedRetrying text ... Connection Closed, which dropped the reply.auto-reply sent (text)logs after reconnect, and the reply arrived successfully.Evidence
Attach at least one:
Verification logs:
main: reconnect succeeded, then later send attempts loggedRetrying text ... Connection Closedauto-reply sent (text)pnpm test extensions/whatsapp/src/monitor-inbox.streams-inbound-messages.test.ts-> 1 file, 17 tests passedpnpm build-> passedeac0982b17Human Verification (required)
What you personally verified (not just CI), and how:
pnpm build; and manually reproduced the broken behavior onmainversus the fixed behavior on this branch with a forced WhatsApp reconnect.Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.