Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1944,8 +1944,12 @@ impl Client {
// Don't fail login - PDO will retry via ensure_e2e_sessions fallback
}

// === Passive Tasks (mimics WhatsApp Web's PassiveTaskManager) ===
// WhatsApp Web executes passive tasks (like PreKey upload) BEFORE sending the active IQ.
// Sync own device list so DM fan-out includes all companions
check_generation!();
if let Err(e) = client_clone.sync_own_device_list().await {
client_clone.log_sync_error("sync own device list", &e);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

check_generation!();
if !client_clone.is_connected() {
debug!("Skipping passive tasks: connection closed");
Expand Down
40 changes: 36 additions & 4 deletions src/usync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,13 @@ impl Client {
.iter()
.map(|d| wacore::store::traits::DeviceInfo {
device_id: d.device as u32,
key_index: existing_key_indices
.get(&(d.device as u32))
.copied()
.flatten(),
// Server-returned key_index takes priority over cached
key_index: d.key_index.or_else(|| {
existing_key_indices
.get(&(d.device as u32))
.copied()
.flatten()
}),
})
.collect();

Expand Down Expand Up @@ -158,6 +161,35 @@ impl Client {

Ok(all_devices)
}

/// Sync own device list from the server, bypassing cache.
/// Matches WA Web's `syncMyDeviceList()` called during bootstrap.
pub(crate) async fn sync_own_device_list(&self) -> Result<(), anyhow::Error> {
let device_snapshot = self.persistence_manager.get_device_snapshot().await;

let mut jids = Vec::with_capacity(2);
if let Some(ref pn) = device_snapshot.pn {
let pn_bare = pn.to_non_ad();
self.invalidate_device_cache(&pn_bare.user).await;
jids.push(pn_bare);
Comment on lines +173 to +174

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep existing own-device cache until refresh succeeds

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 👍 / 👎.

}
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?;
Comment on lines +173 to +186

@coderabbitai coderabbitai Bot Apr 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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, but filter_devices_by_key_index unconditionally retains it, so no fallback to the cached value is needed.
  • Companion devices — the server includes key-index directly in the usync response (e.g., key-index="93"), so existing_key_indices is never needed for them either.
  • raw_id — already handled by the identity-change detection path and only reused when key_index_bytes is absent; for a full own-device sync the server sends key_index_bytes, so raw_id is 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.

log::info!(
"Synced own device list from server: {} devices",
devices.len()
);
Ok(())
}
}

#[cfg(test)]
Expand Down
13 changes: 8 additions & 5 deletions wacore/src/iq/usync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,13 +610,16 @@ impl IqSpec for DeviceListSpec {
continue;
};

let mut device_jid = user_jid.clone();
device_jid.device = device_id;
devices.push(device_jid);
let key_index = device_node
.attrs()
.optional_string("key-index")
.and_then(|s| s.parse::<u32>().ok());
devices.push(crate::usync::UsyncDevice {
device: device_id,
key_index,
});
}

// WA Web: AdvForUsyncApi rejects usync results with companion
// devices but no signedKeyIndexBytes
let has_companion = devices.iter().any(|d| d.device != 0);
if has_companion && key_index_bytes.is_none() {
warn!(
Expand Down
95 changes: 85 additions & 10 deletions wacore/src/usync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,17 @@ pub struct UsyncLidMapping {
pub lid: String,
}

/// Device list with optional phash from usync response
#[derive(Debug, Clone)]
pub struct UsyncDevice {
pub device: u16,
pub key_index: Option<u32>,
}

#[derive(Debug, Clone)]
pub struct UserDeviceList {
/// The user JID (without device suffix)
pub user: Jid,
/// List of device JIDs for this user
pub devices: Vec<Jid>,
/// Participant hash from device-list node (used for cache validation)
pub devices: Vec<UsyncDevice>,
pub phash: Option<String>,
/// Signed key index bytes from `<key-index-list>` (for ADV device filtering)
pub key_index_bytes: Option<Vec<u8>>,
}

Expand Down Expand Up @@ -103,9 +104,14 @@ pub fn parse_get_user_devices_response_with_phash(resp_node: &Node) -> Result<Ve
}
};

let mut device_jid = user_jid.clone();
device_jid.device = device_id;
devices.push(device_jid);
let key_index = device_node
.attrs()
.optional_string("key-index")
.and_then(|s| s.parse::<u32>().ok());
devices.push(UsyncDevice {
device: device_id,
key_index,
});
}

// WA Web: WAWebHandleAdvForUsyncApi.handleADVSyncResult() rejects usync results
Expand Down Expand Up @@ -135,7 +141,14 @@ pub fn parse_get_user_devices_response_with_phash(resp_node: &Node) -> Result<Ve
pub fn parse_get_user_devices_response(resp_node: &Node) -> Result<Vec<Jid>> {
Ok(parse_get_user_devices_response_with_phash(resp_node)?
.into_iter()
.flat_map(|u| u.devices)
.flat_map(|u| {
let user_jid = u.user;
u.devices.into_iter().map(move |d| {
let mut jid = user_jid.clone();
jid.device = d.device;
jid
})
})
.collect())
}

Expand Down Expand Up @@ -415,4 +428,66 @@ mod tests {
assert_eq!(result[0].devices.len(), 0);
assert_eq!(result[0].phash, Some("2:empty".to_string()));
}

#[test]
fn test_server_returned_key_index_is_parsed() {
// Build a response where devices have key-index attributes
// (matches real server: <device id="4" key-index="93"/>)
let device_nodes: Vec<Node> = vec![
NodeBuilder::new("device").attr("id", "0").build(),
NodeBuilder::new("device")
.attr("id", "4")
.attr("key-index", "93")
.build(),
NodeBuilder::new("device")
.attr("id", "24")
.attr("key-index", "113")
.build(),
];

let ki_bytes = build_test_key_index_bytes(&[0, 4, 24]);
let device_list = NodeBuilder::new("device-list")
.children(device_nodes)
.build();
let devices_node = NodeBuilder::new("devices")
.children(vec![
device_list,
NodeBuilder::new("key-index-list")
.attr("ts", "1000")
.bytes(ki_bytes)
.build(),
])
.build();

let user_node = NodeBuilder::new("user")
.attr("jid", "559900001111@s.whatsapp.net")
.children(vec![devices_node])
.build();

let response = NodeBuilder::new("iq")
.children(vec![
NodeBuilder::new("usync")
.children(vec![
NodeBuilder::new("list").children(vec![user_node]).build(),
])
.build(),
])
.build();

let result = parse_get_user_devices_response_with_phash(&response).unwrap();
assert_eq!(result.len(), 1);
assert_eq!(result[0].devices.len(), 3);

// Device 0: no key-index attribute → None
assert_eq!(result[0].devices[0].device, 0);
assert_eq!(result[0].devices[0].key_index, None);

// Device 4: key-index="93"
assert_eq!(result[0].devices[1].device, 4);
assert_eq!(result[0].devices[1].key_index, Some(93));

// Device 24: key-index="113"
assert_eq!(result[0].devices[2].device, 24);
assert_eq!(result[0].devices[2].key_index, Some(113));
}
}
Loading