From 850fac14e35f224b6754c3e178df1ed59476fdc9 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Fri, 24 Apr 2026 12:51:04 -0400 Subject: [PATCH] chore: address copilot comments --- pyproject.toml | 7 +- tui_gateway/event_publisher.py | 65 ++++++++++++++++--- tui_gateway/transport.py | 4 +- uv.lock | 8 +-- web/src/components/ChatSidebar.tsx | 22 +++++-- web/src/components/ModelPickerDialog.tsx | 2 +- web/src/pages/ChatPage.tsx | 8 +++ .../docs/user-guide/features/web-dashboard.md | 2 +- 8 files changed, 87 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 221121524..1418df6d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,11 +34,6 @@ dependencies = [ "edge-tts>=7.2.7,<8", # Skills Hub (GitHub App JWT auth — optional, only needed for bot identity) "PyJWT[crypto]>=2.12.0,<3", # CVE-2026-32597 - # Web dashboard (`hermes dashboard` — localhost SPA + API) - "fastapi>=0.104.0,<1", - "uvicorn[standard]>=0.24.0,<1", - # Embedded Chat tab (/api/pty) on POSIX — required for PTY, not optional - "ptyprocess>=0.7.0,<1; sys_platform != 'win32'", ] [project.optional-dependencies] @@ -83,7 +78,7 @@ termux = [ ] dingtalk = ["dingtalk-stream>=0.20,<1", "alibabacloud-dingtalk>=2.0.0", "qrcode>=7.0,<8"] feishu = ["lark-oapi>=1.5.3,<2", "qrcode>=7.0,<8"] -# Same pins as core; kept so `hermes-agent[web]` stays a no-op alias for older docs/scripts. +# `hermes dashboard` (localhost SPA + API). Not in core to keep the default install lean. web = ["fastapi>=0.104.0,<1", "uvicorn[standard]>=0.24.0,<1"] rl = [ "atroposlib @ git+https://github.com/NousResearch/atropos.git@c20c85256e5a45ad31edf8b7276e9c5ee1995a30", diff --git a/tui_gateway/event_publisher.py b/tui_gateway/event_publisher.py index 5e618bc21..8510b8eac 100644 --- a/tui_gateway/event_publisher.py +++ b/tui_gateway/event_publisher.py @@ -13,12 +13,15 @@ already passes to ``write``). No JSON-RPC envelope here — the dashboard's Failure mode: silent. The agent loop must never block waiting for the sidecar to drain. A dead WS short-circuits all subsequent writes. +Actual ``send`` calls run on a daemon thread so the TeeTransport's +``write`` returns after enqueueing (best-effort; drop when the queue is full). """ from __future__ import annotations import json import logging +import queue import threading from typing import Optional @@ -29,15 +32,21 @@ except ImportError: # pragma: no cover - websockets is a required install path _log = logging.getLogger(__name__) +_DRAIN_STOP = object() + +_QUEUE_MAX = 256 + class WsPublisherTransport: - __slots__ = ("_url", "_lock", "_ws", "_dead") + __slots__ = ("_url", "_lock", "_ws", "_dead", "_q", "_worker") def __init__(self, url: str, *, connect_timeout: float = 2.0) -> None: self._url = url self._lock = threading.Lock() self._ws: Optional[object] = None self._dead = False + self._q: queue.Queue[object] = queue.Queue(maxsize=_QUEUE_MAX) + self._worker: Optional[threading.Thread] = None if ws_connect is None: self._dead = True @@ -51,30 +60,66 @@ class WsPublisherTransport: self._dead = True self._ws = None + return + + self._worker = threading.Thread( + target=self._drain, + name="hermes-ws-pub", + daemon=True, + ) + self._worker.start() + + def _drain(self) -> None: + while True: + item = self._q.get() + if item is _DRAIN_STOP: + return + if not isinstance(item, str): + continue + if self._ws is None: + continue + try: + with self._lock: + if self._ws is not None: + self._ws.send(item) # type: ignore[union-attr] + except Exception as exc: + _log.debug("event publisher write failed: %s", exc) + self._dead = True + self._ws = None + def write(self, obj: dict) -> bool: - if self._dead or self._ws is None: + if self._dead or self._ws is None or self._worker is None: return False + line = json.dumps(obj, ensure_ascii=False) + try: - with self._lock: - self._ws.send(json.dumps(obj, ensure_ascii=False)) # type: ignore[union-attr] + self._q.put_nowait(line) return True - except Exception as exc: - _log.debug("event publisher write failed: %s", exc) - self._dead = True - self._ws = None - + except queue.Full: return False def close(self) -> None: self._dead = True + w = self._worker + if w is not None and w.is_alive(): + try: + self._q.put_nowait(_DRAIN_STOP) + except queue.Full: + # Best-effort: if the queue is wedged, the daemon thread + # will be torn down with the process. + pass + w.join(timeout=3.0) + self._worker = None if self._ws is None: return try: - self._ws.close() # type: ignore[union-attr] + with self._lock: + if self._ws is not None: + self._ws.close() # type: ignore[union-attr] except Exception: pass diff --git a/tui_gateway/transport.py b/tui_gateway/transport.py index f9c71f961..a1b4b283d 100644 --- a/tui_gateway/transport.py +++ b/tui_gateway/transport.py @@ -107,12 +107,14 @@ class TeeTransport: self._secondaries = secondaries def write(self, obj: dict) -> bool: + # Primary first so a slow sidecar (WS publisher) never delays Ink/stdio. + ok = self._primary.write(obj) for sec in self._secondaries: try: sec.write(obj) except Exception: pass - return self._primary.write(obj) + return ok def close(self) -> None: try: diff --git a/uv.lock b/uv.lock index ab3d6a0c5..dfb2f786b 100644 --- a/uv.lock +++ b/uv.lock @@ -9,7 +9,7 @@ resolution-markers = [ ] [options] -exclude-newer = "2026-04-17T15:09:44.835508886Z" +exclude-newer = "2026-04-17T16:49:45.944715922Z" exclude-newer-span = "P7D" [[package]] @@ -1877,7 +1877,6 @@ dependencies = [ { name = "edge-tts" }, { name = "exa-py" }, { name = "fal-client" }, - { name = "fastapi" }, { name = "fire" }, { name = "firecrawl-py" }, { name = "httpx", extra = ["socks"] }, @@ -1885,7 +1884,6 @@ dependencies = [ { name = "openai" }, { name = "parallel-web" }, { name = "prompt-toolkit" }, - { name = "ptyprocess", marker = "sys_platform != 'win32'" }, { name = "pydantic" }, { name = "pyjwt", extra = ["crypto"] }, { name = "python-dotenv" }, @@ -1893,7 +1891,6 @@ dependencies = [ { name = "requests" }, { name = "rich" }, { name = "tenacity" }, - { name = "uvicorn", extra = ["standard"] }, ] [package.optional-dependencies] @@ -2062,7 +2059,6 @@ requires-dist = [ { name = "elevenlabs", marker = "extra == 'tts-premium'", specifier = ">=1.0,<2" }, { name = "exa-py", specifier = ">=2.9.0,<3" }, { name = "fal-client", specifier = ">=0.13.1,<1" }, - { name = "fastapi", specifier = ">=0.104.0,<1" }, { name = "fastapi", marker = "extra == 'rl'", specifier = ">=0.104.0,<1" }, { name = "fastapi", marker = "extra == 'web'", specifier = ">=0.104.0,<1" }, { name = "faster-whisper", marker = "extra == 'voice'", specifier = ">=1.0.0,<2" }, @@ -2109,7 +2105,6 @@ requires-dist = [ { name = "openai", specifier = ">=2.21.0,<3" }, { name = "parallel-web", specifier = ">=0.4.2,<1" }, { name = "prompt-toolkit", specifier = ">=3.0.52,<4" }, - { name = "ptyprocess", marker = "sys_platform != 'win32'", specifier = ">=0.7.0,<1" }, { name = "ptyprocess", marker = "sys_platform != 'win32' and extra == 'pty'", specifier = ">=0.7.0,<1" }, { name = "pydantic", specifier = ">=2.12.5,<3" }, { name = "pyjwt", extras = ["crypto"], specifier = ">=2.12.0,<3" }, @@ -2136,7 +2131,6 @@ requires-dist = [ { name = "tenacity", specifier = ">=9.1.4,<10" }, { name = "tinker", marker = "extra == 'rl'", git = "https://github.com/thinking-machines-lab/tinker.git?rev=30517b667f18a3dfb7ef33fb56cf686d5820ba2b" }, { name = "ty", marker = "extra == 'dev'", specifier = ">=0.0.1a29,<0.0.22" }, - { name = "uvicorn", extras = ["standard"], specifier = ">=0.24.0,<1" }, { name = "uvicorn", extras = ["standard"], marker = "extra == 'rl'", specifier = ">=0.24.0,<1" }, { name = "uvicorn", extras = ["standard"], marker = "extra == 'web'", specifier = ">=0.24.0,<1" }, { name = "wandb", marker = "extra == 'rl'", specifier = ">=0.15.0,<1" }, diff --git a/web/src/components/ChatSidebar.tsx b/web/src/components/ChatSidebar.tsx index 2ee32f97d..6bfac9cfa 100644 --- a/web/src/components/ChatSidebar.tsx +++ b/web/src/components/ChatSidebar.tsx @@ -87,6 +87,7 @@ export function ChatSidebar({ channel, className }: ChatSidebarProps) { const [error, setError] = useState(null); useEffect(() => { + let cancelled = false; const offState = gw.onState(setState); const offSessionInfo = gw.on("session.info", (ev) => { @@ -111,15 +112,26 @@ export function ChatSidebar({ channel, className }: ChatSidebarProps) { // sidecar is independent of the PTY pane's session by design — we // only need a sid to drive the model picker's slash.exec calls. gw.connect() - .then(() => gw.request<{ session_id: string }>("session.create", {})) - .then((created) => { - if (created?.session_id) { - setSessionId(created.session_id); + .then(() => { + if (cancelled) { + return; } + return gw.request<{ session_id: string }>("session.create", {}); }) - .catch((e: Error) => setError(e.message)); + .then((created) => { + if (cancelled || !created?.session_id) { + return; + } + setSessionId(created.session_id); + }) + .catch((e: Error) => { + if (!cancelled) { + setError(e.message); + } + }); return () => { + cancelled = true; offState(); offSessionInfo(); offError(); diff --git a/web/src/components/ModelPickerDialog.tsx b/web/src/components/ModelPickerDialog.tsx index 13a7268ac..d30fb8dd6 100644 --- a/web/src/components/ModelPickerDialog.tsx +++ b/web/src/components/ModelPickerDialog.tsx @@ -138,7 +138,7 @@ export function ModelPickerDialog({ gw, sessionId, onClose, onSubmit }: Props) { return (
e.target === e.currentTarget && onClose()} role="dialog" aria-modal="true" diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 2e46f1fb4..639c6324f 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -443,6 +443,10 @@ export default function ChatPage() { const ws = new WebSocket(url); ws.binaryType = "arraybuffer"; wsRef.current = ws; + // Suppress banner/terminal side-effects when cleanup() calls `ws.close()` + // (React StrictMode remount, route change) so we never write to a + // disposed xterm or setState on an unmounted tree. + let unmounting = false; ws.onopen = () => { setBanner(null); @@ -463,6 +467,9 @@ export default function ChatPage() { ws.onclose = (ev) => { wsRef.current = null; + if (unmounting) { + return; + } if (ev.code === 4401) { setBanner("Auth failed. Reload the page to refresh the session token."); return; @@ -539,6 +546,7 @@ export default function ChatPage() { term.focus(); return () => { + unmounting = true; onDataDisposable.dispose(); onResizeDisposable.dispose(); if (metricsDebounce) clearTimeout(metricsDebounce); diff --git a/website/docs/user-guide/features/web-dashboard.md b/website/docs/user-guide/features/web-dashboard.md index 6adddd126..3f56978aa 100644 --- a/website/docs/user-guide/features/web-dashboard.md +++ b/website/docs/user-guide/features/web-dashboard.md @@ -37,7 +37,7 @@ hermes dashboard --no-open ## Prerequisites -The web dashboard requires FastAPI and Uvicorn. The Chat tab additionally needs `ptyprocess` to spawn the embedded TUI behind a pseudo-terminal. Install both with: +The default `hermes-agent` install does not ship the HTTP stack or PTY helper — those are optional extras. The **web dashboard** needs FastAPI and Uvicorn (`web` extra). The **Chat** tab also needs `ptyprocess` to spawn the embedded TUI behind a pseudo-terminal (`pty` extra on POSIX). Install both with: ```bash pip install 'hermes-agent[web,pty]'