From 3e2b559a7077a4cac94aa9c14c2580d1a891b6c0 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 16:23:18 -0600 Subject: [PATCH 1/8] feat(desktop): absorb persona-only fields into ManagedAgentRecord (Phase 1A slice 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add display_name, name_pool, and is_builtin to ManagedAgentRecord — the three user-facing fields the persona layer carries that the agent record did not already snapshot. All serde-default so existing stores deserialize unchanged; not yet published (the 30177 projection is untouched, per the frozen Phase-1 interface). Part of the unified agent model (#centralize-personas-and-agents): AgentRecord = ManagedAgentRecord + persona-only fields, identity optional. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src-tauri/src/commands/agent_config.rs | 3 +++ desktop/src-tauri/src/commands/agents.rs | 3 +++ .../src-tauri/src/commands/personas/inbound_tests.rs | 3 +++ desktop/src-tauri/src/managed_agents/agent_events.rs | 3 +++ .../src/managed_agents/config_bridge/reader_tests.rs | 3 +++ desktop/src-tauri/src/managed_agents/nest.rs | 3 +++ desktop/src-tauri/src/managed_agents/readiness.rs | 3 +++ desktop/src-tauri/src/managed_agents/runtime/tests.rs | 3 +++ .../src-tauri/src/managed_agents/spawn_hash/tests.rs | 3 +++ desktop/src-tauri/src/managed_agents/team_repair.rs | 3 +++ desktop/src-tauri/src/managed_agents/types.rs | 11 +++++++++++ 11 files changed, 41 insertions(+) diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 0fd2c4c3a6..d1c293c2d2 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -516,6 +516,9 @@ mod tests { last_error: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index c8b5c7bcba..5389511f9c 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -843,6 +843,9 @@ pub async fn create_managed_agent( last_error: None, respond_to: input.respond_to, respond_to_allowlist: respond_to_allowlist.clone(), + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: relay_mesh.clone(), }; diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index aa77b71bf5..a4773e90eb 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -161,6 +161,9 @@ fn local_agent() -> ManagedAgentRecord { last_error: None, respond_to: crate::managed_agents::RespondTo::OwnerOnly, respond_to_allowlist: vec![], + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index fbc7d7b502..860ff1e701 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -176,6 +176,9 @@ mod tests { last_error: Some("some runtime error".to_string()), respond_to: RespondTo::Allowlist, respond_to_allowlist: vec!["79be667e".to_string()], + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index cfb7f29c16..e2a61ffbe4 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -75,6 +75,9 @@ fn test_record() -> ManagedAgentRecord { last_error: None, respond_to: crate::managed_agents::types::RespondTo::OwnerOnly, respond_to_allowlist: vec![], + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index a298704fed..8cc5744b88 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -1147,6 +1147,9 @@ mod tests { respond_to: RespondTo::default(), respond_to_allowlist: vec![], env_vars: std::collections::BTreeMap::new(), + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 241882bb8f..c38f1800c4 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -989,6 +989,9 @@ mod tests { last_error: None, respond_to: Default::default(), respond_to_allowlist: vec![], + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, }; diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index f675ed0a10..77c999bf37 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -160,6 +160,9 @@ fn fixture( last_error: None, respond_to, respond_to_allowlist: allowlist, + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, } } 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 7da1b9654f..1c9b593a4a 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -40,6 +40,9 @@ fn record() -> ManagedAgentRecord { last_error: None, respond_to: Default::default(), respond_to_allowlist: vec![], + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 48d9fdddf4..29cbeddc05 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -309,6 +309,9 @@ mod tests { respond_to: Default::default(), respond_to_allowlist: vec![], env_vars: std::collections::BTreeMap::new(), + display_name: None, + name_pool: Vec::new(), + is_builtin: false, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 3b152c0261..e0ff993daf 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -204,6 +204,17 @@ pub struct ManagedAgentRecord { /// Preserved across mode toggles so users don't lose state. #[serde(default)] pub respond_to_allowlist: Vec, + /// Optional display name distinct from the unique `name` handle. Absorbed + /// from `PersonaRecord.display_name` (unified agent model, Phase 1A). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub display_name: Option, + /// Pool of short thematic names for clones of this agent. Absorbed from + /// `PersonaRecord.name_pool`; feeds clone naming. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub name_pool: Vec, + /// Absorbed from `PersonaRecord.is_builtin`. + #[serde(default)] + pub is_builtin: bool, /// Typed marker for relay-mesh agents. `Some(_)` means this agent runs its /// inference through Buzz's relay-mesh local endpoint; the `model_ref` is /// the served model id to route to. `None` is a normal agent. From 7cb67ae320bcfac22982c67ddbad6ea405b4308a Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 16:35:43 -0600 Subject: [PATCH 2/8] feat(desktop): identity-optional AgentRecord groundwork (Phase 1A slice 2a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add slug (former PersonaRecord.id, preserves 30175 d_tag coordinates for key-less definitions) and is_active (archived-definition flag) to ManagedAgentRecord, and teach hydrate_keys to skip key-less records — definitions mint keys on first start, so there is no keyring entry to look up and no outage to report. desktop-tauri tests: 1020 passed, 0 failed. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src-tauri/src/commands/agent_config.rs | 2 ++ desktop/src-tauri/src/commands/agents.rs | 2 ++ .../src-tauri/src/commands/personas/inbound_tests.rs | 2 ++ desktop/src-tauri/src/managed_agents/agent_events.rs | 2 ++ .../src/managed_agents/config_bridge/reader_tests.rs | 2 ++ desktop/src-tauri/src/managed_agents/nest.rs | 2 ++ desktop/src-tauri/src/managed_agents/readiness.rs | 2 ++ desktop/src-tauri/src/managed_agents/runtime/tests.rs | 2 ++ .../src-tauri/src/managed_agents/spawn_hash/tests.rs | 2 ++ desktop/src-tauri/src/managed_agents/storage.rs | 5 +++++ desktop/src-tauri/src/managed_agents/team_repair.rs | 2 ++ desktop/src-tauri/src/managed_agents/types.rs | 10 ++++++++++ 12 files changed, 35 insertions(+) diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index d1c293c2d2..bfc192aedb 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -517,8 +517,10 @@ mod tests { respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 5389511f9c..b6006d23ac 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -844,8 +844,10 @@ pub async fn create_managed_agent( respond_to: input.respond_to, respond_to_allowlist: respond_to_allowlist.clone(), display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: relay_mesh.clone(), }; diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index a4773e90eb..f332783c13 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -162,8 +162,10 @@ fn local_agent() -> ManagedAgentRecord { respond_to: crate::managed_agents::RespondTo::OwnerOnly, respond_to_allowlist: vec![], display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 860ff1e701..aa03d0d183 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -177,8 +177,10 @@ mod tests { respond_to: RespondTo::Allowlist, respond_to_allowlist: vec!["79be667e".to_string()], display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index e2a61ffbe4..a01449387a 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -76,8 +76,10 @@ fn test_record() -> ManagedAgentRecord { respond_to: crate::managed_agents::types::RespondTo::OwnerOnly, respond_to_allowlist: vec![], display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index 8cc5744b88..86ced1e4c6 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -1148,8 +1148,10 @@ mod tests { respond_to_allowlist: vec![], env_vars: std::collections::BTreeMap::new(), display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c38f1800c4..008cc6a26d 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -990,8 +990,10 @@ mod tests { respond_to: Default::default(), respond_to_allowlist: vec![], display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, }; diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 77c999bf37..8f0b8f6c54 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -161,8 +161,10 @@ fn fixture( respond_to, respond_to_allowlist: allowlist, display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, } } 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 1c9b593a4a..5f51aaf180 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -41,8 +41,10 @@ fn record() -> ManagedAgentRecord { respond_to: Default::default(), respond_to_allowlist: vec![], display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 21cf4673f4..b69d9b986a 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -214,6 +214,11 @@ fn hydrate_keys(records: &mut [ManagedAgentRecord]) { /// key this boot." fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) { for record in records.iter_mut() { + // A key-less definition (no pubkey yet — unified agent model) has no + // keyring entry by construction; keys are minted on first start. + if record.pubkey.is_empty() { + continue; + } if record.private_key_nsec.is_empty() { match store.load(&agent_keyring_name(&record.pubkey)) { Ok(Some(nsec)) => record.private_key_nsec = nsec, diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 29cbeddc05..740d0a85dd 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -310,8 +310,10 @@ mod tests { respond_to_allowlist: vec![], env_vars: std::collections::BTreeMap::new(), display_name: None, + slug: None, name_pool: Vec::new(), is_builtin: false, + is_active: true, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index e0ff993daf..9a493f6e30 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -208,6 +208,12 @@ pub struct ManagedAgentRecord { /// from `PersonaRecord.display_name` (unified agent model, Phase 1A). #[serde(default, skip_serializing_if = "Option::is_none")] pub display_name: Option, + /// Stable definition slug — the former `PersonaRecord.id`. Key-less + /// records (definitions not yet instantiated) publish kind:30175 at + /// `d_tag = slug`, preserving the pre-merge event coordinates. `None` for + /// agents created directly (never persona-backed). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub slug: Option, /// Pool of short thematic names for clones of this agent. Absorbed from /// `PersonaRecord.name_pool`; feeds clone naming. #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -215,6 +221,10 @@ pub struct ManagedAgentRecord { /// Absorbed from `PersonaRecord.is_builtin`. #[serde(default)] pub is_builtin: bool, + /// Absorbed from `PersonaRecord.is_active` — `false` means an archived + /// definition hidden from pickers. Defaults `true` for existing records. + #[serde(default = "default_record_active")] + pub is_active: bool, /// Typed marker for relay-mesh agents. `Some(_)` means this agent runs its /// inference through Buzz's relay-mesh local endpoint; the `model_ref` is /// the served model id to route to. `None` is a normal agent. From 19d83475e74a72b362f2a06408e01717474c546d Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 16:47:08 -0600 Subject: [PATCH 3/8] feat(desktop): record-first harness resolution (Phase 1A slice 2b) Add record.runtime (absorbed from PersonaRecord.runtime) and record_agent_command(): override > record.runtime > legacy persona lookup > default. Convert all record-based effective_agent_command call sites (spawn, restore, summary, spawn_hash, readiness, model discovery, config surface, profile reconcile) to the record-first path. Records without a materialized runtime resolve exactly as before via the persona fallback, so pre-migration stores see zero behavior change. effective_agent_command remains for persona-scoped callers (persona dialogs, divergence checks) until the store migration lands. desktop-tauri tests: 1024 passed (4 new record_agent_command tests). Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../src-tauri/src/commands/agent_config.rs | 13 +- .../src-tauri/src/commands/agent_models.rs | 12 +- desktop/src-tauri/src/commands/agents.rs | 8 +- .../src/commands/personas/inbound_tests.rs | 1 + .../src/managed_agents/agent_events.rs | 1 + .../config_bridge/reader_tests.rs | 1 + .../src-tauri/src/managed_agents/discovery.rs | 124 +++++++++++++++++- desktop/src-tauri/src/managed_agents/nest.rs | 1 + .../src-tauri/src/managed_agents/readiness.rs | 8 +- .../src-tauri/src/managed_agents/restore.rs | 7 +- .../src-tauri/src/managed_agents/runtime.rs | 12 +- .../src/managed_agents/runtime/tests.rs | 1 + .../src/managed_agents/spawn_hash.rs | 8 +- .../src/managed_agents/spawn_hash/tests.rs | 1 + .../src/managed_agents/team_repair.rs | 1 + desktop/src-tauri/src/managed_agents/types.rs | 6 + 16 files changed, 153 insertions(+), 52 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index bfc192aedb..d3ad6a8e27 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -12,9 +12,9 @@ use crate::{ NormalizedField, RuntimeConfigSurface, SessionConfigCache, }, }, - current_instance_id, effective_agent_command, known_acp_runtime, load_managed_agents, - load_personas, resolve_effective_prompt_model_provider, save_managed_agents, - sync_managed_agent_processes, KnownAcpRuntime, ManagedAgentRecord, PersonaRecord, + current_instance_id, known_acp_runtime, load_managed_agents, load_personas, + resolve_effective_prompt_model_provider, save_managed_agents, sync_managed_agent_processes, + KnownAcpRuntime, ManagedAgentRecord, PersonaRecord, }, }; @@ -242,11 +242,7 @@ pub async fn get_agent_config_surface( }; let personas = load_personas(&app).unwrap_or_default(); - let effective_cmd = effective_agent_command( - record.persona_id.as_deref(), - &personas, - record.agent_command_override.as_deref(), - ); + let effective_cmd = crate::managed_agents::record_agent_command(&record, &personas); let runtime_meta = known_acp_runtime(&effective_cmd); let session_cache = state.get_session_cache(&pubkey); @@ -518,6 +514,7 @@ mod tests { respond_to_allowlist: vec![], display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index f54a1dc114..ae10d92b13 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -62,11 +62,7 @@ pub async fn get_agent_models( // so model discovery runs against the persona's current harness, not the // frozen record snapshot. An explicit per-agent override wins. let personas = load_personas(&app).unwrap_or_default(); - let effective_command = crate::managed_agents::effective_agent_command( - record.persona_id.as_deref(), - &personas, - record.agent_command_override.as_deref(), - ); + let effective_command = crate::managed_agents::record_agent_command(record, &personas); let args = normalize_agent_args(&effective_command, record.agent_args.clone()); @@ -936,11 +932,7 @@ pub async fn update_managed_agent( // not the frozen snapshot, so an inherited harness picks the right // default avatar. let personas = load_personas(&app).unwrap_or_default(); - let effective_command = crate::managed_agents::effective_agent_command( - record.persona_id.as_deref(), - &personas, - record.agent_command_override.as_deref(), - ); + let effective_command = crate::managed_agents::record_agent_command(record, &personas); let avatar_url = record .avatar_url .clone() diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index b6006d23ac..029995de91 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -845,6 +845,7 @@ pub async fn create_managed_agent( respond_to_allowlist: respond_to_allowlist.clone(), display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, @@ -1047,11 +1048,8 @@ pub async fn start_managed_agent( // profile reconcile (the create-time snapshot may be empty or stale for // a persona-inherited harness). let reconcile_personas = load_personas(&app).unwrap_or_default(); - let reconcile_effective_command = crate::managed_agents::effective_agent_command( - record.persona_id.as_deref(), - &reconcile_personas, - record.agent_command_override.as_deref(), - ); + let reconcile_effective_command = + crate::managed_agents::record_agent_command(record, &reconcile_personas); let reconcile = ProfileReconcileData { private_key_nsec: record.private_key_nsec.clone(), diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index f332783c13..9889e14344 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -163,6 +163,7 @@ fn local_agent() -> ManagedAgentRecord { respond_to_allowlist: vec![], display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index aa03d0d183..ef38196339 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -178,6 +178,7 @@ mod tests { respond_to_allowlist: vec!["79be667e".to_string()], display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index a01449387a..7cd2fe750a 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -77,6 +77,7 @@ fn test_record() -> ManagedAgentRecord { respond_to_allowlist: vec![], display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 8ace99ee60..7a5cdcef9e 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -285,6 +285,40 @@ pub fn default_agent_command() -> String { .to_string() } +/// Record-first harness resolution (unified agent model, Phase 1A). +/// +/// Resolution order: +/// 1. explicit override (non-empty) — a deliberate per-instance pin; +/// 2. the record's own `runtime` id mapped to its primary command — +/// records materialize their runtime at create/migration time; +/// 3. legacy fallback: the linked persona's `runtime` (records created +/// before the unified model carry `persona_id` but no `runtime`); +/// 4. `default_agent_command()`. +pub fn record_agent_command( + record: &crate::managed_agents::types::ManagedAgentRecord, + personas: &[crate::managed_agents::types::PersonaRecord], +) -> String { + if let Some(pin) = record + .agent_command_override + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + { + return pin.to_string(); + } + + if let Some(command) = record + .runtime + .as_deref() + .and_then(known_acp_runtime_exact) + .and_then(|r| r.commands.first().copied()) + { + return command.to_string(); + } + + effective_agent_command(record.persona_id.as_deref(), personas, None) +} + /// Resolve the agent command (harness) for a spawn/deploy/summary. The linked /// persona wins so persona harness edits propagate on the next spawn. An /// explicit per-instance override (`agent_command_override`) takes precedence. @@ -770,8 +804,9 @@ mod tests { use super::{ classify_runtime, create_time_agent_command_override, default_agent_command, divergent_agent_command_override, effective_agent_command, find_via_login_shell, - managed_agent_avatar_url, normalize_agent_args, update_time_agent_command_override, - BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, + managed_agent_avatar_url, normalize_agent_args, record_agent_command, + update_time_agent_command_override, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, + CODEX_AVATAR_URL, GOOSE_AVATAR_URL, }; use crate::managed_agents::AcpAvailabilityStatus; @@ -986,6 +1021,91 @@ mod tests { ); } + /// Minimal record for `record_agent_command` tests. Only the resolution + /// inputs (runtime / persona_id / agent_command_override) vary. + fn record_with( + runtime: Option<&str>, + persona_id: Option<&str>, + override_cmd: Option<&str>, + ) -> crate::managed_agents::types::ManagedAgentRecord { + crate::managed_agents::types::ManagedAgentRecord { + pubkey: String::new(), + name: "r".to_string(), + persona_id: persona_id.map(str::to_string), + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: String::new(), + agent_command: String::new(), + agent_command_override: override_cmd.map(str::to_string), + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 0, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + mcp_toolsets: None, + start_on_app_launch: false, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + env_vars: std::collections::BTreeMap::new(), + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: runtime.map(str::to_string), + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + relay_mesh: None, + } + } + + #[test] + fn record_agent_command_own_runtime_wins_over_persona() { + // A record with its own materialized runtime never consults the + // persona list — the unified-model resolution. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + let record = record_with(Some("claude"), Some("p1"), None); + assert_eq!(record_agent_command(&record, &personas), "claude-agent-acp"); + } + + #[test] + fn record_agent_command_override_beats_runtime() { + let record = record_with(Some("claude"), None, Some("codex-acp")); + assert_eq!(record_agent_command(&record, &[]), "codex-acp"); + } + + #[test] + fn record_agent_command_legacy_persona_fallback() { + // Pre-migration record: persona_id set, no runtime — resolves through + // the legacy persona path unchanged. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + let record = record_with(None, Some("p1"), None); + assert_eq!(record_agent_command(&record, &personas), "goose"); + } + + #[test] + fn record_agent_command_bare_record_defaults() { + let record = record_with(None, None, None); + assert_eq!(record_agent_command(&record, &[]), default_agent_command()); + } + #[test] fn effective_agent_command_inherits_persona_runtime() { // No override → persona runtime id maps to its primary command. diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index 86ced1e4c6..af2aa047ad 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -1149,6 +1149,7 @@ mod tests { env_vars: std::collections::BTreeMap::new(), display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 008cc6a26d..3addae2d6b 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -45,7 +45,6 @@ use crate::managed_agents::{ agent_env::baked_build_env, config_bridge::read_goose_file_config, discovery::{known_acp_runtime, resolve_command, KnownAcpRuntime}, - effective_agent_command, env_vars::merged_user_env, types::{ManagedAgentRecord, PersonaRecord}, }; @@ -86,11 +85,7 @@ pub(crate) fn resolve_effective_agent_env( personas: &[PersonaRecord], runtime: Option<&KnownAcpRuntime>, ) -> EffectiveAgentEnv { - let effective_command = effective_agent_command( - record.persona_id.as_deref(), - personas, - record.agent_command_override.as_deref(), - ); + let effective_command = crate::managed_agents::record_agent_command(record, personas); // Layer 1: baked build defaults (floor — internal builds only; OSS = empty). let mut env = baked_build_env(); @@ -991,6 +986,7 @@ mod tests { respond_to_allowlist: vec![], display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 78122680d6..6030cb6d5d 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -333,11 +333,8 @@ pub async fn restore_managed_agents_on_launch( // Resolve the effective harness for the avatar-fallback // derivation (the snapshot may be empty/stale for an inherited // harness). Mirrors the UI start path. - let effective_command = crate::managed_agents::effective_agent_command( - record.persona_id.as_deref(), - &reconcile_personas, - record.agent_command_override.as_deref(), - ); + let effective_command = + crate::managed_agents::record_agent_command(record, &reconcile_personas); Some(( pubkey.clone(), crate::commands::ProfileReconcileData { diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0e12066961..841128b708 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1369,11 +1369,7 @@ pub fn build_managed_agent_summary( // Resolve the effective harness the same way, then derive args/mcp from it, // so the UI reflects the persona's current harness (or an explicit pin). - let effective_command = crate::managed_agents::effective_agent_command( - record.persona_id.as_deref(), - personas, - record.agent_command_override.as_deref(), - ); + let effective_command = crate::managed_agents::record_agent_command(record, personas); let effective_args = normalize_agent_args(&effective_command, record.agent_args.clone()); let effective_mcp_command = known_acp_runtime(&effective_command) .and_then(|r| r.mcp_command) @@ -1522,11 +1518,7 @@ pub fn spawn_agent_child( // command, so we recompute them from the effective value rather than the // frozen record snapshot. Mirrors the model resolution below. let personas = super::load_personas(app).unwrap_or_default(); - let effective_command = super::effective_agent_command( - record.persona_id.as_deref(), - &personas, - record.agent_command_override.as_deref(), - ); + let effective_command = super::record_agent_command(record, &personas); let agent_args = normalize_agent_args(&effective_command, record.agent_args.clone()); let resolved_acp_command = resolve_command(&record.acp_command) .ok_or_else(|| missing_command_message(&record.acp_command, "ACP harness command"))?; diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 8f0b8f6c54..e2de35c5b4 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -162,6 +162,7 @@ fn fixture( respond_to_allowlist: allowlist, display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 087a87bc3f..527275a12b 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -27,7 +27,7 @@ use std::hash::{DefaultHasher, Hash, Hasher}; use super::{ - effective_agent_command, known_acp_runtime, normalize_agent_args, + known_acp_runtime, normalize_agent_args, persona_events::persona_snapshot_with_agent_config_fallback, resolve_effective_agent_env, types::{ManagedAgentRecord, PersonaRecord}, @@ -64,11 +64,7 @@ pub(crate) fn spawn_config_hash( } let record = &record; - let effective_command = effective_agent_command( - record.persona_id.as_deref(), - personas, - record.agent_command_override.as_deref(), - ); + let effective_command = crate::managed_agents::record_agent_command(record, personas); let runtime_meta = known_acp_runtime(&effective_command); let effective = resolve_effective_agent_env(record, personas, runtime_meta); 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 5f51aaf180..425e3c695b 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -42,6 +42,7 @@ fn record() -> ManagedAgentRecord { respond_to_allowlist: vec![], display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 740d0a85dd..9e6847e3b1 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -311,6 +311,7 @@ mod tests { env_vars: std::collections::BTreeMap::new(), display_name: None, slug: None, + runtime: None, name_pool: Vec::new(), is_builtin: false, is_active: true, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 9a493f6e30..12ef2d5c50 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -214,6 +214,12 @@ pub struct ManagedAgentRecord { /// agents created directly (never persona-backed). #[serde(default, skip_serializing_if = "Option::is_none")] pub slug: Option, + /// Absorbed from `PersonaRecord.runtime` — the preferred ACP runtime ID + /// (e.g. 'goose', 'claude'). Record-first command resolution reads this + /// before falling back to legacy persona lookup; populated by the store + /// migration and at create time. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub runtime: Option, /// Pool of short thematic names for clones of this agent. Absorbed from /// `PersonaRecord.name_pool`; feeds clone naming. #[serde(default, skip_serializing_if = "Vec::is_empty")] From 288ec693ed177e2a06f7b083004fb237a1917e83 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 16:59:50 -0600 Subject: [PATCH 4/8] feat(desktop): materialize agent runtimes at boot (Phase 1A slice 3a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add materialize_agent_runtimes to run_boot_migrations: copies each persona-linked record's runtime onto the record itself (idempotent, never overwrites an existing runtime, skips unlinked/unknown personas). After this runs, record_agent_command resolves from the record's own runtime — the persona list is no longer consulted on the spawn path. Includes Pinky's cutover acceptance test: spawn_config_hash is IDENTICAL before vs after materialization for a persona-linked record, so no running agent shows a spurious restart badge at migration. desktop-tauri tests: 1028 passed. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../src/managed_agents/spawn_hash/tests.rs | 22 ++++++ desktop/src-tauri/src/migration.rs | 53 +++++++++++++++ desktop/src-tauri/src/migration_tests.rs | 68 +++++++++++++++++++ 3 files changed, 143 insertions(+) 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 425e3c695b..5e91862f77 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -79,6 +79,28 @@ fn hash_is_deterministic() { ); } +#[test] +fn materializing_runtime_keeps_hash_stable() { + // Migration cutover invariant (Phase 1A): materializing the linked + // persona's runtime onto the record must NOT change the spawn hash — + // otherwise every running persona-linked agent would show a spurious + // restart badge right after migration. Pre-migration the command resolves + // through the persona fallback; post-migration through record.runtime. + // Same persona, same runtime, same command → same hash. + let personas = vec![persona("p1", Some("goose"), "Persona prompt.")]; + + let mut pre = record(); + pre.persona_id = Some("p1".into()); + + let mut post = pre.clone(); + post.runtime = Some("goose".into()); + + assert_eq!( + spawn_config_hash(&pre, &personas, "wss://ws.example"), + spawn_config_hash(&post, &personas, "wss://ws.example") + ); +} + #[test] fn record_env_var_edit_changes_hash() { let rec = record(); diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 32fd09acab..0cc678ce06 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -153,6 +153,7 @@ pub fn run_boot_migrations(app: &tauri::AppHandle) { eprintln!("buzz-desktop: sync-team-personas: {e}"); } reconcile_provider_mcp_commands(app); + materialize_agent_runtimes(app); } /// Copy one-time app state from the legacy app identifier directory to @@ -1260,6 +1261,58 @@ fn rename_provider_to_runtime_in_personas(path: &Path) { }); } +/// Materialize each persona-linked agent record's `runtime` from its linked +/// persona (unified agent model, Phase 1A). After this, spawn resolution reads +/// the record's own runtime (`record_agent_command` step 2) instead of the +/// live persona — same effective command by construction, so the spawn-config +/// hash is unchanged and no running agent shows a spurious restart badge (see +/// `spawn_hash::tests::materializing_runtime_keeps_hash_stable`). +/// +/// Idempotent: records that already carry `runtime` are untouched, as are +/// records with no linked persona or a persona without a runtime (both keep +/// resolving through the legacy fallback path unchanged). +pub fn materialize_agent_runtimes(app: &tauri::AppHandle) { + let Ok(current_dir) = app.path().app_data_dir() else { + return; + }; + let mut dirs = vec![current_dir.clone()]; + if let Some(canonical) = canonical_dev_data_dir(¤t_dir) { + if canonical.exists() && canonical != current_dir { + dirs.push(canonical); + } + } + for dir in dirs { + let path = dir.join("agents/managed-agents.json"); + if path.exists() { + materialize_runtimes_in_file(&path); + } + } +} + +fn materialize_runtimes_in_file(path: &Path) { + let persona_runtimes = load_persona_runtimes(path); + if persona_runtimes.is_empty() { + return; + } + patch_json_records(path, |obj| { + if obj.contains_key("runtime") { + return false; + } + let Some(runtime) = obj + .get("persona_id") + .and_then(|v| v.as_str()) + .and_then(|pid| persona_runtimes.get(pid)) + else { + return false; + }; + obj.insert( + "runtime".to_string(), + serde_json::Value::String(runtime.clone()), + ); + true + }); +} + pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { let Ok(dir) = app.path().app_data_dir() else { return; diff --git a/desktop/src-tauri/src/migration_tests.rs b/desktop/src-tauri/src/migration_tests.rs index 66ee8dcc7a..a8fba9d73b 100644 --- a/desktop/src-tauri/src/migration_tests.rs +++ b/desktop/src-tauri/src/migration_tests.rs @@ -759,6 +759,74 @@ fn rename_provider_to_runtime_preserves_existing_runtime_over_provider() { assert_eq!(records[0]["provider"], "old-value"); } +#[test] +fn materialize_runtimes_copies_persona_runtime_onto_record() { + let dir = tempfile::tempdir().unwrap(); + write_personas_json( + dir.path(), + &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([{ "name": "Fizz", "persona_id": "persona-1" }]), + ); + materialize_runtimes_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["runtime"], "goose"); +} + +#[test] +fn materialize_runtimes_is_idempotent_and_preserves_existing() { + // A record that already carries runtime (even one diverging from its + // persona) is never overwritten, and the second run rewrites nothing. + let dir = tempfile::tempdir().unwrap(); + write_personas_json( + dir.path(), + &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + { "name": "Fizz", "persona_id": "persona-1", "runtime": "claude" }, + { "name": "Buzz", "persona_id": "persona-1" } + ]), + ); + let agents_path = dir.path().join("agents/managed-agents.json"); + materialize_runtimes_in_file(&agents_path); + let records = read_agents_json(dir.path()); + assert_eq!( + records[0]["runtime"], "claude", + "existing runtime preserved" + ); + assert_eq!(records[1]["runtime"], "goose"); + + let before = std::fs::read_to_string(&agents_path).unwrap(); + materialize_runtimes_in_file(&agents_path); + let after = std::fs::read_to_string(&agents_path).unwrap(); + assert_eq!(before, after, "second run must be a no-op"); +} + +#[test] +fn materialize_runtimes_skips_unlinked_and_unknown_personas() { + let dir = tempfile::tempdir().unwrap(); + write_personas_json( + dir.path(), + &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + { "name": "NoPersona" }, + { "name": "GonePersona", "persona_id": "deleted" } + ]), + ); + let agents_path = dir.path().join("agents/managed-agents.json"); + let before = std::fs::read_to_string(&agents_path).unwrap(); + materialize_runtimes_in_file(&agents_path); + let after = std::fs::read_to_string(&agents_path).unwrap(); + assert_eq!(before, after, "no linked runtime → untouched file"); +} + #[test] fn reconcile_mcp_commands_clears_stale_buzz_mcp_server() { let dir = tempfile::tempdir().unwrap(); From 90b9abfc41fe61b6f0dbbe73cc5a9070c0c2e685 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 17:03:35 -0600 Subject: [PATCH 5/8] test(desktop): pin unified-model fields out of the 30177 wire projection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the opt-IN exclusion test with real (non-empty) values for the Phase 1A record fields (display_name, slug, runtime, name_pool, is_builtin, is_active) and assert none reach the published projection — publishing them is a Phase 2 relay-canonicalization decision, and this makes the frozen-interface contract with the boot reconcile executable. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../src/managed_agents/agent_events.rs | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index ef38196339..f9ead88013 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -176,12 +176,14 @@ mod tests { last_error: Some("some runtime error".to_string()), respond_to: RespondTo::Allowlist, respond_to_allowlist: vec!["79be667e".to_string()], - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, + // Unified-model fields carry real values so the exclusion test + // proves they are absent from the wire, not vacuously empty. + display_name: Some("Display Name Secretish".to_string()), + slug: Some("sample-slug".to_string()), + runtime: Some("goose".to_string()), + name_pool: vec!["poolname".to_string()], + is_builtin: true, + is_active: false, relay_mesh: None, } } @@ -243,6 +245,16 @@ mod tests { assert!(!json.contains("last_exit_code")); assert!(!json.contains("last_error")); assert!(!json.contains("some runtime error")); + + // Unified-agent-model fields (Phase 1A) — deliberately NOT published + // yet; adding them to the wire projection is a Phase 2 (relay + // canonicalization) decision, not a record-shape side effect. + assert!(!json.contains("display_name"), "leaked display_name"); + assert!(!json.contains("\"slug\""), "leaked slug"); + assert!(!json.contains("\"runtime\""), "leaked runtime"); + assert!(!json.contains("name_pool"), "leaked name_pool"); + assert!(!json.contains("is_builtin"), "leaked is_builtin"); + assert!(!json.contains("is_active"), "leaked is_active"); assert!(!json.contains("backend_agent_id")); assert!(!json.contains("provider_binary_path")); assert!(!json.contains("relay_url")); From fd8b20e17e58cc0bdcc286470598c07375eb354d Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 18:00:49 -0600 Subject: [PATCH 6/8] feat(desktop): absorb team-lineage fields onto AgentRecord (Phase 1A slice 3b-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit source_team + source_team_persona_slug move onto ManagedAgentRecord — the definition-side team lineage the persona layer carried. With these, the record is a strict superset of PersonaRecord's user-facing fields and the store fold no longer loses information. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src-tauri/src/commands/agent_config.rs | 2 ++ desktop/src-tauri/src/commands/agents.rs | 2 ++ .../src-tauri/src/commands/personas/inbound_tests.rs | 2 ++ desktop/src-tauri/src/managed_agents/agent_events.rs | 2 ++ .../src/managed_agents/config_bridge/reader_tests.rs | 2 ++ desktop/src-tauri/src/managed_agents/discovery.rs | 2 ++ desktop/src-tauri/src/managed_agents/nest.rs | 2 ++ desktop/src-tauri/src/managed_agents/readiness.rs | 2 ++ desktop/src-tauri/src/managed_agents/runtime/tests.rs | 2 ++ .../src-tauri/src/managed_agents/spawn_hash/tests.rs | 2 ++ desktop/src-tauri/src/managed_agents/team_repair.rs | 2 ++ desktop/src-tauri/src/managed_agents/types.rs | 10 ++++++++++ 12 files changed, 32 insertions(+) diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index d3ad6a8e27..7836de7959 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -518,6 +518,8 @@ mod tests { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 029995de91..0fc7a16e93 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -849,6 +849,8 @@ pub async fn create_managed_agent( name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: relay_mesh.clone(), }; diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index 9889e14344..b1397cff2d 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -167,6 +167,8 @@ fn local_agent() -> ManagedAgentRecord { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index f9ead88013..cba7b599a1 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -184,6 +184,8 @@ mod tests { name_pool: vec!["poolname".to_string()], is_builtin: true, is_active: false, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 7cd2fe750a..290b2e60c5 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -81,6 +81,8 @@ fn test_record() -> ManagedAgentRecord { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 7a5cdcef9e..7545fe95e7 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -1072,6 +1072,8 @@ mod tests { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index af2aa047ad..47ceccd004 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -1153,6 +1153,8 @@ mod tests { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 3addae2d6b..8d5f27c56f 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -990,6 +990,8 @@ mod tests { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, }; diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index e2de35c5b4..343b091e94 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -166,6 +166,8 @@ fn fixture( name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } 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 5e91862f77..12aade9a3b 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -46,6 +46,8 @@ fn record() -> ManagedAgentRecord { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 9e6847e3b1..a609a5d45b 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -315,6 +315,8 @@ mod tests { name_pool: Vec::new(), is_builtin: false, is_active: true, + source_team: None, + source_team_persona_slug: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 12ef2d5c50..38a4d7466f 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -231,6 +231,16 @@ pub struct ManagedAgentRecord { /// definition hidden from pickers. Defaults `true` for existing records. #[serde(default = "default_record_active")] pub is_active: bool, + /// Absorbed from `PersonaRecord.source_team` — team ID when this + /// definition was imported from a team directory (team definitions are + /// non-editable). Distinct from `persona_team_dir`/`persona_name_in_team`, + /// which are the instance-side spawn plumbing. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source_team: Option, + /// Absorbed from `PersonaRecord.source_team_persona_slug` — the + /// definition's slug within its source team. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source_team_persona_slug: Option, /// Typed marker for relay-mesh agents. `Some(_)` means this agent runs its /// inference through Buzz's relay-mesh local endpoint; the `model_ref` is /// the served model id to route to. `None` is a normal agent. From cd58fc01035cb446a19a2de4a875e926d2eaca22 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 18:08:14 -0600 Subject: [PATCH 7/8] feat(desktop): persona <-> agent-record fold mapping (Phase 1A slice 3b-2) PersonaRecord::into_agent_record projects a persona onto a key-less unified record (id -> slug, preserving the 30175 d_tag coordinate; identity empty until first start mints keys). The inverse to_persona_view presents slugged definitions in the legacy shape for the persona command surface until Phase 1B unifies the UI. Round-trip is field-exact (test-pinned via serde_json::Value equality). Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src-tauri/src/managed_agents/types.rs | 91 +++++++++++++++++++ .../src/managed_agents/types/tests.rs | 67 ++++++++++++++ 2 files changed, 158 insertions(+) diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 38a4d7466f..8eb905240c 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -67,6 +67,97 @@ pub struct PersonaRecord { pub updated_at: String, } +impl PersonaRecord { + /// Project this persona onto a key-less unified [`ManagedAgentRecord`] + /// (Phase 1A store fold). Identity fields stay empty — keys are minted on + /// first start. `PersonaRecord.id` becomes `slug`, preserving the 30175 + /// event coordinate (`d_tag = slug`) across the fold. + // Wired in by the stage-3 fold migration (same PR); allow until then. + #[allow(dead_code)] + pub fn into_agent_record(self) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: String::new(), + name: self.display_name.clone(), + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: self.avatar_url, + acp_command: DEFAULT_ACP_COMMAND.to_string(), + agent_command: String::new(), + agent_command_override: None, + agent_args: Vec::new(), + mcp_command: String::new(), + turn_timeout_seconds: DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: default_agent_parallelism(), + system_prompt: (!self.system_prompt.is_empty()).then_some(self.system_prompt), + model: self.model, + provider: self.provider, + persona_source_version: None, + mcp_toolsets: None, + env_vars: self.env_vars, + start_on_app_launch: false, + runtime_pid: None, + backend: BackendKind::default(), + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: self.created_at, + updated_at: self.updated_at, + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: RespondTo::default(), + respond_to_allowlist: Vec::new(), + display_name: Some(self.display_name), + slug: Some(self.id), + runtime: self.runtime, + name_pool: self.name_pool, + is_builtin: self.is_builtin, + is_active: self.is_active, + source_team: self.source_team, + source_team_persona_slug: self.source_team_persona_slug, + relay_mesh: None, + } + } +} + +impl ManagedAgentRecord { + /// Present a key-less definition record back in the legacy + /// [`PersonaRecord`] shape — the compatibility view the persona command + /// surface serves until Phase 1B unifies the UI. Inverse of + /// [`PersonaRecord::into_agent_record`] for the fields personas carry. + // Wired in by the stage-3 fold shims (same PR); allow until then. + #[allow(dead_code)] + pub fn to_persona_view(&self) -> Option { + let slug = self.slug.clone()?; + Some(PersonaRecord { + id: slug, + display_name: self + .display_name + .clone() + .unwrap_or_else(|| self.name.clone()), + avatar_url: self.avatar_url.clone(), + system_prompt: self.system_prompt.clone().unwrap_or_default(), + runtime: self.runtime.clone(), + model: self.model.clone(), + provider: self.provider.clone(), + name_pool: self.name_pool.clone(), + is_builtin: self.is_builtin, + is_active: self.is_active, + source_team: self.source_team.clone(), + source_team_persona_slug: self.source_team_persona_slug.clone(), + env_vars: self.env_vars.clone(), + created_at: self.created_at.clone(), + updated_at: self.updated_at.clone(), + }) + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct RelayAgentInfo { pub pubkey: String, diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 37404fb593..752a7a04df 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -467,3 +467,70 @@ fn sample_agent_record() -> ManagedAgentRecord { ) .expect("sample record") } + +// ── PersonaRecord ↔ AgentRecord fold mapping (Phase 1A) ───────────────────── + +fn sample_persona() -> PersonaRecord { + PersonaRecord { + id: "custom:helper".to_string(), + display_name: "Helper".to_string(), + avatar_url: Some("https://example.com/a.png".to_string()), + system_prompt: "You help.".to_string(), + runtime: Some("goose".to_string()), + model: Some("gpt-x".to_string()), + provider: Some("openai".to_string()), + name_pool: vec!["Nimble".to_string()], + is_builtin: false, + is_active: true, + source_team: Some("team-1".to_string()), + source_team_persona_slug: Some("helper".to_string()), + env_vars: [("K".to_string(), "v".to_string())].into_iter().collect(), + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-02T00:00:00Z".to_string(), + } +} + +#[test] +fn persona_into_agent_record_is_keyless_and_slugged() { + let record = sample_persona().into_agent_record(); + assert!(record.pubkey.is_empty(), "fold must not mint identity"); + assert!(record.private_key_nsec.is_empty()); + assert_eq!(record.slug.as_deref(), Some("custom:helper")); + assert_eq!(record.display_name.as_deref(), Some("Helper")); + assert_eq!(record.system_prompt.as_deref(), Some("You help.")); + assert_eq!(record.runtime.as_deref(), Some("goose")); + assert_eq!(record.source_team.as_deref(), Some("team-1")); + assert_eq!(record.env_vars.get("K").map(String::as_str), Some("v")); +} + +#[test] +fn persona_view_round_trips_through_agent_record() { + let persona = sample_persona(); + let view = persona + .clone() + .into_agent_record() + .to_persona_view() + .expect("slugged record must present a persona view"); + assert_eq!( + serde_json::to_value(&view).unwrap(), + serde_json::to_value(&persona).unwrap(), + "fold + view must round-trip every persona field" + ); +} + +#[test] +fn keyed_record_without_slug_has_no_persona_view() { + let mut record = sample_persona().into_agent_record(); + record.slug = None; + assert!( + record.to_persona_view().is_none(), + "instances (no slug) are not definitions" + ); +} + +#[test] +fn empty_prompt_folds_to_none() { + let mut persona = sample_persona(); + persona.system_prompt = String::new(); + assert_eq!(persona.into_agent_record().system_prompt, None); +} From f39cca99a641a5e7994e7080df10695de4e43c3f Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 7 Jul 2026 20:58:08 -0600 Subject: [PATCH 8/8] refactor(desktop): split oversized files to clear the size ratchet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file-size guard flagged five files at their limits. Real splits, not limit bumps: - discovery.rs / nest.rs: inline test modules move to discovery/tests.rs and nest/tests.rs (the repo's established pattern) — both files drop under the 1000-line default (802 / 679). - migration.rs: materialize_agent_runtimes + its tests move to a migration/materialize.rs child module (reuses the parent's private JSON-patch helpers). - commands/agents.rs: the kind:0 profile-reconcile surface (ProfileReconcileData, reconcile_agent_profile, legacy-avatar backfill, needs-sync predicate) moves to agents_profile.rs. All five ratchet entries lowered to the new sizes so the freed headroom cannot silently refill. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/scripts/check-file-sizes.mjs | 18 +- desktop/src-tauri/src/commands/agents.rs | 165 +--- .../src-tauri/src/commands/agents_profile.rs | 170 ++++ .../src-tauri/src/managed_agents/discovery.rs | 581 +---------- .../src/managed_agents/discovery/tests.rs | 554 +++++++++++ desktop/src-tauri/src/managed_agents/nest.rs | 905 +----------------- .../src/managed_agents/nest/tests.rs | 901 +++++++++++++++++ desktop/src-tauri/src/migration.rs | 55 +- .../src-tauri/src/migration/materialize.rs | 140 +++ desktop/src-tauri/src/migration_tests.rs | 68 -- 10 files changed, 1790 insertions(+), 1767 deletions(-) create mode 100644 desktop/src-tauri/src/commands/agents_profile.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/tests.rs create mode 100644 desktop/src-tauri/src/managed_agents/nest/tests.rs create mode 100644 desktop/src-tauri/src/migration/materialize.rs diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index cb69f5e506..d03e67e2b1 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -70,7 +70,9 @@ const overrides = new Map([ // across the local-archive + agent-metric-archive PR series. store_tests.rs // (~731 lines) is under 1000 so needs no override. ["src-tauri/src/archive/mod_tests.rs", 1208], - ["src-tauri/src/commands/agents.rs", 1443], + // unified-agent-model 1A.1: profile reconcile split to agents_profile.rs, + // ratcheting 1443 -> 1295. Queued to split further in the A2 fold. + ["src-tauri/src/commands/agents.rs", 1295], // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS // const + build_thread_replies_filter helper, mirroring the channel sibling so // the two p-gate filters can't drift) plus two guard unit tests. The file was @@ -90,7 +92,9 @@ const overrides = new Map([ // queued to split with the rest of this list. // +4 lines: adopt shared create_symlink wrapper (behavior-preserving refactor // for multi-line rustfmt expansion of the skills symlink call site). - ["src-tauri/src/managed_agents/nest.rs", 1575], + // unified-agent-model 1A.1: inline test module moved to nest/tests.rs, + // ratcheting 1575 -> 679 (under the 1000 default; entry kept as a ratchet). + ["src-tauri/src/managed_agents/nest.rs", 679], // harness-persona-sync: persona-runtime resolution threaded into the spawn // path here. Load-bearing feature growth; queued to split in the resolver // unify refactor followup. +26 for resolve_effective_prompt_model_provider @@ -153,20 +157,26 @@ const overrides = new Map([ // same-runtime / alias / sentinel / non-override / persona-less test matrix // (~135 lines, mostly tests) so a deliberate Custom pin survives the update // path instead of being dropped back to inherit. Load-bearing, not debt. - ["src-tauri/src/managed_agents/discovery.rs", 1259], + // unified-agent-model 1A.1: inline test module moved to discovery/tests.rs, + // ratcheting 1259 -> 802 (under the 1000 default; entry kept as a ratchet). + ["src-tauri/src/managed_agents/discovery.rs", 802], // migration_tests.rs carries the harness-sync migration coverage plus the // patch_json_records owner-only writeback regression test (SECURITY.md:90 // crash-safe 0o600 fallback). Load-bearing security + feature coverage, not // generic debt growth. Approved override; still queued to split. Event-sync // (persona/team event reconcile) tests were split out to event_sync_tests.rs // and the limit ratcheted 1410 → 1110. + // unified-agent-model 1A.1: materialize tests live with their module in + // migration/materialize.rs; ratchet held at 1110. ["src-tauri/src/migration_tests.rs", 1110], ["src-tauri/src/nostr_convert.rs", 1126], ["src/shared/api/relayClientSession.ts", 1022], // Boot-time event sync (persona/team/agent event reconcile) was split out // to event_sync.rs, ratcheting this limit 1575 → 1310. Remaining content is // the pre-identity data migrations; still queued to split further. - ["src-tauri/src/migration.rs", 1310], + // unified-agent-model 1A.1: materialize_agent_runtimes split to + // migration/materialize.rs, ratcheting 1310 -> 1297. + ["src-tauri/src/migration.rs", 1297], // onMarkRead + isUnread prop threading (mirrors the onMarkUnread prop // already here) for the single-toggle mark-read/unread menu item — a small // overage from load-bearing per-message plumbing, not generic debt growth. diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 0fc7a16e93..ac29591f5f 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -983,27 +983,6 @@ pub async fn create_managed_agent( } /// Data needed for background profile reconciliation after agent start. -pub(crate) struct ProfileReconcileData { - pub(crate) private_key_nsec: String, - pub(crate) name: String, - pub(crate) relay_url: String, - /// Expected avatar URL for the published profile. `None` for legacy records - /// that predate the `avatar_url` field — these will be backfilled from the - /// relay's existing kind:0 profile on first reconciliation. - pub(crate) avatar_url: Option, - pub(crate) auth_tag: Option, - /// The agent's pubkey (hex). Needed to update the persisted record during - /// avatar backfill migration. - pub(crate) pubkey: String, - /// The agent's command (e.g. "goose"). Used as fallback when no profile - /// exists on the relay during avatar backfill. - pub(crate) agent_command: String, - /// Persona ID if this agent was created from a persona. Used during avatar - /// backfill to recover the correct avatar from the persona record when the - /// relay profile has been corrupted. - pub(crate) persona_id: Option, -} - #[tauri::command] pub async fn start_managed_agent( pubkey: String, @@ -1144,144 +1123,6 @@ pub async fn start_managed_agent( result } -/// Resolve the avatar to backfill for a legacy agent record (pre-PR-921, no -/// stored `avatar_url`). -/// -/// Priority: the persona's avatar wins, because the old reconciliation code -/// could have overwritten the relay's kind:0 `picture` with the command default -/// — making the relay an unreliable source for persona-backed agents. Only fall -/// back to the relay's `picture`, then the command icon, for agents with no -/// persona avatar to recover from. -fn resolve_legacy_avatar( - persona_avatar: Option, - relay_picture: Option, - agent_command: &str, -) -> String { - persona_avatar - .or(relay_picture) - .or_else(|| managed_agent_avatar_url(agent_command)) - .unwrap_or_default() -} - -/// Reconcile an agent's kind:0 profile on the relay. -/// -/// Queries the relay for the agent's existing profile and re-publishes if missing -/// or stale (display_name or picture mismatch). This is fire-and-forget — errors -/// are returned to the caller for logging but never block agent startup. -/// -/// For legacy records (pre-PR-921) where `avatar_url` is `None`, this function -/// backfills via `resolve_legacy_avatar` — preferring the persona record's avatar -/// over the relay's `picture`, since the old code may have corrupted the relay -/// profile — and persists the updated record. After backfill, normal -/// reconciliation proceeds. -/// -/// Query and publish target the relay returned by `effective_agent_relay_url` -/// for every agent regardless of backend: an explicit per-agent `relay_url` -/// wins, and a blank one falls back to the active workspace relay. This keeps -/// reconciliation following the session's relay for never-pinned agents while -/// honoring a deliberate pin wherever it points. -pub(crate) async fn reconcile_agent_profile( - state: &AppState, - app: &AppHandle, - agent_pubkey: &str, - data: &ProfileReconcileData, -) -> Result<(), String> { - use crate::relay::{query_agent_profile, sync_managed_agent_profile}; - - // An explicit per-agent relay wins; an empty one falls back to the active - // workspace relay. Resolved once and used for both the read and write-back. - let relay_url = crate::relay::effective_agent_relay_url( - &data.relay_url, - &relay_ws_url_with_override(state), - ); - - // Query the relay for the agent's existing kind:0 profile. - let existing = query_agent_profile(state, &relay_url, agent_pubkey).await?; - - // Resolve the expected avatar — backfilling for legacy records that have no - // stored avatar_url yet. - let expected_avatar = match data.avatar_url.as_deref() { - Some(url) => url.to_string(), - None => { - // Legacy record: the relay profile may have been corrupted by the - // old reconciliation code (it overwrote the persona avatar with the - // command default), so the persona record is the authoritative source. - let persona_avatar = data.persona_id.as_ref().and_then(|pid| { - load_personas(app) - .ok()? - .into_iter() - .find(|p| p.id == *pid)? - .avatar_url - }); - - let backfilled = resolve_legacy_avatar( - persona_avatar, - existing.as_ref().and_then(|info| info.picture.clone()), - &data.agent_command, - ); - - // Persist the backfilled avatar so this migration only runs once. - if !backfilled.is_empty() { - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let mut records = load_managed_agents(app)?; - if let Some(record) = records.iter_mut().find(|r| r.pubkey == data.pubkey) { - record.avatar_url = Some(backfilled.clone()); - save_managed_agents(app, &records)?; - } - } - - backfilled - } - }; - - let expected_avatar = if expected_avatar.is_empty() { - None - } else { - Some(expected_avatar) - }; - - if !profile_needs_sync(existing.as_ref(), &data.name, expected_avatar.as_deref()) { - return Ok(()); - } - - let agent_keys = Keys::parse(&data.private_key_nsec) - .map_err(|e| format!("failed to parse agent keys: {e}"))?; - - sync_managed_agent_profile( - state, - &relay_url, - &agent_keys, - &data.name, - expected_avatar.as_deref(), - data.auth_tag.as_deref(), - ) - .await -} - -/// Decide whether a published profile is missing or stale relative to the -/// expected name and avatar. A missing profile always needs sync; a present -/// one is stale when either the display name or picture diverges. -fn profile_needs_sync( - existing: Option<&crate::relay::AgentProfileInfo>, - expected_name: &str, - expected_avatar: Option<&str>, -) -> bool { - match existing { - None => true, - Some(info) => { - let name_matches = info.display_name.as_deref() == Some(expected_name); - let picture_matches = info.picture.as_deref() == expected_avatar; - !name_matches || !picture_matches - } - } -} - -// Async so the blocking body (disk reads/writes + process termination) runs off -// the main UI thread via spawn_blocking. State is re-derived from the owned -// AppHandle inside the closure (`State<'_, _>` is borrowed, MutexGuard is !Send). #[tauri::command] pub async fn stop_managed_agent( pubkey: String, @@ -1442,6 +1283,12 @@ pub fn get_managed_agent_log( // 3. Desktop's existing presence polling sees "offline" — UI updates automatically // No backend Tauri command needed. Presence IS the status. +#[path = "agents_profile.rs"] +mod profile; +#[cfg(test)] +use profile::{profile_needs_sync, resolve_legacy_avatar}; +pub(crate) use profile::{reconcile_agent_profile, ProfileReconcileData}; + #[cfg(test)] #[path = "agents_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/commands/agents_profile.rs b/desktop/src-tauri/src/commands/agents_profile.rs new file mode 100644 index 0000000000..236fd9f4cc --- /dev/null +++ b/desktop/src-tauri/src/commands/agents_profile.rs @@ -0,0 +1,170 @@ +//! Agent kind:0 profile reconciliation — split from `agents.rs` (file-size +//! guard). Owns the reconcile data carrier, the legacy-avatar backfill, and +//! the needs-sync predicate. + +use tauri::AppHandle; + +use crate::app_state::AppState; +use crate::managed_agents::managed_agent_avatar_url; + +use super::*; + +pub(crate) struct ProfileReconcileData { + pub(crate) private_key_nsec: String, + pub(crate) name: String, + pub(crate) relay_url: String, + /// Expected avatar URL for the published profile. `None` for legacy records + /// that predate the `avatar_url` field — these will be backfilled from the + /// relay's existing kind:0 profile on first reconciliation. + pub(crate) avatar_url: Option, + pub(crate) auth_tag: Option, + /// The agent's pubkey (hex). Needed to update the persisted record during + /// avatar backfill migration. + pub(crate) pubkey: String, + /// The agent's command (e.g. "goose"). Used as fallback when no profile + /// exists on the relay during avatar backfill. + pub(crate) agent_command: String, + /// Persona ID if this agent was created from a persona. Used during avatar + /// backfill to recover the correct avatar from the persona record when the + /// relay profile has been corrupted. + pub(crate) persona_id: Option, +} + +/// Resolve the avatar to backfill for a legacy agent record (pre-PR-921, no +/// stored `avatar_url`). +/// +/// Priority: the persona's avatar wins, because the old reconciliation code +/// could have overwritten the relay's kind:0 `picture` with the command default +/// — making the relay an unreliable source for persona-backed agents. Only fall +/// back to the relay's `picture`, then the command icon, for agents with no +/// persona avatar to recover from. +pub(super) fn resolve_legacy_avatar( + persona_avatar: Option, + relay_picture: Option, + agent_command: &str, +) -> String { + persona_avatar + .or(relay_picture) + .or_else(|| managed_agent_avatar_url(agent_command)) + .unwrap_or_default() +} + +/// Reconcile an agent's kind:0 profile on the relay. +/// +/// Queries the relay for the agent's existing profile and re-publishes if missing +/// or stale (display_name or picture mismatch). This is fire-and-forget — errors +/// are returned to the caller for logging but never block agent startup. +/// +/// For legacy records (pre-PR-921) where `avatar_url` is `None`, this function +/// backfills via `resolve_legacy_avatar` — preferring the persona record's avatar +/// over the relay's `picture`, since the old code may have corrupted the relay +/// profile — and persists the updated record. After backfill, normal +/// reconciliation proceeds. +/// +/// Query and publish target the relay returned by `effective_agent_relay_url` +/// for every agent regardless of backend: an explicit per-agent `relay_url` +/// wins, and a blank one falls back to the active workspace relay. This keeps +/// reconciliation following the session's relay for never-pinned agents while +/// honoring a deliberate pin wherever it points. +pub(crate) async fn reconcile_agent_profile( + state: &AppState, + app: &AppHandle, + agent_pubkey: &str, + data: &ProfileReconcileData, +) -> Result<(), String> { + use crate::relay::{query_agent_profile, sync_managed_agent_profile}; + + // An explicit per-agent relay wins; an empty one falls back to the active + // workspace relay. Resolved once and used for both the read and write-back. + let relay_url = crate::relay::effective_agent_relay_url( + &data.relay_url, + &relay_ws_url_with_override(state), + ); + + // Query the relay for the agent's existing kind:0 profile. + let existing = query_agent_profile(state, &relay_url, agent_pubkey).await?; + + // Resolve the expected avatar — backfilling for legacy records that have no + // stored avatar_url yet. + let expected_avatar = match data.avatar_url.as_deref() { + Some(url) => url.to_string(), + None => { + // Legacy record: the relay profile may have been corrupted by the + // old reconciliation code (it overwrote the persona avatar with the + // command default), so the persona record is the authoritative source. + let persona_avatar = data.persona_id.as_ref().and_then(|pid| { + load_personas(app) + .ok()? + .into_iter() + .find(|p| p.id == *pid)? + .avatar_url + }); + + let backfilled = resolve_legacy_avatar( + persona_avatar, + existing.as_ref().and_then(|info| info.picture.clone()), + &data.agent_command, + ); + + // Persist the backfilled avatar so this migration only runs once. + if !backfilled.is_empty() { + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let mut records = load_managed_agents(app)?; + if let Some(record) = records.iter_mut().find(|r| r.pubkey == data.pubkey) { + record.avatar_url = Some(backfilled.clone()); + save_managed_agents(app, &records)?; + } + } + + backfilled + } + }; + + let expected_avatar = if expected_avatar.is_empty() { + None + } else { + Some(expected_avatar) + }; + + if !profile_needs_sync(existing.as_ref(), &data.name, expected_avatar.as_deref()) { + return Ok(()); + } + + let agent_keys = Keys::parse(&data.private_key_nsec) + .map_err(|e| format!("failed to parse agent keys: {e}"))?; + + sync_managed_agent_profile( + state, + &relay_url, + &agent_keys, + &data.name, + expected_avatar.as_deref(), + data.auth_tag.as_deref(), + ) + .await +} + +/// Decide whether a published profile is missing or stale relative to the +/// expected name and avatar. A missing profile always needs sync; a present +/// one is stale when either the display name or picture diverges. +pub(super) fn profile_needs_sync( + existing: Option<&crate::relay::AgentProfileInfo>, + expected_name: &str, + expected_avatar: Option<&str>, +) -> bool { + match existing { + None => true, + Some(info) => { + let name_matches = info.display_name.as_deref() == Some(expected_name); + let picture_matches = info.picture.as_deref() == expected_avatar; + !name_matches || !picture_matches + } + } +} + +// Async so the blocking body (disk reads/writes + process termination) runs off +// the main UI thread via spawn_blocking. State is re-derived from the owned +// AppHandle inside the closure (`State<'_, _>` is borrowed, MutexGuard is !Send). diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 7545fe95e7..09ac897f3c 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -798,583 +798,4 @@ pub fn managed_agent_avatar_url(command: &str) -> Option { } #[cfg(test)] -mod tests { - use std::path::PathBuf; - - use super::{ - classify_runtime, create_time_agent_command_override, default_agent_command, - divergent_agent_command_override, effective_agent_command, find_via_login_shell, - managed_agent_avatar_url, normalize_agent_args, record_agent_command, - update_time_agent_command_override, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, - CODEX_AVATAR_URL, GOOSE_AVATAR_URL, - }; - use crate::managed_agents::AcpAvailabilityStatus; - - #[test] - fn resolves_known_avatar_for_bare_command() { - let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); - - assert_eq!(avatar_url, GOOSE_AVATAR_URL); - } - - #[test] - fn resolves_known_avatar_for_command_paths_and_aliases() { - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/codex-acp"), - Some(CODEX_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("Claude Code"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url(r"C:\Tools\claude-agent-acp.exe"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/claude-code-acp"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - } - - #[test] - fn returns_none_for_unknown_commands() { - assert!(managed_agent_avatar_url("custom-agent").is_none()); - } - - #[test] - fn default_agent_command_resolves_bundled_buzz_agent() { - // The create-path default must be the bundled buzz-agent, never the - // bare `goose` that isn't on PATH on a stock Windows install. - assert_eq!(default_agent_command(), "buzz-agent"); - // And buzz-agent takes no `acp` arg — confirm no arg leakage from the default. - assert_eq!( - normalize_agent_args(&default_agent_command(), vec!["acp".into()]), - Vec::::new() - ); - } - - #[test] - fn normalizes_claude_and_codex_args_to_empty() { - assert_eq!( - normalize_agent_args("claude-agent-acp", vec!["acp".into()]), - Vec::::new() - ); - assert_eq!( - normalize_agent_args("claude-code-acp", vec!["acp".into()]), - Vec::::new() - ); - assert_eq!( - normalize_agent_args("codex-acp", vec!["acp".into()]), - Vec::::new() - ); - } - - #[test] - fn resolves_buzz_agent_avatar() { - assert_eq!( - managed_agent_avatar_url("buzz-agent"), - Some(BUZZ_AGENT_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/buzz-agent"), - Some(BUZZ_AGENT_AVATAR_URL.to_string()) - ); - } - - #[test] - fn normalizes_buzz_agent_args_to_empty() { - assert_eq!( - normalize_agent_args("buzz-agent", Vec::new()), - Vec::::new() - ); - assert_eq!( - normalize_agent_args("buzz-agent", vec!["acp".into()]), - Vec::::new() - ); - } - - #[test] - fn login_shell_lookup_treats_command_as_data() { - let marker = - std::env::temp_dir().join(format!("buzz-discovery-marker-{}", uuid::Uuid::new_v4())); - let payload = format!("doesnotexist; touch {} #", marker.display()); - - let resolved = find_via_login_shell(&payload); - - assert!( - resolved.is_none(), - "payload should not resolve to a command" - ); - assert!( - !marker.exists(), - "shell lookup must not execute injected commands" - ); - } - - #[cfg(unix)] - #[test] - fn explicit_path_resolution_ignores_non_executable_files() { - use std::os::unix::fs::PermissionsExt; - - let dir = - std::env::temp_dir().join(format!("buzz-discovery-path-{}", uuid::Uuid::new_v4())); - std::fs::create_dir_all(&dir).expect("create temp dir"); - let bin = dir.join("buzz-acp"); - std::fs::write(&bin, "").expect("write placeholder"); - std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o644)) - .expect("chmod placeholder"); - - assert!( - super::resolve_workspace_command(bin.to_str().expect("utf8 path")).is_none(), - "non-executable placeholder must not resolve" - ); - - std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) - .expect("chmod executable"); - assert_eq!( - super::resolve_workspace_command(bin.to_str().expect("utf8 path")), - Some(bin.clone()) - ); - - let _ = std::fs::remove_dir_all(dir); - } - - #[test] - fn classifies_available_when_adapter_found() { - let (status, cmd, path) = classify_runtime( - Some(("goose", PathBuf::from("/usr/local/bin/goose"))), - None, - false, - ); - assert_eq!(status, AcpAvailabilityStatus::Available); - assert_eq!(cmd.as_deref(), Some("goose")); - assert_eq!(path.as_deref(), Some("/usr/local/bin/goose")); - } - - #[test] - fn classifies_adapter_missing_when_cli_present() { - let (status, cmd, path) = classify_runtime(None, Some("claude"), true); - assert_eq!(status, AcpAvailabilityStatus::AdapterMissing); - assert!(cmd.is_none()); - assert!(path.is_none()); - } - - #[test] - fn classifies_not_installed_when_nothing_found() { - let (status, cmd, path) = classify_runtime(None, Some("claude"), false); - assert_eq!(status, AcpAvailabilityStatus::NotInstalled); - assert!(cmd.is_none()); - assert!(path.is_none()); - } - - #[test] - fn classifies_not_installed_when_no_underlying_cli() { - let (status, cmd, path) = classify_runtime(None, None, false); - assert_eq!(status, AcpAvailabilityStatus::NotInstalled); - assert!(cmd.is_none()); - assert!(path.is_none()); - } - - #[test] - fn classifies_cli_missing_when_adapter_found_but_cli_absent() { - let (status, cmd, path) = classify_runtime( - Some(("codex-acp", PathBuf::from("/opt/homebrew/bin/codex-acp"))), - Some("codex"), - false, - ); - assert_eq!(status, AcpAvailabilityStatus::CliMissing); - assert_eq!(cmd.as_deref(), Some("codex-acp")); - assert_eq!(path.as_deref(), Some("/opt/homebrew/bin/codex-acp")); - } - - fn persona_with_runtime( - id: &str, - runtime: Option<&str>, - ) -> crate::managed_agents::PersonaRecord { - crate::managed_agents::PersonaRecord { - id: id.to_string(), - display_name: id.to_string(), - avatar_url: None, - system_prompt: String::new(), - runtime: runtime.map(str::to_string), - model: None, - provider: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - env_vars: std::collections::BTreeMap::new(), - created_at: "2026-06-09T00:00:00Z".to_string(), - updated_at: "2026-06-09T00:00:00Z".to_string(), - } - } - - #[test] - fn effective_agent_command_explicit_override_wins() { - // An explicit pin beats the persona's runtime. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - effective_agent_command(Some("p1"), &personas, Some("codex-acp")), - "codex-acp" - ); - } - - /// Minimal record for `record_agent_command` tests. Only the resolution - /// inputs (runtime / persona_id / agent_command_override) vary. - fn record_with( - runtime: Option<&str>, - persona_id: Option<&str>, - override_cmd: Option<&str>, - ) -> crate::managed_agents::types::ManagedAgentRecord { - crate::managed_agents::types::ManagedAgentRecord { - pubkey: String::new(), - name: "r".to_string(), - persona_id: persona_id.map(str::to_string), - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: String::new(), - agent_command: String::new(), - agent_command_override: override_cmd.map(str::to_string), - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 0, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - mcp_toolsets: None, - start_on_app_launch: false, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_binary_path: None, - persona_team_dir: None, - persona_name_in_team: None, - env_vars: std::collections::BTreeMap::new(), - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - respond_to: Default::default(), - respond_to_allowlist: vec![], - display_name: None, - slug: None, - runtime: runtime.map(str::to_string), - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - relay_mesh: None, - } - } - - #[test] - fn record_agent_command_own_runtime_wins_over_persona() { - // A record with its own materialized runtime never consults the - // persona list — the unified-model resolution. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - let record = record_with(Some("claude"), Some("p1"), None); - assert_eq!(record_agent_command(&record, &personas), "claude-agent-acp"); - } - - #[test] - fn record_agent_command_override_beats_runtime() { - let record = record_with(Some("claude"), None, Some("codex-acp")); - assert_eq!(record_agent_command(&record, &[]), "codex-acp"); - } - - #[test] - fn record_agent_command_legacy_persona_fallback() { - // Pre-migration record: persona_id set, no runtime — resolves through - // the legacy persona path unchanged. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - let record = record_with(None, Some("p1"), None); - assert_eq!(record_agent_command(&record, &personas), "goose"); - } - - #[test] - fn record_agent_command_bare_record_defaults() { - let record = record_with(None, None, None); - assert_eq!(record_agent_command(&record, &[]), default_agent_command()); - } - - #[test] - fn effective_agent_command_inherits_persona_runtime() { - // No override → persona runtime id maps to its primary command. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - effective_agent_command(Some("p1"), &personas, None), - "claude-agent-acp" - ); - } - - #[test] - fn effective_agent_command_empty_override_is_inherit() { - // A blank/whitespace override is treated as "inherit", not a pin. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - effective_agent_command(Some("p1"), &personas, Some(" ")), - "goose" - ); - } - - #[test] - fn effective_agent_command_falls_back_to_default() { - // No override, no persona runtime, and a deleted persona all fall back - // to the bundled default. - let personas = vec![persona_with_runtime("p1", None)]; - assert_eq!( - effective_agent_command(Some("p1"), &personas, None), - default_agent_command() - ); - assert_eq!( - effective_agent_command(Some("gone"), &personas, None), - default_agent_command() - ); - assert_eq!( - effective_agent_command(None, &personas, None), - default_agent_command() - ); - } - - #[test] - fn divergent_override_none_when_picked_matches_persona_runtime() { - // The persona-backed create/edit flow sends the persona's resolved - // command. It must be treated as "inherit" (None), not a pin. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - divergent_agent_command_override(Some("p1"), &personas, Some("goose")), - None - ); - } - - #[test] - fn divergent_override_none_for_alternate_command_of_same_runtime() { - // A client with only `claude-code-acp` installed sends that command for - // a `claude` persona whose primary command is `claude-agent-acp`. Both - // map to the `claude` runtime, so it inherits — string equality would - // wrongly bake a pin (CRITICAL-3). - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - divergent_agent_command_override(Some("p1"), &personas, Some("claude-code-acp")), - None - ); - } - - #[test] - fn divergent_override_some_when_picked_is_different_runtime() { - // A deliberate pin to a different runtime is preserved. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - divergent_agent_command_override(Some("p1"), &personas, Some("codex-acp")), - Some("codex-acp".to_string()) - ); - } - - #[test] - fn divergent_override_none_for_empty_or_absent_pick() { - // The "Inherit from persona" sentinel (empty) and a name-only edit - // (absent) both clear the pin. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - divergent_agent_command_override(Some("p1"), &personas, Some(" ")), - None - ); - assert_eq!( - divergent_agent_command_override(Some("p1"), &personas, None), - None - ); - } - - #[test] - fn create_time_override_none_when_persona_runtime_not_installed() { - // CRITICAL-3 (Case 3): a `claude`-persona agent created on a machine - // where the claude adapter isn't installed. `resolvePersonaRuntime` - // falls back to the default (`buzz-agent`) and sends THAT command with - // `harness_override` false (the user did not pick it). At create this - // is a fallback, not a deliberate pin — it must store `None` so the - // agent inherits the persona's runtime once it's installed and the - // persona is re-edited. Baking `Some("buzz-agent")` here is the exact - // bug this resolver chain exists to kill. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - create_time_agent_command_override(Some("p1"), &personas, Some("buzz-agent"), false), - None - ); - } - - #[test] - fn create_time_override_some_when_user_deliberately_overrides_installed_runtime() { - // Case 2 + deliberate override: the persona's `claude` runtime IS - // available, but the user explicitly picked `codex` in a deploy dialog's - // runtime selector ("overriding persona preferences"), so the frontend - // sends `codex-acp` with `harness_override` true. This is a real pin and - // MUST be preserved — returning `None` would silently swallow the - // deliberate override and inherit `claude` on spawn. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - create_time_agent_command_override(Some("p1"), &personas, Some("codex-acp"), true), - Some("codex-acp".to_string()) - ); - } - - #[test] - fn create_time_override_none_when_persona_runtime_installed() { - // Case 2: the persona's runtime is available, so `resolvePersonaRuntime` - // sends the persona's own command with no override. Inherits — no pin. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - create_time_agent_command_override(Some("p1"), &personas, Some("goose"), false), - None - ); - } - - #[test] - fn create_time_override_preserves_selected_runtime_alias() { - // A `claude` persona inherits the primary command `claude-agent-acp`, - // but discovery may select an installed alias such as `claude-code-acp`. - // When UI marks that create-time selection as explicit, preserve the - // alias so the first spawn uses a command known to be installed. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - create_time_agent_command_override( - Some("p1"), - &personas, - Some("claude-code-acp"), - true - ), - Some("claude-code-acp".to_string()) - ); - } - - #[test] - fn create_time_override_inherits_exact_persona_command() { - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - create_time_agent_command_override( - Some("p1"), - &personas, - Some("claude-agent-acp"), - true - ), - None - ); - } - - #[test] - fn create_time_override_preserves_pin_for_persona_less_create() { - // The standalone CreateAgentDialog creates persona-LESS agents. With no - // persona to inherit, the picked command IS the agent's harness and must - // be preserved as a real pin (divergence from the bundled default), - // regardless of the override flag. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - create_time_agent_command_override(None, &personas, Some("codex-acp"), false), - Some("codex-acp".to_string()) - ); - } - - #[test] - fn update_time_override_preserves_same_runtime_pin_when_overriding() { - // The bug this fixes: the user picks "Custom command" in the edit - // dialog and saves `goose` verbatim for a goose persona. That is a - // deliberate pin (harness_override true) — it must be kept so future - // persona runtime edits stop propagating, even though it maps to the - // persona's own runtime. `divergent_agent_command_override` alone would - // wrongly drop it to `None`. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - update_time_agent_command_override(Some("p1"), &personas, Some("goose"), true), - Some("goose".to_string()) - ); - } - - #[test] - fn update_time_override_preserves_exact_persona_command_when_overriding() { - // Even when the pick is byte-identical to the persona's own command, an - // explicit Custom selection (harness_override true) is a deliberate pin - // and is preserved. This is the core divergence from the create-time - // contract: at update, equality reached the force branch only because - // the user picked Custom. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - update_time_agent_command_override( - Some("p1"), - &personas, - Some("claude-agent-acp"), - true - ), - Some("claude-agent-acp".to_string()) - ); - } - - #[test] - fn update_time_override_preserves_alias_pin_when_overriding() { - // A `claude` persona with an installed `claude-code-acp` alias: picking - // it as a Custom pin is a deliberate divergence from the primary - // command and must be preserved when overriding. - let personas = vec![persona_with_runtime("p1", Some("claude"))]; - assert_eq!( - update_time_agent_command_override( - Some("p1"), - &personas, - Some("claude-code-acp"), - true - ), - Some("claude-code-acp".to_string()) - ); - } - - #[test] - fn update_time_override_defers_to_divergent_when_not_overriding() { - // Without the explicit intent bit (e.g. a name-only edit that still - // echoes the command), the persona stays authoritative: a same-runtime - // command inherits, a different runtime pins. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - update_time_agent_command_override(Some("p1"), &personas, Some("goose"), false), - None - ); - assert_eq!( - update_time_agent_command_override(Some("p1"), &personas, Some("codex-acp"), false), - Some("codex-acp".to_string()) - ); - } - - #[test] - fn update_time_override_clears_pin_for_inherit_sentinel() { - // The empty "Inherit from persona" sentinel always clears the pin, - // regardless of the override flag. - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - update_time_agent_command_override(Some("p1"), &personas, Some(" "), true), - None - ); - assert_eq!( - update_time_agent_command_override(Some("p1"), &personas, None, true), - None - ); - } - - #[test] - fn update_time_override_preserves_pin_for_persona_less_agent() { - // A persona-less agent has no runtime to inherit, so any picked command - // is a real pin — preserved even without the override flag (mirrors the - // create-time persona-less contract). - let personas = vec![persona_with_runtime("p1", Some("goose"))]; - assert_eq!( - update_time_agent_command_override(None, &personas, Some("codex-acp"), false), - Some("codex-acp".to_string()) - ); - } -} +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs new file mode 100644 index 0000000000..fc9c315cdf --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -0,0 +1,554 @@ +use std::path::PathBuf; + +use super::{ + classify_runtime, create_time_agent_command_override, default_agent_command, + divergent_agent_command_override, effective_agent_command, find_via_login_shell, + managed_agent_avatar_url, normalize_agent_args, record_agent_command, + update_time_agent_command_override, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, + CODEX_AVATAR_URL, GOOSE_AVATAR_URL, +}; +use crate::managed_agents::AcpAvailabilityStatus; + +#[test] +fn resolves_known_avatar_for_bare_command() { + let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); + + assert_eq!(avatar_url, GOOSE_AVATAR_URL); +} + +#[test] +fn resolves_known_avatar_for_command_paths_and_aliases() { + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/codex-acp"), + Some(CODEX_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("Claude Code"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url(r"C:\Tools\claude-agent-acp.exe"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/claude-code-acp"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); +} + +#[test] +fn returns_none_for_unknown_commands() { + assert!(managed_agent_avatar_url("custom-agent").is_none()); +} + +#[test] +fn default_agent_command_resolves_bundled_buzz_agent() { + // The create-path default must be the bundled buzz-agent, never the + // bare `goose` that isn't on PATH on a stock Windows install. + assert_eq!(default_agent_command(), "buzz-agent"); + // And buzz-agent takes no `acp` arg — confirm no arg leakage from the default. + assert_eq!( + normalize_agent_args(&default_agent_command(), vec!["acp".into()]), + Vec::::new() + ); +} + +#[test] +fn normalizes_claude_and_codex_args_to_empty() { + assert_eq!( + normalize_agent_args("claude-agent-acp", vec!["acp".into()]), + Vec::::new() + ); + assert_eq!( + normalize_agent_args("claude-code-acp", vec!["acp".into()]), + Vec::::new() + ); + assert_eq!( + normalize_agent_args("codex-acp", vec!["acp".into()]), + Vec::::new() + ); +} + +#[test] +fn resolves_buzz_agent_avatar() { + assert_eq!( + managed_agent_avatar_url("buzz-agent"), + Some(BUZZ_AGENT_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/buzz-agent"), + Some(BUZZ_AGENT_AVATAR_URL.to_string()) + ); +} + +#[test] +fn normalizes_buzz_agent_args_to_empty() { + assert_eq!( + normalize_agent_args("buzz-agent", Vec::new()), + Vec::::new() + ); + assert_eq!( + normalize_agent_args("buzz-agent", vec!["acp".into()]), + Vec::::new() + ); +} + +#[test] +fn login_shell_lookup_treats_command_as_data() { + let marker = + std::env::temp_dir().join(format!("buzz-discovery-marker-{}", uuid::Uuid::new_v4())); + let payload = format!("doesnotexist; touch {} #", marker.display()); + + let resolved = find_via_login_shell(&payload); + + assert!( + resolved.is_none(), + "payload should not resolve to a command" + ); + assert!( + !marker.exists(), + "shell lookup must not execute injected commands" + ); +} + +#[cfg(unix)] +#[test] +fn explicit_path_resolution_ignores_non_executable_files() { + use std::os::unix::fs::PermissionsExt; + + let dir = std::env::temp_dir().join(format!("buzz-discovery-path-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).expect("create temp dir"); + let bin = dir.join("buzz-acp"); + std::fs::write(&bin, "").expect("write placeholder"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o644)) + .expect("chmod placeholder"); + + assert!( + super::resolve_workspace_command(bin.to_str().expect("utf8 path")).is_none(), + "non-executable placeholder must not resolve" + ); + + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) + .expect("chmod executable"); + assert_eq!( + super::resolve_workspace_command(bin.to_str().expect("utf8 path")), + Some(bin.clone()) + ); + + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn classifies_available_when_adapter_found() { + let (status, cmd, path) = classify_runtime( + Some(("goose", PathBuf::from("/usr/local/bin/goose"))), + None, + false, + ); + assert_eq!(status, AcpAvailabilityStatus::Available); + assert_eq!(cmd.as_deref(), Some("goose")); + assert_eq!(path.as_deref(), Some("/usr/local/bin/goose")); +} + +#[test] +fn classifies_adapter_missing_when_cli_present() { + let (status, cmd, path) = classify_runtime(None, Some("claude"), true); + assert_eq!(status, AcpAvailabilityStatus::AdapterMissing); + assert!(cmd.is_none()); + assert!(path.is_none()); +} + +#[test] +fn classifies_not_installed_when_nothing_found() { + let (status, cmd, path) = classify_runtime(None, Some("claude"), false); + assert_eq!(status, AcpAvailabilityStatus::NotInstalled); + assert!(cmd.is_none()); + assert!(path.is_none()); +} + +#[test] +fn classifies_not_installed_when_no_underlying_cli() { + let (status, cmd, path) = classify_runtime(None, None, false); + assert_eq!(status, AcpAvailabilityStatus::NotInstalled); + assert!(cmd.is_none()); + assert!(path.is_none()); +} + +#[test] +fn classifies_cli_missing_when_adapter_found_but_cli_absent() { + let (status, cmd, path) = classify_runtime( + Some(("codex-acp", PathBuf::from("/opt/homebrew/bin/codex-acp"))), + Some("codex"), + false, + ); + assert_eq!(status, AcpAvailabilityStatus::CliMissing); + assert_eq!(cmd.as_deref(), Some("codex-acp")); + assert_eq!(path.as_deref(), Some("/opt/homebrew/bin/codex-acp")); +} + +fn persona_with_runtime(id: &str, runtime: Option<&str>) -> crate::managed_agents::PersonaRecord { + crate::managed_agents::PersonaRecord { + id: id.to_string(), + display_name: id.to_string(), + avatar_url: None, + system_prompt: String::new(), + runtime: runtime.map(str::to_string), + model: None, + provider: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: std::collections::BTreeMap::new(), + created_at: "2026-06-09T00:00:00Z".to_string(), + updated_at: "2026-06-09T00:00:00Z".to_string(), + } +} + +#[test] +fn effective_agent_command_explicit_override_wins() { + // An explicit pin beats the persona's runtime. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + effective_agent_command(Some("p1"), &personas, Some("codex-acp")), + "codex-acp" + ); +} + +/// Minimal record for `record_agent_command` tests. Only the resolution +/// inputs (runtime / persona_id / agent_command_override) vary. +fn record_with( + runtime: Option<&str>, + persona_id: Option<&str>, + override_cmd: Option<&str>, +) -> crate::managed_agents::types::ManagedAgentRecord { + crate::managed_agents::types::ManagedAgentRecord { + pubkey: String::new(), + name: "r".to_string(), + persona_id: persona_id.map(str::to_string), + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: String::new(), + agent_command: String::new(), + agent_command_override: override_cmd.map(str::to_string), + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 0, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + mcp_toolsets: None, + start_on_app_launch: false, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + env_vars: std::collections::BTreeMap::new(), + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: runtime.map(str::to_string), + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + relay_mesh: None, + } +} + +#[test] +fn record_agent_command_own_runtime_wins_over_persona() { + // A record with its own materialized runtime never consults the + // persona list — the unified-model resolution. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + let record = record_with(Some("claude"), Some("p1"), None); + assert_eq!(record_agent_command(&record, &personas), "claude-agent-acp"); +} + +#[test] +fn record_agent_command_override_beats_runtime() { + let record = record_with(Some("claude"), None, Some("codex-acp")); + assert_eq!(record_agent_command(&record, &[]), "codex-acp"); +} + +#[test] +fn record_agent_command_legacy_persona_fallback() { + // Pre-migration record: persona_id set, no runtime — resolves through + // the legacy persona path unchanged. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + let record = record_with(None, Some("p1"), None); + assert_eq!(record_agent_command(&record, &personas), "goose"); +} + +#[test] +fn record_agent_command_bare_record_defaults() { + let record = record_with(None, None, None); + assert_eq!(record_agent_command(&record, &[]), default_agent_command()); +} + +#[test] +fn effective_agent_command_inherits_persona_runtime() { + // No override → persona runtime id maps to its primary command. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + effective_agent_command(Some("p1"), &personas, None), + "claude-agent-acp" + ); +} + +#[test] +fn effective_agent_command_empty_override_is_inherit() { + // A blank/whitespace override is treated as "inherit", not a pin. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + effective_agent_command(Some("p1"), &personas, Some(" ")), + "goose" + ); +} + +#[test] +fn effective_agent_command_falls_back_to_default() { + // No override, no persona runtime, and a deleted persona all fall back + // to the bundled default. + let personas = vec![persona_with_runtime("p1", None)]; + assert_eq!( + effective_agent_command(Some("p1"), &personas, None), + default_agent_command() + ); + assert_eq!( + effective_agent_command(Some("gone"), &personas, None), + default_agent_command() + ); + assert_eq!( + effective_agent_command(None, &personas, None), + default_agent_command() + ); +} + +#[test] +fn divergent_override_none_when_picked_matches_persona_runtime() { + // The persona-backed create/edit flow sends the persona's resolved + // command. It must be treated as "inherit" (None), not a pin. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + divergent_agent_command_override(Some("p1"), &personas, Some("goose")), + None + ); +} + +#[test] +fn divergent_override_none_for_alternate_command_of_same_runtime() { + // A client with only `claude-code-acp` installed sends that command for + // a `claude` persona whose primary command is `claude-agent-acp`. Both + // map to the `claude` runtime, so it inherits — string equality would + // wrongly bake a pin (CRITICAL-3). + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + divergent_agent_command_override(Some("p1"), &personas, Some("claude-code-acp")), + None + ); +} + +#[test] +fn divergent_override_some_when_picked_is_different_runtime() { + // A deliberate pin to a different runtime is preserved. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + divergent_agent_command_override(Some("p1"), &personas, Some("codex-acp")), + Some("codex-acp".to_string()) + ); +} + +#[test] +fn divergent_override_none_for_empty_or_absent_pick() { + // The "Inherit from persona" sentinel (empty) and a name-only edit + // (absent) both clear the pin. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + divergent_agent_command_override(Some("p1"), &personas, Some(" ")), + None + ); + assert_eq!( + divergent_agent_command_override(Some("p1"), &personas, None), + None + ); +} + +#[test] +fn create_time_override_none_when_persona_runtime_not_installed() { + // CRITICAL-3 (Case 3): a `claude`-persona agent created on a machine + // where the claude adapter isn't installed. `resolvePersonaRuntime` + // falls back to the default (`buzz-agent`) and sends THAT command with + // `harness_override` false (the user did not pick it). At create this + // is a fallback, not a deliberate pin — it must store `None` so the + // agent inherits the persona's runtime once it's installed and the + // persona is re-edited. Baking `Some("buzz-agent")` here is the exact + // bug this resolver chain exists to kill. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + create_time_agent_command_override(Some("p1"), &personas, Some("buzz-agent"), false), + None + ); +} + +#[test] +fn create_time_override_some_when_user_deliberately_overrides_installed_runtime() { + // Case 2 + deliberate override: the persona's `claude` runtime IS + // available, but the user explicitly picked `codex` in a deploy dialog's + // runtime selector ("overriding persona preferences"), so the frontend + // sends `codex-acp` with `harness_override` true. This is a real pin and + // MUST be preserved — returning `None` would silently swallow the + // deliberate override and inherit `claude` on spawn. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + create_time_agent_command_override(Some("p1"), &personas, Some("codex-acp"), true), + Some("codex-acp".to_string()) + ); +} + +#[test] +fn create_time_override_none_when_persona_runtime_installed() { + // Case 2: the persona's runtime is available, so `resolvePersonaRuntime` + // sends the persona's own command with no override. Inherits — no pin. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + create_time_agent_command_override(Some("p1"), &personas, Some("goose"), false), + None + ); +} + +#[test] +fn create_time_override_preserves_selected_runtime_alias() { + // A `claude` persona inherits the primary command `claude-agent-acp`, + // but discovery may select an installed alias such as `claude-code-acp`. + // When UI marks that create-time selection as explicit, preserve the + // alias so the first spawn uses a command known to be installed. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + create_time_agent_command_override(Some("p1"), &personas, Some("claude-code-acp"), true), + Some("claude-code-acp".to_string()) + ); +} + +#[test] +fn create_time_override_inherits_exact_persona_command() { + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + create_time_agent_command_override(Some("p1"), &personas, Some("claude-agent-acp"), true), + None + ); +} + +#[test] +fn create_time_override_preserves_pin_for_persona_less_create() { + // The standalone CreateAgentDialog creates persona-LESS agents. With no + // persona to inherit, the picked command IS the agent's harness and must + // be preserved as a real pin (divergence from the bundled default), + // regardless of the override flag. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + create_time_agent_command_override(None, &personas, Some("codex-acp"), false), + Some("codex-acp".to_string()) + ); +} + +#[test] +fn update_time_override_preserves_same_runtime_pin_when_overriding() { + // The bug this fixes: the user picks "Custom command" in the edit + // dialog and saves `goose` verbatim for a goose persona. That is a + // deliberate pin (harness_override true) — it must be kept so future + // persona runtime edits stop propagating, even though it maps to the + // persona's own runtime. `divergent_agent_command_override` alone would + // wrongly drop it to `None`. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, Some("goose"), true), + Some("goose".to_string()) + ); +} + +#[test] +fn update_time_override_preserves_exact_persona_command_when_overriding() { + // Even when the pick is byte-identical to the persona's own command, an + // explicit Custom selection (harness_override true) is a deliberate pin + // and is preserved. This is the core divergence from the create-time + // contract: at update, equality reached the force branch only because + // the user picked Custom. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, Some("claude-agent-acp"), true), + Some("claude-agent-acp".to_string()) + ); +} + +#[test] +fn update_time_override_preserves_alias_pin_when_overriding() { + // A `claude` persona with an installed `claude-code-acp` alias: picking + // it as a Custom pin is a deliberate divergence from the primary + // command and must be preserved when overriding. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, Some("claude-code-acp"), true), + Some("claude-code-acp".to_string()) + ); +} + +#[test] +fn update_time_override_defers_to_divergent_when_not_overriding() { + // Without the explicit intent bit (e.g. a name-only edit that still + // echoes the command), the persona stays authoritative: a same-runtime + // command inherits, a different runtime pins. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, Some("goose"), false), + None + ); + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, Some("codex-acp"), false), + Some("codex-acp".to_string()) + ); +} + +#[test] +fn update_time_override_clears_pin_for_inherit_sentinel() { + // The empty "Inherit from persona" sentinel always clears the pin, + // regardless of the override flag. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, Some(" "), true), + None + ); + assert_eq!( + update_time_agent_command_override(Some("p1"), &personas, None, true), + None + ); +} + +#[test] +fn update_time_override_preserves_pin_for_persona_less_agent() { + // A persona-less agent has no runtime to inherit, so any picked command + // is a real pin — preserved even without the override flag (mirrors the + // create-time persona-less contract). + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + assert_eq!( + update_time_agent_command_override(None, &personas, Some("codex-acp"), false), + Some("codex-acp".to_string()) + ); +} diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index 47ceccd004..ab3250a9ae 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -675,907 +675,4 @@ pub fn try_regenerate_nest(app: &AppHandle) { } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn nest_dir_is_under_home() { - if let Some(dir) = nest_dir() { - // Accepts both .buzz (prod) and .buzz-dev (dev) depending on - // whether init_nest_dir was called before this test ran. - let name = dir.file_name().and_then(|n| n.to_str()).unwrap_or(""); - assert!( - name == NEST_DIR_PROD || name == NEST_DIR_DEV, - "nest_dir must end with .buzz or .buzz-dev, got {dir:?}" - ); - } - } - - #[test] - fn init_nest_dir_prod_sets_buzz() { - // init_nest_dir is idempotent (OnceLock) — once set, subsequent calls - // are no-ops. We can only test the fallback path if the OnceLock is - // unset, which is only true in a fresh process. Instead, verify that - // nest_dir() always returns a path ending with a valid nest suffix. - let dir = nest_dir(); - if let Some(d) = dir { - let name = d.file_name().and_then(|n| n.to_str()).unwrap_or(""); - assert!( - name == NEST_DIR_PROD || name == NEST_DIR_DEV, - "nest_dir suffix must be .buzz or .buzz-dev, got {d:?}" - ); - } - } - - #[test] - fn ensure_nest_creates_all_dirs_and_agents_md() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - - ensure_nest_at(&root).unwrap(); - - // All subdirectories exist. - for dir in NEST_DIRS { - assert!(root.join(dir).is_dir(), "{dir}/ should exist"); - } - // REPOS is provisioned separately (may be a symlink); with no - // repos_dir configured it lands as a real directory. - assert!(root.join("REPOS").is_dir(), "REPOS/ should exist"); - - // AGENTS.md was written with default content. - let content = fs::read_to_string(root.join("AGENTS.md")).unwrap(); - assert_eq!(content, AGENTS_MD); - - // Permissions are 700 on Unix for root and all subdirs. - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let mode = fs::metadata(&root).unwrap().permissions().mode() & 0o777; - assert_eq!(mode, 0o700, "root should be 700"); - for dir in NEST_DIRS { - let mode = fs::metadata(root.join(dir)).unwrap().permissions().mode() & 0o777; - assert_eq!(mode, 0o700, "{dir}/ should be 700"); - } - let repos_mode = fs::metadata(root.join("REPOS")) - .unwrap() - .permissions() - .mode() - & 0o777; - assert_eq!(repos_mode, 0o700, "REPOS/ should be 700"); - } - } - - #[test] - fn ensure_nest_is_idempotent_and_preserves_custom_content() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - - // First call creates everything. - ensure_nest_at(&root).unwrap(); - - // User customizes AGENTS.md. - let agents = root.join("AGENTS.md"); - fs::write(&agents, "my custom instructions").unwrap(); - - // Second call succeeds and does not overwrite. - ensure_nest_at(&root).unwrap(); - - assert_eq!( - fs::read_to_string(&agents).unwrap(), - "my custom instructions" - ); - - // All dirs still exist. - for dir in NEST_DIRS { - assert!(root.join(dir).is_dir(), "{dir}/ should still exist"); - } - } - - #[cfg(unix)] - #[test] - fn ensure_nest_rejects_symlink_root() { - let tmp = tempfile::tempdir().unwrap(); - let target = tmp.path().join("real_dir"); - fs::create_dir(&target).unwrap(); - let link = tmp.path().join(".buzz"); - std::os::unix::fs::symlink(&target, &link).unwrap(); - - let result = ensure_nest_at(&link); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("symlink")); - } - - #[test] - fn ensure_nest_creates_skill_file() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - - // Canonical location under .agents. - let skill = root.join(".agents/skills/buzz-cli/SKILL.md"); - assert!(skill.exists(), "SKILL.md should exist at .agents path"); - let content = fs::read_to_string(&skill).unwrap(); - assert_eq!(content, BUZZ_CLI_SKILL_MD); - - // On unix, harness-specific symlinks should resolve to the canonical dir. - #[cfg(unix)] - { - for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { - let link = root.join(dir).join("buzz-cli"); - assert!( - link.symlink_metadata().unwrap().file_type().is_symlink(), - "{dir}/buzz-cli should be a symlink" - ); - assert!( - link.join("SKILL.md").exists(), - "symlink at {dir}/buzz-cli should resolve to dir with SKILL.md" - ); - } - } - } - - #[test] - fn ensure_nest_does_not_overwrite_skill_file() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - - let skill = root.join(".agents/skills/buzz-cli/SKILL.md"); - fs::write(&skill, "custom skill content").unwrap(); - - ensure_nest_at(&root).unwrap(); - assert_eq!(fs::read_to_string(&skill).unwrap(), "custom skill content"); - } - - #[cfg(unix)] - #[test] - fn ensure_nest_skill_dir_has_700_permissions() { - use std::os::unix::fs::PermissionsExt; - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - // Canonical path and all provider parent dirs should be locked down. - // Symlinks (e.g. .goose/skills/buzz-cli) are skipped by the chmod loop. - for dir in [ - ".agents", - ".agents/skills", - ".agents/skills/buzz-cli", - ".goose", - ".goose/skills", - ".claude", - ".claude/skills", - ".codex", - ".codex/skills", - ] { - let path = root.join(dir); - let mode = fs::metadata(&path).unwrap().permissions().mode() & 0o777; - assert_eq!(mode, 0o700, "{dir} should be 700"); - } - } - - #[cfg(unix)] - #[test] - fn ensure_nest_skips_permissions_on_symlinked_child() { - use std::os::unix::fs::PermissionsExt; - - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - - // First call creates the real nest. - ensure_nest_at(&root).unwrap(); - - // Replace REPOS/ with a symlink to an external directory. - let external = tmp.path().join("external"); - fs::create_dir(&external).unwrap(); - fs::set_permissions(&external, fs::Permissions::from_mode(0o755)).unwrap(); - fs::remove_dir(root.join("REPOS")).unwrap(); - std::os::unix::fs::symlink(&external, root.join("REPOS")).unwrap(); - - // Second call should succeed — it skips chmod on the symlinked child. - ensure_nest_at(&root).unwrap(); - - // The external directory's permissions should be unchanged (755, not 700). - let mode = fs::metadata(&external).unwrap().permissions().mode() & 0o777; - assert_eq!( - mode, 0o755, - "symlinked child's target should not be chmod'd" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_nest_migrates_old_skill_dir() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - - // Simulate a pre-migration install: real directory at old path. - // Create the nest first to get all dirs, then simulate old layout. - ensure_nest_at(&root).unwrap(); - - // Remove the symlink and new skill dir, recreate old real dir. - let _ = fs::remove_file(root.join(".claude/skills/buzz-cli")); - let _ = fs::remove_dir_all(root.join(".agents/skills/buzz-cli")); - let old_skill_dir = root.join(".claude/skills/buzz-cli"); - fs::create_dir_all(&old_skill_dir).unwrap(); - fs::write(old_skill_dir.join("SKILL.md"), "user edited skill").unwrap(); - - // Delete version file to force refresh. - let _ = fs::remove_file(root.join(".agents/skills/buzz-cli/.skill-version")); - - // Re-run ensure_nest_at — should trigger migration in refresh_skill_md_if_stale. - ensure_nest_at(&root).unwrap(); - - // New canonical location exists with user's content preserved. - let new_skill = root.join(".agents/skills/buzz-cli/SKILL.md"); - assert!(new_skill.exists(), "SKILL.md should exist at new path"); - assert_eq!(fs::read_to_string(&new_skill).unwrap(), "user edited skill"); - - // Old path is now a symlink, not a real directory. - let old_path = root.join(".claude/skills/buzz-cli"); - assert!( - old_path - .symlink_metadata() - .unwrap() - .file_type() - .is_symlink(), - "old path should now be a symlink" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_skill_symlinks_are_idempotent() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - // Second call should succeed without errors. - ensure_nest_at(&root).unwrap(); - // All symlinks still valid and point to relative targets. - for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { - let link = root.join(dir).join("buzz-cli"); - assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); - assert!( - link.join("SKILL.md").exists(), - "symlink at {dir}/buzz-cli should resolve to dir with SKILL.md" - ); - let target = fs::read_link(&link).unwrap(); - assert_eq!( - target.to_str().unwrap(), - format!("../../{CANONICAL_SKILL_DIR}"), - "symlink at {dir}/buzz-cli should use relative target" - ); - } - } - - #[cfg(unix)] - #[test] - fn ensure_skill_symlinks_skips_existing_path_during_initial_pass() { - // ensure_skill_symlinks skips any path where symlink_metadata succeeds. - // However, refresh_skill_md_if_stale (called after ensure_skill_symlinks) - // migrates pre-existing real directories at .claude/skills/buzz-cli to - // symlinks. This test verifies the end-to-end behavior: a pre-existing real - // dir at the claude path is migrated to a symlink. - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - // Pre-create a real directory where a symlink would go. - let real_dir = root.join(".claude/skills/buzz-cli"); - fs::create_dir_all(&real_dir).unwrap(); - // Place SKILL.md so migration preserves it. - fs::write(real_dir.join("SKILL.md"), "custom skill content").unwrap(); - - ensure_nest_at(&root).unwrap(); - - // Migration converts the real dir to a symlink; content is moved to canonical path. - assert!( - real_dir - .symlink_metadata() - .unwrap() - .file_type() - .is_symlink(), - ".claude/skills/buzz-cli should be migrated to a symlink" - ); - // The canonical path now holds the migrated content. - let canonical = root.join(".agents/skills/buzz-cli/SKILL.md"); - assert_eq!( - fs::read_to_string(&canonical).unwrap(), - "custom skill content" - ); - } - - #[cfg(unix)] - #[test] - fn ensure_skill_symlinks_skip_dangling_symlink() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - // Pre-create a dangling symlink where the .codex link would go. - let codex_skills = root.join(".codex/skills"); - fs::create_dir_all(&codex_skills).unwrap(); - let dangling = codex_skills.join("buzz-cli"); - std::os::unix::fs::symlink("/nonexistent/target", &dangling).unwrap(); - - ensure_nest_at(&root).unwrap(); - - // Dangling symlink should be left alone (not clobbered). - assert!(dangling - .symlink_metadata() - .unwrap() - .file_type() - .is_symlink()); - assert_eq!( - fs::read_link(&dangling).unwrap().to_str().unwrap(), - "/nonexistent/target" - ); - } - - #[test] - fn cli_link_name_prod_is_buzz() { - assert_eq!(cli_link_name(false), "buzz"); - } - - #[test] - fn cli_link_name_dev_is_buzz_dev() { - assert_eq!(cli_link_name(true), "buzz-dev"); - } - - #[cfg(unix)] - #[test] - fn ensure_cli_symlink_creates_symlink_prod() { - let tmp = tempfile::tempdir().unwrap(); - let exe_parent = tmp.path().join("MacOS"); - fs::create_dir(&exe_parent).unwrap(); - fs::write(exe_parent.join("buzz"), "binary").unwrap(); - - let local_bin = tmp.path().join("local_bin"); - fs::create_dir_all(&local_bin).unwrap(); - - // Prod link name is "buzz"; simulate the symlink creation path. - let link = local_bin.join(cli_link_name(false)); - std::os::unix::fs::symlink(exe_parent.join("buzz"), &link).unwrap(); - assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); - assert_eq!(fs::read_link(&link).unwrap(), exe_parent.join("buzz")); - } - - #[cfg(unix)] - #[test] - fn ensure_cli_symlink_creates_symlink_dev() { - let tmp = tempfile::tempdir().unwrap(); - let exe_parent = tmp.path().join("MacOS"); - fs::create_dir(&exe_parent).unwrap(); - fs::write(exe_parent.join("buzz"), "binary").unwrap(); - - let local_bin = tmp.path().join("local_bin"); - fs::create_dir_all(&local_bin).unwrap(); - - // Dev link must be "buzz-dev", never "buzz". - assert_eq!(cli_link_name(true), "buzz-dev"); - - let link = local_bin.join(cli_link_name(true)); - std::os::unix::fs::symlink(exe_parent.join("buzz"), &link).unwrap(); - assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); - assert_eq!(fs::read_link(&link).unwrap(), exe_parent.join("buzz")); - // Prod link must not exist — the two builds don't touch each other. - assert!(!local_bin.join("buzz").exists()); - } - - #[cfg(unix)] - #[test] - fn ensure_cli_symlink_does_not_clobber_regular_file_prod() { - let tmp = tempfile::tempdir().unwrap(); - let local_bin = tmp.path().join("local_bin"); - fs::create_dir_all(&local_bin).unwrap(); - let link = local_bin.join(cli_link_name(false)); - fs::write(&link, "user-installed binary").unwrap(); - - // Regular files are preserved — the Ok(_) branch skips them. - assert!(link.symlink_metadata().unwrap().file_type().is_file()); - assert_eq!(fs::read_to_string(&link).unwrap(), "user-installed binary"); - } - - #[cfg(unix)] - #[test] - fn ensure_cli_symlink_does_not_clobber_regular_file_dev() { - let tmp = tempfile::tempdir().unwrap(); - let local_bin = tmp.path().join("local_bin"); - fs::create_dir_all(&local_bin).unwrap(); - let link = local_bin.join(cli_link_name(true)); - fs::write(&link, "user-installed buzz-dev binary").unwrap(); - - // Regular files at the dev path are also preserved. - assert!(link.symlink_metadata().unwrap().file_type().is_file()); - assert_eq!( - fs::read_to_string(&link).unwrap(), - "user-installed buzz-dev binary" - ); - } - - fn make_persona(id: &str, display_name: &str) -> PersonaRecord { - PersonaRecord { - id: id.to_string(), - display_name: display_name.to_string(), - avatar_url: None, - system_prompt: String::new(), - runtime: None, - model: None, - provider: None, - name_pool: vec![], - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - env_vars: std::collections::BTreeMap::new(), - created_at: String::new(), - updated_at: String::new(), - } - } - - fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { - ManagedAgentRecord { - pubkey: String::new(), - name: name.to_string(), - persona_id: persona_id.map(|s| s.to_string()), - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: String::new(), - agent_command: String::new(), - agent_command_override: None, - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 0, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - mcp_toolsets: None, - start_on_app_launch: false, - runtime_pid: None, - backend: BackendKind::default(), - backend_agent_id: None, - provider_binary_path: None, - persona_team_dir: None, - persona_name_in_team: None, - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - respond_to: RespondTo::default(), - respond_to_allowlist: vec![], - env_vars: std::collections::BTreeMap::new(), - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - relay_mesh: None, - } - } - - #[test] - fn test_render_dynamic_section_with_agents() { - let personas = vec![make_persona("p1", "Builder")]; - let agents = vec![make_agent("Kit", Some("p1"))]; - let output = render_dynamic_section(&personas, &agents, "ws://example.com:3000"); - assert!(output.contains("| Kit | Builder | @Kit |")); - assert!(output.contains("| Name | Persona | How to address |")); - assert!(output.contains("## Workspace")); - } - - #[test] - fn test_render_dynamic_section_empty() { - let output = render_dynamic_section(&[], &[], "ws://example.com:3000"); - assert!(output.contains("No agents deployed yet")); - } - - #[test] - fn test_render_dynamic_section_agent_no_persona() { - let personas = vec![make_persona("p1", "Builder")]; - let agents = vec![make_agent("Scout", Some("nonexistent"))]; - let output = render_dynamic_section(&personas, &agents, "ws://example.com:3000"); - assert!(output.contains("| Scout | — | @Scout |")); - } - - #[test] - fn test_upsert_managed_section_with_markers() { - let tmp = tempfile::tempdir().unwrap(); - let file = tmp.path().join("AGENTS.md"); - fs::write( - &file, - "# Header\n\nsome content\n\n\nold section\n\n\nafter\n", - ) - .unwrap(); - - upsert_managed_section(&file, "new section").unwrap(); - - let result = fs::read_to_string(&file).unwrap(); - assert!(result.contains("")); - assert!(result.contains("new section")); - assert!(!result.contains("old section")); - assert!(result.contains("# Header")); - assert!(result.contains("some content")); - assert!(result.contains("after")); - } - - #[test] - fn test_upsert_managed_section_without_markers() { - let tmp = tempfile::tempdir().unwrap(); - let file = tmp.path().join("AGENTS.md"); - fs::write(&file, "# Header\n\nexisting content\n").unwrap(); - - upsert_managed_section(&file, "injected section").unwrap(); - - let result = fs::read_to_string(&file).unwrap(); - assert!(result.contains("# Header")); - assert!(result.contains("existing content")); - assert!(result.contains("")); - assert!(result.contains("injected section")); - let begin_pos = result.find("\nsome middle content\n\nold section\n", - ) - .unwrap(); - - upsert_managed_section(&file, "new section").unwrap(); - - let result = fs::read_to_string(&file).unwrap(); - - assert!(result.contains("# Header"), "original header must survive"); - assert!( - result.contains("new section"), - "new content must be present" - ); - assert!( - result.contains("some middle content"), - "content between markers must survive" - ); - - // Exactly one BEGIN marker in the output (the orphan was stripped, new one appended). - assert_eq!( - result.matches(BEGIN_MARKER).count(), - 1, - "exactly one BEGIN marker after orphan cleanup" - ); - - // The single BEGIN marker must have a matching END marker after it. - let begin_pos = result - .find(BEGIN_MARKER) - .expect("BEGIN marker must be present"); - let end_pos = result[begin_pos..].find(END_MARKER).map(|p| begin_pos + p); - assert!( - end_pos.is_some(), - "an END marker must appear after the appended BEGIN marker" - ); - } - - #[test] - fn test_upsert_begin_only_no_end() { - // A file with BEGIN but no END has an orphan marker. - // find_managed_markers returns None (no END found after BEGIN), - // so strip_orphan_begin_marker removes the BEGIN line. - // Content that followed the orphan BEGIN is preserved (only the marker line is stripped, - // not the body that came after it). - let tmp = tempfile::tempdir().unwrap(); - let file = tmp.path().join("AGENTS.md"); - fs::write( - &file, - "# Header\n\nsome content\n\n\norphaned section without end marker\n", - ) - .unwrap(); - - upsert_managed_section(&file, "fresh section").unwrap(); - - let result = fs::read_to_string(&file).unwrap(); - - assert!(result.contains("# Header"), "original header must survive"); - assert!( - result.contains("some content"), - "original body must survive" - ); - assert!( - result.contains("fresh section"), - "new content must be present" - ); - - let begin_pos = result - .find(BEGIN_MARKER) - .expect("BEGIN marker must be present"); - let end_pos = result.find(END_MARKER).expect("END marker must be present"); - assert!( - begin_pos < end_pos, - "the appended BEGIN marker must precede the appended END marker" - ); - - // Exactly one BEGIN marker after orphan cleanup. - assert_eq!( - result.matches(BEGIN_MARKER).count(), - 1, - "exactly one BEGIN marker after orphan cleanup" - ); - } - - #[test] - fn test_upsert_duplicate_markers() { - let tmp = tempfile::tempdir().unwrap(); - let file = tmp.path().join("AGENTS.md"); - fs::write( - &file, - "# Header\n\n\nfirst block\n\n\nbetween blocks\n\n\nsecond block\n\n", - ) - .unwrap(); - - upsert_managed_section(&file, "replaced").unwrap(); - - let result = fs::read_to_string(&file).unwrap(); - - assert!( - result.contains("replaced"), - "replacement content must be present" - ); - assert!( - !result.contains("first block"), - "first block must be replaced" - ); - assert!( - result.contains("second block"), - "second pair content must survive" - ); - assert!( - result.contains("between blocks"), - "text between pairs must survive" - ); - } - - #[test] - fn test_upsert_marker_in_code_block() { - let tmp = tempfile::tempdir().unwrap(); - let file = tmp.path().join("AGENTS.md"); - // Indented by 4 spaces — not at column 0, so should NOT match as a real marker. - fs::write( - &file, - "# Header\n\n \n\nReal content here\n", - ) - .unwrap(); - - upsert_managed_section(&file, "appended content").unwrap(); - - let result = fs::read_to_string(&file).unwrap(); - - assert!( - result.contains(" "), - "indented marker inside code block must be preserved verbatim" - ); - assert!( - result.contains("appended content"), - "new content must be appended" - ); - assert!( - result.contains("Real content here"), - "existing body must survive" - ); - - // The real markers appended at the end must be at line-start (column 0). - let begin_pos = result - .find("\nexisting section\n\n", - ) - .unwrap(); - - upsert_managed_section(&file, "same content").unwrap(); - let after_first = fs::read_to_string(&file).unwrap(); - - upsert_managed_section(&file, "same content").unwrap(); - let after_second = fs::read_to_string(&file).unwrap(); - - assert_eq!( - after_first, after_second, - "upsert must be idempotent: second call must not alter the file" - ); - } - - #[test] - fn refresh_agents_md_writes_version_file() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - let version = fs::read_to_string(root.join(".nest-agents-version")).unwrap(); - assert_eq!(version.trim(), NEST_AGENTS_VERSION.to_string()); - } - - #[test] - fn refresh_skill_md_writes_version_file() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - let version = - fs::read_to_string(root.join(".agents/skills/buzz-cli/.skill-version")).unwrap(); - assert_eq!(version.trim(), NEST_SKILL_VERSION.to_string()); - } - - #[test] - fn refresh_agents_md_preserves_managed_section() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - - // Simulate a managed section update. - let agents_md = root.join("AGENTS.md"); - upsert_managed_section( - &agents_md, - "## Active Agents\n\n| Name | Role |\n|------|------|\n| Kit | Builder |", - ) - .unwrap(); - - // Remove version file to simulate an upgrade. - fs::remove_file(root.join(".nest-agents-version")).unwrap(); - - // Re-run ensure_nest_at (triggers refresh). - ensure_nest_at(&root).unwrap(); - - let content = fs::read_to_string(&agents_md).unwrap(); - // Static content should be refreshed (from template). - assert!( - content.starts_with("# Buzz Nest"), - "template header must be present" - ); - // Managed section should be preserved. - assert!( - content.contains("Kit"), - "managed section agent table must survive refresh" - ); - assert!(content.contains(BEGIN_MARKER), "BEGIN marker must survive"); - assert!(content.contains(END_MARKER), "END marker must survive"); - } - - #[test] - fn refresh_skips_when_version_current() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - - // Manually change AGENTS.md content after version file is written. - let agents_md = root.join("AGENTS.md"); - fs::write(&agents_md, "user modified content").unwrap(); - - // Re-run ensure_nest_at — version file is current, so no refresh. - ensure_nest_at(&root).unwrap(); - - let content = fs::read_to_string(&agents_md).unwrap(); - assert_eq!( - content, "user modified content", - "should not overwrite when version is current" - ); - } - - #[test] - fn refresh_skill_overwrites_on_version_bump() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path().join(".buzz"); - ensure_nest_at(&root).unwrap(); - - let skill_md = root.join(".agents/skills/buzz-cli/SKILL.md"); - fs::write(&skill_md, "stale skill content").unwrap(); - - // Remove version file to simulate upgrade. - let _ = fs::remove_file(root.join(".agents/skills/buzz-cli/.skill-version")); - - ensure_nest_at(&root).unwrap(); - - let content = fs::read_to_string(&skill_md).unwrap(); - assert_eq!( - content, BUZZ_CLI_SKILL_MD, - "SKILL.md must be refreshed on version bump" - ); - } -} +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs new file mode 100644 index 0000000000..8b273f9769 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -0,0 +1,901 @@ +use super::*; + +#[test] +fn nest_dir_is_under_home() { + if let Some(dir) = nest_dir() { + // Accepts both .buzz (prod) and .buzz-dev (dev) depending on + // whether init_nest_dir was called before this test ran. + let name = dir.file_name().and_then(|n| n.to_str()).unwrap_or(""); + assert!( + name == NEST_DIR_PROD || name == NEST_DIR_DEV, + "nest_dir must end with .buzz or .buzz-dev, got {dir:?}" + ); + } +} + +#[test] +fn init_nest_dir_prod_sets_buzz() { + // init_nest_dir is idempotent (OnceLock) — once set, subsequent calls + // are no-ops. We can only test the fallback path if the OnceLock is + // unset, which is only true in a fresh process. Instead, verify that + // nest_dir() always returns a path ending with a valid nest suffix. + let dir = nest_dir(); + if let Some(d) = dir { + let name = d.file_name().and_then(|n| n.to_str()).unwrap_or(""); + assert!( + name == NEST_DIR_PROD || name == NEST_DIR_DEV, + "nest_dir suffix must be .buzz or .buzz-dev, got {d:?}" + ); + } +} + +#[test] +fn ensure_nest_creates_all_dirs_and_agents_md() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + + ensure_nest_at(&root).unwrap(); + + // All subdirectories exist. + for dir in NEST_DIRS { + assert!(root.join(dir).is_dir(), "{dir}/ should exist"); + } + // REPOS is provisioned separately (may be a symlink); with no + // repos_dir configured it lands as a real directory. + assert!(root.join("REPOS").is_dir(), "REPOS/ should exist"); + + // AGENTS.md was written with default content. + let content = fs::read_to_string(root.join("AGENTS.md")).unwrap(); + assert_eq!(content, AGENTS_MD); + + // Permissions are 700 on Unix for root and all subdirs. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = fs::metadata(&root).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o700, "root should be 700"); + for dir in NEST_DIRS { + let mode = fs::metadata(root.join(dir)).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o700, "{dir}/ should be 700"); + } + let repos_mode = fs::metadata(root.join("REPOS")) + .unwrap() + .permissions() + .mode() + & 0o777; + assert_eq!(repos_mode, 0o700, "REPOS/ should be 700"); + } +} + +#[test] +fn ensure_nest_is_idempotent_and_preserves_custom_content() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + + // First call creates everything. + ensure_nest_at(&root).unwrap(); + + // User customizes AGENTS.md. + let agents = root.join("AGENTS.md"); + fs::write(&agents, "my custom instructions").unwrap(); + + // Second call succeeds and does not overwrite. + ensure_nest_at(&root).unwrap(); + + assert_eq!( + fs::read_to_string(&agents).unwrap(), + "my custom instructions" + ); + + // All dirs still exist. + for dir in NEST_DIRS { + assert!(root.join(dir).is_dir(), "{dir}/ should still exist"); + } +} + +#[cfg(unix)] +#[test] +fn ensure_nest_rejects_symlink_root() { + let tmp = tempfile::tempdir().unwrap(); + let target = tmp.path().join("real_dir"); + fs::create_dir(&target).unwrap(); + let link = tmp.path().join(".buzz"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + + let result = ensure_nest_at(&link); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("symlink")); +} + +#[test] +fn ensure_nest_creates_skill_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + + // Canonical location under .agents. + let skill = root.join(".agents/skills/buzz-cli/SKILL.md"); + assert!(skill.exists(), "SKILL.md should exist at .agents path"); + let content = fs::read_to_string(&skill).unwrap(); + assert_eq!(content, BUZZ_CLI_SKILL_MD); + + // On unix, harness-specific symlinks should resolve to the canonical dir. + #[cfg(unix)] + { + for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { + let link = root.join(dir).join("buzz-cli"); + assert!( + link.symlink_metadata().unwrap().file_type().is_symlink(), + "{dir}/buzz-cli should be a symlink" + ); + assert!( + link.join("SKILL.md").exists(), + "symlink at {dir}/buzz-cli should resolve to dir with SKILL.md" + ); + } + } +} + +#[test] +fn ensure_nest_does_not_overwrite_skill_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + + let skill = root.join(".agents/skills/buzz-cli/SKILL.md"); + fs::write(&skill, "custom skill content").unwrap(); + + ensure_nest_at(&root).unwrap(); + assert_eq!(fs::read_to_string(&skill).unwrap(), "custom skill content"); +} + +#[cfg(unix)] +#[test] +fn ensure_nest_skill_dir_has_700_permissions() { + use std::os::unix::fs::PermissionsExt; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + // Canonical path and all provider parent dirs should be locked down. + // Symlinks (e.g. .goose/skills/buzz-cli) are skipped by the chmod loop. + for dir in [ + ".agents", + ".agents/skills", + ".agents/skills/buzz-cli", + ".goose", + ".goose/skills", + ".claude", + ".claude/skills", + ".codex", + ".codex/skills", + ] { + let path = root.join(dir); + let mode = fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o700, "{dir} should be 700"); + } +} + +#[cfg(unix)] +#[test] +fn ensure_nest_skips_permissions_on_symlinked_child() { + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + + // First call creates the real nest. + ensure_nest_at(&root).unwrap(); + + // Replace REPOS/ with a symlink to an external directory. + let external = tmp.path().join("external"); + fs::create_dir(&external).unwrap(); + fs::set_permissions(&external, fs::Permissions::from_mode(0o755)).unwrap(); + fs::remove_dir(root.join("REPOS")).unwrap(); + std::os::unix::fs::symlink(&external, root.join("REPOS")).unwrap(); + + // Second call should succeed — it skips chmod on the symlinked child. + ensure_nest_at(&root).unwrap(); + + // The external directory's permissions should be unchanged (755, not 700). + let mode = fs::metadata(&external).unwrap().permissions().mode() & 0o777; + assert_eq!( + mode, 0o755, + "symlinked child's target should not be chmod'd" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_nest_migrates_old_skill_dir() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + + // Simulate a pre-migration install: real directory at old path. + // Create the nest first to get all dirs, then simulate old layout. + ensure_nest_at(&root).unwrap(); + + // Remove the symlink and new skill dir, recreate old real dir. + let _ = fs::remove_file(root.join(".claude/skills/buzz-cli")); + let _ = fs::remove_dir_all(root.join(".agents/skills/buzz-cli")); + let old_skill_dir = root.join(".claude/skills/buzz-cli"); + fs::create_dir_all(&old_skill_dir).unwrap(); + fs::write(old_skill_dir.join("SKILL.md"), "user edited skill").unwrap(); + + // Delete version file to force refresh. + let _ = fs::remove_file(root.join(".agents/skills/buzz-cli/.skill-version")); + + // Re-run ensure_nest_at — should trigger migration in refresh_skill_md_if_stale. + ensure_nest_at(&root).unwrap(); + + // New canonical location exists with user's content preserved. + let new_skill = root.join(".agents/skills/buzz-cli/SKILL.md"); + assert!(new_skill.exists(), "SKILL.md should exist at new path"); + assert_eq!(fs::read_to_string(&new_skill).unwrap(), "user edited skill"); + + // Old path is now a symlink, not a real directory. + let old_path = root.join(".claude/skills/buzz-cli"); + assert!( + old_path + .symlink_metadata() + .unwrap() + .file_type() + .is_symlink(), + "old path should now be a symlink" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_skill_symlinks_are_idempotent() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + // Second call should succeed without errors. + ensure_nest_at(&root).unwrap(); + // All symlinks still valid and point to relative targets. + for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { + let link = root.join(dir).join("buzz-cli"); + assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); + assert!( + link.join("SKILL.md").exists(), + "symlink at {dir}/buzz-cli should resolve to dir with SKILL.md" + ); + let target = fs::read_link(&link).unwrap(); + assert_eq!( + target.to_str().unwrap(), + format!("../../{CANONICAL_SKILL_DIR}"), + "symlink at {dir}/buzz-cli should use relative target" + ); + } +} + +#[cfg(unix)] +#[test] +fn ensure_skill_symlinks_skips_existing_path_during_initial_pass() { + // ensure_skill_symlinks skips any path where symlink_metadata succeeds. + // However, refresh_skill_md_if_stale (called after ensure_skill_symlinks) + // migrates pre-existing real directories at .claude/skills/buzz-cli to + // symlinks. This test verifies the end-to-end behavior: a pre-existing real + // dir at the claude path is migrated to a symlink. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + // Pre-create a real directory where a symlink would go. + let real_dir = root.join(".claude/skills/buzz-cli"); + fs::create_dir_all(&real_dir).unwrap(); + // Place SKILL.md so migration preserves it. + fs::write(real_dir.join("SKILL.md"), "custom skill content").unwrap(); + + ensure_nest_at(&root).unwrap(); + + // Migration converts the real dir to a symlink; content is moved to canonical path. + assert!( + real_dir + .symlink_metadata() + .unwrap() + .file_type() + .is_symlink(), + ".claude/skills/buzz-cli should be migrated to a symlink" + ); + // The canonical path now holds the migrated content. + let canonical = root.join(".agents/skills/buzz-cli/SKILL.md"); + assert_eq!( + fs::read_to_string(&canonical).unwrap(), + "custom skill content" + ); +} + +#[cfg(unix)] +#[test] +fn ensure_skill_symlinks_skip_dangling_symlink() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + // Pre-create a dangling symlink where the .codex link would go. + let codex_skills = root.join(".codex/skills"); + fs::create_dir_all(&codex_skills).unwrap(); + let dangling = codex_skills.join("buzz-cli"); + std::os::unix::fs::symlink("/nonexistent/target", &dangling).unwrap(); + + ensure_nest_at(&root).unwrap(); + + // Dangling symlink should be left alone (not clobbered). + assert!(dangling + .symlink_metadata() + .unwrap() + .file_type() + .is_symlink()); + assert_eq!( + fs::read_link(&dangling).unwrap().to_str().unwrap(), + "/nonexistent/target" + ); +} + +#[test] +fn cli_link_name_prod_is_buzz() { + assert_eq!(cli_link_name(false), "buzz"); +} + +#[test] +fn cli_link_name_dev_is_buzz_dev() { + assert_eq!(cli_link_name(true), "buzz-dev"); +} + +#[cfg(unix)] +#[test] +fn ensure_cli_symlink_creates_symlink_prod() { + let tmp = tempfile::tempdir().unwrap(); + let exe_parent = tmp.path().join("MacOS"); + fs::create_dir(&exe_parent).unwrap(); + fs::write(exe_parent.join("buzz"), "binary").unwrap(); + + let local_bin = tmp.path().join("local_bin"); + fs::create_dir_all(&local_bin).unwrap(); + + // Prod link name is "buzz"; simulate the symlink creation path. + let link = local_bin.join(cli_link_name(false)); + std::os::unix::fs::symlink(exe_parent.join("buzz"), &link).unwrap(); + assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); + assert_eq!(fs::read_link(&link).unwrap(), exe_parent.join("buzz")); +} + +#[cfg(unix)] +#[test] +fn ensure_cli_symlink_creates_symlink_dev() { + let tmp = tempfile::tempdir().unwrap(); + let exe_parent = tmp.path().join("MacOS"); + fs::create_dir(&exe_parent).unwrap(); + fs::write(exe_parent.join("buzz"), "binary").unwrap(); + + let local_bin = tmp.path().join("local_bin"); + fs::create_dir_all(&local_bin).unwrap(); + + // Dev link must be "buzz-dev", never "buzz". + assert_eq!(cli_link_name(true), "buzz-dev"); + + let link = local_bin.join(cli_link_name(true)); + std::os::unix::fs::symlink(exe_parent.join("buzz"), &link).unwrap(); + assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); + assert_eq!(fs::read_link(&link).unwrap(), exe_parent.join("buzz")); + // Prod link must not exist — the two builds don't touch each other. + assert!(!local_bin.join("buzz").exists()); +} + +#[cfg(unix)] +#[test] +fn ensure_cli_symlink_does_not_clobber_regular_file_prod() { + let tmp = tempfile::tempdir().unwrap(); + let local_bin = tmp.path().join("local_bin"); + fs::create_dir_all(&local_bin).unwrap(); + let link = local_bin.join(cli_link_name(false)); + fs::write(&link, "user-installed binary").unwrap(); + + // Regular files are preserved — the Ok(_) branch skips them. + assert!(link.symlink_metadata().unwrap().file_type().is_file()); + assert_eq!(fs::read_to_string(&link).unwrap(), "user-installed binary"); +} + +#[cfg(unix)] +#[test] +fn ensure_cli_symlink_does_not_clobber_regular_file_dev() { + let tmp = tempfile::tempdir().unwrap(); + let local_bin = tmp.path().join("local_bin"); + fs::create_dir_all(&local_bin).unwrap(); + let link = local_bin.join(cli_link_name(true)); + fs::write(&link, "user-installed buzz-dev binary").unwrap(); + + // Regular files at the dev path are also preserved. + assert!(link.symlink_metadata().unwrap().file_type().is_file()); + assert_eq!( + fs::read_to_string(&link).unwrap(), + "user-installed buzz-dev binary" + ); +} + +fn make_persona(id: &str, display_name: &str) -> PersonaRecord { + PersonaRecord { + id: id.to_string(), + display_name: display_name.to_string(), + avatar_url: None, + system_prompt: String::new(), + runtime: None, + model: None, + provider: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: std::collections::BTreeMap::new(), + created_at: String::new(), + updated_at: String::new(), + } +} + +fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: String::new(), + name: name.to_string(), + persona_id: persona_id.map(|s| s.to_string()), + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: String::new(), + agent_command: String::new(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 0, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + mcp_toolsets: None, + start_on_app_launch: false, + runtime_pid: None, + backend: BackendKind::default(), + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: RespondTo::default(), + respond_to_allowlist: vec![], + env_vars: std::collections::BTreeMap::new(), + display_name: None, + slug: None, + runtime: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + relay_mesh: None, + } +} + +#[test] +fn test_render_dynamic_section_with_agents() { + let personas = vec![make_persona("p1", "Builder")]; + let agents = vec![make_agent("Kit", Some("p1"))]; + let output = render_dynamic_section(&personas, &agents, "ws://example.com:3000"); + assert!(output.contains("| Kit | Builder | @Kit |")); + assert!(output.contains("| Name | Persona | How to address |")); + assert!(output.contains("## Workspace")); +} + +#[test] +fn test_render_dynamic_section_empty() { + let output = render_dynamic_section(&[], &[], "ws://example.com:3000"); + assert!(output.contains("No agents deployed yet")); +} + +#[test] +fn test_render_dynamic_section_agent_no_persona() { + let personas = vec![make_persona("p1", "Builder")]; + let agents = vec![make_agent("Scout", Some("nonexistent"))]; + let output = render_dynamic_section(&personas, &agents, "ws://example.com:3000"); + assert!(output.contains("| Scout | — | @Scout |")); +} + +#[test] +fn test_upsert_managed_section_with_markers() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write( + &file, + "# Header\n\nsome content\n\n\nold section\n\n\nafter\n", + ) + .unwrap(); + + upsert_managed_section(&file, "new section").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + assert!(result.contains("")); + assert!(result.contains("new section")); + assert!(!result.contains("old section")); + assert!(result.contains("# Header")); + assert!(result.contains("some content")); + assert!(result.contains("after")); +} + +#[test] +fn test_upsert_managed_section_without_markers() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write(&file, "# Header\n\nexisting content\n").unwrap(); + + upsert_managed_section(&file, "injected section").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + assert!(result.contains("# Header")); + assert!(result.contains("existing content")); + assert!(result.contains("")); + assert!(result.contains("injected section")); + let begin_pos = result.find("\nsome middle content\n\nold section\n", + ) + .unwrap(); + + upsert_managed_section(&file, "new section").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!(result.contains("# Header"), "original header must survive"); + assert!( + result.contains("new section"), + "new content must be present" + ); + assert!( + result.contains("some middle content"), + "content between markers must survive" + ); + + // Exactly one BEGIN marker in the output (the orphan was stripped, new one appended). + assert_eq!( + result.matches(BEGIN_MARKER).count(), + 1, + "exactly one BEGIN marker after orphan cleanup" + ); + + // The single BEGIN marker must have a matching END marker after it. + let begin_pos = result + .find(BEGIN_MARKER) + .expect("BEGIN marker must be present"); + let end_pos = result[begin_pos..].find(END_MARKER).map(|p| begin_pos + p); + assert!( + end_pos.is_some(), + "an END marker must appear after the appended BEGIN marker" + ); +} + +#[test] +fn test_upsert_begin_only_no_end() { + // A file with BEGIN but no END has an orphan marker. + // find_managed_markers returns None (no END found after BEGIN), + // so strip_orphan_begin_marker removes the BEGIN line. + // Content that followed the orphan BEGIN is preserved (only the marker line is stripped, + // not the body that came after it). + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write( + &file, + "# Header\n\nsome content\n\n\norphaned section without end marker\n", + ) + .unwrap(); + + upsert_managed_section(&file, "fresh section").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!(result.contains("# Header"), "original header must survive"); + assert!( + result.contains("some content"), + "original body must survive" + ); + assert!( + result.contains("fresh section"), + "new content must be present" + ); + + let begin_pos = result + .find(BEGIN_MARKER) + .expect("BEGIN marker must be present"); + let end_pos = result.find(END_MARKER).expect("END marker must be present"); + assert!( + begin_pos < end_pos, + "the appended BEGIN marker must precede the appended END marker" + ); + + // Exactly one BEGIN marker after orphan cleanup. + assert_eq!( + result.matches(BEGIN_MARKER).count(), + 1, + "exactly one BEGIN marker after orphan cleanup" + ); +} + +#[test] +fn test_upsert_duplicate_markers() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + fs::write( + &file, + "# Header\n\n\nfirst block\n\n\nbetween blocks\n\n\nsecond block\n\n", + ) + .unwrap(); + + upsert_managed_section(&file, "replaced").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!( + result.contains("replaced"), + "replacement content must be present" + ); + assert!( + !result.contains("first block"), + "first block must be replaced" + ); + assert!( + result.contains("second block"), + "second pair content must survive" + ); + assert!( + result.contains("between blocks"), + "text between pairs must survive" + ); +} + +#[test] +fn test_upsert_marker_in_code_block() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("AGENTS.md"); + // Indented by 4 spaces — not at column 0, so should NOT match as a real marker. + fs::write( + &file, + "# Header\n\n \n\nReal content here\n", + ) + .unwrap(); + + upsert_managed_section(&file, "appended content").unwrap(); + + let result = fs::read_to_string(&file).unwrap(); + + assert!( + result.contains(" "), + "indented marker inside code block must be preserved verbatim" + ); + assert!( + result.contains("appended content"), + "new content must be appended" + ); + assert!( + result.contains("Real content here"), + "existing body must survive" + ); + + // The real markers appended at the end must be at line-start (column 0). + let begin_pos = result + .find("\nexisting section\n\n", + ) + .unwrap(); + + upsert_managed_section(&file, "same content").unwrap(); + let after_first = fs::read_to_string(&file).unwrap(); + + upsert_managed_section(&file, "same content").unwrap(); + let after_second = fs::read_to_string(&file).unwrap(); + + assert_eq!( + after_first, after_second, + "upsert must be idempotent: second call must not alter the file" + ); +} + +#[test] +fn refresh_agents_md_writes_version_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + let version = fs::read_to_string(root.join(".nest-agents-version")).unwrap(); + assert_eq!(version.trim(), NEST_AGENTS_VERSION.to_string()); +} + +#[test] +fn refresh_skill_md_writes_version_file() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + let version = fs::read_to_string(root.join(".agents/skills/buzz-cli/.skill-version")).unwrap(); + assert_eq!(version.trim(), NEST_SKILL_VERSION.to_string()); +} + +#[test] +fn refresh_agents_md_preserves_managed_section() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + + // Simulate a managed section update. + let agents_md = root.join("AGENTS.md"); + upsert_managed_section( + &agents_md, + "## Active Agents\n\n| Name | Role |\n|------|------|\n| Kit | Builder |", + ) + .unwrap(); + + // Remove version file to simulate an upgrade. + fs::remove_file(root.join(".nest-agents-version")).unwrap(); + + // Re-run ensure_nest_at (triggers refresh). + ensure_nest_at(&root).unwrap(); + + let content = fs::read_to_string(&agents_md).unwrap(); + // Static content should be refreshed (from template). + assert!( + content.starts_with("# Buzz Nest"), + "template header must be present" + ); + // Managed section should be preserved. + assert!( + content.contains("Kit"), + "managed section agent table must survive refresh" + ); + assert!(content.contains(BEGIN_MARKER), "BEGIN marker must survive"); + assert!(content.contains(END_MARKER), "END marker must survive"); +} + +#[test] +fn refresh_skips_when_version_current() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + + // Manually change AGENTS.md content after version file is written. + let agents_md = root.join("AGENTS.md"); + fs::write(&agents_md, "user modified content").unwrap(); + + // Re-run ensure_nest_at — version file is current, so no refresh. + ensure_nest_at(&root).unwrap(); + + let content = fs::read_to_string(&agents_md).unwrap(); + assert_eq!( + content, "user modified content", + "should not overwrite when version is current" + ); +} + +#[test] +fn refresh_skill_overwrites_on_version_bump() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + + let skill_md = root.join(".agents/skills/buzz-cli/SKILL.md"); + fs::write(&skill_md, "stale skill content").unwrap(); + + // Remove version file to simulate upgrade. + let _ = fs::remove_file(root.join(".agents/skills/buzz-cli/.skill-version")); + + ensure_nest_at(&root).unwrap(); + + let content = fs::read_to_string(&skill_md).unwrap(); + assert_eq!( + content, BUZZ_CLI_SKILL_MD, + "SKILL.md must be refreshed on version bump" + ); +} diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 0cc678ce06..bdc81333e4 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -1261,58 +1261,6 @@ fn rename_provider_to_runtime_in_personas(path: &Path) { }); } -/// Materialize each persona-linked agent record's `runtime` from its linked -/// persona (unified agent model, Phase 1A). After this, spawn resolution reads -/// the record's own runtime (`record_agent_command` step 2) instead of the -/// live persona — same effective command by construction, so the spawn-config -/// hash is unchanged and no running agent shows a spurious restart badge (see -/// `spawn_hash::tests::materializing_runtime_keeps_hash_stable`). -/// -/// Idempotent: records that already carry `runtime` are untouched, as are -/// records with no linked persona or a persona without a runtime (both keep -/// resolving through the legacy fallback path unchanged). -pub fn materialize_agent_runtimes(app: &tauri::AppHandle) { - let Ok(current_dir) = app.path().app_data_dir() else { - return; - }; - let mut dirs = vec![current_dir.clone()]; - if let Some(canonical) = canonical_dev_data_dir(¤t_dir) { - if canonical.exists() && canonical != current_dir { - dirs.push(canonical); - } - } - for dir in dirs { - let path = dir.join("agents/managed-agents.json"); - if path.exists() { - materialize_runtimes_in_file(&path); - } - } -} - -fn materialize_runtimes_in_file(path: &Path) { - let persona_runtimes = load_persona_runtimes(path); - if persona_runtimes.is_empty() { - return; - } - patch_json_records(path, |obj| { - if obj.contains_key("runtime") { - return false; - } - let Some(runtime) = obj - .get("persona_id") - .and_then(|v| v.as_str()) - .and_then(|pid| persona_runtimes.get(pid)) - else { - return false; - }; - obj.insert( - "runtime".to_string(), - serde_json::Value::String(runtime.clone()), - ); - true - }); -} - pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { let Ok(dir) = app.path().app_data_dir() else { return; @@ -1324,6 +1272,9 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { rename_provider_to_runtime_in_personas(&path); } +mod materialize; +pub use materialize::materialize_agent_runtimes; + #[cfg(test)] #[path = "migration_test_support.rs"] mod test_support; diff --git a/desktop/src-tauri/src/migration/materialize.rs b/desktop/src-tauri/src/migration/materialize.rs new file mode 100644 index 0000000000..5930920dd2 --- /dev/null +++ b/desktop/src-tauri/src/migration/materialize.rs @@ -0,0 +1,140 @@ +//! Phase 1A (unified agent model): boot-time materialization of each +//! persona-linked agent record's `runtime` onto the record itself. +//! +//! Child module of `migration` so it reuses the parent's private JSON-patch +//! helpers (`patch_json_records`, `load_persona_runtimes`, +//! `canonical_dev_data_dir`). + +use std::path::Path; + +use tauri::Manager as _; + +use super::{canonical_dev_data_dir, load_persona_runtimes, patch_json_records}; + +/// Materialize each persona-linked agent record's `runtime` from its linked +/// persona (unified agent model, Phase 1A). After this, spawn resolution reads +/// the record's own runtime (`record_agent_command` step 2) instead of the +/// live persona — same effective command by construction, so the spawn-config +/// hash is unchanged and no running agent shows a spurious restart badge (see +/// `spawn_hash::tests::materializing_runtime_keeps_hash_stable`). +/// +/// Idempotent: records that already carry `runtime` are untouched, as are +/// records with no linked persona or a persona without a runtime (both keep +/// resolving through the legacy fallback path unchanged). +pub fn materialize_agent_runtimes(app: &tauri::AppHandle) { + let Ok(current_dir) = app.path().app_data_dir() else { + return; + }; + let mut dirs = vec![current_dir.clone()]; + if let Some(canonical) = canonical_dev_data_dir(¤t_dir) { + if canonical.exists() && canonical != current_dir { + dirs.push(canonical); + } + } + for dir in dirs { + let path = dir.join("agents/managed-agents.json"); + if path.exists() { + materialize_runtimes_in_file(&path); + } + } +} + +fn materialize_runtimes_in_file(path: &Path) { + let persona_runtimes = load_persona_runtimes(path); + if persona_runtimes.is_empty() { + return; + } + patch_json_records(path, |obj| { + if obj.contains_key("runtime") { + return false; + } + let Some(runtime) = obj + .get("persona_id") + .and_then(|v| v.as_str()) + .and_then(|pid| persona_runtimes.get(pid)) + else { + return false; + }; + obj.insert( + "runtime".to_string(), + serde_json::Value::String(runtime.clone()), + ); + true + }); +} + +#[cfg(test)] +mod tests { + use super::materialize_runtimes_in_file; + use crate::migration::test_support::{ + read_agents_json, write_agents_json, write_personas_json, + }; + + #[test] + fn materialize_runtimes_copies_persona_runtime_onto_record() { + let dir = tempfile::tempdir().unwrap(); + write_personas_json( + dir.path(), + &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([{ "name": "Fizz", "persona_id": "persona-1" }]), + ); + materialize_runtimes_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["runtime"], "goose"); + } + + #[test] + fn materialize_runtimes_is_idempotent_and_preserves_existing() { + // A record that already carries runtime (even one diverging from its + // persona) is never overwritten, and the second run rewrites nothing. + let dir = tempfile::tempdir().unwrap(); + write_personas_json( + dir.path(), + &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + { "name": "Fizz", "persona_id": "persona-1", "runtime": "claude" }, + { "name": "Buzz", "persona_id": "persona-1" } + ]), + ); + let agents_path = dir.path().join("agents/managed-agents.json"); + materialize_runtimes_in_file(&agents_path); + let records = read_agents_json(dir.path()); + assert_eq!( + records[0]["runtime"], "claude", + "existing runtime preserved" + ); + assert_eq!(records[1]["runtime"], "goose"); + + let before = std::fs::read_to_string(&agents_path).unwrap(); + materialize_runtimes_in_file(&agents_path); + let after = std::fs::read_to_string(&agents_path).unwrap(); + assert_eq!(before, after, "second run must be a no-op"); + } + + #[test] + fn materialize_runtimes_skips_unlinked_and_unknown_personas() { + let dir = tempfile::tempdir().unwrap(); + write_personas_json( + dir.path(), + &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + { "name": "NoPersona" }, + { "name": "GonePersona", "persona_id": "deleted" } + ]), + ); + let agents_path = dir.path().join("agents/managed-agents.json"); + let before = std::fs::read_to_string(&agents_path).unwrap(); + materialize_runtimes_in_file(&agents_path); + let after = std::fs::read_to_string(&agents_path).unwrap(); + assert_eq!(before, after, "no linked runtime → untouched file"); + } +} diff --git a/desktop/src-tauri/src/migration_tests.rs b/desktop/src-tauri/src/migration_tests.rs index a8fba9d73b..66ee8dcc7a 100644 --- a/desktop/src-tauri/src/migration_tests.rs +++ b/desktop/src-tauri/src/migration_tests.rs @@ -759,74 +759,6 @@ fn rename_provider_to_runtime_preserves_existing_runtime_over_provider() { assert_eq!(records[0]["provider"], "old-value"); } -#[test] -fn materialize_runtimes_copies_persona_runtime_onto_record() { - let dir = tempfile::tempdir().unwrap(); - write_personas_json( - dir.path(), - &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), - ); - write_agents_json( - dir.path(), - &serde_json::json!([{ "name": "Fizz", "persona_id": "persona-1" }]), - ); - materialize_runtimes_in_file(&dir.path().join("agents/managed-agents.json")); - let records = read_agents_json(dir.path()); - assert_eq!(records[0]["runtime"], "goose"); -} - -#[test] -fn materialize_runtimes_is_idempotent_and_preserves_existing() { - // A record that already carries runtime (even one diverging from its - // persona) is never overwritten, and the second run rewrites nothing. - let dir = tempfile::tempdir().unwrap(); - write_personas_json( - dir.path(), - &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), - ); - write_agents_json( - dir.path(), - &serde_json::json!([ - { "name": "Fizz", "persona_id": "persona-1", "runtime": "claude" }, - { "name": "Buzz", "persona_id": "persona-1" } - ]), - ); - let agents_path = dir.path().join("agents/managed-agents.json"); - materialize_runtimes_in_file(&agents_path); - let records = read_agents_json(dir.path()); - assert_eq!( - records[0]["runtime"], "claude", - "existing runtime preserved" - ); - assert_eq!(records[1]["runtime"], "goose"); - - let before = std::fs::read_to_string(&agents_path).unwrap(); - materialize_runtimes_in_file(&agents_path); - let after = std::fs::read_to_string(&agents_path).unwrap(); - assert_eq!(before, after, "second run must be a no-op"); -} - -#[test] -fn materialize_runtimes_skips_unlinked_and_unknown_personas() { - let dir = tempfile::tempdir().unwrap(); - write_personas_json( - dir.path(), - &serde_json::json!([{ "id": "persona-1", "displayName": "Alice", "runtime": "goose" }]), - ); - write_agents_json( - dir.path(), - &serde_json::json!([ - { "name": "NoPersona" }, - { "name": "GonePersona", "persona_id": "deleted" } - ]), - ); - let agents_path = dir.path().join("agents/managed-agents.json"); - let before = std::fs::read_to_string(&agents_path).unwrap(); - materialize_runtimes_in_file(&agents_path); - let after = std::fs::read_to_string(&agents_path).unwrap(); - assert_eq!(before, after, "no linked runtime → untouched file"); -} - #[test] fn reconcile_mcp_commands_clears_stale_buzz_mcp_server() { let dir = tempfile::tempdir().unwrap();