fix(cli): don't replay transcript on the session's first benign SIGWINCH - #65444
fix(cli): don't replay transcript on the session's first benign SIGWINCH#65444halaprix wants to merge 1 commit into
Conversation
The resize recovery treated the first SIGWINCH of a session as a width change (no prior width to compare against), running the Ctrl+L-style viewport clear + _OUTPUT_HISTORY replay. The 2J clear preserves scrollback, so everything in the deque printed a second copy below the still-visible original. After --continue/--resume the deque holds the whole "Previous Conversation" recap plus the first live exchange, so a benign resize signal (GNOME Terminal tab bar appearing, monitor-scale change, focus events) duplicated the entire conversation. Seed the width baseline when the resize hook is installed, and replay only on an observed width change. The baseline is read from app.output — get_app() at install time is still the DummyApplication whose DummyOutput reports a fake 80 columns, which would turn the first real signal back into a phantom width change. A real initial maximize or restore still differs from the seeded width and is still recovered (NousResearch#49120 behavior preserved; verified in a pty harness both ways). Fixes NousResearch#65293
alt-glitch
added
type/bug
|
Thanks for tracing the replay path rather than changing prompt_toolkit behavior. Current The changed test coverage exercises the first-signal no-replay case, real width changes, direct Automated hermes-sweeper review. |
teknium1
added
sweeper:blast-moderate
SummarySixteen PRs address or provide implementation context for this resume and terminal-rendering complex. #65444 directly targets #65293’s demonstrated cause—the first benign SIGWINCH triggering a viewport clear and replay of resumed history—while the other diffs cover persistence, resume selection or output, exit chrome, resize recovery, and interrupt recovery. Related pull requests
Duplicates#3183 and #3225 overlap on #3123, with the applicable work represented by merged #3315; #11868 is superseded by #31939, #33649 by #45586, #34064 by #54058, and #34588 by #34834. #59729 and #65444 overlap in resize recovery but are not duplicates: #59729 adds status-bar/configuration policy, while #65444 fixes the first-signal baseline error. Suggested consolidationKeep #65444 open with a salvage path: mechanically reapply its startup width baseline, observed-change predicate, and focused regression tests onto current main, as supported by both the recorded best-fix verdict and the maintainer-bot keep_open review. Keep #59729 separate for its persisted-statusbar correction and optional scrollback-rebuild work, with the irreversible CSI 3J effect made explicit; the already-closed precursors remain superseded by #3315, #31939, #45586, #54058, and #34834. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I65293(["issue #65293 (open)"])
P65444["PR #65444 (open)"]
P65444 -->|best fix| I65293
class I65293 open
class P65444 open
class P65444 best
class P65444 target
click I65293 "https://github.com/NousResearch/hermes-agent/issues/65293"
click P65444 "https://github.com/NousResearch/hermes-agent/pull/65444"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 16 pull requests and 7 issues in this complex. Each diff was read against this issue; Assessment working set: 130 kB of PR diffs, 61 kB of issue/PR text, 17 kB of discussion (21 comments), 22 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Merged via PR #86260 as part of the duplicated-CLI-chrome class fix. Your commit was cherry-picked onto current main with your authorship preserved in git log. Thank you! |
4 participants
What
Stops the classic CLI from duplicating the entire resumed conversation after the session's first benign SIGWINCH, fixing #65293.
cli.py—_recover_after_resizenow replays the transcript only on an observed width change (prev_width is not None and new_width != prev_width); the hook installation is extracted into_install_resize_recovery(app), which seeds the width baseline at startup.tests/cli/test_cli_force_redraw.py— newTestFirstSigwinchBaselineclass: first-signal-same-width does not replay; real change after baseline still replays; baseline seeding (incl. the DummyOutput trap below); probe-failure fallbacks.Why — the actual mechanism (differs from the issue's suspicion)
The reporter suspected prompt_toolkit's renderer buffer. The real chain is Hermes's own resize recovery:
--continue/--resumeseeds the "Previous Conversation" panel into_OUTPUT_HISTORY(cli_agent_setup_mixin.py:688) — by design, it's what Ctrl+L replays. The first live exchange is appended too (_cprint→_record_output_history)._recover_after_resizetreated the session's first SIGWINCH as a width change ("no prior width to compare against; treat it as a change"), running the Ctrl+L-style recovery: CSI 2J viewport clear + full_replay_output_history().Reproduced deterministically in a pty harness (fixed-size pty, mock OpenAI endpoint, seeded 112-message session): a clean send produces no duplication; injecting one SIGWINCH with unchanged dimensions produced CSI 2J 4 bytes later and a byte-exact second copy of the recap + exchange.
The fix — and a subtle trap
Seed
_last_resize_widthwhen the resize hook is installed, and require an observed change to replay. One trap made the obvious version of this fix silently wrong: the baseline must be read fromapp.output, not_get_tui_terminal_width()— at install time (beforeapp.run())get_app()still returns prompt_toolkit'sDummyApplication, whoseDummyOutput.get_size()reports a hardcoded 80 columns. Seeding that fake width turns the first real signal back into a phantom "width change".app.outputis the same object the running app's resize handler measures, so install-time and signal-time widths are comparable.A genuine initial maximize/restore still differs from the seeded baseline and is still recovered — the #49120 duplicated-status-bar fix is fully preserved.
How to test
Automated:
End-to-end (performed, pty harness with a mock model and a resumed 112-message session):
Manual repro from the issue: resume a 50+ message session in GNOME Terminal, send a message, toggle fullscreen or open a second tab (any benign WINCH) — previously duplicated everything; now stays clean.
Ctrl+Lbehavior is untouched.Known remaining trade-off (pre-existing, out of scope): on a real width change the replay still re-paints content that survives in scrollback — that's the deliberate #49120 trade-off and matches Ctrl+L semantics.
Platforms tested
Linux (x86_64), GNOME-class terminal semantics simulated via pty. Change is pure Python signal/width bookkeeping — no platform-specific I/O.
Fixes #65293