From a1711e75e224cf2487e8929384d91d5d2511ac19 Mon Sep 17 00:00:00 2001 From: lunchboxfortwo Date: Mon, 29 Jun 2026 00:08:19 -0400 Subject: [PATCH] fix(providers): reject non-object tool-call arguments instead of panicking (#9832) Signed-off-by: Lunchbox Signed-off-by: Douwe M Osinga Signed-off-by: Goose Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Douwe M Osinga Co-authored-by: Goose Co-authored-by: Michael Neale --- .../goose-providers/src/formats/anthropic.rs | 48 ++++- crates/goose-providers/src/formats/openai.rs | 192 +++++++++++++++++- crates/goose/src/agents/agent.rs | 102 +++++++--- crates/goose/src/providers/formats/bedrock.rs | 34 ++++ .../goose/src/providers/formats/databricks.rs | 112 +++++++++- crates/goose/tests/agent.rs | 182 +++++++++++++++++ 6 files changed, 625 insertions(+), 45 deletions(-) diff --git a/crates/goose-providers/src/formats/anthropic.rs b/crates/goose-providers/src/formats/anthropic.rs index d26ba4d09..163c7f318 100644 --- a/crates/goose-providers/src/formats/anthropic.rs +++ b/crates/goose-providers/src/formats/anthropic.rs @@ -201,8 +201,16 @@ fn format_messages_with_options( })); } Err(_tool_error) => { - // Skip malformed tool requests - they shouldn't be sent to Anthropic - // This maintains the existing behavior for ToolRequest errors + // The paired tool response carries the parse error and + // serializes to a tool_result below; Anthropic rejects a + // tool_result without a preceding tool_use, so emit a + // placeholder tool_use with the same id to keep history valid. + content.push(json!({ + TYPE_FIELD: TOOL_USE_TYPE, + ID_FIELD: tool_request.id, + NAME_FIELD: "unparseable_tool_call", + INPUT_FIELD: json!({}) + })); } } } @@ -1572,6 +1580,42 @@ mod tests { assert!(!value.is_null()); } + #[test] + fn test_unparseable_tool_request_emits_placeholder_tool_use() { + use rmcp::model::{ErrorCode, ErrorData}; + + let err = ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Tool arguments for id call_bad must be a JSON object".to_string(), + None, + ); + let mut response = Message::user(); + response.add_tool_response_with_metadata("call_bad", Err(err.clone()), None); + let messages = vec![ + Message::assistant().with_tool_request("call_bad", Err(err)), + response, + ]; + + let spec = format_messages(&messages); + + let mut open = std::collections::HashSet::new(); + for m in &spec { + for block in m["content"].as_array().into_iter().flatten() { + match block["type"].as_str() { + Some("tool_use") => { + open.insert(block["id"].as_str().unwrap().to_string()); + } + Some("tool_result") => { + let id = block["tool_use_id"].as_str().unwrap(); + assert!(open.contains(id), "orphan tool_result for id {id:?}"); + } + _ => {} + } + } + } + assert!(open.contains("call_bad")); + } + #[test] fn test_args_to_input_value_preserves_existing_args() { let args = object!({"query": "rust"}); diff --git a/crates/goose-providers/src/formats/openai.rs b/crates/goose-providers/src/formats/openai.rs index 4c4e8acc9..0dd54b707 100644 --- a/crates/goose-providers/src/formats/openai.rs +++ b/crates/goose-providers/src/formats/openai.rs @@ -41,6 +41,17 @@ where Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) } +fn describe_json_value(value: &Value) -> &'static str { + match value { + Value::Array(_) => "an array", + Value::String(_) => "a string", + Value::Number(_) => "a number", + Value::Bool(_) => "a boolean", + Value::Null => "null", + Value::Object(_) => "an object", + } +} + fn is_reserved_request_param_key(key: &str) -> bool { matches!(key, "messages" | "model" | "stream" | "stream_options") } @@ -285,11 +296,27 @@ pub fn format_messages_with_options( tool_calls.as_array_mut().unwrap().push(tool_call_json); } - Err(e) => { - output.push(json!({ - "role": "tool", - "content": format!("Error: {}", e), - "tool_call_id": request.id + Err(_e) => { + // An unparseable tool call still needs a valid assistant + // `tool_calls` entry. Emitting the error as a bare `role:"tool"` + // message (the old behavior) leaves the paired tool response — + // which carries the parse error — as an orphan `role:"tool"` with + // no preceding assistant `tool_calls`, which strict + // OpenAI-compatible APIs reject. Emit a placeholder call with the + // same id so the history stays well-formed; the error rides on the + // following tool response. + let tool_calls = converted + .as_object_mut() + .unwrap() + .entry("tool_calls") + .or_insert(json!([])); + tool_calls.as_array_mut().unwrap().push(json!({ + "id": request.id, + "type": "function", + "function": { + "name": "unparseable_tool_call", + "arguments": "{}", + } })); } }, @@ -698,7 +725,7 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { )); } else { match parse_tool_arguments(&arguments_str) { - Some(params) => { + Some(params) if params.is_object() => { content.push(MessageContent::tool_request_with_metadata( id, Ok(CallToolRequestParams::new(function_name) @@ -706,6 +733,28 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { metadata.as_ref(), )); } + // Valid JSON but NOT an object (a bare array/string/number). + // Weaker models emit this; surface a tool error so the model + // retries with a proper object instead of crashing the run + // (rmcp's `object()` debug-asserts on non-objects). + Some(other) => { + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Tool arguments for {} (id {}) must be a JSON object, got {}. Raw arguments: '{}'", + function_name, + id, + describe_json_value(&other), + arguments_str + )), + data: None, + }; + content.push(MessageContent::tool_request_with_metadata( + id, + Err(error), + metadata.as_ref(), + )); + } None => { let message_text = truncation_error_message(&arguments_str) .unwrap_or_else(|| { @@ -1150,11 +1199,26 @@ where ) } else { match parse_tool_arguments(arguments) { - Some(params) => MessageContent::tool_request_with_metadata( + Some(params) if params.is_object() => MessageContent::tool_request_with_metadata( id.clone(), Ok(CallToolRequestParams::new(function_name.clone()).with_arguments(object(params))), metadata.as_ref(), ), + // Valid JSON but NOT an object (a bare array/string/number). + // Surface a tool error so the model retries instead of + // crashing the run (rmcp's `object()` debug-asserts on + // non-objects). Mirrors the non-streaming decoder. + Some(other) => { + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Tool arguments for {} (id {}) must be a JSON object, got {}. Raw arguments: '{}'", + function_name, id, describe_json_value(&other), arguments + )), + data: None, + }; + MessageContent::tool_request_with_metadata(id.clone(), Err(error), metadata.as_ref()) + } None => { let message_text = truncation_error_message(arguments) .unwrap_or_else(|| { @@ -1990,6 +2054,40 @@ mod tests { Ok(()) } + #[test] + fn test_response_to_message_non_object_arguments() -> anyhow::Result<()> { + // Weaker models sometimes emit tool arguments that are valid JSON but + // not an object (here, a bare array). This must surface as a tool error, + // NOT panic via rmcp's `object()` debug-assert. + let mut response: Value = serde_json::from_str(OPENAI_TOOL_USE_RESPONSE)?; + response["choices"][0]["message"]["tool_calls"][0]["function"]["arguments"] = + json!("[1, 2, 3]"); + + let message = response_to_message(&response)?; + + if let MessageContent::ToolRequest(request) = &message.content[0] { + match &request.tool_call { + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: msg, + data: None, + }) => { + assert!(msg.contains("must be a JSON object")); + assert!(msg.contains("an array")); + assert!( + msg.contains("example_fn"), + "error must name the original tool so the model can retry it: {msg}" + ); + } + _ => panic!("Expected InvalidParameters error for non-object args"), + } + } else { + panic!("Expected ToolRequest content"); + } + + Ok(()) + } + #[test] fn test_response_to_message_empty_argument() -> anyhow::Result<()> { let mut response: Value = serde_json::from_str(OPENAI_TOOL_USE_RESPONSE)?; @@ -2778,6 +2876,42 @@ data: [DONE]"#; panic!("Expected tool call message with nested extra_content metadata"); } + #[tokio::test] + async fn test_streaming_non_object_arguments_does_not_panic() -> anyhow::Result<()> { + // Streamed tool call whose arguments are valid JSON but NOT an object. + // Must yield an INVALID_PARAMS tool error, not panic via rmcp `object()`. + let response_lines = r#"data: {"model":"test-model","choices":[{"delta":{"role":"assistant","tool_calls":[{"id":"call_bad","function":{"name":"test_tool","arguments":"[1, 2, 3]"},"type":"function","index":0}]},"index":0,"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":100,"completion_tokens":10,"total_tokens":110},"object":"chat.completion.chunk","id":"test-id","created":1234567890} +data: [DONE]"#; + + let response_stream = + tokio_stream::iter(response_lines.lines().map(|line| Ok(line.to_string()))); + let messages = response_to_streaming_message(response_stream); + pin!(messages); + + while let Some(Ok((message, _usage))) = messages.next().await { + if let Some(msg) = message { + if let MessageContent::ToolRequest(request) = &msg.content[0] { + match &request.tool_call { + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: m, + .. + }) => { + assert!(m.contains("must be a JSON object")); + assert!( + m.contains("test_tool"), + "error must name the original tool so the model can retry it: {m}" + ); + return Ok(()); + } + _ => panic!("expected INVALID_PARAMS for non-object streamed args"), + } + } + } + } + panic!("expected a tool request message"); + } + #[tokio::test] async fn test_streaming_response_extracts_inline_think_blocks() -> anyhow::Result<()> { let response_lines = concat!( @@ -3833,4 +3967,48 @@ data: [DONE]"#; assert!(!is_valid_function_name("hello world")); assert!(!is_valid_function_name("hello@world")); } + + #[test] + fn formatter_post_parse_error_history_is_wellformed() { + use rmcp::model::{ErrorCode, ErrorData}; + let err = ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Tool arguments for id call_bad must be a JSON object".to_string(), + None, + ); + // Shape the agent loop builds today for a failed parse: + let request_msg = Message::assistant().with_tool_request("call_bad", Err(err.clone())); + let mut final_resp = Message::user(); + final_resp.add_tool_response_with_metadata("call_bad", Err(err), None); + let messages = vec![ + Message::user().with_text("do the thing"), + request_msg, + final_resp, + ]; + + let spec = format_messages(&messages, &ImageFormat::OpenAi); + + let mut open = std::collections::HashSet::new(); + for m in &spec { + match m.get("role").and_then(|v| v.as_str()) { + Some("assistant") => { + for tc in m + .get("tool_calls") + .and_then(|v| v.as_array()) + .into_iter() + .flatten() + { + if let Some(id) = tc.get("id").and_then(|v| v.as_str()) { + open.insert(id.to_string()); + } + } + } + Some("tool") => { + let id = m.get("tool_call_id").and_then(|v| v.as_str()).unwrap_or(""); + assert!(open.contains(id), "orphan role:tool message for id {id:?}"); + } + _ => {} + } + } + } } diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 0cdbe9f48..de29dfedc 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -2093,6 +2093,13 @@ impl Agent { } if goose_mode == GooseMode::Chat { for request in remaining_requests.iter() { + // An unparseable tool call should surface the parse error + // (added in the Err branch below), not a successful skip — + // otherwise the model sees a malformed call as "skipped OK" + // and can't correct the arguments. + if request.tool_call.is_err() { + continue; + } if let Some(response) = request_to_response_map.get_mut(&request.id) { response.add_tool_response_with_metadata( request.id.clone(), @@ -2289,40 +2296,71 @@ impl Agent { }; for request in frontend_requests.iter().chain(remaining_requests.iter()) { - if let Err(err) = &request.tool_call { - let err_msg = err.message.to_string(); - error!("Tool call could not be parsed: {}", err_msg); - yield AgentEvent::Message( - Message::assistant().with_text(err_msg) - ); - exit_chat = true; - break; - } else { - let mut request_msg = Message::assistant() - .with_id(format!("msg_{}", Uuid::new_v4())); + let mut request_msg = Message::assistant() + .with_id(format!("msg_{}", Uuid::new_v4())); - for thinking in &response_thinking { - request_msg = request_msg.with_content(thinking.clone()); - } - - request_msg = request_msg - .with_tool_request_with_metadata( - request.id.clone(), - request.tool_call.clone(), - request.metadata.as_ref(), - request.tool_meta.clone(), - ); - let final_response = request_to_response_map - .remove(&request.id) - .unwrap_or_else(|| Message::user().with_generated_id()); - // Response placeholder is created before tools run, so clamp request to avoid inverted ordering. - if request_msg.created > final_response.created { - request_msg.created = final_response.created; - } - messages_to_add.push(request_msg); - yield AgentEvent::Message(final_response.clone()); - messages_to_add.push(final_response); + for thinking in &response_thinking { + request_msg = request_msg.with_content(thinking.clone()); } + + // For an unparseable tool call (Err), store a valid + // placeholder Ok tool-call in history instead of the Err. This + // keeps the conversation well-formed through EVERY provider + // formatter's normal Ok path — so we don't have to special-case + // each formatter's Err arm — and preserves provider metadata + // (e.g. thought signatures), which is passed through below and + // copied by the Ok path. The actual parse error rides on the + // paired tool response. + let history_tool_call = match &request.tool_call { + Ok(_) => request.tool_call.clone(), + Err(_) => Ok(CallToolRequestParams::new( + "unparseable_tool_call", + ) + .with_arguments(serde_json::Map::new())), + }; + request_msg = request_msg + .with_tool_request_with_metadata( + request.id.clone(), + history_tool_call, + request.metadata.as_ref(), + request.tool_meta.clone(), + ); + + let final_response = match &request.tool_call { + Ok(_) => request_to_response_map + .remove(&request.id) + .unwrap_or_else(|| Message::user().with_generated_id()), + Err(error) => { + error!("Tool call could not be parsed: {error}"); + let mut response = request_to_response_map + .remove(&request.id) + .unwrap_or_else(|| Message::user().with_generated_id()); + // Only feed the parse error back if this id isn't + // already answered. In Chat mode the skip branch above + // already added a tool response for it; adding another + // here would duplicate the tool_call_id (which strict + // providers reject). + let already_answered = response.content.iter().any(|c| { + matches!(c, MessageContent::ToolResponse(r) if r.id == request.id) + }); + if !already_answered { + response.add_tool_response_with_metadata( + request.id.clone(), + Err(error.clone()), + request.metadata.as_ref(), + ); + } + response + } + }; + + // Response placeholder is created before tools run, so clamp request to avoid inverted ordering. + if request_msg.created > final_response.created { + request_msg.created = final_response.created; + } + messages_to_add.push(request_msg); + yield AgentEvent::Message(final_response.clone()); + messages_to_add.push(final_response); } no_tools_called = false; diff --git a/crates/goose/src/providers/formats/bedrock.rs b/crates/goose/src/providers/formats/bedrock.rs index 08a225c9c..b1b512dc7 100644 --- a/crates/goose/src/providers/formats/bedrock.rs +++ b/crates/goose/src/providers/formats/bedrock.rs @@ -150,8 +150,14 @@ pub fn to_bedrock_message_content(content: &MessageContent) -> Result Result Result<()> { + use rmcp::model::{ErrorCode, ErrorData}; + // An unparseable tool call (ToolRequest(Err)) must still produce a tool_use + // with a non-empty name; otherwise Bedrock rejects the tool_use / orphans the + // paired tool_result. Mirrors the OpenAI/Databricks/Anthropic formatters. + let err = ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Tool arguments must be a JSON object".to_string(), + None, + ); + let content = MessageContent::tool_request("call_bad".to_string(), Err(err)); + match to_bedrock_message_content(&content)? { + bedrock::ContentBlock::ToolUse(tu) => { + assert_eq!(tu.tool_use_id, "call_bad"); + assert_eq!(tu.name, "unparseable_tool_call"); + } + other => panic!("expected ToolUse, got {other:?}"), + } + Ok(()) + } + #[test] fn test_cache_points_with_tool_response_messages() -> Result<()> { use chrono::Utc; diff --git a/crates/goose/src/providers/formats/databricks.rs b/crates/goose/src/providers/formats/databricks.rs index ce63261db..8c675f521 100644 --- a/crates/goose/src/providers/formats/databricks.rs +++ b/crates/goose/src/providers/formats/databricks.rs @@ -189,9 +189,21 @@ fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec { - content_array - .push(json!({"type": "text", "text": format!("Error: {}", e)})); + Err(_e) => { + // Mirror the OpenAI formatter: emitting the error as assistant + // text leaves no `tool_calls` entry, so the paired tool response + // orphans (a `role:"tool"` with no preceding assistant + // `tool_calls`) and strict APIs reject it. Emit a placeholder + // call with the same id; the error rides on the tool response. + let tool_calls = converted.tool_calls.get_or_insert_default(); + tool_calls.push(json!({ + "id": request.id, + "type": "function", + "function": { + "name": "unparseable_tool_call", + "arguments": "{}", + } + })); } } } @@ -423,13 +435,27 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { content.push(MessageContent::tool_request(id, Err(error))); } else { match goose_providers::json::parse_tool_arguments(&arguments_str) { - Some(params) => { + Some(params) if params.is_object() => { content.push(MessageContent::tool_request( id, Ok(CallToolRequestParams::new(function_name) .with_arguments(object(params))), )); } + // Valid JSON but NOT an object (a bare array/string/number). + // Surface a tool error so the model retries instead of + // crashing the run (rmcp's `object()` debug-asserts). + Some(_) => { + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!( + "Tool arguments for {} (id {}) must be a JSON object. Raw arguments: '{}'", + function_name, id, arguments_str + )), + data: None, + }; + content.push(MessageContent::tool_request(id, Err(error))); + } None => { let message_text = goose_providers::json::truncation_error_message(&arguments_str) @@ -1039,6 +1065,39 @@ mod tests { Ok(()) } + #[test] + fn test_response_to_message_non_object_arguments() -> anyhow::Result<()> { + // Weaker models sometimes emit tool arguments that are valid JSON but + // not an object (here, a bare array). This must surface as a tool error, + // NOT panic via rmcp's `object()` debug-assert. + let mut response: Value = serde_json::from_str(OPENAI_TOOL_USE_RESPONSE)?; + response["choices"][0]["message"]["tool_calls"][0]["function"]["arguments"] = + json!("[1, 2, 3]"); + + let message = response_to_message(&response)?; + + if let MessageContent::ToolRequest(request) = &message.content[0] { + match &request.tool_call { + Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: msg, + data: None, + }) => { + assert!(msg.contains("must be a JSON object")); + assert!( + msg.contains("example_fn"), + "error must name the original tool so the model can retry it: {msg}" + ); + } + _ => panic!("Expected InvalidParameters error for non-object args"), + } + } else { + panic!("Expected ToolRequest content"); + } + + Ok(()) + } + #[test] fn test_response_to_message_empty_argument() -> anyhow::Result<()> { let mut response: Value = serde_json::from_str(OPENAI_TOOL_USE_RESPONSE)?; @@ -1429,6 +1488,51 @@ mod tests { Ok(()) } + #[test] + fn format_messages_post_parse_error_history_is_wellformed() -> anyhow::Result<()> { + // An unparseable tool call (ToolRequest(Err)) paired with its error tool + // response must not serialize as an orphan role:"tool" message. + use rmcp::model::{ErrorCode, ErrorData}; + let err = ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Tool arguments for id call_bad must be a JSON object".to_string(), + None, + ); + let request_msg = Message::assistant().with_tool_request("call_bad", Err(err.clone())); + let mut final_resp = Message::user(); + final_resp.add_tool_response_with_metadata("call_bad", Err(err), None); + let messages = vec![ + Message::user().with_text("do the thing"), + request_msg, + final_resp, + ]; + + let spec = serde_json::to_value(format_messages(&messages, &ImageFormat::OpenAi))?; + let mut open = std::collections::HashSet::new(); + for m in spec.as_array().unwrap() { + match m.get("role").and_then(|v| v.as_str()) { + Some("assistant") => { + for tc in m + .get("tool_calls") + .and_then(|v| v.as_array()) + .into_iter() + .flatten() + { + if let Some(id) = tc.get("id").and_then(|v| v.as_str()) { + open.insert(id.to_string()); + } + } + } + Some("tool") => { + let id = m.get("tool_call_id").and_then(|v| v.as_str()).unwrap_or(""); + assert!(open.contains(id), "orphan role:tool message for id {id:?}"); + } + _ => {} + } + } + Ok(()) + } + #[test] fn test_format_messages_tool_request_with_some_arguments() -> anyhow::Result<()> { // Test that tool calls with Some arguments are properly JSON-serialized diff --git a/crates/goose/tests/agent.rs b/crates/goose/tests/agent.rs index 2738c3fec..140d2b981 100644 --- a/crates/goose/tests/agent.rs +++ b/crates/goose/tests/agent.rs @@ -629,6 +629,188 @@ mod tests { } } + #[cfg(test)] + mod unparseable_tool_call_tests { + use super::*; + use async_trait::async_trait; + use goose::agents::{AgentConfig, SessionConfig}; + use goose::config::permission::PermissionManager; + use goose::config::GooseMode; + use goose::conversation::message::{Message, MessageContent}; + use goose::providers::base::{ + stream_from_single_message, MessageStream, Provider, ProviderDef, ProviderMetadata, + }; + use goose::session::session_manager::SessionType; + use goose::session::SessionManager; + use goose_providers::conversation::token_usage::{ProviderUsage, Usage}; + use goose_providers::errors::ProviderError; + use goose_providers::model::ModelConfig; + use rmcp::model::{ErrorCode, ErrorData, Tool}; + use std::path::PathBuf; + use std::sync::atomic::{AtomicUsize, Ordering}; + use tempfile::TempDir; + + /// First turn returns a tool request that failed to parse (mirroring what + /// the decoders emit for non-object arguments), subsequent turns return + /// plain text so the loop can finish. + struct UnparseableToolProvider { + call_count: AtomicUsize, + } + + impl UnparseableToolProvider { + fn new() -> Self { + Self { + call_count: AtomicUsize::new(0), + } + } + } + + impl goose::providers::base::ProviderDescriptor for UnparseableToolProvider { + fn metadata() -> ProviderMetadata { + ProviderMetadata { + name: "mock-unparseable".to_string(), + display_name: "Mock Unparseable Provider".to_string(), + description: "Mock provider for unparseable tool call tests".to_string(), + default_model: "mock-model".to_string(), + known_models: vec![], + model_doc_link: "".to_string(), + config_keys: vec![], + setup_steps: vec![], + model_selection_hint: None, + fast_model: None, + } + } + } + + impl ProviderDef for UnparseableToolProvider { + type Provider = Self; + + fn from_env( + _extensions: Vec, + _tls_config: Option, + ) -> futures::future::BoxFuture<'static, anyhow::Result> { + Box::pin(async { Ok(Self::new()) }) + } + } + + #[async_trait] + impl Provider for UnparseableToolProvider { + async fn stream( + &self, + _model_config: &ModelConfig, + _session_id: &str, + _system_prompt: &str, + _messages: &[Message], + _tools: &[Tool], + ) -> Result { + let n = self.call_count.fetch_add(1, Ordering::SeqCst); + let message = if n == 0 { + let error = ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Tool arguments must be a JSON object".to_string(), + None, + ); + Message::assistant().with_tool_request("call_bad", Err(error)) + } else { + Message::assistant().with_text("Recovered after the bad tool call.") + }; + + let usage = ProviderUsage::new( + "mock-model".to_string(), + Usage::new(Some(10), Some(5), Some(15)), + ); + Ok(stream_from_single_message(message, usage)) + } + + fn get_name(&self) -> &str { + "mock-unparseable" + } + } + + /// An unparseable tool call should be fed back to the model as a tool + /// response error so it can retry, rather than terminating the run. + #[tokio::test] + async fn test_unparseable_tool_call_feeds_back_and_continues() -> Result<()> { + let temp_dir = TempDir::new().unwrap(); + let data_dir = temp_dir.path().to_path_buf(); + let session_manager = Arc::new(SessionManager::new(data_dir.clone())); + let agent = Agent::with_config(AgentConfig::new( + session_manager.clone(), + Arc::new(PermissionManager::new(data_dir)), + None, + GooseMode::default(), + false, + GoosePlatform::GooseCli, + )); + let provider = Arc::new(UnparseableToolProvider::new()); + + let session = session_manager + .create_session( + PathBuf::default(), + "unparseable-tool-test".to_string(), + SessionType::Hidden, + GooseMode::default(), + ) + .await?; + + agent + .update_provider( + provider.clone(), + ModelConfig::new("mock-model"), + &session.id, + ) + .await?; + + let session_config = SessionConfig { + id: session.id, + schedule_id: None, + max_turns: Some(5), + retry_config: None, + }; + + let reply_stream = agent + .reply(Message::user().with_text("Hello"), session_config, None) + .await?; + tokio::pin!(reply_stream); + + let mut saw_tool_response_error = false; + let mut saw_recovery_text = false; + while let Some(event) = reply_stream.next().await { + if let Ok(AgentEvent::Message(message)) = event { + for content in &message.content { + match content { + MessageContent::ToolResponse(response) + if response.id == "call_bad" && response.tool_result.is_err() => + { + saw_tool_response_error = true; + } + MessageContent::Text(text) + if text.text.contains("Recovered after the bad tool call") => + { + saw_recovery_text = true; + } + _ => {} + } + } + } + } + + assert!( + saw_tool_response_error, + "expected an error tool response fed back to the model for the unparseable call" + ); + assert!( + saw_recovery_text, + "expected the loop to continue to a second provider turn instead of terminating" + ); + assert!( + provider.call_count.load(Ordering::SeqCst) >= 2, + "provider should have been called again after the bad tool call" + ); + Ok(()) + } + } + #[cfg(test)] mod tool_pair_summarization_tests { use super::*;