Stage 3 review: put the terminal back after a half-written paint

The backend buffers a paint and flushes it as one write, so a failure part-way through that write leaves the terminal holding a prefix: cursor saved and moved into the band, autowrap off, some cells replaced and some not. Unwinding the Python call undoes none of it -- those sequences are already on the wire. The painter now restores wrap mode and cursor on that path, before anything else touches the terminal. Releasing the margins first would save a cursor still sitting in the band and put it back there afterwards, which is what the rollback was doing. It also discards its baseline: the band is showing something no frame describes, so the next paint has to be a full one rather than a diff against a frame that was never finished. Fixing this in the painter rather than in startup covers every caller. An ordinary refresh mid-session fails the same way and left the same state behind. The regression drives a real partial emission -- a stream that writes a prefix of one batch and then raises. The earlier tests replaced paint() outright, so nothing was ever emitted and they could not have seen this.
tleonhardt · Sep 12, 2026 · Sep 10, 2026 · Sep 10, 2026 · Sep 10, 2026 · Sep 10, 2026
commit d1ef33894162761d28befaf095775e88a95f184c
@@ -25,6 +25,7 @@
 where the painter owns the cursor.
 """
 
+from contextlib import suppress
 from dataclasses import dataclass
 from typing import TYPE_CHECKING
 
@@ -374,33 +375,64 @@ def paint(self, prepared: PreparedFrame) -> bool:
                 return False
 
             top_row = geometry.physical_rows - geometry.reserved_rows + 1
-            # Anything another writer left buffered goes out first, so the band is painted
-            # after the output it was meant to follow rather than in the middle of it.
-            self._output.flush()
-            # DECSC saves the cursor *and* the current attributes, and DECRC restores both, so
-            # the renderer's next write lands where and how it expects.
-            self._output.write_raw(cursor_save_sequence())
-            self._output.disable_autowrap()
-            for row_index, column, cells in runs:
-                self._output.write_raw(_cursor_position_sequence(top_row + row_index, column + 1))
-                style: str | None = None
-                for cell in cells:
-                    if cell.is_continuation:
-                        continue
-                    if cell.style != style:
-                        self._output.set_attributes(prepared.attrs[cell.style], prepared.color_depth)
-                        style = cell.style
-                    self._output.write(cell.char)
-            if self._autowrap_after_paint:
-                self._output.enable_autowrap()
-            self._output.write_raw(cursor_restore_sequence())
-            self._output.flush()
+            try:
+                # Anything another writer left buffered goes out first, so the band is painted
+                # after the output it was meant to follow rather than in the middle of it.
+                self._output.flush()
+                # DECSC saves the cursor *and* the current attributes, and DECRC restores
+                # both, so the renderer's next write lands where and how it expects.
+                self._output.write_raw(cursor_save_sequence())
+                self._output.disable_autowrap()
+                for row_index, column, cells in runs:
+                    self._output.write_raw(_cursor_position_sequence(top_row + row_index, column + 1))
+                    style: str | None = None
+                    for cell in cells:
+                        if cell.is_continuation:
+                            continue
+                        if cell.style != style:
+                            self._output.set_attributes(prepared.attrs[cell.style], prepared.color_depth)
+                            style = cell.style
+                        self._output.write(cell.char)
+                if self._autowrap_after_paint:
+                    self._output.enable_autowrap()
+                self._output.write_raw(cursor_restore_sequence())
+                self._output.flush()
+            except BaseException:
+                self._recover_from_failed_paint()
+                raise
 
             self._last_frame = frame
             self._last_attrs = prepared.attrs
             self._last_band = band
             return True
 
+    def _recover_from_failed_paint(self) -> None:
+        """Undo what a half-finished paint left on the terminal.
+
+        The backend buffers a paint and flushes it as one write, so a failure part-way through
+        that write leaves the terminal holding a prefix: the cursor saved and moved into the
+        band, autowrap off, some cells replaced and some not. None of that is undone by
+        unwinding the Python call -- the sequences are already on the wire.
+
+        Two things follow. The wrap mode and cursor are put back, because leaving autowrap off
+        makes the next ordinary line of output wrap where it should not, and leaving the cursor
+        in the band makes the next write land in the toolbar. And the baseline is discarded:
+        the band is now showing something no frame describes, so the next paint has to be a
+        full one rather than a diff against a frame that was never finished.
+
+        The restoration is itself a write to a terminal that has just failed one, so its own
+        failure is suppressed -- the original is the one worth propagating.
+        """
+        self.invalidate()
+        with suppress(Exception):
+            if self._autowrap_after_paint:
+                self._output.enable_autowrap()
+            # DECRC returns to whatever was last saved. After a partial batch that is either
+            # this paint's own save or the one the margin change made, so the cursor lands
+            # somewhere known rather than wherever the truncated write stopped.
+            self._output.write_raw(cursor_restore_sequence())
+            self._output.flush()
+
 
 def _cursor_position_sequence(row: int, column: int) -> str:
     """Build a one-based absolute cursor move.

@@ -476,3 +476,41 @@ def test_the_filter_is_restored_when_it_is_still_ours(self) -> None:
             assert container.filter is original
         finally:
             harness.close()
