fix: do not announce presence on partial creds.update - #2740
Conversation
The `creds.update` handler announces the push name whenever `creds.me?.name !== update.me?.name`. Partial updates carry no `me` — pre-key churn, and the updates emitted while handling incoming messages — so `name` is `undefined` and the comparison is true for any session that has a stored push name. The `name!` assertion is exactly that assumption. What goes out is a typeless `<presence />`, because encodeBinaryNode filters out undefined attributes. A presence without a type reads as available; the inbound handler in Socket/chats.ts applies the same rule: `attrs.type === 'unavailable' ? 'unavailable' : 'available'`. So the account is marked online on ordinary credential churn — including once per incoming message — no matter what `markOnlineOnConnect` is set to. While it is online, WhatsApp stops pushing notifications to the primary phone, which is the symptom users actually notice. Guarding on a non-empty string keeps genuine push-name updates announced and drops the rest. The assertion goes with it, since the guard now proves the type. Reported in WhiskeySockets#2553. Same root cause and the same one-line fix were identified in WhiskeySockets#2627, which was closed by the stale bot rather than applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks for opening this pull request and contributing to the project! The next step is for the maintainers to review your changes. If everything looks good, it will be approved and merged into the main branch. In the meantime, anyone in the community is encouraged to test this pull request and provide feedback. ✅ How to confirm it worksIf you’ve tested this PR, please comment below with: This helps us speed up the review and merge process. 📦 To test this PR locally:If you encounter any issues or have feedback, feel free to comment as well. |
📝 WalkthroughWalkthroughThe credentials update handler now emits push-name presence updates only when ChangesPush name validation
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Socket/socket.ts (1)
1054-1060: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd integration coverage for push-name presence emission.
This changes wire-level protocol behavior but adds no test. Cover partial updates without
me, empty names, unchanged names, and valid changed names, asserting whethersendNodeemits the expected presence stanza.As per coding guidelines, “New protocol paths or stanza handlers should have integration tests mocking binary nodes, not e2e tests.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Socket/socket.ts` around lines 1054 - 1060, Add integration coverage for the push-name presence logic around the updated pushName path, using mocked binary nodes and observing sendNode calls. Verify that partial updates without creds.me and empty names do not emit presence, unchanged names do not emit presence, and valid changed names emit the expected presence stanza with the name attribute.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Socket/socket.ts`:
- Around line 1054-1060: Add integration coverage for the push-name presence
logic around the updated pushName path, using mocked binary nodes and observing
sendNode calls. Verify that partial updates without creds.me and empty names do
not emit presence, unchanged names do not emit presence, and valid changed names
emit the expected presence stanza with the name attribute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c5a4c3bd-a336-4fc1-b1c0-bdd5481c37b5
📒 Files selected for processing (1)
src/Socket/socket.ts
|
This PR is stale because it has been open for 14 days with no activity. Remove the stale label or comment or this will be closed in 14 days |
Fixes the root cause behind #2553. The same diagnosis and the same one-line fix were posted in
#2627 and acknowledged there, but that issue was closed by the stale bot rather than applied, so
masterstill carries the code.The bug
Socket/socket.tsannounces the push name on everycreds.update:A partial update carries no
me— pre-key churn, and the updates emitted while handling incomingmessages — so
nameisundefined, andcreds.me?.name !== undefinedistruefor any sessionthat has a stored push name. The
name!assertion is precisely the assumption that fails.What then goes out is a typeless
<presence />:encodeBinaryNodefilters outundefinedattributes, so nothing is left. And a presence without a type means available — the inbound
handler in
Socket/chats.tsapplies exactly that rule:The result is an account marked online on ordinary credential churn, including once per
incoming message, whatever
markOnlineOnConnectis set to. Nothing is logged, because the sendsucceeds. While the account is online, WhatsApp stops pushing notifications to the primary phone
— which is the symptom people actually report.
Reproducing it without sending anything
Read-only client,
markOnlineOnConnect: false, already paired:reads as offline.
keeps it online.
The change
Guard on a non-empty string. Genuine push-name updates are still announced; partial credential
patches are not. The
!assertion goes with it, since the guard now proves the type.Verification
npm install(which runsprepare→tsc -P tsconfig.build.json) compiles clean.prettier --check src/Socket/socket.tspasses.makeSocketfactory, andI did not want to bolt fragile scaffolding onto an unfamiliar suite for a two-condition guard.
Happy to add one if you would like it, in whatever shape you prefer.
Summary by cubic
Stop announcing presence on partial
creds.updateto prevent unintended “online” status and missed phone notifications. Only send a presence update when the push name actually changes.src/Socket/socket.ts: send presence only ifupdate.me?.nameis a non-empty string and differs fromcreds.me?.name.namedirectly toattrs.<presence />on partial updates, which previously marked accounts as online during pre-key churn or incoming message handling.Written for commit c9d5b42. Summary will update on new commits.
Summary by CodeRabbit