fix: expand Ink partial clears to prevent ghost content in screenshots
Ink (React CLI framework) clears its output using repeated EL2+CUU1 sequences, one per previously-drawn line. When /clear resets Ink's internal line counter, the next frame only erases a few lines instead of the full previous output. In a real terminal the old content is in scrollback and invisible, but pyte's fixed-size screen retains it, producing ghost content (e.g. duplicated prompts) in SVG screenshots. Added AltScreen.expand_clear_sequences() which detects runs of 3+ EL2+CUU1 pairs that don't reach row 0 and extends them to erase all lines up to the top of the screen. Both DockerExecSession and TerminalSession call this before feeding data to pyte. Also made on_session_end() idempotent (contextlib.suppress KeyError) to prevent a race when close_session() and natural session exit both call it. Added docs/ink-clear-fix.md with root cause analysis, byte-level explanation, and reproduction script.
This commit is contained in:
@@ -0,0 +1,110 @@
|
|||||||
|
# Ink Partial Clear Fix
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
When CLI applications built with [Ink](https://github.com/vadimdemedes/ink) (React for terminals) — such as GitHub Copilot CLI — execute a `/clear` command, the resulting screenshot shows **ghost content**: old conversation lines persist above the fresh prompt.
|
||||||
|
|
||||||
|
The real terminal displays correctly, but the pyte-based screen buffer used for SVG screenshot generation retains the stale content.
|
||||||
|
|
||||||
|
### Example
|
||||||
|
|
||||||
|
Before `/clear`, the screen has 30 lines of conversation. After `/clear`, the user sees only a fresh 6-line prompt, but the screenshot shows:
|
||||||
|
|
||||||
|
```
|
||||||
|
Row 0: [old prompt header] ← ghost content
|
||||||
|
Row 1: [────────────────]
|
||||||
|
Row 2: [❯ hello]
|
||||||
|
...
|
||||||
|
Row 20: [● old response text] ← ghost content
|
||||||
|
Row 21: [/workspace[main]] ← fresh prompt (duplicated)
|
||||||
|
Row 22: [────────────────]
|
||||||
|
Row 23: [❯ Type @ to mention files]
|
||||||
|
Row 24: [────────────────]
|
||||||
|
Row 25: [shift+tab cycle mode]
|
||||||
|
```
|
||||||
|
|
||||||
|
## Root Cause
|
||||||
|
|
||||||
|
Ink uses a **line-by-line erase** pattern to clear its previous frame before drawing the next one:
|
||||||
|
|
||||||
|
```
|
||||||
|
ESC[2K (EL2 — erase entire line)
|
||||||
|
ESC[1A (CUU1 — cursor up one row)
|
||||||
|
```
|
||||||
|
|
||||||
|
This pair is repeated once per line of the **previous** frame. Ink tracks how many lines it rendered and erases exactly that many before redrawing.
|
||||||
|
|
||||||
|
When `/clear` is issued:
|
||||||
|
|
||||||
|
1. Ink resets its internal "previous output height" counter to 0.
|
||||||
|
2. On the next render cycle, Ink erases **0 old lines** (counter is 0), then draws the fresh prompt (~6 lines).
|
||||||
|
3. The subsequent render erases 6 lines (the prompt it just drew), redraws — correct from now on.
|
||||||
|
|
||||||
|
In a real terminal, the old content has already **scrolled into the scrollback buffer** and is invisible. But pyte's fixed-size `Screen` keeps all content in the visible buffer. The 6-line erase only clears rows at the bottom, leaving rows 0–24 with orphaned old content.
|
||||||
|
|
||||||
|
### Byte-level example
|
||||||
|
|
||||||
|
Cursor at row 30 after rendering a full conversation:
|
||||||
|
|
||||||
|
```
|
||||||
|
Frame N (normal): EL2+CUU1 × 28 (clears rows 30→2) + redraw 28 lines ✓
|
||||||
|
/clear resets counter
|
||||||
|
Frame N+1 (broken): EL2+CUU1 × 6 (clears rows 30→24) + redraw 6 lines ✗
|
||||||
|
Rows 0–23 still contain old content!
|
||||||
|
```
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
`AltScreen.expand_clear_sequences()` in `alt_screen.py` pre-processes incoming terminal data before it reaches pyte. It detects runs of 3+ `EL2+CUU1` pairs and, if the run doesn't reach row 0, extends it with additional pairs so the erase covers all lines from the cursor position up to the top of the screen.
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Before fix: 6 pairs clear rows 30→24, leaving 0–23 dirty
|
||||||
|
data = b"\x1b[2K\x1b[1A" * 6
|
||||||
|
|
||||||
|
# After expand_clear_sequences(): 30 pairs clear rows 30→0
|
||||||
|
data = screen.expand_clear_sequences(data)
|
||||||
|
# Now contains 30 pairs of EL2+CUU1
|
||||||
|
```
|
||||||
|
|
||||||
|
Both `DockerExecSession._update_screen()` and `TerminalSession._update_screen()` call this method after C1 normalization and before feeding data to `pyte.ByteStream`.
|
||||||
|
|
||||||
|
### Why this is safe
|
||||||
|
|
||||||
|
- The fix only triggers on runs of **3 or more** `EL2+CUU1` pairs (normal editing uses 1–2 at most).
|
||||||
|
- It only **adds** additional erase operations for lines that would already be empty in a real terminal (they scrolled into scrollback).
|
||||||
|
- The extra erases are no-ops if the lines are already blank.
|
||||||
|
- Short runs (< 3 pairs) and runs that already reach row 0 are left unchanged.
|
||||||
|
|
||||||
|
## Reproducing
|
||||||
|
|
||||||
|
```python
|
||||||
|
from webterm.alt_screen import AltScreen
|
||||||
|
import pyte
|
||||||
|
|
||||||
|
screen = AltScreen(132, 45)
|
||||||
|
stream = pyte.ByteStream(screen)
|
||||||
|
|
||||||
|
# Draw 27 lines of content
|
||||||
|
for i in range(27):
|
||||||
|
stream.feed(f"Content line {i}\r\n".encode())
|
||||||
|
|
||||||
|
# Ink /clear: only erases 6 lines
|
||||||
|
partial_clear = b"\x1b[2K\x1b[1A" * 6 + b"\x1b[2K\x1b[G"
|
||||||
|
|
||||||
|
# Without fix: old content remains on rows 0–20
|
||||||
|
# With fix: all rows are cleared
|
||||||
|
expanded = screen.expand_clear_sequences(partial_clear)
|
||||||
|
stream.feed(expanded)
|
||||||
|
|
||||||
|
# Draw fresh prompt
|
||||||
|
stream.feed(b"Fresh prompt\r\n")
|
||||||
|
|
||||||
|
non_empty = [line for line in screen.display if line.strip()]
|
||||||
|
assert len(non_empty) == 1 # Only "Fresh prompt"
|
||||||
|
```
|
||||||
|
|
||||||
|
## Related
|
||||||
|
|
||||||
|
- `WEBTERM_SCREENSHOT_FORCE_REDRAW` env var — sends SIGWINCH to force app redraw before screenshots, but doesn't fix this issue since Ink still only erases its tracked line count.
|
||||||
|
- `docs/tmux-da-response-filtering.md` — another terminal compatibility fix.
|
||||||
|
- pyte's known limitations with partial screen clearing are noted in `README.md`.
|
||||||
@@ -12,10 +12,16 @@ the screen buffer when switching between main and alternate screen modes.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import copy
|
import copy
|
||||||
|
import re
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
import pyte
|
import pyte
|
||||||
|
|
||||||
|
# Pattern to match a run of 3+ (EL2 + CUU1) pairs used by Ink/React CLI
|
||||||
|
# to erase the previous frame before drawing the next one.
|
||||||
|
_INK_CLEAR_PATTERN = re.compile(rb"(\x1b\[2K\x1b\[1A){3,}")
|
||||||
|
_EL2_CUU1 = b"\x1b[2K\x1b[1A"
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from pyte.screens import Char
|
from pyte.screens import Char
|
||||||
|
|
||||||
@@ -125,3 +131,34 @@ class AltScreen(pyte.Screen):
|
|||||||
self._saved_cursor = None
|
self._saved_cursor = None
|
||||||
|
|
||||||
super().resize(lines, columns)
|
super().resize(lines, columns)
|
||||||
|
|
||||||
|
def expand_clear_sequences(self, data: bytes) -> bytes:
|
||||||
|
"""Expand partial line-by-line clears to cover the full screen.
|
||||||
|
|
||||||
|
CLI frameworks like Ink (React for terminals) erase their previous
|
||||||
|
output using repeated ``EL2 + CUU1`` (erase line, cursor up) sequences.
|
||||||
|
When the application's ``/clear`` command resets the framework's internal
|
||||||
|
line counter, the next frame only erases a few lines instead of the full
|
||||||
|
previous output. In a real terminal the old content has scrolled into the
|
||||||
|
scrollback buffer, but pyte keeps it visible, producing ghost content in
|
||||||
|
screenshots.
|
||||||
|
|
||||||
|
This method detects such partial clears and extends them so that all
|
||||||
|
lines from the cursor position up to row 0 are erased.
|
||||||
|
"""
|
||||||
|
if _EL2_CUU1 not in data:
|
||||||
|
return data
|
||||||
|
|
||||||
|
cursor_y = self.cursor.y
|
||||||
|
|
||||||
|
def _extend(match: re.Match[bytes]) -> bytes:
|
||||||
|
nonlocal cursor_y
|
||||||
|
run = match.group(0)
|
||||||
|
pair_count = len(run) // len(_EL2_CUU1)
|
||||||
|
extra = cursor_y - pair_count
|
||||||
|
cursor_y = max(cursor_y - pair_count, 0)
|
||||||
|
if extra > 0:
|
||||||
|
return run + _EL2_CUU1 * extra
|
||||||
|
return run
|
||||||
|
|
||||||
|
return _INK_CLEAR_PATTERN.sub(_extend, data)
|
||||||
|
|||||||
@@ -321,6 +321,7 @@ class DockerExecSession(Session):
|
|||||||
normalized, self._utf8_buffer = _normalize_c1_controls(data, self._utf8_buffer)
|
normalized, self._utf8_buffer = _normalize_c1_controls(data, self._utf8_buffer)
|
||||||
if not normalized:
|
if not normalized:
|
||||||
return
|
return
|
||||||
|
normalized = self._screen.expand_clear_sequences(normalized)
|
||||||
self._stream.feed(normalized)
|
self._stream.feed(normalized)
|
||||||
if self._screen.dirty:
|
if self._screen.dirty:
|
||||||
self._change_counter += 1
|
self._change_counter += 1
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import contextlib
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
import shlex
|
import shlex
|
||||||
@@ -70,6 +71,7 @@ class SessionManager:
|
|||||||
self.sessions.pop(session_id, None)
|
self.sessions.pop(session_id, None)
|
||||||
route_key = self.routes.get_key(session_id)
|
route_key = self.routes.get_key(session_id)
|
||||||
if route_key is not None:
|
if route_key is not None:
|
||||||
|
with contextlib.suppress(KeyError):
|
||||||
del self.routes[route_key]
|
del self.routes[route_key]
|
||||||
log.debug("Session %s ended", session_id)
|
log.debug("Session %s ended", session_id)
|
||||||
|
|
||||||
|
|||||||
@@ -259,6 +259,7 @@ class TerminalSession(Session):
|
|||||||
normalized, self._utf8_buffer = _normalize_c1_controls(data, self._utf8_buffer)
|
normalized, self._utf8_buffer = _normalize_c1_controls(data, self._utf8_buffer)
|
||||||
if not normalized:
|
if not normalized:
|
||||||
return
|
return
|
||||||
|
normalized = self._screen.expand_clear_sequences(normalized)
|
||||||
self._stream.feed(normalized)
|
self._stream.feed(normalized)
|
||||||
# Increment change counter when screen is modified
|
# Increment change counter when screen is modified
|
||||||
if self._screen.dirty:
|
if self._screen.dirty:
|
||||||
|
|||||||
@@ -129,3 +129,80 @@ class TestAltScreen:
|
|||||||
stream.feed("\x1b[2J") # ED 2 - erase entire display
|
stream.feed("\x1b[2J") # ED 2 - erase entire display
|
||||||
|
|
||||||
assert all(line.strip() == "" for line in screen.display)
|
assert all(line.strip() == "" for line in screen.display)
|
||||||
|
|
||||||
|
|
||||||
|
class TestExpandClearSequences:
|
||||||
|
"""Tests for expand_clear_sequences (Ink partial clear fix)."""
|
||||||
|
|
||||||
|
def test_no_clear_sequences(self):
|
||||||
|
"""Data without EL2+CUU1 runs is returned unchanged."""
|
||||||
|
screen = AltScreen(80, 24)
|
||||||
|
data = b"Hello world\r\n"
|
||||||
|
assert screen.expand_clear_sequences(data) == data
|
||||||
|
|
||||||
|
def test_short_clear_not_expanded(self):
|
||||||
|
"""Runs of fewer than 3 EL2+CUU1 pairs are not modified."""
|
||||||
|
screen = AltScreen(80, 24)
|
||||||
|
data = b"\x1b[2K\x1b[1A\x1b[2K\x1b[1A" # 2 pairs
|
||||||
|
assert screen.expand_clear_sequences(data) == data
|
||||||
|
|
||||||
|
def test_full_clear_not_expanded(self):
|
||||||
|
"""A clear that already reaches row 0 is not extended."""
|
||||||
|
screen = AltScreen(80, 24)
|
||||||
|
stream = pyte.ByteStream(screen)
|
||||||
|
# Put cursor at row 5
|
||||||
|
stream.feed(b"\r\n" * 5)
|
||||||
|
assert screen.cursor.y == 5
|
||||||
|
|
||||||
|
# 5-pair clear already covers rows 5 down to 0
|
||||||
|
data = b"\x1b[2K\x1b[1A" * 5
|
||||||
|
result = screen.expand_clear_sequences(data)
|
||||||
|
assert result == data
|
||||||
|
|
||||||
|
def test_partial_clear_is_extended(self):
|
||||||
|
"""A partial clear that doesn't reach row 0 gets extended."""
|
||||||
|
screen = AltScreen(80, 24)
|
||||||
|
stream = pyte.ByteStream(screen)
|
||||||
|
# Draw content to push cursor to row 20
|
||||||
|
for i in range(20):
|
||||||
|
stream.feed(f"Line {i}\r\n".encode())
|
||||||
|
assert screen.cursor.y == 20
|
||||||
|
|
||||||
|
# Only clear 5 lines (should extend to clear all 20)
|
||||||
|
data = b"\x1b[2K\x1b[1A" * 5
|
||||||
|
result = screen.expand_clear_sequences(data)
|
||||||
|
expected_pairs = 20 # extend from 5 to 20
|
||||||
|
assert result.count(b"\x1b[2K\x1b[1A") == expected_pairs
|
||||||
|
|
||||||
|
def test_partial_clear_produces_correct_screen(self):
|
||||||
|
"""Simulates Ink /clear: partial clear + redraw leaves clean screen."""
|
||||||
|
screen = AltScreen(80, 24)
|
||||||
|
stream = pyte.ByteStream(screen)
|
||||||
|
|
||||||
|
# Draw 15 lines of content (Ink frame 1)
|
||||||
|
for i in range(15):
|
||||||
|
stream.feed(f"Old line {i}\r\n".encode())
|
||||||
|
|
||||||
|
# Ink /clear: only clears 5 lines then redraws fresh prompt
|
||||||
|
clear = b"\x1b[2K\x1b[1A" * 5 + b"\x1b[2K\x1b[G"
|
||||||
|
new_content = b"Fresh prompt\r\n"
|
||||||
|
|
||||||
|
expanded = screen.expand_clear_sequences(clear)
|
||||||
|
stream.feed(expanded)
|
||||||
|
stream.feed(new_content)
|
||||||
|
|
||||||
|
# Old content should be gone
|
||||||
|
non_empty = [line.rstrip() for line in screen.display if line.strip()]
|
||||||
|
assert len(non_empty) == 1
|
||||||
|
assert non_empty[0] == "Fresh prompt"
|
||||||
|
|
||||||
|
def test_data_around_clear_preserved(self):
|
||||||
|
"""Text before and after a clear run is preserved."""
|
||||||
|
screen = AltScreen(80, 24)
|
||||||
|
stream = pyte.ByteStream(screen)
|
||||||
|
stream.feed(b"\r\n" * 10)
|
||||||
|
|
||||||
|
data = b"before\x1b[2K\x1b[1A" * 0 + b"before" + b"\x1b[2K\x1b[1A" * 5 + b"after"
|
||||||
|
result = screen.expand_clear_sequences(data)
|
||||||
|
assert result.startswith(b"before")
|
||||||
|
assert result.endswith(b"after")
|
||||||
|
|||||||
@@ -105,6 +105,21 @@ class TestSessionManager:
|
|||||||
assert session_id not in manager.sessions
|
assert session_id not in manager.sessions
|
||||||
assert route_key not in manager.routes
|
assert route_key not in manager.routes
|
||||||
|
|
||||||
|
def test_on_session_end_idempotent(self, mock_poller, mock_path, sample_apps):
|
||||||
|
"""Test session end cleanup is idempotent."""
|
||||||
|
manager = SessionManager(mock_poller, mock_path, sample_apps)
|
||||||
|
|
||||||
|
session_id = SessionID("test-session")
|
||||||
|
route_key = RouteKey("test-route")
|
||||||
|
manager.sessions[session_id] = MagicMock()
|
||||||
|
manager.routes[route_key] = session_id
|
||||||
|
|
||||||
|
manager.on_session_end(session_id)
|
||||||
|
manager.on_session_end(session_id)
|
||||||
|
|
||||||
|
assert session_id not in manager.sessions
|
||||||
|
assert route_key not in manager.routes
|
||||||
|
|
||||||
def test_on_session_end_nonexistent(self, mock_poller, mock_path, sample_apps):
|
def test_on_session_end_nonexistent(self, mock_poller, mock_path, sample_apps):
|
||||||
"""Test session end for non-existent session."""
|
"""Test session end for non-existent session."""
|
||||||
manager = SessionManager(mock_poller, mock_path, sample_apps)
|
manager = SessionManager(mock_poller, mock_path, sample_apps)
|
||||||
|
|||||||
Reference in New Issue
Block a user