From 85b32bd9f0dc8bd604010440d007bf713170f715 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Fri, 17 Oct 2025 17:32:03 -0400 Subject: [PATCH] Revert "Rewrite extension management tools (#5057)" This reverts commit bccfa0156c41ade75d404ae0c3587389541b16fb. --- crates/goose-cli/src/commands/configure.rs | 7 + crates/goose-cli/src/session/builder.rs | 2 - crates/goose/src/agents/agent.rs | 196 ++++++- crates/goose/src/agents/extension.rs | 25 +- crates/goose/src/agents/extension_manager.rs | 6 +- .../src/agents/extension_manager_extension.rs | 546 ------------------ crates/goose/src/agents/mod.rs | 1 - crates/goose/src/agents/platform_tools.rs | 108 ++++ crates/goose/src/agents/router_tools.rs | 14 +- crates/goose/src/agents/tool_route_manager.rs | 9 +- .../src/agents/tool_router_index_manager.rs | 28 + crates/goose/src/execution/manager.rs | 2 - .../src/permission/permission_inspector.rs | 6 +- .../goose/src/permission/permission_judge.rs | 6 +- crates/goose/src/prompts/system.md | 9 +- crates/goose/src/prompts/system_gpt_4.1.md | 4 +- crates/goose/tests/agent.rs | 269 --------- .../settings/extensions/agent-api.test.ts | 25 - .../settings/extensions/agent-api.ts | 15 +- 19 files changed, 353 insertions(+), 925 deletions(-) delete mode 100644 crates/goose/src/agents/extension_manager_extension.rs diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index 2af5bde6bb79..692a998d231a 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -3,6 +3,9 @@ use cliclack::spinner; use console::style; use goose::agents::extension::ToolInfo; use goose::agents::extension_manager::get_parameter_names; +use goose::agents::platform_tools::{ + PLATFORM_LIST_RESOURCES_TOOL_NAME, PLATFORM_READ_RESOURCE_TOOL_NAME, +}; use goose::agents::Agent; use goose::agents::{extension::Envs, ExtensionConfig}; use goose::config::declarative_providers::{create_custom_provider, remove_custom_provider}; @@ -1446,6 +1449,10 @@ pub async fn configure_tool_permissions_dialog() -> Result<(), Box> { .list_tools(Some(selected_extension_name.clone())) .await .into_iter() + .filter(|tool| { + tool.name != PLATFORM_LIST_RESOURCES_TOOL_NAME + && tool.name != PLATFORM_READ_RESOURCE_TOOL_NAME + }) .map(|tool| { ToolInfo::new( &tool.name, diff --git a/crates/goose-cli/src/session/builder.rs b/crates/goose-cli/src/session/builder.rs index c078990f8523..ace3c24df65c 100644 --- a/crates/goose-cli/src/session/builder.rs +++ b/crates/goose-cli/src/session/builder.rs @@ -324,8 +324,6 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> CliSession { .extension_manager .set_context(PlatformExtensionContext { session_id: session_id.clone(), - extension_manager: Some(Arc::downgrade(&agent.extension_manager)), - tool_route_manager: Some(Arc::downgrade(&agent.tool_route_manager)), }) .await; diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 8598536744f9..a9dcc2cb0657 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -10,9 +10,12 @@ use uuid::Uuid; use crate::agents::extension::{ExtensionConfig, ExtensionError, ExtensionResult, ToolInfo}; use crate::agents::extension_manager::{get_parameter_names, ExtensionManager}; -use crate::agents::extension_manager_extension::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; use crate::agents::final_output_tool::{FINAL_OUTPUT_CONTINUATION_MESSAGE, FINAL_OUTPUT_TOOL_NAME}; -use crate::agents::platform_tools::PLATFORM_MANAGE_SCHEDULE_TOOL_NAME; +use crate::agents::platform_tools::{ + PLATFORM_LIST_RESOURCES_TOOL_NAME, PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME, + PLATFORM_MANAGE_SCHEDULE_TOOL_NAME, PLATFORM_READ_RESOURCE_TOOL_NAME, + PLATFORM_SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME, +}; use crate::agents::prompt_manager::PromptManager; use crate::agents::recipe_tools::dynamic_task_tools::{ create_dynamic_task, create_dynamic_task_tool, DYNAMIC_TASK_TOOL_NAME_PREFIX, @@ -29,7 +32,7 @@ use crate::agents::tool_route_manager::ToolRouteManager; use crate::agents::tool_router_index_manager::ToolRouterIndexManager; use crate::agents::types::SessionConfig; use crate::agents::types::{FrontendTool, ToolResultReceiver}; -use crate::config::{get_enabled_extensions, Config}; +use crate::config::{get_enabled_extensions, get_extension_by_name, Config}; use crate::context_mgmt::DEFAULT_COMPACTION_THRESHOLD; use crate::conversation::{debug_conversation_fix, fix_conversation, Conversation}; use crate::mcp_utils::ToolResult; @@ -85,7 +88,7 @@ pub struct ToolCategorizeResult { /// The main goose Agent pub struct Agent { pub(super) provider: Mutex>>, - pub extension_manager: Arc, + pub extension_manager: ExtensionManager, pub(super) sub_recipe_manager: Mutex, pub(super) tasks_manager: TasksManager, pub(super) final_output_tool: Arc>>, @@ -97,7 +100,7 @@ pub struct Agent { pub(super) tool_result_tx: mpsc::Sender<(String, ToolResult>)>, pub(super) tool_result_rx: ToolResultReceiver, - pub tool_route_manager: Arc, + pub(super) tool_route_manager: ToolRouteManager, pub(super) scheduler_service: Mutex>>, pub(super) retry_manager: RetryManager, pub(super) tool_inspection_manager: ToolInspectionManager, @@ -160,7 +163,7 @@ impl Agent { Self { provider: Mutex::new(None), - extension_manager: Arc::new(ExtensionManager::new()), + extension_manager: ExtensionManager::new(), sub_recipe_manager: Mutex::new(SubRecipeManager::new()), tasks_manager: TasksManager::new(), final_output_tool: Arc::new(Mutex::new(None)), @@ -171,7 +174,7 @@ impl Agent { confirmation_rx: Mutex::new(confirm_rx), tool_result_tx: tool_tx, tool_result_rx: Arc::new(Mutex::new(tool_rx)), - tool_route_manager: Arc::new(ToolRouteManager::new()), + tool_route_manager: ToolRouteManager::new(), scheduler_service: Mutex::new(None), retry_manager: RetryManager::new(), tool_inspection_manager: Self::create_default_tool_inspection_manager(), @@ -401,6 +404,28 @@ impl Agent { return (request_id, Ok(ToolCallResult::from(result))); } + if tool_call.name == PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME { + let extension_name = tool_call + .arguments + .as_ref() + .and_then(|args| args.get("extension_name")) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let action = tool_call + .arguments + .as_ref() + .and_then(|args| args.get("action")) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let (request_id, result) = self + .manage_extensions(action, extension_name, request_id) + .await; + + return (request_id, Ok(ToolCallResult::from(result))); + } + if tool_call.name == FINAL_OUTPUT_TOOL_NAME { return if let Some(final_output_tool) = self.final_output_tool.lock().await.as_mut() { let result = final_output_tool.execute_tool_call(tool_call.clone()).await; @@ -463,12 +488,12 @@ impl Agent { let parent_session_id = session.id.to_string(); let parent_working_dir = session.working_dir.clone(); - // Get extensions from the agent's runtime state rather than global config - // This ensures subagents inherit extensions that were dynamically enabled by the parent - let extensions = self.get_extension_configs().await; - - let task_config = - TaskConfig::new(provider, parent_session_id, parent_working_dir, extensions); + let task_config = TaskConfig::new( + provider, + parent_session_id, + parent_working_dir, + get_enabled_extensions(), + ); let arguments = match tool_call.arguments.clone() { Some(args) => Value::Object(args), @@ -535,6 +560,31 @@ impl Agent { .map(Value::Object) .unwrap_or(Value::Object(serde_json::Map::new())); create_dynamic_task(arguments, &self.tasks_manager, loaded_extensions).await + } else if tool_call.name == PLATFORM_READ_RESOURCE_TOOL_NAME { + // Check if the tool is read_resource and handle it separately + let arguments = tool_call + .arguments + .clone() + .map(Value::Object) + .unwrap_or(Value::Object(serde_json::Map::new())); + ToolCallResult::from( + self.extension_manager + .read_resource(arguments, cancellation_token.unwrap_or_default()) + .await, + ) + } else if tool_call.name == PLATFORM_LIST_RESOURCES_TOOL_NAME { + let arguments = tool_call + .arguments + .clone() + .map(Value::Object) + .unwrap_or(Value::Object(serde_json::Map::new())); + ToolCallResult::from( + self.extension_manager + .list_resources(arguments, cancellation_token.unwrap_or_default()) + .await, + ) + } else if tool_call.name == PLATFORM_SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME { + ToolCallResult::from(self.extension_manager.search_available_extensions().await) } else if self.is_frontend_tool(&tool_call.name).await { // For frontend tools, return an error indicating we need frontend execution ToolCallResult::from(Err(ErrorData::new( @@ -603,6 +653,109 @@ impl Agent { Ok(()) } + #[allow(clippy::too_many_lines)] + pub(super) async fn manage_extensions( + &self, + action: String, + extension_name: String, + request_id: String, + ) -> (String, Result, ErrorData>) { + if self.tool_route_manager.is_router_functional().await { + let selector = self.tool_route_manager.get_router_tool_selector().await; + if let Some(selector) = selector { + let selector_action = if action == "disable" { "remove" } else { "add" }; + let selector = Arc::new(selector); + if let Err(e) = ToolRouterIndexManager::update_extension_tools( + &selector, + &self.extension_manager, + &extension_name, + selector_action, + ) + .await + { + return ( + request_id, + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to update LLM index: {}", e), + None, + )), + ); + } + } + } + if action == "disable" { + let result = self + .extension_manager + .remove_extension(&extension_name) + .await + .map(|_| { + vec![Content::text(format!( + "The extension '{}' has been disabled successfully", + extension_name + ))] + }) + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)); + return (request_id, result); + } + + let config = match get_extension_by_name(&extension_name) { + Some(config) => config, + None => { + return ( + request_id, + Err(ErrorData::new( + ErrorCode::RESOURCE_NOT_FOUND, + format!( + "Extension '{}' not found. Please check the extension name and try again.", + extension_name + ), + None, + )), + ) + } + }; + let result = self + .extension_manager + .add_extension(config) + .await + .map(|_| { + vec![Content::text(format!( + "The extension '{}' has been installed successfully", + extension_name + ))] + }) + .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)); + + // Update LLM index if operation was successful and LLM routing is functional + if result.is_ok() && self.tool_route_manager.is_router_functional().await { + let selector = self.tool_route_manager.get_router_tool_selector().await; + if let Some(selector) = selector { + let llm_action = if action == "disable" { "remove" } else { "add" }; + let selector = Arc::new(selector); + if let Err(e) = ToolRouterIndexManager::update_extension_tools( + &selector, + &self.extension_manager, + &extension_name, + llm_action, + ) + .await + { + return ( + request_id, + Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to update LLM index: {}", e), + None, + )), + ); + } + } + } + + (request_id, result) + } + pub async fn add_extension(&self, extension: ExtensionConfig) -> ExtensionResult<()> { match &extension { ExtensionConfig::Frontend { @@ -671,10 +824,21 @@ impl Agent { if extension_name.is_none() || extension_name.as_deref() == Some("platform") { // Add platform tools - // TODO: migrate the manage schedule tool as well - prefixed_tools.extend([platform_tools::manage_schedule_tool()]); + prefixed_tools.extend([ + platform_tools::search_available_extensions_tool(), + platform_tools::manage_extensions_tool(), + platform_tools::manage_schedule_tool(), + ]); // Dynamic task tool prefixed_tools.push(create_dynamic_task_tool()); + + // Add resource tools if supported + if self.extension_manager.supports_resources().await { + prefixed_tools.extend([ + platform_tools::read_resource_tool(), + platform_tools::list_resources_tool(), + ]); + } } if extension_name.is_none() { @@ -1051,7 +1215,7 @@ impl Agent { let mut enable_extension_request_ids = vec![]; for request in &remaining_requests { if let Ok(tool_call) = &request.tool_call { - if tool_call.name == MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE { + if tool_call.name == PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME { enable_extension_request_ids.push(request.id.clone()); } } diff --git a/crates/goose/src/agents/extension.rs b/crates/goose/src/agents/extension.rs index ab41a7cdea0d..0af92592c464 100644 --- a/crates/goose/src/agents/extension.rs +++ b/crates/goose/src/agents/extension.rs @@ -1,4 +1,3 @@ -use crate::agents::extension_manager_extension; use crate::agents::todo_extension; use std::collections::HashMap; @@ -35,8 +34,8 @@ impl ProcessExit { } } -pub static PLATFORM_EXTENSIONS: Lazy> = Lazy::new( - || { +pub static PLATFORM_EXTENSIONS: Lazy> = + Lazy::new(|| { let mut map = HashMap::new(); map.insert( @@ -50,28 +49,12 @@ pub static PLATFORM_EXTENSIONS: Lazy }, ); - map.insert( - "extensionmanager", - PlatformExtensionDef { - name: extension_manager_extension::EXTENSION_NAME, - description: - "Enable extension management tools for discovering, enabling, and disabling extensions", - default_enabled: true, - client_factory: |ctx| Box::new(extension_manager_extension::ExtensionManagerClient::new(ctx).unwrap()), - }, - ); - map - }, -); + }); -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct PlatformExtensionContext { pub session_id: Option, - pub extension_manager: - Option>, - pub tool_route_manager: - Option>, } #[derive(Debug, Clone)] diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 06dde1d3c61c..c5b29f124975 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -238,11 +238,7 @@ impl ExtensionManager { pub fn new() -> Self { Self { extensions: Mutex::new(HashMap::new()), - context: Mutex::new(PlatformExtensionContext { - session_id: None, - extension_manager: None, - tool_route_manager: None, - }), + context: Mutex::new(PlatformExtensionContext { session_id: None }), } } diff --git a/crates/goose/src/agents/extension_manager_extension.rs b/crates/goose/src/agents/extension_manager_extension.rs deleted file mode 100644 index 099ec70b6f57..000000000000 --- a/crates/goose/src/agents/extension_manager_extension.rs +++ /dev/null @@ -1,546 +0,0 @@ -use crate::agents::extension::PlatformExtensionContext; -use crate::agents::mcp_client::{Error, McpClientTrait}; -use crate::agents::tool_router_index_manager::ToolRouterIndexManager; -use crate::config::get_extension_by_name; -use anyhow::Result; -use async_trait::async_trait; -use indoc::indoc; -use rmcp::model::{ - CallToolResult, Content, ErrorCode, ErrorData, GetPromptResult, Implementation, - InitializeResult, JsonObject, ListPromptsResult, ListResourcesResult, ListToolsResult, - ProtocolVersion, ReadResourceResult, ServerCapabilities, ServerNotification, Tool, - ToolAnnotations, ToolsCapability, -}; -use schemars::{schema_for, JsonSchema}; -use serde::{Deserialize, Serialize}; -use serde_json::Value; -use std::sync::Arc; -use tokio::sync::mpsc; -use tokio_util::sync::CancellationToken; -use tracing::error; - -pub static EXTENSION_NAME: &str = "Extension Manager"; -// pub static DISPLAY_NAME: &str = "Extension Manager"; - -#[derive(Debug, thiserror::Error)] -pub enum ExtensionManagerToolError { - #[error("Unknown tool: {tool_name}")] - UnknownTool { tool_name: String }, - - #[error("Extension manager not available")] - ManagerUnavailable, - - #[error("Missing required parameter: {param_name}")] - MissingParameter { param_name: String }, - - #[error("Invalid action: {action}. Must be 'enable' or 'disable'")] - InvalidAction { action: String }, - - #[error("Extension operation failed: {message}")] - OperationFailed { message: String }, - - #[error("Failed to deserialize parameters: {0}")] - DeserializationError(#[from] serde_json::Error), -} - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "lowercase")] -pub enum ManageExtensionAction { - Enable, - Disable, -} - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct ManageExtensionsParams { - pub action: ManageExtensionAction, - pub extension_name: String, -} - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct ReadResourceParams { - pub uri: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub extension_name: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct ListResourcesParams { - #[serde(skip_serializing_if = "Option::is_none")] - pub extension_name: Option, -} - -pub const READ_RESOURCE_TOOL_NAME: &str = "read_resource"; -pub const LIST_RESOURCES_TOOL_NAME: &str = "list_resources"; -pub const SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME: &str = "search_available_extensions"; -pub const MANAGE_EXTENSIONS_TOOL_NAME: &str = "manage_extensions"; -pub const MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE: &str = "extensionmanager__manage_extensions"; - -pub struct ExtensionManagerClient { - info: InitializeResult, - #[allow(dead_code)] - context: PlatformExtensionContext, -} - -impl ExtensionManagerClient { - pub fn new(context: PlatformExtensionContext) -> Result { - let info = InitializeResult { - protocol_version: ProtocolVersion::V_2025_03_26, - capabilities: ServerCapabilities { - tools: Some(ToolsCapability { - list_changed: Some(false), - }), - resources: None, - prompts: None, - completions: None, - experimental: None, - logging: None, - }, - server_info: Implementation { - name: EXTENSION_NAME.to_string(), - title: Some(EXTENSION_NAME.to_string()), - version: "1.0.0".to_string(), - icons: None, - website_url: None, - }, - instructions: Some(indoc! {r#" - Extension Management - - Use these tools to discover, enable, and disable extensions, as well as review resources. - - Available tools: - - search_available_extensions: Find extensions available to enable/disable - - manage_extensions: Enable or disable extensions - - list_resources: List resources from extensions - - read_resource: Read specific resources from extensions - - Use search_available_extensions when you need to find what extensions are available. - Use manage_extensions to enable or disable specific extensions by name. - Use list_resources and read_resource to work with extension data and resources. - "#}.to_string()), - }; - - Ok(Self { info, context }) - } - - async fn handle_search_available_extensions( - &self, - ) -> Result, ExtensionManagerToolError> { - if let Some(weak_ref) = &self.context.extension_manager { - if let Some(extension_manager) = weak_ref.upgrade() { - match extension_manager.search_available_extensions().await { - Ok(content) => Ok(content), - Err(e) => Err(ExtensionManagerToolError::OperationFailed { - message: format!("Failed to search available extensions: {}", e.message), - }), - } - } else { - Err(ExtensionManagerToolError::ManagerUnavailable) - } - } else { - Err(ExtensionManagerToolError::ManagerUnavailable) - } - } - - async fn handle_manage_extensions( - &self, - arguments: Option, - ) -> Result, ExtensionManagerToolError> { - let arguments = arguments.ok_or(ExtensionManagerToolError::MissingParameter { - param_name: "arguments".to_string(), - })?; - - let params: ManageExtensionsParams = - serde_json::from_value(serde_json::Value::Object(arguments))?; - - match self - .manage_extensions_impl(params.action, params.extension_name) - .await - { - Ok(content) => Ok(content), - Err(error_data) => Err(ExtensionManagerToolError::OperationFailed { - message: error_data.message.to_string(), - }), - } - } - - #[allow(clippy::too_many_lines)] - async fn manage_extensions_impl( - &self, - action: ManageExtensionAction, - extension_name: String, - ) -> Result, ErrorData> { - let extension_manager = self - .context - .extension_manager - .as_ref() - .and_then(|weak| weak.upgrade()) - .ok_or_else(|| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - "Extension manager is no longer available".to_string(), - None, - ) - })?; - - let tool_route_manager = self - .context - .tool_route_manager - .as_ref() - .and_then(|weak| weak.upgrade()); - - // Update tool router index if router is functional - if let Some(tool_route_manager) = &tool_route_manager { - if tool_route_manager.is_router_functional().await { - let selector = tool_route_manager.get_router_tool_selector().await; - if let Some(selector) = selector { - let selector_action = if action == ManageExtensionAction::Disable { - "remove" - } else { - "add" - }; - let selector = Arc::new(selector); - if let Err(e) = ToolRouterIndexManager::update_extension_tools( - &selector, - &extension_manager, - &extension_name, - selector_action, - ) - .await - { - return Err(ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to update LLM index: {}", e), - None, - )); - } - } - } - } - - if action == ManageExtensionAction::Disable { - let result = extension_manager - .remove_extension(&extension_name) - .await - .map(|_| { - vec![Content::text(format!( - "The extension '{}' has been disabled successfully", - extension_name - ))] - }) - .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)); - return result; - } - - let config = match get_extension_by_name(&extension_name) { - Some(config) => config, - None => { - return Err(ErrorData::new( - ErrorCode::RESOURCE_NOT_FOUND, - format!( - "Extension '{}' not found. Please check the extension name and try again.", - extension_name - ), - None, - )); - } - }; - - let result = extension_manager - .add_extension(config) - .await - .map(|_| { - vec![Content::text(format!( - "The extension '{}' has been installed successfully", - extension_name - ))] - }) - .map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None)); - - // Update LLM index if operation was successful and LLM routing is functional - if result.is_ok() { - if let Some(tool_route_manager) = &tool_route_manager { - if tool_route_manager.is_router_functional().await { - let selector = tool_route_manager.get_router_tool_selector().await; - if let Some(selector) = selector { - let llm_action = if action == ManageExtensionAction::Disable { - "remove" - } else { - "add" - }; - let selector = Arc::new(selector); - if let Err(e) = ToolRouterIndexManager::update_extension_tools( - &selector, - &extension_manager, - &extension_name, - llm_action, - ) - .await - { - return Err(ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to update LLM index: {}", e), - None, - )); - } - } - } - } - } - - result - } - - async fn handle_list_resources( - &self, - arguments: Option, - ) -> Result, ExtensionManagerToolError> { - if let Some(weak_ref) = &self.context.extension_manager { - if let Some(extension_manager) = weak_ref.upgrade() { - let params = arguments - .map(serde_json::Value::Object) - .unwrap_or(serde_json::Value::Object(serde_json::Map::new())); - - match extension_manager - .list_resources(params, tokio_util::sync::CancellationToken::default()) - .await - { - Ok(content) => Ok(content), - Err(e) => Err(ExtensionManagerToolError::OperationFailed { - message: format!("Failed to list resources: {}", e.message), - }), - } - } else { - Err(ExtensionManagerToolError::ManagerUnavailable) - } - } else { - Err(ExtensionManagerToolError::ManagerUnavailable) - } - } - - async fn handle_read_resource( - &self, - arguments: Option, - ) -> Result, ExtensionManagerToolError> { - if let Some(weak_ref) = &self.context.extension_manager { - if let Some(extension_manager) = weak_ref.upgrade() { - let params = arguments - .map(serde_json::Value::Object) - .unwrap_or(serde_json::Value::Object(serde_json::Map::new())); - - match extension_manager - .read_resource(params, tokio_util::sync::CancellationToken::default()) - .await - { - Ok(content) => Ok(content), - Err(e) => Err(ExtensionManagerToolError::OperationFailed { - message: format!("Failed to read resource: {}", e.message), - }), - } - } else { - Err(ExtensionManagerToolError::ManagerUnavailable) - } - } else { - Err(ExtensionManagerToolError::ManagerUnavailable) - } - } - - #[allow(clippy::too_many_lines)] - async fn get_tools(&self) -> Vec { - let mut tools = vec![ - Tool::new( - SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME.to_string(), - "Searches for additional extensions available to help complete tasks. - Use this tool when you're unable to find a specific feature or functionality you need to complete your task, or when standard approaches aren't working. - These extensions might provide the exact tools needed to solve your problem. - If you find a relevant one, consider using your tools to enable it.".to_string(), - Arc::new( - serde_json::json!({ - "type": "object", - "required": [], - "properties": {} - }) - .as_object() - .expect("Schema must be an object") - .clone() - ), - ).annotate(ToolAnnotations { - title: Some("Discover extensions".to_string()), - read_only_hint: Some(true), - destructive_hint: Some(false), - idempotent_hint: Some(false), - open_world_hint: Some(false), - }), - Tool::new( - MANAGE_EXTENSIONS_TOOL_NAME.to_string(), - "Tool to manage extensions and tools in goose context. - Enable or disable extensions to help complete tasks. - Enable or disable an extension by providing the extension name. - ".to_string(), - Arc::new( - serde_json::to_value(schema_for!(ManageExtensionsParams)) - .expect("Failed to serialize schema") - .as_object() - .expect("Schema must be an object") - .clone() - ), - ).annotate(ToolAnnotations { - title: Some("Enable or disable an extension".to_string()), - read_only_hint: Some(false), - destructive_hint: Some(false), - idempotent_hint: Some(false), - open_world_hint: Some(false), - }), - ]; - - // Only add resource tools if extension manager supports resources - if let Some(weak_ref) = &self.context.extension_manager { - if let Some(extension_manager) = weak_ref.upgrade() { - if extension_manager.supports_resources().await { - tools.extend([ - Tool::new( - LIST_RESOURCES_TOOL_NAME.to_string(), - indoc! {r#" - List resources from an extension(s). - - Resources allow extensions to share data that provide context to LLMs, such as - files, database schemas, or application-specific information. This tool lists resources - in the provided extension, and returns a list for the user to browse. If no extension - is provided, the tool will search all extensions for the resource. - "#}.to_string(), - Arc::new( - serde_json::to_value(schema_for!(ListResourcesParams)) - .expect("Failed to serialize schema") - .as_object() - .expect("Schema must be an object") - .clone() - ), - ).annotate(ToolAnnotations { - title: Some("List resources".to_string()), - read_only_hint: Some(true), - destructive_hint: Some(false), - idempotent_hint: Some(false), - open_world_hint: Some(false), - }), - Tool::new( - READ_RESOURCE_TOOL_NAME.to_string(), - indoc! {r#" - Read a resource from an extension. - - Resources allow extensions to share data that provide context to LLMs, such as - files, database schemas, or application-specific information. This tool searches for the - resource URI in the provided extension, and reads in the resource content. If no extension - is provided, the tool will search all extensions for the resource. - "#}.to_string(), - Arc::new( - serde_json::to_value(schema_for!(ReadResourceParams)) - .expect("Failed to serialize schema") - .as_object() - .expect("Schema must be an object") - .clone() - ), - ).annotate(ToolAnnotations { - title: Some("Read a resource".to_string()), - read_only_hint: Some(true), - destructive_hint: Some(false), - idempotent_hint: Some(false), - open_world_hint: Some(false), - }), - ]); - } - } - } - - tools - } -} - -#[async_trait] -impl McpClientTrait for ExtensionManagerClient { - async fn list_resources( - &self, - _next_cursor: Option, - _cancellation_token: CancellationToken, - ) -> Result { - Err(Error::TransportClosed) - } - - async fn read_resource( - &self, - _uri: &str, - _cancellation_token: CancellationToken, - ) -> Result { - // Extension manager doesn't expose resources directly - Err(Error::TransportClosed) - } - - async fn list_tools( - &self, - _next_cursor: Option, - _cancellation_token: CancellationToken, - ) -> Result { - Ok(ListToolsResult { - tools: self.get_tools().await, - next_cursor: None, - }) - } - - async fn call_tool( - &self, - name: &str, - arguments: Option, - _cancellation_token: CancellationToken, - ) -> Result { - let result = match name { - SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME => { - self.handle_search_available_extensions().await - } - MANAGE_EXTENSIONS_TOOL_NAME => self.handle_manage_extensions(arguments).await, - LIST_RESOURCES_TOOL_NAME => self.handle_list_resources(arguments).await, - READ_RESOURCE_TOOL_NAME => self.handle_read_resource(arguments).await, - _ => Err(ExtensionManagerToolError::UnknownTool { - tool_name: name.to_string(), - }), - }; - - match result { - Ok(content) => Ok(CallToolResult::success(content)), - Err(error) => { - // Log the error for debugging - error!("Extension manager tool '{}' failed: {}", name, error); - - // Return proper error result with is_error flag set - Ok(CallToolResult { - content: vec![Content::text(error.to_string())], - is_error: Some(true), // ✅ Properly mark as error - structured_content: None, - meta: None, - }) - } - } - } - - async fn list_prompts( - &self, - _next_cursor: Option, - _cancellation_token: CancellationToken, - ) -> Result { - Err(Error::TransportClosed) - } - - async fn get_prompt( - &self, - _name: &str, - _arguments: Value, - _cancellation_token: CancellationToken, - ) -> Result { - Err(Error::TransportClosed) - } - - async fn subscribe(&self) -> mpsc::Receiver { - mpsc::channel(1).1 - } - - fn get_info(&self) -> Option<&InitializeResult> { - Some(&self.info) - } -} diff --git a/crates/goose/src/agents/mod.rs b/crates/goose/src/agents/mod.rs index 7876510d58bb..23dabbb8b095 100644 --- a/crates/goose/src/agents/mod.rs +++ b/crates/goose/src/agents/mod.rs @@ -2,7 +2,6 @@ mod agent; pub mod extension; pub mod extension_malware_check; pub mod extension_manager; -pub mod extension_manager_extension; pub mod final_output_tool; mod large_response_handler; pub mod mcp_client; diff --git a/crates/goose/src/agents/platform_tools.rs b/crates/goose/src/agents/platform_tools.rs index 57bde5eaeb3e..bdebac8ed9ac 100644 --- a/crates/goose/src/agents/platform_tools.rs +++ b/crates/goose/src/agents/platform_tools.rs @@ -1,8 +1,116 @@ use indoc::indoc; use rmcp::model::{Tool, ToolAnnotations}; use rmcp::object; + +pub const PLATFORM_READ_RESOURCE_TOOL_NAME: &str = "platform__read_resource"; +pub const PLATFORM_LIST_RESOURCES_TOOL_NAME: &str = "platform__list_resources"; +pub const PLATFORM_SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME: &str = + "platform__search_available_extensions"; +pub const PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME: &str = "platform__manage_extensions"; pub const PLATFORM_MANAGE_SCHEDULE_TOOL_NAME: &str = "platform__manage_schedule"; +pub fn read_resource_tool() -> Tool { + Tool::new( + PLATFORM_READ_RESOURCE_TOOL_NAME.to_string(), + indoc! {r#" + Read a resource from an extension. + + Resources allow extensions to share data that provide context to LLMs, such as + files, database schemas, or application-specific information. This tool searches for the + resource URI in the provided extension, and reads in the resource content. If no extension + is provided, the tool will search all extensions for the resource. + "#}.to_string(), + object!({ + "type": "object", + "required": ["uri"], + "properties": { + "uri": {"type": "string", "description": "Resource URI"}, + "extension_name": {"type": "string", "description": "Optional extension name"} + } + }) + ).annotate(ToolAnnotations { + title: Some("Read a resource".to_string()), + read_only_hint: Some(true), + destructive_hint: Some(false), + idempotent_hint: Some(false), + open_world_hint: Some(false), + }) +} + +pub fn list_resources_tool() -> Tool { + Tool::new( + PLATFORM_LIST_RESOURCES_TOOL_NAME.to_string(), + indoc! {r#" + List resources from an extension(s). + + Resources allow extensions to share data that provide context to LLMs, such as + files, database schemas, or application-specific information. This tool lists resources + in the provided extension, and returns a list for the user to browse. If no extension + is provided, the tool will search all extensions for the resource. + "#} + .to_string(), + object!({ + "type": "object", + "properties": { + "extension_name": {"type": "string", "description": "Optional extension name"} + } + }), + ) + .annotate(ToolAnnotations { + title: Some("List resources".to_string()), + read_only_hint: Some(true), + destructive_hint: Some(false), + idempotent_hint: Some(false), + open_world_hint: Some(false), + }) +} + +pub fn search_available_extensions_tool() -> Tool { + Tool::new( + PLATFORM_SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME.to_string(), + "Searches for additional extensions available to help complete tasks. + Use this tool when you're unable to find a specific feature or functionality you need to complete your task, or when standard approaches aren't working. + These extensions might provide the exact tools needed to solve your problem. + If you find a relevant one, consider using your tools to enable it.".to_string(), + object!({ + "type": "object", + "required": [], + "properties": {} + }) + ).annotate(ToolAnnotations { + title: Some("Discover extensions".to_string()), + read_only_hint: Some(true), + destructive_hint: Some(false), + idempotent_hint: Some(false), + open_world_hint: Some(false), + }) +} + +pub fn manage_extensions_tool() -> Tool { + Tool::new( + PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME.to_string(), + "Tool to manage extensions and tools in goose context. + Enable or disable extensions to help complete tasks. + Enable or disable an extension by providing the extension name. + " + .to_string(), + object!({ + "type": "object", + "required": ["action", "extension_name"], + "properties": { + "action": {"type": "string", "description": "The action to perform", "enum": ["enable", "disable"]}, + "extension_name": {"type": "string", "description": "The name of the extension to enable"} + } + }), + ).annotate(ToolAnnotations { + title: Some("Enable or disable an extension".to_string()), + read_only_hint: Some(false), + destructive_hint: Some(false), + idempotent_hint: Some(false), + open_world_hint: Some(false), + }) +} + pub fn manage_schedule_tool() -> Tool { Tool::new( PLATFORM_MANAGE_SCHEDULE_TOOL_NAME.to_string(), diff --git a/crates/goose/src/agents/router_tools.rs b/crates/goose/src/agents/router_tools.rs index 9a268eb29377..22c29a221306 100644 --- a/crates/goose/src/agents/router_tools.rs +++ b/crates/goose/src/agents/router_tools.rs @@ -1,6 +1,6 @@ -use crate::agents::extension_manager_extension::{ - LIST_RESOURCES_TOOL_NAME, MANAGE_EXTENSIONS_TOOL_NAME, READ_RESOURCE_TOOL_NAME, - SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME, +use super::platform_tools::{ + PLATFORM_LIST_RESOURCES_TOOL_NAME, PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME, + PLATFORM_READ_RESOURCE_TOOL_NAME, PLATFORM_SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME, }; use indoc::indoc; use rmcp::model::{Tool, ToolAnnotations}; @@ -57,9 +57,9 @@ pub fn llm_search_tool_prompt() -> String { - {} - {} "#, - SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME, - MANAGE_EXTENSIONS_TOOL_NAME, - READ_RESOURCE_TOOL_NAME, - LIST_RESOURCES_TOOL_NAME + PLATFORM_SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME, + PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME, + PLATFORM_READ_RESOURCE_TOOL_NAME, + PLATFORM_LIST_RESOURCES_TOOL_NAME ) } diff --git a/crates/goose/src/agents/tool_route_manager.rs b/crates/goose/src/agents/tool_route_manager.rs index 670806070210..774181b3f02f 100644 --- a/crates/goose/src/agents/tool_route_manager.rs +++ b/crates/goose/src/agents/tool_route_manager.rs @@ -17,12 +17,6 @@ pub struct ToolRouteManager { router_disabled_override: Mutex, } -impl Default for ToolRouteManager { - fn default() -> Self { - Self::new() - } -} - impl ToolRouteManager { pub fn new() -> Self { Self { @@ -103,6 +97,9 @@ impl ToolRouteManager { // Wrap selector in Arc for the index manager methods let selector_arc = Arc::new(selector); + // First index platform tools + ToolRouterIndexManager::index_platform_tools(&selector_arc, extension_manager).await?; + if reindex_all.unwrap_or(false) { let enabled_extensions = extension_manager.list_extensions().await?; for extension_name in enabled_extensions { diff --git a/crates/goose/src/agents/tool_router_index_manager.rs b/crates/goose/src/agents/tool_router_index_manager.rs index d3e65467e965..320b10290338 100644 --- a/crates/goose/src/agents/tool_router_index_manager.rs +++ b/crates/goose/src/agents/tool_router_index_manager.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use tracing; use crate::agents::extension_manager::ExtensionManager; +use crate::agents::platform_tools; use crate::agents::router_tool_selector::RouterToolSelector; /// Manages tool indexing operations for the router when LLM routing is enabled @@ -73,4 +74,31 @@ impl ToolRouterIndexManager { Ok(()) } + + /// Indexes platform tools (search_available_extensions, manage_extensions, etc.) + pub async fn index_platform_tools( + selector: &Arc>, + extension_manager: &ExtensionManager, + ) -> Result<()> { + let mut tools = Vec::new(); + + // Add the standard platform tools + tools.push(platform_tools::search_available_extensions_tool()); + tools.push(platform_tools::manage_extensions_tool()); + + // Add resource tools if supported + if extension_manager.supports_resources().await { + tools.push(platform_tools::read_resource_tool()); + tools.push(platform_tools::list_resources_tool()); + } + + // Index all platform tools at once + selector + .index_tools(&tools, "platform") + .await + .map_err(|e| anyhow!("Failed to index platform tools: {}", e))?; + + tracing::info!("Indexed platform tools for LLM search"); + Ok(()) + } } diff --git a/crates/goose/src/execution/manager.rs b/crates/goose/src/execution/manager.rs index 3d86ff092b29..46bf84b74385 100644 --- a/crates/goose/src/execution/manager.rs +++ b/crates/goose/src/execution/manager.rs @@ -120,8 +120,6 @@ impl AgentManager { .extension_manager .set_context(PlatformExtensionContext { session_id: Some(session_id.clone()), - extension_manager: Some(Arc::downgrade(&agent.extension_manager)), - tool_route_manager: Some(Arc::downgrade(&agent.tool_route_manager)), }) .await; if let Some(provider) = &*self.default_provider.read().await { diff --git a/crates/goose/src/permission/permission_inspector.rs b/crates/goose/src/permission/permission_inspector.rs index fe4a347d4b43..9cb392bc85ca 100644 --- a/crates/goose/src/permission/permission_inspector.rs +++ b/crates/goose/src/permission/permission_inspector.rs @@ -1,4 +1,4 @@ -use crate::agents::extension_manager_extension::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; +use crate::agents::platform_tools::PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME; use crate::config::permission::PermissionLevel; use crate::config::PermissionManager; use crate::conversation::message::{Message, ToolRequest}; @@ -164,7 +164,7 @@ impl ToolInspector for PermissionInspector { InspectionAction::Allow } // 4. Special case for extension management - else if tool_name == MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE { + else if tool_name == PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME { InspectionAction::RequireApproval(Some( "Extension management requires approval for security".to_string(), )) @@ -189,7 +189,7 @@ impl ToolInspector for PermissionInspector { } InspectionAction::Deny => "User permission denies this tool".to_string(), InspectionAction::RequireApproval(_) => { - if tool_name == MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE { + if tool_name == PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME { "Extension management requires user approval".to_string() } else { "Tool requires user approval".to_string() diff --git a/crates/goose/src/permission/permission_judge.rs b/crates/goose/src/permission/permission_judge.rs index ecf25ee7578c..5e54fbfd0dca 100644 --- a/crates/goose/src/permission/permission_judge.rs +++ b/crates/goose/src/permission/permission_judge.rs @@ -1,4 +1,4 @@ -use crate::agents::extension_manager_extension::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; +use crate::agents::platform_tools::PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME; use crate::config::permission::PermissionLevel; use crate::config::PermissionManager; use crate::conversation::message::{Message, MessageContent, ToolRequest}; @@ -188,7 +188,7 @@ pub async fn check_tool_permissions( } else if mode == "auto" { approved.push(request.clone()); } else { - if tool_call.name == MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE { + if tool_call.name == PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME { extension_request_ids.push(request.id.clone()); } @@ -442,7 +442,7 @@ mod tests { let enable_extension = ToolRequest { id: "tool_3".to_string(), tool_call: Ok(CallToolRequestParam { - name: MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE.into(), + name: PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME.into(), arguments: Some(object!({"action": "enable", "extension_name": "data_fetcher"})), }), }; diff --git a/crates/goose/src/prompts/system.md b/crates/goose/src/prompts/system.md index 5af468ae0b8b..f8bda7cd9610 100644 --- a/crates/goose/src/prompts/system.md +++ b/crates/goose/src/prompts/system.md @@ -14,12 +14,9 @@ Extensions allow other applications to provide context to goose. Extensions conn tools. You are capable of dynamically plugging into new extensions and learning how to use them. You solve higher level problems using the tools in these extensions, and can interact with multiple at once. - -If the Extension Manager extension is enabled, you can use the search_available_extensions tool to discover additional -extensions that can help with your task. To enable or disable extensions, use the manage_extensions tool with the -extension_name. You should only enable extensions found from the search_available_extensions tool. -If Extension Manager is not available, you can only work with currently enabled extensions and cannot dynamically load -new ones. +Use the search_available_extensions tool to find additional extensions to enable to help with your task. To enable +extensions, use the enable_extension tool and provide the extension_name. You should only enable extensions found from +the search_available_extensions tool. {% if (extensions is defined) and extensions %} Because you dynamically load extensions, your conversation history may refer diff --git a/crates/goose/src/prompts/system_gpt_4.1.md b/crates/goose/src/prompts/system_gpt_4.1.md index e588a244b6c9..1578efaf4dc7 100644 --- a/crates/goose/src/prompts/system_gpt_4.1.md +++ b/crates/goose/src/prompts/system_gpt_4.1.md @@ -28,9 +28,7 @@ Your model may have varying knowledge cut-off dates depending on when they were Extensions allow other applications to provide context to goose. Extensions connect goose to different data sources and tools. You are capable of dynamically plugging into new extensions and learning how to use them. You solve higher level problems using the tools in these extensions, and can interact with multiple at once. - -If the Extension Manager extension is enabled, you can use the search_available_extensions tool to discover additional extensions that can help with your task. To enable or disable extensions, use the manage_extensions tool with the extension_name. You should only enable extensions found from the search_available_extensions tool. -If Extension Manager is not available, you can only work with currently enabled extensions and cannot dynamically load new ones. +Use the search_available_extensions tool to find additional extensions to enable to help with your task. To enable extensions, use the enable_extension tool and provide the extension_name. You should only enable extensions found from the search_available_extensions tool. {% if (extensions is defined) and extensions %} Because you dynamically load extensions, your conversation history may refer diff --git a/crates/goose/tests/agent.rs b/crates/goose/tests/agent.rs index e97f0bfb53c8..af5d531c4153 100644 --- a/crates/goose/tests/agent.rs +++ b/crates/goose/tests/agent.rs @@ -3,7 +3,6 @@ use std::sync::Arc; use anyhow::Result; use futures::StreamExt; use goose::agents::{Agent, AgentEvent}; -use goose::config::extensions::{set_extension, ExtensionEntry}; use goose::conversation::message::Message; use goose::conversation::Conversation; use goose::model::ModelConfig; @@ -1140,271 +1139,3 @@ mod max_turns_tests { Ok(()) } } - -#[cfg(test)] -mod extension_manager_tests { - use super::*; - use goose::agents::extension::{ExtensionConfig, PlatformExtensionContext}; - use goose::agents::extension_manager_extension::{ - MANAGE_EXTENSIONS_TOOL_NAME, SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME, - }; - use rmcp::model::CallToolRequestParam; - use rmcp::object; - - async fn setup_agent_with_extension_manager() -> Agent { - // Add the TODO extension to the config so it can be discovered by search_available_extensions - // Set it as disabled initially so tests can enable it - let todo_extension_entry = ExtensionEntry { - enabled: false, - config: ExtensionConfig::Platform { - name: "todo".to_string(), - description: - "Enable a todo list for Goose so it can keep track of what it is doing" - .to_string(), - bundled: Some(true), - available_tools: vec![], - }, - }; - set_extension(todo_extension_entry); - - let agent = Agent::new(); - - agent - .extension_manager - .set_context(PlatformExtensionContext { - session_id: Some("test_session".to_string()), - extension_manager: Some(Arc::downgrade(&agent.extension_manager)), - tool_route_manager: Some(Arc::downgrade(&agent.tool_route_manager)), - }) - .await; - - // Now add the extension manager platform extension - let ext_config = ExtensionConfig::Platform { - name: "extensionmanager".to_string(), - description: "Extension Manager".to_string(), - bundled: Some(true), - available_tools: vec![], - }; - - agent - .add_extension(ext_config) - .await - .expect("Failed to add extension manager"); - agent - } - - #[tokio::test] - async fn test_extension_manager_tools_available() { - let agent = setup_agent_with_extension_manager().await; - let tools = agent.list_tools(None).await; - - // Note: Tool names are prefixed with the normalized extension name "extensionmanager" - // not the display name "Extension Manager" - let search_tool = tools.iter().find(|tool| { - tool.name == format!("extensionmanager__{SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME}") - }); - assert!( - search_tool.is_some(), - "search_available_extensions tool should be available" - ); - - let manage_tool = tools - .iter() - .find(|tool| tool.name == format!("extensionmanager__{MANAGE_EXTENSIONS_TOOL_NAME}")); - assert!( - manage_tool.is_some(), - "manage_extensions tool should be available" - ); - } - - #[tokio::test] - async fn test_search_available_extensions_tool_call() { - let agent = setup_agent_with_extension_manager().await; - - let tool_call = CallToolRequestParam { - name: format!("extensionmanager__{SEARCH_AVAILABLE_EXTENSIONS_TOOL_NAME}").into(), - arguments: Some(object!({})), - }; - - let (_, result) = agent - .dispatch_tool_call(tool_call, "request_1".to_string(), None, None) - .await; - - assert!(result.is_ok(), "search_available_extensions should succeed"); - let call_result = result.unwrap().result.await; - assert!( - call_result.is_ok(), - "search_available_extensions execution should succeed" - ); - - let content = call_result.unwrap(); - assert!(!content.is_empty(), "Should return some content"); - - // Verify the content contains expected text - let text = content.first().unwrap().as_text().unwrap(); - assert!( - text.text.contains("Extensions available to enable:"), - "Content should contain 'Extensions available to enable:'" - ); - } - - #[tokio::test] - async fn test_manage_extensions_enable_disable_success() { - let agent = setup_agent_with_extension_manager().await; - - let enable_call = CallToolRequestParam { - name: format!("extensionmanager__{MANAGE_EXTENSIONS_TOOL_NAME}").into(), - arguments: Some(object!({ - "action": "enable", - "extension_name": "todo" - })), - }; - let (_, result) = agent - .dispatch_tool_call(enable_call, "request_3a".to_string(), None, None) - .await; - assert!(result.is_ok()); - let call_result = result.unwrap().result.await; - assert!( - call_result.is_ok(), - "manage_extensions enable execution should succeed" - ); - - let content = call_result.unwrap(); - let text = content.first().unwrap().as_text().unwrap(); - assert!( - text.text.contains("todo") && text.text.contains("installed successfully"), - "Response should indicate success, got: {}", - text.text - ); - - // Now disable it - let disable_call = CallToolRequestParam { - name: format!("extensionmanager__{MANAGE_EXTENSIONS_TOOL_NAME}").into(), - arguments: Some(object!({ - "action": "disable", - "extension_name": "todo" - })), - }; - - let (_, result) = agent - .dispatch_tool_call(disable_call, "request_3b".to_string(), None, None) - .await; - - assert!(result.is_ok(), "manage_extensions disable should succeed"); - let call_result = result.unwrap().result.await; - assert!( - call_result.is_ok(), - "manage_extensions disable execution should succeed" - ); - - let content = call_result.unwrap(); - assert!(!content.is_empty(), "Should return confirmation message"); - - // Verify the message indicates success - let text = content.first().unwrap().as_text().unwrap(); - assert!( - text.text.contains("successfully") || text.text.contains("disabled"), - "Response should indicate success, got: {}", - text.text - ); - } - - #[tokio::test] - async fn test_manage_extensions_missing_parameters() { - let agent = setup_agent_with_extension_manager().await; - - // Call manage_extensions without action parameter - let tool_call = CallToolRequestParam { - name: format!("extensionmanager__{MANAGE_EXTENSIONS_TOOL_NAME}").into(), - arguments: Some(object!({ - "extension_name": "todo" - })), - }; - - let (_, result) = agent - .dispatch_tool_call(tool_call, "request_4".to_string(), None, None) - .await; - - assert!(result.is_ok(), "Tool call should return a result"); - let call_result = result.unwrap().result.await; - assert!( - call_result.is_ok(), - "Tool execution should return an error result" - ); - - let content = call_result.unwrap(); - let text = content.first().unwrap().as_text().unwrap(); - assert!( - text.text.contains("action") || text.text.contains("parameter"), - "Error should mention missing action parameter" - ); - } - - #[tokio::test] - async fn test_manage_extensions_invalid_action() { - let agent = setup_agent_with_extension_manager().await; - - let tool_call = CallToolRequestParam { - name: format!("extensionmanager__{MANAGE_EXTENSIONS_TOOL_NAME}").into(), - arguments: Some(object!({ - "action": "invalid_action", - "extension_name": "todo" - })), - }; - - let (_, result) = agent - .dispatch_tool_call(tool_call, "request_6".to_string(), None, None) - .await; - - assert!(result.is_ok(), "Tool call should return a result"); - let call_result = result.unwrap().result.await; - assert!( - call_result.is_ok(), - "Tool execution should return an error result" - ); - - let content = call_result.unwrap(); - let text = content.first().unwrap().as_text().unwrap(); - assert!( - text.text.contains("Invalid action") - || text.text.contains("enable") - || text.text.contains("disable"), - "Error should mention invalid action, got: {}", - text.text - ); - } - - #[tokio::test] - async fn test_manage_extensions_extension_not_found() { - let agent = setup_agent_with_extension_manager().await; - - // Try to enable a non-existent extension - let tool_call = CallToolRequestParam { - name: format!("extensionmanager__{MANAGE_EXTENSIONS_TOOL_NAME}").into(), - arguments: Some(object!({ - "action": "enable", - "extension_name": "nonexistent_extension_12345" - })), - }; - - let (_, result) = agent - .dispatch_tool_call(tool_call, "request_7".to_string(), None, None) - .await; - - assert!(result.is_ok(), "Tool call should return a result"); - let call_result = result.unwrap().result.await; - assert!( - call_result.is_ok(), - "Tool execution should return an error result" - ); - - // Check that the error message indicates extension not found - let content = call_result.unwrap(); - let text = content.first().unwrap().as_text().unwrap(); - assert!( - text.text.contains("not found") || text.text.contains("Extension"), - "Error should mention extension not found, got: {}", - text.text - ); - } -} diff --git a/ui/desktop/src/components/settings/extensions/agent-api.test.ts b/ui/desktop/src/components/settings/extensions/agent-api.test.ts index b5e943b0cc82..9df1c2305cac 100644 --- a/ui/desktop/src/components/settings/extensions/agent-api.test.ts +++ b/ui/desktop/src/components/settings/extensions/agent-api.test.ts @@ -313,31 +313,6 @@ describe('Agent API', () => { }), }); }); - - it('should not mutate the original extension config', async () => { - const originalConfig: ExtensionConfig = { - type: 'stdio', - name: 'Extension Manager', - description: 'Test description', - cmd: 'python', - args: ['script.py'], - }; - - const mockResponse = { - ok: true, - text: vi.fn().mockResolvedValue('{"error": false}'), - }; - mockFetch.mockResolvedValue(mockResponse); - - const { replaceWithShims } = await import('./utils'); - vi.mocked(replaceWithShims).mockResolvedValue('/path/to/shim'); - - await addToAgent(originalConfig, {}, 'test-session'); - - // Verify the original config was not mutated - expect(originalConfig.name).toBe('Extension Manager'); - expect(originalConfig.cmd).toBe('python'); - }); }); describe('removeFromAgent', () => { diff --git a/ui/desktop/src/components/settings/extensions/agent-api.ts b/ui/desktop/src/components/settings/extensions/agent-api.ts index 7143779b78d0..bc229a597e25 100644 --- a/ui/desktop/src/components/settings/extensions/agent-api.ts +++ b/ui/desktop/src/components/settings/extensions/agent-api.ts @@ -156,26 +156,21 @@ export async function addToAgent( options: ToastServiceOptions = {}, sessionId: string ): Promise { - // Create a copy to avoid mutating the original extension object - const extensionCopy: ExtensionConfig = { ...extension }; - try { - if (extensionCopy.type === 'stdio') { - extensionCopy.cmd = await replaceWithShims(extensionCopy.cmd); + if (extension.type === 'stdio') { + extension.cmd = await replaceWithShims(extension.cmd); } - extensionCopy.name = sanitizeName(extensionCopy.name); + extension.name = sanitizeName(extension.name); - return await extensionApiCall('/extensions/add', extensionCopy, options, sessionId); + return await extensionApiCall('/extensions/add', extension, options, sessionId); } catch (error) { // Check if this is a 428 error and make the message more descriptive if (error instanceof Error && error.message && error.message.includes('428')) { const enhancedError = new Error( 'Failed to add extension. Goose Agent was still starting up. Please try again.' ); - console.error( - `Failed to add extension ${extensionCopy.name} to agent: ${enhancedError.message}` - ); + console.error(`Failed to add extension ${extension.name} to agent: ${enhancedError.message}`); throw enhancedError; } throw error;