[codex] Fix Windows dashboard chat PTY

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:<cols>;<rows>]` 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:<cols>;<rows>]` 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.
 

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,38 +101,48 @@ 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(
                     "The `ptyprocess` package is missing. "
                     "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
 

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=<session_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: