Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds default tools that can use MCP resources #619

Merged
merged 15 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 7 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
212 changes: 198 additions & 14 deletions crates/goose/src/agents/capabilities.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use chrono::{DateTime, TimeZone, Utc};
use futures::stream::{FuturesUnordered, StreamExt};
use mcp_client::McpService;
use rust_decimal_macros::dec;
use std::collections::HashMap;
Expand All @@ -14,6 +15,7 @@ use crate::providers::base::{Provider, ProviderUsage};
use mcp_client::client::{ClientCapabilities, ClientInfo, McpClient, McpClientTrait};
use mcp_client::transport::{SseTransport, StdioTransport, Transport};
use mcp_core::{Content, Tool, ToolCall, ToolError, ToolResult};
use serde_json::Value;

// By default, we set it to Jan 1, 2020 if the resource does not have a timestamp
// This is to ensure that the resource is considered less important than resources with a more recent timestamp
Expand Down Expand Up @@ -303,25 +305,207 @@ impl Capabilities {
.map(Arc::clone)
}

/// Dispatch a single tool call to the appropriate client
#[instrument(skip(self, tool_call), fields(input, output))]
pub async fn dispatch_tool_call(&self, tool_call: ToolCall) -> ToolResult<Vec<Content>> {
// Function that gets executed for read_resource tool
async fn read_resource(&self, params: Value) -> Result<Vec<Content>, ToolError> {
let uri = params
.get("uri")
.and_then(|v| v.as_str())
.ok_or_else(|| ToolError::InvalidParameters("Missing 'uri' parameter".to_string()))?;

let system_name = params.get("system_name").and_then(|v| v.as_str());

// If system name is provided, we can just look it up
if system_name.is_some() {
let result = self
.read_resource_from_system(uri, system_name.unwrap())
.await?;
return Ok(result);
}

// If system name is not provided, we need to search for the resource across all systems
// Loop through each system and try to read the resource, don't raise an error if the resource is not found
for system_name in self.clients.keys() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can skip some steps by searching only self.resource_capable_system (like below)

let result = self.read_resource_from_system(uri, system_name).await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: either make a note that we assume no risk of collision for generic names or ensure num_matches == 1 before proceeding?

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm good point, if two systems have the same resource uri, it would find the first match and exit the loop
i'll put a TODO comment, not sure if we'd want to find the uri across all systems or not

match result {
Ok(result) => return Ok(result),
Err(_) => continue,
}
}

// None of the systems had the resource so we raise an error
let available_systems = self
.clients
.keys()
.map(|s| s.as_str())
.collect::<Vec<&str>>()
.join(", ");
let error_msg = format!(
"Resource with uri '{}' not found. Here are the available systems: {}",
uri, available_systems
);

Err(ToolError::InvalidParameters(error_msg))
}

async fn read_resource_from_system(
&self,
uri: &str,
system_name: &str,
) -> Result<Vec<Content>, ToolError> {
let available_systems = self
.clients
.keys()
.map(|s| s.as_str())
.collect::<Vec<&str>>()
.join(", ");
let error_msg = format!(
"System '{}' not found. Here are the available systems: {}",
system_name, available_systems
);

let client = self
.get_client_for_tool(&tool_call.name)
.ok_or_else(|| ToolError::NotFound(tool_call.name.clone()))?;
.clients
.get(system_name)
.ok_or(ToolError::InvalidParameters(error_msg))?;

let client_guard = client.lock().await;
let read_result = client_guard.read_resource(uri).await.map_err(|_| {
ToolError::ExecutionError(format!("Could not read resource with uri: {}", uri))
})?;

let mut result = Vec::new();
for content in read_result.contents {
// Only reading the text resource content; skipping the blob content cause it's too long
if let mcp_core::resource::ResourceContents::TextResourceContents { text, .. } = content
{
let content_str = format!("{}\n\n{}", uri, text);
result.push(Content::text(content_str));
}
}

let tool_name = tool_call
.name
.split("__")
.nth(1)
.ok_or_else(|| ToolError::NotFound(tool_call.name.clone()))?;
Ok(result)
}

