fmt: Applied uniform formatting to skills implementation
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
@@ -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
@@ -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
@@ -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());
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
+1
-1
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user