diff --git a/AGENTS.md b/AGENTS.md index d36b270368..463de67281 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -475,6 +475,33 @@ The patched llama.cpp staged runtime has its own ABI version, tracked in `skippy For changes in `crates/mesh-llm-ui/`, use components and compose interfaces consistently with shadcn/ui patterns. Prefer extending existing primitives in `src/components/ui/` over ad-hoc markup. +### Terminal dashboard integrity + +The dashboard renders to the **controlling terminal** (`/dev/tty`, `CONOUT$`), +never to fd 1 or fd 2, and while it owns the screen those two descriptors are +redirected into it (`crates/mesh-llm-tui/src/output/console_capture.rs`). Stray +`println!`/`eprintln!`, inherited child stderr, and native llama.cpp output +therefore arrive as dashboard events instead of painting over the frame. + +Consequences worth knowing before changing this area: + +- Do not point the TUI backend, or the enter/exit escape writers, at + `io::stderr()`. Capture would then redirect the dashboard into its own pipe. +- Anything restoring the terminal (panic hooks, emergency writers) must release + the capture *before* writing, or the message goes into the pipe. +- Capture cannot intercept a write to the tty by a process that did not inherit + our descriptors. `R` is the repair for that case: a physical clear plus a + ratatui diff invalidation. A logical `Clear` widget does not fix backend + desynchronization. +- Do not perform that clear on every draw. It repaints from blank and reads as + a black blink; measured on an idle dashboard it was ~2.6 full repaints/s. + +Prefer `OutputEvent` or `tracing` in runtime code regardless — captured lines +have no level and are shown with a `stdout` context. When adding a `tracing` +target that must reach the dashboard, add a directive for it in +`runtime_tracing_subscriber`: `EnvFilter::from_default_env()` defaults to ERROR, +so an unlisted target's `warn!` is dropped before the writer sees it. + ## Testing Read `docs/design/TESTING.md` before running tests. It has all test scenarios, remote deploy instructions, and cleanup commands. diff --git a/Cargo.lock b/Cargo.lock index abfb73a633..9fbe432f60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4296,6 +4296,7 @@ dependencies = [ "crossterm 0.28.1", "mesh-llm-events", "ratatui", + "rustix 1.1.4", "serde_json", "tokio", "tracing", diff --git a/crates/mesh-llm-host-runtime/src/runtime/tracing_writer.rs b/crates/mesh-llm-host-runtime/src/runtime/tracing_writer.rs index 670de6a287..f82ee103d0 100644 --- a/crates/mesh-llm-host-runtime/src/runtime/tracing_writer.rs +++ b/crates/mesh-llm-host-runtime/src/runtime/tracing_writer.rs @@ -318,8 +318,11 @@ pub(super) fn init_runtime_tracing() -> Result<()> { pub(super) fn init_embedded_runtime_tracing() -> Result<()> { let subscriber = runtime_tracing_subscriber()?; if let Err(err) = tracing::subscriber::set_global_default(subscriber) { - eprintln!( - "mesh-llm embedded runtime using existing tracing subscriber; could not install mesh-llm subscriber: {err}" + // The host already had a subscriber, so this warning has somewhere to + // go. Writing it raw would put it straight onto the dashboard frame. + tracing::warn!( + error = %err, + "mesh-llm embedded runtime using existing tracing subscriber" ); } Ok(()) diff --git a/crates/mesh-llm-tui/Cargo.toml b/crates/mesh-llm-tui/Cargo.toml index 7dbf90ced8..436efb4c4d 100644 --- a/crates/mesh-llm-tui/Cargo.toml +++ b/crates/mesh-llm-tui/Cargo.toml @@ -24,3 +24,8 @@ ratatui = "0.30" serde_json.workspace = true tokio = { version = "1", features = ["macros", "rt", "sync", "time"] } tracing = "0.1" + +[target.'cfg(unix)'.dependencies] +# Safe wrappers for the dup/dup2/pipe plumbing behind console capture. Used in +# preference to raw libc so this crate keeps `#![forbid(unsafe_code)]`. +rustix = { version = "1", features = ["event", "stdio"] } diff --git a/crates/mesh-llm-tui/src/output/console_capture.rs b/crates/mesh-llm-tui/src/output/console_capture.rs new file mode 100644 index 0000000000..17f7080ccc --- /dev/null +++ b/crates/mesh-llm-tui/src/output/console_capture.rs @@ -0,0 +1,504 @@ +//! Redirects the process's stdout and stderr into the dashboard while it owns +//! the screen. +//! +//! Converting individual `println!`/`eprintln!` call sites cannot close this +//! hole, because the writers are not all ours: `plugin/runtime.rs` hands +//! spawned plugins `Stdio::inherit()`, the staged llama.cpp runtime is C, and +//! third-party crates print whatever they like. All of them write to fd 1 or +//! fd 2, so that is where the interception belongs. +//! +//! While installed, fd 1 and fd 2 point at a pipe. A reader thread turns each +//! line into an `OutputEvent`, so stray output shows up as a dashboard event +//! instead of painting over the frame. On restore the original descriptors are +//! put back and any still-buffered lines are written to the real stderr so +//! nothing is silently swallowed. If a child keeps an old pipe open across a +//! later dashboard session, that stale reader follows the current capture +//! state and routes its output into the new dashboard. +//! +//! This is only safe because the dashboard renders to the controlling terminal +//! (see [`super::terminal_out`]) rather than to fd 2; installing capture while +//! the dashboard still rendered to stderr would redirect the dashboard into its +//! own pipe. + +#[cfg(unix)] +pub(in crate::output) use unix::ConsoleCapture; + +#[cfg(not(unix))] +pub(in crate::output) use fallback::ConsoleCapture; + +#[cfg(unix)] +mod unix { + use mesh_llm_events::OutputEvent; + use rustix::event::{PollFd, PollFlags, poll}; + use rustix::io::fcntl_dupfd_cloexec; + use rustix::stdio::{dup2_stderr, dup2_stdout, stderr, stdout}; + use std::fs::File; + use std::io::{self, PipeReader, Read, Write}; + use std::os::fd::OwnedFd; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::time::Duration; + + /// How long a partial line may sit unflushed before it is shown anyway. + const IDLE_FLUSH: Duration = Duration::from_millis(150); + + /// Lines longer than this are split rather than buffered without bound, so + /// a child spewing bytes with no newline cannot grow the reader's buffer + /// until the process dies. + const MAX_CAPTURED_LINE_BYTES: usize = 8 * 1024; + + /// Readers can outlive the dashboard session that spawned them when a + /// child inherits a pipe write end, so delivery follows global activity. + static ACTIVE_CAPTURES: AtomicUsize = AtomicUsize::new(0); + + pub(in crate::output) struct ConsoleCapture { + saved_stdout: OwnedFd, + saved_stderr: OwnedFd, + active: bool, + } + + impl ConsoleCapture { + /// Point fd 1 and fd 2 at a pipe drained into the dashboard. + pub(in crate::output) fn install() -> io::Result { + // Anything already buffered belongs on the real terminal, not in + // the pipe we are about to install. + let _ = io::stdout().flush(); + let _ = io::stderr().flush(); + + // Both pipe ends are close-on-exec. `dup2` clears that flag on the + // descriptor it installs, so children still inherit the redirected + // fd 1/2 while the read end stays private to this process. + let (read_fd, write_fd) = io::pipe()?; + + let saved_stdout = fcntl_dupfd_cloexec(stdout(), 0)?; + let saved_stderr = fcntl_dupfd_cloexec(stderr(), 0)?; + // The reader keeps its own handle on the real stderr so it can + // still deliver lines after the dashboard goes away. + let reader_stderr = fcntl_dupfd_cloexec(stderr(), 0)?; + + // Start the reader before redirecting either descriptor. Once the + // capture is registered, stale readers from earlier children also + // route into the active dashboard. + register_capture_reader(&ACTIVE_CAPTURES, || spawn_reader(read_fd, reader_stderr))?; + + if let Err(error) = dup2_stdout(&write_fd) { + unregister_capture(&ACTIVE_CAPTURES); + return Err(error.into()); + } + if let Err(error) = dup2_stderr(&write_fd) { + // stdout was already redirected. Restore it before the pipe + // handles drop so a partial install cannot strand fd 1. + let _ = dup2_stdout(&saved_stdout); + unregister_capture(&ACTIVE_CAPTURES); + return Err(error.into()); + } + drop(write_fd); + + Ok(Self { + saved_stdout, + saved_stderr, + active: true, + }) + } + + /// Put the original descriptors back. Safe to call more than once. + pub(in crate::output) fn restore(&mut self) -> io::Result<()> { + if !self.active { + return Ok(()); + } + let _ = io::stdout().flush(); + let _ = io::stderr().flush(); + dup2_stdout(&self.saved_stdout)?; + dup2_stderr(&self.saved_stderr)?; + // Keep restoration retryable until both descriptors are back. + self.active = false; + unregister_capture(&ACTIVE_CAPTURES); + Ok(()) + } + } + + impl Drop for ConsoleCapture { + fn drop(&mut self) { + let _ = self.restore(); + } + } + + /// The reader is deliberately detached rather than joined. A plugin child + /// that inherited the write end keeps the pipe open past restore, so a join + /// could block shutdown indefinitely; the thread instead exits on EOF + /// whenever that arrives, and dies with the process at worst. + fn spawn_reader(mut read_fd: PipeReader, original_stderr: OwnedFd) -> io::Result<()> { + std::thread::Builder::new() + .name("mesh-console-capture".to_string()) + .spawn(move || { + let mut passthrough = File::from(original_stderr); + let mut pending = Vec::new(); + let mut buffer = [0u8; 4096]; + // Wait for data, but not forever: a writer that emitted a + // partial line and then went quiet (`print!`, a `\r` progress + // counter) must still be shown rather than sit in this buffer + // until the next newline arrives. + while let Ok(readable) = wait_for_input(&read_fd, IDLE_FLUSH) { + if !readable { + for line in take_pending_lines(&mut pending, true) { + deliver(line, &mut passthrough); + } + continue; + } + match read_fd.read(&mut buffer) { + Ok(0) | Err(_) => break, + Ok(count) => pending.extend_from_slice(&buffer[..count]), + } + // A writer with no line breaks at all must not grow this + // buffer without bound. + let force = pending.len() >= MAX_CAPTURED_LINE_BYTES; + for line in take_pending_lines(&mut pending, force) { + deliver(line, &mut passthrough); + } + } + for line in take_pending_lines(&mut pending, true) { + deliver(line, &mut passthrough); + } + }) + .map(|_| ()) + } + + pub(super) fn register_capture_reader( + active_captures: &AtomicUsize, + spawn_reader: impl FnOnce() -> io::Result<()>, + ) -> io::Result<()> { + active_captures.fetch_add(1, Ordering::AcqRel); + if let Err(error) = spawn_reader() { + unregister_capture(active_captures); + return Err(error); + } + Ok(()) + } + + fn unregister_capture(active_captures: &AtomicUsize) { + let result = active_captures.fetch_update(Ordering::AcqRel, Ordering::Acquire, |count| { + count.checked_sub(1) + }); + debug_assert!(result.is_ok(), "capture activity count underflowed"); + } + + fn dashboard_capture_active(active_captures: &AtomicUsize) -> bool { + active_captures.load(Ordering::Acquire) > 0 + } + + fn deliver(text: String, passthrough: &mut File) { + deliver_with( + text, + &ACTIVE_CAPTURES, + passthrough, + mesh_llm_events::emit_event, + ); + } + + pub(super) fn deliver_with( + text: String, + active_captures: &AtomicUsize, + passthrough: &mut W, + emit: E, + ) where + W: Write, + E: FnOnce(OutputEvent) -> io::Result<()>, + { + if text.trim().is_empty() { + return; + } + if dashboard_capture_active(active_captures) { + // Failure here means the dashboard sink is gone; fall back to the + // real terminal rather than dropping the line. + if emit(dashboard_event(text.clone())).is_ok() { + return; + } + } + let _ = writeln!(passthrough, "{text}"); + } + + /// Split `pending` into displayable lines. + /// + /// `\r` terminates a line as well as `\n` so carriage-return progress + /// counters surface instead of accumulating. When `flush_remainder` is set + /// the trailing partial line is taken too. + pub(super) fn take_pending_lines(pending: &mut Vec, flush_remainder: bool) -> Vec { + let mut lines = Vec::new(); + while let Some(index) = pending + .iter() + .position(|byte| *byte == b'\n' || *byte == b'\r') + { + let mut line: Vec = pending.drain(..=index).collect(); + line.pop(); + lines.push(decode(&line)); + } + if flush_remainder && !pending.is_empty() { + let remainder = std::mem::take(pending); + lines.push(decode(&remainder)); + } + lines + } + + fn decode(line: &[u8]) -> String { + // Captured bytes are arbitrary. A raw escape sequence rendered into a + // dashboard cell would move the cursor and corrupt the very frame this + // exists to protect, so control characters are stripped here. + String::from_utf8_lossy(line) + .chars() + .filter_map(|character| match character { + // A tab is alignment, not damage — llama.cpp's loader lines are + // full of them. Dropping it would run two columns together, so + // it degrades to a space instead. + '\t' => Some(' '), + character if character.is_control() => None, + character => Some(character), + }) + .collect::() + .trim_end() + .to_string() + } + + /// Block until the pipe has data or `timeout` elapses. `Ok(false)` is a + /// timeout, `Err` means the descriptor is unusable and the reader stops. + fn wait_for_input(read_fd: &PipeReader, timeout: Duration) -> io::Result { + let mut fds = [PollFd::new(read_fd, PollFlags::IN)]; + let timeout = timeout.try_into().map_err(io::Error::other)?; + loop { + match poll(&mut fds, Some(&timeout)) { + Ok(0) => return Ok(false), + Ok(_) => return Ok(true), + Err(rustix::io::Errno::INTR) => continue, + Err(err) => return Err(err.into()), + } + } + } + + /// Captured output has no level of its own. Anything that looks like a + /// complaint is surfaced as a warning so it is not lost among info rows; + /// the `stdout` context tells the reader it was intercepted rather than + /// emitted through the normal event path. + pub(super) fn dashboard_event(message: String) -> OutputEvent { + let lowered = message.to_ascii_lowercase(); + let looks_like_a_problem = ["error", "warn", "failed", "panic"] + .iter() + .any(|needle| lowered.contains(needle)); + let context = Some("stdout".to_string()); + if looks_like_a_problem { + OutputEvent::Warning { message, context } + } else { + OutputEvent::Info { message, context } + } + } +} + +#[cfg(not(unix))] +mod fallback { + use std::io; + + /// Descriptor-level capture is POSIX-specific. On other platforms the + /// dashboard still renders to the controlling terminal, and stray output + /// remains repairable with the `R` key. + pub(in crate::output) struct ConsoleCapture; + + impl ConsoleCapture { + pub(in crate::output) fn install() -> io::Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "console capture requires a POSIX platform", + )) + } + + pub(in crate::output) fn restore(&mut self) -> io::Result<()> { + Ok(()) + } + } +} + +#[cfg(all(test, unix))] +mod tests { + use super::unix::{dashboard_event, deliver_with, register_capture_reader, take_pending_lines}; + use mesh_llm_events::OutputEvent; + use std::io::{Read, Write}; + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::mpsc; + + fn split(input: &[u8], flush_remainder: bool) -> Vec { + let mut pending = input.to_vec(); + take_pending_lines(&mut pending, flush_remainder) + } + + #[test] + fn captured_output_splits_on_newlines() { + assert_eq!( + split(b"first\nsecond\nthird\n", false), + vec!["first", "second", "third"] + ); + } + + #[test] + fn captured_output_holds_a_partial_line_until_it_is_flushed() { + // Mid-write: the rest of the line may still be coming. + assert_eq!(split(b"done\npartial", false), vec!["done"]); + // The writer went quiet, so show it rather than wait forever. This is + // the `print!`-with-no-newline case, which otherwise never surfaces. + assert_eq!(split(b"done\npartial", true), vec!["done", "partial"]); + } + + #[test] + fn captured_output_treats_carriage_returns_as_line_ends() { + // llama.cpp-style progress counters overwrite one line with `\r`. + assert_eq!( + split(b"loading 10%\rloading 20%\r", false), + vec!["loading 10%", "loading 20%"] + ); + } + + #[test] + fn captured_output_strips_control_characters() { + // The whole point is to keep stray bytes off the frame. An escape + // sequence rendered into a dashboard cell would move the cursor and + // corrupt the frame this exists to protect. + assert_eq!( + split(b"\x1b[10;5HXXXX\n", false), + vec!["[10;5HXXXX"], + "escape bytes must not survive into a rendered cell" + ); + } + + #[test] + fn captured_output_keeps_tabs_as_spaces() { + // llama.cpp separates its loader columns with tabs. Stripping them as + // control characters would run the columns together. + assert_eq!( + split(b"llm_load_print_meta: n_ctx\t= 4096\n", false), + vec!["llm_load_print_meta: n_ctx = 4096"] + ); + } + + #[test] + fn captured_output_is_classified_by_what_it_says() { + assert!(matches!( + dashboard_event("llama_model_loader: loaded meta data".to_string()), + OutputEvent::Info { .. } + )); + assert!(matches!( + dashboard_event("ggml_cuda_init: failed to initialise".to_string()), + OutputEvent::Warning { .. } + )); + } + + #[test] + fn captured_output_is_labelled_as_intercepted() { + let OutputEvent::Info { context, .. } = dashboard_event("plain line".to_string()) else { + panic!("expected an info event"); + }; + assert_eq!( + context.as_deref(), + Some("stdout"), + "the dashboard should show that this line was intercepted, not emitted" + ); + } + + #[test] + fn stale_reader_routes_child_output_to_reentered_dashboard() { + let active_captures = AtomicUsize::new(1); + let (mut read_fd, write_fd) = std::io::pipe().expect("pipe should open"); + let (child_ready_tx, child_ready_rx) = mpsc::channel(); + let (write_tx, write_rx) = mpsc::channel(); + let child = std::thread::spawn(move || { + let mut retained_write_fd = write_fd; + child_ready_tx + .send(()) + .expect("parent should wait for retained pipe"); + write_rx.recv().expect("parent should release child writer"); + retained_write_fd + .write_all(b"late child output\n") + .expect("child output should write"); + }); + + child_ready_rx + .recv() + .expect("child should retain the first pipe write end"); + active_captures.fetch_sub(1, Ordering::AcqRel); + active_captures.fetch_add(1, Ordering::AcqRel); + write_tx.send(()).expect("child should still be waiting"); + + let mut captured = String::new(); + read_fd + .read_to_string(&mut captured) + .expect("reader should receive late child output"); + child.join().expect("child writer should exit"); + + let mut emitted = Vec::new(); + let mut passthrough = Vec::new(); + deliver_with( + captured.trim_end().to_string(), + &active_captures, + &mut passthrough, + |event| { + emitted.push(event); + Ok(()) + }, + ); + + assert_eq!(emitted.len(), 1, "late output belongs in the new dashboard"); + assert!(passthrough.is_empty(), "it must not bypass the dashboard"); + } + + #[test] + fn stale_reader_routes_output_during_reader_registration() { + let active_captures = Arc::new(AtomicUsize::new(0)); + let (deliver_tx, deliver_rx) = mpsc::channel(); + let (delivered_tx, delivered_rx) = mpsc::channel(); + let reader_activity = Arc::clone(&active_captures); + let stale_reader = std::thread::spawn(move || { + deliver_rx + .recv() + .expect("registration should release reader"); + let mut emitted = Vec::new(); + let mut passthrough = Vec::new(); + deliver_with( + "concurrent stale output".to_string(), + &reader_activity, + &mut passthrough, + |event| { + emitted.push(event); + Ok(()) + }, + ); + delivered_tx + .send(()) + .expect("registration should await delivery"); + (emitted, passthrough) + }); + + register_capture_reader(&active_captures, || { + deliver_tx.send(()).expect("stale reader should be waiting"); + delivered_rx + .recv() + .expect("stale reader should deliver during registration"); + Ok(()) + }) + .expect("reader registration should succeed"); + + let (emitted, passthrough) = stale_reader.join().expect("stale reader should exit"); + assert_eq!( + emitted.len(), + 1, + "concurrent output belongs in the dashboard" + ); + assert!(passthrough.is_empty(), "it must not bypass the dashboard"); + } + + #[test] + fn failed_reader_registration_rolls_back_capture_activity() { + let active_captures = AtomicUsize::new(0); + + let result = register_capture_reader(&active_captures, || { + Err(std::io::Error::other("reader spawn failed")) + }); + + assert!(result.is_err()); + assert_eq!(active_captures.load(Ordering::Acquire), 0); + } +} diff --git a/crates/mesh-llm-tui/src/output/dashboard.rs b/crates/mesh-llm-tui/src/output/dashboard.rs index 5ff49248b2..2881876eed 100644 --- a/crates/mesh-llm-tui/src/output/dashboard.rs +++ b/crates/mesh-llm-tui/src/output/dashboard.rs @@ -39,6 +39,7 @@ pub(super) enum DashboardAction { selected_row: Option, }, Resize(DashboardLayoutState), + RequestFullRepaint, } impl DashboardState { @@ -241,6 +242,7 @@ impl DashboardState { pub(in crate::output) fn reduce(&mut self, action: DashboardAction) { match action { + DashboardAction::RequestFullRepaint => self.pending_full_repaint = true, DashboardAction::OutputEvent(event) => self.apply_output_event(&event), DashboardAction::SnapshotUpdated(snapshot) => self.apply_snapshot(&snapshot), DashboardAction::FocusNextPanel => { @@ -1641,6 +1643,14 @@ impl DashboardState { self.reduce(DashboardAction::ToggleEventsFollow); Some(TuiControlFlow::Continue) } + // The status bar has advertised `R Refresh` since the dashboard + // shipped without anything behind it. This is the repair path for + // corruption that capture cannot intercept — anything written to + // the tty by a process that did not inherit our descriptors. + TuiEvent::Key(TuiKeyEvent::Char('r' | 'R')) if !self.events_filter.editing => { + self.reduce(DashboardAction::RequestFullRepaint); + Some(TuiControlFlow::Continue) + } _ => None, } } diff --git a/crates/mesh-llm-tui/src/output/formatting.rs b/crates/mesh-llm-tui/src/output/formatting.rs index b9e3cc2927..bd3e2bc5c6 100644 --- a/crates/mesh-llm-tui/src/output/formatting.rs +++ b/crates/mesh-llm-tui/src/output/formatting.rs @@ -1,11 +1,14 @@ +use super::console_capture::ConsoleCapture; use super::logging_projection::{projected_json_fields, projected_message, projected_pretty_text}; +use super::terminal_out::TerminalOut; use super::{ ConsoleSessionMode, DashboardAction, DashboardLayoutState, DashboardSnapshot, DashboardSnapshotProvider, DashboardState, LogFormat, ModelProgressStatus, OutputEvent, OutputSink, OutputSinkFuture, PRETTY_TUI_JOIN_TOKEN_PANEL_HEIGHT, PRETTY_TUI_MIN_DASHBOARD_WIDTH, PRETTY_TUI_REDRAW_INTERVAL, PRETTY_TUI_SNAPSHOT_INTERVAL, RuntimeStatus, TuiControlFlow, TuiEvent, TuiTerminal, draw_tui_dashboard_with_terminal, - format_invite_mesh_label, render_dashboard_text, strip_leading_severity_icon, + format_invite_mesh_label, render_dashboard_text, repair_tui_terminal, + strip_leading_severity_icon, }; use chrono::{SecondsFormat, Utc}; use crossterm::{ @@ -629,6 +632,10 @@ pub struct InteractiveDashboardFormatter { pub(in crate::output) tui_entered: Arc, pub(in crate::output) panic_restored: Arc, pub(in crate::output) dirty: bool, + /// Holds fd 1/2 redirected into the dashboard for as long as it owns the + /// screen. `None` when capture could not be installed, in which case stray + /// output still lands on the frame and `R` remains the repair. + pub(in crate::output) console_capture: Option, } impl InteractiveDashboardFormatter { @@ -652,7 +659,17 @@ impl InteractiveDashboardFormatter { self.panic_restored.load(Ordering::Acquire) } + /// Restore the real stdout/stderr. Idempotent, and safe on the panic path. + pub(super) fn release_console_capture(&mut self) { + if let Some(mut capture) = self.console_capture.take() { + let _ = capture.restore(); + } + } + pub(super) fn mark_panic_restored(&mut self) { + // A panic message is about to be written to stderr; it must not go + // into the capture pipe. + self.release_console_capture(); self.terminal = None; self.terminal_active = false; self.dirty = false; @@ -706,12 +723,34 @@ impl InteractiveDashboardFormatter { if self.terminal_active { return Ok(()); } - write_tui_enter()?; + let mut out = TerminalOut::open(); + // Capture is only installed when the dashboard has a descriptor of its + // own. Redirecting stderr while still rendering to it would send every + // frame into the capture pipe. + let capture_is_safe = out.is_private(); + write_tui_enter_to_writer(&mut out)?; self.mark_terminal_escape_written(); - let backend = CrosstermBackend::new(io::stderr()); - let mut terminal = Terminal::new(backend).map_err(io::Error::other)?; - terminal.hide_cursor().map_err(io::Error::other)?; + let backend = CrosstermBackend::new(out); + let mut terminal = match Terminal::new(backend).map_err(io::Error::other) { + Ok(terminal) => terminal, + Err(error) => { + self.rollback_terminal_enter(); + return Err(error); + } + }; + if let Err(error) = terminal.hide_cursor().map_err(io::Error::other) { + // Drop the buffered backend before writing the rollback sequence; + // otherwise its final flush could hide the cursor again afterward. + drop(terminal); + self.rollback_terminal_enter(); + return Err(error); + } self.terminal = Some(terminal); + if capture_is_safe { + // A failure here is not fatal: the dashboard works without capture, + // it is just no longer immune to stray writes. + self.console_capture = ConsoleCapture::install().ok(); + } Ok(()) } @@ -724,12 +763,34 @@ impl InteractiveDashboardFormatter { self.dirty = true; } + fn rollback_terminal_enter(&mut self) { + self.rollback_terminal_enter_with(write_tui_exit); + } + + pub(super) fn rollback_terminal_enter_with(&mut self, exit: impl FnOnce() -> io::Result<()>) { + // The setup error remains the useful result, but teardown is still + // attempted. Reset the bookkeeping even if that best-effort write + // fails so a later enter can retry instead of observing a phantom TUI. + let _ = exit(); + self.release_console_capture(); + self.terminal = None; + self.terminal_active = false; + self.dirty = false; + self.tui_entered.store(false, Ordering::Release); + } + pub(super) fn exit_terminal(&mut self) -> io::Result<()> { if !self.terminal_active { return Ok(()); } + // Put fd 1/2 back before tearing the screen down, so anything written + // during shutdown reaches the real terminal instead of a pipe whose + // reader is about to stop projecting into a dashboard that is gone. + self.release_console_capture(); if let Some(mut terminal) = self.terminal.take() { - terminal.show_cursor().map_err(io::Error::other)?; + // `write_tui_exit` also shows the cursor, so a backend-specific + // show failure must not prevent the alternate-screen teardown. + let _ = terminal.show_cursor(); } self.terminal_active = false; self.dirty = false; @@ -755,6 +816,14 @@ impl InteractiveDashboardFormatter { let terminal = self.terminal.as_mut().ok_or_else(|| { io::Error::other("pretty TUI terminal missing while terminal mode is active") })?; + if self.state.pending_full_repaint { + // Cleared only once the repair actually happened: if the erase + // fails, the next draw would be an ordinary diff against a screen + // ratatui still believes is intact, and the damage would survive + // the key press the operator already made. + repair_tui_terminal(terminal)?; + self.state.pending_full_repaint = false; + } draw_tui_dashboard_with_terminal(terminal, &self.state)?; self.dirty = false; Ok(true) @@ -1516,14 +1585,8 @@ pub(super) fn dashboard_layout_for_terminal_size(columns: u16, rows: u16) -> Das ) } -pub(super) fn write_tui_enter() -> io::Result<()> { - let mut stderr = io::stderr().lock(); - write_tui_enter_to_writer(&mut stderr) -} - pub(super) fn write_tui_exit() -> io::Result<()> { - let mut stderr = io::stderr().lock(); - write_tui_exit_to_writer(&mut stderr) + write_tui_exit_to_writer(&mut TerminalOut::open()) } #[cfg(test)] diff --git a/crates/mesh-llm-tui/src/output/mod.rs b/crates/mesh-llm-tui/src/output/mod.rs index 3b69dd9f19..85e96cef5b 100644 --- a/crates/mesh-llm-tui/src/output/mod.rs +++ b/crates/mesh-llm-tui/src/output/mod.rs @@ -15,12 +15,14 @@ use tokio::time::Duration; mod fatal; pub use fatal::{emit_fatal_error, emit_fatal_panic}; +mod console_capture; mod dashboard; mod formatting; mod logging_projection; mod merging; pub(in crate::output) mod rendering; mod state; +mod terminal_out; #[cfg(test)] mod tests; diff --git a/crates/mesh-llm-tui/src/output/rendering/mod.rs b/crates/mesh-llm-tui/src/output/rendering/mod.rs index bb4fac2a37..7a887649a2 100644 --- a/crates/mesh-llm-tui/src/output/rendering/mod.rs +++ b/crates/mesh-llm-tui/src/output/rendering/mod.rs @@ -1,5 +1,6 @@ #[cfg(test)] use super::PRETTY_TUI_LIST_HIGHLIGHT_SYMBOL_WIDTH; +use super::terminal_out::TerminalOut; use super::{ DashboardEventsFilterState, DashboardPanel, DashboardState, MeshEventState, PRETTY_TUI_EVENT_LEVEL_WIDTH, PRETTY_TUI_EVENTS_COLUMN_PERCENT, @@ -25,7 +26,7 @@ use ratatui::{ }; #[cfg(test)] use std::fmt::Write as _; -use std::io; + use tokio::time::Duration; mod events; @@ -160,7 +161,7 @@ pub(super) enum TuiEventRow<'a> { Padding, } -pub(in crate::output) type TuiTerminal = Terminal>; +pub(in crate::output) type TuiTerminal = Terminal>; pub(in crate::output) fn dashboard_status_line( state: &DashboardState, width: u16, diff --git a/crates/mesh-llm-tui/src/output/rendering/processes.rs b/crates/mesh-llm-tui/src/output/rendering/processes.rs index 0712cc5dca..8cf5e86664 100644 --- a/crates/mesh-llm-tui/src/output/rendering/processes.rs +++ b/crates/mesh-llm-tui/src/output/rendering/processes.rs @@ -1,5 +1,5 @@ use super::super::{ - DashboardEndpointRow, DashboardModelRow, DashboardProcessRow, + DashboardEndpointRow, DashboardModelRow, DashboardPanelViewState, DashboardProcessRow, PRETTY_TUI_WEBSERVER_PROCESS_HEADER_LABEL, llama_process_model_name, model_name_without_variant_suffix, model_names_match, }; @@ -62,8 +62,9 @@ pub(in crate::output) fn render_process_table( return; } - let [model_width, pid_width, port_width, status_width] = + let widths = llama_process_column_widths_for_rows(inner_area.width, &state.llama_process_rows); + let [model_width, pid_width, port_width, status_width] = widths; let available_rows = usize::from(inner_area.height.saturating_sub(1)); let rows = state .llama_process_rows @@ -74,45 +75,37 @@ pub(in crate::output) fn render_process_table( .map(|(_, row)| { let model = llama_process_model_metadata(row, &state.loaded_model_rows); let model_name = model.map(|model| model.name.as_str()).unwrap_or(&row.name); - Row::new(vec![ - Cell::from(truncate_with_ellipsis( - model_name_without_variant_suffix(model_name), - model_width, - )), - Cell::from(truncate_with_ellipsis( - &format_dashboard_pid((row.pid != 0).then_some(row.pid)), - pid_width, - )), - Cell::from(truncate_with_ellipsis(&row.port.to_string(), port_width)), - process_status_cell(&row.status, status_width), - ]) + Row::new(present_columns( + widths, + [ + Cell::from(truncate_with_ellipsis( + model_name_without_variant_suffix(model_name), + model_width, + )), + Cell::from(truncate_with_ellipsis( + &format_dashboard_pid((row.pid != 0).then_some(row.pid)), + pid_width, + )), + Cell::from(truncate_with_ellipsis(&row.port.to_string(), port_width)), + process_status_cell(&row.status, status_width), + ], + )) }) .collect::>(); - let selected_local_index = view - .selected_row - .map(|selected| selected.saturating_sub(view.scroll_offset)); - let mut table_state = TableState::default(); - table_state.select(selected_local_index); - let table = Table::new( - rows, + render_populated_process_table( + frame, + inner_area, + view, + widths, [ - Constraint::Fill(1), - Constraint::Length(u16::try_from(pid_width).unwrap_or(u16::MAX)), - Constraint::Length(u16::try_from(port_width).unwrap_or(u16::MAX)), - Constraint::Length(u16::try_from(status_width).unwrap_or(u16::MAX)), + "MODEL".to_string(), + "PID".to_string(), + "PORT".to_string(), + right_align_text("STATE", status_width), ], - ) - .header(process_table_header_row([ - "MODEL".to_string(), - "PID".to_string(), - "PORT".to_string(), - right_align_text("STATE", status_width), - ])) - .column_spacing(1) - .highlight_symbol(if is_focused { "› " } else { " " }) - .highlight_spacing(HighlightSpacing::Always) - .row_highlight_style(process_table_highlight_style(is_focused)); - frame.render_stateful_widget(table, inner_area, &mut table_state); + rows, + is_focused, + ); } DashboardPanel::Webserver => { if state.webserver_rows.is_empty() { @@ -124,8 +117,9 @@ pub(in crate::output) fn render_process_table( return; } - let [label_width, pid_width, port_width, status_width] = + let widths = webserver_process_column_widths_for_rows(inner_area.width, &state.webserver_rows); + let [label_width, pid_width, port_width, status_width] = widths; let available_rows = usize::from(inner_area.height.saturating_sub(1)); let rows = state .webserver_rows @@ -134,50 +128,65 @@ pub(in crate::output) fn render_process_table( .skip(view.scroll_offset) .take(available_rows) .map(|(_, row)| { - Row::new(vec![ - Cell::from(truncate_with_ellipsis(&row.label, label_width)), - Cell::from(truncate_with_ellipsis( - &format_dashboard_pid(row.pid), - pid_width, - )), - Cell::from(truncate_with_ellipsis( - &format_dashboard_port(row.port), - port_width, - )), - process_status_cell(&row.status, status_width), - ]) + Row::new(present_columns( + widths, + [ + Cell::from(truncate_with_ellipsis(&row.label, label_width)), + Cell::from(truncate_with_ellipsis( + &format_dashboard_pid(row.pid), + pid_width, + )), + Cell::from(truncate_with_ellipsis( + &format_dashboard_port(row.port), + port_width, + )), + process_status_cell(&row.status, status_width), + ], + )) }) .collect::>(); - let selected_local_index = view - .selected_row - .map(|selected| selected.saturating_sub(view.scroll_offset)); - let mut table_state = TableState::default(); - table_state.select(selected_local_index); - let table = Table::new( - rows, + render_populated_process_table( + frame, + inner_area, + view, + widths, [ - Constraint::Fill(1), - Constraint::Length(u16::try_from(pid_width).unwrap_or(u16::MAX)), - Constraint::Length(u16::try_from(port_width).unwrap_or(u16::MAX)), - Constraint::Length(u16::try_from(status_width).unwrap_or(u16::MAX)), + PRETTY_TUI_WEBSERVER_PROCESS_HEADER_LABEL.to_string(), + "PID".to_string(), + "PORT".to_string(), + right_align_text("STATE", status_width), ], - ) - .header(process_table_header_row([ - PRETTY_TUI_WEBSERVER_PROCESS_HEADER_LABEL.to_string(), - "PID".to_string(), - "PORT".to_string(), - right_align_text("STATE", status_width), - ])) - .column_spacing(1) - .highlight_symbol(if is_focused { "› " } else { " " }) - .highlight_spacing(HighlightSpacing::Always) - .row_highlight_style(process_table_highlight_style(is_focused)); - frame.render_stateful_widget(table, inner_area, &mut table_state); + rows, + is_focused, + ); } _ => {} } } +fn render_populated_process_table( + frame: &mut Frame, + inner_area: Rect, + view: DashboardPanelViewState, + widths: [usize; 4], + labels: [String; 4], + rows: Vec>, + is_focused: bool, +) { + let selected_local_index = view + .selected_row + .map(|selected| selected.saturating_sub(view.scroll_offset)); + let mut table_state = TableState::default(); + table_state.select(selected_local_index); + let table = Table::new(rows, process_table_constraints(widths)) + .header(process_table_header_row(present_columns(widths, labels))) + .column_spacing(1) + .highlight_symbol(if is_focused { "› " } else { " " }) + .highlight_spacing(HighlightSpacing::Always) + .row_highlight_style(process_table_highlight_style(is_focused)); + frame.render_stateful_widget(table, inner_area, &mut table_state); +} + pub(in crate::output) fn combine_panel_rect(title_area: Rect, body_area: Rect) -> Rect { Rect { x: title_area.x, @@ -245,8 +254,8 @@ pub(in crate::output) fn process_table_highlight_style(is_focused: bool) -> Styl } } -pub(in crate::output) fn process_table_header_row( - labels: [String; N], +pub(in crate::output) fn process_table_header_row>( + labels: L, ) -> Row<'static> { let theme = tui_theme(); Row::new(labels.into_iter().map(|label| { @@ -363,18 +372,65 @@ pub(in crate::output) fn webserver_process_column_widths_for_rows( ) } +/// Column widths for a process table, in render order. A width of `0` means +/// the column does not fit and is not rendered at all. +/// +/// Columns are surrendered from the right — STATE first, then PORT — because +/// the text column identifies the row. Crushing it instead produces the +/// one-character `MODEL` column that made narrow dashboards unreadable, and +/// truncating a header to `STA` is not an improvement over dropping it. pub(in crate::output) fn process_column_widths( body_width: u16, min_text_width: usize, pid_width: usize, status_width: usize, ) -> [usize; 4] { - let port_width = 5usize; - let reserved_width = pid_width + port_width + status_width + 3 + 2; - let text_width = usize::from(body_width) - .saturating_sub(reserved_width) - .max(min_text_width); - [text_width, pid_width, port_width, status_width] + const PORT_WIDTH: usize = 5; + // Ratatui always reserves room for the row highlight symbol. + const HIGHLIGHT_WIDTH: usize = 2; + + let available = usize::from(body_width).saturating_sub(HIGHLIGHT_WIDTH); + for (port_width, status_width) in [(PORT_WIDTH, status_width), (PORT_WIDTH, 0), (0, 0)] { + let rendered_columns = 2 + usize::from(port_width > 0) + usize::from(status_width > 0); + let fixed_width = + pid_width + port_width + status_width + rendered_columns.saturating_sub(1); + if available >= fixed_width + min_text_width { + return [available - fixed_width, pid_width, port_width, status_width]; + } + } + + // Narrower than even one text cell + spacing + PID. Preserve the table's + // width invariant by surrendering PID as well. + if available < pid_width.saturating_add(2) { + return [available, 0, 0, 0]; + } + + // Keep text + PID and let the text truncate. + [available - pid_width - 1, pid_width, 0, 0] +} + +/// Constraints for the columns that survived [`process_column_widths`]. +/// +/// These are exact `Length`s rather than a `Fill`, so what ratatui lays out is +/// what the cell contents were truncated to. The two disagreeing was the +/// original defect: the text was fitted to a floored width while the column got +/// whatever `Fill(1)` had left, which could be a single character. +pub(in crate::output) fn process_table_constraints(widths: [usize; 4]) -> Vec { + widths + .into_iter() + .filter(|width| *width > 0) + .map(|width| Constraint::Length(u16::try_from(width).unwrap_or(u16::MAX))) + .collect() +} + +/// Drop the labels/cells whose column was not rendered. +pub(in crate::output) fn present_columns(widths: [usize; 4], values: [T; 4]) -> Vec { + values + .into_iter() + .zip(widths) + .filter(|(_, width)| *width > 0) + .map(|(value, _)| value) + .collect() } pub(in crate::output) fn process_pid_width(pids: I) -> usize diff --git a/crates/mesh-llm-tui/src/output/rendering/tui.rs b/crates/mesh-llm-tui/src/output/rendering/tui.rs index 8e97d8ab7c..0e9cfbb96d 100644 --- a/crates/mesh-llm-tui/src/output/rendering/tui.rs +++ b/crates/mesh-llm-tui/src/output/rendering/tui.rs @@ -5,20 +5,62 @@ use super::{ render_model_progress_loader, render_models_panel, render_process_table, render_processes_panel, render_requests_panel, render_tui_logo, tui_layout, tui_theme, }; -use std::io; +use ratatui::backend::Backend; +use std::{fmt::Display, io}; pub(in crate::output) fn draw_tui_dashboard_with_terminal( terminal: &mut TuiTerminal, state: &DashboardState, ) -> io::Result<()> { - terminal.hide_cursor().map_err(io::Error::other)?; + draw_tui_dashboard_with_backend(terminal, state) +} + +pub(in crate::output) fn draw_tui_dashboard_with_backend( + terminal: &mut ratatui::Terminal, + state: &DashboardState, +) -> io::Result<()> +where + B: Backend, + B::Error: Display, +{ + terminal + .hide_cursor() + .map_err(|error| io::Error::other(error.to_string()))?; terminal .set_cursor_position((0, 0)) - .map_err(io::Error::other)?; + .map_err(|error| io::Error::other(error.to_string()))?; terminal .draw(|frame| render_tui_frame(frame, state)) .map(|_| ()) - .map_err(io::Error::other) + .map_err(|error| io::Error::other(error.to_string())) +} + +/// Repair a physically desynchronized screen. +/// +/// Ratatui diffs against its own idea of the screen, so once something else has +/// written to the terminal the damaged cells are never redrawn — they match the +/// buffer ratatui believes is displayed. Erasing the real screen and discarding +/// both internal buffers forces the next draw to emit every cell. +/// +/// This runs only when the operator asks for it with `R`. Doing it on every +/// frame also works, but repaints the whole screen from blank ~2.6 times a +/// second on an idle dashboard, which reads as a black blink. +pub(in crate::output) fn repair_tui_terminal( + terminal: &mut ratatui::Terminal, +) -> io::Result<()> +where + B: Backend, + B::Error: Display, +{ + terminal + .backend_mut() + .clear() + .map_err(|error| io::Error::other(error.to_string()))?; + // `swap_buffers` resets the inactive buffer before toggling, so two swaps + // reset both buffers and leave the original active index in place. + terminal.swap_buffers(); + terminal.swap_buffers(); + Ok(()) } pub(in crate::output) fn render_tui_frame(frame: &mut Frame, state: &DashboardState) { diff --git a/crates/mesh-llm-tui/src/output/state.rs b/crates/mesh-llm-tui/src/output/state.rs index 26f7ec2f2d..e804c8348b 100644 --- a/crates/mesh-llm-tui/src/output/state.rs +++ b/crates/mesh-llm-tui/src/output/state.rs @@ -627,6 +627,9 @@ pub struct DashboardState { pub(super) startup_milestones: BTreeSet, pub(super) startup_lifecycle: StartupLifecycleState, pub(super) shutdown_in_progress: bool, + /// Set by the `R` key. Consumed by the next render, which repairs the + /// physical screen before drawing. Costs nothing until it is pressed. + pub(super) pending_full_repaint: bool, } impl Default for DashboardState { @@ -670,6 +673,7 @@ impl Default for DashboardState { startup_milestones: BTreeSet::new(), startup_lifecycle: StartupLifecycleState::default(), shutdown_in_progress: false, + pending_full_repaint: false, }; state.apply_layout(panel_layout); state diff --git a/crates/mesh-llm-tui/src/output/terminal_out.rs b/crates/mesh-llm-tui/src/output/terminal_out.rs new file mode 100644 index 0000000000..a8d6474acc --- /dev/null +++ b/crates/mesh-llm-tui/src/output/terminal_out.rs @@ -0,0 +1,83 @@ +//! The writer the dashboard paints through. +//! +//! The dashboard used to render straight to `io::stderr()`, which is also +//! where `eprintln!`, `tracing`'s default writer, inherited child stderr, and +//! most noisy C libraries write. Sharing that descriptor is what made stray +//! output land *on top of* the dashboard instead of in it, and it is why no +//! amount of converting individual call sites could ever close the hole. +//! +//! Rendering to the controlling terminal instead gives the dashboard a channel +//! nothing else holds a descriptor for, which in turn frees fd 1 and fd 2 to be +//! captured (see [`super::console_capture`]) without fighting over the screen. +//! This is the same reason `less`, `fzf`, and `vim` open the tty directly +//! rather than trusting their standard descriptors. + +use std::fs::{File, OpenOptions}; +use std::io::{self, BufWriter, Write}; + +/// Where terminal control sequences and dashboard frames are written. +/// +/// Falls back to stderr when there is no controlling terminal to open — a +/// piped or redirected session never reaches the dashboard path anyway, but +/// the fallback keeps enter/exit sequences working for the fallback console. +pub(in crate::output) enum TerminalOut { + Tty(BufWriter), + Stderr(BufWriter), +} + +impl TerminalOut { + pub(in crate::output) fn open() -> Self { + match open_controlling_terminal() { + Some(file) => Self::Tty(BufWriter::new(file)), + None => Self::Stderr(BufWriter::new(io::stderr())), + } + } + + /// True when the dashboard owns a descriptor that is independent of fd 1 + /// and fd 2. Console capture is only safe to install in that case, + /// otherwise redirecting stderr would redirect the dashboard itself. + pub(in crate::output) fn is_private(&self) -> bool { + matches!(self, Self::Tty(_)) + } +} + +#[cfg(unix)] +fn open_controlling_terminal() -> Option { + OpenOptions::new() + .read(true) + .write(true) + .open("/dev/tty") + .ok() +} + +#[cfg(windows)] +fn open_controlling_terminal() -> Option { + // `CONOUT$` is the Windows analogue: it resolves to the active console + // screen buffer regardless of how the standard handles were redirected. + OpenOptions::new() + .read(true) + .write(true) + .open("CONOUT$") + .ok() +} + +#[cfg(not(any(unix, windows)))] +fn open_controlling_terminal() -> Option { + None +} + +impl Write for TerminalOut { + fn write(&mut self, buf: &[u8]) -> io::Result { + match self { + Self::Tty(file) => file.write(buf), + Self::Stderr(stderr) => stderr.write(buf), + } + } + + fn flush(&mut self) -> io::Result<()> { + match self { + Self::Tty(file) => file.flush(), + Self::Stderr(stderr) => stderr.flush(), + } + } +} diff --git a/crates/mesh-llm-tui/src/output/tests/formatting.rs b/crates/mesh-llm-tui/src/output/tests/formatting.rs index db57f5010c..341ebeed7b 100644 --- a/crates/mesh-llm-tui/src/output/tests/formatting.rs +++ b/crates/mesh-llm-tui/src/output/tests/formatting.rs @@ -1330,6 +1330,30 @@ pub(super) fn tui_terminal_setup_marks_cleanup_required_after_enter_escape() { assert!(formatter.terminal.is_none()); } +#[test] +pub(super) fn tui_terminal_setup_rollback_resets_state_even_when_exit_fails() { + let mut formatter = InteractiveDashboardFormatter::default(); + formatter.mark_terminal_escape_written(); + let mut exit_attempted = false; + + formatter.rollback_terminal_enter_with(|| { + exit_attempted = true; + Err(io::Error::other("simulated terminal exit failure")) + }); + + assert!(exit_attempted); + assert!(formatter.terminal.is_none()); + assert!(!formatter.terminal_active); + assert!(!formatter.tui_entered()); + assert!(!formatter.dirty); + + formatter.mark_terminal_escape_written(); + assert!( + formatter.terminal_active && formatter.tui_entered(), + "a later terminal setup must be able to retry normally" + ); +} + #[test] pub(super) fn tui_panic_restore_flag_tracks_terminal_entry() { let mut formatter = InteractiveDashboardFormatter::default(); diff --git a/crates/mesh-llm-tui/src/output/tests/mod.rs b/crates/mesh-llm-tui/src/output/tests/mod.rs index 04ec8cde27..45a23c0381 100644 --- a/crates/mesh-llm-tui/src/output/tests/mod.rs +++ b/crates/mesh-llm-tui/src/output/tests/mod.rs @@ -2,8 +2,8 @@ use super::*; use crate::output::formatting::*; use crate::output::rendering::*; use ratatui::{ - Terminal, - backend::TestBackend, + Terminal, TerminalOptions, Viewport, + backend::{Backend, CrosstermBackend, TestBackend}, buffer::Buffer, layout::{Constraint, Direction, Layout, Rect}, style::Modifier, @@ -11,9 +11,9 @@ use ratatui::{ }; use serde_json::Value; use std::{ - io::Write as _, + io::{self, Write as _}, sync::{ - Arc, + Arc, Mutex, atomic::{AtomicBool, Ordering}, }, }; diff --git a/crates/mesh-llm-tui/src/output/tests/rendering.rs b/crates/mesh-llm-tui/src/output/tests/rendering.rs index 2dec60ac17..7a55715d2f 100644 --- a/crates/mesh-llm-tui/src/output/tests/rendering.rs +++ b/crates/mesh-llm-tui/src/output/tests/rendering.rs @@ -1689,3 +1689,248 @@ pub(super) fn planned_rows_transition_from_not_ready_to_ready_events() { ); assert_eq!(state.loaded_model_rows[0].status, RuntimeStatus::Ready); } + +/// A writer that keeps the bytes a `CrosstermBackend` emits so a test can +/// assert on the escape sequences that actually reach the terminal. +#[derive(Clone, Default)] +struct RecordingWriter { + bytes: Arc>>, +} + +impl std::io::Write for RecordingWriter { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.bytes + .lock() + .expect("recording writer lock should not be poisoned") + .extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +fn recording_terminal() -> ( + Terminal>, + Arc>>, +) { + let bytes = Arc::new(Mutex::new(Vec::new())); + let backend = CrosstermBackend::new(RecordingWriter { + bytes: Arc::clone(&bytes), + }); + // A fixed viewport keeps the backend from querying a terminal that does not + // exist under the test harness. + let terminal = Terminal::with_options( + backend, + TerminalOptions { + viewport: Viewport::Fixed(Rect::new(0, 0, 100, 32)), + }, + ) + .expect("recording backend should initialize"); + (terminal, bytes) +} + +fn recorded_contains_full_screen_clear(bytes: &Arc>>) -> bool { + bytes + .lock() + .expect("recording writer lock should not be poisoned") + .windows(b"\x1b[2J".len()) + .any(|window| window == b"\x1b[2J") +} + +#[test] +pub(super) fn tui_refresh_key_requests_a_full_repaint() { + let mut state = DashboardState::default(); + assert!( + !state.pending_full_repaint, + "a fresh dashboard should not be asking for a repair" + ); + + let control = state.apply_tui_event(TuiEvent::Key(TuiKeyEvent::Char('r'))); + + assert!(matches!(control, TuiControlFlow::Continue)); + assert!( + state.pending_full_repaint, + "R is advertised in the status bar and must request a physical repair" + ); +} + +#[test] +pub(super) fn tui_refresh_key_accepts_the_uppercase_it_advertises() { + // The status bar reads `R Refresh`, and Shift+R arrives as `Char('R')`. + let mut state = DashboardState::default(); + + let control = state.apply_tui_event(TuiEvent::Key(TuiKeyEvent::Char('R'))); + + assert!(matches!(control, TuiControlFlow::Continue)); + assert!( + state.pending_full_repaint, + "the key the status bar prints must be the key that works" + ); +} + +#[test] +pub(super) fn tui_refresh_key_types_into_the_filter_instead_of_repainting() { + let mut state = DashboardState::default(); + state.reduce(DashboardAction::StartEventsFilterEdit); + + state.apply_tui_event(TuiEvent::Key(TuiKeyEvent::Char('r'))); + + assert!( + !state.pending_full_repaint, + "R while editing the filter is filter text, not a repair request" + ); + assert_eq!(state.events_filter.query, "r"); +} + +#[test] +pub(super) fn tui_repair_redraws_cells_an_out_of_band_write_corrupted() { + let state = DashboardState::default(); + let mut terminal = + Terminal::new(TestBackend::new(100, 32)).expect("test backend should initialize"); + draw_tui_dashboard_with_backend(&mut terminal, &state) + .expect("initial dashboard draw should succeed"); + + let area = terminal.backend().buffer().area; + let (x, y) = (0..area.width) + .flat_map(|x| (0..area.height).map(move |y| (x, y))) + .find(|&(x, y)| terminal.backend().buffer()[(x, y)].symbol() != " ") + .expect("dashboard should paint at least one cell"); + let expected_symbol = terminal.backend().buffer()[(x, y)].symbol().to_string(); + + // Exactly what a stray write to the terminal does: the physical screen + // changes while ratatui's idea of it does not. + let stale_cell = ratatui::buffer::Cell::new("X"); + terminal + .backend_mut() + .draw(std::iter::once((x, y, &stale_cell))) + .expect("out-of-band physical write should succeed"); + + // An ordinary redraw cannot heal it: the cell already matches the buffer + // ratatui believes is on screen, so it is diffed away. + draw_tui_dashboard_with_backend(&mut terminal, &state).expect("redraw should succeed"); + assert_eq!( + terminal.backend().buffer()[(x, y)].symbol(), + "X", + "a plain redraw is expected to leave out-of-band damage in place" + ); + + repair_tui_terminal(&mut terminal).expect("repair should succeed"); + draw_tui_dashboard_with_backend(&mut terminal, &state) + .expect("redraw after repair should succeed"); + + assert_eq!( + terminal.backend().buffer()[(x, y)].symbol(), + expected_symbol, + "repair must invalidate ratatui's diff so the next draw repaints every cell" + ); +} + +#[test] +pub(super) fn tui_steady_state_draws_emit_no_full_screen_clear() { + let (mut terminal, bytes) = recording_terminal(); + let state = DashboardState::default(); + + for _ in 0..8 { + draw_tui_dashboard_with_backend(&mut terminal, &state).expect("draw should succeed"); + } + + assert!( + !recorded_contains_full_screen_clear(&bytes), + "clearing the screen on every frame repaints from blank and reads as a black blink" + ); +} + +#[test] +pub(super) fn tui_repair_emits_a_physical_full_screen_clear() { + let (mut terminal, bytes) = recording_terminal(); + + repair_tui_terminal(&mut terminal).expect("repair should succeed"); + + assert!( + recorded_contains_full_screen_clear(&bytes), + "a logical ratatui Clear widget cannot fix backend desync; the repair must erase the real screen" + ); +} + +#[test] +pub(super) fn process_columns_keep_every_column_when_there_is_room() { + let [text, pid, port, status] = process_column_widths(80, 8, 5, 8); + + assert!(text >= 8, "the identifying column must never be crushed"); + assert_eq!((pid, port, status), (5, 5, 8)); +} + +#[test] +pub(super) fn process_columns_drop_state_before_crushing_the_model_name() { + // The width at which all four columns stop fitting. + let [text, _pid, port, status] = process_column_widths(26, 8, 5, 8); + + assert_eq!(status, 0, "STATE is the first column to be surrendered"); + assert_eq!(port, 5, "PORT still fits once STATE is gone"); + assert!( + text >= 8, + "the model name keeps its minimum instead of collapsing: got {text}" + ); +} + +#[test] +pub(super) fn process_columns_drop_port_next_and_still_keep_the_name() { + let [text, pid, port, status] = process_column_widths(18, 8, 5, 8); + + assert_eq!((port, status), (0, 0)); + assert_eq!(pid, 5, "PID is the last column to go"); + assert!(text >= 8, "the model name keeps its minimum: got {text}"); +} + +#[test] +pub(super) fn process_columns_drop_pid_before_exceeding_the_table_width() { + let pid_width = 5; + + // body width includes the two cells reserved for the highlight symbol. + assert_eq!( + process_column_widths((pid_width + 3) as u16, 8, pid_width, 8), + [pid_width + 1, 0, 0, 0], + "available == pid width + 1 cannot also fit text and spacing" + ); + assert_eq!( + process_column_widths((pid_width + 4) as u16, 8, pid_width, 8), + [1, pid_width, 0, 0], + "available == pid width + 2 exactly fits text, spacing, and PID" + ); +} + +#[test] +pub(super) fn process_table_renders_only_the_columns_that_fit() { + let widths = [9usize, 5, 0, 0]; + + assert_eq!(process_table_constraints(widths).len(), 2); + assert_eq!( + present_columns(widths, ["MODEL", "PID", "PORT", "STATE"]), + vec!["MODEL", "PID"] + ); +} + +#[test] +pub(super) fn tui_narrow_dashboard_keeps_the_model_name_readable() { + let mut state = DashboardState::default(); + state.reduce(DashboardAction::Resize(dashboard_layout_for_terminal_size( + 100, 32, + ))); + state.reduce(DashboardAction::SnapshotUpdated(snapshot_fixture(2, 30))); + + let rendered = render_tui_frame_snapshot(&state, 100, 32); + + // Regression: the MODEL column used to be laid out with `Fill(1)`, which + // ignored the floor the cell text was truncated to and collapsed to a + // single character at this width. + assert!( + rendered.contains("MODEL"), + "the model header must not be truncated away:\n{rendered}" + ); + assert!( + rendered.contains("llama-ser"), + "the model name must stay recognizable at 100 columns:\n{rendered}" + ); +} diff --git a/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index f1b4f01594..946f013cc3 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -1901,12 +1901,6 @@ "macro_name": "eprintln!" } ], - "crates/mesh-llm-host-runtime/src/runtime/tracing_writer.rs": [ - { - "line": 321, - "macro_name": "eprintln!" - } - ], "crates/mesh-llm-identity/src/keychain.rs": [ { "line": 236,