fix(file-tools): resolve local V4A patch paths before apply
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 <sswdarius@gmail.com>
This commit is contained in:
parent
8c172726c8
commit
fcd5e2cc61
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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}")
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue