Skip to content

Fix dashboard session resume terminal remount - #51529

Closed
X1pheR wants to merge 1 commit into
NousResearch:mainfrom
X1pheR:fix/dashboard-resume-xterm-host
Closed

Fix dashboard session resume terminal remount#51529
X1pheR wants to merge 1 commit into
NousResearch:mainfrom
X1pheR:fix/dashboard-resume-xterm-host

Conversation

@X1pheR

@X1pheR X1pheR commented Jun 23, 2026

Copy link
Copy Markdown

Summary

  • keep dashboard chat on the requested ?resume= target instead of rewriting it after the PTY has already started
  • key the xterm host by PTY identity and clear stale xterm DOM during cleanup before remounting
  • avoid WebGL for embedded dashboard chat because resumed buffers can blank after renderer/layout settling
  • skip sidecar/event sockets when the sidebar is rendered as a model/session rail with showTools=false

Testing

  • npm run typecheck --workspace web
  • npm run build --workspace web
  • npx eslint src/pages/ChatPage.tsx src/components/ChatSidebar.tsx (passes with existing warnings only)
  • manual verification in the dashboard: resuming sessions from /sessions / right rail no longer turns the terminal black after the delayed refresh; text remains visible

@alt-glitch alt-glitch added type/bug Something isn't working comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have labels Jun 23, 2026

@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 dashboard remount race. The current code still has the triggering sequence: web/src/pages/ChatPage.tsx:347-356 asynchronously rewrites ?resume=, while the PTY effect is keyed by channel/resumeParam at web/src/pages/ChatPage.tsx:1168.

Problems

  • Removing the lookup drops the intentional latest-child behavior added in b12a5a72b (Follow latest child session on dashboard resume). The comment says callers should resolve before navigation, but this PR does not add such a caller-side resolution.
  • The showTools branches are stale against current main: ChatSidebarProps has no such prop at web/src/components/ChatSidebar.tsx:73-80, and both ChatPage call sites omit it at web/src/pages/ChatPage.tsx:1364-1369 and 1484-1489.
  • There is no regression coverage for a delayed descendant lookup racing an already-connected PTY.

Suggested changes

  • Preserve descendant canonicalisation before the live terminal mounts, and add a delayed-resolution remount regression test.
  • Rework or omit the stale showTools portion against current ChatSidebar's actual API.

Automated hermes-sweeper review.

@@ -169,6 +170,18 @@ export function ChatSidebar({

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.

Current main no longer declares or passes showTools: ChatSidebarProps is at current web/src/components/ChatSidebar.tsx:73-80, and both ChatPage call sites omit it. This branch must be reworked against the current sidebar API before salvage; a direct cherry-pick will not typecheck.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 15, 2026
@X1pheR X1pheR closed this Aug 3, 2026
@X1pheR
X1pheR deleted the fix/dashboard-resume-xterm-host branch August 3, 2026 15:13
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/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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