Skip to content

[WIP] Fix SKDM recipient cache update timing - #389

Closed
jlucaso1 with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-skdm-recipients-cache-update
Closed

[WIP] Fix SKDM recipient cache update timing#389
jlucaso1 with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-skdm-recipients-cache-update

Conversation

Copilot AI commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original prompt

This section details on the original issue you should resolve

<issue_title>fix: SKDM recipient cache should be updated after server ACK, not after stanza build</issue_title>
<issue_description>## Problem

update_skdm_recipients() in src/send.rs marks devices as having received the sender key after prepare_group_stanza() succeeds but before the stanza is actually sent to the server. This means if the network send fails (disconnect, timeout, etc.), devices are incorrectly marked as having the sender key.

How WhatsApp Web does it

Research of captured WA Web JS (docs/captured-js/6hPqTt28IVI.js:53637-53707, hE1cdfp8vOc.js:1629-1657) shows:

  1. WA Web calls markHasSenderKey(groupJid, skDistribList) after the server ACK — only when the server confirms receipt
  2. WA Web marks the planned skDistribList (not per-device encryption success), which is the same as our approach
  3. Per-device companion encryption failures are silently dropped (companion catch returns undefined, filtered by filter(Boolean)), and those devices are still marked as having the key
  4. Only primary device encryption failures abort the entire send

Current behavior (whatsapp-rust)

prepare_group_stanza() → Ok(stanza)
update_skdm_recipients()  ← HERE: marks devices before send
send_node(stanza)         ← network send may fail

Expected behavior (matching WA Web)

prepare_group_stanza() → Ok(stanza)
send_node(stanza) → Ok(())
update_skdm_recipients()  ← Should mark AFTER successful send

Additional finding

The CodeRabbit review also noted that we persist the planned device list rather than the actually encrypted list. However, WA Web does the same thing — it marks the entire skDistribList regardless of per-device encryption outcome. So our behavior here is correct and matches the official client.

Impact

Low in practice — if send_node fails, the connection is likely dropped and the client reconnects with a fresh state. But for correctness, the cache update should happen after server confirmation.

Files

  • src/send.rsupdate_skdm_recipients() calls and their placement relative to send_node()
  • wacore/src/send.rsprepare_group_stanza() return type (currently Result<Node>)</issue_description>

<agent_instructions>make sure everything compiles, run cargo fmt --all and cargo clippy --all --tests in the end too</agent_instructions>

Comments on the Issue (you are @copilot in this section)


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

@jlucaso1

Copy link
Copy Markdown
Collaborator

@copilot implement this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: SKDM recipient cache should be updated after server ACK, not after stanza build

2 participants