From 9fd73601c80e756af4513d9ffb076c7ab273cd3d Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Wed, 4 Mar 2026 11:00:34 -0500 Subject: [PATCH] feat(skills): load skill bodies on demand in compact mode --- src/config/schema.rs | 13 ++-- src/skills/mod.rs | 179 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 158 insertions(+), 34 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index db21d2a3d..410ca7a8b 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1225,11 +1225,11 @@ impl Default for AgentSessionConfig { #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, Default)] #[serde(rename_all = "snake_case")] pub enum SkillsPromptInjectionMode { - /// Inline full skill instructions and tool metadata into the system prompt. - #[default] - Full, /// Inline only compact skill metadata (name/description/location) and load details on demand. + #[default] Compact, + /// Inline full skill instructions and tool metadata into the system prompt. + Full, } fn parse_skills_prompt_injection_mode(raw: &str) -> Option { @@ -1261,7 +1261,8 @@ pub struct SkillsConfig { #[serde(default)] pub allow_scripts: bool, /// Controls how skills are injected into the system prompt. - /// `full` preserves legacy behavior. `compact` keeps context small and loads skills on demand. + /// `compact` (default) keeps context small and loads skills on demand. + /// `full` preserves legacy behavior as an opt-in. #[serde(default)] pub prompt_injection_mode: SkillsPromptInjectionMode, /// Optional ClawhHub API token for authenticated skill downloads. @@ -9774,7 +9775,7 @@ mod tests { assert!(!c.skills.allow_scripts); assert_eq!( c.skills.prompt_injection_mode, - SkillsPromptInjectionMode::Full + SkillsPromptInjectionMode::Compact ); assert!(c.workspace_dir.to_string_lossy().contains("workspace")); assert!(c.config_path.to_string_lossy().contains("config.toml")); @@ -12380,7 +12381,7 @@ requires_openai_auth = true assert!(config.skills.open_skills_dir.is_none()); assert_eq!( config.skills.prompt_injection_mode, - SkillsPromptInjectionMode::Full + SkillsPromptInjectionMode::Compact ); std::env::set_var("ZEROCLAW_OPEN_SKILLS_ENABLED", "true"); diff --git a/src/skills/mod.rs b/src/skills/mod.rs index b198d9f9d..231672ab3 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -62,6 +62,11 @@ struct SkillManifest { prompts: Vec, } +#[derive(Debug, Clone, Serialize, Deserialize)] +struct SkillMetadataManifest { + skill: SkillMeta, +} + #[derive(Debug, Clone, Serialize, Deserialize)] struct SkillMeta { name: String, @@ -78,9 +83,24 @@ fn default_version() -> String { "0.1.0".to_string() } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SkillLoadMode { + Full, + MetadataOnly, +} + +impl SkillLoadMode { + fn from_prompt_mode(mode: crate::config::SkillsPromptInjectionMode) -> Self { + match mode { + crate::config::SkillsPromptInjectionMode::Full => Self::Full, + crate::config::SkillsPromptInjectionMode::Compact => Self::MetadataOnly, + } + } +} + /// Load all skills from the workspace skills directory pub fn load_skills(workspace_dir: &Path) -> Vec { - load_skills_with_open_skills_config(workspace_dir, None, None, None, None) + load_skills_with_open_skills_config(workspace_dir, None, None, None, None, SkillLoadMode::Full) } /// Load skills using runtime config values (preferred at runtime). @@ -91,6 +111,21 @@ pub fn load_skills_with_config(workspace_dir: &Path, config: &crate::config::Con config.skills.open_skills_dir.as_deref(), Some(config.skills.allow_scripts), Some(&config.skills.trusted_skill_roots), + SkillLoadMode::from_prompt_mode(config.skills.prompt_injection_mode), + ) +} + +fn load_skills_full_with_config( + workspace_dir: &Path, + config: &crate::config::Config, +) -> Vec { + load_skills_with_open_skills_config( + workspace_dir, + Some(config.skills.open_skills_enabled), + config.skills.open_skills_dir.as_deref(), + Some(config.skills.allow_scripts), + Some(&config.skills.trusted_skill_roots), + SkillLoadMode::Full, ) } @@ -100,6 +135,7 @@ fn load_skills_with_open_skills_config( config_open_skills_dir: Option<&str>, config_allow_scripts: Option, config_trusted_skill_roots: Option<&[String]>, + load_mode: SkillLoadMode, ) -> Vec { let mut skills = Vec::new(); let allow_scripts = config_allow_scripts.unwrap_or(false); @@ -109,13 +145,14 @@ fn load_skills_with_open_skills_config( if let Some(open_skills_dir) = ensure_open_skills_repo(config_open_skills_enabled, config_open_skills_dir) { - skills.extend(load_open_skills(&open_skills_dir, allow_scripts)); + skills.extend(load_open_skills(&open_skills_dir, allow_scripts, load_mode)); } skills.extend(load_workspace_skills( workspace_dir, allow_scripts, &trusted_skill_roots, + load_mode, )); skills } @@ -124,9 +161,10 @@ fn load_workspace_skills( workspace_dir: &Path, allow_scripts: bool, trusted_skill_roots: &[PathBuf], + load_mode: SkillLoadMode, ) -> Vec { let skills_dir = workspace_dir.join("skills"); - load_skills_from_directory(&skills_dir, allow_scripts, trusted_skill_roots) + load_skills_from_directory(&skills_dir, allow_scripts, trusted_skill_roots, load_mode) } fn resolve_trusted_skill_roots(workspace_dir: &Path, raw_roots: &[String]) -> Vec { @@ -218,6 +256,7 @@ fn load_skills_from_directory( skills_dir: &Path, allow_scripts: bool, trusted_skill_roots: &[PathBuf], + load_mode: SkillLoadMode, ) -> Vec { if !skills_dir.exists() { return Vec::new(); @@ -281,11 +320,11 @@ fn load_skills_from_directory( let md_path = path.join("SKILL.md"); if manifest_path.exists() { - if let Ok(skill) = load_skill_toml(&manifest_path) { + if let Ok(skill) = load_skill_toml(&manifest_path, load_mode) { skills.push(skill); } } else if md_path.exists() { - if let Ok(skill) = load_skill_md(&md_path, &path) { + if let Ok(skill) = load_skill_md(&md_path, &path, load_mode) { skills.push(skill); } } @@ -294,13 +333,13 @@ fn load_skills_from_directory( skills } -fn load_open_skills(repo_dir: &Path, allow_scripts: bool) -> Vec { +fn load_open_skills(repo_dir: &Path, allow_scripts: bool, load_mode: SkillLoadMode) -> Vec { // Modern open-skills layout stores skill packages in `skills//SKILL.md`. // Prefer that structure to avoid treating repository docs (e.g. CONTRIBUTING.md) // as executable skills. let nested_skills_dir = repo_dir.join("skills"); if nested_skills_dir.is_dir() { - return load_skills_from_directory(&nested_skills_dir, allow_scripts, &[]); + return load_skills_from_directory(&nested_skills_dir, allow_scripts, &[], load_mode); } let mut skills = Vec::new(); @@ -350,7 +389,7 @@ fn load_open_skills(repo_dir: &Path, allow_scripts: bool) -> Vec { } } - if let Ok(skill) = load_open_skill_md(&path) { + if let Ok(skill) = load_open_skill_md(&path, load_mode) { skills.push(skill); } } @@ -544,25 +583,42 @@ fn mark_open_skills_synced(repo_dir: &Path) -> Result<()> { } /// Load a skill from a SKILL.toml manifest -fn load_skill_toml(path: &Path) -> Result { +fn load_skill_toml(path: &Path, load_mode: SkillLoadMode) -> Result { let content = std::fs::read_to_string(path)?; - let manifest: SkillManifest = toml::from_str(&content)?; - - Ok(Skill { - name: manifest.skill.name, - description: manifest.skill.description, - version: manifest.skill.version, - author: manifest.skill.author, - tags: manifest.skill.tags, - tools: manifest.tools, - prompts: manifest.prompts, - location: Some(path.to_path_buf()), - always: false, - }) + match load_mode { + SkillLoadMode::Full => { + let manifest: SkillManifest = toml::from_str(&content)?; + Ok(Skill { + name: manifest.skill.name, + description: manifest.skill.description, + version: manifest.skill.version, + author: manifest.skill.author, + tags: manifest.skill.tags, + tools: manifest.tools, + prompts: manifest.prompts, + location: Some(path.to_path_buf()), + always: false, + }) + } + SkillLoadMode::MetadataOnly => { + let manifest: SkillMetadataManifest = toml::from_str(&content)?; + Ok(Skill { + name: manifest.skill.name, + description: manifest.skill.description, + version: manifest.skill.version, + author: manifest.skill.author, + tags: manifest.skill.tags, + tools: Vec::new(), + prompts: Vec::new(), + location: Some(path.to_path_buf()), + always: false, + }) + } + } } /// Load a skill from a SKILL.md file (simpler format) -fn load_skill_md(path: &Path, dir: &Path) -> Result { +fn load_skill_md(path: &Path, dir: &Path, load_mode: SkillLoadMode) -> Result { let content = std::fs::read_to_string(path)?; let (fm, body) = parse_front_matter(&content); let mut name = dir @@ -617,6 +673,10 @@ fn load_skill_md(path: &Path, dir: &Path) -> Result { } else { body.to_string() }; + let prompts = match load_mode { + SkillLoadMode::Full => vec![prompt_body], + SkillLoadMode::MetadataOnly => Vec::new(), + }; Ok(Skill { name, @@ -625,19 +685,23 @@ fn load_skill_md(path: &Path, dir: &Path) -> Result { author, tags: Vec::new(), tools: Vec::new(), - prompts: vec![prompt_body], + prompts, location: Some(path.to_path_buf()), always, }) } -fn load_open_skill_md(path: &Path) -> Result { +fn load_open_skill_md(path: &Path, load_mode: SkillLoadMode) -> Result { let content = std::fs::read_to_string(path)?; let name = path .file_stem() .and_then(|n| n.to_str()) .unwrap_or("open-skill") .to_string(); + let prompts = match load_mode { + SkillLoadMode::Full => vec![content.clone()], + SkillLoadMode::MetadataOnly => Vec::new(), + }; Ok(Skill { name, @@ -646,7 +710,7 @@ fn load_open_skill_md(path: &Path) -> Result { author: Some("besoeasy/open-skills".to_string()), tags: vec!["open-skills".to_string()], tools: Vec::new(), - prompts: vec![content], + prompts, location: Some(path.to_path_buf()), always: false, }) @@ -2227,7 +2291,7 @@ pub fn handle_command(command: crate::SkillCommands, config: &crate::config::Con } crate::SkillCommands::List => { - let skills = load_skills_with_config(workspace_dir, config); + let skills = load_skills_full_with_config(workspace_dir, config); if skills.is_empty() { println!("No skills installed."); println!(); @@ -2679,7 +2743,7 @@ Body text that should be included. ) .unwrap(); - let skill = load_skill_md(&skill_md, &skill_dir).unwrap(); + let skill = load_skill_md(&skill_md, &skill_dir, SkillLoadMode::Full).unwrap(); assert_eq!(skill.name, "overridden-name"); assert_eq!(skill.version, "2.1.3"); assert_eq!(skill.author.as_deref(), Some("alice")); @@ -3048,6 +3112,65 @@ description = "Bare minimum" assert_ne!(skills[0].name, "CONTRIBUTING"); } + #[test] + fn load_skills_with_config_compact_mode_uses_metadata_only() { + let dir = tempfile::tempdir().unwrap(); + let workspace_dir = dir.path().join("workspace"); + let skills_dir = workspace_dir.join("skills"); + fs::create_dir_all(&skills_dir).unwrap(); + + let md_skill = skills_dir.join("md-meta"); + fs::create_dir_all(&md_skill).unwrap(); + fs::write( + md_skill.join("SKILL.md"), + "# Metadata\nMetadata summary line\nUse this only when needed.\n", + ) + .unwrap(); + + let toml_skill = skills_dir.join("toml-meta"); + fs::create_dir_all(&toml_skill).unwrap(); + fs::write( + toml_skill.join("SKILL.toml"), + r#" +[skill] +name = "toml-meta" +description = "Toml metadata description" +version = "1.2.3" + +[[tools]] +name = "dangerous-tool" +description = "Should not preload" +kind = "shell" +command = "echo no" + +prompts = ["Do not preload me"] +"#, + ) + .unwrap(); + + let mut config = crate::config::Config::default(); + config.workspace_dir = workspace_dir.clone(); + config.skills.prompt_injection_mode = crate::config::SkillsPromptInjectionMode::Compact; + + let mut skills = load_skills_with_config(&workspace_dir, &config); + skills.sort_by(|a, b| a.name.cmp(&b.name)); + + assert_eq!(skills.len(), 2); + + let md = skills.iter().find(|skill| skill.name == "md-meta").unwrap(); + assert_eq!(md.description, "Metadata summary line"); + assert!(md.prompts.is_empty()); + assert!(md.tools.is_empty()); + + let toml = skills + .iter() + .find(|skill| skill.name == "toml-meta") + .unwrap(); + assert_eq!(toml.description, "Toml metadata description"); + assert!(toml.prompts.is_empty()); + assert!(toml.tools.is_empty()); + } + // ── is_registry_source ──────────────────────────────────────────────────── // ── registry install: directory naming ───────────────────────────────────