harness(openai): recover tool-call args corrupted by leaked chat-template delimiters

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 `<tool_call|>`)
+            // before placing the call in `function.arguments`, turning
+            // otherwise-valid JSON into an unparseable blob. Seen in the wild
+            // leaking a trailing `<tool_call|>` 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|>",
+    "<tool_call|>",
+    "</tool_call>",
+    "<tool_call>",
+];
+
+/// 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}<tool_call|>`).
+/// 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}<tool_call|>{"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<Value> {
+    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()
+        && let Ok(value) = serde_json::from_str::<Value>(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::<Value>();
+    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<String> {
+    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`].

src/harness/providers/openai/test.rs

@@ -1263,3 +1263,122 @@ fn merge_system_is_noop_without_system_messages() {
     let input = vec![Message::user("just user")];
     assert_eq!(merge_system_into_user(&input), input);
 }
+
+/// 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 `<tool_call|>` 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}<tool_call|>"#,
+    ))
+    .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 `<tool_call>…</tool_call>`; both
+    // delimiters must be stripped before parsing.
+    let response = parse_response(tool_call_body(r#"<tool_call>{"q":"hi"}</tool_call>"#))
+        .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_call|>{"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"]}}<tool_call|>"#,
+    ))
+    .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 <tool_call|> literally"}"#)).unwrap();
+    assert_eq!(
+        response.tool_calls()[0].arguments,
+        json!({ "body": "use <tool_call|> 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<u8>> = vec![
+        b"data: {\"choices\":[{\"delta\":{\"tool_calls\":[{\"index\":0,\"id\":\"call-x\",\"function\":{\"name\":\"composio_execute\",\"arguments\":\"{\\\"q\\\":1}<tool_call|>\"}}]},\"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 }));
+}