From 03f03e8185f8348a041c1a0f74bb29063124c769 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Jul 2026 12:01:56 +1000 Subject: [PATCH 01/12] feat(desktop): pin managed agents to a home relay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop agent records from floating to whichever workspace is active — the relay URL becomes the backend-side ownership key for agents (Phase 1 of lazy multi-workspace agents): - Stamp the resolved workspace relay onto records at create, on an explicit relay edit that clears the field, and on snapshot/team imports, instead of persisting blank. - Migrate legacy blank-relay records on the first apply_workspace after boot. Behavior-preserving: blank resolved to exactly that relay at boot restore. - Add a shared relay-URL normalizer (trailing slash, scheme/host case) and hash the normalized resolved relay in the spawn-config hash so cosmetic URL differences no longer trip the restart badge. - Add a rebind_agent_relay command, invoked by updateCommunity when a community's relay URL is edited, re-pinning records from the old URL onto the new one so those agents don't orphan. effective_agent_relay_url keeps its blank-to-workspace fallback as defense-in-depth for records that escaped stamping. The pure record mutations live in the new managed_agents::relay_pinning module. Tested: cargo test --manifest-path desktop/src-tauri/Cargo.toml (1420 passed), clippy -D warnings, rustfmt, desktop pnpm test (2920 passed), tsc, biome. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- desktop/src-tauri/src/app_state.rs | 3 + .../src-tauri/src/commands/agent_models.rs | 13 +- desktop/src-tauri/src/commands/agents.rs | 19 +- .../src/commands/personas/snapshot/import.rs | 2 +- .../src-tauri/src/commands/team_snapshot.rs | 2 +- desktop/src-tauri/src/commands/workspace.rs | 77 ++++++- desktop/src-tauri/src/lib.rs | 1 + desktop/src-tauri/src/managed_agents/mod.rs | 2 + .../src/managed_agents/relay_pinning.rs | 190 ++++++++++++++++++ .../src/managed_agents/spawn_hash.rs | 20 +- .../src/managed_agents/spawn_hash/tests.rs | 32 +++ desktop/src-tauri/src/relay.rs | 94 ++++++++- .../features/communities/useCommunities.tsx | 18 ++ desktop/src/shared/api/tauriManagedAgents.ts | 13 ++ desktop/src/testing/e2eBridge.ts | 3 + 15 files changed, 460 insertions(+), 29 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/relay_pinning.rs diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 3d910c3133..71f155b18a 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -38,6 +38,8 @@ pub struct AppState { /// records. Disabled by the agent-managed profiles experiment so an agent's /// own profile updates are not overwritten on start or restore. pub managed_agent_profile_reconcile_enabled: AtomicBool, + /// One-shot gate: first `apply_workspace` runs `pin_blank_agent_relays`. + pub agent_relay_stamp_pending: AtomicBool, /// Shared shutdown signal checked by launch-time agent restoration. pub shutdown_started: AtomicBool, /// Serializes the restore spawn/register transition with shutdown cleanup, @@ -201,6 +203,7 @@ pub fn build_app_state() -> AppState { relay_url_override: Mutex::new(None), managed_agent_restore_pending: AtomicBool::new(false), managed_agent_profile_reconcile_enabled: AtomicBool::new(true), + agent_relay_stamp_pending: AtomicBool::new(true), shutdown_started: AtomicBool::new(false), managed_agent_restore_transition: Mutex::new(()), identity_mutation: Mutex::new(()), diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 97772b12a5..559b57cacb 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -819,11 +819,16 @@ pub async fn update_managed_agent( // turn_timeout_seconds is intentionally not applied here — // BUZZ_ACP_TURN_TIMEOUT is deprecated and ignored by the harness. // Use idle_timeout_seconds or max_turn_duration_seconds instead. - // Store the relay override exactly as supplied (trimmed). An explicit - // value pins the agent; empty falls back to the workspace relay at - // read-time. A name-only edit (relay_url == None) leaves the pin intact. + // Re-pin the relay on an explicit edit. An explicit value pins the + // agent there (trimmed, as supplied); clearing the field re-pins to + // the active workspace relay — blank is never persisted, so a record + // can't float to whichever workspace is active at a later read. A + // name-only edit (relay_url == None) leaves the pin intact. if let Some(relay_url) = input.relay_url { - record.relay_url = relay_url.trim().to_string(); + record.relay_url = crate::relay::effective_agent_relay_url( + &relay_url, + &relay_ws_url_with_override(&state), + ); } if let Some(acp_command) = input.acp_command { record.acp_command = acp_command; diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 6ebe6b7c4a..940427258a 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -468,15 +468,16 @@ pub async fn create_managed_agent( .to_bech32() .map_err(|error| format!("failed to encode private key: {error}"))?; - // Store the relay override exactly as supplied (trimmed). An explicit - // value pins the agent; empty stays empty and resolves to the active - // workspace relay at read-time. Uniform for Local and Provider. - let resolved_relay_url = input - .relay_url - .as_deref() - .map(str::trim) - .unwrap_or("") - .to_string(); + // Pin the agent to its home relay at create. An explicit value wins + // (trimmed, as supplied); a blank input is stamped with the active + // workspace relay so the record never floats to whichever workspace + // is active at a later read. Uniform for Local and Provider. + // (`effective_agent_relay_url`'s blank fallback remains as + // defense-in-depth for legacy records that predate stamping.) + let resolved_relay_url = crate::relay::effective_agent_relay_url( + input.relay_url.as_deref().unwrap_or(""), + &relay_ws_url_with_override(&state), + ); (keys, private_key_nsec, pubkey, resolved_relay_url, input) }; diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index 9e2b8b3921..8e83daaf6c 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -434,7 +434,7 @@ pub async fn confirm_agent_snapshot_import( persona_id: Some(persona_id.clone()), private_key_nsec: private_key_nsec.clone(), auth_tag: auth_tag.clone(), - relay_url: String::new(), // resolves to workspace relay at runtime + relay_url: relay_ws_url_with_override(&state), // pinned to the importing workspace avatar_url: effective_avatar.clone(), // Machine-local commands: derive from the runtime catalog at // spawn time — never manufacture from snapshot data. diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index dc376ca064..6bdd59768d 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -555,7 +555,7 @@ pub async fn confirm_team_snapshot_import( persona_id: Some(definition.id.clone()), private_key_nsec: private_key_nsec.clone(), auth_tag: auth_tag.clone(), - relay_url: String::new(), + relay_url: relay_ws_url_with_override(&state), // pinned to the importing workspace avatar_url: effective_avatar_url.clone(), acp_command: crate::managed_agents::DEFAULT_ACP_COMMAND.to_string(), agent_command: String::new(), diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index cb17ad608e..6ae4543744 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -5,8 +5,9 @@ use tauri::{AppHandle, Emitter, Manager, State}; use crate::app_state::AppState; use crate::managed_agents::{ - effective_repos_dir, ensure_repos_symlink, nest_dir, restore_managed_agents_on_launch, - try_regenerate_nest, write_persisted_repos_dir, + effective_repos_dir, ensure_repos_symlink, load_managed_agents, nest_dir, + rebind_agent_relay_urls, restore_managed_agents_on_launch, save_managed_agents, + stamp_blank_agent_relay_urls, try_regenerate_nest, write_persisted_repos_dir, }; use crate::relay; @@ -141,7 +142,7 @@ pub async fn apply_workspace( // ── Apply all state changes (nothing below can fail) ────────────────── { let mut override_guard = state.relay_url_override.lock().map_err(|e| e.to_string())?; - *override_guard = Some(relay_url); + *override_guard = Some(relay_url.clone()); } if let Some(keys) = parsed_keys { @@ -156,6 +157,22 @@ pub async fn apply_workspace( .managed_agent_profile_reconcile_enabled .store(!agent_managed_profiles.unwrap_or(false), Ordering::Release); + // ── One-shot legacy migration (non-fatal) ───────────────────────────── + // Pin any blank-relay agent record to the first workspace applied + // after boot — exactly what blank would have resolved to at boot + // restore, so this is behavior-preserving while stopping the record + // from floating to a later workspace switch. Runs before the restore + // trigger below so restored agents spawn from stamped records. A + // failure is logged and retried on the next boot. + if state + .agent_relay_stamp_pending + .swap(false, Ordering::AcqRel) + { + if let Err(error) = pin_blank_agent_relays(&app, &state, &relay_url) { + eprintln!("buzz-desktop: blank agent relay migration failed: {error}"); + } + } + // ── Filesystem side-effect (non-fatal) ──────────────────────────────── // Persist the *effective* repos_dir (None when the candidate failed // validation) for the backend to read at boot, then re-point REPOS to @@ -233,3 +250,57 @@ pub async fn apply_workspace( Ok(()) } + +/// Stamp legacy blank-relay agent records with the applied workspace relay. +/// +/// Store-lock + load/save wrapper around [`stamp_blank_agent_relay_urls`]; +/// returns the number of records stamped. +fn pin_blank_agent_relays( + app: &AppHandle, + state: &AppState, + relay_url: &str, +) -> Result { + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let mut records = load_managed_agents(app)?; + let changed = stamp_blank_agent_relay_urls(&mut records, relay_url); + if changed > 0 { + save_managed_agents(app, &records)?; + } + Ok(changed) +} + +/// Re-pin managed-agent records from `old_relay_url` onto `new_relay_url`. +/// +/// Called by the frontend when a community's relay URL is edited. Every agent +/// record is pinned to its home relay, so without this rebind the edited +/// community's agents would stay pinned to — and orphan on — the old URL. +/// Matching is normalized (trailing slash, scheme/host case). Returns the +/// number of records rebound. +#[tauri::command] +pub async fn rebind_agent_relay( + old_relay_url: String, + new_relay_url: String, + app: AppHandle, +) -> Result { + tokio::task::spawn_blocking(move || { + if new_relay_url.trim().is_empty() { + return Err("new relay URL is required".to_string()); + } + let state = app.state::(); + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let mut records = load_managed_agents(&app)?; + let changed = rebind_agent_relay_urls(&mut records, &old_relay_url, &new_relay_url); + if changed > 0 { + save_managed_agents(&app, &records)?; + } + Ok(changed) + }) + .await + .map_err(|e| format!("spawn_blocking failed: {e}"))? +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index da1ea50f7c..fdefbbbe02 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -938,6 +938,7 @@ pub fn run() { confirm_pairing_sas, cancel_pairing, apply_workspace, + rebind_agent_relay, validate_repos_dir, get_active_workspace, fetch_workspace_icon, diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 3a9c66b1ab..98f91df89a 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -21,6 +21,7 @@ mod process_lifecycle; pub(crate) mod readiness; pub(crate) mod reconcile; mod relay_mesh; +mod relay_pinning; mod repos; mod restore; pub mod retention; @@ -60,6 +61,7 @@ pub(crate) use readiness::{ agent_readiness, resolve_effective_agent_env, AgentReadiness, Requirement, }; pub use relay_mesh::*; +pub(crate) use relay_pinning::{rebind_agent_relay_urls, stamp_blank_agent_relay_urls}; pub use repos::{ effective_repos_dir, ensure_repos_symlink, resolve_repos_at_boot, validate_repos_dir, write_persisted_repos_dir, diff --git a/desktop/src-tauri/src/managed_agents/relay_pinning.rs b/desktop/src-tauri/src/managed_agents/relay_pinning.rs new file mode 100644 index 0000000000..609a5ba684 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/relay_pinning.rs @@ -0,0 +1,190 @@ +//! Relay-pinning maintenance for managed-agent records. +//! +//! Every agent record is pinned to a home relay URL (the backend-side +//! ownership key): the relay is stamped at create, legacy blank records are +//! stamped on the first `apply_workspace` after boot, and pins follow a +//! community's relay-URL edit via rebind. These are the pure record +//! mutations behind those flows; the store-lock + load/save wrappers live in +//! `commands::workspace`. + +use super::types::ManagedAgentRecord; +use crate::util::now_iso; + +/// Stamp every keyed record whose `relay_url` is blank with `relay_url`, +/// pinning legacy floating records to a home relay. +/// +/// Run against the first workspace applied after boot, this is +/// behavior-preserving: blank resolved to exactly that relay at boot restore +/// (`effective_agent_relay_url`). Once stamped, the record keeps spawning, +/// publishing, and reconciling on its own relay no matter which workspace is +/// active later. Key-less definitions (templates) are left alone. Returns how +/// many records changed. +pub(crate) fn stamp_blank_agent_relay_urls( + records: &mut [ManagedAgentRecord], + relay_url: &str, +) -> usize { + let relay_url = relay_url.trim(); + if relay_url.is_empty() { + return 0; + } + let mut changed = 0; + for record in records.iter_mut() { + if record.pubkey.is_empty() || !record.relay_url.trim().is_empty() { + continue; + } + record.relay_url = relay_url.to_string(); + record.updated_at = now_iso(); + changed += 1; + } + changed +} + +/// Re-pin every keyed record pinned to `old_relay_url` onto `new_relay_url`. +/// +/// Used when a community's relay URL is edited: records are pinned to their +/// home relay, so without the rebind the community's agents would orphan on +/// the old URL. Matching is normalized (`relay_urls_equivalent` — trailing +/// slash, scheme/host case); blank records are not touched (they belong to +/// the blank-relay migration, not to `old_relay_url`). Returns how many +/// records changed. +pub(crate) fn rebind_agent_relay_urls( + records: &mut [ManagedAgentRecord], + old_relay_url: &str, + new_relay_url: &str, +) -> usize { + let new_relay_url = new_relay_url.trim(); + if new_relay_url.is_empty() { + return 0; + } + let mut changed = 0; + for record in records.iter_mut() { + if record.pubkey.is_empty() + || record.relay_url.trim().is_empty() + || record.relay_url == new_relay_url + || !crate::relay::relay_urls_equivalent(&record.relay_url, old_relay_url) + { + continue; + } + record.relay_url = new_relay_url.to_string(); + record.updated_at = now_iso(); + changed += 1; + } + changed +} + +#[cfg(test)] +mod tests { + use super::{rebind_agent_relay_urls, stamp_blank_agent_relay_urls, ManagedAgentRecord}; + + fn record_with_relay(pubkey: &str, relay_url: &str) -> ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "{pubkey}", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "{relay_url}", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }}"# + )) + .expect("sample record") + } + + // ── stamp_blank_agent_relay_urls ───────────────────────────────────────── + + #[test] + fn stamp_pins_blank_records_and_leaves_pinned_ones() { + let mut records = vec![ + record_with_relay("agent-blank", ""), + record_with_relay("agent-whitespace", " "), + record_with_relay("agent-pinned", "wss://relay-other.example"), + ]; + + let changed = stamp_blank_agent_relay_urls(&mut records, "wss://relay-home.example"); + + assert_eq!(changed, 2); + assert_eq!(records[0].relay_url, "wss://relay-home.example"); + assert_eq!(records[1].relay_url, "wss://relay-home.example"); + // An existing pin is ownership — stamping must never overwrite it. + assert_eq!(records[2].relay_url, "wss://relay-other.example"); + } + + #[test] + fn stamp_is_idempotent_once_records_are_pinned() { + // Second boot (or a redundant call) finds nothing blank: exactly-once + // semantics come from the records themselves, not just the app flag. + let mut records = vec![record_with_relay("agent-blank", "")]; + stamp_blank_agent_relay_urls(&mut records, "wss://relay-a.example"); + + let changed = stamp_blank_agent_relay_urls(&mut records, "wss://relay-b.example"); + + assert_eq!(changed, 0); + assert_eq!(records[0].relay_url, "wss://relay-a.example"); + } + + #[test] + fn stamp_ignores_blank_workspace_relay_and_definitions() { + // A blank stamp value would just re-create the floating record; a + // key-less definition (template) has no home relay to pin. + let mut records = vec![ + record_with_relay("agent-blank", ""), + record_with_relay("", ""), + ]; + + assert_eq!(stamp_blank_agent_relay_urls(&mut records, " "), 0); + assert_eq!( + stamp_blank_agent_relay_urls(&mut records, "wss://relay.example"), + 1 + ); + assert_eq!(records[1].relay_url, "", "definition must stay untouched"); + } + + // ── rebind_agent_relay_urls ────────────────────────────────────────────── + + #[test] + fn rebind_moves_normalized_matches_only() { + let mut records = vec![ + // Cosmetic mismatch with the old URL — still the same relay. + record_with_relay("agent-slash", "WSS://Relay-Old.Example/"), + record_with_relay("agent-exact", "wss://relay-old.example"), + record_with_relay("agent-other", "wss://relay-other.example"), + record_with_relay("agent-blank", ""), + ]; + + let changed = rebind_agent_relay_urls( + &mut records, + "wss://relay-old.example", + "wss://relay-new.example", + ); + + assert_eq!(changed, 2); + assert_eq!(records[0].relay_url, "wss://relay-new.example"); + assert_eq!(records[1].relay_url, "wss://relay-new.example"); + assert_eq!(records[2].relay_url, "wss://relay-other.example"); + // Blank is the migration's job — rebinding it would pin a floating + // record to a community it may never have belonged to. + assert_eq!(records[3].relay_url, ""); + } + + #[test] + fn rebind_rejects_blank_target_and_skips_already_bound() { + let mut records = vec![record_with_relay("agent-a", "wss://relay-old.example")]; + assert_eq!( + rebind_agent_relay_urls(&mut records, "wss://relay-old.example", " "), + 0 + ); + assert_eq!(records[0].relay_url, "wss://relay-old.example"); + + // Records already carrying the exact target value are not counted. + let mut records = vec![record_with_relay("agent-a", "wss://relay.example")]; + assert_eq!( + rebind_agent_relay_urls(&mut records, "wss://relay.example", "wss://relay.example"), + 0 + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 63a57ce5da..093c3068d6 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -15,9 +15,11 @@ //! apply on a plain restart and are hashed via the same prospective //! re-snapshot. Harness command, args/mcp, env layering, and the record //! fields the spawn env writes read are hashed as spawn resolves them. -//! - The relay URL is hashed in resolved form (`effective_agent_relay_url`): -//! a record with a blank relay spawns against the active workspace relay, -//! so a workspace relay change means a restart would change what runs. +//! - The relay URL is hashed in resolved, normalized form +//! (`effective_agent_relay_url` + `normalize_relay_url`): a record with a +//! blank relay spawns against the active workspace relay, so a workspace +//! relay change means a restart would change what runs — while a cosmetic +//! URL difference (trailing slash, scheme/host case) does not. //! - Channel membership is not an input: agents pick up channel changes live //! (#1468), never via restart. //! @@ -105,9 +107,15 @@ pub(crate) fn spawn_config_hash( effective.env.hash(&mut hasher); // Record fields the spawn env writes read directly. The relay is hashed - // resolved: a blank record relay spawns on the workspace relay, so a - // workspace relay change must trip the badge. - crate::relay::effective_agent_relay_url(&record.relay_url, workspace_relay).hash(&mut hasher); + // resolved AND normalized: a blank record relay spawns on the workspace + // relay, so a workspace relay change must trip the badge — but a cosmetic + // difference (trailing slash, scheme/host case) between a stamped record + // and a re-entered workspace URL must not. + crate::relay::normalize_relay_url(&crate::relay::effective_agent_relay_url( + &record.relay_url, + workspace_relay, + )) + .hash(&mut hasher); // Prompt and runtime-layered team instructions use the same resolver as spawn. effective_spawn_prompt(record).hash(&mut hasher); effective_team_instructions(record, teams).hash(&mut hasher); diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index 9b79881625..4a81980075 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -200,6 +200,38 @@ fn workspace_relay_change_ignored_for_pinned_record_relay() { ); } +#[test] +fn cosmetic_pin_difference_does_not_change_hash() { + // The relay is hashed normalized: trailing slash and scheme/host case are + // the same relay, so re-entering a pin in a cosmetically different form + // must not badge. + let mut slashed = record(); + slashed.relay_url = "WS://Localhost:3000/".into(); + assert_eq!( + spawn_config_hash(&record(), &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&slashed, &[], &[], "wss://ws.example", &Default::default()) + ); +} + +#[test] +fn cosmetic_workspace_relay_difference_ignored_for_blank_record_relay() { + // Post-migration stability: a record stamped with the workspace relay (or + // a legacy blank one resolving to it) must not badge when the frontend + // later supplies the same relay with a trailing slash or different case. + let mut rec = record(); + rec.relay_url = String::new(); + assert_eq!( + spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), + spawn_config_hash( + &rec, + &[], + &[], + "WSS://Relay-A.Example/", + &Default::default() + ) + ); +} + #[test] fn respond_to_allowlist_edit_changes_hash() { let rec = record(); diff --git a/desktop/src-tauri/src/relay.rs b/desktop/src-tauri/src/relay.rs index 646c19913a..f5652f1243 100644 --- a/desktop/src-tauri/src/relay.rs +++ b/desktop/src-tauri/src/relay.rs @@ -57,10 +57,11 @@ pub fn relay_api_base_url_with_override(state: &AppState) -> String { /// Selects the relay a managed agent should use for a relay operation. /// /// An explicit per-agent `relay_url` always wins (highest precedence), pinning -/// the agent to that relay regardless of the active workspace. An empty or -/// whitespace-only `relay_url` falls back to the active workspace relay, which -/// resolves at read-time so a never-set record reconciles, spawns, and re-syncs -/// on the session's relay instead of a stale stored value. Uniform for both +/// the agent to that relay regardless of the active workspace. Every record is +/// stamped with its home relay at create (and legacy blank records on the +/// first `apply_workspace` after boot), so the empty/whitespace fallback to +/// the active workspace relay is defense-in-depth only — it keeps a record +/// that somehow escaped stamping operational instead of dead. Uniform for both /// Local and Provider backends. pub fn effective_agent_relay_url(record_relay: &str, workspace_relay: &str) -> String { let pinned = record_relay.trim(); @@ -71,6 +72,38 @@ pub fn effective_agent_relay_url(record_relay: &str, workspace_relay: &str) -> S } } +/// Canonical form of a relay URL for identity comparisons — NOT for +/// connecting (callers keep the stored value for actual I/O). +/// +/// Agent records are pinned to their home relay by URL, so "is this record's +/// relay the workspace's relay" must not be defeated by cosmetic differences: +/// surrounding whitespace, trailing slashes, or scheme/host case (both are +/// case-insensitive per RFC 3986). Any path or query is preserved +/// case-sensitively. +pub fn normalize_relay_url(url: &str) -> String { + let trimmed = url.trim().trim_end_matches('/'); + match trimmed.split_once("://") { + Some((scheme, rest)) => { + let (authority, path) = match rest.find('/') { + Some(index) => rest.split_at(index), + None => (rest, ""), + }; + format!( + "{}://{}{}", + scheme.to_ascii_lowercase(), + authority.to_ascii_lowercase(), + path + ) + } + None => trimmed.to_string(), + } +} + +/// Whether two relay URLs identify the same relay after normalization. +pub fn relay_urls_equivalent(a: &str, b: &str) -> bool { + normalize_relay_url(a) == normalize_relay_url(b) +} + pub fn relay_http_base_url(relay_url: &str) -> String { let trimmed = relay_url.trim().trim_end_matches('/'); @@ -611,7 +644,8 @@ pub async fn submit_event_with_keys( mod tests { use super::{ build_profile_event, classify_intercepted_response, effective_agent_relay_url, - parse_command_response, relay_http_base_url, MALFORMED_RESPONSE_MESSAGE, + normalize_relay_url, parse_command_response, relay_http_base_url, relay_urls_equivalent, + MALFORMED_RESPONSE_MESSAGE, }; use serde::Deserialize; @@ -655,6 +689,56 @@ mod tests { ); } + // ── normalize_relay_url / relay_urls_equivalent ────────────────────────── + + #[test] + fn normalize_strips_whitespace_and_trailing_slashes() { + assert_eq!( + normalize_relay_url(" wss://relay.example.com// "), + "wss://relay.example.com" + ); + } + + #[test] + fn normalize_lowercases_scheme_and_host_only() { + // Scheme and authority are case-insensitive (RFC 3986); a path is not. + assert_eq!( + normalize_relay_url("WSS://Relay.Example.COM:3000/Path"), + "wss://relay.example.com:3000/Path" + ); + } + + #[test] + fn normalize_passes_through_schemeless_values() { + // Not a URL — nothing to case-fold beyond trim/slash cleanup. + assert_eq!(normalize_relay_url("not-a-url/"), "not-a-url"); + } + + #[test] + fn equivalence_ignores_cosmetic_differences() { + assert!(relay_urls_equivalent( + "wss://Relay.Example/", + " wss://relay.example" + )); + } + + #[test] + fn equivalence_distinguishes_real_differences() { + // Different host, port, or scheme = a different relay. + assert!(!relay_urls_equivalent( + "wss://relay-a.example", + "wss://relay-b.example" + )); + assert!(!relay_urls_equivalent( + "ws://relay.example:3000", + "ws://relay.example:3001" + )); + assert!(!relay_urls_equivalent( + "ws://relay.example", + "wss://relay.example" + )); + } + // ── relay_http_base_url scheme conversion ──────────────────────────────── #[test] diff --git a/desktop/src/features/communities/useCommunities.tsx b/desktop/src/features/communities/useCommunities.tsx index 48668a374b..590e28f842 100644 --- a/desktop/src/features/communities/useCommunities.tsx +++ b/desktop/src/features/communities/useCommunities.tsx @@ -16,6 +16,7 @@ import { saveActiveCommunityId, saveCommunities, } from "./communityStorage"; +import { rebindAgentRelay } from "@/shared/api/tauriManagedAgents"; import { removeSelfProfileCachesForRelay } from "@/features/profile/lib/selfProfileStorage"; import { removeChannelSnapshotForRelay } from "@/features/channels/channelSnapshot"; import { removeMessageSnapshotsForRelay } from "@/features/messages/lib/messageSnapshot"; @@ -224,6 +225,23 @@ function useCommunitiesInternal(): UseCommunitiesReturn { ); if (result.kind === "updated") { + // Agent records are pinned to their home relay, so a relay-URL edit + // must re-pin them onto the new URL or they orphan on the old one. + // Fire-and-forget: a failure leaves the pins on the old URL, which + // stays recoverable by re-editing the community. + const previous = communitiesRef.current.find((w) => w.id === id); + if ( + previous && + updates.relayUrl !== undefined && + updates.relayUrl !== previous.relayUrl + ) { + rebindAgentRelay(previous.relayUrl, updates.relayUrl).catch( + (error) => { + console.error("failed to rebind agents to edited relay:", error); + }, + ); + } + setCommunitiesState((prev) => { const next = prev.map((w) => w.id === id ? { ...w, ...updates } : w, diff --git a/desktop/src/shared/api/tauriManagedAgents.ts b/desktop/src/shared/api/tauriManagedAgents.ts index f9e7625788..3bb16ac849 100644 --- a/desktop/src/shared/api/tauriManagedAgents.ts +++ b/desktop/src/shared/api/tauriManagedAgents.ts @@ -46,3 +46,16 @@ export async function setManagedAgentAutoRestart( ); return fromRawManagedAgent(response); } + +// Re-pin managed-agent records from oldRelayUrl onto newRelayUrl. Called when +// a community's relay URL is edited so its agents don't orphan on the old URL. +// Returns the number of records rebound. +export async function rebindAgentRelay( + oldRelayUrl: string, + newRelayUrl: string, +): Promise { + return invokeTauri("rebind_agent_relay", { + oldRelayUrl, + newRelayUrl, + }); +} diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 6727524c08..cf72d8cf63 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -8847,6 +8847,9 @@ export function maybeInstallE2eTauriMocks() { } return; } + case "rebind_agent_relay": + // Mock store has no pinned agent records; nothing to rebind. + return 0; case "get_profile": return handleGetProfile(activeConfig); case "update_profile": From 2e2babb969a6d4ad4a096adbf5fd7d62ac619158 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Jul 2026 12:16:13 +1000 Subject: [PATCH 02/12] feat(desktop): lazily activate workspace agents on visit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract agent activation out of the one-shot boot restore into a relay-filtered step that runs on every workspace apply (Phase 2 of lazy multi-workspace agents, building on the Phase 1 relay pinning): - Rename restore_managed_agents_on_launch to activate_workspace_agents, taking the applied workspace relay and starting only local start-on-launch agents pinned to that relay (normalized match; a blank pin matches the visited workspace, mirroring the effective_agent_relay_url defense-in-depth fallback). - Call it from every apply_workspace instead of only the first — boot restore becomes the session's first activation. The one-shot managed_agent_restore_pending flag now gates only the mesh-llm Share Compute restore. - Track activated relays in AppState (activated_agent_relays) so each workspace activates at most once per app session: bouncing A→B→A never resurrects agents the user manually stopped in A. Nothing is stopped on switch — each workspace's agents keep running against their own relay. - Gate the boot-time repos-dir/identity-recovery safety checks behind a new session-wide managed_agent_activation_enabled flag, and run the stale-process/orphan sweeps only on the session's first activation so a later activation cannot reap a concurrent activation's not-yet-tracked children. Tested: cargo test --manifest-path desktop/src-tauri/Cargo.toml (1424 passed, incl. 7 new activation unit tests), clippy -D warnings, rustfmt, cargo check --features mesh-llm. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- desktop/scripts/check-file-sizes.mjs | 2 +- desktop/src-tauri/src/app_state.rs | 57 ++-- desktop/src-tauri/src/commands/workspace.rs | 83 ++---- desktop/src-tauri/src/lib.rs | 20 +- .../src-tauri/src/managed_agents/restore.rs | 282 +++++++++++++++--- .../src/managed_agents/spawn_hash.rs | 2 +- desktop/src-tauri/src/migration.rs | 2 +- 7 files changed, 310 insertions(+), 138 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index bf7177e6c2..e89c5f3f7f 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -374,7 +374,7 @@ const overrides = new Map([ // +5 (1068 -> 1073): merge with main, which independently added the // managed_agent_profile_reconcile_enabled flag (field + doc + init) under // its own 1042-line override. Union of two separately approved additions. - ["src-tauri/src/app_state.rs", 1073], + ["src-tauri/src/app_state.rs", 1054], // multi-slot splitting + no-op suppression (#1309): the ReadStateManager // class grew from ~700 lines to ~1019 with the addition of // splitContextsIntoBudgetedSlots (pure fn + 5 tests), publishSplitSlots, diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 71f155b18a..70ff474d74 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, io::Write, sync::{ atomic::{AtomicBool, AtomicU16}, @@ -18,26 +18,29 @@ use crate::managed_agents::ManagedAgentProcess; pub struct AppState { pub keys: Mutex, pub http_client: reqwest::Client, - /// A no-redirect client for authenticated relay media fetches (download, - /// clipboard copy, snapshot, editor). Every caller pre-validates the URL - /// origin, but the app-wide `http_client` follows redirects by default, so - /// a relay `/media/` URL returning a 3xx to an off-origin or private host - /// would forward the minted media Authorization header across origins — - /// a redirect-hop SSRF. This client treats any 3xx as a non-success - /// response (surfaced as an error) so the auth token never leaves the - /// validated relay origin. - pub media_fetch_client: reqwest::Client, /// Workspace-provided relay URL override. Set by `apply_workspace` on app /// init and takes priority over env vars and compile-time defaults. pub relay_url_override: Mutex>, - /// Set during backend setup when managed agents are eligible for launch - /// restore. `apply_workspace` consumes it after installing the workspace - /// relay and identity, so agents never start against the fallback relay. + /// One-shot gate consumed by the first `apply_workspace` after boot to + /// restore Share Compute (mesh-llm). Agent activation itself is governed + /// by `managed_agent_activation_enabled` + `activated_agent_relays` and + /// runs on every `apply_workspace`. pub managed_agent_restore_pending: AtomicBool, /// Whether desktop may repair managed-agent kind:0 profiles from its local /// records. Disabled by the agent-managed profiles experiment so an agent's /// own profile updates are not overwritten on start or restore. pub managed_agent_profile_reconcile_enabled: AtomicBool, + /// Session-wide gate for lazy agent activation. Set during backend setup + /// only when the boot-time repos-dir and identity-recovery safety checks + /// allow agents to start; when false, no `apply_workspace` may auto-start + /// agents this session. + pub managed_agent_activation_enabled: AtomicBool, + /// Normalized relay URLs whose workspaces were already activated this app + /// session. A workspace's agents lazily start on the first visit and at + /// most once per session — bouncing A→B→A must not resurrect agents the + /// user manually stopped in A. Never stops agents: switching away leaves + /// the other workspace's agents running against their own relay. + pub activated_agent_relays: Mutex>, /// One-shot gate: first `apply_workspace` runs `pin_blank_agent_relays`. pub agent_relay_stamp_pending: AtomicBool, /// Shared shutdown signal checked by launch-time agent restoration. @@ -152,27 +155,6 @@ fn identity_from_env() -> Option { } } -/// Build the no-redirect HTTP client used for authenticated relay media -/// fetches (download / copy). -/// -/// This client is a security boundary, not a convenience: it carries a minted -/// media `Authorization` header, so it MUST NOT follow redirects. A relay 3xx -/// to an off-origin or private host would otherwise forward that header across -/// origins (a redirect-hop SSRF). `redirect::Policy::none()` returns the 3xx -/// verbatim so the caller can reject it. -/// -/// Returned as a `Result` so the fail-closed invariant is testable — callers -/// must never substitute a redirect-following client on build failure. Shares -/// the localhost `resolve`/pool config with the app-wide `http_client`. -pub fn build_media_fetch_client() -> reqwest::Result { - reqwest::Client::builder() - .resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0))) - .pool_idle_timeout(std::time::Duration::from_secs(10)) - .pool_max_idle_per_host(1) - .redirect(reqwest::redirect::Policy::none()) - .build() -} - pub fn build_app_state() -> AppState { // Env var takes precedence (dev/CI). If absent, resolve_persisted_identity() // in setup() will replace the ephemeral placeholder with a persisted key. @@ -195,14 +177,11 @@ pub fn build_app_state() -> AppState { .pool_max_idle_per_host(1) .build() .unwrap_or_else(|_| reqwest::Client::new()), - media_fetch_client: build_media_fetch_client().expect( - "media_fetch_client must build with redirect::Policy::none(); a \ - redirect-following fallback would forward the minted media auth \ - header across origins (redirect-hop SSRF)", - ), relay_url_override: Mutex::new(None), managed_agent_restore_pending: AtomicBool::new(false), managed_agent_profile_reconcile_enabled: AtomicBool::new(true), + managed_agent_activation_enabled: AtomicBool::new(false), + activated_agent_relays: Mutex::new(HashSet::new()), agent_relay_stamp_pending: AtomicBool::new(true), shutdown_started: AtomicBool::new(false), managed_agent_restore_transition: Mutex::new(()), diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 6ae4543744..3622338dc6 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -5,9 +5,9 @@ use tauri::{AppHandle, Emitter, Manager, State}; use crate::app_state::AppState; use crate::managed_agents::{ - effective_repos_dir, ensure_repos_symlink, load_managed_agents, nest_dir, - rebind_agent_relay_urls, restore_managed_agents_on_launch, save_managed_agents, - stamp_blank_agent_relay_urls, try_regenerate_nest, write_persisted_repos_dir, + activate_workspace_agents, effective_repos_dir, ensure_repos_symlink, load_managed_agents, + nest_dir, rebind_agent_relay_urls, save_managed_agents, stamp_blank_agent_relay_urls, + try_regenerate_nest, write_persisted_repos_dir, }; use crate::relay; @@ -104,10 +104,10 @@ pub async fn apply_workspace( relay_url: String, nsec: Option, repos_dir: Option, - agent_managed_profiles: Option, app: AppHandle, ) -> Result<(), String> { let restore_app = app.clone(); + let activation_relay_url = relay_url.clone(); tokio::task::spawn_blocking(move || { let state = app.state::(); @@ -150,13 +150,6 @@ pub async fn apply_workspace( *keys_guard = keys; } - // Keep the backend-side reconcile guard aligned with the frontend - // experiment before launch-time restore can spawn any agents. Missing - // means the stable behavior: desktop remains authoritative. - state - .managed_agent_profile_reconcile_enabled - .store(!agent_managed_profiles.unwrap_or(false), Ordering::Release); - // ── One-shot legacy migration (non-fatal) ───────────────────────────── // Pin any blank-relay agent record to the first workspace applied // after boot — exactly what blank would have resolved to at boot @@ -200,53 +193,35 @@ pub async fn apply_workspace( .await .map_err(|e| format!("spawn_blocking failed: {e}"))??; - let state = restore_app.state::(); - let restore_pending = state + // Lazy per-workspace activation: runs on EVERY apply, and self-gates so + // each workspace's agents start at most once per app session (launch + // restore is simply the first activation). Nothing is stopped on switch — + // the previous workspace's agents keep running against their own relay. + // The one-shot restore flag is consumed only for Share Compute (mesh-llm), + // which must be up before its dependent agents spawn. + #[cfg(feature = "mesh-llm")] + let mesh_restore_pending = restore_app + .state::() .managed_agent_restore_pending .swap(false, Ordering::AcqRel); - - // The coordinator starts before React applies the selected workspace, so - // its startup publication may have used the fallback relay and placeholder - // identity. Correct it off the command path so an unavailable relay cannot - // hold the frontend on its loading gate. On initial launch, restore MeshLLM - // first so a slow stopped-status request cannot overwrite a newly restored - // serving status, then restore managed agents after the admission identity - // has been published (or the bounded publication attempt has timed out). - #[cfg(feature = "mesh-llm")] - { - let app = restore_app.clone(); - tauri::async_runtime::spawn(async move { - let state = app.state::(); - if restore_pending { - if let Err(error) = - crate::commands::mesh_llm::restore_mesh_sharing(&app, &state).await - { - eprintln!("buzz-desktop: failed to restore Share Compute: {error}"); - } - } - crate::mesh_llm::publish_current_status_once(&app, "workspace apply").await; - if restore_pending { - if let Err(error) = - restore_managed_agents_on_launch(&app, &state.shutdown_started).await - { - eprintln!("buzz-desktop: failed to restore managed agents: {error}"); - } - } - }); - } - - #[cfg(not(feature = "mesh-llm"))] - if restore_pending { - let app = restore_app.clone(); - tauri::async_runtime::spawn(async move { - let state = app.state::(); - if let Err(error) = - restore_managed_agents_on_launch(&app, &state.shutdown_started).await + let app = restore_app.clone(); + tauri::async_runtime::spawn(async move { + let state = app.state::(); + #[cfg(feature = "mesh-llm")] + if mesh_restore_pending { + if let Err(error) = crate::commands::mesh_llm::restore_mesh_sharing(&app, &state).await { - eprintln!("buzz-desktop: failed to restore managed agents: {error}"); + eprintln!("buzz-desktop: failed to restore Share Compute: {error}"); } - }); - } + } + #[cfg(feature = "mesh-llm")] + crate::mesh_llm::publish_current_status_once(&app, "workspace apply").await; + if let Err(error) = + activate_workspace_agents(&app, &state.shutdown_started, &activation_relay_url).await + { + eprintln!("buzz-desktop: failed to activate workspace agents: {error}"); + } + }); Ok(()) } diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index fdefbbbe02..da60dec4f1 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -526,7 +526,7 @@ pub fn run() { // Backfill the pinned persona snapshot for any pre-existing agent // that predates the record-authoritative-spawn cutover (persona_id // set but no source_version). Must run before - // restore_managed_agents_on_launch so no agent spawns from an empty + // activate_workspace_agents so no agent spawns from an empty // snapshot. Synchronous and best-effort — a failure here must not // block launch, but a missing persona is logged loudly inside. if let Err(e) = backfill_persona_snapshots(&app_handle) { @@ -655,16 +655,22 @@ pub fn run() { }); } - // Defer launch-time agent restoration until `apply_workspace` has - // installed the active workspace relay and identity. Starting here - // would race React initialization and send agents whose saved record - // has no relay override to the localhost fallback. Preserve the - // boot-time repos and identity recovery safety gates by only marking - // restoration pending when both allow it. + // Defer agent activation until `apply_workspace` has installed a + // workspace relay and identity. Starting here would race React + // initialization and send agents whose saved record has no relay + // override to the localhost fallback. Each workspace's agents then + // lazily activate on its first visit (`activate_workspace_agents`); + // the boot restore is simply the first activation. Preserve the + // boot-time repos and identity recovery safety gates by only + // enabling activation (and the one-shot mesh-llm restore) when + // both allow it. if restore_agents && !recovery_mode { state .managed_agent_restore_pending .store(true, Ordering::Release); + state + .managed_agent_activation_enabled + .store(true, Ordering::Release); } // Periodic sweep: reap orphaned agents from dead instances every 60s. diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index e4fc18b39f..ff96902155 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -3,10 +3,11 @@ use super::relay_mesh_model_id; use super::{ find_managed_agent_mut, kill_stale_tracked_processes, load_managed_agents, load_personas, save_managed_agents, spawn_agent_child, sync_managed_agent_processes, BackendKind, - ManagedAgentProcess, + ManagedAgentProcess, ManagedAgentRecord, }; use crate::app_state::AppState; use crate::util; +use std::collections::HashSet; use std::sync::atomic::{AtomicBool, Ordering}; use tauri::Manager; @@ -15,7 +16,7 @@ type AgentSpawnResult = (String, SpawnResult); /// Backfill the pinned persona snapshot for pre-existing agents created before /// the record became the spawn source of truth. Runs once at launch, before -/// `restore_managed_agents_on_launch` spawns anything, so no agent boots from an +/// `activate_workspace_agents` spawns anything, so no agent boots from an /// empty snapshot. /// /// Only records with a `persona_id` but no `persona_source_version` are touched. @@ -73,15 +74,73 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String> Ok(()) } -/// Restore managed agents that were running before the app was closed. +/// Scope of a relay activation granted by [`begin_relay_activation`]. +pub(crate) struct RelayActivation { + /// True only for the session's very first activation (the boot restore): + /// the one-shot stale-process and orphan sweeps run only then. Later + /// activations must not re-sweep — a concurrent activation's freshly + /// spawned children are not yet tracked and would read as orphans. + pub run_boot_sweeps: bool, +} + +/// Record `relay_url` as activated for this app session, deciding whether the +/// caller may start its agents. +/// +/// Returns `None` when the URL is blank or this relay was already activated +/// this session — the caller must not start agents again, so bouncing A→B→A +/// never resurrects an agent the user manually stopped in A. Activation is +/// marked at attempt time (a failed attempt is not retried until relaunch), +/// matching the old one-shot restore flag. Relays are keyed normalized +/// (trailing slash, scheme/host case), so cosmetic URL differences cannot +/// re-activate a workspace. +pub(crate) fn begin_relay_activation( + activated_relays: &mut HashSet, + relay_url: &str, +) -> Option { + let normalized = crate::relay::normalize_relay_url(relay_url); + if normalized.is_empty() { + return None; + } + let run_boot_sweeps = activated_relays.is_empty(); + if !activated_relays.insert(normalized) { + return None; + } + Some(RelayActivation { run_boot_sweeps }) +} + +/// Whether `record` auto-starts when `workspace_relay_url` is activated: +/// a local start-on-launch agent pinned to this relay. A blank pin matches +/// the workspace being activated — mirroring `effective_agent_relay_url`'s +/// defense-in-depth fallback — so a record that somehow escaped relay +/// stamping still auto-starts against the relay it resolves to at spawn. +pub(crate) fn record_activates_on_relay( + record: &ManagedAgentRecord, + workspace_relay_url: &str, +) -> bool { + if !record.start_on_app_launch || record.backend != BackendKind::Local { + return false; + } + let pinned = record.relay_url.trim(); + pinned.is_empty() || crate::relay::relay_urls_equivalent(pinned, workspace_relay_url) +} + +/// Lazily activate a workspace's managed agents. +/// +/// Called from every `apply_workspace`: starts local `start_on_app_launch` +/// agents pinned to `workspace_relay_url` that are not already running — the +/// launch restore is simply the session's first activation. Each workspace +/// activates at most once per app session ([`begin_relay_activation`]), and +/// nothing is ever stopped here: switching away leaves the previous +/// workspace's agents running against their own relay. /// /// Split into three phases to minimise lock contention with the frontend: /// A (under lock): sync process state, cleanup, collect agents to start /// B (no locks): resolve commands and spawn processes in parallel /// C (re-lock): write back PIDs and status to records on disk -pub async fn restore_managed_agents_on_launch( +pub async fn activate_workspace_agents( app: &tauri::AppHandle, shutdown_started: &AtomicBool, + workspace_relay_url: &str, ) -> Result<(), String> { if shutdown_started.load(Ordering::SeqCst) { return Ok(()); @@ -89,7 +148,27 @@ pub async fn restore_managed_agents_on_launch( let state = app.state::(); - // ── Phase A (under lock): housekeeping + collect agents to restore ── + // Session-wide boot gate: repos-dir resolution and identity recovery are + // decided once at launch. When either fails closed, no workspace may + // auto-start agents this session. + if !state + .managed_agent_activation_enabled + .load(Ordering::Acquire) + { + return Ok(()); + } + + let Some(activation) = ({ + let mut activated_relays = state + .activated_agent_relays + .lock() + .map_err(|error| error.to_string())?; + begin_relay_activation(&mut activated_relays, workspace_relay_url) + }) else { + return Ok(()); + }; + + // ── Phase A (under lock): housekeeping + collect agents to activate ── let mut agents_to_start: Vec; { let _store_guard = state @@ -111,40 +190,51 @@ pub async fn restore_managed_agents_on_launch( &mut runtimes, &super::current_instance_id(app), ); - changed |= - kill_stale_tracked_processes(&mut records, &runtimes, &super::current_instance_id(app)); - let tracked_pids: Vec = records - .iter() - .filter_map(|r| r.runtime_pid) - .chain(runtimes.values().map(|rt| rt.child.id())) - .collect(); - super::sweep_orphaned_agent_processes(app, &tracked_pids); - - // System-wide sweep: enumerate all user processes and kill any known - // agent binaries not tracked by this session. Catches orphans whose - // PID files were already cleaned up (e.g. agent workers in their own - // process group whose parent harness exited). - super::sweep_system_agent_processes(&super::current_instance_id(app), &tracked_pids); - - // Dead-instance reaping: find agents belonging to Buzz instances - // whose desktop process is no longer running and reap them. - super::reap_dead_instance_agents(&super::current_instance_id(app), &tracked_pids); - - // Exact-path sweep: kill any buzz-acp process whose executable path - // matches this bundle's harness binary but is not in the tracked set. - // Complements the env-var sweep above — catches orphans that predate - // BUZZ_MANAGED_AGENT injection or lost their PID-file receipt. - // - // TODO: the three sweeps above each walk the PID table independently. - // A future consolidation should collect a single shared process snapshot - // at the top of this block and thread it through all sweep functions, - // replacing the three separate kernel enumerations. - super::sweep_untracked_bundle_harnesses(&tracked_pids); + // One-shot boot cleanup: previous-session leftovers and orphans are + // swept only on the session's first activation. Later activations + // must not sweep — a concurrent activation's or UI start's freshly + // spawned children are not yet in the tracked set and would be killed + // as orphans. + if activation.run_boot_sweeps { + changed |= kill_stale_tracked_processes( + &mut records, + &runtimes, + &super::current_instance_id(app), + ); + + let tracked_pids: Vec = records + .iter() + .filter_map(|r| r.runtime_pid) + .chain(runtimes.values().map(|rt| rt.child.id())) + .collect(); + super::sweep_orphaned_agent_processes(app, &tracked_pids); + + // System-wide sweep: enumerate all user processes and kill any known + // agent binaries not tracked by this session. Catches orphans whose + // PID files were already cleaned up (e.g. agent workers in their own + // process group whose parent harness exited). + super::sweep_system_agent_processes(&super::current_instance_id(app), &tracked_pids); + + // Dead-instance reaping: find agents belonging to Buzz instances + // whose desktop process is no longer running and reap them. + super::reap_dead_instance_agents(&super::current_instance_id(app), &tracked_pids); + + // Exact-path sweep: kill any buzz-acp process whose executable path + // matches this bundle's harness binary but is not in the tracked set. + // Complements the env-var sweep above — catches orphans that predate + // BUZZ_MANAGED_AGENT injection or lost their PID-file receipt. + // + // TODO: the three sweeps above each walk the PID table independently. + // A future consolidation should collect a single shared process snapshot + // at the top of this block and thread it through all sweep functions, + // replacing the three separate kernel enumerations. + super::sweep_untracked_bundle_harnesses(&tracked_pids); + } let candidates: Vec = records .iter() - .filter(|record| record.start_on_app_launch && record.backend == BackendKind::Local) + .filter(|record| record_activates_on_relay(record, workspace_relay_url)) .map(|record| record.pubkey.clone()) .collect(); @@ -380,3 +470,125 @@ fn persist_restore_error( record.last_error = Some(error); save_managed_agents(app, &records) } + +#[cfg(test)] +mod tests { + use super::{begin_relay_activation, record_activates_on_relay, ManagedAgentRecord}; + use std::collections::HashSet; + + /// Minimal record with the fields the activation filter reads. Everything + /// else takes its serde default (`start_on_app_launch` defaults to true, + /// `backend` to Local). + fn record_on_relay(relay_url: &str) -> ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "agent-pubkey", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "{relay_url}", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }}"# + )) + .expect("sample record") + } + + // ── begin_relay_activation: once per relay per session ────────────────── + + #[test] + fn first_activation_of_session_runs_boot_sweeps() { + let mut activated = HashSet::new(); + + let activation = begin_relay_activation(&mut activated, "wss://relay-a.example") + .expect("first visit must activate"); + + assert!(activation.run_boot_sweeps, "boot restore sweeps orphans"); + } + + #[test] + fn second_workspace_activates_without_boot_sweeps() { + let mut activated = HashSet::new(); + begin_relay_activation(&mut activated, "wss://relay-a.example"); + + let activation = begin_relay_activation(&mut activated, "wss://relay-b.example") + .expect("a new workspace must activate"); + + assert!( + !activation.run_boot_sweeps, + "sweeps are one-shot: a later activation could kill a concurrent \ + activation's untracked children" + ); + } + + #[test] + fn revisiting_a_workspace_does_not_reactivate() { + // A→B→A: the second visit to A must not restart agents the user + // stopped there, even when the URL differs only cosmetically. + let mut activated = HashSet::new(); + begin_relay_activation(&mut activated, "wss://relay-a.example"); + begin_relay_activation(&mut activated, "wss://relay-b.example"); + + assert!(begin_relay_activation(&mut activated, "wss://relay-a.example").is_none()); + assert!( + begin_relay_activation(&mut activated, "WSS://Relay-A.Example/").is_none(), + "normalized matching: cosmetic URL differences must not re-activate" + ); + } + + #[test] + fn blank_relay_never_activates_nor_consumes_the_boot_sweep() { + let mut activated = HashSet::new(); + + assert!(begin_relay_activation(&mut activated, " ").is_none()); + + // The blank no-op must not have claimed the session's boot sweep. + let activation = begin_relay_activation(&mut activated, "wss://relay-a.example") + .expect("real relay still activates"); + assert!(activation.run_boot_sweeps); + } + + // ── record_activates_on_relay: relay-filtered candidate selection ─────── + + #[test] + fn record_activates_only_on_its_pinned_relay() { + let record = record_on_relay("wss://relay-a.example"); + + assert!(record_activates_on_relay(&record, "wss://relay-a.example")); + assert!( + record_activates_on_relay(&record, "WSS://Relay-A.Example/"), + "pin matching is normalized" + ); + assert!( + !record_activates_on_relay(&record, "wss://relay-b.example"), + "visiting another workspace must not start this agent" + ); + } + + #[test] + fn blank_pin_activates_on_the_visited_workspace() { + // Defense-in-depth for a record that escaped relay stamping: it spawns + // against the active workspace relay (`effective_agent_relay_url`), so + // it activates with the workspace being visited. + let record = record_on_relay(""); + assert!(record_activates_on_relay(&record, "wss://relay-a.example")); + } + + #[test] + fn opted_out_and_provider_records_never_activate() { + let mut record = record_on_relay("wss://relay-a.example"); + record.start_on_app_launch = false; + assert!(!record_activates_on_relay(&record, "wss://relay-a.example")); + + let mut record = record_on_relay("wss://relay-a.example"); + record.backend = super::BackendKind::Provider { + id: "blox".to_string(), + config: serde_json::Value::Null, + }; + assert!(!record_activates_on_relay(&record, "wss://relay-a.example")); + } +} diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 093c3068d6..4efe853cbc 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -11,7 +11,7 @@ //! - Inputs mirror what a start would actually run: the start/restore paths //! re-snapshot the linked persona's prompt/model/provider/env onto the //! record immediately before spawning (`start_local_agent_with_preflight`, -//! `restore_managed_agents_on_launch`), so persona edits to those fields DO +//! `activate_workspace_agents`), so persona edits to those fields DO //! apply on a plain restart and are hashed via the same prospective //! re-snapshot. Harness command, args/mcp, env layering, and the record //! fields the spawn env writes read are hashed as spawn resolves them. diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index b0e3359e53..0bb7cc0dea 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -104,7 +104,7 @@ fn copy_dir_all(src: &Path, dst: &Path) -> std::io::Result<()> { /// Run every data migration that must complete before identity resolution and /// agent restore. Ordering is load-bearing: `migrate_legacy_app_data_dir` must /// precede any disk read, and `sync_shared_agent_data` must precede -/// `restore_managed_agents_on_launch` (which reads `managed-agents.json`). +/// `activate_workspace_agents` (which reads `managed-agents.json`). /// Identity-dependent migrations (persona/team event signing) run separately in /// boot setup after the persisted identity is resolved. /// From ee40613898bcf902caf2933198fe0ba04872d52a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Jul 2026 13:44:37 +1000 Subject: [PATCH 03/12] feat(desktop): scope the agents UI per community MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filter every managed-agent surface to the active community's relay so concurrently running workspaces stop bleeding into each other's UI (Phase 3 of lazy multi-workspace agents, building on the Phase 1 relay pinning and Phase 2 lazy activation): - Add agentRelayScope helpers: a frontend mirror of the backend relay normalizer plus agentBelongsToRelay / partitionAgentsByRelay / hasRunningAgentInCommunity. A blank pin follows the active community (same defense-in-depth fallback as effective_agent_relay_url), and a missing provider/community degrades to unscoped rather than blanking every surface. - Scope the useManagedAgentActions list — and the bulk stop, start/ stop/delete lookups, and presence derived from it — to the active relay via a new lenient useActiveRelayUrl hook. Persona delete keeps counting instances against the unscoped record set since deleting a persona removes instances in every community. - Gate the 5s managed-agents liveness poll on a running agent in this community; agents running in other communities render no process state here, so they no longer keep the poll alive. - Hold auto-restart for agents pinned to other communities — their working/observer signals are read from the active relay only, so a pure workspace switch must never fire a restart — and re-check the pin in the pre-fire re-fetch so a rebind cannot restart a foreign agent. - Scope mergeKnownAgentPubkeys' managed-agent source to the active relay; the relay-agent (kind:10100) source stays unfiltered. - Surface an "N agents running in other communities" line in the agents header so concurrent background agents stay discoverable. Tested: desktop pnpm test (2939 passed, incl. new agentRelayScope, knownAgentPubkeys, and autoRestartPolicy unit tests), tsc --noEmit, biome check, Playwright smoke project (537 passed, 5 failed; 2 of the failures passed on rerun and the other 3 — channels intro-scroll, video review mode, shared-compute empty state — reproduce on baseline HEAD without this change, so all 5 are pre-existing flakes). Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../features/agents/agentRelayScope.test.mjs | 129 ++++++++++++++++++ .../src/features/agents/agentRelayScope.ts | 98 +++++++++++++ desktop/src/features/agents/hooks.ts | 12 +- .../agents/knownAgentPubkeys.test.mjs | 57 ++++++++ .../src/features/agents/knownAgentPubkeys.ts | 19 ++- .../agents/lib/autoRestartPolicy.test.mjs | 26 ++++ .../features/agents/lib/autoRestartPolicy.ts | 9 ++ .../agents/lib/useAutoRestartPolicy.ts | 9 +- desktop/src/features/agents/ui/AgentsView.tsx | 21 ++- .../agents/ui/useManagedAgentActions.ts | 27 +++- .../features/agents/useKnownAgentPubkeys.tsx | 8 +- .../features/communities/useCommunities.tsx | 13 ++ 12 files changed, 414 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/agents/agentRelayScope.test.mjs create mode 100644 desktop/src/features/agents/agentRelayScope.ts diff --git a/desktop/src/features/agents/agentRelayScope.test.mjs b/desktop/src/features/agents/agentRelayScope.test.mjs new file mode 100644 index 0000000000..ecb5d69f7a --- /dev/null +++ b/desktop/src/features/agents/agentRelayScope.test.mjs @@ -0,0 +1,129 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + agentBelongsToRelay, + hasRunningAgentInCommunity, + normalizeRelayUrlForCompare, + partitionAgentsByRelay, +} from "./agentRelayScope.ts"; + +const RELAY_A = "ws://relay-a.example.com:3000"; +const RELAY_B = "wss://relay-b.example.com"; + +// ── normalizeRelayUrlForCompare ────────────────────────────────────────────── +// Must agree with the Rust `normalize_relay_url` (desktop/src-tauri/src/relay.rs) +// because record pins are stamped by the backend and compared here. These +// vectors mirror the Rust unit tests. + +test("normalize_stripsWhitespaceAndTrailingSlashes", () => { + assert.equal( + normalizeRelayUrlForCompare(" wss://relay.example.com// "), + "wss://relay.example.com", + ); +}); + +test("normalize_lowercasesSchemeAndAuthority_preservesPathCase", () => { + assert.equal( + normalizeRelayUrlForCompare("WSS://Relay.Example.COM:3000/Path"), + "wss://relay.example.com:3000/Path", + ); +}); + +test("normalize_schemelessInput_passesThroughTrimmed", () => { + assert.equal(normalizeRelayUrlForCompare("not-a-url/"), "not-a-url"); +}); + +// ── agentBelongsToRelay ────────────────────────────────────────────────────── + +test("belongs_exactMatch", () => { + assert.equal(agentBelongsToRelay(RELAY_A, RELAY_A), true); +}); + +test("belongs_cosmeticDifferences_stillMatch", () => { + // Trailing slash and scheme/host case must not split an agent from its + // community — this is exactly what the shared normalizer exists for. + assert.equal( + agentBelongsToRelay("WS://RELAY-A.example.com:3000/", RELAY_A), + true, + ); +}); + +test("belongs_differentRelay_doesNotMatch", () => { + assert.equal(agentBelongsToRelay(RELAY_A, RELAY_B), false); +}); + +test("belongs_blankPin_followsActiveCommunity", () => { + // Defense-in-depth mirror of the backend's `effective_agent_relay_url` + // blank fallback: a record that escaped stamping follows the visited + // community instead of vanishing from every community. + assert.equal(agentBelongsToRelay("", RELAY_A), true); + assert.equal(agentBelongsToRelay(" ", RELAY_B), true); + assert.equal(agentBelongsToRelay(undefined, RELAY_A), true); +}); + +test("belongs_noActiveCommunityRelay_degradesToUnscoped", () => { + assert.equal(agentBelongsToRelay(RELAY_A, null), true); + assert.equal(agentBelongsToRelay(RELAY_A, ""), true); + assert.equal(agentBelongsToRelay(RELAY_A, undefined), true); +}); + +// ── partitionAgentsByRelay: the agent-list filter ──────────────────────────── + +test("partition_scopesListToActiveCommunity", () => { + // Two communities' agents coexist after lazy activation; the list the + // user sees in community A must contain only A's agents (plus blank-pin + // strays), with B's surfaced only through the "other communities" count. + const agents = [ + { pubkey: "a1", relayUrl: RELAY_A }, + { pubkey: "b1", relayUrl: RELAY_B }, + { pubkey: "a2", relayUrl: `${RELAY_A}/` }, + { pubkey: "stray", relayUrl: "" }, + ]; + + const { inCommunity, other } = partitionAgentsByRelay(agents, RELAY_A); + + assert.deepEqual( + inCommunity.map((agent) => agent.pubkey), + ["a1", "a2", "stray"], + ); + assert.deepEqual( + other.map((agent) => agent.pubkey), + ["b1"], + ); +}); + +test("partition_undefinedAgents_yieldsEmpty", () => { + const { inCommunity, other } = partitionAgentsByRelay(undefined, RELAY_A); + assert.deepEqual(inCommunity, []); + assert.deepEqual(other, []); +}); + +// ── hasRunningAgentInCommunity: the polling gate ───────────────────────────── + +test("pollingGate_runningAgentInOtherCommunity_doesNotPoll", () => { + // A workspace switch leaves the previous community's agents running; they + // must not keep this community's 5s liveness poll alive. + const agents = [ + { relayUrl: RELAY_B, status: "running" }, + { relayUrl: RELAY_A, status: "stopped" }, + ]; + + assert.equal(hasRunningAgentInCommunity(agents, RELAY_A), false); +}); + +test("pollingGate_runningAgentInThisCommunity_polls", () => { + const agents = [ + { relayUrl: RELAY_B, status: "running" }, + { relayUrl: RELAY_A, status: "running" }, + ]; + + assert.equal(hasRunningAgentInCommunity(agents, RELAY_A), true); +}); + +test("pollingGate_blankPinRunningAgent_polls", () => { + assert.equal( + hasRunningAgentInCommunity([{ relayUrl: "", status: "running" }], RELAY_A), + true, + ); +}); diff --git a/desktop/src/features/agents/agentRelayScope.ts b/desktop/src/features/agents/agentRelayScope.ts new file mode 100644 index 0000000000..5decfa823f --- /dev/null +++ b/desktop/src/features/agents/agentRelayScope.ts @@ -0,0 +1,98 @@ +/** + * Relay-scoping for managed agents. Every managed-agent record is pinned to + * its home relay (`ManagedAgent.relayUrl`, stamped by the backend at create), + * and the desktop UI presents agents per community — so every surface that + * lists, counts, or acts on managed agents must scope to the active + * community's relay through these helpers. Agents pinned to other relays + * keep running in the background; they are just not "in" this community. + */ + +/** + * Canonical form of a relay URL for identity comparisons — NOT for + * connecting. Frontend mirror of `normalize_relay_url` in + * `desktop/src-tauri/src/relay.rs`; the two must agree because record pins + * are stamped by the backend and compared here: trim, strip trailing + * slashes, lowercase scheme + authority (case-insensitive per RFC 3986), + * preserve any path or query case-sensitively. + * + * Distinct from `normalizeRelayUrl` in `communityStorage.ts` (input + * canonicalisation: prepends `wss://`) and in `selfProfileStorage.ts` + * (storage keys: lowercases the whole URL, path included). + */ +export function normalizeRelayUrlForCompare(url: string): string { + const trimmed = url.trim().replace(/\/+$/, ""); + const schemeEnd = trimmed.indexOf("://"); + if (schemeEnd === -1) { + return trimmed; + } + const scheme = trimmed.slice(0, schemeEnd); + const rest = trimmed.slice(schemeEnd + "://".length); + const pathStart = rest.indexOf("/"); + const authority = pathStart === -1 ? rest : rest.slice(0, pathStart); + const path = pathStart === -1 ? "" : rest.slice(pathStart); + return `${scheme.toLowerCase()}://${authority.toLowerCase()}${path}`; +} + +/** + * Whether an agent record belongs to the given community relay. + * + * A blank pin follows the active community — the same defense-in-depth + * fallback as the backend's `effective_agent_relay_url` for records that + * escaped stamping. A blank/absent community relay (no provider, no active + * community yet) degrades to unscoped rather than blanking every surface. + */ +export function agentBelongsToRelay( + agentRelayUrl: string | null | undefined, + communityRelayUrl: string | null | undefined, +): boolean { + const community = communityRelayUrl?.trim() ?? ""; + if (community === "") { + return true; + } + const pinned = agentRelayUrl?.trim() ?? ""; + if (pinned === "") { + return true; + } + return ( + normalizeRelayUrlForCompare(pinned) === + normalizeRelayUrlForCompare(community) + ); +} + +/** + * Split agents into those pinned to the active community's relay and the + * rest. `inCommunity` drives the agents screen; `other` exists only for + * cross-community affordances (the "running in other communities" count). + */ +export function partitionAgentsByRelay( + agents: readonly T[] | undefined, + communityRelayUrl: string | null | undefined, +): { inCommunity: T[]; other: T[] } { + const inCommunity: T[] = []; + const other: T[] = []; + for (const agent of agents ?? []) { + if (agentBelongsToRelay(agent.relayUrl, communityRelayUrl)) { + inCommunity.push(agent); + } else { + other.push(agent); + } + } + return { inCommunity, other }; +} + +/** + * The managed-agents polling gate: poll only while an agent *in this + * community* is running. Agents running in other communities don't render + * process state on this community's surfaces, so they must not keep its + * 5s liveness poll alive. + */ +export function hasRunningAgentInCommunity( + agents: readonly { relayUrl?: string | null; status: string }[] | undefined, + communityRelayUrl: string | null | undefined, +): boolean { + return (agents ?? []).some( + (agent) => + agent.status === "running" && + agentBelongsToRelay(agent.relayUrl, communityRelayUrl), + ); +} diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index 43146e9932..9ceeb0220e 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -1,6 +1,7 @@ import * as React from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { hasRunningAgentInCommunity } from "@/features/agents/agentRelayScope"; import { connectAcpRuntime, discoverAcpAuthMethods, @@ -11,6 +12,7 @@ import { ensureChannelAgentPresetInChannel, provisionChannelManagedAgent, } from "@/features/agents/channelAgents"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import { resolveSnapshotAvatarPng } from "@/features/agents/ui/snapshotAvatarPng"; import { channelsQueryKey, @@ -310,6 +312,7 @@ export function useRelayAgentsQuery(options?: { enabled?: boolean }) { } export function useManagedAgentsQuery(options?: { enabled?: boolean }) { + const activeRelayUrl = useActiveRelayUrl(); return useQuery({ enabled: options?.enabled ?? true, queryKey: managedAgentsQueryKey, @@ -321,10 +324,11 @@ export function useManagedAgentsQuery(options?: { enabled?: boolean }) { // with no relay event to signal it, so this poll is the only liveness // path for them. When nothing is running there IS an event path — // `agents-data-changed` (control-plane changes) — so the idle branch - // drops its poll entirely rather than falling back to 30s. - return agents?.some((agent) => agent.status === "running") - ? 5_000 - : false; + // drops its poll entirely rather than falling back to 30s. Scoped to + // the active community's relay: agents left running in other + // communities render no process state on this community's surfaces, + // so they must not keep its poll alive. + return hasRunningAgentInCommunity(agents, activeRelayUrl) ? 5_000 : false; }, }); } diff --git a/desktop/src/features/agents/knownAgentPubkeys.test.mjs b/desktop/src/features/agents/knownAgentPubkeys.test.mjs index 2b7b3e18af..57ae4a088d 100644 --- a/desktop/src/features/agents/knownAgentPubkeys.test.mjs +++ b/desktop/src/features/agents/knownAgentPubkeys.test.mjs @@ -34,3 +34,60 @@ test("normalisesCaseAndWhitespace_dedupingAcrossSources", () => { assert.deepEqual([...merged], [MANAGED]); }); + +// ── relay scoping ──────────────────────────────────────────────────────────── + +const RELAY_A = "ws://relay-a.example.com:3000"; +const RELAY_B = "wss://relay-b.example.com"; + +test("managedAgentPinnedToOtherRelay_excludedWhenScoped", () => { + // With multiple communities' agents running concurrently, a managed agent + // pinned to relay B is not an agent "in" community A — it must not enter + // A's known-agent baseline just because it's locally managed. + const merged = mergeKnownAgentPubkeys( + [ + { pubkey: MANAGED, relayUrl: RELAY_B }, + // Cosmetic URL differences must not split an agent from its community. + { pubkey: RELAY, relayUrl: `${RELAY_A}/` }, + ], + undefined, + RELAY_A, + ); + + assert.deepEqual([...merged], [RELAY]); +}); + +test("blankPinnedRelay_followsActiveCommunity", () => { + // Defense-in-depth mirror of the backend's blank-relay fallback: a record + // that escaped stamping belongs to whichever community is visited. + const merged = mergeKnownAgentPubkeys( + [{ pubkey: MANAGED, relayUrl: "" }], + undefined, + RELAY_A, + ); + + assert.deepEqual([...merged], [MANAGED]); +}); + +test("relayAgents_neverRelayFiltered", () => { + // Relay agents come from the active relay's own kind:10100 profiles — + // already community-scoped at the source, so the merge must keep them + // even when a managed-agent scope is in effect. + const merged = mergeKnownAgentPubkeys( + [{ pubkey: MANAGED, relayUrl: RELAY_B }], + [{ pubkey: RELAY }], + RELAY_A, + ); + + assert.deepEqual([...merged], [RELAY]); +}); + +test("noActiveRelay_degradesToUnscopedMerge", () => { + const merged = mergeKnownAgentPubkeys( + [{ pubkey: MANAGED, relayUrl: RELAY_B }], + undefined, + null, + ); + + assert.deepEqual([...merged], [MANAGED]); +}); diff --git a/desktop/src/features/agents/knownAgentPubkeys.ts b/desktop/src/features/agents/knownAgentPubkeys.ts index 978509af09..51a98f5236 100644 --- a/desktop/src/features/agents/knownAgentPubkeys.ts +++ b/desktop/src/features/agents/knownAgentPubkeys.ts @@ -1,3 +1,4 @@ +import { agentBelongsToRelay } from "@/features/agents/agentRelayScope"; import { normalizePubkey } from "@/shared/lib/pubkey"; /** @@ -5,15 +6,27 @@ import { normalizePubkey } from "@/shared/lib/pubkey"; * normalised via `normalizePubkey` so membership checks work against * normalised pubkeys. * - * Structurally typed on `{ pubkey }` so node unit tests don't need to build - * full `ManagedAgent`/`RelayAgent` values. + * Managed agents are scoped to `activeRelayUrl` (the active community's + * relay): the baseline answers "is this pubkey an agent *in this + * community*", and a locally managed agent pinned to another community's + * relay is not — it neither posts here nor appears in this community's + * directory. An agent genuinely present on both relays is still covered by + * the relay-agent source (kind:10100 from the active relay), which is never + * relay-filtered. Omitting `activeRelayUrl` degrades to the unscoped merge. + * + * Structurally typed on `{ pubkey, relayUrl? }` so node unit tests don't + * need to build full `ManagedAgent`/`RelayAgent` values. */ export function mergeKnownAgentPubkeys( - managedAgents: readonly { pubkey: string }[] | undefined, + managedAgents: + | readonly { pubkey: string; relayUrl?: string | null }[] + | undefined, relayAgents: readonly { pubkey: string }[] | undefined, + activeRelayUrl?: string | null, ): ReadonlySet { const pubkeys = new Set(); for (const agent of managedAgents ?? []) { + if (!agentBelongsToRelay(agent.relayUrl, activeRelayUrl)) continue; pubkeys.add(normalizePubkey(agent.pubkey)); } for (const agent of relayAgents ?? []) { diff --git a/desktop/src/features/agents/lib/autoRestartPolicy.test.mjs b/desktop/src/features/agents/lib/autoRestartPolicy.test.mjs index d36d4c7a18..165100eb93 100644 --- a/desktop/src/features/agents/lib/autoRestartPolicy.test.mjs +++ b/desktop/src/features/agents/lib/autoRestartPolicy.test.mjs @@ -23,6 +23,7 @@ function greenInputs(overrides = {}) { workingSource: "none", connected: true, isLocalBackend: true, + inActiveCommunity: true, isRunning: true, edgeConsumed: false, quiescentForMs: AUTO_RESTART_QUIESCENCE_MS, @@ -55,6 +56,7 @@ const NEVER_FIRE_ROWS = [ ["typing source alone defers", { workingSource: "typing" }], ["observer relay not connected", { connected: false }], ["remote backend", { isLocalBackend: false }], + ["agent pinned to another community's relay", { inActiveCommunity: false }], ["agent not running", { isRunning: false }], [ "edge already consumed (one attempt per rising edge)", @@ -72,6 +74,30 @@ for (const [label, overrides] of NEVER_FIRE_ROWS) { }); } +// ── workspace switch ───────────────────────────────────────────────────────── + +test("a pure workspace switch never fires a restart", () => { + // Switching communities leaves the previous community's agents running, + // and their working/observer signals are read from the ACTIVE relay only — + // so from the new community they read as idle even mid-turn. Even with a + // fully elapsed quiescence window and every other gate green, an agent + // pinned to another community's relay must hold, whatever the (blind) + // connected/working signals claim. + for (const overrides of [ + {}, + { connected: true, working: false, workingSource: "none" }, + { quiescentForMs: AUTO_RESTART_QUIESCENCE_MS * 10 }, + ]) { + assert.equal( + decideAutoRestart( + greenInputs({ ...overrides, inActiveCommunity: false }), + ), + "hold", + "an out-of-community fire is a kill decided on blind data", + ); + } +}); + // ── the quiescence window ─────────────────────────────────────────────────── test("arms (does not fire) before the window elapses", () => { diff --git a/desktop/src/features/agents/lib/autoRestartPolicy.ts b/desktop/src/features/agents/lib/autoRestartPolicy.ts index ad7a18a253..033f7e18d2 100644 --- a/desktop/src/features/agents/lib/autoRestartPolicy.ts +++ b/desktop/src/features/agents/lib/autoRestartPolicy.ts @@ -33,6 +33,13 @@ export type AutoRestartInputs = { connected: boolean; /** Only local agents can be restarted by this loop. */ isLocalBackend: boolean; + /** Whether the agent is pinned to the active community's relay. Agents + * from other communities keep running after a workspace switch, but their + * working/observer signals are read from the ACTIVE relay only — they + * look idle and disconnected here regardless of what they're actually + * doing, so any decision about them would be made on blind data. A pure + * workspace switch must never fire a restart. */ + inActiveCommunity: boolean; /** Agent process status from the summary ("running" required). */ isRunning: boolean; /** Edge-trigger state: true when this needsRestart rising edge has @@ -58,6 +65,7 @@ export function decideAutoRestart( workingSource, connected, isLocalBackend, + inActiveCommunity, isRunning, edgeConsumed, quiescentForMs, @@ -67,6 +75,7 @@ export function decideAutoRestart( if (!autoRestartEnabled) return "hold"; if (!needsRestart) return "hold"; if (!isLocalBackend) return "hold"; + if (!inActiveCommunity) return "hold"; if (!isRunning) return "hold"; if (!connected) return "hold"; // Any working signal — observer OR typing — defers. `working` and diff --git a/desktop/src/features/agents/lib/useAutoRestartPolicy.ts b/desktop/src/features/agents/lib/useAutoRestartPolicy.ts index 7e30930900..bafcdea2c3 100644 --- a/desktop/src/features/agents/lib/useAutoRestartPolicy.ts +++ b/desktop/src/features/agents/lib/useAutoRestartPolicy.ts @@ -5,12 +5,14 @@ import { managedAgentsQueryKey, useManagedAgentsQuery, } from "@/features/agents/hooks"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import { startManagedAgent, stopManagedAgent, } from "@/shared/api/tauriManagedAgents"; import { listManagedAgents } from "@/shared/api/tauri"; import type { ManagedAgent } from "@/shared/api/types"; +import { agentBelongsToRelay } from "../agentRelayScope"; import { getAgentObserverSnapshot } from "../observerRelayStore"; import { getAgentWorkingState } from "../agentWorkingSignal"; import { @@ -36,6 +38,7 @@ const POLICY_TICK_MS = 15_000; export function useAutoRestartPolicy() { const queryClient = useQueryClient(); const agents: ManagedAgent[] | undefined = useManagedAgentsQuery().data; + const activeRelayUrl = useActiveRelayUrl(); const edgesRef = React.useRef(new Map()); const inFlightRef = React.useRef(new Set()); const [, setTick] = React.useState(0); @@ -71,6 +74,7 @@ export function useAutoRestartPolicy() { workingSource: working.source, connected: observer.connectionState === "open", isLocalBackend: agent.backend.type === "local", + inActiveCommunity: agentBelongsToRelay(agent.relayUrl, activeRelayUrl), isRunning, edgeConsumed: edge.consumed, quiescentForMs: edge.armedAt === null ? 0 : now - edge.armedAt, @@ -97,13 +101,16 @@ export function useAutoRestartPolicy() { void (async () => { try { - // Pre-fire re-fetch: shrink the stale-decision window to ~0. + // Pre-fire re-fetch: shrink the stale-decision window to ~0. The + // relay pin is re-checked too — a rebind (community relay edit) + // between decision and fire must not restart a foreign agent. const fresh = await listManagedAgents(); const current = fresh.find((a) => a.pubkey === agent.pubkey); if ( !current?.needsRestart || !current.autoRestartOnConfigChange || current.status !== "running" || + !agentBelongsToRelay(current.relayUrl, activeRelayUrl) || getAgentWorkingState(agent.pubkey).source !== "none" ) { return; diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index a00ad6291e..cbe6df262f 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -136,7 +136,21 @@ export function AgentsView() { ) : null} } - description="Set up and manage your agents." + description={ + <> + Set up and manage your agents. + {agents.otherCommunityRunningCount > 0 ? ( + + {agents.otherCommunityRunningCount === 1 + ? "1 agent running in other communities." + : `${agents.otherCommunityRunningCount} agents running in other communities.`} + + ) : null} + + } title="Agents" />
@@ -323,7 +337,10 @@ export function AgentsView() { {personas.personaToDelete ? ( a.personaId === personas.personaToDelete?.id, ).length } diff --git a/desktop/src/features/agents/ui/useManagedAgentActions.ts b/desktop/src/features/agents/ui/useManagedAgentActions.ts index 4eb9b6ed03..b38128b0e8 100644 --- a/desktop/src/features/agents/ui/useManagedAgentActions.ts +++ b/desktop/src/features/agents/ui/useManagedAgentActions.ts @@ -12,8 +12,10 @@ import { useStopManagedAgentMutation, useDeleteManagedAgentMutation, } from "@/features/agents/hooks"; +import { partitionAgentsByRelay } from "@/features/agents/agentRelayScope"; import { useGlobalAgentConfig } from "@/features/agents/useGlobalAgentConfig"; import { useChannelsQuery } from "@/features/channels/hooks"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import { usePresenceQuery } from "@/features/presence/hooks"; import type { AgentPersona, @@ -75,16 +77,33 @@ export function useManagedAgentActions() { return () => window.clearTimeout(timeoutId); }, []); + // The agents screen is presented per community ("Agents in this + // community"), so scope the list — and every action derived from it (bulk + // stop, start/stop/delete lookups, presence) — to records pinned to the + // active community's relay. Agents pinned elsewhere keep running; they + // surface only through `otherCommunityRunningCount`. + const activeRelayUrl = useActiveRelayUrl(); + const allManagedAgents = managedAgentsQuery.data; + const relayPartition = React.useMemo( + () => partitionAgentsByRelay(allManagedAgents, activeRelayUrl), + [allManagedAgents, activeRelayUrl], + ); + const managedAgents = React.useMemo( () => - [...(managedAgentsQuery.data ?? [])].sort((left, right) => { + [...relayPartition.inCommunity].sort((left, right) => { const activeScore = (s: string) => s === "running" || s === "deployed" ? 1 : 0; const diff = activeScore(right.status) - activeScore(left.status); if (diff !== 0) return diff; return left.name.localeCompare(right.name); }), - [managedAgentsQuery.data], + [relayPartition], + ); + + const otherCommunityRunningCount = React.useMemo( + () => relayPartition.other.filter(isManagedAgentActive).length, + [relayPartition], ); // Observer ingestion is owner-global (useAgentObserverIngestion in // AppShell); this hook only reads derived state. @@ -395,6 +414,10 @@ export function useManagedAgentActions() { managedAgentLogQuery, managedPresenceQuery, managedAgents, + /** Unscoped record set — for cross-community concerns only (e.g. how + * many instances a persona delete would remove across ALL communities). */ + allManagedAgents, + otherCommunityRunningCount, managedPubkeys, channelIdToName, channelsByPubkey, diff --git a/desktop/src/features/agents/useKnownAgentPubkeys.tsx b/desktop/src/features/agents/useKnownAgentPubkeys.tsx index e9fe7b9a9b..1ae9a7efaa 100644 --- a/desktop/src/features/agents/useKnownAgentPubkeys.tsx +++ b/desktop/src/features/agents/useKnownAgentPubkeys.tsx @@ -5,6 +5,7 @@ import { useRelayAgentsQuery, } from "@/features/agents/hooks"; import { mergeKnownAgentPubkeys } from "@/features/agents/knownAgentPubkeys"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import { useStableSet } from "@/shared/hooks/useStableReference"; const EMPTY_KNOWN_AGENT_PUBKEYS: ReadonlySet = new Set(); @@ -41,10 +42,13 @@ export function KnownAgentPubkeysProvider({ }) { const managedAgents = useManagedAgentsQuery().data; const relayAgents = useRelayAgentsQuery().data; + // Scope managed agents to the active community's relay: another + // community's still-running agents are not agents "in" this community. + const activeRelayUrl = useActiveRelayUrl(); const merged = React.useMemo( - () => mergeKnownAgentPubkeys(managedAgents, relayAgents), - [managedAgents, relayAgents], + () => mergeKnownAgentPubkeys(managedAgents, relayAgents, activeRelayUrl), + [managedAgents, relayAgents, activeRelayUrl], ); const stable = useStableSet(merged); diff --git a/desktop/src/features/communities/useCommunities.tsx b/desktop/src/features/communities/useCommunities.tsx index 590e28f842..9a7759393a 100644 --- a/desktop/src/features/communities/useCommunities.tsx +++ b/desktop/src/features/communities/useCommunities.tsx @@ -112,6 +112,19 @@ export function useCommunities(): UseCommunitiesReturn { return ctx; } +/** + * Lenient read of the active community's relay URL — the key that scopes + * managed agents to a community (records are pinned to their home relay). + * Unlike `useCommunities`, this returns `null` instead of throwing outside + * `CommunitiesProvider`, so relay-scoping consumers (agent lists, polling + * gates, the auto-restart policy) degrade to unscoped in provider-less + * mounts rather than crashing. + */ +export function useActiveRelayUrl(): string | null { + const ctx = useContext(CommunitiesContext); + return ctx?.activeCommunity?.relayUrl ?? null; +} + function useCommunitiesInternal(): UseCommunitiesReturn { const [communities, setCommunitiesState] = useState(loadCommunities); From 81cd8d0dfc60523d26dfe69d82cf93ed0ed70a88 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Jul 2026 14:20:27 +1000 Subject: [PATCH 04/12] feat(desktop): isolate agent repos dirs per workspace Make agents immune to the shared REPOS symlink re-pointing on workspace switches (Phase 4 of lazy multi-workspace agents, building on the Phase 1-3 relay pinning, lazy activation, and UI scoping): - Persist repos_dir per relay URL in a new .repos-dirs.json nest dotfile (keys normalized like agent relay pins), written by every apply_workspace alongside the single-value .repos-dir that keeps driving the boot-time symlink resolve. The REPOS symlink itself keeps following the active workspace as a human/tooling convention. - At spawn, resolve the agent's own workspace entry (keyed by its effective relay) to a canonical real path and hand it to the child as BUZZ_REPOS_DIR, so a mid-task switch that re-points REPOS can no longer land a workspace-A agent in workspace B's checkouts. A configured-but-unresolvable entry fails the spawn closed (same rationale as resolve_repos_at_boot); no entry falls back to the nest REPOS path, preserving pre-map behavior. - Adjust the nest AGENTS.md instructions (template v5) that referenced REPOS/ relatively: agents are told to address checkouts through $BUZZ_REPOS_DIR whenever it is set. - Hash the raw per-relay map value into the spawn-config hash so a repos-dir edit badges needsRestart while a pure workspace switch (and filesystem state alone) cannot. - Migrate the map entry in rebind_agent_relay when a community's relay URL is edited, so rebound agents keep their repos-dir isolation. - Split the grown repos.rs test module into repos/tests.rs (file-size guard), mirroring nest/tests.rs and spawn_hash/tests.rs. Tested: cargo test --manifest-path desktop/src-tauri/Cargo.toml (1433 passed, incl. 7 new repos-map/resolution tests and 2 new spawn-hash tests), clippy --all-targets -D warnings, rustfmt --check, cargo check --features mesh-llm. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- desktop/src-tauri/src/commands/workspace.rs | 27 +- desktop/src-tauri/src/managed_agents/mod.rs | 5 +- desktop/src-tauri/src/managed_agents/nest.rs | 2 +- .../src/managed_agents/nest_agents.md | 3 +- desktop/src-tauri/src/managed_agents/repos.rs | 669 ++++------------ .../src/managed_agents/repos/tests.rs | 714 ++++++++++++++++++ .../src-tauri/src/managed_agents/runtime.rs | 37 +- .../src/managed_agents/spawn_hash.rs | 13 + .../src/managed_agents/spawn_hash/tests.rs | 422 +++++++++-- .../src-tauri/src/migration/backfill_tests.rs | 4 + 10 files changed, 1335 insertions(+), 561 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/repos/tests.rs diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 3622338dc6..79a344f1c6 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -6,8 +6,9 @@ use tauri::{AppHandle, Emitter, Manager, State}; use crate::app_state::AppState; use crate::managed_agents::{ activate_workspace_agents, effective_repos_dir, ensure_repos_symlink, load_managed_agents, - nest_dir, rebind_agent_relay_urls, save_managed_agents, stamp_blank_agent_relay_urls, - try_regenerate_nest, write_persisted_repos_dir, + nest_dir, persist_workspace_repos_dir, rebind_agent_relay_urls, rebind_workspace_repos_dir, + save_managed_agents, stamp_blank_agent_relay_urls, try_regenerate_nest, + write_persisted_repos_dir, }; use crate::relay; @@ -180,6 +181,18 @@ pub async fn apply_workspace( if let Err(error) = write_persisted_repos_dir(nest, effective_repos_dir.as_deref()) { eprintln!("buzz-desktop: persist repos dir failed: {error}"); } + // Per-relay map entry for THIS workspace (Phase 4 — REPOS + // isolation): spawn resolves an agent's repos dir from this map + // by the agent's pinned relay, so agents stay immune to the REPOS + // symlink re-point below when a later switch moves it. Must land + // before the activation task spawned after this closure starts + // this workspace's agents. A bad candidate clears the entry, same + // as the dotfile above. + if let Err(error) = + persist_workspace_repos_dir(nest, &relay_url, effective_repos_dir.as_deref()) + { + eprintln!("buzz-desktop: persist per-relay repos dir failed: {error}"); + } if let Err(error) = ensure_repos_symlink(nest, effective_repos_dir.as_deref()) { eprintln!("buzz-desktop: repos dir setup failed: {error}"); let _ = app.emit("repos-dir-error", error); @@ -274,6 +287,16 @@ pub async fn rebind_agent_relay( if changed > 0 { save_managed_agents(&app, &records)?; } + // Carry the workspace's per-relay repos_dir along with its agents — + // spawn resolves BUZZ_REPOS_DIR from the map by pinned relay, so + // leaving the entry on the old URL would silently drop repos-dir + // isolation for every rebound agent. Non-fatal: the next + // apply_workspace of the edited community re-stamps the entry. + if let Some(nest) = nest_dir() { + if let Err(error) = rebind_workspace_repos_dir(&nest, &old_relay_url, &new_relay_url) { + eprintln!("buzz-desktop: rebind per-relay repos dir failed: {error}"); + } + } Ok(changed) }) .await diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 98f91df89a..2215c2003b 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -63,8 +63,9 @@ pub(crate) use readiness::{ pub use relay_mesh::*; pub(crate) use relay_pinning::{rebind_agent_relay_urls, stamp_blank_agent_relay_urls}; pub use repos::{ - effective_repos_dir, ensure_repos_symlink, resolve_repos_at_boot, validate_repos_dir, - write_persisted_repos_dir, + effective_repos_dir, ensure_repos_symlink, persist_workspace_repos_dir, + rebind_workspace_repos_dir, resolve_agent_repos_dir, resolve_repos_at_boot, validate_repos_dir, + workspace_repos_dir_for_relay, write_persisted_repos_dir, }; pub use restore::*; pub use runtime::*; diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index e13ab2baad..ef81a98c55 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -46,7 +46,7 @@ const BUZZ_CLI_SKILL_MD: &str = include_str!("nest_skill.md"); /// Template content version for AGENTS.md static content (above managed markers). /// Bump this when changing `nest_agents.md` to trigger refresh on existing installs. /// Version 1 is implicitly "before this mechanism existed" (no version file). -const NEST_AGENTS_VERSION: u32 = 4; +const NEST_AGENTS_VERSION: u32 = 5; /// Template content version for SKILL.md. /// Bump this when changing `nest_skill.md` to trigger refresh on existing installs. diff --git a/desktop/src-tauri/src/managed_agents/nest_agents.md b/desktop/src-tauri/src/managed_agents/nest_agents.md index 7cb7489b85..1a11c98bc5 100644 --- a/desktop/src-tauri/src/managed_agents/nest_agents.md +++ b/desktop/src-tauri/src/managed_agents/nest_agents.md @@ -11,7 +11,7 @@ Your persistent workspace. Created once by the Buzz desktop app. The static cont | `RESEARCH/` | Findings, notes, and reference material | | `WORK_LOGS/` | Session logs — what was tried, learned, decided | | `OUTBOX/` | Shareable docs for external readers (no frontmatter) | -| `REPOS/` | Source checkouts. Work in an existing local checkout when one exists; clone here only when none does | +| `REPOS/` | Source checkouts. Work in an existing local checkout when one exists; clone here only when none does. If `BUZZ_REPOS_DIR` is set in your environment, use that absolute path instead of `REPOS/` — see Core Guidelines | | `.scratch/` | Temporary working files — treat as disposable between sessions | Filenames: `ALL_CAPS_WITH_UNDERSCORES.md` (e.g., `OAUTH_FLOW_NOTES.md`). @@ -37,6 +37,7 @@ created: 2026-01-15 ## Core Guidelines +- **Reach source checkouts via `$BUZZ_REPOS_DIR`** — when set, this env var is the absolute path to *your* workspace's repos directory. Always address checkouts through it (e.g. `"$BUZZ_REPOS_DIR"/project`), never the relative `REPOS/` link: the `REPOS` symlink follows whichever workspace is active in the desktop app and can re-point mid-task. Only fall back to `REPOS/` when the env var is unset - **Local first** — check `RESEARCH/`, `GUIDES/`, `PLANS/` before external searches - **Write findings down** — if you research something, save it to `RESEARCH/` - **Cite sources** — no claim without a path, link, or reference diff --git a/desktop/src-tauri/src/managed_agents/repos.rs b/desktop/src-tauri/src/managed_agents/repos.rs index 1c77c5722e..b7c6b8d536 100644 --- a/desktop/src-tauri/src/managed_agents/repos.rs +++ b/desktop/src-tauri/src/managed_agents/repos.rs @@ -4,7 +4,15 @@ //! symlink to a user-configured `repos_dir`, letting agents work in existing //! local checkouts instead of re-cloning. [`ensure_repos_symlink`] reconciles //! `REPOS` with the configured path; [`validate_repos_dir`] guards the input. - +//! +//! The symlink follows the *active* workspace — a human/tooling convention. +//! Agents are made immune to it: every applied workspace's `repos_dir` is +//! also persisted per relay URL ([`persist_workspace_repos_dir`]), and spawn +//! resolves the agent's own workspace entry to a real path +//! ([`resolve_agent_repos_dir`]) so a later workspace switch re-pointing +//! `REPOS` cannot move a running agent into another workspace's checkouts. + +use std::collections::BTreeMap; use std::fs; use std::io; use std::path::{Path, PathBuf}; @@ -229,6 +237,155 @@ pub fn write_persisted_repos_dir(nest_root: &Path, repos_dir: Option<&str>) -> R } } +// ── Per-relay repos_dir map ─────────────────────────────────────────────── + +/// Filename of the dotfile persisting every workspace's `repos_dir`, keyed by +/// normalized relay URL. Distinct from [`REPOS_DIR_FILE`], which holds only +/// the *active* workspace's value for the boot-time symlink resolve. +const REPOS_DIR_MAP_FILE: &str = ".repos-dirs.json"; + +/// Read the per-relay `repos_dir` map from `nest_root/.repos-dirs.json`. +/// +/// Keys are normalized relay URLs ([`crate::relay::normalize_relay_url`]); +/// values are raw trimmed `repos_dir` strings, exactly as persisted by +/// [`persist_workspace_repos_dir`]. An absent, unreadable, or malformed file +/// yields an empty map — the map is an isolation layer over spawn, not a boot +/// dependency, so it degrades to the pre-map default rather than failing. +/// Keys are re-normalized and empty entries dropped on read, as defense +/// against a hand-edited file. +fn read_repos_dir_map(nest_root: &Path) -> BTreeMap { + let Ok(raw) = fs::read_to_string(nest_root.join(REPOS_DIR_MAP_FILE)) else { + return BTreeMap::new(); + }; + let Ok(parsed) = serde_json::from_str::>(&raw) else { + eprintln!("buzz-desktop: {REPOS_DIR_MAP_FILE} is malformed; ignoring"); + return BTreeMap::new(); + }; + parsed + .into_iter() + .filter_map(|(relay, dir)| { + let key = crate::relay::normalize_relay_url(&relay); + let value = dir.trim().to_string(); + (!key.is_empty() && !value.is_empty()).then_some((key, value)) + }) + .collect() +} + +/// Persist the per-relay `repos_dir` map, removing the file when empty +/// (mirrors [`write_persisted_repos_dir`]'s clear-by-removal). +fn write_repos_dir_map(nest_root: &Path, map: &BTreeMap) -> Result<(), String> { + let path = nest_root.join(REPOS_DIR_MAP_FILE); + if map.is_empty() { + return match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("remove {}: {e}", path.display())), + }; + } + let json = serde_json::to_string_pretty(map) + .map_err(|e| format!("serialize {REPOS_DIR_MAP_FILE}: {e}"))?; + fs::write(&path, format!("{json}\n")).map_err(|e| format!("write {}: {e}", path.display())) +} + +/// Upsert (or clear, on `None`/empty) the applied workspace's `repos_dir` in +/// the per-relay map. +/// +/// Written on every `apply_workspace`, alongside the single-value +/// [`REPOS_DIR_FILE`]: that dotfile drives the active-workspace `REPOS` +/// symlink at boot, while this map is what makes agents immune to the symlink +/// — spawn resolves the agent's own workspace entry via +/// [`resolve_agent_repos_dir`], keyed by the agent's pinned relay. Values are +/// stored raw (trimmed, not canonicalized) for the same reason +/// [`effective_repos_dir`] persists raw. +pub fn persist_workspace_repos_dir( + nest_root: &Path, + relay_url: &str, + repos_dir: Option<&str>, +) -> Result<(), String> { + let key = crate::relay::normalize_relay_url(relay_url); + if key.is_empty() { + return Err("cannot persist a repos dir for an empty relay URL".to_string()); + } + let mut map = read_repos_dir_map(nest_root); + match repos_dir.map(str::trim).filter(|s| !s.is_empty()) { + Some(value) => { + if map.get(&key).map(String::as_str) == Some(value) { + return Ok(()); + } + map.insert(key, value.to_string()); + } + None => { + if map.remove(&key).is_none() { + return Ok(()); + } + } + } + write_repos_dir_map(nest_root, &map) +} + +/// Move a workspace's per-relay `repos_dir` entry when the community's relay +/// URL is edited — the map analogue of `rebind_agent_relay_urls` for agent +/// records. Without this, agents rebound to the new URL would resolve no map +/// entry and silently fall back to the shared `REPOS` default. Returns +/// whether an entry moved; moving onto an existing entry overwrites it (the +/// edit just happened, so the edited community's value is freshest). +pub fn rebind_workspace_repos_dir( + nest_root: &Path, + old_relay_url: &str, + new_relay_url: &str, +) -> Result { + let old_key = crate::relay::normalize_relay_url(old_relay_url); + let new_key = crate::relay::normalize_relay_url(new_relay_url); + if new_key.is_empty() || old_key == new_key { + return Ok(false); + } + let mut map = read_repos_dir_map(nest_root); + let Some(value) = map.remove(&old_key) else { + return Ok(false); + }; + map.insert(new_key, value); + write_repos_dir_map(nest_root, &map)?; + Ok(true) +} + +/// The raw persisted `repos_dir` for the workspace identified by `relay_url`, +/// or `None` when that workspace has no override (or has not been applied +/// since the per-relay map was introduced). Raw and un-canonicalized so it is +/// a stable spawn-config-hash input — filesystem state changes alone must not +/// flip the restart badge; [`resolve_agent_repos_dir`] does the real-path +/// resolution separately at spawn. +pub fn workspace_repos_dir_for_relay(nest_root: &Path, relay_url: &str) -> Option { + read_repos_dir_map(nest_root).remove(&crate::relay::normalize_relay_url(relay_url)) +} + +/// Resolve the repos directory a spawning agent should be handed +/// (`BUZZ_REPOS_DIR`), from the per-relay map entry for the agent's effective +/// relay. +/// +/// - **Entry present** → the validated, canonical **real path** of that +/// workspace's `repos_dir`. Real, so the agent stays in its own workspace's +/// checkouts even while the `REPOS` symlink follows a different active +/// workspace. An unresolvable entry (unmounted volume, deleted dir) is an +/// `Err` — the spawn must fail CLOSED, mirroring [`resolve_repos_at_boot`]: +/// a refused start is recoverable, work landing in another workspace's +/// checkout is not. +/// - **No entry** → the nest's `REPOS` path (the shared in-nest default). +/// Deliberately NOT canonicalized: workspaces without an override share the +/// in-nest `REPOS` by design, and baking in a symlink target that happens +/// to be active now would be worse than the convention path. +pub fn resolve_agent_repos_dir(nest_root: &Path, relay_url: &str) -> Result { + match workspace_repos_dir_for_relay(nest_root, relay_url) { + Some(dir) => validate_repos_dir(nest_root, &dir).map_err(|reason| { + format!( + "repos dir `{dir}` configured for this agent's workspace ({relay_url}) cannot be \ + resolved: {reason}; refusing to start the agent so it cannot work in another \ + workspace's repos directory" + ) + }), + None => Ok(nest_root.join("REPOS")), + } +} + /// Decide whether launch-time agent restore is safe given the boot symlink /// outcome. Fails CLOSED: when a `repos_dir` was configured but its symlink /// could not be resolved, restoring agents would let one clone into the empty @@ -279,512 +436,4 @@ pub fn resolve_repos_at_boot(nest_root: &Path) -> bool { } #[cfg(test)] -mod tests { - use super::*; - - // ── ensure_repos_symlink ────────────────────────────────────────────── - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_none_creates_real_dir() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - - ensure_repos_symlink(&root, None).unwrap(); - - let repos = root.join("REPOS"); - assert!(repos.is_dir(), "REPOS should be a real directory"); - assert!( - !repos.symlink_metadata().unwrap().file_type().is_symlink(), - "REPOS should not be a symlink when repos_dir is None" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_none_reverts_existing_symlink_to_real_dir() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - let payload = external.join("KEEP.md"); - fs::write(&payload, "data").unwrap(); - - // First point REPOS at the external dir, then clear the field. - ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); - ensure_repos_symlink(&root, None).unwrap(); - - let repos = root.join("REPOS"); - assert!( - repos.is_dir(), - "REPOS should be a real directory after clear" - ); - assert!( - !repos.symlink_metadata().unwrap().file_type().is_symlink(), - "REPOS should no longer be a symlink after clearing repos_dir" - ); - assert!( - payload.exists(), - "clearing repos_dir must not touch the external target's contents" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_absent_creates_symlink() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - - ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); - - let repos = root.join("REPOS"); - assert!(repos.symlink_metadata().unwrap().file_type().is_symlink()); - assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); - } - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_repoints_existing_wrong_symlink() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let old = tmp.path().join("old"); - let new = tmp.path().join("new"); - fs::create_dir_all(&old).unwrap(); - fs::create_dir_all(&new).unwrap(); - let payload = old.join("KEEP.md"); - fs::write(&payload, "data").unwrap(); - - ensure_repos_symlink(&root, Some(old.to_str().unwrap())).unwrap(); - ensure_repos_symlink(&root, Some(new.to_str().unwrap())).unwrap(); - - let repos = root.join("REPOS"); - assert_eq!(repos.read_link().unwrap(), new.canonicalize().unwrap()); - assert!( - payload.exists(), - "re-pointing a symlink must not touch the old target's contents" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_correct_symlink_is_noop() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let external = tmp.path().join("Development").canonicalize_or_make(); - - ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); - ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); - - let repos = root.join("REPOS"); - assert_eq!(repos.read_link().unwrap(), external); - } - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_empty_real_dir_converts() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(root.join("REPOS")).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - - ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); - - let repos = root.join("REPOS"); - assert!( - repos.symlink_metadata().unwrap().file_type().is_symlink(), - "an empty real REPOS should convert to a symlink" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_repos_symlink_nonempty_real_dir_refuses_and_preserves() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - let repos = root.join("REPOS"); - fs::create_dir_all(&repos).unwrap(); - let checkout = repos.join("buzz"); - fs::create_dir_all(&checkout).unwrap(); - fs::write(checkout.join("code.rs"), "fn main() {}").unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - - let result = ensure_repos_symlink(&root, Some(external.to_str().unwrap())); - - assert!(result.is_err(), "non-empty real REPOS must refuse"); - assert!( - !repos.symlink_metadata().unwrap().file_type().is_symlink(), - "refused REPOS must stay a real directory" - ); - assert!( - checkout.join("code.rs").exists(), - "refusal must never delete existing repositories" - ); - } - - // ensure_nest_at must NOT clobber an existing REPOS symlink on startup. - // Regression guard for Finding 1: the startup `ensure_repos_symlink(_, None)` - // call used to remove a configured symlink and mint an empty real REPOS, - // which async-restored agents could write into — the FE re-point then - // refused the now-non-empty dir, silently breaking a configured repos_dir. - #[cfg(unix)] - #[test] - fn ensure_nest_startup_preserves_existing_repos_symlink() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - - // First launch creates the real nest with a real REPOS dir. - crate::managed_agents::ensure_nest_at(&root).unwrap(); - - // Simulate a configured repos_dir: REPOS points at an external dir - // holding agent checkouts. - let external = tmp.path().join("Development"); - fs::create_dir(&external).unwrap(); - fs::write(external.join("KEEP.md"), "data").unwrap(); - fs::remove_dir(root.join("REPOS")).unwrap(); - std::os::unix::fs::symlink(&external, root.join("REPOS")).unwrap(); - - // Next launch must leave the configured symlink intact. - crate::managed_agents::ensure_nest_at(&root).unwrap(); - - let repos = root.join("REPOS"); - assert!( - repos.symlink_metadata().unwrap().file_type().is_symlink(), - "an existing REPOS symlink must survive startup" - ); - assert_eq!(repos.read_link().unwrap(), external); - assert!( - external.join("KEEP.md").exists(), - "the symlink's target contents must be untouched" - ); - } - - #[cfg(unix)] - #[test] - fn validate_repos_dir_rejects_tilde_relative_and_missing() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - - assert!(validate_repos_dir(&root, "~/Development").is_err()); - assert!(validate_repos_dir(&root, "relative/path").is_err()); - assert!(validate_repos_dir(&root, "/no/such/dir/here").is_err()); - - let file = tmp.path().join("afile"); - fs::write(&file, "x").unwrap(); - assert!(validate_repos_dir(&root, file.to_str().unwrap()).is_err()); - } - - #[cfg(unix)] - #[test] - fn validate_repos_dir_rejects_nest_ancestor() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join("home").join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let parent = root.parent().unwrap(); - - assert!( - validate_repos_dir(&root, parent.to_str().unwrap()).is_err(), - "a parent of the nest would nest REPOS inside its own target" - ); - } - - // ── persisted repos_dir dotfile ─────────────────────────────────────── - - #[test] - fn persisted_repos_dir_roundtrips_write_read_clear() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - - assert_eq!(read_persisted_repos_dir(&root), None, "absent → None"); - - write_persisted_repos_dir(&root, Some(" /Users/me/Development ")).unwrap(); - assert_eq!( - read_persisted_repos_dir(&root).as_deref(), - Some("/Users/me/Development"), - "value is trimmed on write/read" - ); - - write_persisted_repos_dir(&root, None).unwrap(); - assert_eq!(read_persisted_repos_dir(&root), None, "cleared → None"); - assert!( - !root.join(".repos-dir").exists(), - "clearing removes the dotfile" - ); - } - - #[test] - fn persisted_repos_dir_empty_value_clears() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - write_persisted_repos_dir(&root, Some("/Users/me/Development")).unwrap(); - - write_persisted_repos_dir(&root, Some(" ")).unwrap(); - assert_eq!( - read_persisted_repos_dir(&root), - None, - "a whitespace-only value clears the override" - ); - } - - #[test] - fn persisted_repos_dir_clear_when_absent_is_ok() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - - write_persisted_repos_dir(&root, None).expect("clearing an absent dotfile is not an error"); - } - - #[cfg(unix)] - #[test] - fn boot_resolves_symlink_from_persisted_value_into_empty_repos() { - // Mirrors the boot sequence: ensure_nest leaves REPOS an empty real - // dir, then the setup hook reads the persisted value and symlinks. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(root.join("REPOS")).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - - write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); - let persisted = read_persisted_repos_dir(&root); - ensure_repos_symlink(&root, persisted.as_deref()).unwrap(); - - let repos = root.join("REPOS"); - assert!( - repos.symlink_metadata().unwrap().file_type().is_symlink(), - "boot must convert the empty real REPOS into a symlink" - ); - assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); - } - - #[cfg(unix)] - #[test] - fn boot_leaves_already_correct_symlink_untouched() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - - write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); - // First boot converts; second boot must be a noop. - let persisted = read_persisted_repos_dir(&root); - ensure_repos_symlink(&root, persisted.as_deref()).unwrap(); - ensure_repos_symlink(&root, persisted.as_deref()).unwrap(); - - let repos = root.join("REPOS"); - assert!(repos.symlink_metadata().unwrap().file_type().is_symlink()); - assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); - } - - #[cfg(unix)] - #[test] - fn boot_with_cleared_value_reverts_symlink_to_real_dir() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - fs::write(external.join("KEEP.md"), "data").unwrap(); - - // Configure, then clear the field. - write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); - ensure_repos_symlink(&root, read_persisted_repos_dir(&root).as_deref()).unwrap(); - write_persisted_repos_dir(&root, None).unwrap(); - - // Next boot reads None and reverts REPOS to a real in-nest dir. - ensure_repos_symlink(&root, read_persisted_repos_dir(&root).as_deref()).unwrap(); - - let repos = root.join("REPOS"); - assert!( - !repos.symlink_metadata().unwrap().file_type().is_symlink(), - "clearing the persisted value reverts REPOS to a real dir" - ); - assert!( - external.join("KEEP.md").exists(), - "reverting must not touch the external target's contents" - ); - } - - #[cfg(unix)] - #[test] - fn effective_repos_dir_drives_persisted_dotfile_for_all_three_cases() { - // Pins the CRITICAL persist decision on `.repos-dir` CONTENTS, not just - // a return value: a bad path must clear the dotfile so the next boot's - // `should_restore_agents(false, _)` restores agents (the regression - // this hardening fixes). Drives each case through the same - // effective→persist path `apply_workspace` uses. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let good = tmp.path().join("Development"); - fs::create_dir_all(&good).unwrap(); - let good_str = good.to_str().unwrap(); - - // Pre-seed a value so each case proves it overwrites/clears, not just - // that an absent dotfile stays absent. - let seed = |root: &Path| write_persisted_repos_dir(root, Some(good_str)).unwrap(); - let persist = |root: &Path, candidate: Option<&str>| { - let effective = effective_repos_dir(root, candidate); - // Mirror apply_workspace: Err clears the override (persist None). - let to_persist = effective.unwrap_or(None); - write_persisted_repos_dir(root, to_persist.as_deref()).unwrap(); - }; - - // Bad path → Err → dotfile cleared (the CRITICAL). - seed(&root); - persist(&root, Some("/no/such/dir/here")); - assert_eq!( - read_persisted_repos_dir(&root), - None, - "a bad repos_dir must clear `.repos-dir` so the next boot restores agents" - ); - - // Good path → Ok(Some(raw)) → dotfile holds the raw trimmed value. - persist(&root, Some(&format!(" {good_str} "))); - assert_eq!( - read_persisted_repos_dir(&root).as_deref(), - Some(good_str), - "a valid repos_dir must persist the raw trimmed path (not the canonical path)" - ); - - // Empty/whitespace → Ok(None) → dotfile cleared. - seed(&root); - persist(&root, Some(" ")); - assert_eq!( - read_persisted_repos_dir(&root), - None, - "an empty repos_dir clears the override" - ); - - // None candidate → Ok(None) → dotfile cleared. - seed(&root); - persist(&root, None); - assert_eq!( - read_persisted_repos_dir(&root), - None, - "no repos_dir clears the override" - ); - } - - // ── resolve_repos_at_boot (boot sequence + fail-closed gate) ────────── - - #[cfg(unix)] - #[test] - fn resolve_repos_at_boot_converts_empty_real_repos_and_allows_restore() { - // Drives the real boot sequence: ensure_repos_setup_default (called by - // ensure_nest) provisions REPOS as an empty real dir, then the setup - // hook calls resolve_repos_at_boot. Asserts the convert happens at that - // position and restore is allowed. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let external = tmp.path().join("Development"); - fs::create_dir_all(&external).unwrap(); - write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); - - ensure_repos_setup_default(&root).unwrap(); - let repos = root.join("REPOS"); - assert!( - repos.is_dir() && !repos.symlink_metadata().unwrap().file_type().is_symlink(), - "ensure_nest provisions REPOS as an empty real dir before the boot resolve" - ); - - let restore = resolve_repos_at_boot(&root); - - assert!(restore, "restore proceeds when the symlink resolves"); - assert!( - repos.symlink_metadata().unwrap().file_type().is_symlink(), - "boot resolve converts the empty real REPOS into the configured symlink" - ); - assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); - } - - #[cfg(unix)] - #[test] - fn resolve_repos_at_boot_fails_closed_when_target_unresolvable() { - // Persisted repos_dir whose target does not exist at boot (transiently - // unavailable external volume) → ensure_repos_symlink Errs. Restore must - // be skipped and REPOS left as the empty real dir, never the symlink, so - // no agent clones into the wrong place. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - let missing = tmp.path().join("not-mounted-yet"); - write_persisted_repos_dir(&root, Some(missing.to_str().unwrap())).unwrap(); - ensure_repos_setup_default(&root).unwrap(); - - let restore = resolve_repos_at_boot(&root); - - assert!( - !restore, - "restore must be skipped when a configured repos_dir cannot resolve at boot" - ); - let repos = root.join("REPOS"); - assert!( - !repos.symlink_metadata().unwrap().file_type().is_symlink(), - "REPOS must not become a symlink to an unresolved target" - ); - } - - #[cfg(unix)] - #[test] - fn resolve_repos_at_boot_allows_restore_with_no_repos_dir() { - // No configured repos_dir → the real in-nest REPOS default is correct, - // restore proceeds normally (the fail-closed gate must not fire here). - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - fs::create_dir_all(&root).unwrap(); - ensure_repos_setup_default(&root).unwrap(); - - assert!( - resolve_repos_at_boot(&root), - "restore proceeds when no repos_dir is configured" - ); - } - - #[test] - fn should_restore_agents_only_blocks_configured_unresolved_boot() { - assert!( - should_restore_agents(false, &Ok(())), - "no repos_dir, symlink ok → restore" - ); - assert!( - should_restore_agents(false, &Err("boom".into())), - "no repos_dir → real-dir default is correct even on Err → restore" - ); - assert!( - should_restore_agents(true, &Ok(())), - "configured repos_dir resolved → restore" - ); - assert!( - !should_restore_agents(true, &Err("boom".into())), - "configured repos_dir unresolved → fail closed, skip restore" - ); - } - - /// Test helper: canonicalize a path, creating it as a directory first. - #[cfg(unix)] - trait CanonicalizeOrMake { - fn canonicalize_or_make(&self) -> PathBuf; - } - #[cfg(unix)] - impl CanonicalizeOrMake for PathBuf { - fn canonicalize_or_make(&self) -> PathBuf { - fs::create_dir_all(self).unwrap(); - self.canonicalize().unwrap() - } - } -} +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/repos/tests.rs b/desktop/src-tauri/src/managed_agents/repos/tests.rs new file mode 100644 index 0000000000..67a93df548 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/repos/tests.rs @@ -0,0 +1,714 @@ +use super::*; + +// ── ensure_repos_symlink ────────────────────────────────────────────── + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_none_creates_real_dir() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + ensure_repos_symlink(&root, None).unwrap(); + + let repos = root.join("REPOS"); + assert!(repos.is_dir(), "REPOS should be a real directory"); + assert!( + !repos.symlink_metadata().unwrap().file_type().is_symlink(), + "REPOS should not be a symlink when repos_dir is None" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_none_reverts_existing_symlink_to_real_dir() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + let payload = external.join("KEEP.md"); + fs::write(&payload, "data").unwrap(); + + // First point REPOS at the external dir, then clear the field. + ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); + ensure_repos_symlink(&root, None).unwrap(); + + let repos = root.join("REPOS"); + assert!( + repos.is_dir(), + "REPOS should be a real directory after clear" + ); + assert!( + !repos.symlink_metadata().unwrap().file_type().is_symlink(), + "REPOS should no longer be a symlink after clearing repos_dir" + ); + assert!( + payload.exists(), + "clearing repos_dir must not touch the external target's contents" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_absent_creates_symlink() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + + ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); + + let repos = root.join("REPOS"); + assert!(repos.symlink_metadata().unwrap().file_type().is_symlink()); + assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); +} + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_repoints_existing_wrong_symlink() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let old = tmp.path().join("old"); + let new = tmp.path().join("new"); + fs::create_dir_all(&old).unwrap(); + fs::create_dir_all(&new).unwrap(); + let payload = old.join("KEEP.md"); + fs::write(&payload, "data").unwrap(); + + ensure_repos_symlink(&root, Some(old.to_str().unwrap())).unwrap(); + ensure_repos_symlink(&root, Some(new.to_str().unwrap())).unwrap(); + + let repos = root.join("REPOS"); + assert_eq!(repos.read_link().unwrap(), new.canonicalize().unwrap()); + assert!( + payload.exists(), + "re-pointing a symlink must not touch the old target's contents" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_correct_symlink_is_noop() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let external = tmp.path().join("Development").canonicalize_or_make(); + + ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); + ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); + + let repos = root.join("REPOS"); + assert_eq!(repos.read_link().unwrap(), external); +} + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_empty_real_dir_converts() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(root.join("REPOS")).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + + ensure_repos_symlink(&root, Some(external.to_str().unwrap())).unwrap(); + + let repos = root.join("REPOS"); + assert!( + repos.symlink_metadata().unwrap().file_type().is_symlink(), + "an empty real REPOS should convert to a symlink" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_repos_symlink_nonempty_real_dir_refuses_and_preserves() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + let repos = root.join("REPOS"); + fs::create_dir_all(&repos).unwrap(); + let checkout = repos.join("buzz"); + fs::create_dir_all(&checkout).unwrap(); + fs::write(checkout.join("code.rs"), "fn main() {}").unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + + let result = ensure_repos_symlink(&root, Some(external.to_str().unwrap())); + + assert!(result.is_err(), "non-empty real REPOS must refuse"); + assert!( + !repos.symlink_metadata().unwrap().file_type().is_symlink(), + "refused REPOS must stay a real directory" + ); + assert!( + checkout.join("code.rs").exists(), + "refusal must never delete existing repositories" + ); +} + +// ensure_nest_at must NOT clobber an existing REPOS symlink on startup. +// Regression guard for Finding 1: the startup `ensure_repos_symlink(_, None)` +// call used to remove a configured symlink and mint an empty real REPOS, +// which async-restored agents could write into — the FE re-point then +// refused the now-non-empty dir, silently breaking a configured repos_dir. +#[cfg(unix)] +#[test] +fn ensure_nest_startup_preserves_existing_repos_symlink() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + + // First launch creates the real nest with a real REPOS dir. + crate::managed_agents::ensure_nest_at(&root).unwrap(); + + // Simulate a configured repos_dir: REPOS points at an external dir + // holding agent checkouts. + let external = tmp.path().join("Development"); + fs::create_dir(&external).unwrap(); + fs::write(external.join("KEEP.md"), "data").unwrap(); + fs::remove_dir(root.join("REPOS")).unwrap(); + std::os::unix::fs::symlink(&external, root.join("REPOS")).unwrap(); + + // Next launch must leave the configured symlink intact. + crate::managed_agents::ensure_nest_at(&root).unwrap(); + + let repos = root.join("REPOS"); + assert!( + repos.symlink_metadata().unwrap().file_type().is_symlink(), + "an existing REPOS symlink must survive startup" + ); + assert_eq!(repos.read_link().unwrap(), external); + assert!( + external.join("KEEP.md").exists(), + "the symlink's target contents must be untouched" + ); +} + +#[cfg(unix)] +#[test] +fn validate_repos_dir_rejects_tilde_relative_and_missing() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + assert!(validate_repos_dir(&root, "~/Development").is_err()); + assert!(validate_repos_dir(&root, "relative/path").is_err()); + assert!(validate_repos_dir(&root, "/no/such/dir/here").is_err()); + + let file = tmp.path().join("afile"); + fs::write(&file, "x").unwrap(); + assert!(validate_repos_dir(&root, file.to_str().unwrap()).is_err()); +} + +#[cfg(unix)] +#[test] +fn validate_repos_dir_rejects_nest_ancestor() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("home").join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let parent = root.parent().unwrap(); + + assert!( + validate_repos_dir(&root, parent.to_str().unwrap()).is_err(), + "a parent of the nest would nest REPOS inside its own target" + ); +} + +// ── persisted repos_dir dotfile ─────────────────────────────────────── + +#[test] +fn persisted_repos_dir_roundtrips_write_read_clear() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + assert_eq!(read_persisted_repos_dir(&root), None, "absent → None"); + + write_persisted_repos_dir(&root, Some(" /Users/me/Development ")).unwrap(); + assert_eq!( + read_persisted_repos_dir(&root).as_deref(), + Some("/Users/me/Development"), + "value is trimmed on write/read" + ); + + write_persisted_repos_dir(&root, None).unwrap(); + assert_eq!(read_persisted_repos_dir(&root), None, "cleared → None"); + assert!( + !root.join(".repos-dir").exists(), + "clearing removes the dotfile" + ); +} + +#[test] +fn persisted_repos_dir_empty_value_clears() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + write_persisted_repos_dir(&root, Some("/Users/me/Development")).unwrap(); + + write_persisted_repos_dir(&root, Some(" ")).unwrap(); + assert_eq!( + read_persisted_repos_dir(&root), + None, + "a whitespace-only value clears the override" + ); +} + +#[test] +fn persisted_repos_dir_clear_when_absent_is_ok() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + write_persisted_repos_dir(&root, None).expect("clearing an absent dotfile is not an error"); +} + +#[cfg(unix)] +#[test] +fn boot_resolves_symlink_from_persisted_value_into_empty_repos() { + // Mirrors the boot sequence: ensure_nest leaves REPOS an empty real + // dir, then the setup hook reads the persisted value and symlinks. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(root.join("REPOS")).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + + write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); + let persisted = read_persisted_repos_dir(&root); + ensure_repos_symlink(&root, persisted.as_deref()).unwrap(); + + let repos = root.join("REPOS"); + assert!( + repos.symlink_metadata().unwrap().file_type().is_symlink(), + "boot must convert the empty real REPOS into a symlink" + ); + assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); +} + +#[cfg(unix)] +#[test] +fn boot_leaves_already_correct_symlink_untouched() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + + write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); + // First boot converts; second boot must be a noop. + let persisted = read_persisted_repos_dir(&root); + ensure_repos_symlink(&root, persisted.as_deref()).unwrap(); + ensure_repos_symlink(&root, persisted.as_deref()).unwrap(); + + let repos = root.join("REPOS"); + assert!(repos.symlink_metadata().unwrap().file_type().is_symlink()); + assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); +} + +#[cfg(unix)] +#[test] +fn boot_with_cleared_value_reverts_symlink_to_real_dir() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + fs::write(external.join("KEEP.md"), "data").unwrap(); + + // Configure, then clear the field. + write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); + ensure_repos_symlink(&root, read_persisted_repos_dir(&root).as_deref()).unwrap(); + write_persisted_repos_dir(&root, None).unwrap(); + + // Next boot reads None and reverts REPOS to a real in-nest dir. + ensure_repos_symlink(&root, read_persisted_repos_dir(&root).as_deref()).unwrap(); + + let repos = root.join("REPOS"); + assert!( + !repos.symlink_metadata().unwrap().file_type().is_symlink(), + "clearing the persisted value reverts REPOS to a real dir" + ); + assert!( + external.join("KEEP.md").exists(), + "reverting must not touch the external target's contents" + ); +} + +#[cfg(unix)] +#[test] +fn effective_repos_dir_drives_persisted_dotfile_for_all_three_cases() { + // Pins the CRITICAL persist decision on `.repos-dir` CONTENTS, not just + // a return value: a bad path must clear the dotfile so the next boot's + // `should_restore_agents(false, _)` restores agents (the regression + // this hardening fixes). Drives each case through the same + // effective→persist path `apply_workspace` uses. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let good = tmp.path().join("Development"); + fs::create_dir_all(&good).unwrap(); + let good_str = good.to_str().unwrap(); + + // Pre-seed a value so each case proves it overwrites/clears, not just + // that an absent dotfile stays absent. + let seed = |root: &Path| write_persisted_repos_dir(root, Some(good_str)).unwrap(); + let persist = |root: &Path, candidate: Option<&str>| { + let effective = effective_repos_dir(root, candidate); + // Mirror apply_workspace: Err clears the override (persist None). + let to_persist = effective.unwrap_or(None); + write_persisted_repos_dir(root, to_persist.as_deref()).unwrap(); + }; + + // Bad path → Err → dotfile cleared (the CRITICAL). + seed(&root); + persist(&root, Some("/no/such/dir/here")); + assert_eq!( + read_persisted_repos_dir(&root), + None, + "a bad repos_dir must clear `.repos-dir` so the next boot restores agents" + ); + + // Good path → Ok(Some(raw)) → dotfile holds the raw trimmed value. + persist(&root, Some(&format!(" {good_str} "))); + assert_eq!( + read_persisted_repos_dir(&root).as_deref(), + Some(good_str), + "a valid repos_dir must persist the raw trimmed path (not the canonical path)" + ); + + // Empty/whitespace → Ok(None) → dotfile cleared. + seed(&root); + persist(&root, Some(" ")); + assert_eq!( + read_persisted_repos_dir(&root), + None, + "an empty repos_dir clears the override" + ); + + // None candidate → Ok(None) → dotfile cleared. + seed(&root); + persist(&root, None); + assert_eq!( + read_persisted_repos_dir(&root), + None, + "no repos_dir clears the override" + ); +} + +// ── per-relay repos_dir map ─────────────────────────────────────────── + +#[test] +fn repos_dir_map_roundtrips_upsert_and_clear() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example"), + None, + "absent map → None" + ); + + persist_workspace_repos_dir(&root, "wss://relay-a.example", Some(" /Users/me/DevA ")) + .unwrap(); + persist_workspace_repos_dir(&root, "wss://relay-b.example", Some("/Users/me/DevB")).unwrap(); + + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example").as_deref(), + Some("/Users/me/DevA"), + "value is trimmed on write" + ); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-b.example").as_deref(), + Some("/Users/me/DevB"), + "entries are independent per relay" + ); + + // Clearing one workspace's override must not touch the other's. + persist_workspace_repos_dir(&root, "wss://relay-a.example", None).unwrap(); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example"), + None + ); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-b.example").as_deref(), + Some("/Users/me/DevB") + ); + + // Clearing the last entry removes the file entirely. + persist_workspace_repos_dir(&root, "wss://relay-b.example", Some(" ")).unwrap(); + assert!( + !root.join(".repos-dirs.json").exists(), + "an empty map removes the dotfile" + ); +} + +#[test] +fn repos_dir_map_keys_are_normalized() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + // Cosmetic URL differences (case, trailing slash) are the same relay: + // a lookup must find the entry, and a re-persist must overwrite it + // rather than minting a second entry. + persist_workspace_repos_dir(&root, "WSS://Relay-A.Example/", Some("/Users/me/DevA")).unwrap(); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example").as_deref(), + Some("/Users/me/DevA") + ); + + persist_workspace_repos_dir(&root, "wss://relay-a.example", Some("/Users/me/Other")).unwrap(); + assert_eq!( + workspace_repos_dir_for_relay(&root, "WSS://Relay-A.Example/").as_deref(), + Some("/Users/me/Other"), + "cosmetic variants address the same entry" + ); + assert_eq!( + read_repos_dir_map(&root).len(), + 1, + "no duplicate entries for cosmetic variants" + ); +} + +#[test] +fn repos_dir_map_rejects_empty_relay_and_survives_malformed_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + assert!( + persist_workspace_repos_dir(&root, " ", Some("/Users/me/DevA")).is_err(), + "an empty relay URL has no identity to key on" + ); + + fs::write(root.join(".repos-dirs.json"), "not-json{{{").unwrap(); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example"), + None, + "a malformed map degrades to empty rather than failing" + ); + persist_workspace_repos_dir(&root, "wss://relay-a.example", Some("/Users/me/DevA")).unwrap(); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example").as_deref(), + Some("/Users/me/DevA"), + "persisting over a malformed file recovers it" + ); +} + +#[test] +fn rebind_workspace_repos_dir_moves_entry_to_new_relay() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + persist_workspace_repos_dir(&root, "wss://old.example", Some("/Users/me/DevA")).unwrap(); + + let moved = + rebind_workspace_repos_dir(&root, "wss://old.example/", "WSS://New.Example").unwrap(); + + assert!(moved); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://old.example"), + None, + "the old relay's entry is gone" + ); + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://new.example").as_deref(), + Some("/Users/me/DevA"), + "the entry follows the edited relay URL" + ); + + // Rebinding a relay with no entry is a no-op, not an error. + assert!(!rebind_workspace_repos_dir(&root, "wss://absent.example", "wss://x.example").unwrap()); + // A cosmetic-only edit (same normalized relay) moves nothing. + assert!(!rebind_workspace_repos_dir(&root, "wss://new.example", "WSS://New.Example/").unwrap()); +} + +// ── resolve_agent_repos_dir (spawn-time isolation) ──────────────────── + +// THE Phase-4 immunity property: each agent resolves its own workspace's +// repos dir as a real path, regardless of where the shared REPOS symlink +// currently points (it follows the ACTIVE workspace). +#[cfg(unix)] +#[test] +fn resolve_agent_repos_dir_is_immune_to_active_workspace_symlink() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let dir_a = tmp.path().join("DevA"); + let dir_b = tmp.path().join("DevB"); + fs::create_dir_all(&dir_a).unwrap(); + fs::create_dir_all(&dir_b).unwrap(); + + // Workspace A and B each applied with their own repos_dir; B is the + // active workspace, so REPOS symlinks to B's dir. + persist_workspace_repos_dir( + &root, + "wss://relay-a.example", + Some(dir_a.to_str().unwrap()), + ) + .unwrap(); + persist_workspace_repos_dir( + &root, + "wss://relay-b.example", + Some(dir_b.to_str().unwrap()), + ) + .unwrap(); + ensure_repos_symlink(&root, Some(dir_b.to_str().unwrap())).unwrap(); + + assert_eq!( + resolve_agent_repos_dir(&root, "wss://relay-a.example").unwrap(), + dir_a.canonicalize().unwrap(), + "an A-pinned agent resolves A's real dir even while REPOS points at B" + ); + assert_eq!( + resolve_agent_repos_dir(&root, "wss://relay-b.example").unwrap(), + dir_b.canonicalize().unwrap() + ); +} + +#[test] +fn resolve_agent_repos_dir_fails_closed_when_override_unresolvable() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let missing = tmp.path().join("not-mounted-yet"); + persist_workspace_repos_dir( + &root, + "wss://relay-a.example", + Some(missing.to_str().unwrap()), + ) + .unwrap(); + + let result = resolve_agent_repos_dir(&root, "wss://relay-a.example"); + + let error = result.expect_err("a configured-but-unresolvable repos dir must refuse spawn"); + assert!( + error.contains("refusing to start"), + "the error explains the fail-closed refusal: {error}" + ); +} + +#[test] +fn resolve_agent_repos_dir_defaults_to_nest_repos_without_entry() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + assert_eq!( + resolve_agent_repos_dir(&root, "wss://relay-a.example").unwrap(), + root.join("REPOS"), + "no override → the shared in-nest REPOS default (pre-map behavior)" + ); +} + +// ── resolve_repos_at_boot (boot sequence + fail-closed gate) ────────── + +#[cfg(unix)] +#[test] +fn resolve_repos_at_boot_converts_empty_real_repos_and_allows_restore() { + // Drives the real boot sequence: ensure_repos_setup_default (called by + // ensure_nest) provisions REPOS as an empty real dir, then the setup + // hook calls resolve_repos_at_boot. Asserts the convert happens at that + // position and restore is allowed. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let external = tmp.path().join("Development"); + fs::create_dir_all(&external).unwrap(); + write_persisted_repos_dir(&root, Some(external.to_str().unwrap())).unwrap(); + + ensure_repos_setup_default(&root).unwrap(); + let repos = root.join("REPOS"); + assert!( + repos.is_dir() && !repos.symlink_metadata().unwrap().file_type().is_symlink(), + "ensure_nest provisions REPOS as an empty real dir before the boot resolve" + ); + + let restore = resolve_repos_at_boot(&root); + + assert!(restore, "restore proceeds when the symlink resolves"); + assert!( + repos.symlink_metadata().unwrap().file_type().is_symlink(), + "boot resolve converts the empty real REPOS into the configured symlink" + ); + assert_eq!(repos.read_link().unwrap(), external.canonicalize().unwrap()); +} + +#[cfg(unix)] +#[test] +fn resolve_repos_at_boot_fails_closed_when_target_unresolvable() { + // Persisted repos_dir whose target does not exist at boot (transiently + // unavailable external volume) → ensure_repos_symlink Errs. Restore must + // be skipped and REPOS left as the empty real dir, never the symlink, so + // no agent clones into the wrong place. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + let missing = tmp.path().join("not-mounted-yet"); + write_persisted_repos_dir(&root, Some(missing.to_str().unwrap())).unwrap(); + ensure_repos_setup_default(&root).unwrap(); + + let restore = resolve_repos_at_boot(&root); + + assert!( + !restore, + "restore must be skipped when a configured repos_dir cannot resolve at boot" + ); + let repos = root.join("REPOS"); + assert!( + !repos.symlink_metadata().unwrap().file_type().is_symlink(), + "REPOS must not become a symlink to an unresolved target" + ); +} + +#[cfg(unix)] +#[test] +fn resolve_repos_at_boot_allows_restore_with_no_repos_dir() { + // No configured repos_dir → the real in-nest REPOS default is correct, + // restore proceeds normally (the fail-closed gate must not fire here). + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + ensure_repos_setup_default(&root).unwrap(); + + assert!( + resolve_repos_at_boot(&root), + "restore proceeds when no repos_dir is configured" + ); +} + +#[test] +fn should_restore_agents_only_blocks_configured_unresolved_boot() { + assert!( + should_restore_agents(false, &Ok(())), + "no repos_dir, symlink ok → restore" + ); + assert!( + should_restore_agents(false, &Err("boom".into())), + "no repos_dir → real-dir default is correct even on Err → restore" + ); + assert!( + should_restore_agents(true, &Ok(())), + "configured repos_dir resolved → restore" + ); + assert!( + !should_restore_agents(true, &Err("boom".into())), + "configured repos_dir unresolved → fail closed, skip restore" + ); +} + +/// Test helper: canonicalize a path, creating it as a directory first. +#[cfg(unix)] +trait CanonicalizeOrMake { + fn canonicalize_or_make(&self) -> PathBuf; +} +#[cfg(unix)] +impl CanonicalizeOrMake for PathBuf { + fn canonicalize_or_make(&self) -> PathBuf { + fs::create_dir_all(self).unwrap(); + self.canonicalize().unwrap() + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 31b31e00d0..a3951e11a4 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1330,16 +1330,28 @@ pub fn build_managed_agent_summary( let needs_restart = runtimes.get(&record.pubkey).is_some_and(|runtime| { use tauri::Manager; let state = app.state::(); + let workspace_relay = crate::relay::relay_ws_url_with_override(&state); let global_for_hash = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let teams_for_hash = crate::managed_agents::load_teams(app).unwrap_or_default(); + // Repos-dir hash input mirrors spawn: the raw per-relay map value for + // the agent's EFFECTIVE relay. Keyed by the agent's own pin, so a + // workspace switch cannot move it; an edit to the agent's own + // community repos dir can — and should badge. + let repos_dir_for_hash = crate::managed_agents::nest_dir().and_then(|nest| { + crate::managed_agents::workspace_repos_dir_for_relay( + &nest, + &crate::relay::effective_agent_relay_url(&record.relay_url, &workspace_relay), + ) + }); let hash_drift = runtime.spawn_config_hash != crate::managed_agents::spawn_hash::spawn_config_hash( record, personas, &teams_for_hash, - &crate::relay::relay_ws_url_with_override(&state), + &workspace_relay, &global_for_hash, + repos_dir_for_hash.as_deref(), ); let availability_drift = super::availability_drift( runtime.adapter_availability.as_ref(), @@ -1542,6 +1554,25 @@ pub fn spawn_agent_child( ) }; + // The agent's workspace repos directory, resolved to a REAL path from the + // per-relay map keyed by the agent's effective relay. The nest's REPOS + // symlink follows the ACTIVE workspace (a human/tooling convention) and + // can re-point mid-task on a workspace switch; handing the resolved path + // as BUZZ_REPOS_DIR makes this agent immune (the nest AGENTS.md instructs + // agents to prefer it over the relative REPOS/ path). Fails CLOSED when + // this workspace has a configured repos dir that cannot be resolved right + // now (e.g. unmounted volume) — same rationale as resolve_repos_at_boot: + // a refused start is recoverable, work landing in another workspace's + // checkouts is not. No map entry falls back to the nest REPOS path, + // preserving pre-map behavior. + let resolved_repos_dir = super::nest_dir() + .map(|nest| super::resolve_agent_repos_dir(&nest, &effective_relay_url)) + .transpose()?; + // Hash input: the RAW map value (stable against filesystem state), read + // once here so the stamp below and this spawn's env agree by construction. + let repos_dir_for_hash = super::nest_dir() + .and_then(|nest| super::workspace_repos_dir_for_relay(&nest, &effective_relay_url)); + // Augment PATH for DMG launches so child processes can find: // - bundled CLI via ~/.local/bin symlink // - nvm-managed node/npm (nvm initializes only in interactive shells) @@ -1572,6 +1603,9 @@ pub fn spawn_agent_child( command.env("RUST_LOG", child_rust_log_filter()); command.env("BUZZ_PRIVATE_KEY", &record.private_key_nsec); command.env("BUZZ_RELAY_URL", &effective_relay_url); + if let Some(repos_dir) = &resolved_repos_dir { + command.env("BUZZ_REPOS_DIR", repos_dir); + } command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(",")); match &resolved_mcp_command { @@ -1900,6 +1934,7 @@ pub fn spawn_agent_child( &teams, &effective_relay_url, &global, + repos_dir_for_hash.as_deref(), ); // Stamp the adapter availability for runtimes with a version gate (codex diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 4efe853cbc..0c9f2f900b 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -22,6 +22,11 @@ //! URL difference (trailing slash, scheme/host case) does not. //! - Channel membership is not an input: agents pick up channel changes live //! (#1468), never via restart. +//! - The workspace repos dir is hashed as the raw per-relay map value for the +//! agent's effective relay (the source of the spawned `BUZZ_REPOS_DIR`): +//! keyed by the agent's own pin, a workspace switch cannot move it, but an +//! edit to the agent's community repos dir means a restart would change +//! where the agent works. //! //! The hash never crosses a process or persistence boundary, so //! `DefaultHasher` (not stable across Rust releases) is sufficient. @@ -64,6 +69,9 @@ pub(crate) fn effective_team_instructions( /// Digest the effective spawn configuration of `record` under the current /// `personas`, resolving a blank record relay against `workspace_relay`. +/// `workspace_repos_dir` is the raw per-relay repos-dir map value for the +/// agent's effective relay (callers look it up via +/// `workspace_repos_dir_for_relay`); it feeds the spawned `BUZZ_REPOS_DIR`. /// Pure — no `AppHandle`, no disk, no keyring. pub(crate) fn spawn_config_hash( record: &ManagedAgentRecord, @@ -71,6 +79,7 @@ pub(crate) fn spawn_config_hash( teams: &[TeamRecord], workspace_relay: &str, global: &GlobalAgentConfig, + workspace_repos_dir: Option<&str>, ) -> u64 { // Prospective re-snapshot: apply the same `apply_persona_snapshot` the // start/restore paths run right before spawning, so the hash covers what a @@ -116,6 +125,10 @@ pub(crate) fn spawn_config_hash( workspace_relay, )) .hash(&mut hasher); + // The source value of the spawned BUZZ_REPOS_DIR. Hashed raw (not + // canonicalized), so filesystem state changes alone cannot flip the badge + // — only an actual config edit (or a relay rebind moving the entry) can. + workspace_repos_dir.hash(&mut hasher); // Prompt and runtime-layered team instructions use the same resolver as spawn. effective_spawn_prompt(record).hash(&mut hasher); effective_team_instructions(record, teams).hash(&mut hasher); diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index 4a81980075..57ec7b103e 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -85,8 +85,22 @@ fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> AgentDefinition { fn hash_is_deterministic() { let rec = record(); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -112,14 +126,16 @@ fn materializing_runtime_keeps_hash_stable() { &personas, &[], "wss://ws.example", - &Default::default() + &Default::default(), + None ), spawn_config_hash( &post, &personas, &[], "wss://ws.example", - &Default::default() + &Default::default(), + None ) ); } @@ -132,8 +148,22 @@ fn record_env_var_edit_changes_hash() { .env_vars .insert("SOME_KEY".into(), "some-value".into()); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -143,8 +173,22 @@ fn record_prompt_edit_changes_hash() { let mut edited = record(); edited.system_prompt = Some("Edited prompt.".into()); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -157,8 +201,22 @@ fn persona_runtime_edit_changes_hash() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &before, + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &after, + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -172,8 +230,22 @@ fn persona_prompt_edit_changes_hash() { let before = [persona("pers", Some("goose"), "old prompt")]; let after = [persona("pers", Some("goose"), "new prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &before, + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &after, + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -184,8 +256,22 @@ fn workspace_relay_change_trips_hash_for_blank_record_relay() { let mut rec = record(); rec.relay_url = String::new(); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), - spawn_config_hash(&rec, &[], &[], "wss://relay-b.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://relay-a.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &[], + &[], + "wss://relay-b.example", + &Default::default(), + None + ) ); } @@ -195,8 +281,22 @@ fn workspace_relay_change_ignored_for_pinned_record_relay() { // a workspace relay change must NOT badge a pinned agent. let rec = record(); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), - spawn_config_hash(&rec, &[], &[], "wss://relay-b.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://relay-a.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &[], + &[], + "wss://relay-b.example", + &Default::default(), + None + ) ); } @@ -208,8 +308,22 @@ fn cosmetic_pin_difference_does_not_change_hash() { let mut slashed = record(); slashed.relay_url = "WS://Localhost:3000/".into(); assert_eq!( - spawn_config_hash(&record(), &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&slashed, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &record(), + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &slashed, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -221,13 +335,21 @@ fn cosmetic_workspace_relay_difference_ignored_for_blank_record_relay() { let mut rec = record(); rec.relay_url = String::new(); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), + spawn_config_hash( + &rec, + &[], + &[], + "wss://relay-a.example", + &Default::default(), + None + ), spawn_config_hash( &rec, &[], &[], "WSS://Relay-A.Example/", - &Default::default() + &Default::default(), + None ) ); } @@ -239,8 +361,22 @@ fn respond_to_allowlist_edit_changes_hash() { edited.respond_to = RespondTo::Allowlist; edited.respond_to_allowlist = vec!["a".repeat(64)]; assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -252,8 +388,22 @@ fn allowlist_ignored_when_mode_is_not_allowlist() { let mut edited = record(); edited.respond_to_allowlist = vec!["a".repeat(64)]; assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -270,8 +420,22 @@ fn allowlist_normalization_equivalent_edits_do_not_change_hash() { "a".repeat(64), // duplicate ]; assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -283,8 +447,22 @@ fn allowlist_content_edit_still_changes_hash() { let mut edited = rec.clone(); edited.respond_to_allowlist = vec!["b".repeat(64)]; assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -297,8 +475,22 @@ fn explicit_default_max_turn_duration_does_not_change_hash() { edited.max_turn_duration_seconds = Some(crate::managed_agents::types::DEFAULT_AGENT_MAX_TURN_DURATION_SECONDS); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -308,8 +500,22 @@ fn non_default_max_turn_duration_changes_hash() { let mut edited = record(); edited.max_turn_duration_seconds = Some(42); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -324,8 +530,22 @@ fn non_spawn_bookkeeping_fields_do_not_change_hash() { edited.last_started_at = Some("later".into()); edited.last_exit_code = Some(0); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ) ); } @@ -355,14 +575,16 @@ fn resnapshot_does_not_clobber_record_quad_with_definition_absent_quad() { &quadless_definition, &[], "wss://ws.example", - &Default::default() + &Default::default(), + None ), spawn_config_hash( &rec, &definition_with_quad, &[], "wss://ws.example", - &Default::default() + &Default::default(), + None ), "definition quad must not leak into the spawn hash of an existing instance" ); @@ -378,8 +600,22 @@ fn empty_prompt_hashes_like_absent_prompt() { let mut empty = record(); empty.system_prompt = Some(String::new()); assert_eq!( - spawn_config_hash(&absent, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&empty, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash( + &absent, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &empty, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), ); } @@ -395,8 +631,22 @@ fn definition_runtime_edit_changes_hash_for_materialized_record() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), + spawn_config_hash( + &rec, + &before, + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &after, + &[], + "wss://ws.example", + &Default::default(), + None + ), "definition runtime edit must badge a materialized, override-free instance" ); } @@ -413,8 +663,22 @@ fn agent_command_override_beats_definition_runtime_change() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_eq!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), + spawn_config_hash( + &rec, + &before, + &[], + "wss://ws.example", + &Default::default(), + None + ), + spawn_config_hash( + &rec, + &after, + &[], + "wss://ws.example", + &Default::default(), + None + ), "explicit override must win regardless of definition runtime change" ); } @@ -438,19 +702,89 @@ fn missing_definition_leaves_materialized_runtime_in_hash() { no_personas, &[], "wss://ws.example", - &Default::default() + &Default::default(), + None ), spawn_config_hash( &no_runtime, no_personas, &[], "wss://ws.example", - &Default::default() + &Default::default(), + None ), "materialized runtime must still affect hash when definition is absent" ); } +#[test] +fn workspace_repos_dir_change_trips_hash() { + // Editing the agent's community repos dir changes where a restart would + // put BUZZ_REPOS_DIR → the badge must trip; likewise configuring or + // clearing an override. + let rec = record(); + let base = spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + Some("/Users/me/DevA"), + ); + assert_ne!( + base, + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + Some("/Users/me/DevB"), + ), + "a repos dir edit must badge" + ); + assert_ne!( + base, + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + None + ), + "clearing the override must badge" + ); +} + +#[test] +fn workspace_repos_dir_is_stable_across_recomputes() { + // The map value is keyed by the agent's pinned relay, so a pure workspace + // switch supplies the SAME value on recompute — no spurious badge. This + // pins that equal inputs hash equal (with and without an override). + let rec = record(); + for repos_dir in [None, Some("/Users/me/DevA")] { + assert_eq!( + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + repos_dir, + ), + spawn_config_hash( + &rec, + &[], + &[], + "wss://ws.example", + &Default::default(), + repos_dir, + ), + ); + } +} + #[test] fn effective_spawn_prompt_matches_hash_semantics() { // The env write and the hash share effective_spawn_prompt — this row diff --git a/desktop/src-tauri/src/migration/backfill_tests.rs b/desktop/src-tauri/src/migration/backfill_tests.rs index 9d2731d241..7966f45b75 100644 --- a/desktop/src-tauri/src/migration/backfill_tests.rs +++ b/desktop/src-tauri/src/migration/backfill_tests.rs @@ -137,6 +137,7 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { &[], "wss://ws.example", &Default::default(), + None, ); backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); @@ -153,6 +154,7 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { &[], "wss://ws.example", &Default::default(), + None, ); assert_eq!( @@ -186,6 +188,7 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { &[], "wss://ws.example", &Default::default(), + None, ); backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); @@ -202,6 +205,7 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { &[], "wss://ws.example", &Default::default(), + None, ); assert_eq!(hash_before, hash_after); From b94c6e9ed309a994aae7709456fe22ef4303d7a9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Jul 2026 23:01:52 +1000 Subject: [PATCH 05/12] fix(desktop): harden the relay-edit and repos-map concurrency edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the review findings on the lazy multi-workspace agent stack (review of f3baa96) — four hardening fixes, no behavior change on the happy path: - Serialize apply_workspace's .repos-dirs.json upsert on managed_agents_store_lock. A community relay edit fires rebind_agent_relay (which moves map entries under that lock) nearly simultaneously with the reinit apply, and both are read-modify-writes of the same file — unserialized, one side's update could be lost. - Write .repos-dirs.json via temp file + rename. A crash mid-write left truncated JSON, which read_repos_dir_map degrades to an empty map — silently sending spawns back to the shared REPOS fallback, exactly the cross-workspace hazard the map exists to prevent. - Pin the Rust/TS relay-URL normalizers together with a shared fixture (desktop/fixtures/relay-url-normalization.json) consumed by both relay tests and agentRelayScope.test.mjs, so an edit that lands on only one side fails the other side's tests instead of shipping a scoping skew. Both normalizers' doc comments now point at it. - Sequence updateCommunity's state commit (and the reinit it triggers) after the rebindAgentRelay IPC settles. Fire-and-forget let apply_workspace(newUrl) race ahead of the rebind: activation ran while start-on-launch agents were still pinned to the old URL and marked the relay activated for the session, silently skipping them until app relaunch. A rebind failure still commits — pins stay recoverable by re-editing the community. The fifth finding (stale "running in other communities" count) was already resolved by the slow cross-community poll tier added in 0f8fa7e. Split relay.rs's inline test module into relay/tests.rs (file-size guard, mirroring repos/tests.rs). Tested: cargo test --manifest-path desktop/src-tauri/Cargo.toml (1435 passed, incl. the new temp-file and fixture-agreement tests), clippy --all-targets -D warnings, rustfmt, desktop pnpm test (2955 passed, incl. the new fixture-agreement test), tsc --noEmit, biome check, pnpm check:file-sizes. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- desktop/fixtures/relay-url-normalization.json | 47 +++ desktop/src-tauri/src/commands/workspace.rs | 23 +- desktop/src-tauri/src/managed_agents/repos.rs | 11 +- .../src/managed_agents/repos/tests.rs | 24 ++ desktop/src-tauri/src/relay.rs | 348 +---------------- desktop/src-tauri/src/relay/tests.rs | 365 ++++++++++++++++++ .../features/agents/agentRelayScope.test.mjs | 28 +- .../src/features/agents/agentRelayScope.ts | 4 +- .../features/communities/useCommunities.tsx | 45 ++- 9 files changed, 527 insertions(+), 368 deletions(-) create mode 100644 desktop/fixtures/relay-url-normalization.json create mode 100644 desktop/src-tauri/src/relay/tests.rs diff --git a/desktop/fixtures/relay-url-normalization.json b/desktop/fixtures/relay-url-normalization.json new file mode 100644 index 0000000000..f8789dbe01 --- /dev/null +++ b/desktop/fixtures/relay-url-normalization.json @@ -0,0 +1,47 @@ +[ + { + "input": "wss://relay.example.com", + "canonical": "wss://relay.example.com", + "note": "already canonical" + }, + { + "input": " wss://relay.example.com// ", + "canonical": "wss://relay.example.com", + "note": "surrounding whitespace and trailing slashes are cosmetic" + }, + { + "input": "WSS://Relay.Example.COM:3000/Path", + "canonical": "wss://relay.example.com:3000/Path", + "note": "scheme and authority are case-insensitive (RFC 3986); the path is not" + }, + { + "input": "WS://RELAY-A.example.com:3000/", + "canonical": "ws://relay-a.example.com:3000", + "note": "ws scheme and port survive; case and trailing slash fold away" + }, + { + "input": "wss://Relay.Example.com/Path?Query=Value", + "canonical": "wss://relay.example.com/Path?Query=Value", + "note": "path and query are preserved case-sensitively" + }, + { + "input": "wss://relay.example.com/path//", + "canonical": "wss://relay.example.com/path", + "note": "trailing slashes after a path are cosmetic too" + }, + { + "input": "not-a-url/", + "canonical": "not-a-url", + "note": "schemeless values pass through trim/slash cleanup only" + }, + { + "input": "", + "canonical": "", + "note": "empty stays empty" + }, + { + "input": " ", + "canonical": "", + "note": "whitespace-only trims to empty" + } +] diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 79a344f1c6..9f05036ed0 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -187,11 +187,24 @@ pub async fn apply_workspace( // symlink re-point below when a later switch moves it. Must land // before the activation task spawned after this closure starts // this workspace's agents. A bad candidate clears the entry, same - // as the dotfile above. - if let Err(error) = - persist_workspace_repos_dir(nest, &relay_url, effective_repos_dir.as_deref()) - { - eprintln!("buzz-desktop: persist per-relay repos dir failed: {error}"); + // as the dotfile above. Serialized on the store lock: a community + // relay edit fires rebind_agent_relay (which moves this map's + // entries under that lock) nearly simultaneously with this apply, + // and both are read-modify-writes of the same file — unserialized, + // one side's update would be lost. + match state.managed_agents_store_lock.lock() { + Ok(_store_guard) => { + if let Err(error) = persist_workspace_repos_dir( + nest, + &relay_url, + effective_repos_dir.as_deref(), + ) { + eprintln!("buzz-desktop: persist per-relay repos dir failed: {error}"); + } + } + Err(error) => { + eprintln!("buzz-desktop: persist per-relay repos dir skipped: {error}"); + } } if let Err(error) = ensure_repos_symlink(nest, effective_repos_dir.as_deref()) { eprintln!("buzz-desktop: repos dir setup failed: {error}"); diff --git a/desktop/src-tauri/src/managed_agents/repos.rs b/desktop/src-tauri/src/managed_agents/repos.rs index b7c6b8d536..3ad7d2c1c6 100644 --- a/desktop/src-tauri/src/managed_agents/repos.rs +++ b/desktop/src-tauri/src/managed_agents/repos.rs @@ -273,6 +273,12 @@ fn read_repos_dir_map(nest_root: &Path) -> BTreeMap { /// Persist the per-relay `repos_dir` map, removing the file when empty /// (mirrors [`write_persisted_repos_dir`]'s clear-by-removal). +/// +/// The write goes through a temp file + rename so a crash mid-write cannot +/// leave truncated JSON behind: [`read_repos_dir_map`] degrades a malformed +/// file to an *empty* map, which would silently send every workspace's spawn +/// back to the shared `REPOS` fallback — exactly the cross-workspace hazard +/// the map exists to prevent. fn write_repos_dir_map(nest_root: &Path, map: &BTreeMap) -> Result<(), String> { let path = nest_root.join(REPOS_DIR_MAP_FILE); if map.is_empty() { @@ -284,7 +290,10 @@ fn write_repos_dir_map(nest_root: &Path, map: &BTreeMap) -> Resu } let json = serde_json::to_string_pretty(map) .map_err(|e| format!("serialize {REPOS_DIR_MAP_FILE}: {e}"))?; - fs::write(&path, format!("{json}\n")).map_err(|e| format!("write {}: {e}", path.display())) + let tmp = nest_root.join(format!("{REPOS_DIR_MAP_FILE}.tmp")); + fs::write(&tmp, format!("{json}\n")).map_err(|e| format!("write {}: {e}", tmp.display()))?; + fs::rename(&tmp, &path) + .map_err(|e| format!("rename {} -> {}: {e}", tmp.display(), path.display())) } /// Upsert (or clear, on `None`/empty) the applied workspace's `repos_dir` in diff --git a/desktop/src-tauri/src/managed_agents/repos/tests.rs b/desktop/src-tauri/src/managed_agents/repos/tests.rs index 67a93df548..ff8e2cfc9b 100644 --- a/desktop/src-tauri/src/managed_agents/repos/tests.rs +++ b/desktop/src-tauri/src/managed_agents/repos/tests.rs @@ -498,6 +498,30 @@ fn repos_dir_map_rejects_empty_relay_and_survives_malformed_file() { ); } +#[test] +fn repos_dir_map_write_leaves_no_temp_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + fs::create_dir_all(&root).unwrap(); + + // The map is written via temp file + rename so a crash mid-write can + // never leave truncated JSON (read degrades malformed to an empty map, + // silently dropping isolation). A stale temp file — e.g. from a crash + // between write and rename — must be overwritten, and no temp file may + // outlive a successful write. + fs::write(root.join(".repos-dirs.json.tmp"), "half-written{").unwrap(); + persist_workspace_repos_dir(&root, "wss://relay-a.example", Some("/Users/me/DevA")).unwrap(); + + assert_eq!( + workspace_repos_dir_for_relay(&root, "wss://relay-a.example").as_deref(), + Some("/Users/me/DevA") + ); + assert!( + !root.join(".repos-dirs.json.tmp").exists(), + "the temp file is renamed into place, not left behind" + ); +} + #[test] fn rebind_workspace_repos_dir_moves_entry_to_new_relay() { let tmp = tempfile::tempdir().unwrap(); diff --git a/desktop/src-tauri/src/relay.rs b/desktop/src-tauri/src/relay.rs index f5652f1243..16b30e10cb 100644 --- a/desktop/src-tauri/src/relay.rs +++ b/desktop/src-tauri/src/relay.rs @@ -80,6 +80,12 @@ pub fn effective_agent_relay_url(record_relay: &str, workspace_relay: &str) -> S /// surrounding whitespace, trailing slashes, or scheme/host case (both are /// case-insensitive per RFC 3986). Any path or query is preserved /// case-sensitively. +/// +/// Must stay in lockstep with the frontend mirror +/// (`normalizeRelayUrlForCompare` in `desktop/src/features/agents/ +/// agentRelayScope.ts`) — the agreement is pinned by the shared fixture +/// `desktop/fixtures/relay-url-normalization.json`, consumed by both sides' +/// unit tests. Extend the fixture with any behavior change. pub fn normalize_relay_url(url: &str) -> String { let trimmed = url.trim().trim_end_matches('/'); match trimmed.split_once("://") { @@ -641,344 +647,4 @@ pub async fn submit_event_with_keys( // ── Tests ─────────────────────────────────────────────────────────────────── #[cfg(test)] -mod tests { - use super::{ - build_profile_event, classify_intercepted_response, effective_agent_relay_url, - normalize_relay_url, parse_command_response, relay_http_base_url, relay_urls_equivalent, - MALFORMED_RESPONSE_MESSAGE, - }; - use serde::Deserialize; - - // ── effective_agent_relay_url: per-agent override precedence ───────────── - - #[test] - fn explicit_relay_wins_over_workspace() { - // An explicit per-agent relay pins the agent there regardless of the - // active workspace — this is the override taking highest precedence. - assert_eq!( - effective_agent_relay_url("wss://relay.other.com", "wss://staging.example.com"), - "wss://relay.other.com" - ); - } - - #[test] - fn explicit_relay_wins_even_when_equal_to_workspace() { - // No special-casing when the pin happens to match the active workspace. - assert_eq!( - effective_agent_relay_url("wss://staging.example.com", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - #[test] - fn empty_relay_falls_back_to_workspace() { - // A never-set record resolves to the active workspace relay at read-time, - // so a stale stored default can never make it load-bearing. - assert_eq!( - effective_agent_relay_url("", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - #[test] - fn whitespace_only_relay_falls_back_to_workspace() { - // Whitespace-only is treated as unset, same as empty. - assert_eq!( - effective_agent_relay_url(" ", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - // ── normalize_relay_url / relay_urls_equivalent ────────────────────────── - - #[test] - fn normalize_strips_whitespace_and_trailing_slashes() { - assert_eq!( - normalize_relay_url(" wss://relay.example.com// "), - "wss://relay.example.com" - ); - } - - #[test] - fn normalize_lowercases_scheme_and_host_only() { - // Scheme and authority are case-insensitive (RFC 3986); a path is not. - assert_eq!( - normalize_relay_url("WSS://Relay.Example.COM:3000/Path"), - "wss://relay.example.com:3000/Path" - ); - } - - #[test] - fn normalize_passes_through_schemeless_values() { - // Not a URL — nothing to case-fold beyond trim/slash cleanup. - assert_eq!(normalize_relay_url("not-a-url/"), "not-a-url"); - } - - #[test] - fn equivalence_ignores_cosmetic_differences() { - assert!(relay_urls_equivalent( - "wss://Relay.Example/", - " wss://relay.example" - )); - } - - #[test] - fn equivalence_distinguishes_real_differences() { - // Different host, port, or scheme = a different relay. - assert!(!relay_urls_equivalent( - "wss://relay-a.example", - "wss://relay-b.example" - )); - assert!(!relay_urls_equivalent( - "ws://relay.example:3000", - "ws://relay.example:3001" - )); - assert!(!relay_urls_equivalent( - "ws://relay.example", - "wss://relay.example" - )); - } - - // ── relay_http_base_url scheme conversion ──────────────────────────────── - - #[test] - fn loopback_ws_localhost_preserves_authority() { - // Tenant host-binding keys off the HTTP Host/authority. The desktop must - // not rewrite localhost to 127.0.0.1, or local dev HTTP calls target a - // different unmapped community than the WebSocket URL. - assert_eq!( - relay_http_base_url("ws://localhost:3000"), - "http://localhost:3000" - ); - } - - #[test] - fn loopback_trailing_slash_removed_authority_preserved() { - assert_eq!( - relay_http_base_url("ws://localhost:3000/"), - "http://localhost:3000" - ); - } - - #[test] - fn remote_wss_host_unchanged() { - assert_eq!( - relay_http_base_url("wss://relay.example.com"), - "https://relay.example.com" - ); - } - - #[test] - fn loopback_ipv4_literal_unchanged() { - assert_eq!( - relay_http_base_url("ws://127.0.0.1:3000"), - "http://127.0.0.1:3000" - ); - } - - #[test] - fn localhost_substring_host_unchanged() { - assert_eq!( - relay_http_base_url("ws://localhost.evil.com:3000"), - "http://localhost.evil.com:3000" - ); - } - - #[test] - fn loopback_wss_localhost_preserves_authority() { - assert_eq!( - relay_http_base_url("wss://localhost:3000"), - "https://localhost:3000" - ); - } - - // ── classify_intercepted_response ──────────────────────────────────────── - - #[test] - fn intercepted_cloudflare_host_returns_some() { - let result = classify_intercepted_response("sqprod.cloudflareaccess.com", "text/html"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!( - msg.starts_with("relay unreachable:"), - "should have unreachable prefix" - ); - assert!(msg.contains("Cloudflare"), "should mention Cloudflare"); - } - - #[test] - fn intercepted_cloudflare_apex_host_returns_some() { - // The apex domain itself should also match. - let result = classify_intercepted_response("cloudflareaccess.com", "application/json"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!(msg.starts_with("relay unreachable:")); - assert!(msg.contains("Cloudflare")); - } - - #[test] - fn intercepted_non_cloudflare_html_returns_some() { - let result = - classify_intercepted_response("proxy.corporate.example", "text/html; charset=utf-8"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!(msg.starts_with("relay unreachable:")); - } - - #[test] - fn normal_relay_json_returns_none() { - let result = classify_intercepted_response("relay.myapp.example.com", "application/json"); - assert!(result.is_none()); - } - - #[test] - fn content_type_case_insensitive() { - // Uppercase content-type must still be detected. - let result = classify_intercepted_response("proxy.example.com", "TEXT/HTML"); - assert!(result.is_some()); - assert!(result.unwrap().starts_with("relay unreachable:")); - } - - #[test] - fn evil_suffix_does_not_match_cloudflare() { - // A host whose suffix happens to contain the Cloudflare string but is - // not actually a subdomain must NOT match. - let result = classify_intercepted_response( - "notcloudflareaccess.com.evil.example", - "application/json", - ); - assert!( - result.is_none(), - "false suffix match should not trigger Cloudflare branch" - ); - } - - // classify_request_error requires a real reqwest::Error (not publicly - // constructable) — tested indirectly through integration; skipped here. - - // ── parse_json_response malformed-body contract ────────────────────────── - - #[test] - fn malformed_response_message_stays_off_unreachable_bucket() { - // A reached-but-malformed 2xx body is not a connectivity failure. If this - // message ever regains the "relay unreachable:" prefix, the frontend - // classifier would misroute it as unreachable — pin that it never does. - assert!( - !MALFORMED_RESPONSE_MESSAGE.starts_with("relay unreachable:"), - "malformed-response message must not match the unreachable prefix" - ); - } - - // ── parse_command_response ─────────────────────────────────────────────── - - #[derive(Debug, Deserialize, PartialEq)] - struct ChannelCreated { - channel_id: String, - } - - #[test] - fn parse_command_response_decodes_typed_payload() { - let msg = r#"response:{"channel_id":"abc123"}"#; - let parsed: ChannelCreated = parse_command_response(msg).expect("should parse"); - assert_eq!( - parsed, - ChannelCreated { - channel_id: "abc123".to_string() - } - ); - } - - #[test] - fn parse_command_response_accepts_raw_json_fallback() { - // Backward-compat: relays that emit raw JSON (no prefix) still work. - let msg = r#"{"channel_id":"abc"}"#; - let parsed: ChannelCreated = parse_command_response(msg).expect("fallback parse"); - assert_eq!( - parsed, - ChannelCreated { - channel_id: "abc".to_string() - } - ); - } - - #[test] - fn parse_command_response_rejects_invalid_prefixed_json() { - let msg = "response:not-json"; - let result: Result = parse_command_response(msg); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("response parse failed")); - } - - #[test] - fn parse_command_response_rejects_garbage() { - let msg = "totally not json or response"; - let result: Result = parse_command_response(msg); - assert!(result.is_err()); - } - - // ── build_profile_event ────────────────────────────────────────────────── - - /// Generate a valid NIP-OA auth tag JSON string signed by a fresh owner key - /// and addressed to `agent_keys`. - /// - /// Uses `nostr_compat` (nostr 0.36) for the owner keys because - /// `buzz_sdk_pkg::nip_oa::compute_auth_tag` expects nostr 0.36 types. - /// The agent pubkey is bridged via hex encoding. - fn make_valid_auth_tag(agent_keys: &nostr::Keys) -> String { - let owner_keys = nostr::Keys::generate(); - let agent_pubkey_hex = agent_keys.public_key().to_hex(); - let agent_compat_pubkey = - nostr::PublicKey::from_hex(&agent_pubkey_hex).expect("valid hex pubkey should parse"); - buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_compat_pubkey, "") - .expect("compute_auth_tag should not fail with distinct keys") - } - - #[test] - fn profile_event_with_valid_auth_tag() { - let agent_keys = nostr::Keys::generate(); - let tag_json = make_valid_auth_tag(&agent_keys); - let event = build_profile_event(&agent_keys, "TestBot", None, Some(&tag_json)) - .expect("should succeed with a valid auth tag"); - - // Exactly one "auth" tag must be present. - let auth_tags: Vec<_> = event - .tags - .iter() - .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) - .collect(); - assert_eq!(auth_tags.len(), 1, "expected exactly 1 auth tag"); - - // Must be a kind:0 (Metadata) event. - assert_eq!(event.kind, nostr::Kind::Metadata); - } - - #[test] - fn profile_event_without_auth_tag() { - let agent_keys = nostr::Keys::generate(); - let event = build_profile_event(&agent_keys, "TestBot", None, None) - .expect("should succeed without an auth tag"); - - // No "auth" tags should be present. - let auth_tags: Vec<_> = event - .tags - .iter() - .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) - .collect(); - assert_eq!(auth_tags.len(), 0, "expected no auth tags"); - - assert_eq!(event.kind, nostr::Kind::Metadata); - } - - #[test] - fn profile_event_rejects_invalid_auth_tag() { - let agent_keys = nostr::Keys::generate(); - // Structurally valid JSON array but with a bogus signature — verification must fail. - let bad_json = format!(r#"["auth","{}","","{}"]"#, "a".repeat(64), "b".repeat(128)); - let result = build_profile_event(&agent_keys, "TestBot", None, Some(&bad_json)); - assert!(result.is_err(), "should reject an invalid auth tag"); - assert!( - result.unwrap_err().contains("verification failed"), - "error message should mention verification failure" - ); - } -} +mod tests; diff --git a/desktop/src-tauri/src/relay/tests.rs b/desktop/src-tauri/src/relay/tests.rs new file mode 100644 index 0000000000..7690ecc0fa --- /dev/null +++ b/desktop/src-tauri/src/relay/tests.rs @@ -0,0 +1,365 @@ +use super::{ + build_profile_event, classify_intercepted_response, effective_agent_relay_url, + normalize_relay_url, parse_command_response, relay_http_base_url, relay_urls_equivalent, + MALFORMED_RESPONSE_MESSAGE, +}; +use serde::Deserialize; + +// ── effective_agent_relay_url: per-agent override precedence ───────────── + +#[test] +fn explicit_relay_wins_over_workspace() { + // An explicit per-agent relay pins the agent there regardless of the + // active workspace — this is the override taking highest precedence. + assert_eq!( + effective_agent_relay_url("wss://relay.other.com", "wss://staging.example.com"), + "wss://relay.other.com" + ); +} + +#[test] +fn explicit_relay_wins_even_when_equal_to_workspace() { + // No special-casing when the pin happens to match the active workspace. + assert_eq!( + effective_agent_relay_url("wss://staging.example.com", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +#[test] +fn empty_relay_falls_back_to_workspace() { + // A never-set record resolves to the active workspace relay at read-time, + // so a stale stored default can never make it load-bearing. + assert_eq!( + effective_agent_relay_url("", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +#[test] +fn whitespace_only_relay_falls_back_to_workspace() { + // Whitespace-only is treated as unset, same as empty. + assert_eq!( + effective_agent_relay_url(" ", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +// ── normalize_relay_url / relay_urls_equivalent ────────────────────────── + +/// One vector of the shared fixture consumed by BOTH this test and the +/// frontend mirror's (`agentRelayScope.test.mjs`): record pins are +/// stamped by `normalize_relay_url` and compared by the frontend's +/// `normalizeRelayUrlForCompare`, so an edit that lands on only one side +/// must fail the other side's tests instead of shipping a scoping skew. +#[derive(Deserialize)] +struct NormalizationVector { + input: String, + canonical: String, +} + +#[test] +fn normalize_agrees_with_shared_frontend_fixture() { + let vectors: Vec = serde_json::from_str(include_str!( + "../../../fixtures/relay-url-normalization.json" + )) + .unwrap(); + assert!(!vectors.is_empty(), "fixture must not be empty"); + for vector in &vectors { + assert_eq!( + normalize_relay_url(&vector.input), + vector.canonical, + "input: {:?}", + vector.input + ); + } +} + +#[test] +fn normalize_strips_whitespace_and_trailing_slashes() { + assert_eq!( + normalize_relay_url(" wss://relay.example.com// "), + "wss://relay.example.com" + ); +} + +#[test] +fn normalize_lowercases_scheme_and_host_only() { + // Scheme and authority are case-insensitive (RFC 3986); a path is not. + assert_eq!( + normalize_relay_url("WSS://Relay.Example.COM:3000/Path"), + "wss://relay.example.com:3000/Path" + ); +} + +#[test] +fn normalize_passes_through_schemeless_values() { + // Not a URL — nothing to case-fold beyond trim/slash cleanup. + assert_eq!(normalize_relay_url("not-a-url/"), "not-a-url"); +} + +#[test] +fn equivalence_ignores_cosmetic_differences() { + assert!(relay_urls_equivalent( + "wss://Relay.Example/", + " wss://relay.example" + )); +} + +#[test] +fn equivalence_distinguishes_real_differences() { + // Different host, port, or scheme = a different relay. + assert!(!relay_urls_equivalent( + "wss://relay-a.example", + "wss://relay-b.example" + )); + assert!(!relay_urls_equivalent( + "ws://relay.example:3000", + "ws://relay.example:3001" + )); + assert!(!relay_urls_equivalent( + "ws://relay.example", + "wss://relay.example" + )); +} + +// ── relay_http_base_url scheme conversion ──────────────────────────────── + +#[test] +fn loopback_ws_localhost_preserves_authority() { + // Tenant host-binding keys off the HTTP Host/authority. The desktop must + // not rewrite localhost to 127.0.0.1, or local dev HTTP calls target a + // different unmapped community than the WebSocket URL. + assert_eq!( + relay_http_base_url("ws://localhost:3000"), + "http://localhost:3000" + ); +} + +#[test] +fn loopback_trailing_slash_removed_authority_preserved() { + assert_eq!( + relay_http_base_url("ws://localhost:3000/"), + "http://localhost:3000" + ); +} + +#[test] +fn remote_wss_host_unchanged() { + assert_eq!( + relay_http_base_url("wss://relay.example.com"), + "https://relay.example.com" + ); +} + +#[test] +fn loopback_ipv4_literal_unchanged() { + assert_eq!( + relay_http_base_url("ws://127.0.0.1:3000"), + "http://127.0.0.1:3000" + ); +} + +#[test] +fn localhost_substring_host_unchanged() { + assert_eq!( + relay_http_base_url("ws://localhost.evil.com:3000"), + "http://localhost.evil.com:3000" + ); +} + +#[test] +fn loopback_wss_localhost_preserves_authority() { + assert_eq!( + relay_http_base_url("wss://localhost:3000"), + "https://localhost:3000" + ); +} + +// ── classify_intercepted_response ──────────────────────────────────────── + +#[test] +fn intercepted_cloudflare_host_returns_some() { + let result = classify_intercepted_response("sqprod.cloudflareaccess.com", "text/html"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!( + msg.starts_with("relay unreachable:"), + "should have unreachable prefix" + ); + assert!(msg.contains("Cloudflare"), "should mention Cloudflare"); +} + +#[test] +fn intercepted_cloudflare_apex_host_returns_some() { + // The apex domain itself should also match. + let result = classify_intercepted_response("cloudflareaccess.com", "application/json"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!(msg.starts_with("relay unreachable:")); + assert!(msg.contains("Cloudflare")); +} + +#[test] +fn intercepted_non_cloudflare_html_returns_some() { + let result = + classify_intercepted_response("proxy.corporate.example", "text/html; charset=utf-8"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!(msg.starts_with("relay unreachable:")); +} + +#[test] +fn normal_relay_json_returns_none() { + let result = classify_intercepted_response("relay.myapp.example.com", "application/json"); + assert!(result.is_none()); +} + +#[test] +fn content_type_case_insensitive() { + // Uppercase content-type must still be detected. + let result = classify_intercepted_response("proxy.example.com", "TEXT/HTML"); + assert!(result.is_some()); + assert!(result.unwrap().starts_with("relay unreachable:")); +} + +#[test] +fn evil_suffix_does_not_match_cloudflare() { + // A host whose suffix happens to contain the Cloudflare string but is + // not actually a subdomain must NOT match. + let result = + classify_intercepted_response("notcloudflareaccess.com.evil.example", "application/json"); + assert!( + result.is_none(), + "false suffix match should not trigger Cloudflare branch" + ); +} + +// classify_request_error requires a real reqwest::Error (not publicly +// constructable) — tested indirectly through integration; skipped here. + +// ── parse_json_response malformed-body contract ────────────────────────── + +#[test] +fn malformed_response_message_stays_off_unreachable_bucket() { + // A reached-but-malformed 2xx body is not a connectivity failure. If this + // message ever regains the "relay unreachable:" prefix, the frontend + // classifier would misroute it as unreachable — pin that it never does. + assert!( + !MALFORMED_RESPONSE_MESSAGE.starts_with("relay unreachable:"), + "malformed-response message must not match the unreachable prefix" + ); +} + +// ── parse_command_response ─────────────────────────────────────────────── + +#[derive(Debug, Deserialize, PartialEq)] +struct ChannelCreated { + channel_id: String, +} + +#[test] +fn parse_command_response_decodes_typed_payload() { + let msg = r#"response:{"channel_id":"abc123"}"#; + let parsed: ChannelCreated = parse_command_response(msg).expect("should parse"); + assert_eq!( + parsed, + ChannelCreated { + channel_id: "abc123".to_string() + } + ); +} + +#[test] +fn parse_command_response_accepts_raw_json_fallback() { + // Backward-compat: relays that emit raw JSON (no prefix) still work. + let msg = r#"{"channel_id":"abc"}"#; + let parsed: ChannelCreated = parse_command_response(msg).expect("fallback parse"); + assert_eq!( + parsed, + ChannelCreated { + channel_id: "abc".to_string() + } + ); +} + +#[test] +fn parse_command_response_rejects_invalid_prefixed_json() { + let msg = "response:not-json"; + let result: Result = parse_command_response(msg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("response parse failed")); +} + +#[test] +fn parse_command_response_rejects_garbage() { + let msg = "totally not json or response"; + let result: Result = parse_command_response(msg); + assert!(result.is_err()); +} + +// ── build_profile_event ────────────────────────────────────────────────── + +/// Generate a valid NIP-OA auth tag JSON string signed by a fresh owner key +/// and addressed to `agent_keys`. +/// +/// Uses `nostr_compat` (nostr 0.36) for the owner keys because +/// `buzz_sdk_pkg::nip_oa::compute_auth_tag` expects nostr 0.36 types. +/// The agent pubkey is bridged via hex encoding. +fn make_valid_auth_tag(agent_keys: &nostr::Keys) -> String { + let owner_keys = nostr::Keys::generate(); + let agent_pubkey_hex = agent_keys.public_key().to_hex(); + let agent_compat_pubkey = + nostr::PublicKey::from_hex(&agent_pubkey_hex).expect("valid hex pubkey should parse"); + buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_compat_pubkey, "") + .expect("compute_auth_tag should not fail with distinct keys") +} + +#[test] +fn profile_event_with_valid_auth_tag() { + let agent_keys = nostr::Keys::generate(); + let tag_json = make_valid_auth_tag(&agent_keys); + let event = build_profile_event(&agent_keys, "TestBot", None, Some(&tag_json)) + .expect("should succeed with a valid auth tag"); + + // Exactly one "auth" tag must be present. + let auth_tags: Vec<_> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) + .collect(); + assert_eq!(auth_tags.len(), 1, "expected exactly 1 auth tag"); + + // Must be a kind:0 (Metadata) event. + assert_eq!(event.kind, nostr::Kind::Metadata); +} + +#[test] +fn profile_event_without_auth_tag() { + let agent_keys = nostr::Keys::generate(); + let event = build_profile_event(&agent_keys, "TestBot", None, None) + .expect("should succeed without an auth tag"); + + // No "auth" tags should be present. + let auth_tags: Vec<_> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) + .collect(); + assert_eq!(auth_tags.len(), 0, "expected no auth tags"); + + assert_eq!(event.kind, nostr::Kind::Metadata); +} + +#[test] +fn profile_event_rejects_invalid_auth_tag() { + let agent_keys = nostr::Keys::generate(); + // Structurally valid JSON array but with a bogus signature — verification must fail. + let bad_json = format!(r#"["auth","{}","","{}"]"#, "a".repeat(64), "b".repeat(128)); + let result = build_profile_event(&agent_keys, "TestBot", None, Some(&bad_json)); + assert!(result.is_err(), "should reject an invalid auth tag"); + assert!( + result.unwrap_err().contains("verification failed"), + "error message should mention verification failure" + ); +} diff --git a/desktop/src/features/agents/agentRelayScope.test.mjs b/desktop/src/features/agents/agentRelayScope.test.mjs index ecb5d69f7a..b625c60ca7 100644 --- a/desktop/src/features/agents/agentRelayScope.test.mjs +++ b/desktop/src/features/agents/agentRelayScope.test.mjs @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import test from "node:test"; import { @@ -13,8 +14,31 @@ const RELAY_B = "wss://relay-b.example.com"; // ── normalizeRelayUrlForCompare ────────────────────────────────────────────── // Must agree with the Rust `normalize_relay_url` (desktop/src-tauri/src/relay.rs) -// because record pins are stamped by the backend and compared here. These -// vectors mirror the Rust unit tests. +// because record pins are stamped by the backend and compared here. The +// agreement contract is the shared fixture below, consumed by both this file +// and the Rust unit tests (`normalize_agrees_with_shared_frontend_fixture`), +// so an edit to either normalizer fails the other side's tests instead of +// shipping a scoping skew. + +test("normalize_agreesWithSharedBackendFixture", () => { + const vectors = JSON.parse( + readFileSync( + new URL( + "../../../fixtures/relay-url-normalization.json", + import.meta.url, + ), + "utf8", + ), + ); + assert.ok(vectors.length > 0, "fixture must not be empty"); + for (const { input, canonical } of vectors) { + assert.equal( + normalizeRelayUrlForCompare(input), + canonical, + `input: ${JSON.stringify(input)}`, + ); + } +}); test("normalize_stripsWhitespaceAndTrailingSlashes", () => { assert.equal( diff --git a/desktop/src/features/agents/agentRelayScope.ts b/desktop/src/features/agents/agentRelayScope.ts index 5decfa823f..8092b188ef 100644 --- a/desktop/src/features/agents/agentRelayScope.ts +++ b/desktop/src/features/agents/agentRelayScope.ts @@ -13,7 +13,9 @@ * `desktop/src-tauri/src/relay.rs`; the two must agree because record pins * are stamped by the backend and compared here: trim, strip trailing * slashes, lowercase scheme + authority (case-insensitive per RFC 3986), - * preserve any path or query case-sensitively. + * preserve any path or query case-sensitively. The agreement is pinned by + * the shared fixture `desktop/fixtures/relay-url-normalization.json`, + * consumed by both sides' unit tests — extend it with any behavior change. * * Distinct from `normalizeRelayUrl` in `communityStorage.ts` (input * canonicalisation: prepends `wss://`) and in `selfProfileStorage.ts` diff --git a/desktop/src/features/communities/useCommunities.tsx b/desktop/src/features/communities/useCommunities.tsx index 9a7759393a..f6a72e49e1 100644 --- a/desktop/src/features/communities/useCommunities.tsx +++ b/desktop/src/features/communities/useCommunities.tsx @@ -238,33 +238,42 @@ function useCommunitiesInternal(): UseCommunitiesReturn { ); if (result.kind === "updated") { + const commit = () => { + setCommunitiesState((prev) => { + const next = prev.map((w) => + w.id === id ? { ...w, ...updates } : w, + ); + saveCommunities(next); + return next; + }); + + if (result.requiresReinit) { + setReinitKey((k) => k + 1); + } + }; + // Agent records are pinned to their home relay, so a relay-URL edit // must re-pin them onto the new URL or they orphan on the old one. - // Fire-and-forget: a failure leaves the pins on the old URL, which - // stays recoverable by re-editing the community. + // The commit — and the reinit-triggered applyCommunity it fires — + // waits for the rebind: apply_workspace on the new URL activates + // start-on-launch agents and marks that relay activated for the + // session, so if it raced ahead of the rebind, agents still pinned + // to the old URL would be silently skipped until app relaunch. A + // rebind failure still commits — the pins stay on the old URL, + // which is recoverable by re-editing the community. const previous = communitiesRef.current.find((w) => w.id === id); if ( previous && updates.relayUrl !== undefined && updates.relayUrl !== previous.relayUrl ) { - rebindAgentRelay(previous.relayUrl, updates.relayUrl).catch( - (error) => { + rebindAgentRelay(previous.relayUrl, updates.relayUrl) + .catch((error) => { console.error("failed to rebind agents to edited relay:", error); - }, - ); - } - - setCommunitiesState((prev) => { - const next = prev.map((w) => - w.id === id ? { ...w, ...updates } : w, - ); - saveCommunities(next); - return next; - }); - - if (result.requiresReinit) { - setReinitKey((k) => k + 1); + }) + .finally(commit); + } else { + commit(); } } From 2b72652096db76a76c2009ae6790f3e4e3fac9a8 Mon Sep 17 00:00:00 2001 From: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Date: Fri, 17 Jul 2026 21:07:58 -0400 Subject: [PATCH 06/12] fix(desktop): preserve current workspace activation seams Reconcile the multi-workspace agent stack with current main without dropping the authenticated-media redirect boundary, agent-managed profile setting, or shared-compute status publication. Keep the file-size exceptions explicit for the combined state and command wiring. Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> --- desktop/scripts/check-file-sizes.mjs | 6 +++- desktop/src-tauri/src/app_state.rs | 35 +++++++++++++++++++++ desktop/src-tauri/src/commands/workspace.rs | 8 +++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index e89c5f3f7f..7bec3f636f 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -374,7 +374,11 @@ const overrides = new Map([ // +5 (1068 -> 1073): merge with main, which independently added the // managed_agent_profile_reconcile_enabled flag (field + doc + init) under // its own 1042-line override. Union of two separately approved additions. - ["src-tauri/src/app_state.rs", 1054], + ["src-tauri/src/app_state.rs", 1090], + // lazy-workspace-activation adds seven command/state wiring lines to lib.rs. + // Keeping the registration and safety-gate setup together is clearer than a + // one-off extraction whose only purpose would be satisfying the line cap. + ["src-tauri/src/lib.rs", 1007], // multi-slot splitting + no-op suppression (#1309): the ReadStateManager // class grew from ~700 lines to ~1019 with the addition of // splitContextsIntoBudgetedSlots (pure fn + 5 tests), publishSplitSlots, diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 70ff474d74..00a6c59e6b 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -18,6 +18,15 @@ use crate::managed_agents::ManagedAgentProcess; pub struct AppState { pub keys: Mutex, pub http_client: reqwest::Client, + /// A no-redirect client for authenticated relay media fetches (download, + /// clipboard copy, snapshot, editor). Every caller pre-validates the URL + /// origin, but the app-wide `http_client` follows redirects by default, so + /// a relay `/media/` URL returning a 3xx to an off-origin or private host + /// would forward the minted media Authorization header across origins — + /// a redirect-hop SSRF. This client treats any 3xx as a non-success + /// response (surfaced as an error) so the auth token never leaves the + /// validated relay origin. + pub media_fetch_client: reqwest::Client, /// Workspace-provided relay URL override. Set by `apply_workspace` on app /// init and takes priority over env vars and compile-time defaults. pub relay_url_override: Mutex>, @@ -155,6 +164,27 @@ fn identity_from_env() -> Option { } } +/// Build the no-redirect HTTP client used for authenticated relay media +/// fetches (download / copy). +/// +/// This client is a security boundary, not a convenience: it carries a minted +/// media `Authorization` header, so it MUST NOT follow redirects. A relay 3xx +/// to an off-origin or private host would otherwise forward that header across +/// origins (a redirect-hop SSRF). `redirect::Policy::none()` returns the 3xx +/// verbatim so the caller can reject it. +/// +/// Returned as a `Result` so the fail-closed invariant is testable — callers +/// must never substitute a redirect-following client on build failure. Shares +/// the localhost `resolve`/pool config with the app-wide `http_client`. +pub fn build_media_fetch_client() -> reqwest::Result { + reqwest::Client::builder() + .resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0))) + .pool_idle_timeout(std::time::Duration::from_secs(10)) + .pool_max_idle_per_host(1) + .redirect(reqwest::redirect::Policy::none()) + .build() +} + pub fn build_app_state() -> AppState { // Env var takes precedence (dev/CI). If absent, resolve_persisted_identity() // in setup() will replace the ephemeral placeholder with a persisted key. @@ -177,6 +207,11 @@ pub fn build_app_state() -> AppState { .pool_max_idle_per_host(1) .build() .unwrap_or_else(|_| reqwest::Client::new()), + media_fetch_client: build_media_fetch_client().expect( + "media_fetch_client must build with redirect::Policy::none(); a \ + redirect-following fallback would forward the minted media auth \ + header across origins (redirect-hop SSRF)", + ), relay_url_override: Mutex::new(None), managed_agent_restore_pending: AtomicBool::new(false), managed_agent_profile_reconcile_enabled: AtomicBool::new(true), diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 9f05036ed0..4af457e0fd 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -105,6 +105,7 @@ pub async fn apply_workspace( relay_url: String, nsec: Option, repos_dir: Option, + agent_managed_profiles: Option, app: AppHandle, ) -> Result<(), String> { let restore_app = app.clone(); @@ -151,6 +152,13 @@ pub async fn apply_workspace( *keys_guard = keys; } + // Keep the backend-side reconcile guard aligned with the frontend + // experiment before workspace activation can spawn any agents. Missing + // means the stable behavior: desktop remains authoritative. + state + .managed_agent_profile_reconcile_enabled + .store(!agent_managed_profiles.unwrap_or(false), Ordering::Release); + // ── One-shot legacy migration (non-fatal) ───────────────────────────── // Pin any blank-relay agent record to the first workspace applied // after boot — exactly what blank would have resolved to at boot From 6d1d4a5a70f4ff7e1f80dd5198dca2b2243ff8e3 Mon Sep 17 00:00:00 2001 From: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Date: Fri, 17 Jul 2026 21:12:49 -0400 Subject: [PATCH 07/12] refactor(desktop): expose workspace activation planning Separate the once-per-relay spawn decision from Tauri persistence and OS child creation so orchestration tests can drive A-to-B-to-A activation with a fake running-process view. Keep store locking, boot sweeps, parallel spawning, shutdown serialization, and PID writeback in the production wrapper. Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> --- .../src-tauri/src/managed_agents/restore.rs | 96 ++++++++++++------- 1 file changed, 64 insertions(+), 32 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index ff96902155..eaa76566ec 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -124,6 +124,37 @@ pub(crate) fn record_activates_on_relay( pinned.is_empty() || crate::relay::relay_urls_equivalent(pinned, workspace_relay_url) } +/// Pure activation decision produced after process housekeeping has reconciled +/// the record store with the live runtime view. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct WorkspaceActivationPlan { + pub run_boot_sweeps: bool, + pub spawn_pubkeys: Vec, +} + +/// Claim `workspace_relay_url` for this session and select the records that its +/// activation should spawn. The caller owns persistence, process creation, and +/// shutdown synchronization; tests can drive the complete A→B→A decision +/// sequence without constructing a Tauri app or an OS child process. +pub(crate) fn plan_workspace_activation( + records: &[ManagedAgentRecord], + already_running: &HashSet, + activated_relays: &mut HashSet, + workspace_relay_url: &str, +) -> Option { + let activation = begin_relay_activation(activated_relays, workspace_relay_url)?; + let spawn_pubkeys = records + .iter() + .filter(|record| record_activates_on_relay(record, workspace_relay_url)) + .filter(|record| !already_running.contains(&record.pubkey)) + .map(|record| record.pubkey.clone()) + .collect(); + Some(WorkspaceActivationPlan { + run_boot_sweeps: activation.run_boot_sweeps, + spawn_pubkeys, + }) +} + /// Lazily activate a workspace's managed agents. /// /// Called from every `apply_workspace`: starts local `start_on_app_launch` @@ -158,16 +189,6 @@ pub async fn activate_workspace_agents( return Ok(()); } - let Some(activation) = ({ - let mut activated_relays = state - .activated_agent_relays - .lock() - .map_err(|error| error.to_string())?; - begin_relay_activation(&mut activated_relays, workspace_relay_url) - }) else { - return Ok(()); - }; - // ── Phase A (under lock): housekeeping + collect agents to activate ── let mut agents_to_start: Vec; { @@ -191,12 +212,39 @@ pub async fn activate_workspace_agents( &super::current_instance_id(app), ); + let mut already_running = HashSet::new(); + for (pubkey, runtime) in runtimes.iter_mut() { + if runtime.child.try_wait().ok().flatten().is_none() { + already_running.insert(pubkey.clone()); + } + } + already_running.extend(records.iter().filter_map(|record| { + record + .runtime_pid + .filter(|pid| super::process_is_running(*pid)) + .map(|_| record.pubkey.clone()) + })); + let Some(plan) = ({ + let mut activated_relays = state + .activated_agent_relays + .lock() + .map_err(|error| error.to_string())?; + plan_workspace_activation( + &records, + &already_running, + &mut activated_relays, + workspace_relay_url, + ) + }) else { + return Ok(()); + }; + // One-shot boot cleanup: previous-session leftovers and orphans are // swept only on the session's first activation. Later activations // must not sweep — a concurrent activation's or UI start's freshly // spawned children are not yet in the tracked set and would be killed // as orphans. - if activation.run_boot_sweeps { + if plan.run_boot_sweeps { changed |= kill_stale_tracked_processes( &mut records, &runtimes, @@ -232,30 +280,14 @@ pub async fn activate_workspace_agents( super::sweep_untracked_bundle_harnesses(&tracked_pids); } - let candidates: Vec = records + let planned_pubkeys: HashSet<&str> = + plan.spawn_pubkeys.iter().map(String::as_str).collect(); + agents_to_start = records .iter() - .filter(|record| record_activates_on_relay(record, workspace_relay_url)) - .map(|record| record.pubkey.clone()) + .filter(|record| planned_pubkeys.contains(record.pubkey.as_str())) + .cloned() .collect(); - let mut to_start = Vec::new(); - for pubkey in &candidates { - if let Some(runtime) = runtimes.get_mut(pubkey) { - if runtime.child.try_wait().ok().flatten().is_none() { - continue; - } - } - if let Some(record) = records.iter().find(|r| r.pubkey == *pubkey) { - if let Some(pid) = record.runtime_pid { - if super::process_is_running(pid) { - continue; - } - } - to_start.push(record.clone()); - } - } - agents_to_start = to_start; - // Re-snapshot persona config for agents about to be restored, matching // the interactive spawn path so auto-start agents also pick up the // current persona on app launch. From 2cd376b5353bf5cd174c242d7de1094c2130e07e Mon Sep 17 00:00:00 2001 From: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Date: Fri, 17 Jul 2026 21:28:19 -0400 Subject: [PATCH 08/12] test(desktop): cover one-shot relay pin migration gate Extract the pending relay-stamp gate behind a synchronous helper so the production apply_workspace call and focused test share the same one-shot behavior. The helper completes the stamp before returning; workspace activation remains scheduled only after the enclosing blocking apply finishes. Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> --- desktop/src-tauri/src/commands/workspace.rs | 56 +++++++++++++++++---- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 4af457e0fd..a428573f33 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -1,6 +1,6 @@ use nostr::Keys; use serde::{Deserialize, Serialize}; -use std::sync::atomic::Ordering; +use std::sync::atomic::{AtomicBool, Ordering}; use tauri::{AppHandle, Emitter, Manager, State}; use crate::app_state::AppState; @@ -166,14 +166,13 @@ pub async fn apply_workspace( // from floating to a later workspace switch. Runs before the restore // trigger below so restored agents spawn from stamped records. A // failure is logged and retried on the next boot. - if state - .agent_relay_stamp_pending - .swap(false, Ordering::AcqRel) - { - if let Err(error) = pin_blank_agent_relays(&app, &state, &relay_url) { - eprintln!("buzz-desktop: blank agent relay migration failed: {error}"); - } - } + run_pending_agent_relay_stamp(&state.agent_relay_stamp_pending, || { + pin_blank_agent_relays(&app, &state, &relay_url) + }) + .unwrap_or_else(|error| { + eprintln!("buzz-desktop: blank agent relay migration failed: {error}"); + None + }); // ── Filesystem side-effect (non-fatal) ──────────────────────────────── // Persist the *effective* repos_dir (None when the candidate failed @@ -260,6 +259,45 @@ pub async fn apply_workspace( Ok(()) } +fn run_pending_agent_relay_stamp( + pending: &AtomicBool, + stamp: impl FnOnce() -> Result, +) -> Result, String> { + if !pending.swap(false, Ordering::AcqRel) { + return Ok(None); + } + stamp().map(Some) +} + +#[cfg(test)] +mod tests { + use super::run_pending_agent_relay_stamp; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + + #[test] + fn agent_relay_stamp_runs_once_and_completes_before_returning() { + let pending = AtomicBool::new(true); + let stamps = AtomicUsize::new(0); + + let first = run_pending_agent_relay_stamp(&pending, || { + stamps.fetch_add(1, Ordering::SeqCst); + Ok(7) + }) + .expect("first stamp succeeds"); + assert_eq!(first, Some(7)); + assert_eq!(stamps.load(Ordering::SeqCst), 1); + + let second = run_pending_agent_relay_stamp(&pending, || { + stamps.fetch_add(1, Ordering::SeqCst); + Ok(8) + }) + .expect("second apply skips the migration"); + assert_eq!(second, None); + assert_eq!(stamps.load(Ordering::SeqCst), 1); + assert!(!pending.load(Ordering::Acquire)); + } +} + /// Stamp legacy blank-relay agent records with the applied workspace relay. /// /// Store-lock + load/save wrapper around [`stamp_blank_agent_relay_urls`]; From 6d5c1ea85e5d5b2415d05da7801432889a9f1b93 Mon Sep 17 00:00:00 2001 From: npub1jmc9dt2lyvzu3h0kxlwxt5zg4fxp9476awyxw6gwxn72g6cw7exqs64whm <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Date: Fri, 17 Jul 2026 21:22:35 -0400 Subject: [PATCH 09/12] test(desktop): cover apply_workspace activation orchestration Drive plan_workspace_activation across the cross-community switch sequence (apply A, switch to B, switch back to A), asserting the observable spawn plan at each step. The existing restore tests exercise begin_relay_activation and record_activates_on_relay as isolated predicates; these close the coverage gap by proving the composition: a later apply must not re-spawn A, stop A to serve B, or let a blank-pinned record float from A to B. Both core invariants are verified load-bearing by mutation: removing the already_running exclusion double-spawns a live agent; neutering the relay filter leaks B's agent into A's plan. Test-only; no production lines touched. Co-authored-by: npub1jmc9dt2lyvzu3h0kxlwxt5zg4fxp9476awyxw6gwxn72g6cw7exqs64whm <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1jmc9dt2lyvzu3h0kxlwxt5zg4fxp9476awyxw6gwxn72g6cw7exqs64whm <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> --- .../src-tauri/src/managed_agents/restore.rs | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index eaa76566ec..1205baf0d7 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -624,3 +624,238 @@ mod tests { assert!(!record_activates_on_relay(&record, "wss://relay-a.example")); } } + +/// Orchestration-level coverage of the `apply_workspace` → activation decision. +/// +/// The tests above exercise `begin_relay_activation` and +/// `record_activates_on_relay` as isolated predicates. These drive the full +/// cross-community *sequence* through [`plan_workspace_activation`] — apply A, +/// switch to B, switch back to A — asserting the spawn decision (and the +/// blank-pin migration that gates it) at each step. That composition is what +/// the fast-switch defect regresses: an isolated predicate can stay correct +/// while a later apply wrongly re-spawns A, stops A to serve B, or lets a +/// blank-pinned record float from A to B. Assertions are on the observable +/// plan (`spawn_pubkeys`, `run_boot_sweeps`, `None`), not the predicates. +#[cfg(test)] +mod orchestration_tests { + use super::{plan_workspace_activation, ManagedAgentRecord}; + use crate::managed_agents::stamp_blank_agent_relay_urls; + use std::collections::HashSet; + + const RELAY_A: &str = "wss://relay-a.example"; + const RELAY_B: &str = "wss://relay-b.example"; + + /// A local start-on-launch record with an explicit pubkey and pin. Distinct + /// pubkeys let a single record set model several agents across the switch + /// sequence. + fn agent(pubkey: &str, relay_url: &str) -> ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "{pubkey}", + "name": "{pubkey}", + "private_key_nsec": "nsec1fake", + "relay_url": "{relay_url}", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }}"# + )) + .expect("sample record") + } + + fn spawn_set( + records: &[ManagedAgentRecord], + relay: &str, + activated: &mut HashSet, + ) -> Option> { + let running = HashSet::new(); + plan_workspace_activation(records, &running, activated, relay) + .map(|plan| plan.spawn_pubkeys) + } + + /// Applying community A starts only A's agent — B's pinned agent is not + /// spawned by A's activation. + #[test] + fn apply_a_spawns_only_a() { + let records = vec![agent("agent-a", RELAY_A), agent("agent-b", RELAY_B)]; + let mut activated = HashSet::new(); + + let plan = plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_A) + .expect("first apply activates"); + + assert_eq!(plan.spawn_pubkeys, vec!["agent-a".to_string()]); + assert!( + plan.run_boot_sweeps, + "first activation of the session sweeps" + ); + } + + /// Fast-switch A→B starts only B and never stops or restarts A: A is already + /// running, the plan carries no stop channel, and B's plan omits A. + #[test] + fn switch_to_b_spawns_only_b_and_leaves_a_running() { + let records = vec![agent("agent-a", RELAY_A), agent("agent-b", RELAY_B)]; + let mut activated = HashSet::new(); + + // Apply A first (consumes the session boot sweep), then switch to B with + // A's process live. + plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_A) + .expect("apply A"); + let mut running = HashSet::new(); + running.insert("agent-a".to_string()); + + let plan = plan_workspace_activation(&records, &running, &mut activated, RELAY_B) + .expect("switching to a new workspace activates it"); + + assert_eq!( + plan.spawn_pubkeys, + vec!["agent-b".to_string()], + "B activation spawns only B's agent" + ); + assert!( + !plan.run_boot_sweeps, + "a later activation must not re-run the one-shot boot sweeps" + ); + // Nothing in the plan can stop A: the type has no stop channel, and A + // remains in the running set the caller holds across the switch. + assert!( + running.contains("agent-a"), + "A stays running across the switch" + ); + } + + /// Switching back to a workspace already activated this session is a no-op: + /// A→B→A must not resurrect an agent the user stopped in A. + #[test] + fn revisiting_a_does_not_respawn() { + let records = vec![agent("agent-a", RELAY_A), agent("agent-b", RELAY_B)]; + let mut activated = HashSet::new(); + + plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_A) + .expect("apply A"); + plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_B) + .expect("apply B"); + + // A user who stopped agent-a in A leaves it not-running; revisiting A + // must still decline to plan, so the stop is honored. + assert!( + plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_A).is_none(), + "a re-activated relay yields no plan, so no agent is respawned" + ); + } + + /// Home pins and agent keys are stable across the whole switch sequence: the + /// planner reads records immutably, so a switch can never rewrite an agent's + /// identity or move its pin. + #[test] + fn keys_and_pins_are_stable_across_the_sequence() { + let records = vec![agent("agent-a", RELAY_A), agent("agent-b", RELAY_B)]; + let mut activated = HashSet::new(); + + for relay in [RELAY_A, RELAY_B, RELAY_A, RELAY_B] { + let _ = spawn_set(&records, relay, &mut activated); + } + + assert_eq!(records[0].pubkey, "agent-a"); + assert_eq!(records[0].relay_url, RELAY_A); + assert_eq!(records[1].pubkey, "agent-b"); + assert_eq!(records[1].relay_url, RELAY_B); + } + + /// A legacy blank-pinned record is stamped to the first workspace applied, + /// then never floats to a later workspace switch. This is the composition of + /// the one-shot stamp migration (before activation) with the relay-filtered + /// planner: on plain main the same record would spawn against whichever + /// workspace was visited, because a blank pin matches every relay. + #[test] + fn blank_record_stamps_to_first_workspace_and_never_floats() { + let mut records = vec![agent("agent-blank", "")]; + let mut activated = HashSet::new(); + + // Before stamping, a blank pin matches any relay — proving the float + // hazard the migration closes. + assert!( + plan_workspace_activation(&records, &HashSet::new(), &mut HashSet::new(), RELAY_B) + .expect("blank pin activates on any relay") + .spawn_pubkeys + == vec!["agent-blank".to_string()], + "a blank pin would otherwise activate on B too — this is the float" + ); + + // Apply A: the one-shot migration stamps the blank record to A, then A's + // activation spawns it. + let stamped = stamp_blank_agent_relay_urls(&mut records, RELAY_A); + assert_eq!(stamped, 1, "the blank record is stamped on the first apply"); + assert_eq!( + records[0].relay_url, RELAY_A, + "stamped to the first workspace" + ); + + let plan_a = plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_A) + .expect("apply A"); + assert_eq!(plan_a.spawn_pubkeys, vec!["agent-blank".to_string()]); + + // Switch to B: the now-A-pinned record is filtered out — it does not + // float to B. + let mut running = HashSet::new(); + running.insert("agent-blank".to_string()); + let plan_b = plan_workspace_activation(&records, &running, &mut activated, RELAY_B) + .expect("apply B"); + assert!( + plan_b.spawn_pubkeys.is_empty(), + "a stamped record must not float to a later workspace" + ); + } + + /// Within a single activation, a record already running on the *same* relay + /// is not spawned again — the `already_running` exclusion, not just the + /// relay filter, keeps the plan idempotent. Two agents share relay A; one is + /// live when A is applied, so only the down one is planned. + #[test] + fn a_running_agent_on_the_active_relay_is_not_respawned() { + let records = vec![agent("agent-a1", RELAY_A), agent("agent-a2", RELAY_A)]; + let mut activated = HashSet::new(); + let mut running = HashSet::new(); + running.insert("agent-a1".to_string()); // a1 already up on A + + let plan = plan_workspace_activation(&records, &running, &mut activated, RELAY_A) + .expect("apply A"); + + assert_eq!( + plan.spawn_pubkeys, + vec!["agent-a2".to_string()], + "only the not-running A agent is spawned; a1 is not double-spawned" + ); + } + + /// A failed spawn in A must not cause B's activation to adopt A's record. + /// A claimed its relay activation even though its process never came up, so + /// it is absent from `already_running`; B's relay-filtered plan still omits + /// A's A-pinned record. + #[test] + fn failed_spawn_in_a_does_not_leak_into_b() { + let records = vec![agent("agent-a", RELAY_A), agent("agent-b", RELAY_B)]; + let mut activated = HashSet::new(); + + // Apply A: A is planned for spawn, but model the spawn as having failed — + // A never enters the running set. + let plan_a = plan_workspace_activation(&records, &HashSet::new(), &mut activated, RELAY_A) + .expect("apply A"); + assert_eq!(plan_a.spawn_pubkeys, vec!["agent-a".to_string()]); + let running = HashSet::new(); // A's spawn failed: not running. + + // Switch to B: only B is planned. A's A-pinned record is not selected on + // B even though it is neither running nor successfully started. + let plan_b = plan_workspace_activation(&records, &running, &mut activated, RELAY_B) + .expect("apply B"); + assert_eq!( + plan_b.spawn_pubkeys, + vec!["agent-b".to_string()], + "a failed A spawn must not make B adopt A's record" + ); + } +} From b49523e7f7f0ef5c29ef2661f2baf891d12a159f Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 17 Jul 2026 21:22:27 -0400 Subject: [PATCH 10/12] fix(desktop): relay-scope the channel agent attach path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the cross-community silent-success trap: a managed agent pinned to community A's relay could be selected and "attached" to a channel while Desktop was viewing community B — membership was added against a process frozen on relay A that never hears B (visible-but-deaf, with `membershipAdded: true` reported over a deaf agent). Two guards on the attach path, both using the existing `agentBelongsToRelay` relay-scope helper: 1. `pickPreferredChannelPresetAgent` now filters reuse candidates to the active community's relay before selection, so a foreign-relay agent is never reused; `ensureChannelAgentPresetInChannel` falls through to creating a new agent on the active relay (correct per-community behavior for scope (a)). 2. `attachManagedAgentToChannel` gains a REQUIRED `activeRelayUrl` and asserts the agent belongs to it via the exported `assertAgentBelongsToActiveRelay` guard, throwing an actionable error (names the agent, its home relay, and the active relay) rather than reporting false success. Required — not optional — so the compiler finds every caller and no future direct caller can silently bypass it. The relay is sourced inside the mutation/attachment hooks via `useActiveRelayUrl()` and threaded down, so UI callers are unchanged. All five call sites (attach/ensure/create mutations, template apply, and the created-agent attachment hook) surface the thrown message rather than swallow it. Direct unit tests exercise the real selection and attach guards (not a re-derivation of `agentBelongsToRelay`): a B-pinned running agent is not selected in A (both member and name-match branches) and throws on direct attach, while same-relay and blank-pin agents remain eligible. Stacked on wren/agents-every-community-successor. Co-authored-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Signed-off-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc --- .../features/agents/channelAgents.test.mjs | 123 ++++++++++++++++++ desktop/src/features/agents/channelAgents.ts | 97 +++++++++++--- desktop/src/features/agents/hooks.ts | 26 +++- .../useCreatedAgentChannelAttachment.ts | 16 ++- .../channel-templates/useApplyTemplate.ts | 8 +- 5 files changed, 239 insertions(+), 31 deletions(-) create mode 100644 desktop/src/features/agents/channelAgents.test.mjs diff --git a/desktop/src/features/agents/channelAgents.test.mjs b/desktop/src/features/agents/channelAgents.test.mjs new file mode 100644 index 0000000000..d783d898b2 --- /dev/null +++ b/desktop/src/features/agents/channelAgents.test.mjs @@ -0,0 +1,123 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + assertAgentBelongsToActiveRelay, + pickPreferredChannelPresetAgent, +} from "./channelAgents.ts"; + +// Regression guard for the cross-community "silent-success trap": before this +// fix, a managed agent pinned to community A's relay could be selected and +// "attached" while operating in community B — membership was added against a +// process frozen on relay A that never hears B (Max's baseline). Both guards +// below close that path. They exercise the real selection/attach guards, not a +// re-derivation of `agentBelongsToRelay` (Mari's coverage requirement). + +const RELAY_A = "wss://relay-a.example"; +const RELAY_B = "wss://relay-b.example"; +const PUB_A = "a".repeat(64); +const PUB_B = "b".repeat(64); + +function makeAgent(overrides = {}) { + return { + pubkey: PUB_A, + name: "goose", + relayUrl: RELAY_A, + agentCommand: "goose", + status: "running", + updatedAt: "2026-01-15T00:00:00Z", + ...overrides, + }; +} + +test("pickPreferredChannelPresetAgent: excludes a foreign-relay agent (in-channel branch)", () => { + // A-pinned running agent that is already a member of the B channel: it must + // NOT be reused, so the caller falls through to creating a B agent. + const agentA = makeAgent({ pubkey: PUB_A, relayUrl: RELAY_A }); + const memberPubkeys = new Set([PUB_A]); + + const picked = pickPreferredChannelPresetAgent( + [agentA], + memberPubkeys, + "goose", + "goose", + RELAY_B, + ); + + assert.equal(picked, undefined, "foreign-relay agent must not be selected"); +}); + +test("pickPreferredChannelPresetAgent: excludes a foreign-relay agent (name-match branch)", () => { + const agentA = makeAgent({ pubkey: PUB_A, relayUrl: RELAY_A }); + + const picked = pickPreferredChannelPresetAgent( + [agentA], + new Set(), // not a member — exercises the name-match fallback branch + "goose", + "goose", + RELAY_B, + ); + + assert.equal(picked, undefined, "foreign-relay agent must not match by name"); +}); + +test("pickPreferredChannelPresetAgent: selects a same-relay agent", () => { + const agentB = makeAgent({ pubkey: PUB_B, relayUrl: RELAY_B }); + + const picked = pickPreferredChannelPresetAgent( + [agentB], + new Set([PUB_B]), + "goose", + "goose", + RELAY_B, + ); + + assert.equal(picked?.pubkey, PUB_B, "home-relay agent is reusable"); +}); + +test("pickPreferredChannelPresetAgent: blank-pin agent follows the active relay", () => { + // Defense in depth: a record that escaped stamping (blank relayUrl) follows + // the active community rather than being hidden everywhere. + const blank = makeAgent({ pubkey: PUB_B, relayUrl: "" }); + + const picked = pickPreferredChannelPresetAgent( + [blank], + new Set([PUB_B]), + "goose", + "goose", + RELAY_B, + ); + + assert.equal( + picked?.pubkey, + PUB_B, + "blank-pin agent is eligible in any community", + ); +}); + +test("assertAgentBelongsToActiveRelay: throws an actionable error for a foreign agent", () => { + const agentA = makeAgent({ name: "haiku-bot", relayUrl: RELAY_A }); + + assert.throws( + () => assertAgentBelongsToActiveRelay(agentA, RELAY_B), + (err) => { + // Actionable per Eva's refinement: names the agent, its home relay, and + // the active relay — not a bare boolean-y message. + assert.ok(err instanceof Error); + assert.match(err.message, /haiku-bot/, "names the agent"); + assert.match(err.message, /relay-a\.example/, "names the home relay"); + assert.match(err.message, /relay-b\.example/, "names the active relay"); + return true; + }, + ); +}); + +test("assertAgentBelongsToActiveRelay: does not throw for a same-relay agent", () => { + const agentB = makeAgent({ relayUrl: RELAY_B }); + assert.doesNotThrow(() => assertAgentBelongsToActiveRelay(agentB, RELAY_B)); +}); + +test("assertAgentBelongsToActiveRelay: does not throw for a blank-pin agent", () => { + const blank = makeAgent({ relayUrl: "" }); + assert.doesNotThrow(() => assertAgentBelongsToActiveRelay(blank, RELAY_B)); +}); diff --git a/desktop/src/features/agents/channelAgents.ts b/desktop/src/features/agents/channelAgents.ts index 4eb951f6d4..40ef189ee6 100644 --- a/desktop/src/features/agents/channelAgents.ts +++ b/desktop/src/features/agents/channelAgents.ts @@ -5,6 +5,7 @@ import { pickPreferredManagedAgent, } from "@/features/agents/agentReuse"; export { findReusableAgent } from "@/features/agents/agentReuse"; +import { agentBelongsToRelay } from "@/features/agents/agentRelayScope"; import { normalizePubkey } from "@/shared/lib/pubkey"; import { resolveManagedAgentAvatarUrl } from "@/features/agents/ui/managedAgentAvatar"; import { @@ -104,13 +105,41 @@ export type CreateChannelManagedAgentsResult = { failures: CreateChannelManagedAgentBatchFailure[]; }; +/** + * Relay invariant guard for channel attachment. An agent serves its home + * relay; attaching a foreign-relay agent to a channel on the active relay + * would add membership against a process that cannot hear this community — + * the silent-success trap where `membershipAdded: true` reports over a deaf + * agent. Throws an actionable error naming the agent, its home relay, and + * the active relay so the caller's onError surfaces a real message. + * + * Exported so the attach path and its regression tests exercise the same + * guard rather than a re-derivation of `agentBelongsToRelay`. + */ +export function assertAgentBelongsToActiveRelay( + agent: Pick, + activeRelayUrl: string | null, +): void { + if (!agentBelongsToRelay(agent.relayUrl, activeRelayUrl)) { + throw new Error( + `Agent "${agent.name}" belongs to ${agent.relayUrl || "another community"} ` + + `and cannot be added to a channel on ${activeRelayUrl || "this community"}. ` + + `Add it from its home community, or create a new agent here.`, + ); + } +} + export async function attachManagedAgentToChannel( channelId: string, input: AttachManagedAgentToChannelInput, + activeRelayUrl: string | null, ) { const role = input.role ?? "bot"; const ensureRunning = input.ensureRunning ?? true; const agentPubkey = normalizePubkey(input.agent.pubkey); + + assertAgentBelongsToActiveRelay(input.agent, activeRelayUrl); + const membershipResult = await addChannelMembers({ channelId, pubkeys: [input.agent.pubkey], @@ -164,14 +193,23 @@ function buildChannelAgentName(runtimeId: string, runtimeLabel: string) { return runtimeLabel.trim().toLowerCase() || "agent"; } -function pickPreferredChannelPresetAgent( +export function pickPreferredChannelPresetAgent( agents: ManagedAgent[], memberPubkeys: ReadonlySet, runtimeCommand: string, expectedName: string, + activeRelayUrl: string | null, ) { + // Only agents pinned to the active community's relay are reusable here. + // Selecting a foreign-relay agent would attach a process that cannot hear + // this community; excluding it lets the caller fall through to creating a + // new agent on the active relay (correct per-community behavior). + const relayScoped = agents.filter((agent) => + agentBelongsToRelay(agent.relayUrl, activeRelayUrl), + ); + const inChannelAgent = pickPreferredManagedAgent( - agents.filter( + relayScoped.filter( (agent) => commandsMatch(agent.agentCommand, runtimeCommand) && memberPubkeys.has(normalizePubkey(agent.pubkey)), @@ -182,7 +220,7 @@ function pickPreferredChannelPresetAgent( } return pickPreferredManagedAgent( - agents.filter( + relayScoped.filter( (agent) => commandsMatch(agent.agentCommand, runtimeCommand) && agent.name.trim().toLowerCase() === expectedName.trim().toLowerCase(), @@ -193,6 +231,7 @@ function pickPreferredChannelPresetAgent( export async function ensureChannelAgentPresetInChannel( channelId: string, input: EnsureChannelAgentPresetInput, + activeRelayUrl: string | null, ): Promise { const role = input.role ?? "bot"; const ensureRunning = input.ensureRunning ?? true; @@ -210,14 +249,19 @@ export async function ensureChannelAgentPresetInChannel( memberPubkeys, input.runtime.command, expectedName, + activeRelayUrl, ); if (existingAgent) { - const attached = await attachManagedAgentToChannel(channelId, { - agent: existingAgent, - role, - ensureRunning, - }); + const attached = await attachManagedAgentToChannel( + channelId, + { + agent: existingAgent, + role, + ensureRunning, + }, + activeRelayUrl, + ); return { ...attached, created: false, @@ -233,11 +277,15 @@ export async function ensureChannelAgentPresetInChannel( mcpCommand: input.runtime.mcpCommand ?? "", spawnAfterCreate: false, }); - const attached = await attachManagedAgentToChannel(channelId, { - agent: created.agent, - role, - ensureRunning, - }); + const attached = await attachManagedAgentToChannel( + channelId, + { + agent: created.agent, + role, + ensureRunning, + }, + activeRelayUrl, + ); return { ...attached, @@ -379,17 +427,22 @@ export async function provisionChannelManagedAgent( export async function createChannelManagedAgent( channelId: string, input: CreateChannelManagedAgentInput, + activeRelayUrl: string | null, context?: { managedAgents?: ManagedAgent[]; channelMemberPubkeys?: ReadonlySet; }, ): Promise { const provisioned = await provisionChannelManagedAgent(input, context); - const attached = await attachManagedAgentToChannel(channelId, { - agent: provisioned.agent, - role: input.role ?? "bot", - ensureRunning: input.ensureRunning ?? true, - }); + const attached = await attachManagedAgentToChannel( + channelId, + { + agent: provisioned.agent, + role: input.role ?? "bot", + ensureRunning: input.ensureRunning ?? true, + }, + activeRelayUrl, + ); return { ...attached, @@ -401,6 +454,7 @@ export async function createChannelManagedAgent( export async function createChannelManagedAgents( channelId: string, inputs: readonly CreateChannelManagedAgentInput[], + activeRelayUrl: string | null, ): Promise { // Fetch managed agents and channel members once for smart reuse checks. const [managedAgents, members] = await Promise.all([ @@ -421,7 +475,12 @@ export async function createChannelManagedAgents( for (let i = 0; i < inputs.length; i++) { const input = inputs[i]; try { - const result = await createChannelManagedAgent(channelId, input, context); + const result = await createChannelManagedAgent( + channelId, + input, + activeRelayUrl, + context, + ); successes.push(result); } catch (error) { failures.push({ diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index 9ceeb0220e..6bf7a102cf 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -581,6 +581,7 @@ export function useAttachManagedAgentToChannelMutation( channelId: string | null, ) { const queryClient = useQueryClient(); + const activeRelayUrl = useActiveRelayUrl(); return useMutation({ mutationFn: async ( @@ -592,7 +593,11 @@ export function useAttachManagedAgentToChannelMutation( throw new Error("No channel selected."); } - return attachManagedAgentToChannel(effectiveChannelId, rest); + return attachManagedAgentToChannel( + effectiveChannelId, + rest, + activeRelayUrl, + ); }, onSuccess: (result, variables) => { const effectiveChannelId = variables.channelId ?? channelId; @@ -627,6 +632,7 @@ export function useAttachManagedAgentToChannelMutation( export function useEnsureChannelAgentPresetMutation(channelId: string | null) { const queryClient = useQueryClient(); + const activeRelayUrl = useActiveRelayUrl(); return useMutation({ mutationFn: async ( @@ -636,7 +642,11 @@ export function useEnsureChannelAgentPresetMutation(channelId: string | null) { throw new Error("No channel selected."); } - return ensureChannelAgentPresetInChannel(channelId, input); + return ensureChannelAgentPresetInChannel( + channelId, + input, + activeRelayUrl, + ); }, onSettled: () => { invalidateAgentQueriesInBackground(queryClient, channelId); @@ -646,6 +656,7 @@ export function useEnsureChannelAgentPresetMutation(channelId: string | null) { export function useCreateChannelManagedAgentMutation(channelId: string | null) { const queryClient = useQueryClient(); + const activeRelayUrl = useActiveRelayUrl(); return useMutation({ mutationFn: async ( @@ -657,9 +668,11 @@ export function useCreateChannelManagedAgentMutation(channelId: string | null) { throw new Error("No channel selected."); } - const result = await createChannelManagedAgents(effectiveChannelId, [ - rest, - ]); + const result = await createChannelManagedAgents( + effectiveChannelId, + [rest], + activeRelayUrl, + ); const success = result.successes[0]; if (success) { return success; @@ -745,6 +758,7 @@ export function useCreateChannelManagedAgentsMutation( channelId: string | null, ) { const queryClient = useQueryClient(); + const activeRelayUrl = useActiveRelayUrl(); return useMutation({ mutationFn: async ( @@ -754,7 +768,7 @@ export function useCreateChannelManagedAgentsMutation( throw new Error("No channel selected."); } - return createChannelManagedAgents(channelId, inputs); + return createChannelManagedAgents(channelId, inputs, activeRelayUrl); }, onSettled: () => { invalidateAgentQueriesInBackground(queryClient, channelId); diff --git a/desktop/src/features/agents/useCreatedAgentChannelAttachment.ts b/desktop/src/features/agents/useCreatedAgentChannelAttachment.ts index e016766c2e..5b7fe7cc5c 100644 --- a/desktop/src/features/agents/useCreatedAgentChannelAttachment.ts +++ b/desktop/src/features/agents/useCreatedAgentChannelAttachment.ts @@ -2,6 +2,7 @@ import * as React from "react"; import { attachManagedAgentToChannel } from "./channelAgents"; import type { AgentChannelAttachmentFailure } from "./channelAttachmentFailure"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import type { Channel, CreateManagedAgentResponse } from "@/shared/api/types"; type TargetChannel = Pick; @@ -17,6 +18,7 @@ export function useCreatedAgentChannelAttachment() { React.useState(null); const targetChannelRef = React.useRef(null); const [isRetryingAttachment, setIsRetryingAttachment] = React.useState(false); + const activeRelayUrl = useActiveRelayUrl(); async function attach( created: CreateManagedAgentResponse, @@ -24,11 +26,15 @@ export function useCreatedAgentChannelAttachment() { ) { targetChannelRef.current = targetChannel; try { - const attached = await attachManagedAgentToChannel(targetChannel.id, { - agent: created.agent, - role: "bot", - ensureRunning: true, - }); + const attached = await attachManagedAgentToChannel( + targetChannel.id, + { + agent: created.agent, + role: "bot", + ensureRunning: true, + }, + activeRelayUrl, + ); created.agent = attached.agent; targetChannelRef.current = null; setAttachmentFailure(null); diff --git a/desktop/src/features/channel-templates/useApplyTemplate.ts b/desktop/src/features/channel-templates/useApplyTemplate.ts index 1f7d066fad..4babfe3022 100644 --- a/desktop/src/features/channel-templates/useApplyTemplate.ts +++ b/desktop/src/features/channel-templates/useApplyTemplate.ts @@ -12,6 +12,7 @@ import { import { resolvePersonaRuntime } from "@/features/agents/lib/resolvePersonaRuntime"; import { resolveTeamPersonas } from "@/features/agents/lib/teamPersonas"; import { useLastRuntime } from "@/features/agents/lib/useLastRuntime"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import { useChannelTemplatesQuery } from "@/features/channel-templates/hooks"; import { setCanvas } from "@/shared/api/tauri"; import type { ChannelTemplate } from "@/shared/api/types"; @@ -33,6 +34,7 @@ export function useApplyTemplate() { const personasQuery = usePersonasQuery(); const teamsQuery = useTeamsQuery(); const { lastRuntimeId } = useLastRuntime(); + const activeRelayUrl = useActiveRelayUrl(); async function applyCanvas( templateId: string | undefined, @@ -132,7 +134,11 @@ export function useApplyTemplate() { if (inputs.length === 0) return; try { - const result = await createChannelManagedAgents(channelId, inputs); + const result = await createChannelManagedAgents( + channelId, + inputs, + activeRelayUrl, + ); if (result.failures.length > 0) { const { toast } = await import("sonner"); toast.warning( From a332b5e53491fbaf94fc38632aacb5b544f09fc2 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 17 Jul 2026 21:39:40 -0400 Subject: [PATCH 11/12] fix(desktop): relay-scope the channel agent reuse finders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complete the cross-community reuse fix on the *creation* path. The attach guards (commit b49523e7f) stopped a foreign-relay agent from being silently attached, but the reuse finders that decide "reuse existing vs create new" were still relay-unfiltered: a persona/generic agent pinned to community A's relay would surface as a reuse candidate while Desktop viewed community B, so creating a B agent could return the deaf A agent instead of a fresh B one — user-visible wrongness (Eva's scope call). Relay-scope all three reuse finders with the existing `agentBelongsToRelay` helper: 1. `findReusablePersonaAgent` / `findReusableGenericAgent` gain a REQUIRED `activeRelayUrl` and filter candidates to the active community's relay before preference selection. A foreign-relay candidate is excluded, so the caller falls through to creating a fresh agent on the active relay. 2. `findReusableAgent` (the routing wrapper) threads `activeRelayUrl` through to both finders. 3. Both consumers pass it: `provisionChannelManagedAgent`'s context (via `createChannelManagedAgents(..., activeRelayUrl)` and the provision mutation hook) and the `useReusableAgentDetection` UI guardrail, each sourcing the relay from `useActiveRelayUrl()`. The param is REQUIRED so the compiler finds every caller and no future call can silently bypass the scope. `agentBelongsToRelay` stays permissive for blank pins / blank active relay, so existing tests and legacy blank-pin records are unaffected. Four unit tests cover the new behavior: a B-pinned persona/generic candidate is excluded when the active relay is A (fresh agent created, no error), a same-relay candidate is still reused, and the wrapper routes the foreign-relay persona case to a fresh agent. 42/42 agentReuse tests pass; tsc --noEmit and biome check clean. Stacked on wren/agents-every-community-successor (commit #4 on b49523e7f). Co-authored-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Signed-off-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc --- .../src/features/agents/agentReuse.test.mjs | 65 +++++++++++++++++++ desktop/src/features/agents/agentReuse.ts | 12 +++- desktop/src/features/agents/channelAgents.ts | 5 +- desktop/src/features/agents/hooks.ts | 2 + .../channels/ui/useReusableAgentDetection.ts | 29 ++++++--- 5 files changed, 102 insertions(+), 11 deletions(-) diff --git a/desktop/src/features/agents/agentReuse.test.mjs b/desktop/src/features/agents/agentReuse.test.mjs index cc85a1de88..cf60c0d09e 100644 --- a/desktop/src/features/agents/agentReuse.test.mjs +++ b/desktop/src/features/agents/agentReuse.test.mjs @@ -308,6 +308,71 @@ test("findReusableGenericAgent: command matching uses normalization", () => { assert.equal(result, agent); }); +test("findReusablePersonaAgent: excludes agent pinned to a different relay", () => { + const agent = makeAgent({ + personaId: "persona-1", + pubkey: PUB_A, + relayUrl: "wss://relay-a.example", + }); + const channelMembers = new Set([PUB_B]); + const result = findReusablePersonaAgent( + [agent], + "persona-1", + channelMembers, + "wss://relay-b.example", + ); + assert.equal(result, undefined); +}); + +test("findReusablePersonaAgent: reuses agent pinned to the active relay", () => { + const agent = makeAgent({ + personaId: "persona-1", + pubkey: PUB_A, + relayUrl: "wss://relay-b.example", + }); + const channelMembers = new Set([PUB_B]); + const result = findReusablePersonaAgent( + [agent], + "persona-1", + channelMembers, + "wss://relay-b.example", + ); + assert.equal(result, agent); +}); + +test("findReusableGenericAgent: excludes agent pinned to a different relay", () => { + const agent = makeAgent({ + agentCommand: "goose", + personaId: null, + systemPrompt: null, + relayUrl: "wss://relay-a.example", + }); + const channelMembers = new Set([PUB_B]); + const result = findReusableGenericAgent( + [agent], + "goose", + channelMembers, + "wss://relay-b.example", + ); + assert.equal(result, undefined); +}); + +test("findReusableAgent: foreign-relay persona candidate yields fresh agent (no reuse)", () => { + const agent = makeAgent({ + personaId: "p1", + pubkey: PUB_A, + relayUrl: "wss://relay-a.example", + }); + const channelMembers = new Set([PUB_B]); + const result = findReusableAgent( + [agent], + channelMembers, + { personaId: "p1", command: "goose" }, + "wss://relay-b.example", + ); + assert.equal(result, undefined); +}); + test("findReusableAgent: routes to persona search when personaId provided", () => { const agent = makeAgent({ personaId: "p1", pubkey: PUB_A }); const channelMembers = new Set([PUB_B]); diff --git a/desktop/src/features/agents/agentReuse.ts b/desktop/src/features/agents/agentReuse.ts index b0d8007035..3a438d3d42 100644 --- a/desktop/src/features/agents/agentReuse.ts +++ b/desktop/src/features/agents/agentReuse.ts @@ -1,3 +1,4 @@ +import { agentBelongsToRelay } from "@/features/agents/agentRelayScope"; import type { ManagedAgent } from "@/shared/api/types"; /** Inline normalization — avoids runtime dependency on @/shared/lib/pubkey. */ @@ -50,11 +51,13 @@ export function findReusablePersonaAgent( agents: ManagedAgent[], personaId: string, channelMemberPubkeys: ReadonlySet, + activeRelayUrl: string | null | undefined, ): ManagedAgent | undefined { const candidates = agents.filter( (agent) => agent.personaId === personaId && - !channelMemberPubkeys.has(normalizePubkey(agent.pubkey)), + !channelMemberPubkeys.has(normalizePubkey(agent.pubkey)) && + agentBelongsToRelay(agent.relayUrl, activeRelayUrl), ); return pickPreferredManagedAgent(candidates); } @@ -63,13 +66,15 @@ export function findReusableGenericAgent( agents: ManagedAgent[], command: string, channelMemberPubkeys: ReadonlySet, + activeRelayUrl: string | null | undefined, ): ManagedAgent | undefined { const candidates = agents.filter( (agent) => !agent.personaId && !agent.systemPrompt?.trim() && commandsMatch(agent.agentCommand, command) && - !channelMemberPubkeys.has(normalizePubkey(agent.pubkey)), + !channelMemberPubkeys.has(normalizePubkey(agent.pubkey)) && + agentBelongsToRelay(agent.relayUrl, activeRelayUrl), ); return pickPreferredManagedAgent(candidates); } @@ -86,12 +91,14 @@ export function findReusableAgent( systemPrompt?: string; command: string; }, + activeRelayUrl: string | null | undefined, ): ManagedAgent | undefined { if (input.personaId) { return findReusablePersonaAgent( agents, input.personaId, channelMemberPubkeys, + activeRelayUrl, ); } if (!input.systemPrompt?.trim()) { @@ -99,6 +106,7 @@ export function findReusableAgent( agents, input.command, channelMemberPubkeys, + activeRelayUrl, ); } return undefined; diff --git a/desktop/src/features/agents/channelAgents.ts b/desktop/src/features/agents/channelAgents.ts index 40ef189ee6..132894d3e8 100644 --- a/desktop/src/features/agents/channelAgents.ts +++ b/desktop/src/features/agents/channelAgents.ts @@ -299,6 +299,7 @@ export async function provisionChannelManagedAgent( context?: { managedAgents?: ManagedAgent[]; channelMemberPubkeys?: ReadonlySet; + activeRelayUrl?: string | null; }, ): Promise { const trimmedName = input.name.trim(); @@ -319,6 +320,7 @@ export async function provisionChannelManagedAgent( context.managedAgents, input.personaId, context.channelMemberPubkeys, + context.activeRelayUrl, ); if (reusable) { // Apply the caller's respondTo settings so the user's permission @@ -359,6 +361,7 @@ export async function provisionChannelManagedAgent( context.managedAgents, input.runtime.command, context.channelMemberPubkeys, + context.activeRelayUrl, ); if (reusable) { const needsRespondToUpdate = @@ -464,7 +467,7 @@ export async function createChannelManagedAgents( const channelMemberPubkeys = new Set( members.map((m) => normalizePubkey(m.pubkey)), ); - const context = { managedAgents, channelMemberPubkeys }; + const context = { managedAgents, channelMemberPubkeys, activeRelayUrl }; // Sequential loop: each agent must be fully created and its relay membership // written before the next starts. Concurrent writes to the replaceable diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index 6bf7a102cf..852172f9e7 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -711,6 +711,7 @@ export function useProvisionChannelManagedAgentMutation( channelId: string | null, ) { const queryClient = useQueryClient(); + const activeRelayUrl = useActiveRelayUrl(); return useMutation({ mutationFn: async ( @@ -731,6 +732,7 @@ export function useProvisionChannelManagedAgentMutation( channelMemberPubkeys: new Set( members.map((member) => normalizePubkey(member.pubkey)), ), + activeRelayUrl, }); }, onSuccess: (result) => { diff --git a/desktop/src/features/channels/ui/useReusableAgentDetection.ts b/desktop/src/features/channels/ui/useReusableAgentDetection.ts index c35286a4a1..f1719064b8 100644 --- a/desktop/src/features/channels/ui/useReusableAgentDetection.ts +++ b/desktop/src/features/channels/ui/useReusableAgentDetection.ts @@ -5,6 +5,7 @@ import { useManagedAgentsQuery, } from "@/features/agents/hooks"; import { useChannelMembersQuery } from "@/features/channels/hooks"; +import { useActiveRelayUrl } from "@/features/communities/useCommunities"; import { normalizePubkey } from "@/shared/lib/pubkey"; import type { AcpRuntime, ManagedAgent } from "@/shared/api/types"; @@ -25,6 +26,7 @@ export function useReusableAgentDetection( ): ManagedAgent | undefined { const managedAgentsQuery = useManagedAgentsQuery(); const channelMembersQuery = useChannelMembersQuery(channelId, enabled); + const activeRelayUrl = useActiveRelayUrl(); return React.useMemo(() => { const agents = managedAgentsQuery.data; @@ -36,10 +38,15 @@ export function useReusableAgentDetection( // For persona selection: check the first selected persona if (selectedPersonas.length === 1 && !includeGeneric) { - return findReusableAgent(agents, memberPubkeys, { - personaId: selectedPersonas[0].id, - command: selectedRuntime.command, - }); + return findReusableAgent( + agents, + memberPubkeys, + { + personaId: selectedPersonas[0].id, + command: selectedRuntime.command, + }, + activeRelayUrl, + ); } // For generic agent with no custom prompt @@ -48,10 +55,15 @@ export function useReusableAgentDetection( selectedPersonas.length === 0 && !customPrompt.trim() ) { - return findReusableAgent(agents, memberPubkeys, { - command: selectedRuntime.command, - systemPrompt: customPrompt, - }); + return findReusableAgent( + agents, + memberPubkeys, + { + command: selectedRuntime.command, + systemPrompt: customPrompt, + }, + activeRelayUrl, + ); } return undefined; @@ -62,5 +74,6 @@ export function useReusableAgentDetection( selectedPersonas, includeGeneric, customPrompt, + activeRelayUrl, ]); } From bf34c4eeebda3725078a6d1dd928bdbeaf320893 Mon Sep 17 00:00:00 2001 From: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Date: Fri, 17 Jul 2026 21:59:27 -0400 Subject: [PATCH 12/12] refactor(desktop): extract agent query invalidation Move the cohesive background query-invalidation helpers out of the oversized agents hooks module. Preserve the existing hooks exports and behavior while restoring meaningful headroom under the desktop file-size limit. Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> --- .../features/agents/agentQueryInvalidation.ts | 77 +++++++++++++++++ desktop/src/features/agents/hooks.ts | 83 +++---------------- 2 files changed, 88 insertions(+), 72 deletions(-) create mode 100644 desktop/src/features/agents/agentQueryInvalidation.ts diff --git a/desktop/src/features/agents/agentQueryInvalidation.ts b/desktop/src/features/agents/agentQueryInvalidation.ts new file mode 100644 index 0000000000..2fc4752c8e --- /dev/null +++ b/desktop/src/features/agents/agentQueryInvalidation.ts @@ -0,0 +1,77 @@ +import type { QueryClient } from "@tanstack/react-query"; + +import { channelsQueryKey } from "@/features/channels/hooks"; +import type { Channel } from "@/shared/api/types"; + +export const relayAgentsQueryKey = ["relay-agents"] as const; +export const managedAgentsQueryKey = ["managed-agents"] as const; + +type InvalidateAgentQueriesOptions = { + refetchChannels?: boolean; +}; + +async function invalidateAgentQueries( + queryClient: QueryClient, + channelId: string | null, + options: InvalidateAgentQueriesOptions = {}, +) { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }), + queryClient.invalidateQueries({ queryKey: relayAgentsQueryKey }), + queryClient.invalidateQueries({ + queryKey: channelsQueryKey, + refetchType: options.refetchChannels === false ? "none" : "active", + }), + ...(channelId + ? [ + queryClient.invalidateQueries({ + queryKey: ["channels", channelId, "members"], + }), + ] + : []), + ]); +} + +function refreshAgentQueriesInBackground(task: () => Promise) { + void task().catch((error) => { + console.error("Failed to refresh agent queries", error); + }); +} + +export function invalidateAgentQueriesInBackground( + queryClient: QueryClient, + channelId: string | null, + options?: InvalidateAgentQueriesOptions, +) { + refreshAgentQueriesInBackground(() => + invalidateAgentQueries(queryClient, channelId, options), + ); +} + +export function isCachedDmChannel( + queryClient: QueryClient, + channelId: string | null, +) { + if (!channelId) { + return false; + } + + return Boolean( + queryClient + .getQueryData(channelsQueryKey) + ?.some( + (channel) => channel.id === channelId && channel.channelType === "dm", + ), + ); +} + +export function invalidateManagedAgentQueriesInBackground( + queryClient: QueryClient, +) { + refreshAgentQueriesInBackground(() => + Promise.all([ + queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }), + queryClient.invalidateQueries({ queryKey: relayAgentsQueryKey }), + ]), + ); +} diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index 852172f9e7..9b9005fe56 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -2,6 +2,13 @@ import * as React from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { hasRunningAgentInCommunity } from "@/features/agents/agentRelayScope"; +import { + invalidateAgentQueriesInBackground, + invalidateManagedAgentQueriesInBackground, + isCachedDmChannel, + managedAgentsQueryKey, + relayAgentsQueryKey, +} from "@/features/agents/agentQueryInvalidation"; import { connectAcpRuntime, discoverAcpAuthMethods, @@ -102,8 +109,10 @@ export type { ProvisionChannelManagedAgentResult, } from "@/features/agents/channelAgents"; -export const relayAgentsQueryKey = ["relay-agents"] as const; -export const managedAgentsQueryKey = ["managed-agents"] as const; +export { + managedAgentsQueryKey, + relayAgentsQueryKey, +} from "@/features/agents/agentQueryInvalidation"; export const personasQueryKey = ["personas"] as const; export const teamsQueryKey = ["teams"] as const; export const acpRuntimesQueryKey = ["acp-runtimes"] as const; @@ -112,76 +121,6 @@ export const managedAgentPrereqsQueryKey = ["managed-agent-prereqs"] as const; export const backendProvidersQueryKey = ["backend-providers"] as const; export const gitBashPrerequisiteQueryKey = ["git-bash-prerequisite"] as const; -type InvalidateAgentQueriesOptions = { - refetchChannels?: boolean; -}; - -async function invalidateAgentQueries( - queryClient: ReturnType, - channelId: string | null, - options: InvalidateAgentQueriesOptions = {}, -) { - await Promise.all([ - queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }), - queryClient.invalidateQueries({ queryKey: relayAgentsQueryKey }), - queryClient.invalidateQueries({ - queryKey: channelsQueryKey, - refetchType: options.refetchChannels === false ? "none" : "active", - }), - ...(channelId - ? [ - queryClient.invalidateQueries({ - queryKey: ["channels", channelId, "members"], - }), - ] - : []), - ]); -} - -function refreshAgentQueriesInBackground(task: () => Promise) { - void task().catch((error) => { - console.error("Failed to refresh agent queries", error); - }); -} - -function invalidateAgentQueriesInBackground( - queryClient: ReturnType, - channelId: string | null, - options?: InvalidateAgentQueriesOptions, -) { - refreshAgentQueriesInBackground(() => - invalidateAgentQueries(queryClient, channelId, options), - ); -} - -function isCachedDmChannel( - queryClient: ReturnType, - channelId: string | null, -) { - if (!channelId) { - return false; - } - - return Boolean( - queryClient - .getQueryData(channelsQueryKey) - ?.some( - (channel) => channel.id === channelId && channel.channelType === "dm", - ), - ); -} - -function invalidateManagedAgentQueriesInBackground( - queryClient: ReturnType, -) { - refreshAgentQueriesInBackground(() => - Promise.all([ - queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }), - queryClient.invalidateQueries({ queryKey: relayAgentsQueryKey }), - ]), - ); -} - export function useAcpRuntimesQuery(options?: { enabled?: boolean }) { return useQuery({ enabled: options?.enabled ?? true,