fix(chat): namespace PTY attach token by resume target to fix session switching - #61107
fix(chat): namespace PTY attach token by resume target to fix session switching#61107rblundon wants to merge 1 commit into
Conversation
When the user selects a different session from the ChatSessionList, `?resume=<id>` changes and the WebSocket effect re-runs. However, `ptyAttachToken()` was reading a single static localStorage key (`hermes.pty.token.chat`) regardless of which session was being opened. `PTY_REGISTRY.attach_or_spawn()` found the existing PTY alive under that token and reattached to it — completely ignoring the new `resume` param, which is only used when spawning a *new* PTY. The result: clicking any previous session in the Chat tab opened a new terminal in the *old* session's context instead of resuming the selected one. Fix: namespace the localStorage key by `resumeParam` so each session ID gets its own stable attach token. A fresh chat (no resume) continues to use the bare key as before. Cross-browser persistence of the keep-alive identity is preserved within each conversation; switching conversations always spawns a new PTY with the correct resume target. Introduced by: 79f4f78 feat(chat): persist attach token, reconnect on transient close
Duplicate of #61045 (earliest-open, created ~2.5h earlier). Both PRs change only |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the keep-alive reattach path. The reported resume-switch defect is present on current main: ChatPage sends resume but retains one static attach token (web/src/pages/ChatPage.tsx:799-805), and the backend reuses a live PTY solely by that token (hermes_cli/web_server.py:15430-15453).
Problems
- This scopes the browser key by resume target only. A profile switch still sends the same attach token to a registry that ignores
profile;ChatPagepasses profile independently atweb/src/pages/ChatPage.tsx:809. - There is no regression coverage for distinct resume targets. Current coverage proves same-token reuse only in
tests/test_pty_keepalive_ws.py:47-51.
Suggested changes
- Scope the backend registry key by requested resume target and profile, and use that same key for detach.
- Add same-resume reuse plus different-resume and different-profile spawn tests.
This is an automated hermes-sweeper review.
| @@ -710,7 +717,7 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { | |||
| // Keep-alive identity: reattach to this tab's living PTY across | |||
There was a problem hiding this comment.
This scopes the browser token by resume target but not by scopedProfile. The request still sends profile independently, while current pty_ws keys the registry only by attach_token; switching profiles can therefore reattach a PTY running under the previous profile. Scope the server registry key by requested resume and profile (and use it for detach), with a profile-switch regression test.
Summary
Selecting a previous session from the Chat tab's session list spawned a new terminal in the old session's context instead of resuming the selected one.
Root Cause
79f4f78faintroduced PTY keep-alive: the browser stores an attach token inlocalStorage["hermes.pty.token.chat"]andPTY_REGISTRY.attach_or_spawn()reuses the existing PTY process when the same token reconnects.The token key was a single static string regardless of which session was open. When the user clicked a different session:
?resume=<new-id>changed → channel UUID regenerated → WebSocket effect re-ranptyAttachToken(false)returned the same old token from localStorageresumeparam in_resolve_chat_argvis only used when spawning a new PTY — on reattach it's ignored entirelyFix
Namespace the localStorage key by
resumeParamso each session ID gets its own stable PTY attach token:ptyAttachTokennow accepts an optionalresumeKeyand useshermes.pty.token.chat.<resumeKey>as the storage key when non-empty. A fresh chat (no resume param) continues to use the bare key as before.Keep-alive across refresh/transient disconnects is fully preserved — only switching to a different session now correctly spawns a new PTY.
Testing
Affected version
Introduced in
79f4f78fa feat(chat): persist attach token, reconnect on transient close