Conversation
- Server: channel-keyed PTY registry with configurable grace-period eviction (HERMES_PTY_RECONNECT_GRACE_SECONDS, default 300s). Same channel reconnecting within grace reattaches the live TUI instead of spawning a fresh one. - Server: PtyBridge maintains a 256 KiB scrollback ring buffer; replayed to the browser on reattach with a [reconnected] marker. - Server: shutdown handler closes all live bridges; concurrent attach attempts on same channel rejected with WS code 4409. - Client: ChatPage persists channel id + last session id in localStorage; resume-target change clears stored channel. - Tests: TestScrollback + TestPtySurvival cover reattach, grace eviction, scrollback replay, concurrent-attach rejection.
cpmidnite
pushed a commit
to cpmidnite/hermes-agent
that referenced
this pull request
May 28, 2026
Collaborator
|
Closing — the keep-alive/reattach approach for dashboard /chat PTYs landed on main in #60515 (salvage of #50084 by @TinkerOfThings, who filed the same fix later with a token-keyed registry + ring-buffer replay + explicit close-code contract). You were the earliest to attack this problem (May 28) — thanks for pushing on it; credit noted in the salvage PR body. The shipped design covers your channel-keyed survival + scrollback-replay goals. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(dashboard): survive browser disconnects in /chat PTY
Problem
The dashboard
/chattab opens a WebSocket to/api/pty, which spawns ahermes --tuichild behind a POSIX pseudo-terminal. When the browser disconnects — laptop sleep, tab loses focus long enough for the WS to time out, network blip, etc. — the WS handler hitsWebSocketDisconnect,finallycallsbridge.close(), and the TUI child gets killed.On reconnect, the front-end opens a fresh WS, the server spawns a brand-new
hermes --tui, and the user's conversation appears lost even though the underlying session is still on disk. Hitting "Resume session" from the sidebar gets back to the session, but xterm.js scrollback is also gone because the front-end always generates a new channel id per mount, forcing a clean re-mount.This is a recurring papercut for anyone using the in-browser dashboard chat across naturally bursty browsing patterns.
Approach
Three coordinated changes — server-side PTY survival + scrollback ring buffer + stable client-side channel id:
1. Server: channel-keyed PTY registry with grace-period eviction
_PTY_BY_CHANNEL: dict[str, _PtySession]keyed by the same channel idpty_wsalready accepts via query param.bridge.close()immediately. Instead they mark the session detached, recordlast_attached_at, and schedule a one-shotasyncio.Task(_evict_after_grace) that closes the bridge afterHERMES_PTY_RECONNECT_GRACE_SECONDS(default300s, capped at3600s) if no reattach happens./quit), evict and spawn fresh.4409+ a one-line ANSI error frame, so a second tab can't hijack the active session.@app.on_event("shutdown")closes all live bridges sohermes dashboardexit doesn't leak children. (We're aware FastAPI deprecateson_eventin favor oflifespan; the rest ofweb_server.pystill useson_eventso this stays consistent — happy to migrate the whole file in a follow-up PR if maintainers prefer.)2. Server: PtyBridge scrollback ring buffer
PtyBridge.__init__takes newscrollback_bytes(default256 * 1024).read()is appended via_append_scrollback()which trims FIFO to the configured limit.snapshot_scrollback()returns the buffer asbytes(safe to call from the event loop thread after the executor-sidereadcompletes; single-writer single-reader on the buffer).pty_wssends\x1b[2J\x1b[H(clear + home), the snapshot bytes, then a grey[reconnected]marker before resuming the live pump. Fresh spawns are unchanged — TUI paints its own opening frame.3. Client: stable channel id + last-session memory
ChatPage.tsxnow persists channel id inlocalStorage["hermes.chat.channel"]so a tab refresh reuses the same channel (and thus the same surviving PTY).?resume=changes (tracked viahermes.chat.channel_resume) — so jumping to a different session lineage still gets a fresh PTY.localStorage["hermes.chat.last_session_id"]; if the user lands on bare/chat(no?resume=), we set the URL param from storage so reload-without-bookmark also resumes.localStorageaccess is wrapped in try/catch — Safari private browsing falls back to ephemeral ids without crashing.[A-Za-z0-9._-]{1,128}regex_VALID_CHANNEL_REenforces server-side; malformed stored values are discarded.What's intentionally NOT in this PR
oncloseloop is a separate feature with its own retry/backoff/auth-refresh edge cases.hermes dashboardprocess restart. Registry is in-process only. Restart still loses live PTYs (TUI children die with the parent).tui_gateway/ Ink / the TUI itself. This PR is purely the PTY plumbing./api/wsJSON-RPC sidebar. That endpoint sits right next topty_wsand was left untouched.Testing
Added
TestScrollback(3 cases) andTestPtySurvival(4 cases):test_scrollback_collects_recent_bytes/test_scrollback_trims_to_limit/test_scrollback_empty_after_spawntest_pty_survival_reattach_same_channel_reuses_pty— connects, disconnects, reconnects with same channel, asserts identical bridge PID.test_pty_grace_evicts_after_timeout— disconnect + sleep past grace, asserts channel evicted and bridge dead. Grace overridden to0.2svia env var to keep test fast.test_pty_reattach_replays_scrollback— asserts the[reconnected]marker plus echoed bytes arrive on reattach.test_pty_second_concurrent_attach_rejected— second concurrent attach gets WS close4409.Tests use
TestClientwith_resolve_chat_argvmonkeypatched to a trivial/bin/shecho loop, so they don't depend on a realhermes --tuibuild. All 4TestPtySurvivaltests pass; all 3TestScrollbacktests pass.Verification run:
Manual smoke-test against the real ASGI app via
TestClient:Files touched
hermes_cli/web_server.pyhermes_cli/pty_bridge.pytests/hermes_cli/test_pty_bridge.pytests/hermes_cli/test_web_server.pyweb/src/pages/ChatPage.tsxCompatibility / config knobs
HERMES_PTY_RECONNECT_GRACE_SECONDS— float seconds, default300, max3600, invalid values fall back to default.localStoragekeys introduced:hermes.chat.channel,hermes.chat.channel_resume,hermes.chat.last_session_id. None collide with existing dashboard storage.hermes --tuiitself. Shutdown handler reaps everything onhermes dashboardexit so it's bounded.Notes for reviewers
on_event→lifespanmigration) if you'd prefer a different shape.@app.on_event("shutdown")is consistent with existing usages in the same file; flagged in case a sweep makes sense as a follow-up.4409WS close code is a stretched-meaning use of HTTP 409 Conflict in the WS code-space (4000-4999 is the application-defined range, RFC 6455). Open to a different code if maintainers have a convention.