diff --git a/src/tool_parsing.py b/src/tool_parsing.py index b13f3b0a17..2811d9b7f6 100644 --- a/src/tool_parsing.py +++ b/src/tool_parsing.py @@ -1375,13 +1375,34 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]: # XML-like text inside JSON argument values stays data instead of # selecting a different tool. json_body_seen = False + # Wrapper spans consumed below; used to mask wrapper bodies out of the + # bare-invoke fallback so markup inside a malformed JSON payload stays + # data (issue #5333) while later real calls are recovered (#6014). + wrapper_spans: list[tuple[int, int]] = [] + skip_before = -1 for _ms, inner_start, inner_end, _me in _iter_delimited( text, _XML_TOOL_CALL_OPEN_RE, _XML_TOOL_CALL_CLOSE_RE ): + wrapper_spans.append((_ms, _me)) + if inner_start < skip_before: + # Already consumed by a string-aware (extended) parse below. + continue body = text[inner_start:inner_end] if _looks_like_json_body(body): json_body_seen = True block = _parse_json_tool_call_body(body) + if not block: + # Issue #6013: a closer token inside a JSON string value + # ends the delimiter span early, so the body fails to + # decode. Retry with each later closer in turn; the first + # one whose body decodes is the real wrapper end. + for close_m in _XML_TOOL_CALL_CLOSE_RE.finditer(text, _me): + block = _parse_json_tool_call_body( + text[inner_start:close_m.start()] + ) + if block: + skip_before = close_m.end() + break if block: blocks.append(block) continue @@ -1398,6 +1419,13 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]: # complete inner tool tag, but forget the closing . if not blocks: for m in _XML_OPEN_TOOL_CALL_RE.finditer(text): + # The opener-to-EOS match also fires when the wrapper WAS + # closed (group(1) then swallows the closer and everything + # after). Trust it only where the closed-span scan found + # nothing, or it would mask later valid calls (#6014). + if any(ws <= m.start() < we for ws, we in wrapper_spans): + continue + wrapper_spans.append((m.start(), m.end())) body = m.group(1) if _looks_like_json_body(body): # Same fail-closed rule as above for an unclosed wrapper. @@ -1417,11 +1445,24 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]: if block: blocks.append(block) # 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): + # produced a valid block (blocks non-empty) — this rescan covers the + # full text and markup inside a JSON payload must stay data (#5333). + # Issue #6014: when JSON wrapper bodies were seen but all malformed, + # scan with every wrapper span masked out so a later valid bare call + # is recovered while markup inside the malformed payload stays data. + scan_text = None + if not blocks: + if not json_body_seen: + scan_text = text + elif wrapper_spans: + chars = list(text) + for ws, we in wrapper_spans: + for i in range(max(0, ws), min(we, len(chars))): + if chars[i] != "\n": + chars[i] = " " + scan_text = "".join(chars) + if scan_text is not None: + for inv_name, inv_body in _iter_xml_invoke(scan_text): block = _parse_xml_invoke(inv_name, inv_body) if block: blocks.append(block) diff --git a/src/tool_schemas.py b/src/tool_schemas.py index 7585f3e9d7..f71977828a 100644 --- a/src/tool_schemas.py +++ b/src/tool_schemas.py @@ -1414,9 +1414,20 @@ def function_call_to_tool_block(name: str, arguments: str) -> Optional[ToolBlock # Convert structured args back to the text format each tool expects if tool_type == "bash": - content = args.get("command", "") + payload = args.get("command", "") + # Issue #6012: a Qwen/Hermes JSON wrapper with a non-string command + # (list/object/number) must fail closed — coercing it produced a + # ToolBlock whose content isn't a str and crashed or mangled execution. + if payload is not None and not isinstance(payload, str): + logger.warning(f"Rejecting non-string command payload for function call {name}: {payload!r}") + return None + content = payload or "" elif tool_type == "python": - content = args.get("code", "") + payload = args.get("code", "") + if payload is not None and not isinstance(payload, str): + logger.warning(f"Rejecting non-string code payload for function call {name}: {payload!r}") + return None + content = payload or "" elif tool_type == "web_search": queries = args.get("queries") if isinstance(queries, list) and queries: diff --git a/tests/test_tool_parsing_wrapper_recovery.py b/tests/test_tool_parsing_wrapper_recovery.py new file mode 100644 index 0000000000..d283bc463c --- /dev/null +++ b/tests/test_tool_parsing_wrapper_recovery.py @@ -0,0 +1,72 @@ +"""Wrapper-recovery regressions for Qwen/Hermes text-mode tool calls. + +Issues: #6014 (a malformed wrapper suppressed later valid bare calls), +#6013 (a closer token inside a JSON string value ended the wrapper span +early), #6012 (non-string command/code payloads were coerced instead of +rejected). Wrapper markers are built via concatenation so this file never +embeds a raw wrapper sequence that scanners could trip over. +""" +import src.agent_tools # noqa: F401 (break agent_tools<->tool_parsing import cycle) +from src.tool_parsing import parse_tool_blocks +from src.tool_schemas import function_call_to_tool_block + +OPEN = "<" + "tool_call>" +CLOSE = "" + + +def test_6014_malformed_wrapper_then_bare_invoke_parses(): + text = ( + OPEN + '{"name": "write_file", "arguments": {broken json' + CLOSE + "\n" + "Now run this:\n" + 'echo hi' + ) + blocks = parse_tool_blocks(text) + assert [(b.tool_type, b.content) for b in blocks] == [("bash", "echo hi")] + + +def test_6013_closer_inside_json_string_value(): + payload = ( + '{"name": "write_file", "arguments": ' + '{"path": "n.txt", "content": "hello ' + CLOSE + ' world"}}' + ) + text = OPEN + payload + CLOSE + blocks = parse_tool_blocks(text) + assert len(blocks) == 1 + assert blocks[0].tool_type == "write_file" + assert ("hello " + CLOSE + " world") in blocks[0].content + + +def test_6012_nonstring_command_rejected(): + for bad in ('["ls", "-la"]', '{"cmd": "ls"}', "1"): + text = OPEN + '{"name": "bash", "arguments": {"command": ' + bad + "}}" + CLOSE + assert parse_tool_blocks(text) == [], bad + assert function_call_to_tool_block("bash", '{"command": ["ls"]}') is None + + +def test_6012_nonstring_python_code_rejected(): + text = OPEN + '{"name": "python", "arguments": {"code": [1, 2]}}' + CLOSE + assert parse_tool_blocks(text) == [] + assert function_call_to_tool_block("python", '{"code": {"x": 1}}') is None + + +def test_5333_markup_inside_malformed_json_stays_data(): + text = ( + OPEN + '{"name": "write_file", "arguments": {broken ' + 'echo unsafe' + ) + assert parse_tool_blocks(text) == [] + + +def test_valid_json_wrapper_still_parses(): + text = OPEN + '{"name": "bash", "arguments": {"command": "ls"}}' + CLOSE + blocks = parse_tool_blocks(text) + assert [(b.tool_type, b.content) for b in blocks] == [("bash", "ls")] + + +def test_two_wrappers_with_malformed_first_recover(): + text = ( + OPEN + '{"name": "bash", "arguments": {broken' + CLOSE + "\n" + + OPEN + '{"name": "bash", "arguments": {"command": "pwd"}}' + CLOSE + ) + blocks = parse_tool_blocks(text) + assert [(b.tool_type, b.content) for b in blocks] == [("bash", "pwd")] \ No newline at end of file