hermes-agent/tests/tools/test_windows_native_support.py
Teknium 9de893e3b0 feat(windows): close native-Windows install gaps — crash-free startup, UTF-8 stdio, tzdata dep, docs
Native Windows (with Git for Windows installed) can now run the Hermes CLI
and gateway end-to-end without crashing.  install.ps1 already existed and
the Git Bash terminal backend was already wired up — this PR fills the
remaining gaps discovered by auditing every Windows-unsafe primitive
(`signal.SIGKILL`, `os.kill(pid, 0)` probes, bare `fcntl`/`termios`
imports) and by comparing hermes against how Claude Code, OpenCode, Codex,
and Cline handle native Windows.

## What changed

### UTF-8 stdio (new module)
- `hermes_cli/stdio.py` — single `configure_windows_stdio()` entry point.
  Flips the console code page to CP_UTF8 (65001), reconfigures
  `sys.stdout`/`stderr`/`stdin` to UTF-8, sets `PYTHONIOENCODING` + `PYTHONUTF8`
  for subprocesses.  No-op on non-Windows.  Opt out via `HERMES_DISABLE_WINDOWS_UTF8=1`.
- Called early in `cli.py::main`, `hermes_cli/main.py::main`, and
  `gateway/run.py::main` so Unicode banners (box-drawing, geometric
  symbols, non-Latin chat text) don't `UnicodeEncodeError` on cp1252
  consoles.

### Crash sites fixed
- `hermes_cli/main.py:7970` (hermes update → stuck gateway sweep): raw
  `os.kill(pid, _signal.SIGKILL)` → `gateway.status.terminate_pid(pid, force=True)`
  which routes through `taskkill /T /F` on Windows.
- `hermes_cli/profiles.py::_stop_gateway_process`: same fix — also
  converted SIGTERM path to `terminate_pid()` and widened OSError catch
  on the intermediate `os.kill(pid, 0)` probe.
- `hermes_cli/kanban_db.py:2914, 3041`: raw `signal.SIGKILL` →
  `getattr(signal, "SIGKILL", signal.SIGTERM)` fallback (matches the
  pattern already used in `gateway/status.py`).

### OSError widening on `os.kill(pid, 0)` probes
Windows raises `OSError` (WinError 87) for a gone PID instead of
`ProcessLookupError`.  Widened the catch at:
- `gateway/run.py:15101` (`--replace` wait-for-exit loop — without this,
  the loop busy-spins the full 10s every Windows gateway start)
- `hermes_cli/gateway.py:228, 460, 940`
- `hermes_cli/profiles.py:777`
- `tools/process_registry.py::_is_host_pid_alive`
- `tools/browser_tool.py:1170, 1206`

### Dashboard PTY graceful degradation
`hermes_cli/pty_bridge.py` depends on `fcntl`/`termios`/`ptyprocess`,
none of which exist on native Windows.  Previously a Windows dashboard
would crash on `import hermes_cli.web_server` because of a top-level
import.  Now:
- `hermes_cli/web_server.py` wraps the pty_bridge import in
  `try/except ImportError` and sets `_PTY_BRIDGE_AVAILABLE=False`.
- The `/api/pty` WebSocket handler returns a friendly "use WSL2 for
  this tab" message instead of exploding.
- Every other dashboard feature (sessions, jobs, metrics, config
  editor) runs natively on Windows.

