fix(cli): recover prompt_toolkit paint after tmux attach
Same-width SIGWINCH (typical tmux attach) skipped screen clear and left previous_screen inconsistent, crashing redraw with 'cell' object has no attribute 'char'. Always clear on resize recovery and retry _output_screen_diff with previous_screen=None on AttributeError/TypeError.
This commit is contained in:
@@ -3614,6 +3614,58 @@ def _strip_leaked_bracketed_paste_wrappers(text: str) -> str:
|
||||
return strip_leaked_bracketed_paste_wrappers(text)
|
||||
|
||||
|
||||
def _hermes_call_output_screen_diff(
|
||||
orig_osd,
|
||||
app,
|
||||
output,
|
||||
screen,
|
||||
current_pos,
|
||||
color_depth,
|
||||
previous_screen,
|
||||
last_style,
|
||||
is_done,
|
||||
full_screen,
|
||||
attrs_for_style_string,
|
||||
style_string_has_style,
|
||||
size,
|
||||
previous_width,
|
||||
):
|
||||
"""Call prompt_toolkit ``_output_screen_diff`` with Hermes resize guards.
|
||||
|
||||
1. Inflate ``previous_screen.height`` when the new screen is taller so pt
|
||||
skips the reserve-vertical-space cursor move that stamps chrome into
|
||||
scrollback (pt #29 / Hermes #26137).
|
||||
2. On AttributeError/TypeError from a corrupt previous paint buffer
|
||||
(classic after tmux attach with same width), retry once with
|
||||
``previous_screen=None`` so pt first-paints cleanly instead of crashing
|
||||
the event loop with ``'cell' object has no attribute 'char'``.
|
||||
"""
|
||||
try:
|
||||
if previous_screen is not None and hasattr(previous_screen, "height"):
|
||||
if previous_screen.height < screen.height:
|
||||
previous_screen.height = screen.height
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
try:
|
||||
return orig_osd(
|
||||
app, output, screen, current_pos, color_depth,
|
||||
previous_screen, last_style, is_done, full_screen,
|
||||
attrs_for_style_string, style_string_has_style,
|
||||
size, previous_width,
|
||||
)
|
||||
except (AttributeError, TypeError):
|
||||
# Corrupt previous_screen / row cells after client reattach.
|
||||
return orig_osd(
|
||||
app, output, screen, current_pos, color_depth,
|
||||
None, # previous_screen → first-paint erase path
|
||||
None, # last_style
|
||||
is_done, full_screen,
|
||||
attrs_for_style_string, style_string_has_style,
|
||||
size, 0, # previous_width → treat as changed
|
||||
)
|
||||
|
||||
|
||||
def _apply_bracketed_paste_timeout_patch() -> None:
|
||||
"""Patch prompt_toolkit to recover from torn bracketed-paste sequences.
|
||||
|
||||
@@ -5162,9 +5214,14 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
||||
# reach, leaving a duplicated status bar stranded above the live origin.
|
||||
# Ctrl+L / /redraw clears it cleanly, so route the resize path through
|
||||
# the SAME recovery: wipe the visible viewport (banner-safe — CSI 2J
|
||||
# only, never CSI 3J) and replay the transcript so nothing is lost.
|
||||
# Row-count-only changes skip this (no reflow → no ghost) to avoid an
|
||||
# unnecessary full repaint.
|
||||
# by default; CSI 3J only when display.cli_rebuild_scrollback_on_redraw
|
||||
# is enabled) and replay the transcript so nothing is lost.
|
||||
# Same-width SIGWINCH (tmux attach, benign focus/tab signals) is left
|
||||
# untouched — no clear, no replay — because a 2J without replay erases
|
||||
# the visible transcript and a replay against preserved scrollback
|
||||
# duplicates it (#65293). The stale-previous_screen crash tmux attach
|
||||
# used to trigger is handled by _hermes_call_output_screen_diff's
|
||||
# retry-with-first-paint instead (#83874).
|
||||
try:
|
||||
new_width = self._get_tui_terminal_width()
|
||||
except Exception:
|
||||
@@ -18110,20 +18167,20 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
||||
other code path's behavior.
|
||||
|
||||
Critical: do NOT replace a None previous_screen with
|
||||
a fresh Screen() — that would skip the proper
|
||||
reset_attributes()+erase_down() at L178-185 which
|
||||
fires when previous_screen is None (first-paint /
|
||||
a fresh Screen() on the happy path — that would skip
|
||||
the proper reset_attributes()+erase_down() at L178-185
|
||||
which fires when previous_screen is None (first-paint /
|
||||
width-change). Without that reset, ANSI styles
|
||||
leak between renders.
|
||||
"""
|
||||
try:
|
||||
if previous_screen is not None and hasattr(previous_screen, "height"):
|
||||
if previous_screen.height < screen.height:
|
||||
previous_screen.height = screen.height
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return _orig_osd(
|
||||
Safety net: if the diff crashes with AttributeError /
|
||||
TypeError (corrupt previous_screen after tmux attach —
|
||||
"'cell' object has no attribute 'char'"), retry once
|
||||
with previous_screen=None so pt takes the first-paint
|
||||
erase path instead of wedging the event loop.
|
||||
"""
|
||||
return _hermes_call_output_screen_diff(
|
||||
_orig_osd,
|
||||
app, output, screen, current_pos, color_depth,
|
||||
previous_screen, last_style, is_done, full_screen,
|
||||
attrs_for_style_string, style_string_has_style,
|
||||
|
||||
@@ -123,6 +123,81 @@ class TestForceFullRedraw:
|
||||
assert events[:3] == ["erase", "scrollback_wipe", "replay"]
|
||||
assert events.index("scrollback_wipe") < events.index("original_resize")
|
||||
|
||||
def test_same_width_sigwinch_is_left_untouched(self, bare_cli, monkeypatch):
|
||||
"""Same-width SIGWINCH (tmux attach, benign focus/tab signals) must not
|
||||
clear the viewport or replay: a 2J without replay erases the visible
|
||||
transcript, and a replay duplicates it (#65293). The tmux-attach
|
||||
stale-paint crash is handled by _hermes_call_output_screen_diff's
|
||||
retry instead (#83874)."""
|
||||
app = MagicMock()
|
||||
events = []
|
||||
app.renderer.output.erase_screen.side_effect = lambda: events.append("erase")
|
||||
app.renderer.output.write_raw.side_effect = lambda *_: events.append("scrollback_wipe")
|
||||
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: 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)
|
||||
|
||||
assert "erase" not in events
|
||||
assert "replay" not in events
|
||||
assert "scrollback_wipe" not in events
|
||||
assert events == ["original_resize"]
|
||||
assert bare_cli._last_resize_width == 120
|
||||
assert bare_cli._status_bar_suppressed_after_resize is True
|
||||
|
||||
def test_output_screen_diff_retries_on_corrupt_previous_screen(self, bare_cli):
|
||||
"""Corrupt previous_screen must not wedge the paint loop.
|
||||
|
||||
After tmux attach, _output_screen_diff can raise AttributeError
|
||||
('cell' object has no attribute 'char'). Retry with previous_screen=None.
|
||||
"""
|
||||
calls = []
|
||||
|
||||
def fake_osd(
|
||||
app, output, screen, current_pos, color_depth,
|
||||
previous_screen, last_style, is_done, full_screen,
|
||||
attrs_for_style_string, style_string_has_style,
|
||||
size, previous_width,
|
||||
):
|
||||
calls.append((previous_screen, previous_width, last_style))
|
||||
if previous_screen is not None:
|
||||
# Exact failure mode from the classic CLI event loop.
|
||||
raise AttributeError("'cell' object has no attribute 'char'")
|
||||
return ("ok", current_pos, last_style)
|
||||
|
||||
screen = MagicMock()
|
||||
screen.height = 10
|
||||
previous = MagicMock()
|
||||
previous.height = 8
|
||||
|
||||
result = cli_mod._hermes_call_output_screen_diff(
|
||||
fake_osd,
|
||||
app=None,
|
||||
output=None,
|
||||
screen=screen,
|
||||
current_pos=None,
|
||||
color_depth=None,
|
||||
previous_screen=previous,
|
||||
last_style="style",
|
||||
is_done=False,
|
||||
full_screen=False,
|
||||
attrs_for_style_string=None,
|
||||
style_string_has_style=None,
|
||||
size=None,
|
||||
previous_width=80,
|
||||
)
|
||||
|
||||
assert result[0] == "ok"
|
||||
assert len(calls) == 2
|
||||
assert calls[0][0] is previous
|
||||
assert previous.height == 10 # height inflate still applied first
|
||||
assert calls[1] == (None, 0, None)
|
||||
|
||||
def test_resize_recovery_is_debounced(self, bare_cli, monkeypatch):
|
||||
timers = []
|
||||
calls = []
|
||||
|
||||
Reference in New Issue
Block a user