mirror of
https://github.com/block/buzz.git
synced 2026-10-29 13:31:44 +00:00
refactor(buzz-agent): use goose's skills instead of buzz's duplicate
buzz-agent grew its own skill discovery, prompt index and load_skill tool before goose had one. goose now ships goose::skills plus a SkillsClient registered in PLATFORM_EXTENSIONS, so buzz's version was a strictly smaller duplicate: the same three project directories, but goose also reads the global directories, plugin-installed skills and its own builtins, and supports args templating buzz never had. Delete builtin.rs, builtin_client.rs and hints.rs (1,204 lines) and register goose's skills extension by name so its factory runs. goose declares it unprefixed_tools, so the model now sees load_skill rather than buzz__load_skill. skills.rs keeps only the prompt index, because the goose code that renders it lives on SkillOperation, which is pub(super). The hint-file guard from hints.rs moves there rather than being dropped, and now matches filename literals instead of the bare words so it can document itself. Signed-off-by: Michael Neale <michael.neale@gmail.com> Co-authored-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -1,575 +0,0 @@
|
||||
//! Built-in tools that run in-process, bypassing MCP.
|
||||
//!
|
||||
//! Currently: `load_skill` — reads a skill's full SKILL.md body from disk
|
||||
//! and returns it so the agent can load skill content on demand rather than
|
||||
//! having every skill inlined into the system prompt at session start.
|
||||
|
||||
use serde_json::{json, Value};
|
||||
|
||||
use crate::hints::{strip_frontmatter, SkillEntry, MAX_SKILL_BODY_BYTES};
|
||||
use crate::types::truncate_at_boundary;
|
||||
use crate::types::{ToolDef, ToolResult, ToolResultContent};
|
||||
|
||||
pub const LOAD_SKILL_TOOL: &str = "load_skill";
|
||||
|
||||
/// Return the `ToolDef` for `load_skill` to include in the LLM tool list.
|
||||
pub fn load_skill_def() -> ToolDef {
|
||||
ToolDef {
|
||||
name: LOAD_SKILL_TOOL.to_owned(),
|
||||
description: "Load the full content of a skill by name. \
|
||||
Call this before using a skill — the system prompt lists skill names \
|
||||
and descriptions only; the full instructions are loaded on demand. \
|
||||
To load a supporting file within a skill, use the form \
|
||||
\"skill-name/relative/path\" (e.g. \"my-skill/references/foo.md\")."
|
||||
.to_owned(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"description": "The skill name as listed in the Available Skills section, \
|
||||
or \"skill-name/relative/path\" to load a supporting file."
|
||||
}
|
||||
},
|
||||
"required": ["name"]
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Execute a `load_skill` call. Returns a `ToolResult` on success or a
|
||||
/// user-visible error result if the skill is not found or cannot be read.
|
||||
pub async fn call_load_skill(arguments: &Value, skills: &[SkillEntry]) -> ToolResult {
|
||||
let name = match arguments.get("name").and_then(Value::as_str) {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
return error_result("load_skill: missing required argument \"name\"");
|
||||
}
|
||||
};
|
||||
|
||||
// Two forms:
|
||||
// "skill-name" → load SKILL.md body + ## Supporting Files section
|
||||
// "skill-name/rel/path" → load a specific supporting file
|
||||
if let Some((skill_name, rel_path)) = name.split_once('/') {
|
||||
return load_supporting_file(skill_name, rel_path, skills).await;
|
||||
}
|
||||
|
||||
// Plain skill-name form: load SKILL.md body.
|
||||
let entry = match skills.iter().find(|s| s.name == name) {
|
||||
Some(e) => e,
|
||||
None => {
|
||||
let available: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect();
|
||||
return error_result(&format!(
|
||||
"load_skill: skill {name:?} not found. Available: {available:?}"
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
// Read the file off the async executor to avoid blocking a Tokio worker.
|
||||
let skill_path = entry.path.clone();
|
||||
let raw = match tokio::task::spawn_blocking(move || std::fs::read_to_string(&skill_path))
|
||||
.await
|
||||
.unwrap_or_else(|e| Err(std::io::Error::other(e)))
|
||||
{
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
return error_result(&format!("load_skill: could not read {:?}: {e}", entry.path));
|
||||
}
|
||||
};
|
||||
|
||||
// Strip the YAML frontmatter — the agent already knows name/description
|
||||
// from the system prompt; return only the body.
|
||||
let body = strip_frontmatter(&raw);
|
||||
|
||||
let mut output = body.to_owned();
|
||||
|
||||
// Append ## Supporting Files section if this skill has any.
|
||||
if !entry.supporting_files.is_empty() {
|
||||
let skill_dir = entry.path.parent().unwrap_or(&entry.path);
|
||||
output.push_str("\n\n## Supporting Files\n\n");
|
||||
for file in &entry.supporting_files {
|
||||
if let Ok(rel) = file.strip_prefix(skill_dir) {
|
||||
let rel_str = rel.to_string_lossy().replace('\\', "/");
|
||||
output.push_str(&format!(
|
||||
"- {} (load_skill(name: \"{}/{}\"))\n",
|
||||
rel_str, entry.name, rel_str
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Apply the size cap to the full output (body + Supporting Files section)
|
||||
// so the total tool result stays within MAX_SKILL_BODY_BYTES.
|
||||
let output = if output.len() > MAX_SKILL_BODY_BYTES {
|
||||
truncate_at_boundary(&output, MAX_SKILL_BODY_BYTES).to_owned()
|
||||
} else {
|
||||
output
|
||||
};
|
||||
|
||||
ToolResult {
|
||||
provider_id: String::new(),
|
||||
content: vec![ToolResultContent::Text(output)],
|
||||
is_error: false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Load a supporting file identified by `skill_name/rel_path`.
|
||||
/// Matches against the pre-enumerated `supporting_files` list and applies a
|
||||
/// canonicalize-based traversal guard before reading.
|
||||
async fn load_supporting_file(
|
||||
skill_name: &str,
|
||||
rel_path: &str,
|
||||
skills: &[SkillEntry],
|
||||
) -> ToolResult {
|
||||
let rel_path = rel_path.replace('\\', "/");
|
||||
|
||||
let entry = match skills.iter().find(|s| s.name == skill_name) {
|
||||
Some(e) => e,
|
||||
None => {
|
||||
let available: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect();
|
||||
return error_result(&format!(
|
||||
"load_skill: skill {skill_name:?} not found. Available: {available:?}"
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
let skill_dir = match entry.path.parent() {
|
||||
Some(d) => d,
|
||||
None => {
|
||||
return error_result(&format!(
|
||||
"load_skill: could not determine skill directory for {skill_name:?}"
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
// Match rel_path against the pre-enumerated supporting_files list.
|
||||
let matched = entry.supporting_files.iter().find(|f| {
|
||||
f.strip_prefix(skill_dir)
|
||||
.map(|r| r.to_string_lossy().replace('\\', "/") == rel_path)
|
||||
.unwrap_or(false)
|
||||
});
|
||||
|
||||
let file_path = match matched {
|
||||
Some(p) => p,
|
||||
None => {
|
||||
let available: Vec<String> = entry
|
||||
.supporting_files
|
||||
.iter()
|
||||
.filter_map(|f| {
|
||||
f.strip_prefix(skill_dir)
|
||||
.ok()
|
||||
.map(|r| r.to_string_lossy().replace('\\', "/"))
|
||||
})
|
||||
.collect();
|
||||
if available.is_empty() {
|
||||
return error_result(&format!(
|
||||
"load_skill: skill {skill_name:?} has no supporting files."
|
||||
));
|
||||
}
|
||||
return error_result(&format!(
|
||||
"load_skill: file {rel_path:?} not found in skill {skill_name:?}. \
|
||||
Available: {available:?}"
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
// Traversal guard: canonicalize both paths and verify the file stays inside
|
||||
// the skill directory. Fail hard if the skill directory itself can't be
|
||||
// canonicalized — a degraded guard is worse than no guard.
|
||||
let canonical_skill_dir = match skill_dir.canonicalize() {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
return error_result(&format!(
|
||||
"load_skill: could not canonicalize skill directory for {skill_name:?}: {e}"
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
// Clone the path so we can move it into spawn_blocking.
|
||||
let file_path = file_path.clone();
|
||||
let skill_name = skill_name.to_owned();
|
||||
let rel_path_owned = rel_path.clone();
|
||||
|
||||
match tokio::task::spawn_blocking(move || file_path.canonicalize().map(|c| (c, file_path)))
|
||||
.await
|
||||
.unwrap_or_else(|e| Err(std::io::Error::other(e)))
|
||||
{
|
||||
Ok((canonical_file, resolved_path)) if canonical_file.starts_with(&canonical_skill_dir) => {
|
||||
match tokio::task::spawn_blocking(move || std::fs::read_to_string(&resolved_path))
|
||||
.await
|
||||
.unwrap_or_else(|e| Err(std::io::Error::other(e)))
|
||||
{
|
||||
Ok(content) => {
|
||||
let output = format!(
|
||||
"# Loaded: {}/{}\n\n{}\n\n---\nFile loaded into context.",
|
||||
skill_name, rel_path_owned, content
|
||||
);
|
||||
let output = if output.len() > MAX_SKILL_BODY_BYTES {
|
||||
truncate_at_boundary(&output, MAX_SKILL_BODY_BYTES).to_owned()
|
||||
} else {
|
||||
output
|
||||
};
|
||||
ToolResult {
|
||||
provider_id: String::new(),
|
||||
content: vec![ToolResultContent::Text(output)],
|
||||
is_error: false,
|
||||
}
|
||||
}
|
||||
Err(e) => error_result(&format!(
|
||||
"load_skill: could not read {skill_name:?}/{rel_path_owned}: {e}"
|
||||
)),
|
||||
}
|
||||
}
|
||||
Ok(_) => error_result(&format!(
|
||||
"load_skill: refusing to load {skill_name:?}/{rel_path_owned}: \
|
||||
resolves outside the skill directory"
|
||||
)),
|
||||
Err(e) => error_result(&format!(
|
||||
"load_skill: could not resolve {skill_name:?}/{rel_path_owned}: {e}"
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn error_result(msg: &str) -> ToolResult {
|
||||
ToolResult {
|
||||
provider_id: String::new(),
|
||||
content: vec![ToolResultContent::Text(msg.to_owned())],
|
||||
is_error: true,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn text_content(result: &ToolResult) -> String {
|
||||
match &result.content[0] {
|
||||
ToolResultContent::Text(t) => t.clone(),
|
||||
ToolResultContent::Image { .. } => panic!("unexpected Image content in test"),
|
||||
}
|
||||
}
|
||||
|
||||
fn make_skill(name: &str, description: &str, path: PathBuf) -> SkillEntry {
|
||||
SkillEntry {
|
||||
name: name.to_owned(),
|
||||
description: description.to_owned(),
|
||||
path,
|
||||
supporting_files: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn make_skill_with_files(
|
||||
name: &str,
|
||||
description: &str,
|
||||
path: PathBuf,
|
||||
supporting_files: Vec<PathBuf>,
|
||||
) -> SkillEntry {
|
||||
SkillEntry {
|
||||
name: name.to_owned(),
|
||||
description: description.to_owned(),
|
||||
path,
|
||||
supporting_files,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_missing_name_arg() {
|
||||
let result = call_load_skill(&serde_json::json!({}), &[]).await;
|
||||
assert!(result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(text.contains("missing required argument"), "got: {text}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_skill_not_found() {
|
||||
let result = call_load_skill(&serde_json::json!({"name": "no-such"}), &[]).await;
|
||||
assert!(result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(text.contains("not found"), "got: {text}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_returns_body_strips_frontmatter() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_md = tmp.path().join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: test\ndescription: A test\n---\nSkill body here.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let skills = vec![make_skill("test", "A test", skill_md)];
|
||||
let result = call_load_skill(&serde_json::json!({"name": "test"}), &skills).await;
|
||||
assert!(!result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(text.contains("Skill body here."), "got: {text}");
|
||||
assert!(
|
||||
!text.contains("---"),
|
||||
"frontmatter should be stripped: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_appends_supporting_files_section() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
let skill_md = skill_dir.join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: my-skill\ndescription: desc\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let refs_dir = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs_dir).unwrap();
|
||||
let ref_file = refs_dir.join("foo.md");
|
||||
std::fs::write(&ref_file, "Reference content.").unwrap();
|
||||
|
||||
let skills = vec![make_skill_with_files(
|
||||
"my-skill",
|
||||
"desc",
|
||||
skill_md,
|
||||
vec![ref_file],
|
||||
)];
|
||||
let result = call_load_skill(&serde_json::json!({"name": "my-skill"}), &skills).await;
|
||||
assert!(!result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(text.contains("Body."), "body missing: {text}");
|
||||
assert!(
|
||||
text.contains("## Supporting Files"),
|
||||
"missing Supporting Files section: {text}"
|
||||
);
|
||||
assert!(
|
||||
text.contains("references/foo.md"),
|
||||
"missing file listing: {text}"
|
||||
);
|
||||
assert!(
|
||||
text.contains("load_skill(name: \"my-skill/references/foo.md\")"),
|
||||
"missing load_skill hint: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_no_supporting_files_section_when_empty() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_md = tmp.path().join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: bare\ndescription: desc\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let skills = vec![make_skill("bare", "desc", skill_md)];
|
||||
let result = call_load_skill(&serde_json::json!({"name": "bare"}), &skills).await;
|
||||
assert!(!result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(
|
||||
!text.contains("## Supporting Files"),
|
||||
"should not have Supporting Files section when none: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_supporting_file_returns_content() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
let skill_md = skill_dir.join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: my-skill\ndescription: desc\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let refs_dir = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs_dir).unwrap();
|
||||
let ref_file = refs_dir.join("foo.md");
|
||||
std::fs::write(&ref_file, "Reference content here.").unwrap();
|
||||
|
||||
let skills = vec![make_skill_with_files(
|
||||
"my-skill",
|
||||
"desc",
|
||||
skill_md,
|
||||
vec![ref_file],
|
||||
)];
|
||||
let result = call_load_skill(
|
||||
&serde_json::json!({"name": "my-skill/references/foo.md"}),
|
||||
&skills,
|
||||
)
|
||||
.await;
|
||||
assert!(!result.is_error, "expected success, got error");
|
||||
let text = text_content(&result);
|
||||
assert!(
|
||||
text.contains("Reference content here."),
|
||||
"file content missing: {text}"
|
||||
);
|
||||
assert!(
|
||||
text.contains("# Loaded: my-skill/references/foo.md"),
|
||||
"missing header: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_supporting_file_not_found_lists_available() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
let skill_md = skill_dir.join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: my-skill\ndescription: desc\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let refs_dir = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs_dir).unwrap();
|
||||
let ref_file = refs_dir.join("foo.md");
|
||||
std::fs::write(&ref_file, "content").unwrap();
|
||||
|
||||
let skills = vec![make_skill_with_files(
|
||||
"my-skill",
|
||||
"desc",
|
||||
skill_md,
|
||||
vec![ref_file],
|
||||
)];
|
||||
let result = call_load_skill(
|
||||
&serde_json::json!({"name": "my-skill/references/missing.md"}),
|
||||
&skills,
|
||||
)
|
||||
.await;
|
||||
assert!(result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(text.contains("not found"), "got: {text}");
|
||||
assert!(
|
||||
text.contains("references/foo.md"),
|
||||
"should list available: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_no_supporting_files_error_message() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_md = tmp.path().join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: bare\ndescription: desc\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let skills = vec![make_skill("bare", "desc", skill_md)];
|
||||
let result =
|
||||
call_load_skill(&serde_json::json!({"name": "bare/anything.md"}), &skills).await;
|
||||
assert!(result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(text.contains("no supporting files"), "got: {text}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_traversal_guard_rejects_escape() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path().join("my-skill");
|
||||
std::fs::create_dir_all(&skill_dir).unwrap();
|
||||
let skill_md = skill_dir.join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
"---\nname: my-skill\ndescription: desc\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Create a file outside the skill dir that we'll try to reference.
|
||||
let outside_file = tmp.path().join("secret.txt");
|
||||
std::fs::write(&outside_file, "secret content").unwrap();
|
||||
|
||||
// Manually construct a SkillEntry with a supporting_files entry that
|
||||
// points outside the skill dir — simulating a crafted/malicious entry.
|
||||
// The traversal guard should catch this.
|
||||
let skills = vec![make_skill_with_files(
|
||||
"my-skill",
|
||||
"desc",
|
||||
skill_md.clone(),
|
||||
vec![outside_file.clone()],
|
||||
)];
|
||||
|
||||
// The slash form splits "my-skill/../secret.txt" into skill_name="my-skill"
|
||||
// and rel_path="../secret.txt". strip_prefix(skill_dir) on outside_file
|
||||
// fails, so it won't match any supporting_files entry — the pre-enumeration
|
||||
// guard rejects it before the canonicalize guard even fires.
|
||||
let result = call_load_skill(
|
||||
&serde_json::json!({"name": "my-skill/../secret.txt"}),
|
||||
&skills,
|
||||
)
|
||||
.await;
|
||||
assert!(result.is_error, "traversal attempt should be rejected");
|
||||
let text = text_content(&result);
|
||||
assert!(
|
||||
!text.contains("secret content"),
|
||||
"secret content must not be returned: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_truncates_large_body() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
let skill_md = skill_dir.join("SKILL.md");
|
||||
// Build a body that exceeds MAX_SKILL_BODY_BYTES (32 KiB).
|
||||
let large_body = "x".repeat(40 * 1024);
|
||||
std::fs::write(
|
||||
&skill_md,
|
||||
format!("---\nname: big\ndescription: desc\n---\n{large_body}\n"),
|
||||
)
|
||||
.unwrap();
|
||||
// Add a supporting file so the Supporting Files section is also appended
|
||||
// before the cap is applied.
|
||||
let refs_dir = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs_dir).unwrap();
|
||||
let ref_file = refs_dir.join("extra.md");
|
||||
std::fs::write(&ref_file, "extra content").unwrap();
|
||||
|
||||
let skills = vec![make_skill_with_files(
|
||||
"big",
|
||||
"desc",
|
||||
skill_md,
|
||||
vec![ref_file],
|
||||
)];
|
||||
let result = call_load_skill(&serde_json::json!({"name": "big"}), &skills).await;
|
||||
assert!(!result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(
|
||||
text.len() <= MAX_SKILL_BODY_BYTES,
|
||||
"output length {} exceeds MAX_SKILL_BODY_BYTES {}",
|
||||
text.len(),
|
||||
MAX_SKILL_BODY_BYTES
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_load_skill_truncates_large_supporting_file() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
let skill_md = skill_dir.join("SKILL.md");
|
||||
std::fs::write(&skill_md, "---\nname: big\ndescription: desc\n---\nBody.\n").unwrap();
|
||||
|
||||
let refs_dir = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs_dir).unwrap();
|
||||
let ref_file = refs_dir.join("huge.md");
|
||||
std::fs::write(&ref_file, "x".repeat(MAX_SKILL_BODY_BYTES * 2)).unwrap();
|
||||
|
||||
let skills = vec![make_skill_with_files(
|
||||
"big",
|
||||
"desc",
|
||||
skill_md,
|
||||
vec![ref_file],
|
||||
)];
|
||||
let result = call_load_skill(
|
||||
&serde_json::json!({"name": "big/references/huge.md"}),
|
||||
&skills,
|
||||
)
|
||||
.await;
|
||||
assert!(!result.is_error);
|
||||
let text = text_content(&result);
|
||||
assert!(
|
||||
text.len() <= MAX_SKILL_BODY_BYTES,
|
||||
"output length {} exceeds MAX_SKILL_BODY_BYTES {}",
|
||||
text.len(),
|
||||
MAX_SKILL_BODY_BYTES
|
||||
);
|
||||
assert!(
|
||||
text.starts_with("# Loaded: big/references/huge.md"),
|
||||
"missing supporting-file header: {text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1,122 +0,0 @@
|
||||
//! Buzz's in-process tools, exposed to Goose as a platform extension.
|
||||
//!
|
||||
//! Goose has two ways to take a tool the embedder implements in Rust:
|
||||
//!
|
||||
//! * [`ExtensionConfig::Frontend`] — Goose *advertises* the tool but refuses to
|
||||
//! run it. It returns "Frontend tool execution required" and blocks until the
|
||||
//! embedder calls `handle_tool_result`. Dispatch is strictly sequential, has
|
||||
//! no timeout, and the result channel carries no request id, so a single
|
||||
//! missed result wedges the agent for the rest of the session and the cancel
|
||||
//! token will not free it.
|
||||
//! * [`ExtensionConfig::Platform`] + `extension_manager.add_client` with an
|
||||
//! [`McpClientTrait`] — Goose dispatches it through the ordinary tool path,
|
||||
//! so concurrency, per-request timeouts and `notifications/cancelled` all
|
||||
//! work, and there is nothing for us to correlate.
|
||||
//!
|
||||
//! This uses the second. It is the shape Maple settled on for its own
|
||||
//! in-process developer and skills tools.
|
||||
//!
|
||||
//! The only tool here is `load_skill`. The system prompt carries the skill
|
||||
//! *index* (names and descriptions, from [`crate::hints`]); bodies are pulled
|
||||
//! on demand so a large skill library costs nothing per turn.
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use goose::agents::mcp_client::McpClientTrait;
|
||||
use goose::agents::{ExtensionConfig, ToolCallContext};
|
||||
use rmcp::model::{
|
||||
CallToolResult, ContentBlock, InitializeResult, JsonObject, ListToolsResult, Tool,
|
||||
};
|
||||
use rmcp::service::ServiceError as Error;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::hints::SkillEntry;
|
||||
|
||||
/// Extension key and display name. Goose prefixes tool names with the
|
||||
/// extension name (`{extension}__{tool}`) unless the extension is registered
|
||||
/// as unprefixed, so this shows up in the model's tool list.
|
||||
pub const BUILTIN_EXTENSION_NAME: &str = "buzz";
|
||||
|
||||
/// Goose-side declaration for the extension backed by [`BuiltinClient`].
|
||||
pub fn builtin_extension_config() -> ExtensionConfig {
|
||||
ExtensionConfig::Platform {
|
||||
name: BUILTIN_EXTENSION_NAME.to_string(),
|
||||
description: "Buzz built-in tools".to_string(),
|
||||
display_name: Some("Buzz".to_string()),
|
||||
bundled: Some(true),
|
||||
available_tools: vec![crate::builtin::LOAD_SKILL_TOOL.to_string()],
|
||||
}
|
||||
}
|
||||
|
||||
/// In-process MCP client serving [`crate::builtin`]'s tools.
|
||||
pub struct BuiltinClient {
|
||||
skills: Vec<SkillEntry>,
|
||||
}
|
||||
|
||||
impl BuiltinClient {
|
||||
pub fn new(skills: Vec<SkillEntry>) -> Self {
|
||||
Self { skills }
|
||||
}
|
||||
|
||||
fn load_skill_tool() -> Tool {
|
||||
let def = crate::builtin::load_skill_def();
|
||||
let schema = match def.input_schema {
|
||||
serde_json::Value::Object(map) => Arc::new(map),
|
||||
_ => Arc::new(serde_json::Map::new()),
|
||||
};
|
||||
Tool::new(def.name, def.description, schema)
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl McpClientTrait for BuiltinClient {
|
||||
async fn list_tools(
|
||||
&self,
|
||||
_session_id: &str,
|
||||
_next_cursor: Option<String>,
|
||||
_cancel_token: CancellationToken,
|
||||
) -> Result<ListToolsResult, Error> {
|
||||
Ok(ListToolsResult {
|
||||
tools: vec![Self::load_skill_tool()],
|
||||
next_cursor: None,
|
||||
meta: None,
|
||||
..Default::default()
|
||||
})
|
||||
}
|
||||
|
||||
async fn call_tool(
|
||||
&self,
|
||||
_ctx: &ToolCallContext,
|
||||
name: &str,
|
||||
arguments: Option<JsonObject>,
|
||||
_cancel_token: CancellationToken,
|
||||
) -> Result<CallToolResult, Error> {
|
||||
if name != crate::builtin::LOAD_SKILL_TOOL {
|
||||
return Ok(CallToolResult::error(vec![ContentBlock::text(format!(
|
||||
"unknown tool: {name}"
|
||||
))]));
|
||||
}
|
||||
|
||||
let args = arguments
|
||||
.map(serde_json::Value::Object)
|
||||
.unwrap_or(serde_json::Value::Null);
|
||||
let result = crate::builtin::call_load_skill(&args, &self.skills).await;
|
||||
|
||||
let content: Vec<ContentBlock> = result
|
||||
.content
|
||||
.iter()
|
||||
.map(|c| ContentBlock::text(c.as_text_lossy()))
|
||||
.collect();
|
||||
|
||||
Ok(if result.is_error {
|
||||
CallToolResult::error(content)
|
||||
} else {
|
||||
CallToolResult::success(content)
|
||||
})
|
||||
}
|
||||
|
||||
fn get_info(&self) -> Option<&InitializeResult> {
|
||||
None
|
||||
}
|
||||
}
|
||||
@@ -1,507 +0,0 @@
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
pub const MAX_SKILL_BODY_BYTES: usize = 32 * 1024;
|
||||
const SKILL_DIRS: &[&str] = &[".agents/skills", ".goose/skills", ".claude/skills"];
|
||||
|
||||
fn home_dir() -> Option<PathBuf> {
|
||||
std::env::var("HOME").ok().map(PathBuf::from)
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct SkillEntry {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
/// Absolute path to the SKILL.md file; used by `load_skill` to read on demand.
|
||||
pub path: PathBuf,
|
||||
/// Absolute paths to every non-SKILL.md file in the skill directory tree.
|
||||
/// Pre-enumerated at discovery time so `load_skill` can match by relative path
|
||||
/// without doing arbitrary filesystem lookups at call time.
|
||||
pub supporting_files: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
/// Handles both normal repos (`.git/` dir) and worktrees (`.git` file).
|
||||
fn parse_skill_frontmatter(content: &str) -> Option<(String, String)> {
|
||||
// Must start with `---`
|
||||
let rest = content.strip_prefix("---\n")?;
|
||||
// Find the closing `---`
|
||||
let close_pos = rest.find("\n---")?;
|
||||
let yaml_block = &rest[..close_pos];
|
||||
|
||||
let map: HashMap<String, serde_yaml::Value> = serde_yaml::from_str(yaml_block).ok()?;
|
||||
let name = map
|
||||
.get("name")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(str::to_string)?;
|
||||
let description = map
|
||||
.get("description")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(str::trim)
|
||||
.unwrap_or("")
|
||||
.to_string();
|
||||
|
||||
Some((name, description))
|
||||
}
|
||||
|
||||
fn scan_skill_dir(dir: &Path, seen: &mut HashSet<String>, skills: &mut Vec<SkillEntry>) {
|
||||
let Ok(entries) = std::fs::read_dir(dir) else {
|
||||
return;
|
||||
};
|
||||
let mut subdirs: Vec<PathBuf> = entries
|
||||
.filter_map(|e| e.ok())
|
||||
// Use std::fs::metadata (follows symlinks) rather than DirEntry::file_type
|
||||
// (which returns FileType::Symlink for symlinks, causing is_dir() to return
|
||||
// false even when the symlink target is a directory).
|
||||
.filter(|e| {
|
||||
std::fs::metadata(e.path())
|
||||
.map(|m| m.is_dir())
|
||||
.unwrap_or(false)
|
||||
})
|
||||
.map(|e| e.path())
|
||||
.collect();
|
||||
subdirs.sort();
|
||||
|
||||
for subdir in subdirs {
|
||||
let skill_md = subdir.join("SKILL.md");
|
||||
let Ok(content) = std::fs::read_to_string(&skill_md) else {
|
||||
continue;
|
||||
};
|
||||
let Some((name, description)) = parse_skill_frontmatter(&content) else {
|
||||
continue;
|
||||
};
|
||||
if seen.contains(&name) {
|
||||
continue;
|
||||
}
|
||||
seen.insert(name.clone());
|
||||
|
||||
// Collect supporting files: every non-SKILL.md file in the skill dir tree.
|
||||
// Don't descend into subdirs that themselves have a SKILL.md — those are
|
||||
// separate skills with their own entries.
|
||||
let supporting_files = collect_supporting_files(&subdir);
|
||||
|
||||
skills.push(SkillEntry {
|
||||
name,
|
||||
description,
|
||||
path: skill_md,
|
||||
supporting_files,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Walk `skill_dir` recursively and return the absolute path of every file
|
||||
/// that is not `SKILL.md`. Subdirectories that contain their own `SKILL.md`
|
||||
/// are treated as separate skills and are not descended into.
|
||||
fn collect_supporting_files(skill_dir: &Path) -> Vec<PathBuf> {
|
||||
let mut result = Vec::new();
|
||||
let mut visited_dirs = HashSet::new();
|
||||
collect_supporting_files_impl(skill_dir, &mut result, &mut visited_dirs);
|
||||
result.sort();
|
||||
result
|
||||
}
|
||||
|
||||
fn collect_supporting_files_impl(
|
||||
current: &Path,
|
||||
out: &mut Vec<PathBuf>,
|
||||
visited_dirs: &mut HashSet<PathBuf>,
|
||||
) {
|
||||
let Ok(canonical_current) = current.canonicalize() else {
|
||||
return;
|
||||
};
|
||||
if !visited_dirs.insert(canonical_current) {
|
||||
return;
|
||||
}
|
||||
|
||||
let Ok(entries) = std::fs::read_dir(current) else {
|
||||
return;
|
||||
};
|
||||
let mut items: Vec<_> = entries.filter_map(|e| e.ok()).collect();
|
||||
items.sort_by_key(|e| e.path());
|
||||
|
||||
for entry in items {
|
||||
let path = entry.path();
|
||||
// Use std::fs::metadata (follows symlinks) so symlinked subdirs and files
|
||||
// inside a skill directory are handled correctly.
|
||||
let ft = match std::fs::metadata(&path) {
|
||||
Ok(m) => m,
|
||||
Err(_) => continue,
|
||||
};
|
||||
if ft.is_dir() {
|
||||
// Don't descend into subdirs that are themselves skills.
|
||||
if path.join("SKILL.md").is_file() {
|
||||
continue;
|
||||
}
|
||||
collect_supporting_files_impl(&path, out, visited_dirs);
|
||||
} else if ft.is_file() && path.file_name().and_then(|n| n.to_str()) != Some("SKILL.md") {
|
||||
out.push(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn discover_skills_impl(cwd: &Path, home: Option<&Path>) -> Vec<SkillEntry> {
|
||||
let mut seen = HashSet::new();
|
||||
let mut skills = Vec::new();
|
||||
|
||||
for dir_suffix in SKILL_DIRS {
|
||||
scan_skill_dir(&cwd.join(dir_suffix), &mut seen, &mut skills);
|
||||
}
|
||||
|
||||
if let Some(home) = home {
|
||||
scan_skill_dir(&home.join(".agents/skills"), &mut seen, &mut skills);
|
||||
}
|
||||
|
||||
skills
|
||||
}
|
||||
|
||||
/// Discover the skills available from `cwd`.
|
||||
///
|
||||
/// Hint *files* are goose's job (`PromptManager::build_system_prompt` walks
|
||||
/// them from the git root). This is only the skill index, which stays ours
|
||||
/// because `load_skill` is served by [`crate::builtin_client`] and needs the
|
||||
/// same [`SkillEntry`] list the tool dispatches against.
|
||||
pub fn discover_skills(cwd: &Path) -> Vec<SkillEntry> {
|
||||
discover_skills_impl(cwd, home_dir().as_deref())
|
||||
}
|
||||
|
||||
/// Render the skill index for the system prompt.
|
||||
///
|
||||
/// Returns an empty string for an empty slate so the caller can skip the
|
||||
/// extra entirely rather than adding an empty section.
|
||||
pub fn skill_index(skills: &[SkillEntry]) -> String {
|
||||
if skills.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
|
||||
let mut out = String::from("# Available Skills\n");
|
||||
for skill in skills {
|
||||
out.push_str(&format!("- {}: {}\n", skill.name, skill.description));
|
||||
}
|
||||
// Goose namespaces platform-extension tools as `{extension}__{tool}`
|
||||
// (`extension_manager.rs:1415`), so name the tool the way the model
|
||||
// actually sees it -- otherwise the prompt advertises a tool that does
|
||||
// not exist in its tool list.
|
||||
out.push_str(&format!(
|
||||
"\nUse the `{}__{}` tool to read the full content of a skill before using it.\n",
|
||||
crate::builtin_client::BUILTIN_EXTENSION_NAME,
|
||||
crate::builtin::LOAD_SKILL_TOOL,
|
||||
));
|
||||
out
|
||||
}
|
||||
|
||||
/// Strip the YAML frontmatter block from a skill file's content and return
|
||||
/// the body. If no valid frontmatter is found, returns the content unchanged.
|
||||
pub(crate) fn strip_frontmatter(content: &str) -> &str {
|
||||
let Some(rest) = content.strip_prefix("---\n") else {
|
||||
return content;
|
||||
};
|
||||
let Some(close_pos) = rest.find("\n---") else {
|
||||
return content;
|
||||
};
|
||||
let after = &rest[close_pos + 4..]; // skip "\n---"
|
||||
after.strip_prefix('\n').unwrap_or(after)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn discover_skills_finds_across_dirs() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path();
|
||||
|
||||
// Skill in .agents/skills/
|
||||
let agents_skill = cwd.join(".agents/skills/my-skill");
|
||||
std::fs::create_dir_all(&agents_skill).unwrap();
|
||||
std::fs::write(
|
||||
agents_skill.join("SKILL.md"),
|
||||
"---\nname: my-skill\ndescription: A skill\n---\nSkill body here.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Skill in .goose/skills/
|
||||
let goose_skill = cwd.join(".goose/skills/other-skill");
|
||||
std::fs::create_dir_all(&goose_skill).unwrap();
|
||||
std::fs::write(
|
||||
goose_skill.join("SKILL.md"),
|
||||
"---\nname: other-skill\ndescription: Another skill\n---\nOther body.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let skills = discover_skills_impl(cwd, None);
|
||||
assert_eq!(skills.len(), 2);
|
||||
let names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect();
|
||||
assert!(names.contains(&"my-skill"), "missing my-skill");
|
||||
assert!(names.contains(&"other-skill"), "missing other-skill");
|
||||
// Paths should point to the SKILL.md files.
|
||||
for skill in &skills {
|
||||
assert!(skill.path.exists(), "path does not exist: {:?}", skill.path);
|
||||
assert!(skill.path.ends_with("SKILL.md"));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discover_skills_dedup_by_name() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path();
|
||||
|
||||
// Same name in .agents/skills/ (first) and .goose/skills/ (second)
|
||||
let agents_skill = cwd.join(".agents/skills/shared");
|
||||
std::fs::create_dir_all(&agents_skill).unwrap();
|
||||
std::fs::write(
|
||||
agents_skill.join("SKILL.md"),
|
||||
"---\nname: shared\ndescription: from agents\n---\nAgents body.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let goose_skill = cwd.join(".goose/skills/shared");
|
||||
std::fs::create_dir_all(&goose_skill).unwrap();
|
||||
std::fs::write(
|
||||
goose_skill.join("SKILL.md"),
|
||||
"---\nname: shared\ndescription: from goose\n---\nGoose body.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let skills = discover_skills_impl(cwd, None);
|
||||
assert_eq!(skills.len(), 1, "duplicate name should be deduplicated");
|
||||
assert_eq!(
|
||||
skills[0].description, "from agents",
|
||||
"first wins (.agents/)"
|
||||
);
|
||||
// Path should point to the .agents/ version (first wins).
|
||||
assert!(skills[0]
|
||||
.path
|
||||
.to_str()
|
||||
.unwrap()
|
||||
.contains(".agents/skills/shared"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discover_skills_skips_missing_name() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path();
|
||||
|
||||
let skill_dir = cwd.join(".agents/skills/no-name");
|
||||
std::fs::create_dir_all(&skill_dir).unwrap();
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\ndescription: No name here\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let skills = discover_skills_impl(cwd, None);
|
||||
assert!(skills.is_empty(), "entry without name should be skipped");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discover_skills_global_skills_loaded() {
|
||||
let home = TempDir::new().unwrap();
|
||||
let cwd = TempDir::new().unwrap();
|
||||
let skill_dir = home.path().join(".agents/skills/global-skill");
|
||||
std::fs::create_dir_all(&skill_dir).unwrap();
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: global-skill\ndescription: A global skill\n---\nGlobal body.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let skills = discover_skills_impl(cwd.path(), Some(home.path()));
|
||||
assert_eq!(skills.len(), 1);
|
||||
assert_eq!(skills[0].name, "global-skill");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discover_skills_project_wins_over_global() {
|
||||
let home = TempDir::new().unwrap();
|
||||
let cwd = TempDir::new().unwrap();
|
||||
|
||||
let project_skill = cwd.path().join(".agents/skills/shared");
|
||||
std::fs::create_dir_all(&project_skill).unwrap();
|
||||
std::fs::write(
|
||||
project_skill.join("SKILL.md"),
|
||||
"---\nname: shared\ndescription: from project\n---\nProject body.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let global_skill = home.path().join(".agents/skills/shared");
|
||||
std::fs::create_dir_all(&global_skill).unwrap();
|
||||
std::fs::write(
|
||||
global_skill.join("SKILL.md"),
|
||||
"---\nname: shared\ndescription: from global\n---\nGlobal body.\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let skills = discover_skills_impl(cwd.path(), Some(home.path()));
|
||||
assert_eq!(skills.len(), 1, "duplicate name should be deduplicated");
|
||||
assert_eq!(
|
||||
skills[0].description, "from project",
|
||||
"project-level should win over global"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discover_skills_no_home_dir() {
|
||||
let cwd = TempDir::new().unwrap();
|
||||
let skill_dir = cwd.path().join(".agents/skills/local");
|
||||
std::fs::create_dir_all(&skill_dir).unwrap();
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: local\ndescription: Local skill\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let skills = discover_skills_impl(cwd.path(), None);
|
||||
assert_eq!(skills.len(), 1);
|
||||
assert_eq!(skills[0].name, "local");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_supporting_files_finds_non_skill_md_files() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
// SKILL.md should be excluded.
|
||||
std::fs::write(skill_dir.join("SKILL.md"), "---\nname: x\n---\n").unwrap();
|
||||
// A references subdir with files.
|
||||
let refs = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs).unwrap();
|
||||
std::fs::write(refs.join("foo.md"), "foo").unwrap();
|
||||
std::fs::write(refs.join("bar.md"), "bar").unwrap();
|
||||
// A script at the top level.
|
||||
std::fs::write(skill_dir.join("setup.sh"), "#!/bin/sh").unwrap();
|
||||
|
||||
let files = collect_supporting_files(skill_dir);
|
||||
let names: Vec<String> = files
|
||||
.iter()
|
||||
.map(|p| p.file_name().unwrap().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
assert!(
|
||||
names.contains(&"foo.md".to_owned()),
|
||||
"missing foo.md: {names:?}"
|
||||
);
|
||||
assert!(
|
||||
names.contains(&"bar.md".to_owned()),
|
||||
"missing bar.md: {names:?}"
|
||||
);
|
||||
assert!(
|
||||
names.contains(&"setup.sh".to_owned()),
|
||||
"missing setup.sh: {names:?}"
|
||||
);
|
||||
assert!(
|
||||
!names.contains(&"SKILL.md".to_owned()),
|
||||
"SKILL.md should be excluded: {names:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_supporting_files_does_not_descend_into_nested_skills() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
std::fs::write(skill_dir.join("SKILL.md"), "---\nname: x\n---\n").unwrap();
|
||||
std::fs::write(skill_dir.join("helper.sh"), "#!/bin/sh").unwrap();
|
||||
|
||||
// A nested subdir that is itself a skill — should not be descended into.
|
||||
let nested = skill_dir.join("nested-skill");
|
||||
std::fs::create_dir_all(&nested).unwrap();
|
||||
std::fs::write(nested.join("SKILL.md"), "---\nname: nested\n---\n").unwrap();
|
||||
std::fs::write(nested.join("secret.md"), "should not appear").unwrap();
|
||||
|
||||
let files = collect_supporting_files(skill_dir);
|
||||
let names: Vec<String> = files
|
||||
.iter()
|
||||
.map(|p| p.file_name().unwrap().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
assert!(
|
||||
names.contains(&"helper.sh".to_owned()),
|
||||
"missing helper.sh: {names:?}"
|
||||
);
|
||||
assert!(
|
||||
!names.contains(&"secret.md".to_owned()),
|
||||
"nested skill's files should not appear: {names:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn collect_supporting_files_skips_symlink_cycles() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let skill_dir = tmp.path();
|
||||
std::fs::write(skill_dir.join("SKILL.md"), "---\nname: x\n---\n").unwrap();
|
||||
|
||||
let refs = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs).unwrap();
|
||||
let guide = refs.join("guide.md");
|
||||
std::fs::write(&guide, "guide").unwrap();
|
||||
std::os::unix::fs::symlink(&refs, refs.join("loop")).unwrap();
|
||||
|
||||
let files = collect_supporting_files(skill_dir);
|
||||
assert_eq!(files, vec![guide]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discover_skills_populates_supporting_files() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path();
|
||||
let skill_dir = cwd.join(".agents/skills/with-refs");
|
||||
std::fs::create_dir_all(&skill_dir).unwrap();
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: with-refs\ndescription: Has refs\n---\nBody.\n",
|
||||
)
|
||||
.unwrap();
|
||||
let refs = skill_dir.join("references");
|
||||
std::fs::create_dir_all(&refs).unwrap();
|
||||
std::fs::write(refs.join("guide.md"), "guide content").unwrap();
|
||||
|
||||
let skills = discover_skills_impl(cwd, None);
|
||||
assert_eq!(skills.len(), 1);
|
||||
assert_eq!(skills[0].name, "with-refs");
|
||||
assert_eq!(skills[0].supporting_files.len(), 1);
|
||||
assert!(
|
||||
skills[0].supporting_files[0].ends_with("references/guide.md"),
|
||||
"unexpected path: {:?}",
|
||||
skills[0].supporting_files[0]
|
||||
);
|
||||
}
|
||||
|
||||
/// The bug this module's shrink was for: buzz-agent used to add its own
|
||||
/// hint-file scan as a prompt extra, while goose's `build_system_prompt`
|
||||
/// was already loading the same files. Every hint landed in the system
|
||||
/// prompt twice. Nothing here may reintroduce hint *file* loading -- that
|
||||
/// is goose's job now.
|
||||
///
|
||||
/// The banned names are assembled at runtime so this guard does not trip
|
||||
/// over its own source text.
|
||||
#[test]
|
||||
fn this_module_does_not_load_hint_files() {
|
||||
let source = include_str!("hints.rs");
|
||||
for banned in [format!("AGENTS{}md", "."), format!(".goose{}", "hints")] {
|
||||
assert!(
|
||||
!source.contains(&banned),
|
||||
"hints.rs references {banned}: hint-file loading belongs to \
|
||||
goose's PromptManager, or hints render twice in the prompt"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_skill_index_is_empty_for_an_empty_slate() {
|
||||
// An empty section would still be added as a prompt extra, spending
|
||||
// tokens on a header with nothing under it.
|
||||
assert!(skill_index(&[]).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_skill_index_names_the_tool_as_the_model_sees_it() {
|
||||
let skills = vec![SkillEntry {
|
||||
name: "widget".to_string(),
|
||||
description: "makes widgets".to_string(),
|
||||
path: PathBuf::from("/tmp/widget/SKILL.md"),
|
||||
supporting_files: Vec::new(),
|
||||
}];
|
||||
let index = skill_index(&skills);
|
||||
assert!(index.contains("- widget: makes widgets"));
|
||||
// Namespaced, because goose advertises platform tools as
|
||||
// `{extension}__{tool}`; the bare name is not in the model's tool list.
|
||||
assert!(index.contains("buzz__load_skill"));
|
||||
}
|
||||
}
|
||||
@@ -21,16 +21,14 @@
|
||||
//! * `keepalive` — see `agent.rs`.
|
||||
|
||||
pub mod agent;
|
||||
pub mod builtin;
|
||||
pub mod builtin_client;
|
||||
pub mod config;
|
||||
pub mod hints;
|
||||
pub mod hooks;
|
||||
pub mod loop_drive;
|
||||
pub mod model;
|
||||
pub mod ops;
|
||||
pub mod prompt;
|
||||
pub mod session_store;
|
||||
pub mod skills;
|
||||
pub mod steer;
|
||||
pub mod tools;
|
||||
pub mod turn_state;
|
||||
@@ -251,31 +249,39 @@ async fn build_agent(
|
||||
// goose's loader is also a superset: `.goosehints` as well as `AGENTS.md`,
|
||||
// the filename list configurable via `CONTEXT_FILE_NAMES`, gitignore-aware
|
||||
// filtering, and `@file` reference expansion bounded at the git root.
|
||||
let skills = crate::hints::discover_skills(std::path::Path::new(cwd));
|
||||
// Skills are goose's too, for the same reason as hints.
|
||||
//
|
||||
// buzz-agent grew its own discovery, index and `load_skill` tool
|
||||
// (`hints.rs` + `builtin.rs` + `builtin_client.rs`, ~1.2k lines) before
|
||||
// goose had one. goose has since shipped `goose::skills` and registers a
|
||||
// `SkillsClient` as a platform extension, so ours was a strictly smaller
|
||||
// duplicate: same three project directories, but goose also reads the
|
||||
// global ones, plugin-installed skills and its own builtins, and supports
|
||||
// `args` templating we never had.
|
||||
//
|
||||
// Registered by *name* rather than by handing over a client: `skills` is
|
||||
// in goose's `PLATFORM_EXTENSIONS` table, so `add_extension` runs its
|
||||
// factory. It is declared `unprefixed_tools: true` there, which is why the
|
||||
// tool the model sees is plain `load_skill` and not `skills__load_skill`.
|
||||
let skills = goose::skills::discover_skills(Some(std::path::Path::new(cwd)));
|
||||
if !skills.is_empty() {
|
||||
prompt
|
||||
.add_extra("buzz_skills", crate::hints::skill_index(&skills))
|
||||
.await;
|
||||
}
|
||||
|
||||
// `load_skill` runs in-process, registered the way Maple does it: a
|
||||
// platform extension backed by an `McpClientTrait`, NOT
|
||||
// `ExtensionConfig::Frontend`. Goose advertises frontend tools but refuses
|
||||
// to dispatch them -- it blocks on an uncorrelated result channel until the
|
||||
// embedder calls `handle_tool_result`, with no timeout and no cancellation,
|
||||
// so one missed result wedges the session. A platform client goes through
|
||||
// goose's ordinary tool path instead. See `crate::builtin_client`.
|
||||
if !skills.is_empty() {
|
||||
agent
|
||||
.extension_manager
|
||||
.add_client(
|
||||
crate::builtin_client::BUILTIN_EXTENSION_NAME.to_string(),
|
||||
crate::builtin_client::builtin_extension_config(),
|
||||
std::sync::Arc::new(crate::builtin_client::BuiltinClient::new(skills)),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
let ext = ExtensionConfig::Platform {
|
||||
name: goose::skills::EXTENSION_NAME.to_string(),
|
||||
description: "Skills".to_string(),
|
||||
display_name: Some("Skills".to_string()),
|
||||
bundled: Some(true),
|
||||
available_tools: Vec::new(),
|
||||
};
|
||||
if let Err(e) = agent.add_extension(ext, &session_id).await {
|
||||
// Unlike an MCP server, a missing skills extension is not fatal:
|
||||
// the agent still has every other tool. Losing it silently would
|
||||
// be worse than losing it loudly, hence the warning.
|
||||
tracing::warn!(error = %e, "skills extension unavailable");
|
||||
} else {
|
||||
prompt
|
||||
.add_extra("skills", crate::skills::skill_index(&skills))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
let agent = Arc::new(agent);
|
||||
|
||||
157
crates/buzz-agent/src/skills.rs
Normal file
157
crates/buzz-agent/src/skills.rs
Normal file
@@ -0,0 +1,157 @@
|
||||
//! The skill index for the system prompt.
|
||||
//!
|
||||
//! Discovery, `load_skill`, argument templating, supporting-file loading and
|
||||
//! the path-escape guard are all [`goose::skills`]. The only thing left here
|
||||
//! is rendering the index, and only because the piece of goose that renders it
|
||||
//! (`ops_skills::skill_instructions`) is reachable through `SkillOperation`,
|
||||
//! which is `pub(super)` — its concrete operations stay internal to goose
|
||||
//! while the `Operation` trait is public.
|
||||
//!
|
||||
//! So buzz gets goose's skills the way an embedder can: the `skills` platform
|
||||
//! extension supplies the tool, and this supplies the index that tells the
|
||||
//! model the tool is worth calling. The wording matches goose's own so the two
|
||||
//! do not drift into describing the same tool differently.
|
||||
|
||||
use goose::custom_requests::{SourceEntry, SourceType};
|
||||
|
||||
/// Render the index of available skills for the system prompt.
|
||||
///
|
||||
/// Empty for an empty slate, so the caller can skip the prompt extra entirely
|
||||
/// rather than spending tokens on a header with nothing under it.
|
||||
pub fn skill_index(skills: &[SourceEntry]) -> String {
|
||||
let mut entries: Vec<&SourceEntry> = skills
|
||||
.iter()
|
||||
.filter(|skill| {
|
||||
matches!(
|
||||
skill.source_type,
|
||||
SourceType::Skill | SourceType::BuiltinSkill
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
if entries.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
// Sorted on the same key as goose's own renderer, so the section is stable
|
||||
// between rounds. An index that reorders itself would invalidate the
|
||||
// provider's prompt cache every turn for no benefit.
|
||||
entries.sort_by(|a, b| (&a.name, &a.path).cmp(&(&b.name, &b.path)));
|
||||
|
||||
let mut out = String::from(
|
||||
"# Skills\n\nYou have these skills at your disposal. Load one when it can help \
|
||||
with the task or when the user asks for it:",
|
||||
);
|
||||
for skill in entries {
|
||||
out.push_str(&format!("\n- {}: {}", skill.name, skill.description));
|
||||
}
|
||||
// Unprefixed on purpose. goose registers `skills` with
|
||||
// `unprefixed_tools: true` (`platform_extensions/mod.rs:217`), so the
|
||||
// model's tool list carries `load_skill`, not `skills__load_skill`.
|
||||
// Naming it the other way would advertise a tool that is not there.
|
||||
out.push_str("\n\nLoad a skill with the `load_skill` tool before using it.\n");
|
||||
out
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn skill(name: &str, source_type: SourceType) -> SourceEntry {
|
||||
SourceEntry {
|
||||
source_type,
|
||||
name: name.to_string(),
|
||||
description: format!("does {name}"),
|
||||
content: String::new(),
|
||||
path: format!("/tmp/{name}"),
|
||||
global: false,
|
||||
writable: false,
|
||||
supporting_files: Vec::new(),
|
||||
properties: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_empty_slate_renders_nothing() {
|
||||
assert!(skill_index(&[]).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn builtin_and_filesystem_skills_both_appear() {
|
||||
let index = skill_index(&[
|
||||
skill("widget", SourceType::Skill),
|
||||
skill("doc-guide", SourceType::BuiltinSkill),
|
||||
]);
|
||||
assert!(index.contains("- widget: does widget"));
|
||||
assert!(index.contains("- doc-guide: does doc-guide"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_skill_sources_are_left_out() {
|
||||
// `discover_skills` returns `SourceEntry`, which also models projects.
|
||||
// Listing a project as a skill would advertise a `load_skill` call
|
||||
// that cannot succeed.
|
||||
let index = skill_index(&[skill("some-project", SourceType::Project)]);
|
||||
assert!(index.is_empty(), "unexpected index: {index:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_tool_is_named_the_way_the_model_sees_it() {
|
||||
// goose exposes the skills extension unprefixed; `skills__load_skill`
|
||||
// is not in the tool list and would be a dead reference.
|
||||
let index = skill_index(&[skill("widget", SourceType::Skill)]);
|
||||
assert!(index.contains("`load_skill`"));
|
||||
assert!(!index.contains("skills__load_skill"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_order_is_stable() {
|
||||
let unsorted = [
|
||||
skill("zebra", SourceType::Skill),
|
||||
skill("alpha", SourceType::Skill),
|
||||
];
|
||||
let index = skill_index(&unsorted);
|
||||
let alpha = index.find("alpha").expect("alpha listed");
|
||||
let zebra = index.find("zebra").expect("zebra listed");
|
||||
assert!(alpha < zebra, "index must not reorder between rounds");
|
||||
}
|
||||
}
|
||||
|
||||
/// Nothing in this crate may *load* hint files.
|
||||
///
|
||||
/// buzz-agent used to walk `AGENTS.md` itself while goose's
|
||||
/// `PromptManager::build_system_prompt` was already loading the same files, so
|
||||
/// every hint landed in the system prompt twice. This guard outlived the
|
||||
/// module it was written in (`hints.rs`, deleted along with the duplicate
|
||||
/// skills code it shared a file with), so it moves here rather than being
|
||||
/// dropped.
|
||||
///
|
||||
/// It looks for the *filename literals* a loader would need, not for the
|
||||
/// names in prose -- the previous version banned the bare word and could only
|
||||
/// live in a file that never explained itself. The literals are assembled at
|
||||
/// runtime so the guard does not trip over its own source text.
|
||||
#[cfg(test)]
|
||||
mod hint_files_are_gooses_job {
|
||||
const SOURCES: &[(&str, &str)] = &[
|
||||
("lib.rs", include_str!("lib.rs")),
|
||||
("prompt.rs", include_str!("prompt.rs")),
|
||||
("loop_drive.rs", include_str!("loop_drive.rs")),
|
||||
("skills.rs", include_str!("skills.rs")),
|
||||
];
|
||||
|
||||
#[test]
|
||||
fn no_module_loads_hint_files() {
|
||||
let quote = '"';
|
||||
// Assembled rather than written out, so this test's own source does
|
||||
// not count as a violation of the rule it enforces.
|
||||
for name in [format!("AGENTS{}md", '.'), format!(".goose{}", "hints")] {
|
||||
let literal = format!("{quote}{name}{quote}");
|
||||
for (file, source) in SOURCES {
|
||||
assert!(
|
||||
!source.contains(&literal),
|
||||
"{file} has a {literal} string literal: hint-file loading \
|
||||
belongs to goose's PromptManager, or hints render twice \
|
||||
in the prompt"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,10 +1,11 @@
|
||||
//! Proves `load_skill` and `AGENTS.md` hints survive the goose swap.
|
||||
//!
|
||||
//! Both were briefly lost in the port. `load_skill` is registered as a goose
|
||||
//! *platform* extension backed by an `McpClientTrait` (the shape Maple uses),
|
||||
//! so goose dispatches it on its ordinary tool path. Goose namespaces such
|
||||
//! tools as `{extension}__{tool}`, which the system prompt has to match — a
|
||||
//! prompt naming a tool that is not in the tool list is a silent failure.
|
||||
//! Both were briefly lost in the port. Skills are now goose's `skills`
|
||||
//! platform extension rather than a buzz reimplementation, and goose declares
|
||||
//! it `unprefixed_tools: true`, so the model sees plain `load_skill` — not
|
||||
//! `skills__load_skill`. buzz only renders the index, and an index naming a
|
||||
//! tool that is not in the tool list is a silent failure, so this pins both
|
||||
//! halves against each other.
|
||||
|
||||
use std::io::{BufRead, BufReader, Write};
|
||||
use std::net::TcpListener;
|
||||
@@ -100,7 +101,7 @@ fn spawn_provider() -> (String, mpsc::Receiver<(String, Vec<String>)>) {
|
||||
"id":"c","object":"chat.completion.chunk","created":1,"model":"fake-model",
|
||||
"choices":[{"index":0,"delta":{"role":"assistant","tool_calls":[{
|
||||
"index":0,"id":"call_1","type":"function",
|
||||
"function":{"name":"buzz__load_skill","arguments":"{\"name\":\"widget-maker\"}"}
|
||||
"function":{"name":"load_skill","arguments":"{\"name\":\"widget-maker\"}"}
|
||||
}]},"finish_reason":null}]
|
||||
});
|
||||
let d = json!({
|
||||
@@ -198,7 +199,8 @@ fn workspace() -> tempfile::TempDir {
|
||||
let dir = tempfile::tempdir().expect("tmp");
|
||||
let root = dir.path();
|
||||
std::fs::write(root.join("AGENTS.md"), "Always ship the sprocket first.").unwrap();
|
||||
// hints.rs walks up to the git root; give it one so the walk terminates here.
|
||||
// goose's hint loader walks up to the git root; give it one so the walk
|
||||
// terminates inside the temp workspace rather than in the real repo.
|
||||
std::fs::create_dir_all(root.join(".git")).unwrap();
|
||||
let skill = root.join(".agents/skills/widget-maker");
|
||||
std::fs::create_dir_all(&skill).unwrap();
|
||||
@@ -264,7 +266,7 @@ fn agents_md_and_skill_index_reach_the_model_and_load_skill_resolves() {
|
||||
"skill BODY was inlined — the point of load_skill is that it is not:\n{first}"
|
||||
);
|
||||
assert!(
|
||||
tool_lists[0].iter().any(|t| t == "buzz__load_skill"),
|
||||
tool_lists[0].iter().any(|t| t == "load_skill"),
|
||||
"load_skill not advertised: {:?}",
|
||||
tool_lists[0]
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user