From 3e1bfb2935faea19cb1ae9965918fcc5638bf6cb Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Mon, 28 Sep 2026 01:51:01 -0700
Subject: [PATCH] fix(execute_code): report in-script tool errors the script
 ignored

A script that calls hermes_tools.write_file/patch and never prints the
returned {"error": ...} got status=success with empty output, so the
model believed the write happened. In a cross-agent bench this cost 3 of
18 Opus runs 4-6 extra turns (write_file refused by the read-before-write
guard, then re-probing and re-emitting whole files). The result now
carries tool_errors for that cell, and the in-script write_file doc says
existing files must be read first.
---
 tests/tools/test_code_kernel.py | 16 ++++++++++++++++
 tools/code_execution_rpc.py     | 27 +++++++++++++++++++++++++--
 tools/code_execution_tool.py    | 13 +++++++++----
 tools/code_kernel.py            |  6 ++++++
 tools/code_kernel_remote.py     |  6 ++++--
 5 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/tests/tools/test_code_kernel.py b/tests/tools/test_code_kernel.py
index 1eb892f1f8..9846656f0e 100644
--- a/tests/tools/test_code_kernel.py
+++ b/tests/tools/test_code_kernel.py
@@ -428,6 +428,22 @@ class TestKernelOwnershipAndLifecycle(unittest.TestCase):
             self.assertIsNotNone(proc.returncode)
 
 
+class TestInScriptToolErrors(unittest.TestCase):
+    def test_ignored_helper_error_is_reported_for_that_cell_only(self):
+        """A script that drops a helper's {"error": ...} return must not read as a clean success."""
+        def _handle(tool_name, tool_args, task_id=None):
+            if tool_name == "write_file":
+                return json.dumps({"error": "Refusing to overwrite a.py: never read"})
+            return json.dumps({"ok": True})
+
+        with _kernel_config(), patch("model_tools.handle_function_call", new=_handle):
+            failed = _run("import hermes_tools\nhermes_tools.write_file('a.py', 'x')\nprint('done')\n")
+            clean = _run("import hermes_tools\nhermes_tools.web_search(query='q')\n")
+        self.assertEqual(failed["status"], "success", failed)
+        self.assertEqual(failed["tool_errors"], [{"tool": "write_file", "error": "Refusing to overwrite a.py: never read"}])
+        self.assertNotIn("tool_errors", clean)
+
+
 class TestPerCellRpcAuthority(unittest.TestCase):
     """Interpreter state persists across cells; RPC authority must not."""
 
