audit (round 2): more WA Web protocol compliance fixes - #624
Merged
Conversation
WA Web only emits `<ack class="message">` for newsletter deliveries (WAWebOutMessageDeliverCommonAckMixin). Regular DMs / groups / status broadcasts rely on the <receipt> stanza alone — sending both the ack and the delivery receipt is redundant and wire-divergent. Narrow should_ack: message → only ack when from is a newsletter JID receipt | notification | call → unchanged Existing test extended to lock in the new matrix (DM/group must not ack, newsletter must).
WAWebSendProfilePictureJob builds the set vs remove IQ as:
set: <iq xmlns="w:profile:picture" type="set"><picture type="image">{bytes}</picture></iq>
remove: <iq xmlns="w:profile:picture" type="set"/> (no <picture> child)
Rust was emitting an empty <picture type="image"/> on remove, which is
spec-divergent and can yield 400/406 from strict server-side parsers.
Existing remove-own test was asserting the buggy shape; replaced by two
tests pinning the WA-Web-correct empty-IQ shape for both own and group.
WAWebReceiptAck: `participant: r && r !== e ? DEVICE_JID(r) : DROP_ATTR` — participant is dropped when it duplicates the destination (the flipped `from`). Rust was always echoing the participant attribute when present in the source stanza, even when redundant. Apply the filter in both encode_ack_bytes (hot path) and build_ack_node (test helper). Two new tests pin the drop / keep matrix.
WA Web's OutGroupsCreateRequest documents `<ephemeral expiration trigger>` where `trigger` (0..20) carries the disappearing-mode source (per-chat setting vs device-wide default). The existing builder only emitted `expiration`, so callers couldn't propagate that signal. Add SetGroupEphemeralIq::trigger field and an enable_with_trigger constructor; existing enable / disable stay backwards-compatible (default trigger = None, attribute omitted on the wire).
WAWebInGroupsParticipantRequestCodeCanBeSentMixin: when an add fails
with error=403 (privacy-blocked), the server attaches a V4 invite
token: <add_request code="..." expiration="N"/>. The app can fall
back to sending a GroupInviteMessage with those fields instead of
silently failing the add.
ParticipantChangeResponse was dropping the child, so the recovery path
was unreachable. Replaced the derive with a hand-rolled ProtocolNode
impl that captures it into an AddRequestInfo { code, expiration }.
Two new tests pin the 403-with-token / non-403 matrix.
WAWebHandleGroupNotificationConst lists 45 GROUP_NOTIFICATION_TAGs; GroupNotificationAction covered 24, falling through the rest to the Unknown catch-all so consumers couldn't react to community-admin churn or community-policy toggles. Add: - LinkedGroupPromote / LinkedGroupDemote (community parent admin changes) - Suspended / Unsuspended (Meta-moderation toggles) - AutoAddDisabled, IsCapiHostedGroup, GroupSafetyCheck, LimitSharingEnabled - AllowAdminReports / NotAllowAdminReports / Reports - AllowNonAdminSubGroupCreation / NotAllowNonAdminSubGroupCreation - CreatedSubGroupSuggestion / RevokedSubGroupSuggestions (raw node) - ChangeNumber (carries new <participant>) Three new tests cover the toggle table, the participant-carrying variants, and the change_number flow.
OutGroupsCreateRequest accepts ~17 child types on <create>. We only
emitted a subset. Add the two most useful that the audit flagged:
- linked_parent: lets a subgroup be created atomically as a child of
an existing community, removing an extra round-trip
- description: inline <description id="<token>"><body>{text}</body></description>
so callers don't have to follow up with a separate set-description IQ
Both are optional; existing callers are unaffected.
…s to attach WAWebMessageSendUtils upgrades a bare `conversation` body to `extended_text_message` whenever a ContextInfo field needs to be populated, so the ephemeral timer can ride on it. The Rust helper returned false instead, silently dropping the timer. set_ephemeral_expiration now moves the conversation text into a new extended_text_message with the timer attached, matching WA Web's wire output. Two new tests pin the promote + empty-message paths.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughTightens client ACK rules and encoding, adds a synchronous flag-only shutdown, emits no ChangesProtocol & IQ Updates
Sequence Diagram (client ACK & shutdown)sequenceDiagram
participant Client
participant IncomingStanza
participant AckBuilder
Client->>IncomingStanza: should_ack(stanza)
IncomingStanza-->>Client: eligibility (receipt/notification/call OR newsletter/status@broadcast)
Client->>AckBuilder: encode_ack_bytes(stanza, participant?)
AckBuilder-->>Client: ack bytes (omit redundant participant when participant == from)
Client->>Client: signal_shutdown_sync() sets flags and notifiers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
You want this to work right — review the ACK rules, group IQ changes, and MessageExt promotions with priority. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wacore/src/iq/contacts.rs (1)
196-201:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUpdate the Remove wire format documentation.
Look, we need our documentation to match reality. The code now emits an empty IQ with NO
<picture>child when removing (content is None), but this doc example still shows<picture type="image"/>. That's unacceptable—developers will read this and implement the wrong thing.📝 Fix the wire format example
/// ## Wire Format (Remove) /// ```xml /// <iq xmlns="w:profile:picture" type="set" to="s.whatsapp.net" id="..."> -/// <picture type="image"/> /// </iq> /// ```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/iq/contacts.rs` around lines 196 - 201, Update the "Remove wire format" docblock to match runtime behavior: the IQ emitted when removing a profile picture contains no <picture> child (content is None), so remove the `<picture type="image"/>` line from the example in the doc comment in contacts.rs (the doc block starting "## Wire Format (Remove)") and show an empty <iq ...>...</iq> example instead to reflect that the IQ has no child elements.wacore/src/proto_helpers.rs (1)
156-159:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the trait doc: behavior changed for bare
conversation.The docs still state this returns
falsefor bareconversation, but Line 361 now promotes toextended_text_messageand returnstrue.Suggested fix
- /// Sets `context_info.expiration` on the first message type found. - /// Creates a default `context_info` if needed. Returns `false` for - /// bare `conversation` messages (use `ExtendedTextMessage` instead). + /// Sets `context_info.expiration` on the first message type found. + /// Creates a default `context_info` if needed. Bare `conversation` + /// payloads are promoted to `extended_text_message` so expiration can attach. + /// Returns `false` only when no compatible payload exists. fn set_ephemeral_expiration(&mut self, expiration: u32) -> bool;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/proto_helpers.rs` around lines 156 - 159, Update the trait doc for set_ephemeral_expiration to reflect the changed behavior: it now promotes bare conversation messages to extended_text_message (creating a default context_info if needed) and returns true instead of false; modify the sentence that currently says "Returns `false` for bare `conversation` messages (use `ExtendedTextMessage` instead)" to describe the promotion and the true return value, referencing the set_ephemeral_expiration method name so reviewers can find the implementation.wacore/src/iq/groups.rs (1)
421-445:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent conflicting create payloads (
<parent/>+<linked_parent/>).This path can emit both tags when
is_parentandlinked_parentare both set, which creates an ambiguous wire shape for group creation.Suggested fix
pub fn build_create_group_node(options: &GroupCreateOptions) -> Node { let mut children = Vec::new(); + debug_assert!( + !(options.is_parent && options.linked_parent.is_some()), + "`linked_parent` is only valid for subgroup creation (is_parent = false)" + ); @@ - if let Some(parent_jid) = &options.linked_parent { + if !options.is_parent + && let Some(parent_jid) = &options.linked_parent + { children.push( NodeBuilder::new("linked_parent") .attr("jid", parent_jid) .build(), ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/iq/groups.rs` around lines 421 - 445, The code can emit both a <parent/> and a <linked_parent/> tag when options.is_parent is true and options.linked_parent is Some, causing conflicting payloads; update the logic around NodeBuilder usage so these are mutually exclusive (e.g., only build the parent block when options.is_parent && options.linked_parent.is_none(), or if both are set choose one canonical behavior such as preferring linked_parent and skipping the parent block); adjust the checks around options.is_parent, options.linked_parent and the NodeBuilder calls (references: options.is_parent, options.linked_parent, NodeBuilder::new("parent"), NodeBuilder::new("linked_parent"), children.push(...)) so the resulting children never include both tags simultaneously.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/iq/groups.rs`:
- Around line 1773-1779: The enable_with_trigger constructor currently accepts
any u32 for trigger; add a validation that trigger is within WA's allowed range
(0..=20) and reject out-of-range values instead of allowing them through.
Concretely, update enable_with_trigger to check if trigger <= 20 and return a
Result (e.g., Result<Self, Error> or Option<Self>) or otherwise surface an
explicit error when invalid; also add the same range check in the encoder path
that serializes the ephemeral IQ (the encoder/serializer that reads this struct)
so invalid trigger values cannot be encoded on-wire; keep the check in both the
constructor (enable_with_trigger) and the encoder to ensure safety.
In `@wacore/src/stanza/groups.rs`:
- Around line 975-1005: The test test_parse_suspended_toggle_variants currently
asserts by comparing Debug strings; replace that brittle check with type-safe
pattern matches: after building notif via GroupNotification::try_from_node_ref,
use matches! (or a match) on notif.actions[0] to assert it is the expected enum
variant for each tag (e.g., assert matches!(notif.actions[0], Action::Suspended
| Action::Unsuspended | Action::AutoAddDisabled | Action::IsCapiHostedGroup |
Action::GroupSafetyCheck | Action::LimitSharingEnabled |
Action::AllowAdminReports | Action::NotAllowAdminReports | Action::Reports |
Action::AllowNonAdminSubGroupCreation |
Action::NotAllowNonAdminSubGroupCreation) as appropriate per table entry), so
the test fails only when the parser produces a different enum variant; keep
using GroupNotification::try_from_node_ref and notif.actions to locate the value
being matched.
---
Outside diff comments:
In `@wacore/src/iq/contacts.rs`:
- Around line 196-201: Update the "Remove wire format" docblock to match runtime
behavior: the IQ emitted when removing a profile picture contains no <picture>
child (content is None), so remove the `<picture type="image"/>` line from the
example in the doc comment in contacts.rs (the doc block starting "## Wire
Format (Remove)") and show an empty <iq ...>...</iq> example instead to reflect
that the IQ has no child elements.
In `@wacore/src/iq/groups.rs`:
- Around line 421-445: The code can emit both a <parent/> and a <linked_parent/>
tag when options.is_parent is true and options.linked_parent is Some, causing
conflicting payloads; update the logic around NodeBuilder usage so these are
mutually exclusive (e.g., only build the parent block when options.is_parent &&
options.linked_parent.is_none(), or if both are set choose one canonical
behavior such as preferring linked_parent and skipping the parent block); adjust
the checks around options.is_parent, options.linked_parent and the NodeBuilder
calls (references: options.is_parent, options.linked_parent,
NodeBuilder::new("parent"), NodeBuilder::new("linked_parent"),
children.push(...)) so the resulting children never include both tags
simultaneously.
In `@wacore/src/proto_helpers.rs`:
- Around line 156-159: Update the trait doc for set_ephemeral_expiration to
reflect the changed behavior: it now promotes bare conversation messages to
extended_text_message (creating a default context_info if needed) and returns
true instead of false; modify the sentence that currently says "Returns `false`
for bare `conversation` messages (use `ExtendedTextMessage` instead)" to
describe the promotion and the true return value, referencing the
set_ephemeral_expiration method name so reviewers can find the implementation.
🪄 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: 14269a6a-fbfe-46ca-a390-070f1c593e7e
📒 Files selected for processing (5)
src/client.rswacore/src/iq/contacts.rswacore/src/iq/groups.rswacore/src/proto_helpers.rswacore/src/stanza/groups.rs
![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: 5c19276c45
ℹ️ 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".
| }; | ||
| match tag { | ||
| "receipt" | "notification" | "call" => true, | ||
| "message" => from.to_jid().is_some_and(|j| j.is_newsletter()), |
There was a problem hiding this comment.
Narrowing should_ack so <message> only acks newsletters means status messages (from="status@broadcast") no longer get any acknowledgement at all: this branch now returns false for them, and send_delivery_receipt already skips status chats (should_send_delivery_receipt filters STATUS_BROADCAST_USER). In practice that drops the only ack path that previously existed for status stanzas, which can cause repeated deliveries/retries and inconsistent status processing on long-lived sessions.
Useful? React with 👍 / 👎.
B1 status broadcast safety net: should_ack now also returns true for status@broadcast <message> stanzas. send_delivery_receipt skips them, so without the ack the server would have no acknowledgement at all. Pre-PR behavior is preserved until the status-receipt path (B4 territory) lands. B7 LimitSharingEnabled trigger: the variant now carries an Option<u32> trigger captured from the <limit_sharing_enabled trigger=N> attribute (range 0..20 per WASmaxInGroupsGroupInfoMixin). Empty toggle keeps trigger = None. B10 set_context_info parity: extends the same conversation→extended_text promotion to set_context_info, so mentions / quotes also attach when the body is a bare conversation. Existing test updated to assert the new promote behavior; new test pins the false-on-empty-message path. B14 description id format: align with SetGroupDescriptionIq's existing 8-char hex (random u32). Was 16-char hex from 8 random bytes.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wacore/src/proto_helpers.rs (1)
135-138:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the doc comment—it contradicts the new implementation.
The doc says this returns
falsefor bare conversation messages, but the code now promotes conversation toextended_text_messageand returnstrue(lines 366-375). That's a fix we needed to prevent silently dropping ephemeral timers, but the documentation needs to reflect reality.📝 Proposed fix
/// Sets `context_info.expiration` on the first message type found. -/// Creates a default `context_info` if needed. Returns `false` for -/// bare `conversation` messages (use `ExtendedTextMessage` instead). +/// Creates a default `context_info` if needed. Promotes bare `conversation` +/// messages to `ExtendedTextMessage` to carry the expiration. fn set_ephemeral_expiration(&mut self, expiration: u32) -> bool;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/proto_helpers.rs` around lines 135 - 138, Update the doc comment for set_ephemeral_expiration to match the implementation: state that it sets context_info.expiration on the first message type found (creating a default context_info if needed), and that it will promote a bare ConversationMessage into an ExtendedTextMessage to attach the expiration (returning true); only return false if no suitable message type exists to attach the expiration. Reference the function name set_ephemeral_expiration and the promotion behavior (ConversationMessage -> ExtendedTextMessage) in the comment so the docs no longer contradict the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/iq/groups.rs`:
- Around line 438-461: This creates a fresh StdRng via rand::make_rng each time
an inline description is built; extract the 8-hex-id generation into a shared
helper (e.g., generate_description_id) and have both this creation path (in the
block that checks options.description and uses NodeBuilder::new("description"))
and SetGroupDescriptionIq::new call that helper so RNG/seeding strategy is
centralized and consistent; keep the local use rand::RngExt as _ import
placement consistent with the existing pattern.
- Around line 1140-1205: The ParticipantChangeResponse::try_from_node_ref impl
silently swallows two errors: replace attrs.jid("jid") with
attrs.optional_jid("jid").ok_or_else(|| anyhow::anyhow!("participant missing
required 'jid' attribute"))? so missing JIDs propagate an error like the
GroupParticipantResponse pattern, and change the add_request parsing (the
node.get_optional_child("add_request") closure) to fail fast instead of using
.and_then(): when the child exists, explicitly require the "code" attribute and
parse "expiration" to u64 returning an anyhow::Error on missing/invalid values
(so AddRequestInfo is Some only on successful parse, and malformed add_request
returns Err rather than silently yielding None).
In `@wacore/src/stanza/groups.rs`:
- Around line 499-528: In parse_action, avoid silently truncating large u64
values when building LimitSharingEnabled.trigger: replace the current
optional_u64(...).map(|t| t as u32) with optional_u64(...).and_then(|t|
t.try_into().ok()) so overflow yields None; apply the same try_into().ok()
pattern for similar fields such as Ephemeral and GrowthLocked if you later
update them. Use the symbols LimitSharingEnabled, parse_action, and optional_u64
to locate and change the mapping.
---
Outside diff comments:
In `@wacore/src/proto_helpers.rs`:
- Around line 135-138: Update the doc comment for set_ephemeral_expiration to
match the implementation: state that it sets context_info.expiration on the
first message type found (creating a default context_info if needed), and that
it will promote a bare ConversationMessage into an ExtendedTextMessage to attach
the expiration (returning true); only return false if no suitable message type
exists to attach the expiration. Reference the function name
set_ephemeral_expiration and the promotion behavior (ConversationMessage ->
ExtendedTextMessage) in the comment so the docs no longer contradict the code.
🪄 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: c77e8e1d-1c20-4d65-bf62-cc7be7e4fcfa
📒 Files selected for processing (4)
src/client.rswacore/src/iq/groups.rswacore/src/proto_helpers.rswacore/src/stanza/groups.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60910caead
ℹ️ 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 mut attrs = node.attrs(); | ||
| let jid = attrs.jid("jid"); |
There was a problem hiding this comment.
jid when parsing participant change responses
ParticipantChangeResponse::try_from_node_ref now reads jid via attrs.jid("jid"), which returns a default empty JID when the attribute is missing or unparsable instead of failing. Before this change, the derived parser treated jid as required and returned an error. In malformed or partial <participant> replies, this silently produces a bogus jid and lets the response look valid, which can corrupt downstream participant-update handling keyed by JID.
Useful? React with 👍 / 👎.
Round-3 review follow-ups: * SetGroupEphemeralIq trigger: assert against EPHEMERAL_TRIGGER_MAX (=20) in enable_with_trigger; debug_assert at encode time for defense. Out-of-range values are server-rejected per WASmaxInGroupsGroupInfoMixin's attrIntRange(0,20). * test_parse_suspended_toggle_variants: switch from Debug-string starts_with to type-safe matches! on each variant — parser regressions (landing in Unknown) now fail loudly. * contacts.rs profile-pic doc: remove example now shows an empty <iq/>, matching the build_iq output after the earlier fix. * GroupCreateOptions: <parent> and <linked_parent> are semantically exclusive (community vs subgroup-of-community). When both are set, linked_parent wins and the parent-only children (allow_non_admin_sub_group_creation, create_general_chat) are dropped. * generate_description_id: extracted shared helper used by both GroupCreateIq (inline description) and SetGroupDescriptionIq, so RNG/seeding strategy stays centralized. * ParticipantChangeResponse::try_from_node_ref: hard-fail on missing jid attr (mirrors GroupParticipantResponse) and on malformed <add_request> (missing code, non-numeric expiration). 403 with a dropped V4 invite token is a server bug we want to surface. * parse_action u64→u32: replace truncation with try_into().ok() for Ephemeral.expiration, Ephemeral.trigger, GrowthLocked.expiration, and LimitSharingEnabled.trigger. Out-of-range values become None / 0 instead of silently wrapping. Skipped: doc update for set_ephemeral_expiration — already addressed in the previous round; current text already describes the promotion and the true/false semantics.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d87b531a7d
ℹ️ 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".
Comment on lines
+437
to
+441
| if let Some(parent_jid) = &options.linked_parent { | ||
| children.push( | ||
| NodeBuilder::new("linked_parent") | ||
| .attr("jid", parent_jid) | ||
| .build(), |
There was a problem hiding this comment.
When both linked_parent and is_parent are set, this branch intentionally serializes the request as a subgroup (<linked_parent>, no <parent>), but GroupCreateIq::parse_response still overlays is_parent_group = true from options.is_parent. That leaves the immediate create response misclassified as a community parent (and can mislead group_type()/follow-up logic) until a later metadata fetch corrects it. The overlay logic should ignore is_parent when linked_parent is present so request and parsed response stay consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wacore/src/stanza/groups.rs (3)
296-296:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate this comment - two more variants now call
.to_owned().The comment claims only Create/Link/Unlink call
.to_owned(), butCreatedSubGroupSuggestionandRevokedSubGroupSuggestionsalso storeraw: Nodeand clone their child node at lines 536 and 539. This matters because it documents where we pay the allocation cost.📝 Proposed fix
- /// Most fields are parsed zero-copy. Only `Create`/`Link`/`Unlink` actions - /// call `.to_owned()` on their specific child node (structurally required to store `raw: Node`). + /// Most fields are parsed zero-copy. Only `Create`, `Link`, `Unlink`, + /// `CreatedSubGroupSuggestion`, and `RevokedSubGroupSuggestions` actions + /// call `.to_owned()` on their specific child node (structurally required to store `raw: Node`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/stanza/groups.rs` at line 296, Update the doc comment that currently states only Create/Link/Unlink call `.to_owned()` to also list CreatedSubGroupSuggestion and RevokedSubGroupSuggestions as variants that clone their child node into `raw: Node`; locate the comment near the variant enum/documentation in groups.rs and add those two variant names (or rephrase to "Create, Link, Unlink, CreatedSubGroupSuggestion, and RevokedSubGroupSuggestions") so it accurately reflects the allocation points (see the constructors storing `raw: Node` for CreatedSubGroupSuggestion and RevokedSubGroupSuggestions around the code that clones the child node).
1117-1191:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis regression guard needs to cover the 16 new variants you just added.
The test comment says it guards against PascalCase discriminator leaks (line 1112-1113). You added LinkedGroupPromote, LinkedGroupDemote, Suspended, Unsuspended, AutoAddDisabled, IsCapiHostedGroup, GroupSafetyCheck, LimitSharingEnabled, AllowAdminReports, NotAllowAdminReports, Reports, AllowNonAdminSubGroupCreation, NotAllowNonAdminSubGroupCreation, CreatedSubGroupSuggestion, RevokedSubGroupSuggestions, and ChangeNumber — none of them are in the samples array. If the WireEnum derive breaks for these new variants, this test won't catch it. That's exactly what regression guards are supposed to prevent.
🛡️ Proposed fix - add samples for all new variants
GroupNotificationAction::Unlink { unlink_type: "x".into(), unlink_reason: None, raw: dummy_node, }, + GroupNotificationAction::LinkedGroupPromote { + participants: vec![], + }, + GroupNotificationAction::LinkedGroupDemote { + participants: vec![], + }, + GroupNotificationAction::Suspended, + GroupNotificationAction::Unsuspended, + GroupNotificationAction::AutoAddDisabled, + GroupNotificationAction::IsCapiHostedGroup, + GroupNotificationAction::GroupSafetyCheck, + GroupNotificationAction::LimitSharingEnabled { trigger: None }, + GroupNotificationAction::AllowAdminReports, + GroupNotificationAction::NotAllowAdminReports, + GroupNotificationAction::Reports, + GroupNotificationAction::AllowNonAdminSubGroupCreation, + GroupNotificationAction::NotAllowNonAdminSubGroupCreation, + GroupNotificationAction::CreatedSubGroupSuggestion { + raw: dummy_node.clone(), + }, + GroupNotificationAction::RevokedSubGroupSuggestions { + raw: dummy_node.clone(), + }, + GroupNotificationAction::ChangeNumber { + participants: vec![], + }, GroupNotificationAction::Unknown { tag: "future_tag".into(), },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/stanza/groups.rs` around lines 1117 - 1191, The samples Vec<GroupNotificationAction> used as the regression guard is missing the 16 newly added enum variants; update the samples definition (the samples variable in groups.rs) to include instances for LinkedGroupPromote, LinkedGroupDemote, Suspended, Unsuspended, AutoAddDisabled, IsCapiHostedGroup, GroupSafetyCheck, LimitSharingEnabled, AllowAdminReports, NotAllowAdminReports, Reports, AllowNonAdminSubGroupCreation, NotAllowNonAdminSubGroupCreation, CreatedSubGroupSuggestion, RevokedSubGroupSuggestions, and ChangeNumber so the test exercises their WireEnum discriminators (create minimal valid variants similar to the existing samples: use placeholder strings/zeros/empty vecs/None where fields exist).
331-331:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate this comment too - it's duplicating the stale claim.
Same issue: the comment says only Create/Link/Unlink call
.to_owned(), but you've added two more variants that do this.📝 Proposed fix
-/// Only `Create`/`Link`/`Unlink` call `.to_owned()` because those variants store `raw: Node`. +/// Only `Create`, `Link`, `Unlink`, `CreatedSubGroupSuggestion`, and +/// `RevokedSubGroupSuggestions` call `.to_owned()` because those variants store `raw: Node`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/stanza/groups.rs` at line 331, Update the stale comment that currently claims "Only `Create`/`Link`/`Unlink` call `.to_owned()`" to enumerate all enum variants that actually call `.to_owned()` (including the newer variants you added), and state that those variants store `raw: Node`; in short, replace the old phrase with a correct list: `Create`, `Link`, `Unlink` and the two newly added variants so the comment accurately reflects which variants store `raw: Node` and call `.to_owned()`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/iq/groups.rs`:
- Around line 433-443: The response parsing currently overrides the on-wire
`linked_parent` precedence by unconditionally setting is_parent_group when
options.is_parent is true; update GroupCreateIq::parse_response() so it first
checks for a `linked_parent` element in the response (or the original
options.linked_parent) and only sets is_parent_group when no linked_parent is
present—i.e., if response.contains("linked_parent") or
options.linked_parent.is_some() then ensure is_parent_group is not set,
otherwise honor options.is_parent; adjust the logic that sets is_parent_group
accordingly.
---
Outside diff comments:
In `@wacore/src/stanza/groups.rs`:
- Line 296: Update the doc comment that currently states only Create/Link/Unlink
call `.to_owned()` to also list CreatedSubGroupSuggestion and
RevokedSubGroupSuggestions as variants that clone their child node into `raw:
Node`; locate the comment near the variant enum/documentation in groups.rs and
add those two variant names (or rephrase to "Create, Link, Unlink,
CreatedSubGroupSuggestion, and RevokedSubGroupSuggestions") so it accurately
reflects the allocation points (see the constructors storing `raw: Node` for
CreatedSubGroupSuggestion and RevokedSubGroupSuggestions around the code that
clones the child node).
- Around line 1117-1191: The samples Vec<GroupNotificationAction> used as the
regression guard is missing the 16 newly added enum variants; update the samples
definition (the samples variable in groups.rs) to include instances for
LinkedGroupPromote, LinkedGroupDemote, Suspended, Unsuspended, AutoAddDisabled,
IsCapiHostedGroup, GroupSafetyCheck, LimitSharingEnabled, AllowAdminReports,
NotAllowAdminReports, Reports, AllowNonAdminSubGroupCreation,
NotAllowNonAdminSubGroupCreation, CreatedSubGroupSuggestion,
RevokedSubGroupSuggestions, and ChangeNumber so the test exercises their
WireEnum discriminators (create minimal valid variants similar to the existing
samples: use placeholder strings/zeros/empty vecs/None where fields exist).
- Line 331: Update the stale comment that currently claims "Only
`Create`/`Link`/`Unlink` call `.to_owned()`" to enumerate all enum variants that
actually call `.to_owned()` (including the newer variants you added), and state
that those variants store `raw: Node`; in short, replace the old phrase with a
correct list: `Create`, `Link`, `Unlink` and the two newly added variants so the
comment accurately reflects which variants store `raw: Node` and call
`.to_owned()`.
🪄 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: 570e9f97-49f7-4dc7-9a18-dc28b128e723
📒 Files selected for processing (4)
wacore/src/iq/contacts.rswacore/src/iq/groups.rswacore/src/proto_helpers.rswacore/src/stanza/groups.rs
…mments * GroupCreateIq::parse_response: a linked subgroup is never a community parent. When the response carries <linked_parent> (or the request specified one), don't promote is_parent_group even with is_parent set. * GroupNotification: update both doc lines that list which variants call .to_owned() (Create/Link/Unlink) to also include CreatedSubGroupSuggestion and RevokedSubGroupSuggestions. * serialize_discriminator_matches_wire_tag: extend the samples Vec with the 16 newly added variants so every WireEnum discriminator is exercised. * Trim verbose docs and inline comments added in earlier rounds; drop em dashes and JS-file/line references per project style.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wacore/src/iq/groups.rs (2)
430-453: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winThe silent drop of
closed/allow_non_admin_sub_group_creation/create_general_chatwhen bothlinked_parentandis_parentare set worries me.The test at lines 3887-3921 locks this in as intended behavior, and the doc on
linked_parentsays "mutually exclusive withis_parent" — fine. But theGroupCreateOptionsbuilder accepts both at the same time with zero feedback. If someone wires upis_parent=true, closed=true, linked_parent=Some(...)by mistake, we ship a subgroup and pretend the community flags never existed. Consider either adebug_assert!(options.linked_parent.is_none() || !options.is_parent)inbuild_create_group_node, or refactoringGroupCreateOptionsinto a sum type so the two modes can't co-exist. At minimum, atracing::warn!so this misuse leaves a trail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/iq/groups.rs` around lines 430 - 453, The code silently drops parent-related flags when both options.linked_parent and options.is_parent are set; update build_create_group_node to detect this misuse by adding a debug_assert!(options.linked_parent.is_none() || !options.is_parent) and also emit a tracing::warn! (including values of options.linked_parent, options.closed, options.allow_non_admin_sub_group_creation, options.create_general_chat) when both are present so the misuse is logged; alternatively consider converting GroupCreateOptions into a sum type (enum) that prevents linked_parent and is_parent from coexisting, but at minimum add the assert+warn in build_create_group_node to surface the issue.
1762-1819:⚠️ Potential issue | 🟠 Major | ⚡ Quick winI want this trigger validation tightened up — release builds shouldn't be able to send junk.
enable_with_triggerpanics withassert!whentrigger > 20. Good. Buttrigger: Option<u32>ispub, so any caller can doSetGroupEphemeralIq { group_jid, expiration: Some(e), trigger: Some(99) }and walk right past the constructor. Then at line 1812 thedebug_assert!is a no-op in release, and the bogustrigger="99"lands on the wire and the server rejects (or worse, accepts the wrong disappearing-mode source).Two options that close the gap:
🔒 Option A — make the field private, force the constructor
- /// `trigger` attr on `<ephemeral>` (0..=[`EPHEMERAL_TRIGGER_MAX`]); - /// identifies the disappearing-mode source. `None` omits the attr. - pub trigger: Option<u32>, + /// `trigger` attr on `<ephemeral>` (0..=[`EPHEMERAL_TRIGGER_MAX`]); + /// identifies the disappearing-mode source. `None` omits the attr. + /// Only mutable via `enable_with_trigger` to enforce the range. + trigger: Option<u32>,🔒 Option B — clamp/skip in the encoder so bad values never wire
- if let Some(trigger) = self.trigger { - debug_assert!( - trigger <= EPHEMERAL_TRIGGER_MAX, - "ephemeral trigger out of range (0..={EPHEMERAL_TRIGGER_MAX}): {trigger}" - ); - b = b.attr("trigger", trigger); + if let Some(trigger) = self.trigger + && trigger <= EPHEMERAL_TRIGGER_MAX + { + b = b.attr("trigger", trigger); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/iq/groups.rs` around lines 1762 - 1819, The trigger validation must be enforced at encode time so bad values can't be sent; in SetGroupEphemeralIq::build_iq (where NodeBuilder is used) replace the debug_assert! with a runtime check on self.trigger against EPHEMERAL_TRIGGER_MAX and only add the "trigger" attr when it is within range (e.g., if let Some(trigger) = self.trigger { if trigger <= EPHEMERAL_TRIGGER_MAX { b = b.attr("trigger", trigger); } } ), thereby skipping any out-of-range trigger instead of emitting it; alternatively make the trigger field private and force construction via enable_with_trigger if you prefer the constructor-only approach.wacore/src/proto_helpers.rs (1)
5-10: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMove fast, but don't leave dead docs around — the
set_context_info_on_message!macro is gone.This doc comment still advertises a
set_context_info_on_message!callback macro that was deleted in this PR (it's been inlined astry_attach!insideset_context_info). The comment now lies to the next person who reads it. Trim it to reflect reality.♻️ Suggested wording
-/// Invokes a callback macro with the list of all message types that have `context_info`. -/// -/// This macro ensures both `for_each_context_info_message!` and `set_context_info_on_message!` -/// use the same list of message types, making it easy to add new types in one place. -/// -/// When WhatsApp adds new message types with context_info, add them here. +/// Invokes a callback macro with the list of all message types that carry `context_info`. +/// +/// Single source of truth used by `for_each_context_info_message!` and the inline +/// `try_attach!` / `try_set!` / `check!` macros in `MessageExt`. Add new variants here +/// when WhatsApp ships new message types with `context_info`.As per coding guidelines: "Keep code comments concise; explain why, not what".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/proto_helpers.rs` around lines 5 - 10, The doc comment wrongly references the removed macro set_context_info_on_message! — update the comment above the for_each_context_info_message! macro to remove that reference and state the current truth: say that this list is used by for_each_context_info_message! and by set_context_info (which inlines try_attach!), and that new WhatsApp message types with context_info should be added here; keep the comment short and factual about why the list exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/iq/groups.rs`:
- Around line 273-277: The inline description field is currently raw
Option<String> which bypasses the GroupDescription validation used by
SetGroupDescriptionIq; change the field type to Option<GroupDescription> (or
another validated wrapper) so both create-paths share the same contract, and
update the builder to accept a string input but validate/convert into
GroupDescription during build (use GroupDescription::new / TryFrom and return a
build error on failure). Also update the emission/serialization code that
referenced the old description string to use the inner validated
GroupDescription value (e.g., its inner str) when emitting the <create> stanza,
and ensure GROUP_DESCRIPTION_MAX_LENGTH validation is enforced in that
conversion path.
In `@wacore/src/proto_helpers.rs`:
- Around line 944-952: The test
test_set_context_info_returns_false_on_empty_message only checks the boolean
return value; improve it by also asserting that no fields were mutated — call
msg.set_context_info(context) and then assert that msg.conversation.is_none()
and msg.extended_text_message.is_none() (or the equivalent absence checks on
wa::Message) so it matches the stricter contract used by
set_ephemeral_expiration_returns_false_on_empty_message and will catch
regressions that create unwanted fields.
In `@wacore/src/stanza/groups.rs`:
- Around line 522-528: Add a regression test that asserts overflow behavior for
the limit_sharing_enabled.trigger parsing: when parsing
GroupNotificationAction::LimitSharingEnabled (the code path using
node.attrs().optional_u64("trigger").and_then(|t| t.try_into().ok())), include a
case with trigger="4294967296" and assert the field is treated as None (i.e.,
overflow does not truncate to u32); do the same for the other equivalent parser
block mentioned around the 1042-1063 area to lock in the non-truncating
behavior.
---
Outside diff comments:
In `@wacore/src/iq/groups.rs`:
- Around line 430-453: The code silently drops parent-related flags when both
options.linked_parent and options.is_parent are set; update
build_create_group_node to detect this misuse by adding a
debug_assert!(options.linked_parent.is_none() || !options.is_parent) and also
emit a tracing::warn! (including values of options.linked_parent,
options.closed, options.allow_non_admin_sub_group_creation,
options.create_general_chat) when both are present so the misuse is logged;
alternatively consider converting GroupCreateOptions into a sum type (enum) that
prevents linked_parent and is_parent from coexisting, but at minimum add the
assert+warn in build_create_group_node to surface the issue.
- Around line 1762-1819: The trigger validation must be enforced at encode time
so bad values can't be sent; in SetGroupEphemeralIq::build_iq (where NodeBuilder
is used) replace the debug_assert! with a runtime check on self.trigger against
EPHEMERAL_TRIGGER_MAX and only add the "trigger" attr when it is within range
(e.g., if let Some(trigger) = self.trigger { if trigger <= EPHEMERAL_TRIGGER_MAX
{ b = b.attr("trigger", trigger); } } ), thereby skipping any out-of-range
trigger instead of emitting it; alternatively make the trigger field private and
force construction via enable_with_trigger if you prefer the constructor-only
approach.
In `@wacore/src/proto_helpers.rs`:
- Around line 5-10: The doc comment wrongly references the removed macro
set_context_info_on_message! — update the comment above the
for_each_context_info_message! macro to remove that reference and state the
current truth: say that this list is used by for_each_context_info_message! and
by set_context_info (which inlines try_attach!), and that new WhatsApp message
types with context_info should be added here; keep the comment short and factual
about why the list exists.
🪄 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: 64aedba6-0939-4218-ad2b-108b2b0c66b3
📒 Files selected for processing (5)
src/client.rswacore/src/iq/contacts.rswacore/src/iq/groups.rswacore/src/proto_helpers.rswacore/src/stanza/groups.rs
* GroupCreateOptions.description: Option<String> -> Option<GroupDescription> so create-paths share the same length-cap contract as SetGroupDescriptionIq (GROUP_DESCRIPTION_MAX_LENGTH). * build_create_group_node: debug_assert + log::warn when both linked_parent and is_parent are set; existing build-output test is now cfg_attr(debug_assertions, should_panic) so it exercises both modes. * SetGroupEphemeralIq::build_iq: replace debug_assert! with a runtime guard that silently drops an out-of-range trigger instead of emitting it. Constructor still asserts on misuse; this is the defence-in-depth path for direct field assignment. * parse_action: regression tests for >u32::MAX overflow on limit_sharing_enabled.trigger and ephemeral.expiration/trigger (try_into already covers it; tests pin the non-truncating behaviour). * test_set_context_info_returns_false_on_empty_message: also assert msg.conversation and msg.extended_text_message stay None. * with_context_info_fields macro doc: drop stale reference to the removed set_context_info_on_message! macro.
3 tasks
9 tasks
4 tasks
This was referenced Jun 7, 2026
This was referenced Jul 21, 2026
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.
Summary
Round-2 audit against
docs/captured-js/. Eight isolated fixes (one per fix commit) plus four review-followup commits incorporating reviewer feedback. No public API breaks: new optional fields default to existing behavior.Fixes
fix(client): stop double-acking regular messagesWAWebSendDeliveryReceiptJobemits a single<receipt>per incoming message;<ack class="message">is reserved for newsletter deliveries (OutMessageDeliverCommonAckMixin). Rust was emitting both.should_acknow returnstruefor the"message"tag only whenfromis a newsletter orstatus@broadcastJID. The status branch stays acked becausesend_delivery_receiptstill skips status; otherwise the server would get no acknowledgement at all.fix(profile): omit <picture> child from profile-picture remove IQWAWebSendProfilePictureJobbuilds the set IQ with<picture type="image">{bytes}</picture>and the remove IQ with no child at all. Rust was emitting<picture type="image"/>on remove.SetProfilePictureSpec::build_iqnow produces empty content whenimage_dataisNone.fix(client): drop redundant participant attr from ack when equal to fromWAWebReceiptAck:participant: r && r !== e ? DEVICE_JID(r) : DROP_ATTR. Rust always echoedparticipantif present in the source stanza, even when it duplicatedto(the flippedfrom). The filter now lives in bothencode_ack_bytes(hot path) andbuild_ack_node(test helper).feat(group): expose disappearing-mode trigger on SetGroupEphemeralIqOutGroupsCreateRequestdocuments<ephemeral expiration trigger>wheretrigger(0..=20) carries the disappearing-mode source. The existing builder only emittedexpiration. Newenable_with_triggerconstructor sets it; range is asserted at construction (EPHEMERAL_TRIGGER_MAX = 20) anddebug_asserted inbuild_iqfor defense.feat(group): parse <add_request> child on ParticipantChangeResponse 403WAWebInGroupsParticipantRequestCodeCanBeSentMixin: onerror="403"the server attaches a V4 invite token:<add_request code="..." expiration="N"/>. The app can fall back toGroupInviteMessagecarrying those fields when privacy blocks a direct add.ParticipantChangeResponsewas dropping the child silently. Replaced the derive with a hand-rolledProtocolNodeimpl that captures it into anAddRequestInfo { code, expiration }. Missingjidand malformed<add_request>now hard-fail (mirrorsGroupParticipantResponse).feat(groups): add 14 missing w:gp2 notification variantsWAWebHandleGroupNotificationConstlists 45 tags;GroupNotificationActioncovered 24. The rest collapsed intoUnknown, so consumers couldn't react to community-admin churn or community-policy toggles.New variants:
LinkedGroupPromote/LinkedGroupDemoteSuspended/UnsuspendedAutoAddDisabled,IsCapiHostedGroup,GroupSafetyCheckLimitSharingEnabled { trigger: Option<u32> }AllowAdminReports/NotAllowAdminReports/ReportsAllowNonAdminSubGroupCreation/NotAllowNonAdminSubGroupCreationCreatedSubGroupSuggestion { raw }/RevokedSubGroupSuggestions { raw }ChangeNumber { participants }feat(group): support linked_parent + inline description on GroupCreateIqOutGroupsCreateRequestaccepts<linked_parent jid=...>and<description id=... body>...</body></description>children on<create>. We only emitted a subset. Add both as optional fields:linked_parent: subgroup created atomically as a child of a communitydescription: inline description, avoids a follow-up SetGroupDescription IQ<parent>(this group IS a community) and<linked_parent>(this group is a subgroup of X) are mutually exclusive. When both are set,linked_parentwins on both the build path and theparse_responseoverlay — a linked subgroup is never promoted tois_parent_groupeven when the request flag is set.The description-id helper is shared with
SetGroupDescriptionIq, so RNG/seeding stays centralized.fix(send): promote bare conversation to extended_text when timer needs to attachWAWebMessageSendUtilsupgrades a bareconversationbody toextended_text_messagewhenever aContextInfofield needs to attach (mention, quote, expiration). The Rustset_ephemeral_expirationandset_context_inforeturnedfalse, silently dropping the data. Both now promote the message and returntrueafter promotion.Review-followup commits
60910cae: status broadcast ack safety net,LimitSharingEnabled.triggerpayload,set_context_infopromotion parity, description-id format alignment withSetGroupDescriptionIq.d87b531a: trigger range 0..=20 validation, type-safe matches in toggle test, sharedgenerate_description_idhelper, hard-fail parse for missing jid / malformed add_request,try_intofor u64→u32 (Ephemeral,GrowthLocked,LimitSharingEnabled).b444e352: subgroup parse precedence fix, doc lists updated for.to_owned()callers, samples Vec covers all 16 new variants, comment trim per project style.Skipped
decrypt-fail="hide": needs threadingdecrypt_fail_modethrough severaldispatch_parsed_messagecallsites. Impact is server-side analytics only; deferred to a focused follow-up.Test plan
cargo test --workspace --exclude e2e-tests --exclude bench-integration✅cargo clippy --workspace --tests --exclude e2e-tests -- -D warnings✅