From 8ae921dd68a3372f14ee84b5aa78f7c48d18cacf Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Thu, 14 May 2026 08:37:04 +0530 Subject: [PATCH] fix(normalization): guard function.arguments against malformed JSON and default to {} (#1645) Co-authored-by: Steven Enamakel --- src/openhuman/providers/compatible.rs | 14 ++++-- src/openhuman/providers/compatible_parse.rs | 23 ++++++--- src/openhuman/providers/compatible_tests.rs | 55 +++++++++++++++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/src/openhuman/providers/compatible.rs b/src/openhuman/providers/compatible.rs index 90c5adc8c..8936acc55 100644 --- a/src/openhuman/providers/compatible.rs +++ b/src/openhuman/providers/compatible.rs @@ -12,7 +12,9 @@ mod compatible_stream; mod compatible_types; #[cfg(test)] -pub(crate) use compatible_parse::{parse_sse_line, strip_think_tags}; +pub(crate) use compatible_parse::{ + parse_provider_tool_call_from_value, parse_sse_line, strip_think_tags, +}; #[cfg(test)] pub(crate) use compatible_types::ResponsesResponse; @@ -993,10 +995,12 @@ impl OpenAiCompatibleProvider { None } else { // Try to parse as JSON first so downstream - // `normalize_function_arguments` can handle the - // usual Value path; fall back to a JSON-string - // value if the accumulated text isn't valid - // JSON yet. + // `normalize_function_arguments` can take the + // usual Value (object) path; fall back to a + // JSON-string value for partially-assembled or + // permanently malformed fragments. + // `normalize_function_arguments` validates and + // discards malformed strings (OPENHUMAN-TAURI-6F). Some( serde_json::from_str(&c.arguments) .unwrap_or(serde_json::Value::String(c.arguments)), diff --git a/src/openhuman/providers/compatible_parse.rs b/src/openhuman/providers/compatible_parse.rs index 7a5c1662a..1d8e070e4 100644 --- a/src/openhuman/providers/compatible_parse.rs +++ b/src/openhuman/providers/compatible_parse.rs @@ -119,8 +119,18 @@ pub(crate) fn normalize_function_arguments(arguments: Option) Some(serde_json::Value::String(raw)) => { if raw.trim().is_empty() { "{}".to_string() - } else { + } else if serde_json::from_str::(&raw).is_ok() { raw + } else { + // OPENHUMAN-TAURI-6F: model emitted malformed JSON in + // `function.arguments`. Log the discard so it's traceable + // without leaking argument contents (which may contain PII). + log::warn!( + "[providers] normalize_function_arguments: \ + discarding malformed JSON string (len={}) — substituting {{}}", + raw.len() + ); + "{}".to_string() } } Some(serde_json::Value::Null) | None => "{}".to_string(), @@ -140,11 +150,12 @@ pub(crate) fn parse_provider_tool_call_from_value( call.id }, name: call.name, - arguments: if call.arguments.trim().is_empty() { - "{}".to_string() - } else { - call.arguments - }, + // Route through normalize_function_arguments so malformed + // JSON strings in pre-deserialized ProviderToolCall values + // receive the same guard as the function.arguments path below. + arguments: normalize_function_arguments(Some(serde_json::Value::String( + call.arguments, + ))), }); } } diff --git a/src/openhuman/providers/compatible_tests.rs b/src/openhuman/providers/compatible_tests.rs index 30d7272c4..a8e43d4e0 100644 --- a/src/openhuman/providers/compatible_tests.rs +++ b/src/openhuman/providers/compatible_tests.rs @@ -1147,3 +1147,58 @@ fn parse_sse_line_done_sentinel() { let result = parse_sse_line(line).unwrap(); assert_eq!(result, None); } + +#[test] +fn normalize_function_arguments_valid_json_string_preserved() { + let v = Some(serde_json::Value::String(r#"{"path":"/tmp"}"#.to_string())); + assert_eq!(normalize_function_arguments(v), r#"{"path":"/tmp"}"#); +} + +#[test] +fn normalize_function_arguments_invalid_json_string_falls_back_to_empty_object() { + // OPENHUMAN-TAURI-6F: model emitted malformed JSON in `function.arguments`. + // Forwarding the raw string back upstream causes a 400 from the backend's + // `json.loads`. Substitute `{}` instead. + for raw in ["{a:1}", "{'k':'v'}", "{\n", "{,}"] { + let v = Some(serde_json::Value::String(raw.to_string())); + assert_eq!(normalize_function_arguments(v), "{}", "raw = {raw:?}"); + } +} + +#[test] +fn normalize_function_arguments_empty_or_null_becomes_empty_object() { + assert_eq!( + normalize_function_arguments(Some(serde_json::Value::String(" ".to_string()))), + "{}" + ); + assert_eq!( + normalize_function_arguments(Some(serde_json::Value::Null)), + "{}" + ); + assert_eq!(normalize_function_arguments(None), "{}"); +} + +#[test] +fn normalize_function_arguments_object_value_serializes() { + let v = Some(serde_json::json!({"path": "/tmp"})); + assert_eq!(normalize_function_arguments(v), r#"{"path":"/tmp"}"#); +} + +#[test] +fn parse_provider_tool_call_from_value_guards_malformed_arguments() { + // OPENHUMAN-TAURI-6F: the early-return path in + // `parse_provider_tool_call_from_value` previously bypassed + // `normalize_function_arguments`, forwarding malformed JSON strings + // directly. Verify the guard now applies on both code paths. + let value = serde_json::json!({ + "id": "call_bad", + "name": "shell", + "arguments": "{a:1}" + }); + let result = parse_provider_tool_call_from_value(&value); + let call = result.expect("should produce a ToolCall"); + assert_eq!( + call.arguments, "{}", + "malformed arguments string must be normalised to {{}} via the first-path guard" + ); +}