Skip to content

fix(dashboard): scope PTY attach token by resume session - #67217

Closed
thelonewander3r wants to merge 1 commit into
NousResearch:mainfrom
thelonewander3r:feature/dashboard-session-switch-pty
Closed

fix(dashboard): scope PTY attach token by resume session#67217
thelonewander3r wants to merge 1 commit into
NousResearch:mainfrom
thelonewander3r:feature/dashboard-session-switch-pty

Conversation

@thelonewander3r

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • On WSL dashboard (hermes dashboard/chat): create chat A with history → New chat B → New chat C → click A in the sidebar → A's transcript loads
  • Hard-refresh after deploying the rebuilt web_dist so the new bundle is used
  • Refresh while staying on the same ?resume= session still reattaches keep-alive (same scoped token)
  • Profile-scoped chat (?profile=) keeps a separate attach token from the default profile
  • Existing /api/pty keepalive tests still pass: scripts/run_tests.sh tests/test_pty_keepalive_ws.py -q

Keep-alive used one localStorage token for every /chat tab, so New chat
then clicking an older session could reattach the wrong living PTY. Scope
the token by resume id and profile so sidebar switches load the matching
transcript.
@alt-glitch alt-glitch added type/bug Something isn't working 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 needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #64154, #64163, and #61045. This is the clean profile+resume frontend scope; the open alternatives retain broader lifecycle work or resume-only semantics.

@thelonewander3r

Copy link
Copy Markdown
Contributor Author

After re-review against current main and #60745: closing this PR.

Backend #60745 already scopes the keep-alive PTY registry by canonical resume + profile, with regression coverage for session switches and parent→descendant reattach (test_attach_token_reuses_canonical_resume). Frontend scoping by the raw resumeParam localStorage key is redundant on that base and can mint distinct attach tokens for parent vs child before the server canonicalizes them, which risks duplicate PTYs for one conversation.

Our local A→B→C→A “fix” was likely from rebuilding web_dist / hard-refresh rather than this hunk being the real attach-key fix. Also dropping the incorrect #60772 reference — that issue is unrelated (Windows Desktop readiness parser).

If #64425 / #63701 still reproduce on current main without this change, they need a different root-cause investigation (e.g. descendant/subagent walk or history render), not another attach-token layer. Leaving those issues for verification rather than claiming Fixes here.

Sorry for the noise — thanks for the triage note linking #61045 / related work.

@thelonewander3r

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #60745; see comment above.

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

Labels

comp/dashboard Web dashboard / control panel UI (dashboard/, landing) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists 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.

2 participants