+
+
+class PartialWriteStream(TtyStringIO):
+    """Writes a prefix of one flushed batch and then fails, as a real terminal can."""
+
+    def __init__(self, fail_on_write: int, keep: int = 12) -> None:
+        super().__init__()
+        self._writes = 0
+        self._fail_on_write = fail_on_write
+        self._keep = keep
+
+    def write(self, text: str) -> int:
+        self._writes += 1
+        if self._writes == self._fail_on_write:
+            super().write(text[: self._keep])
+            raise OSError("terminal went away")
+        return super().write(text)
+
+
+class TestPartialStartupPaint:
+    def test_a_half_written_first_paint_leaves_no_state_behind(self) -> None:
+        """Review finding: rollback released the margins over a cursor still in the band."""
+        harness = Harness()
+        try:
+            # Batch one installs the margins; batch two is the first paint.
+            harness.stream = PartialWriteStream(fail_on_write=2)
+            harness.backend.stdout = harness.stream
+            with pytest.raises(OSError, match="terminal went away"):
+                harness.toolbar.start()
+
+            written = harness.stream.getvalue()
+            # Wrap mode and cursor are put back before the margins are released, so the
+            # release does not save a cursor that is still inside the band.
+            assert written.index("\x1b[?7h") < written.index("\x1b[r")
+            assert harness.toolbar.is_active is False
+            assert harness.app.output is harness.backend
+        finally:
+            harness.close()
@@ -9,6 +9,7 @@
 import io
 import re
 import threading
+from typing import Any
 
 import pytest
 from prompt_toolkit.data_structures import Size
@@ -617,3 +618,95 @@ def toolbar_paint() -> None:
         # The writer blocked inside leaf I/O, holding no higher-level lock -- which is what
         # keeps the rest of cmd2 able to make progress while the terminal is backed up.
         assert stream.locks_held_while_blocked == ()
+
+
+class PartialWriteStream(io.StringIO):
+    """Writes a prefix of one flushed batch and then fails, as a real terminal can.
+
+    A test that replaces ``paint`` entirely never emits anything, so it cannot see what a
+    half-written batch leaves behind. The backend buffers a whole paint and flushes it in one
+    ``write``, so cutting that write short is what puts the terminal into the state this is
+    about: autowrap off, cursor in the band, nothing restored.
+    """
+
+    def __init__(self, fail_on_write: int, keep: int = 12) -> None:
+        super().__init__()
+        self._writes = 0
+        self._fail_on_write = fail_on_write
+        self._keep = keep
+
+    def isatty(self) -> bool:
+        return True
+
+    def write(self, text: str) -> int:
+        self._writes += 1
+        if self._writes == self._fail_on_write:
+            super().write(text[: self._keep])
+            raise OSError("terminal went away")
+        return super().write(text)
+
+
+class TestPartialPaint:
+    def make(self, fail_on_write: int = 2) -> tuple[ToolbarPainter, PartialWriteStream, Any]:
+        """Build a painter over a terminal that fails part-way through one flushed batch.
+
+        Batch one is the margin install, so the default targets the first paint.
+        """
+        stream = PartialWriteStream(fail_on_write=fail_on_write)
+        screen = {"rows": 24, "columns": 5}
+        output = Vt100_Output(stream, lambda: Size(rows=screen["rows"], columns=screen["columns"]))
+        display = ResizableDisplay(output, screen)
+        assert display.acquire() is True
+        painter = ToolbarPainter(
+            display=display,
+            lock=TerminalLock(),
+            style=DummyStyle(),
+            color_depth=ColorDepth.DEPTH_8_BIT,
+        )
+        return painter, stream, display
+
+    def test_a_partial_paint_restores_wrap_and_cursor_state(self) -> None:
+        """Leaving autowrap off would make the next ordinary line wrap where it should not."""
+        painter, stream, _display = self.make()
+        prepared = painter.prepare(lambda: "hi")
+        assert prepared is not None
+        with pytest.raises(OSError, match="terminal went away"):
+            painter.paint(prepared)
+
+        written = stream.getvalue()
+        assert "\x1b[?7l" in written  # the paint really did start emitting
+        assert written.endswith("\x1b[?7h\x1b8")  # and the cleanup really did finish it
+
+    def test_a_partial_paint_discards_the_baseline(self) -> None:
+        """Some cells were overwritten and some were not; what the band shows is unknown."""
+        painter, _stream, _display = self.make(fail_on_write=3)
+        first = painter.prepare(lambda: "hi")
+        assert first is not None
+        assert painter.paint(first) is True
+        assert painter.last_frame is not None
+
+        second = painter.prepare(lambda: "zz")
+        assert second is not None
+        with pytest.raises(OSError, match="terminal went away"):
+            painter.paint(second)
+        assert painter.last_frame is None
+
+    def test_the_next_paint_after_a_failure_is_a_full_one(self) -> None:
+        """A diff against the discarded baseline would skip the cells that never arrived."""
+        painter, stream, _display = self.make(fail_on_write=3)
+        first = painter.prepare(lambda: "hi")
+        assert first is not None
+        painter.paint(first)
+
+        second = painter.prepare(lambda: "zz")
+        assert second is not None
+        with pytest.raises(OSError, match="terminal went away"):
+            painter.paint(second)
+
+        stream.truncate(0)
+        stream.seek(0)
+        again = painter.prepare(lambda: "hi")
+        assert again is not None
+        assert painter.paint(again) is True
+        # The whole band, from its first column: not just the cells that differ from "hi".
+        assert "\x1b[24;1Hhi   " in re.sub(r"\x1b\[[0-9;]*m", "", stream.getvalue())