diff --git a/CHANGELOG.md b/CHANGELOG.md index 4962a370..9347d15d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -156,6 +156,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 paths. Defense-in-depth: if a future code path enables the flags explicitly, the suspend handlers won't leak them to a Vim / less / shell child that hasn't asked for them. +- **`load_skill` tool** (#434) — model-callable tool that takes a + skill id and returns the SKILL.md body plus the sibling + companion-file list in one call. Faster than the existing + `read_file` + `list_dir` dance; surfaces the skill's + description as a quote block at the head so a single tool + result is self-contained. Resolves the skills directory with + the same hierarchy `App::new` uses (`.agents/skills` → + `skills` → `~/.deepseek/skills`). Available in Plan and + Agent/Yolo modes. - **RLM tool family** (#512) — `rlm` tool cards map to `ToolFamily::Rlm` and render `rlm`, not `swarm`. Stale "swarm" wording cleaned out of docs / comments / tests. diff --git a/crates/tui/src/core/engine/tool_setup.rs b/crates/tui/src/core/engine/tool_setup.rs index 220d4706..35675b85 100644 --- a/crates/tui/src/core/engine/tool_setup.rs +++ b/crates/tui/src/core/engine/tool_setup.rs @@ -18,6 +18,7 @@ impl Engine { .with_git_tools() .with_git_history_tools() .with_diagnostics_tool() + .with_skill_tools() .with_validation_tools() .with_runtime_task_tools() .with_todo_tool(todo_list) diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index d07b57cb..27f9907d 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -182,6 +182,28 @@ impl SkillRegistry { /// Render a compact model-visible skills block. /// /// The full `SKILL.md` body is intentionally not included here. This mirrors +/// Resolve the active skills directory given a workspace, mirroring the +/// hierarchy `App::new` walks: `/.agents/skills` → +/// `/skills` → [`default_skills_dir`] (`~/.deepseek/skills`). +/// Returns the first directory that exists, or the global default +/// (which itself falls back to `/tmp/deepseek/skills` if the user +/// has no home directory). +/// +/// Used by the `load_skill` tool (#434) and other surfaces that need +/// the same lookup order without re-implementing it. +#[must_use] +pub fn resolve_skills_dir(workspace: &Path) -> PathBuf { + let agents = workspace.join(".agents").join("skills"); + if agents.exists() { + return agents; + } + let local = workspace.join("skills"); + if local.exists() { + return local; + } + default_skills_dir() +} + /// Codex's progressive-disclosure contract: the model sees skill names, /// descriptions, and paths up front, then opens the specific `SKILL.md` only /// when a skill is relevant. diff --git a/crates/tui/src/tools/mod.rs b/crates/tui/src/tools/mod.rs index 85d9d63e..9d8771c1 100644 --- a/crates/tui/src/tools/mod.rs +++ b/crates/tui/src/tools/mod.rs @@ -23,6 +23,7 @@ pub mod rlm; pub mod search; pub mod shell; mod shell_output; +pub mod skill; pub mod spec; pub mod subagent; pub mod tasks; diff --git a/crates/tui/src/tools/registry.rs b/crates/tui/src/tools/registry.rs index e1812567..007e3e22 100644 --- a/crates/tui/src/tools/registry.rs +++ b/crates/tui/src/tools/registry.rs @@ -360,6 +360,16 @@ impl ToolRegistryBuilder { self.with_tool(Arc::new(DiagnosticsTool)) } + /// Include the `load_skill` tool (#434) so the model can pull a + /// SKILL.md body + companion file list into context with one + /// call instead of `read_file` + `list_dir` against the path + /// shown in the system prompt's `## Skills` section. + #[must_use] + pub fn with_skill_tools(self) -> Self { + use super::skill::LoadSkillTool; + self.with_tool(Arc::new(LoadSkillTool)) + } + /// Include project mapping tools. #[must_use] pub fn with_project_tools(self) -> Self { @@ -547,6 +557,7 @@ impl ToolRegistryBuilder { .with_git_history_tools() .with_diagnostics_tool() .with_project_tools() + .with_skill_tools() .with_test_runner_tool() .with_validation_tools() .with_runtime_task_tools() diff --git a/crates/tui/src/tools/skill.rs b/crates/tui/src/tools/skill.rs new file mode 100644 index 00000000..20e1af04 --- /dev/null +++ b/crates/tui/src/tools/skill.rs @@ -0,0 +1,280 @@ +//! `load_skill` tool — fetch a `SKILL.md` body and its companion-file +//! list into the model's context (#434). +//! +//! ## Why a tool when skills already surface in the system prompt? +//! +//! `prompts.rs::system_prompt_for_mode_with_context_and_skills` injects +//! a one-line listing of every available skill (name + description + +//! file path) so the model knows what's in the catalogue at the start +//! of every turn. The full body of each skill is *not* loaded — that +//! would blow the prompt budget the moment a user has half a dozen +//! skills installed. +//! +//! Two paths exist for the model to actually read a skill: +//! +//! 1. The existing progressive-disclosure pattern: model spots a +//! skill in the catalogue, calls `read_file ` from the +//! listing. +//! 2. (this tool) `load_skill name=` — single call, name-based +//! lookup, also enumerates the sibling files in the skill's +//! directory so the model sees the companion resources without +//! a separate `list_dir`. +//! +//! Both are valid; the tool is the higher-level affordance and +//! avoids the two-call dance for skills that ship with multiple +//! resource files. + +use async_trait::async_trait; +use serde_json::{Value, json}; + +use crate::skills::{Skill, SkillRegistry, resolve_skills_dir}; + +use super::spec::{ + ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, +}; + +pub struct LoadSkillTool; + +#[async_trait] +impl ToolSpec for LoadSkillTool { + fn name(&self) -> &'static str { + "load_skill" + } + + fn description(&self) -> &'static str { + "Load a skill (SKILL.md body + companion file list) into the next turn's context. \ + Use this when the user names a skill or the task clearly matches a skill listed in the system prompt's `## Skills` section. Faster than read_file + list_dir." + } + + fn input_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "Skill id (the `name` field from the SKILL.md frontmatter, also shown in the `## Skills` listing)." + } + }, + "required": ["name"], + "additionalProperties": false + }) + } + + fn capabilities(&self) -> Vec { + vec![ToolCapability::ReadOnly] + } + + fn approval_requirement(&self) -> ApprovalRequirement { + ApprovalRequirement::Auto + } + + fn supports_parallel(&self) -> bool { + true + } + + async fn execute(&self, input: Value, context: &ToolContext) -> Result { + let name = input + .get("name") + .and_then(Value::as_str) + .ok_or_else(|| ToolError::missing_field("name"))? + .trim(); + if name.is_empty() { + return Err(ToolError::invalid_input( + "`name` must be a non-empty string", + )); + } + + let skills_dir = resolve_skills_dir(&context.workspace); + let registry = SkillRegistry::discover(&skills_dir); + let Some(skill) = registry.get(name) else { + // Build a name-list hint so the model can see what IS + // available without a follow-up `list_skills` call. + let available: Vec<&str> = registry.list().iter().map(|s| s.name.as_str()).collect(); + let hint = if available.is_empty() { + format!( + "no skills found in {}; expected `//SKILL.md`", + skills_dir.display() + ) + } else { + format!( + "skill `{name}` not found. Available: {}", + available.join(", ") + ) + }; + return Err(ToolError::execution_failed(hint)); + }; + + let body = format_skill_body(skill); + Ok(ToolResult::success(body).with_metadata(json!({ + "skill_name": skill.name, + "skill_path": skill.path.display().to_string(), + "companion_files": collect_companion_files(skill) + .into_iter() + .map(|p| p.display().to_string()) + .collect::>(), + }))) + } +} + +/// Render the skill body the model will see. Includes the description +/// up top so a single tool result is self-contained — no need to +/// cross-reference the system-prompt catalogue. Companion-file paths +/// land at the bottom under a clearly-named heading so the model can +/// open them with `read_file` if they're relevant to the task. +fn format_skill_body(skill: &Skill) -> String { + let mut out = String::new(); + out.push_str(&format!("# Skill: {}\n\n", skill.name)); + if !skill.description.trim().is_empty() { + out.push_str(&format!("> {}\n\n", skill.description.trim())); + } + out.push_str(&format!("Source: `{}`\n\n", skill.path.display())); + out.push_str("## SKILL.md\n\n"); + out.push_str(skill.body.trim()); + out.push('\n'); + + let companions = collect_companion_files(skill); + if !companions.is_empty() { + out.push_str("\n## Companion files\n\n"); + out.push_str( + "Sibling files in the skill directory. Use `read_file` to open them when the task requires.\n\n", + ); + for path in &companions { + out.push_str(&format!("- `{}`\n", path.display())); + } + } + out +} + +/// List sibling files of `SKILL.md` in the skill's own directory. +/// Skips the `SKILL.md` itself and any nested directories so the +/// listing stays focused on at-hand resources. Sorted lexically for +/// deterministic output (matters for transcript diffing in tests). +fn collect_companion_files(skill: &Skill) -> Vec { + let Some(dir) = skill.path.parent() else { + return Vec::new(); + }; + let mut entries: Vec = match std::fs::read_dir(dir) { + Ok(rd) => rd + .flatten() + .filter_map(|entry| { + let path = entry.path(); + let is_file = entry.file_type().is_ok_and(|ft| ft.is_file()); + let is_skill_md = path.file_name().and_then(|s| s.to_str()) == Some("SKILL.md"); + if is_file && !is_skill_md { + Some(path) + } else { + None + } + }) + .collect(), + Err(_) => Vec::new(), + }; + entries.sort(); + entries +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::tempdir; + + fn write_skill(dir: &std::path::Path, name: &str, description: &str, body: &str) { + let skill_dir = dir.join(name); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {description}\n---\n{body}\n"), + ) + .unwrap(); + } + + #[test] + fn load_skill_returns_skill_body_with_description_header() { + let tmp = tempdir().unwrap(); + write_skill( + tmp.path(), + "review-pr", + "Run a focused PR review", + "# Steps\n1. Read the diff.\n2. Comment.\n", + ); + let skill = SkillRegistry::discover(tmp.path()) + .get("review-pr") + .unwrap() + .clone(); + let body = format_skill_body(&skill); + assert!(body.contains("# Skill: review-pr")); + assert!(body.contains("Run a focused PR review")); + assert!(body.contains("# Steps")); + assert!(body.contains("Read the diff.")); + } + + #[test] + fn collect_companion_files_lists_siblings_excluding_skill_md() { + let tmp = tempdir().unwrap(); + let skill_dir = tmp.path().join("rich-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: rich-skill\ndescription: x\n---\nbody\n", + ) + .unwrap(); + fs::write(skill_dir.join("script.py"), "print('hi')").unwrap(); + fs::write(skill_dir.join("data.json"), "{}").unwrap(); + // Nested directory — skipped by collect_companion_files. + fs::create_dir_all(skill_dir.join("subdir")).unwrap(); + + let registry = SkillRegistry::discover(tmp.path()); + let skill = registry.get("rich-skill").unwrap(); + let files = collect_companion_files(skill); + let names: Vec = files + .iter() + .filter_map(|p| p.file_name().and_then(|s| s.to_str().map(str::to_string))) + .collect(); + assert_eq!( + names, + vec!["data.json".to_string(), "script.py".to_string()] + ); + } + + #[test] + fn collect_companion_files_returns_empty_for_solo_skill() { + let tmp = tempdir().unwrap(); + write_skill(tmp.path(), "solo", "Just a skill", "body"); + let registry = SkillRegistry::discover(tmp.path()); + let skill = registry.get("solo").unwrap(); + assert!(collect_companion_files(skill).is_empty()); + } + + #[test] + fn format_skill_body_emits_companion_files_section_when_present() { + let tmp = tempdir().unwrap(); + let skill_dir = tmp.path().join("skill-with-friends"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: skill-with-friends\ndescription: x\n---\nbody\n", + ) + .unwrap(); + fs::write(skill_dir.join("helper.sh"), "#!/bin/sh\necho hi").unwrap(); + + let registry = SkillRegistry::discover(tmp.path()); + let skill = registry.get("skill-with-friends").unwrap(); + let body = format_skill_body(skill); + assert!(body.contains("## Companion files")); + assert!(body.contains("helper.sh")); + } + + #[test] + fn format_skill_body_skips_companion_section_when_solo() { + let tmp = tempdir().unwrap(); + write_skill(tmp.path(), "solo", "x", "body"); + let registry = SkillRegistry::discover(tmp.path()); + let skill = registry.get("solo").unwrap(); + let body = format_skill_body(skill); + assert!( + !body.contains("## Companion files"), + "solo skills shouldn't emit an empty Companion files section" + ); + } +}