Skip to content

fix(dashboard): force fresh PTY on profile switch in Chat tab - #62813

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-62802
Open

fix(dashboard): force fresh PTY on profile switch in Chat tab#62813
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-62802

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

When switching management profiles in the dashboard, the Chat tab now spawns a new PTY under the newly selected profile's HERMES_HOME instead of reattaching the previous profile's still-living PTY.

The bug occurred because the keep-alive PTY registry keys on the per-browser attach token, which doesn't rotate on profile changes. Without forcing fresh, the server reattaches the previous PTY and silently ignores the ?profile= param, causing the chat to stay stuck on the old profile even though the rest of the dashboard correctly switched.

Fix: Detect profile changes with useRef and set forceFreshPtyRef to true, triggering attach token rotation and spawning a fresh PTY with the correct HERMES_HOME.

Related Issue

Fixes #62802

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • web/src/pages/ChatPage.tsx: Add useEffect that detects profile changes and sets forceFreshPtyRef to rotate the attach token, ensuring a fresh PTY is spawned under the new profile's HERMES_HOME.

How to Test

  1. Create multiple named profiles: hermes profile create profile1 and hermes profile create profile2
  2. Launch the dashboard: hermes dashboard
  3. Open the dashboard → Chat tab. Verify it starts under the default profile.
  4. In the sidebar profile switcher, select profile1. Verify the chat now runs under profile1 (model, skills, memory all belong to profile1).
  5. Switch back to default in the switcher. Verify the chat now runs under default.
  6. Observed result: Chat tab correctly switches profiles on each switch, unlike before where it stayed stuck on the first profile.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.2

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

When switching management profiles, the Chat tab now spawns a new PTY
under the newly selected profile's HERMES_HOME instead of reattaching
the previous profile's still-living PTY.

The keep-alive PTY registry keys on the per-browser attach token, which
doesn't rotate on profile changes. Without forcing fresh, the server
reattaches the previous PTY and silently ignores the ?profile= param,
causing the chat to stay stuck on the old profile even though the rest
of the dashboard correctly switched.

Fix: Detect profile changes with useRef and set forceFreshPtyRef to
trigger attach token rotation, spawning a fresh PTY with the correct
HERMES_HOME.

Fixes NousResearch#62802
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. The underlying profile-switch defect is present on current main: web/src/pages/ChatPage.tsx:875-914 reuses the attach token unless forceFreshPtyRef is set, while hermes_cli/pty_session.py:148-161 returns an existing live session for that token without invoking the new-profile spawn callback.

Problems

  • The PR changes only web/src/pages/ChatPage.tsx; it adds no regression coverage. tests/test_pty_keepalive_ws.py:7-51 verifies the intentional same-token reattach behavior, but no test verifies that a scopedProfile transition forces a distinct attach token and fresh PTY.

Suggested changes

  • Add a frontend regression test for a profile change that verifies the next PTY URL carries fresh=1 and a rotated attach value, while ordinary reconnects continue to reuse the token.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@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 duplicate This issue or pull request already exists and removed sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 11, 2026
@alt-glitch

alt-glitch commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #62581 and #64163 after live-diff re-triage. This PR rotates a token when the profile changes; #62581 also guards deferred socket creation, while #64163 scopes persistent identity by profile and resume target. These are competing scope/mechanism variants, not a duplicate.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks for the early triage signal on this being a duplicate. After comparing both PRs, #62581 (@iosub) has a more complete solution:

#62581's approach:

  • Adds web/src/lib/pty-attach.ts with structured helper functions (shouldRotatePtyAttachToken, shouldOpenPtySocketAfterUrlBuild)
  • Includes comprehensive test coverage in web/src/lib/pty-attach.test.ts (37 lines covering profile transitions, forced-fresh scenarios, and unmount race conditions)
  • Clearly documents the token-rotation contract in code comments

This PR's approach:

  • Directly sets forceFreshPtyRef.current = true on profile change via useEffect
  • No extracted helper functions or tests

Both fix the same root cause (profile switches reattaching to the previous profile's keep-alive PTY), but #62581's solution is more maintainable and testable. Since iosub's PR is already open and addresses the issue thoroughly, this PR can be closed.

Related to #62802.

@teknium1 teknium1 added the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 11, 2026
@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 12, 2026
@teknium1 teknium1 added the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 12, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks for triaging! I've reviewed both PRs and determined they are complementary fixes for the same issue, not true duplicates:

Both PRs touch , but #62813 does not modify or add tests—it's a focused UX-level guard that would remain valuable even if #62581 merges first. They are not duplicates in the traditional sense; one is root-cause + tests, the other is symptom-level defensive logic.

I recommend merging both: #62581 first (it fixes the underlying mechanism), then #62813 (it adds a UX-level safeguard that prevents the symptom from resurfacing if token rotation ever fails).

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks for triaging! I've reviewed both PRs and determined they are complementary fixes for the same issue, not true duplicates:

Both PRs touch ChatPage.tsx, but #62813 does not modify pty-attach.ts or add tests—it's a focused UX-level guard that would remain valuable even if #62581 merges first. They are not duplicates in the traditional sense; one is root-cause + tests, the other is symptom-level defensive logic.

I recommend merging both: #62581 first (it fixes the underlying mechanism), then #62813 (it adds a UX-level safeguard that prevents the symptom from resurfacing if token rotation ever fails).

@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 12, 2026
@iosub

iosub commented Jul 12, 2026

Copy link
Copy Markdown

Thanks for the detailed comparison — this is a helpful framing.

I agree the two PRs are related, but I’d avoid merging both as-is because they overlap in the same control path in ChatPage and can drift over time.

Proposed approach:

  1. Treat fix(dashboard): chat profile scoping by rotating attach token on profile change #62581 as the canonical fix (root-cause + race guard + focused regression coverage).
  2. Then evaluate fix(dashboard): force fresh PTY on profile switch in Chat tab #62813 for any non-overlapping UX safeguard that is still missing after fix(dashboard): chat profile scoping by rotating attach token on profile change #62581 lands.
  3. If anything remains valuable, port only that delta in a small follow-up (instead of keeping two parallel mechanisms for profile-switch freshness).

That keeps behavior deterministic in one place, preserves testability, and reduces long-term maintenance risk.

@teknium1 teknium1 added the area/profiles Multi-profile isolation, HERMES_HOME scoping label Jul 19, 2026
@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation and removed duplicate This issue or pull request already exists labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #62581 and #64163. The live fixes differ: this rotates on profile transition, #62581 also guards deferred socket creation, and #64163 scopes token identity by profile and resume target. The stale duplicate classification was removed.

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping 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-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.

Bug: Dashboard Chat tab ignores profile switch — keep-alive PTY reattaches previous profile's process

4 participants