Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/bot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use std::future::Future;
use std::marker::PhantomData;
use std::pin::Pin;
use std::sync::Arc;
use std::time::Duration;
use thiserror::Error;
use wacore::proto_helpers::MessageBuilderExt;
use wacore::runtime::Runtime;
Expand Down Expand Up @@ -565,6 +566,7 @@ pub struct BotBuilder<
cache_config: CacheConfig,
wanted_pre_key_count: Option<usize>,
resend_rate_limit: Option<(u32, u32)>,
app_state_key_wait: Option<Duration>,
task_instrument: Option<Arc<dyn wacore::stats::TaskInstrument>>,
alloc_meter: Option<Arc<wacore::stats::AllocMeter>>,
_marker: PhantomData<(B, T, H, R)>,
Expand All @@ -589,6 +591,7 @@ impl BotBuilder<MissingBackend, DefaultTransportState, DefaultHttpState, Default
cache_config: CacheConfig::default(),
wanted_pre_key_count: None,
resend_rate_limit: None,
app_state_key_wait: None,
task_instrument: None,
alloc_meter: None,
_marker: PhantomData,
Expand Down Expand Up @@ -617,6 +620,7 @@ impl<B, T, H, R> BotBuilder<B, T, H, R> {
cache_config: self.cache_config,
wanted_pre_key_count: self.wanted_pre_key_count,
resend_rate_limit: self.resend_rate_limit,
app_state_key_wait: self.app_state_key_wait,
task_instrument: self.task_instrument,
alloc_meter: self.alloc_meter,
_marker: PhantomData,
Expand Down Expand Up @@ -1031,6 +1035,16 @@ impl<B, T, H, R> BotBuilder<B, T, H, R> {
self
}

/// Override how long the initial critical app-state sync waits for the
/// encrypted app-state key-share before attempting the snapshot (default 5s).
/// Raise it when a large concurrent history sync can delay the key-share past
/// the default, which otherwise fails critical sync with "didn't find app
/// state key" and drops the account's saved contact names.
pub fn with_app_state_key_wait(mut self, wait: Duration) -> Self {
self.app_state_key_wait = Some(wait);
self
}

/// Set an initial push name on the device before connecting.
///
/// This is included in the `ClientPayload` during registration, allowing the
Expand Down Expand Up @@ -1185,6 +1199,10 @@ impl BotBuilder<Provided, Provided, Provided, Provided> {
client.set_resend_rate_limit(burst, refill_per_min);
}

if let Some(wait) = self.app_state_key_wait {
client.set_app_state_key_wait(wait);
}

Ok(Bot {
client,
sync_task_receiver: Some(sync_task_receiver),
Expand Down Expand Up @@ -1636,6 +1654,37 @@ mod tests {
);
}

#[tokio::test]
async fn test_bot_builder_app_state_key_wait() {
let backend = create_test_sqlite_backend().await;
let bot = Bot::builder()
.with_backend_arc(backend)
.with_transport_factory(TokioWebSocketTransportFactory::new())
.with_http_client(MockHttpClient)
.with_app_state_key_wait(Duration::from_secs(30))
.with_runtime(TokioRuntime)
.build()
.await
.expect("Failed to build bot with custom app-state key wait");

assert_eq!(bot.client().app_state_key_wait(), Duration::from_secs(30));
}

#[tokio::test]
async fn test_bot_builder_default_app_state_key_wait() {
let backend = create_test_sqlite_backend().await;
let bot = Bot::builder()
.with_backend_arc(backend)
.with_transport_factory(TokioWebSocketTransportFactory::new())
.with_http_client(MockHttpClient)
.with_runtime(TokioRuntime)
.build()
.await
.expect("Failed to build bot");

assert_eq!(bot.client().app_state_key_wait(), Duration::from_secs(5));
}

#[tokio::test]
async fn registered_handlers_accumulate_instead_of_replacing() {
let backend = create_test_sqlite_backend().await;
Expand Down
8 changes: 8 additions & 0 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,14 @@ pub struct Client {
/// Clamped to the protocol-safe range at upload time.
pub(crate) wanted_pre_key_count: AtomicUsize,

/// How long the initial critical app-state sync waits for the encrypted
/// app-state key-share before attempting the snapshot fetch. Default 5s; set
/// via [`BotBuilder::with_app_state_key_wait`] or [`Client::set_app_state_key_wait`].
/// Raise it when a large concurrent history sync can delay the key-share past
/// the default, which otherwise fails critical sync with "didn't find app
/// state key" and drops the account's saved contact names.
pub(crate) app_state_key_wait_ms: AtomicU64,
Comment on lines +795 to +801

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The field doc repeats everything already covered in the public-API doc comments (set_app_state_key_wait, app_state_key_wait, and the builder method), and the AGENTS.md rule says to be concise and explain why, not what. Private/crate-visible fields should carry only the "why" that isn't apparent from the name or public API.

Suggested change
/// How long the initial critical app-state sync waits for the encrypted
/// app-state key-share before attempting the snapshot fetch. Default 5s; set
/// via [`BotBuilder::with_app_state_key_wait`] or [`Client::set_app_state_key_wait`].
/// Raise it when a large concurrent history sync can delay the key-share past
/// the default, which otherwise fails critical sync with "didn't find app
/// state key" and drops the account's saved contact names.
pub(crate) app_state_key_wait_ms: AtomicU64,
/// Milliseconds; see [`Client::set_app_state_key_wait`]. Raising this lets the
/// key-share win the race against a heavy history sync at pairing time.
pub(crate) app_state_key_wait_ms: AtomicU64,

Context Used: AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/client.rs
Line: 795-801

Comment:
The field doc repeats everything already covered in the public-API doc comments (`set_app_state_key_wait`, `app_state_key_wait`, and the builder method), and the AGENTS.md rule says to be concise and explain *why*, not *what*. Private/crate-visible fields should carry only the "why" that isn't apparent from the name or public API.

```suggestion
    /// Milliseconds; see [`Client::set_app_state_key_wait`]. Raising this lets the
    /// key-share win the race against a heavy history sync at pairing time.
    pub(crate) app_state_key_wait_ms: AtomicU64,
```

**Context Used:** AGENTS.md ([source](https://app.greptile.com/oxidezap/github/oxidezap/whatsapp-rust/-/custom-context?memory=26029e85-0dae-44f2-ab23-b8de43e5e9c7))

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code


/// Cache configuration for TTL and capacity of all caches.
/// Stored for use by lazily-initialized caches (group_cache).
pub(crate) cache_config: CacheConfig,
Expand Down
13 changes: 13 additions & 0 deletions src/client/accessors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ impl Client {
self.wanted_pre_key_count.load(Ordering::Relaxed)
}

/// Set how long the initial critical app-state sync waits for the encrypted
/// app-state key-share before attempting the snapshot. Read once when the
/// initial sync starts, so set it before connecting.
pub fn set_app_state_key_wait(&self, wait: Duration) {
self.app_state_key_wait_ms
.store(wait.as_millis() as u64, Ordering::Relaxed);
Comment on lines +76 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 wait.as_millis() returns u128, and as u64 silently truncates values beyond u64::MAX milliseconds. While no realistic timeout would overflow (u64::MAX ms ≈ 584 million years), the cast defeats Clippy's cast_possible_truncation pedantic lint and is inconsistent with the codebase's general avoidance of lossy casts. A saturating conversion makes the intent explicit.

Suggested change
self.app_state_key_wait_ms
.store(wait.as_millis() as u64, Ordering::Relaxed);
self.app_state_key_wait_ms
.store(u64::try_from(wait.as_millis()).unwrap_or(u64::MAX), Ordering::Relaxed);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/client/accessors.rs
Line: 76-77

Comment:
`wait.as_millis()` returns `u128`, and `as u64` silently truncates values beyond `u64::MAX` milliseconds. While no realistic timeout would overflow (u64::MAX ms ≈ 584 million years), the cast defeats Clippy's `cast_possible_truncation` pedantic lint and is inconsistent with the codebase's general avoidance of lossy casts. A saturating conversion makes the intent explicit.

```suggestion
        self.app_state_key_wait_ms
            .store(u64::try_from(wait.as_millis()).unwrap_or(u64::MAX), Ordering::Relaxed);
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: wait.as_millis() is u128, so the direct as u64 cast can silently truncate very large durations. A checked/saturating conversion makes the bound explicit and avoids a lossy cast.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/client/accessors.rs, line 77:

<comment>`wait.as_millis()` is `u128`, so the direct `as u64` cast can silently truncate very large durations. A checked/saturating conversion makes the bound explicit and avoids a lossy cast.</comment>

<file context>
@@ -69,6 +69,19 @@ impl Client {
+    /// initial sync starts, so set it before connecting.
+    pub fn set_app_state_key_wait(&self, wait: Duration) {
+        self.app_state_key_wait_ms
+            .store(wait.as_millis() as u64, Ordering::Relaxed);
+    }
+
</file context>
Suggested change
.store(wait.as_millis() as u64, Ordering::Relaxed);
.store(u64::try_from(wait.as_millis()).unwrap_or(u64::MAX), Ordering::Relaxed);

}

/// The configured initial app-state key-share wait.
pub fn app_state_key_wait(&self) -> Duration {
Duration::from_millis(self.app_state_key_wait_ms.load(Ordering::Relaxed))
}

/// Retune the per-chat outbound resend rate limiter live (no reconnect).
///
/// Outbound resends to a chat are bounded by a token bucket: `burst` is the
Expand Down
1 change: 1 addition & 0 deletions src/client/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ impl Client {
override_version,
skip_history_sync: AtomicBool::new(false),
wanted_pre_key_count: AtomicUsize::new(crate::prekeys::DEFAULT_WANTED_PRE_KEY_COUNT),
app_state_key_wait_ms: AtomicU64::new(5_000),
cache_config,
self_weak: std::sync::OnceLock::new(),
saver_handle: std::sync::OnceLock::new(),
Expand Down
5 changes: 3 additions & 2 deletions src/client/node_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,13 +918,14 @@ impl Client {
.initial_app_state_keys_received
.load(Ordering::Relaxed)
{
let key_wait = client_clone.app_state_key_wait();
debug!(
target: "Client/AppState",
"Waiting up to 5s for app state keys..."
"Waiting up to {key_wait:?} for app state keys..."
);
let _ = rt_timeout(
&*client_clone.runtime,
Duration::from_secs(5),
key_wait,
client_clone.initial_keys_synced_notifier.listen(),
)
.await;
Expand Down