mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-12 03:42:08 +00:00
fix(windows): quote cache paths in bash + augment PATH so rg/bash resolve on first launch
Three interrelated bugs from teknium1's first interactive chat on Windows: 1. **Snapshot/cwd file paths unquoted in bash command strings.** The session bootstrap and per-command wrapper interpolated ``self._snapshot_path`` / ``self._cwd_file`` unquoted into bash commands like ``export -p > C:/Users/ryanc/.../hermes-snap-xxx.sh``. Git Bash's MSYS2 layer handles ``C:/...`` paths correctly ONLY when quoted; unquoted, the colon and forward-slash get glob-parsed and the redirect targets a bogus path. Symptom: every terminal command emitted two ``C:/Users/.../hermes-snap-*.sh (No such file or directory)`` lines that bled into stdout (``stderr=STDOUT`` on the local backend) and corrupted file contents when the agent wrote to scratch paths via the terminal tool. Fix: ``shlex.quote()`` every interpolation of ``_snapshot_path`` and ``_cwd_file`` in base.py — no-op on POSIX (the paths contain no shell-metachars), critical on Windows. 2. **Stale PATH on first hermes launch after install.** ``install.ps1`` adds the PortableGit ``cmd`` / ``bin`` / ``usr\bin`` directories to the Windows **User** PATH via ``SetEnvironmentVariable(..., "User")``. That write propagates to newly *spawned* processes only — already-running shells (including the one the user types ``hermes`` into immediately after install) retain their old PATH. So hermes starts with a PATH that doesn't include bash, rg, grep, ssh — and ``search_files`` reports "rg/find not available" when the user clearly just installed them. Fix: new ``_augment_path_with_known_tools()`` helper called from ``configure_windows_stdio()`` on startup. Prepends the Hermes-managed Git directories + the WinGet Links directory (where ripgrep lands) to ``os.environ['PATH']`` if they exist on disk but aren't already in PATH. Subsequent subprocess calls (including bash spawns via ``_find_bash()``) inherit the augmented PATH and find everything. No-op on POSIX and when the directories don't exist. 3. **Root cause of "file content corruption".** #1 was the proximate cause. Errors like ``C:/Users/.../hermes-snap-xxx.sh: No such file or directory`` were emitted on stderr by the failed redirect, captured into stdout via ``stderr=subprocess.STDOUT``, and if the agent used terminal commands like ``cat > file`` the leaked error bytes became part of the file. Fixing #1 eliminates this entirely. ## Tests All 77 Windows-compat tests still pass on Linux (POSIX path is shlex.quote('/tmp/foo.sh') → '/tmp/foo.sh' — unchanged). ## Not addressed here (would need a bigger design) - Python file tools (``write_file``, ``read_file``) and the bash-backed terminal tool see DIFFERENT views of ``/tmp`` on Windows. Python treats ``/tmp`` as ``C:\tmp`` (drive-relative), Git Bash's MSYS2 treats it as a virtual mount to the PortableGit install's ``tmp\``. Would need a translation shim in the Python tools to resolve bash-virtual paths to their native-Windows equivalents. Workaround for users today: use absolute native paths (``C:\Users\you\...``) instead of ``/tmp/...`` when crossing between terminal and Python file tools.
This commit is contained in:
parent
cf9b2df57a
commit
787d964ea1
2 changed files with 98 additions and 10 deletions
|
|
@ -339,15 +339,24 @@ class BaseEnvironment(ABC):
|
|||
# change the working directory (e.g. bashrc `cd ~`). Without this,
|
||||
# pwd -P captures the profile's directory, not terminal.cwd.
|
||||
_quoted_cwd = shlex.quote(self.cwd)
|
||||
# Quote the snapshot / cwd-file paths so Git Bash on Windows handles
|
||||
# ``C:/Users/...``-shaped paths without glob-splitting the colon or
|
||||
# tripping on drive letters. On POSIX this is a no-op (no colons /
|
||||
# special chars in a /tmp path). Previously unquoted interpolation
|
||||
# caused ``C:/Users/.../hermes-snap-*.sh: No such file or directory``
|
||||
# errors on Windows, leaking via stderr (merged into stdout on Linux
|
||||
# backends) into every terminal-tool response.
|
||||
_quoted_snap = shlex.quote(self._snapshot_path)
|
||||
_quoted_cwd_file = shlex.quote(self._cwd_file)
|
||||
bootstrap = (
|
||||
f"export -p > {self._snapshot_path}\n"
|
||||
f"declare -f | grep -vE '^_[^_]' >> {self._snapshot_path}\n"
|
||||
f"alias -p >> {self._snapshot_path}\n"
|
||||
f"echo 'shopt -s expand_aliases' >> {self._snapshot_path}\n"
|
||||
f"echo 'set +e' >> {self._snapshot_path}\n"
|
||||
f"echo 'set +u' >> {self._snapshot_path}\n"
|
||||
f"export -p > {_quoted_snap}\n"
|
||||
f"declare -f | grep -vE '^_[^_]' >> {_quoted_snap}\n"
|
||||
f"alias -p >> {_quoted_snap}\n"
|
||||
f"echo 'shopt -s expand_aliases' >> {_quoted_snap}\n"
|
||||
f"echo 'set +e' >> {_quoted_snap}\n"
|
||||
f"echo 'set +u' >> {_quoted_snap}\n"
|
||||
f"builtin cd {_quoted_cwd} 2>/dev/null || true\n"
|
||||
f"pwd -P > {self._cwd_file} 2>/dev/null || true\n"
|
||||
f"pwd -P > {_quoted_cwd_file} 2>/dev/null || true\n"
|
||||
f"printf '\\n{self._cwd_marker}%s{self._cwd_marker}\\n' \"$(pwd -P)\"\n"
|
||||
)
|
||||
try:
|
||||
|
|
@ -389,6 +398,13 @@ class BaseEnvironment(ABC):
|
|||
re-dumps env vars, and emits CWD markers."""
|
||||
escaped = command.replace("'", "'\\''")
|
||||
|
||||
# Quote the snapshot / cwd-file paths so Git Bash on Windows handles
|
||||
# ``C:/Users/...``-shaped paths without glob-splitting the colon or
|
||||
# tripping on drive letters. POSIX paths are unaffected. See
|
||||
# :meth:`init_session` for the same fix on the bootstrap block.
|
||||
_quoted_snap = shlex.quote(self._snapshot_path)
|
||||
_quoted_cwd_file = shlex.quote(self._cwd_file)
|
||||
|
||||
parts = []
|
||||
|
||||
# Source snapshot (env vars from previous commands).
|
||||
|
|
@ -399,7 +415,7 @@ class BaseEnvironment(ABC):
|
|||
# silent here, but the redirect is harmless.
|
||||
if self._snapshot_ready:
|
||||
parts.append(
|
||||
f"source {self._snapshot_path} >/dev/null 2>&1 || true"
|
||||
f"source {_quoted_snap} >/dev/null 2>&1 || true"
|
||||
)
|
||||
|
||||
# Preserve bare ``~`` expansion, but rewrite ``~/...`` through
|
||||
|
|
@ -414,10 +430,10 @@ class BaseEnvironment(ABC):
|
|||
|
||||
# Re-dump env vars to snapshot (last-writer-wins for concurrent calls)
|
||||
if self._snapshot_ready:
|
||||
parts.append(f"export -p > {self._snapshot_path} 2>/dev/null || true")
|
||||
parts.append(f"export -p > {_quoted_snap} 2>/dev/null || true")
|
||||
|
||||
# Write CWD to file (local reads this) and stdout marker (remote parses this)
|
||||
parts.append(f"pwd -P > {self._cwd_file} 2>/dev/null || true")
|
||||
parts.append(f"pwd -P > {_quoted_cwd_file} 2>/dev/null || true")
|
||||
# Use a distinct line for the marker. The leading \n ensures
|
||||
# the marker starts on its own line even if the command doesn't
|
||||
# end with a newline (e.g. printf 'exact'). We'll strip this
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue