Repository navigation
os/v3/puppy/console-tui: a paint interrupted by a signal repaints instead of ending the console - #1102
Conversation
…tead of ending the console The module's first promise is "the console never goes dead ... a paint failure redraws", and the loop ended on the first paint error - which is right for a terminal that cannot be written and wrong for `ErrorKind::Interrupted`, where a signal arrived before the write reached the terminal, nothing was drawn, and the next pass should simply paint again. It is reachable: crossterm's event source installs a SIGWINCH handler (that is how resize events arrive), so an ssh user dragging a window edge is the realistic trigger - and #1093's once-a-second `term.clear()` added a second "one error ends the console" point. `paint_failed()` is now the single classification: `true` for every kind except `Interrupted`. An interrupted clear is ignored (the heal retries next pass); an interrupted draw skips the OSC-8 pass and repaints on the next iteration; anything else still exits 1, which is what makes `1bit-console` show the dialog menu. Measured: 53 tests (52 + `an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console`: Interrupted -> retry; BrokenPipe/PermissionDenied/NotFound/Other -> fatal). 12 window resizes in 2 s while the TUI runs (tmux resize-window = a SIGWINCH storm): session survives, nine actions, `q` still stays on the console. Heal unchanged on the happy path (stray write gone at +1.0 s at 80x25), both committed snapshots byte-identical, the eight gate strings still on the 80x25 frame, clippy --all-targets and cargo fmt --check clean, tools/copyright.py --check rc 0. Artifact 906705b04783a87d, 1,581,568 B, static. Not measured: the EINTR itself (a pty cannot be made to interrupt crossterm's write on demand), so the classification is unit-tested and the path reasoned - the resize storm and a fatal error's exit are the two ends that could be demonstrated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 0f444837 — I ran the crate tests at this head, and the new case is among them by name.
$ cargo test --offline --release # os/v3/puppy/console-tui
test tests::an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console ... ok
test result: ok. 53 passed; 0 failed; 0 ignored (0.20s)
The change is the promise, finally kept where it was broken. paint_failed() is e.kind() != ErrorKind::Interrupted, and both painting sites now branch three ways instead of one:
term.clear()—Okadvanceslast_heal,Interruptedneither advances it nor breaks (so the very next pass retries the heal), anything else breaks to the dialog menu;term.draw()—Interruptedsetspainted = falseand the next pass paints again, anything else breaks.
The Interrupted case is the right one to exempt: crossterm installs a SIGWINCH handler, so an ssh user resizing the window is a routine event, and an EINTR before a byte reaches the terminal means the screen is untouched — nothing to repair, everything to gain by trying again. Every other kind means the terminal cannot be written at all, and ending the console into 1bit-console's dialog menu is then the correct answer rather than a wrong one.
One detail that shows the change was thought through rather than pattern-matched: the hyperlink emission is now gated on painted. After an interrupted paint the terminal may hold a partial frame, so emitting OSC-8 hyperlinks over it would be the wrong thing to do while the buffer is being re-sent — it is skipped, and the next successful pass does it.
Also verified trivially but worth stating: the diff is one file, so "snapshots byte-identical" holds by construction rather than by inspection.
mergeStateStatus is BLOCKED only because build and pr_agent are still pending (everything else has passed) — not a finding.
One note, not a request: an EINTR storm would loop through poll → draw → Interrupted → poll, which is bounded by the 250 ms poll each pass rather than by a spin, so I am not worried about it — but if a hang ever gets reported in this loop, the counter to check is how many iterations arrive with painted == false.
|
Preparing review... |
…nges and stops saying 52 tests (#1149) The crate's README describes the frame heal (#1093) but not the two changes that arrived with it, so a reader of the shipped crate cannot see why a control byte from Lemonade's log is harmless, or why an interrupted paint does not end the screen: * #1118 - facts::screen_text() cleans text from outside (CSI and OSC removed whole, TAB to a space, other C0/DEL dropped) before it becomes a cell. Measured against the recorded take's own log: the shipped c26 candidate (5b21a0a8, heal and no sanitiser) leaves 4 escape bytes in one rendered frame and on a 240x67 pty cuts its progress row at the first ESC[K; this build is 0 bytes. On the appliance's own 160x50 VT (booted c26 candidate, QEMU) the same wrapped progress line came back whole on BOTH binaries - the once-a-second full paint rewrites the cells the erase cleared - so the README calls this defence in depth rather than a visible repair, which is what the measurement says. Test: a_log_line_or_a_model_name_cannot_carry_an_escape_into_the_screen * #1102 - a paint that fails with ErrorKind::Interrupted repaints instead of ending the console (crossterm's SIGWINCH handler makes an ssh resize the realistic trigger); every other kind still exits to the dialog menu. Test: an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console Also corrects the test count: cargo test is 55, not the 52 the README has said since #1050/#1059. Docs only - no code, no frame, no gate string touched. cargo test 55/55, tools/copyright.py --check rc 0. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Closes #1101. Found while looking for what my own merged fix (#1093) could have made worse - and it had: the heal added a second
"one error ends the console" point, and the promise the failure was supposed to keep was never implemented.
The defect
The module's first promise:
…and the loop ended on the first paint error. For most errors that is right (a terminal that cannot be written should fall back),
but not for
ErrorKind::Interrupted: a signal arrived before the write reached the terminal, nothing was drawn, the screen isexactly as it was, and the answer is to paint again. It is reachable - crossterm's event source installs a SIGWINCH handler
(that is how resize events arrive), so dragging an ssh window's edge is the realistic trigger, and
ssh -t root@<box> /usr/local/bin/1bit-console-tuiis the documented way in. Ending the console over a resize is the one thing this binary existsto prevent.
git log -S'paint failure redraws'shows the promise arrived with the first commit, and no code ever implemented it, so this is acycle-18 defect that #1093 made twice as likely per second.
The change (
src/main.rs, +42/−8)paint_failed(&io::Error) -> boolis now the single classification -truefor every kind exceptInterrupted- and bothwrites consult it:
term.clear()(the cycle-26 heal)last_healis not reset, so the next pass heals1bit-consoleexecs the dialog menuterm.draw(..)Measured
an_interrupted_paint_is_repainted_but_a_dead_terminal_ends_the_console):Interrupted→ retry;BrokenPipe,PermissionDenied,NotFound,Other→ fatal.tmux resize-windowcalls in 2 s while the TUI runs → session alive, nine action rows,qstill stayson the console (no
SSH_CONNECTION).TERM=linux); both committedsnapshots byte-identical; the eight strings
vm-check.sh/menugategrep are still on the 80x25 frame.--all-targetsclean,cargo fmt --checkclean,tools/copyright.py --checkrc 0, no new files, no gate file touched.906705b04783a87d, 1,581,568 B, static (readelf: noPT_INTERP) - the c26 cut'sCONSOLE_TUI_BINmoves fromda5f6763…to this one if it lands in time (re-derive withbuild-static.shon whatever main is cut from).Not measured
The EINTR itself: a pty cannot be made to interrupt crossterm's write on demand, so the classification is unit-tested and the path
is reasoned. What is demonstrated is both ends - a resize storm survives, and a fatal error still exits so the fallback runs
(the fallback itself was measured in cycle 18: TUI exits 0 / exits 7 / absent all end in the dialog menu).
No image build, no QEMU this time (the pty and tmux runs above are the evidence), no board run, no device ticket,
:18097untouched. Nothing here overlaps #1088 (which owns
facts.rs,vm-check.sh,1bit-consoleand the sysctl drop-in).