Drain the pty, and wait on the screen instead of the clock #85

Merged

a73x merged 1 commit from worktree-agent-a654cbd45b312f7d3 into main

2026-08-13 07:50:00 +00:00

a73x commented

2026-08-13 07:46:42 +00:00

(Migrated from git.a73x.sh)

Drain the pty, and wait on the screen instead of the clock

tests/tui_review_test.rs failed intermittently, always on the two
$EDITOR tests, always by hanging to the harness deadline rather than by
failing an assertion. It was neither a race against a slow machine nor
anything in the TUI.

The cause. run_dashboard_driven collected the child's output with
wait_with_output(), which reads nothing until the child exits. script
copies the pty master to its stdout in a single loop, so when that write
blocks it also stops reading its own stdin — keystrokes stop being
forwarded, the q never arrives, and the dashboard sits in event::poll
until the deadline kills it. Caught in the act, script is in state
anon_pipe_write.

What made it intermittent is that a pipe's capacity is not a constant.
Linux gives a new pipe 64 KiB only while the user is under
fs/pipe-user-pages-soft; over it, new pipes get one page's worth — 8 KiB.
A machine running a test suite crosses that line continually. Sampling a
fresh pipe every 200ms through one run of this file gave 8 KiB on 71 of
200 samples.

8 KiB is exactly where these tests sit. The dashboard has written ~4.9 KiB
by the time the patch detail pane is up; suspending to $EDITOR leaves the
alternate screen and returns to a full repaint, taking it to ~9.3 KiB. Only
the editor tests cross the line, which is why only they failed. The limit
is per-user, so --test-threads=1 does not avoid it and another process
of the same user can push a test over — which is what "order-dependent"
really was.

This also explains the two things that did not fit a timing story: a wider
sleep budget cannot help, because the bytes are already written before the
sleep begins; and an unrelated change to how the pane draws can appear to
fix it, by moving the byte count back under the line until the next change
moves it over again.

Proved by A/B on unmodified main with pipe capacity as the only variable:
held over the soft limit, 3/3 runs failed on exactly those two tests; the
fix passed 12/12 under the same pressure.

The fix. Read stdout and stderr on background threads for as long as
the child lives, so the pty is never the thing that blocks. That makes the
live screen readable while the program runs, so the tests can wait on what
they actually mean:

the first frame, rather than 600ms, before typing anything — raw mode is
enabled before the first draw, so a frame is proof that keys reach the
program and not the terminal's line editor;
the frame that shows the last key landed, rather than a few hundred ms;
the dashboard's own message that the editor round trip finished.

Matching for a wait ignores whitespace, because ratatui writes only the
cells that changed and a blank that was already blank never reaches the
stream. Assertions keep their exact text: no assertion is weakened, and a
wait that gives up only makes the assertion that follows fail sooner, with
the screen attached. Verified by breaking the abort message and watching
the test fail on it.

One wall-clock wait remains, in
the_dashboards_own_write_does_not_raise_the_banner. Its claim is an
absence, and an absence has nothing to wait for; it now sleeps one tip-poll
period of the program's own rather than a guess about the machine, and says
so.

The file went from 33.8s to 2.8s, and from flaky to 12/12 consecutive green
under the conditions that used to make it fail every time.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

Migrated from git-collab patche 4a2dba54547c0c6d3971ef0543cca2d71c393f6a.

Original author: a73x dev@a73x.sh.

Fixes #82

Revision 1: b22b1de446821c32bf56777cc500aaa4f1952e54; base 65e0e8219efb7ddddcac8ff44b7499bc806d19aa

Drain the pty, and wait on the screen instead of the clock `tests/tui_review_test.rs` failed intermittently, always on the two `$EDITOR` tests, always by hanging to the harness deadline rather than by failing an assertion. It was neither a race against a slow machine nor anything in the TUI. **The cause.** `run_dashboard_driven` collected the child's output with `wait_with_output()`, which reads nothing until the child exits. `script` copies the pty master to its stdout in a single loop, so when that write blocks it also stops reading its own stdin — keystrokes stop being forwarded, the `q` never arrives, and the dashboard sits in `event::poll` until the deadline kills it. Caught in the act, `script` is in state `anon_pipe_write`. What made it intermittent is that a pipe's capacity is not a constant. Linux gives a new pipe 64 KiB only while the user is under `fs/pipe-user-pages-soft`; over it, new pipes get one page's worth — 8 KiB. A machine running a test suite crosses that line continually. Sampling a fresh pipe every 200ms through one run of this file gave 8 KiB on 71 of 200 samples. 8 KiB is exactly where these tests sit. The dashboard has written ~4.9 KiB by the time the patch detail pane is up; suspending to `$EDITOR` leaves the alternate screen and returns to a full repaint, taking it to ~9.3 KiB. Only the editor tests cross the line, which is why only they failed. The limit is per-*user*, so `--test-threads=1` does not avoid it and another process of the same user can push a test over — which is what "order-dependent" really was. This also explains the two things that did not fit a timing story: a wider sleep budget cannot help, because the bytes are already written before the sleep begins; and an unrelated change to how the pane draws can appear to fix it, by moving the byte count back under the line until the next change moves it over again. Proved by A/B on unmodified `main` with pipe capacity as the only variable: held over the soft limit, 3/3 runs failed on exactly those two tests; the fix passed 12/12 under the same pressure. **The fix.** Read stdout and stderr on background threads for as long as the child lives, so the pty is never the thing that blocks. That makes the live screen readable while the program runs, so the tests can wait on what they actually mean: - the first frame, rather than 600ms, before typing anything — raw mode is enabled before the first draw, so a frame is proof that keys reach the program and not the terminal's line editor; - the frame that shows the last key landed, rather than a few hundred ms; - the dashboard's own message that the editor round trip finished. Matching for a wait ignores whitespace, because ratatui writes only the cells that changed and a blank that was already blank never reaches the stream. Assertions keep their exact text: no assertion is weakened, and a wait that gives up only makes the assertion that follows fail sooner, with the screen attached. Verified by breaking the abort message and watching the test fail on it. One wall-clock wait remains, in `the_dashboards_own_write_does_not_raise_the_banner`. Its claim is an absence, and an absence has nothing to wait for; it now sleeps one tip-poll period of the program's own rather than a guess about the machine, and says so. The file went from 33.8s to 2.8s, and from flaky to 12/12 consecutive green under the conditions that used to make it fail every time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- Migrated from git-collab patche `4a2dba54547c0c6d3971ef0543cca2d71c393f6a`. Original author: a73x <dev@a73x.sh>. Fixes #82 Revision 1: `b22b1de446821c32bf56777cc500aaa4f1952e54`; base `65e0e8219efb7ddddcac8ff44b7499bc806d19aa`