From a22fbba340d990879480a729a01d0904258ac599 Mon Sep 17 00:00:00 2001 From: halaprix Date: Thu, 16 Jul 2026 09:30:49 +0200 Subject: [PATCH] fix(cli): don't replay transcript on the session's first benign SIGWINCH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (#49120 behavior preserved; verified in a pty harness both ways). Fixes #65293 --- cli.py | 64 +++++++++++++--- tests/cli/test_cli_force_redraw.py | 116 +++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 9 deletions(-) diff --git a/cli.py b/cli.py index 9b065001f0..9a05655502 100644 --- a/cli.py +++ b/cli.py @@ -5121,9 +5121,21 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): except Exception: new_width = None prev_width = getattr(self, "_last_resize_width", None) - # First resize of the session has no prior width to compare against; - # treat it as a change so an initial maximize/restore is covered too. - width_changed = new_width is not None and new_width != prev_width + # Replay only on an OBSERVED width change. The first signal of a + # session must not count as one (#65293): GNOME Terminal and friends + # deliver benign SIGWINCHes (tab bar appearing, monitor-scale change, + # focus events), and a 2J+replay against preserved scrollback + # duplicates everything ``_OUTPUT_HISTORY`` holds — after a resume + # that is the entire "Previous Conversation" recap plus the first + # live exchange. ``_install_resize_recovery`` seeds the baseline at + # startup, so an initial maximize/restore still differs from it and + # is still recovered; with no baseline (width probe failed) this + # signal just records one for the next comparison. + width_changed = ( + new_width is not None + and prev_width is not None + and new_width != prev_width + ) if width_changed: try: self._clear_prompt_toolkit_screen(app, rebuild_scrollback=False) @@ -5222,6 +5234,45 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): self._resize_recovery_pending = False self._recover_after_resize(app, original_on_resize) + def _install_resize_recovery(self, app) -> None: + """Route prompt_toolkit's ``_on_resize`` through the debounced + ghost-clearing recovery (#5474/#49120) and record the current terminal + width as the baseline for width-change detection. + + Seeding the baseline here is what keeps the session's FIRST SIGWINCH + honest (#65293): ``_recover_after_resize`` replays the transcript only + on an observed width change, and without a startup baseline it could + not tell a benign signal (GNOME Terminal tab bar, monitor-scale + change) from a real one. An initial maximize/restore still differs + from the seeded width, so it is still recovered. + + The probe reads ``app.output`` directly — NOT + ``_get_tui_terminal_width`` — because this runs before ``app.run()``, + when ``get_app()`` still returns prompt_toolkit's DummyApplication + whose DummyOutput reports a hardcoded 80 columns; seeding that fake + width would make the first real signal look like a width change and + resurrect the duplicate-replay bug this exists to fix. + ``app.output`` is the same object the running app's resize handler + measures, so install-time and signal-time widths are comparable. + """ + width = None + try: + width = app.output.get_size().columns + except Exception: + width = None + if not width or width <= 0: + try: + width = shutil.get_terminal_size((80, 24)).columns + except Exception: + width = None + self._last_resize_width = width + original_on_resize = app._on_resize + + def _resize_clear_ghosts(): + self._schedule_resize_recovery(app, original_on_resize) + + app._on_resize = _resize_clear_ghosts + def _status_bar_context_style(self, percent_used: Optional[int]) -> str: if percent_used is None: return "class:status-bar-dim" @@ -18036,12 +18087,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): # don't permanently freeze the input (issue #16263). Idempotent. _apply_bracketed_paste_timeout_patch() - _original_on_resize = app._on_resize - - def _resize_clear_ghosts(): - self._schedule_resize_recovery(app, _original_on_resize) - - app._on_resize = _resize_clear_ghosts + self._install_resize_recovery(app) def spinner_loop(): while not self._should_exit: diff --git a/tests/cli/test_cli_force_redraw.py b/tests/cli/test_cli_force_redraw.py index d3ea248e12..5e4dd1201e 100644 --- a/tests/cli/test_cli_force_redraw.py +++ b/tests/cli/test_cli_force_redraw.py @@ -142,3 +142,119 @@ class TestForceFullRedraw: bare_cli._app = app bare_cli._force_full_redraw() # must not raise + + +class TestFirstSigwinchBaseline: + """Bug #65293: the session's FIRST SIGWINCH used to be force-treated as a + width change (no prior width to compare against), so a benign resize + signal — GNOME Terminal tab bar appearing, monitor-scale change, focus + events — cleared the viewport and replayed ``_OUTPUT_HISTORY``. After a + resume that deque holds the whole "Previous Conversation" recap plus the + first live exchange, so everything reprinted as a duplicate. A replay + must require an OBSERVED width change against a recorded baseline. + """ + + def test_first_sigwinch_with_unchanged_width_does_not_replay( + self, bare_cli, monkeypatch + ): + app = MagicMock() + events = [] + app.renderer.output.erase_screen.side_effect = lambda: events.append("erase") + original_on_resize = lambda: events.append("original_resize") + + bare_cli._status_bar_suppressed_after_resize = False + # No baseline recorded yet — the pre-fix code forced width_changed=True. + assert getattr(bare_cli, "_last_resize_width", None) is None + monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 120) + monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) + monkeypatch.setattr( + cli_mod, "_replay_output_history", lambda: events.append("replay") + ) + + bare_cli._recover_after_resize(app, original_on_resize) + + # Width did not change — no clear, no replay, straight to prompt_toolkit. + assert events == ["original_resize"] + app.renderer.output.erase_screen.assert_not_called() + # The signal still records the baseline for the next comparison. + assert bare_cli._last_resize_width == 120 + + def test_real_width_change_after_baseline_still_replays( + self, bare_cli, monkeypatch + ): + """The #49120 recovery (2J + replay) must still fire on a real change.""" + app = MagicMock() + events = [] + app.renderer.output.erase_screen.side_effect = lambda: events.append("erase") + original_on_resize = lambda: events.append("original_resize") + + bare_cli._status_bar_suppressed_after_resize = False + bare_cli._last_resize_width = 120 + monkeypatch.setattr(bare_cli, "_get_tui_terminal_width", lambda: 90) + monkeypatch.setattr(bare_cli, "_schedule_status_bar_unsuppress", lambda *_: None) + monkeypatch.setattr( + cli_mod, "_replay_output_history", lambda: events.append("replay") + ) + + bare_cli._recover_after_resize(app, original_on_resize) + + assert "erase" in events and "replay" in events + assert bare_cli._last_resize_width == 90 + + def test_install_resize_recovery_seeds_width_baseline(self, bare_cli): + """Hook installation records the CURRENT width as the baseline, so an + initial maximize/restore (a real change vs that baseline) is still + recovered while a same-size first signal is not. + + The baseline must come from ``app.output`` — the same object the + running app measures on SIGWINCH — not from ``get_app()``, which + before ``app.run()`` is a DummyApplication reporting a fake 80 cols. + """ + app = MagicMock() + app.output.get_size.return_value.columns = 132 + scheduled = [] + bare_cli._schedule_resize_recovery = lambda *a, **k: scheduled.append(a) + + original = app._on_resize + bare_cli._install_resize_recovery(app) + + assert bare_cli._last_resize_width == 132 + assert app._on_resize is not original # hook installed + app._on_resize() # simulated SIGWINCH → routes to the debouncer + assert len(scheduled) == 1 + assert scheduled[0][0] is app + assert scheduled[0][1] is original + + def test_install_resize_recovery_falls_back_to_shutil( + self, bare_cli, monkeypatch + ): + """A dead app.output probe falls back to shutil, never to the + DummyApplication's fake width.""" + import os as os_mod + + app = MagicMock() + app.output.get_size.side_effect = RuntimeError("not attached") + monkeypatch.setattr( + cli_mod.shutil, + "get_terminal_size", + lambda _default: os_mod.terminal_size((97, 40)), + ) + + bare_cli._install_resize_recovery(app) + + assert bare_cli._last_resize_width == 97 + + def test_install_resize_recovery_survives_width_probe_failure( + self, bare_cli, monkeypatch + ): + app = MagicMock() + app.output.get_size.side_effect = RuntimeError("not attached") + + def _boom(_default): + raise RuntimeError("no tty") + + monkeypatch.setattr(cli_mod.shutil, "get_terminal_size", _boom) + + bare_cli._install_resize_recovery(app) # must not raise + + assert getattr(bare_cli, "_last_resize_width", None) is None