Skip to content

fix(dashboard): force fresh PTY spawn when resume target changes - #64444

Open
daniloneto wants to merge 2 commits into
NousResearch:mainfrom
daniloneto:fix/web-dashboard-pty-session-resume-reattach
Open

fix(dashboard): force fresh PTY spawn when resume target changes#64444
daniloneto wants to merge 2 commits into
NousResearch:mainfrom
daniloneto:fix/web-dashboard-pty-session-resume-reattach

Conversation

@daniloneto

Copy link
Copy Markdown

Bug

When resuming a session in the web dashboard (sidebar click or Sessions → History → "Resume in Chat"), every resume-after-the-first silently lands on the original session's PTY regardless of which session was clicked. Only the first session after a dashboard restart works correctly.

Root cause: the PTY keep-alive registry (PtySessionRegistry.attach_or_spawn) returns the existing session when the same attach_token is presented. The old session was spawned with HERMES_TUI_RESUME pointing at the first session, so every subsequent reattach ignores the new ?resume= parameter in the WebSocket URL.

Fix (3 files, 3 parts)

1. hermes_cli/pty_session.pyclose_if_exists

New method on PtySessionRegistry that closes and removes a session by key. Allows the keep-alive path to discard a stale PTY before attach_or_spawn reuses it.

2. hermes_cli/web_server.py — Two guards in pty_ws

  • force_fresh guard (line 15718): Restrict resume = None to only fire when there is no explicit ?resume= param. Without this, a sidebar click sending ?resume=TARGET_ID&fresh=1 has its resume target silently dropped by force_fresh.

  • close_if_exists call (line 15773): When resume is non-None, close any existing PTY session in the registry before calling attach_or_spawn. This forces a fresh PTY spawn with the updated argv/env (containing the correct HERMES_TUI_RESUME).

3. web/src/pages/ChatPage.tsx — Resume param tracking

Track the previous resumeParam via a ref. When it changes (user clicks a different session in the sidebar), set forceFreshPtyRef.current = true so the next WebSocket connection carries fresh=1.

Testing

  1. Start the dashboard and open /chat (creates keep-alive PTY)
  2. Navigate to Sessions → History and click "Resume in Chat" on session A → session A loads ✅
  3. Go back and click "Resume in Chat" on session B → session B loads (was broken - landed on A) ✅
  4. Reload the page with ?resume=SESSION_B → session B loads ✅
  5. Click a session in the sidebar → that session loads ✅

Closes #63701.

Three-part fix for dashboard session resume after keep-alive PTY:

1. **Backend (pty_session.py)**: Add  method to
   PtySessionRegistry — discards an existing PTY session so
    is forced to create a new one with updated
   argv/env.

2. **Backend (web_server.py)**: Two changes in :
   - Guard  behind  inside the
      block — prevents  from dropping an
     explicit resume target ( from
     sidebar click).
   - Call  when
      is set — ensures the registry spawns a fresh PTY
     with the new  instead of reattaching the
     stale session.

3. **Frontend (ChatPage.tsx)**: Track previous  via
   ref and set  when the sidebar switches to a
   different session, so the WebSocket connection carries
   .

Without (1)+(2), every resume-after-the-first silently lands on
the original session's PTY regardless of which session was clicked
in the sidebar — the PTY registry returns the existing session
(whose  env var points at the first session)
and the new argv is never used.

Closes NousResearch#63701.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 14, 2026
@alt-glitch

alt-glitch commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this PR now uses target-aware PTY identity (resume session + profile) so same-target reconnects preserve the live PTY while changed targets respawn it. It remains complementary to #61313 and #62058, and related to the closed earlier teardown variant #63731; its separate dashboard-backup argument fix is bundled and needs maintainer review.

@daniloneto

Copy link
Copy Markdown
Author

All three are needed — the PR combines two orthogonal bugs under one label:

