fix: persona is source of truth at spawn + thread-depth conventions (#930)

Signed-off-by: Wes <wesbillman@users.noreply.github.com> Signed-off-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
2026-08-18 06:50:31 +02:00 · 2026-06-09 23:58:45 +00:00
parent 73cd8d082d
commit 877048d68f
6 changed files with 461 additions and 270 deletions
@@ -27,9 +27,10 @@ Run `sprout --help` or `sprout <group> --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 <thread-root-event-id>` 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 <thread-root-event-id>`), 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 <UUID> --since <ts>`. When `since` is set without `before`, results are oldest-first (chronological).

 ## Startup Recovery
@@ -33,6 +33,7 @@ const overrides = new Map([
  ["src-tauri/src/commands/agents.rs", 1287],
  ["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],
@@ -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
@@ -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<Vec<PersonaRecord>, String> {
    let path = personas_store_path(app)?;
    let now = now_iso();
@@ -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<String>,
+    record_model: Option<String>,
+) -> (Option<String>, Option<String>, Option<String>) {
+    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<PersonaRecord> = 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<String>,
-        auth_tag: Option<String>,
-    ) -> 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;
@@ -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<String>,
+    auth_tag: Option<String>,
+) -> 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"),
+        ]
+    );
+}