From bf38cae9aca54036d5af856306b4c08875e5c6ba Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Thu, 9 Jul 2026 23:39:26 +0530 Subject: [PATCH 1/4] harness(openai): recover tool-call args corrupted by leaked chat-template delimiters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some OpenAI-compatible gateways fail to strip a model's chat-template tool-call delimiters (e.g. a trailing ``) before placing the call in `function.arguments`. The otherwise-valid JSON then fails to parse, so the tool call is unusable and the sub-agent produces no valid call — orphaning the parent's join/reduce (empty reply, no terminal event). `parse_tool_arguments` (shared by the streaming `OpenAiStreamAcc::into_response` reduce and the unary `parse_response`) now attempts one conservative repair on a parse failure before erroring: 1. strip known chat-template tool-call markers and re-parse; else 2. recover the first complete leading JSON object (handles a valid call followed by trailing marker noise or a second concatenated fragment). The repair runs only after the raw string has already failed to parse, so well-formed arguments are never rewritten. If the input is still unparseable it fails fast with the existing clear, non-retryable Model error — never a never-resolving tool call that stalls the agent loop. Adds unit tests for marker-stripping recovery, tag-wrapped args, concatenated fragments, fail-fast on genuinely malformed args, a valid-args-with-marker no-op, and the SSE streaming reduce path. Closes tinyhumansai/openhuman#4766 --- src/harness/providers/openai/convert.rs | 101 ++++++++++++++++++++-- src/harness/providers/openai/test.rs | 110 ++++++++++++++++++++++++ 2 files changed, 206 insertions(+), 5 deletions(-) diff --git a/src/harness/providers/openai/convert.rs b/src/harness/providers/openai/convert.rs index 93254bf..b0422dc 100644 --- a/src/harness/providers/openai/convert.rs +++ b/src/harness/providers/openai/convert.rs @@ -255,11 +255,102 @@ pub(super) fn parse_tool_arguments( if raw.trim().is_empty() { return Ok(Value::Object(Map::new())); } - serde_json::from_str(raw).map_err(|err| { - TinyAgentsError::Model(format!( - "{context} contained invalid JSON arguments for tool call `{call_id}` (`{name}`): {err}; raw arguments: {raw:?}" - )) - }) + match serde_json::from_str(raw) { + Ok(value) => Ok(value), + Err(err) => { + // Some OpenAI-compatible gateways fail to strip a model's + // chat-template tool-call delimiters (e.g. a trailing ``) + // before placing the call in `function.arguments`, turning + // otherwise-valid JSON into an unparseable blob. Seen in the wild + // leaking a trailing `` into a Composio `composio_execute` + // call, which then never parses — orphaning the sub-agent's reduce. + // Attempt one conservative repair before failing. `recover_tool_arguments` + // runs only after `raw` has already failed to parse, so well-formed + // arguments are never rewritten. + if let Some(value) = recover_tool_arguments(raw) { + return Ok(value); + } + // Still unparseable after repair: fail fast with a clear, + // non-retryable error. Surfacing the failure keeps the malformed + // call from becoming a never-resolving tool call that stalls the + // agent loop (and the parent's join/reduce) indefinitely. + Err(TinyAgentsError::Model(format!( + "{context} contained invalid JSON arguments for tool call `{call_id}` (`{name}`): {err}; raw arguments: {raw:?}" + ))) + } + } +} + +/// Chat-template tool-call delimiters that some OpenAI-compatible gateways fail +/// to strip before placing a call in `function.arguments`. Different model +/// families (Hermes/Qwen/Kimi/…) wrap tool calls in their own markers; a +/// leaked one turns valid argument JSON unparseable. +const TOOL_CALL_TEMPLATE_MARKERS: &[&str] = &[ + "<|tool_calls_section_end|>", + "<|tool_call_begin|>", + "<|tool_call_end|>", + "<|tool_call|>", + "<|tool_sep|>", + "", + "", + "", +]; + +/// Attempts to recover a usable arguments object from a tool-call arguments +/// string that failed to parse as JSON. +/// +/// Two conservative strategies, tried in order; the caller only invokes this +/// *after* the raw string has already failed `serde_json::from_str`, so this can +/// never rewrite arguments that were already valid: +/// +/// 1. Strip leaked chat-template tool-call delimiters (see +/// [`TOOL_CALL_TEMPLATE_MARKERS`]) and re-parse — recovers a valid call whose +/// only corruption is a leaked marker (e.g. `{"a":1}`). +/// 2. Take the first complete JSON *object* from the front of the (marker-stripped) +/// string — recovers a valid leading call followed by trailing template noise +/// or a second concatenated fragment (e.g. `{"a":1}{"b":2}`). +/// +/// Restricting strategy 2 to a leading `Value::Object` keeps it from accepting a +/// bare number/string scraped out of surrounding noise as if it were the call's +/// arguments. Returns `None` when neither strategy yields valid object-shaped JSON, +/// so the caller still fails fast on genuinely malformed input. +fn recover_tool_arguments(raw: &str) -> Option { + let stripped = strip_tool_call_markers(raw); + let candidate = stripped.as_deref().unwrap_or(raw); + + // Strategy 1: the marker-stripped string parses cleanly on its own. + if stripped.is_some() { + if let Ok(value) = serde_json::from_str::(candidate) { + return Some(value); + } + } + + // Strategy 2: recover the first complete JSON value if it is an object. + let mut values = + serde_json::Deserializer::from_str(candidate.trim_start()).into_iter::(); + match values.next() { + Some(Ok(value @ Value::Object(_))) => Some(value), + _ => None, + } +} + +/// Removes any [`TOOL_CALL_TEMPLATE_MARKERS`] found in `raw` and trims the +/// result. Returns `Some(cleaned)` only when a marker was actually present and +/// the trimmed result is non-empty; otherwise `None` (nothing to strip). +fn strip_tool_call_markers(raw: &str) -> Option { + let mut cleaned = raw.to_string(); + let mut changed = false; + for &marker in TOOL_CALL_TEMPLATE_MARKERS { + if cleaned.contains(marker) { + cleaned = cleaned.replace(marker, ""); + changed = true; + } + } + if !changed { + return None; + } + let trimmed = cleaned.trim(); + (!trimmed.is_empty()).then(|| trimmed.to_string()) } /// Converts an OpenAI [`UsageWire`] into the harness-neutral [`Usage`]. diff --git a/src/harness/providers/openai/test.rs b/src/harness/providers/openai/test.rs index e33242e..1eeb49f 100644 --- a/src/harness/providers/openai/test.rs +++ b/src/harness/providers/openai/test.rs @@ -1089,3 +1089,113 @@ fn derive_profile_populates_known_context_windows() { None ); } + +/// Builds an OpenAI-shaped non-streaming response body carrying a single tool +/// call whose `function.arguments` string is `raw` (verbatim, including any +/// corruption). Used to exercise `parse_tool_arguments` recovery/fail-fast. +fn tool_call_body(raw: &str) -> serde_json::Value { + json!({ + "id": "chatcmpl-toolargs", + "choices": [ + { + "message": { + "role": "assistant", + "tool_calls": [ + { + "id": "call-1", + "type": "function", + "function": { "name": "composio_execute", "arguments": raw } + } + ] + }, + "finish_reason": "tool_calls" + } + ] + }) +} + +#[test] +fn recovers_tool_args_with_leaked_trailing_template_marker() { + // Regression (openhuman#4766): an OpenAI-compatible gateway leaked a + // model's trailing `` chat-template delimiter into + // `function.arguments`. The JSON is otherwise valid, so stripping the + // marker must recover the call instead of failing the turn. + let response = + parse_response(tool_call_body(r#"{"tool":"GMAIL_CREATE_EMAIL_DRAFT","x":1}"#)) + .expect("leaked marker must be recovered"); + let calls = response.tool_calls(); + assert_eq!(calls.len(), 1); + assert_eq!( + calls[0].arguments, + json!({ "tool": "GMAIL_CREATE_EMAIL_DRAFT", "x": 1 }) + ); +} + +#[test] +fn recovers_tool_args_wrapped_in_tool_call_tags() { + // Some templates wrap the whole call in `…`; both + // delimiters must be stripped before parsing. + let response = parse_response(tool_call_body(r#"{"q":"hi"}"#)) + .expect("wrapped args must be recovered"); + assert_eq!(response.tool_calls()[0].arguments, json!({ "q": "hi" })); +} + +#[test] +fn recovers_first_call_when_fragments_are_concatenated() { + // A leaked delimiter can also sit *between* two concatenated argument + // objects (e.g. an accumulator over-merge). After stripping the marker the + // string is `{...}{...}`; recovery keeps the first complete object. + let response = + parse_response(tool_call_body(r#"{"tool":"gmail"}{"tool":"other"}"#)) + .expect("leading call must be recovered"); + assert_eq!(response.tool_calls()[0].arguments, json!({ "tool": "gmail" })); +} + +#[test] +fn still_fails_fast_on_genuinely_malformed_tool_args() { + // The exact corruption seen in the wild carries a stray `]` *inside* the + // JSON — not just a leaked delimiter — so it cannot be safely repaired. It + // must fail fast (a clear, non-retryable model error) rather than hang. + let err = parse_response(tool_call_body(r#"{"arguments":{"body":"hi"]}}"#)) + .expect_err("unrepairable args must fail closed"); + assert!(matches!(err, TinyAgentsError::Model(_)), "got {err:?}"); + let message = err.to_string(); + assert!(message.contains("call-1"), "{message}"); + assert!(message.contains("raw arguments"), "{message}"); +} + +#[test] +fn valid_tool_args_containing_marker_substring_are_left_intact() { + // A legitimate arguments object whose *string value* contains the marker + // text parses cleanly on the first attempt, so the repair path never runs + // and the value is preserved verbatim. + let response = + parse_response(tool_call_body(r#"{"body":"use literally"}"#)).unwrap(); + assert_eq!( + response.tool_calls()[0].arguments, + json!({ "body": "use literally" }) + ); +} + +#[tokio::test] +async fn sse_stream_recovers_tool_args_with_leaked_template_marker() { + // The streaming reduce (`OpenAiStreamAcc::into_response`) shares + // `parse_tool_arguments`, so a leaked trailing marker across the terminal + // must reconstruct a usable call instead of finishing as a model error + // (which is what orphaned the parent's join/reduce in openhuman#4766). + let raw: Vec> = vec![ + b"data: {\"choices\":[{\"delta\":{\"tool_calls\":[{\"index\":0,\"id\":\"call-x\",\"function\":{\"name\":\"composio_execute\",\"arguments\":\"{\\\"q\\\":1}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n\n".to_vec(), + b"data: [DONE]\n\n".to_vec(), + ]; + + let items = collect_sse(raw).await; + let mut merged = StreamAccumulator::new(); + for item in &items { + merged.push(item); + } + let response = merged.finish().expect("stream must not fail on a recoverable marker"); + let calls = response.tool_calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "composio_execute"); + assert_eq!(calls[0].arguments, json!({ "q": 1 })); +} From 9b02be3f3747c1161d13429ca302abcfcfed912f Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 10 Jul 2026 00:31:10 +0530 Subject: [PATCH 2/4] style: rustfmt openai provider test --- src/harness/providers/openai/test.rs | 29 ++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/harness/providers/openai/test.rs b/src/harness/providers/openai/test.rs index 1eeb49f..c2ccdbb 100644 --- a/src/harness/providers/openai/test.rs +++ b/src/harness/providers/openai/test.rs @@ -1120,9 +1120,10 @@ fn recovers_tool_args_with_leaked_trailing_template_marker() { // model's trailing `` chat-template delimiter into // `function.arguments`. The JSON is otherwise valid, so stripping the // marker must recover the call instead of failing the turn. - let response = - parse_response(tool_call_body(r#"{"tool":"GMAIL_CREATE_EMAIL_DRAFT","x":1}"#)) - .expect("leaked marker must be recovered"); + let response = parse_response(tool_call_body( + r#"{"tool":"GMAIL_CREATE_EMAIL_DRAFT","x":1}"#, + )) + .expect("leaked marker must be recovered"); let calls = response.tool_calls(); assert_eq!(calls.len(), 1); assert_eq!( @@ -1145,10 +1146,14 @@ fn recovers_first_call_when_fragments_are_concatenated() { // A leaked delimiter can also sit *between* two concatenated argument // objects (e.g. an accumulator over-merge). After stripping the marker the // string is `{...}{...}`; recovery keeps the first complete object. - let response = - parse_response(tool_call_body(r#"{"tool":"gmail"}{"tool":"other"}"#)) - .expect("leading call must be recovered"); - assert_eq!(response.tool_calls()[0].arguments, json!({ "tool": "gmail" })); + let response = parse_response(tool_call_body( + r#"{"tool":"gmail"}{"tool":"other"}"#, + )) + .expect("leading call must be recovered"); + assert_eq!( + response.tool_calls()[0].arguments, + json!({ "tool": "gmail" }) + ); } #[test] @@ -1156,8 +1161,10 @@ fn still_fails_fast_on_genuinely_malformed_tool_args() { // The exact corruption seen in the wild carries a stray `]` *inside* the // JSON — not just a leaked delimiter — so it cannot be safely repaired. It // must fail fast (a clear, non-retryable model error) rather than hang. - let err = parse_response(tool_call_body(r#"{"arguments":{"body":"hi"]}}"#)) - .expect_err("unrepairable args must fail closed"); + let err = parse_response(tool_call_body( + r#"{"arguments":{"body":"hi"]}}"#, + )) + .expect_err("unrepairable args must fail closed"); assert!(matches!(err, TinyAgentsError::Model(_)), "got {err:?}"); let message = err.to_string(); assert!(message.contains("call-1"), "{message}"); @@ -1193,7 +1200,9 @@ async fn sse_stream_recovers_tool_args_with_leaked_template_marker() { for item in &items { merged.push(item); } - let response = merged.finish().expect("stream must not fail on a recoverable marker"); + let response = merged + .finish() + .expect("stream must not fail on a recoverable marker"); let calls = response.tool_calls(); assert_eq!(calls.len(), 1); assert_eq!(calls[0].name, "composio_execute"); From 54de67fc8de0a0614099afcaa0a2edd31eaa4b0e Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 10 Jul 2026 00:36:39 +0530 Subject: [PATCH 3/4] fix(prompt): hash serialized bytes instead of io::Write into Sha256 Sha256 no longer implements std::io::Write, so serde_json::to_writer into the hasher fails to compile (E0277) under `clippy --all-targets -D warnings`. Serialize to a Vec and feed the bytes via Digest::update instead. Pre-existing break on tinyagents main; folded in here to unblock this PR's Rust SDK check. --- src/harness/prompt/mod.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/harness/prompt/mod.rs b/src/harness/prompt/mod.rs index 054aed1..1757827 100644 --- a/src/harness/prompt/mod.rs +++ b/src/harness/prompt/mod.rs @@ -308,14 +308,16 @@ impl PromptBuilder { "tools": self.tools, }); - // Stream the canonical JSON straight into the digest; `Sha256` - // implements `std::io::Write`. + // Serialize the canonical JSON, then feed the bytes into the digest. + // (`Sha256` no longer implements `std::io::Write`, so hash the + // serialized bytes directly instead of streaming via `to_writer`.) let mut hasher = Sha256::new(); - if serde_json::to_writer(&mut hasher, &payload).is_err() { - // Serializing an already-materialized `Value` does not fail in - // practice; fall back to hashing empty data for a stable result. - hasher = Sha256::new(); + if let Ok(bytes) = serde_json::to_vec(&payload) { + hasher.update(&bytes); } + // Serializing an already-materialized `Value` does not fail in + // practice; on the unexpected error path the digest hashes empty + // data for a stable result. let digest = hasher.finalize(); digest.iter().map(|byte| format!("{byte:02x}")).collect() } From 9c178b2d217338498f62825aac580a81c250e3fc Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 10 Jul 2026 01:00:54 +0530 Subject: [PATCH 4/4] fix(openai): collapse nested if into a let-chain (clippy::collapsible_if) --- src/harness/providers/openai/convert.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/harness/providers/openai/convert.rs b/src/harness/providers/openai/convert.rs index b0422dc..6fd71a8 100644 --- a/src/harness/providers/openai/convert.rs +++ b/src/harness/providers/openai/convert.rs @@ -319,10 +319,10 @@ fn recover_tool_arguments(raw: &str) -> Option { let candidate = stripped.as_deref().unwrap_or(raw); // Strategy 1: the marker-stripped string parses cleanly on its own. - if stripped.is_some() { - if let Ok(value) = serde_json::from_str::(candidate) { - return Some(value); - } + if stripped.is_some() + && let Ok(value) = serde_json::from_str::(candidate) + { + return Some(value); } // Strategy 2: recover the first complete JSON value if it is an object.