Frontend-only approaches (#61313 force-fresh, #62058 token rotation, #61107 namespacing) fix session switch in PTY registry but do not fix the server-side ?resume=TARGET_ID&fresh=1 bug: pty_ws unconditionally sets resume = None when fresh=1, so the resume target is dropped before attach_or_spawn even runs. No frontend token trick helps if the server discards the param first.

Backend-only (close_if_exists, #63731 repo) fixes the PTY registry stale session but does not fix the frontend failing to send fresh=1 when the sidebar switches sessions — on page reload forceFreshPtyRef is false (new mount), the same attach token is reused, and the stale PTY is returned.

This PR: frontend sends fresh=1 on session switch + server doesn't drop explicit resume when fresh=1 + server closes stale PTY before spawn. Covers both code paths (page reload and sidebar click) with no gap.

Would be happy to drop any part that the maintainer considers redundant once they pick a direction — just explaining why they're together here.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the attach-token/root-environment mismatch; the current-main premise is real: PtySessionRegistry.attach_or_spawn() returns a live same-key session without executing the new spawn closure (hermes_cli/pty_session.py:148-166).

Problems

  • The proposed close_if_exists condition is based on resume is not None, rather than a changed target. ChatPage's transient-drop path intentionally reconnects with the same attach token to preserve the living PTY (web/src/pages/ChatPage.tsx:1051-1055); because resumeParam remains in the URL, this change would terminate and respawn that PTY instead of reattaching it.
  • There is no regression coverage for target isolation or resumed reconnects. Current keep-alive coverage asserts only same-token reuse (tests/test_pty_keepalive_ws.py:7-52).
  • The replacement if not resume: resume = None is a no-op after removing the old assignment.

Suggested changes

  • Make PTY identity target-aware (including profile), or track the spawned target and tear down only when it changes; retain same-target reattachment.
  • Add tests for same-target reuse, changed-target spawning, explicit resume with fresh=1, and profile isolation.
  • Remove the no-op branch while retaining the active-session-file invalidation.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 16, 2026
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 16, 2026
@alt-glitch alt-glitch removed sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 16, 2026
The old code called PTY_REGISTRY.close_if_exists(attach_token) whenever
resume was not None. This broke transient-drop reconnects: the browser
reconnects with the same ?attach= AND same ?resume= (they stay in the URL),
but the unconditional close killed the PTY on every WS reconnect, forcing
a pointless respawn.

Fix moves the teardown decision into attach_or_spawn via a target_id param.
- attach_or_spawn stores the target on PtySession and compares it on reuse
- Same attach_key + same target_id → reattach (transient drop, refresh)
- Same attach_key + different target_id → close old, spawn fresh (sidebar
  session switch, profile change)
- No target_id → backward-compat reattach

Also removes the dead 'if not resume: resume = None' branch.
@daniloneto

Copy link
Copy Markdown
Author

Target-aware PTY identity (replaces close_if_exists approach)

The previous approach called PTY_REGISTRY.close_if_exists(attach_token) whenever resume is not None. This broke transient-drop reconnects: the browser reconnects with the same ?attach= + ?resume= after a brief transport loss, but the unconditional kill respawned the PTY instead of reattaching.

Fix

Moves the teardown decision inside attach_or_spawn via a new target_id parameter:

  • PtySession stores the target_id it was spawned with (pty_session.py:46)
  • attach_or_spawn compares the new target_id against the existing sessions. If they differ → close old, spawn fresh. Same/no target_id → reattach (pty_session.py:155-162)
  • web_server.py passes a composite target_id = f"{resume}|{profile}" instead of calling close_if_exists beforehand (web_server.py:15772-15785)
  • Removed dead branch if not resume: resume = None

Target identity includes both resume session and profile — switching sessions or profiles via the dashboard triggers a fresh PTY; transient drops and page refreshes reattach to the living one.

Tests

  • test_same_target_reattaches — same key + same target_id → reattach, no spawn
  • test_different_target_respawns — same key, different target_id → close + respawn
  • test_target_id_none_reattaches — no target_id → backward-compat reattach
  • test_target_id_isolates_by_profile — same resume, different profile → respawn
  • test_attach_token_with_resume_reattaches_same_session — WS-level: same ?attach= + same ?resume= → reattach (the transient-drop scenario)

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

Labels

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-broad Sweeper blast radius: broad — a core path most sessions hit 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.

**Web dashboard: clicking a previous session opens a blank new session (/api/pty ?attach= reattach discards the ?resume= spawn env)**

3 participants