fix: sync own device list at login and parse server key-index in usync - #479
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughPost-login startup now performs an explicit synchronization of the client's own device list: a new Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,230,255,0.5)
participant Client
end
rect rgba(220,255,220,0.5)
participant PersistenceMgr
end
rect rgba(255,230,240,0.5)
participant UsyncServer
end
Client->>Client: check_generation!()
Client->>PersistenceMgr: read own device snapshot
PersistenceMgr-->>Client: local snapshot (pn/lid)
Client->>Client: derive target JIDs (pn,lid → non-ad)
Client->>Client: invalidate device cache entries
alt no targets
Client->>Client: return early
else has targets
Client->>UsyncServer: get_user_devices(jids)
UsyncServer-->>Client: UserDeviceList with UsyncDevice{device, key_index?}
Client->>Client: update caches/log fetched count
Client->>Client: log_sync_error on non-fatal errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Bencher Report![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client.rs`:
- Around line 1949-1951: Replace the raw warn! call used after
client_clone.sync_own_device_list().await with the existing helper
log_sync_error() so the sync failure logs follow the same reconnect/shutdown
suppression as other startup syncs; locate the block where you call
client_clone.sync_own_device_list().await and change the error branch to call
log_sync_error("sync_own_device_list", &e) (or the project’s log_sync_error
signature) instead of warn!("Failed to sync own device list: {e:?}").
In `@src/usync.rs`:
- Around line 170-183: The current sequence calls
invalidate_device_cache(&pn_bare.user) / invalidate_device_cache(&lid_bare.user)
before calling get_user_devices(&jids), which deletes the persisted
DeviceListRecord and loses key_index and raw_id metadata that get_user_devices
expects to merge; to fix, avoid deleting the persisted record before refresh:
either (A) move calls to invalidate_device_cache so they happen after devices =
self.get_user_devices(&jids).await? or (B) snapshot the existing
DeviceListRecord (read prior record for each user) before
invalidate_device_cache and after devices = self.get_user_devices(&jids).await
merge the saved key_index and raw_id back into the returned DeviceListRecord
entries; locate and update the invalidate_device_cache, get_user_devices, and
the device_snapshot / jids logic to implement one of these approaches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d5c628ca-a8bb-4912-b0fb-8cf9281f5d77
📒 Files selected for processing (2)
src/client.rssrc/usync.rs
| self.invalidate_device_cache(&pn_bare.user).await; | ||
| jids.push(pn_bare); | ||
| } | ||
| if let Some(ref lid) = device_snapshot.lid { | ||
| let lid_bare = lid.to_non_ad(); | ||
| self.invalidate_device_cache(&lid_bare.user).await; | ||
| jids.push(lid_bare); | ||
| } | ||
|
|
||
| if jids.is_empty() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let devices = self.get_user_devices(&jids).await?; |
There was a problem hiding this comment.
This refresh discards the metadata source that get_user_devices() uses to preserve key_index and raw_id.
get_user_devices() merges the previous DeviceListRecord so key_index values learned via account_sync and the prior raw_id survive usync responses that omit key_index_bytes. invalidate_device_cache() deletes that persisted record first, so this login sync can rewrite our own device list without that metadata unless the server happens to resend it. Force a network refresh without deleting the previous record first, or snapshot and merge the old key_index/raw_id fields back into the refreshed result.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/usync.rs` around lines 170 - 183, The current sequence calls
invalidate_device_cache(&pn_bare.user) / invalidate_device_cache(&lid_bare.user)
before calling get_user_devices(&jids), which deletes the persisted
DeviceListRecord and loses key_index and raw_id metadata that get_user_devices
expects to merge; to fix, avoid deleting the persisted record before refresh:
either (A) move calls to invalidate_device_cache so they happen after devices =
self.get_user_devices(&jids).await? or (B) snapshot the existing
DeviceListRecord (read prior record for each user) before
invalidate_device_cache and after devices = self.get_user_devices(&jids).await
merge the saved key_index and raw_id back into the returned DeviceListRecord
entries; locate and update the invalidate_device_cache, get_user_devices, and
the device_snapshot / jids logic to implement one of these approaches.
There was a problem hiding this comment.
It deletes from DB at line 186. This means get_user_devices won't find the old record in DB either → existing_key_indices will be empty → key_index
falls back to None for devices that the server doesn't return key-index for.
But now that we parse key-index from the server response (our fix), this is actually fine — the server provides key-index for companion devices. The
only case where the old cached key_index matters is device 0 (which has no key-index attribute in the response), but device 0 is always kept by
filter_devices_by_key_index regardless of key_index.
Let me verify by checking the actual server response from the logs:
← no key-index (always kept)
← server provides it
← server provides it
← server provides it
Server provides key-index for all companion devices. The existing_key_indices fallback is only for device 0, which doesn't need it. The invalidation
before fetch is fine with our key-index parsing fix. Skip.
There was a problem hiding this comment.
@jlucaso1, that's a solid analysis and the log evidence makes it clear — thanks for walking through it.
To summarize the reasoning for posterity:
- Device 0 — server never sends
key-index, butfilter_devices_by_key_indexunconditionally retains it, so no fallback to the cached value is needed. - Companion devices — the server includes
key-indexdirectly in the usync response (e.g.,key-index="93"), soexisting_key_indicesis never needed for them either. raw_id— already handled by the identity-change detection path and only reused whenkey_index_bytesis absent; for a full own-device sync the server sendskey_index_bytes, soraw_idis decoded fresh.
The invalidation-before-fetch sequence is therefore safe given that the server supplies all necessary metadata in the response. I'll withdraw the concern.
✏️ Learnings added
Learnt from: jlucaso1
Repo: jlucaso1/whatsapp-rust PR: 479
File: src/usync.rs:173-186
Timestamp: 2026-04-01T22:10:15.983Z
Learning: In `src/usync.rs` (`Client::sync_own_device_list`), calling `invalidate_device_cache` before `get_user_devices` for the own-JID refresh is intentional and safe. The WhatsApp server always provides `key-index` attributes for companion devices (device id > 0) in the usync response. Device 0 never has a `key-index` attribute but is unconditionally kept by `filter_devices_by_key_index`, so the `existing_key_indices` fallback in `get_user_devices` is never needed for the own-device-list sync case.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2c460d4 to
6507182
Compare
Two bugs fixed: 1. Own device list not synced at login (WA Web calls syncMyDeviceList during bootstrap). Without this, companion devices added after initial pairing were missing from the DM fan-out. 2. Server-returned key-index attributes on <device> nodes were discarded by the usync parser, causing ADV filtering to remove companion devices that had key_index=None in the stale cache. Changes: - New UsyncDevice struct carries device + key_index from server response - Usync parser extracts key-index attribute from <device> nodes - Server key_index takes priority over cached value in device list processing - New sync_own_device_list() called during post-login init
6507182 to
6fe4c35
Compare
jlucaso1
changed the title
|
@claude review |
|
Claude finished @jlucaso1's task in 1m 7s —— View job Code Review: PR #479 — Sync own device list at login
Overall this is a clean, well-scoped fix. The two bugs are clearly identified, the fix is minimal, and the test covers the core parsing change. A few observations:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fe4c35496
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.invalidate_device_cache(&pn_bare.user).await; | ||
| jids.push(pn_bare); |
There was a problem hiding this comment.
sync_own_device_list() deletes the persisted/cache device record before attempting the network usync, so a transient failure during that fetch (e.g., reconnect race or IQ error) leaves the account with no local own-device list. Because DM send later does get_user_devices(own_jid)?, this turns a best-effort startup sync into a hard send failure path whenever the refetch also fails, instead of falling back to the previously known devices. Consider only replacing/invalidation after a successful fetch, or restoring old data on failure.
Useful? React with 👍 / 👎.
1 participant
Summary
DM messages were not reaching WA Web and other companion devices. Two bugs:
Own device list not synced at login — WA Web calls
syncMyDeviceList()during bootstrap. We weren't doing this, so companion devices added after pairing were missing from the DM fan-out.Server-returned
key-indexdiscarded — The usync parser extracted device IDs but ignoredkey-indexattributes. Without key-index, ADV filtering removed all companion devices.Changes
wacore/src/usync.rs: NewUsyncDevicestruct withdevice + key_index. Parser extractskey-indexattribute. Unit test added.wacore/src/iq/usync.rs: Same parser fix for the IQ-level response handler.src/usync.rs: Server key_index takes priority over cached. Newsync_own_device_list().src/client.rs: Callsync_own_device_list()during post-login init, useslog_sync_errorfor shutdown suppression.Test plan
test_server_returned_key_index_is_parsed— verifies key-index parsing from usync responseSynced own device list from server: 8 devices, DM fan-out includes WA WebSummary by CodeRabbit
New Features
Bug Fixes