From c36c4f46992f69f8077799779594b45300f90e99 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Wed, 3 Jun 2026 12:24:10 -0600 Subject: [PATCH] feat: updated enabled_skills handling to support both list and comma-separated strings --- config.example.yaml | 5 ++- config.role.example.md | 5 +-- src/config/app_config.rs | 5 +-- src/config/mod.rs | 69 +++++++++++++++++++++++++++++++++++++- src/config/role.rs | 37 +++++++++++++++++--- src/config/session.rs | 17 ++++++++-- src/config/skill_policy.rs | 18 +++------- 7 files changed, 129 insertions(+), 27 deletions(-) diff --git a/config.example.yaml b/config.example.yaml index 28b71c9..8aa7729 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -131,10 +131,13 @@ visible_skills: # The universe of skills allowed to be enabled - frontend-ui-ux - git-master enabled_skills: null # Which skills are available by default (no role/agent/session active). null = all visible. - # Example: only expose two skills in the bare REPL. + # Accepts either a YAML list or a comma-separated string. + # Example (list form): # enabled_skills: # - git-master # - ai-slop-remover + # Example (comma-separated form): + # enabled_skills: git-master,ai-slop-remover # ---- Auto-Continue (Todo System) ---- # The auto-continue system provides built-in task tracking for improved reliability. diff --git a/config.role.example.md b/config.role.example.md index 3cfe437..a1724cf 100644 --- a/config.role.example.md +++ b/config.role.example.md @@ -12,8 +12,9 @@ enabled_tools: fs_ls,fs_cat # A comma-separated list of tools to enabl enabled_mcp_servers: github,gitmcp # A comma-separated list of MCP servers to enable for this role skills_enabled: true # Master switch for skills in this role (default: inherit from global). # Skills also require `function_calling_support: true` in the global config. -enabled_skills: git-master,ai-slop-remover # Comma-separated list of skills available when this role is active. - # Must be a subset of global `visible_skills`. Omit to inherit the global default. +enabled_skills: # Skills available when this role is active. Accepts a YAML list (preferred) + - git-master # or a comma-separated string (e.g. `enabled_skills: git-master,ai-slop-remover`). + - ai-slop-remover # Must be a subset of global `visible_skills`. Omit to inherit the global default. prompt: null # A custom prompt to use for this role that will immediately query # the model for output instead of using the instructions below # Auto-Continue (Todo System) diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 07b5669..aa96770 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -38,7 +38,8 @@ pub struct AppConfig { pub visible_tools: Option>, pub skills_enabled: bool, - pub enabled_skills: Option, + #[serde(default, deserialize_with = "super::deserialize_csv_or_vec")] + pub enabled_skills: Option>, pub visible_skills: Option>, pub mcp_server_support: bool, @@ -400,7 +401,7 @@ impl AppConfig { } if let Some(v) = super::read_env_value::(&get_env_name("enabled_skills")) { - self.enabled_skills = v; + self.enabled_skills = v.map(|raw| super::csv_to_vec(&raw)); } if let Some(Some(v)) = super::read_env_bool(&get_env_name("mcp_server_support")) { diff --git a/src/config/mod.rs b/src/config/mod.rs index 70efb43..51d3264 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -200,7 +200,8 @@ pub struct Config { pub visible_tools: Option>, pub skills_enabled: bool, - pub enabled_skills: Option, + #[serde(default, deserialize_with = "deserialize_csv_or_vec")] + pub enabled_skills: Option>, pub visible_skills: Option>, pub mcp_server_support: bool, @@ -783,6 +784,72 @@ where Ok(value) } +pub(super) fn csv_to_vec(raw: &str) -> Vec { + raw.split(',') + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) + .collect() +} + +pub(super) fn deserialize_csv_or_vec<'de, D>( + deserializer: D, +) -> std::result::Result>, D::Error> +where + D: serde::Deserializer<'de>, +{ + use serde::de::{self, SeqAccess, Visitor}; + use std::fmt; + + struct CsvOrVec; + + impl<'de> Visitor<'de> for CsvOrVec { + type Value = Option>; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a comma-separated string, a list of strings, or null") + } + + fn visit_str(self, value: &str) -> std::result::Result { + Ok(Some(csv_to_vec(value))) + } + + fn visit_string(self, value: String) -> std::result::Result { + Ok(Some(csv_to_vec(&value))) + } + + fn visit_none(self) -> std::result::Result { + Ok(None) + } + + fn visit_some>( + self, + deserializer: D2, + ) -> std::result::Result { + deserializer.deserialize_any(self) + } + + fn visit_unit(self) -> std::result::Result { + Ok(None) + } + + fn visit_seq>( + self, + mut seq: A, + ) -> std::result::Result { + let mut vec = Vec::new(); + while let Some(item) = seq.next_element::()? { + let trimmed = item.trim().to_string(); + if !trimmed.is_empty() { + vec.push(trimmed); + } + } + Ok(Some(vec)) + } + } + + deserializer.deserialize_option(CsvOrVec) +} + fn read_env_bool(key: &str) -> Option> { let value = env::var(key).ok()?; Some(parse_bool(&value)) diff --git a/src/config/role.rs b/src/config/role.rs index cf2d42c..b3f77b8 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -57,8 +57,12 @@ pub struct Role { 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( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::deserialize_csv_or_vec" + )] + enabled_skills: Option>, #[serde(skip_serializing_if = "Option::is_none")] auto_continue: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -103,7 +107,7 @@ impl Role { 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()), + "enabled_skills" => role.enabled_skills = parse_enabled_skills_value(value), "auto_continue" => role.auto_continue = value.as_bool(), "max_auto_continues" => { role.max_auto_continues = value.as_u64().map(|v| v as usize) @@ -157,7 +161,8 @@ impl Role { metadata.push(format!("skills_enabled: {skills_enabled}")); } if let Some(enabled_skills) = &self.enabled_skills { - metadata.push(format!("enabled_skills: {enabled_skills}")); + let inline = serde_json::to_string(enabled_skills).unwrap_or_else(|_| "[]".to_string()); + metadata.push(format!("enabled_skills: {inline}")); } if let Some(auto_continue) = self.auto_continue { metadata.push(format!("auto_continue: {auto_continue}")); @@ -287,7 +292,7 @@ impl Role { self.skills_enabled } - pub fn enabled_skills(&self) -> Option<&str> { + pub fn enabled_skills(&self) -> Option<&[String]> { self.enabled_skills.as_deref() } @@ -392,6 +397,28 @@ impl RoleLike for Role { } } +fn parse_enabled_skills_value(value: &Value) -> Option> { + if value.is_null() { + return None; + } + + if let Some(s) = value.as_str() { + return Some(csv_to_vec(s)); + } + + if let Some(arr) = value.as_array() { + let items: Vec = arr + .iter() + .filter_map(|v| v.as_str().map(|s| s.trim().to_string())) + .filter(|s| !s.is_empty()) + .collect(); + + return Some(items); + } + + None +} + fn parse_structure_prompt(prompt: &str) -> (&str, Vec<(&str, &str)>) { let mut text = prompt; let mut search_input = true; diff --git a/src/config/session.rs b/src/config/session.rs index 0839712..b098ff2 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -30,8 +30,12 @@ pub struct Session { 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( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::deserialize_csv_or_vec" + )] + enabled_skills: Option>, #[serde(skip_serializing_if = "Option::is_none")] save_session: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -83,10 +87,17 @@ impl Session { self.skills_enabled } - pub fn enabled_skills(&self) -> Option<&str> { + pub fn enabled_skills(&self) -> Option<&[String]> { self.enabled_skills.as_deref() } + pub fn set_skills_enabled(&mut self, value: Option) { + if self.skills_enabled != value { + self.skills_enabled = value; + self.dirty = true; + } + } + 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 index a47ffa8..e52fb30 100644 --- a/src/config/skill_policy.rs +++ b/src/config/skill_policy.rs @@ -67,10 +67,10 @@ impl SkillPolicy { .map(|v| v.iter().cloned().collect()); let enabled_raw: Option> = session - .and_then(|s| parse_csv_opt(s.enabled_skills())) + .and_then(|s| s.enabled_skills().map(|v| v.to_vec())) .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())); + .or_else(|| role.and_then(|r| r.enabled_skills().map(|v| v.to_vec()))) + .or_else(|| global.enabled_skills.clone()); let enabled: HashSet = match enabled_raw { Some(explicit) => { @@ -107,18 +107,10 @@ impl SkillPolicy { } } -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::*; + use super::super::csv_to_vec; fn always_true(_: &str) -> bool { true @@ -135,7 +127,7 @@ mod tests { ) -> AppConfig { AppConfig { skills_enabled, - enabled_skills: enabled.map(|s| s.to_string()), + enabled_skills: enabled.map(csv_to_vec), visible_skills: visible.map(|v| v.iter().map(|s| s.to_string()).collect()), ..AppConfig::default() }