From a46b4ce51df73d7480afe7abbbf88d9c68727a92 Mon Sep 17 00:00:00 2001 From: simonbbby Date: Wed, 29 Jul 2026 14:38:47 -0400 Subject: [PATCH] fix(gate): don't read file content as a tool failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detect_failure() text-greps the tool response for failure words. For Bash that response is a command log, so the heuristic is right. For Edit/Write it is the file itself, so editing a doc containing 'Failed attempts', a changelog line 'fixed the failure', or a fixture with '3 errors' was reported as a failed tool call — and the agent was then told not to report completion. In one repo this fired on essentially every edit to a status doc that has a '## Failed attempts' section. Content tools signal failure structurally (success / ok / exit_code), never in prose, so they now consult only that signal: - new structural_success() reads explicit result fields only - exit_success() delegates to it, then falls back to the text heuristic - detect_failure() returns early for CONTENT_TOOLS Bash behaviour is unchanged, including textual-only failures with no exit code. A genuinely failed Edit is still caught, via the structural signal. tests/test_content_tool_failures.py covers it: 14 checks, and 7 of them fail against the unpatched parser. --- scripts/gate/parse_tool_result.py | 24 +++++++-- tests/test_content_tool_failures.py | 78 +++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 tests/test_content_tool_failures.py diff --git a/scripts/gate/parse_tool_result.py b/scripts/gate/parse_tool_result.py index 2fb77da..0711cb7 100644 --- a/scripts/gate/parse_tool_result.py +++ b/scripts/gate/parse_tool_result.py @@ -27,6 +27,9 @@ r"tests? failed|build failed|lint failed)" ) SUCCESS_RE = re.compile(r"(?i)\b(passed|success|succeeded|0 failed|build completed|done|valid)\b") +CONTENT_TOOLS = frozenset( + {"Edit", "Write", "NotebookEdit", "MultiEdit", "Read", "NotebookRead"} +) MUTATING_BASH_RE = re.compile( r"(?i)\b(apply_patch|python\s+.*\s+-m\s+compileall|chmod|mkdir|mv|cp|rm|touch|" r"npm\s+run\s+build|pnpm\s+build|yarn\s+build)\b" @@ -65,7 +68,8 @@ def command_from_input(input_data: dict[str, Any]) -> str: return "" -def exit_success(input_data: dict[str, Any], text: str) -> bool | None: +def structural_success(input_data: dict[str, Any]) -> bool | None: + """Success/failure from explicit result fields only — never from response text.""" candidates = [input_data, input_data.get("tool_response")] for candidate in candidates: if isinstance(candidate, dict): @@ -78,6 +82,13 @@ def exit_success(input_data: dict[str, Any], text: str) -> bool | None: return value == 0 if isinstance(value, str) and value.isdigit(): return int(value) == 0 + return None + + +def exit_success(input_data: dict[str, Any], text: str) -> bool | None: + structural = structural_success(input_data) + if structural is not None: + return structural if FAILURE_RE.search(text): return False if SUCCESS_RE.search(text): @@ -91,8 +102,15 @@ def is_verification_command(command: str) -> bool: def detect_failure(input_data: dict[str, Any]) -> dict[str, Any] | None: text = response_text(input_data.get("tool_response", input_data)) - success = exit_success(input_data, text) - if success is False or (success is None and FAILURE_RE.search(text)): + structural = structural_success(input_data) + if structural is False: + return {"kind": "tool-result", "summary": redact(text or command_from_input(input_data), 240)} + # For file-content tools the response text IS the file, not a command log, so + # words like "failed" or "3 errors" inside the content are not tool failures. + # These tools signal failure structurally (handled above), never in prose. + if str(input_data.get("tool_name") or "") in CONTENT_TOOLS: + return None + if structural is None and FAILURE_RE.search(text): return {"kind": "tool-result", "summary": redact(text or command_from_input(input_data), 240)} return None diff --git a/tests/test_content_tool_failures.py b/tests/test_content_tool_failures.py new file mode 100644 index 0000000..046bf68 --- /dev/null +++ b/tests/test_content_tool_failures.py @@ -0,0 +1,78 @@ +#!/usr/bin/env python3 +"""Regression: file content must not be mistaken for a tool failure. + +detect_failure() text-greps the tool response for failure words. For Bash that +response is a command log, so the heuristic is right. For Edit/Write it is the +FILE ITSELF — so editing a doc containing "Failed attempts", a changelog saying +"fixed the failure", or a test fixture with "3 errors" was reported as a failed +tool call, and the agent was told not to report completion. + +These tools signal failure structurally (success/ok/exit_code), never in prose, +so content tools now consult only that signal. Bash behaviour is unchanged. +""" + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts", "gate")) + +from parse_tool_result import detect_failure # noqa: E402 + + +def edit(content, tool="Edit"): + return {"tool_name": tool, "tool_input": {"file_path": "/repo/notes.md"}, + "tool_response": {"content": content}} + + +# (label, payload, expected_failure) +CASES = [ + # --- the bug: prose inside an edited file is not a tool failure --- + ("edit: '## Failed attempts' heading", edit("## Failed attempts"), False), + ("edit: prose 'documents a failure mode'", edit("documents a failure mode"), False), + ("edit: 'probe reports 1 error, 0 warnings'", edit("probe reports 1 error, 0 warnings"), False), + ("edit: changelog 'fixed the failure'", edit("- fixed the failure in the parser"), False), + ("edit: 'error: ' inside a code sample", edit("example output:\nerror: cannot find module"), False), + ("write: same content via Write", edit("## Failed attempts", tool="Write"), False), + ("read: same content via Read", edit("build failed", tool="Read"), False), + ("edit: clean content (control)", edit("all good here"), False), + + # --- still caught: content tools that really failed report it structurally --- + ("edit: structural success=False", { + "tool_name": "Edit", "tool_input": {"file_path": "/repo/a.py"}, + "tool_response": {"success": False, "error": "String to replace not found"}}, True), + ("write: structural exit_code=1", { + "tool_name": "Write", "tool_input": {"file_path": "/repo/a.py"}, + "tool_response": {"exit_code": 1}}, True), + + # --- unchanged: Bash output really is a command log --- + ("bash: nonzero exit", { + "tool_name": "Bash", "tool_input": {"command": "pytest"}, + "tool_response": {"stdout": "2 failed, 3 passed", "exit_code": 1}}, True), + ("bash: textual failure, no exit code", { + "tool_name": "Bash", "tool_input": {"command": "npm run build"}, + "tool_response": {"stdout": "build failed"}}, True), + ("bash: traceback", { + "tool_name": "Bash", "tool_input": {"command": "python x.py"}, + "tool_response": {"stdout": "Traceback (most recent call last):"}}, True), + ("bash: clean run", { + "tool_name": "Bash", "tool_input": {"command": "pytest"}, + "tool_response": {"stdout": "5 passed in 0.31s", "exit_code": 0}}, False), +] + + +def main(): + bad = 0 + for label, payload, want in CASES: + got = bool(detect_failure(payload)) + if got != want: + bad += 1 + print(f"MISMATCH expected_failure={want} got={got} {label}") + if bad: + print(f"RESULT: {bad}/{len(CASES)} mismatches") + return 1 + print(f"RESULT: all pass ({len(CASES)} checks)") + return 0 + + +if __name__ == "__main__": + sys.exit(main())