diff --git a/src/config/agent.rs b/src/config/agent.rs index 557343d..8ee1dbc 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -337,12 +337,10 @@ 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() } diff --git a/src/config/mod.rs b/src/config/mod.rs index 99d88a9..1e2c453 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -268,6 +268,7 @@ pub fn install_builtins() -> Result<()> { Functions::install_builtin_global_tools(false)?; Agent::install_builtin_agents(false)?; Macro::install_macros(false)?; + Skill::install_builtin_skills(false)?; Ok(()) } @@ -276,18 +277,20 @@ pub enum AssetCategory { Agents, Macros, Functions, + Skills, #[value(name = "mcp_config")] McpConfig, } impl AssetCategory { - pub const NAMES: [&'static str; 4] = ["agents", "macros", "functions", "mcp_config"]; + pub const NAMES: [&'static str; 5] = ["agents", "macros", "functions", "skills", "mcp_config"]; pub fn parse(name: &str) -> Option { match name { "agents" => Some(Self::Agents), "macros" => Some(Self::Macros), "functions" => Some(Self::Functions), + "skills" => Some(Self::Skills), "mcp_config" => Some(Self::McpConfig), _ => None, } @@ -333,6 +336,7 @@ pub fn install_assets(category: AssetCategory) -> Result<()> { AssetCategory::Agents => ("agents", paths::agents_data_dir()), AssetCategory::Macros => ("macros", paths::macros_dir()), AssetCategory::Functions => ("functions", paths::functions_dir()), + AssetCategory::Skills => ("skills", paths::skills_dir()), AssetCategory::McpConfig => ("MCP config", paths::mcp_config_file()), }; @@ -345,6 +349,7 @@ pub fn install_assets(category: AssetCategory) -> Result<()> { AssetCategory::Agents => Agent::install_builtin_agents(true)?, AssetCategory::Macros => Macro::install_macros(true)?, AssetCategory::Functions => Functions::install_builtin_global_tools(true)?, + AssetCategory::Skills => Skill::install_builtin_skills(true)?, AssetCategory::McpConfig => Functions::install_mcp_config()?, } diff --git a/src/config/paths.rs b/src/config/paths.rs index 59d3724..768e26d 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -72,12 +72,10 @@ pub fn skills_dir() -> PathBuf { } } -#[allow(dead_code)] pub fn skill_dir(name: &str) -> PathBuf { skills_dir().join(name) } -#[allow(dead_code)] pub fn skill_file(name: &str) -> PathBuf { skill_dir(name).join("SKILL.md") } @@ -251,7 +249,6 @@ pub fn has_macro(name: &str) -> bool { names.contains(&name.to_string()) } -#[allow(dead_code)] pub fn list_skills() -> Vec { let mut names = Vec::new(); if let Ok(rd) = read_dir(skills_dir()) { @@ -270,7 +267,6 @@ pub fn list_skills() -> Vec { names } -#[allow(dead_code)] pub fn has_skill(name: &str) -> bool { skill_file(name).is_file() } diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 5a5b192..e03be25 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -4019,6 +4019,43 @@ mod tests { ); } + #[test] + #[serial] + fn install_builtin_skills_force_overwrites_only_with_force() { + let _guard = TestConfigDirGuard::new(); + + Skill::install_builtin_skills(false).unwrap(); + let file = paths::skill_file("git-master"); + assert!(file.exists(), "git-master skill should be installed"); + + write(&file, "SENTINEL").unwrap(); + Skill::install_builtin_skills(false).unwrap(); + assert_eq!( + read_to_string(&file).unwrap(), + "SENTINEL", + "non-force install must not overwrite an existing skill" + ); + + Skill::install_builtin_skills(true).unwrap(); + assert_ne!( + read_to_string(&file).unwrap(), + "SENTINEL", + "force install must overwrite the existing skill" + ); + } + + #[test] + #[serial] + fn install_builtin_skills_installs_all_bundled() { + let _guard = TestConfigDirGuard::new(); + + Skill::install_builtin_skills(false).unwrap(); + assert!(paths::skill_file("git-master").exists()); + assert!(paths::skill_file("ai-slop-remover").exists()); + assert!(paths::skill_file("code-review").exists()); + assert!(paths::skill_file("frontend-ui-ux").exists()); + } + #[test] #[serial] fn install_functions_force_preserves_user_mcp_json() { diff --git a/src/config/role.rs b/src/config/role.rs index faad211..85c8a78 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -285,12 +285,10 @@ 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() } diff --git a/src/config/session.rs b/src/config/session.rs index 7401d25..0839712 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -79,12 +79,10 @@ 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() } diff --git a/src/config/skill.rs b/src/config/skill.rs index 5400038..544a512 100644 --- a/src/config/skill.rs +++ b/src/config/skill.rs @@ -6,6 +6,7 @@ use rust_embed::Embed; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::sync::LazyLock; +use log::{debug, info}; #[derive(Embed)] #[folder = "assets/skills/"] @@ -39,7 +40,6 @@ pub struct Skill { auto_unload: Option, } -#[allow(dead_code)] impl Skill { pub fn new(name: &str, content: &str) -> Self { let mut metadata = ""; @@ -84,11 +84,36 @@ impl Skill { skill } - pub fn builtin(name: &str) -> Result { - let content = SkillsAsset::get(&format!("{name}/SKILL.md")) - .ok_or_else(|| anyhow!("Unknown skill `{name}`"))?; - let content = unsafe { std::str::from_utf8_unchecked(&content.data) }; - Ok(Skill::new(name, content)) + pub fn install_builtin_skills(force: bool) -> Result<()> { + info!( + "Installing built-in skills in {}", + paths::skills_dir().display() + ); + + for file in SkillsAsset::iter() { + debug!("Processing skill file: {}", file.as_ref()); + + let embedded_file = SkillsAsset::get(&file).ok_or_else(|| { + anyhow!("Failed to load embedded skill file: {}", file.as_ref()) + })?; + let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) }; + let file_path = paths::skills_dir().join(file.as_ref()); + + if file_path.exists() && !force { + debug!( + "Skill file already exists, skipping: {}", + file_path.display() + ); + continue; + } + + ensure_parent_exists(&file_path)?; + info!("Creating skill file: {}", file_path.display()); + let mut skill_file = File::create(&file_path)?; + Write::write_all(&mut skill_file, content.as_bytes())?; + } + + Ok(()) } pub fn load(name: &str) -> Result { @@ -99,12 +124,6 @@ impl Skill { Ok(Skill::new(name, &content)) } - pub fn list_builtin_skill_names() -> Vec { - SkillsAsset::iter() - .filter_map(|v| v.strip_suffix("/SKILL.md").map(|v| v.to_string())) - .collect() - } - pub fn name(&self) -> &str { &self.name } @@ -307,11 +326,4 @@ mod tests { assert!(skill.is_compatible(false, false)); } - - #[test] - fn builtin_returns_err_for_unknown_skill() { - let result = Skill::builtin("nonexistent_skill_xyz"); - - assert!(result.is_err()); - } } diff --git a/src/config/skill_policy.rs b/src/config/skill_policy.rs index 0672198..5f42818 100644 --- a/src/config/skill_policy.rs +++ b/src/config/skill_policy.rs @@ -7,15 +7,12 @@ 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, @@ -101,7 +98,6 @@ impl SkillPolicy { Ok(Self { skills_enabled, - visible, enabled, }) } @@ -160,7 +156,6 @@ mod tests { .unwrap(); assert!(policy.skills_enabled); - assert!(policy.visible.is_none()); assert!(policy.enabled.is_empty()); } diff --git a/src/config/skill_registry.rs b/src/config/skill_registry.rs index c43afd3..0aa3fa6 100644 --- a/src/config/skill_registry.rs +++ b/src/config/skill_registry.rs @@ -5,7 +5,6 @@ use anyhow::{Result, bail}; use indexmap::IndexMap; use std::collections::BTreeSet; -#[allow(dead_code)] #[derive(Clone, Default)] pub struct SkillRegistry { loaded: IndexMap, diff --git a/src/function/mod.rs b/src/function/mod.rs index 487038e..8ff69e7 100644 --- a/src/function/mod.rs +++ b/src/function/mod.rs @@ -355,7 +355,6 @@ impl Functions { self.declarations.extend(todo::todo_function_declarations()); } - #[allow(dead_code)] pub fn append_skill_functions(&mut self) { self.declarations .extend(skill::skill_function_declarations()); diff --git a/src/function/skill.rs b/src/function/skill.rs index ec663ed..9a24c36 100644 --- a/src/function/skill.rs +++ b/src/function/skill.rs @@ -9,7 +9,6 @@ use serde_json::{Value, json}; pub const SKILL_FUNCTION_PREFIX: &str = "skill__"; -#[allow(dead_code)] pub fn skill_function_declarations() -> Vec { vec![ FunctionDeclaration {