Skip to content

feat: unknown device detection and deferred device sync - #480

Merged
jlucaso1 merged 3 commits into
mainfrom
feat/unknown-device-handling
Apr 2, 2026
Merged

feat: unknown device detection and deferred device sync#480
jlucaso1 merged 3 commits into
mainfrom
feat/unknown-device-handling

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add WA Web-compliant isFromKnownDevice() check after group message decryption
  • Send correct retry reason UnknownCompanionNoPrekey (code 11) instead of NoSession (code 1) for unknown devices
  • Batch unknown-device users during offline sync via PendingDeviceSync, flush with a single usync query after offline delivery completes (with 2s delay matching WA Web's OFFLINE_DEVICE_SYNC_DELAY)
  • Invalidate stale device cache before sync to force network fetch of updated device lists
  • Parse offline attribute from message stanzas to distinguish online vs offline dispatch

Context

When receiving group messages (skmsg) from a device not in our device list (e.g., sender added device :10 while we were offline and we only know :8/:9), we were:

  1. Attempting decryption → NoSenderKeyState error
  2. Sending retry receipt with wrong reason code (NoSession = 1)
  3. Never querying the sender's device list to learn about the new device

WA Web handles this differently (WAWebHandleMsgProcessUtils.preProcessMsg):

  • Checks isFromKnownDevice(author) after successful decryption
  • Online: triggers immediate syncDeviceListJob (usync query)
  • Offline: batches into OfflinePendingDeviceCache, flushes via doPendingDeviceSync() after offline delivery ends (2s delay)
  • Sends retry reason UnknownCompanionNoPrekey = 11

This was observed causing 13+ unrecoverable messages per offline sync from a single user who added a new device.

Test plan

  • cargo fmt --all passes
  • cargo clippy --all --tests clean
  • All 1,101 tests pass (0 failures)
  • Deploy to Docker, verify logs show UnknownCompanionNoPrekey instead of NoSession for unknown devices
  • Verify usync query fires after offline sync completion for batched users
  • Verify subsequent messages from previously-unknown devices decrypt successfully

Summary by CodeRabbit

  • New Features

    • Background batching and periodic flush of pending device syncs to recover devices after offline delivery
    • Offline-delivered messages now carry an explicit offline marker for clearer status
  • Improvements

    • Unknown-device handling now triggers targeted re-syncs, sends retry receipts, and avoids processing undecryptable payloads
    • Retry logic updated to include keys earlier for certain unknown-device cases and to improve device cache invalidation during reconnection

WA Web checks `isFromKnownDevice(author)` after decryption and rejects
messages from devices not in the local device list, triggering a usync
query to learn about new devices. We were missing this entirely, causing
unrecoverable messages from users who added new devices while we were
offline.

- Add `UnknownCompanionNoPrekey` retry reason (code 11) matching WA Web
- Add `offline` field to `MessageInfo` for online/offline dispatch
- Add `is_from_known_device()` check in `process_group_enc_batch`
- Add `PendingDeviceSync` to batch offline unknown-device users
- Flush pending device sync after offline delivery completes
- Use correct retry reason in `NoSenderKeyState` error arm
@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5466f90a-a06c-461a-a0a0-9ba596b622f8

📥 Commits

Reviewing files that changed from the base of the PR and between 560be4e and 75b32fd.

📒 Files selected for processing (3)
  • src/message.rs
  • src/pending_device_sync.rs
  • src/retry.rs

📝 Walkthrough

Walkthrough

Adds a pending-device sync system: unknown-device reports during offline sync are queued and batch-resolved after offline completion; message decryption paths detect unknown companion devices and trigger device-list fetches; retry reasons adjusted to include UnknownCompanionNoPrekey for early key inclusion; MessageInfo gains an is_offline flag.

Changes

Cohort / File(s) Summary
Pending device queue & integration
src/pending_device_sync.rs, src/lib.rs, src/client.rs
New PendingDeviceSync (async-mutex HashSet) added, exposed as a crate module, and wired into Client (field init and cleared during connection cleanup).
Flush & user-sync
src/usync.rs, src/handlers/ib.rs
New Client::flush_pending_device_sync() drains pending JIDs, invalidates device cache, and calls get_user_devices(); offline <ib> handler schedules a delayed flush after offline completion.
Message processing & unknown-device handling
src/message.rs, src/pdo.rs
Group decryption paths now check is_from_known_device(); unknown devices trigger handle_unknown_device_sync() which deduplicates and either queues JIDs or spawns immediate device-list fetches; PDO now sets is_offline: false.
Device registry helper
src/client/device_registry.rs
Added pub(crate) async fn is_from_known_device(&self, sender: &wacore_binary::jid::Jid) -> bool to check local device existence via has_device.
Protocol retry behavior
wacore/src/protocol/retry.rs, src/retry.rs
Added RetryReason variants (discriminants 10–13) including UnknownCompanionNoPrekey; should_include_keys() treats UnknownCompanionNoPrekey like NoSession; client retry construction now delegates to that helper.
Message model & parsing
wacore/src/types/message.rs, wacore/src/messages.rs
MessageInfo gains pub is_offline: bool; parser sets is_offline from stanza attributes ("offline" presence); tests updated to reflect explicit is_offline.
Retry logic cleanup
src/retry.rs
Removed local key-inclusion heuristics and now relies on shared wacore::protocol::retry::should_include_keys(...); tests updated accordingly.

Sequence Diagram

sequenceDiagram
    participant Handler as <ib> Offline Handler
    participant Client
    participant MsgProc as Message Processor
    participant Pending as PendingDeviceSync
    participant UserSync as User Sync / get_user_devices
    participant Registry as Device Registry

    Handler->>Client: complete_offline_sync(count)
    Handler->>Client: (spawn) flush_pending_device_sync() after 2s

    rect rgba(100, 150, 200, 0.5)
    MsgProc->>Registry: attempt decrypt / determine sender state
    Registry-->>MsgProc: Success / NoSenderKeyState
    MsgProc->>Client: is_from_known_device(sender)?
    Client-->>MsgProc: false (unknown) / true (known)
    alt unknown
        MsgProc->>Client: handle_unknown_device_sync(info)
        alt info.is_offline
            Client->>Pending: add(sender_jid)
        else
            Client->>UserSync: invalidate + get_user_devices([user])
        end
    else known
        MsgProc->>MsgProc: proceed to handle plaintext
    end
    end

    Note over Pending,UserSync: Later (flush)
    Client->>Pending: take_all()
    Pending-->>Client: Vec<Jid>
    Client->>UserSync: get_user_devices(&pending)
    UserSync-->>Client: device lists
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

A rabbit nibbles queues at night 🐇
Unknown devices brought to light—
Offline whispers softly stored,
Then flushed in batches, trouble cured.
Hops, fetch, and sync — a tidy delight.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding unknown device detection and deferred device sync batching during offline sync.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/unknown-device-handling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jlucaso1

jlucaso1 commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Apr 2, 2026

Copy link
Copy Markdown

Claude finished @jlucaso1's task in 1m 25s —— View job


Code Review: PR #480 — Unknown Device Detection and Deferred Device Sync

  • Read all changed files
  • Analyze protocol correctness
  • Check error handling and concurrency
  • Post review feedback

Overall this is a well-structured PR that correctly mirrors WA Web's isFromKnownDevice / OfflinePendingDeviceCache / doPendingDeviceSync pattern. The scope is tight, the new module is clean, and the retry reason codes are correct. A few issues worth addressing:


Bug: Successfully decrypted messages are silently dropped for unknown devices

src/message.rs:1027-1037 — In the Ok(padded_plaintext) arm, when decryption succeeds but is_from_known_device() returns false, the code sends a retry receipt and continues, discarding the successfully decrypted plaintext. The message is never delivered to the application.

WA Web's preProcessMsg processes the message normally and triggers a device sync in the background — it doesn't discard the content. The current implementation means any message from a device not yet in the local registry (e.g., a race during online delivery, or stale cache) is silently lost and the sender gets asked to re-send.

This is the most important issue in the PR. Consider processing the decrypted plaintext (call handle_decrypted_plaintext) and then triggering the device sync, rather than dropping the message:

Ok(padded_plaintext) => {
    // WA Web: isFromKnownDevice() in preProcessMsg
    if !self.is_from_known_device(&info.source.sender).await {
        warn!("[msg:{}] Unknown device {}, triggering device sync",
            info.id, info.source.sender);
        self.handle_unknown_device_sync(info).await;
        // Don't send retry or skip — process the message normally
    }

    if let Err(e) = self.clone()
        .handle_decrypted_plaintext("skmsg", &padded_plaintext, padding_version, info)
        .await
    {
        log::warn!("Failed processing group plaintext (batch): {e:?}");
    }
}

Fix this →


Nit: offline field type is slightly awkward

wacore/src/types/message.rs:149-150 — The field is Option<bool> but only ever set to Some(true) or None. It's used as info.offline.is_some() everywhere, never checking the bool value. A plain bool (defaulting to false) would be simpler and avoid the Option<bool> three-state ambiguity. If there's a reason for the Option (e.g., distinguishing "attribute absent" from "attribute present with empty value"), a comment explaining why would help.


Minor: get_user_devices return value unused in flush

src/usync.rs:203-209flush_pending_device_sync calls get_user_devices and logs the device count, but the returned devices aren't used for anything. If get_user_devices has the side effect of updating the device registry cache (which it likely does), this is fine — but worth a brief comment confirming that the call is for its side effect, not its return value.


Minor: No dedup for immediate (online) device syncs

src/message.rs:1123-1131 — For online messages, handle_unknown_device_sync spawns an immediate get_user_devices call for every message from an unknown device. If a burst of messages arrives from the same unknown device before the first sync completes, this could fire many redundant usync queries. The offline path handles this correctly via HashSet dedup. Consider similar dedup for online, or at minimum note it as a known limitation.


Test coverage

The new should_include_keys_unknown_companion_retry_1 test in wacore/src/protocol/retry.rs is good. No unit tests were added for PendingDeviceSync (add/take_all/clear) or is_from_known_device, though these are simple enough that integration/e2e tests may suffice. The existing 1,101 tests passing is reassuring.


Summary

Good implementation that correctly aligns with WA Web behavior for unknown device handling. The critical issue is the message drop on successful decryption in the Ok arm — this should be fixed before merge. The other items are minor improvements.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c757bb75e

ℹ️ 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 thread src/message.rs
let client = Arc::clone(self);
self.runtime
.spawn(Box::pin(async move {
if let Err(e) = client.get_user_devices(&[user_jid]).await {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Force-refresh device list for unknown-device recovery

Calling get_user_devices here does not guarantee a server sync when the sender already has a cached/DB device record, because get_user_devices returns registry hits without fetching from network (src/usync.rs, early get_devices_from_registry fast-path). In the exact unknown-device scenario this change targets (known user adds a new companion), the stale record usually exists and this call becomes a no-op, so the new device is never learned and subsequent messages keep failing with retries.

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

🐰 Bencher Report

Branchfeat/unknown-device-handling
Testbedubuntu-latest

🚨 1 Alert

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()Instructions
instructions x 1e3
📈 plot
🚷 threshold
🚨 alert (🔔)
47.13 x 1e3
(+8.54%)Baseline: 43.42 x 1e3
45.59 x 1e3
(103.37%)

Click to view all benchmark results
BenchmarkInstructionsBenchmark Result
instructions
(Result Δ%)
Upper Boundary
instructions
(Limit %)
binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled()📈 view plot
🚷 view threshold
6,197.00
(-4.10%)Baseline: 6,462.13
6,785.23
(91.33%)
binary_benchmark::child_iteration_group::bench_get_children_by_tag📈 view plot
🚷 view threshold
524,304.00
(-25.63%)Baseline: 705,016.70
740,267.54
(70.83%)
binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled()📈 view plot
🚷 view threshold
20,868.00
(-5.28%)Baseline: 22,030.98
23,132.53
(90.21%)
binary_benchmark::marshal_group::bench_marshal_allocating📈 view plot
🚷 view threshold
98,202.00
(-13.94%)Baseline: 114,107.17
119,812.53
(81.96%)
binary_benchmark::marshal_group::bench_marshal_auto_allocating📈 view plot
🚷 view threshold
98,230.00
(-9.34%)Baseline: 108,350.54
113,768.07
(86.34%)
binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating📈 view plot
🚷 view threshold
532,948.00
(-0.10%)Baseline: 533,475.24
560,149.01
(95.14%)
binary_benchmark::marshal_group::bench_marshal_auto_long_string📈 view plot
🚷 view threshold
15,870.00
(-4.33%)Baseline: 16,588.21
17,417.62
(91.11%)
binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating📈 view plot
🚷 view threshold
14,715,201.00
(-7.36%)Baseline: 15,884,476.37
16,678,700.19
(88.23%)
binary_benchmark::marshal_group::bench_marshal_exact_allocating📈 view plot
🚷 view threshold
118,358.00
(-19.18%)Baseline: 146,442.61
153,764.74
(76.97%)
binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating📈 view plot
🚷 view threshold
534,378.00
(-0.10%)Baseline: 534,896.89
561,641.73
(95.15%)
binary_benchmark::marshal_group::bench_marshal_exact_long_string📈 view plot
🚷 view threshold
17,919.00
(-3.86%)Baseline: 18,639.32
19,571.28
(91.56%)
binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating📈 view plot
🚷 view threshold
28,066,347.00
(-20.15%)Baseline: 35,149,238.27
36,906,700.18
(76.05%)
binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating📈 view plot
🚷 view threshold
533,387.00
(-0.10%)Baseline: 533,914.24
560,609.96
(95.14%)
binary_benchmark::marshal_group::bench_marshal_long_string📈 view plot
🚷 view threshold
15,843.00
(-6.88%)Baseline: 17,013.96
17,864.66
(88.68%)
binary_benchmark::marshal_group::bench_marshal_many_children_allocating📈 view plot
🚷 view threshold
14,716,627.00
(-7.36%)Baseline: 15,885,623.44
16,679,904.61
(88.23%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer📈 view plot
🚷 view threshold
107,945.00
(-10.98%)Baseline: 121,262.03
127,325.13
(84.78%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer📈 view plot
🚷 view threshold
98,302.00
(-9.33%)Baseline: 108,422.54
113,843.67
(86.35%)
binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled()📈 view plot
🚷 view threshold
90,974.00
(-5.03%)Baseline: 95,790.64
100,580.17
(90.45%)
binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled()📈 view plot
🚷 view threshold
7,378.00
(-3.23%)Baseline: 7,624.51
8,005.74
(92.16%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled()📈 view plot
🚷 view threshold
91,005.00
(-1.64%)Baseline: 92,524.36
97,150.58
(93.67%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled()📈 view plot
🚷 view threshold
7,401.00
(+0.33%)Baseline: 7,376.59
7,745.42
(95.55%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled()📈 view plot
🚷 view threshold
106,790.00
(-1.40%)Baseline: 108,309.36
113,724.83
(93.90%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled()📈 view plot
🚷 view threshold
8,913.00
(+0.27%)Baseline: 8,888.59
9,333.02
(95.50%)
binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled()📈 view plot
🚷 view threshold
41,989.00
(-6.99%)Baseline: 45,146.99
47,404.34
(88.58%)
binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled()📈 view plot
🚷 view threshold
2,717.00
(-2.39%)Baseline: 2,783.47
2,922.64
(92.96%)
binary_benchmark::unpack_group::bench_unpack_compressed📈 view plot
🚷 view threshold
556,092.00
(+1.25%)Baseline: 549,236.33
576,698.14
(96.43%)
binary_benchmark::unpack_group::bench_unpack_uncompressed📈 view plot
🚷 view threshold
771.00
(-0.24%)Baseline: 772.82
811.46
(95.01%)
libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data()📈 view plot
🚷 view threshold
27,745,189.00
(+0.17%)Baseline: 27,699,318.15
29,084,284.05
(95.40%)
libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message()📈 view plot
🚷 view threshold
5,544,828.00
(-0.05%)Baseline: 5,547,702.19
5,825,087.30
(95.19%)
libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session()📈 view plot
🚷 view threshold
175,061.00
(-1.27%)Baseline: 177,310.69
186,176.22
(94.03%)
libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session()📈 view plot
🚷 view threshold
175,710.00
(-1.33%)Baseline: 178,080.89
186,984.93
(93.97%)
libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users()📈 view plot
🚷 view threshold
17,241,495.00
(-0.24%)Baseline: 17,282,186.35
18,146,295.66
(95.01%)
libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender()📈 view plot
🚷 view threshold
298,417.00
(+0.53%)Baseline: 296,850.78
311,693.32
(95.74%)
libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message()📈 view plot
🚷 view threshold
12,704,004.00
(+0.87%)Baseline: 12,594,876.64
13,224,620.47
(96.06%)
libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution()📈 view plot
🚷 view threshold
719,597.00
(+0.36%)Baseline: 716,998.69
752,848.62
(95.58%)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()📈 view plot
🚷 view threshold
🚨 view alert (🔔)
47,129.00
(+8.54%)Baseline: 43,421.84
45,592.93
(103.37%)

libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction()📈 view plot
🚷 view threshold
15,561,842.00
(+0.00%)Baseline: 15,561,768.64
16,339,857.07
(95.24%)
libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages()📈 view plot
🚷 view threshold
5,378,778.00
(-1.73%)Baseline: 5,473,731.73
5,747,418.32
(93.59%)
libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session()📈 view plot
🚷 view threshold
312,188.00
(-60.19%)Baseline: 784,144.19
823,351.40
(37.92%)
libsignal_benchmark::signature_group::bench_key_generation keygen📈 view plot
🚷 view threshold
2,830,547.00
(+0.17%)Baseline: 2,825,738.56
2,967,025.49
(95.40%)
libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message()📈 view plot
🚷 view threshold
3,452,844.00
(-0.50%)Baseline: 3,470,219.51
3,643,730.48
(94.76%)
libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message()📈 view plot
🚷 view threshold
123,117,188.00
(-1.79%)Baseline: 125,355,877.92
131,623,671.82
(93.54%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message()📈 view plot
🚷 view threshold
12,002.00
(+1.44%)Baseline: 11,831.21
12,422.77
(96.61%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message()📈 view plot
🚷 view threshold
3,930.00
(+2.34%)Baseline: 3,840.20
4,032.21
(97.47%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended()📈 view plot
🚷 view threshold
87,073.00
(-0.75%)Baseline: 87,726.74
92,113.07
(94.53%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple()📈 view plot
🚷 view threshold
79,022.00
(-0.92%)Baseline: 79,757.89
83,745.78
(94.36%)
reporting_token_benchmark::key_derivation_group::bench_key_derivation📈 view plot
🚷 view threshold
50,375.00
(-1.01%)Baseline: 50,888.53
53,432.95
(94.28%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message()📈 view plot
🚷 view threshold
5,956.00
(+2.97%)Baseline: 5,784.26
6,073.47
(98.07%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message()📈 view plot
🚷 view threshold
2,227.00
(+4.23%)Baseline: 2,136.63
2,243.46
(99.27%)
reporting_token_benchmark::token_calculation_group::bench_token_calculation📈 view plot
🚷 view threshold
21,920.00
(-0.00%)Baseline: 21,920.23
23,016.24
(95.24%)
🐰 View full continuous benchmarking report in Bencher

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/handlers/ib.rs`:
- Around line 164-170: The detached task running
client_clone.flush_pending_device_sync() can run before queued senders finish;
change the flow to wait for the offline delivery barrier first (use
wait_for_offline_delivery_end() where the offline marker is handled) and then
run flush_pending_device_sync() so it executes after the barrier—either call
client.flush_pending_device_sync().await directly or spawn it and await the
JoinHandle instead of .detach(); keep using Arc::clone(&client) and
client.runtime.spawn if you must offload, but ensure you await completion so
pending senders aren’t dropped by reconnect cleanup.

In `@src/usync.rs`:
- Around line 203-213: The code drains the pending queue with take_all() before
calling self.get_user_devices(&pending).await, so on Err(e) those pending users
are lost; change the error branch to reinsert the drained items back into the
pending queue (use the same queue that provided take_all(), e.g., call its
push_all/push or equivalent) and preserve their order so a later flush can
retry; specifically, in the match around self.get_user_devices(&pending).await,
on Err(e) call the queue re-enqueue method to requeue the local pending variable
(only on error) before logging the warning.

In `@wacore/src/protocol/retry.rs`:
- Around line 100-102: The early-key-inclusion condition in src/retry.rs must
match the helper logic: update the computation that currently uses reason ==
RetryReason::NoSession to also consider RetryReason::UnknownCompanionNoPrekey so
unknown-companion retries include keys on retry `#1`; specifically modify the
include_keys_early (or equivalent) boolean in the retry path to use the same
combined condition (reason == RetryReason::NoSession || reason ==
RetryReason::UnknownCompanionNoPrekey) and keep the existing retry_count >=
MIN_RETRY_COUNT_FOR_KEYS logic so the function/method that computes whether to
include keys (referenced as include_keys_early, RetryReason::NoSession,
RetryReason::UnknownCompanionNoPrekey, and MIN_RETRY_COUNT_FOR_KEYS) behaves
identically to the helper change.

In `@wacore/src/types/message.rs`:
- Around line 149-150: The field `offline` in the message struct is currently an
Option<bool> but is only ever set to Some(true) or None and callers use
.is_some(); change the field to a two-state representation (prefer `is_offline:
bool` or a small enum like `DeliveryState::{Online, Offline}`) to remove
unreachable `Some(false)` and clarify intent, then update all
constructors/fallbacks that set `offline` to use the new default (false or
Online) and replace all `.offline.is_some()` checks with the new boolean or enum
pattern matches; ensure the struct definition in message.rs and every usage site
(constructors, deserializers, and conditionals) are updated accordingly.
🪄 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: c78f7796-d9b6-42a8-be23-e72be1f2d93e

📥 Commits

Reviewing files that changed from the base of the PR and between d3abc50 and 1c757bb.

📒 Files selected for processing (11)
  • src/client.rs
  • src/client/device_registry.rs
  • src/handlers/ib.rs
  • src/lib.rs
  • src/message.rs
  • src/pdo.rs
  • src/pending_device_sync.rs
  • src/usync.rs
  • wacore/src/messages.rs
  • wacore/src/protocol/retry.rs
  • wacore/src/types/message.rs

Comment thread src/handlers/ib.rs
Comment on lines +164 to +170
let client_clone = Arc::clone(&client);
client
.runtime
.spawn(Box::pin(async move {
client_clone.flush_pending_device_sync().await;
}))
.detach();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Wait for the offline pipeline to drain before flushing.

This is fired off the terminal <offline> marker, but a few lines above the file already treats “offline delivery ended” as a separate barrier via wait_for_offline_delivery_end(). Running flush_pending_device_sync() in a detached task here can miss senders that are still queued for message processing, and reconnect cleanup can clear the pending set before the task runs.

Suggested fix
                 debug!(target: "Client/OfflineSync", "Offline sync completed, received {} items", count);
                 client.complete_offline_sync(count);
-
-                let client_clone = Arc::clone(&client);
-                client
-                    .runtime
-                    .spawn(Box::pin(async move {
-                        client_clone.flush_pending_device_sync().await;
-                    }))
-                    .detach();
+                client.wait_for_offline_delivery_end().await;
+                if !client.is_shutting_down() {
+                    client.flush_pending_device_sync().await;
+                }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/handlers/ib.rs` around lines 164 - 170, The detached task running
client_clone.flush_pending_device_sync() can run before queued senders finish;
change the flow to wait for the offline delivery barrier first (use
wait_for_offline_delivery_end() where the offline marker is handled) and then
run flush_pending_device_sync() so it executes after the barrier—either call
client.flush_pending_device_sync().await directly or spawn it and await the
JoinHandle instead of .detach(); keep using Arc::clone(&client) and
client.runtime.spawn if you must offload, but ensure you await completion so
pending senders aren’t dropped by reconnect cleanup.

Comment thread src/usync.rs
Comment thread wacore/src/protocol/retry.rs
Comment thread wacore/src/types/message.rs Outdated
Comment on lines +149 to +150
/// Stanza `offline` attribute. `Some` = offline delivery, `None` = online.
pub offline: Option<bool>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether `MessageInfo.offline` is actually used as a tri-state anywhere.
rg -n --type rust '\boffline\s*:\s*(Some\(true\)|Some\(false\)|None)' .
rg -n --type rust '\.offline\b' .

Repository: jlucaso1/whatsapp-rust

Length of output: 216


Model offline as a two-state field instead of Option<bool>.

Current usage only creates Some(true) or None and checks presence via .is_some(); the Some(false) state is unreachable dead code. Use bool is_offline or a small enum to clarify the contract and remove offline: None boilerplate from fallback constructors.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wacore/src/types/message.rs` around lines 149 - 150, The field `offline` in
the message struct is currently an Option<bool> but is only ever set to
Some(true) or None and callers use .is_some(); change the field to a two-state
representation (prefer `is_offline: bool` or a small enum like
`DeliveryState::{Online, Offline}`) to remove unreachable `Some(false)` and
clarify intent, then update all constructors/fallbacks that set `offline` to use
the new default (false or Online) and replace all `.offline.is_some()` checks
with the new boolean or enum pattern matches; ensure the struct definition in
message.rs and every usage site (constructors, deserializers, and conditionals)
are updated accordingly.

- P1: invalidate stale device cache before usync query so new devices
  are actually fetched from network (not served from stale registry)
- Fix include_keys_early in src/retry.rs send path to also cover
  UnknownCompanionNoPrekey (was only checking NoSession); deduplicate
  by calling wacore helper instead of inlining logic
- Re-enqueue pending users on flush failure so they survive transient
  network errors
- Add 2s delay before flush matching WA Web OFFLINE_DEVICE_SYNC_DELAY
- Change offline: Option<bool> → is_offline: bool (simpler, no
  unreachable Some(false) state)
@jlucaso1

jlucaso1 commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator Author

@claude review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 560be4e3dc

ℹ️ 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 thread src/handlers/ib.rs
Comment on lines +170 to +174
.runtime
.sleep(std::time::Duration::from_secs(2))
.await;
client_clone.flush_pending_device_sync().await;
}))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Flush pending device sync after offline queue fully settles

This schedules exactly one flush_pending_device_sync() call 2 seconds after the offline end marker, but unknown-device entries are enqueued from handle_unknown_device_sync while offline messages are still being processed. Under large offline backlogs, message handlers can continue adding users after this one-shot flush runs, leaving those users stuck in PendingDeviceSync with no later trigger to flush them, so device lists never refresh and retries can keep failing for those senders.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Apr 2, 2026

Copy link
Copy Markdown

Claude finished @jlucaso1's task in 1m 45s —— View job


Code Review: PR #480 — Unknown Device Detection and Deferred Device Sync

  • Read all changed files
  • Analyze protocol correctness and concurrency
  • Check error handling and edge cases
  • Post review feedback

Well-structured PR that correctly mirrors WA Web's isFromKnownDevice / OfflinePendingDeviceCache / doPendingDeviceSync pattern. The retry reason codes are correct, the should_include_keys helper is properly wired, and the fix-up commit addressed the CodeRabbit findings (re-enqueue on failure, caller-side early key inclusion). A few issues remain:


Bug: Successfully decrypted messages are dropped for unknown devices

src/message.rs:1027-1037 — When decryption succeeds but is_from_known_device() returns false, the code sends a retry receipt and continues, discarding the successfully decrypted plaintext. The message is never delivered to the application.

WA Web's preProcessMsg processes the message normally and triggers a device sync in the background — it doesn't discard the content. This means any message from a device not yet in the local registry is silently lost and the sender gets asked to re-send.

This is the most critical issue. The fix is to process the decrypted plaintext normally and only trigger the device sync as a side effect:

Ok(padded_plaintext) => {
    // WA Web: isFromKnownDevice() in preProcessMsg
    if !self.is_from_known_device(&info.source.sender).await {
        warn!(
            "[msg:{}] Unknown device {}, triggering device sync",
            info.id, info.source.sender
        );
        self.handle_unknown_device_sync(info).await;
        // Don't send retry or skip — process the message normally
    }

    if let Err(e) = self
        .clone()
        .handle_decrypted_plaintext("skmsg", &padded_plaintext, padding_version, info)
        .await
    {
        log::warn!("Failed processing group plaintext (batch): {e:?}");
    }
}

Fix this →


Race: Detached flush can miss late-queued senders

src/handlers/ib.rs:164-175 — The flush is fired from the <offline> end marker with a 2s delay, but message handlers may still be processing offline messages and calling handle_unknown_device_syncpending_device_sync.add() after this fires. Under large offline backlogs, senders added after the one-shot flush have no subsequent trigger and remain stuck in the pending queue forever.

Options:

  1. Wait for the offline delivery barrier (if one exists) before spawning the flush task.
  2. Have the flush loop until the pending set is empty (with a small sleep between iterations).
  3. Document this as a known limitation — the 2s delay is a heuristic that works for typical backlogs.

No dedup for immediate (online) device syncs

src/message.rs:1123-1133 — For online messages, handle_unknown_device_sync spawns an immediate get_user_devices call for every message from an unknown device. If a burst of messages arrives from the same unknown device, this could fire many redundant usync queries before the first completes. The offline path handles this via HashSet dedup. Consider a similar in-flight guard for online syncs, or note it as a known limitation.


Nit: src/retry.rs duplicates logic from wacore

src/retry.rs:726-728 — The early key inclusion logic is duplicated:

let include_keys_early =
    reason == RetryReason::NoSession || reason == RetryReason::UnknownCompanionNoPrekey;
let keys_node = if retry_count >= MIN_RETRY_COUNT_FOR_KEYS || include_keys_early {

This is the same logic as wacore::protocol::retry::should_include_keys(). Consider using the shared helper to avoid divergence:

let keys_node = if wacore::protocol::retry::should_include_keys(retry_count, reason) {

Nit: has_device returns false on backend error

src/client/device_registry.rs:134-140 — If the backend lookup fails (DB error), has_device returns false, which makes is_from_known_device return false, triggering a device sync and (in the success path) dropping the message. This conflates "unknown device" with "DB temporarily unavailable." Worth noting even if it's pre-existing behavior.


Everything else looks solid. The PendingDeviceSync struct is clean, the should_include_keys test coverage is good, error re-enqueue was addressed in the fixup commit, and invalidate_device_cache before get_user_devices correctly forces a network fetch. The is_offline field change from Option<bool> to bool in the fixup is cleaner — good call.


- Dedup online device syncs via pending_device_sync set (prevents
  redundant usync queries when burst of messages from same unknown device)
- Use wacore::protocol::retry::should_include_keys() in send path
  instead of inlining the logic (removes duplication)
- Remove unused MIN_RETRY_COUNT_FOR_KEYS constant from src/retry.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/handlers/ib.rs (1)

164-175: ⚠️ Potential issue | 🟠 Major

Detached delayed flush can lose pending sync work during reconnect/shutdown.

This fire-and-forget task sleeps 2s and flushes later, but pending_device_sync is in-memory and can be cleared by connection cleanup before the detached task runs, causing permanent loss of queued users.

💡 Proposed fix
-                let client_clone = Arc::clone(&client);
-                client
-                    .runtime
-                    .spawn(Box::pin(async move {
-                        // WA Web: OFFLINE_DEVICE_SYNC_DELAY = 2000ms
-                        client_clone
-                            .runtime
-                            .sleep(std::time::Duration::from_secs(2))
-                            .await;
-                        client_clone.flush_pending_device_sync().await;
-                    }))
-                    .detach();
+                client.wait_for_offline_delivery_end().await;
+                if !client.is_shutting_down() {
+                    // WA Web: OFFLINE_DEVICE_SYNC_DELAY = 2000ms
+                    client
+                        .runtime
+                        .sleep(std::time::Duration::from_secs(2))
+                        .await;
+                    if !client.is_shutting_down() {
+                        client.flush_pending_device_sync().await;
+                    }
+                }
#!/bin/bash
# Verify ordering/race-sensitive call sites around offline completion and pending queue lifecycle.
rg -n -C3 'complete_offline_sync|wait_for_offline_delivery_end|flush_pending_device_sync|pending_device_sync\.clear|\.detach\(' src/handlers/ib.rs src/usync.rs src/client.rs
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/handlers/ib.rs` around lines 164 - 175, The detached sleep+flush task
(created via client.runtime.spawn(...).detach()) can run after connection
cleanup and lose in-memory pending_device_sync; replace the fire-and-forget
detach with a cancellation-aware approach: spawn the delayed task with a
JoinHandle tied to the client's lifecycle (do not call .detach()), or capture a
Weak reference to the client (instead of Arc::clone) and early-return if it has
been dropped, and ensure the client's shutdown/reconnect path awaits or aborts
the JoinHandle so flush_pending_device_sync is executed or cancelled
deterministically; locate the spawn/detach call, runtime.sleep,
flush_pending_device_sync, and pending_device_sync to implement this
lifecycle-aware fix.
🤖 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/message.rs`:
- Around line 1117-1133: The code currently derives user_jid via
info.source.sender.to_non_ad(), which preserves `@lid` and causes LID senders to
never refresh companion device lists; change the query JID derivation to prefer
info.source.sender_alt when present and fall back to
info.source.sender.to_non_ad() otherwise, then use that normalized JID for
pending_device_sync.add(), invalidate_device_cache(&query_jid.user), and
get_user_devices(&[query_jid]) so LID→PN normalization matches other call sites
(see functions/methods: pending_device_sync.add, invalidate_device_cache,
get_user_devices, info.source.sender_alt, info.source.sender.to_non_ad()).

In `@src/retry.rs`:
- Around line 726-728: The code duplicates the include-keys logic; replace the
local condition around include_keys_early and the keys_node guard (which uses
reason == RetryReason::NoSession || reason ==
RetryReason::UnknownCompanionNoPrekey and retry_count >=
MIN_RETRY_COUNT_FOR_KEYS) with a call to the shared helper
wacore::protocol::retry::should_include_keys, passing the current reason and
retry_count (remove include_keys_early and the duplicated constants), so the
decision to include keys is delegated to should_include_keys(reason,
retry_count) and both tests and production use the same logic.

---

Duplicate comments:
In `@src/handlers/ib.rs`:
- Around line 164-175: The detached sleep+flush task (created via
client.runtime.spawn(...).detach()) can run after connection cleanup and lose
in-memory pending_device_sync; replace the fire-and-forget detach with a
cancellation-aware approach: spawn the delayed task with a JoinHandle tied to
the client's lifecycle (do not call .detach()), or capture a Weak reference to
the client (instead of Arc::clone) and early-return if it has been dropped, and
ensure the client's shutdown/reconnect path awaits or aborts the JoinHandle so
flush_pending_device_sync is executed or cancelled deterministically; locate the
spawn/detach call, runtime.sleep, flush_pending_device_sync, and
pending_device_sync to implement this lifecycle-aware fix.
🪄 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: 27277a60-fd37-4d65-8455-d4ad1214965f

📥 Commits

Reviewing files that changed from the base of the PR and between 1c757bb and 560be4e.

📒 Files selected for processing (7)
  • src/handlers/ib.rs
  • src/message.rs
  • src/pdo.rs
  • src/retry.rs
  • src/usync.rs
  • wacore/src/messages.rs
  • wacore/src/types/message.rs

Comment thread src/message.rs
Comment on lines +1117 to +1133
let user_jid = info.source.sender.to_non_ad();

if info.is_offline {
log::debug!("Queueing {} for pending device sync (offline)", user_jid);
self.pending_device_sync.add(user_jid).await;
} else {
log::debug!("Triggering immediate device sync for {}", user_jid);
let client = Arc::clone(self);
self.runtime
.spawn(Box::pin(async move {
// Invalidate stale record so get_user_devices hits the network
client.invalidate_device_cache(&user_jid.user).await;
if let Err(e) = client.get_user_devices(&[user_jid]).await {
log::warn!("Immediate device sync failed: {e:?}");
}
}))
.detach();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== handle_unknown_device_sync =="
sed -n '1115,1135p' src/message.rs

echo
echo "== get_user_devices / flush_pending_device_sync definitions =="
rg -nP --type rust -C8 '^\s*(pub(?:\([^)]*\))?\s+)?async\s+fn\s+(get_user_devices|flush_pending_device_sync)\b'

echo
echo "== LID->PN normalization around device queries =="
rg -n --type rust -C4 'get_phone_number|phone_jid_for_lid_user|sender_alt|is_lid\(|to_non_ad\(|pending_device_sync'

Repository: jlucaso1/whatsapp-rust

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== handle_unknown_device_sync function signature and context ==="
sed -n '1100,1140p' src/message.rs | head -50

echo
echo "=== Client fields and lid_pn_cache access ==="
rg -n 'struct Client|pub.*lid_pn_cache' src/client.rs | head -20

echo
echo "=== Check if sender_alt is available in handle_unknown_device_sync (MessageInfo struct) ==="
rg -n 'pub struct MessageInfo|pub sender_alt' --type rust wacore/ | grep -A2 MessageInfo

echo
echo "=== Examples of sender_alt usage for LID→PN conversion elsewhere ==="
rg -n 'sender_alt' src/message.rs | head -10

Repository: jlucaso1/whatsapp-rust

Length of output: 2628


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== MessageInfo struct with sender_alt field ==="
sed -n '132,150p' wacore/src/types/message.rs

echo
echo "=== Verify sender_alt is populated in group messages with LID senders ==="
sed -n '170,195p' wacore/src/messages.rs

echo
echo "=== cache_lid_pn_from_message implementation ==="
rg -n 'cache_lid_pn_from_message' src/message.rs -A 10 | head -20

Repository: jlucaso1/whatsapp-rust

Length of output: 2736


Use sender_alt to derive the device-query JID for LID senders.

info.source.sender.to_non_ad() preserves @lid identifiers, and this value is passed directly to both pending_device_sync.add() and get_user_devices(). For group messages with LID-mode addressing, info.source.sender_alt contains the corresponding phone-number JID and is reliably populated. Without using sender_alt here, LID senders will continuously refresh against their cached @lid identity and never update companion device lists.

Derive the query JID as:

  • If sender_alt.is_some(), prefer it (the alternate form for device queries)
  • Otherwise, fall back to sender.to_non_ad() (for non-group or PN-mode groups)

This matches the pattern used elsewhere in the codebase (e.g., wacore/src/send.rs:1010–1018) where LID→PN normalization is the caller's responsibility before issuing device queries.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/message.rs` around lines 1117 - 1133, The code currently derives user_jid
via info.source.sender.to_non_ad(), which preserves `@lid` and causes LID senders
to never refresh companion device lists; change the query JID derivation to
prefer info.source.sender_alt when present and fall back to
info.source.sender.to_non_ad() otherwise, then use that normalized JID for
pending_device_sync.add(), invalidate_device_cache(&query_jid.user), and
get_user_devices(&[query_jid]) so LID→PN normalization matches other call sites
(see functions/methods: pending_device_sync.add, invalidate_device_cache,
get_user_devices, info.source.sender_alt, info.source.sender.to_non_ad()).

Comment thread src/retry.rs Outdated
@jlucaso1
jlucaso1 merged commit 3d7c5bb into main Apr 2, 2026
7 of 8 checks passed
@jlucaso1
jlucaso1 deleted the feat/unknown-device-handling branch April 2, 2026 04:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 75b32fda0e

ℹ️ 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 thread src/message.rs
Comment on lines +1120 to +1121
if !self.pending_device_sync.add(user_jid.clone()).await {
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Clear pending flag after online sync attempts

handle_unknown_device_sync inserts every sender into pending_device_sync before branching on info.is_offline, but the online path never removes that user after the spawned immediate usync finishes (success or failure). In an online session, if that first usync fails transiently, subsequent unknown-device messages for the same user hit this early return and skip all further sync attempts; and because flush_pending_device_sync() is only triggered by the IB offline-end flow, recovery can remain stuck until a future reconnect/offline cycle.

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant