diff --git a/crates/goose-providers/src/formats/openai.rs b/crates/goose-providers/src/formats/openai.rs index a824e56a6653..4b265c11ac6a 100644 --- a/crates/goose-providers/src/formats/openai.rs +++ b/crates/goose-providers/src/formats/openai.rs @@ -2,7 +2,7 @@ use crate::conversation::message::{Message, MessageContent, ProviderMetadata}; use crate::conversation::token_usage::{ProviderUsage, Usage}; use crate::errors::ProviderError; use crate::images::{convert_image, detect_image_path, load_image_file, ImageFormat}; -use crate::json::safely_parse_json; +use crate::json::{parse_tool_arguments, truncation_error_message}; use crate::mcp_utils::extract_text_from_resource; use crate::model::ModelConfig; use crate::thinking::{ @@ -653,8 +653,8 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { metadata.as_ref(), )); } else { - match safely_parse_json(&arguments_str) { - Ok(params) => { + match parse_tool_arguments(&arguments_str) { + Some(params) => { content.push(MessageContent::tool_request_with_metadata( id, Ok(CallToolRequestParams::new(function_name) @@ -662,13 +662,14 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { metadata.as_ref(), )); } - Err(e) => { + None => { + let message_text = truncation_error_message(&arguments_str) + .unwrap_or_else(|| { + format!("Could not interpret tool use parameters for id {id}") + }); let error = ErrorData { code: ErrorCode::INVALID_PARAMS, - message: Cow::from(format!( - "Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'", - id, e, arguments_str - )), + message: Cow::from(message_text), data: None, }; content.push(MessageContent::tool_request_with_metadata( @@ -1085,12 +1086,6 @@ where for index in sorted_indices { if let Some((id, function_name, arguments, extra_fields)) = tool_call_data.get(&index) { - let parsed = if arguments.is_empty() { - Ok(json!({})) - } else { - safely_parse_json(arguments) - }; - let metadata = if let Some(sig) = &last_signature { let mut combined = extra_fields.clone().unwrap_or_default(); combined.insert( @@ -1102,26 +1097,34 @@ where extra_fields.as_ref().filter(|m| !m.is_empty()).cloned() }; - let content = match parsed { - Ok(params) => { - MessageContent::tool_request_with_metadata( + let content = if arguments.is_empty() { + MessageContent::tool_request_with_metadata( + id.clone(), + Ok(CallToolRequestParams::new(function_name.clone()).with_arguments(object(json!({})))), + metadata.as_ref(), + ) + } else { + match parse_tool_arguments(arguments) { + Some(params) => MessageContent::tool_request_with_metadata( id.clone(), Ok(CallToolRequestParams::new(function_name.clone()).with_arguments(object(params))), metadata.as_ref(), - ) - }, - Err(e) => { - let error = ErrorData { - code: ErrorCode::INVALID_PARAMS, - message: Cow::from(format!( - "Could not interpret tool use parameters for id {}: {}", - id, e - )), - 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(|| { + format!("Could not interpret tool use parameters for id {id}") + }); + let error = ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(message_text), + data: None, + }; + MessageContent::tool_request_with_metadata(id.clone(), Err(error), metadata.as_ref()) + } } }; + contents.push(content); } } @@ -1934,7 +1937,7 @@ mod tests { message: msg, data: None, }) => { - assert!(msg.starts_with("Could not interpret tool use parameters")); + assert!(msg.contains("tool arguments") || msg.contains("truncated")); } _ => panic!("Expected InvalidParameters error"), } diff --git a/crates/goose-providers/src/json.rs b/crates/goose-providers/src/json.rs index c8a23418b242..c1a4baabd966 100644 --- a/crates/goose-providers/src/json.rs +++ b/crates/goose-providers/src/json.rs @@ -122,6 +122,121 @@ pub fn json_escape_control_chars_in_string(s: &str) -> String { r } +/// Detect whether a raw tool-arguments string looks truncated (the model hit +/// its output-token limit mid-JSON). Returns true when the string has +/// unbalanced or unclosed structural delimiters — whether the cut-off happened +/// mid-value (e.g. `{"path":"/a` with no closing quote) or after a nested +/// closer but before the outer object closed (e.g. `{"items":[1,2]` where the +/// outer `{` is still open). +pub fn looks_truncated(args: &str) -> bool { + let trimmed = args.trim_end(); + if trimmed.is_empty() { + return false; + } + + let mut in_string = false; + let mut escape_next = false; + let mut depth = Vec::new(); + + for c in trimmed.chars() { + if in_string { + if escape_next { + escape_next = false; + } else if c == '\\' { + escape_next = true; + } else if c == '"' { + in_string = false; + } + continue; + } + + match c { + '"' => in_string = true, + '{' => depth.push('}'), + '[' => depth.push(']'), + '}' | ']' => { + if depth.last() == Some(&c) { + depth.pop(); + } else { + return true; + } + } + _ => {} + } + } + + in_string || escape_next || !depth.is_empty() +} + +/// Build an actionable error message for tool arguments that could not be +/// parsed. `args` is the raw, accumulated arguments string from the provider. +/// +/// The message distinguishes truncation (likely from the output token limit) +/// from other malformation, and includes a snippet of where parsing broke. +pub fn truncation_error_message(args: &str) -> Option { + if args.is_empty() { + return None; + } + + if serde_json::from_str::(args).is_ok() { + return None; + } + + let trimmed = args.trim_end(); + let is_truncated = looks_truncated(trimmed); + + let snippet = { + let len = trimmed.chars().count(); + if len > 80 { + let s: String = trimmed + .chars() + .rev() + .take(80) + .collect::>() + .into_iter() + .rev() + .collect(); + format!("…{s}") + } else { + trimmed.to_string() + } + }; + + let guidance = if is_truncated { + "The model's response was truncated — it hit the output token limit while generating this tool call. \ + Try increasing max_tokens for this provider or breaking the task into smaller steps." + } else { + "The model produced malformed tool arguments. Try resending your message or breaking the task into smaller steps." + }; + + Some(format!( + "{guidance}\nReceived {} characters; cut off at: {snippet}", + trimmed.chars().count() + )) +} + +/// Parse tool-call arguments, returning `None` when the input looks truncated +/// so callers can surface an actionable error rather than invoking a tool with +/// incomplete arguments. Non-truncated malformation (e.g. unescaped control +/// characters some models emit) is still repaired via [`safely_parse_json`]. +pub fn parse_tool_arguments(args: &str) -> Option { + if args.is_empty() { + return Some(serde_json::Value::Object(serde_json::Map::new())); + } + + if let Ok(value) = serde_json::from_str::(args) { + return Some(value); + } + + if !looks_truncated(args) { + if let Ok(value) = safely_parse_json(args) { + return Some(value); + } + } + + None +} + #[cfg(test)] mod tests { use super::*; @@ -218,4 +333,96 @@ mod tests { "Hello\\u0001World" ); } + + #[test] + fn test_truncation_error_message_valid_json() { + assert!(truncation_error_message(r#"{"key":"value"}"#).is_none()); + assert!(truncation_error_message(r#"{}"#).is_none()); + assert!(truncation_error_message(r#"{"a":[1,2],"b":{"c":3}}"#).is_none()); + assert!(truncation_error_message(r#"[1,2,3]"#).is_none()); + assert!(truncation_error_message(r#"{"a":{"b":"c"}}"#).is_none()); + assert!(truncation_error_message("").is_none()); + } + + #[test] + fn test_looks_truncated_nested_closers() { + // Truncated after inner array closes, but outer object still open. + assert!(looks_truncated(r#"{"items":[1,2]"#)); + // Truncated after inner object closes, but outer object still open. + assert!(looks_truncated(r#"{"patch":{"path":"x"}"#)); + // Truncated mid-string. + assert!(looks_truncated( + r##"{"path":"/report.md","content":"# cut"## + )); + // Truncated mid-key. + assert!(looks_truncated(r#"{"key":"val"#)); + + // Well-formed JSON is NOT truncated. + assert!(!looks_truncated(r#"{"key":"value"}"#)); + assert!(!looks_truncated(r#"{"a":[1,2],"b":{"c":3}}"#)); + assert!(!looks_truncated(r#"[1,2,3]"#)); + assert!(!looks_truncated(r#"{"a":{"b":"c"}}"#)); + assert!(!looks_truncated(r#"{}"#)); + assert!(!looks_truncated("")); + } + + #[test] + fn test_parse_tool_arguments_nested_closers_truncated() { + // These end with ] or } so the old check passed, but the outer object + // is still open — silently repairing these would invoke tools with + // incomplete arguments. + let case1 = r#"{"items":[1,2]"#; + assert!(parse_tool_arguments(case1).is_none()); + + let case2 = r#"{"patch":{"path":"x"}"#; + assert!(parse_tool_arguments(case2).is_none()); + } + + #[test] + fn test_parse_tool_arguments_control_char_recovery() { + // Unescaped control chars (raw newline) inside a string value should + // still parse successfully via safely_parse_json fallback. + let args = "{\"key\": \"value\nwith newline\"}"; + let parsed = parse_tool_arguments(args).expect("control-char JSON should parse"); + assert_eq!(parsed["key"], "value\nwith newline"); + } + + #[test] + fn test_parse_tool_arguments_truncated_fails() { + let truncated = r##"{"path":"/report.md","content":"# Big report that got cut"##; + assert!( + parse_tool_arguments(truncated).is_none(), + "truncated JSON should NOT parse (would silently invoke tool with truncated content)" + ); + } + + #[test] + fn test_parse_tool_arguments_strict_json() { + let valid = r#"{"key":"value"}"#; + assert!(parse_tool_arguments(valid).is_some()); + assert!(parse_tool_arguments("").is_some()); + } + + #[test] + fn test_truncation_error_message_truncated() { + let truncated = r##"{"path":"/report.md","content":"# Big report that got cut"##; + let msg = + truncation_error_message(truncated).expect("truncated args should produce an error"); + assert!(msg.contains("truncated"), "msg: {msg}"); + assert!( + msg.contains("max_tokens") || msg.contains("smaller steps"), + "msg: {msg}" + ); + assert!(msg.contains("cut off at:"), "msg: {msg}"); + } + + #[test] + fn test_truncation_error_message_malformed() { + // Malformed JSON that ends with } (not truncated, just broken). + // safely_parse_json should fail too, so truncation_error_message fires. + let malformed = r##"{"key": }"##; + let msg = + truncation_error_message(malformed).expect("malformed args should produce an error"); + assert!(msg.contains("malformed"), "msg: {msg}"); + } } diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index c2299e83b268..107bce837b32 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -2194,7 +2194,15 @@ impl Agent { .collect(); for request in frontend_requests.iter().chain(remaining_requests.iter()) { - if request.tool_call.is_ok() { + 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())); @@ -2221,18 +2229,6 @@ impl Agent { messages_to_add.push(request_msg); yield AgentEvent::Message(final_response.clone()); messages_to_add.push(final_response); - } else { - error!( - "Tool call could not be parsed: {}", - request.tool_call.as_ref().unwrap_err(), - ); - yield AgentEvent::Message( - Message::assistant().with_text( - "A tool call could not be parsed — the response may have been truncated. Try breaking the task into smaller steps or resending your message." - ) - ); - exit_chat = true; - break; } } diff --git a/crates/goose/src/providers/formats/anthropic.rs b/crates/goose/src/providers/formats/anthropic.rs index fa5315b36b43..0af81a5b9930 100644 --- a/crates/goose/src/providers/formats/anthropic.rs +++ b/crates/goose/src/providers/formats/anthropic.rs @@ -752,6 +752,7 @@ where let mut final_usage: Option = None; let mut message_id: Option = None; let mut thinking: Option = None; + let mut stop_reason: Option = None; while let Some(line_result) = stream.next().await { let line = line_result?; @@ -878,18 +879,20 @@ where } } if let Some(tool_id) = current_tool_id.take() { - // Tool call finished, yield complete tool call if let Some((name, args)) = accumulated_tool_calls.remove(&tool_id) { let parsed_args = if args.is_empty() { json!({}) } else { - match serde_json::from_str::(&args) { - Ok(parsed) => parsed, - Err(_) => { - // If parsing fails, create an error tool request + match goose_providers::json::parse_tool_arguments(&args) { + Some(parsed) => parsed, + None => { + let message_text = goose_providers::json::truncation_error_message(&args) + .unwrap_or_else(|| { + format!("Could not parse tool arguments: {args}") + }); let error = ErrorData::new( ErrorCode::INVALID_PARAMS, - format!("Could not parse tool arguments: {}", args), + message_text, None, ); let mut message = Message::new( @@ -934,6 +937,11 @@ where } if let Some(delta) = event.data.get("delta") { let stop_details = delta.get("stop_details").filter(|d| !d.is_null()); + if stop_reason.is_none() { + if let Some(sr) = delta.get("stop_reason").and_then(|v| v.as_str()) { + stop_reason = Some(sr.to_string()); + } + } if delta.get("stop_reason").and_then(|v| v.as_str()) == Some(STOP_REASON_REFUSAL) { let str_field = |key: &str| stop_details .and_then(|d| d.get(key)) @@ -980,6 +988,38 @@ where } } + // A tool_use block left open at stream end never received its + // content_block_stop, so its args are truncated rather than complete. + if !accumulated_tool_calls.is_empty() { + let truncated_by_limit = stop_reason.as_deref() == Some("max_tokens"); + let mut ids: Vec = accumulated_tool_calls.keys().cloned().collect(); + ids.sort(); + for id in ids { + if let Some((_name, args)) = accumulated_tool_calls.remove(&id) { + let guidance = if truncated_by_limit { + "The model's response was truncated — it hit the output token limit while generating this tool call. \ + Try increasing max_tokens for this provider or breaking the task into smaller steps." + } else { + "A tool call was not completed before the stream ended. \ + Try resending your message or breaking the task into smaller steps." + }; + let snippet_len = args.chars().count(); + let tail: String = args.chars().rev().take(80).collect::>().into_iter().rev().collect(); + let message_text = format!( + "{guidance}\nReceived {snippet_len} characters of arguments; cut off at: …{tail}" + ); + let error = ErrorData::new(ErrorCode::INVALID_PARAMS, message_text, None); + let mut message = Message::new( + Role::Assistant, + chrono::Utc::now().timestamp(), + vec![MessageContent::tool_request(id, Err(error))], + ); + message.id = message_id.clone(); + yield (Some(message), None); + } + } + } + if let Some(usage) = final_usage { yield (None, Some(usage)); } @@ -1720,6 +1760,7 @@ mod tests { redacted_thinking: Vec, text: Vec, tool_calls: Vec, + tool_errors: Vec, } async fn collect_stream(events: &str) -> StreamedParts { @@ -1740,11 +1781,10 @@ mod tests { MessageContent::Text(t) => { parts.text.push(t.text.clone()); } - MessageContent::ToolRequest(req) => { - if let Ok(call) = &req.tool_call { - parts.tool_calls.push(call.name.to_string()); - } - } + MessageContent::ToolRequest(req) => match &req.tool_call { + Ok(call) => parts.tool_calls.push(call.name.to_string()), + Err(e) => parts.tool_errors.push(e.message.to_string()), + }, _ => {} } } @@ -2034,4 +2074,100 @@ mod tests { ); assert!(parts.text[0].contains("context_window")); } + + #[tokio::test] + async fn test_streaming_truncated_tool_args_in_content_block_stop() { + // Block is closed by content_block_stop, but the concatenated deltas form + // truncated JSON (each fragment is valid; together they're unterminated). + let events = concat!( + r##"data: {"type":"message_start","message":{"id":"msg_t","role":"assistant","content":[],"model":"glm-4.7","usage":{"input_tokens":10,"output_tokens":0}}}"##, + "\n", + r##"data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"tool_t","name":"write","input":{}}}"##, + "\n", + r#"data: {"type":"content_block_delta","index":0,"delta":{"type":"input_json_delta","partial_json":"{\"path\":\"/some/path.md\","}}"#, + "\n", + r##"data: {"type":"content_block_delta","index":0,"delta":{"type":"input_json_delta","partial_json":"\"content\":\"# Very long markdown"}}"##, + "\n", + r#"data: {"type":"content_block_stop","index":0}"#, + "\n", + r#"data: {"type":"message_delta","delta":{"stop_reason":"max_tokens"},"usage":{"output_tokens":4096}}"#, + "\n", + r#"data: {"type":"message_stop"}"#, + ); + + let parts = collect_stream(events).await; + assert_eq!( + parts.tool_errors.len(), + 1, + "expected one tool error, got: {:?}", + parts.tool_errors + ); + let msg = &parts.tool_errors[0]; + assert!( + msg.contains("truncated") || msg.contains("output token limit"), + "expected actionable truncation message, got: {}", + msg + ); + assert!( + msg.contains("max_tokens") || msg.contains("smaller steps"), + "expected guidance to increase max_tokens or break up the task, got: {}", + msg + ); + } + + #[tokio::test] + async fn test_streaming_truncated_tool_args_no_content_block_stop() { + // The stream ends with the tool_use block still open (no content_block_stop), + // which is what happens when the model is cut off mid-tool-call. + let events = concat!( + r##"data: {"type":"message_start","message":{"id":"msg_t2","role":"assistant","content":[],"model":"glm-4.7","usage":{"input_tokens":10,"output_tokens":0}}}"##, + "\n", + r##"data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"tool_t2","name":"write","input":{}}}"##, + "\n", + r#"data: {"type":"content_block_delta","index":0,"delta":{"type":"input_json_delta","partial_json":"{\"path\":\"/report.md\","}}"#, + "\n", + r##"data: {"type":"content_block_delta","index":0,"delta":{"type":"input_json_delta","partial_json":"\"content\":\"# Big report that got cut off mid"}}"##, + "\n", + r#"data: {"type":"message_delta","delta":{"stop_reason":"max_tokens"},"usage":{"output_tokens":8192}}"#, + "\n", + r#"data: {"type":"message_stop"}"#, + ); + + let parts = collect_stream(events).await; + assert_eq!( + parts.tool_errors.len(), + 1, + "expected one tool error for the dropped/truncated tool call, got: {:?}", + parts.tool_errors + ); + let msg = &parts.tool_errors[0]; + assert!( + msg.contains("truncated") || msg.contains("output token limit"), + "expected actionable truncation message, got: {}", + msg + ); + } + + #[tokio::test] + async fn test_streaming_complete_tool_call_unaffected() { + // Regression guard: a normal, complete tool call must still parse and + // produce no error even though stop_reason handling is added. + let events = concat!( + r#"data: {"type":"message_start","message":{"id":"msg_ok","role":"assistant","content":[],"model":"glm-4.7","usage":{"input_tokens":10,"output_tokens":0}}}"#, + "\n", + r#"data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"tool_ok","name":"write","input":{}}}"#, + "\n", + r#"data: {"type":"content_block_delta","index":0,"delta":{"type":"input_json_delta","partial_json":"{\"path\":\"/ok.md\",\"content\":\"hello\"}"}}"#, + "\n", + r#"data: {"type":"content_block_stop","index":0}"#, + "\n", + r#"data: {"type":"message_delta","delta":{"stop_reason":"tool_use"},"usage":{"output_tokens":15}}"#, + "\n", + r#"data: {"type":"message_stop"}"#, + ); + + let parts = collect_stream(events).await; + assert_eq!(parts.tool_calls, vec!["write"]); + assert!(parts.tool_errors.is_empty()); + } } diff --git a/crates/goose/src/providers/formats/databricks.rs b/crates/goose/src/providers/formats/databricks.rs index b86542c87eab..496aff4da445 100644 --- a/crates/goose/src/providers/formats/databricks.rs +++ b/crates/goose/src/providers/formats/databricks.rs @@ -11,7 +11,6 @@ use goose_providers::formats::openai::{ openai_reasoning_effort_for_thinking, sanitize_function_name, }; use goose_providers::images::{convert_image, detect_image_path, load_image_file, ImageFormat}; -use goose_providers::json::safely_parse_json; use rmcp::model::{ object, AnnotateAble, CallToolRequestParams, Content, ErrorCode, ErrorData, RawContent, ResourceContents, Role, Tool, @@ -423,21 +422,25 @@ pub fn response_to_message(response: &Value) -> anyhow::Result { }; content.push(MessageContent::tool_request(id, Err(error))); } else { - match safely_parse_json(&arguments_str) { - Ok(params) => { + match goose_providers::json::parse_tool_arguments(&arguments_str) { + Some(params) => { content.push(MessageContent::tool_request( id, Ok(CallToolRequestParams::new(function_name) .with_arguments(object(params))), )); } - Err(e) => { + None => { + let message_text = + goose_providers::json::truncation_error_message(&arguments_str) + .unwrap_or_else(|| { + format!( + "Could not interpret tool use parameters for id {id}" + ) + }); let error = ErrorData { code: ErrorCode::INVALID_PARAMS, - message: Cow::from(format!( - "Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'", - id, e, arguments_str - )), + message: Cow::from(message_text), data: None, }; content.push(MessageContent::tool_request(id, Err(error))); @@ -1025,7 +1028,7 @@ mod tests { message: msg, data: None, }) => { - assert!(msg.starts_with("Could not interpret tool use parameters")); + assert!(msg.contains("tool arguments") || msg.contains("truncated")); } _ => panic!("Expected InvalidParameters error"), }