From 8b0bdc40be36b4d3ee8c88754307a4fbc8b4c1f5 Mon Sep 17 00:00:00 2001 From: Israel Lot Date: Wed, 29 Jul 2026 13:05:12 +0000 Subject: [PATCH 1/2] fix(execute_code): tolerate tool payloads with a trailing hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit search_files appends "[Hint: Results truncated. Use offset=N ...]" after its JSON object whenever results are truncated (tools/file_tools.py:1928). The generated hermes_tools shim parsed responses with a strict json.loads(), so any truncated search inside execute_code raised JSONDecodeError("Extra data") even though the tool call itself succeeded — the failure looked like the wrapper "choking on output". Add a shared _parse_tool_result() to the sandbox shim's common helpers and use it from both the UDS and file transports (the file transport had the same latent bug). It raw_decode()s the first JSON value and keeps any trailing text under "_hint" instead of discarding it. Truly malformed payloads still raise. --- tests/tools/test_code_execution.py | 62 ++++++++++++++++++++++++++++++ tools/code_execution_tool.py | 44 ++++++++++++++------- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/tests/tools/test_code_execution.py b/tests/tools/test_code_execution.py index a5ea6c7835490..2422284a985a7 100644 --- a/tests/tools/test_code_execution.py +++ b/tests/tools/test_code_execution.py @@ -140,6 +140,68 @@ def test_file_transport_serializes_seq_allocation(self): self.assertIn("with _seq_lock:", src) +class TestGeneratedToolResultParsing(unittest.TestCase): + """Regression: tool payloads with a trailing human-readable hint. + + Several tools (search_files when results are truncated, notably) emit a + JSON object followed by a blank line and a "[Hint: ...]" string. A strict + json.loads() on that raises JSONDecodeError("Extra data"), which used to + surface inside execute_code as a spurious parse failure even though the + underlying tool call succeeded. + """ + + def _parser_for(self, transport): + src = generate_hermes_tools_module(["search_files"], transport=transport) + ns = {} + exec(compile(src, "hermes_tools", "exec"), ns) + self.assertIn("_parse_tool_result", ns) + return ns["_parse_tool_result"] + + def test_both_transports_define_the_parser(self): + for transport in ("uds", "file"): + src = generate_hermes_tools_module(["search_files"], transport=transport) + self.assertIn("def _parse_tool_result(raw):", src) + # Neither transport should hand the raw payload to a strict loads(). + self.assertNotIn("result = json.loads(raw)", src) + + def test_plain_json_object_roundtrips(self): + for transport in ("uds", "file"): + parse = self._parser_for(transport) + result = parse(json.dumps({"total_count": 2, "matches_text": "a\nb"})) + self.assertEqual(result["total_count"], 2) + self.assertNotIn("_hint", result) + + def test_trailing_hint_is_preserved_not_raised(self): + hint = "[Hint: Results truncated. Use offset=50 to see more.]" + payload = json.dumps({"total_count": 50, "truncated": True}) + "\n\n" + hint + for transport in ("uds", "file"): + parse = self._parser_for(transport) + result = parse(payload) + self.assertEqual(result["total_count"], 50) + self.assertTrue(result["truncated"]) + self.assertEqual(result["_hint"], hint) + + def test_existing_hint_key_is_not_clobbered(self): + payload = json.dumps({"ok": True, "_hint": "from tool"}) + "\n\ntrailing" + parse = self._parser_for("uds") + self.assertEqual(parse(payload)["_hint"], "from tool") + + def test_double_encoded_payload_still_unwraps(self): + payload = json.dumps(json.dumps({"output": "hi", "exit_code": 0})) + parse = self._parser_for("file") + self.assertEqual(parse(payload)["output"], "hi") + + def test_non_json_trailing_on_json_list_does_not_raise(self): + payload = "[1, 2, 3]\n\n[Hint: more]" + parse = self._parser_for("uds") + self.assertEqual(parse(payload), [1, 2, 3]) + + def test_truly_malformed_payload_still_raises(self): + parse = self._parser_for("uds") + with self.assertRaises(json.JSONDecodeError): + parse("not json at all") + + class TestExecuteCodeRemoteTempDir(unittest.TestCase): def test_execute_remote_uses_backend_temp_dir_for_sandbox(self): class FakeEnv: diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index c69b2d5fdb215..88d95b3aa129e 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -416,6 +416,35 @@ def retry(fn, max_attempts=3, delay=2): time.sleep(delay * (2 ** attempt)) raise last_err + +def _parse_tool_result(raw): + """Parse an RPC tool-result payload into Python data. + + Some Hermes tools append a human-readable hint AFTER the JSON payload -- + e.g. search_files emits a JSON object, a blank line, then + "[Hint: Results truncated. Use offset=50 ...]" whenever results were + truncated. A strict json.loads() raises JSONDecodeError("Extra data") on + those, which used to surface inside execute_code as a spurious parse + failure even though the tool call itself succeeded. Decode the first JSON + value and keep any trailing text under the "_hint" key so nothing is lost. + """ + text = raw.strip() if isinstance(raw, str) else raw + trailing = "" + try: + result = json.loads(text) + except json.JSONDecodeError: + result, end = json.JSONDecoder().raw_decode(text) + trailing = text[end:].strip() + if isinstance(result, str): + # Doubly-encoded payload: the outer JSON value is itself a JSON string. + try: + return _parse_tool_result(result) + except (json.JSONDecodeError, TypeError): + return result + if trailing and isinstance(result, dict) and "_hint" not in result: + result["_hint"] = trailing + return result + ''' # ---- UDS transport (local backend) --------------------------------------- @@ -476,13 +505,7 @@ def _call(tool_name, args): if buf.endswith(b"\\n"): break raw = buf.decode().strip() - result = json.loads(raw) - if isinstance(result, str): - try: - return json.loads(result) - except (json.JSONDecodeError, TypeError): - return result - return result + return _parse_tool_result(raw) ''' @@ -542,12 +565,7 @@ def _call(tool_name, args): except OSError: pass - result = json.loads(raw) - if isinstance(result, str): - try: - return json.loads(result) - except (json.JSONDecodeError, TypeError): - return result + result = _parse_tool_result(raw) return result ''' From adc79ba2c8372560666258db3360c5f970c33a79 Mon Sep 17 00:00:00 2001 From: Israel Lot Date: Thu, 30 Jul 2026 17:56:35 +0000 Subject: [PATCH 2/2] fix(execute_code): keep the trailing hint when unwrapping a double-encoded payload Review feedback on #74100: - The recursive unwrap for doubly-encoded payloads discarded any suffix captured at the outer level, so a hint after the outer JSON string was silently lost. Decode iteratively and carry the suffix through the unwrap, letting an inner hint win when both levels have one. - Drop the source-substring assertion in favour of behavioral coverage: every parser case now runs against both the uds and file transports, plus regression cases for outer- and inner-level trailing hints. --- tests/tools/test_code_execution.py | 58 +++++++++++++++++++++--------- tools/code_execution_tool.py | 30 +++++++++++----- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/tests/tools/test_code_execution.py b/tests/tools/test_code_execution.py index 2422284a985a7..6686e69baef9a 100644 --- a/tests/tools/test_code_execution.py +++ b/tests/tools/test_code_execution.py @@ -157,13 +157,6 @@ def _parser_for(self, transport): self.assertIn("_parse_tool_result", ns) return ns["_parse_tool_result"] - def test_both_transports_define_the_parser(self): - for transport in ("uds", "file"): - src = generate_hermes_tools_module(["search_files"], transport=transport) - self.assertIn("def _parse_tool_result(raw):", src) - # Neither transport should hand the raw payload to a strict loads(). - self.assertNotIn("result = json.loads(raw)", src) - def test_plain_json_object_roundtrips(self): for transport in ("uds", "file"): parse = self._parser_for(transport) @@ -183,23 +176,56 @@ def test_trailing_hint_is_preserved_not_raised(self): def test_existing_hint_key_is_not_clobbered(self): payload = json.dumps({"ok": True, "_hint": "from tool"}) + "\n\ntrailing" - parse = self._parser_for("uds") - self.assertEqual(parse(payload)["_hint"], "from tool") + for transport in ("uds", "file"): + parse = self._parser_for(transport) + self.assertEqual(parse(payload)["_hint"], "from tool") def test_double_encoded_payload_still_unwraps(self): payload = json.dumps(json.dumps({"output": "hi", "exit_code": 0})) - parse = self._parser_for("file") - self.assertEqual(parse(payload)["output"], "hi") + for transport in ("uds", "file"): + parse = self._parser_for(transport) + self.assertEqual(parse(payload)["output"], "hi") + + def test_double_encoded_payload_keeps_outer_trailing_hint(self): + """The suffix survives the double-encoded unwrap. + + The outer value is a JSON string holding the real payload, and the hint + sits after that outer string. Unwrapping must not discard it. + """ + hint = "[Hint: Results truncated. Use offset=50 to see more.]" + payload = json.dumps(json.dumps({"ok": True})) + "\n\n" + hint + for transport in ("uds", "file"): + parse = self._parser_for(transport) + result = parse(payload) + self.assertTrue(result["ok"]) + self.assertEqual(result["_hint"], hint) + + def test_double_encoded_payload_keeps_inner_trailing_hint(self): + """A hint after the inner payload is preserved too.""" + hint = "[Hint: inner]" + payload = json.dumps(json.dumps({"ok": True}) + "\n\n" + hint) + for transport in ("uds", "file"): + parse = self._parser_for(transport) + result = parse(payload) + self.assertTrue(result["ok"]) + self.assertEqual(result["_hint"], hint) def test_non_json_trailing_on_json_list_does_not_raise(self): payload = "[1, 2, 3]\n\n[Hint: more]" - parse = self._parser_for("uds") - self.assertEqual(parse(payload), [1, 2, 3]) + for transport in ("uds", "file"): + parse = self._parser_for(transport) + self.assertEqual(parse(payload), [1, 2, 3]) + + def test_plain_string_payload_returned_as_is(self): + for transport in ("uds", "file"): + parse = self._parser_for(transport) + self.assertEqual(parse(json.dumps("just a message")), "just a message") def test_truly_malformed_payload_still_raises(self): - parse = self._parser_for("uds") - with self.assertRaises(json.JSONDecodeError): - parse("not json at all") + for transport in ("uds", "file"): + parse = self._parser_for(transport) + with self.assertRaises(json.JSONDecodeError): + parse("not json at all") class TestExecuteCodeRemoteTempDir(unittest.TestCase): diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index 88d95b3aa129e..f079e05b0248c 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -427,20 +427,32 @@ def _parse_tool_result(raw): those, which used to surface inside execute_code as a spurious parse failure even though the tool call itself succeeded. Decode the first JSON value and keep any trailing text under the "_hint" key so nothing is lost. + + A payload can also be doubly encoded (the outer JSON value is itself a JSON + string), and the trailing text can sit after either the outer or the inner + payload. Unwrap iteratively and carry the suffix through the unwrap instead + of dropping it. """ + def _decode(text): + """Decode the first JSON value in `text` -> (value, trailing_text).""" + try: + return json.loads(text), "" + except json.JSONDecodeError: + value, end = json.JSONDecoder().raw_decode(text) + return value, text[end:].strip() + text = raw.strip() if isinstance(raw, str) else raw - trailing = "" - try: - result = json.loads(text) - except json.JSONDecodeError: - result, end = json.JSONDecoder().raw_decode(text) - trailing = text[end:].strip() - if isinstance(result, str): + result, trailing = _decode(text) + while isinstance(result, str): # Doubly-encoded payload: the outer JSON value is itself a JSON string. try: - return _parse_tool_result(result) + inner, inner_trailing = _decode(result.strip()) except (json.JSONDecodeError, TypeError): - return result + break + result = inner + # A hint attached to the inner payload is the more specific one; keep + # the outer suffix when the inner level had none. + trailing = inner_trailing or trailing if trailing and isinstance(result, dict) and "_hint" not in result: result["_hint"] = trailing return result