feat(tools): load_skill model-callable tool (#434)
Adds a `load_skill` tool that takes a skill id and returns the
SKILL.md body plus the sibling companion-file list in one tool
call. The existing progressive-disclosure pattern (system prompt
lists skills → model `read_file <path>`) still works; this tool
is the higher-level affordance for skills that ship with multiple
resource files.
Implementation:
* `LoadSkillTool` lives in `crates/tui/src/tools/skill.rs`. Read-
only, auto-approved, parallel-safe.
* On call, resolves the active skills directory via the new
`skills::resolve_skills_dir` helper, which mirrors
`App::new`'s hierarchy: `<workspace>/.agents/skills` →
`<workspace>/skills` → `~/.deepseek/skills`. No new plumbing
through ToolContext — the workspace is already there.
* Returns the skill body wrapped in a self-contained block:
description quote, source path, the SKILL.md verbatim, and a
`## Companion files` section listing siblings (sorted lex,
deterministic for tests). Solo skills skip the companions
section entirely so the tool result stays tight.
* Errors with a helpful hint when the name is unknown — the
hint includes the catalogue ("Available: foo, bar, baz") so
the model can recover without an extra discovery call.
* Wired into `ToolRegistryBuilder::with_skill_tools` and pulled
into both Agent and Plan tool-setup paths. Plan mode benefits
because skills are read-only references that planners often
need.
Tests:
5 unit tests covering: description-headed body, companion
enumeration excluding SKILL.md and nested dirs, empty result
for solo skills, and the conditional `## Companion files`
section.
This commit is contained in:
Hunter Bown
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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: `<workspace>/.agents/skills` →
|
||||
/// `<workspace>/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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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 <path>` from the
|
||||
//! listing.
|
||||
//! 2. (this tool) `load_skill name=<id>` — 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<ToolCapability> {
|
||||
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<ToolResult, ToolError> {
|
||||
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 `<dir>/<skill>/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::<Vec<String>>(),
|
||||
})))
|
||||
}
|
||||
}
|
||||
|
||||
/// 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<std::path::PathBuf> {
|
||||
let Some(dir) = skill.path.parent() else {
|
||||
return Vec::new();
|
||||
};
|
||||
let mut entries: Vec<std::path::PathBuf> = 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<String> = 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"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user