diff --git a/tools/code_execution_rpc.py b/tools/code_execution_rpc.py
index e1a5400149..de15d75c84 100644
--- a/tools/code_execution_rpc.py
+++ b/tools/code_execution_rpc.py
@@ -108,11 +108,34 @@ def _handle_rpc_request(request: dict, *, allowed_tools: frozenset, tool_call_co
         logger.error("Tool call failed in %s: %s", where, exc, exc_info=True)
         result = tool_error(str(exc))
     tool_call_counter[0] += 1
-    tool_call_log.append({"tool": tool_name, "args_preview": str(tool_args)[:80],
-                          "duration": round(time.monotonic() - call_start, 2)})
+    entry = {"tool": tool_name, "args_preview": str(tool_args)[:80],
+             "duration": round(time.monotonic() - call_start, 2)}
+    error = _result_error(result)
+    if error:
+        entry["error"] = error
+    tool_call_log.append(entry)
     return result
 
 
+def _result_error(result) -> str:
+    """The ``error`` text of a JSON-object tool result, else ``""``."""
+    if not isinstance(result, str) or not result.startswith("{") or '"error"' not in result:
+        return ""
+    try:
+        body = json.loads(result)
+    except ValueError:
+        return ""
+    error = body.get("error") if isinstance(body, dict) else None
+    return str(error)[:300] if error else ""
+
+
+def tool_errors_since(tool_call_log: list, start: int = 0) -> list:
+    """Failed in-script tool calls since *start*, for the execute_code result: a script that
+    ignores a helper's ``{"error": ...}`` return would otherwise report plain success while
+    the write/patch it relied on never happened."""
+    return [{"tool": e["tool"], "error": e["error"]} for e in tool_call_log[start:] if e.get("error")][:5]
+
+
 def _rpc_server_loop(server_sock: socket.socket, task_id: str, tool_call_log: list,
                      tool_call_counter: list, max_tool_calls: int, allowed_tools: frozenset,
                      stop_event: threading.Event, rpc_token: str, dispatch=None):
diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py
index eeb8a475df..03df70733f 100644
--- a/tools/code_execution_tool.py
+++ b/tools/code_execution_tool.py
@@ -30,7 +30,7 @@ from tools.registry import registry, tool_error
 from hermes_time import get_timezone_name
 from tools.code_execution_env import _resolve_child_cwd, _resolve_child_python
 from tools.code_execution_rpc import (
-    _execute_checked, _private_dirs_cmd, _remote_write, _rpc_poll_loop,
+    _execute_checked, _private_dirs_cmd, _remote_write, _rpc_poll_loop, tool_errors_since,
 )
 from tools.tool_output_truncate import head_tail_split, truncation_notice
 
@@ -576,6 +576,8 @@ def _finish_remote_kernel_result(kernel_result: Dict[str, Any], *,
     result = _remote_result(kernel_result.get("status", "error"), stdout_text, exec_start,
                             {"tool_calls_made": kernel_result.get("tool_calls_made", 0)},
                             kernel=kernel_result.get("kernel", {"remote": True}))
+    if kernel_result.get("tool_errors"):
+        result["tool_errors"] = kernel_result["tool_errors"]
     if result["status"] == "timeout":
         _apply_timeout(result, f"Cell timed out after {timeout}s; the remote session kernel was "
                                "killed and its state was lost. The next call starts fresh.")
@@ -596,7 +598,7 @@ def _run_remote_per_call(env, env_type: str, code: str, effective_task_id: str,
     serve file-RPC from a polling thread, run, clean up."""
     sandbox_dir = f"{_env_temp_dir(env)}/hermes_exec_{uuid.uuid4().hex[:12]}"
     quoted_sandbox_dir = shlex.quote(sandbox_dir)
-    tool_call_counter, stop_event, rpc_thread = [0], threading.Event(), None
+    tool_call_counter, tool_call_log, stop_event, rpc_thread = [0], [], threading.Event(), None
     try:
         # Private dirs: the sandbox lives under a shared temp dir and carries the
         # RPC token (in req files) and tool results. Fail closed on setup
@@ -612,7 +614,7 @@ def _run_remote_per_call(env, env_type: str, code: str, effective_task_id: str,
         # See #30882.
         rpc_thread = threading.Thread(
             target=propagate_context_to_thread(_rpc_poll_loop), daemon=True,
-            args=(env, f"{sandbox_dir}/rpc", effective_task_id, [], tool_call_counter,
+            args=(env, f"{sandbox_dir}/rpc", effective_task_id, tool_call_log, tool_call_counter,
                   max_tool_calls, sandbox_tools, stop_event, rpc_token))
         rpc_thread.start()
         # The token travels in a sourced env file, never in argv. No umask on
@@ -639,6 +641,9 @@ def _run_remote_per_call(env, env_type: str, code: str, effective_task_id: str,
             logger.debug("Failed to clean up remote sandbox %s", sandbox_dir)
     result = _remote_result(status, stdout_text, exec_start,
                             {"exit_code": exit_code, "tool_calls_made": tool_call_counter[0]})
+    tool_errors = tool_errors_since(tool_call_log)
+    if tool_errors:
+        result["tool_errors"] = tool_errors
     if status == "timeout":
         _apply_timeout(result, f"Script timed out after {timeout}s and was killed.")
         logger.warning("execute_code (remote) timed out after %ss (limit %ss) with %d tool calls",
@@ -858,7 +863,7 @@ _TOOL_DOC_LINES = [
      "    No LLM summarization. Pages over char_limit (default 15000) are head+tail truncated; full text stored on disk (path in the content footer)."),
     ("read_file", "  read_file(path: str, offset: int = 1, limit: int = 2000) -> dict\n"
      "    Lines are 1-indexed. Returns {\"content\": \"...\", \"total_lines\": N}"),
-    ("write_file", "  write_file(path: str, content: str) -> dict\n    Always overwrites the entire file."),
+    ("write_file", "  write_file(path: str, content: str) -> dict\n    Always overwrites the entire file; an existing file must be read_file'd first or the write is refused."),
     ("search_files", "  search_files(pattern: str, target=\"content\", path=\".\", file_glob=None, limit=50, order=\"discovery\") -> dict\n"
      "    target: \"content\" (search inside files) or \"files\" (find files by name). Returns {\"matches\": [...]}"),
     ("patch", "  patch(path: str, old_string: str, new_string: str, replace_all: bool = False) -> dict\n"
diff --git a/tools/code_kernel.py b/tools/code_kernel.py
index 8da8629297..ec09679576 100644
--- a/tools/code_kernel.py
+++ b/tools/code_kernel.py
@@ -314,6 +314,7 @@ class SessionKernel:
         self.stop_event = threading.Event()
         self.death_pipe_w: Optional[int] = None
         self.tool_call_log: List = []
+        self.cell_log_start = 0
         self.tool_call_counter: List[int] = [0]
         # Cells currently attached (bumped under the registry lock on selection, dropped when the
         # cell settles). Reaping/cap-eviction skip attached kernels: tearing one down mid-spawn
@@ -802,6 +803,10 @@ def _cell_result(kernel: SessionKernel, key: Tuple, status: str, payload: Dict[s
                    "execution_count": kernel.execution_count, "state_reset": state_reset},
     }
     result.update(stdout_metadata)
+    from tools.code_execution_rpc import tool_errors_since
+    tool_errors = tool_errors_since(kernel.tool_call_log, kernel.cell_log_start)
+    if tool_errors:
+        result["tool_errors"] = tool_errors
     # Cell-side spill (runner clipped before replying): same read_file recipe as the host-side spill.
     cell_spill = str(payload.get("stdout_spill_path", "") or "")
     if cell_spill and payload.get("stdout_clipped"):
@@ -882,6 +887,7 @@ def _run_cell(kernel: SessionKernel, key: Tuple, code: str, *, task_id: str, chi
             assert kernel.proc is not None and kernel.proc.stdin is not None
             # Per-cell tool budget: the RPC loop enforces counter < max; reset without restarting.
             kernel.tool_call_counter[0] = 0
+            kernel.cell_log_start = len(kernel.tool_call_log)
             kernel.raw.drain(), kernel.stderr.drain()  # raw output leaked between cells belongs to no cell
             kernel.cell_authority = authority
             kernel.proc.stdin.write((json.dumps({"id": uuid.uuid4().hex, "code": code}) + "\n").encode("utf-8"))
diff --git a/tools/code_kernel_remote.py b/tools/code_kernel_remote.py
index e8065e3a69..e029b0a8b8 100644
--- a/tools/code_kernel_remote.py
+++ b/tools/code_kernel_remote.py
@@ -366,6 +366,7 @@ def execute_in_remote_kernel(
 def _run_attached_cell(kernel: RemoteKernel, key: Tuple, code: str, *, env, task_env_id: str,
                        sandbox_tools: frozenset, timeout: int, max_tool_calls: int,
                        reused: bool, state_reset: bool, state_lost: bool) -> Dict[str, Any]:
+    from tools.code_execution_rpc import tool_errors_since
     from tools.code_execution_tool import _rpc_poll_loop
     from tools.thread_context import propagate_context_to_thread
     # Clean stale tool-RPC requests from a previous cell before arming this cell's poll loop, so
@@ -375,12 +376,12 @@ def _run_attached_cell(kernel: RemoteKernel, key: Tuple, code: str, *, env, task
         kernel.sh(f"rm -f {q_rpc}/req_* {q_rpc}/res_*", timeout=10)
     except Exception:
         pass
-    tool_call_counter, stop_event = [0], threading.Event()
+    tool_call_counter, tool_call_log, stop_event = [0], [], threading.Event()
     # Per-cell RPC thread carrying THIS call's approval/session context — the remote analogue
     # of CellAuthority: authority lives exactly as long as the cell's poll loop.
     rpc_thread = threading.Thread(
         target=propagate_context_to_thread(_rpc_poll_loop), daemon=True,
-        args=(env, f"{kernel.kernel_dir}/rpc", task_env_id, [], tool_call_counter,
+        args=(env, f"{kernel.kernel_dir}/rpc", task_env_id, tool_call_log, tool_call_counter,
               max_tool_calls, sandbox_tools, stop_event, kernel.rpc_token))
     rpc_thread.start()
     cell_status, cell_payload = "no-result", {}
@@ -401,6 +402,7 @@ def _run_attached_cell(kernel: RemoteKernel, key: Tuple, code: str, *, env, task
     result: Dict[str, Any] = {
         "status": "error", "stdout": cell_payload.get("stdout", ""), "stderr": cell_payload.get("stderr", ""),
         "traceback": cell_payload.get("traceback", ""), "tool_calls_made": tool_call_counter[0], "kernel": kernel_info,
+        "tool_errors": tool_errors_since(tool_call_log),
     }
     if cell_status in ("timeout", "protocol-error", "no-result"):
         # No safe way to interrupt one cell in place (same contract as local): kill, report, respawn.