fix: WA Web-compliant DM multi-device fanout with phash validation - #524
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDM send now resolves devices registry-first (fallback to network), builds per-device fanout including all recipient and companion devices (excluding the exact sender), computes and returns a DM participant hash (phash) via Changes
Sequence Diagram(s)sequenceDiagram
participant Sender as Sender Client
participant Reg as Device Registry
participant E2E as E2E Session Manager
participant Server as WA Server
Sender->>Reg: get_devices_from_registry(recipient, own)
alt registry miss
Reg-->>Sender: miss
Sender->>Reg: get_user_devices(recipient/own) (network)
Reg-->>Sender: devices
else cached
Reg-->>Sender: devices
end
Sender->>Sender: filter non-hosted, exclude exact sender, dedupe -> all_dm_jids
Sender->>E2E: ensure_e2e_sessions(all_dm_jids)
E2E-->>Sender: sessions ready
Sender->>Sender: prepare_dm_stanza(all_dm_jids) => PreparedDmStanza(node, phash)
Sender->>Server: send(prepared.node)
Server-->>Sender: ack (phash)
alt ack phash mismatch & recipient is user (not group/status)
Sender->>Reg: invalidate_device_cache(recipient.user)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@claude review |
|
Claude finished @jlucaso1's task in 2m 27s —— View job Code Review: DM Multi-Device Fanout
Overall this is a well-structured change that aligns the DM send path with WA Web behavior. The local-registry-first approach, hosted device filtering, and phash validation are all sound. A few observations: Potential issue: empty device list after hosted filtering
The PR description explicitly says "if record exists but all devices are filtered, don't invent a bare JID target (matches WA Web without biz-coex gate)" — so this appears intentional. Just flagging that this edge case would result in a message sent to nobody (no server fanout either). If in practice this only happens for business-coex accounts that WA Web also skips, it's fine. Missing
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/send.rs`:
- Around line 1080-1096: The warmup currently calls
ensure_e2e_sessions(&all_dm_jids) before excluding the current sender device,
allowing a self-session to be created; update the code that builds all_dm_jids
(the match on recipient_cached and the handling of own_cached) to explicitly
filter out the current sender's full JID from own_devices (the same device JID
that prepare_dm_stanza later filters) before reserving/appending and before
calling ensure_e2e_sessions, so ensure_e2e_sessions never receives the sender's
device JID.
- Around line 669-670: The current call to
client.invalidate_device_cache(&jid.user) is wrong for group/status sends
because jid.user is the chat ID (group/status) not a participant account; change
the logic in send.rs so that invalidate_device_cache is only invoked for 1:1
sends (detect via the send path that handles direct JIDs) and for group or
status sends either do not call invalidate_device_cache with the chat ID or
instead accept and use the actual participant user IDs passed into the task to
invalidate each participant’s device cache; update the code paths that call
client.invalidate_device_cache and the task signature that triggers this call so
group/status flows pass the affected participant user IDs when eviction is
required.
In `@wacore/src/send.rs`:
- Around line 708-715: Compute participant_list_hash from the filtered DM fanout
(the list returned by partition_dm_devices / the variable holding the DM-target
devices) instead of from the raw all_devices; update both the
DeviceSentMessage.phash assignment and the stanza-level phash to use
MessageUtils::participant_list_hash(filtered_list). This ensures you call
MessageUtils::participant_list_hash on the same device list that is actually
sent (i.e., after removing the current sender device) so DeviceSentMessage.phash
and the stanza phash match the <participants> set.
🪄 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: c0d8b7f0-6caf-40a0-8ee8-3784d14d74d3
📒 Files selected for processing (2)
src/send.rswacore/src/send.rs
Benchmark Results1 regression(s) detected (>2% threshold):
58 unchanged benchmark(s)
|
![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: 0845e0269f
ℹ️ 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".
| let recipient_plaintext = MessageUtils::encode_and_pad(&message_for_encryption); | ||
|
|
||
| // Compute phash before DSM so own companions learn the device set | ||
| let phash = MessageUtils::participant_list_hash(&all_devices).ok(); |
There was a problem hiding this comment.
phash is derived from all_devices before partition_dm_devices removes the exact sending device, so the hash can include a device that never gets an <enc> payload. When the local own-device list contains the current sender (common for multi-device accounts), this makes the stanza-level phash inconsistent with the actual DM fanout set and can trigger false phash-mismatch invalidations (unnecessary cache/DB churn and avoidable re-syncs).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/send.rs`:
- Around line 669-672: The device-cache invalidation currently runs for any
non-group chat but should skip the WhatsApp status broadcast JID (whose jid.user
== "status"); update the conditional around
client.invalidate_device_cache(&jid.user).await so it only runs when
!jid.is_group() AND jid.user != "status" (i.e., skip status@broadcast) to avoid
meaningless DB calls — change the check that surrounds invalidate_device_cache
to include this user check.
🪄 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: 45b8eef6-6b57-4f01-9e37-487f24ed8d19
📒 Files selected for processing (2)
src/send.rswacore/src/send.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/send.rs`:
- Around line 1092-1104: After merging recipient_cached and own_cached into
all_dm_jids, remove duplicate device entries before the
is_sender-filter/ensure_e2e_sessions call: deduplicate all_dm_jids (e.g. by
turning it into a HashSet keyed by the device identity or by sorting+deduping on
the JID+device tuple) so that ensure_e2e_sessions() and prepare_dm_stanza()
never see duplicate companion devices; update the code around the all_dm_jids
manipulation (the block that appends own_devices and the subsequent retain using
own_lid) to perform this dedup step just before calling ensure_e2e_sessions().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf25577f5a
ℹ️ 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".
| if let Some(mut own_devices) = own_cached { | ||
| own_devices.retain(|j| !j.is_hosted()); | ||
| all_dm_jids.append(&mut own_devices); |
There was a problem hiding this comment.
This merge path can duplicate targets when the recipient is your own account ("message yourself"): recipient_cached and own_cached both contain the same devices, and append adds them twice. The duplicated all_dm_jids is then used for DM encryption, so the stanza can contain repeated <to> payloads and a phash over duplicates, which can trigger unnecessary phash-mismatch invalidations and resend churn for self-DMs. Deduplicate the merged list (after PN/LID normalization) before session setup/encryption.
Useful? React with 👍 / 👎.
cf25577 to
bcda340
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/send.rs (1)
1092-1104:⚠️ Potential issue | 🟠 MajorDeduplicate
all_dm_jidsfor self-DM (note-to-self) sends.When
totargets the current account, bothrecipient_cachedandown_cachedresolve to the same user's devices. After appending at Line 1094,all_dm_jidscontains duplicates of companion devices. The sender filter (Lines 1100-1104) only removes the exact sender device, leaving other duplicates intact.This causes duplicate
<to>nodes in the stanza, incorrectphashcomputation, and redundant session warmup.Suggested fix — deduplicate before ensure_e2e_sessions
// Exclude exact sender device (WA Web: isMeDevice in getFanOutList) // so ensure_e2e_sessions never creates a self-session let own_lid = device_snapshot.lid.as_ref(); all_dm_jids.retain(|j| { let is_sender = (j.is_same_user_as(own_jid) && j.device == own_jid.device) || own_lid.is_some_and(|lid| j.is_same_user_as(lid) && j.device == lid.device); !is_sender }); + // Dedup for self-DMs where recipient and own devices overlap + wacore::types::jid::sort_dedup_by_device(&mut all_dm_jids); self.ensure_e2e_sessions(&all_dm_jids).await?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/send.rs` around lines 1092 - 1104, all_dm_jids can contain duplicate JIDs when both recipient_cached and own_cached resolve to the same account, causing duplicate <to> nodes and redundant session work; after appending own_devices (the append at the block that uses own_cached) and after the sender filter that removes only the exact sender, deduplicate all_dm_jids (e.g., by JID+device identity) before calling ensure_e2e_sessions so only unique device JIDs are used; locate the all_dm_jids variable in src/send.rs and apply a stable dedupe (or HashSet-based unique filter) right after the retain that excludes the sender and before ensure_e2e_sessions to prevent duplicate entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/send.rs`:
- Around line 1092-1104: all_dm_jids can contain duplicate JIDs when both
recipient_cached and own_cached resolve to the same account, causing duplicate
<to> nodes and redundant session work; after appending own_devices (the append
at the block that uses own_cached) and after the sender filter that removes only
the exact sender, deduplicate all_dm_jids (e.g., by JID+device identity) before
calling ensure_e2e_sessions so only unique device JIDs are used; locate the
all_dm_jids variable in src/send.rs and apply a stable dedupe (or HashSet-based
unique filter) right after the retain that excludes the sender and before
ensure_e2e_sessions to prevent duplicate entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f88653bb-e4f5-4771-8c07-c4090d6e6a8c
📒 Files selected for processing (2)
src/send.rswacore/src/send.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcda340940
ℹ️ 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".
| if !jid.is_group() && !jid.is_status_broadcast() { | ||
| client.invalidate_device_cache(&jid.user).await; | ||
| } |
There was a problem hiding this comment.
prepare_dm_stanza now hashes both recipient and own companion targets into DM phash, but this mismatch handler only invalidates the recipient user’s device cache (jid.user). If the mismatch is caused by our own stale companion list (e.g., a newly linked device), subsequent sends keep using the stale own-device registry and continue missing self-fanout/resync for that device. The DM phash self-healing path should also invalidate the local account’s device cache, not just the destination user.
Useful? React with 👍 / 👎.
|
@claude review |
|
Claude finished @jlucaso1's task in 2m 38s —— View job Code Review: DM Multi-Device Fanout
This is a solid PR. The previous automated review rounds (CodeRabbit, Codex) caught real issues — phash computed before partition, sender device in 1. Self-DM deduplication (outstanding from prior reviews)
This causes:
Self-DMs are a real use case ("Message Yourself"). A simple dedup after the sender filter would fix this: // After line 1104 (the retain that excludes sender)
all_dm_jids.sort_unstable();
all_dm_jids.dedup();This assumes 2. DM phash mismatch doesn't invalidate own device cache
Consider also invalidating the sender's own device cache: if !jid.is_group() && !jid.is_status_broadcast() {
client.invalidate_device_cache(&jid.user).await;
// Own devices are also in the DM phash — invalidate sender's cache too
if let Some(snapshot) = client.get_device_snapshot().await {
client.invalidate_device_cache(&snapshot.jid.user).await;
}
}3.
|
The DM send path encrypted only for the recipient's bare JID (device 0) and relied on server-side fanout to deliver to companion devices. However, companion devices (WhatsApp Web, Desktop) often receive <unavailable> instead of the encrypted payload, causing "Waiting for this message". Fix: fetch all recipient devices via get_user_devices() and encrypt for each one individually, same as mobile clients do. Falls back to bare JID if no devices are known. Combined with 704218e (retry receipt on <unavailable> for receive), this fixes bidirectional linked device message delivery: - Receive: bot gets <unavailable> → retry receipt → PDO relay ✓ (704218e) - Send: bot encrypts for all devices → direct delivery ✓ (this commit) Tested: messages now appear instantly on both primary phone AND WhatsApp Web/Desktop clients.
…ering Reworks the DM multi-device fanout from #523 to match WA Web behavior: - Local registry first: read device list from local cache/DB only on the send path (WAWebApiDeviceList.getDeviceIds reads IndexedDB). Network fetch via get_user_devices only on cache miss, errors non-fatal. - Both recipient AND own devices use local-first pattern (no hard .await? on get_user_devices for either). - Filter hosted/Cloud API devices (device 99 / @HosteD) from DM fanout (WAWebDBDeviceListFanout excludes these). - Bare JID fallback only when no local record exists (not when all devices are hosted-filtered). - Compute and embed phash in DM stanzas + DSM wrapper so server can signal device-list drift via ack. - Invalidate device cache on phash mismatch for self-healing. - Avoid LID-migration side effects on hot path by checking cache before calling get_user_devices. Co-authored-by: Mathias Caldas <mathiascaldas@gmail.com>
…sent set - Exclude exact sender device (PN + LID) from all_dm_jids before ensure_e2e_sessions, matching WA Web's isMeDevice filter in getFanOutList. Prevents creating a self-session. - Guard invalidate_device_cache to non-group JIDs only — device cache is user-scoped, group/status JID user parts are chat IDs not accounts. - Compute phash after partition_dm_devices from the actual sent set (recipient_devices + own_other_devices), so DSM.phash and stanza phash match the <participants> set rather than including the sender.
When a PreKeyMessage references a consumed one-time prekey, the session may exist under a PN address (legacy migration scenario). Attempting PN→LID migration lets Signal use the existing ratchet state to decrypt without needing the one-time prekey — same pattern already used for SessionNotFound errors.
WA Web only sends phash on group/broadcast stanzas, not DMs. The server returns phash in the ACK on mismatch. Keep the local phash computation for spawn_phash_validation comparison but don't embed it in the stanza attr or DeviceSentMessage proto. Returns PreparedDmStanza from prepare_dm_stanza so the caller can access the computed phash without it being on the wire.
390e4c8 to
c3a01d1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3a01d1d5e
ℹ️ 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".
| let ack = if let Some(phash) = dm_phash | ||
| && let Some(msg_id) = stanza_to_send |
There was a problem hiding this comment.
This condition now checks dm_phash, which is only populated in the 1:1 DM branch, so normal group sends no longer register an ACK waiter even when their stanza carries a phash (set by prepare_group_stanza). As a result, group phash mismatches are never observed and the self-healing invalidation path in spawn_phash_validation no longer runs for group messages, leaving stale sender-key/device cache state in place after server-reported drift.
Useful? React with 👍 / 👎.
- Dedup all_dm_jids after sender exclusion to handle self-DMs where recipient and own device lists overlap (WA Web uses Map for dedup) - Invalidate own device cache on DM phash mismatch, not just recipient (WA Web: syncDeviceListJob([recipient, me])) - Swap inverted invalidate_group_cache booleans: true for group path, false for DM path
|
@claude review |
|
Claude finished @jlucaso1's task in 2m 50s —— View job Code Review: DM Multi-Device Fanout (Round 3)
This is the third review pass, covering all 7 commits (d44d1a9 through 0b7fe97). The PR has evolved well — most of the issues flagged by CodeRabbit, Codex, and prior Claude reviews have been addressed. Here's the current state: Previously flagged issues — now resolved
Remaining issues1. Group phash ACK validation is now silently broken P1
This is a regression from the pre-PR behavior where phash was read from the stanza's The status broadcast path ( Fix: For the group branch within let ack = if let Some(phash) = dm_phash
.or_else(|| stanza_to_send.attrs().optional_string("phash").map(|s| s.into_owned()))
&& let Some(msg_id) = stanza_to_send.attrs().optional_string("id").map(|s| s.into_owned())
{The 2.
This is now correct — status/group sends should invalidate the group cache on phash mismatch, and DMs should not. The earlier Claude review (round 2) incorrectly flagged this as "inverted." Looking at commit 0b7fe97, the booleans were swapped to their correct positions. Apologies for the false flag in round 2. 3. Own device cache invalidation uses
4. Minor:
What looks good
VerdictOne actionable issue: the group phash ACK regression (issue 1) is real and should be fixed before merge — group sends through |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/send.rs (1)
1064-1096:⚠️ Potential issue | 🟠 MajorSelf-DM sends can produce duplicate device entries in
all_dm_jids.When sending a note-to-self (where
totargets the sender's own account), bothrecipient_cachedandown_cachedresolve to the same device list. After Lines 1084-1091 build fromrecipient_cachedand Lines 1093-1096 appendown_cached, the list contains duplicates.These duplicates propagate to
ensure_e2e_sessions(unnecessary session checks) andprepare_dm_stanza(duplicate encryption operations, potentially malformed phash).Consider deduplicating after building the list:
Suggested fix
if let Some(mut own_devices) = own_cached { own_devices.retain(|j| !j.is_hosted()); all_dm_jids.append(&mut own_devices); } + // Dedup for self-DM case where recipient == own account + wacore::types::jid::sort_dedup_by_device(&mut all_dm_jids); + // Exclude exact sender device (WA Web: isMeDevice in getFanOutList)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/send.rs` around lines 1064 - 1096, The DM fanout builds all_dm_jids from recipient_cached and then appends own_cached which causes duplicate device entries for self-DMs; after building all_dm_jids (the Vec built from recipient_cached/recipient_bare and appended own_devices) deduplicate entries before calling ensure_e2e_sessions and prepare_dm_stanza—e.g., collapse by device JID identity (use the JID equality/hash or Jid::to_string) preserving one entry per device; update the code around resolve_encryption_jid, get_devices_from_registry, and the all_dm_jids construction to perform this dedupe so ensure_e2e_sessions and prepare_dm_stanza receive unique device JIDs.
🤖 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/message.rs`:
- Around line 954-973: When handling the post-UntrustedIdentity retry where
retry_err is checked for InvalidPreKeyId (the branch that currently maps to
RetryReason::InvalidKeyId), call try_pn_to_lid_migration_decrypt(...) with the
same arguments used in the earlier direct InvalidPreKeyId branch
(sender_encryption_jid, &signal_address, &parsed_message, &mut adapter, &mut
rng, &enc_type, padding_version, info) and if it returns true treat it as a
successful decrypt (set any_success / continue) instead of immediately
converting the error to RetryReason::InvalidKeyId; this mirrors the PN→LID
fallback logic already implemented and ensures legacy PN-session migrations are
attempted after clearing an UntrustedIdentity.
---
Duplicate comments:
In `@src/send.rs`:
- Around line 1064-1096: The DM fanout builds all_dm_jids from recipient_cached
and then appends own_cached which causes duplicate device entries for self-DMs;
after building all_dm_jids (the Vec built from recipient_cached/recipient_bare
and appended own_devices) deduplicate entries before calling ensure_e2e_sessions
and prepare_dm_stanza—e.g., collapse by device JID identity (use the JID
equality/hash or Jid::to_string) preserving one entry per device; update the
code around resolve_encryption_jid, get_devices_from_registry, and the
all_dm_jids construction to perform this dedupe so ensure_e2e_sessions and
prepare_dm_stanza receive unique device JIDs.
🪄 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: f90e9d37-6fc9-42ba-9046-380373a7f62e
📒 Files selected for processing (3)
src/message.rssrc/send.rswacore/src/send.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b7fe97f29
ℹ️ 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".
| Some(mut devices) => { | ||
| devices.retain(|j| !j.is_hosted()); | ||
| devices |
There was a problem hiding this comment.
When recipient_cached exists but becomes empty after hosted-device filtering, this branch returns an empty recipient fanout instead of adding recipient_bare. In that case the DM stanza can be built with only own-device payloads (or no recipient payload at all), so the remote user cannot decrypt/receive the message until cache state changes. This is a send-path regression versus the previous unconditional bare-recipient fallback and is reproducible with empty/stale registry records or hosted-only cached entries.
Useful? React with 👍 / 👎.
The post-UntrustedIdentity retry path handled InvalidPreKeyId by immediately sending a retry receipt without attempting PN→LID session migration. This mirrors the migration already added for the direct InvalidPreKeyId error path.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/send.rs (1)
1147-1162:⚠️ Potential issue | 🟠 MajorRestore phash ACK validation for regular group sends.
dm_phashis only populated in the 1:1 branch, so the waiter registration below now returnsNonefor non-status group messages. That meansspawn_phash_validation()never runs after a group send, and stale sender-key / group caches will no longer self-heal on a group participant hash mismatch.Suggested fix
- let ack = if let Some(phash) = dm_phash + let phash_for_validation = if let Some(phash) = dm_phash { + Some((phash, false)) + } else if tc_issue_target.is_group() { + stanza_to_send + .attrs() + .optional_string("phash") + .map(|s| (s.into_owned(), true)) + } else { + None + }; + + let ack = if let Some((phash, invalidate_group_cache)) = phash_for_validation && let Some(msg_id) = stanza_to_send .attrs() .optional_string("id") .map(|s| s.into_owned()) { let rx = self.register_ack_waiter(&msg_id).await; - Some((rx, phash, msg_id)) + Some((rx, phash, invalidate_group_cache, msg_id)) } else { None }; @@ - if let Some((rx, phash, msg_id)) = ack { - self.spawn_phash_validation(rx, phash, tc_issue_target.clone(), false, msg_id); + if let Some((rx, phash, invalidate_group_cache, msg_id)) = ack { + self.spawn_phash_validation( + rx, + phash, + tc_issue_target.clone(), + invalidate_group_cache, + msg_id, + ); }Also applies to: 1165-1186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/send.rs` around lines 1147 - 1162, The phash ACK validation was lost for non-1:1 group sends because dm_phash is only set in the 1:1 branch; restore it by assigning dm_phash = prepared.phash after calling wacore::send::prepare_dm_stanza (and any equivalent prepare_* call used for group messages) so the waiter registration receives a Some(phash) for regular group sends too; ensure the subsequent waiter registration and spawn_phash_validation() call use that dm_phash value (affecting the code paths around prepare_dm_stanza/ prepared.node and the waiter registration / spawn_phash_validation invocation in the nearby block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/send.rs`:
- Around line 1147-1162: The phash ACK validation was lost for non-1:1 group
sends because dm_phash is only set in the 1:1 branch; restore it by assigning
dm_phash = prepared.phash after calling wacore::send::prepare_dm_stanza (and any
equivalent prepare_* call used for group messages) so the waiter registration
receives a Some(phash) for regular group sends too; ensure the subsequent waiter
registration and spawn_phash_validation() call use that dm_phash value
(affecting the code paths around prepare_dm_stanza/ prepared.node and the waiter
registration / spawn_phash_validation invocation in the nearby block).
2 participants
Summary
Reworks DM multi-device encryption to match WA Web's actual architecture, verified against captured JS (
WAWebSendUserMsgJob,WAWebDBDeviceListFanout,WAWebApiDeviceList,WAWebSendMsgCreateFanoutStanza).Based on the work in #523 by @mcaldas — encrypts for all known recipient devices instead of bare JID only, fixing "Waiting for this message" on linked devices.
Changes
get_user_devicesonly on cache miss, errors non-fatal — falls back to bare JID (server fanout).await?failure on either@hosted/@hosted.lidfrom DM fanout (WAWebDBDeviceListFanoutskipsdevice.id === 99 || device.isHosted)participant_list_hashand embed in stanzaphashattr +DeviceSentMessage.phashfield. Server can now signal device-list drift via ack, andspawn_phash_validationinvalidates device cache on mismatch for self-healingget_user_devicesto avoid unnecessary write-heavy usync queries on the hot pathNot implemented
<enc>fast path (WAWebSendMsgCreateFanoutStanzaline 26): WA Web uses a bare<enc>node (no<participants>wrapper) when there's exactly 1 primary device. Ourencrypt_for_devicesalways wraps in<to jid=...>nodes — implementing the fast path would require refactoring the encryption layer. The<participants>form is accepted by the server regardless.isHostedper-device flag: WA Web storesisHostedper device in IDB records. OurDeviceInfoonly hasdevice_id+key_index. We rely onJid::is_hosted()(device 99 +@hostedserver) which covers the known hosted device patterns.WA Web compliance matrix
getDeviceIds→ IndexedDB)get_devices_from_registry)device.id === 99 || device.isHostedJid::is_hosted()(device 99 + @HosteD server)syncDeviceListJob→ resendinvalidate_device_cache→ next send re-fetches<participants>always)Test plan
cargo fmt --allcleancargo clippy --all --testscleanCloses #523
Summary by CodeRabbit
Bug Fixes
Reliability & Performance
Compatibility