fix(status): drop <meta status_setting> on reactions and revokes - #569
Merged
Merged
Conversation
Status reactions started getting NACK'd with 479 SmaxInvalid after PR #568 actually put stanzas on the wire (previously they aborted client-side). Root cause: `send_status_message` was attaching `<meta status_setting>` to every non-revoke send, including reactions. That meta describes the POSTER's privacy on their own status. WA Web attaches it only inside `WAWebEncryptAndSendStatusMsg`, which runs for status posts. Reactions route through `WAWebSendReactionMsgAction` → `WAWebSendAddonMsgChatAction` → `sendAddonRecord` and never visit the status-post path, so the meta is absent. Baileys doesn't emit `status_setting` at all. Only whatsapp-rust was gratuitously attaching it to reactions, which the server rejects. Extracted `wacore::send::status_carries_privacy_meta(&Message)` as a pure helper (true only for actual posts — not reactions, not revokes) and drove `send_status_message` off it. Six unit tests pin the classification: text post, image post, reaction, enc-reaction, revoke, non-revoke protocol message. Fixes reactions to LID-only contacts not appearing on the poster's side and the resulting silent failure in consumer bots.
📝 WalkthroughWalkthroughThis PR delegates status privacy-meta decision to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 `@wacore/src/send.rs`:
- Around line 1431-1437: The function status_carries_privacy_meta should inspect
the unwrapped/inner message like the other classifiers do instead of only
top-level fields; modify status_carries_privacy_meta to first descend into
wrappers (e.g., ephemeral_message and device_sent_message) to obtain the inner
wa::Message reference and then check that inner message's protocol_message,
reaction_message, and enc_reaction_message fields (symbols: ephemeral_message,
device_sent_message, protocol_message, reaction_message, enc_reaction_message)
so Revokes and Reactions wrapped inside those containers are detected and the
privacy meta is not attached.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0d462190-9dfd-45c1-8308-cbc8d3849477
📒 Files selected for processing (2)
src/send.rswacore/src/send.rs
Benchmark Results59 unchanged benchmark(s)
|
Match the existing classifier pattern in this file (see stanza_type_from_message): call unwrap_message() before inspecting protocol_message / reaction_message / enc_reaction_message, so a reaction or revoke wrapped inside ephemeral_message, device_sent_message, or any view-once wrapper is detected and the privacy meta is still omitted. Without this, a wrapped reaction would slip past and re-trigger the 479 SmaxInvalid NACK the PR is meant to fix. Two regression tests added: reaction inside ephemeral_message and revoke inside device_sent_message.
- assemble_status_participants: check own_lid.user before allocating a non-ad copy, so the final to_non_ad() only runs when the push is actually going to happen. Previously the copy was built up-front and discarded whenever own was already in the resolved list. - resolve_skdm_targets: inspect server/user on the borrowed &Jid before deciding whether to swap LID→PN, instead of allocating `base = jid.to_non_ad()` and then throwing it away on the PN branch. Saves one Jid (user CompactString + fields) per participant that has a phone mapping — typically own + any resolved-from-PN recipient. Pure refactors. All 29 workspace test suites remain green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
After PR #568 fixed the client-side abort on LID-only recipients, status reactions actually reached the server — and started coming back as NACK with
error=\"479\"(SmaxInvalid). Reactions never appeared on the poster's side.Observed wire exchange:
Cause
send_status_messageattached<meta status_setting>to every non-revoke send. That semantic is wrong:status_settingdescribes the poster's privacy on their own status — meaningless when reacting to someone else's content. WA Web handles the split architecturally:WAWebEncryptAndSendStatusMsgattaches the meta, readingWAWebUserPrefsStatus.getStatusList().setting.WAWebSendReactionMsgAction→WAWebSendAddonMsgChatAction→sendAddonRecord. Never visits the status-post path, never attaches the meta.Upstream Baileys doesn't emit
status_settingon any stanza (grepstatus_settinginWhiskeySockets/Baileys/src/Socket/messages-send.tsreturns zero).whatsapp-rust was the lone outlier, gratuitously attaching the meta to every status-shaped send regardless of whether it was an actual post or a reaction.
Fix
New pure helper in
wacore::send:Returns
trueonly for actual status posts — not reactions (reaction_messageorenc_reaction_message), not revokes (protocol_message.type == Revoke). Descendsephemeral_message/device_sent_message/ view-once wrappers viaunwrap_message(same patternstanza_type_from_messageuses) so a reaction nested inside a wrapper is still detected.send_status_messagecalls this before buildingextra_stanza_nodes. Reactions and revokes ship without the meta; posts keep it.Tests
Eight unit tests in
wacore::send::tests::status_carries_privacy_meta:true_for_text_post—ExtendedTextMessagetrue_for_image_post—ImageMessagefalse_for_reaction—ReactionMessage(primary regression guard for 479)false_for_enc_reaction—EncReactionMessagefalse_for_revoke—ProtocolMessage { type: Revoke }true_for_non_revoke_protocol_message— e.g.EphemeralSettingfalse_for_reaction_inside_ephemeral_wrapper— wrapped-reaction regression guardfalse_for_revoke_inside_device_sent_wrapper— wrapped-revoke regression guardPerf polish
Two small Jid-allocation wins landed alongside the fix:
assemble_status_participants: compare againstown_lid.userbefore callingto_non_ad(), so the non-ad copy only runs when the push is actually needed. Saves one Jid clone per call when own is already in the resolved list.resolve_skdm_targets: inspectis_lid()/useron the borrowed&Jidbefore picking PN vs LID, instead of pre-allocatingbase = jid.to_non_ad()and then discarding it on the PN branch. Saves one Jid clone per participant that has a phone mapping (typically own + any PN-resolved recipient).Pure refactors, no behavior change.
Verification
cargo fmt --allcargo clippy --all --tests --exclude e2e-tests— cleancargo test --workspace --exclude e2e-tests --exclude bench-integration— 29 suites greenScope note
The proposal that surfaced this bug also claimed upstream Baileys emits reactions as
type=\"text\"rather than\"reaction\". I verified againstWhiskeySockets/Baileys/master/src/Socket/messages-send.ts:1050-1051: Baileys does emit'reaction'for reaction messages, same as whatsapp-rust and WA Web. Not a divergence; not addressed here.