fix(dashboard): key keep-alive PTY on resume+profile, not just tab token (#61284) - #61320
fix(dashboard): key keep-alive PTY on resume+profile, not just tab token (#61284)#61320kyssta-exe wants to merge 1 commit into
Conversation
When switching chat sessions in the dashboard, the PTY registry was keyed only on the per-tab attach_token, which is stable across session switches. attach_or_spawn() found the existing PTY from session A and reattached instead of spawning a new PTY for session B. Fix: fold resume and profile into the registry key so each distinct session/profile combination gets its own PTY. Keep-alive is preserved: - Same attach_token + same resume → reattach (browser refresh) - Same attach_token + different resume → spawn new PTY (session switch) Fixes NousResearch#61284
7a0f48e to
43ebb84
Compare
Duplicate of #60745 — both are OPEN, both touch only |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused backend fix. The wrong-session premise is present on current main: hermes_cli/web_server.py:15452-15454 calls the registry with only attach_token, and hermes_cli/pty_session.py:151-153 returns an existing live PTY without spawning the newly resolved argv.
Problems
- The new key derives from
resumeafter the default-chat active-session fallback athermes_cli/web_server.py:15405-15406. A first default-chat connection can use an empty resume key, while a refresh uses the persisted session ID and creates a second PTY rather than reattaching. - The PR adds no regression coverage beyond current main's same-token test in
tests/test_pty_keepalive_ws.py:7-52.
Suggested changes
- Capture the raw query
resumebefore fallback and use that raw value, plusprofile, for the registry identity. - Add resume-switch, profile-switch, and default-chat-after-fallback regression tests. Open PR #60745 (
1a5680ca) already demonstrates that shape and coverage.
Automated hermes-sweeper review.
| return | ||
|
|
||
| # Keep-alive path: the PTY outlives this socket; reattach by token. | ||
| # Key on attach_token + resume + profile so switching to a different |
There was a problem hiding this comment.
Use the raw query resume captured before active-session-file fallback here. On default /chat, that fallback can change resume from empty to the persisted session ID between refreshes, causing this key to change and the live keep-alive PTY to be replaced.
|
Stale — no merge activity for 4-6 days. Can resubmit if still needed. |
Summary
Fixes #61284 — Dashboard "Silent WebSocket" on Session Switch.
Root cause
The keep-alive PTY registry (
PtySessionRegistry) was keyed only on the per-tabattach_token, which is stable across session switches. When the user selects a different chat session:attach_tokenis sentattach_or_spawn()finds the existing PTY running session A and reattaches--resume Bis never spawnedThe
code=1005 / messages=1log line in the issue is benign teardown of the superseded socket, not the root cause.Fix
Fold
resume(which session to resume) andprofileinto the registry key using NUL-byte concatenation:attach_token+ sameresume→ reattach (browser refresh preserves keep-alive)attach_token+ differentresume→ spawn new PTY (session switch)profileis also included for the same latent bug with profile switchingThe detach call is updated to use the same compound key.
Verification
max_sessions+ reaper