From 31b013171349bfeee2fc802bcade1b8327135d0a Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 05:29:55 -0500 Subject: [PATCH] fix(cli): preserve familiar agent access precedence --- src-rust/crates/cli/src/main.rs | 14 ++-- src-rust/crates/core/src/coven_shared.rs | 93 +++++++++++++++++++++++- 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src-rust/crates/cli/src/main.rs b/src-rust/crates/cli/src/main.rs index 044c904..a72ec6c 100644 --- a/src-rust/crates/cli/src/main.rs +++ b/src-rust/crates/cli/src/main.rs @@ -766,12 +766,12 @@ async fn main() -> anyhow::Result<()> { query_config.provider_registry = Some(provider_registry.clone()); // Wire in the named agent (--agent flag). - // Merge built-in default agents + Coven familiars with user-defined agents. - // Order: built-ins → familiars (built-ins win) → settings.json agents (user wins). + // Merge built-in default agents + settings agents + Coven familiars. + // Familiar ids keep their trusted access tier over project settings. let tools = if let Some(ref agent_name) = cli.agent { query_config.agent_name = Some(agent_name.clone()); - let mut all_agents = claurst_core::coven_shared::default_agents_with_familiars(); - all_agents.extend(config.agents.clone()); + let all_agents = + claurst_core::coven_shared::default_agents_with_familiars_and_config(&config.agents); if let Some(def) = all_agents.get(agent_name) { let access = def.access.clone(); query_config.agent_definition = Some(def.clone()); @@ -2833,8 +2833,10 @@ async fn run_interactive( if app.agent_mode_changed { app.agent_mode_changed = false; let mode = app.agent_mode.as_deref().unwrap_or("build"); - let mut all_agents = claurst_core::coven_shared::default_agents_with_familiars(); - all_agents.extend(cmd_ctx.config.agents.clone()); + let all_agents = + claurst_core::coven_shared::default_agents_with_familiars_and_config( + &cmd_ctx.config.agents, + ); if let Some(def) = all_agents.get(mode) { base_query_config.agent_name = Some(mode.to_string()); base_query_config.agent_definition = Some(def.clone()); diff --git a/src-rust/crates/core/src/coven_shared.rs b/src-rust/crates/core/src/coven_shared.rs index c07a9ff..77f4f16 100644 --- a/src-rust/crates/core/src/coven_shared.rs +++ b/src-rust/crates/core/src/coven_shared.rs @@ -192,8 +192,9 @@ pub fn familiar_to_agent_definition( /// /// Built-in agents win on id collision (familiars share lowercase keyspace /// with `build`/`plan`/`explore`, so collisions are unexpected — but the -/// rule keeps `build` etc. inviolate). Callers extend with user-defined -/// `config.agents` afterwards so user overrides still win. +/// rule keeps `build` etc. inviolate). When merging settings-defined agents, +/// use [`default_agents_with_familiars_and_config`] so familiar ids keep their +/// trusted access tier instead of being shadowed by project configuration. pub fn default_agents_with_familiars( ) -> std::collections::HashMap { let mut map = crate::config::default_agents(); @@ -206,6 +207,32 @@ pub fn default_agents_with_familiars( map } +/// Return built-in agents, settings-defined agents, and familiars with the +/// correct security precedence for runtime agent resolution. +/// +/// Settings-defined agents may override built-ins as before, but a familiar id +/// from `~/.coven/familiars.toml` cannot be shadowed by project settings. That +/// keeps the `/agents` familiar picker and runtime tool filter resolving the +/// same trusted [`crate::config::AgentDefinition::access`] tier. +pub fn default_agents_with_familiars_and_config( + config_agents: &std::collections::HashMap, +) -> std::collections::HashMap { + let builtins = crate::config::default_agents(); + let mut map = builtins.clone(); + map.extend(config_agents.clone()); + + if let Some(fams) = load_familiars() { + for fam in &fams { + let (id, def) = familiar_to_agent_definition(fam); + if !builtins.contains_key(&id) { + map.insert(id, def); + } + } + } + + map +} + // --------------------------------------------------------------------------- // Skills (~/.coven/skills//metadata.json) // --------------------------------------------------------------------------- @@ -459,6 +486,68 @@ access = "search-only" assert_eq!(merged.get("cody").map(|d| d.access.as_str()), Some("full")); } + #[test] + fn default_agents_with_familiars_and_config_keeps_familiar_over_settings_shadow() { + let _g = with_coven_home(|home| { + fs::write( + home.join("familiars.toml"), + r#" +[[familiar]] +id = "cody" +display_name = "Cody" +role = "Code" +"#, + ) + .unwrap(); + }); + + let mut config_agents = std::collections::HashMap::new(); + config_agents.insert( + "cody".to_string(), + crate::config::AgentDefinition { + description: Some("Project-controlled shadow".to_string()), + model: None, + temperature: None, + prompt: Some("Run shell commands".to_string()), + access: "full".to_string(), + visible: true, + max_turns: None, + color: None, + }, + ); + + let merged = default_agents_with_familiars_and_config(&config_agents); + let cody = merged.get("cody").expect("familiar should be present"); + assert_eq!(cody.access, DEFAULT_FAMILIAR_ACCESS); + let prompt = cody.prompt.as_deref().unwrap_or_default(); + assert!(prompt.contains("Cody")); + assert!(!prompt.contains("Run shell commands")); + } + + #[test] + fn default_agents_with_familiars_and_config_preserves_settings_builtin_override() { + let _g = with_coven_home(|_| {}); + let mut config_agents = std::collections::HashMap::new(); + config_agents.insert( + "build".to_string(), + crate::config::AgentDefinition { + description: Some("Custom build".to_string()), + model: None, + temperature: None, + prompt: Some("Custom build prompt".to_string()), + access: "read-only".to_string(), + visible: true, + max_turns: None, + color: None, + }, + ); + + let merged = default_agents_with_familiars_and_config(&config_agents); + let build = merged.get("build").expect("build agent should be present"); + assert_eq!(build.access, "read-only"); + assert_eq!(build.description.as_deref(), Some("Custom build")); + } + #[test] fn canonicalize_access_tier_accepts_canonical_lowercase() { assert_eq!(canonicalize_access_tier("full"), Some("full"));