diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 8a698954a0..700d5e8dcf 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -619,29 +619,46 @@ impl AcpClient { /// Send `session/new` and return the full response alongside the session ID. /// /// `cwd` must be an absolute path. `mcp_servers` may be empty. - /// `system_prompt` is included in the request when `Some` — agents that - /// support the field will use it; others ignore unknown fields per JSON-RPC. + /// + /// `system_prompt` controls how the prompt text is delivered: + /// + /// - `None` — no system-prompt field in the request (legacy framing). + /// - `Some(SystemPromptTransport::Field(text))` — bare `systemPrompt` field + /// (ACP protocol v2, buzz-agent, goose unused). + /// - `Some(SystemPromptTransport::ClaudeMeta(text))` — `_meta.systemPrompt` + /// as `{"append": text}`, keeping claude-agent-acp's native preset intact. + /// /// `session_title` rides in `_meta.sessionTitle` when `Some`; `_meta` is /// omitted entirely otherwise, since adapters may distinguish an absent - /// member from a null one. + /// member from a null one. When both `ClaudeMeta` and `session_title` are + /// present the two `_meta` members are merged into a single object. + /// /// Callers use [`extract_model_config_options`] and [`extract_model_state`] /// to pull model info from the raw result. pub async fn session_new_full( &mut self, cwd: &str, mcp_servers: Vec, - system_prompt: Option<&str>, + system_prompt: Option>, session_title: Option<&str>, ) -> Result { let mut params = serde_json::json!({ "cwd": cwd, "mcpServers": mcp_servers, }); - if let Some(sp) = system_prompt { - params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); + match system_prompt { + Some(SystemPromptTransport::Field(sp)) => { + params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); + } + Some(SystemPromptTransport::ClaudeMeta(sp)) => { + // Merge into _meta so sessionTitle (set below) is not clobbered. + params["_meta"]["systemPrompt"] = serde_json::json!({ "append": sp }); + } + None => {} } if let Some(title) = session_title { - params["_meta"] = serde_json::json!({ "sessionTitle": title }); + // Merge — _meta may already carry systemPrompt from ClaudeMeta above. + params["_meta"]["sessionTitle"] = serde_json::Value::String(title.to_owned()); } let result = self.send_request("session/new", params).await?; let session_id = result["sessionId"] @@ -663,7 +680,7 @@ impl AcpClient { &mut self, cwd: &str, mcp_servers: Vec, - system_prompt: Option<&str>, + system_prompt: Option>, session_title: Option<&str>, ) -> Result { Ok(self @@ -2038,6 +2055,22 @@ pub struct SessionNewResponse { pub raw: serde_json::Value, } +/// How to deliver a system prompt on `session/new`. +/// +/// The two variants match the two mechanisms supported by current adapters: +/// +/// - **`Field`** — bare `systemPrompt` field (ACP protocol v2, buzz-agent). +/// - **`ClaudeMeta`** — `_meta.systemPrompt: {"append": text}`, used by +/// `claude-agent-acp` to append to the adapter's own native system prompt +/// while keeping its tool-use preset intact. +#[derive(Debug, Clone, PartialEq)] +pub enum SystemPromptTransport<'a> { + /// Deliver as a bare top-level `systemPrompt` field. + Field(&'a str), + /// Deliver as `_meta.systemPrompt: {"append": text}`. + ClaudeMeta(&'a str), +} + /// How to switch to a particular model on a session. #[derive(Debug, Clone, PartialEq, serde::Serialize)] #[serde(tag = "type")] @@ -3271,7 +3304,12 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], Some("Custom system prompt"), None) + .session_new_full( + "/tmp", + vec![], + Some(SystemPromptTransport::Field("Custom system prompt")), + None, + ) .await .expect("session_new_full should succeed"); @@ -3423,6 +3461,87 @@ mod tests { ); } + // ── claude-agent-acp _meta.systemPrompt transport ───────────────────── + + #[tokio::test] + async fn session_new_full_sends_claude_meta_system_prompt_when_claude_meta_transport() { + // When ClaudeMeta transport is requested, the prompt must appear as + // _meta.systemPrompt: {"append": text} — never as a bare systemPrompt field. + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_claude","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full( + "/tmp", + vec![], + Some(SystemPromptTransport::ClaudeMeta("Be concise")), + None, + ) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert!( + received["params"].get("systemPrompt").is_none(), + "bare systemPrompt must not be present for ClaudeMeta transport" + ); + assert_eq!( + received["params"]["_meta"]["systemPrompt"]["append"].as_str(), + Some("Be concise"), + "_meta.systemPrompt.append must carry the prompt text" + ); + } + + #[tokio::test] + async fn session_new_full_merges_claude_meta_and_session_title_into_single_meta_object() { + // Both ClaudeMeta prompt and session_title must coexist under _meta — + // the prompt must not clobber sessionTitle or vice versa. + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_merged","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full( + "/tmp", + vec![], + Some(SystemPromptTransport::ClaudeMeta("Be concise")), + Some("Fizz · #buzz-dev"), + ) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert_eq!( + received["params"]["_meta"]["systemPrompt"]["append"].as_str(), + Some("Be concise"), + "_meta.systemPrompt.append must be present" + ); + assert_eq!( + received["params"]["_meta"]["sessionTitle"].as_str(), + Some("Fizz · #buzz-dev"), + "_meta.sessionTitle must be present alongside systemPrompt" + ); + } + // ── Goose-native steer scaffold (PR follow-up to #1160) ────────────── /// Helper: spawn an inert `cat` subprocess so we have a real AcpClient diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 348bc138e4..64edf68ee2 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -32,6 +32,7 @@ use uuid::Uuid; use crate::acp::{ extract_model_config_options, extract_model_state, model_in_catalog, resolve_model_switch_method, AcpClient, AcpError, McpServer, ModelSwitchMethod, StopReason, + SystemPromptTransport, }; use crate::config::{compose_session_title, DedupMode, PermissionMode}; use crate::observer; @@ -171,6 +172,13 @@ pub struct OwnedAgent { pub protocol_version: u32, } +/// Package name reported by `claude-agent-acp` in its `initialize` response. +/// Any adapter reporting this name supports `_meta.systemPrompt: {append: ...}` +/// on `session/new` — the feature landed in v0.6.0 (Oct 2025), before the +/// `@zed-industries/claude-code-acp` → `@agentclientprotocol/claude-agent-acp` +/// rename, so the new name is a reliable capability gate. +const CLAUDE_AGENT_ACP_NAME: &str = "@agentclientprotocol/claude-agent-acp"; + fn has_system_prompt_support( protocol_version: u32, agent_name: &str, @@ -178,20 +186,25 @@ fn has_system_prompt_support( ) -> bool { if agent_name == "goose" { goose_system_prompt_supported == Some(true) + } else if agent_name == CLAUDE_AGENT_ACP_NAME { + true } else { protocol_version >= 2 } } -fn session_new_system_prompt( +fn session_new_system_prompt<'a>( is_goose: bool, protocol_version: u32, - prompt: Option<&str>, -) -> Option<&str> { - if is_goose || protocol_version < 2 { + agent_name: &str, + prompt: Option<&'a str>, +) -> Option> { + if is_goose || (protocol_version < 2 && agent_name != CLAUDE_AGENT_ACP_NAME) { None + } else if agent_name == CLAUDE_AGENT_ACP_NAME { + prompt.map(SystemPromptTransport::ClaudeMeta) } else { - prompt + prompt.map(SystemPromptTransport::Field) } } @@ -907,6 +920,7 @@ async fn create_session_and_apply_model( session_new_system_prompt( is_goose, agent.protocol_version, + &agent.agent_name, combined_system_prompt.as_deref(), ), session_title.as_deref(), @@ -4003,18 +4017,48 @@ mod tests { assert!(has_system_prompt_support(2, "goose", Some(true))); assert!(has_system_prompt_support(1, "goose", Some(true))); assert!(has_system_prompt_support(2, "buzz-agent", None)); + // Goose never receives system prompt via session/new (uses post-hoc method). assert_eq!( - session_new_system_prompt(true, 2, Some("instructions")), + session_new_system_prompt(true, 2, "goose", Some("instructions")), None ); + // Protocol-v2 non-goose gets Field transport. assert_eq!( - session_new_system_prompt(false, 2, Some("instructions")), - Some("instructions") + session_new_system_prompt(false, 2, "buzz-agent", Some("instructions")), + Some(SystemPromptTransport::Field("instructions")) ); + // Protocol-v1 non-goose, non-claude gets None (legacy user-message framing). assert_eq!( - session_new_system_prompt(false, 1, Some("instructions")), + session_new_system_prompt(false, 1, "codex", Some("instructions")), None ); + // claude-agent-acp gets ClaudeMeta transport regardless of protocol version. + assert_eq!( + session_new_system_prompt(false, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions")), + Some(SystemPromptTransport::ClaudeMeta("instructions")) + ); + assert_eq!( + session_new_system_prompt(true, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions")), + None, + "goose path must never produce a transport even when agent_name matches" + ); + } + + #[test] + fn claude_agent_acp_has_system_prompt_support_regardless_of_protocol_version() { + // claude-agent-acp declares protocolVersion:1 but supports _meta.systemPrompt; + // has_system_prompt_support must return true so user-message framing is suppressed. + assert!(has_system_prompt_support(1, CLAUDE_AGENT_ACP_NAME, None)); + assert!(has_system_prompt_support(2, CLAUDE_AGENT_ACP_NAME, None)); + } + + #[test] + fn old_zed_adapter_name_falls_through_to_protocol_version_gate() { + // The renamed @zed-industries package predates the _meta.systemPrompt support, + // so it must not be treated as capable and stays on legacy user-message framing. + let old_name = "@zed-industries/claude-code-acp"; + assert!(!has_system_prompt_support(1, old_name, None)); + assert!(has_system_prompt_support(2, old_name, None)); } #[test] diff --git a/crates/buzz-cli/src/commands/mod.rs b/crates/buzz-cli/src/commands/mod.rs index 8691590636..1ccc37a702 100644 --- a/crates/buzz-cli/src/commands/mod.rs +++ b/crates/buzz-cli/src/commands/mod.rs @@ -12,9 +12,37 @@ pub mod notes; pub mod pack; pub mod patches; pub mod pr; +pub mod projects; pub mod reactions; pub mod repos; pub mod social; pub mod upload; pub mod users; pub mod workflows; + +use crate::{client::normalize_write_response, error::CliError}; + +/// Parse a relay write-response JSON blob, mapping a duplicate (dominated) +/// write to [`CliError::Conflict`] with the caller-supplied message. +/// +/// Used by every command that publishes an NIP-33 addressable event and +/// needs to tell accepted from duplicate/dominated. +pub fn parse_write_response(raw: &str, conflict_msg: &str) -> Result { + let response: serde_json::Value = serde_json::from_str(raw) + .map_err(|e| CliError::Other(format!("relay response is not JSON: {e} ({raw})")))?; + let accepted = response + .get("accepted") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + let message = response + .get("message") + .and_then(serde_json::Value::as_str) + .unwrap_or(""); + if !accepted { + return Err(CliError::Other(format!("relay rejected event: {message}"))); + } + if message == "duplicate" || message.starts_with("duplicate:") { + return Err(CliError::Conflict(conflict_msg.to_string())); + } + Ok(normalize_write_response(raw)) +} diff --git a/crates/buzz-cli/src/commands/projects.rs b/crates/buzz-cli/src/commands/projects.rs new file mode 100644 index 0000000000..e6798dbfc4 --- /dev/null +++ b/crates/buzz-cli/src/commands/projects.rs @@ -0,0 +1,1198 @@ +//! `buzz projects` commands — NIP-MP kind:30621 write path. +//! +//! All mutations follow a read-modify-write pattern: +//! 1. Fetch the caller's own live head via `kinds:[30621] + authors:[self] + #d:[slug]`. +//! 2. Mutate the tag set (strip `auth`, apply change). +//! 3. Re-validate the full envelope through Layer A before submitting. +//! 4. Set `created_at = head.created_at + 1` (never wall-clock) to avoid +//! overwriting a concurrently advancing head. +//! +//! Limitations recorded in this phase: +//! - Relay hints are read-preserved but not authored (`--repo` carries +//! a coordinate only; existing hinted tags survive RMW unchanged). +//! - `delete` targets signer-self only (NIP-OA owner-delete path deferred). +//! - Deletion durability against later arrival (watermark follow-up) is +//! not in scope. + +use buzz_core::kind::KIND_PROJECT; +use buzz_sdk::{ + build_delete_addressable, build_project, build_project_with_tags, ProjectMemberCoord, + PROJECT_D_MAX_LEN, +}; +use nostr::{Event, EventBuilder, Tag, Timestamp}; + +use crate::client::BuzzClient; +use crate::commands::parse_write_response; +use crate::error::CliError; + +// ── Buzz repo-ID grammar (bare --repo shorthand) ───────────────────────────── + +/// Pattern for a Buzz-hosted repo identifier (bare `--repo` shorthand). +/// `[a-zA-Z0-9._-]{1,64}` — no colons, so guaranteed collision-free with +/// `30617::` full coordinates. +fn is_bare_repo_id(s: &str) -> bool { + !s.is_empty() + && s.len() <= 64 + && s.chars() + .all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '_' || c == '-') +} + +/// Expand a CLI `--repo` argument into a full `30617::` coordinate. +/// +/// Bare form (`[a-zA-Z0-9._-]{1,64}`): owner defaults to the caller's pubkey. +/// Full form (`30617::`): used verbatim. +fn expand_repo_coord(s: &str, caller_pubkey: &str) -> Result { + if is_bare_repo_id(s) { + // Bare form: expand to full coordinate with caller as owner. + let full = format!("30617:{caller_pubkey}:{s}"); + ProjectMemberCoord::parse_full(&full) + .map_err(|e| CliError::Usage(format!("invalid repo coordinate: {e}"))) + } else { + // Full form: must be parseable as a complete coordinate. + ProjectMemberCoord::parse_full(s) + .map_err(|e| CliError::Usage(format!("invalid repo coordinate: {e}"))) + } +} + +// ── Head-fetch helper ───────────────────────────────────────────────────────── + +fn parse_events(json: &str) -> Result, CliError> { + serde_json::from_str(json) + .map_err(|e| CliError::Other(format!("failed to parse relay response: {e}"))) +} + +/// Fetch the caller's own live kind:30621 head for `slug`. +async fn fetch_own_project(client: &BuzzClient, slug: &str) -> Result, CliError> { + fetch_project(client, slug, None).await +} + +/// Fetch a project head by slug and optional owner pubkey. +async fn fetch_project( + client: &BuzzClient, + slug: &str, + owner: Option<&str>, +) -> Result, CliError> { + let pubkey = match owner { + Some(pk) => { + crate::validate::validate_hex64(pk)?; + pk.to_string() + } + None => client.keys().public_key().to_hex(), + }; + let filter = serde_json::json!({ + "kinds": [KIND_PROJECT], + "authors": [pubkey], + "#d": [slug], + "limit": 1, + }); + let raw = client.query(&filter).await?; + let mut events = parse_events(&raw)?; + events.sort_by_key(|e| std::cmp::Reverse(e.created_at)); + Ok(events.into_iter().next()) +} + +// ── Tag helpers ─────────────────────────────────────────────────────────────── + +fn tag_name(tag: &Tag) -> Option<&str> { + tag.as_slice().first().map(String::as_str) +} + +fn tag_value(tag: &Tag) -> Option<&str> { + tag.as_slice().get(1).map(String::as_str) +} + +fn make_tag(parts: &[&str]) -> Result { + Tag::parse(parts.iter().copied()) + .map_err(|e| CliError::Other(format!("tag construction failed: {e}"))) +} + +// ── Submit helper ───────────────────────────────────────────────────────────── + +async fn submit_project(client: &BuzzClient, builder: EventBuilder) -> Result<(), CliError> { + let event = client.sign_event(builder)?; + let raw = client.submit_event(event).await?; + println!( + "{}", + parse_write_response(&raw, "project changed concurrently; retry")? + ); + Ok(()) +} + +// ── Build helpers ───────────────────────────────────────────────────────────── + +/// Advance the `created_at` counter off an observed head. +fn next_timestamp(head: &Event) -> Result { + head.created_at + .as_secs() + .checked_add(1) + .map(Timestamp::from) + .ok_or_else(|| CliError::Other("project timestamp cannot be advanced".into())) +} + +/// Strip `auth` from a tag list and pass the resulting envelope through +/// Layer A validation. Returns a validated `EventBuilder` at `next_ts`. +fn rebuild_project( + content: &str, + tags: Vec, + next_ts: Timestamp, +) -> Result { + // Strip auth tags. + let clean_tags: Vec = tags + .into_iter() + .filter(|t| tag_name(t) != Some("auth")) + .collect(); + + build_project_with_tags(content, clean_tags) + .map_err(|e| CliError::Other(format!("envelope validation failed: {e}"))) + .map(|b| b.custom_created_at(next_ts)) +} + +// ── Command implementations ─────────────────────────────────────────────────── + +/// `buzz projects create` +pub async fn cmd_create( + client: &BuzzClient, + slug: &str, + repos: &[String], + name: Option<&str>, + description: Option<&str>, + channel: Option<&str>, + visibility: Option<&str>, +) -> Result<(), CliError> { + // ── Local validation (all checks before any .await) ─────────────────── + validate_project_slug(slug)?; + + let caller_pubkey = client.keys().public_key().to_hex(); + + // Expand and validate repo coordinates. + let members: Vec = repos + .iter() + .map(|r| expand_repo_coord(r, &caller_pubkey)) + .collect::, _>>()?; + + // Dedupe: preserve first occurrence, reject duplicates with Usage. + let mut seen = std::collections::HashSet::new(); + for m in &members { + if !seen.insert(m.coord.clone()) { + return Err(CliError::Usage(format!( + "duplicate --repo coordinate in this invocation: {:?}", + m.coord + ))); + } + } + + // Validate optional metadata (early, before any network call). + if let Some(ch) = channel { + crate::validate::validate_uuid(ch)?; + } + if let Some(vis) = visibility { + validate_visibility(vis)?; + } + if let Some(n) = name { + if n.len() > 256 { + return Err(CliError::Usage(format!( + "project name must not exceed 256 bytes (got {})", + n.len() + ))); + } + } + + // ── Network: collision preflight ────────────────────────────────────── + if fetch_own_project(client, slug).await?.is_some() { + return Err(CliError::Conflict(format!( + "project {slug:?} already exists; use 'buzz projects update' to modify it" + ))); + } + + // ── Build via Layer B (enforces all writer policy) ──────────────────── + let builder = build_project(slug, name, description, &members, channel, visibility) + .map_err(|e| CliError::Usage(e.to_string()))?; + submit_project(client, builder).await +} + +/// `buzz projects get` +pub async fn cmd_get(client: &BuzzClient, slug: &str, owner: Option<&str>) -> Result<(), CliError> { + validate_project_slug(slug)?; + let resp = match fetch_project(client, slug, owner).await? { + Some(event) => serde_json::json!({ + "event_id": event.id.to_hex(), + "pubkey": event.pubkey.to_hex(), + "created_at": event.created_at.as_secs(), + "kind": event.kind.as_u16(), + "tags": event.tags.iter().map(|t| t.as_slice().to_vec()).collect::>(), + "content": event.content, + }), + None => { + let owner_desc = owner.unwrap_or("current identity"); + return Err(CliError::NotFound(format!( + "project {slug:?} not found for {owner_desc}" + ))); + } + }; + println!("{resp}"); + Ok(()) +} + +/// `buzz projects list` +pub async fn cmd_list( + client: &BuzzClient, + owner: Option<&str>, + limit: Option, +) -> Result<(), CliError> { + let pubkey = match owner { + Some(pk) => { + crate::validate::validate_hex64(pk)?; + pk.to_string() + } + None => client.keys().public_key().to_hex(), + }; + let mut filter = serde_json::json!({ + "kinds": [KIND_PROJECT], + "authors": [pubkey], + }); + if let Some(n) = limit { + filter["limit"] = serde_json::json!(n); + } + let resp = client.query(&filter).await?; + println!("{resp}"); + Ok(()) +} + +/// `buzz projects add-repo` +pub async fn cmd_add_repo( + client: &BuzzClient, + slug: &str, + repos: &[String], +) -> Result<(), CliError> { + validate_project_slug(slug)?; + let caller_pubkey = client.keys().public_key().to_hex(); + + // ── Local validation before any .await ──────────────────────────────── + let new_members: Vec = repos + .iter() + .map(|r| expand_repo_coord(r, &caller_pubkey)) + .collect::, _>>()?; + + // Dedupe within this invocation: first occurrence wins, duplicate → Usage. + let mut seen = std::collections::HashSet::new(); + for m in &new_members { + if !seen.insert(m.coord.clone()) { + return Err(CliError::Usage(format!( + "duplicate --repo coordinate in this invocation: {:?}", + m.coord + ))); + } + } + + // ── Network: fetch head ─────────────────────────────────────────────── + let head = fetch_own_project(client, slug) + .await? + .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; + let next_ts = next_timestamp(&head)?; + + // Build the new tag set: keep existing tags (including hinted members), + // append new members only if not already present (by coordinate). + let mut tags: Vec = head.tags.iter().cloned().collect(); + let existing_coords: std::collections::HashSet = head + .tags + .iter() + .filter(|t| tag_name(t) == Some("a")) + .filter_map(|t| tag_value(t).map(String::from)) + .collect(); + let mut added = 0usize; + for m in &new_members { + if !existing_coords.contains(m.coord.as_str()) { + let parts = m.to_tag_parts(); + let parts_ref: Vec<&str> = parts.iter().map(String::as_str).collect(); + tags.push( + Tag::parse(parts_ref.iter().copied()) + .map_err(|e| CliError::Other(format!("member tag construction failed: {e}")))?, + ); + added += 1; + } + } + + // All requested coordinates were already present — no change to publish. + if added == 0 { + return Err(CliError::Conflict(format!( + "all requested repositories are already members of project {slug:?}" + ))); + } + + let builder = rebuild_project(&head.content, tags, next_ts)?; + submit_project(client, builder).await +} + +/// `buzz projects remove-repo` +pub async fn cmd_remove_repo( + client: &BuzzClient, + slug: &str, + repos: &[String], +) -> Result<(), CliError> { + validate_project_slug(slug)?; + let caller_pubkey = client.keys().public_key().to_hex(); + + // ── Local validation before any .await ──────────────────────────────── + let to_remove: Vec = repos + .iter() + .map(|r| expand_repo_coord(r, &caller_pubkey)) + .collect::, _>>()?; + + // ── Network: fetch head ─────────────────────────────────────────────── + let head = fetch_own_project(client, slug) + .await? + .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; + let next_ts = next_timestamp(&head)?; + + // Verify all requested repos exist in the project. + let existing_coords: std::collections::HashSet = head + .tags + .iter() + .filter(|t| tag_name(t) == Some("a")) + .filter_map(|t| tag_value(t).map(String::from)) + .collect(); + for m in &to_remove { + if !existing_coords.contains(m.coord.as_str()) { + return Err(CliError::NotFound(format!( + "project {slug:?} does not contain member {:?}", + m.coord + ))); + } + } + + let remove_coords: std::collections::HashSet<&str> = + to_remove.iter().map(|m| m.coord.as_str()).collect(); + + // Keep all tags except auth and the removed members. + let tags: Vec = head + .tags + .iter() + .filter(|t| { + if tag_name(t) == Some("auth") { + return false; + } + if tag_name(t) == Some("a") { + if let Some(coord) = tag_value(t) { + return !remove_coords.contains(coord); + } + } + true + }) + .cloned() + .collect(); + + // Single rebuild validates the full envelope and strips any remaining auth. + let builder = rebuild_project(&head.content, tags, next_ts)?; + submit_project(client, builder).await +} + +/// `buzz projects update` +/// +/// Requires at least one setter or clearer; a no-op call is a usage error. +#[allow(clippy::too_many_arguments)] +pub async fn cmd_update( + client: &BuzzClient, + slug: &str, + name: Option<&str>, + clear_name: bool, + description: Option<&str>, + clear_description: bool, + channel: Option<&str>, + clear_channel: bool, + visibility: Option<&str>, + clear_visibility: bool, +) -> Result<(), CliError> { + // Guard: at least one mutation required. The clap `ArgGroup` with + // `required(true).multiple(true)` enforces this at parse time; this + // runtime check is a defense-in-depth safety net for callers that invoke + // `cmd_update` directly (e.g. tests and future programmatic callers). + let has_mutation = name.is_some() + || clear_name + || description.is_some() + || clear_description + || channel.is_some() + || clear_channel + || visibility.is_some() + || clear_visibility; + if !has_mutation { + return Err(CliError::Usage( + "buzz projects update requires at least one of: \ + --name, --clear-name, --description, --clear-description, \ + --channel, --clear-channel, --visibility, --clear-visibility" + .into(), + )); + } + + validate_project_slug(slug)?; + if let Some(ch) = channel { + crate::validate::validate_uuid(ch)?; + } + if let Some(vis) = visibility { + validate_visibility(vis)?; + } + + let head = fetch_own_project(client, slug) + .await? + .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; + let next_ts = next_timestamp(&head)?; + + // Build the new tag set. For each singleton metadata field: + // - setter present: replace value (strip old, append new) + // - clear flag set: drop the tag + // - neither: keep existing + // Non-singleton / non-metadata tags (d, a, unknown) are preserved as-is. + let singleton_fields = ["name", "description", "buzz-channel", "buzz-visibility"]; + let mut tags: Vec = head + .tags + .iter() + .filter(|t| { + if tag_name(t) == Some("auth") { + return false; + } + // Drop singletons we're replacing or clearing. + if let Some(field) = tag_name(t) { + if singleton_fields.contains(&field) { + let clear = match field { + "name" => clear_name || name.is_some(), + "description" => clear_description || description.is_some(), + "buzz-channel" => clear_channel || channel.is_some(), + "buzz-visibility" => clear_visibility || visibility.is_some(), + _ => false, + }; + return !clear; + } + } + true + }) + .cloned() + .collect(); + + // Append new singleton values. + if let Some(n) = name { + tags.push(make_tag(&["name", n])?); + } + if let Some(d) = description { + tags.push(make_tag(&["description", d])?); + } + if let Some(ch) = channel { + tags.push(make_tag(&["buzz-channel", ch])?); + } + if let Some(vis) = visibility { + tags.push(make_tag(&["buzz-visibility", vis])?); + } + + let builder = build_project_with_tags(&head.content, tags) + .map_err(|e| CliError::Other(format!("envelope validation failed: {e}")))? + .custom_created_at(next_ts); + submit_project(client, builder).await +} + +/// `buzz projects delete` +/// +/// Head-based and verified: +/// 1. Fetch own live head — `NotFound` if absent. +/// 2. Build tombstone at `head.created_at + 1`. +/// 3. Submit. +/// 4. Re-query the coordinate; if a newer head survived → `Conflict`. +pub async fn cmd_delete(client: &BuzzClient, slug: &str) -> Result<(), CliError> { + validate_project_slug(slug)?; + + let head = fetch_own_project(client, slug) + .await? + .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; + let next_ts = next_timestamp(&head)?; + + let pubkey_hex = client.keys().public_key().to_hex(); + let tombstone = build_delete_addressable(KIND_PROJECT, &pubkey_hex, slug) + .map_err(|e| CliError::Other(format!("failed to build delete event: {e}")))? + .custom_created_at(next_ts); + + let event = client.sign_event(tombstone)?; + let raw = client.submit_event(event).await?; + parse_write_response(&raw, "delete event was dominated; a newer head exists")?; + + // Post-submit verification: re-query to confirm the head is gone. + if let Some(survivor) = fetch_own_project(client, slug).await? { + // A newer head survived the tombstone. + return Err(CliError::Conflict(format!( + "project {slug:?} still exists (head at {}); a concurrent write raced the delete", + survivor.created_at.as_secs() + ))); + } + + println!("{}", serde_json::json!({ "deleted": slug, "status": "ok" })); + Ok(()) +} + +// ── Validation helpers ──────────────────────────────────────────────────────── + +/// Validate a project slug: non-empty, ≤1024 bytes, verbatim. +/// Does NOT impose the Buzz repo-ID grammar — project slugs are more permissive. +fn validate_project_slug(slug: &str) -> Result<(), CliError> { + if slug.is_empty() { + return Err(CliError::Usage("project slug must not be empty".into())); + } + if slug.len() > PROJECT_D_MAX_LEN { + return Err(CliError::Usage(format!( + "project slug must not exceed {PROJECT_D_MAX_LEN} bytes (got {})", + slug.len() + ))); + } + Ok(()) +} + +/// Validate a `buzz-visibility` value at the writer level. +fn validate_visibility(vis: &str) -> Result<(), CliError> { + if vis != "listed" && vis != "unlisted" { + return Err(CliError::Usage(format!( + "visibility must be 'listed' or 'unlisted' (got {vis:?})" + ))); + } + Ok(()) +} + +// ── Dispatch ────────────────────────────────────────────────────────────────── + +pub async fn dispatch(cmd: crate::ProjectsCmd, client: &BuzzClient) -> Result<(), CliError> { + use crate::ProjectsCmd; + match cmd { + ProjectsCmd::Create { + slug, + repo, + name, + description, + channel, + visibility, + } => { + cmd_create( + client, + &slug, + &repo, + name.as_deref(), + description.as_deref(), + channel.as_deref(), + visibility.map(|v| v.as_str()), + ) + .await + } + ProjectsCmd::Get { slug, owner } => cmd_get(client, &slug, owner.as_deref()).await, + ProjectsCmd::List { owner, limit } => cmd_list(client, owner.as_deref(), limit).await, + ProjectsCmd::AddRepo { slug, repo } => cmd_add_repo(client, &slug, &repo).await, + ProjectsCmd::RemoveRepo { slug, repo } => cmd_remove_repo(client, &slug, &repo).await, + ProjectsCmd::Update { + slug, + name, + clear_name, + description, + clear_description, + channel, + clear_channel, + visibility, + clear_visibility, + } => { + cmd_update( + client, + &slug, + name.as_deref(), + clear_name, + description.as_deref(), + clear_description, + channel.as_deref(), + clear_channel, + visibility.map(|v| v.as_str()), + clear_visibility, + ) + .await + } + ProjectsCmd::Delete { slug } => cmd_delete(client, &slug).await, + } +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use buzz_sdk::{validate_project_envelope, PROJECT_MEMBER_CAP}; + use nostr::Tag; + + use super::*; + + // ── Coordinate expansion ────────────────────────────────────────────────── + + const OWNER_HEX: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const OWNER_B_HEX: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + + #[test] + fn expand_repo_coord_bare_expands_with_caller_pubkey() { + let coord = expand_repo_coord("my-repo", OWNER_HEX).unwrap(); + assert_eq!(coord.coord, format!("30617:{OWNER_HEX}:my-repo")); + } + + #[test] + fn expand_repo_coord_full_passes_through() { + let full = format!("30617:{OWNER_HEX}:some-repo"); + let coord = expand_repo_coord(&full, OWNER_B_HEX).unwrap(); + // Owner from the full coord, not the caller. + assert_eq!(coord.coord, full); + } + + #[test] + fn expand_repo_coord_full_cross_owner() { + let full = format!("30617:{OWNER_B_HEX}:infra"); + let coord = expand_repo_coord(&full, OWNER_HEX).unwrap(); + assert_eq!(coord.coord, full); + } + + #[test] + fn expand_repo_coord_rejects_uppercase_owner() { + let upper = "30617:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:buzz"; + assert!(expand_repo_coord(upper, OWNER_HEX).is_err()); + } + + #[test] + fn expand_repo_coord_rejects_coordinate_shaped_bare_value() { + // A value with a colon is never a bare id. + let not_bare = "30617:something"; + // parse_full will fail because it's not a valid full coordinate either. + assert!(expand_repo_coord(not_bare, OWNER_HEX).is_err()); + } + + // ── validate_project_slug ───────────────────────────────────────────────── + + #[test] + fn validate_project_slug_accepts_normal() { + assert!(validate_project_slug("my-project").is_ok()); + assert!(validate_project_slug("platform:v2").is_ok()); // colons allowed — more permissive than repo-id + } + + #[test] + fn validate_project_slug_rejects_empty() { + assert!(validate_project_slug("").is_err()); + } + + #[test] + fn validate_project_slug_rejects_over_1024() { + let long = "a".repeat(1025); + assert!(validate_project_slug(&long).is_err()); + } + + #[test] + fn validate_project_slug_accepts_1024() { + let at_limit = "a".repeat(1024); + assert!(validate_project_slug(&at_limit).is_ok()); + } + + // ── validate_visibility ─────────────────────────────────────────────────── + + #[test] + fn validate_visibility_accepts_listed_and_unlisted() { + assert!(validate_visibility("listed").is_ok()); + assert!(validate_visibility("unlisted").is_ok()); + } + + #[test] + fn validate_visibility_rejects_unknown_token() { + assert!(validate_visibility("chartreuse").is_err()); + assert!(validate_visibility("").is_err()); + } + + // ── is_bare_repo_id ─────────────────────────────────────────────────────── + + #[test] + fn bare_repo_id_accepts_valid() { + assert!(is_bare_repo_id("buzz")); + assert!(is_bare_repo_id("my-repo_1.0")); + } + + #[test] + fn bare_repo_id_rejects_colon() { + assert!(!is_bare_repo_id("30617:something")); + assert!(!is_bare_repo_id("has:colon")); + } + + #[test] + fn bare_repo_id_rejects_empty() { + assert!(!is_bare_repo_id("")); + } + + #[test] + fn bare_repo_id_rejects_over_64() { + let long = "a".repeat(65); + assert!(!is_bare_repo_id(&long)); + } + + // ── tag helpers ─────────────────────────────────────────────────────────── + + fn make_test_tag(parts: &[&str]) -> Tag { + Tag::parse(parts.iter().copied()).unwrap() + } + + // ── rebuild_project: hinted / unknown tag preservation ─────────────────── + + #[test] + fn rebuild_project_preserves_hinted_member_tags() { + // A member 'a' tag with a relay hint must survive RMW untouched. + let coord = format!("30617:{OWNER_HEX}:buzz"); + let hint = "wss://relay.example.com"; + let tags = vec![ + make_test_tag(&["d", "platform"]), + Tag::parse(["a", &coord, hint]).unwrap(), + ]; + let ts = Timestamp::from(1_700_000_001u64); + let b = rebuild_project("", tags, ts).unwrap(); + let ev = b.sign_with_keys(&nostr::Keys::generate()).expect("sign"); + let a_tag = ev + .tags + .iter() + .find(|t| tag_name(t) == Some("a")) + .expect("a tag present"); + assert_eq!( + a_tag.as_slice(), + &["a".to_string(), coord, hint.to_string()], + "relay hint must survive rebuild" + ); + } + + #[test] + fn rebuild_project_preserves_unknown_tags() { + let tags = vec![ + make_test_tag(&["d", "platform"]), + make_test_tag(&["future-metadata", "value"]), + ]; + let ts = Timestamp::from(1_700_000_001u64); + let b = rebuild_project("", tags, ts).unwrap(); + let ev = b.sign_with_keys(&nostr::Keys::generate()).expect("sign"); + assert!(ev + .tags + .iter() + .any(|t| tag_name(t) == Some("future-metadata"))); + } + + #[test] + fn rebuild_project_strips_auth_tag() { + let tags = vec![ + make_test_tag(&["d", "platform"]), + make_test_tag(&["auth", &"a".repeat(64), "kind=30617", &"b".repeat(128)]), + ]; + let ts = Timestamp::from(1_700_000_001u64); + let b = rebuild_project("", tags, ts).unwrap(); + let ev = b.sign_with_keys(&nostr::Keys::generate()).expect("sign"); + assert!( + !ev.tags.iter().any(|t| tag_name(t) == Some("auth")), + "auth tag must be stripped" + ); + } + + #[test] + fn rebuild_project_rejects_over_cap_foreign_head() { + // A foreign head with 65 members must fail Layer A on republish. + let mut tags = vec![make_test_tag(&["d", "wide"])]; + for i in 0..=64u32 { + let coord = format!("30617:{OWNER_HEX}:repo-{i:02}"); + tags.push(make_test_tag(&["a", &coord])); + } + assert_eq!( + tags.iter().filter(|t| tag_name(t) == Some("a")).count(), + 65, + "65 a-tags" + ); + let ts = Timestamp::from(1_700_000_001u64); + // rebuild_project strips auth, but 65 a-tags still exceeds cap. + assert!( + rebuild_project("", tags, ts).is_err(), + "over-cap foreign head must fail rebuild" + ); + } + + #[test] + fn rebuild_project_at_exact_cap_succeeds() { + let mut tags = vec![make_test_tag(&["d", "wide"])]; + for i in 0..PROJECT_MEMBER_CAP { + let coord = format!("30617:{OWNER_HEX}:repo-{i:02}"); + tags.push(make_test_tag(&["a", &coord])); + } + let ts = Timestamp::from(1_700_000_001u64); + assert!(rebuild_project("", tags, ts).is_ok()); + } + + // ── clear-flag semantics ────────────────────────────────────────────────── + + /// Build a minimal head Event for testing update semantics without the relay. + fn make_head_tags(extra: &[Tag]) -> Vec { + let mut tags = vec![make_test_tag(&["d", "platform"])]; + tags.extend_from_slice(extra); + tags + } + + #[allow(clippy::too_many_arguments)] + fn apply_update_tags( + head_tags: Vec, + name: Option<&str>, + clear_name: bool, + description: Option<&str>, + clear_description: bool, + channel: Option<&str>, + clear_channel: bool, + visibility: Option<&str>, + clear_visibility: bool, + ) -> Vec { + // Replicate the tag-mutation logic from cmd_update (sans relay I/O). + let singleton_fields = ["name", "description", "buzz-channel", "buzz-visibility"]; + let mut tags: Vec = head_tags + .iter() + .filter(|t| { + if tag_name(t) == Some("auth") { + return false; + } + if let Some(field) = tag_name(t) { + if singleton_fields.contains(&field) { + let clear = match field { + "name" => clear_name || name.is_some(), + "description" => clear_description || description.is_some(), + "buzz-channel" => clear_channel || channel.is_some(), + "buzz-visibility" => clear_visibility || visibility.is_some(), + _ => false, + }; + return !clear; + } + } + true + }) + .cloned() + .collect(); + if let Some(n) = name { + tags.push(make_test_tag(&["name", n])); + } + if let Some(d) = description { + tags.push(make_test_tag(&["description", d])); + } + if let Some(ch) = channel { + tags.push(make_test_tag(&["buzz-channel", ch])); + } + if let Some(vis) = visibility { + tags.push(make_test_tag(&["buzz-visibility", vis])); + } + tags + } + + #[test] + fn update_omission_preserves_existing_field() { + let head = make_head_tags(&[make_test_tag(&["name", "Old Name"])]); + let result = apply_update_tags(head, None, false, None, false, None, false, None, false); + assert!(result.iter().any(|t| tag_value(t) == Some("Old Name"))); + } + + #[test] + fn update_setter_replaces_existing_field() { + let head = make_head_tags(&[make_test_tag(&["name", "Old Name"])]); + let result = apply_update_tags( + head, + Some("New Name"), + false, + None, + false, + None, + false, + None, + false, + ); + assert!(result.iter().any(|t| tag_value(t) == Some("New Name"))); + assert!(!result.iter().any(|t| tag_value(t) == Some("Old Name"))); + } + + #[test] + fn update_clear_drops_existing_field() { + let head = make_head_tags(&[make_test_tag(&["name", "Old Name"])]); + let result = apply_update_tags(head, None, true, None, false, None, false, None, false); + assert!(!result.iter().any(|t| tag_name(t) == Some("name"))); + } + + #[test] + fn update_clear_visibility_drops_tag() { + let head = make_head_tags(&[make_test_tag(&["buzz-visibility", "unlisted"])]); + let result = apply_update_tags(head, None, false, None, false, None, false, None, true); + assert!(!result + .iter() + .any(|t| tag_name(t) == Some("buzz-visibility"))); + } + + #[test] + fn update_exactly_one_singleton_after_replace() { + // Start with a buzz-channel; replace with a new one; must have exactly one. + let uuid1 = "3580ca9b-47b4-4af9-b22a-1068778f26c6"; + let uuid2 = "00000000-0000-0000-0000-000000000000"; + let head = make_head_tags(&[make_test_tag(&["buzz-channel", uuid1])]); + let result = apply_update_tags( + head, + None, + false, + None, + false, + Some(uuid2), + false, + None, + false, + ); + let channels: Vec<_> = result + .iter() + .filter(|t| tag_name(t) == Some("buzz-channel")) + .collect(); + assert_eq!(channels.len(), 1); + assert_eq!(tag_value(channels[0]), Some(uuid2)); + } + + // ── duplicate-member rejection on republish ─────────────────────────────── + + #[test] + fn duplicate_member_in_foreign_head_fails_rebuild() { + let coord = format!("30617:{OWNER_HEX}:buzz"); + let tags = vec![ + make_test_tag(&["d", "platform"]), + make_test_tag(&["a", &coord]), + make_test_tag(&["a", &coord]), // duplicate + ]; + let ts = Timestamp::from(1_700_000_001u64); + assert!(rebuild_project("", tags, ts).is_err()); + } + + // ── validate_project_envelope integration ──────────────────────────────── + + #[test] + fn validate_project_envelope_accepts_hinted_member() { + let coord = format!("30617:{OWNER_HEX}:buzz"); + let tags = vec![ + make_test_tag(&["d", "platform"]), + Tag::parse(["a", &coord, "wss://relay.example.com"]).unwrap(), + ]; + assert!(validate_project_envelope(&tags, "").is_ok()); + } + + #[test] + fn validate_project_envelope_rejects_four_element_member() { + let coord = format!("30617:{OWNER_HEX}:buzz"); + let tags = vec![ + make_test_tag(&["d", "platform"]), + Tag::parse(["a", &coord, "wss://relay.example.com", "extra"]).unwrap(), + ]; + assert!(validate_project_envelope(&tags, "").is_err()); + } + + // ── next_timestamp ordering ─────────────────────────────────────────────── + + /// `next_timestamp` must return `head.created_at + 1` regardless of the wall + /// clock. NIP-MP Deletion rule: a tombstone older than the live head does + /// NOT remove it, so we must advance strictly off the observed head — never + /// use wall-clock time, which could be behind a head that was bumped + /// multiple times in the same second. + #[test] + fn next_timestamp_returns_head_plus_one_when_head_is_ahead_of_wall_clock() { + // Build a minimal signed event with a created_at far in the future. + let keys = nostr::Keys::generate(); + let far_future_ts = Timestamp::from(9_999_999_999u64); // year 2286 + let tags = vec![ + make_test_tag(&["d", "platform"]), + make_test_tag(&["a", &format!("30617:{OWNER_HEX}:buzz")]), + ]; + let builder = rebuild_project("", tags, far_future_ts).expect("valid head envelope"); + let head = builder.sign_with_keys(&keys).expect("sign"); + // Verify the event actually has our future timestamp. + assert_eq!(head.created_at, far_future_ts); + + // next_timestamp must return far_future + 1, not now(). + let next = next_timestamp(&head).expect("no overflow"); + assert_eq!( + next.as_secs(), + far_future_ts.as_secs() + 1, + "tombstone must be strictly after head, even when head is far in the future" + ); + } + + // ── empty update guard ──────────────────────────────────────────────────── + + /// `cmd_update` with no setters or clearers must return `CliError::Usage` + /// before making any network call. The guard is synchronous (before the + /// first `.await`) so we can drive it with a dummy client whose address + /// would reject any real connection attempt. + #[tokio::test] + async fn empty_update_returns_usage_error_before_any_network_call() { + let keys = nostr::Keys::generate(); + // Port 9 is the discard protocol — any real connect will be refused + // immediately, but the guard fires before the first await so this + // never reaches the network. + let client = crate::client::BuzzClient::new("http://127.0.0.1:9".into(), keys, None, None) + .expect("client construction"); + + let err = cmd_update( + &client, "my-slug", None, false, // name / clear_name + None, false, // description / clear_description + None, false, // channel / clear_channel + None, false, // visibility / clear_visibility + ) + .await + .expect_err("empty update must fail"); + + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage, got {err:?}" + ); + } + + // ── no-network malformed-input tests ───────────────────────────────────── + // + // All three cases use port 9 (discard protocol): any real connection is + // refused immediately, but local validation fires before the first .await + // so the network is never touched. + + fn discard_client() -> crate::client::BuzzClient { + let keys = nostr::Keys::generate(); + crate::client::BuzzClient::new("http://127.0.0.1:9".into(), keys, None, None) + .expect("client construction") + } + + /// Invalid visibility token must return Usage before touching the relay. + #[tokio::test] + async fn create_invalid_visibility_returns_usage_before_any_network_call() { + let client = discard_client(); + let err = cmd_create( + &client, + "my-slug", + &["buzz".to_string()], + None, + None, + None, + Some("chartreuse"), + ) + .await + .expect_err("invalid visibility must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for invalid visibility, got {err:?}" + ); + } + + /// A name longer than 256 bytes must return Usage before touching the relay. + #[tokio::test] + async fn create_overlong_name_returns_usage_before_any_network_call() { + let client = discard_client(); + let long_name = "a".repeat(257); + let err = cmd_create( + &client, + "my-slug", + &["buzz".to_string()], + Some(&long_name), + None, + None, + None, + ) + .await + .expect_err("overlong name must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for overlong name, got {err:?}" + ); + } + + /// A malformed --repo coordinate must return Usage before touching the relay. + #[tokio::test] + async fn create_malformed_repo_returns_usage_before_any_network_call() { + let client = discard_client(); + let err = cmd_create( + &client, + "my-slug", + &["nope:bad".to_string()], + None, + None, + None, + None, + ) + .await + .expect_err("malformed repo must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for malformed repo, got {err:?}" + ); + } + + /// A malformed --repo coordinate on add-repo must return Usage before touching the relay. + #[tokio::test] + async fn add_repo_malformed_coord_returns_usage_before_any_network_call() { + let client = discard_client(); + let err = cmd_add_repo(&client, "my-slug", &["nope:bad".to_string()]) + .await + .expect_err("malformed repo must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for malformed repo on add-repo, got {err:?}" + ); + } + + /// A malformed --repo coordinate on remove-repo must return Usage before touching the relay. + #[tokio::test] + async fn remove_repo_malformed_coord_returns_usage_before_any_network_call() { + let client = discard_client(); + let err = cmd_remove_repo(&client, "my-slug", &["nope:bad".to_string()]) + .await + .expect_err("malformed repo must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for malformed repo on remove-repo, got {err:?}" + ); + } + + // ── duplicate --repo within one invocation ──────────────────────────────── + + /// Supplying the same coordinate twice in one create call must return Usage + /// (names the duplicate) before any network call. + #[tokio::test] + async fn create_duplicate_repo_returns_usage_before_any_network_call() { + let client = discard_client(); + let coord = format!("30617:{OWNER_HEX}:buzz"); + let err = cmd_create( + &client, + "my-slug", + &[coord.clone(), coord.clone()], + None, + None, + None, + None, + ) + .await + .expect_err("duplicate repo must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for duplicate repo, got {err:?}" + ); + // Error message must name the duplicate coordinate. + assert!( + format!("{err}").contains("buzz"), + "Usage message must name the duplicate coordinate, got {err:?}" + ); + } + + /// Supplying the same coordinate twice in one add-repo call must return Usage + /// (names the duplicate) before any network call. + #[tokio::test] + async fn add_repo_duplicate_coord_returns_usage_before_any_network_call() { + let client = discard_client(); + let coord = format!("30617:{OWNER_HEX}:buzz"); + let err = cmd_add_repo(&client, "my-slug", &[coord.clone(), coord.clone()]) + .await + .expect_err("duplicate repo must fail"); + assert!( + matches!(err, CliError::Usage(_)), + "expected CliError::Usage for duplicate repo on add-repo, got {err:?}" + ); + } + + // ── create collision guard ──────────────────────────────────────────────── + + // The create-collision Conflict path is pinned by the live transcript + // (step: duplicate create → Conflict, exit=5). No relay mock is available + // for a unit test; the no-network tests above cover all pre-await paths. + + // ── add-repo no-op guard ────────────────────────────────────────────────── + + // The add-repo no-op Conflict path is pinned by the live transcript + // (step 7: buzz already present → exit=5). No relay mock is available + // for a unit test; the async no-network tests above cover all pre-await paths. +} diff --git a/crates/buzz-cli/src/commands/repos.rs b/crates/buzz-cli/src/commands/repos.rs index 608d495055..15e064d9c3 100644 --- a/crates/buzz-cli/src/commands/repos.rs +++ b/crates/buzz-cli/src/commands/repos.rs @@ -4,7 +4,8 @@ use buzz_core::{ }; use nostr::{Event, EventBuilder, Tag, Timestamp}; -use crate::client::{normalize_write_response, BuzzClient}; +use crate::client::BuzzClient; +use crate::commands::parse_write_response; use crate::error::CliError; use crate::validate::validate_repo_id; @@ -186,25 +187,10 @@ fn protection_rules_json(event: &Event) -> Result { } fn validate_write_response(raw: &str) -> Result { - let response: serde_json::Value = serde_json::from_str(raw) - .map_err(|error| CliError::Other(format!("relay response is not JSON: {error} ({raw})")))?; - let accepted = response - .get("accepted") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - let message = response - .get("message") - .and_then(serde_json::Value::as_str) - .unwrap_or(""); - if !accepted { - return Err(CliError::Other(format!("relay rejected event: {message}"))); - } - if message == "duplicate" || message.starts_with("duplicate:") { - return Err(CliError::Conflict( - "repository changed concurrently; fetch the latest rules and retry".into(), - )); - } - Ok(normalize_write_response(raw)) + parse_write_response( + raw, + "repository changed concurrently; fetch the latest rules and retry", + ) } async fn submit_repo_update(client: &BuzzClient, builder: EventBuilder) -> Result<(), CliError> { diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index 6227d6287b..b0985abf62 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -212,6 +212,9 @@ enum Cmd { /// Announce and discover git repositories (NIP-34) #[command(subcommand)] Repos(ReposCmd), + /// Create and manage multi-repo projects (NIP-MP) + #[command(subcommand)] + Projects(ProjectsCmd), /// Send, get, list, and set status on git patches (NIP-34) #[command(subcommand)] Patches(PatchesCmd), @@ -1255,6 +1258,122 @@ pub enum RepoPushRole { Member, } +/// Visibility of a multi-repo project listing. +#[derive(Clone, Copy, Debug, clap::ValueEnum)] +pub enum ProjectVisibility { + /// Project appears in public listings (default). + Listed, + /// Project is hidden from public listings. + Unlisted, +} + +impl ProjectVisibility { + pub fn as_str(self) -> &'static str { + match self { + ProjectVisibility::Listed => "listed", + ProjectVisibility::Unlisted => "unlisted", + } + } +} + +#[derive(Subcommand)] +pub enum ProjectsCmd { + /// Create a new multi-repo project (NIP-MP kind:30621) + /// + /// Requires at least one --repo. Fails with Conflict if the project already exists. + Create { + /// Project identifier (slug), up to 1024 bytes + slug: String, + /// Member repository coordinate: bare Buzz repo id (e.g. `buzz`) or full + /// `30617::` for cross-owner or colon-bearing repo ids. + /// At least one --repo is required. + #[arg(long = "repo", required = true)] + repo: Vec, + /// Display name (≤256 bytes) + #[arg(long)] + name: Option, + /// Description (≤2048 bytes) + #[arg(long)] + description: Option, + /// Associated Buzz channel UUID + #[arg(long)] + channel: Option, + /// Visibility: `listed` (default) or `unlisted` + #[arg(long)] + visibility: Option, + }, + /// Get a project by slug + Get { + /// Project slug + slug: String, + /// Owner pubkey (64-char hex). Defaults to the current identity. + #[arg(long)] + owner: Option, + }, + /// List projects + List { + /// Owner pubkey (64-char hex). Defaults to the current identity. + #[arg(long)] + owner: Option, + /// Maximum number of results + #[arg(long)] + limit: Option, + }, + /// Add one or more member repositories to a project + #[command(name = "add-repo")] + AddRepo { + /// Project slug + slug: String, + /// Member repository coordinate (bare id or full `30617::`) + #[arg(long = "repo", required = true)] + repo: Vec, + }, + /// Remove one or more member repositories from a project + #[command(name = "remove-repo")] + RemoveRepo { + /// Project slug + slug: String, + /// Member repository coordinate to remove (bare id or full `30617::`) + #[arg(long = "repo", required = true)] + repo: Vec, + }, + /// Update project metadata (at least one setter or clearer required) + #[command(group = clap::ArgGroup::new("mutation").required(true).multiple(true))] + Update { + /// Project slug + slug: String, + /// Set the display name + #[arg(long, group = "mutation")] + name: Option, + /// Remove the display name + #[arg(long, group = "mutation", conflicts_with = "name")] + clear_name: bool, + /// Set the description + #[arg(long, group = "mutation")] + description: Option, + /// Remove the description + #[arg(long, group = "mutation", conflicts_with = "description")] + clear_description: bool, + /// Set the associated Buzz channel UUID + #[arg(long, group = "mutation")] + channel: Option, + /// Remove the associated channel + #[arg(long, group = "mutation", conflicts_with = "channel")] + clear_channel: bool, + /// Set visibility: `listed` or `unlisted` + #[arg(long, group = "mutation")] + visibility: Option, + /// Remove the visibility tag (absence defaults to `listed`) + #[arg(long, group = "mutation", conflicts_with = "visibility")] + clear_visibility: bool, + }, + /// Delete a project (head-based tombstone; verified after submit) + Delete { + /// Project slug + slug: String, + }, +} + #[derive(Subcommand)] pub enum PatchesCmd { /// Send a git patch (NIP-34 kind:1617) @@ -1796,6 +1915,41 @@ pub enum ModerationCmd { }, } +/// Normalize hand-authored `BUZZ_AUTH_TAG` input to strict JSON. +/// +/// `.env` files and shell exports sometimes carry the tag in the unquoted +/// shorthand `[auth,,,]` (quotes dropped by hand). +/// When the input is not valid JSON but is bracket-delimited, rewrite it as +/// a JSON array of the comma-separated fields (an empty field `,,` becomes +/// `""`, matching the canonical form `["auth","hex","","hex"]`). +/// +/// This is presentation-layer leniency at the configuration edge only: the +/// output is always fed through the SDK's strict `parse_auth_tag` / +/// `verify_auth_tag`, which enforce structure, hex, the conditions grammar, +/// and the BIP-340 signature. Inputs that are already valid JSON — or not +/// recognizable as the shorthand — are returned unchanged so the strict +/// parser reports the error on the original bytes. +fn normalize_auth_tag_input(input: &str) -> String { + let trimmed = input.trim(); + if serde_json::from_str::(trimmed).is_ok() { + return trimmed.to_owned(); + } + if trimmed.starts_with('[') && trimmed.ends_with(']') { + let fields: Vec<&str> = trimmed[1..trimmed.len() - 1] + .split(',') + .map(str::trim) + .collect(); + // Only a plausible 4-field auth tag is rewritten; anything else is + // passed through untouched for the strict parser to reject with an + // error that references the caller's original input. + if fields.len() == 4 && !fields.iter().any(|f| f.contains('"')) { + // serde_json cannot fail serializing a Vec<&str>. + return serde_json::to_string(&fields).expect("string array serializes"); + } + } + trimmed.to_owned() +} + async fn run(cli: Cli) -> Result<(), CliError> { let relay_url = client::normalize_relay_url(&cli.relay); @@ -1816,17 +1970,28 @@ async fn run(cli: Cli) -> Result<(), CliError> { .map_err(|e| CliError::Key(format!("invalid BUZZ_PRIVATE_KEY: {e}")))?; // NIP-OA: parse and verify the auth tag if provided. + // + // `BUZZ_AUTH_TAG` is hand-authored configuration, so the unquoted raw + // shorthand `[auth,hex,,hex]` is normalized to JSON here — at this input + // edge only. The SDK grammar and the `x-auth-tag` wire format stay strict + // JSON; all validation and signature verification happen on the strict + // path below, unchanged. let (auth_tag, auth_tag_json) = match cli.auth_tag { - Some(ref json) if !json.is_empty() => { - let tag = buzz_sdk::nip_oa::parse_auth_tag(json) + Some(ref input) if !input.is_empty() => { + let json = normalize_auth_tag_input(input); + let tag = buzz_sdk::nip_oa::parse_auth_tag(&json) .map_err(|e| CliError::Auth(format!("BUZZ_AUTH_TAG is malformed: {e}")))?; - buzz_sdk::nip_oa::verify_auth_tag(json, &keys.public_key()).map_err(|e| { + buzz_sdk::nip_oa::verify_auth_tag(&json, &keys.public_key()).map_err(|e| { CliError::Auth(format!( "BUZZ_AUTH_TAG verification failed for pubkey {}: {e}", keys.public_key().to_hex() )) })?; - (Some(tag), Some(json.clone())) + // Canonical wire form derives from the parsed-and-verified tag + // (same shape as buzz-acp's RestClient), never from raw input. + let canonical = serde_json::to_string(tag.as_slice()) + .map_err(|e| CliError::Auth(format!("BUZZ_AUTH_TAG serialization failed: {e}")))?; + (Some(tag), Some(canonical)) } _ => (None, None), }; @@ -1847,6 +2012,7 @@ async fn run(cli: Cli) -> Result<(), CliError> { Cmd::Social(sub) => commands::social::dispatch(sub, &client).await, Cmd::Notes(sub) => commands::notes::dispatch(sub, &client).await, Cmd::Repos(sub) => commands::repos::dispatch(sub, &client).await, + Cmd::Projects(sub) => commands::projects::dispatch(sub, &client).await, Cmd::Patches(sub) => commands::patches::dispatch(sub, &client).await, Cmd::Issues(sub) => commands::issues::dispatch(sub, &client).await, Cmd::Pr(sub) => commands::pr::dispatch(sub, &client).await, @@ -1863,6 +2029,51 @@ mod tests { use super::*; use clap::CommandFactory; + /// Raw shorthand `[auth,hex,,hex]` normalizes to strict JSON; the empty + /// conditions field becomes `""`. + #[test] + fn normalize_auth_tag_raw_shorthand() { + let owner = "a".repeat(64); + let sig = "b".repeat(128); + + let raw = format!("[auth,{owner},,{sig}]"); + let json = normalize_auth_tag_input(&raw); + let parsed: Vec = serde_json::from_str(&json).expect("output must be JSON"); + assert_eq!(parsed, vec!["auth", &owner, "", &sig]); + + // With conditions and surrounding whitespace (shell/.env artifacts). + let raw = format!(" [auth, {owner} , kind=9, {sig}] \n"); + let json = normalize_auth_tag_input(&raw); + let parsed: Vec = serde_json::from_str(&json).expect("output must be JSON"); + assert_eq!(parsed, vec!["auth", &owner, "kind=9", &sig]); + } + + /// Valid JSON input passes through byte-identical (modulo outer trim) — + /// the normalizer must never rewrite well-formed input. + #[test] + fn normalize_auth_tag_json_passthrough() { + let owner = "a".repeat(64); + let sig = "b".repeat(128); + let json_in = serde_json::json!(["auth", owner, "kind=9", sig]).to_string(); + assert_eq!(normalize_auth_tag_input(&json_in), json_in); + } + + /// Inputs that are neither JSON nor a plausible 4-field shorthand pass + /// through unchanged, so the strict parser rejects the original bytes. + #[test] + fn normalize_auth_tag_leaves_garbage_untouched() { + for garbage in [ + "not a tag", + "[auth,too,few]", + "[a,b,c,d,e]", + r#"[auth,"quoted",x,y]"#, // quote chars => not the shorthand + "[]", + "{\"auth\":1}", + ] { + assert_eq!(normalize_auth_tag_input(garbage), garbage.trim()); + } + } + /// Smoke test: CLI definition is valid and parseable. #[test] fn cli_definition_is_valid() { @@ -1911,6 +2122,7 @@ mod tests { "pack", "patches", "pr", + "projects", "reactions", "repos", "social", @@ -2067,6 +2279,18 @@ mod tests { names(&cmd, "patches"), vec!["get", "list", "send", "status"] ); + assert_eq!( + names(&cmd, "projects"), + vec![ + "add-repo", + "create", + "delete", + "get", + "list", + "remove-repo", + "update" + ] + ); assert_eq!( names(&cmd, "issues"), vec!["create", "get", "list", "status"] @@ -2104,6 +2328,7 @@ mod tests { ("pack", 2), ("patches", 4), ("pr", 5), + ("projects", 7), ("reactions", 3), ("repos", 5), ("social", 7), @@ -2171,4 +2396,111 @@ mod tests { .join("\n") ); } + + // ── projects update mutation group ──────────────────────────────────────── + + /// Multiple independent fields must be accepted in the same invocation. + #[test] + fn projects_update_multi_field_is_accepted() { + assert!( + Cli::try_parse_from([ + "buzz", + "projects", + "update", + "my-slug", + "--name", + "X", + "--description", + "Y", + ]) + .is_ok(), + "--name and --description together must be accepted" + ); + } + + /// A setter for one field and a clearer for a different field must be accepted. + #[test] + fn projects_update_setter_with_other_clearer_is_accepted() { + assert!( + Cli::try_parse_from([ + "buzz", + "projects", + "update", + "my-slug", + "--name", + "X", + "--clear-description", + ]) + .is_ok(), + "--name with --clear-description must be accepted" + ); + } + + /// A setter and its own clearer are mutually exclusive — clap must reject this. + #[test] + fn projects_update_setter_with_own_clearer_is_rejected() { + assert!( + Cli::try_parse_from([ + "buzz", + "projects", + "update", + "my-slug", + "--name", + "X", + "--clear-name", + ]) + .is_err(), + "--name and --clear-name together must be rejected by clap" + ); + } + + /// Providing no mutation options at all must be rejected by clap (required group). + #[test] + fn projects_update_no_mutation_is_rejected_by_clap() { + // Without credentials, a valid parse would reach authentication and fail + // with auth_error — but a clap-level rejection happens before any I/O. + // We verify it's a clap error (not just any error) by checking the error + // kind is not a runtime/auth failure — Cli::try_parse_from returns Err + // immediately for argument violations. + assert!( + Cli::try_parse_from(["buzz", "projects", "update", "my-slug"]).is_err(), + "update with no setters or clearers must be rejected at parse time" + ); + } + + /// An unrecognised visibility token must be rejected by clap before any I/O. + #[test] + fn projects_create_invalid_visibility_is_rejected_by_clap() { + assert!( + Cli::try_parse_from([ + "buzz", + "projects", + "create", + "my-slug", + "--repo", + "buzz", + "--visibility", + "chartreuse", + ]) + .is_err(), + "--visibility chartreuse must be rejected at parse time" + ); + } + + /// An unrecognised visibility token on update must be rejected by clap before any I/O. + #[test] + fn projects_update_invalid_visibility_is_rejected_by_clap() { + assert!( + Cli::try_parse_from([ + "buzz", + "projects", + "update", + "my-slug", + "--visibility", + "chartreuse", + ]) + .is_err(), + "--visibility chartreuse on update must be rejected at parse time" + ); + } } diff --git a/crates/buzz-relay/src/api/git/cas_publish.rs b/crates/buzz-relay/src/api/git/cas_publish.rs index c213e2913e..50bb36d818 100644 --- a/crates/buzz-relay/src/api/git/cas_publish.rs +++ b/crates/buzz-relay/src/api/git/cas_publish.rs @@ -1370,6 +1370,15 @@ mod tests { ); } + #[test] + fn published_head_moves_to_surviving_branch_after_current_branch_deletion() { + let refs = BTreeMap::from([("refs/heads/master".to_string(), "1".repeat(40))]); + assert_eq!( + resolve_published_head(&refs, "refs/heads/main".to_string(), "refs/heads/main"), + "refs/heads/master" + ); + } + #[test] fn digest_from_key_strips_prefix() { let k = format!("manifests/{}", "a".repeat(64)); diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index 380e33ef09..1a5a2b80dd 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -1302,7 +1302,7 @@ pub async fn receive_pack( state.config.bind_addr.port() ); let hooks_dir = repo.path().join("hooks").display().to_string(); - let hook_env = vec![ + let mut hook_env = vec![ ("BUZZ_HOOK_URL", hook_url), ( "BUZZ_HOOK_SECRET", @@ -1315,13 +1315,8 @@ pub async fn receive_pack( auth.tenant.community().as_uuid().to_string(), ), ("BUZZ_PUSHER_PUBKEY", pusher_hex.clone()), - // Override any repo-local core.hooksPath setting; defense in - // depth even though the hydrated workspace has no inherited - // config. - ("GIT_CONFIG_COUNT", "1".to_string()), - ("GIT_CONFIG_KEY_0", "core.hooksPath".to_string()), - ("GIT_CONFIG_VALUE_0", hooks_dir), ]; + hook_env.extend(receive_pack_git_config(hooks_dir)); // Run receive-pack against the tempdir. Returns the *owned* subprocess // output (PackOutput) — crucially NOT a Response, so the post-push @@ -1349,6 +1344,23 @@ pub async fn receive_pack( Ok(finalize_push(&state, ctx).await) } +/// Per-process git configuration for the hydrated receive-pack workspace. +fn receive_pack_git_config(hooks_dir: String) -> Vec<(&'static str, String)> { + vec![ + // Override any repo-local core.hooksPath setting; defense in depth + // even though the hydrated workspace has no inherited config. + ("GIT_CONFIG_COUNT", "2".to_string()), + ("GIT_CONFIG_KEY_0", "core.hooksPath".to_string()), + ("GIT_CONFIG_VALUE_0", hooks_dir), + // A bare repository rejects deletion of its symbolic HEAD branch by + // default. Hydrated repositories are ephemeral, and cas_publish + // selects a surviving branch for the next manifest HEAD, so allow + // receive-pack to apply the deletion before that selection runs. + ("GIT_CONFIG_KEY_1", "receive.denyDeleteCurrent".to_string()), + ("GIT_CONFIG_VALUE_1", "ignore".to_string()), + ] +} + /// Buffered output of a `git --stateless-rpc` subprocess. /// /// The handler holds this as an owned value between subprocess completion @@ -2159,11 +2171,148 @@ mod track_c_tests { use buzz_core::CommunityId; use nostr::{EventBuilder, Keys, Kind, Tag}; use std::collections::BTreeMap; + use std::io::Write; + use std::process::Output; fn oid_sha1() -> String { "cb09a769da1c01f458fa6959d4e8eded38fac8d3".to_string() } + fn run_test_git(cwd: &Path, args: &[&str], extra_env: &[(&str, String)]) -> Output { + let mut cmd = std::process::Command::new("git"); + cmd.current_dir(cwd) + .args(args) + .env_clear() + .env("PATH", std::env::var("PATH").unwrap_or_default()) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("HOME", "/dev/null"); + for (key, value) in extra_env { + cmd.env(key, value); + } + cmd.output().expect("run git") + } + + fn run_test_receive_pack(repo: &Path, request: &[u8], extra_env: &[(&str, String)]) -> Output { + let mut cmd = std::process::Command::new("git"); + cmd.arg("receive-pack") + .arg("--stateless-rpc") + .arg(repo) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .env_clear() + .env("PATH", std::env::var("PATH").unwrap_or_default()) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("HOME", "/dev/null"); + for (key, value) in extra_env { + cmd.env(key, value); + } + + let mut child = cmd.spawn().expect("spawn receive-pack"); + child + .stdin + .take() + .expect("receive-pack stdin") + .write_all(request) + .expect("write receive-pack request"); + child.wait_with_output().expect("wait for receive-pack") + } + + fn assert_git_success(output: Output, operation: &str) { + assert!( + output.status.success(), + "{operation} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + #[test] + fn receive_pack_config_allows_deleting_current_branch() { + let root = tempfile::TempDir::new().expect("tempdir"); + let remote = root.path().join("remote.git"); + let source = root.path().join("source"); + let remote_arg = remote.to_str().expect("utf-8 remote path"); + let source_arg = source.to_str().expect("utf-8 source path"); + + assert_git_success( + run_test_git( + root.path(), + &["init", "--bare", "--initial-branch=main", remote_arg], + &[], + ), + "initialize bare remote", + ); + assert_git_success( + run_test_git( + root.path(), + &["init", "--initial-branch=main", source_arg], + &[], + ), + "initialize source repository", + ); + assert_git_success( + run_test_git(source.as_path(), &["config", "user.name", "Buzz Test"], &[]), + "configure user name", + ); + assert_git_success( + run_test_git( + source.as_path(), + &["config", "user.email", "buzz-test@example.com"], + &[], + ), + "configure user email", + ); + std::fs::write(source.join("README.md"), "test\n").expect("write fixture"); + assert_git_success( + run_test_git(source.as_path(), &["add", "README.md"], &[]), + "stage fixture", + ); + assert_git_success( + run_test_git(source.as_path(), &["commit", "-m", "fixture"], &[]), + "commit fixture", + ); + assert_git_success( + run_test_git( + source.as_path(), + &["push", remote_arg, "main:main", "main:master"], + &[], + ), + "seed main and master", + ); + + let oid_output = run_test_git(remote.as_path(), &["rev-parse", "refs/heads/main"], &[]); + assert!(oid_output.status.success()); + let old_oid = String::from_utf8(oid_output.stdout) + .expect("utf-8 oid") + .trim() + .to_string(); + let command = format!( + "{old_oid} {} refs/heads/main\0report-status\n", + "0".repeat(40) + ); + let mut request = format!("{:04x}", command.len() + 4).into_bytes(); + request.extend_from_slice(command.as_bytes()); + request.extend_from_slice(b"0000"); + + let git_config = receive_pack_git_config(remote.join("hooks").display().to_string()); + let output = run_test_receive_pack(remote.as_path(), &request, &git_config); + assert!( + output.status.success(), + "receive-pack failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !receive_pack_report_rejected(&output.stdout), + "receive-pack rejected the deletion: {}", + String::from_utf8_lossy(&output.stdout) + ); + + assert!(!remote.join("refs/heads/main").exists()); + assert!(remote.join("refs/heads/master").exists()); + } + /// A gzip-encoded request body is transparently inflated before it /// reaches the git subprocess. Git's smart-HTTP client gzips the /// upload-pack/receive-pack request body past a size threshold (fires diff --git a/crates/buzz-sdk/src/builders.rs b/crates/buzz-sdk/src/builders.rs index ecaf2e5505..11c3c0a164 100644 --- a/crates/buzz-sdk/src/builders.rs +++ b/crates/buzz-sdk/src/builders.rs @@ -11,8 +11,8 @@ use buzz_core::{ KIND_GIT_STATUS_CLOSED, KIND_GIT_STATUS_DRAFT, KIND_GIT_STATUS_MERGED, KIND_GIT_STATUS_OPEN, KIND_IA_ARCHIVE_REQUEST, KIND_IA_UNARCHIVE_REQUEST, KIND_MODERATION_BAN, KIND_MODERATION_RESOLVE_REPORT, KIND_MODERATION_TIMEOUT, - KIND_MODERATION_UNBAN, KIND_MODERATION_UNTIMEOUT, KIND_PRESENCE_UPDATE, KIND_USER_STATUS, - KIND_WORKFLOW_DEF, KIND_WORKFLOW_TRIGGER, + KIND_MODERATION_UNBAN, KIND_MODERATION_UNTIMEOUT, KIND_PRESENCE_UPDATE, KIND_PROJECT, + KIND_USER_STATUS, KIND_WORKFLOW_DEF, KIND_WORKFLOW_TRIGGER, }, observer::{ content_looks_like_nip44, OBSERVER_AGENT_TAG, OBSERVER_FRAME_CONTROL, OBSERVER_FRAME_TAG, @@ -1522,12 +1522,7 @@ pub fn build_workflow_delete( author_pubkey: &str, workflow_id: Uuid, ) -> Result { - let pk = check_pubkey_hex(author_pubkey, "author_pubkey")?; - let tags = vec![tag(&[ - "a", - &format!("{}:{pk}:{workflow_id}", KIND_WORKFLOW_DEF), - ])?]; - Ok(EventBuilder::new(Kind::Custom(KIND_DELETION as u16), "").tags(tags)) + build_delete_addressable(KIND_WORKFLOW_DEF, author_pubkey, &workflow_id.to_string()) } /// Build a workflow trigger event (kind 46020). @@ -1861,6 +1856,364 @@ pub fn build_unarchive_identity_request( ) } +// ─── NIP-MP: Multi-repo projects (kind:30621) ──────────────────────────────── +// +// Public surface: +// • `validate_project_envelope` — Layer A protocol validator (8 ingest rules) +// • `build_project_with_tags` — Layer A raw builder (content + tags, no canonicalization) +// • `ProjectMemberCoord` — parsed member coordinate + optional relay hint +// • `build_project` — Layer B writer-policy builder +// • `build_delete_addressable` — generic NIP-09 kind:5 coordinate delete +// +// Byte-length bounds from NIP-MP §Relay Processing: +/// Maximum byte length of a project `d` tag value. +pub const PROJECT_D_MAX_LEN: usize = 1024; +/// Maximum byte length of a project `name` tag value. +pub const PROJECT_NAME_MAX: usize = 256; +/// Maximum byte length of a project `description` tag value. +pub const PROJECT_DESCRIPTION_MAX: usize = 2048; +/// Maximum byte length of a project `buzz-channel` tag value. +pub const PROJECT_CHANNEL_MAX: usize = 256; +/// Maximum byte length of a project `buzz-visibility` tag value. +pub const PROJECT_VISIBILITY_MAX: usize = 256; +/// Maximum number of `a` member tags per project event (checked before dedup). +pub const PROJECT_MEMBER_CAP: usize = 64; + +/// A validated NIP-MP member `a`-tag coordinate with an optional relay hint. +/// +/// Equality and `Hash` are by `coord` only (per spec: duplicate detection ignores hint). +#[derive(Clone, Debug)] +pub struct ProjectMemberCoord { + /// The full `30617::` coordinate string. + pub coord: String, + /// Optional opaque relay hint (third `a`-tag element, never validated by content). + pub hint: Option, +} + +impl PartialEq for ProjectMemberCoord { + fn eq(&self, other: &Self) -> bool { + self.coord == other.coord + } +} + +impl Eq for ProjectMemberCoord {} + +impl std::hash::Hash for ProjectMemberCoord { + fn hash(&self, state: &mut H) { + self.coord.hash(state); + } +} + +impl ProjectMemberCoord { + /// Parse a full `30617::` coordinate string. + /// + /// Accepts an optional relay hint as the third colon-separated element + /// after the split, but the split is always first-two-colons: kind, owner, + /// everything-else-as-repo-d. + /// + /// Rules enforced: + /// - Exactly three segments after splitting on the first two colons + /// - First segment must be the literal string `"30617"` + /// - Second segment must be exactly 64 lowercase hex characters + /// - Third segment (repo-d) must be non-empty + /// - Uppercase owners are rejected (never normalized) + pub fn parse_full(coord: &str) -> Result { + // Split on first two colons only: kind:owner:rest + let mut parts = coord.splitn(3, ':'); + let kind_part = parts.next().unwrap_or(""); + let owner_part = parts.next().unwrap_or(""); + let rest = parts.next().unwrap_or(""); + + if kind_part != "30617" { + return Err(SdkError::InvalidInput(format!( + "member coordinate must start with '30617:' (got kind {kind_part:?})" + ))); + } + if owner_part.len() != 64 || !owner_part.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(SdkError::InvalidInput(format!( + "member owner must be a 64-character hex pubkey (got {owner_part:?})" + ))); + } + // Reject uppercase (spec: lowercase hex required) + if owner_part.chars().any(|c| c.is_ascii_uppercase()) { + return Err(SdkError::InvalidInput( + "member owner hex must be lowercase".into(), + )); + } + if rest.is_empty() { + return Err(SdkError::InvalidInput( + "member coordinate repo-d must not be empty".into(), + )); + } + Ok(ProjectMemberCoord { + coord: format!("30617:{owner_part}:{rest}"), + hint: None, + }) + } + + /// Returns the `a`-tag element slice: `[coord]` or `[coord, hint]`. + pub fn to_tag_parts(&self) -> Vec { + let mut parts = vec!["a".to_string(), self.coord.clone()]; + if let Some(h) = &self.hint { + parts.push(h.clone()); + } + parts + } +} + +/// **Layer A**: Validate a complete kind:30621 envelope against the 8 NIP-MP +/// ingest rules. This is the single source of protocol truth used by both +/// `build_project_with_tags` (raw path) and `build_project` (policy path). +/// +/// Rules enforced (matches relay `buzz-db` ingest logic): +/// 1. `d` cardinality: exactly one `d` tag. +/// 2. `d` value: non-empty, ≤1024 bytes. +/// 3. Member cap: raw count of every `a` tag ≤ 64 (checked **before** per-tag +/// parsing, matching relay rule order). +/// 4. Member tag arity: every `a` tag has 2 or 3 elements (no more, no fewer). +/// 5. Member coordinate grammar: first-two-colons split; kind literal `"30617"`; +/// owner lowercase 64-hex; repo-d non-empty verbatim. +/// 6. Member deduplication: coordinate equality only (hint ignored); any +/// coordinate that appears more than once is a duplicate. +/// 7. Singleton metadata: each of `name`, `description`, `buzz-channel`, +/// `buzz-visibility` appears at most once. +/// 8. Metadata byte lengths: `name` ≤256, `description` ≤2048, +/// `buzz-channel` ≤256, `buzz-visibility` ≤256. +pub fn validate_project_envelope(tags: &[Tag], _content: &str) -> Result<(), SdkError> { + // --- Rule 1 & 2: d tag --- + let d_tags: Vec<&Tag> = tags.iter().filter(|t| tag_name(t) == Some("d")).collect(); + match d_tags.len() { + 0 => { + return Err(SdkError::InvalidInput( + "project must have exactly one 'd' tag (rule: d-cardinality)".into(), + )) + } + 1 => {} + _ => { + return Err(SdkError::InvalidInput( + "project must have exactly one 'd' tag (rule: d-cardinality)".into(), + )) + } + } + let d_val = tag_value(d_tags[0]).unwrap_or(""); + if d_val.is_empty() { + return Err(SdkError::InvalidInput( + "project 'd' tag must not be empty (rule: d-empty)".into(), + )); + } + if d_val.len() > PROJECT_D_MAX_LEN { + return Err(SdkError::InvalidInput(format!( + "project 'd' tag exceeds {PROJECT_D_MAX_LEN} bytes (rule: d-empty)" + ))); + } + + let a_tags: Vec<&Tag> = tags.iter().filter(|t| tag_name(t) == Some("a")).collect(); + + // --- Rule 3: member cap (checked before per-tag parsing, matching relay rule order) --- + if a_tags.len() > PROJECT_MEMBER_CAP { + return Err(SdkError::InvalidInput(format!( + "project exceeds member cap of {PROJECT_MEMBER_CAP} (got {}) (rule: member-cap)", + a_tags.len() + ))); + } + + // --- Rule 4: member arity --- + for a in &a_tags { + let len = a.as_slice().len() - 1; // exclude the "a" name element + if !(1..=2).contains(&len) { + return Err(SdkError::InvalidInput(format!( + "member 'a' tag must have 1 or 2 value elements (got {len}) (rule: member-tag-arity)" + ))); + } + } + + // --- Rules 5 & 6: coordinate grammar + deduplication --- + let mut seen_coords: std::collections::HashSet = std::collections::HashSet::new(); + for a in &a_tags { + let coord_val = tag_value(a).unwrap_or(""); + ProjectMemberCoord::parse_full(coord_val).map_err(|e| { + SdkError::InvalidInput(format!("{e} (rule: member-coordinate-malformed)")) + })?; + if !seen_coords.insert(coord_val.to_string()) { + return Err(SdkError::InvalidInput(format!( + "duplicate member coordinate {coord_val:?} (rule: member-duplicate)" + ))); + } + } + + // --- Rules 7 & 8: singleton metadata + byte bounds --- + let singleton_fields = [ + ( + "name", + PROJECT_NAME_MAX, + "metadata-cardinality", + "metadata-length", + ), + ( + "description", + PROJECT_DESCRIPTION_MAX, + "metadata-cardinality", + "metadata-length", + ), + ( + "buzz-channel", + PROJECT_CHANNEL_MAX, + "metadata-cardinality", + "metadata-length", + ), + ( + "buzz-visibility", + PROJECT_VISIBILITY_MAX, + "metadata-cardinality", + "metadata-length", + ), + ]; + for (field, max_bytes, card_rule, len_rule) in singleton_fields { + let matches: Vec<&Tag> = tags.iter().filter(|t| tag_name(t) == Some(field)).collect(); + if matches.len() > 1 { + return Err(SdkError::InvalidInput(format!( + "project must have at most one '{field}' tag (rule: {card_rule})" + ))); + } + if let Some(t) = matches.first() { + let val = tag_value(t).unwrap_or(""); + if val.len() > max_bytes { + return Err(SdkError::InvalidInput(format!( + "'{field}' tag exceeds {max_bytes} bytes (rule: {len_rule})" + ))); + } + } + } + + Ok(()) +} + +/// Helper: tag name (first element). +fn tag_name(tag: &Tag) -> Option<&str> { + tag.as_slice().first().map(String::as_str) +} + +/// Helper: tag value (second element). +fn tag_value(tag: &Tag) -> Option<&str> { + tag.as_slice().get(1).map(String::as_str) +} + +/// **Layer A raw builder**: Build a kind:30621 project event from a raw +/// `content` string and a raw `tags` slice, without any canonicalization. +/// +/// Validates the entire envelope through `validate_project_envelope` before +/// accepting it. The caller is responsible for supplying the correct `d` tag. +/// This is the path exercised by fixture conformance tests and by read-modify- +/// write mutations in the CLI. +pub fn build_project_with_tags(content: &str, tags: Vec) -> Result { + validate_project_envelope(&tags, content)?; + Ok(EventBuilder::new(Kind::Custom(KIND_PROJECT as u16), content).tags(tags)) +} + +/// **Layer B writer-policy builder**: Build a kind:30621 project event with +/// enforced writer policy: +/// - The `d` tag is constructed from `slug`; `check_project_slug` rejects +/// an empty or over-length slug. +/// - `channel` must be a valid UUID string. +/// - `visibility` must be `"listed"` or `"unlisted"`. +/// - Content is always empty. +/// - Member coordinates are parsed through `ProjectMemberCoord::parse_full`. +/// +/// The resulting envelope is validated through Layer A before the builder is +/// returned. +pub fn build_project( + slug: &str, + name: Option<&str>, + description: Option<&str>, + members: &[ProjectMemberCoord], + channel: Option<&str>, + visibility: Option<&str>, +) -> Result { + // Slug validation + if slug.is_empty() { + return Err(SdkError::InvalidInput( + "project slug must not be empty".into(), + )); + } + if slug.len() > PROJECT_D_MAX_LEN { + return Err(SdkError::InvalidInput(format!( + "project slug must not exceed {PROJECT_D_MAX_LEN} bytes (got {})", + slug.len() + ))); + } + + // Channel UUID validation + if let Some(ch) = channel { + uuid::Uuid::parse_str(ch).map_err(|_| { + SdkError::InvalidInput(format!("buzz-channel must be a valid UUID (got {ch:?})")) + })?; + } + + // Visibility enum validation + if let Some(vis) = visibility { + if vis != "listed" && vis != "unlisted" { + return Err(SdkError::InvalidInput(format!( + "buzz-visibility must be 'listed' or 'unlisted' (got {vis:?})" + ))); + } + } + + let mut tags: Vec = Vec::new(); + tags.push(tag(&["d", slug])?); + + if let Some(n) = name { + tags.push(tag(&["name", n])?); + } + if let Some(d) = description { + tags.push(tag(&["description", d])?); + } + for m in members { + let tag_parts = m.to_tag_parts(); + let parts: Vec<&str> = tag_parts.iter().map(|s| s.as_str()).collect(); + // Safety: to_tag_parts always produces ["a", coord, ...hint] + tags.push( + Tag::parse(parts.iter().copied()).map_err(|e| SdkError::InvalidTag(e.to_string()))?, + ); + } + if let Some(ch) = channel { + tags.push(tag(&["buzz-channel", ch])?); + } + if let Some(vis) = visibility { + tags.push(tag(&["buzz-visibility", vis])?); + } + + build_project_with_tags("", tags) +} + +/// **Generic NIP-09 coordinate delete**: Build a kind:5 deletion event with +/// a single `a`-tag addressing `::`. +/// +/// Validates: +/// - `kind` is an addressable kind (10000–19999 or 30000–39999). +/// - `pubkey` is a 64-character lowercase hex string. +/// - `d` is non-empty. +/// +/// `build_workflow_delete` delegates to this function. +pub fn build_delete_addressable( + kind: u32, + pubkey: &str, + d: &str, +) -> Result { + let is_addressable = (10000..20000).contains(&kind) || (30000..40000).contains(&kind); + if !is_addressable { + return Err(SdkError::InvalidInput(format!( + "kind {kind} is not an addressable kind (must be 10000–19999 or 30000–39999)" + ))); + } + let pk = check_pubkey_hex(pubkey, "pubkey")?; + if d.is_empty() { + return Err(SdkError::InvalidInput("d must not be empty".into())); + } + let coord = format!("{kind}:{pk}:{d}"); + let tags = vec![tag(&["a", &coord])?]; + Ok(EventBuilder::new(Kind::Custom(KIND_DELETION as u16), "").tags(tags)) +} + #[cfg(test)] mod tests { use super::*; @@ -3933,4 +4286,280 @@ mod tests { .iter() .any(|t| t.as_slice().first().map(String::as_str) == Some("replaced-by"))); } + + // ── NIP-MP cap-before-arity ordering ───────────────────────────────────── + + /// When an envelope exceeds the member cap AND contains a malformed `a` tag, + /// the validator must fire `member-cap` (rule 3) — not `member-tag-arity` + /// (rule 4). This matches the relay's ingest ordering and means a client + /// sending an oversized list never receives a per-tag parse error. + #[test] + fn validate_project_envelope_cap_wins_over_arity_when_both_fail() { + let owner = "a".repeat(64); + // Build 65 well-formed `a` tags — enough to trigger the cap. + let mut tags = vec![Tag::parse(["d", "platform"]).unwrap()]; + for i in 0..65usize { + let coord = format!("30617:{owner}:repo-{i}"); + tags.push(Tag::parse(["a", &coord]).unwrap()); + } + // Also add one malformed tag (four elements) that would fire + // member-tag-arity if evaluated before the cap check. + let coord_extra = format!("30617:{owner}:repo-extra"); + tags.push( + Tag::parse([ + "a", + &coord_extra, + "wss://relay.example.com", + "extra-element", + ]) + .unwrap(), + ); + + let err = validate_project_envelope(&tags, "").unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("member-cap"), + "expected member-cap to win, got: {msg}" + ); + assert!( + !msg.contains("member-tag-arity"), + "arity rule must not fire before cap rule, got: {msg}" + ); + } + + // ── Layer B writer-policy builder ─────────────────────────────────────── + + const OWNER64: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const VALID_UUID: &str = "3580ca9b-47b4-4af9-b22a-1068778f26c6"; + + fn member_coord(repo: &str) -> ProjectMemberCoord { + ProjectMemberCoord::parse_full(&format!("30617:{OWNER64}:{repo}")).unwrap() + } + + #[test] + fn build_project_emitted_envelope_has_correct_shape() { + // slug, name, description, channel, visibility, and one member. + let m = member_coord("buzz"); + let ev = sign( + build_project( + "my-proj", + Some("My Project"), + Some("A description"), + &[m], + Some(VALID_UUID), + Some("listed"), + ) + .expect("Layer B must accept valid inputs"), + ); + + // Kind must be 30621. + assert_eq!(ev.kind.as_u16(), KIND_PROJECT as u16); + // Content must be empty (Layer B policy). + assert!(ev.content.is_empty(), "content must be empty"); + + let all_tags: Vec> = ev.tags.iter().map(|t| t.as_slice().to_vec()).collect(); + + // d tag must be present exactly once. + let d_tags: Vec<_> = all_tags.iter().filter(|t| t[0] == "d").collect(); + assert_eq!(d_tags.len(), 1); + assert_eq!(d_tags[0][1], "my-proj"); + + // name, description, buzz-channel, buzz-visibility present. + let name_tags: Vec<_> = all_tags.iter().filter(|t| t[0] == "name").collect(); + assert_eq!(name_tags.len(), 1); + assert_eq!(name_tags[0][1], "My Project"); + + let desc_tags: Vec<_> = all_tags.iter().filter(|t| t[0] == "description").collect(); + assert_eq!(desc_tags.len(), 1); + assert_eq!(desc_tags[0][1], "A description"); + + let ch_tags: Vec<_> = all_tags.iter().filter(|t| t[0] == "buzz-channel").collect(); + assert_eq!(ch_tags.len(), 1); + assert_eq!(ch_tags[0][1], VALID_UUID); + + let vis_tags: Vec<_> = all_tags + .iter() + .filter(|t| t[0] == "buzz-visibility") + .collect(); + assert_eq!(vis_tags.len(), 1); + assert_eq!(vis_tags[0][1], "listed"); + + // member a tag. + let a_tags: Vec<_> = all_tags.iter().filter(|t| t[0] == "a").collect(); + assert_eq!(a_tags.len(), 1); + assert_eq!(a_tags[0][1], format!("30617:{OWNER64}:buzz")); + } + + #[test] + fn build_project_optional_fields_absent_when_not_supplied() { + let m = member_coord("core"); + let ev = sign( + build_project("my-proj", None, None, &[m], None, None) + .expect("minimal build must succeed"), + ); + let names: Vec<_> = ev + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("name")) + .collect(); + assert!(names.is_empty(), "name tag must not be emitted when absent"); + } + + #[test] + fn build_project_rejects_empty_slug() { + let m = member_coord("r"); + let err = build_project("", None, None, &[m], None, None).unwrap_err(); + assert!( + matches!(err, SdkError::InvalidInput(_)), + "empty slug must be InvalidInput, got: {err:?}" + ); + assert!(err.to_string().contains("empty")); + } + + #[test] + fn build_project_rejects_overlong_slug() { + let long_slug = "a".repeat(PROJECT_D_MAX_LEN + 1); + let m = member_coord("r"); + let err = build_project(&long_slug, None, None, &[m], None, None).unwrap_err(); + assert!(matches!(err, SdkError::InvalidInput(_))); + } + + #[test] + fn build_project_rejects_invalid_channel_uuid() { + let m = member_coord("r"); + let err = build_project("slug", None, None, &[m], Some("not-a-uuid"), None).unwrap_err(); + assert!(matches!(err, SdkError::InvalidInput(_))); + assert!(err.to_string().contains("UUID") || err.to_string().contains("uuid")); + } + + #[test] + fn build_project_rejects_invalid_visibility_token() { + let m = member_coord("r"); + let err = build_project("slug", None, None, &[m], None, Some("chartreuse")).unwrap_err(); + assert!(matches!(err, SdkError::InvalidInput(_))); + assert!(err.to_string().contains("listed") || err.to_string().contains("unlisted")); + } + + #[test] + fn build_project_rejects_over_cap_members() { + let members: Vec<_> = (0..=PROJECT_MEMBER_CAP) + .map(|i| member_coord(&format!("repo-{i}"))) + .collect(); + let err = build_project("slug", None, None, &members, None, None).unwrap_err(); + assert!(matches!(err, SdkError::InvalidInput(_))); + assert!( + err.to_string().contains("member-cap"), + "over-cap must report member-cap, got: {err}" + ); + } + + #[test] + fn build_project_rejects_duplicate_members() { + let m = member_coord("same"); + let err = build_project("slug", None, None, &[m.clone(), m], None, None).unwrap_err(); + assert!(matches!(err, SdkError::InvalidInput(_))); + assert!( + err.to_string().contains("dedup") || err.to_string().contains("duplicate"), + "duplicate member must report dedup, got: {err}" + ); + } + + #[test] + fn build_project_content_is_always_empty() { + // build_project forces content="" regardless; Layer A also enforces + // that the envelope is valid. Any non-empty content would be dropped. + // This test pins the Layer B content-forced-empty policy. + let m = member_coord("r"); + let ev = sign(build_project("slug", None, None, &[m], None, None).unwrap()); + assert!( + ev.content.is_empty(), + "Layer B must always emit empty content" + ); + } + + // ── NIP-MP conformance fixtures ────────────────────────────────────────── + // `build_project_with_tags` directly. Accept cases must build; reject + // cases must fail with an error message containing the expected rule name. + // A count assertion guards against silent omissions. + // + // `include_str!` path is relative to this source file. + fn nip_mp_fixture_tags(json_tags: &serde_json::Value) -> Vec { + json_tags + .as_array() + .unwrap() + .iter() + .map(|t| { + let parts: Vec = t + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect(); + let parts_ref: Vec<&str> = parts.iter().map(String::as_str).collect(); + Tag::parse(parts_ref.iter().copied()) + .unwrap_or_else(|e| panic!("fixture tag parse error: {e}\n raw: {t}")) + }) + .collect() + } + + #[test] + fn nip_mp_fixtures_all_31_cases_exercised() { + const FIXTURE_JSON: &str = include_str!("../../../docs/nips/NIP-MP.fixtures.json"); + + let data: serde_json::Value = + serde_json::from_str(FIXTURE_JSON).expect("fixture JSON must parse"); + let cases = data["cases"].as_array().expect("cases must be array"); + + // Count gate: the spec says "required to test against this one file" + // with the exact count as-shipped. + assert_eq!( + cases.len(), + 31, + "expected 31 fixture cases, got {} — was NIP-MP.fixtures.json edited?", + cases.len() + ); + + let mut accept_count = 0usize; + let mut reject_count = 0usize; + + for case in cases { + let name = case["name"].as_str().unwrap(); + let expect = case["expect"].as_str().unwrap(); + let template = &case["template"]; + let content = template["content"].as_str().unwrap_or(""); + let tags = nip_mp_fixture_tags(&template["tags"]); + + match expect { + "accept" => { + build_project_with_tags(content, tags).unwrap_or_else(|e| { + panic!("fixture '{name}' (accept) must build successfully, got: {e}") + }); + accept_count += 1; + } + "reject" => { + let reject_rules = case["reject_rules"] + .as_array() + .expect("reject case must have reject_rules") + .iter() + .map(|r| r.as_str().unwrap().to_string()) + .collect::>(); + + let err = build_project_with_tags(content, tags).unwrap_err(); + let err_msg = err.to_string(); + + // The error must mention at least one of the expected rules. + let rule_matched = reject_rules.iter().any(|r| err_msg.contains(r.as_str())); + assert!( + rule_matched, + "fixture '{name}' rejected with wrong rule.\n expected one of: {reject_rules:?}\n got error: {err_msg}" + ); + reject_count += 1; + } + other => panic!("fixture '{name}' has unknown expect value: {other:?}"), + } + } + + assert_eq!(accept_count, 11, "expected 11 accept cases"); + assert_eq!(reject_count, 20, "expected 20 reject cases"); + } } diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 580a74fe43..5bf6462239 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1044,6 +1044,7 @@ dependencies = [ "audioadapter-buffers", "axum", "base64 0.22.1", + "block2", "buzz-agent", "buzz-core", "buzz-media", @@ -6122,9 +6123,9 @@ dependencies = [ [[package]] name = "nostr" -version = "0.44.6" +version = "0.44.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e826dd648489de2c5b293920e20b92932ef820302007c1987c758d4d06eeb2cf" +checksum = "c7d3d987ea7078dc36947cde532637c472a229426702e4331dd7667325378bd9" dependencies = [ "base64 0.22.1", "bech32", @@ -6166,9 +6167,9 @@ dependencies = [ [[package]] name = "nostr-relay-pool" -version = "0.44.1" +version = "0.44.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91b2c039df4f96c4bf7dae52a74fd5516ad6dda83a11c0c69dea91b5255a4f37" +checksum = "c85c54d6ca9aae4ae2bf19a7663ba9db5f45f783f1d24aff55f006386b8b99a1" dependencies = [ "async-utility", "async-wsocket", diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index 196517b9e5..6d393d1b68 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -46,8 +46,9 @@ notify-rust = "4" webkit2gtk = { version = "=2.0.2", features = ["v2_22"] } [target.'cfg(target_os = "macos")'.dependencies] +block2 = { version = "0.6", default-features = false, features = ["std"] } objc2 = { version = "0.6.4", default-features = false } -objc2-app-kit = { version = "0.3.2", default-features = false, features = ["NSHapticFeedback", "NSMenu", "NSMenuItem", "NSStatusItem"] } +objc2-app-kit = { version = "0.3.2", default-features = false, features = ["NSEvent", "NSHapticFeedback", "NSMenu", "NSMenuItem", "NSStatusItem", "block2"] } objc2-foundation = { version = "0.3.2", default-features = false, features = ["NSProcessInfo", "NSString"] } keyring = { version = "3.6.3", default-features = false, features = ["apple-native", "vendored"], optional = true } security-framework = { version = "3.7.0", features = ["OSX_10_15"] } diff --git a/desktop/src-tauri/src/mouse_nav.rs b/desktop/src-tauri/src/mouse_nav.rs new file mode 100644 index 0000000000..cd729f7304 --- /dev/null +++ b/desktop/src-tauri/src/mouse_nav.rs @@ -0,0 +1,142 @@ +//! Native macOS handler for back/forward navigation inputs (mouse X1/X2 +//! buttons and horizontal swipe gestures). +//! +//! WKWebView never delivers these inputs to the web content layer, so a DOM +//! listener can't see them (Safari itself handles them natively in the app +//! layer, not in the page). This module installs an NSEvent local monitor +//! and emits a `mouse-nav` Tauri event that `useBackForwardControls` acts on +//! in the frontend. Two event shapes map to navigation: +//! +//! - `otherMouseUp` with button 3/4 — mice whose X1/X2 buttons reach the app +//! as plain mouse buttons. +//! - `swipe` with a horizontal delta — AppKit's page-swipe gesture +//! (`swipeWithEvent:`): `deltaX > 0` is back, `deltaX < 0` is forward. +//! Sent by mouse drivers that synthesize a page-swipe gesture for the +//! back/forward buttons instead of button-3/4 events (the hardware this +//! was verified on). Stock Apple trackpad and Magic Mouse swipes arrive +//! as phased scroll-wheel events instead, which this module does not +//! handle — that path (`ScrollWheel` + `trackSwipeEventWithOptions:`, +//! which also needs scroll-edge detection) is a follow-up. +//! +//! Compiled macOS-only (via `tray_menu`). Non-macOS X1/X2 behavior is left +//! to the underlying webview. + +/// Maps an `otherMouseUp` button number to a navigation direction. +/// Buttons 3 and 4 are X1 (back) and X2 (forward). +fn direction_for_button(button: isize) -> Option<&'static str> { + match button { + 3 => Some("back"), + 4 => Some("forward"), + _ => None, + } +} + +/// Maps a swipe gesture's horizontal delta to a navigation direction, +/// following the AppKit `swipeWithEvent:` convention: positive is back, +/// negative is forward. A swipe arrives as a begin/end pair and only the +/// end event carries the direction, so `deltaX == 0` maps to `None`. +fn direction_for_swipe(delta_x: f64) -> Option<&'static str> { + if delta_x > 0.0 { + Some("back") + } else if delta_x < 0.0 { + Some("forward") + } else { + None + } +} + +pub fn init(app_handle: &tauri::AppHandle) { + use block2::RcBlock; + use objc2_app_kit::{NSEvent, NSEventMask, NSEventType}; + use tauri::Emitter; + + let app = app_handle.clone(); + let block = RcBlock::new(move |event: std::ptr::NonNull| -> *mut NSEvent { + // SAFETY: the monitor hands us a valid NSEvent for the matched mask. + let ev = unsafe { event.as_ref() }; + + match ev.r#type() { + NSEventType::OtherMouseUp => { + if let Some(direction) = direction_for_button(ev.buttonNumber()) { + // Emit to the main window explicitly instead of + // broadcasting (`emit`) so navigation stays scoped if + // multi-window ever lands. "main" is the default label + // for the single configured window (see deep_link.rs). + let _ = app.emit_to("main", "mouse-nav", direction); + // Swallow the release: nothing downstream should also act + // on it. The matching press deliberately passes through: + // WKWebView never delivers X1/X2 to the page, so the + // unmatched down is inert, and swallowing presses risks + // interfering with AppKit behaviors keyed off mouse-down. + return std::ptr::null_mut(); + } + } + NSEventType::Swipe => { + if let Some(direction) = direction_for_swipe(ev.deltaX()) { + let _ = app.emit_to("main", "mouse-nav", direction); + } + // Pass swipes through: nothing else navigates on them, and + // swallowing mid-gesture events could confuse AppKit's + // gesture tracking. + } + _ => {} + } + + event.as_ptr() + }); + + // SAFETY: the block returns either null or the pointer it was given, both + // valid per the monitor contract. The returned monitor token is + // deliberately leaked: the monitor must live for the whole app lifetime. + let monitor = unsafe { + NSEvent::addLocalMonitorForEventsMatchingMask_handler( + NSEventMask::OtherMouseUp | NSEventMask::Swipe, + &block, + ) + }; + + if let Some(monitor) = monitor { + std::mem::forget(monitor); + } else { + eprintln!("buzz-desktop: mouse-nav: failed to install NSEvent monitor"); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn button_3_is_back() { + assert_eq!(direction_for_button(3), Some("back")); + } + + #[test] + fn button_4_is_forward() { + assert_eq!(direction_for_button(4), Some("forward")); + } + + #[test] + fn other_buttons_do_not_navigate() { + for button in [0, 1, 2, 5, -1] { + assert_eq!(direction_for_button(button), None); + } + } + + #[test] + fn positive_swipe_delta_is_back() { + assert_eq!(direction_for_swipe(1.0), Some("back")); + assert_eq!(direction_for_swipe(0.5), Some("back")); + } + + #[test] + fn negative_swipe_delta_is_forward() { + assert_eq!(direction_for_swipe(-1.0), Some("forward")); + assert_eq!(direction_for_swipe(-0.5), Some("forward")); + } + + #[test] + fn zero_delta_swipe_begin_event_is_ignored() { + assert_eq!(direction_for_swipe(0.0), None); + } +} diff --git a/desktop/src-tauri/src/tray_menu.rs b/desktop/src-tauri/src/tray_menu.rs index d733cd1f13..6dcef0ecf7 100644 --- a/desktop/src-tauri/src/tray_menu.rs +++ b/desktop/src-tauri/src/tray_menu.rs @@ -3,6 +3,11 @@ //! The webview owns the live agent-turn state. It sends the small display //! projection here so the native menu can remain useful while Buzz is hidden. +// Mouse back/forward (X1/X2 buttons and swipe) is also macOS-only native I/O; +// group it here so both platform-layer init paths share one call site in lib.rs. +#[path = "mouse_nav.rs"] +pub(crate) mod mouse_nav; + use std::{ sync::{Mutex, OnceLock}, time::{Duration, Instant}, @@ -488,6 +493,7 @@ pub fn init(app: &AppHandle) -> tauri::Result<()> { if let Err(error) = apply_activity_presentation(&tray, activities, recent_activities) { eprintln!("buzz-desktop: failed to apply tray menu presentation: {error}"); } + mouse_nav::init(app); Ok(()) } diff --git a/desktop/src/app/navigation/backForwardChords.test.mjs b/desktop/src/app/navigation/backForwardChords.test.mjs new file mode 100644 index 0000000000..cb60203d04 --- /dev/null +++ b/desktop/src/app/navigation/backForwardChords.test.mjs @@ -0,0 +1,139 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { matchBackForwardChord } from "./backForwardChords.ts"; + +function chord(overrides = {}) { + return { + altKey: false, + code: "", + ctrlKey: false, + key: "", + metaKey: false, + shiftKey: false, + ...overrides, + }; +} + +// ── macOS: ⌘[ / ⌘] ─────────────────────────────────────────────────────────── + +test("mac: ⌘[ matches back", () => { + assert.equal( + matchBackForwardChord(chord({ key: "[", metaKey: true }), true), + "back", + ); +}); + +test("mac: ⌘] matches forward", () => { + assert.equal( + matchBackForwardChord(chord({ key: "]", metaKey: true }), true), + "forward", + ); +}); + +test("mac: matches by code for non-US layouts", () => { + assert.equal( + matchBackForwardChord( + chord({ code: "BracketLeft", key: "Dead", metaKey: true }), + true, + ), + "back", + ); + assert.equal( + matchBackForwardChord( + chord({ code: "BracketRight", key: "Dead", metaKey: true }), + true, + ), + "forward", + ); +}); + +test("mac: requires meta", () => { + assert.equal(matchBackForwardChord(chord({ key: "[" }), true), null); +}); + +test("mac: rejects extra modifiers", () => { + for (const extra of [ + { altKey: true }, + { ctrlKey: true }, + { shiftKey: true }, + ]) { + assert.equal( + matchBackForwardChord(chord({ key: "[", metaKey: true, ...extra }), true), + null, + ); + } +}); + +test("mac: Alt+arrows do not match (that is the win/linux chord)", () => { + assert.equal( + matchBackForwardChord(chord({ altKey: true, key: "ArrowLeft" }), true), + null, + ); +}); + +test("mac: ⌘←/⌘→ never match — they are line start/end in text editing", () => { + // Deliberately unbound: editable targets must keep receiving ⌘←/⌘→ so + // line-start/line-end editing still works. Only ⌘[ / ⌘] navigate. + assert.equal( + matchBackForwardChord(chord({ key: "ArrowLeft", metaKey: true }), true), + null, + ); + assert.equal( + matchBackForwardChord(chord({ key: "ArrowRight", metaKey: true }), true), + null, + ); +}); + +// ── Windows/Linux: Alt+← / Alt+→ ───────────────────────────────────────────── + +test("win/linux: Alt+ArrowLeft matches back", () => { + assert.equal( + matchBackForwardChord(chord({ altKey: true, key: "ArrowLeft" }), false), + "back", + ); +}); + +test("win/linux: Alt+ArrowRight matches forward", () => { + assert.equal( + matchBackForwardChord(chord({ altKey: true, key: "ArrowRight" }), false), + "forward", + ); +}); + +test("win/linux: requires alt", () => { + assert.equal(matchBackForwardChord(chord({ key: "ArrowLeft" }), false), null); +}); + +test("win/linux: rejects extra modifiers", () => { + for (const extra of [ + { ctrlKey: true }, + { metaKey: true }, + { shiftKey: true }, + ]) { + assert.equal( + matchBackForwardChord( + chord({ altKey: true, key: "ArrowLeft", ...extra }), + false, + ), + null, + ); + } +}); + +test("win/linux: ⌘[ does not match (that is the mac chord)", () => { + assert.equal( + matchBackForwardChord(chord({ key: "[", metaKey: true }), false), + null, + ); +}); + +// ── Non-chord keys never match ──────────────────────────────────────────────── + +test("plain bracket / arrow keys without the platform modifier never match", () => { + for (const isMac of [true, false]) { + for (const key of ["[", "]", "ArrowLeft", "ArrowRight", "a", "Enter"]) { + assert.equal(matchBackForwardChord(chord({ key }), isMac), null); + } + } +}); diff --git a/desktop/src/app/navigation/backForwardChords.ts b/desktop/src/app/navigation/backForwardChords.ts new file mode 100644 index 0000000000..ac81f52ea7 --- /dev/null +++ b/desktop/src/app/navigation/backForwardChords.ts @@ -0,0 +1,51 @@ +/** + * Global back/forward navigation chords. + * + * macOS: ⌘[ / ⌘] — matching Safari, Chrome, Finder, and Slack. + * Windows/Linux: Alt+← / Alt+→ — matching browsers and Slack. + * + * Kept pure (no DOM access) so chord matching can be unit tested; the + * window listener wiring lives in `useBackForwardControls`. + */ + +export type BackForwardDirection = "back" | "forward"; + +export type BackForwardChordEvent = Pick< + KeyboardEvent, + "altKey" | "code" | "ctrlKey" | "key" | "metaKey" | "shiftKey" +>; + +export function matchBackForwardChord( + event: BackForwardChordEvent, + isMac: boolean, +): BackForwardDirection | null { + if (isMac) { + if (!event.metaKey || event.ctrlKey || event.altKey || event.shiftKey) { + return null; + } + + if (event.key === "[" || event.code === "BracketLeft") { + return "back"; + } + + if (event.key === "]" || event.code === "BracketRight") { + return "forward"; + } + + return null; + } + + if (!event.altKey || event.metaKey || event.ctrlKey || event.shiftKey) { + return null; + } + + if (event.key === "ArrowLeft") { + return "back"; + } + + if (event.key === "ArrowRight") { + return "forward"; + } + + return null; +} diff --git a/desktop/src/app/navigation/useBackForwardControls.ts b/desktop/src/app/navigation/useBackForwardControls.ts index 7f7d84f6d7..e5513247d5 100644 --- a/desktop/src/app/navigation/useBackForwardControls.ts +++ b/desktop/src/app/navigation/useBackForwardControls.ts @@ -4,7 +4,10 @@ import { useRouter, useRouterState, } from "@tanstack/react-router"; +import { isTauri } from "@tauri-apps/api/core"; +import { listen } from "@tauri-apps/api/event"; +import { matchBackForwardChord } from "@/app/navigation/backForwardChords"; import { isMacPlatform } from "@/shared/lib/platform"; import { trimMapToSize } from "@/shared/lib/trimMapToSize"; @@ -14,19 +17,6 @@ type RouterHistoryState = { key?: string; }; -function isEditableTarget(target: EventTarget | null): boolean { - if (!(target instanceof HTMLElement)) { - return false; - } - - return ( - target.isContentEditable || - target.closest( - 'input, textarea, select, [contenteditable=""], [contenteditable="true"]', - ) !== null - ); -} - export function useBackForwardControls() { const router = useRouter(); const canGoBack = useCanGoBack(); @@ -81,42 +71,34 @@ export function useBackForwardControls() { }, [canGoForward, router.history]); const handleKeyDown = React.useEffectEvent((event: KeyboardEvent) => { - if (isEditableTarget(event.target)) { + // Note: the chords deliberately fire even when focus is inside an + // editable element. The composer autofocuses on every channel switch + // (`useComposerAutofocus`), so in steady state focus almost always + // lives in a contenteditable — an editable-target guard here made the + // shortcuts effectively dead (#3775). Safe because neither ⌘[ / ⌘] + // (macOS) nor Alt+←/→ (Windows/Linux) carry text-editing semantics, + // and the TipTap editor binds no conflicting shortcuts. + const direction = matchBackForwardChord(event, isMacPlatform()); + + if (direction === "back") { + event.preventDefault(); + goBack(); return; } - const isMac = isMacPlatform(); - const isBackShortcut = isMac - ? event.metaKey && - !event.ctrlKey && - !event.altKey && - !event.shiftKey && - (event.key === "[" || event.code === "BracketLeft") - : event.altKey && - !event.metaKey && - !event.ctrlKey && - !event.shiftKey && - event.key === "ArrowLeft"; - const isForwardShortcut = isMac - ? event.metaKey && - !event.ctrlKey && - !event.altKey && - !event.shiftKey && - (event.key === "]" || event.code === "BracketRight") - : event.altKey && - !event.metaKey && - !event.ctrlKey && - !event.shiftKey && - event.key === "ArrowRight"; - - if (isBackShortcut) { + if (direction === "forward") { event.preventDefault(); + goForward(); + } + }); + + const handleMouseNav = React.useEffectEvent((direction: string) => { + if (direction === "back") { goBack(); return; } - if (isForwardShortcut) { - event.preventDefault(); + if (direction === "forward") { goForward(); } }); @@ -128,6 +110,23 @@ export function useBackForwardControls() { }; }, []); + // macOS: WKWebView never delivers X1/X2 button events or horizontal + // swipe gestures to the DOM, so the native layer catches them + // (`mouse_nav.rs`) and forwards them as a Tauri event. + React.useEffect(() => { + if (!isTauri()) { + return; + } + + const unlistenPromise = listen("mouse-nav", (event) => { + handleMouseNav(event.payload); + }); + + return () => { + void unlistenPromise.then((unlisten) => unlisten()); + }; + }, []); + return { canGoBack, canGoForward, diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index cc6f0467d6..b4a139eb0e 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -1879,3 +1879,201 @@ test("buildTranscript five-section system prompt card is standalone with all sec "prompt context must NOT contain system-prompt sections (Base/System/Team Instructions/Core Memory/Channel Canvas)", ); }); + +// --- claude-agent-acp _meta.systemPrompt.append transport --- + +test("buildTranscript session/new via _meta.systemPrompt.append produces identical standalone card as bare systemPrompt field", () => { + // claude-agent-acp delivers the system prompt at _meta.systemPrompt.append + // instead of the bare systemPrompt field. The observer must extract it and + // build the identical standalone card (same five sections, same acpSource, + // same turnId: null, same placement before the first turn). + const CH = "55555555-5555-5555-5555-555555555555"; + const SYSTEM_PROMPT = [ + "[Base]", + "You are a helpful assistant.", + "", + "[System]", + "Custom persona.", + "", + "---", + "# Team Instructions", + "Always tag on handoff.", + "", + "[Agent Memory \u2014 core]", + "I am Duncan.", + "", + "[Channel Canvas]", + "Canvas revision (event ID): a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + "Last modified: 2026-07-01T10:00:00Z", + "Fetch current content with: buzz canvas get --channel 55555555-5555-5555-5555-555555555555", + ].join("\n"); + + const makeEvents = (params) => [ + { + seq: 1, + timestamp: "2026-07-01T10:00:00.000Z", + kind: "turn_started", + agentIndex: 0, + channelId: CH, + sessionId: null, + turnId: "turn-1", + payload: { source: "channel", triggeringEventIds: [] }, + }, + { + seq: 2, + timestamp: "2026-07-01T10:00:00.100Z", + kind: "acp_write", + agentIndex: 0, + channelId: CH, + sessionId: null, + turnId: "turn-1", + payload: { jsonrpc: "2.0", id: 1, method: "session/new", params }, + }, + { + seq: 3, + timestamp: "2026-07-01T10:00:00.200Z", + kind: "session_resolved", + agentIndex: 0, + channelId: CH, + sessionId: "sess-cc", + turnId: "turn-1", + payload: { sessionId: "sess-cc", isNewSession: true }, + }, + { + seq: 4, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "acp_write", + agentIndex: 0, + channelId: CH, + sessionId: "sess-cc", + turnId: "turn-1", + payload: { + jsonrpc: "2.0", + id: 2, + method: "session/prompt", + params: { + sessionId: "sess-cc", + prompt: [ + { + type: "text", + text: `[Buzz event: @mention]\nEvent ID: ${"a".repeat(64)}\nFrom: x (hex: ${"b".repeat(64)})\nContent: hello`, + }, + { type: "text", text: "[Thread context]\nPrior messages here." }, + ], + }, + }, + }, + ]; + + // Build transcript from the claude-agent-acp _meta transport. + const metaEvents = makeEvents({ + _meta: { systemPrompt: { append: SYSTEM_PROMPT } }, + }); + const metaRaw = buildTranscript(metaEvents); + const metaBlocks = buildTranscriptDisplayBlocks(metaRaw); + const metaFlat = flattenDisplayBlocks(metaBlocks); + + // Also build from the standard bare-field transport for comparison. + const fieldEvents = makeEvents({ systemPrompt: SYSTEM_PROMPT }); + const fieldRaw = buildTranscript(fieldEvents); + const fieldBlocks = buildTranscriptDisplayBlocks(fieldRaw); + + // (a) Both produce exactly one standalone system-prompt single block. + const metaSPBlocks = metaBlocks.filter( + (b) => b.kind === "single" && b.item?.acpSource === "session/new", + ); + const fieldSPBlocks = fieldBlocks.filter( + (b) => b.kind === "single" && b.item?.acpSource === "session/new", + ); + assert.equal( + metaSPBlocks.length, + 1, + "_meta: exactly one standalone system-prompt block", + ); + assert.equal( + fieldSPBlocks.length, + 1, + "field: exactly one standalone system-prompt block", + ); + + // (b) Both carry the same five ordered sections. + const EXPECTED_TITLES = [ + "Base", + "System", + "Team Instructions", + "Core Memory", + "Channel Canvas", + ]; + const metaTitles = (metaSPBlocks[0].item?.sections ?? []).map((s) => s.title); + const fieldTitles = (fieldSPBlocks[0].item?.sections ?? []).map( + (s) => s.title, + ); + assert.deepEqual( + metaTitles, + EXPECTED_TITLES, + "_meta: five sections in order", + ); + assert.deepEqual( + fieldTitles, + EXPECTED_TITLES, + "field: five sections in order", + ); + + // (c) System prompt appears before Prompt context in both display orders. + const metaSPIdx = metaFlat.findIndex((i) => i.title === "System prompt"); + const metaPCIdx = metaFlat.findIndex((i) => i.title === "Prompt context"); + assert.ok(metaSPIdx !== -1, "_meta: System prompt item present"); + assert.ok(metaPCIdx !== -1, "_meta: Prompt context item present"); + assert.ok( + metaSPIdx < metaPCIdx, + `_meta: System prompt (${metaSPIdx}) must precede Prompt context (${metaPCIdx})`, + ); + + // (d) The _meta item has turnId: null (standalone, not in a turn bucket). + const metaSPRawIdx = metaRaw.findIndex((i) => i.title === "System prompt"); + assert.equal( + metaRaw[metaSPRawIdx]?.turnId ?? null, + null, + "_meta: system-prompt item must have turnId=null", + ); +}); + +test("buildTranscript session/new bare systemPrompt field takes precedence over _meta.systemPrompt.append", () => { + // When both transports are present (non-standard but must not regress), + // the standard bare field must win — a reversed ?? would silently use the + // wrong text and the card body would differ from the wire source of truth. + const CH = "66666666-6666-6666-6666-666666666666"; + const events = [ + { + seq: 1, + timestamp: "2026-07-01T10:00:00.000Z", + kind: "acp_write", + agentIndex: 0, + channelId: CH, + sessionId: "sess-both", + turnId: "turn-1", + payload: { + jsonrpc: "2.0", + id: 1, + method: "session/new", + params: { + systemPrompt: "[Base]\nWinner.", + _meta: { systemPrompt: { append: "[Base]\nLoser." } }, + }, + }, + }, + ]; + + const rawItems = buildTranscript(events); + const spItem = rawItems.find((i) => i.title === "System prompt"); + assert.ok(spItem, "System prompt item must be present"); + const bodies = (spItem.sections ?? []).map((s) => s.body).join("|"); + assert.ok( + bodies.includes("Winner"), + "bare systemPrompt must win over _meta.systemPrompt.append", + ); + assert.ok( + !bodies.includes("Loser"), + "_meta.systemPrompt.append must not appear when bare field is present", + ); +}); diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index 962290c6ca..e371bf5fc3 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -872,14 +872,14 @@ export function processTranscriptEvent( } else if (event.kind === "acp_write" && method === "session/new") { // The base + persona prompts ride session/new's systemPrompt, framed by // the harness as [Base]/[System]/[Agent Memory — core]/[Channel Canvas]. - // Each session/new event is keyed by (seq, timestamp) — the same dedup - // pair used by observerRelayStore — so distinct sessions each retain - // their own system-prompt card even across archive rebuilds where two - // processes may emit the same seq. turnId: null keeps it out of turn - // buckets; acpSource "session/new" lets the display grouper place it - // as a standalone card before the session's first turn. + // claude-agent-acp uses _meta.systemPrompt.append instead; both paths + // produce the same standalone card (turnId: null, acpSource "session/new"); + // the bare field takes precedence when both are present. const params = asRecord(payload.params); - const systemPrompt = asString(params.systemPrompt); + const metaPrompt = asString( + asRecord(asRecord(params._meta).systemPrompt).append, + ); + const systemPrompt = asString(params.systemPrompt) ?? metaPrompt; if (systemPrompt) { const sections = parseSystemPromptSections(systemPrompt); if (sections.length > 0) { diff --git a/desktop/tests/e2e/navigation.spec.ts b/desktop/tests/e2e/navigation.spec.ts index f7a96cd568..18db55a50a 100644 --- a/desktop/tests/e2e/navigation.spec.ts +++ b/desktop/tests/e2e/navigation.spec.ts @@ -48,6 +48,37 @@ test("global back and forward move across channel routes", async ({ page }) => { await expect(page.getByTestId("chat-title")).toHaveText("random"); }); +test("back/forward keyboard chords work while the composer has focus", async ({ + page, +}) => { + const backChord = process.platform === "darwin" ? "Meta+[" : "Alt+ArrowLeft"; + const forwardChord = + process.platform === "darwin" ? "Meta+]" : "Alt+ArrowRight"; + + await page.goto("/"); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + await page.getByTestId("channel-random").click(); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + + // The composer autofocuses on channel switch; make the regression + // condition explicit by clicking into it. The chords must still fire + // from inside the contenteditable (#3775). + await page.getByTestId("message-input").click(); + await expect(page.getByTestId("message-input")).toBeFocused(); + + await page.keyboard.press(backChord); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + await page.keyboard.press(forwardChord); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + + // preventDefault kept the chord out of the editor — no stray characters. + await expect(page.getByTestId("message-input")).toHaveText(""); +}); + // FIXME: the forum post "Back to posts" header renders under the fixed top // chrome drag region, which intercepts the click. Pre-existing breakage — // this spec file was never registered in playwright.config.ts until now. diff --git a/mobile/lib/features/channels/channel_detail_page.dart b/mobile/lib/features/channels/channel_detail_page.dart index 044342d101..a24b6c07ee 100644 --- a/mobile/lib/features/channels/channel_detail_page.dart +++ b/mobile/lib/features/channels/channel_detail_page.dart @@ -188,6 +188,11 @@ class ChannelDetailPage extends HookConsumerWidget { messagesState: messagesState, ); + useEffect(() { + final session = ref.read(relaySessionProvider.notifier); + return session.registerVisibleChannel(channel.id); + }, [channel.id]); + // Preload channel member profiles so @mentions resolve correctly. useEffect(() { _preloadMembers(ref, channel.id); diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 614ab054c3..c0525a0b29 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -35,8 +35,12 @@ const _authoredRootIdsPrefix = 'buzz-thread-authored.v1'; class ChannelsNotifier extends AsyncNotifier> { static const _backstopInterval = Duration(seconds: 60); - final List _unsubscribers = []; + final Map _unsubscribersByChannel = {}; + Future _liveSubscriptionQueue = Future.value(); + List _desiredLiveChannels = const []; + Set _desiredLiveChannelIds = const {}; int _subscriptionVersion = 0; + String? _subscriptionRelayBaseUrl; Timer? _backstopTimer; final Map _latestObservedByChannel = {}; final Map> @@ -147,7 +151,10 @@ class ChannelsNotifier extends AsyncNotifier> { .whereType() .toSet() .toList(); - if (channelIds.isEmpty) return const []; + if (channelIds.isEmpty) { + if (subscribeLive) await _subscribeLive(const []); + return const []; + } // Step 2: pull channel metadata in one batched filter. final metas = await session.fetchHistory( @@ -423,50 +430,114 @@ class ChannelsNotifier extends AsyncNotifier> { /// Subscribe per-channel to live events (requires `#h` tag for relay /// channel-scoped fan-out). Also starts a 60s WS backstop poll to detect /// newly created channels we don't yet have subscriptions for. - Future _subscribeLive(List channels) async { - _clearLiveSubscriptions(); - final subscriptionVersion = _subscriptionVersion; - if (ref.read(relaySessionProvider).status != SessionStatus.connected) { - return; - } - - final session = ref.read(relaySessionProvider.notifier); + Future _subscribeLive(List channels) { final channelIds = { for (final channel in channels) if (channel.isMember && !channel.isArchived) channel.id, }; - - final subscriptions = await Future.wait( - channelIds.map((channelId) async { - try { - return await session.subscribe( - NostrFilter( - kinds: EventKind.channelEventKinds, - tags: { - '#h': [channelId], - }, - limit: 0, - ), - _handleLiveEvent, - ); - } catch (error) { - debugPrint( - '[ChannelsNotifier] live subscription failed for $channelId: $error', - ); - return null; - } - }), + final relayBaseUrl = ref.read(relayConfigProvider).baseUrl; + _desiredLiveChannels = channels; + _desiredLiveChannelIds = channelIds; + final subscriptionVersion = ++_subscriptionVersion; + + final sync = _liveSubscriptionQueue.then( + (_) => + _syncLiveSubscriptions(relayBaseUrl, subscriptionVersion, channels), ); + _liveSubscriptionQueue = sync.catchError((Object error, StackTrace stack) { + debugPrint( + '[ChannelsNotifier] live subscription sync failed: $error\n$stack', + ); + }); + return sync; + } + + Future _syncLiveSubscriptions( + String relayBaseUrl, + int subscriptionVersion, + List channels, + ) async { + if (ref.read(relaySessionProvider).status != SessionStatus.connected) { + return; + } + + if (subscriptionVersion != _subscriptionVersion) { + await _syncLiveSubscriptions( + ref.read(relayConfigProvider).baseUrl, + _subscriptionVersion, + _desiredLiveChannels, + ); + return; + } - if (subscriptionVersion != _subscriptionVersion || - ref.read(relaySessionProvider).status != SessionStatus.connected) { - for (final unsubscribe in subscriptions.whereType()) { + if (_subscriptionRelayBaseUrl != relayBaseUrl) { + for (final unsubscribe in _unsubscribersByChannel.values) { unsubscribe(); } + _unsubscribersByChannel.clear(); + _subscriptionRelayBaseUrl = relayBaseUrl; + } + if (ref.read(relayConfigProvider).baseUrl != relayBaseUrl) { + return; + } + final session = ref.read(relaySessionProvider.notifier); + final channelIds = _desiredLiveChannelIds; + + for (final entry in _unsubscribersByChannel.entries.toList()) { + if (channelIds.contains(entry.key)) continue; + _unsubscribersByChannel.remove(entry.key); + entry.value(); + } + + for (final channelId in channelIds) { + if (ref.read(relaySessionProvider).status != SessionStatus.connected) { + return; + } + if (_unsubscribersByChannel.containsKey(channelId)) continue; + try { + final unsubscribe = await session.subscribe( + NostrFilter( + kinds: EventKind.channelEventKinds, + tags: { + '#h': [channelId], + }, + limit: 0, + ), + _handleLiveEvent, + ); + if (ref.read(relaySessionProvider).status != SessionStatus.connected || + !_desiredLiveChannelIds.contains(channelId) || + ref.read(relayConfigProvider).baseUrl != relayBaseUrl || + _subscriptionRelayBaseUrl != relayBaseUrl) { + unsubscribe(); + return; + } + final replaced = _unsubscribersByChannel[channelId]; + if (replaced != null) { + unsubscribe(); + continue; + } + _unsubscribersByChannel[channelId] = unsubscribe; + } catch (error) { + debugPrint( + '[ChannelsNotifier] live subscription failed for $channelId: $error', + ); + } + } + + if (ref.read(relaySessionProvider).status != SessionStatus.connected) { return; } - _unsubscribers.addAll(subscriptions.whereType()); + if (subscriptionVersion != _subscriptionVersion) { + final desiredChannelIds = _desiredLiveChannelIds; + for (final entry in _unsubscribersByChannel.entries.toList()) { + if (desiredChannelIds.contains(entry.key)) continue; + _unsubscribersByChannel.remove(entry.key); + entry.value(); + } + return; + } unawaited(_catchUpUnreadEvents(channels)); @@ -734,10 +805,13 @@ class ChannelsNotifier extends AsyncNotifier> { void _clearLiveSubscriptions() { _subscriptionVersion++; - for (final unsubscribe in _unsubscribers) { + _desiredLiveChannels = const []; + _desiredLiveChannelIds = const {}; + for (final unsubscribe in _unsubscribersByChannel.values) { unsubscribe(); } - _unsubscribers.clear(); + _unsubscribersByChannel.clear(); + _subscriptionRelayBaseUrl = null; _backstopTimer?.cancel(); _backstopTimer = null; } diff --git a/mobile/lib/shared/relay/relay.dart b/mobile/lib/shared/relay/relay.dart index bc11325414..dd153fc928 100644 --- a/mobile/lib/shared/relay/relay.dart +++ b/mobile/lib/shared/relay/relay.dart @@ -5,8 +5,10 @@ export 'media_image.dart'; export 'media_upload.dart'; export 'nostr_filters.dart'; export 'nostr_models.dart'; +export 'relay_closed_policy.dart'; export 'relay_client.dart'; export 'relay_provider.dart'; +export 'relay_rate_limit_gate.dart'; export 'relay_session.dart'; export 'relay_socket.dart'; export 'signed_event_relay.dart'; diff --git a/mobile/lib/shared/relay/relay_closed_policy.dart b/mobile/lib/shared/relay/relay_closed_policy.dart new file mode 100644 index 0000000000..d39084b189 --- /dev/null +++ b/mobile/lib/shared/relay/relay_closed_policy.dart @@ -0,0 +1,39 @@ +/// Recovery policy for a relay `CLOSED` subscription message. +enum RelayClosedClass { + /// A transient failure that may recover when the same REQ is retried. + retryable, + + /// Relay back-pressure that must also arm the shared request gate. + rateLimited, + + /// An authorization, access, or filter failure that cannot recover unchanged. + terminal, +} + +/// Classifies whether a relay `CLOSED` message should be retried. +RelayClosedClass classifyRelayClosed(String message) { + final normalized = message.trim().toLowerCase(); + if (normalized.startsWith('rate-limited:')) { + return RelayClosedClass.rateLimited; + } + if (normalized.startsWith('restricted:') || + normalized.startsWith('auth-required:') || + normalized.startsWith('blocked:') || + normalized.startsWith('invalid:') || + normalized.startsWith('pow:') || + normalized.startsWith('duplicate:') || + normalized.startsWith('unsupported:') || + normalized.startsWith('error: mixed search') || + normalized.startsWith('error: too many subscriptions')) { + return RelayClosedClass.terminal; + } + return RelayClosedClass.retryable; +} + +final _rateLimitRetryPattern = RegExp(r'retry in (\d+)s', caseSensitive: false); + +/// Parses the relay's canonical `retry in Ns` hint, when present. +int? parseRateLimitRetrySeconds(String message) { + final match = _rateLimitRetryPattern.firstMatch(message); + return match == null ? null : int.tryParse(match.group(1)!); +} diff --git a/mobile/lib/shared/relay/relay_rate_limit_gate.dart b/mobile/lib/shared/relay/relay_rate_limit_gate.dart new file mode 100644 index 0000000000..6164013936 --- /dev/null +++ b/mobile/lib/shared/relay/relay_rate_limit_gate.dart @@ -0,0 +1,80 @@ +import 'dart:async'; +import 'dart:math'; + +/// Creates a timer used by [RelayRateLimitGate]. +typedef RelayTimerFactory = + Timer Function(Duration duration, void Function() callback); + +/// Session-owned gate that pauses relay requests after back-pressure. +class RelayRateLimitGate { + /// Default gate duration when the relay omits a positive retry hint. + static const defaultRetrySeconds = 10; + + /// Longest retry hint accepted from a relay response. + static const maxRetrySeconds = 300; + + RelayRateLimitGate({ + DateTime Function()? now, + RelayTimerFactory timerFactory = Timer.new, + }) : _now = now ?? DateTime.now, + _timerFactory = timerFactory; + + final DateTime Function() _now; + final RelayTimerFactory _timerFactory; + DateTime? _expiresAt; + Timer? _timer; + Completer? _completer; + + /// Whether a rate-limit window is currently active. + bool get isActive { + final expiresAt = _expiresAt; + return expiresAt != null && _now().isBefore(expiresAt); + } + + /// Activates or extends the gate without shrinking an existing window. + void activate(int? retryInSeconds) { + final seconds = retryInSeconds != null && retryInSeconds > 0 + ? min(retryInSeconds, maxRetrySeconds) + : defaultRetrySeconds; + final duration = Duration(seconds: seconds); + final newExpiry = _now().add(duration); + final currentExpiry = _expiresAt; + if (currentExpiry != null && !newExpiry.isAfter(currentExpiry)) return; + + _expiresAt = newExpiry; + _timer?.cancel(); + _completer ??= Completer(); + _timer = _timerFactory(duration, _expire); + } + + /// Resolves when the active rate-limit window expires. + Future wait() { + if (!isActive) return Future.value(); + return _completer!.future; + } + + /// Milliseconds remaining in the active window, or zero when inactive. + int remainingMs() { + final expiresAt = _expiresAt; + if (expiresAt == null) return 0; + return max(0, expiresAt.difference(_now()).inMilliseconds); + } + + /// Clears the gate and releases all current waiters. + void reset() { + _timer?.cancel(); + _timer = null; + _expiresAt = null; + final completer = _completer; + _completer = null; + if (completer != null && !completer.isCompleted) completer.complete(); + } + + void _expire() { + _timer = null; + _expiresAt = null; + final completer = _completer; + _completer = null; + if (completer != null && !completer.isCompleted) completer.complete(); + } +} diff --git a/mobile/lib/shared/relay/relay_session.dart b/mobile/lib/shared/relay/relay_session.dart index d34c5405e8..877bd15e82 100644 --- a/mobile/lib/shared/relay/relay_session.dart +++ b/mobile/lib/shared/relay/relay_session.dart @@ -13,7 +13,9 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; import '../auth/auth.dart'; import 'nostr_models.dart'; import 'relay_client.dart'; +import 'relay_closed_policy.dart'; import 'relay_provider.dart'; +import 'relay_rate_limit_gate.dart'; import 'relay_socket.dart'; enum SessionStatus { disconnected, connecting, connected, reconnecting } @@ -40,6 +42,8 @@ class _LiveSubscription { final void Function(String message)? onClosed; Completer? readyCompleter; int? lastSeenCreatedAt; + int closedRetryAttempt = 0; + Timer? closedRetryTimer; _LiveSubscription({ required this.filter, @@ -49,6 +53,13 @@ class _LiveSubscription { }); } +class _ClosedRetry { + final _LiveSubscription subscription; + final int generation; + + _ClosedRetry({required this.subscription, required this.generation}); +} + class _PendingEvent { final Completer completer; final Timer timeout; @@ -78,21 +89,33 @@ class RelaySessionNotifier extends Notifier { RelaySessionNotifier({ http.Client? httpClient, RelaySocketFactory socketFactory = RelaySocket.new, + RelayRateLimitGate? rateLimitGate, + RelayTimerFactory retryTimerFactory = Timer.new, + Future Function(Duration) replayDelay = Future.delayed, }) : _httpClient = httpClient, - _socketFactory = socketFactory; + _socketFactory = socketFactory, + _rateLimitGate = rateLimitGate ?? RelayRateLimitGate(), + _retryTimerFactory = retryTimerFactory, + _replayDelay = replayDelay; final http.Client? _httpClient; final RelaySocketFactory _socketFactory; + final RelayRateLimitGate _rateLimitGate; + final RelayTimerFactory _retryTimerFactory; + final Future Function(Duration) _replayDelay; static const _baseReconnectDelayMs = 1000; static const _maxReconnectDelayMs = 30000; static const _eventBatchMs = 16; static const _reconnectReplaySkewSeconds = 5; + static const _replayBatchSize = 8; + static const _replayInterBatchDelay = Duration(milliseconds: 50); static const _maxRecentDeliveryKeys = 5000; RelaySocket? _socket; final Map _historySubscriptions = {}; final Map _liveSubscriptions = {}; + final Map _pendingClosedRetries = {}; final Map _pendingEvents = {}; final List<_BufferedEvent> _eventBuffer = []; final Set _recentDeliveryKeys = {}; @@ -105,6 +128,9 @@ class RelaySessionNotifier extends Notifier { bool _paused = false; bool _hasConnectedOnce = false; int _connectionGeneration = 0; + final Map _visibleChannelsByOwner = {}; + bool _socketConnected = false; + bool _closedRetryReplayScheduled = false; @override SessionState build() { @@ -158,6 +184,7 @@ class RelaySessionNotifier extends Notifier { if (shouldCloseClient) client.close(); }); if (response.statusCode < 200 || response.statusCode >= 300) { + _activateRateLimitGateFromHttpError(response.body); throw RelayException(response.statusCode, response.body); } final decoded = jsonDecode(response.body); @@ -178,12 +205,30 @@ class RelaySessionNotifier extends Notifier { } } + void _activateRateLimitGateFromHttpError(String body) { + final dynamic decoded; + try { + decoded = jsonDecode(body); + } on FormatException { + return; + } + if (decoded is! Map) return; + final message = decoded['error']; + if (message is! String || + classifyRelayClosed(message) != RelayClosedClass.rateLimited) { + return; + } + _rateLimitGate.activate(parseRateLimitRetrySeconds(message)); + } + /// Fetch historical events matching [filter]. Sends REQ, collects events /// until EOSE, then resolves. One-shot subscription. Future> fetchHistory( NostrFilter filter, { Duration timeout = const Duration(seconds: 8), - }) { + }) async { + if (_rateLimitGate.isActive) await _rateLimitGate.wait(); + if (_disposed) throw StateError('Relay session is disposed'); final subId = _nextSubId('h'); final completer = Completer>(); @@ -214,6 +259,7 @@ class RelaySessionNotifier extends Notifier { void Function(NostrEvent) onEvent, { void Function(String message)? onClosed, }) async { + if (_disposed) throw StateError('Relay session is disposed'); final subId = _nextSubId('l'); final readyCompleter = Completer(); @@ -285,11 +331,35 @@ class RelaySessionNotifier extends Notifier { void debugFlushEventBuffer() => _flushEventBuffer(); @visibleForTesting - void debugHandleConnected() => _handleConnected(_connectionGeneration); + Future debugHandleConnected() => + _handleConnected(_connectionGeneration); + + @visibleForTesting + Future debugReplayLiveSubscriptions() => + _replayLiveSubscriptions(_connectionGeneration); + + @visibleForTesting + void debugDispose() => _dispose(); @visibleForTesting - void debugHandleDisconnected([Object? error]) => - _handleDisconnected(_connectionGeneration, error); + void debugSupersedeConnection() => _connectionGeneration++; + + @visibleForTesting + void debugHandleDisconnected([Object? error]) { + _socketConnected = false; + _handleDisconnected(_connectionGeneration, error); + } + + @visibleForTesting + void debugResetClosedRetriesForDisconnect() { + _socketConnected = false; + _resetAllClosedRetries(); + } + + @visibleForTesting + void debugSetSessionStatus(SessionStatus status) { + _socketConnected = status == SessionStatus.connected; + } @visibleForTesting void debugPauseNow() => _pauseNow(); @@ -302,11 +372,20 @@ class RelaySessionNotifier extends Notifier { void debugAttachSocketForTest(RelaySocket socket) { _socket?.dispose(); _socket = socket; - state = const SessionState(status: SessionStatus.connected); + _socketConnected = true; + } + + /// Registers a visible channel and returns an owner-scoped release callback. + /// The most recently registered owner is prioritized during reconnect replay. + void Function() registerVisibleChannel(String channelId) { + final owner = Object(); + _visibleChannelsByOwner[owner] = channelId; + return () => _visibleChannelsByOwner.remove(owner); } /// Force a reconnect (e.g., returning from background). Future reconnect() async { + _socketConnected = false; await _socket?.disconnect(); _reconnectDelayMs = _baseReconnectDelayMs; final config = ref.read(relayConfigProvider); @@ -321,6 +400,7 @@ class RelaySessionNotifier extends Notifier { void _pauseNow() { _paused = true; + _socketConnected = false; _reconnectTimer?.cancel(); _cancelAllHistory(Exception('App moved to background')); _rejectAllPending(Exception('App moved to background')); @@ -372,18 +452,21 @@ class RelaySessionNotifier extends Notifier { await socket.connect(); } - void _handleConnected(int generation) { + Future _handleConnected(int generation) async { if (_disposed || generation != _connectionGeneration) return; + _socketConnected = true; _hasConnectedOnce = true; _reconnectDelayMs = _baseReconnectDelayMs; state = const SessionState(status: SessionStatus.connected); - _replayLiveSubscriptions(); + await _replayLiveSubscriptions(generation); } void _handleDisconnected(int generation, Object? error) { if (_disposed || generation != _connectionGeneration) return; + _socketConnected = false; _cancelAllHistory(error); _rejectAllPending(error); + _resetAllClosedRetries(); _eventBuffer.clear(); _flushTimer?.cancel(); _flushTimer = null; @@ -413,19 +496,84 @@ class RelaySessionNotifier extends Notifier { /// Replay all live subscriptions after a reconnect, with a time skew to /// catch events that occurred during the disconnect. - void _replayLiveSubscriptions() { - for (final entry in _liveSubscriptions.entries) { - final sub = entry.value; - final since = sub.lastSeenCreatedAt != null - ? sub.lastSeenCreatedAt! - _reconnectReplaySkewSeconds - : null; - final filter = since != null - ? sub.filter.copyWithSince(since) - : sub.filter; - _sendReq(entry.key, filter); + Future _replayLiveSubscriptions(int generation) async { + if (_rateLimitGate.isActive) await _rateLimitGate.wait(); + if (!_isActiveConnection(generation)) return; + + final entries = _liveSubscriptions.entries.toList(); + final visibleChannelId = _visibleChannelsByOwner.isEmpty + ? null + : _visibleChannelsByOwner.values.last; + if (visibleChannelId != null) { + entries.sort((left, right) { + final leftVisible = + left.value.filter.tags['#h']?.contains(visibleChannelId) ?? false; + final rightVisible = + right.value.filter.tags['#h']?.contains(visibleChannelId) ?? false; + if (leftVisible == rightVisible) return 0; + return leftVisible ? -1 : 1; + }); + } + + await _sendReplayBatches(entries, generation); + } + + Future _replayPendingClosedRetries(int generation) async { + if (!_isActiveConnection(generation)) return; + final entries = _pendingClosedRetries.entries + .where((entry) => entry.value.generation == generation) + .map( + (entry) => MapEntry( + entry.key, + entry.value.subscription, + ), + ) + .toList(); + await _sendReplayBatches(entries, generation, pendingClosedRetries: true); + } + + Future _sendReplayBatches( + List> entries, + int generation, { + bool pendingClosedRetries = false, + }) async { + for (var i = 0; i < entries.length; i += _replayBatchSize) { + if (_rateLimitGate.isActive) await _rateLimitGate.wait(); + if (!_isActiveConnection(generation)) return; + final batch = entries.sublist( + i, + min(i + _replayBatchSize, entries.length), + ); + for (final entry in batch) { + if (_liveSubscriptions[entry.key] != entry.value) continue; + if (pendingClosedRetries) { + final pendingRetry = _pendingClosedRetries[entry.key]; + if (pendingRetry?.subscription != entry.value || + pendingRetry?.generation != generation) { + continue; + } + _pendingClosedRetries.remove(entry.key); + } + _sendReq(entry.key, _replayFilter(entry.value)); + } + if (i + _replayBatchSize < entries.length) { + await _replayDelay(_replayInterBatchDelay); + } } } + bool _isActiveConnection(int generation) => + !_disposed && generation == _connectionGeneration; + + NostrFilter _replayFilter(_LiveSubscription subscription) { + final since = subscription.lastSeenCreatedAt; + return since == null + ? subscription.filter + : subscription.filter.copyWithSince( + max(0, since - _reconnectReplaySkewSeconds), + ); + } + void _handleMessage(List data) { if (data.isEmpty) return; final type = data[0] as String; @@ -458,6 +606,7 @@ class RelaySessionNotifier extends Notifier { // Live subscriptions get batched. final liveSub = _liveSubscriptions[subId]; if (liveSub != null) { + _resetClosedRetry(liveSub); // Track last seen timestamp for reconnect replay. if (liveSub.lastSeenCreatedAt == null || event.createdAt > liveSub.lastSeenCreatedAt!) { @@ -485,6 +634,9 @@ class RelaySessionNotifier extends Notifier { // Live subscription: signal ready. final liveSub = _liveSubscriptions[subId]; + if (liveSub != null) { + _resetClosedRetry(liveSub); + } if (liveSub != null && liveSub.readyCompleter != null && !liveSub.readyCompleter!.isCompleted) { @@ -504,9 +656,13 @@ class RelaySessionNotifier extends Notifier { final message = data.length >= 3 && data[2] is String ? data[2] as String : 'subscription closed by relay'; + final closedClass = classifyRelayClosed(message); final historySub = _historySubscriptions.remove(subId); if (historySub != null) { + if (closedClass == RelayClosedClass.rateLimited) { + _rateLimitGate.activate(parseRateLimitRetrySeconds(message)); + } historySub.timeout.cancel(); if (!historySub.completer.isCompleted) { historySub.completer.completeError(Exception(message)); @@ -514,17 +670,87 @@ class RelaySessionNotifier extends Notifier { return; } - final liveSub = _liveSubscriptions.remove(subId); + final liveSub = _liveSubscriptions[subId]; if (liveSub == null) return; - _recentDeliveryKeys.removeWhere((key) => key.startsWith('$subId:')); - final readyCompleter = liveSub.readyCompleter; - if (readyCompleter != null && !readyCompleter.isCompleted) { - readyCompleter.completeError(Exception(message)); + if (closedClass == RelayClosedClass.terminal) { + if (readyCompleter != null && !readyCompleter.isCompleted) { + readyCompleter.completeError(Exception(message)); + } + liveSub.onClosed?.call(message); + _removeLiveSubscription(subId, liveSub); return; } + if (readyCompleter != null && !readyCompleter.isCompleted) { + readyCompleter.complete(); + liveSub.readyCompleter = null; + } + if (liveSub.closedRetryTimer != null) return; + + final attempt = liveSub.closedRetryAttempt; + final backoffMs = attempt >= 5 + ? _maxReconnectDelayMs + : _baseReconnectDelayMs * (1 << attempt); + var delayMs = backoffMs; + if (closedClass == RelayClosedClass.rateLimited) { + final retrySeconds = parseRateLimitRetrySeconds(message); + _rateLimitGate.activate(retrySeconds); + final fallbackMs = + (retrySeconds != null && retrySeconds > 0 + ? min(retrySeconds, RelayRateLimitGate.maxRetrySeconds) + : RelayRateLimitGate.defaultRetrySeconds) * + 1000; + delayMs = max( + backoffMs, + _rateLimitGate.remainingMs() == 0 + ? fallbackMs + : _rateLimitGate.remainingMs(), + ); + } - liveSub.onClosed?.call(message); + liveSub.closedRetryAttempt = attempt + 1; + final retryGeneration = _connectionGeneration; + liveSub.closedRetryTimer = _retryTimerFactory( + Duration(milliseconds: delayMs), + () async { + liveSub.closedRetryTimer = null; + if (!_isActiveConnection(retryGeneration) || + _liveSubscriptions[subId] != liveSub) { + return; + } + if (_rateLimitGate.isActive) await _rateLimitGate.wait(); + if (!_isActiveConnection(retryGeneration) || + _liveSubscriptions[subId] != liveSub || + !_socketConnected) { + return; + } + _pendingClosedRetries[subId] = _ClosedRetry( + subscription: liveSub, + generation: retryGeneration, + ); + _scheduleClosedRetryReplay(retryGeneration); + }, + ); + } + + void _scheduleClosedRetryReplay(int generation) { + if (_closedRetryReplayScheduled) return; + _closedRetryReplayScheduled = true; + scheduleMicrotask(() async { + try { + await _replayPendingClosedRetries(generation); + } finally { + _closedRetryReplayScheduled = false; + _pendingClosedRetries.removeWhere( + (_, retry) => retry.generation != _connectionGeneration, + ); + if (_pendingClosedRetries.values.any( + (retry) => retry.generation == _connectionGeneration, + )) { + _scheduleClosedRetryReplay(_connectionGeneration); + } + } + }); } void _handleOk(List data) { @@ -619,9 +845,41 @@ class RelaySessionNotifier extends Notifier { } void _unsubscribe(String subId) { + final subscription = _liveSubscriptions[subId]; + if (subscription != null) { + _removeLiveSubscription(subId, subscription); + } + _sendClose(subId); + } + + void _removeLiveSubscription(String subId, _LiveSubscription subscription) { + if (_liveSubscriptions[subId] != subscription) return; _liveSubscriptions.remove(subId); + _pendingClosedRetries.remove(subId); + subscription.closedRetryTimer?.cancel(); + subscription.closedRetryTimer = null; _recentDeliveryKeys.removeWhere((key) => key.startsWith('$subId:')); - _sendClose(subId); + } + + void _resetClosedRetry(_LiveSubscription subscription) { + subscription.closedRetryAttempt = 0; + subscription.closedRetryTimer?.cancel(); + subscription.closedRetryTimer = null; + } + + void _cancelAllClosedRetries() { + _pendingClosedRetries.clear(); + for (final subscription in _liveSubscriptions.values) { + subscription.closedRetryTimer?.cancel(); + subscription.closedRetryTimer = null; + } + } + + void _resetAllClosedRetries() { + _pendingClosedRetries.clear(); + for (final subscription in _liveSubscriptions.values) { + _resetClosedRetry(subscription); + } } void _cancelAllHistory(Object? error) { @@ -650,8 +908,18 @@ class RelaySessionNotifier extends Notifier { _reconnectTimer?.cancel(); _flushTimer?.cancel(); _backgroundGraceTimer?.cancel(); + _cancelAllClosedRetries(); + _rateLimitGate.reset(); + _visibleChannelsByOwner.clear(); + _socketConnected = false; _cancelAllHistory(null); _rejectAllPending(null); + final subscriptions = _liveSubscriptions.values.toList(); + _liveSubscriptions.clear(); + for (final subscription in subscriptions) { + subscription.closedRetryTimer?.cancel(); + subscription.closedRetryTimer = null; + } _recentDeliveryKeys.clear(); _socket?.dispose(); _socket = null; diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 899394de23..f95c6fefed 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -229,6 +229,55 @@ Widget _buildTestable({ ); } +Widget _buildNavigationTestable({ + required Channel channelA, + required Channel channelB, + required RelaySessionNotifier relaySession, +}) { + return ProviderScope( + overrides: [ + relaySessionProvider.overrideWith(() => relaySession), + channelMessagesProvider( + channelA.id, + ).overrideWith(() => _FakeMessagesNotifier([], channelId: channelA.id)), + channelMessagesProvider( + channelB.id, + ).overrideWith(() => _FakeMessagesNotifier([], channelId: channelB.id)), + channelTypingProvider( + channelA.id, + ).overrideWith(() => _FakeTypingNotifier([], channelId: channelA.id)), + channelTypingProvider( + channelB.id, + ).overrideWith(() => _FakeTypingNotifier([], channelId: channelB.id)), + channelDetailsProvider( + channelA.id, + ).overrideWith((ref) async => ChannelDetails.fromChannel(channelA)), + channelDetailsProvider( + channelB.id, + ).overrideWith((ref) async => ChannelDetails.fromChannel(channelB)), + channelMembersProvider( + channelA.id, + ).overrideWith((ref) async => const []), + channelMembersProvider( + channelB.id, + ).overrideWith((ref) async => const []), + userCacheProvider.overrideWith(() => _FakeUserCacheNotifier({})), + profileProvider.overrideWith(() => _FakeProfileNotifier()), + channelsProvider.overrideWith( + () => _FakeChannelsNotifier([channelA, channelB]), + ), + relayClientProvider.overrideWithValue( + RelayClient(baseUrl: 'http://localhost:3000'), + ), + savedPrefsProvider.overrideWithValue(_testPrefs), + ], + child: MaterialApp( + theme: AppTheme.light(), + home: ChannelDetailPage(channel: channelA), + ), + ); +} + /// Finder that searches for text within RichText spans. [find.text] only /// matches the top-level text property; this also searches nested TextSpans. Finder findRichText(String text) { @@ -262,6 +311,103 @@ void main() { }); group('ChannelDetailPage', () { + testWidgets( + 'restores the previous channel replay priority after a nested pop', + (tester) async { + final channelA = _channel(id: 'channel-a', name: 'channel A'); + final channelB = _channel(id: 'channel-b', name: 'channel B'); + final socket = _RecordingRelaySocket(); + final relaySession = RelaySessionNotifier(); + relaySession.debugAttachSocketForTest(socket); + + final subscribeB = relaySession.subscribe( + _filterForChannel(channelB.id), + (_) {}, + ); + relaySession.debugHandleMessage(['EOSE', 'l-1']); + await subscribeB; + final subscribeA = relaySession.subscribe( + _filterForChannel(channelA.id), + (_) {}, + ); + relaySession.debugHandleMessage(['EOSE', 'l-2']); + await subscribeA; + + await tester.pumpWidget( + _buildNavigationTestable( + channelA: channelA, + channelB: channelB, + relaySession: relaySession, + ), + ); + await tester.pumpAndSettle(); + + final navigator = Navigator.of( + tester.element(find.byType(ChannelDetailPage)), + ); + navigator.push( + MaterialPageRoute( + builder: (_) => ChannelDetailPage(channel: channelB), + ), + ); + await tester.pumpAndSettle(); + navigator.pop(); + await tester.pumpAndSettle(); + + socket.messages.clear(); + await relaySession.debugReplayLiveSubscriptions(); + + expect(_replayedChannelIds(socket), [channelA.id, channelB.id]); + }, + ); + + testWidgets( + 'keeps replacement channel replay priority after old route disposal', + (tester) async { + final channelA = _channel(id: 'channel-a', name: 'channel A'); + final channelB = _channel(id: 'channel-b', name: 'channel B'); + final socket = _RecordingRelaySocket(); + final relaySession = RelaySessionNotifier(); + relaySession.debugAttachSocketForTest(socket); + + final subscribeA = relaySession.subscribe( + _filterForChannel(channelA.id), + (_) {}, + ); + relaySession.debugHandleMessage(['EOSE', 'l-1']); + await subscribeA; + final subscribeB = relaySession.subscribe( + _filterForChannel(channelB.id), + (_) {}, + ); + relaySession.debugHandleMessage(['EOSE', 'l-2']); + await subscribeB; + + await tester.pumpWidget( + _buildNavigationTestable( + channelA: channelA, + channelB: channelB, + relaySession: relaySession, + ), + ); + await tester.pumpAndSettle(); + + Navigator.of( + tester.element(find.byType(ChannelDetailPage)), + ).pushReplacement( + MaterialPageRoute( + builder: (_) => ChannelDetailPage(channel: channelB), + ), + ); + await tester.pumpAndSettle(); + + socket.messages.clear(); + await relaySession.debugReplayLiveSubscriptions(); + + expect(_replayedChannelIds(socket), [channelB.id, channelA.id]); + }, + ); + testWidgets('debounces same-slot reconnect skeletons before revealing', ( tester, ) async { @@ -2664,9 +2810,12 @@ class _FakeMessagesNotifier extends ChannelMessagesNotifier { List _messages; bool _hasLoadedMessages; - _FakeMessagesNotifier(this._messages, {bool hasLoadedMessages = true}) - : _hasLoadedMessages = hasLoadedMessages, - super(_channelId); + _FakeMessagesNotifier( + this._messages, { + String channelId = _channelId, + bool hasLoadedMessages = true, + }) : _hasLoadedMessages = hasLoadedMessages, + super(channelId); @override AsyncValue> build() => AsyncData(_messages); @@ -2713,7 +2862,8 @@ class _ReconnectingRelaySession extends RelaySessionNotifier { class _FakeTypingNotifier extends ChannelTypingNotifier { final List _entries; - _FakeTypingNotifier(this._entries) : super(_channelId); + _FakeTypingNotifier(this._entries, {String channelId = _channelId}) + : super(channelId); @override List build() => _entries; @@ -2794,6 +2944,42 @@ class _FakeChannelActions extends ChannelActions { } } +class _RecordingRelaySocket extends RelaySocket { + _RecordingRelaySocket() + : super( + wsUrl: 'wss://relay.example', + nsec: null, + onMessage: (_) {}, + onConnected: () {}, + onDisconnected: (_) {}, + ); + + final List> messages = []; + + @override + void send(List payload) => messages.add(payload); + + @override + void dispose() {} +} + +NostrFilter _filterForChannel(String channelId) => NostrFilter( + kinds: EventKind.channelEventKinds, + tags: { + '#h': [channelId], + }, + limit: 0, +); + +List _replayedChannelIds(_RecordingRelaySocket socket) => socket + .messages + .where((message) => message.first == 'REQ') + .map( + (message) => + ((message[2] as Map)['#h'] as List).single as String, + ) + .toList(); + class _TestNavigatorObserver extends NavigatorObserver { int pushCount = 0; diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 79be7a66f7..c128b9be5a 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -52,6 +54,174 @@ void main() { }, ); + test( + 'refreshing an unchanged channel set issues zero new live REQs', + () async { + final session = _FakeRelaySession( + memberships: [ + _membership(_channelA, myPk), + _membership(_channelB, myPk), + ], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'random'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + final initialSubscribeCount = session.totalSubscribeCount; + + await container.read(channelsProvider.notifier).refresh(); + + expect(session.totalSubscribeCount, initialSubscribeCount); + expect(session.unsubscribeCount, 0); + expect(session.subscribeFilters, hasLength(2)); + }, + ); + + test( + 'live subscription diff only removes and adds changed channels', + () async { + final session = _FakeRelaySession( + memberships: [ + _membership(_channelA, myPk), + _membership(_channelB, myPk), + ], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'random'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + session.memberships = [ + _membership(_channelB, myPk), + _membership(_channelD, myPk), + ]; + session.metadata = [ + _meta(id: _channelB, name: 'random'), + _meta(id: _channelD, name: 'support'), + ]; + + await container.read(channelsProvider.notifier).refresh(); + + expect(session.totalSubscribeCount, 3); + expect(session.unsubscribeCount, 1); + expect( + session.subscribeFilters + .map((filter) => filter.tags['#h']!.single) + .toSet(), + {_channelB, _channelD}, + ); + }, + ); + + test( + 'empty channel refresh removes every retained live subscription', + () async { + final session = _FakeRelaySession( + memberships: [ + _membership(_channelA, myPk), + _membership(_channelB, myPk), + ], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'random'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + session.memberships = []; + session.metadata = []; + + await container.read(channelsProvider.notifier).refresh(); + + expect(session.activeChannels, isEmpty); + expect(session.activeSubscriptionCount, 0); + expect(session.unsubscribeCount, 2); + }, + ); + + test( + 'overlapping refreshes retain one live subscription per desired channel', + () async { + final session = _FakeRelaySession( + memberships: [ + _membership(_channelA, myPk), + _membership(_channelB, myPk), + ], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'random'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + session.pauseNextSubscribe(); + session.memberships = [ + _membership(_channelA, myPk), + _membership(_channelB, myPk), + _membership(_channelD, myPk), + ]; + session.metadata = [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'random'), + _meta(id: _channelD, name: 'support'), + ]; + + final firstRefresh = container.read(channelsProvider.notifier).refresh(); + await session.nextSubscribeStarted; + final secondRefresh = container.read(channelsProvider.notifier).refresh(); + session.resumePausedSubscribe(); + await Future.wait([firstRefresh, secondRefresh]); + + expect(session.activeChannels, {_channelA, _channelB, _channelD}); + expect(session.activeSubscriptionCount, 3); + }, + ); + + test( + 'community switch replaces retained live subscriptions on the new relay', + () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'general')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + expect(session.activeChannels, {_channelA}); + + session.setStatus(SessionStatus.disconnected); + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'random')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://new-community.example'); + await Future.delayed(Duration.zero); + session.setStatus(SessionStatus.connected); + await container.read(channelsProvider.future); + await _waitUntil( + () => + session.activeChannels.length == 1 && + session.activeChannels.contains(_channelB), + ); + + expect(session.activeChannels, {_channelB}); + expect(session.activeSubscriptionCount, 1); + expect(session.unsubscribeCount, 1); + }, + ); + test('live channel events update channel lastMessageAt', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -423,6 +593,14 @@ ProviderContainer _buildContainer({required _FakeRelaySession session}) { ); } +Future _waitUntil(bool Function() predicate) async { + for (var i = 0; i < 100; i++) { + if (predicate()) return; + await Future.delayed(Duration.zero); + } + fail('Timed out waiting for asynchronous provider work'); +} + /// Fake [RelaySessionNotifier] that returns canned events from [fetchHistory] /// and records subscribe calls. class _FakeRelaySession extends RelaySessionNotifier { @@ -440,8 +618,40 @@ class _FakeRelaySession extends RelaySessionNotifier { final List historyFilters = []; final List subscribeFilters = []; - final List _listeners = []; + final Map _subscriptions = {}; + int _nextSubscriptionKey = 0; + Completer? _pausedSubscribe; + Completer? _subscribeStarted; int unsubscribeCount = 0; + int totalSubscribeCount = 0; + + Set get activeChannels => { + for (final (filter, _) in _subscriptions.values) ?filter.tags['#h']?.single, + }; + + int get activeSubscriptionCount => _subscriptions.length; + + Future get nextSubscribeStarted async { + final started = _subscribeStarted; + if (started == null) { + throw StateError('No paused subscription is pending'); + } + await started.future; + } + + void pauseNextSubscribe() { + if (_pausedSubscribe != null) { + throw StateError('A subscription is already paused'); + } + _pausedSubscribe = Completer(); + _subscribeStarted = Completer(); + } + + void resumePausedSubscribe() { + final paused = _pausedSubscribe; + if (paused == null) throw StateError('No subscription is paused'); + paused.complete(); + } @override SessionState build() => const SessionState(status: SessionStatus.connected); @@ -483,12 +693,22 @@ class _FakeRelaySession extends RelaySessionNotifier { void Function(NostrEvent) onEvent, { void Function(String message)? onClosed, }) async { + totalSubscribeCount++; subscribeFilters.add(filter); - _listeners.add(onEvent); + final paused = _pausedSubscribe; + if (paused != null) { + _subscribeStarted!.complete(); + await paused.future; + _pausedSubscribe = null; + _subscribeStarted = null; + } + final subscriptionKey = ++_nextSubscriptionKey; + _subscriptions[subscriptionKey] = (filter, onEvent); return () { + final subscription = _subscriptions.remove(subscriptionKey); + if (subscription == null) return; unsubscribeCount++; - subscribeFilters.remove(filter); - _listeners.remove(onEvent); + subscribeFilters.remove(subscription.$1); }; } @@ -498,7 +718,7 @@ class _FakeRelaySession extends RelaySessionNotifier { /// Emit a live event to all subscribers. void emit(NostrEvent event) { - for (final listener in List.of(_listeners)) { + for (final (_, listener) in List.of(_subscriptions.values)) { listener(event); } } diff --git a/mobile/test/shared/relay/relay_closed_policy_test.dart b/mobile/test/shared/relay/relay_closed_policy_test.dart new file mode 100644 index 0000000000..ff78ea89ef --- /dev/null +++ b/mobile/test/shared/relay/relay_closed_policy_test.dart @@ -0,0 +1,45 @@ +import 'package:buzz/shared/relay/relay.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('classifyRelayClosed', () { + const cases = { + 'rate-limited: quota exceeded; retry in 4s': RelayClosedClass.rateLimited, + 'rate-limited: too many concurrent requests': + RelayClosedClass.rateLimited, + 'restricted: access revoked': RelayClosedClass.terminal, + 'auth-required: verification failed': RelayClosedClass.terminal, + 'blocked: community policy': RelayClosedClass.terminal, + 'invalid: malformed filter': RelayClosedClass.terminal, + 'pow: insufficient work': RelayClosedClass.terminal, + 'duplicate: subscription already exists': RelayClosedClass.terminal, + 'unsupported: filter extension': RelayClosedClass.terminal, + 'error: mixed search and channel filter': RelayClosedClass.terminal, + 'error: too many subscriptions': RelayClosedClass.terminal, + 'error: relay temporarily unavailable': RelayClosedClass.retryable, + 'subscription closed by relay': RelayClosedClass.retryable, + }; + + for (final entry in cases.entries) { + test(entry.key, () { + expect(classifyRelayClosed(entry.key), entry.value); + }); + } + }); + + test('parseRateLimitRetrySeconds handles canonical and absent hints', () { + expect( + parseRateLimitRetrySeconds('rate-limited: quota exceeded; retry in 17s'), + 17, + ); + expect(parseRateLimitRetrySeconds('RETRY IN 2S'), 2); + expect(parseRateLimitRetrySeconds('retry in 0s'), 0); + expect(parseRateLimitRetrySeconds('retry in 999999s'), 999999); + final oversizedHint = List.filled(1000, '9').join(); + expect(parseRateLimitRetrySeconds('retry in ${oversizedHint}s'), isNull); + expect( + parseRateLimitRetrySeconds('rate-limited: too many concurrent requests'), + isNull, + ); + }); +} diff --git a/mobile/test/shared/relay/relay_rate_limit_gate_test.dart b/mobile/test/shared/relay/relay_rate_limit_gate_test.dart new file mode 100644 index 0000000000..811d439572 --- /dev/null +++ b/mobile/test/shared/relay/relay_rate_limit_gate_test.dart @@ -0,0 +1,118 @@ +import 'dart:async'; + +import 'package:buzz/shared/relay/relay.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('uses the default for absent and non-positive hints', () { + var now = DateTime.utc(2026); + final timers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => now, + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + + gate.activate(null); + expect(gate.remainingMs(), 10000); + expect(timers.single.duration, const Duration(seconds: 10)); + + now = now.add(const Duration(seconds: 11)); + gate.activate(0); + expect(timers.last.duration, const Duration(seconds: 10)); + }); + + test('clamps large hints to five minutes', () { + final timers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime.utc(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + + gate.activate(999999); + + expect(timers.single.duration, const Duration(seconds: 300)); + expect(gate.remainingMs(), 300000); + }); + + test('overlapping activations only extend the active window', () async { + var now = DateTime.utc(2026); + final timers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => now, + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + + gate.activate(30); + final wait = gate.wait(); + final firstTimer = timers.single; + + now = now.add(const Duration(seconds: 5)); + gate.activate(10); + expect(timers, hasLength(1)); + expect(gate.remainingMs(), 25000); + + gate.activate(40); + expect(timers, hasLength(2)); + expect(firstTimer.isActive, isFalse); + expect(gate.remainingMs(), 40000); + + timers.last.fire(); + await wait; + expect(gate.isActive, isFalse); + expect(gate.remainingMs(), 0); + }); + + test('reset releases waiters and cancels the active timer', () async { + final timers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + + gate.activate(20); + final wait = gate.wait(); + gate.reset(); + + await wait; + expect(timers.single.isActive, isFalse); + expect(gate.isActive, isFalse); + }); +} + +class _ManualTimer implements Timer { + _ManualTimer(this.duration, this._callback); + + final Duration duration; + final void Function() _callback; + bool _active = true; + + void fire() { + if (!_active) return; + _active = false; + _callback(); + } + + @override + void cancel() => _active = false; + + @override + bool get isActive => _active; + + @override + int get tick => _active ? 0 : 1; +} diff --git a/mobile/test/shared/relay/relay_session_test.dart b/mobile/test/shared/relay/relay_session_test.dart index 4647c05e07..b97df0849d 100644 --- a/mobile/test/shared/relay/relay_session_test.dart +++ b/mobile/test/shared/relay/relay_session_test.dart @@ -105,6 +105,178 @@ void main() { ); }); + test('queryRelay arms the rate-limit gate from a 429 retry hint', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + const body = '{"error":"rate-limited: quota exceeded; retry in 4s"}'; + final harness = _queryHarness( + gate: gate, + client: http_testing.MockClient((_) async => http.Response(body, 429)), + ); + addTearDown(harness.container.dispose); + + await expectLater( + harness.session.queryRelay(const []), + throwsA( + isA() + .having((error) => error.statusCode, 'statusCode', 429) + .having((error) => error.body, 'body', body), + ), + ); + + expect(gateTimers.single.duration, const Duration(seconds: 4)); + expect(gate.isActive, isTrue); + }); + + test('queryRelay uses the default gate for a 503 without a hint', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + const body = '{"error":"rate-limited: shared admission unavailable"}'; + final harness = _queryHarness( + gate: gate, + client: http_testing.MockClient((_) async => http.Response(body, 503)), + ); + addTearDown(harness.container.dispose); + + await expectLater( + harness.session.queryRelay(const []), + throwsA( + isA() + .having((error) => error.statusCode, 'statusCode', 503) + .having((error) => error.body, 'body', body), + ), + ); + + expect(gateTimers.single.duration, const Duration(seconds: 10)); + expect(gate.isActive, isTrue); + }); + + test('queryRelay does not arm the gate for a non-rate-limit error', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + const body = '{"error":"not found"}'; + final harness = _queryHarness( + gate: gate, + client: http_testing.MockClient((_) async => http.Response(body, 404)), + ); + addTearDown(harness.container.dispose); + + await expectLater( + harness.session.queryRelay(const []), + throwsA( + isA() + .having((error) => error.statusCode, 'statusCode', 404) + .having((error) => error.body, 'body', body), + ), + ); + + expect(gateTimers, isEmpty); + expect(gate.isActive, isFalse); + }); + + test('queryRelay preserves an error with an unrecognized body', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + const body = 'upstream unavailable'; + final harness = _queryHarness( + gate: gate, + client: http_testing.MockClient((_) async => http.Response(body, 503)), + ); + addTearDown(harness.container.dispose); + + await expectLater( + harness.session.queryRelay(const []), + throwsA( + isA() + .having((error) => error.statusCode, 'statusCode', 503) + .having((error) => error.body, 'body', body), + ), + ); + + expect(gateTimers, isEmpty); + expect(gate.isActive, isFalse); + }); + + test('queryRelay success does not arm the rate-limit gate', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + final harness = _queryHarness( + gate: gate, + client: http_testing.MockClient((_) async => http.Response('[]', 200)), + ); + addTearDown(harness.container.dispose); + + expect(await harness.session.queryRelay(const []), isEmpty); + expect(gateTimers, isEmpty); + expect(gate.isActive, isFalse); + }); + + test('queryRelay does not wait for an active rate-limit gate', () async { + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: _ManualTimer.new, + ); + var requestCount = 0; + final harness = _queryHarness( + gate: gate, + client: http_testing.MockClient((_) async { + requestCount++; + return http.Response('[]', 200); + }), + ); + addTearDown(harness.container.dispose); + // Let the provider's build/dispose churn settle before arming: reading the + // notifier registers `ref.onDispose(_dispose)`, and `_dispose` resets the + // shared gate. Arming before that settles leaves the gate disarmed by the + // time the request runs, which makes this row pass for the wrong reason. + await pumpEventQueue(); + gate.activate(4); + expect(gate.isActive, isTrue); + + final query = harness.session.queryRelay(const []); + await Future.delayed(Duration.zero); + + expect(requestCount, 1); + expect(await query, isEmpty); + // Still armed: the read must neither wait on the gate nor clear it. + expect(gate.isActive, isTrue); + }); + test( 'history timeout rejects instead of returning partial empty data', () async { @@ -328,7 +500,7 @@ void main() { unsubscribe(); }); - test('live subscribe fails when relay closes before ready', () async { + test('terminal CLOSED fails a live subscribe before ready', () async { final session = RelaySessionNotifier(); const filter = NostrFilter(kinds: [EventKind.agentObserverFrame], limit: 0); @@ -352,32 +524,601 @@ void main() { }); test( - 'live onClosed callback runs when relay closes an open subscription', + 'retryable CLOSED before EOSE retains and retries the live sub', () async { - final session = RelaySessionNotifier(); - final closedMessages = []; - const filter = NostrFilter( - kinds: [EventKind.agentObserverFrame], - limit: 0, + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, ); + session.debugAttachSocketForTest(socket); - final subscribe = session.subscribe( - filter, - (_) {}, - onClosed: closedMessages.add, + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: relay overloaded']); + final unsubscribe = await subscribe; + + expect(timers.single.duration, const Duration(seconds: 1)); + timers.single.fire(); + await Future.delayed(Duration.zero); + + expect(_reqs(socket).where((req) => req[1] == 'l-1'), hasLength(2)); + unsubscribe(); + }, + ); + + test('CLOSED retries back off and reset after EOSE', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 1)); + timers.last.fire(); + await Future.delayed(Duration.zero); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 2)); + + session.debugHandleMessage(['EOSE', 'l-1']); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 1)); + unsubscribe(); + }); + + test('CLOSED retry backoff saturates before a high-attempt shift', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + + for (var attempt = 0; attempt < 100; attempt++) { + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect( + timers.last.duration, + attempt >= 5 + ? const Duration(seconds: 30) + : Duration(seconds: 1 << attempt), ); + timers.last.fire(); + await Future.delayed(Duration.zero); + } + + unsubscribe(); + }); + + test('CLOSED retries reset after a delivered event', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + timers.last.fire(); + await Future.delayed(Duration.zero); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 2)); + + session.debugHandleMessage([ + 'EVENT', + 'l-1', + _event(createdAt: 30).toJson(), + ]); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 1)); + unsubscribe(); + }); + + test('CLOSED retries reset after disconnect and reconnect', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + timers.last.fire(); + await Future.delayed(Duration.zero); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 2)); + + session.debugResetClosedRetriesForDisconnect(); + expect(timers.last.isActive, isFalse); + session.debugSetSessionStatus(SessionStatus.connected); + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + expect(timers.last.duration, const Duration(seconds: 1)); + unsubscribe(); + }); + + test('a CLOSED retry timer does not send while disconnected', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + final requestCount = _reqs(socket).length; + + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + session.debugSetSessionStatus(SessionStatus.reconnecting); + timers.single.fire(); + await Future.delayed(Duration.zero); + + expect(_reqs(socket), hasLength(requestCount)); + unsubscribe(); + }); + + test('terminal CLOSED removes a live sub without retrying it', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + await subscribe; + + session.debugHandleMessage(['CLOSED', 'l-1', 'restricted: access revoked']); + await session.debugReplayLiveSubscriptions(); + + expect(timers, isEmpty); + expect(_reqs(socket).where((req) => req[1] == 'l-1'), hasLength(1)); + }); + + test('unsubscribe and dispose cancel CLOSED retry timers', () async { + final timers = <_ManualTimer>[]; + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + timers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + + final firstSubscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await firstSubscribe; + session.debugHandleMessage(['CLOSED', 'l-1', 'error: transient']); + final unsubscribeTimer = timers.last; + unsubscribe(); + expect(unsubscribeTimer.isActive, isFalse); + + final secondSubscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-2']); + await secondSubscribe; + session.debugHandleMessage(['CLOSED', 'l-2', 'error: transient']); + final disposeTimer = timers.last; + session.debugDispose(); + expect(disposeTimer.isActive, isFalse); + }); + + test('rate-limited live CLOSED honours the gate floor', () async { + final retryTimers = <_ManualTimer>[]; + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + final session = RelaySessionNotifier( + rateLimitGate: gate, + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + retryTimers.add(timer); + return timer; + }, + ); + final socket = _RecordingRelaySocket(); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + + session.debugHandleMessage([ + 'CLOSED', + 'l-1', + 'rate-limited: quota exceeded; retry in 4s', + ]); + + expect( + retryTimers.single.duration.inMilliseconds, + inInclusiveRange(3990, 4000), + ); + expect(gateTimers.single.duration, const Duration(seconds: 4)); + unsubscribe(); + }); + + test( + 'rate-limited CLOSED retry does not survive a superseded connection', + () async { + final retryTimers = <_ManualTimer>[]; + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + rateLimitGate: gate, + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + retryTimers.add(timer); + return timer; + }, + ); + session.debugAttachSocketForTest(socket); + final subscribe = session.subscribe(_channelFilter, (_) {}); session.debugHandleMessage(['EOSE', 'l-1']); final unsubscribe = await subscribe; + socket.messages.clear(); + session.debugHandleMessage([ 'CLOSED', 'l-1', - 'restricted: no longer valid', + 'rate-limited: quota exceeded; retry in 4s', ]); + retryTimers.single.fire(); + await Future.delayed(Duration.zero); + expect(_reqs(socket), isEmpty); + + session.debugSupersedeConnection(); + final replacementReplay = session.debugReplayLiveSubscriptions(); + await Future.delayed(Duration.zero); + gateTimers.single.fire(); + await replacementReplay; + await Future.delayed(Duration.zero); - expect(closedMessages, ['restricted: no longer valid']); + expect(_reqs(socket).where((req) => req[1] == 'l-1'), hasLength(1)); unsubscribe(); }, ); + + test('simultaneous rate-limited CLOSED retries are replay-paced', () async { + final retryTimers = <_ManualTimer>[]; + final gateTimers = <_ManualTimer>[]; + final replayDelays = []; + final replayDelayCompleters = >[]; + final gate = RelayRateLimitGate( + now: () => DateTime(2026), + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + rateLimitGate: gate, + retryTimerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + retryTimers.add(timer); + return timer; + }, + replayDelay: (duration) { + replayDelays.add(duration); + final completer = Completer(); + replayDelayCompleters.add(completer); + return completer.future; + }, + ); + session.debugAttachSocketForTest(socket); + + for (var i = 0; i < 30; i++) { + final subscribe = session.subscribe( + _filterForChannel('channel-$i'), + (_) {}, + ); + session.debugHandleMessage(['EOSE', 'l-${i + 1}']); + await subscribe; + } + socket.messages.clear(); + + for (var i = 0; i < 30; i++) { + session.debugHandleMessage([ + 'CLOSED', + 'l-${i + 1}', + 'rate-limited: quota exceeded; retry in 4s', + ]); + } + for (final timer in retryTimers) { + timer.fire(); + } + await Future.delayed(Duration.zero); + expect(_reqs(socket), isEmpty); + + gateTimers.single.fire(); + await Future.delayed(Duration.zero); + expect(_reqs(socket), hasLength(8)); + expect(replayDelays, [const Duration(milliseconds: 50)]); + + for (final expectedCount in [16, 24, 30]) { + replayDelayCompleters.last.complete(); + await Future.delayed(Duration.zero); + expect(_reqs(socket), hasLength(expectedCount)); + } + expect(replayDelays, [ + const Duration(milliseconds: 50), + const Duration(milliseconds: 50), + const Duration(milliseconds: 50), + ]); + session.debugDispose(); + }); + + test('active rate-limit gate does not delay a new live subscribe', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier(rateLimitGate: gate); + session.debugAttachSocketForTest(socket); + gate.activate(4); + + final subscribe = session.subscribe(_channelFilter, (_) {}); + + expect(_reqs(socket), hasLength(1)); + expect(gateTimers.single.duration, const Duration(seconds: 4)); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + unsubscribe(); + session.debugDispose(); + }); + + test('rate-limited history CLOSED gates the next REQ', () async { + final gateTimers = <_ManualTimer>[]; + final gate = RelayRateLimitGate( + timerFactory: (duration, callback) { + final timer = _ManualTimer(duration, callback); + gateTimers.add(timer); + return timer; + }, + ); + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier(rateLimitGate: gate); + session.debugAttachSocketForTest(socket); + + final first = session.fetchHistory(_channelFilter); + session.debugHandleMessage([ + 'CLOSED', + 'h-1', + 'rate-limited: quota exceeded; retry in 4s', + ]); + await expectLater(first, throwsException); + + final second = session.fetchHistory(_channelFilter); + await Future.delayed(Duration.zero); + expect(_reqs(socket), hasLength(1)); + expect(gateTimers.single.duration, const Duration(seconds: 4)); + + gateTimers.single.fire(); + await Future.delayed(Duration.zero); + expect(_reqs(socket), hasLength(2)); + session.debugHandleMessage(['EOSE', 'h-2']); + await second; + }); + + test( + 'visible channel owners restore and ignore out-of-order release', + () async { + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier(); + session.debugAttachSocketForTest(socket); + const channelIds = ['channel-a', 'channel-b', 'channel-c']; + + for (var i = 0; i < channelIds.length; i++) { + final subscribe = session.subscribe( + _filterForChannel(channelIds[i]), + (_) {}, + ); + session.debugHandleMessage(['EOSE', 'l-${i + 1}']); + await subscribe; + } + + final releaseA = session.registerVisibleChannel('channel-a'); + final releaseB = session.registerVisibleChannel('channel-b'); + final releaseC = session.registerVisibleChannel('channel-c'); + releaseB(); + socket.messages.clear(); + await session.debugReplayLiveSubscriptions(); + expect(_replayedChannelIds(socket).first, 'channel-c'); + + releaseC(); + socket.messages.clear(); + await session.debugReplayLiveSubscriptions(); + expect(_replayedChannelIds(socket).first, 'channel-a'); + + releaseB(); + releaseA(); + }, + ); + + test('replay is visible-first and batched eight at a time', () async { + final replayDelays = []; + final replayDelayCompleter = Completer(); + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + replayDelay: (duration) { + replayDelays.add(duration); + return replayDelayCompleter.future; + }, + ); + session.debugAttachSocketForTest(socket); + + for (var i = 0; i < 9; i++) { + final channelId = i == 8 ? _visibleChannelId : 'channel-$i'; + final subscribe = session.subscribe(_filterForChannel(channelId), (_) {}); + session.debugHandleMessage(['EOSE', 'l-${i + 1}']); + await subscribe; + } + socket.messages.clear(); + final releaseVisibleChannel = session.registerVisibleChannel( + _visibleChannelId, + ); + + final replay = session.debugReplayLiveSubscriptions(); + await Future.delayed(Duration.zero); + + final firstBatch = _reqs(socket); + expect(firstBatch, hasLength(8)); + expect((firstBatch.first[2] as Map)['#h'], [ + _visibleChannelId, + ]); + expect(replayDelays, [const Duration(milliseconds: 50)]); + + replayDelayCompleter.complete(); + await replay; + expect(_reqs(socket), hasLength(9)); + releaseVisibleChannel(); + }); + + test( + 'replay generation guard bails after a connection is superseded', + () async { + final replayDelayCompleter = Completer(); + final socket = _RecordingRelaySocket(); + final session = RelaySessionNotifier( + replayDelay: (_) => replayDelayCompleter.future, + ); + session.debugAttachSocketForTest(socket); + + for (var i = 0; i < 9; i++) { + final subscribe = session.subscribe( + _filterForChannel('channel-$i'), + (_) {}, + ); + session.debugHandleMessage(['EOSE', 'l-${i + 1}']); + await subscribe; + } + socket.messages.clear(); + + final replay = session.debugReplayLiveSubscriptions(); + await Future.delayed(Duration.zero); + expect(_reqs(socket), hasLength(8)); + + session.debugSupersedeConnection(); + replayDelayCompleter.complete(); + await replay; + + expect(_reqs(socket), hasLength(8)); + }, + ); + + test('live onClosed callback runs only for a terminal CLOSED', () async { + final session = RelaySessionNotifier(); + final closedMessages = []; + const filter = NostrFilter(kinds: [EventKind.agentObserverFrame], limit: 0); + + final subscribe = session.subscribe( + filter, + (_) {}, + onClosed: closedMessages.add, + ); + session.debugHandleMessage(['EOSE', 'l-1']); + final unsubscribe = await subscribe; + session.debugHandleMessage([ + 'CLOSED', + 'l-1', + 'error: temporarily unavailable', + ]); + expect(closedMessages, isEmpty); + session.debugHandleMessage([ + 'CLOSED', + 'l-1', + 'restricted: no longer valid', + ]); + + expect(closedMessages, ['restricted: no longer valid']); + unsubscribe(); + }); +} + +class _QueryHarness { + final ProviderContainer container; + final RelaySessionNotifier session; + + _QueryHarness({required this.container, required this.session}); +} + +_QueryHarness _queryHarness({ + required RelayRateLimitGate gate, + required http.Client client, +}) { + final session = RelaySessionNotifier(httpClient: client, rateLimitGate: gate); + final container = ProviderContainer( + overrides: [ + relaySessionProvider.overrideWith(() => session), + relayConfigProvider.overrideWith( + () => _FakeRelayConfigNotifier( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + ), + ), + ], + ); + container.read(relaySessionProvider); + return _QueryHarness(container: container, session: session); } class _FakeAuthNotifier extends AuthNotifier { @@ -437,11 +1178,11 @@ class _FakeRelayConfigNotifier extends RelayConfigNotifier { RelayConfig build() => RelayConfig(baseUrl: _baseUrl, nsec: _nsec); } -NostrEvent _event() { - return const NostrEvent( +NostrEvent _event({int createdAt = 20}) { + return NostrEvent( id: 'event-1', pubkey: 'alice', - createdAt: 20, + createdAt: createdAt, kind: EventKind.streamMessageV2, tags: [ ['h', _channelId], @@ -450,3 +1191,72 @@ NostrEvent _event() { sig: 'sig', ); } + +const _visibleChannelId = '99999999-9999-4999-8999-999999999999'; +const _channelFilter = NostrFilter( + kinds: EventKind.channelEventKinds, + tags: { + '#h': [_channelId], + }, + limit: 0, +); + +NostrFilter _filterForChannel(String channelId) => NostrFilter( + kinds: EventKind.channelEventKinds, + tags: { + '#h': [channelId], + }, + limit: 0, +); + +List _replayedChannelIds(_RecordingRelaySocket socket) => _reqs(socket) + .map( + (message) => + ((message[2] as Map)['#h'] as List).single as String, + ) + .toList(); + +List> _reqs(_RecordingRelaySocket socket) => + socket.messages.where((message) => message.first == 'REQ').toList(); + +class _RecordingRelaySocket extends RelaySocket { + _RecordingRelaySocket() + : super( + wsUrl: 'wss://relay.example', + nsec: null, + onMessage: (_) {}, + onConnected: () {}, + onDisconnected: (_) {}, + ); + + final List> messages = []; + + @override + void send(List payload) => messages.add(payload); + + @override + void dispose() {} +} + +class _ManualTimer implements Timer { + _ManualTimer(this.duration, this._callback); + + final Duration duration; + final void Function() _callback; + bool _active = true; + + void fire() { + if (!_active) return; + _active = false; + _callback(); + } + + @override + void cancel() => _active = false; + + @override + bool get isActive => _active; + + @override + int get tick => _active ? 0 : 1; +}