### Dependency
- `pyproject.toml`: add `tzdata>=2023.3; sys_platform == 'win32'` so
  Python's `zoneinfo` works on Windows (which has no IANA tzdata
  shipped with the OS).  Credits @sprmn24 (PR #13182).

### Docs
- README.md: removed "Native Windows is not supported"; added
  PowerShell one-liner and Git-for-Windows prerequisite note.
- `website/docs/getting-started/installation.md`: new Windows section
  with capability matrix (everything native except the dashboard
  `/chat` PTY tab, which is WSL2-only).
- `website/docs/user-guide/windows-wsl-quickstart.md`: reframed as
  "WSL2 as an alternative to native" rather than "the only way".
- `website/docs/developer-guide/contributing.md`: updated
  cross-platform guidance with the `signal.SIGKILL` / `OSError`
  rules we enforce now.
- `website/docs/user-guide/features/web-dashboard.md`: acknowledged
  native Windows works for everything except the embedded PTY pane.

## Why this shape

Pulled from a survey of how other agent codebases handle native
Windows (Claude Code, OpenCode, Codex, Cline):

- All four treat Git Bash as the canonical shell on Windows, same as
  hermes already does in `tools/environments/local.py::_find_bash()`.
- None of them force `SetConsoleOutputCP` — but they don't have to,
  Node/Rust write UTF-16 to the Win32 console API.  Python does not get
  that for free, so we flip CP_UTF8 via ctypes.
- None of them ship PowerShell-as-primary-shell (Claude Code exposes
  PS as a secondary tool; scope creep for this PR).
- All of them use `taskkill /T /F` for force-kill on Windows, which
  is exactly what `gateway.status.terminate_pid(force=True)` does.

## Non-goals (deliberate scope limits)

- No PowerShell-as-a-second-shell tool — worth designing separately.
- No terminal routing rewrite (#12317, #15461, #19800 cluster) — that's
  the hardest design call and needs a separate doc.
- No wholesale `open()` → `open(..., encoding="utf-8")` sweep (Tianworld
  cluster) — will do as follow-up if users hit actual breakage; most
  modern code already specifies it.

## Validation

- 28 new tests in `tests/tools/test_windows_native_support.py` — all
  platform-mocked, pass on Linux CI.  Cover:
  - `configure_windows_stdio` idempotency, opt-out, env-preservation
  - `terminate_pid` taskkill routing, failure → OSError, FileNotFoundError fallback
  - `getattr(signal, "SIGKILL", …)` fallback shape
  - `_is_host_pid_alive` OSError widening (Windows-gone-PID behavior)
  - Source-level checks that all entry points call `configure_windows_stdio`
  - pty_bridge import-guard present in `web_server.py`
  - README no longer says "not supported"
- 12 pre-existing tests in `tests/tools/test_windows_compat.py` still pass.
- `tests/hermes_cli/` ran fully (3909 passed, 9 failures — all confirmed
  pre-existing on main by stash-test).
- `tests/gateway/` ran fully (5021 passed, 1 pre-existing failure).
- `tests/tools/test_process_registry.py` + `test_browser_*` pass.
- Manual smoke: `import hermes_cli.stdio; import gateway.run;
  import hermes_cli.web_server` — all clean, `_PTY_BRIDGE_AVAILABLE=True`
  on Linux (as expected).

## Files

- New: `hermes_cli/stdio.py`, `tests/tools/test_windows_native_support.py`
- Modified: `cli.py`, `gateway/run.py`, `hermes_cli/main.py`,
  `hermes_cli/profiles.py`, `hermes_cli/gateway.py`,
  `hermes_cli/kanban_db.py`, `hermes_cli/pty_bridge.py`,
  `hermes_cli/web_server.py`, `tools/browser_tool.py`,
  `tools/process_registry.py`, `pyproject.toml`, `README.md`, and 4
  docs pages.

Credits to everyone whose prior PR work informed these fixes — see
the co-author trailers.  All of the PRs listed in
`~/.hermes/plans/windows-support-prs.md` fixing `os.kill` / `signal.SIGKILL`
/ UTF-8 stdio / tzdata / README patterns found the same issues; this PR
consolidates them.

Co-authored-by: Philip D'Souza <9472774+PhilipAD@users.noreply.github.com>
Co-authored-by: Arecanon <42595053+ArecaNon@users.noreply.github.com>
Co-authored-by: XiaoXiao0221 <263113677+XiaoXiao0221@users.noreply.github.com>
Co-authored-by: Lars Hagen <1360677+lars-hagen@users.noreply.github.com>
Co-authored-by: Luan Dias <65574834+luandiasrj@users.noreply.github.com>
Co-authored-by: Ruzzgar <ruzzgarcn@gmail.com>
Co-authored-by: sprmn24 <oncuevtv@gmail.com>
Co-authored-by: adybag14-cyber <252811164+adybag14-cyber@users.noreply.github.com>
Co-authored-by: Prasanna28Devadiga <54196612+Prasanna28Devadiga@users.noreply.github.com>
2026-05-08 14:27:40 -07:00

408 lines
16 KiB
Python

"""Behavioral tests for Windows-specific compatibility fixes.
Complements ``tests/tools/test_windows_compat.py`` (which does source-level
pattern linting) with cross-platform-mocked tests that exercise the actual
code paths Hermes takes on native Windows.
Runs on Linux CI — every test mocks ``sys.platform``, ``subprocess.run``,
and ``os.kill`` as needed to simulate Windows behavior without requiring a
Windows runner.
"""
from __future__ import annotations
import importlib
import os
import signal
import subprocess
import sys
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
# ---------------------------------------------------------------------------
# configure_windows_stdio
# ---------------------------------------------------------------------------
class TestConfigureWindowsStdio:
"""``hermes_cli.stdio.configure_windows_stdio`` wiring.
The function must:
- be a no-op on non-Windows
- only configure once per process (idempotent)
- set PYTHONIOENCODING / PYTHONUTF8 without overriding explicit user settings
- reconfigure sys.stdout/stderr/stdin to UTF-8 on Windows
- flip the console code page to CP_UTF8 (65001) via ctypes
- respect HERMES_DISABLE_WINDOWS_UTF8 opt-out
"""
@pytest.fixture(autouse=True)
def _reset_configured(self, monkeypatch):
"""Reload the module before each test so the _CONFIGURED flag resets."""
# Remove from sys.modules so import triggers a fresh load
sys.modules.pop("hermes_cli.stdio", None)
# Fresh import now; tests import from hermes_cli.stdio themselves,
# but this guarantees the module they get is a brand-new copy.
import hermes_cli.stdio as _s
_s._CONFIGURED = False
yield
sys.modules.pop("hermes_cli.stdio", None)
def test_no_op_on_posix(self):
from hermes_cli import stdio
assert stdio.is_windows() is False
result = stdio.configure_windows_stdio()
assert result is False
def test_idempotent(self):
from hermes_cli import stdio
stdio.configure_windows_stdio()
# Second call returns False because _CONFIGURED is set
assert stdio.configure_windows_stdio() is False
def test_windows_path_sets_env_and_reconfigures_streams(self, monkeypatch):
from hermes_cli import stdio
monkeypatch.setattr(stdio, "is_windows", lambda: True)
# Pretend the user has no prior setting
monkeypatch.delenv("PYTHONIOENCODING", raising=False)
monkeypatch.delenv("PYTHONUTF8", raising=False)
monkeypatch.delenv("HERMES_DISABLE_WINDOWS_UTF8", raising=False)
reconfigure_calls = []
def fake_reconfigure(stream, *, encoding="utf-8", errors="replace"):
reconfigure_calls.append((stream, encoding, errors))
cp_calls = []
def fake_flip():
cp_calls.append(True)
monkeypatch.setattr(stdio, "_reconfigure_stream", fake_reconfigure)
monkeypatch.setattr(stdio, "_flip_console_code_page_to_utf8", fake_flip)
result = stdio.configure_windows_stdio()
assert result is True
assert os.environ.get("PYTHONIOENCODING") == "utf-8"
assert os.environ.get("PYTHONUTF8") == "1"
assert len(cp_calls) == 1 # SetConsoleOutputCP path hit
assert len(reconfigure_calls) == 3 # stdout, stderr, stdin
def test_respects_existing_env_var(self, monkeypatch):
"""User's explicit PYTHONIOENCODING wins over our default."""
from hermes_cli import stdio
monkeypatch.setattr(stdio, "is_windows", lambda: True)
monkeypatch.setenv("PYTHONIOENCODING", "latin-1")
monkeypatch.setattr(stdio, "_reconfigure_stream", lambda *a, **kw: None)
monkeypatch.setattr(stdio, "_flip_console_code_page_to_utf8", lambda: None)
stdio.configure_windows_stdio()
assert os.environ["PYTHONIOENCODING"] == "latin-1"
@pytest.mark.parametrize("optout", ["1", "true", "True", "yes"])
def test_disable_flag_short_circuits(self, monkeypatch, optout):
from hermes_cli import stdio
monkeypatch.setattr(stdio, "is_windows", lambda: True)
monkeypatch.setenv("HERMES_DISABLE_WINDOWS_UTF8", optout)
reconfigure_hit = []
monkeypatch.setattr(
stdio,
"_reconfigure_stream",
lambda *a, **kw: reconfigure_hit.append(True),
)
result = stdio.configure_windows_stdio()
assert result is False
assert reconfigure_hit == [], "opt-out must skip stream reconfiguration"
def test_reconfigure_stream_handles_missing_method(self, monkeypatch):
"""StringIO-like objects without .reconfigure() must not blow up."""
from hermes_cli import stdio
import io
buf = io.StringIO()
# Must not raise
stdio._reconfigure_stream(buf)
# ---------------------------------------------------------------------------
# terminate_pid — the centralized kill primitive
# ---------------------------------------------------------------------------
class TestTerminatePidRoutingOnWindows:
"""``gateway.status.terminate_pid`` must use taskkill /T /F on Windows.
On Linux we can't reload gateway/status with sys.platform=win32 because
the module unconditionally imports ``msvcrt`` in that branch. Instead
we patch the module-level ``_IS_WINDOWS`` flag and ``subprocess.run``
on the already-loaded module, which exercises the same branching code.
"""
def test_force_uses_taskkill_on_windows(self, monkeypatch):
from gateway import status
captured = {}
def fake_run(args, **kwargs):
captured["args"] = args
result = MagicMock()
result.returncode = 0
result.stderr = ""
result.stdout = ""
return result
monkeypatch.setattr(status, "_IS_WINDOWS", True)
monkeypatch.setattr(status.subprocess, "run", fake_run)
status.terminate_pid(12345, force=True)
assert captured["args"][0] == "taskkill"
assert "/PID" in captured["args"]
assert "12345" in captured["args"]
assert "/T" in captured["args"]
assert "/F" in captured["args"]
def test_force_taskkill_failure_raises_oserror(self, monkeypatch):
from gateway import status
def fake_run(args, **kwargs):
result = MagicMock()
result.returncode = 128
result.stderr = "ERROR: The process cannot be terminated."
result.stdout = ""
return result
monkeypatch.setattr(status, "_IS_WINDOWS", True)
monkeypatch.setattr(status.subprocess, "run", fake_run)
with pytest.raises(OSError, match="cannot be terminated"):
status.terminate_pid(12345, force=True)
def test_graceful_on_windows_uses_os_kill_sigterm(self, monkeypatch):
"""Non-force path calls os.kill with SIGTERM (Windows has no SIGKILL).
``terminate_pid(pid)`` with force=False bypasses the taskkill branch
and uses ``os.kill`` directly — so platform doesn't actually matter
for the signal choice. Verifies the getattr fallback works.
"""
from gateway import status
captured = {}
def fake_kill(pid, sig):
captured["pid"] = pid
captured["sig"] = sig
monkeypatch.setattr(status.os, "kill", fake_kill)
status.terminate_pid(99, force=False)
assert captured["pid"] == 99
assert captured["sig"] == signal.SIGTERM
def test_taskkill_not_found_falls_back_to_os_kill(self, monkeypatch):
"""On Windows without taskkill (WinPE, containers), fall back gracefully."""
from gateway import status
captured = {}
def fake_run(args, **kwargs):
raise FileNotFoundError(2, "taskkill not found")
def fake_kill(pid, sig):
captured["pid"] = pid
captured["sig"] = sig
monkeypatch.setattr(status, "_IS_WINDOWS", True)
monkeypatch.setattr(status.subprocess, "run", fake_run)
monkeypatch.setattr(status.os, "kill", fake_kill)
status.terminate_pid(42, force=True)
assert captured["pid"] == 42
assert captured["sig"] == signal.SIGTERM
# ---------------------------------------------------------------------------
# SIGKILL fallback pattern
# ---------------------------------------------------------------------------
class TestSigkillFallback:
"""Modules that want SIGKILL must fall back to SIGTERM when absent."""
def test_getattr_fallback_works_when_sigkill_missing(self, monkeypatch):
"""The `getattr(signal, "SIGKILL", signal.SIGTERM)` pattern."""
# Build a stand-in signal module with no SIGKILL attribute
fake_signal = MagicMock()
del fake_signal.SIGKILL # ensure it's absent
fake_signal.SIGTERM = 15
result = getattr(fake_signal, "SIGKILL", fake_signal.SIGTERM)
assert result == 15
def test_getattr_fallback_prefers_sigkill_when_present(self):
"""On POSIX the fallback is a no-op: real SIGKILL wins."""
result = getattr(signal, "SIGKILL", signal.SIGTERM)
assert result == signal.SIGKILL
@pytest.mark.parametrize(
"module_path, line_pattern",
[
("hermes_cli.kanban_db", 'getattr(signal, "SIGKILL", signal.SIGTERM)'),
],
)
def test_module_uses_getattr_fallback(self, module_path, line_pattern):
"""Source-level check that our modules use the safe fallback."""
rel = module_path.replace(".", "/") + ".py"
root = Path(__file__).resolve().parents[2]
source = (root / rel).read_text(encoding="utf-8")
assert line_pattern in source, (
f"{rel} must use the getattr fallback pattern on its SIGKILL site"
)
# ---------------------------------------------------------------------------
# OSError widening on os.kill(pid, 0) probes
# ---------------------------------------------------------------------------
class TestProcessRegistryOSErrorWidening:
"""_is_host_pid_alive must treat Windows' OSError as 'not alive'."""
def test_oserror_treated_as_not_alive(self, monkeypatch):
from tools.process_registry import ProcessRegistry
def fake_kill(pid, sig):
# Simulate Windows' WinError 87 for an unknown PID
raise OSError(22, "Invalid argument")
monkeypatch.setattr("tools.process_registry.os.kill", fake_kill)
assert ProcessRegistry._is_host_pid_alive(12345) is False
def test_permission_error_treated_as_not_alive(self, monkeypatch):
"""Conservative: PermissionError also means 'not alive' (matches existing behavior)."""
from tools.process_registry import ProcessRegistry
def fake_kill(pid, sig):
raise PermissionError(1, "Operation not permitted")
monkeypatch.setattr("tools.process_registry.os.kill", fake_kill)
assert ProcessRegistry._is_host_pid_alive(12345) is False
def test_zero_or_none_pid_returns_false_without_calling_kill(self, monkeypatch):
"""No wasted syscall on falsy pids."""
from tools.process_registry import ProcessRegistry
kill_calls = []
monkeypatch.setattr(
"tools.process_registry.os.kill",
lambda pid, sig: kill_calls.append(pid),
)
assert ProcessRegistry._is_host_pid_alive(None) is False
assert ProcessRegistry._is_host_pid_alive(0) is False
assert kill_calls == []
def test_alive_pid_returns_true(self, monkeypatch):
from tools.process_registry import ProcessRegistry
# os.kill returning None (default) means "probe succeeded → pid alive"
monkeypatch.setattr("tools.process_registry.os.kill", lambda pid, sig: None)
assert ProcessRegistry._is_host_pid_alive(os.getpid()) is True
# ---------------------------------------------------------------------------
# tzdata dependency
# ---------------------------------------------------------------------------
class TestTzdataDependencyDeclared:
"""Windows installs must pull tzdata for zoneinfo to work."""
def test_pyproject_declares_tzdata_for_win32(self):
root = Path(__file__).resolve().parents[2]
source = (root / "pyproject.toml").read_text(encoding="utf-8")
# The dependency line should be conditional on sys_platform == 'win32'
# and should NOT be in the core dependencies for Linux/macOS.
assert (
'tzdata>=2023.3; sys_platform == \'win32\'' in source
or "tzdata>=2023.3; sys_platform == 'win32'" in source
or 'tzdata>=2023.3; sys_platform == "win32"' in source
), "tzdata must be a Windows-only dep in pyproject.toml dependencies"
# ---------------------------------------------------------------------------
# README / docs consistency
# ---------------------------------------------------------------------------
class TestReadmeNoLongerSaysWindowsUnsupported:
"""The README shouldn't claim native Windows isn't supported."""
def test_readme_does_not_say_not_supported(self):
root = Path(__file__).resolve().parents[2]
source = (root / "README.md").read_text(encoding="utf-8")
# Previous string (removed in this PR): "Native Windows is not supported"
assert "Native Windows is not supported" not in source, (
"README.md still says native Windows is not supported — update the "
"install copy to reflect the PowerShell installer."
)
def test_readme_mentions_powershell_installer(self):
root = Path(__file__).resolve().parents[2]
source = (root / "README.md").read_text(encoding="utf-8")
assert "install.ps1" in source, (
"README.md must point at scripts/install.ps1 for Windows users"
)
# ---------------------------------------------------------------------------
# pty_bridge graceful import on Windows
# ---------------------------------------------------------------------------
class TestWebServerPtyBridgeGuard:
"""The web server must not crash if pty_bridge can't import (Windows)."""
def test_import_guard_present_in_source(self):
root = Path(__file__).resolve().parents[2]
source = (root / "hermes_cli" / "web_server.py").read_text(encoding="utf-8")
assert "_PTY_BRIDGE_AVAILABLE" in source
assert "except ImportError" in source, (
"web_server.py must wrap the pty_bridge import in try/except ImportError"
)
def test_pty_handler_checks_availability_flag(self):
"""The /api/pty handler must short-circuit when the bridge is unavailable."""
root = Path(__file__).resolve().parents[2]
source = (root / "hermes_cli" / "web_server.py").read_text(encoding="utf-8")
assert "if not _PTY_BRIDGE_AVAILABLE" in source, (
"/api/pty handler must return a friendly error when PTY is unavailable"
)
# ---------------------------------------------------------------------------
# Entry points wire configure_windows_stdio
# ---------------------------------------------------------------------------
class TestEntryPointsConfigureStdio:
"""cli.py, hermes_cli/main.py, gateway/run.py must call configure_windows_stdio."""
@pytest.mark.parametrize(
"relpath",
["cli.py", "hermes_cli/main.py", "gateway/run.py"],
)
def test_entry_point_calls_configure_stdio(self, relpath):
root = Path(__file__).resolve().parents[2]
source = (root / relpath).read_text(encoding="utf-8")
assert "configure_windows_stdio" in source, (
f"{relpath} must call hermes_cli.stdio.configure_windows_stdio() "
"early in startup so Windows consoles render Unicode without crashing"
)