diff --git a/crates/sprout-acp/src/base_prompt.md b/crates/sprout-acp/src/base_prompt.md index acff3acd79..9e56935361 100644 --- a/crates/sprout-acp/src/base_prompt.md +++ b/crates/sprout-acp/src/base_prompt.md @@ -27,9 +27,10 @@ Run `sprout --help` or `sprout --help` for full usage. - Respond promptly to @mentions. - Be direct. State what you did, what you found, or what you need. No preamble. - Message content supports GitHub-flavored Markdown. Use fenced code blocks with a language tag (` ```python `, ` ```typescript `, etc.) for syntax-highlighted rendering on desktop and mobile. Omitting the language tag renders monochrome. -- When responding in-thread, use `sprout messages send --reply-to ` to keep replies scoped to the thread. Post new top-level messages for new topics. +- Reply to the thread root (`sprout messages send --reply-to `), not the latest message — flat threads stay readable; reply chains bury context 3+ levels deep. One thread = one unit of work: ask sub-questions inline. A real tangent starts a new top-level message. +- Work in the thread, report milestones at the root. The thread is the messy middle — progress, dead ends, clarifying questions, and routine updates. Use a top-level post for channel-visible milestones: picked up, blocked + need input, change ready / PR up, done, or anything teammates skimming only root-level messages must act on. Thread notifications are easy to miss; a top-level post ensures the requester sees the outcome. +- New topic → new top-level message. Don't graft an unrelated task onto an existing thread. - When you are mentioned in multiple threads, prioritize the most recent one chronologically. If someone steers or redirects you in a newer thread while you are working from an older dispatch, reply in the newer thread to acknowledge — do not bury your response in the original thread where it may go unseen. -- When you complete a task (e.g., PR created, implementation finished, research delivered), post a top-level channel message with the result — do not only reply in-thread. Thread notifications are easy to miss; a broadcast message ensures the requester sees the outcome promptly. - No push notifications — poll with `sprout messages get --channel --since `. When `since` is set without `before`, results are oldest-first (chronological). ## Startup Recovery diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 09e46abf56..06ae677947 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -33,6 +33,7 @@ const overrides = new Map([ ["src-tauri/src/commands/agents.rs", 1208], ["src-tauri/src/managed_agents/nest.rs", 1420], ["src-tauri/src/managed_agents/runtime.rs", 1465], + ["src-tauri/src/managed_agents/personas.rs", 1080], ["src-tauri/src/managed_agents/persona_card.rs", 1050], ["src-tauri/src/huddle/tts.rs", 1364], ["src/shared/api/tauri.ts", 1196], diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 174e36a570..953a2ea187 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -524,6 +524,10 @@ pub fn run() { migration::reconcile_provider_mcp_commands(&app_handle); migration::migrate_persona_provider_to_runtime(&app_handle); + if let Err(e) = managed_agents::sync_pack_personas(&app_handle) { + eprintln!("sprout-desktop: sync-pack-personas: {e}"); + } + // Resolve persisted identity key (env var → file → generate+save). // This is fatal — the app should not start with an ephemeral identity // that will be lost on restart, as that silently breaks channel diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 5ae50d2e28..de5f697734 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -952,6 +952,95 @@ pub struct PackSummary { pub path: PathBuf, } +/// Re-read pack directories and update persona records whose source content +/// has changed. Runs on launch so pack edits on disk propagate without +/// manual intervention. +pub fn sync_pack_personas(app: &AppHandle) -> Result<(), String> { + let mut records = load_personas(app)?; + let packs = packs_dir(app)?; + + if !packs.exists() { + return Ok(()); + } + + let mut changed = false; + + for record in records.iter_mut() { + let pack_id = match &record.source_pack { + Some(id) => id.clone(), + None => continue, + }; + let slug = match &record.source_pack_persona_slug { + Some(s) => s.clone(), + None => continue, + }; + + // Find the pack directory whose resolved ID matches + let pack_dir_entries = + fs::read_dir(&packs).map_err(|e| format!("failed to read packs dir: {e}"))?; + + let mut found = false; + for entry in pack_dir_entries { + let entry = entry.map_err(|e| format!("dir entry error: {e}"))?; + let dir = entry.path(); + if !dir.is_dir() { + continue; + } + let resolved = match sprout_persona::resolve::resolve_pack(&dir) { + Ok(r) => r, + Err(_) => continue, + }; + if resolved.id != pack_id { + continue; + } + + // Found the matching pack — find the persona by slug + if let Some(persona) = resolved.personas.iter().find(|p| p.name == slug) { + let mut record_changed = false; + + if record.system_prompt != persona.system_prompt { + record.system_prompt = persona.system_prompt.clone(); + record_changed = true; + } + if record.model != persona.model { + record.model = persona.model.clone(); + record_changed = true; + } + if record.avatar_url != persona.avatar { + record.avatar_url = persona.avatar.clone(); + record_changed = true; + } + if record.display_name != persona.display_name { + record.display_name = persona.display_name.clone(); + record_changed = true; + } + + if record_changed { + record.updated_at = now_iso(); + changed = true; + eprintln!( + "sprout-desktop: sync-pack-personas: updated {:?} from pack {:?}", + record.display_name, pack_id + ); + } + } + found = true; + break; + } + + if !found { + // Pack directory no longer exists or doesn't resolve — skip silently + continue; + } + } + + if changed { + save_personas(app, &records)?; + } + + Ok(()) +} + pub fn load_personas(app: &AppHandle) -> Result, String> { let path = personas_store_path(app)?; let now = now_iso(); diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 00d2a701df..1b1b8187a2 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -6,7 +6,7 @@ use crate::{ managed_agents::{ append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, missing_command_message, normalize_agent_args, open_log_file, resolve_command, - ManagedAgentProcess, ManagedAgentRecord, ManagedAgentSummary, PersonaRecord, + ManagedAgentProcess, ManagedAgentRecord, ManagedAgentSummary, }, util::now_iso, }; @@ -768,6 +768,26 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } +/// Resolve the effective system prompt, model, and provider for a spawn. The +/// linked persona always wins so persona edits propagate on the next spawn; the +/// record snapshot is the fallback only when no persona is linked or it was +/// deleted. Provider comes from the persona (the record has no provider field). +fn resolve_effective_prompt_model_provider( + persona_id: Option<&str>, + personas: &[crate::managed_agents::types::PersonaRecord], + record_prompt: Option, + record_model: Option, +) -> (Option, Option, Option) { + match persona_id.and_then(|pid| personas.iter().find(|p| p.id == pid)) { + Some(p) => ( + Some(p.system_prompt.clone()), + p.model.clone(), + p.provider.clone(), + ), + None => (record_prompt, record_model, None), + } +} + /// 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. @@ -900,32 +920,18 @@ pub fn spawn_agent_child( command.env("SPROUT_ACP_PERSONA_NAME", persona_name); } - // Resolve system prompt, model, and provider: prefer the persona definition - // (if a persona pack is configured and the persona matched), otherwise fall - // back to the record-level overrides. Provider always flows from the persona - // when one is linked (the record has no provider field of its own). - let has_persona_pack = - record.persona_pack_path.is_some() && record.persona_name_in_pack.is_some(); - let persona_record: Option = record.persona_id.as_deref().and_then(|pid| { - super::load_personas(app) - .ok()? - .into_iter() - .find(|p| p.id == pid) - }); - - let (effective_prompt, effective_model, effective_provider) = if has_persona_pack { - match &persona_record { - Some(p) => ( - Some(p.system_prompt.clone()), - p.model.clone(), - p.provider.clone(), - ), - None => (record.system_prompt.clone(), record.model.clone(), None), - } - } else { - let provider = persona_record.as_ref().and_then(|p| p.provider.clone()); - (record.system_prompt.clone(), record.model.clone(), provider) - }; + // Resolve system prompt, model, and provider: the linked persona is the + // source of truth, so persona edits reach the agent on the next spawn. Fall + // back to the record snapshot only when no persona is linked or it was + // deleted. Provider flows from the persona (the record has no provider). + let personas = super::load_personas(app).unwrap_or_default(); + let (effective_prompt, effective_model, effective_provider) = + resolve_effective_prompt_model_provider( + record.persona_id.as_deref(), + &personas, + record.system_prompt.clone(), + record.model.clone(), + ); if let Some(prompt) = &effective_prompt { command.env("SPROUT_ACP_SYSTEM_PROMPT", prompt); @@ -1219,244 +1225,4 @@ fn runtime_metadata_env_vars<'a>( } #[cfg(test)] -mod tests { - use crate::managed_agents::known_acp_runtime; - - #[test] - fn marker_entry_is_namespaced_by_instance_id() { - // The spawn stamp and the sweep matcher must produce identical bytes; - // both go through sprout_marker_entry, so this pins the on-the-wire - // format and guards against a dev build (`...app.dev`) matching a - // release build's (`...app`) agents. - assert_eq!( - super::sprout_marker_entry("xyz.block.sprout.app"), - b"SPROUT_MANAGED_AGENT=xyz.block.sprout.app".to_vec() - ); - assert_ne!( - super::sprout_marker_entry("xyz.block.sprout.app"), - super::sprout_marker_entry("xyz.block.sprout.app.dev") - ); - } - - #[test] - fn sprout_agent_has_mcp_hooks() { - let p = known_acp_runtime("sprout-agent").expect("should resolve"); - assert!(p.mcp_hooks); - assert_eq!(p.mcp_command, Some("sprout-dev-mcp")); - } - - #[test] - fn databricks_defaults_empty_in_oss_build() { - // OSS (and normal test) builds set neither SPROUT_BUILD_DATABRICKS_*, - // so nothing is baked in and no DATABRICKS_* is injected on spawn. - assert!(super::build_databricks_defaults().is_empty()); - } - - #[test] - fn sprout_agent_resolved_via_path() { - assert!(known_acp_runtime("/usr/local/bin/sprout-agent").is_some_and(|p| p.mcp_hooks)); - } - - #[test] - fn goose_has_no_mcp_hooks() { - let p = known_acp_runtime("goose").expect("should resolve"); - assert!(!p.mcp_hooks); - assert_eq!(p.mcp_command, None); - } - - #[test] - fn unknown_command_returns_none() { - assert!(known_acp_runtime("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(), - avatar_url: None, - acp_command: "sprout-acp".into(), - agent_command: "goose".into(), - agent_args: vec![], - mcp_command: String::new(), - 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, - relay_mesh: None, - } - } - - #[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")); - } - - // ── runtime_metadata_env_vars tests ───────────────────────────────────── - - use super::runtime_metadata_env_vars; - - #[test] - fn runtime_metadata_env_vars_injects_model_and_provider() { - let vars = runtime_metadata_env_vars( - Some("GOOSE_MODEL"), - Some("GOOSE_PROVIDER"), - false, - Some("gpt-4o"), - Some("openai"), - ); - assert_eq!( - vars, - vec![("GOOSE_MODEL", "gpt-4o"), ("GOOSE_PROVIDER", "openai")] - ); - } - - #[test] - fn runtime_metadata_env_vars_skips_provider_when_locked() { - let vars = runtime_metadata_env_vars( - None, // claude has no model_env_var - None, // claude has no provider_env_var - true, // provider_locked = true - Some("claude-opus-4-7"), - Some("anthropic"), - ); - assert!(vars.is_empty()); - } - - #[test] - fn runtime_metadata_env_vars_injects_model_even_with_acp_model_switching() { - // sprout-agent has supports_acp_model_switching=true but we still inject - // the model env var because ACP model switching is post-bootstrap - let vars = runtime_metadata_env_vars( - Some("SPROUT_AGENT_MODEL"), - Some("SPROUT_AGENT_PROVIDER"), - false, - Some("goose-claude-4-6-opus"), - Some("databricks"), - ); - assert_eq!( - vars, - vec![ - ("SPROUT_AGENT_MODEL", "goose-claude-4-6-opus"), - ("SPROUT_AGENT_PROVIDER", "databricks"), - ] - ); - } -} +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 0000000000..baa2e44fde --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -0,0 +1,330 @@ +use crate::managed_agents::known_acp_runtime; + +#[test] +fn marker_entry_is_namespaced_by_instance_id() { + // The spawn stamp and the sweep matcher must produce identical bytes; + // both go through sprout_marker_entry, so this pins the on-the-wire + // format and guards against a dev build (`...app.dev`) matching a + // release build's (`...app`) agents. + assert_eq!( + super::sprout_marker_entry("xyz.block.sprout.app"), + b"SPROUT_MANAGED_AGENT=xyz.block.sprout.app".to_vec() + ); + assert_ne!( + super::sprout_marker_entry("xyz.block.sprout.app"), + super::sprout_marker_entry("xyz.block.sprout.app.dev") + ); +} + +#[test] +fn sprout_agent_has_mcp_hooks() { + let p = known_acp_runtime("sprout-agent").expect("should resolve"); + assert!(p.mcp_hooks); + assert_eq!(p.mcp_command, Some("sprout-dev-mcp")); +} + +#[test] +fn databricks_defaults_empty_in_oss_build() { + // OSS (and normal test) builds set neither SPROUT_BUILD_DATABRICKS_*, + // so nothing is baked in and no DATABRICKS_* is injected on spawn. + assert!(super::build_databricks_defaults().is_empty()); +} + +#[test] +fn sprout_agent_resolved_via_path() { + assert!(known_acp_runtime("/usr/local/bin/sprout-agent").is_some_and(|p| p.mcp_hooks)); +} + +#[test] +fn goose_has_no_mcp_hooks() { + let p = known_acp_runtime("goose").expect("should resolve"); + assert!(!p.mcp_hooks); + assert_eq!(p.mcp_command, None); +} + +#[test] +fn unknown_command_returns_none() { + assert!(known_acp_runtime("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(), + avatar_url: None, + acp_command: "sprout-acp".into(), + agent_command: "goose".into(), + agent_args: vec![], + mcp_command: String::new(), + 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, + relay_mesh: None, + } +} + +#[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_model_provider tests ─────────────────── + +fn persona(id: &str, prompt: &str, model: Option<&str>) -> crate::managed_agents::PersonaRecord { + persona_with_provider(id, prompt, model, None) +} + +fn persona_with_provider( + id: &str, + prompt: &str, + model: Option<&str>, + provider: Option<&str>, +) -> crate::managed_agents::PersonaRecord { + crate::managed_agents::PersonaRecord { + id: id.to_string(), + display_name: id.to_string(), + avatar_url: None, + system_prompt: prompt.to_string(), + runtime: None, + model: model.map(str::to_string), + provider: provider.map(str::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-06-09T00:00:00Z".to_string(), + updated_at: "2026-06-09T00:00:00Z".to_string(), + } +} + +#[test] +fn linked_persona_wins_over_record_snapshot() { + let personas = vec![persona_with_provider( + "p1", + "fresh", + Some("m-fresh"), + Some("anthropic"), + )]; + let (prompt, model, provider) = super::resolve_effective_prompt_model_provider( + Some("p1"), + &personas, + Some("stale".into()), + Some("m-stale".into()), + ); + assert_eq!(prompt.as_deref(), Some("fresh")); + assert_eq!(model.as_deref(), Some("m-fresh")); + assert_eq!(provider.as_deref(), Some("anthropic")); +} + +#[test] +fn no_persona_id_falls_back_to_record() { + let personas = vec![persona("p1", "fresh", Some("m-fresh"))]; + let (prompt, model, provider) = super::resolve_effective_prompt_model_provider( + None, + &personas, + Some("record".into()), + Some("m-record".into()), + ); + assert_eq!(prompt.as_deref(), Some("record")); + assert_eq!(model.as_deref(), Some("m-record")); + assert_eq!(provider, None); +} + +#[test] +fn deleted_persona_falls_back_to_record() { + let personas = vec![persona("p1", "fresh", None)]; + let (prompt, model, provider) = super::resolve_effective_prompt_model_provider( + Some("gone"), + &personas, + Some("record".into()), + Some("m-record".into()), + ); + assert_eq!(prompt.as_deref(), Some("record")); + assert_eq!(model.as_deref(), Some("m-record")); + assert_eq!(provider, None); +} + +#[test] +fn persona_with_no_model_clears_stale_record_model() { + let personas = vec![persona("p1", "fresh", None)]; + let (prompt, model, _provider) = super::resolve_effective_prompt_model_provider( + Some("p1"), + &personas, + Some("stale".into()), + Some("m-stale".into()), + ); + assert_eq!(prompt.as_deref(), Some("fresh")); + assert_eq!(model, None); +} + +// ── runtime_metadata_env_vars tests ───────────────────────────────────── + +use super::runtime_metadata_env_vars; + +#[test] +fn runtime_metadata_env_vars_injects_model_and_provider() { + let vars = runtime_metadata_env_vars( + Some("GOOSE_MODEL"), + Some("GOOSE_PROVIDER"), + false, + Some("gpt-4o"), + Some("openai"), + ); + assert_eq!( + vars, + vec![("GOOSE_MODEL", "gpt-4o"), ("GOOSE_PROVIDER", "openai")] + ); +} + +#[test] +fn runtime_metadata_env_vars_skips_provider_when_locked() { + let vars = runtime_metadata_env_vars( + None, // claude has no model_env_var + None, // claude has no provider_env_var + true, // provider_locked = true + Some("claude-opus-4-7"), + Some("anthropic"), + ); + assert!(vars.is_empty()); +} + +#[test] +fn runtime_metadata_env_vars_injects_model_even_with_acp_model_switching() { + // sprout-agent has supports_acp_model_switching=true but we still inject + // the model env var because ACP model switching is post-bootstrap + let vars = runtime_metadata_env_vars( + Some("SPROUT_AGENT_MODEL"), + Some("SPROUT_AGENT_PROVIDER"), + false, + Some("goose-claude-4-6-opus"), + Some("databricks"), + ); + assert_eq!( + vars, + vec![ + ("SPROUT_AGENT_MODEL", "goose-claude-4-6-opus"), + ("SPROUT_AGENT_PROVIDER", "databricks"), + ] + ); +}