mirror of
https://github.com/block/goose.git
synced 2026-10-19 09:34:12 +00:00
fix: load recipe extensions for slash commands and delegation (#12938)
This commit is contained in:
parent
9fa665262b
commit
21d873eec9
4 changed files with 168 additions and 9 deletions
|
|
@ -5,7 +5,7 @@ use crate::agents::state_machine::DELEGATED_META_KEY;
|
|||
use crate::agents::subagent_task_config::DEFAULT_SUBAGENT_MAX_TURNS;
|
||||
use crate::agents::tool_execution::ToolCallContext;
|
||||
use crate::config::paths::Paths;
|
||||
use crate::config::{Config, GooseMode};
|
||||
use crate::config::{resolve_extensions_for_new_session, Config, GooseMode};
|
||||
use crate::conversation::message::Message;
|
||||
use crate::providers;
|
||||
use crate::recipe::build_recipe::build_recipe_from_template;
|
||||
|
|
@ -477,7 +477,7 @@ impl SummonClient {
|
|||
"extensions": {
|
||||
"type": "array",
|
||||
"items": {"type": "string"},
|
||||
"description": "Extensions to enable. Omit to inherit all, empty array for none."
|
||||
"description": "Restrict extensions by name. Omit to use the source recipe's declared extensions, or inherit the parent's if none are declared. Empty array for none."
|
||||
},
|
||||
"provider": {
|
||||
"type": "string",
|
||||
|
|
@ -1162,9 +1162,12 @@ impl SummonClient {
|
|||
recipe: &Recipe,
|
||||
session: &Session,
|
||||
) -> Result<SubagentConfig> {
|
||||
let mut extensions = EnabledExtensionsState::extensions_or_default(
|
||||
Some(&session.extension_data),
|
||||
Config::global(),
|
||||
let mut extensions = resolve_extensions_for_new_session(
|
||||
recipe.extensions.as_deref(),
|
||||
Some(EnabledExtensionsState::extensions_or_default(
|
||||
Some(&session.extension_data),
|
||||
Config::global(),
|
||||
)),
|
||||
);
|
||||
|
||||
if let Some(filter) = ¶ms.extensions {
|
||||
|
|
@ -1181,7 +1184,7 @@ impl SummonClient {
|
|||
.collect();
|
||||
if !unmatched.is_empty() {
|
||||
warn!(
|
||||
"Delegate requested extensions not available in session: {:?}. Available: {:?}",
|
||||
"Delegate requested extensions not available: {:?}. Available: {:?}",
|
||||
unmatched, available_names
|
||||
);
|
||||
}
|
||||
|
|
@ -1501,6 +1504,49 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn delegate_extension_selection() -> Result<()> {
|
||||
let _env = env_lock::lock_env([
|
||||
("GOOSE_SUBAGENT_PROVIDER", Some("openai")),
|
||||
("GOOSE_SUBAGENT_MODEL", Some("test-model")),
|
||||
]);
|
||||
let client = SummonClient::new(create_test_context())?;
|
||||
let mut parent = session_with(goose_providers::model::ModelConfig::new("test-model"));
|
||||
let inherited = Recipe::from_content(
|
||||
"title: Parent\ndescription: Parent\nprompt: Run\nextensions:\n - type: platform\n name: summon\n",
|
||||
)?.extensions.unwrap();
|
||||
EnabledExtensionsState::new(inherited).to_extension_data(&mut parent.extension_data)?;
|
||||
let declared = Recipe::from_content(
|
||||
"title: Child\ndescription: Child\nprompt: Run\nextensions:\n - type: platform\n name: todo\n - type: platform\n name: extensionmanager\n",
|
||||
)?.extensions;
|
||||
for (extensions, filter, expected) in [
|
||||
(None, None, vec!["summon"]),
|
||||
(Some(vec![]), None, vec![]),
|
||||
(declared.clone(), None, vec!["todo", "extensionmanager"]),
|
||||
(declared, Some(vec!["todo".to_string()]), vec!["todo"]),
|
||||
] {
|
||||
let mut recipe = empty_recipe();
|
||||
recipe.extensions = extensions;
|
||||
let params = DelegateParams {
|
||||
extensions: filter,
|
||||
..Default::default()
|
||||
};
|
||||
let resolved = client
|
||||
.resolve_subagent_config(¶ms, &recipe, &parent)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resolved
|
||||
.extensions
|
||||
.iter()
|
||||
.map(|config| config.name())
|
||||
.collect::<Vec<_>>(),
|
||||
expected
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn delegate_persists_child_without_creating_provider() {
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use async_trait::async_trait;
|
|||
use rmcp::model::{CallToolResult, ContentBlock, ErrorCode, ErrorData, Tool};
|
||||
use tracing_futures::Instrument;
|
||||
|
||||
use crate::agents::extension_manager::ExtensionManager;
|
||||
use crate::agents::final_output_tool::{
|
||||
structured_output_unsupported_message, FinalOutputTool, FINAL_OUTPUT_CONTINUATION_MESSAGE,
|
||||
FINAL_OUTPUT_NOT_ALONE_MESSAGE, FINAL_OUTPUT_TOOL_NAME,
|
||||
|
|
@ -20,6 +21,7 @@ use crate::agents::state_machine::{
|
|||
Emitter, GooseEffect, Operation, OperationResult, SlashCommand,
|
||||
};
|
||||
use crate::agents::tool_execution::CHAT_MODE_TOOL_SKIPPED_RESPONSE;
|
||||
use crate::config::extensions::is_extension_available;
|
||||
use crate::config::GooseMode;
|
||||
use crate::conversation::message::{Message, MessageContent};
|
||||
use crate::conversation::Conversation;
|
||||
|
|
@ -81,14 +83,20 @@ pub(crate) fn recipe_prompt_parts(session: &Session) -> Result<Vec<(String, Stri
|
|||
|
||||
pub struct RecipeOperation {
|
||||
provider: Arc<dyn Provider>,
|
||||
extension_manager: Arc<ExtensionManager>,
|
||||
hook_manager: HookManager,
|
||||
response: std::sync::Mutex<Option<Message>>,
|
||||
}
|
||||
|
||||
impl RecipeOperation {
|
||||
pub fn new(provider: Arc<dyn Provider>, hook_manager: HookManager) -> Self {
|
||||
pub fn new(
|
||||
provider: Arc<dyn Provider>,
|
||||
extension_manager: Arc<ExtensionManager>,
|
||||
hook_manager: HookManager,
|
||||
) -> Self {
|
||||
Self {
|
||||
provider,
|
||||
extension_manager,
|
||||
hook_manager,
|
||||
response: std::sync::Mutex::default(),
|
||||
}
|
||||
|
|
@ -151,7 +159,7 @@ impl Operation<Session, GooseEffect> for RecipeOperation {
|
|||
async fn run_command(
|
||||
&self,
|
||||
command: &SlashCommand<'_>,
|
||||
_session: &Session,
|
||||
session: &Session,
|
||||
conversation: &Conversation,
|
||||
emit: &Emitter,
|
||||
) -> Result<OperationResult<GooseEffect>> {
|
||||
|
|
@ -174,6 +182,28 @@ impl Operation<Session, GooseEffect> for RecipeOperation {
|
|||
}
|
||||
}
|
||||
|
||||
for extension in recipe
|
||||
.extensions
|
||||
.iter()
|
||||
.flatten()
|
||||
.filter(|extension| is_extension_available(extension))
|
||||
{
|
||||
if let Err(error) = self
|
||||
.extension_manager
|
||||
.enable(&session.id, extension.clone())
|
||||
.await
|
||||
{
|
||||
return self.command_error(
|
||||
conversation,
|
||||
format!(
|
||||
"Could not load recipe extension '{}': {error}",
|
||||
extension.name()
|
||||
),
|
||||
emit,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "telemetry")]
|
||||
crate::posthog::emit_custom_slash_command_used();
|
||||
|
||||
|
|
|
|||
|
|
@ -22,6 +22,77 @@ use crate::conversation::message::MessageContent;
|
|||
use crate::recipe::build_recipe::build_recipe_from_template;
|
||||
use crate::recipe::{Recipe, Response, SubRecipe};
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_recipe_extensions_remain_available_in_the_session() -> Result<()> {
|
||||
let (pipeline, api) = test_pipeline().await?;
|
||||
pipeline.remove_extension("todo").await?;
|
||||
let recipe_path = pipeline.working_dir().join("extension-repro.yaml");
|
||||
std::fs::write(
|
||||
&recipe_path,
|
||||
r#"
|
||||
version: 1.0.0
|
||||
title: Extension repro
|
||||
description: Load Todo through a recipe command
|
||||
prompt: Run the extension recipe
|
||||
extensions:
|
||||
- type: platform
|
||||
name: todo
|
||||
"#,
|
||||
)?;
|
||||
let mappings =
|
||||
json!([{ "command": "extension-repro", "recipe_path": recipe_path }]).to_string();
|
||||
let _guard = env_lock::lock_env([("SLASH_COMMANDS", Some(mappings.as_str()))]);
|
||||
api.on("Run the extension recipe").reply("recipe complete");
|
||||
let result = pipeline.run(["/extension-repro"]).await?;
|
||||
result.assert_message(-1, Agent, "recipe complete");
|
||||
let call = api.calls().last().cloned().unwrap();
|
||||
assert!(call.advertises_tool("todo__todo_write"));
|
||||
assert!(call.advertises_tool(super::calculator_extension::ADD));
|
||||
|
||||
api.on("write a note after the recipe")
|
||||
.call("todo__todo_write", json!({ "content": "still available" }));
|
||||
api.on("Updated (").reply("note saved");
|
||||
let result = pipeline.run(["write a note after the recipe"]).await?;
|
||||
result.assert_message(-1, Agent, "note saved");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_recipe_extension_failure_is_reported_before_applying_the_recipe() -> Result<()> {
|
||||
let (pipeline, api) = test_pipeline().await?;
|
||||
let recipe_path = pipeline.working_dir().join("broken-extension.json");
|
||||
std::fs::write(
|
||||
&recipe_path,
|
||||
json!({
|
||||
"title": "Broken extension",
|
||||
"description": "Report a recipe extension startup failure",
|
||||
"prompt": "This recipe should not run",
|
||||
"extensions": [{
|
||||
"type": "stdio",
|
||||
"name": "missing-server",
|
||||
"cmd": pipeline.working_dir().join("missing-mcp-binary"),
|
||||
"args": [],
|
||||
"timeout": 1
|
||||
}]
|
||||
})
|
||||
.to_string(),
|
||||
)?;
|
||||
let mappings =
|
||||
json!([{ "command": "broken-extension", "recipe_path": recipe_path }]).to_string();
|
||||
let _guard = env_lock::lock_env([("SLASH_COMMANDS", Some(mappings.as_str()))]);
|
||||
|
||||
let result = pipeline.run(["/broken-extension"]).await?;
|
||||
|
||||
result.assert_message(
|
||||
-1,
|
||||
Agent,
|
||||
"Could not load recipe extension 'missing-server'",
|
||||
);
|
||||
assert_eq!(api.call_count(), 0);
|
||||
assert!(result.session.recipe.is_none());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn inherited_recipe_parameters_instructions_and_extensions_reach_inference() -> Result<()> {
|
||||
let (pipeline, api) = test_pipeline().await?;
|
||||
|
|
@ -117,6 +188,7 @@ async fn recipe_delegation_respects_mode_and_child_turn_limit() -> Result<()> {
|
|||
result.assert_message(-1, Agent, "delegation stayed in chat");
|
||||
|
||||
let (pipeline, api) = test_pipeline().await?;
|
||||
pipeline.remove_extension("todo").await?;
|
||||
let host = api.uri();
|
||||
let _guard = env_lock::lock_env([
|
||||
("OPENAI_API_KEY", Some("fake-openai-no-keyring")),
|
||||
|
|
@ -132,6 +204,9 @@ version: 1.0.0
|
|||
title: Bounded child
|
||||
description: Stops after one turn
|
||||
prompt: Keep taking actions
|
||||
extensions:
|
||||
- type: platform
|
||||
name: todo
|
||||
settings:
|
||||
max_turns: 1
|
||||
"#,
|
||||
|
|
@ -166,6 +241,10 @@ settings:
|
|||
|
||||
let result = pipeline.run(["Delegate the bounded child"]).await?;
|
||||
assert_eq!(api.call_count(), 3);
|
||||
let calls = api.calls();
|
||||
assert!(!calls[0].advertises_tool("todo__todo_write"));
|
||||
assert!(calls[1].advertises_tool("todo__todo_write"));
|
||||
assert!(!calls[2].advertises_tool("todo__todo_write"));
|
||||
let delivery = result
|
||||
.conversation()
|
||||
.messages()
|
||||
|
|
|
|||
|
|
@ -71,7 +71,11 @@ pub(crate) fn build_state_machine(
|
|||
hook_manager.clone(),
|
||||
Arc::clone(&extension_lease),
|
||||
)),
|
||||
Arc::new(RecipeOperation::new(provider.clone(), hook_manager.clone())),
|
||||
Arc::new(RecipeOperation::new(
|
||||
provider.clone(),
|
||||
Arc::clone(&extension_manager),
|
||||
hook_manager.clone(),
|
||||
)),
|
||||
Arc::new(ToolExecutionOperation::new(
|
||||
Arc::clone(&extension_manager),
|
||||
hook_manager.clone(),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue