fix(web_server): wrap _resolve_chat_argv in run_in_executor to prevent event-loop freeze in pty_ws

[davidcampbelldc]

Summary

pty_ws is an async ASGI handler. After await ws.accept() queues the websocket.accept ASGI message, the handler then calls _resolve_chat_argv (which delegates to _make_tui_argv). _make_tui_argv does synchronous filesystem stat checks, shutil.which calls, and conditional subprocess.run([npm, "install"]) / subprocess.run([npm, "run", "build"]). Any slow path freezes the uvicorn event loop until the subprocess returns, during which the queued accept message can't flush to the wire.

Concrete failure mode observed in the wild: behind nginx + Cloudflare, the proxy upstream timeout fires before the npm-build subprocess returns. The proxy closes the connection. Browser sees 502 Bad Gateway. Direct curl (via aws ssm start-session AWS-StartPortForwardingSession, bypassing all proxies) sees "Empty reply from server" after ~14-16 seconds. Nothing logged in the dashboard log because the event loop never had a chance to drain Python's log handler queue either.

Wrapping the call in asyncio.run_in_executor keeps the event loop responsive even when the underlying subprocess work takes 15+ seconds. The 101 response flushes immediately, and the slow work proceeds in a thread pool. Worst case: the user sees a brief pause before chat output starts, instead of a silent 502.

This patch also broadens the exception handling from except SystemExit to also catch the broad Exception family, logging via Python's logging module and emitting a visible error frame to the client. This is defence-in-depth against any future regression — a recent silent-failure story (a filename-mismatch in a since-removed _hermes_ink_bundle_stale function triggering an infinite npm run build loop) would have surfaced as a clear log entry + visible error frame with this patch in place, rather than a 14-second silent timeout that took a strace bisect to diagnose.

Reproduction

Hard to reproduce on current main because the recent TUI refactor (42627b4e refactor(tui): bundle with esbuild, drop runtime node_modules) eliminated most of the blocking subprocess work in _make_tui_argv. But the structural concern remains: any future code touching _resolve_chat_argv that adds back a sync subprocess call (e.g., a new bootstrap helper, a network-bound resolver) would silently re-introduce this class of bug.

To verify the patch works under a simulated slow path:

# Monkey-patch _resolve_chat_argv to sleep 15s, then probe /api/pty.
# Before this patch: connection silently closes (~15s, no response).
# After this patch: 101 flushes immediately, then 15s pause, then chat output.

Related

This was discovered alongside a separate fix being sent as #26834 (proxy_headers=False). Both fixes addressed the same chat-tab-doesn't-load symptom at one deployment behind nginx + Cloudflare. After landing both, the dashboard chat tab works end-to-end through arbitrary reverse-proxy chains.

Tests

The existing tests/hermes_cli/test_web_server.py suite uses Starlette's TestClient which runs pty_ws synchronously in the test event loop — so any sync subprocess call in the handler would block the test runner, not the production event loop. Adding a regression test that monkey-patches _resolve_chat_argv to sleep N seconds and asserts the response time is bounded would lock in this fix. Happy to include in a follow-up.

Discovered by

Diagnosing why chat-tab /api/pty returned 502 at mandy.loadmagic.ai after a separate proxy_headers=True-induced WS 403 fix had already landed. GLM-5.1 (parallel research session) identified the sync-in-async pattern in pty_ws from a source dive on 2026-05-15; the specific subprocess in 0.12.0 (since refactored away) was npm run build triggered by a filename-mismatch in _hermes_ink_bundle_stale. The defence-in-depth wrap is the right systemic fix even after the specific bug is gone.

🤖 Generated with Claude Code

[alt-glitch]

Related: Competing fixes for the same pty_ws event loop blocking issue:

• fix(dashboard): yield event loop after WebSocket accept in pty_ws handler #22873 (open) — asyncio.sleep(0) yield after ws.accept()
• fix(dashboard): resolve chat TUI argv off event loop #26124 (open) — staleness-aware skip for npm rebuild
• fix(cli): run dashboard PTY spawn off the event loop #19943 (closed) — run_in_executor approach (same as this PR)

All target #19914. This PR takes the run_in_executor approach + broader exception handling.

[kshitijk4poor]

Thanks @davidcampbelldc — your diagnosis here is spot-on: resolving the chat TUI argv synchronously in pty_ws blocks the uvicorn event loop while _make_tui_argv does its filesystem/npm work, which is exactly what makes reverse proxies drop the connection.

This overlaps #26124 (by @0xdany), which takes the same off-the-event-loop approach via asyncio.to_thread + an asyncio.Lock to serialize concurrent chat tabs, and was already approved by a maintainer. I've salvaged that one onto current main in #48561 (it had gone stale on a profile param + HTTPException branch added since).

Closing this in favor of #48561 to avoid two competing implementations of the same fix — but your write-up of the 502 / "Empty reply from server" failure mode behind nginx+Cloudflare was a useful confirmation of the root cause. Thank you.