From 037b973793fbd1a0f333ca283f88e800cc769d6e Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Sat, 6 Jun 2026 13:39:38 +1000 Subject: [PATCH] fix(serverless): resolve replaceable events to latest copy; publish agent profile Membership/metadata reads used limit:1 + first(), picking an arbitrary kind:39002/39000 copy when relays disagreed. A stale copy on one relay (e.g. a dropped write) could be read and then clobber members on the next read-modify-write. Add latest_event/latest_by_d_tag (max created_at, id tie-break) and apply at all four read sites; drop the limit truncation in get_channels. Agents only published presence (kind:20001), never a kind:0 profile, so clients showed a raw hex pubkey. Add publish_profile (name/display_name/ about/picture) at startup, sourced from the persona display_name with a title-cased fallback from the agent command (goose -> Goose). Tests: membership latest-wins resolution + profile metadata/name fallback. --- crates/sprout-acp/src/config.rs | 151 +++++++++++--- crates/sprout-acp/src/lib.rs | 109 ++++++++++ desktop/src-tauri/src/commands/channels.rs | 103 ++++++++-- .../src-tauri/src/commands/channels_tests.rs | 192 +++++++++++++++++- 4 files changed, 499 insertions(+), 56 deletions(-) diff --git a/crates/sprout-acp/src/config.rs b/crates/sprout-acp/src/config.rs index dcaddf728..7fa174e47 100644 --- a/crates/sprout-acp/src/config.rs +++ b/crates/sprout-acp/src/config.rs @@ -478,6 +478,13 @@ pub struct Config { /// Per-persona env vars to inject at agent spawn time (e.g., GOOSE_PROVIDER, GOOSE_MODEL). /// Populated from persona pack resolution. Empty when no pack is configured. pub persona_env_vars: Vec<(String, String)>, + /// Agent profile (NIP-01 kind:0 metadata) published at startup so clients + /// render a name/avatar instead of raw hex pubkey. Sourced from the resolved + /// persona (`display_name`, `description`, `avatar`); `None` when no persona + /// pack is configured, in which case no profile is published. + pub profile_name: Option, + pub profile_about: Option, + pub profile_picture: Option, /// Whether to publish encrypted observer frames through the relay. pub relay_observer: bool, /// Agent owner pubkey (hex). Used for `--respond-to=owner-only` gate. @@ -582,6 +589,27 @@ pub fn propagate_legacy_env_vars() { } } +/// Derive a fallback agent profile name from the agent command, so an agent +/// launched without a persona still publishes a readable kind:0 name instead of +/// a bare hex pubkey. Strips any path/extension and title-cases the first +/// letter: `"goose"` → `"Goose"`, `"/usr/bin/claude"` → `"Claude"`. Returns +/// `None` only for an empty/whitespace command. +fn default_profile_name_from_command(agent_command: &str) -> Option { + let base = std::path::Path::new(agent_command) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(agent_command) + .trim(); + if base.is_empty() { + return None; + } + let mut chars = base.chars(); + Some(match chars.next() { + Some(first) => first.to_uppercase().collect::() + chars.as_str(), + None => base.to_string(), + }) +} + impl Config { pub fn from_cli() -> Result { // Legacy env-var propagation is intentionally NOT done here. @@ -751,43 +779,61 @@ impl Config { // // Precedence: CLI/env args > persona values > built-in defaults. // Persona fills in what's missing. Explicit flags always win. - let (persona_system_prompt, persona_model, persona_env_vars) = - match (&args.persona_pack, &args.persona_name) { - (Some(pack_dir), Some(name)) => { - let pack = sprout_persona::resolve::resolve_pack(pack_dir).map_err(|e| { + #[allow(clippy::type_complexity)] + let ( + persona_system_prompt, + persona_model, + persona_env_vars, + persona_display_name, + persona_about, + persona_avatar, + ) = match (&args.persona_pack, &args.persona_name) { + (Some(pack_dir), Some(name)) => { + let pack = sprout_persona::resolve::resolve_pack(pack_dir).map_err(|e| { + ConfigError::ConfigFile(format!( + "failed to resolve pack {}: {e}", + pack_dir.display() + )) + })?; + let persona = pack + .personas + .into_iter() + .find(|p| p.name == *name) + .ok_or_else(|| { ConfigError::ConfigFile(format!( - "failed to resolve pack {}: {e}", + "persona '{name}' not found in pack {}", pack_dir.display() )) })?; - let persona = pack - .personas - .into_iter() - .find(|p| p.name == *name) - .ok_or_else(|| { - ConfigError::ConfigFile(format!( - "persona '{name}' not found in pack {}", - pack_dir.display() - )) - })?; - ( - Some(persona.system_prompt), - persona.model, - persona.goose_env_vars, - ) - } - (Some(_), None) => { - return Err(ConfigError::ConfigFile( - "--persona-pack requires --persona-name".into(), - )); - } - (None, Some(_)) => { - return Err(ConfigError::ConfigFile( - "--persona-name requires --persona-pack".into(), - )); - } - (None, None) => (None, None, vec![]), - }; + // Profile name: prefer the human display_name, fall back to slug. + let display_name = if persona.display_name.trim().is_empty() { + persona.name.clone() + } else { + persona.display_name.clone() + }; + let about = + (!persona.description.trim().is_empty()).then(|| persona.description.clone()); + ( + Some(persona.system_prompt), + persona.model, + persona.goose_env_vars, + Some(display_name), + about, + persona.avatar, + ) + } + (Some(_), None) => { + return Err(ConfigError::ConfigFile( + "--persona-pack requires --persona-name".into(), + )); + } + (None, Some(_)) => { + return Err(ConfigError::ConfigFile( + "--persona-name requires --persona-pack".into(), + )); + } + (None, None) => (None, None, vec![], None, None, None), + }; // Apply persona defaults: CLI/env wins, persona fills gaps. if system_prompt.is_none() { @@ -818,6 +864,13 @@ impl Config { // and an HTTP-bridge channel-discovery crash. let serverless = args.serverless || args.relay_url.contains(','); + // Agent profile name: persona display_name if set, else a title-cased + // fallback derived from the agent command (e.g. "goose" → "Goose") so a + // bare agent still publishes a kind:0 profile and shows a name rather + // than a raw hex pubkey in clients. + let profile_name = + persona_display_name.or_else(|| default_profile_name_from_command(&agent_command)); + let config = Config { keys, relay_url: args.relay_url, @@ -850,6 +903,9 @@ impl Config { respond_to: args.respond_to, respond_to_allowlist, persona_env_vars, + profile_name, + profile_about: persona_about, + profile_picture: persona_avatar, relay_observer: args.relay_observer, agent_owner: args.agent_owner.map(|s| s.trim().to_ascii_lowercase()), no_base_prompt: args.no_base_prompt, @@ -1217,6 +1273,9 @@ mod tests { respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), persona_env_vars: vec![], + profile_name: None, + profile_about: None, + profile_picture: None, relay_observer: false, agent_owner: None, no_base_prompt: false, @@ -1246,6 +1305,32 @@ mod tests { // ── resolve_channel_filters: Mentions mode ─────────────────────────────── + #[test] + fn profile_name_fallback_title_cases_command() { + assert_eq!( + default_profile_name_from_command("goose"), + Some("Goose".to_string()) + ); + } + + #[test] + fn profile_name_fallback_strips_path_and_extension() { + assert_eq!( + default_profile_name_from_command("/usr/local/bin/claude"), + Some("Claude".to_string()) + ); + assert_eq!( + default_profile_name_from_command("agent.sh"), + Some("Agent".to_string()) + ); + } + + #[test] + fn profile_name_fallback_empty_is_none() { + assert_eq!(default_profile_name_from_command(""), None); + assert_eq!(default_profile_name_from_command(" "), None); + } + #[test] fn test_mentions_mode_default_kinds() { let config = test_config(SubscribeMode::Mentions); diff --git a/crates/sprout-acp/src/lib.rs b/crates/sprout-acp/src/lib.rs index 60b460359..95b97b637 100644 --- a/crates/sprout-acp/src/lib.rs +++ b/crates/sprout-acp/src/lib.rs @@ -81,6 +81,55 @@ async fn publish_presence( Ok(()) } +/// Publish the agent's NIP-01 kind:0 profile metadata so clients render a name +/// and avatar instead of the raw hex pubkey. +/// +/// Without this, an agent only ever announces presence (kind:20001) and never +/// tells the network who it is, so every client shows it as `2b5f20…`. We +/// publish on startup (and it is replaceable, so re-publishing on each launch +/// keeps name/avatar current). `name` is required; `about`/`picture` are +/// included only when present. +async fn publish_profile( + publisher: &relay::RelayEventPublisher, + keys: &nostr::Keys, + name: &str, + about: Option<&str>, + picture: Option<&str>, +) -> Result<(), relay::RelayError> { + use nostr::{EventBuilder, Kind}; + + let content = build_profile_metadata_json(name, about, picture); + let event = EventBuilder::new(Kind::Metadata, content) + .tags([]) + .sign_with_keys(keys) + .map_err(|e| relay::RelayError::Http(format!("profile sign error: {e}")))?; + publisher.publish_event(event).await?; + Ok(()) +} + +/// Build the NIP-01 kind:0 metadata JSON content for an agent profile. +/// +/// Sets both `name` and `display_name` (clients read either), and includes +/// `about`/`picture` only when non-empty. +fn build_profile_metadata_json(name: &str, about: Option<&str>, picture: Option<&str>) -> String { + let mut meta = serde_json::Map::new(); + meta.insert("name".into(), serde_json::Value::String(name.to_string())); + meta.insert( + "display_name".into(), + serde_json::Value::String(name.to_string()), + ); + if let Some(about) = about.filter(|s| !s.trim().is_empty()) { + meta.insert("about".into(), serde_json::Value::String(about.to_string())); + } + if let Some(picture) = picture.filter(|s| !s.trim().is_empty()) { + meta.insert( + "picture".into(), + serde_json::Value::String(picture.to_string()), + ); + } + serde_json::Value::Object(meta).to_string() +} + // ── Owner resolution ────────────────────────────────────────────────────────── /// Resolve the agent's owner pubkey at startup. @@ -948,6 +997,26 @@ async fn tokio_main() -> Result<()> { } } + // ── Step 2c.2: Publish agent profile (kind:0) ───────────────────────────── + // So clients show the agent's name/avatar instead of a raw hex pubkey. + // Only when a persona supplied a name (no persona → no profile to publish). + if let Some(name) = config.profile_name.as_deref() { + match publish_profile( + &presence_publisher, + &presence_keys, + name, + config.profile_about.as_deref(), + config.profile_picture.as_deref(), + ) + .await + { + Ok(_) => tracing::info!("published agent profile (name={name})"), + Err(e) => tracing::warn!("failed to publish agent profile: {e}"), + } + } else { + tracing::debug!("no persona display name; skipping kind:0 profile publish"); + } + // ── Step 2d: Resolve agent owner ──────────────────────────────────────── // Priority: SPROUT_AUTH_TAG (NIP-OA attestation) → --agent-owner flag. let startup_owner: Option = resolve_agent_owner(&config); @@ -2735,6 +2804,43 @@ mod owner_cache_tests { } } +#[cfg(test)] +mod profile_metadata_tests { + use super::*; + + #[test] + fn includes_name_and_display_name() { + let json = build_profile_metadata_json("Goose", None, None); + let v: serde_json::Value = serde_json::from_str(&json).expect("valid json"); + assert_eq!(v["name"], "Goose"); + assert_eq!(v["display_name"], "Goose"); + // about/picture omitted when not supplied. + assert!(v.get("about").is_none()); + assert!(v.get("picture").is_none()); + } + + #[test] + fn includes_about_and_picture_when_present() { + let json = build_profile_metadata_json( + "Sami", + Some("A helpful agent"), + Some("https://example.com/a.png"), + ); + let v: serde_json::Value = serde_json::from_str(&json).expect("valid json"); + assert_eq!(v["name"], "Sami"); + assert_eq!(v["about"], "A helpful agent"); + assert_eq!(v["picture"], "https://example.com/a.png"); + } + + #[test] + fn omits_blank_about_and_picture() { + let json = build_profile_metadata_json("Goose", Some(" "), Some("")); + let v: serde_json::Value = serde_json::from_str(&json).expect("valid json"); + assert!(v.get("about").is_none(), "blank about must be omitted"); + assert!(v.get("picture").is_none(), "blank picture must be omitted"); + } +} + #[cfg(test)] mod observer_chunk_coalescer_tests { use super::*; @@ -2872,6 +2978,9 @@ mod build_mcp_servers_tests { respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), persona_env_vars: vec![], + profile_name: None, + profile_about: None, + profile_picture: None, relay_observer: false, agent_owner: None, no_base_prompt: false, diff --git a/desktop/src-tauri/src/commands/channels.rs b/desktop/src-tauri/src/commands/channels.rs index 04e8de968..0fc50f194 100644 --- a/desktop/src-tauri/src/commands/channels.rs +++ b/desktop/src-tauri/src/commands/channels.rs @@ -16,6 +16,59 @@ use crate::{ // the current list, add/remove the pubkey, and re-publish the whole event. // See docs/SPROUT_LITE_MODE.md. +/// Resolve an addressable/replaceable event to its single authoritative copy. +/// +/// kind:39002 (membership) and kind:39000 (metadata) are NIP-01 addressable +/// events: exactly one *logical* event exists per `(kind, author, d-tag)`, and +/// re-publishing supersedes the prior version. In a multi-relay world, however, +/// relays can disagree — a write may land on relays A and B but be dropped +/// (rate-limited, offline) by relay C, leaving C with a **stale** older copy. +/// A query that fans out and merges then sees *both* versions. +/// +/// The correct resolution per NIP-01 is "latest wins": pick the event with the +/// greatest `created_at` (tie-break deterministically by event id). Picking an +/// arbitrary event (e.g. `first()` after a `limit: 1` merge) is a bug — it makes +/// the member list non-deterministic and, worse, makes read-modify-write +/// membership updates clobber members that only exist in the newer copy. +/// +/// Returns `None` for an empty slice. +fn latest_event(events: &[nostr::Event]) -> Option<&nostr::Event> { + events.iter().max_by(|a, b| { + a.created_at + .as_secs() + .cmp(&b.created_at.as_secs()) + // Deterministic tie-break when two copies share a timestamp. + .then_with(|| a.id.to_hex().cmp(&b.id.to_hex())) + }) +} + +/// Group a batch of addressable events by `d`-tag and keep only the latest copy +/// of each (see [`latest_event`] for why "latest" not "arbitrary"). Events +/// without a `d` tag are skipped. +fn latest_by_d_tag(events: &[nostr::Event]) -> std::collections::HashMap { + let mut by_d: std::collections::HashMap = + std::collections::HashMap::new(); + for ev in events { + let Some(d) = ev.tags.iter().find_map(|t| { + let s = t.as_slice(); + (s.len() >= 2 && s[0] == "d").then(|| s[1].clone()) + }) else { + continue; + }; + by_d.entry(d) + .and_modify(|cur| { + let newer = ev.created_at.as_secs() > cur.created_at.as_secs() + || (ev.created_at.as_secs() == cur.created_at.as_secs() + && ev.id.to_hex() > cur.id.to_hex()); + if newer { + *cur = ev; + } + }) + .or_insert(ev); + } + by_d +} + /// Fetch the current members `(pubkey, role)` for a serverless channel from its /// kind:39002 event. Role is the 4th element of the `p` tag (NIP-29), defaulting /// to `member`. Returns an empty list if no members event exists yet. @@ -23,17 +76,20 @@ async fn serverless_current_members( state: &AppState, channel_id: &str, ) -> Result, String> { + // No `limit: 1`: relays can hold divergent copies of this replaceable + // event (a write dropped by one relay leaves it stale). Fetch all copies + // and resolve to the latest by `created_at` — otherwise a read-modify-write + // membership update can be based on a stale list and silently drop members. let events = query_relay( state, &[serde_json::json!({ "kinds": [39002], "#d": [channel_id], - "limit": 1 })], ) .await?; - let Some(ev) = events.first() else { + let Some(ev) = latest_event(&events) else { return Ok(Vec::new()); }; let members = ev @@ -135,23 +191,28 @@ pub async fn get_channels(state: State<'_, AppState>) -> Result channel_ids.dedup(); // Step 2: fetch channel metadata events (kind:39000) for member channels. - // kind:39000 is addressable: exactly one event per `d` tag, so a limit - // equal to the number of ids is both necessary and sufficient. Without - // an explicit limit, multi-value `#d` filters fall through to the relay's - // default LIMIT and can drop results when there are many channels. - let meta_events = if !channel_ids.is_empty() { + // kind:39000 is addressable: one logical event per `d` tag. We do NOT cap + // the limit at `channel_ids.len()`: across multiple relays each channel can + // return several copies (fresh + stale), so a tight limit can truncate and + // drop the fresh copy of one channel while keeping a stale copy of another. + // Over-fetch, then resolve to the latest copy per `d` tag below. + let meta_events_raw = if !channel_ids.is_empty() { query_relay( &state, &[serde_json::json!({ "kinds": [39000], "#d": channel_ids, - "limit": channel_ids.len(), })], ) .await? } else { Vec::new() }; + // Deduplicate to the latest copy per channel (relays may disagree). + let meta_events: Vec = latest_by_d_tag(&meta_events_raw) + .into_values() + .cloned() + .collect(); // Step 3: fetch open channel metadata so the channel browser can show // discoverable channels the user hasn't joined yet. The relay's access @@ -259,15 +320,13 @@ struct ChannelMembership { fn collect_members_by_channel( events: &[nostr::Event], ) -> std::collections::HashMap { + // Resolve each channel's `d`-tag to its latest 39002 across relays first; + // a naive per-event insert would let a stale copy overwrite the fresh one + // depending on iteration order. + let latest = latest_by_d_tag(events); let mut map: std::collections::HashMap = - std::collections::HashMap::with_capacity(events.len()); - for ev in events { - let Some(d) = ev.tags.iter().find_map(|t| { - let s = t.as_slice(); - (s.len() >= 2 && s[0] == "d").then(|| s[1].clone()) - }) else { - continue; - }; + std::collections::HashMap::with_capacity(latest.len()); + for (d, ev) in latest { let Ok(resp) = nostr_convert::channel_members_from_event(ev) else { continue; }; @@ -288,18 +347,17 @@ pub async fn get_channel_details( channel_id: String, state: State<'_, AppState>, ) -> Result { + // Resolve the latest copy across relays (kind:39000 is replaceable too). let events = query_relay( &state, &[serde_json::json!({ "kinds": [39000], "#d": [channel_id], - "limit": 1 })], ) .await?; - events - .first() + latest_event(&events) .map(nostr_convert::channel_detail_from_event) .transpose()? .ok_or_else(|| "channel not found".to_string()) @@ -310,18 +368,19 @@ pub async fn get_channel_members( channel_id: String, state: State<'_, AppState>, ) -> Result { + // Fetch all copies and resolve the latest (relays may disagree); see + // `latest_event`. A stale `limit: 1` pick would hide members added by a + // write that one relay dropped. let events = query_relay( &state, &[serde_json::json!({ "kinds": [39002], "#d": [channel_id], - "limit": 1 })], ) .await?; - let mut response = events - .first() + let mut response = latest_event(&events) .map(nostr_convert::channel_members_from_event) .transpose()? .ok_or_else(|| "channel members not found".to_string())?; diff --git a/desktop/src-tauri/src/commands/channels_tests.rs b/desktop/src-tauri/src/commands/channels_tests.rs index 129a0de0e..078f2f15a 100644 --- a/desktop/src-tauri/src/commands/channels_tests.rs +++ b/desktop/src-tauri/src/commands/channels_tests.rs @@ -2,7 +2,7 @@ // channels.rs under the per-file line cap. use super::*; -use nostr::{EventBuilder, Keys, Kind, Tag}; +use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; /// Build a signed event for testing with the given kind, content, and tags. fn ev(kind: u16, content: &str, tags: Vec>) -> nostr::Event { @@ -17,6 +17,22 @@ fn ev(kind: u16, content: &str, tags: Vec>) -> nostr::Event { .expect("sign") } +/// Like [`ev`] but with an explicit `created_at` and a caller-supplied signing +/// key — so a test can produce multiple versions of the *same* addressable +/// event (same author + d-tag) with different timestamps to exercise +/// "latest wins" resolution. +fn ev_at(keys: &Keys, kind: u16, created_at: u64, tags: Vec>) -> nostr::Event { + let parsed: Vec = tags + .into_iter() + .map(|t| Tag::parse(t).expect("parse tag")) + .collect(); + EventBuilder::new(Kind::from_u16(kind), "") + .tags(parsed) + .custom_created_at(Timestamp::from(created_at)) + .sign_with_keys(keys) + .expect("sign") +} + // A 64-hex pubkey (nostr p-tags require 32-byte hex). const PK_A: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const PK_B: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; @@ -496,3 +512,177 @@ async fn serverless_live_subscription_multi_relay() { state.relay_pool.unsubscribe(&sub_id).await; } + +// ── Replaceable-event "latest wins" resolution ─────────────────────────────── +// +// Regression coverage for the multi-relay membership bug: when relays hold +// divergent copies of a replaceable kind:39002 (one relay dropped a write and +// kept a stale copy), the client must resolve to the LATEST copy by +// `created_at`. Picking an arbitrary copy made the member list flicker and made +// read-modify-write membership updates silently clobber members. + +const PK_BOT: &str = "2b5f20697e34f75726f567dcc6657b7ca4a10afc5d341ec50f34d91fd3014874"; + +#[test] +fn latest_event_picks_newest_by_created_at() { + let keys = Keys::generate(); + // Stale copy: 2 members. Fresh copy: 3 members (bot added later). + let stale = ev_at( + &keys, + 39002, + 1000, + vec![ + vec!["d", "chan"], + vec!["p", PK_A, "", "member"], + vec!["p", PK_B, "", "member"], + ], + ); + let fresh = ev_at( + &keys, + 39002, + 2000, + vec![ + vec!["d", "chan"], + vec!["p", PK_A, "", "member"], + vec!["p", PK_B, "", "member"], + vec!["p", PK_BOT, "", "member"], + ], + ); + + // Whatever order relays return them in, the newest must win. + for events in [ + vec![stale.clone(), fresh.clone()], + vec![fresh.clone(), stale.clone()], + ] { + let chosen = latest_event(&events).expect("some event"); + assert_eq!( + chosen.id, fresh.id, + "must pick the newest copy regardless of input order" + ); + let resp = nostr_convert::channel_members_from_event(chosen).expect("parse members"); + let pks: Vec<&str> = resp.members.iter().map(|m| m.pubkey.as_str()).collect(); + assert!( + pks.contains(&PK_BOT), + "bot added in the newer copy must be present, got {pks:?}" + ); + assert_eq!(resp.members.len(), 3); + } +} + +#[test] +fn latest_event_empty_is_none() { + assert!(latest_event(&[]).is_none()); +} + +#[test] +fn latest_event_tie_break_is_deterministic() { + // Two copies with the SAME created_at must resolve deterministically + // (highest event id), not by chance/order. + let k1 = Keys::generate(); + let k2 = Keys::generate(); + let a = ev_at( + &k1, + 39002, + 5000, + vec![vec!["d", "chan"], vec!["p", PK_A, "", "member"]], + ); + let b = ev_at( + &k2, + 39002, + 5000, + vec![vec!["d", "chan"], vec!["p", PK_B, "", "member"]], + ); + let expected = if a.id.to_hex() > b.id.to_hex() { + a.id + } else { + b.id + }; + assert_eq!(latest_event(&[a.clone(), b.clone()]).unwrap().id, expected); + assert_eq!(latest_event(&[b, a]).unwrap().id, expected); +} + +#[test] +fn latest_by_d_tag_resolves_each_channel_independently() { + let k = Keys::generate(); + // chan-1: stale (1 member) + fresh (2 members). chan-2: single copy. + let c1_stale = ev_at( + &k, + 39002, + 1000, + vec![vec!["d", "chan-1"], vec!["p", PK_A, "", "member"]], + ); + let c1_fresh = ev_at( + &k, + 39002, + 2000, + vec![ + vec!["d", "chan-1"], + vec!["p", PK_A, "", "member"], + vec!["p", PK_BOT, "", "member"], + ], + ); + let c2 = ev_at( + &k, + 39002, + 1500, + vec![vec!["d", "chan-2"], vec!["p", PK_C, "", "member"]], + ); + + let events = [c1_stale, c2.clone(), c1_fresh.clone()]; + let map = latest_by_d_tag(&events); + assert_eq!(map.len(), 2); + assert_eq!( + map.get("chan-1").unwrap().id, + c1_fresh.id, + "chan-1 must be the fresh copy" + ); + assert_eq!(map.get("chan-2").unwrap().id, c2.id); +} + +#[test] +fn latest_by_d_tag_skips_events_without_d_tag() { + let k = Keys::generate(); + let no_d = ev_at(&k, 39002, 1000, vec![vec!["p", PK_A, "", "member"]]); + let with_d = ev_at( + &k, + 39002, + 1000, + vec![vec!["d", "chan"], vec!["p", PK_B, "", "member"]], + ); + let events = [no_d, with_d.clone()]; + let map = latest_by_d_tag(&events); + assert_eq!(map.len(), 1); + assert_eq!(map.get("chan").unwrap().id, with_d.id); +} + +#[test] +fn collect_members_by_channel_uses_latest_copy() { + // The batch path (channel browser member counts) must also resolve to the + // latest copy — a stale copy must not undercount members. + let k = Keys::generate(); + let stale = ev_at( + &k, + 39002, + 1000, + vec![vec!["d", "chan-x"], vec!["p", PK_A, "", "member"]], + ); + let fresh = ev_at( + &k, + 39002, + 2000, + vec![ + vec!["d", "chan-x"], + vec!["p", PK_A, "", "member"], + vec!["p", PK_BOT, "", "member"], + ], + ); + // Stale listed AFTER fresh — naive last-write-wins iteration would have + // picked the stale one; latest_by_d_tag must still choose fresh. + let map = collect_members_by_channel(&[fresh, stale]); + let info = map.get("chan-x").expect("channel present"); + assert_eq!( + info.count, 2, + "must count the fresh 2-member copy, not stale 1-member" + ); + assert!(info.pubkeys.iter().any(|p| p == PK_BOT)); +}