Skip to content

Keep e2e prompt output from drawing over the outer terminal (4.0.0-DEV3) - #20

Open
KeithRaghubar wants to merge 1 commit into
mainfrom
fix/e2e-answer-terminal-bleed
Open

KeithRaghubar wants to merge 1 commit into
mainfrom
fix/e2e-answer-terminal-bleed

Conversation

@KeithRaghubar

Copy link
Copy Markdown
Owner

Problem

During make pre-release, text overlapped on one line near the bottom of the terminal while the terminal's real last line stayed blank. The terminal was fine once make finished.

tools/e2e/answer.py runs a scenario's command on a pseudo-terminal from pty.fork(), which never gets a window size. The child sees 0×0 and shutil.get_terminal_size() falls back to 80×24, but isatty() is true, so sysforge's progress bar runs in TTY mode. It sets a scroll region for rows 1–23 (ESC[1;23r) and paints row 24. answer.py copied the raw stream to stdout, and podman exec carried it to the terminal running make e2e, whatever that terminal's real height. This affects the five scenarios that use answer: 10-quickstart, 20-repo-optin, 45-interactive, 50-maintain and 75-config-merge.

Fix

A TerminalFilter now sits between the pty and stdout:

  • Passes text, newlines, \r, colour (ESC[…m) and erase-to-end-of-line (ESC[K), which work wherever the cursor is.
  • Drops every other sequence: scroll regions, cursor moves, alternate screen, cursor visibility, window titles.
  • Discards text drawn between a cursor save and its restore, which is how ui/progress.py paints the bar row. Stripping only the cursor moves would splice [building] … into content lines. A bar paint is one row, so a newline inside the bracket means it wasn't one, and that text is kept. An unrestored save at the end also keeps its text.
  • Holds a sequence split across two os.read calls until it's complete.

Prompt matching still runs on the unfiltered stream, and the command still runs on a terminal, so the scenarios exercise the same interactive paths.

Docs: tools/e2e/README.md, docs/design/17-release-plan.md (+ DESIGN.md), and a 4.0.0-DEV3 release-note entry under ## Changed.

Tests

  • New tests/test_e2e_answer.py (8 tests): the bar's establish/paint/release sequences; colour, \r and ESC[K surviving; a sequence and a paint each split across reads; an unrestored save, mid-stream and at the end; alternate screen, title and cursor visibility. The last test runs answer.py on a real pty and checks the prompt is still answered while placement is stripped. All 8 failed before the fix. The split-sequence test caught a regex bug during the fix: the short-form branch [0-~] took ESC [ itself as a complete sequence.
  • Checked against sysforge's real progress output (ui.progress session plus phase, run under both versions): the old answer.py emitted ESC[1;23r and the row-24 paint; the new one emits only the content lines.
  • Full suite: 7748 passed, 4 skipped. lint, check-standards, check-design and check-surface are clean.

Tier: this is the e2e harness itself, so I didn't add a new e2e scenario. The pty test above exercises the real pseudo-terminal boundary. I haven't run make e2e locally; CI runs it on this PR.

Checklist

  • Release-note entry in docs/release-notes/unreleased.md, led by its roadmap ID
  • If a user-facing surface changed: make surface committed, and its diff matches what I meant to change (no surface change; check-surface clean)
  • If a name was removed or renamed: the old one is in tools/retired_names.toml (none)
  • The guide for this feature (docs/guides/) still describes it correctly (no user guide covers the e2e harness; tools/e2e/README.md updated)
  • make pre-release (or make ci-local) passes locally (not run; the full suite and the static guards pass)

🤖 Generated with Claude Code

answer.py runs a scenario's command on a pty that has no window size, so
sysforge assumed 80x24: its progress bar set a scroll region for rows
1-23 and painted row 24. answer.py copied the raw bytes to stdout, and
through podman exec they reached the terminal running make e2e or make
pre-release, overlapping text on that row while the real bottom line
stayed blank.

TerminalFilter now sits between the pty and stdout: text, colour, \r and
erase-to-end-of-line pass; every other sequence is dropped, as is text
drawn between a cursor save and its restore (the bar row). A newline
inside that bracket means it was not a one-row paint, so its text is
kept. Sequences split across reads are held until complete. Prompt
matching still reads the unfiltered stream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant