diff --git a/crates/buzz-agent/src/agent.rs b/crates/buzz-agent/src/agent.rs index be0fdc0733..ca618a7421 100644 --- a/crates/buzz-agent/src/agent.rs +++ b/crates/buzz-agent/src/agent.rs @@ -8,23 +8,26 @@ //! | `mcp.rs` | 1139 | `goose::agents::extension_manager` | //! | `auth.rs` | 845 | `goose::providers` (incl. Databricks OAuth) | //! | `agent.rs` | 746 | `goose::agents::Agent::reply` | -//! | `hints.rs` | 726 | *partly* -- see the caveat below | +//! | `hints.rs` | 726 | kept -- goose's loader can't replace it (below) | //! | `catalog.rs` | 631 | `goose::providers::init` model discovery | -//! | `builtin.rs` | 575 | **dropped** -- see the caveat below | +//! | `builtin.rs` | 575 | kept -- served as a goose *frontend* tool (below) | //! | `handoff.rs` | 430 | `goose::context_mgmt` (auto-compaction) | //! -//! Two entries above are NOT clean substitutions, and saying otherwise would -//! be worse than the loss itself: +//! Two entries above are NOT goose substitutions; the old buzz-agent modules +//! are kept and wired into goose instead, because goose's own equivalents +//! don't cover them: //! -//! * **`builtin.rs` / `load_skill` is gone.** `Agent::with_config` loads zero +//! * **`builtin.rs` / `load_skill`.** `Agent::with_config` loads zero //! extensions and `build_agent` only adds the `mcpServers` the harness -//! declares, so goose's `skills` platform extension is never loaded. The -//! `load_skill` tool and `SKILL.md` discovery (old `hints.rs:110-204`) have -//! no replacement. Restoring them means explicitly adding the extension. -//! * **`AGENTS.md` hints are not loaded.** Goose's loader keys off +//! declares, so goose's `skills` platform extension is never loaded. Instead +//! `build_agent` registers `load_skill` as a goose *frontend* extension and +//! [`serve_frontend_tool`] answers the calls in-process (goose yields a +//! `FrontendToolRequest` and blocks the stream until we respond). +//! * **`AGENTS.md` hints / skill index.** Goose's loader keys off //! `GOOSE_HINTS_FILENAME` (`.goosehints`); the old code walked the directory //! chain for `AGENTS.md` plus `~/AGENTS.md`. Every repo here ships the -//! former and none ship the latter, so in practice no hints load at all. +//! latter shape, so `hints.rs` is kept and its output injected via +//! `extend_system_prompt` at session build. //! //! What this file keeps is the part Goose does *not* know about: the mapping //! from `AgentEvent` onto the exact `session/update` notifications `buzz-acp` @@ -137,6 +140,7 @@ pub async fn run_turn( wire_tx: &WireSender, cancel: CancellationToken, hook_extension: Option<&str>, + skills: &[crate::hints::SkillEntry], ) -> (Result, TurnTokens) { let mut tokens = TurnTokens::default(); let mut next_message = Message::user().with_text(prompt_to_text(&prompt)); @@ -157,6 +161,7 @@ pub async fn run_turn( &cancel, &mut tokens, hook_extension, + skills, ) .await { @@ -221,6 +226,7 @@ async fn drive_stream( cancel: &CancellationToken, tokens: &mut TurnTokens, hook_extension: Option<&str>, + skills: &[crate::hints::SkillEntry], ) -> Result { let session_config = SessionConfig { id: session_id.to_string(), @@ -302,6 +308,7 @@ async fn drive_stream( &mut compacted, &mut reflections, &mut open_tool_calls, + skills, ) .await { @@ -390,6 +397,7 @@ async fn handle_event( compacted: &mut bool, reflections: &mut usize, open_tool_calls: &mut Vec, + skills: &[crate::hints::SkillEntry], ) -> Option { match event { AgentEvent::Message(msg) => { @@ -400,6 +408,17 @@ async fn handle_event( open_tool_calls.push(req.id.clone()); } + // Frontend tool calls (`load_skill`) never arrive as + // `ToolRequest`: goose strips them from the normal flow + // (`reply_parts.rs` `categorize_tools`) and yields this + // dedicated variant instead, then blocks the reply stream on + // `tool_result_rx` until we answer. Answer it or the turn + // hangs forever. + if let MessageContent::FrontendToolRequest(req) = content { + open_tool_calls.push(req.id.clone()); + serve_frontend_tool(req, agent, skills).await; + } + if let MessageContent::ToolResponse(resp) = content { open_tool_calls.retain(|id| id != &resp.id); @@ -513,6 +532,36 @@ async fn emit_content(content: &MessageContent, session_id: &str, wire_tx: &Wire .await; } + // Frontend tool calls (`load_skill`) are stripped out of the normal + // `ToolRequest` flow by goose and arrive as this variant instead. The + // desktop still needs the announcement: its response comes back as a + // plain `ToolResponse`, and an update for a never-announced id would + // break the announce→terminal pairing. + MessageContent::FrontendToolRequest(req) => { + let (name, raw) = match &req.tool_call { + Ok(call) => ( + call.name.to_string(), + serde_json::to_value(&call.arguments).unwrap_or(Value::Null), + ), + Err(e) => (String::from("unknown"), json!({ "error": e.to_string() })), + }; + wire::send( + wire_tx, + wire::session_update( + session_id, + json!({ + "sessionUpdate": "tool_call", + "toolCallId": req.id, + "title": name, + "kind": "other", + "status": "in_progress", + "rawInput": raw, + }), + ), + ) + .await; + } + MessageContent::ToolResponse(resp) => { let failed = match &resp.tool_result { Ok(r) => r.is_error.unwrap_or(false), @@ -536,6 +585,68 @@ async fn emit_content(content: &MessageContent, session_id: &str, wire_tx: &Wire } } +/// Serve a goose *frontend* tool call. +/// +/// Goose never dispatches frontend tools itself. `handle_frontend_tool_request` +/// (`goose/src/agents/tool_execution.rs:175`) yields the +/// `FrontendToolRequest` message we are handling right now and then **blocks +/// the reply stream** on `tool_result_rx.recv()` until the embedder calls +/// `agent.handle_tool_result(id, ..)`. That has two consequences: +/// +/// * every `FrontendToolRequest` event MUST be answered — returning without +/// sending a result hangs the turn forever, so even an unrecognised tool +/// name gets an error result rather than silence; +/// * exactly one answer per event — a stray extra send would be picked up by +/// the recv of the *next* frontend call and mis-pair results. +async fn serve_frontend_tool( + req: &goose_provider_types::conversation::message::FrontendToolRequest, + agent: &Arc, + skills: &[crate::hints::SkillEntry], +) { + let result = match &req.tool_call { + Ok(call) if call.name == crate::builtin::LOAD_SKILL_TOOL => { + let args = call + .arguments + .clone() + .map(serde_json::Value::Object) + .unwrap_or(serde_json::Value::Null); + crate::builtin::call_load_skill(&args, skills).await + } + // Only `load_skill` is registered, so this arm is unreachable today — + // but goose is already blocked waiting for this id, so answer anyway. + Ok(call) => crate::builtin::error_result(&format!( + "unknown frontend tool {:?}; only {} is served in-process", + call.name, + crate::builtin::LOAD_SKILL_TOOL + )), + // An Err here means goose did NOT block on the result channel — its + // yield only happens inside the `Ok` branch (`tool_execution.rs:181`). + // Sending would leave a stray result in the queue that mis-pairs the + // NEXT frontend call (the recv trusts the id coming off the channel). + Err(e) => { + tracing::warn!("unparseable frontend tool call {}: {e}", req.id); + return; + } + }; + + let content: Vec = result + .content + .iter() + .map(|c| rmcp::model::Content::text(c.as_text_lossy())) + .collect(); + + agent + .handle_tool_result( + req.id.clone(), + Ok(if result.is_error { + rmcp::model::CallToolResult::error(content) + } else { + rmcp::model::CallToolResult::success(content) + }), + ) + .await; +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/buzz-agent/src/builtin.rs b/crates/buzz-agent/src/builtin.rs new file mode 100644 index 0000000000..95e8487611 --- /dev/null +++ b/crates/buzz-agent/src/builtin.rs @@ -0,0 +1,575 @@ +//! 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 = 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, + ) -> 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}" + ); + } +} diff --git a/crates/buzz-agent/src/hints.rs b/crates/buzz-agent/src/hints.rs new file mode 100644 index 0000000000..90833aa3db --- /dev/null +++ b/crates/buzz-agent/src/hints.rs @@ -0,0 +1,726 @@ +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; + +use crate::types::truncate_at_boundary; + +const MAX_HINTS_BYTES: usize = 128 * 1024; +pub const MAX_SKILL_BODY_BYTES: usize = 32 * 1024; +const SKILL_DIRS: &[&str] = &[".agents/skills", ".goose/skills", ".claude/skills"]; + +fn home_dir() -> Option { + 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, +} + +/// Handles both normal repos (`.git/` dir) and worktrees (`.git` file). +fn find_git_root(start: &Path) -> Option { + let mut current = start.to_path_buf(); + loop { + if current.join(".git").exists() { + return Some(current); + } + match current.parent() { + Some(parent) => current = parent.to_path_buf(), + None => return None, + } + } +} + +fn load_hint_files_impl(cwd: &Path, home: Option<&Path>) -> String { + let mut chain = match find_git_root(cwd) { + Some(root) => { + let mut c: Vec = cwd + .ancestors() + .take_while(|a| a.starts_with(&root)) + .map(|a| a.to_path_buf()) + .collect(); + // ancestors() yields cwd first, root last — reverse for root→cwd. + c.reverse(); + c + } + None => vec![cwd.to_path_buf()], + }; + + // Prepend ~/AGENTS.md as global layer, unless ~ is already in the chain. + if let Some(home) = home { + if !chain.iter().any(|d| d == home) { + chain.insert(0, home.to_path_buf()); + } + } + + let mut result = String::new(); + for dir in &chain { + let path = dir.join("AGENTS.md"); + let Ok(content) = std::fs::read_to_string(&path) else { + continue; + }; + if !result.is_empty() { + result.push_str("\n\n"); + } + let remaining = MAX_HINTS_BYTES.saturating_sub(result.len()); + if remaining == 0 { + break; + } + if content.len() <= remaining { + result.push_str(&content); + } else { + let truncated = truncate_at_boundary(&content, remaining); + result.push_str(truncated); + break; + } + } + result +} + +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 = 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, skills: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + let mut subdirs: Vec = 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 { + 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, + visited_dirs: &mut HashSet, +) { + 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 { + 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 +} + +pub fn build_hints_section(cwd: &Path) -> (String, Vec) { + build_hints_section_impl(cwd, home_dir().as_deref()) +} + +fn build_hints_section_impl(cwd: &Path, home: Option<&Path>) -> (String, Vec) { + let hints_text = load_hint_files_impl(cwd, home); + let skills = discover_skills_impl(cwd, home); + + if hints_text.is_empty() && skills.is_empty() { + return (String::new(), skills); + } + + let mut out = String::from("# Additional Instructions\n"); + + if !hints_text.is_empty() { + out.push_str("\n## Project Hints\n"); + out.push_str(&hints_text); + out.push('\n'); + } + + if !skills.is_empty() { + out.push_str("\n## Available Skills\n"); + for skill in &skills { + out.push_str(&format!("- {}: {}\n", skill.name, skill.description)); + } + out.push_str( + "\nUse the `load_skill` tool to read the full content of a skill before using it.\n", + ); + } + + (out, skills) +} + +/// 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 find_git_root_normal_repo() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir(root.join(".git")).unwrap(); + assert_eq!(find_git_root(root), Some(root.to_path_buf())); + } + + #[test] + fn find_git_root_worktree() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + // .git as a file (worktree) + std::fs::write(root.join(".git"), "gitdir: ../main/.git/worktrees/wt").unwrap(); + assert_eq!(find_git_root(root), Some(root.to_path_buf())); + } + + #[test] + fn find_git_root_none() { + let tmp = TempDir::new().unwrap(); + // No .git anywhere under tmp + let result = find_git_root(tmp.path()); + // In a CI environment the test itself may live inside a real git repo, + // so only assert None when tmp is truly isolated (not a subpath of a git repo). + // We verify by checking that any found root is NOT inside tmp. + if let Some(found) = result { + assert!(!found.starts_with(tmp.path())); + } + } + + #[test] + fn find_git_root_from_subdirectory() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir(root.join(".git")).unwrap(); + let deep = root.join("sub").join("deep"); + std::fs::create_dir_all(&deep).unwrap(); + assert_eq!(find_git_root(&deep), Some(root.to_path_buf())); + } + + #[test] + fn load_hint_files_single_at_cwd() { + let tmp = TempDir::new().unwrap(); + let cwd = tmp.path(); + // No .git → no git root discovery; only cwd is checked. + std::fs::write(cwd.join("AGENTS.md"), "cwd hints").unwrap(); + let result = load_hint_files_impl(cwd, None); + assert_eq!(result, "cwd hints"); + } + + #[test] + fn load_hint_files_git_root_and_cwd() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir(root.join(".git")).unwrap(); + std::fs::write(root.join("AGENTS.md"), "root hints").unwrap(); + let sub = root.join("sub"); + std::fs::create_dir(&sub).unwrap(); + std::fs::write(sub.join("AGENTS.md"), "sub hints").unwrap(); + let result = load_hint_files_impl(&sub, None); + // Root hints must come first. + assert!( + result.starts_with("root hints"), + "expected root hints first, got: {result:?}" + ); + assert!(result.contains("sub hints"), "missing sub hints"); + let root_pos = result.find("root hints").unwrap(); + let sub_pos = result.find("sub hints").unwrap(); + assert!(root_pos < sub_pos, "root hints should precede sub hints"); + } + + #[test] + fn load_hint_files_missing_files() { + let tmp = TempDir::new().unwrap(); + let result = load_hint_files_impl(tmp.path(), None); + assert_eq!(result, ""); + } + + #[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 build_hints_section_empty() { + let tmp = TempDir::new().unwrap(); + let (result, skills) = build_hints_section_impl(tmp.path(), None); + assert_eq!(result, ""); + assert!(skills.is_empty()); + } + + #[test] + fn build_hints_section_combined() { + let tmp = TempDir::new().unwrap(); + let cwd = tmp.path(); + + std::fs::write(cwd.join("AGENTS.md"), "Project-level hints.").unwrap(); + + let skill_dir = cwd.join(".agents/skills/buzz-cli"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: buzz-cli\ndescription: CLI reference for Buzz managed agents\n---\nUse `buzz` to manage agents.\n", + ) + .unwrap(); + + let (result, skills) = build_hints_section_impl(cwd, None); + + assert!( + result.contains("# Additional Instructions"), + "missing header" + ); + assert!(result.contains("## Project Hints"), "missing Project Hints"); + assert!( + result.contains("Project-level hints."), + "missing hints content" + ); + assert!( + result.contains("## Available Skills"), + "missing Available Skills" + ); + assert!( + result.contains("buzz-cli: CLI reference for Buzz managed agents"), + "missing skill bullet" + ); + // Body must NOT be inlined — lazy loading only. + assert!( + !result.contains("Use `buzz` to manage agents."), + "skill body must not be inlined in system prompt" + ); + // The load_skill instruction must be present. + assert!( + result.contains("load_skill"), + "missing load_skill instruction" + ); + // The old ### heading format must not appear. + assert!( + !result.contains("### buzz-cli"), + "skill body heading must not be inlined" + ); + // The returned skills list should contain the discovered skill. + assert_eq!(skills.len(), 1); + assert_eq!(skills[0].name, "buzz-cli"); + } + + #[test] + fn load_hint_files_global_loaded_first() { + let home = TempDir::new().unwrap(); + let cwd = TempDir::new().unwrap(); + std::fs::write(home.path().join("AGENTS.md"), "global hints").unwrap(); + std::fs::write(cwd.path().join("AGENTS.md"), "local hints").unwrap(); + let result = load_hint_files_impl(cwd.path(), Some(home.path())); + let global_pos = result.find("global hints").unwrap(); + let local_pos = result.find("local hints").unwrap(); + assert!( + global_pos < local_pos, + "global hints should precede local hints" + ); + } + + #[test] + fn load_hint_files_home_missing_agents_md() { + let home = TempDir::new().unwrap(); + let cwd = TempDir::new().unwrap(); + std::fs::write(cwd.path().join("AGENTS.md"), "local only").unwrap(); + let result = load_hint_files_impl(cwd.path(), Some(home.path())); + assert_eq!(result, "local only"); + } + + #[test] + fn load_hint_files_no_home_dir() { + let cwd = TempDir::new().unwrap(); + std::fs::write(cwd.path().join("AGENTS.md"), "local only").unwrap(); + let result = load_hint_files_impl(cwd.path(), None); + assert_eq!(result, "local only"); + } + + #[test] + fn load_hint_files_dedup_when_home_in_chain() { + let tmp = TempDir::new().unwrap(); + let home = tmp.path(); + std::fs::write(home.join("AGENTS.md"), "single load").unwrap(); + let result = load_hint_files_impl(home, Some(home)); + assert_eq!( + result.matches("single load").count(), + 1, + "AGENTS.md should be loaded exactly once when CWD is home" + ); + } + + #[test] + fn load_hint_files_dedup_when_home_is_git_root() { + let tmp = TempDir::new().unwrap(); + let home = tmp.path(); + std::fs::create_dir(home.join(".git")).unwrap(); + std::fs::write(home.join("AGENTS.md"), "root+home hints").unwrap(); + let sub = home.join("sub"); + std::fs::create_dir(&sub).unwrap(); + let result = load_hint_files_impl(&sub, Some(home)); + assert_eq!( + result.matches("root+home hints").count(), + 1, + "AGENTS.md should be loaded once when home is git root" + ); + } + + #[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 = 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 = 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] + ); + } +} diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index b679b9b5be..b86b67f901 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -21,7 +21,9 @@ //! * `keepalive` — see `agent.rs`. pub mod agent; +pub mod builtin; pub mod config; +pub mod hints; pub mod hooks; pub mod mesh; pub mod types; @@ -69,6 +71,8 @@ struct Session { /// the override takes effect "from the next prompt" and never mutates a /// turn already in flight (`buzz-agent/src/lib.rs:494-502`). pending_model: Option, + /// Skills discovered at session start; the bodies `load_skill` serves. + skills: Vec, /// Name of the MCP extension carrying `_Stop`/`_PostCompact`, if any. /// See [`crate::hooks`] for why we dispatch these ourselves. hook_extension: Option, @@ -93,7 +97,15 @@ async fn build_agent( cwd: &str, system_prompt: Option<&str>, mcp_servers: &[McpServerStdio], -) -> Result<(Arc, String, Option), AgentError> { +) -> Result< + ( + Arc, + String, + Option, + Vec, + ), + AgentError, +> { let session_manager = Arc::new(SessionManager::instance()); let permission_manager = PermissionManager::instance(); @@ -179,6 +191,44 @@ async fn build_agent( } } + // AGENTS.md hints + the skill index. `system_prompt_extras` are appended + // whether the base prompt came from the override or goose's template + // (`prompt_manager.rs:170-198`), so this survives `override_system_prompt`. + let (hints, skills) = crate::hints::build_hints_section(std::path::Path::new(cwd)); + if !hints.trim().is_empty() { + agent + .extend_system_prompt("buzz_hints".to_string(), hints) + .await; + } + + // `load_skill` runs in-process. The prompt carries only skill names and + // descriptions; the body is pulled on demand, which is the whole point of + // the index above. Registered as a goose *frontend* tool: goose advertises + // it, then hands the call back to us rather than dispatching it itself + // (`agent.rs:1167`). + if !skills.is_empty() { + let def = crate::builtin::load_skill_def(); + let tool = rmcp::model::Tool::new( + def.name.clone(), + def.description.clone(), + match def.input_schema { + serde_json::Value::Object(map) => std::sync::Arc::new(map), + _ => std::sync::Arc::new(serde_json::Map::new()), + }, + ); + let ext = ExtensionConfig::Frontend { + name: "buzz_builtin".to_string(), + description: String::new(), + tools: vec![tool], + instructions: None, + bundled: None, + available_tools: Vec::new(), + }; + if let Err(e) = agent.add_extension(ext, &session.id).await { + tracing::warn!("load_skill unavailable: {e}"); + } + } + let agent = Arc::new(agent); // Find the extension carrying `_Stop`, by asking rather than assuming the @@ -209,7 +259,7 @@ async fn build_agent( tracing::info!(extension = %ext, "lifecycle hooks available"); } - Ok((agent, session.id, hook_extension)) + Ok((agent, session.id, hook_extension, skills)) } /// Keep the model's hands off the lifecycle hooks. @@ -422,7 +472,7 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen .await; } - let (agent, goose_session_id, hook_extension) = + let (agent, goose_session_id, hook_extension, skills) = match build_agent(&app.cfg, &p.cwd, p.system_prompt.as_deref(), &p.mcp_servers).await { Ok(v) => v, Err(e) => { @@ -464,6 +514,7 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen model_override: None, pending_model: None, hook_extension, + skills, accumulated_input_tokens: 0, accumulated_output_tokens: 0, }, @@ -558,7 +609,7 @@ async fn session_prompt(app: &Arc, id: Value, params: Value, wire_tx: &Wire let cancel = CancellationToken::new(); // Single-flight per session, and capture the agent handle. - let (agent, goose_session_id, pending_model, hook_extension) = { + let (agent, goose_session_id, pending_model, hook_extension, skills) = { let mut sessions = app.sessions.lock().await; let Some(s) = sessions.get_mut(&p.session_id) else { return wire::send( @@ -588,6 +639,7 @@ async fn session_prompt(app: &Arc, id: Value, params: Value, wire_tx: &Wire s.goose_session_id.clone(), s.pending_model.take(), s.hook_extension.clone(), + s.skills.clone(), ) }; @@ -626,6 +678,7 @@ async fn session_prompt(app: &Arc, id: Value, params: Value, wire_tx: &Wire wire_tx, cancel, hook_extension.as_deref(), + &skills, ) .await; diff --git a/crates/buzz-agent/src/types.rs b/crates/buzz-agent/src/types.rs index de5e50b3a2..3b52cdc2f0 100644 --- a/crates/buzz-agent/src/types.rs +++ b/crates/buzz-agent/src/types.rs @@ -105,3 +105,61 @@ impl AgentError { } } } + +/// A tool definition advertised to the model. Used by [`crate::builtin`] for +/// the in-process `load_skill` tool. +#[derive(Debug, Clone)] +pub struct ToolDef { + pub name: String, + pub description: String, + pub input_schema: serde_json::Value, +} + +/// One piece of a tool result. +#[derive(Debug, Clone)] +pub enum ToolResultContent { + Text(String), + Image { data: String, mime_type: String }, +} + +impl ToolResultContent { + pub fn as_text_lossy(&self) -> String { + match self { + Self::Text(t) => t.clone(), + Self::Image { mime_type, .. } => format!("[image: {mime_type}]"), + } + } +} + +/// Result of an in-process tool call. +#[derive(Debug, Clone)] +pub struct ToolResult { + pub provider_id: String, + pub content: Vec, + pub is_error: bool, +} + +impl ToolResult { + pub fn text(&self) -> String { + self.content + .iter() + .map(ToolResultContent::as_text_lossy) + .collect::>() + .join("\n") + } +} + +/// Truncate `s` to at most `max` bytes without splitting a UTF-8 character. +/// +/// Lifted from the deleted `mcp.rs`; `hints` and `builtin` both cap what they +/// read off disk with it. +pub fn truncate_at_boundary(s: &str, max: usize) -> &str { + if s.len() <= max { + return s; + } + let mut cut = max; + while cut > 0 && !s.is_char_boundary(cut) { + cut -= 1; + } + &s[..cut] +} diff --git a/crates/buzz-agent/tests/skills.rs b/crates/buzz-agent/tests/skills.rs new file mode 100644 index 0000000000..af792cbbb6 --- /dev/null +++ b/crates/buzz-agent/tests/skills.rs @@ -0,0 +1,277 @@ +//! 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 +//! *frontend* tool, which means goose advertises it to the model but refuses to +//! dispatch it — it returns "Frontend tool execution required" and waits for +//! the embedder to call `handle_tool_result` +//! (`goose/src/agents/agent.rs:1167`). If that wiring is wrong the turn does +//! not fail, it **hangs forever**, so the timeout in `await_response` is the +//! real assertion here. + +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; +use std::process::{Child, Command, Stdio}; +use std::sync::mpsc; +use std::time::Duration; + +use serde_json::{json, Value}; + +/// Fake OpenAI provider that asks for `load_skill` on the first turn, then +/// answers normally. Sends every observed `system` prompt and tool list back. +fn spawn_provider() -> (String, mpsc::Receiver<(String, Vec)>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); + let addr = listener.local_addr().expect("addr"); + let (tx, rx) = mpsc::channel(); + + std::thread::spawn(move || { + let mut call_count = 0usize; + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + let mut reader = BufReader::new(stream.try_clone().expect("clone")); + + let mut request_line = String::new(); + if reader.read_line(&mut request_line).is_err() { + continue; + } + let mut len = 0usize; + loop { + let mut line = String::new(); + if reader.read_line(&mut line).is_err() { + break; + } + let t = line.trim_end(); + if t.is_empty() { + break; + } + if let Some(v) = t + .strip_prefix("content-length: ") + .or_else(|| t.strip_prefix("Content-Length: ")) + { + len = v.parse().unwrap_or(0); + } + } + let mut body = vec![0u8; len]; + use std::io::Read; + let _ = reader.read_exact(&mut body); + + let send = |stream: &mut std::net::TcpStream, ct: &str, payload: String| { + let _ = stream.write_all( + format!( + "HTTP/1.1 200 OK\r\ncontent-type: {ct}\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + payload.len(), payload + ).as_bytes(), + ); + let _ = stream.flush(); + }; + + if request_line.contains("/models") { + send( + &mut stream, + "application/json", + json!({"object":"list","data":[{"id":"fake-model"}]}).to_string(), + ); + continue; + } + + if let Ok(req) = serde_json::from_slice::(&body) { + let system = req["messages"] + .as_array() + .and_then(|m| { + m.iter() + .find(|x| x["role"] == "system") + .and_then(|x| x["content"].as_str()) + .map(str::to_owned) + }) + .unwrap_or_default(); + let tools = req["tools"] + .as_array() + .map(|ts| { + ts.iter() + .filter_map(|t| t["function"]["name"].as_str().map(str::to_owned)) + .collect() + }) + .unwrap_or_default(); + let _ = tx.send((system, tools)); + } + + call_count += 1; + let sse = if call_count == 1 { + // Ask for the skill. If our frontend-tool plumbing is broken, + // goose never resolves this and the turn hangs. + let c = json!({ + "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":"load_skill","arguments":"{\"name\":\"widget-maker\"}"} + }]},"finish_reason":null}] + }); + let d = json!({ + "id":"c","object":"chat.completion.chunk","created":1,"model":"fake-model", + "choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}], + "usage":{"prompt_tokens":5,"completion_tokens":2,"total_tokens":7} + }); + format!("data: {c}\n\ndata: {d}\n\ndata: [DONE]\n\n") + } else { + let c = json!({ + "id":"c","object":"chat.completion.chunk","created":1,"model":"fake-model", + "choices":[{"index":0,"delta":{"role":"assistant","content":"Got the skill."},"finish_reason":null}] + }); + let d = json!({ + "id":"c","object":"chat.completion.chunk","created":1,"model":"fake-model", + "choices":[{"index":0,"delta":{},"finish_reason":"stop"}], + "usage":{"prompt_tokens":5,"completion_tokens":2,"total_tokens":7} + }); + format!("data: {c}\n\ndata: {d}\n\ndata: [DONE]\n\n") + }; + send(&mut stream, "text/event-stream", sse); + } + }); + + (format!("http://{addr}"), rx) +} + +struct Harness { + child: Child, + stdin: std::process::ChildStdin, + stdout: BufReader, + id: i64, +} + +impl Harness { + fn start(base_url: &str, home: &std::path::Path) -> Self { + let mut child = Command::new(env!("CARGO_BIN_EXE_buzz-agent")) + .env("BUZZ_AGENT_PROVIDER", "openai-compat") + .env("BUZZ_AGENT_MODEL", "fake-model") + .env("OPENAI_COMPAT_API_KEY", "k") + .env("OPENAI_COMPAT_BASE_URL", base_url) + .env("HOME", home) + .env("XDG_CONFIG_HOME", home.join("cfg")) + .env("XDG_DATA_HOME", home.join("data")) + .env("GOOSE_DISABLE_KEYRING", "1") + .env("RUST_LOG", "warn") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .expect("spawn"); + let stdin = child.stdin.take().expect("stdin"); + let stdout = BufReader::new(child.stdout.take().expect("stdout")); + Self { + child, + stdin, + stdout, + id: 0, + } + } + + fn call(&mut self, method: &str, params: Value) -> Value { + self.id += 1; + let id = self.id; + writeln!( + self.stdin, + "{}", + json!({"jsonrpc":"2.0","id":id,"method":method,"params":params}) + ) + .expect("write"); + self.stdin.flush().expect("flush"); + loop { + let mut line = String::new(); + let n = self.stdout.read_line(&mut line).expect("read"); + assert_ne!(n, 0, "agent closed stdout awaiting {method}"); + let Ok(msg) = serde_json::from_str::(&line) else { + continue; + }; + if msg.get("id").and_then(Value::as_i64) == Some(id) { + return msg; + } + } + } +} + +impl Drop for Harness { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +/// Build a workspace with an AGENTS.md and one skill. +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. + std::fs::create_dir_all(root.join(".git")).unwrap(); + let skill = root.join(".agents/skills/widget-maker"); + std::fs::create_dir_all(&skill).unwrap(); + std::fs::write( + skill.join("SKILL.md"), + "---\nname: widget-maker\ndescription: Makes widgets to order.\n---\n\ + Step 1: measure. Step 2: cut the flange.", + ) + .unwrap(); + dir +} + +#[test] +fn agents_md_and_skill_index_reach_the_model_and_load_skill_resolves() { + let (base_url, seen) = spawn_provider(); + let home = tempfile::tempdir().expect("home"); + let ws = workspace(); + let mut h = Harness::start(&base_url, home.path()); + + let r = h.call("initialize", json!({"protocolVersion": 2})); + assert_eq!(r["result"]["agentInfo"]["name"], "buzz-agent"); + + let r = h.call( + "session/new", + json!({"cwd": ws.path().to_str().unwrap(), "mcpServers": [], + "systemPrompt": "You are Fizz."}), + ); + let sid = r["result"]["sessionId"] + .as_str() + .unwrap_or_else(|| panic!("session/new failed: {r}")) + .to_string(); + + // The turn only returns if load_skill was served. A broken frontend-tool + // path hangs here rather than erroring. + let r = h.call( + "session/prompt", + json!({"sessionId": sid, "prompt": [{"type":"text","text":"use the widget skill"}]}), + ); + assert_eq!( + r["result"]["stopReason"], "end_turn", + "turn did not complete — load_skill was probably never resolved: {r}" + ); + + let mut systems = Vec::new(); + let mut tool_lists = Vec::new(); + while let Ok((sys, tools)) = seen.recv_timeout(Duration::from_millis(500)) { + systems.push(sys); + tool_lists.push(tools); + } + assert!(!systems.is_empty(), "provider was never called"); + + let first = &systems[0]; + assert!( + first.contains("Always ship the sprocket first."), + "AGENTS.md hints missing from system prompt:\n{first}" + ); + assert!( + first.contains("widget-maker") && first.contains("Makes widgets to order."), + "skill index (name + description) missing from system prompt:\n{first}" + ); + assert!( + !first.contains("Step 2: cut the flange."), + "skill BODY was inlined — the point of load_skill is that it is not:\n{first}" + ); + assert!( + tool_lists[0].iter().any(|t| t == "load_skill"), + "load_skill not advertised: {:?}", + tool_lists[0] + ); + assert!( + systems.len() >= 2, + "expected a second round after the tool result" + ); +}