From 179ab886a5c1abff467b551bb6956a9f450a71af Mon Sep 17 00:00:00 2001 From: Spoofiecus Date: Mon, 14 Sep 2026 18:05:06 +0200 Subject: [PATCH 1/2] fix(agent): recover Qwen/Hermes wrapper calls, reject non-string payloads, unfilter codex models --- routes/model_routes.py | 3 +- src/tool_parsing.py | 51 +++++++++++++-- src/tool_schemas.py | 15 ++++- tests/test_model_routes.py | 15 +++++ tests/test_tool_parsing_wrapper_recovery.py | 72 +++++++++++++++++++++ 5 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 tests/test_tool_parsing_wrapper_recovery.py diff --git a/routes/model_routes.py b/routes/model_routes.py index fcf9e16341..7d9bbf54d8 100644 --- a/routes/model_routes.py +++ b/routes/model_routes.py @@ -604,8 +604,7 @@ def _is_ollama_base(base_url: str) -> bool: "snowflake/arctic-embed", "nvidia/nv-embed", "embed", ) _NON_CHAT_CONTAINS = ( - "-realtime", "-transcribe", "-tts", "-codex", - "codex-", "content-safety", "-safety", "-reward", "nvclip", + "-realtime", "-transcribe", "-tts", "content-safety", "-safety", "-reward", "nvclip", "kosmos", "fuyu", "deplot", "vila", "neva", "gliner", "riva", "-parse", "-embedqa", "-nemoretriever", "topic-control", "calibration", 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_model_routes.py b/tests/test_model_routes.py index d5a5b0fdea..c0dff16a70 100644 --- a/tests/test_model_routes.py +++ b/tests/test_model_routes.py @@ -427,6 +427,21 @@ def test_gpt_audio_is_not_chat(self): def test_legacy_openai_instruct_is_not_chat(self): assert _is_chat_model("gpt-3.5-turbo-instruct") is False + @pytest.mark.parametrize("model_id", [ + "codex-reliable-coding", + "codex-auto-review", + "codex/codex-auto-review", + "codex/gpt-5.3-codex-spark", + "gpt-5.3-codex-spark", + "oc/gpt-5.2-codex", + "opencode/gpt-5.3-codex", + ]) + def test_codex_named_models_are_chat(self, model_id): + # Issue #6218: a model ID containing "codex" must not be filtered on + # name alone — OmniRoute combos like codex-reliable-coding vanished + # from discovery while an identical rename appeared immediately. + assert _is_chat_model(model_id) is True + @pytest.mark.parametrize("bad", [None, 123, 4.5, ["x"], {"a": 1}]) def test_non_string_id_is_treated_as_chat(self, bad): # Defensive boundary: a non-compliant upstream can yield a non-string 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 From 3fa32cef8ab78dffc747ff5d430a0c8901a8cc42 Mon Sep 17 00:00:00 2001 From: Spoofiecus Date: Tue, 15 Sep 2026 08:36:31 +0200 Subject: [PATCH 2/2] chore(p2): drop the codex discovery fix - #6244 already owns it --- routes/model_routes.py | 3 ++- tests/test_model_routes.py | 15 --------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/routes/model_routes.py b/routes/model_routes.py index 7d9bbf54d8..fcf9e16341 100644 --- a/routes/model_routes.py +++ b/routes/model_routes.py @@ -604,7 +604,8 @@ def _is_ollama_base(base_url: str) -> bool: "snowflake/arctic-embed", "nvidia/nv-embed", "embed", ) _NON_CHAT_CONTAINS = ( - "-realtime", "-transcribe", "-tts", "content-safety", "-safety", "-reward", "nvclip", + "-realtime", "-transcribe", "-tts", "-codex", + "codex-", "content-safety", "-safety", "-reward", "nvclip", "kosmos", "fuyu", "deplot", "vila", "neva", "gliner", "riva", "-parse", "-embedqa", "-nemoretriever", "topic-control", "calibration", diff --git a/tests/test_model_routes.py b/tests/test_model_routes.py index c0dff16a70..d5a5b0fdea 100644 --- a/tests/test_model_routes.py +++ b/tests/test_model_routes.py @@ -427,21 +427,6 @@ def test_gpt_audio_is_not_chat(self): def test_legacy_openai_instruct_is_not_chat(self): assert _is_chat_model("gpt-3.5-turbo-instruct") is False - @pytest.mark.parametrize("model_id", [ - "codex-reliable-coding", - "codex-auto-review", - "codex/codex-auto-review", - "codex/gpt-5.3-codex-spark", - "gpt-5.3-codex-spark", - "oc/gpt-5.2-codex", - "opencode/gpt-5.3-codex", - ]) - def test_codex_named_models_are_chat(self, model_id): - # Issue #6218: a model ID containing "codex" must not be filtered on - # name alone — OmniRoute combos like codex-reliable-coding vanished - # from discovery while an identical rename appeared immediately. - assert _is_chat_model(model_id) is True - @pytest.mark.parametrize("bad", [None, 123, 4.5, ["x"], {"a": 1}]) def test_non_string_id_is_treated_as_chat(self, bad): # Defensive boundary: a non-compliant upstream can yield a non-string