From 53f7d137ed8fc39257a10db247e7acd4cc3445fe Mon Sep 17 00:00:00 2001 From: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:42:36 -0700 Subject: [PATCH] fix(windows): native Windows correctness for CLI, gateway status, banner, and WSL browser paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Salvaged from #57016 by @lEWFkRAD: - cli.py: handle file:///C:/... drive-letter URIs on nt (strip the leading slash urlparse leaves); join Termux example paths with literal forward slashes so hints stay POSIX on Windows. - gateway/status.py + hermes_cli/gateway.py: normalize backslashes to forward slashes before the HERMES_HOME substring match so separator style cannot defeat profile ownership detection. - hermes_cli/banner.py: cprint degrades to plain print when prompt_toolkit has no console (NoConsoleScreenBufferError on redirected/absent Windows stdout). - hermes_cli/browser_connect.py: posixpath.join for WSL /mnt/c/... bases (os.path.join would emit backslashes on nt). - Test hardening: symlink skip-guards, USERPROFILE alongside HOME for ntpath.expanduser, SIGKILL absence skipif fixed via monkeypatch, drive-letter URI / separator-normalization / banner-fallback coverage. Dropped from the original PR: tests/cli/conftest.py fixture and the AppSession _output monkeypatch — main's merged tests/cli/conftest.py already handles that prompt_toolkit pollution. --- cli.py | 16 ++++++++++++++-- gateway/status.py | 7 +++++-- hermes_cli/banner.py | 9 ++++++++- hermes_cli/browser_connect.py | 6 +++++- hermes_cli/gateway.py | 6 ++++-- tests/cli/test_cli_browser_connect.py | 11 +++++++++++ tests/cli/test_cli_file_drop.py | 16 ++++++++++++++++ tests/cli/test_cli_image_command.py | 2 ++ tests/cli/test_worktree.py | 15 +++++++++++++++ tests/cli/test_worktree_security.py | 16 ++++++++++++++++ tests/gateway/test_status.py | 9 +++++++++ tests/hermes_cli/test_banner.py | 10 ++++++++++ tests/tools/test_windows_native_support.py | 4 ++-- 13 files changed, 117 insertions(+), 10 deletions(-) diff --git a/cli.py b/cli.py index bb91aa4fc37..a4ce0cdd56a 100644 --- a/cli.py +++ b/cli.py @@ -3235,10 +3235,12 @@ def _termux_example_image_path(filename: str = "cat.png") -> str: "/storage/emulated/0", "/storage/self/primary", ] + # Termux/Android roots are POSIX paths — join with literal forward + # slashes so the hint stays correct even when this renders on Windows. for root in candidates: if os.path.isdir(root): - return os.path.join(root, "Pictures", filename) - return os.path.join("~/storage/shared", "Pictures", filename) + return f"{root}/Pictures/{filename}" + return f"~/storage/shared/Pictures/{filename}" def _split_path_input(raw: str) -> tuple[str, str]: @@ -3309,6 +3311,16 @@ def _resolve_attachment_path(raw_path: str) -> Path | None: expanded = unquote(parsed.path or "") if parsed.netloc and os.name == "nt": expanded = f"//{parsed.netloc}{expanded}" + elif ( + os.name == "nt" + and len(expanded) >= 3 + and expanded[0] == "/" + and expanded[1].isalpha() + and expanded[2] == ":" + ): + # file:///C:/... parses to path "/C:/..." — drop the + # leading slash so it resolves as a drive-letter path. + expanded = expanded[1:] except Exception: expanded = token expanded = os.path.expandvars(os.path.expanduser(expanded)) diff --git a/gateway/status.py b/gateway/status.py index 5787a43821d..ce02648a958 100644 --- a/gateway/status.py +++ b/gateway/status.py @@ -507,9 +507,12 @@ def _command_line_belongs_to_profile(command: str, profile_home: Path) -> bool: explicit ``HERMES_HOME=``) on its argv; the default/root gateway runs bare with no profile flag. """ - command_lc = command.lower() + # Normalize separators before the substring match: on Windows, + # str(Path) renders backslashes while a HERMES_HOME= value on the argv + # may carry forward slashes (Git Bash, JSON configs) — and vice versa. + command_lc = command.lower().replace("\\", "/") profile_name = _profile_name_for_home(profile_home) - home_lc = str(profile_home).lower() + home_lc = str(profile_home).lower().replace("\\", "/") if profile_name is not None and profile_name != "default": profile_lc = profile_name.lower() diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index 4d2b077ec96..7811899aeb3 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -40,7 +40,14 @@ def cprint(text: str): """Print ANSI-colored text through prompt_toolkit's renderer.""" from prompt_toolkit import print_formatted_text as _pt_print from prompt_toolkit.formatted_text import ANSI as _PT_ANSI - _pt_print(_PT_ANSI(text)) + try: + _pt_print(_PT_ANSI(text)) + except Exception: + # prompt_toolkit needs a real console. On Windows, a redirected or + # absent stdout (pythonw.exe, CI, `hermes ... > file`) raises + # NoConsoleScreenBufferError from its Win32Output — display helpers + # must never crash the caller over that, so degrade to plain print. + print(text) # ========================================================================= diff --git a/hermes_cli/browser_connect.py b/hermes_cli/browser_connect.py index 4fcc4cc63c5..af1b04eaee0 100644 --- a/hermes_cli/browser_connect.py +++ b/hermes_cli/browser_connect.py @@ -5,6 +5,7 @@ from __future__ import annotations import logging import os import platform +import posixpath import shlex import shutil import subprocess @@ -95,7 +96,10 @@ def get_chrome_debug_candidates(system: str) -> list[str]: for _, group in install_groups: for base in filter(None, bases): for parts in group: - add(os.path.join(base, *parts)) + # Only called with WSL ``/mnt/c/...`` bases — those are + # POSIX paths regardless of the host OS, so join with + # posixpath (os.path.join would emit backslashes on nt). + add(posixpath.join(base, *parts)) if system == "Darwin": for app in _DARWIN_APPS: diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 067b2c3209b..55b8a196f14 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -359,7 +359,9 @@ def _scan_gateway_pids( looks_like_gateway_runtime_command_line, ) current_home = str(get_hermes_home().resolve()) - current_home_lc = current_home.lower() + # Forward slashes on both sides of the HERMES_HOME= match — see + # gateway.status._command_line_belongs_to_profile, which this mirrors. + current_home_lc = current_home.lower().replace("\\", "/") current_profile_arg = _profile_arg(current_home) current_profile_name = ( current_profile_arg.split()[-1] if current_profile_arg else "" @@ -367,7 +369,7 @@ def _scan_gateway_pids( current_profile_name_lc = current_profile_name.lower() def _matches_current_profile(command: str) -> bool: - command_lc = command.lower() + command_lc = command.lower().replace("\\", "/") if current_profile_name: return ( f"--profile {current_profile_name_lc}" in command_lc diff --git a/tests/cli/test_cli_browser_connect.py b/tests/cli/test_cli_browser_connect.py index 2f17b0595a4..4bdc56cbcf5 100644 --- a/tests/cli/test_cli_browser_connect.py +++ b/tests/cli/test_cli_browser_connect.py @@ -83,6 +83,17 @@ class TestChromeDebugLaunch: assert candidates == [brave, edge] + def test_wsl_install_candidates_keep_posix_separators_on_nt_host(self): + expected = "/mnt/c/Program Files/Google/Chrome/Application/chrome.exe" + + with patch("hermes_cli.browser_connect.shutil.which", return_value=None), \ + patch("hermes_cli.browser_connect.os.path.isfile", side_effect=lambda path: path == expected): + candidates = get_chrome_debug_candidates("Linux") + + assert candidates == [expected] + assert "\\" not in candidates[0] + + def test_wait_for_browser_debug_ready_or_exit_detects_early_exit(self, monkeypatch): class _Proc: def __init__(self): diff --git a/tests/cli/test_cli_file_drop.py b/tests/cli/test_cli_file_drop.py index b6093e21560..00c0fd5d5f8 100644 --- a/tests/cli/test_cli_file_drop.py +++ b/tests/cli/test_cli_file_drop.py @@ -1,6 +1,7 @@ """Tests for _detect_file_drop — file path detection that prevents dragged/pasted absolute paths from being mistaken for slash commands.""" +import os import pytest @@ -157,6 +158,8 @@ class TestEscapedSpaces: img.parent.mkdir(parents=True, exist_ok=True) img.write_bytes(b"\x89PNG\r\n\x1a\n") monkeypatch.setenv("HOME", str(home)) + # ntpath.expanduser ignores HOME (Python 3.8+) — it wants USERPROFILE. + monkeypatch.setenv("USERPROFILE", str(home)) result = _detect_file_drop("~/storage/shared/Pictures/cat.png what is this?") @@ -166,6 +169,19 @@ class TestEscapedSpaces: assert result["remainder"] == "what is this?" + @pytest.mark.skipif(os.name != "nt", reason="Windows drive-letter URI contract") + def test_windows_drive_letter_file_uri_drops_url_leading_slash(self, tmp_path): + image = tmp_path / "drive-uri.png" + image.write_bytes(b"\x89PNG\r\n\x1a\n") + uri = image.as_uri() + assert uri.startswith("file:///") and ":/" in uri + + result = _detect_file_drop(uri) + + assert result is not None + assert result["path"] == image + + # --------------------------------------------------------------------------- # Tests: edge cases # --------------------------------------------------------------------------- diff --git a/tests/cli/test_cli_image_command.py b/tests/cli/test_cli_image_command.py index 0af4635dfa9..573efbe77e9 100644 --- a/tests/cli/test_cli_image_command.py +++ b/tests/cli/test_cli_image_command.py @@ -59,6 +59,8 @@ class TestCollectQueryImages: home = tmp_path / "home" img = _make_image(home / "storage" / "shared" / "Pictures" / "cat.png") monkeypatch.setenv("HOME", str(home)) + # ntpath.expanduser ignores HOME (Python 3.8+) — it wants USERPROFILE. + monkeypatch.setenv("USERPROFILE", str(home)) message, images = _collect_query_images("describe this", "~/storage/shared/Pictures/cat.png") diff --git a/tests/cli/test_worktree.py b/tests/cli/test_worktree.py index 6ca7c4514cc..626ca29bd4a 100644 --- a/tests/cli/test_worktree.py +++ b/tests/cli/test_worktree.py @@ -416,6 +416,21 @@ class TestMultipleWorktrees: assert not Path(info["path"]).exists() +def _can_symlink(): + """Check if we can create symlinks (needs admin/dev-mode on Windows).""" + import tempfile + try: + with tempfile.TemporaryDirectory() as d: + src = Path(d) / "src" + src.write_text("x") + lnk = Path(d) / "lnk" + lnk.symlink_to(src) + return True + except OSError: + return False + + +@pytest.mark.skipif(not _can_symlink(), reason="Symlinks need elevated privileges") class TestWorktreeDirectorySymlink: """Test .worktreeinclude with directories (symlinked).""" diff --git a/tests/cli/test_worktree_security.py b/tests/cli/test_worktree_security.py index bd5aae81cd2..c8c2b89f20a 100644 --- a/tests/cli/test_worktree_security.py +++ b/tests/cli/test_worktree_security.py @@ -6,6 +6,20 @@ from pathlib import Path import pytest +def _can_symlink(): + """Check if we can create symlinks (needs admin/dev-mode on Windows).""" + import tempfile + try: + with tempfile.TemporaryDirectory() as d: + src = Path(d) / "src" + src.write_text("x") + lnk = Path(d) / "lnk" + lnk.symlink_to(src) + return True + except OSError: + return False + + @pytest.fixture def git_repo(tmp_path): """Create a temporary git repo for testing real cli._setup_worktree behavior.""" @@ -76,6 +90,7 @@ class TestWorktreeIncludeSecurity: finally: _force_remove_worktree(info) + @pytest.mark.skipif(not _can_symlink(), reason="Symlinks need elevated privileges") def test_rejects_symlink_that_resolves_outside_repo(self, git_repo): import cli as cli_mod @@ -110,6 +125,7 @@ class TestWorktreeIncludeSecurity: finally: _force_remove_worktree(info) + @pytest.mark.skipif(not _can_symlink(), reason="Symlinks need elevated privileges") def test_allows_valid_directory_include(self, git_repo): import cli as cli_mod diff --git a/tests/gateway/test_status.py b/tests/gateway/test_status.py index 64582219c42..237331782bb 100644 --- a/tests/gateway/test_status.py +++ b/tests/gateway/test_status.py @@ -248,6 +248,15 @@ class TestGatewayRuntimeStatus: ), cmdline + def test_command_line_belongs_to_profile_normalizes_separators(self): + """A Windows argv renders HERMES_HOME with backslashes while the + profile's Path may carry forward slashes (and, on Windows, vice + versa). The separator difference must not defeat the match.""" + home = Path("c:/opt/data/profiles/coder") + cmdline = r"hermes_home=c:\opt\data\profiles\coder hermes gateway run --replace" + assert status._command_line_belongs_to_profile(cmdline, home) is True + + def test_write_runtime_status_explicit_none_clears_stale_fields(self, tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) diff --git a/tests/hermes_cli/test_banner.py b/tests/hermes_cli/test_banner.py index 9493d40de3d..e1dbed11952 100644 --- a/tests/hermes_cli/test_banner.py +++ b/tests/hermes_cli/test_banner.py @@ -9,6 +9,16 @@ import model_tools import tools.mcp_tool +def test_cprint_falls_back_to_plain_print_when_prompt_toolkit_has_no_console(capsys): + with patch( + "prompt_toolkit.print_formatted_text", + side_effect=RuntimeError("no console screen buffer"), + ): + banner.cprint("fallback text") + + assert capsys.readouterr().out == "fallback text\n" + + diff --git a/tests/tools/test_windows_native_support.py b/tests/tools/test_windows_native_support.py index d5e1f9357e6..81be4319b55 100644 --- a/tests/tools/test_windows_native_support.py +++ b/tests/tools/test_windows_native_support.py @@ -52,10 +52,10 @@ class TestConfigureWindowsStdio: yield sys.modules.pop("hermes_cli.stdio", None) - def test_no_op_on_posix(self): + def test_no_op_on_posix(self, monkeypatch): from hermes_cli import stdio - assert stdio.is_windows() is False + monkeypatch.setattr(stdio, "is_windows", lambda: False) result = stdio.configure_windows_stdio() assert result is False