diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 12ce22b175..1b2065e506 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -1003,7 +1003,9 @@ def _commit_tool_result( logger.info("tool %s completed (%.2fs, %d chars)", function_name, tool_duration, success_log_chars) if not blocked: try: - agent._record_file_mutation_result(function_name, function_args, function_result, is_error) + agent._record_file_mutation_result( + function_name, function_args, function_result, is_error, task_id=effective_task_id, + ) except Exception as _ver_err: logging.debug("file-mutation verifier record failed: %s", _ver_err) if agent.verbose_logging: diff --git a/agent/turn_explainers.py b/agent/turn_explainers.py index 9d7252ddbd..9c9bac8f3f 100644 --- a/agent/turn_explainers.py +++ b/agent/turn_explainers.py @@ -168,6 +168,28 @@ _PERSISTENCE_DEFAULT_EXPLANATION = ( ) +def _file_mutation_identity(path: str, task_id: Optional[str]) -> str: + """One key per on-disk target: the file tools' task-resolved absolute path, case-folded + on case-insensitive hosts. A failure recorded as ``notes.md`` and the write that later + lands as ``/repo/notes.md`` (or ``Notes.md`` on Windows) must meet on the same key.""" + try: + from tools.file_tools_paths import _resolve_path_for_task + + resolved = str(_resolve_path_for_task(path, task_id or "default")) + except Exception: + resolved = os.path.abspath(os.path.expanduser(path)) + return os.path.normcase(os.path.normpath(resolved)) + + +def _file_stat_signature(identity: str) -> Optional[tuple]: + """``(mtime_ns, size)`` of the target, ``None`` when it does not exist (or cannot be read).""" + try: + st = os.stat(identity) + except OSError: + return None + return (st.st_mtime_ns, st.st_size) + + def _display_flag_enabled(agent, *, env_var: str, config_key: str, cache_attr: str) -> bool: """``display.`` (default True), cached per agent on ``cache_attr``. @@ -201,11 +223,14 @@ class TurnExplainersMixin: """File-mutation failure footer + turn-completion explainer (see module docstring).""" def _record_file_mutation_result( - self, tool_name: str, args: Dict[str, Any], result: Any, is_error: bool + self, tool_name: str, args: Dict[str, Any], result: Any, is_error: bool, + *, task_id: Optional[str] = None, ) -> None: """Record a ``write_file`` / ``patch`` outcome for the turn-end verifier. - Failures store ``{path: {error_preview, tool}}``; a later success on the same path removes the entry. + Failures store ``{path: {error_preview, tool, identity, stat}}`` keyed by the model's + spelling; ``identity`` is the resolved on-disk target and ``stat`` its signature at + failure time. A later success on the same identity (any spelling) removes the entry. No-op when the per-turn state dict is not initialised (tool dispatched outside ``run_conversation``). """ if tool_name not in _FILE_MUTATING_TOOLS: @@ -233,10 +258,33 @@ class TurnExplainersMixin: # Keep the FIRST error per path unless a later success replaces it. preview = _extract_error_preview(result) for path in targets: - state.setdefault(path, {"tool": tool_name, "error_preview": preview}) + identity = _file_mutation_identity(path, task_id) + state.setdefault(path, { + "tool": tool_name, "error_preview": preview, + "identity": identity, "stat": _file_stat_signature(identity), + }) else: - for path in targets: - state.pop(path, None) + cleared = { + _file_mutation_identity(p, task_id) + for p in (landed_paths if landed else targets) + } + for path, info in list(state.items()): + if info.get("identity", _file_mutation_identity(path, task_id)) in cleared: + state.pop(path, None) + + @staticmethod + def _file_mutations_still_failed(failed: Dict[str, Dict[str, Any]]) -> Dict[str, Dict[str, Any]]: + """Drop entries whose target changed on disk since the failed call. + + The recorder only sees write_file/patch receipts; a terminal redirect or an + execute_code write leaves none. Re-checking the stat signature at turn end keeps + the footer from listing a file that was in fact modified later this turn. Entries + without a snapshot (hand-built dicts) are kept as-is. + """ + return { + path: info for path, info in failed.items() + if "stat" not in info or _file_stat_signature(info["identity"]) == info["stat"] + } def _file_mutation_verifier_enabled(self) -> bool: """``display.file_mutation_verifier`` / ``HERMES_FILE_MUTATION_VERIFIER`` (a patchable seam).""" @@ -280,9 +328,9 @@ class TurnExplainersMixin: return "" lines = [ "⚠️ File-mutation verifier: " - f"{len(failed)} file(s) were NOT modified this turn despite any " + f"{len(failed)} file edit(s) FAILED this turn despite any " "wording above that may suggest otherwise. Run `git status` or " - "`read_file` to confirm." + "`read_file` to confirm what actually landed." ] shown = list(failed.items())[:10] for path, info in shown: diff --git a/agent/turn_finalizer.py b/agent/turn_finalizer.py index a98aa461b3..bef4a3cd37 100644 --- a/agent/turn_finalizer.py +++ b/agent/turn_finalizer.py @@ -348,6 +348,7 @@ def _append_file_mutation_footer(agent, final_response, logger): # Empty/interrupted turns already have other surface text that shouldn't be augmented. _failed = getattr(agent, "_turn_failed_file_mutations", None) or {} if _failed and agent._file_mutation_verifier_enabled(): + _failed = agent._file_mutations_still_failed(_failed) footer = agent._format_file_mutation_failure_footer(_failed) if footer: final_response = final_response.rstrip() + "\n\n" + footer diff --git a/tests/agent/test_file_mutation_verifier.py b/tests/agent/test_file_mutation_verifier.py index 086f198952..7e756a76e1 100644 --- a/tests/agent/test_file_mutation_verifier.py +++ b/tests/agent/test_file_mutation_verifier.py @@ -227,6 +227,44 @@ class TestRecordFileMutationResult: # the initial root cause. assert "first error" in agent._turn_failed_file_mutations["/tmp/a.md"]["error_preview"] + def test_success_under_another_spelling_clears_failure(self, tmp_path, monkeypatch): + """A failure recorded as a relative path is cleared by the write that lands on the + resolved absolute path: entries are matched on the on-disk target, not the model's + spelling (#111771).""" + monkeypatch.setenv("TERMINAL_CWD", str(tmp_path)) + task = "fmv-spelling-task" + agent = _bare_agent() + agent._record_file_mutation_result( + "patch", {"mode": "replace", "path": "notes.md", "old_string": "x", "new_string": "y"}, + json.dumps({"error": "Could not find old_string"}), is_error=True, task_id=task, + ) + assert "notes.md" in agent._turn_failed_file_mutations + landed = str(tmp_path / "notes.md") + agent._record_file_mutation_result( + "write_file", {"path": landed, "content": "y"}, + json.dumps({"bytes_written": 1, "files_modified": [landed]}), is_error=False, task_id=task, + ) + assert agent._turn_failed_file_mutations == {} + + def test_footer_omits_file_changed_after_failed_call(self, tmp_path): + """A file modified after the failed call (terminal redirect, execute_code — no receipt) + is not reported at turn end; an untouched one still is (#111771).""" + changed, untouched = tmp_path / "changed.txt", tmp_path / "untouched.txt" + changed.write_text("v1") + untouched.write_text("c") + agent = _bare_agent() + for target in (changed, untouched): + agent._record_file_mutation_result( + "patch", {"mode": "replace", "path": str(target), "old_string": "x", "new_string": "y"}, + json.dumps({"error": "Could not find old_string"}), is_error=True, + ) + import os + st = changed.stat() + changed.write_text("v2 rewritten") + os.utime(changed, ns=(st.st_atime_ns, st.st_mtime_ns + 1_000_000_000)) + still_failed = agent._file_mutations_still_failed(agent._turn_failed_file_mutations) + assert list(still_failed) == [str(untouched)] + @@ -244,7 +282,10 @@ class TestFormatFooter: out = AIAgent._format_file_mutation_failure_footer( {"/tmp/a.md": {"tool": "patch", "error_preview": "Could not find old_string"}}, ) - assert "1 file(s) were NOT modified" in out + # The recorder only sees tool receipts, so the header states what it knows (the + # call failed), never that no bytes changed (#111771). + assert "1 file edit(s) FAILED" in out + assert "NOT modified" not in out assert "/tmp/a.md" in out assert "Could not find old_string" in out assert "git status" in out # user-actionable hint @@ -255,7 +296,7 @@ class TestFormatFooter: for i in range(15) } out = AIAgent._format_file_mutation_failure_footer(failed) - assert "15 file(s) were NOT modified" in out + assert "15 file edit(s) FAILED" in out assert "… and 5 more" in out # Ten file bullets + header + "and X more" line lines = out.split("\n") diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index e17b0c47d5..d5e868d412 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -2061,12 +2061,12 @@ Both keys are display-only and CLI-only: they are suppressed in quiet mode, when ### File-mutation verifier -When `display.file_mutation_verifier` is `true` (default), Hermes appends a one-line advisory to the assistant's final response whenever a `write_file` or `patch` call failed during the turn and was never superseded by a successful write to the same path. This catches the "batch of parallel patches, half silently fail, model summarises success" class of over-claim without requiring you to manually run `git status` after every edit. +When `display.file_mutation_verifier` is `true` (default), Hermes appends a one-line advisory to the assistant's final response whenever a `write_file` or `patch` call failed during the turn and the target file was neither written successfully afterwards (under any spelling of its path) nor otherwise changed on disk before the turn ended. This catches the "batch of parallel patches, half silently fail, model summarises success" class of over-claim without requiring you to manually run `git status` after every edit. Example footer: ``` -⚠️ File-mutation verifier: 3 file(s) were NOT modified this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm. +⚠️ File-mutation verifier: 3 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm what actually landed. • concepts/automatic-organization.md — [patch] Could not find match for old_string • concepts/lora.md — [patch] Could not find match for old_string • concepts/rag-pipeline.md — [patch] Could not find match for old_string @@ -2074,7 +2074,7 @@ Example footer: Set `file_mutation_verifier: false` (or `HERMES_FILE_MUTATION_VERIFIER=0`) to suppress the footer. The verifier only fires when real failures are outstanding at turn end — a model that retries a failed patch and succeeds within the same turn will not trigger it for that file. -**Trust the verifier over the model's summary.** The footer means the listed files were **not** modified on disk, even if the assistant's closing message says the task is done. Common causes: +**Trust the verifier over the model's summary.** The footer means the listed edit calls **failed** and Hermes saw no later change to those files, even if the assistant's closing message says the task is done. It only tracks `write_file`/`patch` receipts plus a modification-time check at turn end, so run `git status` or `read_file` to confirm what actually landed. Common causes: - **Write denied** — path is on the credential denylist or outside `HERMES_WRITE_SAFE_ROOT` (see [File write safety](./security.md#file-write-safety)) - **Patch mismatch** — `old_string` did not match the file on disk @@ -2083,7 +2083,7 @@ Set `file_mutation_verifier: false` (or `HERMES_FILE_MUTATION_VERIFIER=0`) to su Example footer when writes are blocked: ``` -⚠️ File-mutation verifier: 2 file(s) were NOT modified this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm. +⚠️ File-mutation verifier: 2 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm what actually landed. • ~/.hermes/cron/jobs.json — [patch] Write denied: '…' is outside HERMES_WRITE_SAFE_ROOT (/path/to/project) • ~/.hermes/scripts/monitor.py — [write_file] Write denied: '…' is outside HERMES_WRITE_SAFE_ROOT (/path/to/project) ``` diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/configuration.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/configuration.md index 45a87b7cb3..ec754bc726 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/configuration.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/configuration.md @@ -1232,7 +1232,7 @@ display: 示例页脚： ``` -⚠️ File-mutation verifier: 3 file(s) were NOT modified this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm. +⚠️ File-mutation verifier: 3 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm what actually landed. • concepts/automatic-organization.md — [patch] Could not find match for old_string • concepts/lora.md — [patch] Could not find match for old_string • concepts/rag-pipeline.md — [patch] Could not find match for old_string