From aa2e627a5f555e12800b30e6521a2e3355cdcecc Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Mon, 1 Jun 2026 12:26:30 -0600 Subject: [PATCH] feat: implemented the skills policy to track available skills per context --- src/config/agent.rs | 14 ++ src/config/app_config.rs | 20 ++ src/config/mod.rs | 11 ++ src/config/role.rs | 24 +++ src/config/session.rs | 14 ++ src/config/skill_policy.rs | 367 +++++++++++++++++++++++++++++++++++++ 6 files changed, 450 insertions(+) create mode 100644 src/config/skill_policy.rs diff --git a/src/config/agent.rs b/src/config/agent.rs index fea7baf..557343d 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -337,6 +337,16 @@ impl Agent { &self.config.mcp_servers } + #[allow(dead_code)] + pub fn skills_enabled(&self) -> Option { + self.config.skills_enabled + } + + #[allow(dead_code)] + pub fn enabled_skills(&self) -> Option<&[String]> { + self.config.enabled_skills.as_deref() + } + pub fn conversation_starters(&self) -> Vec { self.config .conversation_starters @@ -615,6 +625,10 @@ pub struct AgentConfig { #[serde(default)] pub global_tools: Vec, #[serde(skip_serializing_if = "Option::is_none")] + pub skills_enabled: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub enabled_skills: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub continuation_prompt: Option, #[serde(default)] pub instructions: String, diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 41fb193..6450d61 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -35,6 +35,10 @@ pub struct AppConfig { pub enabled_tools: Option, pub visible_tools: Option>, + pub skills_enabled: bool, + pub enabled_skills: Option, + pub visible_skills: Option>, + pub mcp_server_support: bool, pub mapping_mcp_servers: IndexMap, pub enabled_mcp_servers: Option, @@ -96,6 +100,10 @@ impl Default for AppConfig { enabled_tools: None, visible_tools: None, + skills_enabled: true, + enabled_skills: None, + visible_skills: None, + mcp_server_support: true, mapping_mcp_servers: Default::default(), enabled_mcp_servers: None, @@ -158,6 +166,10 @@ impl AppConfig { enabled_tools: config.enabled_tools, visible_tools: config.visible_tools, + skills_enabled: config.skills_enabled, + enabled_skills: config.enabled_skills, + visible_skills: config.visible_skills, + mcp_server_support: config.mcp_server_support, mapping_mcp_servers: config.mapping_mcp_servers, enabled_mcp_servers: config.enabled_mcp_servers, @@ -379,6 +391,14 @@ impl AppConfig { self.enabled_tools = v; } + if let Some(Some(v)) = super::read_env_bool(&get_env_name("skills_enabled")) { + self.skills_enabled = v; + } + + if let Some(v) = super::read_env_value::(&get_env_name("enabled_skills")) { + self.enabled_skills = v; + } + if let Some(Some(v)) = super::read_env_bool(&get_env_name("mcp_server_support")) { self.mcp_server_support = v; } diff --git a/src/config/mod.rs b/src/config/mod.rs index 53af22b..99d88a9 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -12,6 +12,7 @@ mod request_context; mod role; mod session; mod skill; +mod skill_policy; mod skill_registry; pub(crate) mod todo; mod tool_scope; @@ -35,6 +36,8 @@ use self::session::Session; #[allow(unused_imports)] pub use self::skill::Skill; #[allow(unused_imports)] +pub use self::skill_policy::SkillPolicy; +#[allow(unused_imports)] pub use self::skill_registry::SkillRegistry; pub use self::update::run_self_update; use crate::client::{ @@ -151,6 +154,10 @@ pub struct Config { pub enabled_tools: Option, pub visible_tools: Option>, + pub skills_enabled: bool, + pub enabled_skills: Option, + pub visible_skills: Option>, + pub mcp_server_support: bool, pub mapping_mcp_servers: IndexMap, pub enabled_mcp_servers: Option, @@ -212,6 +219,10 @@ impl Default for Config { enabled_tools: None, visible_tools: None, + skills_enabled: true, + enabled_skills: None, + visible_skills: None, + mcp_server_support: true, mapping_mcp_servers: Default::default(), enabled_mcp_servers: None, diff --git a/src/config/role.rs b/src/config/role.rs index bcf1e2d..faad211 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -56,6 +56,10 @@ pub struct Role { #[serde(skip_serializing_if = "Option::is_none")] enabled_mcp_servers: Option, #[serde(skip_serializing_if = "Option::is_none")] + skills_enabled: Option, + #[serde(skip_serializing_if = "Option::is_none")] + enabled_skills: Option, + #[serde(skip_serializing_if = "Option::is_none")] auto_continue: Option, #[serde(skip_serializing_if = "Option::is_none")] max_auto_continues: Option, @@ -98,6 +102,10 @@ impl Role { "enabled_mcp_servers" => { role.enabled_mcp_servers = value.as_str().map(|v| v.to_string()) } + "skills_enabled" => role.skills_enabled = value.as_bool(), + "enabled_skills" => { + role.enabled_skills = value.as_str().map(|v| v.to_string()) + } "auto_continue" => role.auto_continue = value.as_bool(), "max_auto_continues" => { role.max_auto_continues = value.as_u64().map(|v| v as usize) @@ -147,6 +155,12 @@ impl Role { if let Some(enabled_mcp_servers) = self.enabled_mcp_servers() { metadata.push(format!("enabled_mcp_servers: {enabled_mcp_servers}")); } + if let Some(skills_enabled) = self.skills_enabled { + metadata.push(format!("skills_enabled: {skills_enabled}")); + } + if let Some(enabled_skills) = &self.enabled_skills { + metadata.push(format!("enabled_skills: {enabled_skills}")); + } if let Some(auto_continue) = self.auto_continue { metadata.push(format!("auto_continue: {auto_continue}")); } @@ -271,6 +285,16 @@ impl Role { self.continuation_prompt.as_deref() } + #[allow(dead_code)] + pub fn skills_enabled(&self) -> Option { + self.skills_enabled + } + + #[allow(dead_code)] + pub fn enabled_skills(&self) -> Option<&str> { + self.enabled_skills.as_deref() + } + pub fn append_to_prompt(&mut self, text: &str) { self.prompt.push_str(text); } diff --git a/src/config/session.rs b/src/config/session.rs index 254334f..7401d25 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -29,6 +29,10 @@ pub struct Session { #[serde(skip_serializing_if = "Option::is_none")] enabled_mcp_servers: Option, #[serde(skip_serializing_if = "Option::is_none")] + skills_enabled: Option, + #[serde(skip_serializing_if = "Option::is_none")] + enabled_skills: Option, + #[serde(skip_serializing_if = "Option::is_none")] save_session: Option, #[serde(skip_serializing_if = "Option::is_none")] compression_threshold: Option, @@ -75,6 +79,16 @@ pub struct Session { } impl Session { + #[allow(dead_code)] + pub fn skills_enabled(&self) -> Option { + self.skills_enabled + } + + #[allow(dead_code)] + pub fn enabled_skills(&self) -> Option<&str> { + self.enabled_skills.as_deref() + } + pub fn new_from_ctx(ctx: &RequestContext, app: &AppConfig, name: &str) -> Self { let role = ctx.extract_role(app); let mut session = Self { diff --git a/src/config/skill_policy.rs b/src/config/skill_policy.rs new file mode 100644 index 0000000..0672198 --- /dev/null +++ b/src/config/skill_policy.rs @@ -0,0 +1,367 @@ +use super::agent::Agent; +use super::app_config::AppConfig; +use super::paths; +use super::role::Role; +use super::session::Session; + +use anyhow::{Result, bail}; +use std::collections::HashSet; + +#[allow(dead_code)] +#[derive(Debug)] +pub struct SkillPolicy { + pub skills_enabled: bool, + pub visible: Option>, + pub enabled: HashSet, +} + +#[allow(dead_code)] +impl SkillPolicy { + pub fn effective( + global: &AppConfig, + role: Option<&Role>, + agent: Option<&Agent>, + session: Option<&Session>, + ) -> Result { + Self::effective_with( + global, + role, + agent, + session, + &paths::has_skill, + &paths::list_skills, + ) + } + + fn effective_with( + global: &AppConfig, + role: Option<&Role>, + agent: Option<&Agent>, + session: Option<&Session>, + skill_exists: &F, + list_installed: &G, + ) -> Result + where + F: Fn(&str) -> bool, + G: Fn() -> Vec, + { + let mut skills_enabled = global.skills_enabled; + if let Some(r) = role + && let Some(false) = r.skills_enabled() + { + skills_enabled = false; + } + + if let Some(a) = agent + && let Some(false) = a.skills_enabled() + { + skills_enabled = false; + } + + if let Some(s) = session + && let Some(false) = s.skills_enabled() + { + skills_enabled = false; + } + + let visible: Option> = global + .visible_skills + .as_ref() + .map(|v| v.iter().cloned().collect()); + + let enabled_raw: Option> = session + .and_then(|s| parse_csv_opt(s.enabled_skills())) + .or_else(|| agent.and_then(|a| a.enabled_skills().map(|v| v.to_vec()))) + .or_else(|| role.and_then(|r| parse_csv_opt(r.enabled_skills()))) + .or_else(|| parse_csv_opt(global.enabled_skills.as_deref())); + + let enabled: HashSet = match enabled_raw { + Some(explicit) => { + let set: HashSet = explicit.into_iter().collect(); + for name in &set { + if !skill_exists(name) { + bail!("enabled_skills references skill '{name}' which is not installed"); + } + + if let Some(vs) = &visible + && !vs.contains(name) + { + bail!( + "enabled_skills references skill '{name}' which is not in visible_skills" + ); + } + } + set + } + None => match &visible { + Some(v) => v.clone(), + None => list_installed().into_iter().collect(), + }, + }; + + Ok(Self { + skills_enabled, + visible, + enabled, + }) + } + + pub fn allows(&self, name: &str) -> bool { + self.skills_enabled && self.enabled.contains(name) + } +} + +fn parse_csv_opt(s: Option<&str>) -> Option> { + s.map(|raw| { + raw.split(',') + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) + .collect() + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn always_true(_: &str) -> bool { + true + } + + fn empty_installed() -> Vec { + Vec::new() + } + + fn make_app_config( + skills_enabled: bool, + enabled: Option<&str>, + visible: Option<&[&str]>, + ) -> AppConfig { + AppConfig { + skills_enabled, + enabled_skills: enabled.map(|s| s.to_string()), + visible_skills: visible.map(|v| v.iter().map(|s| s.to_string()).collect()), + ..AppConfig::default() + } + } + + #[test] + fn defaults_yield_skills_enabled_with_empty_universe() { + let global = AppConfig::default(); + + let policy = SkillPolicy::effective_with( + &global, + None, + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert!(policy.skills_enabled); + assert!(policy.visible.is_none()); + assert!(policy.enabled.is_empty()); + } + + #[test] + fn falls_back_to_all_installed_when_no_level_sets_enabled_skills() { + let global = AppConfig::default(); + let installed = || vec!["alpha".to_string(), "beta".to_string()]; + + let policy = + SkillPolicy::effective_with(&global, None, None, None, &always_true, &installed) + .unwrap(); + + assert_eq!(policy.enabled.len(), 2); + assert!(policy.enabled.contains("alpha")); + assert!(policy.enabled.contains("beta")); + } + + #[test] + fn falls_back_to_visible_when_visible_set_but_no_enabled() { + let global = make_app_config(true, None, Some(&["alpha", "beta"])); + + let policy = SkillPolicy::effective_with( + &global, + None, + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert_eq!(policy.enabled.len(), 2); + assert!(policy.enabled.contains("alpha")); + assert!(policy.enabled.contains("beta")); + } + + #[test] + fn global_enabled_skills_is_effective_when_no_other_levels() { + let global = make_app_config(true, Some("alpha,beta"), Some(&["alpha", "beta", "gamma"])); + + let policy = SkillPolicy::effective_with( + &global, + None, + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert!(policy.enabled.contains("alpha")); + assert!(policy.enabled.contains("beta")); + assert!(!policy.enabled.contains("gamma")); + } + + #[test] + fn role_overrides_global_enabled_skills() { + let global = make_app_config(true, Some("alpha"), Some(&["alpha", "beta"])); + let role = Role::new( + "test", + "---\nenabled_skills: beta\n---\nbody", + ); + + let policy = SkillPolicy::effective_with( + &global, + Some(&role), + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert!(policy.enabled.contains("beta")); + assert!(!policy.enabled.contains("alpha")); + } + + #[test] + fn any_skills_enabled_false_disables_globally() { + let global = make_app_config(true, None, None); + let role = Role::new("test", "---\nskills_enabled: false\n---\nbody"); + + let policy = SkillPolicy::effective_with( + &global, + Some(&role), + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert!(!policy.skills_enabled); + } + + #[test] + fn allows_returns_false_when_skills_disabled() { + let global = AppConfig { + skills_enabled: false, + ..AppConfig::default() + }; + + let policy = SkillPolicy::effective_with( + &global, + None, + None, + None, + &always_true, + &|| vec!["alpha".to_string()], + ) + .unwrap(); + + assert!(!policy.allows("alpha")); + } + + #[test] + fn allows_returns_true_when_skill_in_enabled_set() { + let global = make_app_config(true, Some("alpha"), None); + + let policy = SkillPolicy::effective_with( + &global, + None, + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert!(policy.allows("alpha")); + assert!(!policy.allows("beta")); + } + + #[test] + fn validation_rejects_uninstalled_skill_reference() { + let global = make_app_config(true, Some("ghost"), None); + + let err = SkillPolicy::effective_with( + &global, + None, + None, + None, + &|_| false, + &empty_installed, + ) + .unwrap_err(); + + assert!(err.to_string().contains("not installed")); + assert!(err.to_string().contains("ghost")); + } + + #[test] + fn validation_rejects_skill_not_in_visible_set() { + let global = make_app_config(true, Some("beta"), Some(&["alpha"])); + + let err = SkillPolicy::effective_with( + &global, + None, + None, + None, + &always_true, + &empty_installed, + ) + .unwrap_err(); + + assert!(err.to_string().contains("not in visible_skills")); + assert!(err.to_string().contains("beta")); + } + + #[test] + fn validation_skipped_when_no_explicit_enabled_skills() { + let global = make_app_config(true, None, None); + + let policy = SkillPolicy::effective_with( + &global, + None, + None, + None, + &|_| false, + &empty_installed, + ) + .unwrap(); + + assert!(policy.enabled.is_empty()); + } + + #[test] + fn empty_string_enabled_skills_resolves_to_empty_override() { + let global = make_app_config(true, Some("alpha,beta"), Some(&["alpha", "beta"])); + let role = Role::new("test", "---\nenabled_skills: \"\"\n---\nbody"); + + let policy = SkillPolicy::effective_with( + &global, + Some(&role), + None, + None, + &always_true, + &empty_installed, + ) + .unwrap(); + + assert!(policy.enabled.is_empty()); + } +}