From 971c690328b5df23be3e9458c6239e4327ec2c09 Mon Sep 17 00:00:00 2001 From: Douwe Osinga Date: Tue, 19 May 2026 17:45:14 -0400 Subject: [PATCH] Feat/summon subagent instructions (#9325) Co-authored-by: Douwe Osinga --- crates/goose/src/agents/agent.rs | 34 +- .../src/agents/platform_extensions/summon.rs | 141 ++++++-- scripts/test_subagents.sh | 330 ++++++++++++++++++ 3 files changed, 444 insertions(+), 61 deletions(-) create mode 100755 scripts/test_subagents.sh diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 57dcec5d..1e1c8868 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -22,7 +22,6 @@ use crate::agents::extension_manager::{ get_parameter_names, ExtensionManager, ExtensionManagerCapabilities, }; use crate::agents::final_output_tool::{FINAL_OUTPUT_CONTINUATION_MESSAGE, FINAL_OUTPUT_TOOL_NAME}; -use crate::agents::platform_extensions::summon::discover_filesystem_sources; use crate::agents::platform_extensions::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; use crate::agents::platform_tools::PLATFORM_MANAGE_SCHEDULE_TOOL_NAME; use crate::agents::prompt_manager::PromptManager; @@ -587,17 +586,10 @@ impl Agent { } let initial_messages = conversation.messages().clone(); - let (tools, toolshim_tools, mut system_prompt) = self + let (tools, toolshim_tools, system_prompt) = self .prepare_tools_and_prompt(session_id, working_dir) .await?; - if let Some(instructions) = self.resolve_at_mention(&conversation, working_dir) { - system_prompt = format!( - "{}\n\n# Instructions from active agent:\n\n{}", - system_prompt, instructions - ); - } - let goose_mode = *self.current_goose_mode.lock().await; if goose_mode == GooseMode::SmartApprove { @@ -631,30 +623,6 @@ impl Agent { }) } - fn resolve_at_mention( - &self, - conversation: &Conversation, - working_dir: &std::path::Path, - ) -> Option { - let last_message = conversation.messages().last()?; - if last_message.role == rmcp::model::Role::User { - let after_at = last_message - .as_concat_text() - .trim() - .strip_prefix('@')? - .to_lowercase(); - - for source in discover_filesystem_sources(working_dir) { - let name = source.name.to_lowercase(); - let is_match = after_at == name || after_at.starts_with(&format!("{} ", name)); - if is_match && !source.content.is_empty() { - return Some(source.content.clone()); - } - } - } - None - } - async fn categorize_tools( &self, response: &Message, diff --git a/crates/goose/src/agents/platform_extensions/summon.rs b/crates/goose/src/agents/platform_extensions/summon.rs index a1ba0d07..1beeb79f 100644 --- a/crates/goose/src/agents/platform_extensions/summon.rs +++ b/crates/goose/src/agents/platform_extensions/summon.rs @@ -13,6 +13,7 @@ use crate::recipe::{Recipe, Settings, RECIPE_FILE_EXTENSIONS}; use crate::session::extension_data::EnabledExtensionsState; use crate::session::SessionType; use crate::sources::parse_frontmatter; +use crate::utils::safe_truncate; use anyhow::Result; use async_trait::async_trait; use goose_sdk::custom_requests::{SourceEntry, SourceType}; @@ -34,6 +35,10 @@ use tracing::{info, warn}; pub static EXTENSION_NAME: &str = "summon"; +const SUBAGENT_DESCRIPTION_BUDGET: usize = 160; + +const TASK_LABEL_BUDGET: usize = 60; + fn kind_plural(kind: SourceType) -> &'static str { match kind { SourceType::Subrecipe => "Subrecipes", @@ -43,17 +48,6 @@ fn kind_plural(kind: SourceType) -> &'static str { } } -fn truncate(s: &str, max_len: usize) -> String { - if s.chars().count() <= max_len { - s.to_string() - } else if max_len <= 3 { - "...".to_string() - } else { - let truncated: String = s.chars().take(max_len - 3).collect(); - format!("{}...", truncated) - } -} - #[derive(Debug, Default, Deserialize)] pub struct DelegateParams { pub instructions: Option, @@ -291,6 +285,99 @@ pub fn discover_filesystem_sources(working_dir: &Path) -> Vec { sources } +fn build_subagent_instructions(session: Option<&crate::session::Session>) -> String { + let Some(session) = session else { + return String::new(); + }; + + // filter the sources down to what we want even though currently that is what we get + let mut sources: Vec = discover_filesystem_sources(&session.working_dir) + .into_iter() + .filter(|s| { + matches!( + s.source_type, + SourceType::Agent | SourceType::Recipe | SourceType::Subrecipe + ) + }) + .collect(); + + // If the session is started from a recipe, also use the subrecipes for + // that recipe as delegate targets + if let Some(recipe) = session.recipe.as_ref() { + if let Some(subs) = recipe.sub_recipes.as_ref() { + let mut seen: std::collections::HashSet = + sources.iter().map(|s| s.name.clone()).collect(); + for sr in subs { + if !seen.insert(sr.name.clone()) { + continue; + } + sources.push(SourceEntry { + source_type: SourceType::Subrecipe, + name: sr.name.clone(), + description: sr.description.clone().unwrap_or_default(), + content: String::new(), + path: sr.path.clone(), + global: false, + writable: false, + supporting_files: Vec::new(), + properties: std::collections::HashMap::new(), + }); + } + } + } + + if sources.is_empty() { + return String::new(); + } + + sources.sort_by(|a, b| (&a.source_type, &a.name).cmp(&(&b.source_type, &b.name))); + let subagents: Vec<&SourceEntry> = sources.iter().collect(); + + let names = subagents + .iter() + .map(|s| s.name.as_str()) + .collect::>() + .join(", "); + + let mut out = String::new(); + out.push_str( + "\n\nThe following named subagents are available in this session and \ + can be invoked through the `delegate` tool (run as a subagent) or \ + the `load` tool (read their instructions into your own context):\n", + ); + + let mut current_kind: Option = None; + for s in &subagents { + if current_kind != Some(s.source_type) { + out.push_str(&format!("\n{}:", kind_plural(s.source_type))); + current_kind = Some(s.source_type); + } + out.push_str(&format!( + "\n• {} — {}", + s.name, + safe_truncate(&s.description, SUBAGENT_DESCRIPTION_BUDGET) + )); + } + + out.push_str(&format!( + "\n\nWhen to call a subagent (one of [{names}]):\n\ + • `@` in the user's message — always call that subagent.\n\ + • The user mentions a subagent by name without `@` — infer from \ + context whether they want it invoked, and if so, call it.\n\ + • The user's request strongly matches a subagent's description — \ + call it.\n\n\ + Calling a subagent normally means `delegate(source: \"\", \ + instructions: ...)`, which runs it as an isolated subagent and \ + returns its result. Use `load(source: \"\")` instead if you \ + only want to read the subagent's instructions into your own \ + context. For long-running work, pass `async: true` to `delegate` — \ + it returns a task id immediately, and you collect the result later \ + with `load(source: \"\")`, which waits for completion.", + )); + + out +} + fn round_duration(d: Duration) -> String { let secs = d.as_secs(); if secs < 60 { @@ -341,8 +428,11 @@ impl Drop for SummonClient { impl SummonClient { pub fn new(context: PlatformExtensionContext) -> Result { + let instructions = build_subagent_instructions(context.session.as_deref()); + let info = InitializeResult::new(ServerCapabilities::builder().enable_tools().build()) - .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Summon")); + .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Summon")) + .with_instructions(instructions); Ok(Self { info, @@ -859,7 +949,7 @@ impl SummonClient { output.push_str(&format!( "• {} - {}\n", source.name, - truncate(&source.description, 60) + safe_truncate(&source.description, SUBAGENT_DESCRIPTION_BUDGET) )); } } @@ -1430,16 +1520,11 @@ impl SummonClient { } fn get_task_description(params: &DelegateParams) -> String { - if let Some(source) = ¶ms.source { - if let Some(instructions) = ¶ms.instructions { - format!("{}: {}", source, truncate(instructions, 30)) - } else { - source.clone() - } - } else if let Some(instructions) = ¶ms.instructions { - truncate(instructions, 40) - } else { - "Unknown task".to_string() + match (¶ms.source, ¶ms.instructions) { + (Some(source), Some(instructions)) => format!("{}: {}", source, instructions), + (Some(source), None) => source.clone(), + (None, Some(instructions)) => instructions.clone(), + (None, None) => "Unknown task".to_string(), } } @@ -1474,7 +1559,7 @@ impl SummonClient { .await .map_err(|e| format!("Failed to build task config: {}", e))?; - let description = truncate(&Self::get_task_description(¶ms), 40); + let description = safe_truncate(&Self::get_task_description(¶ms), TASK_LABEL_BUDGET); // Subagents must use Auto until get_agent_messages forwards // ActionRequired messages to the parent. Until then, any mode @@ -1965,10 +2050,10 @@ You review code."#; SummonClient::get_task_description(&make_params(Some("r"), Some("task"))), "r: task" ); - - let long = "x".repeat(100); - let desc = SummonClient::get_task_description(&make_params(None, Some(&long))); - assert!(desc.len() <= 43 && desc.ends_with("...")); + assert_eq!( + SummonClient::get_task_description(&make_params(None, None)), + "Unknown task" + ); } #[test] diff --git a/scripts/test_subagents.sh b/scripts/test_subagents.sh new file mode 100755 index 00000000..abb0771e --- /dev/null +++ b/scripts/test_subagents.sh @@ -0,0 +1,330 @@ +#!/bin/bash +# Local smoke test for subagent @-mention behaviour. +# +# Sets up a workdir with two named subagents on disk, runs goose against +# several prompts, and validates that goose delegates to the right subagent +# in each case. Uses an LLM judge for the fuzzy-match scenarios. +# +# Not wired into CI — run manually: +# bash scripts/test_subagents.sh +# +# Knobs: +# GOOSE_PROVIDER (default: anthropic) +# GOOSE_MODEL (default: claude-haiku-4-5) +# SKIP_BUILD skip cargo build (assumes target/debug/goose already exists) +# KEEP_TESTDIR don't rm the temp workdir on exit (for debugging) +# +# Agent names are deliberately weird ("janpier", "peterjoris") so that they +# won't collide with anything the user might have in ~/.agents, ~/.goose, or +# ~/.claude. The empty-workdir scenario asserts those specific names do NOT +# leak in from elsewhere, which is the practical way to detect global +# pollution without trying to sandbox $HOME (which would break provider +# config loading). + +set -e + +if [ -f .env ]; then + export $(grep -v '^#' .env | xargs) +fi + +if [ -z "$SKIP_BUILD" ]; then + echo "Building goose..." + cargo build --bin goose + echo "" +else + echo "Skipping build (SKIP_BUILD is set)..." + echo "" +fi + +SCRIPT_DIR=$(pwd) +GOOSE_BIN="$SCRIPT_DIR/target/debug/goose" +export PATH="$SCRIPT_DIR/target/debug:$PATH" + +export GOOSE_PROVIDER="${GOOSE_PROVIDER:-anthropic}" +export GOOSE_MODEL="${GOOSE_MODEL:-claude-haiku-4-5}" + +echo "Using provider: $GOOSE_PROVIDER" +echo "Using model: $GOOSE_MODEL" +echo "" + +TESTDIR=$(mktemp -d) +echo "Test workdir: $TESTDIR" +if [ -z "$KEEP_TESTDIR" ]; then + trap 'rm -rf "$TESTDIR"' EXIT +else + echo "(KEEP_TESTDIR set — workdir will not be cleaned up)" +fi + +# Two subagents with deliberately recognizable behaviour and unusual names +# so they can't collide with any pre-existing global agents in +# ~/.agents/agents, ~/.goose/agents, or ~/.claude/agents. +# +# - janpier: a farmer with trick-performing animals (cow, pig, donkey). The +# donkey is the one that speaks. Emits HEEHAW_DONKEY_OK as proof that +# delegation actually executed end-to-end. +# - peterjoris: an expert in the Forth programming language. Emits FORTH_OK +# when it answers a Forth question. + +mkdir -p "$TESTDIR/.agents/agents" + +cat > "$TESTDIR/.agents/agents/janpier.md" << 'EOF' +--- +name: janpier +description: Janpier is a farmer who owns a small farm with three trick-performing animals — a cow, a pig, and a donkey. The donkey is the only one that can speak. +--- +You are Janpier, a farmer. You have three animals: a cow, a pig, and a +donkey. Each knows tricks. The donkey is special because it can speak in +human words. Whenever you are asked anything about the farm, the animals, +or the donkey speaking, include the exact literal marker string +"HEEHAW_DONKEY_OK" somewhere in your reply so the caller can verify you +ran. Then describe what the donkey says. +EOF + +cat > "$TESTDIR/.agents/agents/peterjoris.md" << 'EOF' +--- +name: peterjoris +description: Peterjoris is an expert in the Forth programming language and can write, explain, and debug Forth code. +--- +You are Peterjoris, an expert in the Forth programming language. When +asked anything about Forth — stack manipulation, words, definitions, or +example programs — answer with concrete Forth code and a short +explanation. Always include the literal marker string "FORTH_OK" +somewhere in your reply so the caller can verify you ran. +EOF + +echo "Created subagents in $TESTDIR/.agents/agents/:" +ls "$TESTDIR/.agents/agents/" +echo "" + +RESULTS=() + +# Run goose with a prompt in TESTDIR. We use --no-session for hermeticity. +run_goose() { + local prompt="$1" + local outfile="$2" + (cd "$TESTDIR" && "$GOOSE_BIN" run --text "$prompt" --no-session 2>&1) | tee "$outfile" +} + +# Detect: did the model invoke `delegate` with the expected source? +# The CLI renders these as: +# ▸ delegate +# source janpier +assert_delegated_to() { + local source="$1" + local outfile="$2" + local scenario="$3" + + if grep -qE "▸.*delegate" "$outfile" && grep -qE "^\s*source[[:space:]]+$source\b" "$outfile"; then + echo "✓ $scenario: delegated to $source" + RESULTS+=("✓ $scenario") + return 0 + else + echo "✗ $scenario: did NOT delegate to $source" + RESULTS+=("✗ $scenario") + return 1 + fi +} + +# Detect: did some literal string (e.g. the marker the subagent emits) +# appear in the transcript? This proves the subagent actually ran and its +# output came back, not just that delegate was called. +assert_contains() { + local needle="$1" + local outfile="$2" + local scenario="$3" + + if grep -qF "$needle" "$outfile"; then + echo "✓ $scenario: transcript contains '$needle'" + RESULTS+=("✓ $scenario") + else + echo "✗ $scenario: transcript missing '$needle'" + RESULTS+=("✗ $scenario") + fi +} + +assert_not_contains() { + local needle="$1" + local outfile="$2" + local scenario="$3" + + if grep -qF "$needle" "$outfile"; then + echo "✗ $scenario: transcript unexpectedly contains '$needle'" + RESULTS+=("✗ $scenario") + else + echo "✓ $scenario: transcript does not contain '$needle'" + RESULTS+=("✓ $scenario") + fi +} + +# LLM judge for free-form scenarios where exact-grep is too brittle. +# Returns 0 on PASS, 1 on FAIL. +llm_judge() { + local outfile="$1" + local question="$2" + + local judge_prompt + judge_prompt=$(cat <&1) + echo "$verdict" | tr -d '\r' | grep -Eq '^[[:space:]]*PASS[[:space:]]*$' +} + +assert_judge() { + local outfile="$1" + local question="$2" + local scenario="$3" + + if llm_judge "$outfile" "$question"; then + echo "✓ $scenario (judge)" + RESULTS+=("✓ $scenario (judge)") + else + echo "✗ $scenario (judge)" + RESULTS+=("✗ $scenario (judge)") + fi +} + +# --------------------------------------------------------------------------- +# Scenario 1: explicit @-mention +# --------------------------------------------------------------------------- +echo "=== Scenario 1: explicit @janpier mention ===" +TMP1=$(mktemp) +run_goose "@janpier which of your animals can speak?" "$TMP1" +assert_delegated_to "janpier" "$TMP1" "S1: @janpier delegates to janpier" +assert_contains "HEEHAW_DONKEY_OK" "$TMP1" "S1: janpier's marker surfaces in output" +rm "$TMP1" +echo "" + +# --------------------------------------------------------------------------- +# Scenario 2: name without @ +# Tests the "if the user only mentions the name, still launch the subagent" +# part of summon's instructions. +# --------------------------------------------------------------------------- +echo "=== Scenario 2: bare name (no @) ===" +TMP2=$(mktemp) +run_goose "Ask janpier what tricks his animals can do." "$TMP2" +assert_delegated_to "janpier" "$TMP2" "S2: bare name delegates to janpier" +rm "$TMP2" +echo "" + +# --------------------------------------------------------------------------- +# Scenario 3: description match (no name, no @) +# +# Tests "the user describes a task that matches a subagent's description, +# so the model SHOULD delegate". This is the weakest signal in the spec +# and the assertion is correspondingly soft: +# +# PASS if the model delegated to peterjoris, OR +# PASS if the model otherwise indicated peterjoris was the right tool, +# OR if it produced a correct Forth answer attributable to that +# subagent (which is the user-visible outcome we actually care +# about). +# +# We deliberately do NOT require the FORTH_OK marker here. Even when +# delegation happens, the parent model often re-renders the subagent's +# reply in its own voice and drops literal markers. That's fine for this +# scenario — the contract is "delegate when description matches", not +# "preserve the subagent's literal output verbatim". +# --------------------------------------------------------------------------- +echo "=== Scenario 3: description match (no name) ===" +TMP3=$(mktemp) +run_goose "Write me a hello world program in the Forth programming language." "$TMP3" + +if grep -qE "▸.*delegate" "$TMP3" && grep -qE "^\s*source[[:space:]]+peterjoris\b" "$TMP3"; then + echo "✓ S3: description match delegated to peterjoris" + RESULTS+=("✓ S3: description match delegated to peterjoris") +else + echo "⚠ S3: did not delegate to peterjoris directly — using LLM judge to grade overall behaviour" + assert_judge "$TMP3" \ + "The user asked goose to write a Hello World program in the Forth programming language. The session had a registered subagent named 'peterjoris' described as a Forth expert. Does the transcript show ANY of: (a) goose called the delegate tool with source 'peterjoris', or (b) goose's reply mentions peterjoris (or 'the Forth expert') as the right specialist for this task, or (c) goose produced syntactically plausible Forth code as the answer? ANY of (a), (b), (c) counts as PASS. Only FAIL if none of those apply." \ + "S3: description match handled" +fi +rm "$TMP3" +echo "" + +# --------------------------------------------------------------------------- +# Scenario 4: negative — no subagent matches +# A prompt that doesn't match either agent should NOT delegate. +# --------------------------------------------------------------------------- +echo "=== Scenario 4: negative (no subagent should be invoked) ===" +TMP4=$(mktemp) +run_goose "What is 2 + 2? Reply with just the digit." "$TMP4" +if grep -qE "▸.*delegate" "$TMP4"; then + echo "✗ S4: unexpectedly delegated for an unrelated prompt" + RESULTS+=("✗ S4: spurious delegation on unrelated prompt") +else + echo "✓ S4: no spurious delegation" + RESULTS+=("✓ S4: no spurious delegation") +fi +rm "$TMP4" +echo "" + +# --------------------------------------------------------------------------- +# Scenario 5: empty workdir — janpier/peterjoris must NOT leak +# +# We can't fully sandbox $HOME without breaking provider-config loading, so +# instead of asserting "no agents at all", we assert that the two specific, +# deliberately-weird names we registered for this test (janpier, peterjoris) +# do NOT show up in a fresh workdir's transcript. If they do, summon is +# pulling them from a global location and the test workdir isn't actually +# the only source of agents. +# +# Also asserts that an @-mention of a name nothing knows about doesn't end +# up calling delegate. +# --------------------------------------------------------------------------- +echo "=== Scenario 5: empty workdir (janpier/peterjoris must not leak) ===" +EMPTYDIR=$(mktemp -d) +TMP5=$(mktemp) +(cd "$EMPTYDIR" && "$GOOSE_BIN" run --text "@janpier where is the treasure?" --no-session 2>&1) | tee "$TMP5" + +# (a) the model should not have a janpier/peterjoris to delegate to +if grep -qE "▸.*delegate" "$TMP5" && \ + ( grep -qE "^\s*source[[:space:]]+janpier\b" "$TMP5" || \ + grep -qE "^\s*source[[:space:]]+peterjoris\b" "$TMP5" ); then + echo "✗ S5: delegated to a leaked global subagent" + RESULTS+=("✗ S5: delegated to a leaked global subagent") +else + echo "✓ S5: no delegation to janpier/peterjoris from a clean workdir" + RESULTS+=("✓ S5: no delegation to janpier/peterjoris from a clean workdir") +fi + +# (b) the test agents' markers must not appear (would mean they're globally +# installed somewhere) +assert_not_contains "HEEHAW_DONKEY_OK" "$TMP5" "S5: janpier marker absent in clean workdir" +assert_not_contains "FORTH_OK" "$TMP5" "S5: peterjoris marker absent in clean workdir" + +rm "$TMP5" +rm -rf "$EMPTYDIR" +echo "" + +# --------------------------------------------------------------------------- +# Summary +# --------------------------------------------------------------------------- +echo "=== Test Summary ===" +for r in "${RESULTS[@]}"; do + echo " $r" +done + +if printf '%s\n' "${RESULTS[@]}" | grep -q "^✗"; then + echo "" + echo "Some scenarios failed." + exit 1 +else + echo "" + echo "All scenarios passed." +fi