From dcbf4706dcb94fce76d14507624e3f48a52c9175 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 9 Jun 2026 13:18:03 -0600 Subject: [PATCH 1/3] docs(prompt): sharpen thread-depth and root-reporting conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten the base prompt's threading guidance so agents keep threads shallow (reply to the thread root, not the latest message) and surface milestones at the channel root. Folds the existing "broadcast on completion" bullet into the new "report milestones at the root" bullet to avoid restating the same idea. Proposed convention for team discussion — not a unilateral default change. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- crates/sprout-acp/src/base_prompt.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 From 3ba20722fb3b7b6822e13b3f240e0e961cfc9a4c Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 9 Jun 2026 14:11:40 -0600 Subject: [PATCH 2/3] fix: resolve persona prompt from live record at spawn for non-pack agents Persona edits never reached agents created from a plain persona (not an installed pack): spawn only consulted the live PersonaRecord when both persona_pack_path and persona_name_in_pack were set, so non-pack agents fell through to the frozen record.system_prompt snapshot. Editing the persona left that snapshot stale, and no restart could pick it up. Resolve the linked persona whenever persona_id is set (pack or not); the persona is the source of truth and edits propagate on the next spawn. Fall back to the record snapshot only when no persona is linked or it was deleted. Pack agents are unaffected (they already set persona_id). Extract the resolution into a pure helper and move the runtime test module to runtime/tests.rs (env_vars/tests.rs pattern), keeping runtime.rs under the file-size limit. 4 new unit tests cover persona-wins, no-persona fallback, deleted-persona fallback, and persona-clears-stale-model. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../src-tauri/src/managed_agents/runtime.rs | 302 ++-------------- .../src/managed_agents/runtime/tests.rs | 330 ++++++++++++++++++ 2 files changed, 364 insertions(+), 268 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/runtime/tests.rs 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"), + ] + ); +} From 48698b2514b33bf0b4146e98cae3da37a5bf9239 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Tue, 9 Jun 2026 19:42:48 -0400 Subject: [PATCH 3/3] feat(personas): auto-sync pack-sourced personas on launch Pack edits on disk were not propagating to persona records until the user manually uninstalled and reimported. This left the spawn-time persona resolution (fixed in this branch) reading stale data for pack agents. On startup, iterate persona records with source_pack set, resolve the corresponding pack directory, and update system_prompt, model, avatar_url, and display_name if they differ from the resolved pack content. Signed-off-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 --- desktop/scripts/check-file-sizes.mjs | 1 + desktop/src-tauri/src/lib.rs | 4 + .../src-tauri/src/managed_agents/personas.rs | 89 +++++++++++++++++++ 3 files changed, 94 insertions(+) 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();