refactor: targeted single-device DM retry (matches WA Web) - #549
Conversation
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/7ff9d284-3db6-41ac-a818-e7a19dd5effe Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
jlucaso1
left a comment
jlucaso1
left a comment
There was a problem hiding this comment.
Review: Verified against WhatsApp Web captured JS
WhatsApp Web behavior (WAWebHandleRetryRequest + WAWebUpdateLocalSignalSession)
The WA Web retry flow works differently from ours — worth noting for context:
WA Web approach (targeted resend):
handleRetryRequestextracts the requester:var g = e.isUser() ? r : u(DM:from, group:participant)updateLocalSignalSession(chat, receipt)deletes session for the single requester device (_ = participant || from)ensureE2ESessions([g])— ensures session for only the requester device, not all devices- Resend goes through
WAWebSendRetryMsgJob.sendRetry()— a targeted resend to the specific recipient, NOT through the full DM fanout
Our approach (broad session cleanup + full fanout resend):
delete_dm_retry_sessionsdeletes sessions for all known devices of the requester- Resend goes through
send_message_impl— the full DM fanout path that encrypts for all devices
Why this PR is correct
The architectural difference (WA Web uses targeted resend, we use full fanout resend) is the root cause of the bug. Since send_message_impl encrypts for ALL devices via ensure_e2e_sessions, any device with a stale session gets reused as-is (the has_session check returns true and skips prekey fetch).
The PR solves this correctly: by deleting sessions for all known devices before the full-fanout resend, ensure_e2e_sessions sees has_session = false for every device and fetches fresh prekeys for all of them. This is slightly broader than WA Web's approach but safe because:
- Fresh sessions are always re-established during
ensure_e2e_sessions - No stale PreKeyMessages with consumed prekey IDs can leak through
- Base key collision tracking per-device is safe (keyed by
(address, msg_id))
Code review
-
dm_retry_session_targets— Clean.to_non_ad()strips device info for registry lookup, fallback to just the resolved JID when no registry entry exists. ✓ -
delete_dm_retry_sessions— Base key collision logic correctly applied per-device,flush_signal_cache()once at the end. ✓ -
Tests — Both the happy path (known devices) and fallback (no registry) are covered. ✓
Optional future improvement
A more WA-Web-aligned approach would be to add a targeted send_retry_message that re-encrypts for the specific requester device only (like WAWebSendRetryMsgJob.sendRetry), avoiding the need to delete all device sessions. But that's a bigger refactor and this fix is correct for now.
LGTM — fixes the regression from the per-device DM fanout change.
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/7ff9d284-3db6-41ac-a818-e7a19dd5effe Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/7ff9d284-3db6-41ac-a818-e7a19dd5effe Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/7ff9d284-3db6-41ac-a818-e7a19dd5effe Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
|
The "future improvement" from the review should be implemented in this PR. The current approach (delete all device sessions + full fanout resend) works around the architectural mismatch rather than fixing it. WA Web does a targeted single-device retry, and we should match that. Here's how: WA Web DM Retry Flow (WAWebHandleRetryRequest + WAWebSendRetryMsgJob)Key: WA Web never touches other devices' sessions and never re-fans-out to all devices. The retry is a surgical re-encrypt for the single failing device. Implementation Plan1. Add Model it after
pub async fn prepare_dm_retry_stanza<S, I>(
session_store: &mut S,
identity_store: &mut I,
to_jid: Jid, // user-level JID (chat target)
requester_jid: Jid, // device-specific JID to encrypt for
encryption_jid: Jid, // resolved encryption JID (after LID mapping)
message: &wa::Message,
message_id: String,
retry_count: u8,
account: Option<&wa::AdvSignedDeviceIdentity>,
) -> Result<Node>The stanza structure: <message to="user@lid" id="MSG_ID" type="text">
<participants>
<to jid="user:device@lid.0">
<enc v="2" type="msg|pkmsg" count="N" mediatype="...">CIPHERTEXT</enc>
</to>
</participants>
<device-identity>...</device-identity> <!-- only if prekey message -->
</message>2. Change Revert to single-device session deletion (the original behavior was correct for this), then use the targeted stanza instead of } else {
// DM retry: delete session + base key tracking for ONLY the requesting device
// (matches WAWebUpdateLocalSignalSession which operates on single requester)
self.delete_dm_session(&resolved_jid, &message_id, retry_count).await?;
// Ensure session with only the requesting device
self.ensure_e2e_sessions(std::slice::from_ref(&resolved_jid)).await?;
let device_snapshot = self.persistence_manager.get_device_snapshot().await;
let mut store_adapter = self.signal_adapter().await;
let stanza = wacore::send::prepare_dm_retry_stanza(
&mut store_adapter.session_store,
&mut store_adapter.identity_store,
receipt.source.chat.clone(),
participant_jid,
resolved_jid.clone(),
&original_msg,
message_id,
retry_count,
device_snapshot.account.as_ref(),
).await?;
self.send_node(stanza).await?;
self.flush_signal_cache().await?;
}3. Remove E2E TestThe test should validate the full retry recovery cycle and verify the stanza structure is targeted (not a full fanout). Using the existing #[tokio::test]
async fn test_dm_retry_sends_targeted_stanza() -> anyhow::Result<()> {
let mut client_a = TestClient::connect("e2e_retry_targeted_a").await?;
let mut client_b = TestClient::connect("e2e_retry_targeted_b").await?;
let jid_a = client_a.jid().await;
let jid_b = client_b.jid().await;
// Establish sessions with a full roundtrip
send_and_expect_text(&client_a.client, &mut client_b, &jid_b, "setup-a2b", 30).await?;
send_and_expect_text(&client_b.client, &mut client_a, &jid_a, "setup-b2a", 30).await?;
// Delete B's session with A to force a retry on next message from A
client_b.client.signal().delete_sessions(std::slice::from_ref(&jid_a)).await?;
// Set up stanza waiters BEFORE the send that triggers the retry.
// First waiter catches the initial send, second catches A's retry resend.
let _initial_waiter = client_a.next_sent_message_waiter();
let retry_waiter = client_a.next_sent_message_waiter();
// This send triggers: A encrypts → B can't decrypt → B sends retry → A resends
send_and_expect_text(&client_a.client, &mut client_b, &jid_b, "trigger-retry", 30).await?;
// Capture and verify the retry stanza that A sent
if let Ok(retry_node) = retry_waiter.await {
// Verify targeted: <participants> should have exactly ONE <to> child
// Verify retry indicator: <enc> should have count="..." attribute
}
// Verify bidirectional messaging still works after retry recovery
send_and_expect_text(&client_a.client, &mut client_b, &jid_b, "post-retry-a2b", 15).await?;
send_and_expect_text(&client_b.client, &mut client_a, &jid_a, "post-retry-b2a", 15).await?;
client_a.disconnect().await;
client_b.disconnect().await;
Ok(())
}Key stanza assertions on the retry node:
|
|
@copilot check my comments and start implementing these things. (all the "future improvement" related things) |
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/159b014c-04f1-48f9-90a5-0ffa202f5a54 Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/159b014c-04f1-48f9-90a5-0ffa202f5a54 Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
Implemented the targeted DM retry path in
Started and finished the targeted-retry refactor in |
|
@claude review |
|
Claude finished @jlucaso1's task in 4m 51s —— View job Code Review: Targeted DM Retry Handler
Overall this is a solid refactor that correctly aligns the DM retry path with WA Web's targeted single-device behavior. The stanza structure, session management, and test coverage all look good. A few things to flag: Bug / CorrectnessMissing The old inline code called // delete_dm_retry_session_target (line 546)
self.signal_cache.delete_session(&signal_address).await;
info!("Deleted session for {signal_address} due to retry receipt");
Ok(())The cache deletion happens in-memory, and the flush only occurs at the end of the resend path (line 460). This works because This is low-risk in practice (the retry would just fire again), but worth noting. Consider either:
Group retry: missing
|
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/159b014c-04f1-48f9-90a5-0ffa202f5a54 Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
![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: bd122568bf
ℹ️ 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".
| // DM retry: pairwise resend to the requesting device only. | ||
| self.ensure_e2e_sessions(std::slice::from_ref(&resolved_jid)) |
There was a problem hiding this comment.
This branch now resends a DM retry only to the requesting device, but DM deduplication is still keyed only by (chat, msg_id) (src/retry.rs:131-145). In multi-device chats, once one device’s retry is handled, subsequent retry receipts from the user’s other devices are treated as duplicates and skipped, so those devices never get their targeted resend and remain undecryptable. Deduping needs to include the sender device when DM retries are device-targeted.
Useful? React with 👍 / 👎.
| let stanza = wacore::send::prepare_dm_retry_stanza( | ||
| &mut store_adapter.session_store, | ||
| &mut store_adapter.identity_store, | ||
| receipt.source.chat.clone(), |
There was a problem hiding this comment.
The new DM retry flow encrypts directly with prepare_dm_retry_stanza without taking the per-device session lock, unlike the normal DM send path (src/send.rs:1137-1142). If this retry runs concurrently with another send/decrypt for the same Signal address, both paths can mutate the same ratchet state at once and cause additional undecryptable messages or retry loops. Wrap retry encryption with the same session-locking discipline used by regular DM sends.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR introduces a targeted DM retry mechanism enabling resend of encrypted messages to specific requesting devices after session collisions. It adds session-deletion tracking via Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (retry)
participant SessionMgr as ensure_e2e_sessions
participant StanzaBuilder as prepare_dm_retry_stanza
participant Backend as Backend/Cache
participant Transport as Transport
Client->>Backend: Detect session collision<br/>(base-key mismatch)
Client->>Backend: Peek cached base-key state
Backend-->>Client: Current base-key info
Client->>SessionMgr: ensure_e2e_sessions<br/>(requesting device only)
SessionMgr->>Backend: Establish/refresh session<br/>for target device
Backend-->>SessionMgr: Session ready
SessionMgr-->>Client: Session confirmed
Client->>StanzaBuilder: prepare_dm_retry_stanza<br/>(message, device_jid,<br/>retry_count)
StanzaBuilder->>StanzaBuilder: Encrypt for target device
StanzaBuilder->>StanzaBuilder: Build <participants><to/></to><br/>with enc attributes
StanzaBuilder->>Backend: Check account identity<br/>(for device-identity)
Backend-->>StanzaBuilder: Identity (if available)
StanzaBuilder-->>Client: Retry stanza
Client->>Backend: Delete stale session
Backend->>Backend: Clear signal cache<br/>entry for device
Backend-->>Client: Deletion confirmed
Client->>Transport: Send retry stanza
Transport->>Transport: Flush signal cache
Transport-->>Client: Sent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/retry.rs`:
- Around line 466-549: The delete_dm_retry_session_target function deletes a
session without acquiring the per-sender session lock; before calling
self.signal_cache.delete_session(&signal_address).await (and before the
subsequent info!("Deleted session...")), acquire the per-sender lock via let
lock = self.session_lock_for(signal_address.as_str()).await; then await
lock.lock() and hold the guard while calling delete_session so session deletion
is serialized with concurrent encrypt/decrypt operations (mirrors the pattern
used in process_retry_key_bundle and the registration ID mismatch path).
🪄 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: 3657bf4d-34cf-4563-be22-9626399f95dd
📒 Files selected for processing (3)
src/retry.rstests/e2e/tests/retry_dm_multidevice.rswacore/src/send.rs
| async fn delete_dm_retry_session_target( | ||
| &self, | ||
| device_jid: &Jid, | ||
| message_id: &str, | ||
| retry_count: u8, | ||
| ) -> Result<(), anyhow::Error> { | ||
| // Base key collision detection prevents stale-session retry loops. | ||
| let signal_address = device_jid.to_protocol_address(); | ||
| let device_store = self.persistence_manager.get_device_arc().await; | ||
|
|
||
| // Check for base key collision before deleting the session. | ||
| // Read session through cache for consistent state. | ||
| { | ||
| let device_guard = device_store.read().await; | ||
| let session = self | ||
| .signal_cache | ||
| .peek_session(&signal_address, &*device_guard.backend) | ||
| .await | ||
| .ok() | ||
| .flatten(); | ||
|
|
||
| if let Some(session) = session | ||
| && let Ok(current_base_key) = session.alice_base_key() | ||
| { | ||
| let addr_str = signal_address.as_str(); | ||
| if retry_count == MIN_RETRY_FOR_BASE_KEY_CHECK { | ||
| // Save retry #2's base key so later retries can prove regeneration happened. | ||
| if let Err(e) = device_guard | ||
| .backend | ||
| .save_base_key(addr_str, message_id, current_base_key) | ||
| .await | ||
| { | ||
| warn!("Failed to save base key for {}: {}", signal_address, e); | ||
| } else { | ||
| info!( | ||
| "Saved base key for {} at retry #{} for collision detection", | ||
| signal_address, retry_count | ||
| ); | ||
| } | ||
| } else if retry_count > MIN_RETRY_FOR_BASE_KEY_CHECK { | ||
| // An unchanged base key means we never rebuilt the session. | ||
| match device_guard | ||
| .backend | ||
| .has_same_base_key(addr_str, message_id, current_base_key) | ||
| .await | ||
| { | ||
| Ok(true) => { | ||
| // Collision detected! We haven't regenerated our session. | ||
| warn!( | ||
| "Base key collision detected for {} at retry #{}. \ | ||
| Session hasn't been regenerated. Forcing fresh session.", | ||
| signal_address, retry_count | ||
| ); | ||
| // Clean up base key entry since we're deleting the session | ||
| let _ = device_guard | ||
| .backend | ||
| .delete_base_key(addr_str, message_id) | ||
| .await; | ||
| } | ||
| Ok(false) => { | ||
| // Base key changed, session was regenerated - good! | ||
| info!( | ||
| "Base key changed for {} at retry #{} - session regenerated", | ||
| signal_address, retry_count | ||
| ); | ||
| // Clean up old base key entry | ||
| let _ = device_guard | ||
| .backend | ||
| .delete_base_key(addr_str, message_id) | ||
| .await; | ||
| } | ||
| Err(e) => { | ||
| warn!("Failed to check base key for {}: {}", signal_address, e); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Delete through the cache so resend can't revive a stale in-memory session. | ||
| self.signal_cache.delete_session(&signal_address).await; | ||
| info!("Deleted session for {signal_address} due to retry receipt"); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Missing session lock before session deletion.
The delete_dm_retry_session_target method reads and deletes the session without acquiring the per-sender session lock. This could race with concurrent decrypt operations that also read/modify session state.
The existing code in process_retry_key_bundle (lines 672-673) and the registration ID mismatch path (lines 265-268) both acquire the session lock before session deletion:
let lock = self.session_lock_for(signal_address.as_str()).await;
let _guard = lock.lock().await;
self.signal_cache.delete_session(&signal_address).await;Consider adding the same locking pattern here:
Proposed fix
async fn delete_dm_retry_session_target(
&self,
device_jid: &Jid,
message_id: &str,
retry_count: u8,
) -> Result<(), anyhow::Error> {
// Base key collision detection prevents stale-session retry loops.
let signal_address = device_jid.to_protocol_address();
+ let session_mutex = self.session_lock_for(signal_address.as_str()).await;
+ let _session_guard = session_mutex.lock().await;
+
let device_store = self.persistence_manager.get_device_arc().await;As per coding guidelines: "Use session_locks to serialize per-sender Signal encrypt/decrypt operations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/retry.rs` around lines 466 - 549, The delete_dm_retry_session_target
function deletes a session without acquiring the per-sender session lock; before
calling self.signal_cache.delete_session(&signal_address).await (and before the
subsequent info!("Deleted session...")), acquire the per-sender lock via let
lock = self.session_lock_for(signal_address.as_str()).await; then await
lock.lock() and hold the guard while calling delete_session so session deletion
is serialized with concurrent encrypt/decrypt operations (mirrors the pattern
used in process_retry_key_bundle and the registration ID mismatch path).
Review: Targeted DM Retry (latest commits)The refactor from full-fanout to targeted single-device retry is the right architectural call and matches WA Web. The 1. DM retry dedupe key must include device (Bug -- agrees with Codex P1)Now that DM retries are per-device, the dedupe key at
Fix: include the participant device in the DM dedupe key (same as the group/status path), AND re-add the message to cache after taking for DMs too: // Dedupe key should always include the participant device:
let dedupe_key = {
let mut key = String::with_capacity(64);
receipt.source.chat.push_to(&mut key);
key.push(':');
key.push_str(&message_id);
key.push(':');
participant_jid.push_to(&mut key);
key
};
// After take_recent_message, re-add for DMs too (not just groups):
if is_group_or_status || true { // or just always re-add
self.add_recent_message(&receipt.source.chat, &message_id, &original_msg).await;
}WA Web doesn't have this problem because 2. Missing session lock during DM retry encryption (Bug -- agrees with Codex P2)The normal DM send path acquires per-device session locks before encryption ( Fix: let signal_addr = resolved_jid.to_protocol_address();
let session_mutex = self.session_lock_for(signal_addr.as_str()).await;
let _session_guard = session_mutex.lock().await;
let mut store_adapter = self.signal_adapter().await;
let stanza = wacore::send::prepare_dm_retry_stanza(...).await?;Note: the group retry path ( 3. Stanza structure -- verified correct against WA Web
4. Assessment of other AI reviewsCodex P1 (dedupe key): Valid -- see #1 above. Codex P2 (session lock): Valid -- see #2 above. Claude[bot] review:
SummaryFix #1 and #2, the rest is solid. The targeted retry approach is correct and well-tested. |
|
@copilot check my last comment, we need to solve this issues and make sure not regret anymore. Also check if our changes are DRY and allocation less |
Agent-Logs-Url: https://github.com/jlucaso1/whatsapp-rust/sessions/ccc77905-54cb-4d7f-be26-824b408f1068 Co-authored-by: jlucaso1 <55464917+jlucaso1@users.noreply.github.com>
Fixed in
Handled in |
Review: Latest commit (c9a7585 — dedupe and lock fixes)Both bugs from my previous review are fixed. Quick verification: Dedupe key — Fixed
Session locks — FixedBoth group retry (line ~415) and DM retry (line ~441) paths now acquire Remaining minor observations (non-blocking)
LGTM — ready to drop the WIP tag. |
jlucaso1
changed the title
jlucaso1
left a comment
jlucaso1
left a comment
There was a problem hiding this comment.
Re-review: This is now the proper fix
This version is a significant improvement — it aligns with WA Web's actual retry architecture instead of working around it.
What changed from v1
| Aspect | v1 (broad cleanup) | v2 (targeted resend) |
|---|---|---|
| Session deletion | All known devices | Single requesting device |
| Resend path | send_message_impl (full DM fanout) |
prepare_dm_retry_stanza (single device) |
ensure_e2e_sessions |
All devices (via fanout) | [&resolved_jid] only |
| Dedupe key (DMs) | chat:msg_id |
chat:msg_id:participant |
| Message re-add | Groups/status only | Unconditional |
Verified against WA Web (WAWebHandleRetryRequest)
WA Web's flow:
// 1. requester = single device
var g = e.isUser() ? r : u;
// 2. Session update for single device
yield updateLocalSignalSession(e, t);
// 3. Ensure session for single device
yield ensureE2ESessions([g]);
// 4. Targeted resend
yield sendRetry({to, participant, msgRecord, retryCount});The new PR now mirrors this exactly:
participant_jid = receipt.source.sender.clone()→ single devicedelete_dm_retry_session_target(&resolved_jid, ...)→ single device sessionensure_e2e_sessions(std::slice::from_ref(&resolved_jid))→ single device ✅prepare_dm_retry_stanza(...)→ targeted enc withcountattr ✅
Why this fixes the bug
The stale session for device 33 is never used because the targeted resend only encrypts for the resolved device (device 0). The server routes device 0's fresh enc to the companion. No stale PreKeyMessage with consumed prekeys ever reaches the recipient.
Code quality
prepare_dm_retry_stanza— Clean, mirrors the existingprepare_group_retry_stanzapattern. Includescountattr and conditionaldevice-identityfor pkmsg. ✓build_retry_dedupe_key— Extracted + always includes participant. Fixes a subtle DM bug where two companion devices retrying the same message would get deduplicated. ✓- Unconditional message re-add — Correct. DMs can also have multi-device retries (companion 33, 34, etc.). ✓
- Group retry session lock — Good safety addition, prevents concurrent encrypt races. ✓
- E2E test inspects the actual sent node (participant count, retry count attr) — much stronger than just checking message delivery. ✓
LGTM — this is the correct fix matching WA Web behavior.
WA Web Compliance Audit — PR #549 (Final)Systematic comparison of every behavioral aspect against captured WhatsApp Web JavaScript. Each item cites the specific WA Web module and line. Flow Comparison: WA Web vs PR
Detailed Findings✅ Correctly aligned with WA Web (introduced by this PR)1. Targeted single-device resend
2. Session deletion scoped to requesting device
3. Per-device retry dedup
4. Base key collision detection
5. Session locks before encryption
|
| Check | Status | Notes |
|---|---|---|
| ✅ MAX_RETRY matches | 5 in both |
PostMessageHighRetryCountMetric.js:5 |
| ✅ hasDevice early return | Both return on unknown device | RetryRequest.js:161-195 |
| ✅ Base key threshold | 2 in both |
LocalSignalSession.js:66 |
| ✅ Single-device session deletion | Matches WA Web requester-only scope | LocalSignalSession.js:19 |
| ✅ Single-device resend | Matches WA Web targeted retry | RetryMsgJob.js:54-58 |
✅ count attribute on <enc> |
Set on retries, matches | MsgCreateDeviceStanza.js:153 |
✅ device-identity conditional |
Included on prekey messages | Both stanza builders |
✅ No participant/addressing_mode on DM |
Correctly omitted (group-only) | MsgCreateDeviceStanza.js |
| ✅ Group retry unchanged | Pairwise encrypt to single participant | Not modified by PR |
| ✅ ensureE2ESessions for single device | Called before encryption | RetryRequest.js:200, MsgCreateDeviceStanza.js:20 |
| Pre-existing, more aggressive than WA Web | LocalSignalSession.js:51-79 |
|
decrypt-fail missing on retry stanzas |
Pre-existing omission | MsgCreateDeviceStanza.js:155 |
Verdict
The PR moves the codebase significantly closer to WA Web compliance. The main change (full-fanout → targeted single-device retry) eliminates a major behavioral divergence. All pre-existing gaps (unconditional session deletion, transient message cache, missing decrypt-fail) were present before and are not introduced by this PR.
No new WA Web compliance issues found.
Remaining issue: stale device 33 session on subsequent normal sendsTested with the bartender mock server (updated to However, subsequent normal DMs after the retry still fail because the stale device 33 session was never cleaned up: The issue is that SuggestionThe v1 approach (delete sessions for ALL known devices) would fix both the retry AND subsequent sends. Or alternatively, after the targeted retry resend succeeds, also clean up sessions for sibling devices of the same user. For now, the bartender mock server works around this by always preferring MockPhone relay (device 0 enc → companion) over direct device 33 routing. |
Replace the full DM fanout resend with a WA Web-aligned targeted retry that encrypts only for the requesting device.
What changed
wacore/src/send.rsprepare_dm_retry_stanza: builds a single-device encrypted stanza with<participants><to jid="device">andcount="N"on the<enc>node, matchingWAWebSendMsgCreateDeviceStanza's retry structure.src/retry.rsWAWebUpdateLocalSignalSessionwhich operates on the single requester.prepare_dm_retry_stanza+send_nodeinstead of the fullsend_message_implfanout path.take_recent_messagere-adds unconditionally (not just for groups), so companion devices can still retry the same message.tests/e2e/tests/retry_dm_multidevice.rs<to>target withcount="1"and verifies bidirectional messaging still works.Unit tests
prepare_dm_retry_stanza(attributes,device-identityconditional)dm_retry_deletes_only_requested_session: verifies only the requesting device's session is deletedretry_dedupe_key_per_participant: verifies DM and status dedupe keys differentiate by devicerecent_message_cache_readd_after_take: covers both DM and status re-add scenarios