mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(windows): rewrite native drive paths to /c/ form for bash file ops
ShellFileOperations builds bash commands (wc/head/sed/cat/tee ...) with the target path as an argument. On a Windows/Git-Bash host a native `C:\...` path has its backslashes eaten by bash (and mangled by the msys runtime even when single-quoted) — the "Directory \drivers\etc does not exist; exiting — update your msys package" class of failures. Rewrite a native drive path to forward slashes in `_escape_shell_arg`, reusing the env layer's `_windows_to_msys_path`. Both `C:/...` and `/c/...` fix the backslash bug (the MSYS coreutils resolve either via the POSIX API). We emit `/c/...` purely for consistency: it's the same form `_windows_to_msys_path` already produces for the terminal `cd` (LocalEnvironment._quote_cwd_for_cd), so shell file ops and `cd` share one helper and one path form. Scoped from #55481, which also patched BaseEnvironment._quote_cwd_for_cd — but LocalEnvironment already overrides that through `_windows_to_msys_path`, so on a real Windows host the base branch never ran (the cwd is already `/c/...`). Co-authored-by: konsisumer <der@konsi.org>
This commit is contained in:
parent
fc232f8ce6
commit
d1ad9a0f5d
2 changed files with 50 additions and 1 deletions
|
|
@ -467,6 +467,44 @@ class TestShellFileOpsHelpers:
|
|||
# Should be safely escaped
|
||||
assert result.count("'") >= 4 # wrapping + escaping
|
||||
|
||||
def test_escape_shell_arg_rewrites_windows_drive_paths_to_msys(self, monkeypatch, file_ops):
|
||||
# bash eats backslashes and MSYS mangles ``C:\...``; the Git Bash
|
||||
# ``/c/...`` form is the reliable one (reuses _windows_to_msys_path).
|
||||
import tools.environments.local as local_mod
|
||||
|
||||
monkeypatch.setattr(local_mod, "_IS_WINDOWS", True)
|
||||
assert file_ops._escape_shell_arg(r"C:\Users\alice\notes.txt") == "'/c/Users/alice/notes.txt'"
|
||||
# Non-drive paths are untouched.
|
||||
assert file_ops._escape_shell_arg("/tmp/foo") == "'/tmp/foo'"
|
||||
|
||||
def test_read_file_uses_bash_safe_windows_paths(self, mock_env, monkeypatch):
|
||||
import tools.environments.local as local_mod
|
||||
|
||||
monkeypatch.setattr(local_mod, "_IS_WINDOWS", True)
|
||||
commands = []
|
||||
|
||||
def side_effect(command, **kwargs):
|
||||
commands.append(command)
|
||||
if command.startswith("wc -c"):
|
||||
return {"output": "5\n", "returncode": 0}
|
||||
if command.startswith("head -c"):
|
||||
return {"output": "hello", "returncode": 0}
|
||||
if command.startswith("sed -n"):
|
||||
return {"output": "hello\n", "returncode": 0}
|
||||
if command.startswith("wc -l"):
|
||||
return {"output": "1\n", "returncode": 0}
|
||||
return {"output": "", "returncode": 0}
|
||||
|
||||
mock_env.execute.side_effect = side_effect
|
||||
ops = ShellFileOperations(mock_env)
|
||||
result = ops.read_file(r"C:\Users\alice\notes.txt")
|
||||
|
||||
assert result.error is None
|
||||
assert commands[0] == "wc -c < '/c/Users/alice/notes.txt' 2>/dev/null"
|
||||
assert commands[1] == "head -c 1000 '/c/Users/alice/notes.txt' 2>/dev/null"
|
||||
assert commands[2] == "sed -n '1,500p' '/c/Users/alice/notes.txt'"
|
||||
assert commands[3] == "wc -l < '/c/Users/alice/notes.txt'"
|
||||
|
||||
def test_is_likely_binary_by_extension(self, file_ops):
|
||||
assert file_ops._is_likely_binary("photo.png") is True
|
||||
assert file_ops._is_likely_binary("data.db") is True
|
||||
|
|
|
|||
|
|
@ -958,7 +958,18 @@ class ShellFileOperations(FileOperations):
|
|||
return path
|
||||
|
||||
def _escape_shell_arg(self, arg: str) -> str:
|
||||
"""Escape a string for safe use in shell commands."""
|
||||
"""Escape a string for safe use in shell commands.
|
||||
|
||||
On Windows a native drive path (``C:\\Users\\x``) is first rewritten to
|
||||
the Git Bash / MSYS ``/c/Users/x`` form: bash eats the backslashes and
|
||||
MSYS otherwise mangles ``C:\\...`` (the "Directory \\drivers\\etc does not
|
||||
exist" class of failures). Reuses the env-layer translator so shell file
|
||||
ops and the terminal ``cd`` agree on the path form. No-op off Windows and
|
||||
for non-drive-qualified paths.
|
||||
"""
|
||||
from tools.environments.local import _windows_to_msys_path
|
||||
|
||||
arg = _windows_to_msys_path(arg)
|
||||
# Use single quotes and escape any single quotes in the string
|
||||
return "'" + arg.replace("'", "'\"'\"'") + "'"
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue