mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(windows): native Windows correctness for CLI, gateway status, banner, and WSL browser paths
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.
This commit is contained in:
parent
382282d5a0
commit
53f7d137ed
13 changed files with 117 additions and 10 deletions
16
cli.py
16
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))
|
||||
|
|
|
|||
|
|
@ -507,9 +507,12 @@ def _command_line_belongs_to_profile(command: str, profile_home: Path) -> bool:
|
|||
explicit ``HERMES_HOME=<path>``) 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()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
|
|
@ -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)."""
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue