diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index b2f444fa94..d5577b4bbd 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -189,7 +189,13 @@ pub(crate) fn built_in_persona_definition(id: &str, now: &str) -> Option Option { - available_built_in_personas(bestie_build_enabled()).position(|persona| persona.id == id) + BUILT_IN_PERSONAS + .iter() + .position(|persona| persona.id == id) +} + +fn persona_available_in_build(id: &str, include_bestie: bool) -> bool { + include_bestie || id != BESTIE_PERSONA_ID } fn sort_personas(records: &mut [AgentDefinition]) { @@ -214,10 +220,19 @@ fn sort_personas(records: &mut [AgentDefinition]) { }); } -fn merge_personas(mut stored: Vec, now: &str) -> (Vec, bool) { +#[cfg(test)] +fn merge_personas(stored: Vec, now: &str) -> (Vec, bool) { + merge_personas_for_build(stored, now, bestie_build_enabled()) +} + +fn merge_personas_for_build( + mut stored: Vec, + now: &str, + include_bestie: bool, +) -> (Vec, bool) { let mut changed = false; - for built_in in built_in_persona_records(now) { + for built_in in built_in_persona_records_for_build(now, include_bestie) { if let Some(existing) = stored.iter_mut().find(|record| record.id == built_in.id) { if !existing.is_builtin { existing.is_builtin = true; @@ -231,6 +246,8 @@ fn merge_personas(mut stored: Vec, now: &str) -> (Vec, now: &str) -> (Vec, + include_bestie: bool, +) -> Vec { + records + .into_iter() + .filter(|record| persona_available_in_build(&record.id, include_bestie)) + .collect() +} + +fn definitions_for_save( + records: &[AgentDefinition], + existing: &[AgentDefinition], + include_bestie: bool, +) -> Vec { + let mut complete = records.to_vec(); + for hidden_builtin in existing.iter().filter(|record| { + built_in_order(&record.id).is_some() + && !persona_available_in_build(&record.id, include_bestie) + }) { + if !complete.iter().any(|record| record.id == hidden_builtin.id) { + complete.push(hidden_builtin.clone()); + } + } + sort_personas(&mut complete); + complete +} + /// Soft-deprecate retired built-in personas by appending " (retired)" to /// their display name and marking them inactive. Never removes records — /// the cost is extra records for pre-transition users, but this @@ -372,6 +417,7 @@ pub fn validate_persona_activation_change( pub fn load_personas(app: &AppHandle) -> Result, String> { let now = now_iso(); + let include_bestie = bestie_build_enabled(); // Post-fold: definitions live in the unified agent store, presented in // the legacy shape. Pre-fold stores are converted by @@ -382,12 +428,12 @@ pub fn load_personas(app: &AppHandle) -> Result, String> { .filter_map(|record| record.to_definition_view()) .collect(); - let (records, changed) = merge_personas(records, &now); + let (records, changed) = merge_personas_for_build(records, &now, include_bestie); if changed { - save_personas(app, &records)?; + save_personas_for_build(app, &records, include_bestie)?; } - Ok(records) + Ok(visible_personas_for_build(records, include_bestie)) } /// Read the raw persona records at `path` — no built-in merge, no write-back. @@ -409,12 +455,23 @@ pub(crate) fn load_personas_from_path( } pub fn save_personas(app: &AppHandle, records: &[AgentDefinition]) -> Result<(), String> { - let mut sorted = records.to_vec(); - sort_personas(&mut sorted); + save_personas_for_build(app, records, bestie_build_enabled()) +} + +fn save_personas_for_build( + app: &AppHandle, + records: &[AgentDefinition], + include_bestie: bool, +) -> Result<(), String> { + let existing: Vec<_> = crate::managed_agents::storage::load_agent_definitions(app)? + .iter() + .filter_map(|record| record.to_definition_view()) + .collect(); + let complete = definitions_for_save(records, &existing, include_bestie); // Post-fold: persona saves write key-less definition records into the // unified agent store (instances preserved by `save_agent_definitions`). - let definitions: Vec<_> = sorted + let definitions: Vec<_> = complete .into_iter() .map(|persona| persona.into_agent_record()) .collect(); diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index a5ddea5462..77b8047670 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -1,8 +1,8 @@ use super::{ - built_in_persona_records, built_in_persona_records_for_build, ensure_persona_ids_are_active, - ensure_persona_is_active, merge_personas, migrate_retired_personas, - validate_persona_activation_change, validate_persona_deletion, BUILT_IN_PERSONAS, - RETIRED_PERSONAS, + built_in_persona_records, built_in_persona_records_for_build, definitions_for_save, + ensure_persona_ids_are_active, ensure_persona_is_active, merge_personas, + merge_personas_for_build, migrate_retired_personas, validate_persona_activation_change, + validate_persona_deletion, visible_personas_for_build, BUILT_IN_PERSONAS, RETIRED_PERSONAS, }; use crate::managed_agents::discovery::{default_agent_command, effective_agent_command}; use crate::managed_agents::AgentDefinition; @@ -75,6 +75,120 @@ fn bestie_persona_requires_the_internal_build_capability() { assert!(bestie.is_active); } +#[test] +fn ineligible_build_hides_bestie_without_demoting_it() { + let now = "2026-03-19T00:00:00Z"; + let (eligible, _) = merge_personas_for_build(Vec::new(), now, true); + let original = eligible + .iter() + .find(|record| record.id == "builtin:bestie") + .expect("eligible build should seed Bestie") + .clone(); + + let (ineligible, changed) = merge_personas_for_build(eligible, now, false); + let persisted = ineligible + .iter() + .find(|record| record.id == "builtin:bestie") + .expect("ineligible builds must preserve the durable Bestie definition"); + + assert!( + !changed, + "changing build eligibility must not rewrite the store" + ); + assert!(persisted.is_builtin); + assert_eq!( + serde_json::to_value(persisted).unwrap(), + serde_json::to_value(original).unwrap() + ); + assert!(!visible_personas_for_build(ineligible, false) + .iter() + .any(|record| record.id == "builtin:bestie")); +} + +#[test] +fn unified_store_preserves_bestie_and_instance_across_build_transitions() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("managed-agents.json"); + let now = "2026-03-19T00:00:00Z"; + let (eligible, _) = merge_personas_for_build(Vec::new(), now, true); + let original_bestie = eligible + .iter() + .find(|record| record.id == "builtin:bestie") + .expect("eligible build should seed Bestie") + .clone(); + let mut instance = original_bestie.clone().into_agent_record(); + instance.pubkey = "a".repeat(64); + instance.name = "My Bestie".to_string(); + instance.persona_id = Some("builtin:bestie".to_string()); + instance.slug = None; + instance.display_name = None; + instance.relay_url = "wss://buzz.example".to_string(); + + crate::managed_agents::storage::write_agent_store_to_path( + &path, + eligible + .iter() + .cloned() + .map(AgentDefinition::into_agent_record) + .collect(), + vec![instance.clone()], + ) + .expect("write eligible unified store"); + + let raw = crate::managed_agents::storage::load_agent_store_from_path(&path) + .expect("load ineligible unified store"); + let existing: Vec<_> = raw + .iter() + .filter(|record| record.pubkey.is_empty()) + .filter_map(|record| record.to_definition_view()) + .collect(); + let (all_ineligible, changed) = merge_personas_for_build(existing.clone(), now, false); + assert!( + !changed, + "ineligible load must not mutate durable ownership" + ); + let visible = visible_personas_for_build(all_ineligible, false); + assert!(!visible.iter().any(|record| record.id == "builtin:bestie")); + + let definitions = definitions_for_save(&visible, &existing, false) + .into_iter() + .map(AgentDefinition::into_agent_record) + .collect::>(); + crate::managed_agents::storage::save_agent_definitions_to_path(&path, &definitions) + .expect("save from ineligible build"); + + let after_save = crate::managed_agents::storage::load_agent_store_from_path(&path) + .expect("reload unified store"); + let saved_bestie = after_save + .iter() + .find(|record| record.slug.as_deref() == Some("builtin:bestie")) + .and_then(|record| record.to_definition_view()) + .expect("hidden Bestie definition should survive the save"); + assert!(saved_bestie.is_builtin); + assert_eq!( + serde_json::to_value(saved_bestie).unwrap(), + serde_json::to_value(original_bestie).unwrap() + ); + assert_eq!( + after_save + .iter() + .find(|record| record.pubkey == instance.pubkey) + .and_then(|record| record.persona_id.as_deref()), + Some("builtin:bestie"), + "definition saves must preserve keyed instance references" + ); + + let reloaded_definitions = after_save + .iter() + .filter(|record| record.pubkey.is_empty()) + .filter_map(|record| record.to_definition_view()) + .collect(); + let (eligible_again, _) = merge_personas_for_build(reloaded_definitions, now, true); + assert!(visible_personas_for_build(eligible_again, true) + .iter() + .any(|record| record.id == "builtin:bestie" && record.is_builtin)); +} + #[test] fn merge_personas_preserves_custom_records() { let custom = custom_persona("custom:test", "Custom"); diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 652bb9b9ea..848d64f6fc 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -238,12 +238,16 @@ pub(crate) fn spawn_key_refusal(record: &ManagedAgentRecord) -> Option { /// with fail-loud parse handling. Internal seam; public readers filter. fn load_agent_store(app: &AppHandle) -> Result, String> { let path = managed_agents_store_path(app)?; + load_agent_store_from_path(&path) +} + +pub(crate) fn load_agent_store_from_path(path: &Path) -> Result, String> { if !path.exists() { return Ok(Vec::new()); } - let content = fs::read_to_string(&path) - .map_err(|error| format!("failed to read agent store: {error}"))?; + let content = + fs::read_to_string(path).map_err(|error| format!("failed to read agent store: {error}"))?; serde_json::from_str(&content).map_err(|error| { // Fail loudly and preserve the evidence: a later in-app save rewrites // this file wholesale, which would silently destroy a malformed hand @@ -251,7 +255,7 @@ fn load_agent_store(app: &AppHandle) -> Result, String> // reconcile): the broken content survives as `.invalid` for the user // to recover, and the parse error propagates instead of being // swallowed into an empty store. - backup_invalid_store(&path); + backup_invalid_store(path); format!("failed to parse agent store (preserved as .invalid): {error}") }) } @@ -387,11 +391,19 @@ pub(crate) fn save_agent_definitions( app: &AppHandle, definitions: &[ManagedAgentRecord], ) -> Result<(), String> { - let mut instances = load_agent_store(app)?; + let path = managed_agents_store_path(app)?; + save_agent_definitions_to_path(&path, definitions) +} + +pub(crate) fn save_agent_definitions_to_path( + path: &Path, + definitions: &[ManagedAgentRecord], +) -> Result<(), String> { + let mut instances = load_agent_store_from_path(path)?; instances.retain(|record| !record.pubkey.is_empty()); let mut definitions = definitions.to_vec(); definitions.retain(|record| record.pubkey.is_empty()); - write_agent_store(app, definitions, instances) + write_agent_store_to_path(path, definitions, instances) } /// Serialize definitions + instances into the single unified store file. @@ -399,6 +411,15 @@ pub(crate) fn save_agent_definitions( /// name/pubkey order their save path established. fn write_agent_store( app: &AppHandle, + definitions: Vec, + instances: Vec, +) -> Result<(), String> { + let path = managed_agents_store_path(app)?; + write_agent_store_to_path(&path, definitions, instances) +} + +pub(crate) fn write_agent_store_to_path( + path: &Path, mut definitions: Vec, instances: Vec, ) -> Result<(), String> { @@ -406,7 +427,6 @@ fn write_agent_store( let mut all = definitions; all.extend(instances); - let path = managed_agents_store_path(app)?; let payload = serde_json::to_vec_pretty(&all) .map_err(|error| format!("failed to serialize agent store: {error}"))?; @@ -414,7 +434,7 @@ fn write_agent_store( // fallback. Write it owner-only (`0o600`) unconditionally — harmless for the // keyring-backed case (it is the user's own agent store) and closes the // umask window a post-write `chmod` would leave open. - atomic_write_json_restricted(&path, &payload) + atomic_write_json_restricted(path, &payload) } /// Write each record's in-memory key to the keyring and blank the inline copy diff --git a/desktop/src/features/agents/lib/bestie.test.mjs b/desktop/src/features/agents/lib/bestie.test.mjs index 7142d935e3..e6df3672de 100644 --- a/desktop/src/features/agents/lib/bestie.test.mjs +++ b/desktop/src/features/agents/lib/bestie.test.mjs @@ -22,7 +22,7 @@ test("prefers the built-in Bestie over a name fallback", () => { personaId: "builtin:bestie", }); - assert.equal(pickBestieAgent([chief, bestie]), bestie); + assert.equal(pickBestieAgent([chief, bestie], "wss://buzz.example"), bestie); }); test("reuses an existing Chief of Staff within the active relay", () => { @@ -52,5 +52,36 @@ test("prefers a running Bestie instance", () => { status: "running", }); - assert.equal(pickBestieAgent([stopped, running]), running); + assert.equal( + pickBestieAgent([stopped, running], "wss://buzz.example"), + running, + ); +}); + +test("uses backend-compatible relay identity equivalences", () => { + const loopback = agent({ + relayUrl: "WSS://localhost:443/", + personaId: "builtin:bestie", + }); + + assert.equal(pickBestieAgent([loopback], "wss://[::1]"), loopback); +}); + +test("fails closed without a valid inherited relay", () => { + const bestie = agent({ personaId: "builtin:bestie" }); + + assert.equal(pickBestieAgent([bestie], null), null); + assert.equal(pickBestieAgent([bestie], " "), null); + assert.equal(pickBestieAgent([bestie], "not a relay"), null); + assert.equal(pickBestieAgent([bestie], "wss://user@buzz.example"), null); + assert.equal(pickBestieAgent([bestie], "wss://buzz.example/#fragment"), null); +}); + +test("never selects a Bestie from another relay", () => { + const relayA = agent({ + relayUrl: "wss://a.example", + personaId: "builtin:bestie", + }); + + assert.equal(pickBestieAgent([relayA], "wss://b.example"), null); }); diff --git a/desktop/src/features/agents/lib/bestie.ts b/desktop/src/features/agents/lib/bestie.ts index 98f78ddf39..e1aaad98ca 100644 --- a/desktop/src/features/agents/lib/bestie.ts +++ b/desktop/src/features/agents/lib/bestie.ts @@ -1,13 +1,10 @@ import type { ManagedAgent } from "@/shared/api/types"; +import { canonicalRelayUrl } from "../managedAgentRuntimeStatus.ts"; export const BESTIE_PERSONA_ID = "builtin:bestie"; const BESTIE_FALLBACK_NAMES = new Set(["bestie", "chief of staff"]); -function normalizeRelayUrl(relayUrl: string | null | undefined) { - return relayUrl?.trim().replace(/\/+$/, "").toLowerCase() ?? null; -} - function preferredByLifecycle(agents: readonly ManagedAgent[]) { return ( agents.find((agent) => agent.status === "running") ?? @@ -22,11 +19,10 @@ export function pickBestieAgent( agents: readonly ManagedAgent[], relayUrl?: string | null, ) { - const normalizedRelayUrl = normalizeRelayUrl(relayUrl); + const normalizedRelayUrl = canonicalRelayUrl(relayUrl ?? ""); + if (normalizedRelayUrl === null) return null; const scoped = agents.filter( - (agent) => - !normalizedRelayUrl || - normalizeRelayUrl(agent.relayUrl) === normalizedRelayUrl, + (agent) => canonicalRelayUrl(agent.relayUrl) === normalizedRelayUrl, ); const builtIn = scoped.filter( (agent) => agent.personaId === BESTIE_PERSONA_ID, diff --git a/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs b/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs index edd368eccd..137937685c 100644 --- a/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs +++ b/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs @@ -96,6 +96,8 @@ test("canonicalRelayUrl mirrors the backend pair-key normalization", () => { assert.equal(canonicalRelayUrl("ws://[::1]:3000"), "ws://127.0.0.1:3000"); assert.equal(canonicalRelayUrl("https://relay.example"), null); assert.equal(canonicalRelayUrl("not a url"), null); + assert.equal(canonicalRelayUrl("wss://user@relay.example"), null); + assert.equal(canonicalRelayUrl("wss://relay.example/#fragment"), null); }); test("matches a stored community URL against canonical backend rows", () => { diff --git a/desktop/src/features/agents/managedAgentRuntimeStatus.ts b/desktop/src/features/agents/managedAgentRuntimeStatus.ts index c3a952f7d5..c901c25e51 100644 --- a/desktop/src/features/agents/managedAgentRuntimeStatus.ts +++ b/desktop/src/features/agents/managedAgentRuntimeStatus.ts @@ -78,6 +78,10 @@ export function canonicalRelayUrl(raw: string): string | null { return null; } if (url.protocol !== "ws:" && url.protocol !== "wss:") return null; + if (url.username !== "" || url.password !== "" || url.hash !== "") { + return null; + } + if (url.hostname === "") return null; let host = url.hostname.toLowerCase(); if (host === "localhost" || host === "[::1]" || host.startsWith("127.")) { host = "127.0.0.1";