Skip to content
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/goose-cli/src/session/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> CliSession {
session_id: Some(session_id.clone()),
extension_manager: Some(Arc::downgrade(&agent.extension_manager)),
tool_route_manager: Some(Arc::downgrade(&agent.tool_route_manager)),
sub_recipes: Some(agent.sub_recipes()),
})
.await;

Expand Down
96 changes: 35 additions & 61 deletions crates/goose/src/agents/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ use crate::agents::platform_tools::PLATFORM_MANAGE_SCHEDULE_TOOL_NAME;
use crate::agents::prompt_manager::PromptManager;
use crate::agents::retry::{RetryManager, RetryResult};
use crate::agents::router_tools::ROUTER_LLM_SEARCH_TOOL_NAME;
use crate::agents::subagent_task_config::TaskConfig;
use crate::agents::subagent_tool::{
create_subagent_tool, handle_subagent_tool, SUBAGENT_TOOL_NAME,
};
use crate::agents::subagent_client;
use crate::agents::tool_route_manager::ToolRouteManager;
use crate::agents::tool_router_index_manager::ToolRouterIndexManager;
use crate::agents::types::SessionConfig;
Expand Down Expand Up @@ -85,7 +82,7 @@ pub struct Agent {
pub(super) provider: SharedProvider,

pub extension_manager: Arc<ExtensionManager>,
pub(super) sub_recipes: Mutex<HashMap<String, SubRecipe>>,
pub(super) sub_recipes: Arc<tokio::sync::RwLock<HashMap<String, SubRecipe>>>,
pub(super) final_output_tool: Arc<Mutex<Option<FinalOutputTool>>>,
pub(super) frontend_tools: Mutex<HashMap<String, FrontendTool>>,
pub(super) frontend_instructions: Mutex<Option<String>>,
Expand Down Expand Up @@ -160,7 +157,7 @@ impl Agent {
Self {
provider: provider.clone(),
extension_manager: Arc::new(ExtensionManager::new(provider.clone())),
sub_recipes: Mutex::new(HashMap::new()),
sub_recipes: Arc::new(tokio::sync::RwLock::new(HashMap::new())),
final_output_tool: Arc::new(Mutex::new(None)),
frontend_tools: Mutex::new(HashMap::new()),
frontend_instructions: Mutex::new(None),
Expand Down Expand Up @@ -267,6 +264,10 @@ impl Agent {
let initial_messages = conversation.messages().clone();
let config = Config::global();

if let Err(e) = self.enable_subagent_extension().await {
Comment thread
tlongwell-block marked this conversation as resolved.
Outdated
warn!("Failed to enable subagent extension: {}", e);
}

let (tools, toolshim_tools, system_prompt) =
self.prepare_tools_and_prompt(working_dir).await?;
let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto);
Expand Down Expand Up @@ -399,8 +400,12 @@ impl Agent {
self.extend_system_prompt(final_output_system_prompt).await;
}

pub fn sub_recipes(&self) -> Arc<tokio::sync::RwLock<HashMap<String, SubRecipe>>> {
self.sub_recipes.clone()
}

pub async fn add_sub_recipes(&self, sub_recipes_to_add: Vec<SubRecipe>) {
let mut sub_recipes = self.sub_recipes.lock().await;
let mut sub_recipes = self.sub_recipes.write().await;
for sr in sub_recipes_to_add {
sub_recipes.insert(sr.name.clone(), sr);
}
Expand Down Expand Up @@ -430,20 +435,8 @@ impl Agent {
tool_call: CallToolRequestParam,
request_id: String,
cancellation_token: Option<CancellationToken>,
session: &Session,
_session: &Session,
Comment thread
tlongwell-block marked this conversation as resolved.
Outdated
) -> (String, Result<ToolCallResult, ErrorData>) {
// Prevent subagents from creating other subagents
if session.session_type == SessionType::SubAgent && tool_call.name == SUBAGENT_TOOL_NAME {
return (
request_id,
Err(ErrorData::new(
ErrorCode::INVALID_REQUEST,
"Subagents cannot create other subagents".to_string(),
None,
)),
);
}

if tool_call.name == PLATFORM_MANAGE_SCHEDULE_TOOL_NAME {
let arguments = tool_call
.arguments
Expand Down Expand Up @@ -478,40 +471,7 @@ impl Agent {
}

debug!("WAITING_TOOL_START: {}", tool_call.name);
let result: ToolCallResult = if tool_call.name == SUBAGENT_TOOL_NAME {
let provider = match self.provider().await {
Ok(p) => p,
Err(_) => {
return (
request_id,
Err(ErrorData::new(
ErrorCode::INTERNAL_ERROR,
"Provider is required".to_string(),
None,
)),
);
}
};

let extensions = self.get_extension_configs().await;
let task_config =
TaskConfig::new(provider, &session.id, &session.working_dir, extensions);
let sub_recipes = self.sub_recipes.lock().await.clone();

let arguments = tool_call
.arguments
.clone()
.map(Value::Object)
.unwrap_or(Value::Object(serde_json::Map::new()));

handle_subagent_tool(
arguments,
task_config,
sub_recipes,
session.working_dir.clone(),
cancellation_token,
)
} else if self.is_frontend_tool(&tool_call.name).await {
let result: ToolCallResult = 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(
ErrorCode::INTERNAL_ERROR,
Expand Down Expand Up @@ -678,7 +638,6 @@ impl Agent {
.await
.unwrap_or_default();

let subagents_enabled = self.subagents_enabled().await;
if extension_name.is_none() || extension_name.as_deref() == Some("platform") {
prefixed_tools.push(platform_tools::manage_schedule_tool());
}
Expand All @@ -687,17 +646,32 @@ impl Agent {
if let Some(final_output_tool) = self.final_output_tool.lock().await.as_ref() {
prefixed_tools.push(final_output_tool.tool());
}

if subagents_enabled {
let sub_recipes = self.sub_recipes.lock().await;
let sub_recipes_vec: Vec<_> = sub_recipes.values().cloned().collect();
prefixed_tools.push(create_subagent_tool(&sub_recipes_vec));
}
}

prefixed_tools
}

pub async fn enable_subagent_extension(&self) -> ExtensionResult<()> {
if !self.subagents_enabled().await {
return Ok(());
}
if self
.extension_manager
.is_extension_enabled(subagent_client::EXTENSION_NAME)
.await
{
return Ok(());
}
self.extension_manager
.add_extension(ExtensionConfig::Platform {
name: subagent_client::EXTENSION_NAME.to_string(),
description: "Delegate tasks to independent subagents".to_string(),
bundled: Some(true),
available_tools: vec![],
})
.await
}

pub async fn list_tools_for_router(&self) -> Vec<Tool> {
self.tool_route_manager
.list_tools_for_router(&self.extension_manager)
Expand Down
25 changes: 9 additions & 16 deletions crates/goose/src/agents/code_execution_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,9 @@ impl CodeExecutionClient {
) -> Result<Vec<Content>, String> {
let path = arguments
.as_ref()
.and_then(|a| a.get("path"))
.and_then(|a| a.get("module_path"))
.and_then(|v| v.as_str())
.ok_or("Missing required parameter: path")?;
.ok_or("Missing required parameter: module_path")?;

let tools = self.get_tool_infos().await;
let parts: Vec<&str> = path.trim_start_matches('/').split('/').collect();
Expand Down Expand Up @@ -608,7 +608,7 @@ impl McpClientTrait for CodeExecutionClient {
- Import: import { tool1, tool2 } from "serverName";
- Call: toolName({ param1: value, param2: value })
- All calls are synchronous, return strings
- Last expression is the result
- To capture output: const r = <expression>; r
- No comments in code

BEFORE CALLING: Use read_module("server") to check required parameters.
Expand Down Expand Up @@ -758,12 +758,7 @@ mod tests {

#[tokio::test]
async fn test_execute_code_simple() {
let context = PlatformExtensionContext {
session_id: None,
extension_manager: None,
tool_route_manager: None,
};
let client = CodeExecutionClient::new(context).unwrap();
let client = CodeExecutionClient::new(PlatformExtensionContext::default()).unwrap();

let mut args = JsonObject::new();
args.insert("code".to_string(), Value::String("2 + 2".to_string()));
Expand All @@ -783,15 +778,13 @@ mod tests {

#[tokio::test]
async fn test_read_module_not_found() {
let context = PlatformExtensionContext {
session_id: None,
extension_manager: None,
tool_route_manager: None,
};
let client = CodeExecutionClient::new(context).unwrap();
let client = CodeExecutionClient::new(PlatformExtensionContext::default()).unwrap();

let mut args = JsonObject::new();
args.insert("path".to_string(), Value::String("nonexistent".to_string()));
args.insert(
"module_path".to_string(),
Value::String("nonexistent".to_string()),
);

let result = client.handle_read_module(Some(args)).await;
assert!(result.is_err());
Expand Down
17 changes: 16 additions & 1 deletion crates/goose/src/agents/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ use crate::agents::chatrecall_extension;
use crate::agents::code_execution_extension;
use crate::agents::extension_manager_extension;
use crate::agents::skills_extension;
use crate::agents::subagent_client;
use crate::agents::todo_extension;
use crate::recipe::SubRecipe;
use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::RwLock;

use crate::agents::mcp_client::McpClientTrait;
use crate::config;
Expand Down Expand Up @@ -100,17 +104,28 @@ pub static PLATFORM_EXTENSIONS: Lazy<HashMap<&'static str, PlatformExtensionDef>
},
);

map.insert(
subagent_client::EXTENSION_NAME,
PlatformExtensionDef {
name: subagent_client::EXTENSION_NAME,
description: "Delegate tasks to independent subagents",
default_enabled: false,
client_factory: |ctx| Box::new(subagent_client::SubagentClient::new(ctx).unwrap()),
},
);

map
},
);

#[derive(Clone)]
#[derive(Clone, Default)]
pub struct PlatformExtensionContext {
pub session_id: Option<String>,
pub extension_manager:
Option<std::sync::Weak<crate::agents::extension_manager::ExtensionManager>>,
pub tool_route_manager:
Option<std::sync::Weak<crate::agents::tool_route_manager::ToolRouteManager>>,
pub sub_recipes: Option<Arc<RwLock<HashMap<String, SubRecipe>>>>,
}

#[derive(Debug, Clone)]
Expand Down
39 changes: 21 additions & 18 deletions crates/goose/src/agents/extension_manager.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use anyhow::Result;
use axum::http::{HeaderMap, HeaderName};
use chrono::{DateTime, Utc};
use futures::future;
use futures::stream::{FuturesUnordered, StreamExt};
use futures::{future, FutureExt};
use rmcp::service::{ClientInitializeError, ServiceError};
use rmcp::transport::streamable_http_client::{
AuthRequiredError, StreamableHttpClientTransportConfig, StreamableHttpError,
Expand Down Expand Up @@ -262,6 +262,7 @@ impl ExtensionManager {
session_id: None,
extension_manager: None,
tool_route_manager: None,
sub_recipes: None,
}),
provider,
}
Expand All @@ -280,6 +281,10 @@ impl ExtensionManager {
self.context.lock().await.clone()
}

pub async fn get_provider(&self) -> Option<Arc<dyn crate::providers::base::Provider>> {
self.provider.lock().await.clone()
}

pub async fn supports_resources(&self) -> bool {
self.extensions
.lock()
Expand Down Expand Up @@ -1080,26 +1085,24 @@ impl ExtensionManager {
}

let arguments = tool_call.arguments.clone();
let client = client.clone();
let notifications_receiver = client.lock().await.subscribe().await;

let fut = async move {
let client_guard = client.lock().await;
client_guard
.call_tool(&tool_name, arguments, cancellation_token)
.await
.map_err(|e| match e {
ServiceError::McpError(error_data) => error_data,
_ => {
ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), e.maybe_to_value())
}
})
};
let mut result = client
.lock()
.await
.call_tool_deferred(&tool_name, arguments, cancellation_token)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a new function, how about changing this so that McpClientTrait::call_tool always returns a notification stream, with the default impl calling .subscribe()?

I'm also seeing that all of the platform extensions just return a dummy channel for subscribe, but ToolCallResult has an Option for the notifcations stream, so it would make more sense to have the trait fn itself return an Option

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good idea, but I don't necessarily want to do it in this PR.

.await
.map_err(|e| match e {
ServiceError::McpError(error_data) => error_data,
_ => ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), e.maybe_to_value()),
})?;

Ok(ToolCallResult {
result: Box::new(fut.boxed()),
notification_stream: Some(Box::new(ReceiverStream::new(notifications_receiver))),
})
if result.notification_stream.is_none() {
result.notification_stream =
Some(Box::new(ReceiverStream::new(notifications_receiver)));
}

Ok(result)
}

pub async fn list_prompts_from_extension(
Expand Down
17 changes: 17 additions & 0 deletions crates/goose/src/agents/mcp_client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::action_required_manager::ActionRequiredManager;
use crate::agents::tool_execution::ToolCallResult;
use crate::agents::types::SharedProvider;
use crate::session_context::SESSION_ID_HEADER;
use rmcp::model::{
Expand Down Expand Up @@ -64,6 +65,22 @@ pub trait McpClientTrait: Send + Sync {
cancel_token: CancellationToken,
) -> Result<CallToolResult, Error>;

async fn call_tool_deferred(
&self,
name: &str,
arguments: Option<JsonObject>,
cancel_token: CancellationToken,
) -> Result<ToolCallResult, Error> {
Comment thread
tlongwell-block marked this conversation as resolved.
Outdated
let result = self.call_tool(name, arguments, cancel_token).await;
Ok(ToolCallResult {
result: Box::new(futures::future::ready(result.map_err(|e| match e {
ServiceError::McpError(error_data) => error_data,
_ => ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None),
}))),
notification_stream: None,
})
}

async fn list_prompts(
&self,
next_cursor: Option<String>,
Expand Down
1 change: 1 addition & 0 deletions crates/goose/src/agents/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod router_tool_selector;
mod router_tools;
mod schedule_tool;
pub(crate) mod skills_extension;
pub(crate) mod subagent_client;
pub mod subagent_execution_tool;
pub mod subagent_handler;
mod subagent_task_config;
Expand Down
Loading
Loading