mirror of
https://github.com/block/buzz.git
synced 2026-11-01 15:01:12 +00:00
feat(buzz-agent): restore AGENTS.md hints and load_skill on the goose loop
The swap documented both as losses (B1/B2 in fa8166e60). Restore them by
keeping the old buzz-agent modules and wiring them into goose, rather than
using goose's own equivalents, which don't fit:
* goose's hint loader keys off GOOSE_HINTS_FILENAME (.goosehints); our repos
ship AGENTS.md. hints.rs (directory-chain walk + ~/AGENTS.md + skill
discovery under .agents/skills, .goose/skills, .claude/skills) is kept and
its output injected via extend_system_prompt("buzz_hints") at session
build — system_prompt_extras survive override_system_prompt, so this works
with the persona path too.
* goose's skills platform extension is never loaded (Agent::with_config
loads zero extensions; build_agent only adds the harness's mcpServers).
builtin.rs / load_skill is kept and registered as a goose *frontend*
extension: goose advertises the tool, and we answer the calls in-process.
The frontend-tool wiring had a deadlock in the first cut: it listened for
MessageContent::ToolRequest, but goose strips frontend calls out of the
normal ToolRequest flow (reply_parts.rs categorize_tools) and yields a
dedicated FrontendToolRequest variant instead, then BLOCKS the reply stream
on tool_result_rx.recv() until handle_tool_result is called. The handler
never matched, so the first load_skill call hung the turn forever — this is
what the hung `cargo test --test skills` processes on this machine were.
Now:
* serve_frontend_tool matches FrontendToolRequest, answers every yielded
request exactly once (unknown tool name gets an error result rather than
silence — goose is already blocked on the id), and skips only the Err
parse case, where goose does not block (tool_execution.rs:181 yields
inside the Ok arm only).
* emit_content announces FrontendToolRequest to the desktop as a tool_call
update; its result comes back as a plain ToolResponse, and an update for
a never-announced id would break the announce→terminal pairing that keeps
the UI spinner honest.
The skills integration test drives the full path over stdio against a fake
SSE provider: AGENTS.md content and the skill name/description index must
reach the system prompt, the skill BODY must not (that is the point of
load_skill), load_skill must be advertised, and the turn must complete —
a broken frontend-tool path fails by hanging, so the turn completing IS the
assertion.
53 buzz-agent tests green; fmt + clippy -D warnings clean.
Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -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<StopReason, AgentError>, 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<StopReason, AgentError> {
|
||||
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<String>,
|
||||
skills: &[crate::hints::SkillEntry],
|
||||
) -> Option<StopReason> {
|
||||
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<Agent>,
|
||||
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<rmcp::model::Content> = 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::*;
|
||||
|
||||
575
crates/buzz-agent/src/builtin.rs
Normal file
575
crates/buzz-agent/src/builtin.rs
Normal file
@@ -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<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}"
|
||||
);
|
||||
}
|
||||
}
|
||||
726
crates/buzz-agent/src/hints.rs
Normal file
726
crates/buzz-agent/src/hints.rs
Normal file
@@ -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<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 find_git_root(start: &Path) -> Option<PathBuf> {
|
||||
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<PathBuf> = 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<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
|
||||
}
|
||||
|
||||
pub fn build_hints_section(cwd: &Path) -> (String, Vec<SkillEntry>) {
|
||||
build_hints_section_impl(cwd, home_dir().as_deref())
|
||||
}
|
||||
|
||||
fn build_hints_section_impl(cwd: &Path, home: Option<&Path>) -> (String, Vec<SkillEntry>) {
|
||||
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<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]
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -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<String>,
|
||||
/// Skills discovered at session start; the bodies `load_skill` serves.
|
||||
skills: Vec<crate::hints::SkillEntry>,
|
||||
/// Name of the MCP extension carrying `_Stop`/`_PostCompact`, if any.
|
||||
/// See [`crate::hooks`] for why we dispatch these ourselves.
|
||||
hook_extension: Option<String>,
|
||||
@@ -93,7 +97,15 @@ async fn build_agent(
|
||||
cwd: &str,
|
||||
system_prompt: Option<&str>,
|
||||
mcp_servers: &[McpServerStdio],
|
||||
) -> Result<(Arc<Agent>, String, Option<String>), AgentError> {
|
||||
) -> Result<
|
||||
(
|
||||
Arc<Agent>,
|
||||
String,
|
||||
Option<String>,
|
||||
Vec<crate::hints::SkillEntry>,
|
||||
),
|
||||
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<App>, 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<App>, 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<App>, 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<App>, 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<App>, id: Value, params: Value, wire_tx: &Wire
|
||||
wire_tx,
|
||||
cancel,
|
||||
hook_extension.as_deref(),
|
||||
&skills,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -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<ToolResultContent>,
|
||||
pub is_error: bool,
|
||||
}
|
||||
|
||||
impl ToolResult {
|
||||
pub fn text(&self) -> String {
|
||||
self.content
|
||||
.iter()
|
||||
.map(ToolResultContent::as_text_lossy)
|
||||
.collect::<Vec<_>>()
|
||||
.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]
|
||||
}
|
||||
|
||||
277
crates/buzz-agent/tests/skills.rs
Normal file
277
crates/buzz-agent/tests/skills.rs
Normal file
@@ -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<String>)>) {
|
||||
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::<Value>(&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<std::process::ChildStdout>,
|
||||
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::<Value>(&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"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user