From 5a4b2702f2c621ecb5a0a46877845eaf7ff38092 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Wed, 6 May 2026 23:48:49 -0300 Subject: [PATCH] fix(tui): clear first inline viewport render (#21450) ## Why The alpha TUI can render the initial trust-directory prompt with stale terminal text showing through spaces when startup begins below existing shell output. The first inline viewport transition can happen while the previous viewport is still empty, so the old clear path no-ops before Ratatui draws the prompt. Ratatui then skips blank cells because its previous buffer also thinks those cells are blank, leaving old terminal contents visible inside the prompt. ## What Changed - Clear from the new inline viewport top when the previous viewport is empty during a viewport transition. - Keep the existing clear-from-old-viewport behavior for normal viewport updates. - Add a VT100-backed regression test that pre-fills terminal contents, performs the first viewport clear, and verifies stale text inside the new viewport is removed while shell content above the viewport remains. ## How to Test 1. Start Codex alpha in a terminal that already has visible shell output above the cursor. 2. Use a fresh untrusted project directory so the trust-directory prompt appears. 3. Confirm the prompt text renders cleanly, with spaces staying blank instead of showing fragments of previous shell output. 4. As a regression check, confirm content above the inline viewport is still preserved in terminal scrollback. Targeted tests: - `cargo test -p codex-tui first_viewport_change_clears_from_new_viewport_when_old_viewport_is_empty -- --nocapture` - `cargo test -p codex-tui` --- codex-rs/tui/src/tui.rs | 66 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 24f6e6f94..06417e31e 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -3,6 +3,7 @@ use std::future::Future; use std::io::IsTerminal; use std::io::Result; use std::io::Stdout; +use std::io::Write; use std::io::stdin; use std::io::stdout; use std::panic; @@ -76,8 +77,15 @@ fn should_emit_notification(condition: NotificationCondition, terminal_focused: #[cfg(test)] mod tests { + use std::io::Write as _; + + use super::clear_for_viewport_change; use super::should_emit_notification; + use crate::custom_terminal::Terminal as CustomTerminal; + use crate::test_backend::VT100Backend; use codex_config::types::NotificationCondition; + use ratatui::layout::Position; + use ratatui::layout::Rect; #[test] fn unfocused_notification_condition_is_suppressed_when_focused() { @@ -102,6 +110,47 @@ mod tests { /*terminal_focused*/ false )); } + + #[test] + fn first_viewport_change_clears_from_new_viewport_when_old_viewport_is_empty() { + let width = 12; + let height = 4; + let backend = VT100Backend::new(width, height); + let mut terminal = + CustomTerminal::with_options_and_cursor_position(backend, Position { x: 0, y: 1 }) + .expect("terminal"); + write!( + terminal.backend_mut(), + "shell line\r\nstale cells\r\nmore stale" + ) + .expect("prefill terminal"); + + clear_for_viewport_change( + &mut terminal, + Rect::new( + /*x*/ 0, + /*y*/ 1, + /*width*/ width, + /*height*/ height - 1, + ), + ) + .expect("clear transition"); + + let rows: Vec = terminal + .backend() + .vt100() + .screen() + .rows(/*start*/ 0, width) + .collect(); + assert!( + rows[0].contains("shell line"), + "expected content before the viewport to remain visible, rows: {rows:?}" + ); + assert!( + !rows.iter().skip(1).any(|row| row.contains("stale")), + "expected stale cells inside the new viewport to be cleared, rows: {rows:?}" + ); + } } pub fn set_modes() -> Result<()> { @@ -391,6 +440,18 @@ struct PendingHistoryLines { wrap_policy: HistoryLineWrapPolicy, } +fn clear_for_viewport_change(terminal: &mut CustomTerminal, new_area: Rect) -> Result<()> +where + B: Backend + Write, +{ + let clear_position = if terminal.viewport_area.is_empty() { + new_area.as_position() + } else { + terminal.viewport_area.as_position() + }; + terminal.clear_after_position(clear_position) +} + impl Tui { pub fn new(terminal: Terminal) -> Self { let (draw_tx, _) = broadcast::channel(1); @@ -642,8 +703,9 @@ impl Tui { area.y = size.height - area.height; } if area != terminal.viewport_area { - // TODO(nornagon): probably this could be collapsed with the clear + set_viewport_area above. - terminal.clear()?; + // On startup, the old viewport can still be empty. Clear from the + // new viewport top so stale shell cells do not show through spaces. + clear_for_viewport_change(terminal, area)?; terminal.set_viewport_area(area); }