From 64e79da04390a0c67bad600016792c28f699bb93 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Mon, 1 Jun 2026 12:58:42 -0600 Subject: [PATCH] feat: created built-in functions for listing, loading, and unloading skills --- src/config/request_context.rs | 6 + src/config/skill.rs | 2 + src/config/skill_registry.rs | 20 ++- src/function/mod.rs | 15 ++ src/function/skill.rs | 298 ++++++++++++++++++++++++++++++++++ 5 files changed, 334 insertions(+), 7 deletions(-) create mode 100644 src/function/skill.rs diff --git a/src/config/request_context.rs b/src/config/request_context.rs index c12c7eb..e7a08ce 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -1,5 +1,6 @@ use super::rag_cache::{RagCache, RagKey}; use super::session::Session; +use super::skill_registry::SkillRegistry; use super::todo::TodoList; use super::tool_scope::{McpRuntime, ToolScope}; use super::{ @@ -82,6 +83,7 @@ pub struct RequestContext { pub current_depth: usize, pub auto_continue_count: usize, pub todo_list: TodoList, + pub skill_registry: SkillRegistry, pub last_continuation_response: Option, pub render_mode: RenderMode, @@ -110,6 +112,7 @@ impl RequestContext { current_depth: 0, auto_continue_count: 0, todo_list: TodoList::default(), + skill_registry: SkillRegistry::default(), last_continuation_response: None, render_mode: RenderMode::default(), } @@ -157,6 +160,7 @@ impl RequestContext { current_depth: 0, auto_continue_count: 0, todo_list: TodoList::default(), + skill_registry: SkillRegistry::default(), last_continuation_response: None, render_mode: RenderMode::default(), }) @@ -198,6 +202,7 @@ impl RequestContext { current_depth: self.current_depth, auto_continue_count: 0, todo_list: self.todo_list.clone(), + skill_registry: self.skill_registry.clone(), last_continuation_response: None, render_mode: self.render_mode, } @@ -237,6 +242,7 @@ impl RequestContext { current_depth, auto_continue_count: 0, todo_list: TodoList::default(), + skill_registry: SkillRegistry::default(), last_continuation_response: None, render_mode: parent.render_mode, } diff --git a/src/config/skill.rs b/src/config/skill.rs index 23f85e6..df938ec 100644 --- a/src/config/skill.rs +++ b/src/config/skill.rs @@ -123,9 +123,11 @@ impl Skill { if self.declares_tools() && !function_calling_enabled { return false; } + if self.declares_mcp_servers() && !mcp_enabled { return false; } + true } diff --git a/src/config/skill_registry.rs b/src/config/skill_registry.rs index 0cd2001..7cd5f49 100644 --- a/src/config/skill_registry.rs +++ b/src/config/skill_registry.rs @@ -6,6 +6,7 @@ use indexmap::IndexMap; use std::collections::BTreeSet; #[allow(dead_code)] +#[derive(Clone, Default)] pub struct SkillRegistry { loaded: IndexMap, } @@ -22,13 +23,24 @@ impl SkillRegistry { if self.loaded.contains_key(name) { bail!("Skill '{name}' is already loaded"); } - let skill = Skill::load(name)?; self.loaded.insert(name.to_string(), skill); Ok(()) } + pub fn insert(&mut self, skill: Skill) -> Result<()> { + let name = skill.name().to_string(); + + if self.loaded.contains_key(&name) { + bail!("Skill '{name}' is already loaded"); + } + + self.loaded.insert(name, skill); + + Ok(()) + } + pub fn unload(&mut self, name: &str) -> Result<()> { if self.loaded.shift_remove(name).is_none() { bail!("Skill '{name}' is not loaded"); @@ -85,12 +97,6 @@ impl SkillRegistry { } } -impl Default for SkillRegistry { - fn default() -> Self { - Self::new() - } -} - fn parse_csv(s: Option<&str>) -> BTreeSet { let mut set = BTreeSet::new(); if let Some(raw) = s { diff --git a/src/function/mod.rs b/src/function/mod.rs index 517745d..cc858e4 100644 --- a/src/function/mod.rs +++ b/src/function/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod skill; pub(crate) mod supervisor; pub(crate) mod todo; pub(crate) mod user_interaction; @@ -32,6 +33,7 @@ use std::{ process::{Command, Stdio}, }; use strum_macros::AsRefStr; +use skill::SKILL_FUNCTION_PREFIX; use supervisor::SUPERVISOR_FUNCTION_PREFIX; use todo::TODO_FUNCTION_PREFIX; use user_interaction::USER_FUNCTION_PREFIX; @@ -353,6 +355,12 @@ 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()); + } + pub fn append_supervisor_functions(&mut self) { self.declarations .extend(supervisor::supervisor_function_declarations()); @@ -1039,6 +1047,13 @@ impl ToolCall { json!({"tool_call_error": error_msg}) }) } + _ if cmd_name.starts_with(SKILL_FUNCTION_PREFIX) => { + skill::handle_skill_tool(ctx, &cmd_name, &json_data).unwrap_or_else(|e| { + let error_msg = format!("Skill tool failed: {e}"); + eprintln!("{}", warning_text(&format!("⚠️ {error_msg} ⚠️"))); + json!({"tool_call_error": error_msg}) + }) + } _ if cmd_name.starts_with(SUPERVISOR_FUNCTION_PREFIX) => { supervisor::handle_supervisor_tool(ctx, &cmd_name, &json_data) .await diff --git a/src/function/skill.rs b/src/function/skill.rs new file mode 100644 index 0000000..2c62941 --- /dev/null +++ b/src/function/skill.rs @@ -0,0 +1,298 @@ +use super::{FunctionDeclaration, JsonSchema}; +use crate::config::{RequestContext, Skill, SkillPolicy, paths}; + +use anyhow::{Result, bail}; +use indexmap::IndexMap; +use log::warn; +use serde_json::{Value, json}; + +pub const SKILL_FUNCTION_PREFIX: &str = "skill__"; + +#[allow(dead_code)] +pub fn skill_function_declarations() -> Vec { + vec![ + FunctionDeclaration { + name: format!("{SKILL_FUNCTION_PREFIX}list"), + description: + "List skills available in this context. Returns each skill's name, description, \ + what tools and MCP servers it grants on load, and whether it is currently loaded. \ + Call this to discover skills before using skill__load." + .to_string(), + parameters: JsonSchema { + type_value: Some("object".to_string()), + properties: Some(IndexMap::new()), + ..Default::default() + }, + agent: false, + }, + FunctionDeclaration { + name: format!("{SKILL_FUNCTION_PREFIX}load"), + description: + "Load a skill module into the current context. The skill's instructions and any \ + tools or MCP servers it grants become active for subsequent turns. Call \ + skill__unload when the skill's work is complete to keep the context lean." + .to_string(), + parameters: JsonSchema { + type_value: Some("object".to_string()), + properties: Some(IndexMap::from([( + "name".to_string(), + JsonSchema { + type_value: Some("string".to_string()), + description: Some("Name of the skill to load.".into()), + ..Default::default() + }, + )])), + required: Some(vec!["name".to_string()]), + ..Default::default() + }, + agent: false, + }, + FunctionDeclaration { + name: format!("{SKILL_FUNCTION_PREFIX}unload"), + description: + "Unload a previously loaded skill, removing its instructions and granted tools \ + from the context. Call this when the skill's work is complete." + .to_string(), + parameters: JsonSchema { + type_value: Some("object".to_string()), + properties: Some(IndexMap::from([( + "name".to_string(), + JsonSchema { + type_value: Some("string".to_string()), + description: Some("Name of the skill to unload.".into()), + ..Default::default() + }, + )])), + required: Some(vec!["name".to_string()]), + ..Default::default() + }, + agent: false, + }, + ] +} + +pub fn handle_skill_tool( + ctx: &mut RequestContext, + cmd_name: &str, + args: &Value, +) -> Result { + let action = cmd_name + .strip_prefix(SKILL_FUNCTION_PREFIX) + .unwrap_or(cmd_name); + + let policy = SkillPolicy::effective( + &ctx.app.config, + ctx.role.as_ref(), + ctx.agent.as_ref(), + ctx.session.as_ref(), + )?; + + if !policy.skills_enabled { + return Ok(json!({ + "error": "Skills are disabled in this context" + })); + } + + match action { + "list" => handle_list(ctx, &policy), + "load" => handle_load(ctx, args, &policy), + "unload" => handle_unload(ctx, args), + _ => bail!("Unknown skill action: {action}"), + } +} + +fn handle_list(ctx: &RequestContext, policy: &SkillPolicy) -> Result { + let function_calling_on = ctx.app.config.function_calling_support; + let mcp_on = ctx.app.config.mcp_server_support; + + let mut entries = Vec::new(); + for name in paths::list_skills() { + if !policy.allows(&name) { + continue; + } + + let skill = match Skill::load(&name) { + Ok(s) => s, + Err(e) => { + warn!("Failed to load skill '{name}' for listing: {e}"); + continue; + } + }; + if !skill.is_compatible(function_calling_on, mcp_on) { + warn!( + "Skill '{name}' filtered from list: declares tools or MCP servers but those features are disabled" + ); + continue; + } + + entries.push(json!({ + "name": skill.name(), + "description": skill.description(), + "grants_tools": csv_to_vec(skill.enabled_tools()), + "grants_mcp_servers": csv_to_vec(skill.enabled_mcp_servers()), + "loaded": ctx.skill_registry.is_loaded(skill.name()), + })); + } + + Ok(json!({"skills": entries})) +} + +fn handle_load( + ctx: &mut RequestContext, + args: &Value, + policy: &SkillPolicy, +) -> Result { + let name = match args.get("name").and_then(Value::as_str) { + Some(n) if !n.is_empty() => n, + _ => return Ok(json!({"error": "name is required"})), + }; + + if !policy.allows(name) { + return Ok(json!({ + "error": format!("Skill '{name}' is not enabled in this context") + })); + } + + let skill = match Skill::load(name) { + Ok(s) => s, + Err(e) => { + return Ok(json!({ + "error": format!("Failed to load skill '{name}': {e}") + })); + } + }; + + let function_calling_on = ctx.app.config.function_calling_support; + let mcp_on = ctx.app.config.mcp_server_support; + + let tools_declared = skill + .enabled_tools() + .map(|s| !s.trim().is_empty()) + .unwrap_or(false); + let mcps_declared = skill + .enabled_mcp_servers() + .map(|s| !s.trim().is_empty()) + .unwrap_or(false); + + if tools_declared && !function_calling_on { + return Ok(json!({ + "error": format!( + "Skill '{name}' requires function calling, which is disabled in this context" + ) + })); + } + if mcps_declared && !mcp_on { + return Ok(json!({ + "error": format!( + "Skill '{name}' requires MCP servers, which are disabled in this context" + ) + })); + } + + match ctx.skill_registry.insert(skill) { + Ok(()) => Ok(json!({ + "status": "ok", + "loaded": name, + "message": format!("Skill '{name}' loaded") + })), + Err(e) => Ok(json!({"error": e.to_string()})), + } +} + +fn handle_unload(ctx: &mut RequestContext, args: &Value) -> Result { + let name = match args.get("name").and_then(Value::as_str) { + Some(n) if !n.is_empty() => n, + _ => return Ok(json!({"error": "name is required"})), + }; + + match ctx.skill_registry.unload(name) { + Ok(()) => Ok(json!({ + "status": "ok", + "unloaded": name + })), + Err(e) => Ok(json!({"error": e.to_string()})), + } +} + +fn csv_to_vec(csv: Option<&str>) -> Vec { + csv.map(|raw| { + raw.split(',') + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) + .collect() + }) + .unwrap_or_default() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn declarations_have_three_entries() { + let decls = skill_function_declarations(); + assert_eq!(decls.len(), 3); + } + + #[test] + fn declaration_names_use_skill_prefix() { + let decls = skill_function_declarations(); + + let names: Vec<&str> = decls.iter().map(|d| d.name.as_str()).collect(); + + assert!(names.contains(&"skill__list")); + assert!(names.contains(&"skill__load")); + assert!(names.contains(&"skill__unload")); + } + + #[test] + fn load_and_unload_require_name_parameter() { + let decls = skill_function_declarations(); + for action in ["load", "unload"] { + let decl = decls + .iter() + .find(|d| d.name == format!("skill__{action}")) + .expect("missing declaration"); + + let required = decl + .parameters + .required + .as_ref() + .expect("required field missing"); + + assert!(required.contains(&"name".to_string())); + } + } + + #[test] + fn list_has_no_required_parameters() { + let decls = skill_function_declarations(); + let list_decl = decls + .iter() + .find(|d| d.name == "skill__list") + .expect("skill__list missing"); + + let required = list_decl + .parameters + .required + .as_ref() + .map(|v| v.is_empty()) + .unwrap_or(true); + + assert!(required, "skill__list should have no required parameters"); + } + + #[test] + fn csv_to_vec_empty_input() { + assert!(csv_to_vec(None).is_empty()); + assert!(csv_to_vec(Some("")).is_empty()); + assert!(csv_to_vec(Some(" ")).is_empty()); + } + + #[test] + fn csv_to_vec_parses_and_trims() { + let v = csv_to_vec(Some("a, b ,c,, d")); + + assert_eq!(v, vec!["a", "b", "c", "d"]); + } +}