From 5a8210f75750f360d38211d4c009c25935bec45c Mon Sep 17 00:00:00 2001 From: libokai Date: Tue, 4 Aug 2026 10:44:13 +0000 Subject: [PATCH] fix(tools): parse Hermes/Qwen JSON bodies inside tool_call wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parse_tool_blocks fed wrapper bodies only to the XML iterators (_iter_xml_invoke/_iter_xml_direct), so the canonical Qwen/Hermes text-mode form — a bare JSON object like {"name": "bash", "arguments": {"command": "..."}} inside the wrapper — parsed to zero tool blocks and the agent never executed anything. Pattern 4d only matches OpenAI-style blobs with a literal "function" key, which the Hermes format lacks. Wrapper bodies are now classified first: a JSON-looking body ({ or [) is parsed by the new _parse_json_tool_call_body, which requires an object with a string "name" and rejects a non-object "arguments" instead of coercing it, then converts through the same function_call_to_tool_block used by the XML paths so aliases and per-tool argument formatting stay uniform. JSON-looking bodies fail closed — they are never rescanned by the XML iterators (including the unclosed-wrapper and bare-invoke fallbacks), so XML-like text inside JSON argument values stays data instead of selecting a different tool. Non-JSON bodies keep the existing XML path unchanged. Fixes #5187 --- src/tool_parsing.py | 65 +++++++++++++++++- tests/test_tool_parsing_hermes_json.py | 94 ++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 tests/test_tool_parsing_hermes_json.py diff --git a/src/tool_parsing.py b/src/tool_parsing.py index 2885cc00f..bce90071c 100644 --- a/src/tool_parsing.py +++ b/src/tool_parsing.py @@ -925,6 +925,46 @@ def _parse_xml_direct_tool(name, body) -> Optional[ToolBlock]: return function_call_to_tool_block(mapped, json.dumps(params)) +def _looks_like_json_body(body: str) -> bool: + """True when a wrapper body is JSON, not XML markup.""" + return body.lstrip()[:1] in ("{", "[") + + +def _parse_json_tool_call_body(body: str) -> Optional[ToolBlock]: + """Parse a Qwen/Hermes text-mode wrapper body: bare JSON inside . + + + {"name": "bash", "arguments": {"command": "mkdir -p agent-test"}} + + + Strict by design (issue #5187 / tracker #5333): the body must decode to an + object with a string "name", and "arguments" — when present — must itself + be an object. Anything else returns None rather than being coerced, so a + malformed call is dropped instead of dispatching with mangled arguments. + raw_decode tolerates trailing chatter after the JSON object; the trailing + text is never scanned for tool markup. Conversion goes through + function_call_to_tool_block so aliases and per-tool argument formatting + stay identical to the XML invoke path. + """ + stripped = body.strip() + if not stripped.startswith("{"): + return None + try: + parsed, _end = json.JSONDecoder().raw_decode(stripped) + except json.JSONDecodeError: + return None + if not isinstance(parsed, dict): + return None + name = parsed.get("name") + if not isinstance(name, str) or not name.strip(): + return None + if "arguments" in parsed and not isinstance(parsed["arguments"], dict): + return None + args = parsed.get("arguments", {}) + from src.tool_schemas import function_call_to_tool_block + return function_call_to_tool_block(name.strip().lower(), json.dumps(args)) + + def _iter_stepfun_tool_calls(text: str): """Yield StepFun native tool-call token bodies without regex backtracking.""" pos = 0 @@ -1326,10 +1366,21 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]: if blocks: return blocks # Try wrapped: ... + # A wrapper body that is JSON (Qwen/Hermes text mode, issue #5187) is + # parsed as JSON or dropped — never scanned by the XML iterators, so + # XML-like text inside JSON argument values stays data instead of + # selecting a different tool. + json_body_seen = False for _ms, inner_start, inner_end, _me in _iter_delimited( text, _XML_TOOL_CALL_OPEN_RE, _XML_TOOL_CALL_CLOSE_RE ): body = text[inner_start:inner_end] + if _looks_like_json_body(body): + json_body_seen = True + block = _parse_json_tool_call_body(body) + if block: + blocks.append(block) + continue for inv_name, inv_body in _iter_xml_invoke(body): block = _parse_xml_invoke(inv_name, inv_body) if block: @@ -1344,6 +1395,13 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]: if not blocks: for m in _XML_OPEN_TOOL_CALL_RE.finditer(text): body = m.group(1) + if _looks_like_json_body(body): + # Same fail-closed rule as above for an unclosed wrapper. + json_body_seen = True + block = _parse_json_tool_call_body(body) + if block: + blocks.append(block) + break for inv_name, inv_body in _iter_xml_invoke(body): block = _parse_xml_invoke(inv_name, inv_body) if block: @@ -1354,8 +1412,11 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]: block = _parse_xml_direct_tool(d_name, d_body) if block: blocks.append(block) - # Try bare without wrapper - if not blocks: + # Try bare without wrapper. Skipped when a JSON wrapper body + # was seen but produced no block: this rescan covers the full text, + # wrapper bodies included, and markup inside a (possibly + # malformed) JSON payload must stay data rather than dispatch. + if not blocks and not json_body_seen: for inv_name, inv_body in _iter_xml_invoke(text): block = _parse_xml_invoke(inv_name, inv_body) if block: diff --git a/tests/test_tool_parsing_hermes_json.py b/tests/test_tool_parsing_hermes_json.py new file mode 100644 index 000000000..7ba0550d2 --- /dev/null +++ b/tests/test_tool_parsing_hermes_json.py @@ -0,0 +1,94 @@ +"""Qwen/Hermes text-mode tool calls: bare JSON inside wrappers. + +Issue #5187: {"name": "bash", "arguments": {...}} +parsed to zero blocks because wrapper bodies were only fed to the XML +iterators. The JSON body form now parses through the same canonical +function_call_to_tool_block converter as the XML paths, and JSON-looking +bodies fail closed instead of falling through to XML scanning (tracker #5333): +XML-like text inside JSON argument values must stay data, and a non-object +"arguments" value is rejected rather than coerced. +""" +import src.agent_tools # noqa: F401 (break agent_tools<->tool_parsing import cycle) +from src.tool_parsing import parse_tool_blocks, strip_tool_blocks + +# Verbatim payload from issue #5187. +ISSUE_PAYLOAD = '\n{"name": "bash", "arguments": {"command": "mkdir -p agent-test"}}\n' + + +def test_issue_5187_payload_parses(): + blocks = parse_tool_blocks(ISSUE_PAYLOAD) + assert len(blocks) == 1 + assert blocks[0].tool_type == "bash" + assert blocks[0].content == "mkdir -p agent-test" + + +def test_multiple_sequential_wrappers(): + text = ( + '\n{"name": "bash", "arguments": {"command": "ls"}}\n\n' + 'Now the second step:\n' + '\n{"name": "bash", "arguments": {"command": "pwd"}}\n' + ) + blocks = parse_tool_blocks(text) + assert [(b.tool_type, b.content) for b in blocks] == [("bash", "ls"), ("bash", "pwd")] + + +def test_unclosed_wrapper_still_parses(): + text = '\n{"name": "bash", "arguments": {"command": "ls -la"}}' + blocks = parse_tool_blocks(text) + assert len(blocks) == 1 + assert blocks[0].tool_type == "bash" + assert blocks[0].content == "ls -la" + + +def test_xml_inside_json_arguments_stays_data(): + # P1: a valid JSON body whose argument values contain XML-like tool markup + # must parse as the JSON-named tool; the embedded markup is content. + text = ( + '{"name": "write_file", "arguments": ' + '{"path": "notes.txt", "content": "echo unsafe"}}' + ) + blocks = parse_tool_blocks(text) + assert len(blocks) == 1 + assert blocks[0].tool_type == "write_file" + assert "echo unsafe" in blocks[0].content + assert all(b.tool_type != "bash" for b in blocks) + + +def test_malformed_json_body_never_falls_through_to_xml(): + # P1 fail-closed: a JSON-looking body that doesn't decode must not be + # rescanned as XML, even when it contains well-formed tool markup. + text = ( + '{"name": "write_file", "arguments": {broken json ' + 'echo unsafe' + '' + ) + assert parse_tool_blocks(text) == [] + + +def test_non_dict_arguments_rejected(): + # P2: "arguments" must be an object; scalars/arrays are rejected, not coerced. + for args in ('["ls"]', '"ls"', '1', 'null'): + text = '{"name": "bash", "arguments": %s}' % args + assert parse_tool_blocks(text) == [], f"arguments={args} should be rejected" + + +def test_strip_tool_blocks_removes_json_wrapper_spans(): + text = "Before.\n" + ISSUE_PAYLOAD + "\nAfter." + cleaned = strip_tool_blocks(text) + assert "tool_call" not in cleaned + assert "mkdir -p agent-test" not in cleaned + assert "Before." in cleaned + assert "After." in cleaned + + +def test_xml_body_wrapper_regression(): + # The pre-existing XML wrapper form must keep parsing exactly as before. + text = ( + '' + 'echo hi' + '' + ) + blocks = parse_tool_blocks(text) + assert len(blocks) == 1 + assert blocks[0].tool_type == "bash" + assert blocks[0].content == "echo hi"