fmt: Applied uniform formatting to skills implementation

This commit is contained in:
2026-06-01 15:21:00 -06:00
parent 81a2bd1d00
commit d07caf2a4b
8 changed files with 58 additions and 118 deletions
+1 -1
View File
@@ -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::<String>(&get_env_name("enabled_skills")) {
self.enabled_skills = v;
}
+6 -1
View File
@@ -375,7 +375,12 @@ fn plan_changes(layout: &RemoteLayout) -> Result<InstallPlan> {
}
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 {
+11 -22
View File
@@ -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"),
+1 -3
View File
@@ -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)
+7 -10
View File
@@ -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<Self> {
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);
+25 -75
View File
@@ -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());
}
+6 -5
View File
@@ -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();