From 3ed474a8bda14a3f812f19f9bd5333b6f9eafdf4 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Fri, 22 May 2026 16:50:29 -0700 Subject: [PATCH 1/5] fix: re-read persona system_prompt at spawn time for non-pack agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract resolve_effective_prompt_and_model() as a pure, testable function that resolves the effective system prompt and model for a managed agent. When persona_id is set, looks up the persona in the store and returns its current values. Falls back to the record snapshot when no persona is linked or the persona can't be found. Add 5 unit tests covering: - persona_id matches → reads fresh prompt + model - no persona_id → falls back to record snapshot - persona_id not found in store → falls back to record snapshot - no persona and no record prompt → returns None/None - persona has no model → returns None (doesn't inherit record model) --- desktop/scripts/check-file-sizes.mjs | 2 +- .../src-tauri/src/managed_agents/runtime.rs | 231 +++------------- .../src/managed_agents/runtime/tests.rs | 257 ++++++++++++++++++ 3 files changed, 300 insertions(+), 190 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/runtime/tests.rs diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 91e8c6359..9b6b72198 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -51,7 +51,7 @@ const overrides = new Map([ ["src-tauri/src/commands/agents.rs", 881], // remote agent lifecycle routing (local + provider branches) + scope enforcement + persona pack metadata wiring + mcp_toolsets field + NIP-OA auth_tag in deploy payload ["src-tauri/src/commands/messages.rs", 515], // feed multi-query + NIP-50 search + forum thread resolution + thread ref + reactions via REQ + edit_message media_tags param (Slack-style attachment-editable edits) ["src-tauri/src/nostr_convert.rs", 1150], // 12 Nostr event→model converters (channels, profiles, members, notes, search, agents, relay members) + rank_user_search_results helper for NIP-50 user search + 33 unit tests - ["src-tauri/src/managed_agents/runtime.rs", 1110], // ... + respond-to gate env (SPROUT_ACP_RESPOND_TO[_ALLOWLIST]) + per-mode env builder + tests + persona/agent env_vars spawn merge (helper + tests now in env_vars.rs) + ["src-tauri/src/managed_agents/runtime.rs", 895], // spawn logic + respond-to env builder + persona resolution helper (tests extracted to runtime/tests.rs) ["src-tauri/src/managed_agents/discovery.rs", 680], // KNOWN_ACP_PROVIDERS catalog + resolve_command cache + login_shell_path + classify_provider (four-state: Available/AdapterMissing/CliMissing/NotInstalled) + discover_acp_providers with dynamic install_hint + known_acp_provider/known_acp_provider_exact + normalize_agent_args + 15 unit tests ["src-tauri/src/managed_agents/types.rs", 745], // ManagedAgentRecord/Summary + Create/Update request structs + AcpProviderCatalogEntry + InstallRuntimeResult + RespondTo enum + validate_respond_to_allowlist + tests + persona/agent env_vars field ["src-tauri/src/managed_agents/backend.rs", 700], // provider IPC, validation, discovery, binary resolution + tests + redact_secrets_with for user env values + env_secrets_from_request + redact_env_values_in (shared with model discovery) diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 7818522bd..d29d9c648 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -500,6 +500,37 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } +/// Resolve the effective system prompt and model for a managed agent. +/// +/// When `persona_id` is set, looks up the persona in the provided list and +/// returns its current `system_prompt` and `model`. Falls back to the +/// agent record's stale snapshot (`record_prompt`, `record_model`) when: +/// - `persona_id` is `None` +/// - the persona can't be found in the store (e.g. deleted) +/// +/// Extracted from `spawn_agent_child` for testability. +pub(crate) fn resolve_effective_prompt_and_model( + persona_id: Option<&str>, + personas: &[super::PersonaRecord], + record_prompt: Option<&str>, + record_model: Option<&str>, +) -> (Option, Option) { + let from_persona: Option<(String, Option)> = persona_id.and_then(|pid| { + personas + .iter() + .find(|p| p.id == pid) + .map(|p| (p.system_prompt.clone(), p.model.clone())) + }); + + match from_persona { + Some((prompt, model)) => (Some(prompt), model), + None => ( + record_prompt.map(|s| s.to_owned()), + record_model.map(|s| s.to_owned()), + ), + } +} + /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -622,30 +653,16 @@ pub fn spawn_agent_child( command.env("SPROUT_ACP_PERSONA_NAME", persona_name); } - // Resolve system prompt and model: prefer the persona definition (if a - // persona pack is configured and the persona matched), otherwise fall back - // to the record-level overrides. - let has_persona_pack = - record.persona_pack_path.is_some() && record.persona_name_in_pack.is_some(); - let persona_prompt_and_model: Option<(String, Option)> = has_persona_pack - .then(|| { - record - .persona_id - .as_deref() - .and_then(|pid| { - super::load_personas(app) - .ok()? - .into_iter() - .find(|p| p.id == pid) - }) - .map(|p| (p.system_prompt, p.model)) - }) - .flatten(); - - let (effective_prompt, effective_model) = match persona_prompt_and_model { - Some((prompt, model)) => (Some(prompt), model), - None => (record.system_prompt.clone(), record.model.clone()), - }; + // Resolve system prompt and model via the extracted helper. Re-reads + // the persona store at spawn time so edits take effect without + // deleting and recreating the agent. + let personas = super::load_personas(app).unwrap_or_default(); + let (effective_prompt, effective_model) = resolve_effective_prompt_and_model( + record.persona_id.as_deref(), + &personas, + record.system_prompt.as_deref(), + record.model.as_deref(), + ); if let Some(prompt) = &effective_prompt { command.env("SPROUT_ACP_SYSTEM_PROMPT", prompt); @@ -870,168 +887,4 @@ pub fn stop_managed_agent_process( } #[cfg(test)] -mod tests { - use crate::managed_agents::known_acp_provider; - - #[test] - fn sprout_agent_has_mcp_hooks() { - let p = known_acp_provider("sprout-agent").expect("should resolve"); - assert!(p.mcp_hooks); - assert_eq!(p.mcp_command, Some("sprout-dev-mcp")); - } - - #[test] - fn sprout_agent_resolved_via_path() { - assert!(known_acp_provider("/usr/local/bin/sprout-agent").is_some_and(|p| p.mcp_hooks)); - } - - #[test] - fn goose_has_no_mcp_hooks() { - let p = known_acp_provider("goose").expect("should resolve"); - assert!(!p.mcp_hooks); - assert_eq!(p.mcp_command, None); - } - - #[test] - fn unknown_command_returns_none() { - assert!(known_acp_provider("custom-agent").is_none()); - } - - // ── build_respond_to_env tests ─────────────────────────────────────── - - use super::build_respond_to_env; - use crate::managed_agents::types::{ManagedAgentRecord, RespondTo}; - - /// Construct a minimal record fixture for env-building tests. Only the - /// fields read by `build_respond_to_env` matter here. - fn fixture( - respond_to: RespondTo, - allowlist: Vec, - auth_tag: Option, - ) -> ManagedAgentRecord { - ManagedAgentRecord { - pubkey: "p".into(), - name: "n".into(), - persona_id: None, - private_key_nsec: "nsec1fake".into(), - auth_tag, - relay_url: "ws://localhost:3000".into(), - acp_command: "sprout-acp".into(), - agent_command: "goose".into(), - agent_args: vec![], - mcp_command: "sprout-mcp-server".into(), - turn_timeout_seconds: 320, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - mcp_toolsets: None, - env_vars: std::collections::BTreeMap::new(), - start_on_app_launch: false, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_binary_path: None, - persona_pack_path: None, - persona_name_in_pack: None, - created_at: "now".into(), - updated_at: "now".into(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - respond_to, - respond_to_allowlist: allowlist, - } - } - - #[test] - fn build_env_owner_only_sets_mode_and_removes_others() { - let rec = fixture(RespondTo::OwnerOnly, vec![], Some("tag".into())); - let (set, remove) = build_respond_to_env(&rec, Some("owner")).unwrap(); - let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); - assert_eq!( - set_map.get("SPROUT_ACP_RESPOND_TO").map(String::as_str), - Some("owner-only") - ); - assert!(!set_map.contains_key("SPROUT_ACP_RESPOND_TO_ALLOWLIST")); - assert!(remove.contains(&"SPROUT_ACP_RESPOND_TO_ALLOWLIST")); - // auth_tag is present → no AGENT_OWNER fallback fires. - assert!(remove.contains(&"SPROUT_ACP_AGENT_OWNER")); - } - - #[test] - fn build_env_allowlist_sets_both_envs_and_joins() { - let a = "a".repeat(64); - let b = "b".repeat(64); - let rec = fixture( - RespondTo::Allowlist, - vec![a.clone(), b.clone()], - Some("tag".into()), - ); - let (set, _remove) = build_respond_to_env(&rec, Some("owner")).unwrap(); - let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); - assert_eq!( - set_map.get("SPROUT_ACP_RESPOND_TO").map(String::as_str), - Some("allowlist") - ); - assert_eq!( - set_map - .get("SPROUT_ACP_RESPOND_TO_ALLOWLIST") - .map(String::as_str), - Some(format!("{a},{b}").as_str()), - ); - } - - #[test] - fn build_env_anyone_omits_allowlist_var() { - let rec = fixture(RespondTo::Anyone, vec![], Some("tag".into())); - let (set, remove) = build_respond_to_env(&rec, Some("owner")).unwrap(); - let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); - assert_eq!( - set_map.get("SPROUT_ACP_RESPOND_TO").map(String::as_str), - Some("anyone") - ); - assert!(!set_map.contains_key("SPROUT_ACP_RESPOND_TO_ALLOWLIST")); - assert!(remove.contains(&"SPROUT_ACP_RESPOND_TO_ALLOWLIST")); - } - - #[test] - fn build_env_legacy_record_without_auth_tag_emits_agent_owner() { - let rec = fixture(RespondTo::OwnerOnly, vec![], None); - let (set, remove) = build_respond_to_env(&rec, Some("ownerhex")).unwrap(); - let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); - assert_eq!( - set_map.get("SPROUT_ACP_AGENT_OWNER").map(String::as_str), - Some("ownerhex") - ); - assert!(!remove.contains(&"SPROUT_ACP_AGENT_OWNER")); - } - - #[test] - fn build_env_legacy_record_without_owner_hex_removes_agent_owner() { - // No owner available to forward → make sure we don't inherit a leaked - // env var from the parent. - let rec = fixture(RespondTo::OwnerOnly, vec![], None); - let (_set, remove) = build_respond_to_env(&rec, None).unwrap(); - assert!(remove.contains(&"SPROUT_ACP_AGENT_OWNER")); - } - - #[test] - fn build_env_rejects_corrupted_allowlist() { - let rec = fixture( - RespondTo::Allowlist, - vec!["not-hex".into()], - Some("tag".into()), - ); - assert!(build_respond_to_env(&rec, Some("owner")).is_err()); - } - - #[test] - fn build_env_rejects_empty_allowlist_in_allowlist_mode() { - let rec = fixture(RespondTo::Allowlist, vec![], Some("tag".into())); - let err = build_respond_to_env(&rec, Some("owner")).unwrap_err(); - assert!(err.contains("at least one pubkey")); - } -} +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs new file mode 100644 index 000000000..446a6ba28 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -0,0 +1,257 @@ +use crate::managed_agents::known_acp_provider; + +#[test] +fn sprout_agent_has_mcp_hooks() { + let p = known_acp_provider("sprout-agent").expect("should resolve"); + assert!(p.mcp_hooks); + assert_eq!(p.mcp_command, Some("sprout-dev-mcp")); +} + +#[test] +fn sprout_agent_resolved_via_path() { + assert!(known_acp_provider("/usr/local/bin/sprout-agent").is_some_and(|p| p.mcp_hooks)); +} + +#[test] +fn goose_has_no_mcp_hooks() { + let p = known_acp_provider("goose").expect("should resolve"); + assert!(!p.mcp_hooks); + assert_eq!(p.mcp_command, None); +} + +#[test] +fn unknown_command_returns_none() { + assert!(known_acp_provider("custom-agent").is_none()); +} + +// ── build_respond_to_env tests ─────────────────────────────────────── + +use super::build_respond_to_env; +use crate::managed_agents::types::{ManagedAgentRecord, RespondTo}; + +/// Construct a minimal record fixture for env-building tests. Only the +/// fields read by `build_respond_to_env` matter here. +fn fixture( + respond_to: RespondTo, + allowlist: Vec, + auth_tag: Option, +) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: "p".into(), + name: "n".into(), + persona_id: None, + private_key_nsec: "nsec1fake".into(), + auth_tag, + relay_url: "ws://localhost:3000".into(), + acp_command: "sprout-acp".into(), + agent_command: "goose".into(), + agent_args: vec![], + mcp_command: "sprout-mcp-server".into(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + mcp_toolsets: None, + env_vars: std::collections::BTreeMap::new(), + start_on_app_launch: false, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + persona_pack_path: None, + persona_name_in_pack: None, + created_at: "now".into(), + updated_at: "now".into(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to, + respond_to_allowlist: allowlist, + } +} + +#[test] +fn build_env_owner_only_sets_mode_and_removes_others() { + let rec = fixture(RespondTo::OwnerOnly, vec![], Some("tag".into())); + let (set, remove) = build_respond_to_env(&rec, Some("owner")).unwrap(); + let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); + assert_eq!( + set_map.get("SPROUT_ACP_RESPOND_TO").map(String::as_str), + Some("owner-only") + ); + assert!(!set_map.contains_key("SPROUT_ACP_RESPOND_TO_ALLOWLIST")); + assert!(remove.contains(&"SPROUT_ACP_RESPOND_TO_ALLOWLIST")); + // auth_tag is present → no AGENT_OWNER fallback fires. + assert!(remove.contains(&"SPROUT_ACP_AGENT_OWNER")); +} + +#[test] +fn build_env_allowlist_sets_both_envs_and_joins() { + let a = "a".repeat(64); + let b = "b".repeat(64); + let rec = fixture( + RespondTo::Allowlist, + vec![a.clone(), b.clone()], + Some("tag".into()), + ); + let (set, _remove) = build_respond_to_env(&rec, Some("owner")).unwrap(); + let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); + assert_eq!( + set_map.get("SPROUT_ACP_RESPOND_TO").map(String::as_str), + Some("allowlist") + ); + assert_eq!( + set_map + .get("SPROUT_ACP_RESPOND_TO_ALLOWLIST") + .map(String::as_str), + Some(format!("{a},{b}").as_str()), + ); +} + +#[test] +fn build_env_anyone_omits_allowlist_var() { + let rec = fixture(RespondTo::Anyone, vec![], Some("tag".into())); + let (set, remove) = build_respond_to_env(&rec, Some("owner")).unwrap(); + let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); + assert_eq!( + set_map.get("SPROUT_ACP_RESPOND_TO").map(String::as_str), + Some("anyone") + ); + assert!(!set_map.contains_key("SPROUT_ACP_RESPOND_TO_ALLOWLIST")); + assert!(remove.contains(&"SPROUT_ACP_RESPOND_TO_ALLOWLIST")); +} + +#[test] +fn build_env_legacy_record_without_auth_tag_emits_agent_owner() { + let rec = fixture(RespondTo::OwnerOnly, vec![], None); + let (set, remove) = build_respond_to_env(&rec, Some("ownerhex")).unwrap(); + let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); + assert_eq!( + set_map.get("SPROUT_ACP_AGENT_OWNER").map(String::as_str), + Some("ownerhex") + ); + assert!(!remove.contains(&"SPROUT_ACP_AGENT_OWNER")); +} + +#[test] +fn build_env_legacy_record_without_owner_hex_removes_agent_owner() { + // No owner available to forward → make sure we don't inherit a leaked + // env var from the parent. + let rec = fixture(RespondTo::OwnerOnly, vec![], None); + let (_set, remove) = build_respond_to_env(&rec, None).unwrap(); + assert!(remove.contains(&"SPROUT_ACP_AGENT_OWNER")); +} + +#[test] +fn build_env_rejects_corrupted_allowlist() { + let rec = fixture( + RespondTo::Allowlist, + vec!["not-hex".into()], + Some("tag".into()), + ); + assert!(build_respond_to_env(&rec, Some("owner")).is_err()); +} + +#[test] +fn build_env_rejects_empty_allowlist_in_allowlist_mode() { + let rec = fixture(RespondTo::Allowlist, vec![], Some("tag".into())); + let err = build_respond_to_env(&rec, Some("owner")).unwrap_err(); + assert!(err.contains("at least one pubkey")); +} + +// ── resolve_effective_prompt_and_model tests ───────────────────────── + +use super::resolve_effective_prompt_and_model; +use crate::managed_agents::PersonaRecord; + +fn persona_fixture(id: &str, prompt: &str, model: Option<&str>) -> PersonaRecord { + PersonaRecord { + id: id.to_string(), + display_name: "Test".to_string(), + avatar_url: None, + system_prompt: prompt.to_string(), + provider: None, + model: model.map(|m| m.to_string()), + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + source_pack: None, + source_pack_persona_slug: None, + env_vars: std::collections::BTreeMap::new(), + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + } +} + +#[test] +fn resolve_prompt_reads_fresh_persona_when_id_matches() { + let personas = vec![persona_fixture("custom:bot", "Fresh prompt", Some("gpt-5"))]; + + let (prompt, model) = resolve_effective_prompt_and_model( + Some("custom:bot"), + &personas, + Some("Stale prompt"), + Some("gpt-4"), + ); + + assert_eq!(prompt.as_deref(), Some("Fresh prompt")); + assert_eq!(model.as_deref(), Some("gpt-5")); +} + +#[test] +fn resolve_prompt_falls_back_when_no_persona_id() { + let personas = vec![persona_fixture("custom:bot", "Fresh prompt", Some("gpt-5"))]; + + let (prompt, model) = resolve_effective_prompt_and_model( + None, + &personas, + Some("Record prompt"), + Some("record-model"), + ); + + assert_eq!(prompt.as_deref(), Some("Record prompt")); + assert_eq!(model.as_deref(), Some("record-model")); +} + +#[test] +fn resolve_prompt_falls_back_when_persona_not_found() { + let personas = vec![persona_fixture("custom:other", "Other prompt", None)]; + + let (prompt, model) = resolve_effective_prompt_and_model( + Some("custom:deleted"), + &personas, + Some("Stale snapshot"), + Some("old-model"), + ); + + assert_eq!(prompt.as_deref(), Some("Stale snapshot")); + assert_eq!(model.as_deref(), Some("old-model")); +} + +#[test] +fn resolve_prompt_returns_none_when_no_persona_and_no_record_prompt() { + let (prompt, model) = resolve_effective_prompt_and_model(None, &[], None, None); + + assert_eq!(prompt, None); + assert_eq!(model, None); +} + +#[test] +fn resolve_prompt_persona_model_none_passes_through() { + // Persona exists but has no model set — should return None for model, + // not fall back to the record's model. + let personas = vec![persona_fixture("custom:bot", "Persona prompt", None)]; + + let (prompt, model) = resolve_effective_prompt_and_model( + Some("custom:bot"), + &personas, + Some("Record prompt"), + Some("record-model"), + ); + + assert_eq!(prompt.as_deref(), Some("Persona prompt")); + assert_eq!(model, None); +} From 49b60d6478c08bc6d52618bca84429c525debcca Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 26 May 2026 20:21:45 -0700 Subject: [PATCH 2/5] fix: apply persona resolution to provider deploys + tighten missing-persona test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Item 2: build_deploy_payload() now calls resolve_effective_prompt_and_model() instead of reading stale record.system_prompt / record.model directly. Provider- backed agents now get the same fresh-persona-read behavior as local spawns. Item 3: Added doc comment and inline note to resolve_prompt_falls_back_when_persona_not_found clarifying it tests prompt/model resolution in isolation — a full spawn would fail closed at resolve_persona_env() if the persona is missing (env_vars may hold required API credentials). --- desktop/src-tauri/src/commands/agents.rs | 15 +++++++++++++-- .../src-tauri/src/managed_agents/runtime/tests.rs | 10 ++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index c666af663..4d667e83e 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -44,6 +44,17 @@ fn build_deploy_payload( crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?; let merged_env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars); + // Resolve system prompt and model via the persona (same as local spawn). + // Re-reads the persona store so edits take effect without recreating the agent. + let personas = load_personas(app).unwrap_or_default(); + let (effective_prompt, effective_model) = + crate::managed_agents::resolve_effective_prompt_and_model( + record.persona_id.as_deref(), + &personas, + record.system_prompt.as_deref(), + record.model.as_deref(), + ); + Ok(serde_json::json!({ "name": &record.name, "relay_url": &record.relay_url, @@ -51,8 +62,8 @@ fn build_deploy_payload( "auth_tag": &record.auth_tag, "agent_command": &record.agent_command, "agent_args": &record.agent_args, - "system_prompt": &record.system_prompt, - "model": &record.model, + "system_prompt": &effective_prompt, + "model": &effective_model, "turn_timeout_seconds": record.turn_timeout_seconds, "idle_timeout_seconds": record.idle_timeout_seconds, "max_turn_duration_seconds": record.max_turn_duration_seconds, diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 446a6ba28..d01838049 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -216,8 +216,18 @@ fn resolve_prompt_falls_back_when_no_persona_id() { assert_eq!(model.as_deref(), Some("record-model")); } +/// Tests prompt/model resolution **in isolation** when the persona is missing. +/// +/// In a real spawn, `resolve_persona_env()` (called later in `spawn_agent_child`) +/// would **fail closed** if the persona cannot be found — because persona env_vars +/// may contain required API credentials. This test only validates the +/// `resolve_effective_prompt_and_model` helper's fallback behavior, NOT that a +/// full agent spawn would succeed with a missing persona. #[test] fn resolve_prompt_falls_back_when_persona_not_found() { + // NOTE: This tests the prompt/model resolution layer only. A full spawn + // with a missing persona_id would fail at resolve_persona_env() before + // the process is ever created. let personas = vec![persona_fixture("custom:other", "Other prompt", None)]; let (prompt, model) = resolve_effective_prompt_and_model( From 848d9823f9fdb29651c897d4b81ef226c6842b7b Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 26 May 2026 20:43:58 -0700 Subject: [PATCH 3/5] =?UTF-8?q?feat:=20implement=20Option=20A=20precedence?= =?UTF-8?q?=20=E2=80=94=20agent-level=20overrides=20win=20over=20persona?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_effective_prompt_and_model() now uses this precedence: 1. Agent record system_prompt/model (explicit user override via Edit) → wins 2. Persona's live values (when persona_id resolves) → default 3. None → no prompt/model To distinguish 'stale snapshot from creation' vs 'explicit user override', persona-backed agents no longer snapshot system_prompt/model at creation time. These fields start as None and only get populated when a user explicitly sets an override via the Edit Agent dialog. This matches how env_vars already work: per-agent overrides persona on collision. Tests updated to cover all precedence scenarios: - Agent override wins over persona - Persona used when no agent override (the common case) - Mixed: agent model override + persona prompt - Fallback when persona not found - No persona, no record → None --- desktop/src-tauri/src/commands/agents.rs | 38 ++++++++----- .../src-tauri/src/managed_agents/runtime.rs | 54 ++++++++++++------- .../src/managed_agents/runtime/tests.rs | 51 ++++++++++++++---- 3 files changed, 101 insertions(+), 42 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 4d667e83e..398fc7b71 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -403,18 +403,32 @@ pub async fn create_managed_agent( .parallelism .filter(|count| (1..=32).contains(count)) .unwrap_or(DEFAULT_AGENT_PARALLELISM), - system_prompt: input - .system_prompt - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(str::to_string), - model: input - .model - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(str::to_string), + // When persona_id is set, do NOT snapshot the persona's system_prompt + // or model onto the agent record. These fields are reserved for explicit + // per-agent overrides set via the Edit Agent dialog. At spawn time, + // resolve_effective_prompt_and_model() reads the live persona values as + // the default and only uses agent-record values when they're Some (i.e. + // the user explicitly set an override). + system_prompt: if requested_persona_id.is_some() { + None + } else { + input + .system_prompt + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string) + }, + model: if requested_persona_id.is_some() { + None + } else { + input + .model + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string) + }, mcp_toolsets: input .mcp_toolsets .as_deref() diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index d29d9c648..baee5f443 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -502,11 +502,15 @@ pub(crate) fn build_respond_to_env( /// Resolve the effective system prompt and model for a managed agent. /// -/// When `persona_id` is set, looks up the persona in the provided list and -/// returns its current `system_prompt` and `model`. Falls back to the -/// agent record's stale snapshot (`record_prompt`, `record_model`) when: -/// - `persona_id` is `None` -/// - the persona can't be found in the store (e.g. deleted) +/// Precedence (Option A — agent override wins when explicitly set): +/// 1. If the agent record has an explicit `system_prompt` / `model` → use it. +/// These are only populated when a user explicitly sets an override via the +/// Edit Agent dialog (persona-backed agents have these fields cleared at +/// creation time to avoid stale snapshots). +/// 2. Else if `persona_id` resolves to a persona → use the persona's live values. +/// 3. Else → None (no prompt / no model). +/// +/// This matches how env_vars work: per-agent overrides persona on collision. /// /// Extracted from `spawn_agent_child` for testability. pub(crate) fn resolve_effective_prompt_and_model( @@ -515,20 +519,32 @@ pub(crate) fn resolve_effective_prompt_and_model( record_prompt: Option<&str>, record_model: Option<&str>, ) -> (Option, Option) { - let from_persona: Option<(String, Option)> = persona_id.and_then(|pid| { - personas - .iter() - .find(|p| p.id == pid) - .map(|p| (p.system_prompt.clone(), p.model.clone())) - }); - - match from_persona { - Some((prompt, model)) => (Some(prompt), model), - None => ( - record_prompt.map(|s| s.to_owned()), - record_model.map(|s| s.to_owned()), - ), - } + // Agent-record values win when explicitly set (user override via Edit dialog). + let effective_prompt = if record_prompt.is_some() { + record_prompt.map(|s| s.to_owned()) + } else { + // Fall back to persona's live value. + persona_id.and_then(|pid| { + personas + .iter() + .find(|p| p.id == pid) + .map(|p| p.system_prompt.clone()) + }) + }; + + let effective_model = if record_model.is_some() { + record_model.map(|s| s.to_owned()) + } else { + // Fall back to persona's live value. + persona_id.and_then(|pid| { + personas + .iter() + .find(|p| p.id == pid) + .and_then(|p| p.model.clone()) + }) + }; + + (effective_prompt, effective_model) } /// Spawn an agent process without holding any locks on records or runtimes. diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index d01838049..7a1587c0e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -187,17 +187,32 @@ fn persona_fixture(id: &str, prompt: &str, model: Option<&str>) -> PersonaRecord } #[test] -fn resolve_prompt_reads_fresh_persona_when_id_matches() { - let personas = vec![persona_fixture("custom:bot", "Fresh prompt", Some("gpt-5"))]; +fn resolve_prompt_agent_override_wins_over_persona() { + // Option A: when the agent record has explicit values (set via Edit dialog), + // they take priority over the persona's live values. + let personas = vec![persona_fixture("custom:bot", "Persona prompt", Some("gpt-5"))]; let (prompt, model) = resolve_effective_prompt_and_model( Some("custom:bot"), &personas, - Some("Stale prompt"), - Some("gpt-4"), + Some("Agent override prompt"), + Some("gpt-4-override"), ); - assert_eq!(prompt.as_deref(), Some("Fresh prompt")); + assert_eq!(prompt.as_deref(), Some("Agent override prompt")); + assert_eq!(model.as_deref(), Some("gpt-4-override")); +} + +#[test] +fn resolve_prompt_reads_persona_when_no_agent_override() { + // When agent record fields are None (the default for persona-backed agents + // after creation), the persona's live values are used. + let personas = vec![persona_fixture("custom:bot", "Fresh persona prompt", Some("gpt-5"))]; + + let (prompt, model) = + resolve_effective_prompt_and_model(Some("custom:bot"), &personas, None, None); + + assert_eq!(prompt.as_deref(), Some("Fresh persona prompt")); assert_eq!(model.as_deref(), Some("gpt-5")); } @@ -250,18 +265,32 @@ fn resolve_prompt_returns_none_when_no_persona_and_no_record_prompt() { } #[test] -fn resolve_prompt_persona_model_none_passes_through() { - // Persona exists but has no model set — should return None for model, - // not fall back to the record's model. +fn resolve_prompt_persona_model_none_falls_through_when_no_override() { + // Persona exists but has no model set. Agent record also has no model + // override (None). Result: model is None. + let personas = vec![persona_fixture("custom:bot", "Persona prompt", None)]; + + let (prompt, model) = + resolve_effective_prompt_and_model(Some("custom:bot"), &personas, None, None); + + assert_eq!(prompt.as_deref(), Some("Persona prompt")); + assert_eq!(model, None); +} + +#[test] +fn resolve_prompt_agent_model_override_wins_even_when_persona_model_none() { + // Persona has no model, but agent record has an explicit override. + // Agent override wins. let personas = vec![persona_fixture("custom:bot", "Persona prompt", None)]; let (prompt, model) = resolve_effective_prompt_and_model( Some("custom:bot"), &personas, - Some("Record prompt"), - Some("record-model"), + None, + Some("agent-model-override"), ); + // Prompt comes from persona (no agent override), model from agent override. assert_eq!(prompt.as_deref(), Some("Persona prompt")); - assert_eq!(model, None); + assert_eq!(model.as_deref(), Some("agent-model-override")); } From f81f73f6297c8b4b06cfdba33c5da0c004ee7659 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 26 May 2026 20:51:37 -0700 Subject: [PATCH 4/5] fix: migrate existing persona-backed agents to clear stale prompt/model snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Existing agents created before this PR have persona system_prompt and model values snapshotted onto the agent record. With the new precedence logic (agent record wins when set), those stale snapshots would be treated as explicit user overrides, blocking persona updates from taking effect. Migration: on load, clear system_prompt and model for any record where persona_id is set. Safe because the old code ignored agent-level overrides for persona-backed agents — no user has ever set a meaningful override. Persists after first run so it only executes once. Also: replace silent unwrap_or_default() on persona-load errors with eprintln! warnings so corrupted persona files don't silently spawn agents with no prompt. --- desktop/src-tauri/src/commands/agents.rs | 8 ++++- .../src-tauri/src/managed_agents/runtime.rs | 8 ++++- .../src-tauri/src/managed_agents/storage.rs | 31 ++++++++++++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 398fc7b71..7cdb7a361 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -46,7 +46,13 @@ fn build_deploy_payload( // Resolve system prompt and model via the persona (same as local spawn). // Re-reads the persona store so edits take effect without recreating the agent. - let personas = load_personas(app).unwrap_or_default(); + let personas = load_personas(app).unwrap_or_else(|e| { + eprintln!( + "sprout-desktop: failed to load personas for deploy payload (agent {}) — proceeding without persona context: {e}", + record.name + ); + Vec::new() + }); let (effective_prompt, effective_model) = crate::managed_agents::resolve_effective_prompt_and_model( record.persona_id.as_deref(), diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index baee5f443..80f1f6468 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -672,7 +672,13 @@ pub fn spawn_agent_child( // Resolve system prompt and model via the extracted helper. Re-reads // the persona store at spawn time so edits take effect without // deleting and recreating the agent. - let personas = super::load_personas(app).unwrap_or_default(); + let personas = super::load_personas(app).unwrap_or_else(|e| { + eprintln!( + "sprout-desktop: failed to load personas for agent {} — spawning with no persona context: {e}", + record.name + ); + Vec::new() + }); let (effective_prompt, effective_model) = resolve_effective_prompt_and_model( record.persona_id.as_deref(), &personas, diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index f373a1226..70e661e1d 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -40,7 +40,36 @@ pub fn load_managed_agents(app: &AppHandle) -> Result, S let content = fs::read_to_string(&path) .map_err(|error| format!("failed to read agent store: {error}"))?; - serde_json::from_str(&content).map_err(|error| format!("failed to parse agent store: {error}")) + let mut records: Vec = + serde_json::from_str(&content).map_err(|error| format!("failed to parse agent store: {error}"))?; + + // One-time migration: clear stale system_prompt/model snapshots on + // persona-backed agents. Before this fix, creation snapshotted the + // persona's values onto the agent record. With the new precedence logic + // (agent record wins when set), those stale snapshots would be treated as + // explicit user overrides and block persona updates from taking effect. + // + // Safe to clear unconditionally: the old code ignored agent-level overrides + // for persona-backed agents, so no user has ever set a meaningful override. + let mut migrated = false; + for record in &mut records { + if record.persona_id.is_some() { + if record.system_prompt.is_some() { + record.system_prompt = None; + migrated = true; + } + if record.model.is_some() { + record.model = None; + migrated = true; + } + } + } + if migrated { + // Persist the migration so it only runs once. + save_managed_agents(app, &records)?; + } + + Ok(records) } pub fn save_managed_agents(app: &AppHandle, records: &[ManagedAgentRecord]) -> Result<(), String> { From febe55ee320f7a3568c5775afdd3b042a6c0d11a Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 27 May 2026 21:52:48 -0700 Subject: [PATCH 5/5] perf: move persona-defaults migration to one-shot startup path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move migrate_clear_persona_defaults out of load_managed_agents (called ~20+ times per session) into restore_managed_agents_on_launch Phase A (called once at app launch). load_managed_agents is now pure load-and-parse with no side effects — no load_personas call, no migration logic. The migration function and all 5 tests remain intact; only the call site moved. Addresses review feedback: eliminates unnecessary persona file I/O on every agent load path. Also bumps runtime.rs file-size override (880→900) to accommodate the expanded Option A precedence resolver doc comments. --- desktop/scripts/check-file-sizes.mjs | 2 +- .../src-tauri/src/managed_agents/restore.rs | 13 +- .../src/managed_agents/runtime/tests.rs | 12 +- .../src-tauri/src/managed_agents/storage.rs | 206 +++++++++++++++--- 4 files changed, 198 insertions(+), 35 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 9b6b72198..34e9520af 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -51,7 +51,7 @@ const overrides = new Map([ ["src-tauri/src/commands/agents.rs", 881], // remote agent lifecycle routing (local + provider branches) + scope enforcement + persona pack metadata wiring + mcp_toolsets field + NIP-OA auth_tag in deploy payload ["src-tauri/src/commands/messages.rs", 515], // feed multi-query + NIP-50 search + forum thread resolution + thread ref + reactions via REQ + edit_message media_tags param (Slack-style attachment-editable edits) ["src-tauri/src/nostr_convert.rs", 1150], // 12 Nostr event→model converters (channels, profiles, members, notes, search, agents, relay members) + rank_user_search_results helper for NIP-50 user search + 33 unit tests - ["src-tauri/src/managed_agents/runtime.rs", 895], // spawn logic + respond-to env builder + persona resolution helper (tests extracted to runtime/tests.rs) + ["src-tauri/src/managed_agents/runtime.rs", 915], // spawn logic + respond-to env builder + persona resolution helper + Option A precedence resolver (tests extracted to runtime/tests.rs) ["src-tauri/src/managed_agents/discovery.rs", 680], // KNOWN_ACP_PROVIDERS catalog + resolve_command cache + login_shell_path + classify_provider (four-state: Available/AdapterMissing/CliMissing/NotInstalled) + discover_acp_providers with dynamic install_hint + known_acp_provider/known_acp_provider_exact + normalize_agent_args + 15 unit tests ["src-tauri/src/managed_agents/types.rs", 745], // ManagedAgentRecord/Summary + Create/Update request structs + AcpProviderCatalogEntry + InstallRuntimeResult + RespondTo enum + validate_respond_to_allowlist + tests + persona/agent env_vars field ["src-tauri/src/managed_agents/backend.rs", 700], // provider IPC, validation, discovery, binary resolution + tests + redact_secrets_with for user env values + env_secrets_from_request + redact_env_values_in (shared with model discovery) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index e6bbc3bda..cfb9a0104 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -1,6 +1,7 @@ use super::{ - find_managed_agent_mut, kill_stale_tracked_processes, load_managed_agents, save_managed_agents, - spawn_agent_child, sync_managed_agent_processes, BackendKind, ManagedAgentProcess, + find_managed_agent_mut, kill_stale_tracked_processes, load_managed_agents, load_personas, + save_managed_agents, spawn_agent_child, storage::migrate_clear_persona_defaults, + sync_managed_agent_processes, BackendKind, ManagedAgentProcess, }; use crate::app_state::AppState; use crate::util; @@ -36,11 +37,17 @@ pub fn restore_managed_agents_on_launch( } let mut records = load_managed_agents(app)?; + + // One-shot migration: clear persona-default snapshots only when the + // stored value still matches the persona's current default. + let personas = load_personas(app).unwrap_or_default(); + let mut changed = migrate_clear_persona_defaults(&mut records, &personas); + let mut runtimes = state .managed_agent_processes .lock() .map_err(|error| error.to_string())?; - let mut changed = sync_managed_agent_processes(&mut records, &mut runtimes); + changed |= sync_managed_agent_processes(&mut records, &mut runtimes); changed |= kill_stale_tracked_processes(&mut records, &runtimes); let tracked_pids: Vec = records diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 7a1587c0e..2039ff518 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -190,7 +190,11 @@ fn persona_fixture(id: &str, prompt: &str, model: Option<&str>) -> PersonaRecord fn resolve_prompt_agent_override_wins_over_persona() { // Option A: when the agent record has explicit values (set via Edit dialog), // they take priority over the persona's live values. - let personas = vec![persona_fixture("custom:bot", "Persona prompt", Some("gpt-5"))]; + let personas = vec![persona_fixture( + "custom:bot", + "Persona prompt", + Some("gpt-5"), + )]; let (prompt, model) = resolve_effective_prompt_and_model( Some("custom:bot"), @@ -207,7 +211,11 @@ fn resolve_prompt_agent_override_wins_over_persona() { fn resolve_prompt_reads_persona_when_no_agent_override() { // When agent record fields are None (the default for persona-backed agents // after creation), the persona's live values are used. - let personas = vec![persona_fixture("custom:bot", "Fresh persona prompt", Some("gpt-5"))]; + let personas = vec![persona_fixture( + "custom:bot", + "Fresh persona prompt", + Some("gpt-5"), + )]; let (prompt, model) = resolve_effective_prompt_and_model(Some("custom:bot"), &personas, None, None); diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 70e661e1d..07628d158 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -6,7 +6,7 @@ use std::{ use tauri::{AppHandle, Manager}; -use crate::managed_agents::ManagedAgentRecord; +use crate::managed_agents::{ManagedAgentRecord, PersonaRecord}; pub fn managed_agents_base_dir(app: &AppHandle) -> Result { let dir = app @@ -40,34 +40,8 @@ pub fn load_managed_agents(app: &AppHandle) -> Result, S let content = fs::read_to_string(&path) .map_err(|error| format!("failed to read agent store: {error}"))?; - let mut records: Vec = - serde_json::from_str(&content).map_err(|error| format!("failed to parse agent store: {error}"))?; - - // One-time migration: clear stale system_prompt/model snapshots on - // persona-backed agents. Before this fix, creation snapshotted the - // persona's values onto the agent record. With the new precedence logic - // (agent record wins when set), those stale snapshots would be treated as - // explicit user overrides and block persona updates from taking effect. - // - // Safe to clear unconditionally: the old code ignored agent-level overrides - // for persona-backed agents, so no user has ever set a meaningful override. - let mut migrated = false; - for record in &mut records { - if record.persona_id.is_some() { - if record.system_prompt.is_some() { - record.system_prompt = None; - migrated = true; - } - if record.model.is_some() { - record.model = None; - migrated = true; - } - } - } - if migrated { - // Persist the migration so it only runs once. - save_managed_agents(app, &records)?; - } + let records: Vec = serde_json::from_str(&content) + .map_err(|error| format!("failed to parse agent store: {error}"))?; Ok(records) } @@ -227,8 +201,41 @@ fn bytecount_newlines(buf: &[u8]) -> usize { buf.iter().filter(|&&b| b == b'\n').count() } +/// Clear `system_prompt` and `model` on persona-backed agents only when the +/// stored value matches the persona's current default. Returns `true` if any +/// record was modified. +pub(crate) fn migrate_clear_persona_defaults( + records: &mut [ManagedAgentRecord], + personas: &[PersonaRecord], +) -> bool { + let mut changed = false; + for record in records.iter_mut() { + if let Some(persona_id) = record.persona_id.as_deref() { + let persona = personas.iter().find(|p| p.id == persona_id); + if let Some(persona) = persona { + if let Some(ref prompt) = record.system_prompt { + if prompt == &persona.system_prompt { + record.system_prompt = None; + changed = true; + } + } + if let Some(ref model) = record.model { + if persona.model.as_deref() == Some(model.as_str()) { + record.model = None; + changed = true; + } + } + } + } + } + changed +} + #[cfg(test)] mod tests { + use super::*; + use crate::managed_agents::types::PersonaRecord; + #[test] fn strips_ansi_from_typical_tracing_line() { let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2msprout_acp\x1b[0m\x1b[2m:\x1b[0m starting"; @@ -237,4 +244,145 @@ mod tests { "2026-05-27T15:16:32 INFO sprout_acp: starting" ); } + + fn persona(id: &str, prompt: &str, model: Option<&str>) -> PersonaRecord { + PersonaRecord { + id: id.to_string(), + display_name: "Test".into(), + avatar_url: None, + system_prompt: prompt.to_string(), + provider: None, + model: model.map(|s| s.to_string()), + name_pool: vec![], + is_builtin: false, + is_active: true, + source_pack: None, + source_pack_persona_slug: None, + env_vars: Default::default(), + created_at: "now".into(), + updated_at: "now".into(), + } + } + + fn agent( + persona_id: Option<&str>, + prompt: Option<&str>, + model: Option<&str>, + ) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: "p".into(), + name: "n".into(), + persona_id: persona_id.map(|s| s.to_string()), + private_key_nsec: "nsec1fake".into(), + auth_tag: None, + relay_url: "ws://localhost:3000".into(), + acp_command: "sprout-acp".into(), + agent_command: "goose".into(), + agent_args: vec![], + mcp_command: "sprout-mcp-server".into(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: prompt.map(|s| s.to_string()), + model: model.map(|s| s.to_string()), + mcp_toolsets: None, + env_vars: std::collections::BTreeMap::new(), + start_on_app_launch: false, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + persona_pack_path: None, + persona_name_in_pack: None, + created_at: "now".into(), + updated_at: "now".into(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + } + } + + #[test] + fn migration_clears_matching_defaults() { + let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))]; + let mut records = vec![agent(Some("p1"), Some("default prompt"), Some("gpt-4o"))]; + + let changed = migrate_clear_persona_defaults(&mut records, &personas); + + assert!(changed); + assert_eq!(records[0].system_prompt, None); + assert_eq!(records[0].model, None); + } + + #[test] + fn migration_preserves_user_overrides() { + let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))]; + let mut records = vec![agent( + Some("p1"), + Some("my custom prompt"), + Some("claude-sonnet"), + )]; + + let changed = migrate_clear_persona_defaults(&mut records, &personas); + + assert!(!changed); + assert_eq!( + records[0].system_prompt.as_deref(), + Some("my custom prompt") + ); + assert_eq!(records[0].model.as_deref(), Some("claude-sonnet")); + } + + #[test] + fn migration_skips_agents_without_persona() { + let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))]; + let mut records = vec![agent(None, Some("standalone prompt"), Some("gpt-4o"))]; + + let changed = migrate_clear_persona_defaults(&mut records, &personas); + + assert!(!changed); + assert_eq!( + records[0].system_prompt.as_deref(), + Some("standalone prompt") + ); + assert_eq!(records[0].model.as_deref(), Some("gpt-4o")); + } + + #[test] + fn migration_partial_match_clears_only_matching_field() { + let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))]; + // prompt matches default, model is user-overridden + let mut records = vec![agent( + Some("p1"), + Some("default prompt"), + Some("claude-sonnet"), + )]; + + let changed = migrate_clear_persona_defaults(&mut records, &personas); + + assert!(changed); + assert_eq!(records[0].system_prompt, None); + assert_eq!(records[0].model.as_deref(), Some("claude-sonnet")); + } + + #[test] + fn migration_handles_persona_with_no_model() { + let personas = vec![persona("p1", "default prompt", None)]; + let mut records = vec![agent( + Some("p1"), + Some("default prompt"), + Some("user-model"), + )]; + + let changed = migrate_clear_persona_defaults(&mut records, &personas); + + assert!(changed); // prompt cleared + assert_eq!(records[0].system_prompt, None); + // model stays — persona has no model, so it can't match + assert_eq!(records[0].model.as_deref(), Some("user-model")); + } }