Skip to content

fix(dashboard): redraw TUI after PTY reattach - #68071

Closed
algf wants to merge 1 commit into
NousResearch:mainfrom
algf:codex/fix-pty-reattach-redraw
Closed

fix(dashboard): redraw TUI after PTY reattach#68071
algf wants to merge 1 commit into
NousResearch:mainfrom
algf:codex/fix-pty-reattach-redraw

Conversation

@algf

@algf algf commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • request a complete TUI redraw when /api/pty reuses a keep-alive PTY
  • keep first-attach behavior unchanged
  • cover both the PTY attach contract and the WebSocket reattach path

Root cause

The dashboard replays a bounded raw ANSI tail into a new xterm. Hermes TUI uses the alternate screen and differential rendering, so an arbitrary tail is not a self-contained frame. For long sessions, reconnect can therefore restore only the recently changed status/prompt rows and leave the transcript blank.

Verification

  • uv run --extra dev pytest -q tests/test_pty_session.py tests/test_pty_keepalive_ws.py tests/hermes_cli/test_web_server_pty_reconnect.py
  • uv run --extra dev ruff check hermes_cli/pty_session.py hermes_cli/web_server.py tests/test_pty_session.py tests/test_pty_keepalive_ws.py
  • git diff --check

All 21 focused tests pass.

@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 sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: merged #65239 redraws after a fresh dashboard session; this patch addresses the distinct keep-alive PTY reattach path where a bounded ANSI tail cannot restore a full TUI frame.

@Bryntly Bryntly left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PTY force redraw logic looks good and correctly ensures that the TUI repaints after a replay.

However, I noticed an unrelated regression in hermes_cli/web_server.py. In _apply_model_assignment_sync, the not api_key condition and its associated comment were removed:

-        # Fall back to the provider entry's stored key only when the request
-        # didn't carry one — same precedence as the base_url fill above. An
-        # unconditional overwrite silently discards a key the caller is
-        # rotating in, and model.api_key outranks the environment at client
-        # construction (#62269), so the stale key keeps authenticating.
-        if (
-            not api_key
-            and isinstance(provider_entry, dict)
-            and provider_entry.get(api_key)
-        ):
+        if isinstance(provider_entry, dict) and provider_entry.get("api_key"):

This unconditional overwrite discards a new key provided by the caller if the config already has a key saved, reverting the fix for #62269 and breaking key rotation. Please restore the not api_key check and the comment.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #86332 as part of the stale-repaint class fix (TUI focus regain / session resume / dashboard PTY reattach). Your commit was cherry-picked onto current main with your authorship preserved in git log. Thank you!

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) P2 Medium — degraded but workaround exists 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.

4 participants