Skip to content

fix(dashboard): spawn fresh PTY on session switch from sidebar - #65313

Open
coding-linheng wants to merge 1 commit into
NousResearch:mainfrom
coding-linheng:fix/dashboard-session-switch-pty
Open

fix(dashboard): spawn fresh PTY on session switch from sidebar#65313
coding-linheng wants to merge 1 commit into
NousResearch:mainfrom
coding-linheng:fix/dashboard-session-switch-pty

Conversation

@coding-linheng

Copy link
Copy Markdown

Problem

When switching sessions via the dashboard Chat sidebar, the keep-alive PTY registry reuses the existing terminal process because the attach_token (browser localStorage identity) is unchanged. The old PTY is still running the previous conversation, so the user sees stale history and new messages go to the wrong session.

Root Cause

In pty_ws() (web_server.py), the PTY registry key is the bare attach_token. When the user clicks a different session in the sidebar:

  1. Frontend correctly sets ?resume=<new_session_id> in the URL
  2. ChatPage detects the change and triggers a WebSocket reconnect
  3. Server calls PTY_REGISTRY.attach_or_spawn(attach_token, spawn=_spawn)
  4. The registry finds an existing live PTY with the same token and returns it
  5. The old PTY (still running the previous session) is reused — wrong conversation

The _spawn closure (which would create a new PTY with --resume <new_id>) is never called.

Fix

Include the resume session id in the PTY registry key (attach_token:resume_id) so each unique (session, tab) pair gets its own PTY. Switching back to a previously visited session reattaches to its still-alive PTY (correct behavior). The idle reaper continues to clean up detached sessions after TTL.

Testing

  1. Open dashboard Chat tab
  2. Start a conversation (session A)
  3. Click a different session in the sidebar (session B)
  4. Verify: message history shows session B, new messages go to session B
  5. Switch back to session A
  6. Verify: message history shows session A, terminal state is preserved

When switching sessions via the dashboard Chat sidebar, the keep-alive
PTY registry reused the existing terminal process because the
`attach_token` (browser-localStorage identity) was unchanged. The old
PTY was still running the previous conversation, so the user saw stale
history and new messages went to the wrong session.

Include the `resume` session id in the registry key so each unique
(session, tab) pair gets its own PTY. Switching back to a previously
visited session reattaches to its still-alive PTY (correct behavior).
The idle reaper continues to clean up detached sessions after TTL.

Fixes: session switch from sidebar shows wrong conversation history
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Jul 16, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #60745: both scope keep-alive PTY identity to prevent stale-session reattachment. This variant keys only by resume; #60745 also covers profile identity, so maintainers should choose or consolidate the broader approach.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the registry choke point; current main confirms the reported stale-PTY reuse at hermes_cli/web_server.py:15870-15871 and hermes_cli/pty_session.py:151-153.

Problems

  • The proposed key uses resume after pty_ws() may replace an absent query value from the active-session file (hermes_cli/web_server.py:15818-15824). A default /chat connection first keys as attach:; after the PTY writes its session ID, a refresh keys as attach:<id>, spawning a second PTY and defeating keep-alive.
  • The key omits profile. ChatPage sends profile with the same attach token (web/src/pages/ChatPage.tsx:910-915), while the server uses it to set the child HERMES_HOME (hermes_cli/web_server.py:14941-14954), so profile changes can still reattach the wrong process. This matches the broader scope noted in the #60745 discussion.
  • No regression tests accompany the change; current tests/test_pty_keepalive_ws.py:7-51 only checks same-token reuse.

Suggested changes

  • Capture raw query resume before the fallback, include profile in the identity, and add explicit-switch, default-refresh, profile-isolation, and canonical-parent-to-child coverage.

Automated hermes-sweeper review.

Comment thread hermes_cli/web_server.py
# Registry key includes the resume target so that switching sessions
# in the dashboard sidebar spawns a fresh PTY instead of reattaching
# to the old one (which is still running the previous conversation).
_registry_key = f"{attach_token}:{resume or ''}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resume has already been rewritten by the active-session-file fallback before this point (web_server.py:15818-15824), so a default-chat refresh changes from attach: to attach:<session-id> and spawns a second PTY. Capture raw query resume before that fallback, include profile (which scopes the child HERMES_HOME), and add regressions for default refresh, profile isolation, and explicit resume switching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants