From 11a58b68e1f3045245da7c550457ef7f1461ab43 Mon Sep 17 00:00:00 2001 From: Tyler Longwell Date: Thu, 30 Oct 2025 10:56:04 -0400 Subject: [PATCH 1/2] fix: Add schema-aware numeric coercion for MCP tool arguments --- crates/goose/src/agents/agent.rs | 55 ++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 8dc45289c39a..0e38e2eb2fb2 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -49,7 +49,7 @@ use rmcp::model::{ CallToolRequestParam, Content, ErrorCode, ErrorData, GetPromptResult, Prompt, ServerNotification, Tool, }; -use serde_json::Value; +use serde_json::{json, Value}; use tokio::sync::{mpsc, Mutex}; use tokio_util::sync::CancellationToken; use tracing::{debug, error, info, instrument, warn}; @@ -383,15 +383,66 @@ impl Agent { sub_recipe_manager.add_sub_recipe_tools(sub_recipes); } + /// Apply schema-aware numeric coercion to tool arguments + /// Converts string representations of numbers to actual numbers based on the tool's schema + async fn coerce_numeric_arguments(&self, tool_call: &mut CallToolRequestParam) { + let Some(ref mut args) = tool_call.arguments else { + return; + }; + + let tools = self.list_tools(None).await; + let Some(tool_schema) = tools + .iter() + .find(|t| t.name == tool_call.name) + .map(|t| &t.input_schema) + else { + return; + }; + + let Some(properties) = tool_schema.get("properties").and_then(|p| p.as_object()) else { + return; + }; + + for (key, value) in args.iter_mut() { + let Value::String(s) = value else { + continue; + }; + + let Some(prop_schema) = properties.get(key) else { + continue; + }; + + let should_be_number = match prop_schema.get("type") { + Some(Value::String(t)) => t == "number" || t == "integer", + Some(Value::Array(types)) => types + .iter() + .any(|t| t.as_str().is_some_and(|s| s == "number" || s == "integer")), + _ => false, + }; + + if should_be_number { + if let Ok(n) = s.parse::() { + // Preserve integer types when possible + *value = if n.fract() == 0.0 && n >= i64::MIN as f64 && n <= i64::MAX as f64 { + json!(n as i64) + } else { + json!(n) + }; + } + } + } + } + /// Dispatch a single tool call to the appropriate client #[instrument(skip(self, tool_call, request_id), fields(input, output))] pub async fn dispatch_tool_call( &self, - tool_call: CallToolRequestParam, + mut tool_call: CallToolRequestParam, request_id: String, cancellation_token: Option, session: Option, ) -> (String, Result) { + self.coerce_numeric_arguments(&mut tool_call).await; if tool_call.name == PLATFORM_MANAGE_SCHEDULE_TOOL_NAME { let arguments = tool_call .arguments From 4c37d3188174aa5ad6b00308ba8e3fcf3c35eec9 Mon Sep 17 00:00:00 2001 From: Tyler Longwell Date: Wed, 5 Nov 2025 10:29:08 -0500 Subject: [PATCH 2/2] move coercion per review --- crates/goose/src/agents/agent.rs | 55 +--------------- crates/goose/src/agents/reply_parts.rs | 90 +++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 55 deletions(-) diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 0e38e2eb2fb2..8dc45289c39a 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -49,7 +49,7 @@ use rmcp::model::{ CallToolRequestParam, Content, ErrorCode, ErrorData, GetPromptResult, Prompt, ServerNotification, Tool, }; -use serde_json::{json, Value}; +use serde_json::Value; use tokio::sync::{mpsc, Mutex}; use tokio_util::sync::CancellationToken; use tracing::{debug, error, info, instrument, warn}; @@ -383,66 +383,15 @@ impl Agent { sub_recipe_manager.add_sub_recipe_tools(sub_recipes); } - /// Apply schema-aware numeric coercion to tool arguments - /// Converts string representations of numbers to actual numbers based on the tool's schema - async fn coerce_numeric_arguments(&self, tool_call: &mut CallToolRequestParam) { - let Some(ref mut args) = tool_call.arguments else { - return; - }; - - let tools = self.list_tools(None).await; - let Some(tool_schema) = tools - .iter() - .find(|t| t.name == tool_call.name) - .map(|t| &t.input_schema) - else { - return; - }; - - let Some(properties) = tool_schema.get("properties").and_then(|p| p.as_object()) else { - return; - }; - - for (key, value) in args.iter_mut() { - let Value::String(s) = value else { - continue; - }; - - let Some(prop_schema) = properties.get(key) else { - continue; - }; - - let should_be_number = match prop_schema.get("type") { - Some(Value::String(t)) => t == "number" || t == "integer", - Some(Value::Array(types)) => types - .iter() - .any(|t| t.as_str().is_some_and(|s| s == "number" || s == "integer")), - _ => false, - }; - - if should_be_number { - if let Ok(n) = s.parse::() { - // Preserve integer types when possible - *value = if n.fract() == 0.0 && n >= i64::MIN as f64 && n <= i64::MAX as f64 { - json!(n as i64) - } else { - json!(n) - }; - } - } - } - } - /// Dispatch a single tool call to the appropriate client #[instrument(skip(self, tool_call, request_id), fields(input, output))] pub async fn dispatch_tool_call( &self, - mut tool_call: CallToolRequestParam, + tool_call: CallToolRequestParam, request_id: String, cancellation_token: Option, session: Option, ) -> (String, Result) { - self.coerce_numeric_arguments(&mut tool_call).await; if tool_call.name == PLATFORM_MANAGE_SCHEDULE_TOOL_NAME { let arguments = tool_call .arguments diff --git a/crates/goose/src/agents/reply_parts.rs b/crates/goose/src/agents/reply_parts.rs index 1d8fdaea06b4..7e33de435e00 100644 --- a/crates/goose/src/agents/reply_parts.rs +++ b/crates/goose/src/agents/reply_parts.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use async_stream::try_stream; use futures::stream::StreamExt; +use serde_json::{json, Value}; use tracing::debug; use super::super::agents::Agent; @@ -19,6 +20,79 @@ use crate::agents::recipe_tools::dynamic_task_tools::should_enabled_subagents; use crate::session::SessionManager; use rmcp::model::Tool; +fn coerce_value(s: &str, schema: &Value) -> Value { + let type_str = schema.get("type"); + + match type_str { + Some(Value::String(t)) => match t.as_str() { + "number" | "integer" => try_coerce_number(s), + "boolean" => try_coerce_boolean(s), + _ => Value::String(s.to_string()), + }, + Some(Value::Array(types)) => { + // Try each type in order + for t in types { + if let Value::String(type_name) = t { + match type_name.as_str() { + "number" | "integer" if s.parse::().is_ok() => { + return try_coerce_number(s) + } + "boolean" if matches!(s.to_lowercase().as_str(), "true" | "false") => { + return try_coerce_boolean(s) + } + _ => continue, + } + } + } + Value::String(s.to_string()) + } + _ => Value::String(s.to_string()), + } +} + +fn try_coerce_number(s: &str) -> Value { + if let Ok(n) = s.parse::() { + if n.fract() == 0.0 && n >= i64::MIN as f64 && n <= i64::MAX as f64 { + json!(n as i64) + } else { + json!(n) + } + } else { + Value::String(s.to_string()) + } +} + +fn try_coerce_boolean(s: &str) -> Value { + match s.to_lowercase().as_str() { + "true" => json!(true), + "false" => json!(false), + _ => Value::String(s.to_string()), + } +} + +fn coerce_tool_arguments( + arguments: Option>, + tool_schema: &Value, +) -> Option> { + let args = arguments?; + + let properties = tool_schema.get("properties").and_then(|p| p.as_object())?; + + let mut coerced = serde_json::Map::new(); + + for (key, value) in args.iter() { + let coerced_value = + if let (Value::String(s), Some(prop_schema)) = (value, properties.get(key)) { + coerce_value(s, prop_schema) + } else { + value.clone() + }; + coerced.insert(key.clone(), coerced_value); + } + + Some(coerced) +} + async fn toolshim_postprocess( response: Message, toolshim_tools: &[Tool], @@ -190,13 +264,25 @@ impl Agent { &self, response: &Message, ) -> (Vec, Vec, Message) { - // First collect all tool requests + let tools = self.list_tools(None).await; + + // First collect all tool requests with coercion applied let tool_requests: Vec = response .content .iter() .filter_map(|content| { if let MessageContent::ToolRequest(req) = content { - Some(req.clone()) + let mut coerced_req = req.clone(); + + if let Ok(ref mut tool_call) = coerced_req.tool_call { + if let Some(tool) = tools.iter().find(|t| t.name == tool_call.name) { + let schema_value = Value::Object(tool.input_schema.as_ref().clone()); + tool_call.arguments = + coerce_tool_arguments(tool_call.arguments.clone(), &schema_value); + } + } + + Some(coerced_req) } else { None }