fix(61284): [Bug]: Dashboard chat sessions fail to render due to recent WebSocket re - #62622
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Looks good! No obvious issues found (reviewed in read-only mode).
Reviewed by Hermes Agent
Code Review Summary\n\nLooks good! No obvious issues found (reviewed in read-only mode).\n\n---\nReviewed by Hermes Agent |
Related to the #61284 fix cluster. This takes a client-side approach (queue |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Looks good! No obvious issues found (reviewed in read-only mode).
Reviewed by Hermes Agent
Code Review Summary\n\nLooks good! No obvious issues found (reviewed in read-only mode).\n\n---\nReviewed by Hermes Agent |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for investigating the dashboard session-switch regression. The current implementation needs rework before it can address the reported path.
Problems
setPtyReady()is added inui-tui/packages/hermes-ink/src/ink/terminal.ts:216, but this PR changes only that file and provides no caller. The newptyReadyflag therefore remains false, causing the addedwriteDiffToTerminal()branch to queue output indefinitely.writeDiffToTerminal()is the normal render path inui-tui/packages/hermes-ink/src/ink/ink.tsx:1134, so this is not dashboard-only.- Current main’s session-switch problem is in keep-alive identity:
web/src/pages/ChatPage.tsx:805sends an attach token, andhermes_cli/web_server.py:15452uses that token alone forPTY_REGISTRY.attach_or_spawn(). The registry returns an alive same-key session athermes_cli/pty_session.py:151-153, regardless of requested resume/profile.
Suggested changes
- Rework this around the PTY registry key (resume + profile + attach token) and add regression coverage next to
tests/test_pty_keepalive_ws.py:7for same-token connections targeting different sessions/profiles. - Avoid adding a shared Ink output gate without a concrete dashboard protocol and a caller that opens it.
Automated hermes-sweeper review.
| let ptyReady = false | ||
| const pendingDiffs: Array<{ terminal: Terminal; diff: Diff; skipSyncMarkers: boolean; onDrain?: () => void }> = [] | ||
|
|
||
| export function setPtyReady(): void { |
There was a problem hiding this comment.
setPtyReady() has no caller in this one-file PR, so ptyReady stays false and every render is queued by the new writeDiffToTerminal() branch. Because this module is used by normal Ink rendering, this suppresses output beyond the dashboard path.
Fixes #61284
Changes
Auto-generated by Hermes Harness — reviewed by AI gate before submission.