From 222e86bed64db3c2d55f36f4bec6d015fe152e0c Mon Sep 17 00:00:00 2001 From: Evgeny Boger Date: Sat, 13 Jun 2026 14:09:42 +0000 Subject: [PATCH 1/2] fix(run): emit CRLF for the update-available banner on a TTY The "newer image available" banner is printed from a background task on the launch path, which can resolve *after* the interactive session has put the terminal into raw mode (crossterm::enable_raw_mode in the exec path). In raw mode the terminal's ONLCR output post-processing is off, so the bare "\n" from eprintln! is a line feed with no carriage return: the cursor drops a row but keeps its column, staircasing the second line off the end of the first. Extract the banner into a pure `update_banner(cached, remote, is_tty)` helper that terminates each line with CRLF on a TTY (rendering flush-left regardless of raw mode) and plain LF for redirected stderr (no stray carriage returns in logs/pipes). Add unit tests for both paths. Also convert the now-single-arm match to `if let` (silences clippy::single_match). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/agent-vm/src/run.rs | 70 +++++++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 13 deletions(-) diff --git a/crates/agent-vm/src/run.rs b/crates/agent-vm/src/run.rs index 66c4fe8..6f22475 100644 --- a/crates/agent-vm/src/run.rs +++ b/crates/agent-vm/src/run.rs @@ -1745,6 +1745,37 @@ fn shell_escape(s: &str) -> String { mod tests { use super::*; + // ── update_banner ──────────────────────────────────────────── + + #[test] + fn update_banner_uses_crlf_on_tty_so_second_line_doesnt_staircase() { + let b = update_banner("273bc18ea563", "75fd20fd31c2", true); + // Every line ends in CRLF (the trailing newline included), so a + // raw-mode terminal returns the carriage on each line break. + assert!(b.ends_with("\r\n"), "banner must end with CRLF: {b:?}"); + // The crux of the bug: the second line must be immediately preceded + // by a carriage return so it renders at column 0 instead of trailing + // off the end of the first line. + assert!( + b.contains("\r\n==> Run `agent-vm pull`"), + "second line not preceded by CR: {b:?}" + ); + // Both digests are interpolated into the first line. + assert!(b.contains("cached 273bc18ea563, registry 75fd20fd31c2")); + // No bare LF survives: stripping the CRLF pairs leaves no '\n'. + assert!(!b.replace("\r\n", "").contains('\n'), "found a bare LF: {b:?}"); + } + + #[test] + fn update_banner_uses_plain_lf_off_tty() { + let b = update_banner("aaaa", "bbbb", false); + // Redirected stderr must not pick up carriage returns. + assert!(!b.contains('\r'), "redirected stderr must not get CRs: {b:?}"); + // Exactly two lines, each terminated by a single LF. + assert_eq!(b.matches('\n').count(), 2); + assert!(b.contains("cached aaaa, registry bbbb")); + } + #[test] fn shell_escape_handles_simple_and_quoted() { assert_eq!(shell_escape("foo"), "'foo'"); @@ -2248,22 +2279,35 @@ async fn notify_if_update_available(image: &str) { // request's worth of wait. The banner is best-effort — on timeout we // simply stay quiet and continue with the cached image. let probe = tokio::time::timeout(UPDATE_PROBE_BUDGET, check_for_update(image)); - match probe.await { - Ok(Ok(Some(UpdateState::UpdateAvailable { cached, remote }))) => { - eprintln!( - "==> A newer image is available in the registry (cached {cached}, registry {remote})" - ); - eprintln!( - "==> Run `agent-vm pull` to fetch it. Continuing with the cached image." - ); - } - // UpToDate / NotCached: nothing to say. - // Ok(Err)/None: registry unreachable etc. — stay quiet. - // Err(Elapsed): probe exceeded the budget — stay quiet. - _ => {} + // Only UpdateAvailable prints. Everything else stays quiet: + // UpToDate / NotCached — nothing worth saying. + // Ok(Err) / Ok(None) — registry unreachable etc. + // Err(Elapsed) — probe exceeded the budget. + if let Ok(Ok(Some(UpdateState::UpdateAvailable { cached, remote }))) = probe.await { + eprint!("{}", update_banner(&cached, &remote, std::io::stderr().is_terminal())); } } +/// Render the two-line "update available" banner with a line terminator +/// chosen for where it's being written. +/// +/// This banner runs in a background task (see the spawn on the launch path) +/// and can land *after* the interactive session has switched the terminal +/// to raw mode (crossterm::enable_raw_mode in the exec path). In raw mode +/// the terminal's ONLCR output post-processing is off, so a bare `\n` is a +/// line feed with no carriage return: the cursor drops a row but holds its +/// column, staircasing the second line off the end of the first. Emitting an +/// explicit CRLF on a TTY keeps both lines flush-left whether or not raw mode +/// is active. For redirected stderr (`is_tty == false`) we stay with a plain +/// `\n` so logs and pipes don't pick up stray carriage returns. +fn update_banner(cached: &str, remote: &str, is_tty: bool) -> String { + let nl = if is_tty { "\r\n" } else { "\n" }; + format!( + "==> A newer image is available in the registry (cached {cached}, registry {remote}){nl}\ + ==> Run `agent-vm pull` to fetch it. Continuing with the cached image.{nl}" + ) +} + /// Wall-clock budget for the launch-path update probe. Bounds the worst /// case across all of the probe's registry round-trips so a slow or /// unreachable registry can't stall boot; matches a single request's From a565f233a709461ed67dfc146ca34b930d0b8d1b Mon Sep 17 00:00:00 2001 From: Evgeny Boger Date: Sat, 13 Jun 2026 15:06:24 +0000 Subject: [PATCH 2/2] fix(run): route --auto-publish status lines through the raw-mode-safe terminator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `--auto-publish` "watching" line and per-port event lines (`run.rs`) are emitted from the launch path — the events from a background task during the live, raw-mode session — using bare-`\n` `eprintln!`. Same failure mode as the update banner: under raw mode (ONLCR off) a bare LF doesn't return the carriage, so the line staircases. Extract the CRLF-vs-LF decision into a shared `status_line_ending(is_tty)` helper (the "shared raw-mode-safe writer" the earlier review flagged), have `update_banner` use it, and switch the three auto-publish messages to `eprint!("…{nl}")` with `nl = status_line_ending(stderr().is_terminal())`. The per-event task samples is_terminal() once before its loop — a fd's TTY-ness is stable for the process. Add a unit test for `status_line_ending`. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/agent-vm/src/run.rs | 56 ++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/crates/agent-vm/src/run.rs b/crates/agent-vm/src/run.rs index 6f22475..0df8d68 100644 --- a/crates/agent-vm/src/run.rs +++ b/crates/agent-vm/src/run.rs @@ -981,11 +981,17 @@ pub async fn launch(agent: Agent, args: Args) -> Result { // whether auto-publish is enabled — when disabled the runtime // never emits events, so the subscriber just idles. Cheap. if args.auto_publish { - eprintln!("==> auto-publish: watching guest LISTEN sockets via /proc/net/tcp{{,6}}"); + // These print from a background task during the live (raw-mode) + // session, so terminate each line raw-mode-safely; see + // `status_line_ending`. is_terminal() is stable for the process, so + // the per-event task samples it once before its loop. + let nl = status_line_ending(std::io::stderr().is_terminal()); + eprint!("==> auto-publish: watching guest LISTEN sockets via /proc/net/tcp{{,6}}{nl}"); let sb_for_events = sandbox.clone(); tokio::spawn(async move { let mut events = sb_for_events.port_events().await; use microsandbox::protocol::network::PortEvent; + let nl = status_line_ending(std::io::stderr().is_terminal()); while let Some(event) = events.recv().await { match event { PortEvent::Added { @@ -993,8 +999,8 @@ pub async fn launch(agent: Agent, args: Args) -> Result { host_port, guest_port, } => { - eprintln!( - "==> auto-publish: guest :{guest_port} → host {host_bind}:{host_port}" + eprint!( + "==> auto-publish: guest :{guest_port} → host {host_bind}:{host_port}{nl}" ); } PortEvent::Removed { @@ -1002,8 +1008,8 @@ pub async fn launch(agent: Agent, args: Args) -> Result { host_port, guest_port, } => { - eprintln!( - "==> auto-publish: guest :{guest_port} closed (released {host_bind}:{host_port})" + eprint!( + "==> auto-publish: guest :{guest_port} closed (released {host_bind}:{host_port}){nl}" ); } } @@ -1745,7 +1751,13 @@ fn shell_escape(s: &str) -> String { mod tests { use super::*; - // ── update_banner ──────────────────────────────────────────── + // ── status_line_ending / update_banner ─────────────────────── + + #[test] + fn status_line_ending_is_crlf_on_tty_and_lf_otherwise() { + assert_eq!(status_line_ending(true), "\r\n"); + assert_eq!(status_line_ending(false), "\n"); + } #[test] fn update_banner_uses_crlf_on_tty_so_second_line_doesnt_staircase() { @@ -2288,20 +2300,28 @@ async fn notify_if_update_available(image: &str) { } } -/// Render the two-line "update available" banner with a line terminator -/// chosen for where it's being written. +/// Line terminator for a status line written to stderr from the launch path, +/// given whether stderr is a TTY. /// -/// This banner runs in a background task (see the spawn on the launch path) -/// and can land *after* the interactive session has switched the terminal -/// to raw mode (crossterm::enable_raw_mode in the exec path). In raw mode -/// the terminal's ONLCR output post-processing is off, so a bare `\n` is a -/// line feed with no carriage return: the cursor drops a row but holds its -/// column, staircasing the second line off the end of the first. Emitting an -/// explicit CRLF on a TTY keeps both lines flush-left whether or not raw mode -/// is active. For redirected stderr (`is_tty == false`) we stay with a plain -/// `\n` so logs and pipes don't pick up stray carriage returns. +/// Several launch-path messages are emitted from background tasks — the +/// update-available banner and the `--auto-publish` port events — that can run +/// *after* the interactive session has switched the terminal to raw mode +/// (`crossterm::enable_raw_mode` in the exec path). In raw mode the terminal's +/// ONLCR output post-processing is off, so a bare `\n` is a line feed with no +/// carriage return: the cursor drops a row but holds its column, staircasing +/// the next line off the end of the previous one. A CRLF on a TTY renders +/// flush-left whether or not raw mode is active. For redirected stderr +/// (`is_tty == false`) we keep a plain `\n` so logs and pipes don't pick up +/// stray carriage returns. (Raw mode doesn't change a fd's TTY-ness, so the +/// choice is stable for the life of the process.) +fn status_line_ending(is_tty: bool) -> &'static str { + if is_tty { "\r\n" } else { "\n" } +} + +/// Render the two-line "update available" banner, each line terminated by +/// [`status_line_ending`] so both render flush-left even under raw mode. fn update_banner(cached: &str, remote: &str, is_tty: bool) -> String { - let nl = if is_tty { "\r\n" } else { "\n" }; + let nl = status_line_ending(is_tty); format!( "==> A newer image is available in the registry (cached {cached}, registry {remote}){nl}\ ==> Run `agent-vm pull` to fetch it. Continuing with the cached image.{nl}"