From c7b0ad5d0087db7bdbe3e6be8869f913c10fd51d Mon Sep 17 00:00:00 2001 From: qbit-mirror-bot Date: Wed, 1 Jul 2026 16:23:43 +0000 Subject: [PATCH] fix(terminal): force Git for Windows bash + convert native CWD for bash cd --- tests/tools/test_local_env_windows_msys.py | 71 ++++++++++++++++++++++ tools/environments/base.py | 19 +++--- tools/environments/local.py | 35 +++++++++-- 3 files changed, 114 insertions(+), 11 deletions(-) diff --git a/tests/tools/test_local_env_windows_msys.py b/tests/tools/test_local_env_windows_msys.py index 529e8b2f2ae2..1b05aaaeaceb 100644 --- a/tests/tools/test_local_env_windows_msys.py +++ b/tests/tools/test_local_env_windows_msys.py @@ -26,6 +26,7 @@ LocalEnvironment, _msys_to_windows_path, _resolve_safe_cwd, + _windows_to_msys_path, ) @@ -70,6 +71,35 @@ def test_empty_string(self, monkeypatch): assert _msys_to_windows_path("") == "" +# --------------------------------------------------------------------------- +# _windows_to_msys_path — reverse translation for bash builtin cd +# --------------------------------------------------------------------------- + +class TestWindowsToMsysPath: + def test_noop_on_non_windows(self, monkeypatch): + monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) + assert _windows_to_msys_path(r"C:\Users\NVIDIA") == r"C:\Users\NVIDIA" + + def test_translates_backslash_path(self, monkeypatch): + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + assert _windows_to_msys_path(r"C:\Users\NVIDIA") == "/c/Users/NVIDIA" + assert _windows_to_msys_path(r"D:\Projects\foo bar") == "/d/Projects/foo bar" + + def test_translates_forward_slash_native_path(self, monkeypatch): + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + assert _windows_to_msys_path("C:/Users/NVIDIA") == "/c/Users/NVIDIA" + + def test_translates_drive_root(self, monkeypatch): + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + assert _windows_to_msys_path(r"C:\\") == "/c/" + assert _windows_to_msys_path("D:/") == "/d/" + + def test_does_not_translate_non_drive_path(self, monkeypatch): + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + assert _windows_to_msys_path("/tmp/foo") == "/tmp/foo" + assert _windows_to_msys_path(r"\\server\share") == r"\\server\share" + + # --------------------------------------------------------------------------- # _resolve_safe_cwd — Windows fast path # --------------------------------------------------------------------------- @@ -196,3 +226,44 @@ def test_valid_msys_marker_normalized_to_native(self, monkeypatch, tmp_path): env._extract_cwd_from_output(result) assert env.cwd == str(new_dir) + + +# --------------------------------------------------------------------------- +# Command wrapping — native Windows cwd must be Git Bash-friendly for cd +# --------------------------------------------------------------------------- + +class TestWrapCommandWindowsNativeCwd: + def test_wrap_command_converts_native_cwd_for_builtin_cd(self, monkeypatch): + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + + with patch.object( + LocalEnvironment, "init_session", autospec=True, return_value=None + ): + env = LocalEnvironment(cwd=r"C:\Users\liush", timeout=10) + + env._snapshot_ready = True + wrapped = env._wrap_command("pwd", r"C:\Users\liush") + + assert "builtin cd -- /c/Users/liush || exit 126" in wrapped + assert r"builtin cd -- C:\Users\liush || exit 126" not in wrapped + + def test_init_session_bootstrap_converts_native_cwd_for_cd(self, monkeypatch): + """The snapshot bootstrap ``cd`` must also use the Git-Bash path form, + not just ``_wrap_command`` — otherwise ``pwd -P`` captures the login + shell's directory instead of ``terminal.cwd`` on Windows.""" + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + + captured = {} + + def fake_run_bash(self, cmd_string, *, login=False, timeout=120, stdin_data=None): + captured["script"] = cmd_string + raise RuntimeError("stop after capturing bootstrap") + + monkeypatch.setattr(LocalEnvironment, "_run_bash", fake_run_bash) + + # init_session swallows the exception and falls back; we only need the + # captured bootstrap script to assert the cd target was converted. + LocalEnvironment(cwd=r"C:\Users\liush", timeout=10) + + assert "builtin cd -- /c/Users/liush 2>/dev/null || true" in captured["script"] + assert r"C:\Users\liush" not in captured["script"] diff --git a/tools/environments/base.py b/tools/environments/base.py index 191a30e2a0be..ef5b683aad2c 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -189,7 +189,7 @@ def _file_mtime_key(host_path: str) -> tuple[float, int] | None: class ProcessHandle(Protocol): """Duck type that every backend's _run_bash() must return. - subprocess.Popen satisfies this natively. SDK backends (Modal, Daytona, Tenki) + subprocess.Popen satisfies this natively. SDK backends (Modal, Daytona) return _ThreadedProcessHandle which adapts their blocking calls. """ @@ -205,7 +205,7 @@ def returncode(self) -> int | None: ... class _ThreadedProcessHandle: - """Adapter for SDK backends (Modal, Daytona, Tenki) that have no real subprocess. + """Adapter for SDK backends (Modal, Daytona) that have no real subprocess. Wraps a blocking ``exec_fn() -> (output_str, exit_code)`` in a background thread and exposes a ProcessHandle-compatible interface. An optional @@ -295,7 +295,7 @@ class BaseEnvironment(ABC): interrupt handling, and timeout enforcement. """ - # Subclasses that embed stdin as a heredoc (Modal, Daytona, Tenki) set this. + # Subclasses that embed stdin as a heredoc (Modal, Daytona) set this. _stdin_mode: str = "pipe" # "pipe" or "heredoc" # Snapshot creation timeout (override for slow cold-starts). @@ -361,7 +361,12 @@ def init_session(self): # Restore configured cwd after login shell profile scripts, which may # 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) + # Route through ``_quote_cwd_for_cd`` (not a bare ``shlex.quote``) so + # the Windows subclass override converts a native ``C:\Users\x`` cwd to + # the Git-Bash ``/c/Users/x`` form the bootstrap ``cd`` can resolve. + # Without this the snapshot bootstrap ``cd`` below fails on Windows and + # ``pwd -P`` captures the login shell's directory, not ``terminal.cwd``. + _quoted_cwd = self._quote_cwd_for_cd(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 / @@ -413,7 +418,7 @@ def init_session(self): # Publish atomically only if assembly succeeded; otherwise drop the # partial temp rather than leave it to be sourced or orphaned. f"mv -f {_snap_tmp} {_quoted_snap} || rm -f {_snap_tmp}\n" - f"builtin cd {_quoted_cwd} 2>/dev/null || true\n" + f"builtin cd -- {_quoted_cwd} 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" ) @@ -833,7 +838,7 @@ def _extract_cwd_from_output(self, result: dict): """Parse the __HERMES_CWD_{session}__ marker from stdout output. Updates self.cwd and strips the marker from result["output"]. - Used by remote backends (Docker, SSH, Modal, Daytona, Tenki, Singularity). + Used by remote backends (Docker, SSH, Modal, Daytona, Singularity). """ output = result.get("output", "") marker = self._cwd_marker @@ -870,7 +875,7 @@ def _extract_cwd_from_output(self, result: dict): def _before_execute(self) -> None: """Hook called before each command execution. - Remote backends (SSH, Modal, Daytona, Tenki) override this to trigger + Remote backends (SSH, Modal, Daytona) override this to trigger their FileSyncManager. Bind-mount backends (Docker, Singularity) and Local don't need file sync — the host filesystem is directly visible inside the container/process. diff --git a/tools/environments/local.py b/tools/environments/local.py index 49c87cd081ed..bccba38882c1 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -40,6 +40,24 @@ def _msys_to_windows_path(cwd: str) -> str: return f"{drive}:{tail or chr(92)}" # chr(92) = backslash, avoid raw-string escape +def _windows_to_msys_path(cwd: str) -> str: + """Translate a native Windows path (``C:\\Users\\x``) to Git Bash / + MSYS form (``/c/Users/x``) so ``builtin cd`` resolves it reliably. + + No-ops on non-Windows hosts or for paths that aren't drive-qualified + native Windows paths. Returns the input unchanged when no translation + applies. + """ + if not _IS_WINDOWS or not cwd: + return cwd + m = re.match(r'^([a-zA-Z]):[\\/]*(.*)$', cwd) + if not m: + return cwd + drive = m.group(1).lower() + tail = (m.group(2) or "").replace('\\', '/').lstrip('/') + return f"/{drive}/{tail}" if tail else f"/{drive}/" + + def _resolve_safe_cwd(cwd: str) -> str: """Return ``cwd`` if it exists as a directory, else the nearest existing ancestor. Falls back to ``tempfile.gettempdir()`` only if walking up the @@ -509,10 +527,10 @@ def _find_bash() -> str: if os.path.isfile(candidate): return candidate - found = shutil.which("bash") - if found: - return found - + # Check known Git for Windows install locations before PATH lookup. + # On machines with both WSL and Git for Windows, shutil.which("bash") + # may return WSL's bash (which doesn't understand Windows paths and + # will fail silently). Explicit Git-for-Windows paths avoid that. for candidate in ( os.path.join(os.environ.get("ProgramFiles", r"C:\Program Files"), "Git", "bin", "bash.exe"), os.path.join(os.environ.get("ProgramFiles(x86)", r"C:\Program Files (x86)"), "Git", "bin", "bash.exe"), @@ -521,6 +539,10 @@ def _find_bash() -> str: if candidate and os.path.isfile(candidate): return candidate + found = shutil.which("bash") + if found: + return found + raise RuntimeError( "Git Bash not found. Hermes Agent requires Git for Windows on Windows.\n" "Install it from: https://git-scm.com/download/win\n" @@ -917,6 +939,11 @@ def get_temp_dir(self) -> str: return "/tmp" + @staticmethod + def _quote_cwd_for_cd(cwd: str) -> str: + """Use native paths for Python, but Git Bash-friendly paths for cd.""" + return BaseEnvironment._quote_cwd_for_cd(_windows_to_msys_path(cwd)) + def _run_bash(self, cmd_string: str, *, login: bool = False, timeout: int = 120, stdin_data: str | None = None) -> subprocess.Popen: