-
-
Notifications
You must be signed in to change notification settings - Fork 126
fix(receive): skip PDO placeholder-resend for view-once, ack instead #934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5218,8 +5218,9 @@ async fn test_unavailable_with_enc_skips_unavailable_shortcut() { | |
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This test only proves an ack is eventually sent; it does not verify that PDO is skipped. Add a negative assertion over captured outbound frames so a PDO-then-ack regression fails. Prompt for AI agents |
||
| /// Untrusted companions (web-class `PlatformType`) get the bare stub — | ||
| /// `<unavailable>` without `<enc>`. That path must still emit a | ||
| /// `ViewOnce` `UndecryptableMessage` so consumers surface the failure | ||
| /// while the phone relays via PDO. | ||
| /// `ViewOnce` `UndecryptableMessage` so consumers surface the failure. | ||
| /// (The PDO resend is deliberately skipped for view-once — see | ||
| /// `view_once_stub_acks_without_pdo`.) | ||
| #[tokio::test] | ||
| async fn test_unavailable_without_enc_dispatches_view_once_event() { | ||
| let client = create_test_client_for_retry_with_id("unavailable_stub").await; | ||
|
|
@@ -5236,6 +5237,44 @@ async fn test_unavailable_without_enc_dispatches_view_once_event() { | |
| ); | ||
| } | ||
|
|
||
| /// View-once media is never shared with companion/linked devices, so a PDO | ||
| /// placeholder-resend to our own phone comes back empty — and that peer | ||
| /// round-trip surfaces as a spurious "Finished syncing with WhatsApp on | ||
| /// <device>" notification on the user's phone for every view-once received. | ||
| /// The bare view-once stub must therefore ack the stanza directly (so the | ||
| /// server stops redelivering it) instead of gating the ack on a futile PDO | ||
| /// request, while still surfacing the failure to consumers. | ||
| #[tokio::test] | ||
| async fn view_once_stub_acks_without_pdo() { | ||
| let (client, transport) = capturing_client("view_once_ack").await; | ||
| let recorder = Arc::new(EventRecorder::default()); | ||
| client.register_handler(recorder.clone()); | ||
|
|
||
| let node = build_unavailable_stanza("5511777776666@s.whatsapp.net", "VIEW_ONCE_ACK_1", false); | ||
|
jlucaso1 marked this conversation as resolved.
jlucaso1 marked this conversation as resolved.
|
||
| client.clone().handle_incoming_message(node).await; | ||
|
|
||
| // The (unrecoverable) failure still surfaces to consumers. | ||
| assert_eq!( | ||
| recorder.view_once_unavailable_count(), | ||
| 1, | ||
| "bare view-once stub must still dispatch a ViewOnce UndecryptableMessage", | ||
| ); | ||
|
|
||
| // The stanza is acked so the offline queue drains, without gating on a PDO. | ||
| let mut acked = false; | ||
| for _ in 0..80 { | ||
| if find_message_ack(&transport.sent()).is_some() { | ||
| acked = true; | ||
| break; | ||
| } | ||
| tokio::time::sleep(std::time::Duration::from_millis(25)).await; | ||
| } | ||
| assert!( | ||
| acked, | ||
| "view-once stub must emit an <ack class=\"message\"> to drain the offline queue", | ||
| ); | ||
|
Comment on lines
+5263
to
+5275
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert that PDO was not sent. This only proves an ack eventually appears. A regression that sends the PDO placeholder-resend and then acks would still pass, so the test does not protect the core skip-PDO contract. Add a negative assertion over captured frames for the PDO request shape. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| /// The event struct has no "recovery pending" flag, so consumers cannot | ||
| /// wait for a PDO outcome before surfacing failure — adding a field | ||
| /// here forces a conscious UX decision. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: oxidezap/whatsapp-rust
Length of output: 4499
🏁 Script executed:
Repository: oxidezap/whatsapp-rust
Length of output: 4068
Avoid moving
unavailable_typeherematches!takes this enum by value, so the later log anddispatch_undecryptable_eventuse won’t compile. Match on&unavailable_typeinstead, or clone if you really need ownership.🤖 Prompt for AI Agents