Fix: surface actionable errors for truncated tool-call arguments (#9946)

Co-authored-by: goose <noreply@aaif.ai>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
Vincenzo Palazzo
2026-06-24 20:53:58 +02:00
committed by GitHub
parent 3a93f56671
commit 63d1456b3b
5 changed files with 408 additions and 63 deletions
+33 -30
View File
@@ -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<Message> {
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<Message> {
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);
}
}
@@ -1931,7 +1934,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"),
}
+207
View File
@@ -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<String> {
if args.is_empty() {
return None;
}
if serde_json::from_str::<serde_json::Value>(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::<Vec<_>>()
.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<serde_json::Value> {
if args.is_empty() {
return Some(serde_json::Value::Object(serde_json::Map::new()));
}
if let Ok(value) = serde_json::from_str::<serde_json::Value>(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}");
}
}
+9 -13
View File
@@ -2262,7 +2262,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()));
@@ -2289,18 +2297,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;
}
}
+147 -11
View File
@@ -752,6 +752,7 @@ where
let mut final_usage: Option<ProviderUsage> = None;
let mut message_id: Option<String> = None;
let mut thinking: Option<ThinkingState> = None;
let mut stop_reason: Option<String> = 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::<Value>(&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<String> = 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::<Vec<_>>().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));
}
@@ -1713,6 +1753,7 @@ mod tests {
redacted_thinking: Vec<String>,
text: Vec<String>,
tool_calls: Vec<String>,
tool_errors: Vec<String>,
}
async fn collect_stream(events: &str) -> StreamedParts {
@@ -1733,11 +1774,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()),
},
_ => {}
}
}
@@ -2027,4 +2067,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());
}
}
@@ -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<Message> {
};
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"),
}