Windows bash resolution: prefer Git Bash over WSL/shims

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #56384 fix(terminal): force Git for Windows bash + convert native CWD for bash cd

  merged to main

  files: tests/tools/test_local_env_windows_msys.py, tools/environments/base.py, tools/environments/local.py

  Show the diff

  • 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"]

  • tools/environments/base.py

    @@ -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"
             )

  • 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: