From b1195c3053d96ea8f1fe16e2c60a5068430e6f13 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 2 May 2026 08:04:33 +0000 Subject: [PATCH 1/5] chore: open auto-fix batch claude/friendly-maxwell-f34GI From 0c48f09fc23c7f3debbd9db622fb6fa2f9a5414c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 2 May 2026 08:17:48 +0000 Subject: [PATCH 2/5] test(client): cfg-gate native-only lib tests against wasm32 (#506) Pre-existing: `cargo clippy --target wasm32-unknown-unknown -p willow-client --all-targets` failed with ~209 errors from test modules pulling tokio/std::fs/etc. Production code is WASM-clean (`--lib` passes); only `#[cfg(test)]` modules tripped. Gate test modules with `#[cfg(all(test, not(target_arch = "wasm32")))]` so the wasm-clippy `--all-targets` gate (per resolving-issues skill) can run without false positives masking real wasm regressions. The `test_client` / `test_client_on_hub` helpers are likewise tightened to match `MemNetwork`'s existing native-only gate; they reference `willow_network::mem::MemNetwork` which is already `cfg(all(not(target_arch = "wasm32"), any(test, feature = "test-utils")))`, so this is a correctness fix rather than a behavioural change. No production-code change. Refs #506 --- crates/client/src/base64.rs | 2 +- crates/client/src/emoji.rs | 2 +- crates/client/src/files.rs | 2 +- crates/client/src/invite.rs | 2 +- crates/client/src/joining.rs | 2 +- crates/client/src/lib.rs | 22 +++++++++++----------- crates/client/src/listeners.rs | 2 +- crates/client/src/mentions.rs | 2 +- crates/client/src/nickname.rs | 2 +- crates/client/src/ops.rs | 2 +- crates/client/src/presence.rs | 2 +- crates/client/src/queue.rs | 2 +- crates/client/src/search/mod.rs | 2 +- crates/client/src/state_actors.rs | 2 +- crates/client/src/storage.rs | 2 +- crates/client/src/trust.rs | 2 +- crates/client/src/util.rs | 2 +- crates/client/src/views.rs | 2 +- crates/client/src/worker_cache.rs | 2 +- 19 files changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/client/src/base64.rs b/crates/client/src/base64.rs index b3f717d3..50dbaa92 100644 --- a/crates/client/src/base64.rs +++ b/crates/client/src/base64.rs @@ -60,7 +60,7 @@ pub fn decode(input: &str) -> Option> { Some(result) } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/emoji.rs b/crates/client/src/emoji.rs index a0fe0054..f78a0284 100644 --- a/crates/client/src/emoji.rs +++ b/crates/client/src/emoji.rs @@ -233,7 +233,7 @@ fn builtin(code: &str) -> Option<&'static str> { }) } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/files.rs b/crates/client/src/files.rs index 0a3b54cd..893bc6a4 100644 --- a/crates/client/src/files.rs +++ b/crates/client/src/files.rs @@ -25,7 +25,7 @@ pub async fn download_file( Ok(blobs.get(hash).await?.map(|b| b.to_vec())) } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use willow_network::mem::{MemHub, MemNetwork}; diff --git a/crates/client/src/invite.rs b/crates/client/src/invite.rs index 382a070d..26928d2d 100644 --- a/crates/client/src/invite.rs +++ b/crates/client/src/invite.rs @@ -211,7 +211,7 @@ pub fn endpoint_id_to_ed25519_public(endpoint_id: &willow_identity::EndpointId) *endpoint_id.as_bytes() } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use willow_identity::Identity; diff --git a/crates/client/src/joining.rs b/crates/client/src/joining.rs index 5c26e5ee..8555343f 100644 --- a/crates/client/src/joining.rs +++ b/crates/client/src/joining.rs @@ -354,7 +354,7 @@ impl ClientHandle { } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { //! Tests for the client-side auth guards on invite generation. //! diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 089e002c..e4110e5e 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -48,35 +48,35 @@ mod joining; mod servers; mod voice; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/trust_flow.rs"] mod tests_trust_flow; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/multi_peer_sync.rs"] mod tests_multi_peer_sync; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/queue.rs"] mod tests_queue; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/profile_view.rs"] mod tests_profile_view; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/ephemeral.rs"] mod tests_ephemeral; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/actions.rs"] mod tests_actions; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/voice.rs"] mod tests_voice; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/governance.rs"] mod tests_governance; @@ -1036,7 +1036,7 @@ pub fn reconcile_topic_map( } /// Create a test-only ClientHandle without connecting to the network. -#[cfg(any(test, feature = "test-utils"))] +#[cfg(all(not(target_arch = "wasm32"), any(test, feature = "test-utils")))] pub fn test_client() -> ( ClientHandle, willow_actor::Addr>, @@ -1310,7 +1310,7 @@ pub fn test_client() -> ( /// /// Unlike `test_client()`, multiple clients created with the same `hub` /// can exchange messages through the in-memory gossip mesh. -#[cfg(any(test, feature = "test-utils"))] +#[cfg(all(not(target_arch = "wasm32"), any(test, feature = "test-utils")))] pub async fn test_client_on_hub( hub: &std::sync::Arc, ) -> ( @@ -1323,7 +1323,7 @@ pub async fn test_client_on_hub( (client, broker) } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/listeners.rs b/crates/client/src/listeners.rs index dbea60d6..4b116bce 100644 --- a/crates/client/src/listeners.rs +++ b/crates/client/src/listeners.rs @@ -843,7 +843,7 @@ async fn process_received_message( } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { //! Listener tests for the JoinRequest signer guard (SEC-A-03 / #239). use super::*; diff --git a/crates/client/src/mentions.rs b/crates/client/src/mentions.rs index 3270b6fb..c24c2fa6 100644 --- a/crates/client/src/mentions.rs +++ b/crates/client/src/mentions.rs @@ -245,7 +245,7 @@ fn resolve_mention( None } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use std::collections::HashMap; diff --git a/crates/client/src/nickname.rs b/crates/client/src/nickname.rs index c553198a..b2783c36 100644 --- a/crates/client/src/nickname.rs +++ b/crates/client/src/nickname.rs @@ -93,7 +93,7 @@ impl NicknameStore for MemNicknameStore { } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/ops.rs b/crates/client/src/ops.rs index 973aea17..cccbf1d2 100644 --- a/crates/client/src/ops.rs +++ b/crates/client/src/ops.rs @@ -87,7 +87,7 @@ pub const SERVER_OPS_TOPIC: &str = "_willow_server_ops"; /// Global gossipsub topic for profile broadcasts. pub const PROFILE_TOPIC: &str = "_willow_profiles"; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use willow_identity::Identity; diff --git a/crates/client/src/presence.rs b/crates/client/src/presence.rs index f118aef6..02be5c00 100644 --- a/crates/client/src/presence.rs +++ b/crates/client/src/presence.rs @@ -262,7 +262,7 @@ impl PresenceSnapshot { } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/queue.rs b/crates/client/src/queue.rs index b2b71de5..5aa87efd 100644 --- a/crates/client/src/queue.rs +++ b/crates/client/src/queue.rs @@ -128,7 +128,7 @@ pub fn derive_late_arrival( // ───── Tests ───────────────────────────────────────────────────────────── -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use std::collections::HashSet; diff --git a/crates/client/src/search/mod.rs b/crates/client/src/search/mod.rs index b75176d3..13e81020 100644 --- a/crates/client/src/search/mod.rs +++ b/crates/client/src/search/mod.rs @@ -20,7 +20,7 @@ pub mod query; pub mod status; pub mod tokenize; -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests; pub use bootstrap::{hydrate_index, index_message, reindex_message}; diff --git a/crates/client/src/state_actors.rs b/crates/client/src/state_actors.rs index befae685..15ea7bbe 100644 --- a/crates/client/src/state_actors.rs +++ b/crates/client/src/state_actors.rs @@ -565,7 +565,7 @@ impl Clone for SourceState { // ───── Tests ───────────────────────────────────────────────────────────── -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use willow_identity::Identity; diff --git a/crates/client/src/storage.rs b/crates/client/src/storage.rs index 4e62c2bd..c4d2ee4f 100644 --- a/crates/client/src/storage.rs +++ b/crates/client/src/storage.rs @@ -274,7 +274,7 @@ fn load_raw(key: &str) -> Option> { // ---- Tests ------------------------------------------------------------------ -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/trust.rs b/crates/client/src/trust.rs index 1f0438b9..5946154e 100644 --- a/crates/client/src/trust.rs +++ b/crates/client/src/trust.rs @@ -216,7 +216,7 @@ impl TrustStore for InMemoryTrustStore { } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/util.rs b/crates/client/src/util.rs index 34293e8e..4be57c08 100644 --- a/crates/client/src/util.rs +++ b/crates/client/src/util.rs @@ -76,7 +76,7 @@ pub fn current_time_ms() -> u64 { js_sys::Date::now() as u64 } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; diff --git a/crates/client/src/views.rs b/crates/client/src/views.rs index 0181e03a..64d4844b 100644 --- a/crates/client/src/views.rs +++ b/crates/client/src/views.rs @@ -1031,7 +1031,7 @@ pub fn resolve_display_name( "unknown peer".to_string() } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { //! Projection tests for Phase 2a Task 4 — populated `mentions` in //! `DisplayMessage` and `mentioned` flag in `UnreadStats`. diff --git a/crates/client/src/worker_cache.rs b/crates/client/src/worker_cache.rs index c633d781..c3f75e26 100644 --- a/crates/client/src/worker_cache.rs +++ b/crates/client/src/worker_cache.rs @@ -107,7 +107,7 @@ impl WorkerCache { } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; use willow_identity::Identity; From 4af9add6c73627fc254910ffce34e946443e1b53 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 2 May 2026 08:24:45 +0000 Subject: [PATCH 3/5] fix(web): add referrerpolicy=no-referrer to auto-embed images (SEC-W-04, #243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Peer-supplied http(s):// URLs ending in image extensions auto-embed as . Without `referrerpolicy=no-referrer` the browser sends the page URL via Referer, leaking channel/message context to whatever host the peer chose. IP/UA leak via TCP/TLS is unavoidable once a fetch occurs. Brainstorm rejected `crossorigin=anonymous` — would activate CORS mode and break most legitimate CDN images. Modern browsers strip cookies via SameSite=Lax defaults, so the cookie-leak component is already covered. A user-preference gate (full disable / scheme allowlist) is left as a follow-up (#243); this PR ships the minimum-scope, zero-UX-cost mitigation. Updated the existing browser test (`url_with_image_extension_embeds_inline`) to mirror the new attribute and added an explicit assertion. wasm-pack / Firefox / geckodriver aren't available in this sandbox so the browser test wasn't executed; native `cargo test -p willow-web`, native + wasm32 `cargo clippy -- -D warnings`, and `cargo fmt --check` all pass. Refs #243 --- crates/web/src/components/message.rs | 2 +- crates/web/tests/browser.rs | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/web/src/components/message.rs b/crates/web/src/components/message.rs index 4da51fb2..cf683f07 100644 --- a/crates/web/src/components/message.rs +++ b/crates/web/src/components/message.rs @@ -972,7 +972,7 @@ pub fn MessageView( let url_clone = url.clone(); view! { - embedded image + embedded image } }).collect::>()} diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs index 2a6a92b6..7ff58c29 100644 --- a/crates/web/tests/browser.rs +++ b/crates/web/tests/browser.rs @@ -3440,7 +3440,7 @@ async fn url_with_image_extension_embeds_inline() { let url_clone = url.clone(); view! { - embedded image + embedded image } }).collect::>()} @@ -3466,6 +3466,16 @@ async fn url_with_image_extension_embeds_inline() { img.get_attribute("src").unwrap_or_default(), "https://example.com/cat.png" ); + + // SEC-W-04 (#243): peer-supplied auto-embedded images must carry + // `referrerpolicy="no-referrer"` so the browser does not leak the + // page URL (channel/message context) via the Referer header to a + // hostile peer's chosen host. + assert_eq!( + img.get_attribute("referrerpolicy").unwrap_or_default(), + "no-referrer", + "auto-embedded peer-supplied images must set referrerpolicy=no-referrer" + ); } #[wasm_bindgen_test] From b65ea28798ac6106793287913c1480e1a2e48b49 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 2 May 2026 08:37:34 +0000 Subject: [PATCH 4/5] perf(client): cache truncated SyncRequest reply, invalidate on insert (GEN-08, #268) `topological_sort()` ran O(N) on every WireMessage::SyncRequest, then truncated to 500. For a 50k-event server every sync request paid the full sort + alloc cost. N+1 amplifier per peer. Cache the materialized 500-event Vec on DagState; clear on every successful try_insert_event (and the local-mutation siblings: seed_genesis, build_event, the JoinRequest grant in listeners, plus the server-switch sites in servers.rs that swap ds.managed). Listener checks the cache, recomputes lazily on miss. Test helpers that bypass the listener (replay_dag_into in multi_peer_sync / trust_flow, the sync test in lib.rs) call invalidate_sync_reply_cache explicitly so they don't leak stale defaults. Brainstorm rejected caching on EventDag itself: willow-state's design comment commits it to zero-I/O / pure / no-interior-mutability; cache is a listener concern, belongs at the actor-state layer. Long-term migration to heads-based sync (WorkerRequest::Sync { heads }) remains tracked under #65; this PR closes the per-request waste only. Tests live in crates/client/src/tests/sync_reply_cache.rs: - cached_reply_matches_topological_sort_prefix (semantic preservation) - cache_invalidates_on_insert (invalidation correctness) - cache_hit_returns_identical_vec (cache-hit smoke regression canary) Refs #268 --- crates/client/src/lib.rs | 6 + crates/client/src/listeners.rs | 48 ++++-- crates/client/src/mutations.rs | 14 +- crates/client/src/servers.rs | 7 + crates/client/src/state_actors.rs | 51 +++++++ crates/client/src/tests/multi_peer_sync.rs | 1 + crates/client/src/tests/sync_reply_cache.rs | 161 ++++++++++++++++++++ crates/client/src/tests/trust_flow.rs | 1 + 8 files changed, 272 insertions(+), 17 deletions(-) create mode 100644 crates/client/src/tests/sync_reply_cache.rs diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index e4110e5e..8323f278 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -80,6 +80,10 @@ mod tests_voice; #[path = "tests/governance.rs"] mod tests_governance; +#[cfg(all(test, not(target_arch = "wasm32")))] +#[path = "tests/sync_reply_cache.rs"] +mod tests_sync_reply_cache; + /// How long a typing indicator remains visible after the last typing event, in milliseconds. pub const TYPING_INDICATOR_TTL_MS: u64 = 5_000; @@ -1055,6 +1059,7 @@ pub fn test_client() -> ( ) .expect("genesis insert must succeed in test helper"), stashed: HashMap::new(), + sync_reply_cache: None, }; // Create the general channel in the DAG. @@ -1849,6 +1854,7 @@ mod tests { for event in events_for_b { ds.managed.insert_and_apply(event).ok(); } + ds.invalidate_sync_reply_cache(); }) .await; // Sync B's event_state mirror from the DAG. diff --git a/crates/client/src/listeners.rs b/crates/client/src/listeners.rs index 4b116bce..a1d5728f 100644 --- a/crates/client/src/listeners.rs +++ b/crates/client/src/listeners.rs @@ -203,6 +203,17 @@ async fn topic_listener_loop( // ───── DAG helpers ────────────────────────────────────────────────────────── +/// Compute the cached `WireMessage::SyncRequest` reply payload — the +/// first [`SYNC_REPLY_LIMIT`](state_actors::SYNC_REPLY_LIMIT) events of +/// the DAG's deterministic topological sort. Cache populated lazily on +/// first read after invalidation; cleared by every successful insertion +/// path on `DagState`. See GEN-08 / issue #268. +pub(crate) async fn compute_sync_reply( + dag: &Addr>, +) -> Vec { + willow_actor::state::mutate(dag, |ds| ds.sync_reply_events()).await +} + /// Try to insert an event into the DAG. On success, ManagedDag atomically /// applies it to state and resolves pending events. On chain gap, the /// event is buffered. Duplicates are silently ignored. @@ -221,6 +232,13 @@ async fn try_insert_event(ctx: &ListenerCtx, event: willow_state::Event) { for r in &outcome.resolved { all.push(r.clone()); } + // Any event entering the DAG (including chains drained + // from the pending buffer) changes the topological-sort + // prefix, so invalidate the SyncRequest-reply cache. + // See GEN-08 / issue #268. + if outcome.applied.is_some() || !outcome.resolved.is_empty() { + ds.invalidate_sync_reply_cache(); + } (outcome.applied, all) } Err(willow_state::InsertError::PrevMismatch { @@ -393,20 +411,13 @@ async fn process_received_message( } crate::ops::WireMessage::SyncRequest { state_hash, .. } => { let _ = state_hash; // Legacy field — can't filter by state hash in DAG model. - // TODO: Migrate clients to worker's heads-based sync protocol + // TODO(#65): Migrate clients to worker's heads-based sync protocol // (WorkerRequest::Sync { heads }) for efficient delta sync. - // For now, send the first 500 events from topological sort. - // Receiver will dedup via InsertError::Duplicate. - let events: Vec = willow_actor::state::select(&ctx.dag, |ds| { - ds.managed - .dag() - .topological_sort() - .into_iter() - .take(500) - .cloned() - .collect() - }) - .await; + // For now, send the first SYNC_REPLY_LIMIT events from + // topological sort. Receiver will dedup via InsertError::Duplicate. + // The reply Vec is cached on `DagState` and invalidated on + // every successful DAG insert; see GEN-08 / issue #268. + let events = compute_sync_reply(&ctx.dag).await; if !events.is_empty() { let msg = crate::ops::WireMessage::SyncBatch { events }; if let Some(data) = crate::ops::pack_wire(&msg, &ctx.identity) { @@ -671,7 +682,8 @@ async fn process_received_message( let granted_peer = peer_endpoint; let ts = crate::util::current_time_ms(); let grant_event = willow_actor::state::mutate(&ctx.dag, move |ds| { - ds.managed + let ev = ds + .managed .create_and_insert( &identity, willow_state::EventKind::GrantPermission { @@ -680,7 +692,13 @@ async fn process_received_message( }, ts, ) - .ok() + .ok(); + if ev.is_some() { + // SyncRequest-reply cache must be invalidated on every + // successful insertion path; see GEN-08 / issue #268. + ds.invalidate_sync_reply_cache(); + } + ev }) .await; if let Some(event) = grant_event { diff --git a/crates/client/src/mutations.rs b/crates/client/src/mutations.rs index 68523ca0..7cdf2c8e 100644 --- a/crates/client/src/mutations.rs +++ b/crates/client/src/mutations.rs @@ -100,6 +100,9 @@ impl ClientMutations { ds.managed .insert_and_apply(genesis) .expect("genesis event must insert successfully"); + // SyncRequest-reply cache must be invalidated on every + // successful insertion path; see GEN-08 / issue #268. + ds.invalidate_sync_reply_cache(); ds.managed.state().clone() }) .await; @@ -118,9 +121,16 @@ impl ClientMutations { let dag = self.dag.clone(); util::with_timeout("build_event", async move { willow_actor::state::mutate(&dag, move |ds| { - ds.managed + let result = ds + .managed .create_and_insert(&identity, kind, ts) - .map_err(|e| anyhow::anyhow!("DAG insert failed: {e:?}")) + .map_err(|e| anyhow::anyhow!("DAG insert failed: {e:?}")); + if result.is_ok() { + // SyncRequest-reply cache must be invalidated on every + // successful insertion path; see GEN-08 / issue #268. + ds.invalidate_sync_reply_cache(); + } + result }) .await }) diff --git a/crates/client/src/servers.rs b/crates/client/src/servers.rs index 7aca5622..2db8838c 100644 --- a/crates/client/src/servers.rs +++ b/crates/client/src/servers.rs @@ -30,6 +30,10 @@ impl ClientHandle { ds.managed = willow_state::ManagedDag::empty(crate::state_actors::MAX_CLIENT_PENDING); } + // The cached SyncRequest reply belongs to the *previous* + // server's DAG; the new active DAG has a different topological + // sort. See GEN-08 / issue #268. + ds.invalidate_sync_reply_cache(); ds.managed.state().clone() }) .await; @@ -120,6 +124,9 @@ impl ClientHandle { // Reset managed to empty so seed_genesis creates fresh state. ds.managed = willow_state::ManagedDag::empty(crate::state_actors::MAX_CLIENT_PENDING); + // Cached SyncRequest reply belongs to the previous DAG. + // See GEN-08 / issue #268. + ds.invalidate_sync_reply_cache(); }) .await; } diff --git a/crates/client/src/state_actors.rs b/crates/client/src/state_actors.rs index 15ea7bbe..da1c4920 100644 --- a/crates/client/src/state_actors.rs +++ b/crates/client/src/state_actors.rs @@ -480,6 +480,13 @@ pub struct VoiceState { /// or misbehaving peers sending events with chain gaps. pub(crate) const MAX_CLIENT_PENDING: usize = 5_000; +/// Maximum number of events sent in a single `WireMessage::SyncRequest` +/// reply (a `WireMessage::SyncBatch`). The first N events of the +/// deterministic topological sort. Receiver dedups via +/// `InsertError::Duplicate`. Long-term migration to heads-based sync is +/// tracked under #65; this cap remains until that lands. +pub(crate) const SYNC_REPLY_LIMIT: usize = 500; + /// Combined EventDag + ServerState + PendingBuffer, held in a single /// StateActor via [`ManagedDag`](willow_state::ManagedDag). /// @@ -495,6 +502,15 @@ pub struct DagState { /// When switching servers, the current DAG is stashed and the /// target server's DAG is restored (or a fresh one is created). pub stashed: HashMap, + /// Cached materialized first-`SYNC_REPLY_LIMIT` events of the + /// topological sort, used to answer `WireMessage::SyncRequest`. + /// `None` = stale; will be recomputed on next read. Set to `None` + /// by [`DagState::invalidate_sync_reply_cache`] after every + /// successful DAG insertion (see GEN-08 / issue #268). Lives here + /// rather than on `EventDag` because `willow-state` is intentionally + /// pure / zero-I/O / no-interior-mutability — caching is a listener + /// concern that belongs at the actor-state layer. + pub(crate) sync_reply_cache: Option>, } impl DagState { @@ -515,6 +531,40 @@ impl DagState { pub fn synced(&self) -> bool { self.managed.is_synced() } + + /// Mark the SyncRequest-reply cache as stale. Must be called after + /// every code path that successfully inserts an event into + /// `self.managed` (i.e. whenever `topological_sort()` would return a + /// different prefix). See GEN-08 / issue #268. + pub(crate) fn invalidate_sync_reply_cache(&mut self) { + self.sync_reply_cache = None; + } + + /// Materialize the first [`SYNC_REPLY_LIMIT`] events of the DAG's + /// topological sort, populating the cache on first call after + /// invalidation. Returns a fresh `Vec` (cloned from the cache) ready + /// to ship as a `WireMessage::SyncBatch` payload. + /// + /// Cost on cache hit: one `Vec` clone (~SYNC_REPLY_LIMIT + /// shallow clones). Cost on miss: one `topological_sort()` over the + /// whole DAG plus the same clone. Without this cache every + /// `SyncRequest` paid the full O(N) sort even on a 50k-event DAG — + /// see GEN-08 / issue #268. + pub(crate) fn sync_reply_events(&mut self) -> Vec { + if let Some(cached) = &self.sync_reply_cache { + return cached.clone(); + } + let events: Vec = self + .managed + .dag() + .topological_sort() + .into_iter() + .take(SYNC_REPLY_LIMIT) + .cloned() + .collect(); + self.sync_reply_cache = Some(events.clone()); + events + } } impl Default for DagState { @@ -522,6 +572,7 @@ impl Default for DagState { Self { managed: willow_state::ManagedDag::empty(MAX_CLIENT_PENDING), stashed: HashMap::new(), + sync_reply_cache: None, } } } diff --git a/crates/client/src/tests/multi_peer_sync.rs b/crates/client/src/tests/multi_peer_sync.rs index a30fb3b0..e21e15de 100644 --- a/crates/client/src/tests/multi_peer_sync.rs +++ b/crates/client/src/tests/multi_peer_sync.rs @@ -62,6 +62,7 @@ async fn replay_dag_into( for event in events_for_dag { ds.managed.insert_and_apply(event).ok(); } + ds.invalidate_sync_reply_cache(); }) .await; let state = diff --git a/crates/client/src/tests/sync_reply_cache.rs b/crates/client/src/tests/sync_reply_cache.rs new file mode 100644 index 00000000..987b5f19 --- /dev/null +++ b/crates/client/src/tests/sync_reply_cache.rs @@ -0,0 +1,161 @@ +//! Tests for the cached `WireMessage::SyncRequest` reply payload. +//! +//! Per [GEN-08] (issue #268) the listener used to recompute +//! `topological_sort()` on every `SyncRequest` and then truncate to 500. +//! For a 50k-event server every sync request paid the full O(N) sort +//! and allocation. The fix caches the materialized first-N events on +//! `DagState` and invalidates on every successful insertion. +//! +//! These tests pin three behaviours: +//! +//! 1. **Semantic preservation.** The cached reply matches what +//! `topological_sort().take(SYNC_REPLY_LIMIT)` would produce. +//! 2. **Invalidation correctness.** An insert between two reads +//! surfaces in the second reply. +//! 3. **Cache-hit smoke test.** Two consecutive reads with no +//! insertion in between return byte-identical Vecs (a regression +//! canary in case the cache layer drops back to recomputing). +//! +//! [GEN-08]: https://github.com/willow-org/willow/issues/268 + +use crate::listeners::compute_sync_reply; +use crate::state_actors::SYNC_REPLY_LIMIT; +use crate::test_client; + +/// Build a deterministic baseline by inserting `n` `SetProfile` events +/// (cheap, no permission checks beyond `SendMessages` which the genesis +/// author has implicitly via owner status) and return the sorted-prefix +/// the listener would ship. +async fn topological_prefix( + client: &crate::ClientHandle, +) -> Vec { + willow_actor::state::select(&client.dag_addr, |ds| { + ds.managed + .dag() + .topological_sort() + .into_iter() + .take(SYNC_REPLY_LIMIT) + .cloned() + .collect() + }) + .await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn cached_reply_matches_topological_sort_prefix() { + // Semantic preservation. After enough inserts to cross the truncation + // boundary, the cached reply must equal what the old un-cached path + // produced. + let local = tokio::task::LocalSet::new(); + local + .run_until(async { + let (client, _broker) = test_client(); + + // test_client() seeds genesis (CreateServer) + one + // CreateChannel, so we already have 2 events. Push enough + // SetProfile events to comfortably exceed SYNC_REPLY_LIMIT. + for i in 0..(SYNC_REPLY_LIMIT + 50) { + client + .mutations() + .build_event(willow_state::EventKind::SetProfile { + display_name: format!("name-{i}"), + }) + .await + .expect("local SetProfile must insert"); + } + + let expected = topological_prefix(&client).await; + let cached = compute_sync_reply(&client.dag_addr).await; + + assert_eq!( + cached.len(), + SYNC_REPLY_LIMIT, + "reply must be truncated to SYNC_REPLY_LIMIT" + ); + // Event lacks PartialEq; compare hash sequences. Hashes + // bind every signed field of an event, so identical hash + // sequences imply identical event sequences. + let cached_hashes: Vec<_> = cached.iter().map(|e| e.hash).collect(); + let expected_hashes: Vec<_> = expected.iter().map(|e| e.hash).collect(); + assert_eq!( + cached_hashes, expected_hashes, + "cached reply must match topological_sort().take(SYNC_REPLY_LIMIT)" + ); + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn cache_invalidates_on_insert() { + // Invalidation correctness. A successful insertion between two + // reads must surface in the second reply. + let local = tokio::task::LocalSet::new(); + local + .run_until(async { + let (client, _broker) = test_client(); + + // Genesis + one CreateChannel = 2 events, well under the cap. + let first = compute_sync_reply(&client.dag_addr).await; + let baseline_len = first.len(); + + // Insert one more event via build_event (the path that + // mutations like create_channel / send_message ultimately + // funnel through). + client + .mutations() + .build_event(willow_state::EventKind::SetProfile { + display_name: "after-cache-fill".into(), + }) + .await + .expect("local SetProfile must insert"); + + let second = compute_sync_reply(&client.dag_addr).await; + assert_eq!( + second.len(), + baseline_len + 1, + "post-insert reply must include the new event" + ); + assert!( + second.iter().any(|e| matches!( + &e.kind, + willow_state::EventKind::SetProfile { display_name } + if display_name == "after-cache-fill" + )), + "post-insert reply must contain the SetProfile event" + ); + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn cache_hit_returns_identical_vec() { + // Cache-hit smoke test. Two consecutive reads with no insertion in + // between must produce byte-identical Vecs. Guards against a future + // refactor that drops back to per-call recomputation. + let local = tokio::task::LocalSet::new(); + local + .run_until(async { + let (client, _broker) = test_client(); + + for i in 0..10 { + client + .mutations() + .build_event(willow_state::EventKind::SetProfile { + display_name: format!("name-{i}"), + }) + .await + .expect("local SetProfile must insert"); + } + + let first = compute_sync_reply(&client.dag_addr).await; + let second = compute_sync_reply(&client.dag_addr).await; + + let first_hashes: Vec<_> = first.iter().map(|e| e.hash).collect(); + let second_hashes: Vec<_> = second.iter().map(|e| e.hash).collect(); + assert_eq!( + first_hashes, second_hashes, + "two consecutive reads with no insertion must yield identical replies" + ); + }) + .await; +} diff --git a/crates/client/src/tests/trust_flow.rs b/crates/client/src/tests/trust_flow.rs index c0f9e5d5..79bfa85b 100644 --- a/crates/client/src/tests/trust_flow.rs +++ b/crates/client/src/tests/trust_flow.rs @@ -91,6 +91,7 @@ async fn connected_pair() -> ( for event in events_for_bob { ds.managed.insert_and_apply(event).ok(); } + ds.invalidate_sync_reply_cache(); }) .await; let bob_state = From bb3aa734f11c5f6b5f065d3a3b8dcb3d8ecf0971 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 2 May 2026 08:39:14 +0000 Subject: [PATCH 5/5] docs(skill): retroactive meta-tracker fill-in during sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3rd-trigger rule for structural-deps meta-tracker was reactive — a run could land in a queue with 5+ trackers and no meta-tracker, with no clear instruction whether to consolidate or wait for the next filing trigger. Spell out: when the sweep finds 3+ trackers without a meta-tracker, file the meta-tracker as part of the sweep itself. Pure metadata work, falls under the Coordinator-never-codes exceptions (no source files touched). Surfaced this run: 5 trackers (#246, #247, #249, #481, #485) existed since prior runs; meta-tracker filed retroactively as #510. Refs .claude/skills/resolving-issues/SKILL.md --- .claude/skills/resolving-issues/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/resolving-issues/SKILL.md b/.claude/skills/resolving-issues/SKILL.md index 99ee970f..ae5afa19 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -123,6 +123,8 @@ Fresh agent per issue, scoped to one issue + master branch ref. Steps: 12. **Structural-deps follow-up family path:** dependency-multi-version audits (rand, getrandom, convert_case, bincode, etc.) often look "obvious" but are pinned by transitive crates we don't own — no workspace pin / `[patch]` can collapse them without lying about semver. The first 1–2 finds get individual follow-up trackers. On the **3rd** structural-deps follow-up in this family, file or update a single **upstream-domino meta-tracker issue** instead of another standalone TD-NN follow-up — list the holdout crates, the upstream releases that would unblock each version (e.g. `aes-gcm 0.11` stable, `derive_more 3.x`, `iroh ≥ N`), and link prior individual follow-ups under it. Future runs check the meta-tracker, don't refile the same shape. + **Retroactive meta-tracker fill-in (coordinator-direct, no implementer dispatch).** When a run's triage finds 3+ structural-deps trackers already exist *without* a consolidating meta-tracker, the coordinator files the meta-tracker as part of the step 6 already-fixed sweep — same pattern as closing already-fixed issues, pure metadata work, falls under the "Coordinator never codes" exceptions because no source files are touched. List rows for every active tracker, link them under the meta, comment on each individual tracker citing the meta. Skill compliance is *retroactive*: fix the gap when you spot it, don't leave the next run to re-derive the consolidation. Record the new meta issue under `## Skill Evolution` in the master PR body alongside the lessons. + 13. **Report back** to coordinator: commit SHA on master branch, sites touched, anything unusual. ## Lessons Learned