diff --git a/.claude/skills/resolving-issues/SKILL.md b/.claude/skills/resolving-issues/SKILL.md index 4dbc863e..118585a9 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -230,6 +230,7 @@ Coordinator owns this check (metadata work, allowed under "coordinator never cod ### Implementer-flagged out-of-scope rot - When the implementer surfaces pre-existing rot it intentionally doesn't fix (e.g. unrelated wasm break under `--all-features`, dead-code warnings in untouched files), the coordinator files a follow-up GH issue using `mcp__github__issue_write` (metadata work, allowed under the Coordinator-never-codes rule). Cite the discovery context (which dispatch surfaced it, which commit + gate step) so the next run has full provenance. - This is the same shape as the "implementer files follow-up" rule — coordinator just does the filing because the implementer is single-task and exits after commit. +- **Always reproduce the failure on coordinator HEAD before filing OR dismissing.** A prior run dismissing the same shape as a "sandbox-side flake" doesn't mean it stays a flake — rot accumulates as PRs merge, and a previously-flaky symptom can become a real regression once a related change lands. Run the failing command in the coordinator's checkout (single shot, no retry loops); if it reproduces, file the follow-up with the exact assertion + the git history that exposed it (e.g. `git log --oneline ..HEAD -- ` to find the PR that flipped flake → real). If it doesn't reproduce, don't file — but note the dismissal in the run-end Lessons Learned so the next run has the audit trail. Never rely on a prior dismissal alone; always re-verify. ### Fresh agent per issue - New implementer each issue. No state leak. diff --git a/Cargo.lock b/Cargo.lock index 591b7280..1c4c3be1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6217,6 +6217,7 @@ dependencies = [ "serde", "thiserror 2.0.18", "tokio", + "tracing", "uuid", "willow-identity", "willow-transport", diff --git a/crates/agent/src/auth.rs b/crates/agent/src/auth.rs index 505e0d8c..07760d85 100644 --- a/crates/agent/src/auth.rs +++ b/crates/agent/src/auth.rs @@ -4,16 +4,15 @@ //! Stdio transport skips auth (process isolation). SSE/HTTP transports //! require a bearer token in the `Authorization` header. -use rand::Rng; +use rand::{rngs::OsRng, RngCore}; /// Token prefix for Willow agent tokens. const TOKEN_PREFIX: &str = "wlw_"; /// Generate a 256-bit random bearer token with `wlw_` prefix. pub fn generate_token() -> String { - let mut rng = rand::thread_rng(); let mut bytes = [0u8; 32]; - rng.fill(&mut bytes); + OsRng.fill_bytes(&mut bytes); format!("{}{}", TOKEN_PREFIX, hex::encode(&bytes)) } diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 8323f278..ab36059f 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/search.rs"] +mod tests_search; + #[cfg(all(test, not(target_arch = "wasm32")))] #[path = "tests/sync_reply_cache.rs"] mod tests_sync_reply_cache; diff --git a/crates/client/src/listeners.rs b/crates/client/src/listeners.rs index a1d5728f..4b268a62 100644 --- a/crates/client/src/listeners.rs +++ b/crates/client/src/listeners.rs @@ -411,9 +411,9 @@ 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(#65): Migrate clients to worker's heads-based sync protocol - // (WorkerRequest::Sync { heads }) for efficient delta sync. - // For now, send the first SYNC_REPLY_LIMIT events from + // TODO(#382): Migrate clients to worker's heads-based sync + // protocol (WorkerRequest::Sync { heads }) for efficient delta + // sync. 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. diff --git a/crates/client/src/state.rs b/crates/client/src/state.rs index 398d2bb2..72d19d0e 100644 --- a/crates/client/src/state.rs +++ b/crates/client/src/state.rs @@ -137,12 +137,13 @@ pub struct DisplayMessage { /// Queue-note state for this row (see [`QueueNote`]). /// /// Populated by the view projection in - /// `views::compute_messages_view`. Today the projection defers the - /// real detection to the sync-queue crate and always returns - /// `None` — see `TODO(sync-queue.md)` in `views.rs`. The renderer - /// is already wired for the full tri-state so the UX will light up - /// once detection lands. The grouping predicate in `chat.rs` - /// treats any non-`None` variant as a run-break per + /// `views::compute_messages_view`. Phase 2b (see + /// `docs/plans/2026-04-21-ui-phase-2b-sync-queue.md`) closed the + /// original `TODO(sync-queue.md)` gate: the projection now derives + /// real `Pending` / `LateArrival` values from `QueueMeta`. The + /// renderer is wired for the full tri-state. The grouping + /// predicate in `chat.rs` treats any non-`None` variant as a + /// run-break per /// `docs/specs/2026-04-19-ui-design/message-row.md` §Queue notes. pub queue_note: QueueNote, } diff --git a/crates/client/src/tests/search.rs b/crates/client/src/tests/search.rs new file mode 100644 index 00000000..1eec3de9 --- /dev/null +++ b/crates/client/src/tests/search.rs @@ -0,0 +1,128 @@ +//! Focused contract tests for [`SearchIndexHandle::set_config`] — +//! audit F42 (issue #542). +//! +//! Prior to this file the only direct coverage of `set_config` lived in +//! `crates/client/src/search/tests.rs::handle_tests`, where each test +//! exercised one *side* of the contract (insert gating, recents gating, +//! grove opt-out gating). The audit asked for a focused test that locks +//! in `set_config` itself, independently of Effect wiring or the +//! `Insert` handler's own gating logic. +//! +//! Two complementary assertions live here: +//! +//! 1. [`set_config_round_trip`] — the minimum contract: after +//! `set_config(c)`, a subsequent `config().await` returns exactly +//! `c`. This pins the read/write symmetry of the actor's `SetConfig` +//! + `GetConfig` handler pair. +//! +//! 2. [`set_config_changes_rebuild_query_results`] — the only path by +//! which `set_config` produces an *observable query-result delta* +//! against a previously-indexed corpus is via a subsequent +//! `rebuild`, because the executor itself does not consult config +//! (gating happens at write time in `SearchActor::message_allowed`). +//! This test indexes a two-grove corpus, asserts both groves are +//! queryable, then opts grove `g1` out via `set_config`, rebuilds, +//! and asserts `g1`'s message no longer appears while `g0`'s still +//! does. That sequence proves `set_config` actually mutated the +//! actor's config snapshot in a way the next `rebuild` observes. +//! +//! Why not assert a query-result delta directly after `set_config` +//! without a rebuild? Because the executor in +//! `crates/client/src/search/execute.rs` reads only the index and the +//! query — never the config. Asserting an immediate delta would +//! require either changing production behaviour (queries consulting +//! config) or testing a contract the code doesn't make. We pick the +//! existing contract instead: rebuild reflects the latest config. + +use willow_actor::System; +use willow_identity::Identity; + +use crate::search::{ + parse_query, IndexableMessage, SearchIndexConfig, SearchIndexHandle, SearchScope, +}; + +fn mk(id: &str, body: &str, grove: &str) -> IndexableMessage { + IndexableMessage { + message_id: id.into(), + channel_id: format!("c-{grove}"), + channel_name: "general".into(), + grove_id: Some(grove.into()), + letter_id: None, + author_peer_id: Identity::generate().endpoint_id(), + author_handle: "mira".into(), + author_display_name: "Mira".into(), + timestamp_ms: 100, + body: body.into(), + has_image: false, + has_file: false, + has_link: false, + } +} + +/// Round-trip: writing a config and reading it back returns the same +/// value. Pins the `SetConfig` + `GetConfig` handler pair as a single +/// atomic read-after-write. +#[tokio::test] +async fn set_config_round_trip() { + let sys = System::new(); + let h = SearchIndexHandle::new_in_memory(&sys.handle()); + + let mut per_grove_enabled = std::collections::HashMap::new(); + per_grove_enabled.insert("g-quiet".into(), false); + per_grove_enabled.insert("g-loud".into(), true); + let cfg = SearchIndexConfig { + enabled: false, + horizon_days: 30, + remember_recents: false, + per_grove_enabled, + }; + + h.set_config(cfg.clone()); + assert_eq!(h.config().await, cfg); +} + +/// `set_config` followed by `rebuild` must reflect the new config in +/// query results: a grove opted out via `per_grove_enabled` after the +/// initial index build disappears from queries once the index is +/// rebuilt against the same corpus. +#[tokio::test] +async fn set_config_changes_rebuild_query_results() { + let sys = System::new(); + let h = SearchIndexHandle::new_in_memory(&sys.handle()); + + // Two messages, one per grove, both containing the term `signal`. + let corpus = vec![ + mk("m-keep", "signal here", "g0"), + mk("m-drop", "signal there", "g1"), + ]; + h.rebuild(corpus.clone()).await; + + // Baseline: both groves visible. + let q = parse_query("signal"); + let pre = h.query(&q, &SearchScope::AllGrovesAndLetters).await; + let pre_ids: Vec<_> = pre.iter().map(|r| r.message_id.clone()).collect(); + assert_eq!( + pre.len(), + 2, + "baseline must surface both groves: {pre_ids:?}" + ); + assert!(pre_ids.contains(&"m-keep".into())); + assert!(pre_ids.contains(&"m-drop".into())); + + // Opt grove `g1` out, then rebuild against the same corpus. + let mut cfg = h.config().await; + cfg.per_grove_enabled.insert("g1".into(), false); + h.set_config(cfg); + h.rebuild(corpus).await; + + // Post-rebuild the opted-out grove's message is gone; the kept + // grove's message still hits. + let post = h.query(&q, &SearchScope::AllGrovesAndLetters).await; + let post_ids: Vec<_> = post.iter().map(|r| r.message_id.clone()).collect(); + assert_eq!( + post.len(), + 1, + "after set_config + rebuild only g0 must remain: {post_ids:?}" + ); + assert_eq!(post[0].message_id, "m-keep"); +} diff --git a/crates/client/src/views.rs b/crates/client/src/views.rs index 64d4844b..a13651d2 100644 --- a/crates/client/src/views.rs +++ b/crates/client/src/views.rs @@ -92,13 +92,13 @@ impl ServerRegistry { /// exposes its full membership — see the multi-grove TODO on /// `servers.rs`). pub fn shared_groves(&self, _local: &EndpointId, _other: &EndpointId) -> Vec { - // TODO(multi-grove): plumb `state.members` into `ServerEntry` - // so the intersection can walk every grove the local peer is - // in. Until then, the helper returns the active grove's name - // when we know both peers are members (check deferred to the - // UI which reads `MembersView` for the active server). Return - // an empty Vec rather than fabricating a match — the spec's - // edge case "no shared groves → omit section" covers this. + // TODO(#563): plumb `state.members` into `ServerEntry` so the + // intersection can walk every grove the local peer is in. Until + // then, the helper returns the active grove's name when we know + // both peers are members (check deferred to the UI which reads + // `MembersView` for the active server). Return an empty Vec rather + // than fabricating a match — the spec's edge case "no shared + // groves → omit section" covers this. Vec::new() } } @@ -477,8 +477,10 @@ impl ClientViewHandle { /// Phase 2b: accepts a `queue_meta` snapshot so the projection can /// derive real `QueueNote::Pending` / `QueueNote::LateArrival` values /// for each row via [`crate::queue::derive_pending`] + -/// [`crate::queue::derive_late_arrival`]. Closes the -/// `TODO(sync-queue.md)` gate in this function and in the Phase 2a +/// [`crate::queue::derive_late_arrival`]. Closes the original +/// sync-queue gate (see plan +/// `docs/plans/2026-04-21-ui-phase-2b-sync-queue.md`) in this function +/// and in the Phase 2a plan /// `docs/plans/2026-04-20-ui-phase-2a-message-row.md` at line 490. pub fn compute_messages_view( events: &Arc, @@ -508,8 +510,9 @@ pub fn compute_messages_view( // track a distinct `@handle` (see `profile-card.md` for the target // profile data model); as a stand-in we derive a handle from the // display name via `display_name.to_lowercase().replace(' ', '.')`. - // TODO(profile-card.md): replace the display-name-derived handle - // with the real handle field once profile data is plumbed. + // TODO(plan: docs/plans/2026-04-21-ui-phase-2c-profile-card.md): + // replace the display-name-derived handle with the real handle field + // once profile data is plumbed. let peer_refs: Vec = events .members .keys() @@ -597,11 +600,11 @@ pub fn compute_messages_view( } else { QueueNote::None }; - // TODO(whisper-mode.md): flip via WhisperStart event when - // that phase lands. Phase 2a Task 8 reserves the row - // styling surface (message--whisper class + whisper-badge) - // behind this always-false gate so later work only has to - // swap the projection lookup. + // TODO(#562): flip via WhisperStart event when that phase + // lands. Phase 2a Task 8 reserves the row styling surface + // (message--whisper class + whisper-badge) behind this + // always-false gate so later work only has to swap the + // projection lookup. let whisper = false; DisplayMessage { id: m.id.to_string(), @@ -697,8 +700,10 @@ pub fn compute_unread_view( let mute = event_state.mute_state.get(&local_peer_id).cloned(); // Build a PeerRef list once for the mention parser. Mirrors the - // build in `compute_messages_view`; TODO(profile-card.md) tracks - // swapping display-name-derived handles for real profile handles. + // build in `compute_messages_view`; the + // TODO(plan: docs/plans/2026-04-21-ui-phase-2c-profile-card.md) + // there tracks swapping display-name-derived handles for real + // profile handles. // // `resolve_display_name` needs a `ProfileState` — we only have the // event-state profiles here, so fall back to the event-state entry @@ -789,7 +794,7 @@ pub fn compute_roles_view(events: &Arc) -> RolesView .map(|role| RoleEntry { id: role.id.clone(), name: role.name.clone(), - permissions: role.permissions.iter().map(|p| format!("{p:?}")).collect(), + permissions: role.permissions.iter().map(|p| p.to_string()).collect(), }) .collect(); roles.sort_by(|a, b| a.name.cmp(&b.name)); diff --git a/crates/messaging/Cargo.toml b/crates/messaging/Cargo.toml index de3b7088..3c44b4c1 100644 --- a/crates/messaging/Cargo.toml +++ b/crates/messaging/Cargo.toml @@ -13,6 +13,7 @@ serde = { workspace = true } chrono = { workspace = true } uuid = { workspace = true } thiserror = { workspace = true } +tracing = { workspace = true } [target.'cfg(target_arch = "wasm32")'.dependencies] js-sys = "0.3" diff --git a/crates/messaging/src/hlc.rs b/crates/messaging/src/hlc.rs index f56007cd..5d3d3f82 100644 --- a/crates/messaging/src/hlc.rs +++ b/crates/messaging/src/hlc.rs @@ -72,6 +72,18 @@ impl std::fmt::Display for HlcTimestamp { } } +/// Maximum forward drift, in milliseconds, that a remote HLC timestamp may +/// have over the local wall clock before it is clamped in [`HLC::receive`]. +/// +/// Without this cap a single malicious or buggy peer could broadcast one +/// message with `millis = u64::MAX - k`, permanently poisoning every +/// receiver's HLC and saturating the counter on subsequent local events +/// (Kulkarni et al., 2014, §V — recommended forward-drift bound). +/// +/// 24h is generous enough to absorb realistic NTP skew, mobile sleep, and +/// timezone-misconfigured peers while still bounding worst-case poisoning. +pub const MAX_FORWARD_DRIFT_MS: u64 = 24 * 60 * 60 * 1000; + // ───── HLC state machine ──────────────────────────────────────────────────── /// Returns the current wall-clock time in milliseconds since Unix epoch. @@ -169,15 +181,36 @@ impl HLC { /// /// Returns a new local timestamp that is strictly greater than both the /// local clock and the remote timestamp. + /// + /// The remote `millis` value is clamped to + /// `wall_clock + MAX_FORWARD_DRIFT_MS` before being folded into the + /// `max` chain. This prevents a single peer with a far-future timestamp + /// from permanently poisoning the local HLC (see + /// [`MAX_FORWARD_DRIFT_MS`]). A clamp event emits a `warn!` so ops can + /// surface peers that consistently exceed the drift bound. pub fn receive(&mut self, remote: HlcTimestamp) -> HlcTimestamp { let wall = wall_clock_ms(); - let millis = wall.max(self.latest.millis).max(remote.millis); - let (millis, counter) = if millis == self.latest.millis && millis == remote.millis { + let max_allowed = wall.saturating_add(MAX_FORWARD_DRIFT_MS); + let clamped_remote_millis = if remote.millis > max_allowed { + tracing::warn!( + remote_millis = remote.millis, + wall_ms = wall, + max_allowed, + "HLC remote timestamp exceeded forward-drift cap; clamping" + ); + max_allowed + } else { + remote.millis + }; + + let millis = wall.max(self.latest.millis).max(clamped_remote_millis); + + let (millis, counter) = if millis == self.latest.millis && millis == clamped_remote_millis { bump_counter(millis, self.latest.counter.max(remote.counter)) } else if millis == self.latest.millis { bump_counter(millis, self.latest.counter) - } else if millis == remote.millis { + } else if millis == clamped_remote_millis { bump_counter(millis, remote.counter) } else { (millis, 0) @@ -414,6 +447,90 @@ mod tests { assert!(t2 > t1); } + #[test] + fn receive_clamps_far_future_remote() { + // A malicious peer broadcasting `u64::MAX - 1000` must not be able to + // poison the local HLC. After the call, `latest.millis` must sit + // within roughly `wall + MAX_FORWARD_DRIFT_MS` (plus a tiny epsilon + // for the wall-clock tick between calls). + let wall_before = wall_clock_ms(); + let mut clock = HLC::new(); + + let attack = HlcTimestamp { + millis: u64::MAX - 1000, + counter: 0, + }; + let result = clock.receive(attack); + + let upper_bound = wall_before + .saturating_add(MAX_FORWARD_DRIFT_MS) + .saturating_add(1_000); // 1s slack for wall-clock advance + assert!( + result.millis <= upper_bound, + "receive() must clamp far-future remote (got {}, bound {upper_bound})", + result.millis, + ); + + // Subsequent now() calls must still produce reasonable timestamps, + // not anywhere near u64::MAX. + let next = clock.now(); + assert!( + next.millis <= upper_bound.saturating_add(1_000), + "now() after clamp must remain near wall clock (got {})", + next.millis, + ); + assert!(next > result, "monotonicity must still hold"); + } + + #[test] + fn receive_accepts_within_drift() { + // A remote timestamp 1 hour in the future is well within the 24h + // drift cap and must be adopted unchanged. + let wall = wall_clock_ms(); + let mut clock = HLC::new(); + + let one_hour_ms: u64 = 60 * 60 * 1000; + let remote = HlcTimestamp { + millis: wall + one_hour_ms, + counter: 0, + }; + let result = clock.receive(remote); + + // The HLC should adopt the remote's millis (since it dominates wall + // and latest), not clamp it. + assert_eq!( + result.millis, remote.millis, + "remote within drift bound must be adopted unchanged", + ); + assert!(result > remote); + } + + #[test] + fn receive_clamps_at_exact_boundary() { + // The boundary check is strict (`>`), so a remote exactly one ms over + // the cap must clamp, while the cap itself is still accepted. + let wall = wall_clock_ms(); + let mut clock = HLC::new(); + + let remote = HlcTimestamp { + millis: wall + MAX_FORWARD_DRIFT_MS + 1, + counter: 0, + }; + let result = clock.receive(remote); + + // After clamping the remote down to `wall + MAX_FORWARD_DRIFT_MS`, + // the new latest must equal that cap (possibly +1 if the wall clock + // ticked between the two reads; assert with a small slack window). + let lower = wall.saturating_add(MAX_FORWARD_DRIFT_MS); + let upper = wall.saturating_add(MAX_FORWARD_DRIFT_MS).saturating_add(50); + assert!( + (lower..=upper).contains(&result.millis), + "receive() must clamp to wall + MAX_FORWARD_DRIFT_MS \ + (got {}, expected in [{lower}, {upper}])", + result.millis, + ); + } + #[test] fn receive_when_local_is_ahead_of_remote() { let mut clock = HLC::new(); diff --git a/crates/network/src/traits.rs b/crates/network/src/traits.rs index 5147d9d8..89cd187b 100644 --- a/crates/network/src/traits.rs +++ b/crates/network/src/traits.rs @@ -158,9 +158,10 @@ pub trait Network: Send + Sync + 'static { /// Access the blob store. fn blobs(&self) -> &dyn BlobStore; - // TODO(#119): add connection_events() — stream relay up/down and direct + // TODO(#561): add connection_events() — stream relay up/down and direct // peer connect/disconnect events so the client can surface connectivity - // status in the UI. + // status in the UI. (#119 was the original tracker, closed before the + // placeholder `pending()` impls were replaced — #561 is the live followup.) /// Reachability of the configured relay, or [`RelayStatus::NotConfigured`] /// when none is set. Default impl returns `NotConfigured`; the real diff --git a/crates/state/src/event.rs b/crates/state/src/event.rs index cf168ebe..b081d2c1 100644 --- a/crates/state/src/event.rs +++ b/crates/state/src/event.rs @@ -86,6 +86,30 @@ impl Permission { } } +/// Human-readable permission labels for UI surfaces. +/// +/// Centralising the strings here keeps UX wording in one place and +/// avoids leaking the `Debug`-derived variant identifiers (e.g. +/// `SyncProvider`) into role lists, settings panes, or MCP resources. +/// `Debug` remains available for logs and developer-facing output. +impl std::fmt::Display for Permission { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let label = match self { + Self::SyncProvider => "Sync provider", + Self::ManageChannels => "Manage channels", + Self::ManageRoles => "Manage roles", + Self::SendMessages => "Send messages", + Self::CreateInvite => "Create invite", + // Sentinel from a future/unknown client. Should never reach + // a UI render path in practice — `apply_event` drops these + // before they enter any role's permission set — but Display + // must be total, so emit a clearly-flagged placeholder. + Self::__UnknownLegacy => "Unknown", + }; + f.write_str(label) + } +} + impl<'de> Deserialize<'de> for Permission { /// Custom deserialize that accepts both the enum form (default for /// any format — bincode emits a u32 discriminant, JSON emits the @@ -484,7 +508,17 @@ impl Event { kind: &self.kind, timestamp_hint_ms: self.timestamp_hint_ms, }; - let bytes = bincode::serialize(&signable).expect("event serialization should not fail"); + // Defense-in-depth: bincode of owned Vec/String/integers shouldn't + // fail in practice, but `kind` is attacker-controlled. A malformed + // String produced via `unsafe` could in theory fail to serialize. + // Reject the event instead of panicking on the hot verify path. + let bytes = match bincode::serialize(&signable) { + Ok(b) => b, + Err(e) => { + tracing::warn!(error = %e, "event verify: bincode serialize failed; rejecting"); + return false; + } + }; // Verify hash matches content. if self.hash != EventHash::from_bytes(&bytes) { @@ -588,4 +622,32 @@ mod tests { event.author = id_b.endpoint_id(); assert!(!event.verify()); } + + #[test] + fn verify_returns_false_on_garbage_event() { + // Defense-in-depth: verify() should never panic on adversarial + // input, even if the hash and signature are obviously bogus. + // The bincode-failure branch in verify() is unreachable from safe + // Rust on the current types (owned Vec/String/integers), so this + // test exercises the adjacent hash/sig mismatch path to confirm + // the function returns gracefully instead of panicking. + let id = Identity::generate(); + let mut event = make_event(&id, test_kind()); + event.hash = EventHash::from_bytes(b"not-the-real-hash"); + event.sig = Signature::from_bytes(&[0u8; 64]); + assert!(!event.verify()); + } + + #[test] + fn permission_display_strings() { + // UI surfaces (role lists, settings, MCP resources) render + // permissions via Display. Locking these strings here keeps the + // wording stable and surfaces wording changes as test diffs. + assert_eq!(Permission::SyncProvider.to_string(), "Sync provider"); + assert_eq!(Permission::ManageChannels.to_string(), "Manage channels"); + assert_eq!(Permission::ManageRoles.to_string(), "Manage roles"); + assert_eq!(Permission::SendMessages.to_string(), "Send messages"); + assert_eq!(Permission::CreateInvite.to_string(), "Create invite"); + assert_eq!(Permission::__UnknownLegacy.to_string(), "Unknown"); + } } diff --git a/crates/state/src/sync.rs b/crates/state/src/sync.rs index bae4624f..f88fe1e7 100644 --- a/crates/state/src/sync.rs +++ b/crates/state/src/sync.rs @@ -92,7 +92,14 @@ impl Snapshot { state: &state, heads: sorted_heads, }; - let bytes = bincode::serialize(&input).expect("snapshot serialization should not fail"); + // `state` is locally materialized via `apply_incremental`, which + // rejects malformed events before they enter the DAG. Any failure + // here would indicate an invariant violation, not attacker input. + // Returning Result would ripple through every call site; keep the + // panic but document the invariant. + let bytes = bincode::serialize(&input).expect( + "snapshot serialization invariant violated; should be unreachable post-validation", + ); let hash = EventHash::from_bytes(&bytes); Self { state, heads, hash } } diff --git a/crates/storage/src/role.rs b/crates/storage/src/role.rs index e58b0e44..01ff039b 100644 --- a/crates/storage/src/role.rs +++ b/crates/storage/src/role.rs @@ -282,6 +282,121 @@ mod tests { } } + #[test] + fn set_default_server_routes_subsequent_events() { + // Focused round-trip test for `set_default_server`. + // + // The setter mutates an in-memory field (no SQLite persistence — the + // field exists only on `StorageRole`, not the `StorageEventStore`), + // so we verify the change via the observable read path: `on_event` + // routes events through the configured server ID, which `handle_request` + // then queries by server ID. + // + // Asserts: + // 1. Default ID is `"default"` (events stored without calling the + // setter are queryable under `"default"`). + // 2. After `set_default_server("srv-A")`, new events route to + // `"srv-A"` and are NOT visible under the previous ID. + // 3. Last-write-wins: after a second `set_default_server("srv-B")`, + // newer events route to `"srv-B"` and earlier ones remain under + // their original server. + let store = StorageEventStore::open(":memory:").unwrap(); + let mut role = StorageRole::new(store); + let id = Identity::generate(); + + // (1) Default value — no setter call yet. + let e1 = make_message(&id, 1, EventHash::ZERO, "general"); + role.on_event(&e1); + + let resp = role.handle_request(WorkerRequest::History { + server_id: "default".to_string(), + channel: Some("general".to_string()), + before: None, + limit: 10, + }); + match resp { + WorkerResponse::HistoryPage { events, .. } => assert_eq!( + events.len(), + 1, + "event before set_default_server should land under \"default\"" + ), + _ => panic!("expected HistoryPage"), + } + + // (2) Set to "srv-A" and verify routing changes. + role.set_default_server("srv-A".to_string()); + let e2 = make_message(&id, 2, e1.hash, "general"); + role.on_event(&e2); + + let resp = role.handle_request(WorkerRequest::History { + server_id: "srv-A".to_string(), + channel: Some("general".to_string()), + before: None, + limit: 10, + }); + match resp { + WorkerResponse::HistoryPage { events, .. } => assert_eq!( + events.len(), + 1, + "event after set_default_server(\"srv-A\") should land under \"srv-A\"" + ), + _ => panic!("expected HistoryPage"), + } + + // The "default" bucket still has only the first event — proves the + // setter took effect rather than aliasing both IDs. + let resp = role.handle_request(WorkerRequest::History { + server_id: "default".to_string(), + channel: Some("general".to_string()), + before: None, + limit: 10, + }); + match resp { + WorkerResponse::HistoryPage { events, .. } => assert_eq!( + events.len(), + 1, + "\"default\" bucket should not gain events after switching servers" + ), + _ => panic!("expected HistoryPage"), + } + + // (3) Last-write-wins: switch again to "srv-B". + role.set_default_server("srv-B".to_string()); + let e3 = make_message(&id, 3, e2.hash, "general"); + role.on_event(&e3); + + let resp = role.handle_request(WorkerRequest::History { + server_id: "srv-B".to_string(), + channel: Some("general".to_string()), + before: None, + limit: 10, + }); + match resp { + WorkerResponse::HistoryPage { events, .. } => assert_eq!( + events.len(), + 1, + "event after second set_default_server should land under \"srv-B\"" + ), + _ => panic!("expected HistoryPage"), + } + + // "srv-A" bucket retains its single event. + let resp = role.handle_request(WorkerRequest::History { + server_id: "srv-A".to_string(), + channel: Some("general".to_string()), + before: None, + limit: 10, + }); + match resp { + WorkerResponse::HistoryPage { events, .. } => assert_eq!( + events.len(), + 1, + "\"srv-A\" bucket should be unchanged after switching to \"srv-B\"" + ), + _ => panic!("expected HistoryPage"), + } + } + #[test] fn history_for_unknown_channel_returns_empty() { let store = StorageEventStore::open(":memory:").unwrap(); diff --git a/crates/web/src/components/message.rs b/crates/web/src/components/message.rs index cf683f07..8e8bd8d7 100644 --- a/crates/web/src/components/message.rs +++ b/crates/web/src/components/message.rs @@ -264,9 +264,9 @@ pub fn MessageView( let has_queue_note = queue_note != QueueNote::None; // Phase 2a Task 8: reserve the whisper surface. `message.whisper` // is gated always-false in the projection today (see - // `client/src/views.rs` TODO(whisper-mode.md)); once that phase - // lands the projection will flip it and the class + badge below - // light up automatically. + // `client/src/views.rs` TODO(#562)); once that phase lands the + // projection will flip it and the class + badge below light up + // automatically. let is_whisper = message.whisper; let reply_preview = message.reply_preview.clone(); @@ -854,7 +854,8 @@ pub fn MessageView( // members registry so `@handle` resolves in the row. // The display-name → handle derivation mirrors // `views::compute_messages_view` (see there for the - // `profile-card.md` TODO). + // profile-card plan TODO — + // `docs/plans/2026-04-21-ui-phase-2c-profile-card.md`). use leptos::context::use_context; let app_state = use_context::(); let local_peer_str = app_state @@ -863,10 +864,10 @@ pub fn MessageView( .unwrap_or_default(); let local_peer: Option = local_peer_str.parse().ok(); - // TODO(profile-card.md): use real handles when profile - // data is plumbed. For now handle ≈ display-name - // lowercased with spaces → dots, matching the - // client-side projection. + // TODO(plan: docs/plans/2026-04-21-ui-phase-2c-profile-card.md): + // use real handles when profile data is plumbed. For + // now handle ≈ display-name lowercased with spaces → + // dots, matching the client-side projection. let peers_vec: Vec = app_state .as_ref() .map(|a| { @@ -1122,10 +1123,10 @@ pub fn MessageView( class="toolbar-btn" type="button" aria-label="whisper reply" - // TODO(whisper-mode.md): permission-gated; no-op - // until `WhisperStart` EventKind lands and the - // local peer has permission to send a whisper - // reply to this row's author. + // TODO(#562): permission-gated; no-op until + // `WhisperStart` EventKind lands and the local + // peer has permission to send a whisper reply + // to this row's author. on:click=move |ev| { ev.stop_propagation(); } > {icons::icon_ear()} @@ -1340,9 +1341,9 @@ pub fn MessageView( on:touchend=on_sheet_touchend > // Quick-emoji row — six hit targets from recency. - // TODO(reactions-pins.md): swap `REACTION_EMOJI` - // for the channel-scoped recency list once that - // spec lands. Rendered first so the sheet opens + // TODO(#564): swap `REACTION_EMOJI` for the + // channel-scoped recency list once that spec + // lands. Rendered first so the sheet opens // with the common case one tap away. {if has_react { let cb = react_cb2; @@ -1403,10 +1404,10 @@ pub fn MessageView( // will route there instead. {has_react.then(|| view! { })} diff --git a/crates/web/src/components/message_row/mention.rs b/crates/web/src/components/message_row/mention.rs index ad037414..595711c8 100644 --- a/crates/web/src/components/message_row/mention.rs +++ b/crates/web/src/components/message_row/mention.rs @@ -41,7 +41,8 @@ pub fn MentionPill( }; let title = full_label.unwrap_or_else(|| label.clone()); let aria = format!("mention {title}"); - // TODO(profile-card.md): open the profile popover on click. + // TODO(plan: docs/plans/2026-04-21-ui-phase-2c-profile-card.md): + // open the profile popover on click. view! {