Conversation
…bility The snapshot_path and cwd_file were left as Windows native paths (D:/hermes_data/...) when written into bash scripts that execute in WSL. WSL bash cannot access D:/... — it needs /mnt/d/... Fix: apply _windows_to_msys_path() to snapshot_path and cwd_file before quoting, matching the existing treatment of CWD in init_session().
On Windows, Hermes terminal runs inside WSL where drives are at
/mnt/c, /mnt/d, etc. _windows_to_msys_path was producing /c, /d
format which works in Git Bash but fails in WSL — causing exit 126
on every cd attempt.
Detect WSL via shutil.which('wsl.exe') and generate /mnt/{drive}/...
paths when running on a WSL-capable host.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment — Stale artifact file must be removed before merge.
The actual fix in tools/environments/base.py (35 additions, 7 deletions) is well-motivated and correctly detects WSL via shutil.which("wsl.exe") to generate /mnt/{drive}/... paths instead of Git Bash-style /{drive}/... paths.
Issue: The PR includes tools_environments_base.py (926 additions) at the repo root — this appears to be a stale copy/artifact of the entire tools/environments/base.py file. It should not be committed. Please remove it before merge.
Warnings
- tools_environments_base.py — 926-line stale file at repo root. Must be removed.
Looks Good
- WSL detection via
shutil.which("wsl.exe")is the right approach - Backward compatible (non-WSL paths unchanged)
builtin cd --with--prevents hyphen-prefixed dir names from being parsed as options
Reviewed by Hermes Agent
Icather
left a comment
There was a problem hiding this comment.
Thanks, removed the stale file — was accidentally included when syncing from the production deployment. Only tools/environments/base.py remains now.
|
Closing in favor of PR #56384. This PR's premise — that Hermes terminal on Windows "always runs inside WSL" — doesn't hold against |
What does this PR do?
Fixes
_windows_to_msys_path()to generate WSL-compatible/mnt/drive/...paths on Windows hosts. Currently it produces Git Bash-style/c/...paths which WSL cannot resolve — causing every terminal command to immediately fail with exit 126 on all Windows installations.Impact
The terminal is the primary code execution and system interaction surface for Hermes. With this bug, every command aborts before execution:
builtin cdalways fails against the WSL-invalid path →|| exit 126→ the actual command never runs. Users get zero usable terminal output. All Windows installations are affected because Hermes terminal on Windows always runs inside WSL.Two cascading failures:
source D:/...(unquoted Windows path) fails silently in WSL. After the quoting fix in the first commit, the path became/d/...— also unrecognised by WSL. Session snapshots never load, forcing fallback tobash -lper-command._windows_to_msys_path("D:\hermes_data\.hermes")→/d/hermes_data/.hermes. WSL mounts drives at/mnt/d/, not/d/. Every terminal execution hitsbuiltin cd /d/... || exit 126and aborts.Root Cause
_windows_to_msys_path()hardcodes the Git Bash/MSYS2 mount convention (/{drive}/...) which differs from WSL's mount convention (/mnt/{drive}/...). On Windows, Hermes' terminal tool always runs inside WSL, so the Git Bash convention is never correct.Fix
Detect WSL availability via
shutil.which("wsl.exe")and generate/mnt/{drive}/...when WSL is present. Fall back to/{drive}/...for non-WSL environments. Two files changed (the function and its mirror copy).Related Issue
Observable on every Windows Hermes installation since the path conversion was introduced.
Type of Change
Checklist
_windows_to_msys_path("D:\hermes_data\.hermes")now returns/mnt/d/hermes_data/.hermeson WSL hosts