Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions tests/tools/test_local_env_windows_msys.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
LocalEnvironment,
_msys_to_windows_path,
_resolve_safe_cwd,
_windows_to_msys_path,
)


Expand Down Expand Up @@ -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
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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"]
19 changes: 12 additions & 7 deletions tools/environments/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""

Expand All @@ -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
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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 /
Expand Down Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
35 changes: 31 additions & 4 deletions tools/environments/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"),
Expand All @@ -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"
Expand Down Expand Up @@ -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:
Expand Down
Loading