diff --git a/AGENTS.md b/AGENTS.md index 6c0036efd5a65..16c4afbc533a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -276,8 +276,8 @@ The dashboard embeds the real `hermes --tui` — **not** a rewrite. See `hermes - Browser loads `web/src/pages/ChatPage.tsx`, which mounts xterm.js's `Terminal` with the WebGL renderer, `@xterm/addon-fit` for container-driven resize, and `@xterm/addon-unicode11` for modern wide-character widths. - `/api/pty?token=…` upgrades to a WebSocket; auth uses the same ephemeral `_SESSION_TOKEN` as REST, via query param (browsers can't set `Authorization` on WS upgrade). -- The server spawns whatever `hermes --tui` would spawn, through `ptyprocess` (POSIX PTY — WSL works, native Windows does not). -- Frames: raw PTY bytes each direction; resize via `\x1b[RESIZE:;]` intercepted on the server and applied with `TIOCSWINSZ`. +- The server spawns whatever `hermes --tui` would spawn, through the platform PTY backend (`ptyprocess` on POSIX, `pywinpty`/ConPTY on native Windows). +- Frames: raw PTY bytes each direction; resize via `\x1b[RESIZE:;]` intercepted on the server and forwarded to the platform PTY. **Do not re-implement the primary chat experience in React.** The main transcript, composer/input flow (including slash-command behavior), and PTY-backed terminal belong to the embedded `hermes --tui` — anything new you add to Ink shows up in the dashboard automatically. If you find yourself rebuilding the transcript or composer for the dashboard, stop and extend Ink instead. diff --git a/hermes_cli/pty_bridge.py b/hermes_cli/pty_bridge.py index a1779aa1dd289..6241449d22d91 100644 --- a/hermes_cli/pty_bridge.py +++ b/hermes_cli/pty_bridge.py @@ -2,49 +2,56 @@ Wraps a child process behind a pseudo-terminal so its ANSI output can be streamed to a browser-side terminal emulator (xterm.js) and typed -keystrokes can be fed back in. The only caller today is the -``/api/pty`` WebSocket endpoint in ``hermes_cli.web_server``. +keystrokes can be fed back in. The only caller today is the ``/api/pty`` +WebSocket endpoint in ``hermes_cli.web_server``. Design constraints: -* **POSIX-only.** This module depends on ``fcntl``, ``termios``, and - ``ptyprocess``, none of which exist on native Windows Python. Native - Windows ConPTY is a different API (Windows 10 build 17763+) and would - need a separate Windows implementation (``pywinpty``) — that's tracked - as a future enhancement. On native Windows, importing this module - raises :class:`ImportError` and the dashboard's ``/chat`` tab shows a - WSL-recommended banner instead of crashing. Every other feature in the - dashboard (sessions, jobs, metrics, config editor) works natively. -* **Zero Node dependency on the server side.** We use :mod:`ptyprocess`, - which is a pure-Python wrapper around the OS calls. The browser talks - to the same ``hermes --tui`` binary it would launch from the CLI, so - every TUI feature (slash popover, model picker, tool rows, markdown, - skin engine, clarify/sudo/approval prompts) ships automatically. -* **Byte-safe I/O.** Reads and writes go through the PTY master fd - directly — we avoid :class:`ptyprocess.PtyProcessUnicode` because - streaming ANSI is inherently byte-oriented and UTF-8 boundaries may land - mid-read. +* Native terminal backend per platform. POSIX uses ``ptyprocess`` and Windows + uses ConPTY through ``pywinpty``. Both are exposed through the same + byte-oriented bridge so the dashboard endpoint does not need platform + branches. +* Zero Node dependency on the server side. The browser talks to the same + ``hermes --tui`` binary it would launch from the CLI, so every TUI feature + ships automatically. +* Byte-safe I/O. POSIX reads and writes go through the PTY master fd directly; + Windows ConPTY exposes text reads/writes, so we encode/decode at the bridge + boundary while preserving the WebSocket's byte-stream contract. """ from __future__ import annotations import errno -import fcntl import os import select -import signal -import struct import sys -import termios import time -from typing import Optional, Sequence +from typing import Any, Optional, Sequence -try: - import ptyprocess # type: ignore - _PTY_AVAILABLE = not sys.platform.startswith("win") -except ImportError: # pragma: no cover - dev env without ptyprocess +_IS_WINDOWS = sys.platform.startswith("win") + +if _IS_WINDOWS: + try: + from winpty import PtyProcess as _WinPtyProcess # type: ignore + + _PTY_AVAILABLE = True + _PTY_IMPORT_ERROR: Optional[BaseException] = None + except ImportError as exc: # pragma: no cover - env without pywinpty + _WinPtyProcess = None # type: ignore + _PTY_AVAILABLE = False + _PTY_IMPORT_ERROR = exc ptyprocess = None # type: ignore - _PTY_AVAILABLE = False +else: + try: + import ptyprocess # type: ignore + + _PTY_AVAILABLE = True + _PTY_IMPORT_ERROR = None + except ImportError as exc: # pragma: no cover - dev env without ptyprocess + ptyprocess = None # type: ignore + _PTY_AVAILABLE = False + _PTY_IMPORT_ERROR = exc + _WinPtyProcess = None # type: ignore __all__ = ["PtyBridge", "PtyUnavailableError"] @@ -53,26 +60,26 @@ class PtyUnavailableError(RuntimeError): """Raised when a PTY cannot be created on this platform. - Today this means native Windows (no ConPTY bindings) or a dev - environment missing the ``ptyprocess`` dependency. The dashboard - surfaces the message to the user as a chat-tab banner. + This usually means the platform-specific dependency is missing: + ``ptyprocess`` on POSIX or ``pywinpty`` on Windows. The dashboard surfaces + the message to the user as a chat-tab banner. """ class PtyBridge: - """Thin wrapper around ``ptyprocess.PtyProcess`` for byte streaming. - - Not thread-safe. A single bridge is owned by the WebSocket handler - that spawned it; the reader runs in an executor thread while writes - happen on the event-loop thread. Both sides are OK because the - kernel PTY is the actual synchronization point — we never call - :mod:`ptyprocess` methods concurrently, we only call ``os.read`` and - ``os.write`` on the master fd, which is safe. + """Thin wrapper around a platform PTY process for byte streaming. + + Not thread-safe. A single bridge is owned by the WebSocket handler that + spawned it; the reader runs in an executor thread while writes happen on + the event-loop thread. On POSIX, both sides are OK because the kernel PTY + is the synchronization point. On Windows, reads wait on pywinpty's + socket-backed fd before calling into ConPTY. """ - def __init__(self, proc: "ptyprocess.PtyProcess"): # type: ignore[name-defined] + def __init__(self, proc: Any, *, backend: str): self._proc = proc self._fd: int = proc.fd + self._backend = backend self._closed = False # -- lifecycle -------------------------------------------------------- @@ -94,15 +101,15 @@ def spawn( ) -> "PtyBridge": """Spawn ``argv`` behind a new PTY and return a bridge. - Raises :class:`PtyUnavailableError` if the platform can't host a - PTY. Raises :class:`FileNotFoundError` or :class:`OSError` for - ordinary exec failures (missing binary, bad cwd, etc.). + Raises :class:`PtyUnavailableError` if the platform can't host a PTY. + Raises :class:`FileNotFoundError` or :class:`OSError` for ordinary exec + failures (missing binary, bad cwd, etc.). """ if not _PTY_AVAILABLE: - if sys.platform.startswith("win"): + if _IS_WINDOWS: raise PtyUnavailableError( - "Pseudo-terminals are unavailable on this platform. " - "Hermes Agent supports Windows only via WSL." + "The `pywinpty` package is missing. " + "Install with: pip install pywinpty." ) if ptyprocess is None: raise PtyUnavailableError( @@ -110,22 +117,32 @@ def spawn( "Install with: pip install ptyprocess " "(or pip install -e '.[pty]')." ) - raise PtyUnavailableError("Pseudo-terminals are unavailable.") + detail = f" ({_PTY_IMPORT_ERROR})" if _PTY_IMPORT_ERROR else "" + raise PtyUnavailableError(f"Pseudo-terminals are unavailable{detail}.") + # PTY-hosted programs expect TERM to describe the terminal type. - # CI often runs without TERM in the parent process, which makes - # simple terminal probes like `tput cols` fail before winsize reads. # Preserve explicit caller overrides, but backfill a sensible default # when TERM is missing or blank. spawn_env = (os.environ.copy() if env is None else env.copy()) if not spawn_env.get("TERM"): spawn_env["TERM"] = "xterm-256color" + + if _IS_WINDOWS: + proc = _WinPtyProcess.spawn( # type: ignore[union-attr] + list(argv), + cwd=cwd, + env=spawn_env, + dimensions=(rows, cols), + ) + return cls(proc, backend="windows") + proc = ptyprocess.PtyProcess.spawn( # type: ignore[union-attr] list(argv), cwd=cwd, env=spawn_env, dimensions=(rows, cols), ) - return cls(proc) + return cls(proc, backend="posix") @property def pid(self) -> int: @@ -145,11 +162,11 @@ def read(self, timeout: float = 0.2) -> Optional[bytes]: """Read up to 64 KiB of raw bytes from the PTY master. Returns: - * bytes — zero or more bytes of child output - * empty bytes (``b""``) — no data available within ``timeout`` - * None — child has exited and the master fd is at EOF + * bytes: zero or more bytes of child output + * empty bytes (``b""``): no data available within ``timeout`` + * None: child has exited and the master fd is at EOF - Never blocks longer than ``timeout`` seconds. Safe to call after + Never blocks longer than ``timeout`` seconds. Safe to call after :meth:`close`; returns ``None`` in that case. """ if self._closed: @@ -159,11 +176,23 @@ def read(self, timeout: float = 0.2) -> Optional[bytes]: except (OSError, ValueError): return None if not readable: - return b"" + return None if not self.is_alive() else b"" + + if self._backend == "windows": + try: + data = self._proc.read(65536) + except (EOFError, OSError): + return None + if not data: + return None + if isinstance(data, bytes): + return data + return str(data).encode("utf-8", errors="replace") + try: data = os.read(self._fd, 65536) except OSError as exc: - # EIO on Linux = slave side closed. EBADF = already closed. + # EIO on Linux = slave side closed. EBADF = already closed. if exc.errno in {errno.EIO, errno.EBADF}: return None raise @@ -175,6 +204,13 @@ def write(self, data: bytes) -> None: """Write raw bytes to the PTY master (i.e. the child's stdin).""" if self._closed or not data: return + if self._backend == "windows": + try: + self._proc.write(data.decode("utf-8", errors="replace")) + except (EOFError, OSError): + return + return + # os.write can return a short write under load; loop until drained. view = memoryview(data) while view: @@ -189,9 +225,20 @@ def write(self, data: bytes) -> None: view = view[n:] def resize(self, cols: int, rows: int) -> None: - """Forward a terminal resize to the child via ``TIOCSWINSZ``.""" + """Forward a terminal resize to the child.""" if self._closed: return + if self._backend == "windows": + try: + self._proc.setwinsize(max(1, rows), max(1, cols)) + except Exception: + pass + return + + import fcntl + import struct + import termios + # struct winsize: rows, cols, xpixel, ypixel (all unsigned short) winsize = struct.pack("HHHH", max(1, rows), max(1, cols), 0, 0) try: @@ -202,18 +249,31 @@ def resize(self, cols: int, rows: int) -> None: # -- teardown --------------------------------------------------------- def close(self) -> None: - """Terminate the child (SIGTERM → 0.5s grace → SIGKILL) and close fds. + """Terminate the child and close fds. - Idempotent. Reaping the child is important so we don't leak - zombies across the lifetime of the dashboard process. + Idempotent. Reaping the child is important so we don't leak zombies + across the lifetime of the dashboard process. """ if self._closed: return self._closed = True - # SIGHUP is the conventional "your terminal went away" signal. - # We escalate if the child ignores it. - for sig in (signal.SIGHUP, signal.SIGTERM, signal.SIGKILL): # windows-footgun: ok — POSIX-only module (imports fcntl/termios/ptyprocess at top) + if self._backend == "windows": + try: + self._proc.terminate(force=True) + except Exception: + pass + try: + self._proc.close(force=True) + except Exception: + pass + return + + import signal + + # SIGHUP is the conventional "your terminal went away" signal. We + # escalate if the child ignores it. + for sig in (signal.SIGHUP, signal.SIGTERM, signal.SIGKILL): if not self._proc.isalive(): break try: @@ -229,7 +289,6 @@ def close(self) -> None: except Exception: pass - # Context-manager sugar — handy in tests and ad-hoc scripts. def __enter__(self) -> "PtyBridge": return self diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 4ff30f9e23b7e..e3e7418902221 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -5401,8 +5401,9 @@ async def get_models_analytics(days: int = 30): # /api/pty — PTY-over-WebSocket bridge for the dashboard "Chat" tab. # # The endpoint spawns the same ``hermes --tui`` binary the CLI uses, behind -# a POSIX pseudo-terminal, and forwards bytes + resize escapes across a -# WebSocket. The browser renders the ANSI through xterm.js (see +# the platform PTY backend (ptyprocess on POSIX, pywinpty/ConPTY on Windows), +# and forwards bytes + resize escapes across a WebSocket. The browser renders +# the ANSI through xterm.js (see # web/src/pages/ChatPage.tsx). # # Auth: ``?token=`` query param (browsers can't set @@ -5413,10 +5414,9 @@ async def get_models_analytics(days: int = 30): import re -# PTY bridge is POSIX-only (depends on fcntl/termios/ptyprocess). On native -# Windows the import raises; catch and leave PtyBridge=None so the rest of -# the dashboard (sessions, jobs, metrics, config editor) still loads and the -# /api/pty endpoint cleanly refuses with a WSL-suggested message. +# PTY bridge is optional at import time so the rest of the dashboard +# (sessions, jobs, metrics, config editor) still loads if the platform +# dependency is missing. try: from hermes_cli.pty_bridge import PtyBridge, PtyUnavailableError _PTY_BRIDGE_AVAILABLE = True @@ -5575,9 +5575,9 @@ def _resolve_chat_argv( ) -> tuple[list[str], Optional[str], Optional[dict]]: """Resolve the argv + cwd + env for the chat PTY. - Default: whatever ``hermes --tui`` would run. Tests monkeypatch this - function to inject a tiny fake command (``cat``, ``sh -c 'printf …'``) - so nothing has to build Node or the TUI bundle. + Default: whatever ``hermes --tui`` would run. Tests monkeypatch this + function to inject a tiny platform-native command so nothing has to build + Node or the TUI bundle. Session resume is propagated via the ``HERMES_TUI_RESUME`` env var — matching what ``hermes_cli.main._launch_tui`` does for the CLI path. @@ -5713,14 +5713,14 @@ async def pty_ws(ws: WebSocket) -> None: await ws.accept() - # On native Windows, the POSIX PTY bridge can't be imported. Tell the - # client and close cleanly rather than pretending the feature works. + # If the platform PTY dependency is missing, tell the client and close + # cleanly rather than pretending the feature works. if not _PTY_BRIDGE_AVAILABLE: await ws.send_text( - "\r\n\x1b[31mChat unavailable: the embedded terminal requires a " - "POSIX PTY, which native Windows Python doesn't provide.\x1b[0m\r\n" - "\x1b[33mInstall Hermes inside WSL2 to use the dashboard's /chat " - "tab — the rest of the dashboard works here.\x1b[0m\r\n" + "\r\n\x1b[31mChat unavailable: embedded terminal support is " + "not available in this Hermes environment.\x1b[0m\r\n" + "\x1b[33mInstall the platform PTY dependency (`ptyprocess` on " + "POSIX or `pywinpty` on Windows) and restart the dashboard.\x1b[0m\r\n" ) await ws.close(code=1011) return @@ -5738,7 +5738,6 @@ async def pty_ws(ws: WebSocket) -> None: await ws.close(code=1011) return - try: bridge = PtyBridge.spawn(argv, cwd=cwd, env=env) except PtyUnavailableError as exc: diff --git a/tests/hermes_cli/test_pty_bridge.py b/tests/hermes_cli/test_pty_bridge.py index 4f366fd7218ef..c92f1a1484355 100644 --- a/tests/hermes_cli/test_pty_bridge.py +++ b/tests/hermes_cli/test_pty_bridge.py @@ -1,11 +1,13 @@ -"""Unit tests for hermes_cli.pty_bridge — PTY spawning + byte forwarding. +"""Unit tests for hermes_cli.pty_bridge. -These tests drive the bridge with minimal POSIX processes (echo, env, sleep, -printf) to verify it behaves like a PTY you can read/write/resize/close. +These tests drive the bridge with minimal processes to verify it behaves like +a PTY you can read/write/resize/close. POSIX uses ptyprocess; native Windows +uses pywinpty/ConPTY. """ from __future__ import annotations +import importlib.util import os import shutil import sys @@ -13,13 +15,20 @@ import pytest -pytest.importorskip("ptyprocess", reason="ptyprocess not installed") - from hermes_cli.pty_bridge import PtyBridge, PtyUnavailableError -skip_on_windows = pytest.mark.skipif( - sys.platform.startswith("win"), reason="PTY bridge is POSIX-only" +_POSIX_PTY_AVAILABLE = importlib.util.find_spec("ptyprocess") is not None +_WINDOWS_PTY_AVAILABLE = importlib.util.find_spec("winpty") is not None + +skip_posix_unavailable = pytest.mark.skipif( + sys.platform.startswith("win") or not _POSIX_PTY_AVAILABLE, + reason="POSIX PTY bridge requires ptyprocess", +) + +skip_windows_unavailable = pytest.mark.skipif( + not sys.platform.startswith("win") or not _WINDOWS_PTY_AVAILABLE, + reason="Windows PTY bridge requires pywinpty/ConPTY", ) @@ -37,7 +46,7 @@ def _read_until(bridge: PtyBridge, needle: bytes, timeout: float = 5.0) -> bytes return bytes(buf) -@skip_on_windows +@skip_posix_unavailable class TestPtyBridgeSpawn: def test_is_available_on_posix(self): assert PtyBridge.is_available() is True @@ -54,7 +63,7 @@ def test_spawn_raises_on_missing_argv0(self, tmp_path): PtyBridge.spawn([str(tmp_path / "definitely-not-a-real-binary")]) -@skip_on_windows +@skip_posix_unavailable class TestPtyBridgeIO: def test_reads_child_stdout(self): bridge = PtyBridge.spawn(["/bin/sh", "-c", "printf hermes-ok"]) @@ -65,8 +74,8 @@ def test_reads_child_stdout(self): bridge.close() def test_write_sends_to_child_stdin(self): - # `cat` with no args echoes stdin back to stdout. We write a line, - # read it back, then signal EOF to let cat exit cleanly. + # `cat` with no args echoes stdin back to stdout. We write a line, + # read it back, then close the PTY to let cat exit cleanly. bridge = PtyBridge.spawn([shutil.which("cat") or "cat"]) try: bridge.write(b"hello-pty\n") @@ -79,11 +88,9 @@ def test_read_returns_none_after_child_exits(self): bridge = PtyBridge.spawn(["/bin/sh", "-c", "printf done"]) try: _read_until(bridge, b"done") - # Give the child a beat to exit cleanly, then drain until EOF. deadline = time.monotonic() + 3.0 while bridge.is_alive() and time.monotonic() < deadline: bridge.read(timeout=0.1) - # Next reads after exit should return None (EOF), not raise. got_none = False for _ in range(10): if bridge.read(timeout=0.1) is None: @@ -94,11 +101,9 @@ def test_read_returns_none_after_child_exits(self): bridge.close() -@skip_on_windows +@skip_posix_unavailable class TestPtyBridgeResize: def test_resize_updates_child_winsize(self): - # Query the TTY ioctl directly instead of using tput, which requires - # TERM and fails in GitHub Actions' non-interactive environment. winsize_script = ( "import fcntl, struct, termios, time; " "time.sleep(0.1); " @@ -114,26 +119,24 @@ def test_resize_updates_child_winsize(self): try: bridge.resize(cols=123, rows=45) output = _read_until(bridge, b"45", timeout=5.0) - # tput prints just the numbers, one per line assert b"123" in output assert b"45" in output finally: bridge.close() -@skip_on_windows +@skip_posix_unavailable class TestPtyBridgeClose: def test_close_is_idempotent(self): bridge = PtyBridge.spawn(["/bin/sh", "-c", "sleep 30"]) bridge.close() - bridge.close() # must not raise + bridge.close() assert not bridge.is_alive() def test_close_terminates_long_running_child(self): bridge = PtyBridge.spawn(["/bin/sh", "-c", "sleep 30"]) pid = bridge.pid bridge.close() - # Give the kernel a moment to reap deadline = time.monotonic() + 3.0 reaped = False while time.monotonic() < deadline: @@ -146,7 +149,7 @@ def test_close_terminates_long_running_child(self): assert reaped, f"pid {pid} still running after close()" -@skip_on_windows +@skip_posix_unavailable class TestPtyBridgeEnv: def test_cwd_is_respected(self, tmp_path): bridge = PtyBridge.spawn( @@ -171,9 +174,82 @@ def test_env_is_forwarded(self): bridge.close() +@skip_windows_unavailable +class TestWindowsPtyBridge: + def test_is_available_on_windows(self): + assert PtyBridge.is_available() is True + + def test_reads_child_stdout(self): + bridge = PtyBridge.spawn(["cmd.exe", "/c", "echo hermes-winpty-ok"]) + try: + output = _read_until(bridge, b"hermes-winpty-ok") + assert b"hermes-winpty-ok" in output + finally: + bridge.close() + + def test_write_sends_to_child_stdin(self): + bridge = PtyBridge.spawn(["cmd.exe"]) + try: + _read_until(bridge, b">") + bridge.write(b"echo hello-winpty\r") + output = _read_until(bridge, b"hello-winpty") + assert b"hello-winpty" in output + finally: + bridge.write(b"exit\r") + bridge.close() + + def test_read_returns_none_after_child_exits(self): + bridge = PtyBridge.spawn(["cmd.exe", "/c", "echo done-winpty"]) + try: + _read_until(bridge, b"done-winpty") + deadline = time.monotonic() + 3.0 + while bridge.is_alive() and time.monotonic() < deadline: + bridge.read(timeout=0.1) + got_none = False + for _ in range(10): + if bridge.read(timeout=0.1) is None: + got_none = True + break + assert got_none, "PtyBridge.read did not return None after child EOF" + finally: + bridge.close() + + def test_resize_does_not_raise(self): + bridge = PtyBridge.spawn(["cmd.exe"]) + try: + bridge.resize(cols=99, rows=41) + finally: + bridge.write(b"exit\r") + bridge.close() + + def test_cwd_is_respected(self, tmp_path): + bridge = PtyBridge.spawn(["cmd.exe", "/c", "cd"], cwd=str(tmp_path)) + try: + output = _read_until(bridge, str(tmp_path).encode()) + assert str(tmp_path).encode() in output + finally: + bridge.close() + + def test_env_is_forwarded(self): + bridge = PtyBridge.spawn( + ["cmd.exe", "/c", "echo %HERMES_PTY_TEST%"], + env={**os.environ, "HERMES_PTY_TEST": "winpty-env-works"}, + ) + try: + output = _read_until(bridge, b"winpty-env-works") + assert b"winpty-env-works" in output + finally: + bridge.close() + + def test_close_is_idempotent(self): + bridge = PtyBridge.spawn(["cmd.exe"]) + bridge.close() + bridge.close() + assert not bridge.is_alive() + + class TestPtyBridgeUnavailable: - """Platform fallback semantics — PtyUnavailableError is importable and - carries a user-readable message.""" + """PtyUnavailableError is importable and carries a user-readable message.""" def test_error_carries_user_message(self): err = PtyUnavailableError("platform not supported") diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index d7a5c25a5ccd9..7a36f2bfa41d6 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -2626,21 +2626,43 @@ def test_page_scoped_slots_preserved(self, tmp_path, monkeypatch): # --------------------------------------------------------------------------- # /api/pty WebSocket — terminal bridge for the dashboard "Chat" tab. # -# These tests drive the endpoint with a tiny fake command (typically ``cat`` -# or ``sh -c 'printf …'``) instead of the real ``hermes --tui`` binary. The -# endpoint resolves its argv through ``_resolve_chat_argv``, so tests -# monkeypatch that hook. +# These tests drive the endpoint with a tiny platform-native command instead +# of the real ``hermes --tui`` binary. The endpoint resolves its argv through +# ``_resolve_chat_argv``, so tests monkeypatch that hook. # --------------------------------------------------------------------------- +import importlib.util import sys -skip_on_windows = pytest.mark.skipif( - sys.platform.startswith("win"), reason="PTY bridge is POSIX-only" +_IS_WINDOWS = sys.platform.startswith("win") +_PTY_BACKEND_AVAILABLE = importlib.util.find_spec( + "winpty" if _IS_WINDOWS else "ptyprocess" +) is not None + +skip_pty_backend_unavailable = pytest.mark.skipif( + not _PTY_BACKEND_AVAILABLE, + reason="platform PTY dependency is not installed", +) + +skip_posix_pty_unavailable = pytest.mark.skipif( + _IS_WINDOWS or importlib.util.find_spec("ptyprocess") is None, + reason="test requires POSIX ptyprocess semantics", ) -@skip_on_windows +def _stdout_argv(text: str) -> list[str]: + if _IS_WINDOWS: + return ["cmd.exe", "/c", f"echo {text}"] + return ["/bin/sh", "-c", f"printf {text}"] + + +def _interactive_echo_argv() -> list[str]: + if _IS_WINDOWS: + return ["cmd.exe"] + return ["/bin/cat"] + + class TestPtyWebSocket: @pytest.fixture(autouse=True) def _setup(self, monkeypatch, _isolate_hermes_home): @@ -2671,7 +2693,10 @@ def test_resolve_chat_argv_uses_dashboard_scroll_env(self, monkeypatch): monkeypatch.setattr( main_mod, "_make_tui_argv", - lambda project_root, tui_dev=False: (["node", "dist/entry.js"], "/tmp/ui-tui"), + lambda project_root, tui_dev=False: ( + ["node", "dist/entry.js"], + str(Path.cwd()), + ), ) _argv, _cwd, env = self.ws_module._resolve_chat_argv() @@ -2692,7 +2717,7 @@ def test_rejects_missing_token(self, monkeypatch): monkeypatch.setattr( self.ws_module, "_resolve_chat_argv", - lambda resume=None, sidecar_url=None: (["/bin/cat"], None, None), + lambda resume=None, sidecar_url=None: (_interactive_echo_argv(), None, None), ) from starlette.websockets import WebSocketDisconnect @@ -2705,7 +2730,7 @@ def test_rejects_bad_token(self, monkeypatch): monkeypatch.setattr( self.ws_module, "_resolve_chat_argv", - lambda resume=None, sidecar_url=None: (["/bin/cat"], None, None), + lambda resume=None, sidecar_url=None: (_interactive_echo_argv(), None, None), ) from starlette.websockets import WebSocketDisconnect @@ -2714,12 +2739,23 @@ def test_rejects_bad_token(self, monkeypatch): pass assert exc.value.code == 4401 + def test_missing_bridge_dependency_closes_with_install_message(self, monkeypatch): + monkeypatch.setattr(self.ws_module, "_PTY_BRIDGE_AVAILABLE", False) + + with self.client.websocket_connect(self._url()) as conn: + msg = conn.receive_text() + + assert "platform PTY dependency" in msg + assert "pywinpty" in msg + assert "WSL" not in msg + + @skip_pty_backend_unavailable def test_streams_child_stdout_to_client(self, monkeypatch): monkeypatch.setattr( self.ws_module, "_resolve_chat_argv", lambda resume=None, sidecar_url=None: ( - ["/bin/sh", "-c", "printf hermes-ws-ok"], + _stdout_argv("hermes-ws-ok"), None, None, ), @@ -2742,16 +2778,20 @@ def test_streams_child_stdout_to_client(self, monkeypatch): break assert b"hermes-ws-ok" in buf + @skip_pty_backend_unavailable def test_client_input_reaches_child_stdin(self, monkeypatch): - # ``cat`` echoes stdin back, so a write → read round-trip proves - # the full duplex path. + # The child echoes input, so a write/read round-trip proves the full + # duplex path. monkeypatch.setattr( self.ws_module, "_resolve_chat_argv", - lambda resume=None, sidecar_url=None: (["/bin/cat"], None, None), + lambda resume=None, sidecar_url=None: (_interactive_echo_argv(), None, None), ) with self.client.websocket_connect(self._url()) as conn: - conn.send_bytes(b"round-trip-payload\n") + if _IS_WINDOWS: + conn.send_bytes(b"echo round-trip-payload\r") + else: + conn.send_bytes(b"round-trip-payload\n") buf = b"" import time @@ -2764,6 +2804,7 @@ def test_client_input_reaches_child_stdin(self, monkeypatch): break assert b"round-trip-payload" in buf + @skip_posix_pty_unavailable def test_resize_escape_is_forwarded(self, monkeypatch): # Resize escape gets intercepted and applied via TIOCSWINSZ, then the # child reads the TTY ioctl directly. Avoid tput because CI may not set @@ -2817,7 +2858,7 @@ def _raise(argv, **kwargs): monkeypatch.setattr( self.ws_module, "_resolve_chat_argv", - lambda resume=None, sidecar_url=None: (["/bin/cat"], None, None), + lambda resume=None, sidecar_url=None: (_interactive_echo_argv(), None, None), ) # Patch PtyBridge.spawn at the web_server module's binding. import hermes_cli.web_server as ws_mod @@ -2829,12 +2870,13 @@ def _raise(argv, **kwargs): msg = conn.receive_text() assert "pty missing" in msg or "unavailable" in msg.lower() or "pty" in msg.lower() + @skip_pty_backend_unavailable def test_resume_parameter_is_forwarded_to_argv(self, monkeypatch): captured: dict = {} def fake_resolve(resume=None, sidecar_url=None): captured["resume"] = resume - return (["/bin/sh", "-c", "printf resume-arg-ok"], None, None) + return (_stdout_argv("resume-arg-ok"), None, None) monkeypatch.setattr(self.ws_module, "_resolve_chat_argv", fake_resolve) @@ -2846,6 +2888,7 @@ def fake_resolve(resume=None, sidecar_url=None): pass assert captured.get("resume") == "sess-42" + @skip_pty_backend_unavailable def test_channel_param_propagates_sidecar_url(self, monkeypatch): """When /api/pty is opened with ?channel=, the PTY child gets a HERMES_TUI_SIDECAR_URL env var pointing back at /api/pub on the @@ -2854,7 +2897,7 @@ def test_channel_param_propagates_sidecar_url(self, monkeypatch): def fake_resolve(resume=None, sidecar_url=None): captured["sidecar_url"] = sidecar_url - return (["/bin/sh", "-c", "printf sidecar-ok"], None, None) + return (_stdout_argv("sidecar-ok"), None, None) monkeypatch.setattr(self.ws_module, "_resolve_chat_argv", fake_resolve) monkeypatch.setattr(