Feat/summon subagent instructions - #9325
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82c1740f47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let (tools, toolshim_tools, system_prompt) = self | ||
| .prepare_tools_and_prompt(session_id, working_dir) | ||
| .await?; |
There was a problem hiding this comment.
Keep @mention routing when tool calls are disabled
Removing resolve_at_mention from reply-context setup means @agent now relies entirely on the model calling delegate, but in Goose chat mode all non-frontend tool calls are skipped (GooseMode::Chat path in agent.rs), so explicit @name requests can no longer activate the target subagent instructions. Before this commit, @name injected matching source content directly into the system prompt and worked without tool execution; with this change, the same input can silently degrade to a normal response in chat-mode sessions.
Useful? React with 👍 / 👎.
jamadeo
left a comment
There was a problem hiding this comment.
nice.
As for the extension itself, do you think it should continue to be
- called "summon"
- (en|dis)abled from the extension page?
it is not that clear to me that toggling a box called "summon" under "extensions" would affect goose's ability to use agents I've defined
| out.push_str(&format!( | ||
| "\n• {} — {}", | ||
| s.name, | ||
| truncate(&s.description, 200) |
There was a problem hiding this comment.
200 here seems a tad arbitrary? though I'm not able to find a better limit
There was a problem hiding this comment.
yeah good point. also it is using a locally defined truncate instead of the utility we have. introduced two constants to do it instead
I think we should probably always enable it unless there is a setting like DISSABLE_SUBAGENTS. or maybe we should have some sort of features thing? |
This is my main feedback too. I think we should make one extension which does sub-agents & agents and just call it "sub-agents" |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b80d597909
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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); |
There was a problem hiding this comment.
Refresh summon instructions when source files change
Build the summon instruction text lazily or refresh it per turn instead of freezing it at client construction. with_instructions(...) is populated once from a startup scan, so if a user adds/renames an agent or recipe during the same session, the model keeps an outdated name list and routing guidance and can miss explicit mentions (for example a newly created @agent) until the session/extension is recreated.
Useful? React with 👍 / 👎.
| return String::new(); | ||
| }; | ||
|
|
||
| let sources = discover_filesystem_sources(&session.working_dir); |
There was a problem hiding this comment.
Include recipe subrecipes in generated summon instructions
Populate instruction-time source discovery from the same source set used by runtime delegation (get_sources) or explicitly merge session.recipe.sub_recipes. As written, instruction generation only calls discover_filesystem_sources(...), which does not add SourceType::Subrecipe, so recipe-defined subrecipes are omitted from the advertised names/descriptions and the model loses the intended name/description-based routing hints for those valid delegate targets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb84eec559
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run_goose() { | ||
| local prompt="$1" | ||
| local outfile="$2" | ||
| (cd "$TESTDIR" && "$GOOSE_BIN" run --text "$prompt" --no-session 2>&1) | tee "$outfile" |
There was a problem hiding this comment.
Preserve goose exit status in transcript pipeline
The run_goose helper pipes Goose output through tee, but the script enables only set -e and not pipefail, so a failing Goose invocation can still return success because the pipeline status comes from tee. In failure cases (provider errors, CLI crashes, auth failures), scenarios may be evaluated against partial/error transcripts instead of stopping immediately, which makes this smoke test unreliable as a validator.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b46d31d95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut seen: std::collections::HashSet<String> = | ||
| sources.iter().map(|s| s.name.clone()).collect(); | ||
| for sr in subs { | ||
| if !seen.insert(sr.name.clone()) { | ||
| continue; |
There was a problem hiding this comment.
Align instruction precedence with delegate source resolution
When a filesystem source and a recipe subrecipe share the same name, build_subagent_instructions pre-populates seen from filesystem sources and drops the subrecipe entry, but runtime resolution in get_sources gives subrecipes precedence by adding them first. This creates a mismatch where the model is shown one source description but delegate(source: "name") executes a different source, which can route work to unintended instructions for name-collision cases.
Useful? React with 👍 / 👎.
| else | ||
| echo "✗ $scenario: did NOT delegate to $source" | ||
| RESULTS+=("✗ $scenario") | ||
| return 1 |
There was a problem hiding this comment.
Prevent errexit from stopping the smoke test after one miss
With set -e enabled, assert_delegated_to returning 1 causes the script to exit immediately when an assertion fails (e.g., in Scenario 1 or 2), so later scenarios and the final consolidated summary never run. That defeats this script’s own result-collection design and makes failures less diagnosable because you only see the first miss.
Useful? React with 👍 / 👎.
* main: (70 commits) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) Add Linux musl CLI builds (#9240) feat(acp): paginate session list (#9199) docs: reorganize (#9310) Structured per-provider config block, non-destructive provider switching (#8977) feat(cli): add `goose review` local code review command (#9114) feat(tui): diff viewer (#9260) fix(otel): emit trace_output as span attribute instead of event (#9255) docs: add guide for connecting goose Desktop to a remote goosed server (#9275) fix(config): check file fallback when keyring has no entry (#9279) fix(desktop): ScheduleModal error message styling (#9278) fix(ui): align sidebar hamburger in macOS fullscreen (#9257) ...
* main: (38 commits) [Prompt injection mitigation] Update pattern-based detection to reduce FPs (#9350) feat: add Harbor eval runner (#9138) chore(release): bump version to 1.35.0 (minor) (#9150) Include request URL in provider error messages (#9232) fix(databricks): ensure parallel tool image responses don't interleave tool results (#9241) Surface resolved Databricks model metadata (#9206) Add unified thinking effort control across all providers (#9242) Add Linux desktop Vulkan packages (#9323) chore: update canonical model registry (#9331) feat: slash commands (built-in, skill, recipe) in acp server (#9238) feat: add /goal command for agent self-evaluation before finishing (#9069) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) ...
Co-authored-by: Douwe Osinga <douwe@squareup.com>
Co-authored-by: Douwe Osinga <douwe@squareup.com>
Summary
Make the summon extension handle the @agent stuff. will also call now if you just mention the name or if it just seems fitting. So "hey @pirate, where is the treasure" should work now.