async fn list_resources_from_system(
&self,
system_name: &str,
) -> Result<Vec<Content>, ToolError> {
let client = self.clients.get(system_name).ok_or_else(|| {
ToolError::InvalidParameters(format!("System {} is not valid", system_name))
})?;

let client_guard = client.lock().await;
let result = client_guard
.call_tool(tool_name, tool_call.clone().arguments)
client_guard
.list_resources(None)
.await
.map(|result| result.content)
.map_err(|e| ToolError::ExecutionError(e.to_string()));
.map_err(|e| {
ToolError::ExecutionError(format!(
"Unable to list resources for {}, {:?}",
system_name, e
))
})
.map(|lr| {
let resource_list = lr
.resources
.into_iter()
.map(|r| format!("{}, uri: ({})", r.name, r.uri))
.collect::<Vec<String>>()
.join("\n");

vec![Content::text(resource_list)]
})
}

async fn list_resources(&self, params: Value) -> Result<Vec<Content>, ToolError> {
let system = params.get("system").and_then(|v| v.as_str());

match system {
Some(system_name) => {
// Handle single system case
self.list_resources_from_system(system_name).await
}
None => {
// Handle all systems case using FuturesUnordered
let mut futures = FuturesUnordered::new();

// Create futures for each system
for (system_name, client) in &self.clients {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can skip some steps by iterating over self.resource_capable_systems instead here now?

let client = Arc::clone(client);

futures.push(async move {
let guard = client.lock().await;
guard
.list_resources(None)
.await
.map(|r| (system_name.clone(), r))
.map_err(|e| (system_name.clone(), e))
});
}

let mut all_resources = Vec::new();
let mut errors = Vec::new();

// Process results as they complete
while let Some(result) = futures.next().await {
match result {
Ok((system_name, resource_list)) => {
all_resources.extend(resource_list.resources.into_iter().map(|r| {
format!("{} - {}, uri: ({})", system_name, r.name, r.uri)
}));
}
Err((system_name, e)) => {
errors.push((system_name, e));
}
}
}

// Log any errors that occurred
if !errors.is_empty() {
tracing::error!(
errors = ?errors
.into_iter()
.map(|(sys, e)| format!("{}: {:?}", sys, e))
.collect::<Vec<_>>(),
"errors from listing resources"
);
}

// Sort resources for consistent output
all_resources.sort();

Ok(vec![Content::text(all_resources.join("\n"))])
}
}
}

