From fcd5e2cc61f4f3418e68c4eb81f690b15e33f66d Mon Sep 17 00:00:00 2001 From: dsad Date: Sat, 1 Aug 2026 13:33:00 -0700 Subject: [PATCH] fix(file-tools): resolve local V4A patch paths before apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch_tool resolved V4A header paths against the task workspace for locking, staleness, and reporting, but handed the original (often relative) patch text to file_ops.patch_v4a — which re-resolved headers against the backend env's own cwd. When the two diverge (the git-worktree cwd bug), a relative header landed in a different directory than everything the tool locked and reported: a silent wrong-file write. Rewrite Update/Add/Delete/Move File headers to the resolved absolute paths before apply, only for host-filesystem backends (container/remote namespaces keep their own paths). Header patterns mirror patch_parser (no-space ***Update File: form) and cover Move File: src -> dst. Salvage of #53176 by @necoweb3, reimplemented onto current main (the original branch predates the sensitive-path/Move-header extraction and per-path locking now in patch_tool). Co-authored-by: necoweb3 --- tests/tools/test_file_tools_cwd_resolution.py | 56 +++++++++++++ tools/file_tools.py | 83 ++++++++++++++++++- 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_file_tools_cwd_resolution.py b/tests/tools/test_file_tools_cwd_resolution.py index 2fc5c93a360f0..28f7ac5f5d81e 100644 --- a/tests/tools/test_file_tools_cwd_resolution.py +++ b/tests/tools/test_file_tools_cwd_resolution.py @@ -254,3 +254,59 @@ def test_unregistered_session_never_inherits_another_sessions_record( assert not str(resolved).startswith(str(wt_a)) assert not str(resolved).startswith(str(wt_b)) assert resolved == (main / "target.py").resolve() + + +def test_v4a_patch_applies_to_resolved_workspace_not_backend_cwd( + _isolated_cwd, monkeypatch +): + """V4A patch must edit the path the tool layer resolved, not the shell cwd. + + Regression for the git-worktree cwd bug: ``patch_tool`` resolved header + paths against the task workspace for locking/staleness/reporting, but the + raw (relative) patch text was handed to ``file_ops.patch_v4a``, which + re-resolved it against the backend env's own cwd. A relative header then + landed in a different directory than everything the tool reported. The fix + rewrites headers to the resolved absolute paths before apply. + """ + import json + + workspace, decoy = _isolated_cwd + task_id = "sess-v4a" + + # Tool layer resolves against the workspace (worktree registration path). + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + monkeypatch.setattr(ft, "_file_ops_cache", {}) + terminal_tool.register_task_env_overrides(task_id, {"cwd": str(workspace)}) + + # Backend file_ops lives in the DECOY dir — the divergence the fix closes. + from tools.environments.local import LocalEnvironment + from tools.file_operations import ShellFileOperations + + env = LocalEnvironment(cwd=str(decoy)) + monkeypatch.setattr( + ft, "_get_file_ops", lambda task_id="default": ShellFileOperations(env) + ) + + out = json.loads( + ft.patch_tool( + mode="patch", + patch=( + "*** Begin Patch\n" + "*** Update File: target.py\n" + "@@\n" + "-WORKSPACE_ORIGINAL\n" + "+WORKSPACE_PATCHED\n" + "*** End Patch\n" + ), + task_id=task_id, + ) + ) + + expected = str((workspace / "target.py").resolve()) + assert not out.get("error"), out + assert out.get("resolved_path") == expected + assert out.get("files_modified") == [expected] + # The workspace file — which the tool locked and reported — was edited. + assert (workspace / "target.py").read_text() == "WORKSPACE_PATCHED\n" + # The decoy (backend cwd) was left untouched. + assert (decoy / "target.py").read_text() == "DECOY_ORIGINAL\n" diff --git a/tools/file_tools.py b/tools/file_tools.py index 6a100c0f07d31..737d728c41393 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -439,6 +439,79 @@ def _path_resolution_warning(filepath: str, resolved: Path, task_id: str = "defa return None +def _file_ops_uses_host_paths(file_ops) -> bool: + """Return True when *file_ops* targets the same host filesystem as Hermes. + + Only then may we rewrite V4A header paths to resolved host-absolute + paths: a container/remote backend has its own filesystem namespace where + a host-absolute path would be meaningless. + """ + env = getattr(file_ops, "env", None) + if env is None: + return True + try: + from tools.environments.local import LocalEnvironment + except ImportError: + return True + return isinstance(env, LocalEnvironment) + + +def _rewrite_v4a_patch_paths_for_host( + patch: str, + path_to_resolved: dict, + file_ops, +) -> str: + """Rewrite V4A file headers to the exact host paths the tool layer resolved. + + ``patch_tool`` resolves every header path against the task's workspace for + locking, staleness, and reporting, but historically handed the *original* + patch text to ``file_ops.patch_v4a`` — so the shell layer re-resolved the + (often relative) header against its own cwd, which can differ from the + tool layer's workspace (the git-worktree cwd bug). That made a relative + header land in a different directory than everything else the tool + reported. This rewrites ``*** Update/Add/Delete/Move File:`` headers to the + resolved absolute paths so both layers agree on the target. + + Header patterns mirror ``patch_parser`` (``\\s*`` after ``***`` accepts the + no-space ``***Update File:`` form) and cover ``Move File: src -> dst``. + Only applied when *file_ops* targets the host filesystem. + """ + if not _file_ops_uses_host_paths(file_ops): + return patch + + import re as _re + + def _resolved_or_original(raw: str) -> str: + raw = raw.strip() + return path_to_resolved.get(raw) or raw + + def _replace_single(match): + prefix = match.group(1) + resolved = _resolved_or_original(match.group(2)) + return f"{prefix}{resolved}" + + patch = _re.sub( + r'^(\*\*\*\s*(?:Update|Add|Delete)\s+File:\s*)(.+)$', + _replace_single, + patch, + flags=_re.MULTILINE, + ) + + def _replace_move(match): + prefix = match.group(1) + src = _resolved_or_original(match.group(2)) + dst = _resolved_or_original(match.group(3)) + return f"{prefix}{src} -> {dst}" + + patch = _re.sub( + r'^(\*\*\*\s*Move\s+File:\s*)(.+?)\s*->\s*(.+)$', + _replace_move, + patch, + flags=_re.MULTILINE, + ) + return patch + + def _is_blocked_device_path(path: str) -> bool: """Return True for concrete device/fd paths that can hang reads.""" normalized = os.path.normpath(_expand_tilde(path)) @@ -1768,7 +1841,15 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None, elif mode == "patch": if not patch: return tool_error("patch content required") - result = file_ops.patch_v4a(patch) + # Rewrite V4A headers to the resolved absolute paths so the + # shell layer patches the exact files the tool layer resolved + # (locked/reported). Without this a relative header re-resolves + # against the shell's cwd, which can differ from the workspace + # (git-worktree cwd bug) — landing the edit elsewhere. + patch_for_ops = _rewrite_v4a_patch_paths_for_host( + patch, _path_to_resolved, file_ops + ) + result = file_ops.patch_v4a(patch_for_ops) else: return tool_error(f"Unknown mode: {mode}")