diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 9b6b72198b..34e9520afd 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 e6bbc3bdae..cfb9a01044 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 7a1587c0e0..2039ff518c 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 70e661e1da..07628d1584 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")); + } }