From a058c4c766dd86a47acc6c851615deb8119063dc Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Mon, 1 Jun 2026 15:21:00 -0600 Subject: [PATCH] fmt: Applied uniform formatting to skills implementation --- src/config/app_config.rs | 2 +- src/config/install_remote.rs | 7 ++- src/config/request_context.rs | 33 ++++------- src/config/role.rs | 4 +- src/config/skill.rs | 17 +++--- src/config/skill_policy.rs | 100 +++++++++------------------------- src/config/skill_registry.rs | 11 ++-- src/function/mod.rs | 2 +- 8 files changed, 58 insertions(+), 118 deletions(-) diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 6450d61..057f82f 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -394,7 +394,7 @@ impl AppConfig { 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; } diff --git a/src/config/install_remote.rs b/src/config/install_remote.rs index d900bb1..0c90cc2 100644 --- a/src/config/install_remote.rs +++ b/src/config/install_remote.rs @@ -375,7 +375,12 @@ fn plan_changes(layout: &RemoteLayout) -> Result { } if let Some(src_dir) = &layout.skills { - plan_dir_into(src_dir, &paths::skills_dir(), TopCategory::Skills, &mut files)?; + plan_dir_into( + src_dir, + &paths::skills_dir(), + TopCategory::Skills, + &mut files, + )?; } if let Some(src_dir) = &layout.macros { diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 738315d..284e10a 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -2492,9 +2492,8 @@ impl RequestContext { ensure_parent_exists(&path)?; let is_new = !path.exists(); if is_new { - fs::write(&path, SKILL_SCAFFOLD).with_context(|| { - format!("Failed to scaffold skill at {}", path.display()) - })?; + fs::write(&path, SKILL_SCAFFOLD) + .with_context(|| format!("Failed to scaffold skill at {}", path.display()))?; } let editor = app.editor()?; edit_file(&editor, &path)?; @@ -2506,11 +2505,7 @@ impl RequestContext { Ok(()) } - pub async fn load_skill_repl( - &mut self, - name: &str, - abort_signal: AbortSignal, - ) -> Result<()> { + pub async fn load_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> { if !paths::has_skill(name) { bail!( "Skill '{name}' is not installed (expected at {})", @@ -2563,17 +2558,11 @@ impl RequestContext { Ok(()) } - pub async fn unload_skill_repl( - &mut self, - name: &str, - abort_signal: AbortSignal, - ) -> Result<()> { + pub async fn unload_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> { self.skill_registry.unload(name)?; if let Err(e) = self.refresh_tool_scope(abort_signal).await { - eprintln!( - "Warning: unloaded skill '{name}' but tool scope refresh failed: {e}" - ); + eprintln!("Warning: unloaded skill '{name}' but tool scope refresh failed: {e}"); } println!("✓ Unloaded skill '{name}'."); @@ -3534,7 +3523,8 @@ mod tests { let input = Input::from_str(&ctx, "hello", None); let app = Arc::clone(&ctx.app.config); - ctx.after_chat_completion(app.as_ref(), &input, "response", &[]).unwrap(); + ctx.after_chat_completion(app.as_ref(), &input, "response", &[]) + .unwrap(); assert!(!ctx.skill_registry.is_loaded("ephemeral")); assert!(ctx.skill_registry.is_loaded("persistent")); @@ -3556,11 +3546,10 @@ mod tests { let input = Input::from_str(&ctx, "hello", None); let app = Arc::clone(&ctx.app.config); - let tool_result = ToolResult::new( - crate::function::ToolCall::default(), - serde_json::json!({}), - ); - ctx.after_chat_completion(app.as_ref(), &input, "", &[tool_result]).unwrap(); + let tool_result = + ToolResult::new(crate::function::ToolCall::default(), serde_json::json!({})); + ctx.after_chat_completion(app.as_ref(), &input, "", &[tool_result]) + .unwrap(); assert!( ctx.skill_registry.is_loaded("ephemeral"), diff --git a/src/config/role.rs b/src/config/role.rs index 85c8a78..cf2d42c 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -103,9 +103,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 = 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) diff --git a/src/config/skill.rs b/src/config/skill.rs index 544a512..c5e5d58 100644 --- a/src/config/skill.rs +++ b/src/config/skill.rs @@ -2,11 +2,11 @@ use super::*; use anyhow::Result; use fancy_regex::Regex; +use log::{debug, info}; 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/"] @@ -93,9 +93,8 @@ impl Skill { 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 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()); @@ -118,9 +117,8 @@ impl Skill { pub fn load(name: &str) -> Result { let path = paths::skill_file(name); - let content = read_to_string(&path).with_context(|| { - format!("Failed to read skill '{name}' at {}", path.display()) - })?; + let content = read_to_string(&path) + .with_context(|| format!("Failed to read skill '{name}' at {}", path.display()))?; Ok(Skill::new(name, &content)) } @@ -152,7 +150,7 @@ impl Skill { if self.declares_tools() && !function_calling_enabled { return false; } - + if self.declares_mcp_servers() && !mcp_enabled { return false; } @@ -307,8 +305,7 @@ mod tests { #[test] fn is_compatible_requires_both_when_both_declared() { - let content = - "---\nenabled_tools: shell\nenabled_mcp_servers: github\n---\nbody"; + let content = "---\nenabled_tools: shell\nenabled_mcp_servers: github\n---\nbody"; let skill = Skill::new("test", content); diff --git a/src/config/skill_policy.rs b/src/config/skill_policy.rs index 5f42818..a47ffa8 100644 --- a/src/config/skill_policy.rs +++ b/src/config/skill_policy.rs @@ -145,15 +145,9 @@ mod tests { 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(); + let policy = + SkillPolicy::effective_with(&global, None, None, None, &always_true, &empty_installed) + .unwrap(); assert!(policy.skills_enabled); assert!(policy.enabled.is_empty()); @@ -177,15 +171,9 @@ mod tests { 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(); + 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")); @@ -196,15 +184,9 @@ mod tests { 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(); + let policy = + SkillPolicy::effective_with(&global, None, None, None, &always_true, &empty_installed) + .unwrap(); assert!(policy.enabled.contains("alpha")); assert!(policy.enabled.contains("beta")); @@ -214,10 +196,7 @@ mod tests { #[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 role = Role::new("test", "---\nenabled_skills: beta\n---\nbody"); let policy = SkillPolicy::effective_with( &global, @@ -258,14 +237,9 @@ mod tests { ..AppConfig::default() }; - let policy = SkillPolicy::effective_with( - &global, - None, - None, - None, - &always_true, - &|| vec!["alpha".to_string()], - ) + let policy = SkillPolicy::effective_with(&global, None, None, None, &always_true, &|| { + vec!["alpha".to_string()] + }) .unwrap(); assert!(!policy.allows("alpha")); @@ -275,15 +249,9 @@ mod tests { 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(); + let policy = + SkillPolicy::effective_with(&global, None, None, None, &always_true, &empty_installed) + .unwrap(); assert!(policy.allows("alpha")); assert!(!policy.allows("beta")); @@ -293,15 +261,9 @@ mod tests { 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(); + 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")); @@ -311,15 +273,9 @@ mod tests { 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(); + 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")); @@ -329,15 +285,9 @@ mod tests { 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(); + let policy = + SkillPolicy::effective_with(&global, None, None, None, &|_| false, &empty_installed) + .unwrap(); assert!(policy.enabled.is_empty()); } diff --git a/src/config/skill_registry.rs b/src/config/skill_registry.rs index 0aa3fa6..cf81310 100644 --- a/src/config/skill_registry.rs +++ b/src/config/skill_registry.rs @@ -93,7 +93,11 @@ impl SkillRegistry { tools.extend(parse_csv(skill.enabled_tools())); mcps.extend(parse_csv(skill.enabled_mcp_servers())); if !skip_body && !skill.body().is_empty() { - let separator = if effective.is_empty_prompt() { "" } else { "\n\n" }; + let separator = if effective.is_empty_prompt() { + "" + } else { + "\n\n" + }; effective.append_to_prompt(separator); effective.append_to_prompt(skill.body()); } @@ -242,10 +246,7 @@ mod tests { let tools_str = effective.enabled_tools().unwrap(); let tools: BTreeSet<&str> = tools_str.split(',').collect(); - assert_eq!( - tools, - BTreeSet::from(["fs", "git", "shell", "web_search"]) - ); + assert_eq!(tools, BTreeSet::from(["fs", "git", "shell", "web_search"])); let mcps_str = effective.enabled_mcp_servers().unwrap(); let mcps: BTreeSet<&str> = mcps_str.split(',').collect(); diff --git a/src/function/mod.rs b/src/function/mod.rs index 8ff69e7..ba66837 100644 --- a/src/function/mod.rs +++ b/src/function/mod.rs @@ -22,6 +22,7 @@ use indoc::formatdoc; use rust_embed::Embed; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; +use skill::SKILL_FUNCTION_PREFIX; use std::collections::VecDeque; use std::ffi::OsStr; use std::fs::File; @@ -33,7 +34,6 @@ 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;