/// Dispatch a single tool call to the appropriate client
#[instrument(skip(self, tool_call), fields(input, output))]
pub async fn dispatch_tool_call(&self, tool_call: ToolCall) -> ToolResult<Vec<Content>> {
let result = if tool_call.name == "platform__read_resource" {
// Check if the tool is read_resource and handle it separately
self.read_resource(tool_call.arguments.clone()).await
} else if tool_call.name == "platform__list_resources" {
self.list_resources(tool_call.arguments.clone()).await
} else {
// Else, dispatch tool call based on the prefix naming convention
let client = self
.get_client_for_tool(&tool_call.name)
.ok_or_else(|| ToolError::NotFound(tool_call.name.clone()))?;

let tool_name = tool_call
.name
.split("__")
.nth(1)
.ok_or_else(|| ToolError::NotFound(tool_call.name.clone()))?;

let client_guard = client.lock().await;

client_guard
.call_tool(tool_name, tool_call.clone().arguments)
.await
.map(|result| result.content)
.map_err(|e| ToolError::ExecutionError(e.to_string()))
};

debug!(
"input" = serde_json::to_string(&tool_call).unwrap(),
Expand Down
60 changes: 58 additions & 2 deletions crates/goose/src/agents/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ use crate::providers::base::Provider;
use crate::providers::base::ProviderUsage;
use crate::register_agent;
use crate::token_counter::TokenCounter;
use serde_json::Value;
use indoc::indoc;
use mcp_core::tool::Tool;
use serde_json::{json, Value};

/// Reference implementation of an Agent
pub struct ReferenceAgent {
capabilities: Mutex<Capabilities>,
Expand Down Expand Up @@ -65,7 +68,60 @@ impl Agent for ReferenceAgent {
let mut messages = messages.to_vec();
let reply_span = tracing::Span::current();
let mut capabilities = self.capabilities.lock().await;
let tools = capabilities.get_prefixed_tools().await?;
let mut tools = capabilities.get_prefixed_tools().await?;
// we add in the read_resource tool by default
// TODO: make sure there is no collision with another system's tool name
let read_resource_tool = Tool::new(
"platform__read_resource".to_string(),
indoc! {r#"
Read a resource from a system.

Resources allow systems 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 system, and reads in the resource content. If no system
is provided, the tool will search all systems for the resource.

The read_resource tool is typically used with a search query (can be before or after). Here are two examples:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: i'd remove these extra tips unless we find they are needed tips? not sure if it was tested. i've been noticing these specific examples don't necessarily make it better at using the tools but can confuse it into hallucinating usage that isn't relevant, with frontier models at least

1. Search for files in Google Drive MCP Server, then call read_resource(gdrive:///<file_id>) to read the Google Drive file.
2. You need to gather schema information about a Postgres table before creating a query. So you call read_resource(postgres://<host>/<table>/schema)
to get the schema information for a table and then use to construct your query.
"#}.to_string(),
json!({
"type": "object",
"required": ["uri"],
"properties": {
"uri": {"type": "string", "description": "Resource URI"},
"system_name": {"type": "string", "description": "Optional system name"}
}
}),
);

let list_resources_tool = Tool::new(
"platform__list_resources".to_string(),
indoc! {r#"
List resources from a system(s).

Resources allow systems to share data that provide context to LLMs, such as
files, database schemas, or application-specific information. This tool lists resources
in the provided system, and returns a list for the user to browse. If no system
is provided, the tool will search all systems for the resource.

The list_resources tool is typically used before a read_resource tool call. Here are two examples:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: i'd remove these extra tips unless we find they are needed tips? not sure if it was tested. i've been noticing these specific examples don't necessarily make it better at using the tools but can confuse it into hallucinating usage that isn't relevant, with frontier models at least

1. List files in Google Drive MCP Server, then call read_resource(gdrive:///<file_id>) to read the Google Drive file.
2. You want to see what tables exist in Postgre so you can find schema information about that table before creating a query. So you call list_resources to see whats available then you call read_resource(postgres://<host>/<table>/schema)
to get the schema information for a table and then use to construct your query.
"#}.to_string(),
json!({
"type": "object",
"properties": {
"system_name": {"type": "string", "description": "Optional system name"}
}
}),
);

tools.push(read_resource_tool);
Copy link
Collaborator

Choose a reason for hiding this comment

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

i would conditionally append these tools only if one of the systems has a resource capability. I'd also update the tooltip to include which systems have resources

tools.push(list_resources_tool);

let system_prompt = capabilities.get_system_prompt().await;
let _estimated_limit = capabilities
.provider()
Expand Down
8 changes: 8 additions & 0 deletions crates/goose/src/prompts/system.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ to interactions with sytems that are not currently active. The currently
active systems are below. Each of these systems provides tools that are
in your tool specification.

The format of a tool is "{system_name}__{tool_name}", i.e. the system
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: i think these additions are excludable? not sure if you saw any performance issues with this but i have not seen any problems or a need to warn it about tools beyond the tool descriptions

name followed by the tool name with '__' as the separator.

By default, we add two tools, one called "platform__list_resources"
which can be used to list resources with URIs from all or from a specific
system by name and one called "platform__read_resource", which can
be used to read a resource URI from a system.

# Systems:
{% for system in systems %}

Expand Down
Loading