From c0366cd143fd316d8502f8964ce161e1db447ac7 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:05:51 +0800 Subject: [PATCH] fix(cli): name the limit of the empty-segment fix, and settle the captures the e2e compares (#841) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI found both new end-to-end cases red on Linux and macOS while green on Windows. One was a bad assumption in the test; the other was the test telling me the fix is narrower than the first commit claimed. The real finding is the resize case. On Unix a resize raises SIGWINCH and the shell repaints its prompt, so the segment the resize opens is *not* empty — it holds the repaint. Dropping byte-less segments therefore leaves the newest non-empty segment being a bare prompt, with the pane's output still stranded in the segment sealed behind it: on CI the default form came back as 174 bytes of prompt escapes and no marker. On Windows nothing answers the resize, the segment stays empty, and the fix reaches past it to the output — which is why the assertion passed there. It was asserting an accident of the platform. So the fix stands but is smaller than "a resize no longer costs you the pane's output": it makes the zero-byte answer impossible, and that is all. It cannot do more. Nothing in the byte stream distinguishes a prompt repaint from output the pane meant, so no client-side rule can tell which side of the boundary the answer is on. The boundary is the flaw — the default form's unit is the last resize, an event in the window rather than in the pane — and moving it means redefining what the default returns (the last screenful of the ring, say), which would shrink what every caller with a never-resized pane gets today. Left alone, and said plainly instead: in `what_was_asked_for`'s doc comment, and in a warning in the CLI reference telling anyone reading a pane under the GUI to ask for `--scrollback`. The test now asserts what the fix actually guarantees on every platform — the default form answers with bytes rather than with the resize's placeholder, and it is the end of what `--scrollback` returns, which is what would catch a fix reaching for the wrong segment. The marker is pinned against `--scrollback`, the form that promises to hold it. The `--tail` failure was a race in the test, not in `--tail`: the whole and the tail were separate calls and the pane advanced between them, so the tail carried a prompt line the whole capture had not caught up to. Both cases now read the whole answer on either side of the others and require the two readings to match before comparing anything, which is what makes the comparison a statement about the code rather than about the moment. Both also read `--scrollback` now, so neither depends on where a segment boundary happens to fall, and neither pins exact pane content — the macOS runner prints a zsh banner into the pane. Claude-Session: https://claude.ai/code/session_01UUyWQXzcBAoBzaSX8pc7nU --- crates/tty7-cli/src/backend/real.rs | 12 +++ crates/tty7-cli/tests/cli_e2e.rs | 145 +++++++++++++++++++--------- docs/cli/reference.mdx | 13 +++ 3 files changed, 124 insertions(+), 46 deletions(-) diff --git a/crates/tty7-cli/src/backend/real.rs b/crates/tty7-cli/src/backend/real.rs index e23d4d8c..9447eac6 100644 --- a/crates/tty7-cli/src/backend/real.rs +++ b/crates/tty7-cli/src/backend/real.rs @@ -301,6 +301,18 @@ impl Backend for RealBackend { /// keeps `--scrollback` and the default describing the same bytes. The daemon /// keeps sending them — its trailing `Size` is how an attaching client learns /// the pane's current geometry, which is not ours to take away from here. +/// +/// This makes the zero-byte answer impossible; it does not make the default +/// form robust to a resize, and it cannot. On Unix a resize raises SIGWINCH and +/// the shell repaints its prompt into the new segment, so the newest segment is +/// no longer empty — it holds the repaint, and the output is still stranded in +/// the segment sealed behind it. Nothing in the byte stream distinguishes a +/// prompt repaint from output the pane meant, so no rule here can tell which +/// side of the boundary the answer is on. The boundary itself is the flaw: the +/// default form's unit is the last resize, an event in the window rather than +/// in the pane. Moving it means redefining what the default returns — the last +/// screenful of the whole ring, say — which would shrink what every caller with +/// a never-resized pane gets today, so it is left alone and documented instead. fn what_was_asked_for(segments: Vec, scrollback: bool) -> Vec { let mut segments: Vec = segments .into_iter() diff --git a/crates/tty7-cli/tests/cli_e2e.rs b/crates/tty7-cli/tests/cli_e2e.rs index 7353a402..129514f7 100644 --- a/crates/tty7-cli/tests/cli_e2e.rs +++ b/crates/tty7-cli/tests/cli_e2e.rs @@ -837,14 +837,29 @@ fn capture_plain_returns_text_not_escapes(daemon: &Daemon) { /// A resize must not empty `capture`. /// -/// The daemon's replay ring seals its segment on every resize and opens an -/// empty one at the new geometry, and it replays every segment it holds. The -/// default form keeps "the newest segment", which was that empty placeholder -/// for any pane resized since it last printed — so `capture` answered a live +/// The daemon's replay ring seals its segment on every resize and opens a new +/// one at the new geometry, and it replays every segment it holds. The default +/// form keeps "the newest segment", which on a pane that has printed nothing +/// since the resize is the empty placeholder — so `capture` answered a live /// pane with zero bytes and exit `0`, indistinguishable from a blank one -/// (#841). Nothing about the pane changes here between the two captures except -/// its size, which is what makes the assertion a statement about the replay -/// rather than about timing. +/// (#841). Dropping byte-less segments is what fixes that. +/// +/// What is asserted here is shaped by how much of that is portable. On Unix a +/// resize raises SIGWINCH and the shell repaints its prompt, so the segment the +/// resize opened is *not* empty — it holds the repaint, and the newest +/// non-empty segment is that prompt rather than the one holding the pane's +/// output. On Windows nothing answers the resize, the segment stays empty, and +/// the fix reaches back to the output. So "the default form still carries the +/// marker" is true on one platform and false on the other for reasons that have +/// nothing to do with the fix, and asserting it would be asserting an accident. +/// +/// What holds everywhere is the pair the fix actually guarantees: the default +/// form comes back with bytes rather than with the resize's placeholder, and it +/// is the end of what `--scrollback` returns — that second one is what would +/// catch a fix reaching for the wrong segment. The marker itself is pinned +/// against `--scrollback`, the form that promises to hold it. The segment +/// picking is pinned exactly, on every platform, by the `what_was_asked_for` +/// tests in `backend/real.rs`. fn capture_still_answers_after_a_resize(daemon: &Daemon) { let mut pane = PaneClient::at(daemon.pane_endpoint()) .spawn( @@ -866,14 +881,14 @@ fn capture_still_answers_after_a_resize(daemon: &Daemon) { let deadline = Instant::now() + SETTLE_WITHIN; loop { if daemon - .run_ok(&["capture", &address]) + .run_ok(&["capture", &address, "--scrollback"]) .contains("tty7_e2e_resize_marker") { break; } assert!( Instant::now() < deadline, - "the marker never reached the pane's newest segment" + "the marker never reached the pane's replay" ); std::thread::sleep(Duration::from_millis(200)); } @@ -885,39 +900,66 @@ fn capture_still_answers_after_a_resize(daemon: &Daemon) { cell_h: 16, }) .expect("resize the pane"); - // Long enough for the daemon to have sealed the segment, and for any - // repaint the shell answers a resize with to have landed either way. - std::thread::sleep(Duration::from_millis(1500)); - for form in [ - vec!["capture", &address], - vec!["capture", &address, "--plain"], - vec!["capture", &address, "--scrollback"], - ] { - let out = daemon.run_json(&form); - let text = out["text"].as_str().unwrap_or_default(); + // Each capture below is its own call, so a pane still moving would have + // them disagree for reasons that are not the fix. Reading the whole ring on + // either side of the others and requiring the two readings to match is what + // says the pane held still while they were taken. + loop { + let before = daemon.run_json(&["capture", &address, "--scrollback"]); + let newest = daemon.run_json(&["capture", &address]); + let plain = daemon.run_ok(&["capture", &address, "--plain", "--scrollback"]); + let after = daemon.run_json(&["capture", &address, "--scrollback"]); + + let whole = before["text"].as_str().unwrap_or_default(); + let newest_text = newest["text"].as_str().unwrap_or_default(); + let held_still = before["text"] == after["text"] + && whole.contains("tty7_e2e_resize_marker") + && plain.contains("tty7_e2e_resize_marker"); + if held_still { + assert!( + !newest_text.is_empty(), + "the default form answered with the empty segment the resize \ + opened instead of the newest one holding output: {newest}" + ); + assert!( + newest["bytes"].as_u64().is_some_and(|n| n > 0), + "a capture that carried text has to report the bytes it \ + carried: {newest}" + ); + assert!( + whole.ends_with(newest_text), + "the newest segment has to be the end of the ring it came from, \ + or the default form is answering with some other segment:\n\ + newest: {newest_text:?}\nwhole: {whole:?}" + ); + let _ = pane.detach(); + return; + } assert!( - text.contains("tty7_e2e_resize_marker"), - "tty7 {form:?} lost the pane's output to the empty segment the \ - resize opened: {out}" - ); - assert!( - out["bytes"].as_u64().is_some_and(|n| n > 0), - "a capture that carried text has to report the bytes it carried: {out}" + Instant::now() < deadline, + "the pane never held still after the resize; last ring was:\n{whole}" ); + std::thread::sleep(Duration::from_millis(200)); } - - let _ = pane.detach(); } /// `--tail` against a real pane, whose replay carries a shell prompt, escapes /// and CRLF rather than the tidy fixtures the unit tests craft. /// /// The contract is only "the last N lines of what the command would have -/// printed", so what is pinned is that the tail is a suffix of the whole answer, -/// that it holds the marker the pane printed last, and that it is shorter than -/// what it was cut from — not the exact line count, which depends on how the -/// test machine's shell decorates its prompt. +/// printed", so what is pinned is that the tail is the end of the whole answer, +/// that it holds the marker the pane printed last, and that it dropped the one +/// the pane printed first — not the exact line count, which depends on how the +/// test machine's shell decorates its prompt, and on macOS on the zsh banner +/// the runner's bash prints at startup. +/// +/// The tail and the whole are separate calls, so they have to be taken while +/// the pane is holding still or they describe different moments — which is what +/// made the first version of this test flake on CI, with a tail carrying a +/// prompt line the whole capture had not caught up to yet. Reading the whole +/// answer on either side of the tail and requiring the two to match is what +/// makes the comparison a statement about `--tail`. fn capture_tail_trims_a_real_panes_answer(daemon: &Daemon) { let created = daemon.run_json(&["new", &workdir()]); let pane = created["pane"].as_u64().expect("new prints the pane id"); @@ -930,34 +972,45 @@ fn capture_tail_trims_a_real_panes_answer(daemon: &Daemon) { let deadline = Instant::now() + SETTLE_WITHIN; loop { - let whole = daemon.run_ok(&["capture", &address, "--plain"]); - let tail = daemon.run_ok(&["capture", &address, "--plain", "--tail", "2"]); - let settled = whole.contains("tty7_e2e_tail_line_6") - && tail.contains("tty7_e2e_tail_line_6") - && whole.contains("tty7_e2e_tail_line_1"); + let before = daemon.run_ok(&["capture", &address, "--plain", "--scrollback"]); + let tail = daemon.run_json(&[ + "capture", + &address, + "--plain", + "--scrollback", + "--tail", + "2", + ]); + let after = daemon.run_ok(&["capture", &address, "--plain", "--scrollback"]); + + let tail_text = tail["text"].as_str().unwrap_or_default(); + let settled = before == after + && before.contains("tty7_e2e_tail_line_1") + && before.contains("tty7_e2e_tail_line_6") + && tail_text.contains("tty7_e2e_tail_line_6"); if settled { assert!( - whole.trim_end().ends_with(tail.trim_end()), - "a tail has to be the end of the answer it was cut from:\ntail: {tail:?}\nwhole: {whole:?}" + before.trim_end().ends_with(tail_text.trim_end()), + "a tail has to be the end of the answer it was cut from:\n\ + tail: {tail_text:?}\nwhole: {before:?}" ); assert!( - !tail.contains("tty7_e2e_tail_line_1"), - "two lines cannot still hold the first of six:\n{tail:?}" + !tail_text.contains("tty7_e2e_tail_line_1"), + "two lines cannot still hold the first of six:\n{tail_text:?}" ); // The byte count stays the size of the replay, not of the tail — // that is what says a tail was taken rather than a short capture. - let json = daemon.run_json(&["capture", &address, "--plain", "--tail", "2"]); assert!( - json["bytes"] + tail["bytes"] .as_u64() - .is_some_and(|n| n as usize > tail.len()), - "--tail must not shrink the reported replay size: {json}" + .is_some_and(|n| n as usize > tail_text.len()), + "--tail must not shrink the reported replay size: {tail}" ); return; } assert!( Instant::now() < deadline, - "the six echoes never settled; last capture was:\n{whole}" + "the six echoes never settled; last capture was:\n{before}" ); std::thread::sleep(Duration::from_millis(200)); } diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index 1890fbe2..9b7b81b4 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -138,6 +138,19 @@ The pane's replay. Three independent choices: `--scrollback`. The ring splits into segments on resize, so for a pane that was never resized the two are identical. + + The default form is bounded by the pane's last **resize**, which is an event in + the window rather than in the pane's output. A resize seals the segment holding + everything printed so far and opens a new one, and on Unix the shell answers + the SIGWINCH by repainting its prompt — so straight after a resize the newest + segment can hold that repaint and nothing else, while the command's output sits + in the segment behind it. Nothing in the byte stream tells a repaint apart from + real output, so `capture` cannot decide this for you. When you are reading a + pane whose window may have changed size — anything under the GUI — ask for + `--scrollback`, and reach for `--tail N` if you wanted the tail rather than the + ring. + + **In what form** — without `--plain`, the stored bytes with ANSI escapes intact, decoded as UTF-8 (invalid bytes become U+FFFD). With `--plain`, those bytes replayed through a terminal grid and printed as the text they produced.