tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix: escape control characters in LLM tool call arguments JSON (#2893)

Browse Source 
Signed-off-by: Julian Brown <contact@julianbrown.dev>
Co-authored-by: Julian Brown <jbrown@stripe.com>
This commit is contained in:
Julian Brown
2025-07-29 05:05:01 -04:00
committed by GitHub
parent c84dc392be
commit bc25308d7b
3 changed files with 179 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose/src/providers/formats/databricks.rs
+14 -9
View File
@@ -1,7 +1,7 @@
use crate::message::{Message, MessageContent};
use crate::model::ModelConfig;
use crate::providers::utils::{
convert_image, detect_image_path, is_valid_function_name, load_image_file,
convert_image, detect_image_path, is_valid_function_name, load_image_file, safely_parse_json,
sanitize_function_name, ImageFormat,
};
use anyhow::{anyhow, Error};
@@ -324,14 +324,19 @@ pub fn response_to_message(response: &Value) -> anyhow::Result<Message> {
.as_str()
.unwrap_or_default()
.to_string();
let mut arguments = tool_call["function"]["arguments"]
// Get the raw arguments string from the LLM.
let arguments_str = tool_call["function"]["arguments"]
.as_str()
.unwrap_or_default()
.to_string();
// If arguments is empty, we will have invalid json parsing error later.
if arguments.is_empty() {
arguments = "{}".to_string();
}
// If arguments_str is empty, default to an empty JSON object string.
let arguments_str = if arguments_str.is_empty() {
"{}".to_string()
} else {
arguments_str
};
if !is_valid_function_name(&function_name) {
let error = ToolError::NotFound(format!(
@@ -340,7 +345,7 @@ pub fn response_to_message(response: &Value) -> anyhow::Result<Message> {
));
content.push(MessageContent::tool_request(id, Err(error)));
} else {
match serde_json::from_str::<Value>(&arguments) {
match safely_parse_json(&arguments_str) {
Ok(params) => {
content.push(MessageContent::tool_request(
id,
@@ -349,8 +354,8 @@ pub fn response_to_message(response: &Value) -> anyhow::Result<Message> {
}
Err(e) => {
let error = ToolError::InvalidParameters(format!(
"Could not interpret tool use parameters for id {}: {}",
id, e
"Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'",
id, e, arguments_str
));
content.push(MessageContent::tool_request(id, Err(error)));
}
crates/goose/src/providers/formats/openai.rs
+14 -9
View File
@@ -2,7 +2,7 @@ use crate::message::{Message, MessageContent};
use crate::model::ModelConfig;
use crate::providers::base::{ProviderUsage, Usage};
use crate::providers::utils::{
convert_image, detect_image_path, is_valid_function_name, load_image_file,
convert_image, detect_image_path, is_valid_function_name, load_image_file, safely_parse_json,
sanitize_function_name, ImageFormat,
};
use anyhow::{anyhow, Error};
@@ -284,14 +284,19 @@ pub fn response_to_message(response: &Value) -> anyhow::Result<Message> {
.as_str()
.unwrap_or_default()
.to_string();
let mut arguments = tool_call["function"]["arguments"]
// Get the raw arguments string from the LLM.
let arguments_str = tool_call["function"]["arguments"]
.as_str()
.unwrap_or_default()
.to_string();
// If arguments is empty, we will have invalid json parsing error later.
if arguments.is_empty() {
arguments = "{}".to_string();
}
// If arguments_str is empty, default to an empty JSON object string.
let arguments_str = if arguments_str.is_empty() {
"{}".to_string()
} else {
arguments_str
};
if !is_valid_function_name(&function_name) {
let error = ToolError::NotFound(format!(
@@ -300,7 +305,7 @@ pub fn response_to_message(response: &Value) -> anyhow::Result<Message> {
));
content.push(MessageContent::tool_request(id, Err(error)));
} else {
match serde_json::from_str::<Value>(&arguments) {
match safely_parse_json(&arguments_str) {
Ok(params) => {
content.push(MessageContent::tool_request(
id,
@@ -309,8 +314,8 @@ pub fn response_to_message(response: &Value) -> anyhow::Result<Message> {
}
Err(e) => {
let error = ToolError::InvalidParameters(format!(
"Could not interpret tool use parameters for id {}: {}",
id, e
"Could not interpret tool use parameters for id {}: {}. Raw arguments: '{}'",
id, e, arguments_str
));
content.push(MessageContent::tool_request(id, Err(error)));
}
crates/goose/src/providers/utils.rs
+151
View File
@@ -347,6 +347,72 @@ pub fn emit_debug_trace<T1, T2>(
);
}
/// Safely parse a JSON string that may contain doubly-encoded or malformed JSON.
/// This function first attempts to parse the input string as-is. If that fails,
/// it applies control character escaping and tries again.
///
/// This approach preserves valid JSON like `{"key1": "value1",\n"key2": "value"}`
/// (which contains a literal \n but is perfectly valid JSON) while still fixing
/// broken JSON like `{"key1": "value1\n","key2": "value"}` (which contains an
/// unescaped newline character).
pub fn safely_parse_json(s: &str) -> Result<serde_json::Value, serde_json::Error> {
// First, try parsing the string as-is
match serde_json::from_str(s) {
Ok(value) => Ok(value),
Err(_) => {
// If that fails, try with control character escaping
let escaped = json_escape_control_chars_in_string(s);
serde_json::from_str(&escaped)
}
}
}
/// Helper to escape control characters in a string that is supposed to be a JSON document.
/// This function iterates through the input string `s` and replaces any literal
/// control characters (U+0000 to U+001F) with their JSON-escaped equivalents
/// (e.g., '\n' becomes "\\n", '\u0001' becomes "\\u0001").
///
/// It does NOT escape quotes (") or backslashes (\) because it assumes `s` is a
/// full JSON document, and these characters might be structural (e.g., object delimiters,
/// existing valid escape sequences). The goal is to fix common LLM errors where
/// control characters are emitted raw into what should be JSON string values,
/// making the overall JSON structure unparsable.
///
/// If the input string `s` has other JSON syntax errors (e.g., an unescaped quote
/// *within* a string value like `{"key": "string with " quote"}`), this function
/// will not fix them. It specifically targets unescaped control characters.
pub fn json_escape_control_chars_in_string(s: &str) -> String {
let mut r = String::with_capacity(s.len()); // Pre-allocate for efficiency
for c in s.chars() {
match c {
// ASCII Control characters (U+0000 to U+001F)
'\u{0000}'..='\u{001F}' => {
match c {
'\u{0008}' => r.push_str("\\b"), // Backspace
'\u{000C}' => r.push_str("\\f"), // Form feed
'\n' => r.push_str("\\n"), // Line feed
'\r' => r.push_str("\\r"), // Carriage return
'\t' => r.push_str("\\t"), // Tab
// Other control characters (e.g., NUL, SOH, VT, etc.)
// that don't have a specific short escape sequence.
_ => {
r.push_str(&format!("\\u{:04x}", c as u32));
}
}
}
// Other characters are passed through.
// This includes quotes (") and backslashes (\). If these are part of the
// JSON structure (e.g. {"key": "value"}) or part of an already correctly
// escaped sequence within a string value (e.g. "string with \\\" quote"),
// they are preserved as is. This function does not attempt to fix
// malformed quote or backslash usage *within* string values if the LLM
// generates them incorrectly (e.g. {"key": "unescaped " quote in string"}).
_ => r.push(c),
}
}
r
}
#[cfg(test)]
mod tests {
use super::*;
@@ -572,4 +638,89 @@ mod tests {
assert_eq!(result, expected_status);
}
}
#[test]
fn test_safely_parse_json() {
// Test valid JSON that should parse without escaping (contains proper escape sequence)
let valid_json = r#"{"key1": "value1","key2": "value2"}"#;
let result = safely_parse_json(valid_json).unwrap();
assert_eq!(result["key1"], "value1");
assert_eq!(result["key2"], "value2");
// Test JSON with actual unescaped newlines that needs escaping
let invalid_json = "{\"key1\": \"value1\n\",\"key2\": \"value2\"}";
let result = safely_parse_json(invalid_json).unwrap();
assert_eq!(result["key1"], "value1\n");
assert_eq!(result["key2"], "value2");
// Test already valid JSON - should parse on first try
let good_json = r#"{"test": "value"}"#;
let result = safely_parse_json(good_json).unwrap();
assert_eq!(result["test"], "value");
// Test completely invalid JSON that can't be fixed
let broken_json = r#"{"key": "unclosed_string"#;
assert!(safely_parse_json(broken_json).is_err());
// Test empty object
let empty_json = "{}";
let result = safely_parse_json(empty_json).unwrap();
assert!(result.as_object().unwrap().is_empty());
// Test JSON with escaped newlines (valid JSON) - should parse on first try
let escaped_json = r#"{"key": "value with\nnewline"}"#;
let result = safely_parse_json(escaped_json).unwrap();
assert_eq!(result["key"], "value with\nnewline");
}
#[test]
fn test_json_escape_control_chars_in_string() {
// Test basic control character escaping
assert_eq!(
json_escape_control_chars_in_string("Hello\nWorld"),
"Hello\\nWorld"
);
assert_eq!(
json_escape_control_chars_in_string("Hello\tWorld"),
"Hello\\tWorld"
);
assert_eq!(
json_escape_control_chars_in_string("Hello\rWorld"),
"Hello\\rWorld"
);
// Test multiple control characters
assert_eq!(
json_escape_control_chars_in_string("Hello\n\tWorld\r"),
"Hello\\n\\tWorld\\r"
);
// Test that quotes and backslashes are preserved (not escaped)
assert_eq!(
json_escape_control_chars_in_string("Hello \"World\""),
"Hello \"World\""
);
assert_eq!(
json_escape_control_chars_in_string("Hello\\World"),
"Hello\\World"
);
// Test JSON-like string with control characters
assert_eq!(
json_escape_control_chars_in_string("{\"message\": \"Hello\nWorld\"}"),
"{\"message\": \"Hello\\nWorld\"}"
);
// Test no changes for normal strings
assert_eq!(
json_escape_control_chars_in_string("Hello World"),
"Hello World"
);
// Test other control characters get unicode escapes
assert_eq!(
json_escape_control_chars_in_string("Hello\u{0001}World"),
"Hello\\u0001World"
);
}
}