From 33bdf976ccd1130823d4fe041e4d5075ab511d67 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Fri, 11 Sep 2026 17:32:01 +0000 Subject: [PATCH] Fade Astra composer stars and stabilize cursor redraws (#44879) ## What changed - Fade Astra stars over one second after 15 seconds from the first visible frame, or quickly on composer input, drafts, voice input, and popups. Disconnected key handling also dismisses the animation. - Preserve placeholder and draft text, including spaces. Stop scheduling animation frames when unfocused or finished, while allowing the idle deadline to elapse without terminal focus. - Hide the terminal cursor before repainting and cache its style to avoid redundant anchor repairs. Invalidate cursor state after screen transitions, external programs, resume, and caught-panic recovery. ## Testing Add regression tests and snapshots for fade timing, input dismissal, focus changes, text preservation, and frame scheduling. Add cursor redraw tests and a PTY test for cursor visibility and shape restoration after a caught panic. Update the reconnect test to wait until both the draft and notification are visible. GitOrigin-RevId: 0c9aaa13e6bde820f88e3757457063680d7921a3 --- codex-rs/tui/src/app.rs | 4 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 12 + .../bottom_pane/chat_composer/reconnect.rs | 5 + ...r__sparkle__tests__astra_idle_timeout.snap | 34 ++ ...rkle__tests__astra_placeholder_spaces.snap | 12 + ...__tests__astra_sparkle_interaction_40.snap | 21 + .../src/bottom_pane/chat_composer/sparkle.rs | 254 ++++++++++- .../chat_composer/sparkle_tests.rs | 428 +++++++++++++++++- codex-rs/tui/src/chatwidget/settings.rs | 4 + .../src/chatwidget/tests/status_and_layout.rs | 25 + codex-rs/tui/src/custom_terminal.rs | 26 +- codex-rs/tui/src/custom_terminal/cursor.rs | 18 +- .../src/custom_terminal/tests/cursor_tests.rs | 88 ++++ ...ursor__cursor_style_owned_wide_frames.snap | 5 +- ...or__cursor_style_single_column_frames.snap | 7 +- ...s__cursor__cursor_style_styled_frames.snap | 9 +- ...c_tests__caught_panic_cursor_commands.snap | 7 + codex-rs/tui/src/tui.rs | 9 + codex-rs/tui/src/tui_panic_tests.rs | 177 ++++++++ codex-rs/tui/tests/suite/reconnect.rs | 15 +- 20 files changed, 1097 insertions(+), 63 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_idle_timeout.snap create mode 100644 codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_placeholder_spaces.snap create mode 100644 codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_sparkle_interaction_40.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__tui__panic_tests__caught_panic_cursor_commands.snap create mode 100644 codex-rs/tui/src/tui_panic_tests.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 1c3f2813a9..85c070e1ae 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -906,6 +906,8 @@ impl App { match &event { TuiEvent::FocusLost => { + self.chat_widget + .set_sparkle_terminal_focus(/*focused*/ false); let now = Instant::now(); let thread_id = self.current_displayed_thread_id(); @@ -1019,6 +1021,8 @@ impl App { fn render_chat_widget_frame(&mut self, tui: &mut tui::Tui, screen_size: Size) -> Result { self.sync_thread_title_progress(); + self.chat_widget + .set_sparkle_terminal_focus(tui.is_terminal_focused()); let dashboard_visible = self .chat_widget .selected_index_for_present_view(AGENTS_OVERVIEW_VIEW_ID) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 9f07dfe018..ff2dc80f39 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -9,6 +9,15 @@ //! The plain-text preset keeps command prefixes literal, including `!`, so Enter and Tab //! submit ordinary text without enabling shell mode. //! +//! # Astra Sparkle +//! +//! Selecting Astra animates stars in the untouched composer for 15 seconds from its first visible +//! frame, then fades them smoothly for one second. The deadline runs even without terminal focus. +//! Keys, paste, an existing draft, voice input, or a popup start a quick fade; offline Enter and +//! Tab do too. Placeholder and draft text, including spaces, stay unchanged. Mouse reporting stays +//! disabled for native selection and scrolling; hover or selection alone does not stop the stars. +//! Reselecting Astra starts a new flourish. +//! //! # Mention Menus //! //! By default, `@` lists plugins, filesystem entries, and skills. Skills are hidden when their @@ -1227,6 +1236,7 @@ impl ChatComposer { /// In all cases, clears any paste-burst Enter suppression state so a real paste cannot affect /// the next user Enter key, then syncs popup state. pub fn handle_paste(&mut self, pasted: String) -> bool { + self.interact_with_astra_sparkle(); let pasted = pasted.replace("\r\n", "\n").replace('\r', "\n"); let pasted = sanitize_user_text(pasted.into()); if let Some(query) = self.draft.textarea.vim_query_mut() { @@ -2028,6 +2038,7 @@ impl ChatComposer { return (InputResult::None, false); } + self.interact_with_astra_sparkle(); if self.history_search.is_none() && !self.popups.active() && self.draft.textarea.wants_vim_search_key(key_event) @@ -5068,6 +5079,7 @@ impl ChatComposer { if self.astra_sparkle.is_some() { self.render_sparkle( composer_rect, + textarea_rect, self.cursor_pos_with_textarea_right_reserve(area, textarea_right_reserve), buf, ); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs b/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs index c0669203d5..cf90c35a35 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs @@ -45,6 +45,11 @@ impl ChatComposer { } pub(crate) fn handle_disconnected_key(&mut self, key: KeyEvent) { + if matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) + && !matches!(key.code, KeyCode::Null) + { + self.interact_with_astra_sparkle(); + } self.cancel_history_search(); self.attachments.clear_remote_image_selection(); self.popups.active = ActivePopup::None; diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_idle_timeout.snap b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_idle_timeout.snap new file mode 100644 index 0000000000..dfcb3cf1fb --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_idle_timeout.snap @@ -0,0 +1,34 @@ +--- +source: tui/src/bottom_pane/chat_composer/sparkle_tests.rs +assertion_line: 260 +expression: "format!(\"before timeout\\n{}\\n\\nfading\\n{}\\n\\nfinished\\n{}\",\nbuffer_text(&active), buffer_text(&fading), buffer_text(&finished),)" +--- +before timeout +⠁ ⠄ ⠈ ⠁ ⠐ ⠄ ⢀ +›⠁Ask Codex to do anything ⠈ ⠁ ⠈ ⠄ ⠂ + ⠠⢀⠐ ⠄ ⠄ ⠠ ⠄⠂ ⡀ ⠈ ⠄ + ⠄ ⠐ + ⠈ ⠠ ⠠ ⢀⢀ ⠐ ⠂ ⠐ ⠐ ⡀ + ⠄ ⠄ ⢀ ⢀ ⠠ ⠠ ⠄ ⡀⠂ + ⠈ ⠁ ⠐ ⠐ ⢀ + ? for shortcuts 100% context left + +fading +⠁ ⠄ ⠈ ⠁ ⠐ ⠄ ⢀ +›⠁Ask Codex to do anything ⠈ ⠁ ⠈ ⠄ + ⠠⢀ ⠄⠂ ⡀ ⠈ ⠄ + ⠄ + ⠠ ⠠ ⢀⢀ ⠂ ⠐ ⡀ + ⠄ ⢀ ⠠ ⠠ ⠄ ⡀⠂ + ⠁ ⠐ ⠐ ⢀ + ? for shortcuts 100% context left + +finished + +› Ask Codex to do anything + + + + + + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_placeholder_spaces.snap b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_placeholder_spaces.snap new file mode 100644 index 0000000000..dbcc27329a --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_placeholder_spaces.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/bottom_pane/chat_composer/sparkle_tests.rs +expression: buffer_text(&active) +--- + ⠈ ⢀ ⠈ ⠁ ⠐ ⠐ ⠐ ⠄ ⠈ +›⠁Ask Codex to do anything ⠈ ⠁ ⠁ ⠈ + ⠠ ⠄ ⢀ ⠄ ⠄⠈ ⠄ + ⠐ ⠄ ⡀ ⠠ + ⠈ ⠠⠁ ⠠⠠ ⢀ ⠂ + ⠄ ⠄ ⠐ ⠠ ⠂ ⡀ + ⠁ + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_sparkle_interaction_40.snap b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_sparkle_interaction_40.snap new file mode 100644 index 0000000000..63a37a1e39 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__sparkle__tests__astra_sparkle_interaction_40.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/bottom_pane/chat_composer/sparkle_tests.rs +expression: "format!(\"idle\\n{}\\n\\nfading\\n{}\\n\\nfinished\\n{}\", buffer_text(&active),\nbuffer_text(&fading), buffer_text(&finished),)" +--- +idle + ⢀ ⠁ ⢀ +› Ask Codex to do anything ⠈ + ⠠⢀ ⠠ ⠄ ⠠ + ? for shortcuts 100% context left + +fading + ⠁ ⢀ +› Ask Codex to do anything ⠈ + ⢀ ⠠ ⠄ + ? for shortcuts 100% context left + +finished + +› Ask Codex to do anything + + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/sparkle.rs b/codex-rs/tui/src/bottom_pane/chat_composer/sparkle.rs index 8df42d97ef..6717185672 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/sparkle.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/sparkle.rs @@ -1,7 +1,10 @@ -//! Sparse Astra stars on the existing composer surface, fading with the terminal's colors. -//! `enabled_foreground` owns eligibility, including late-arriving terminal colors. -//! Rendering owns frame scheduling, so hidden composers do not keep animating. +//! Astra stars throughout the untouched composer, using the terminal's colors. +//! Selecting Astra starts one flourish; composer input fades it quickly, or 15 seconds after the +//! first visible frame starts a smooth fade. The deadline keeps running while terminal focus is +//! elsewhere. Only visible, eligible frames schedule more work, and the final frame clears the +//! stars without taking over the terminal's native mouse actions. +use std::cell::Cell; use std::sync::LazyLock; use std::time::Duration; use std::time::Instant; @@ -9,6 +12,7 @@ use std::time::Instant; use codex_config::types::Tui; use ratatui::buffer::Buffer; use ratatui::buffer::CellDiffOption; +use ratatui::layout::Position; use ratatui::layout::Rect; use ratatui::style::Color; use regex_lite::Regex; @@ -24,6 +28,10 @@ use crate::terminal_palette::effective_stdout_color_level; use crate::terminal_palette::rgb_color; const FRAME_TICK: Duration = Duration::from_millis(150); +const IDLE_TIMEOUT: Duration = Duration::from_secs(15); +const IDLE_FADE: Duration = Duration::from_secs(1); +const INTERACTION_FADE: Duration = Duration::from_millis(75); +const FADE_FRAME_TICK: Duration = Duration::from_millis(25); const DOTS: [&str; 8] = ["⠁", "⠂", "⠄", "⠈", "⠐", "⠠", "⡀", "⢀"]; static ASTRA_MODEL: LazyLock = LazyLock::new(|| match Regex::new(r"(?i)\bastra\b") { Ok(regex) => regex, @@ -31,17 +39,36 @@ static ASTRA_MODEL: LazyLock = LazyLock::new(|| match Regex::new(r"(?i)\b }); pub(super) struct Sparkle { - model: String, - whimsy: bool, - animations: bool, started: Instant, + phase: Cell, + terminal_focused: bool, +} + +#[derive(Clone, Copy)] +enum Phase { + Waiting, + Visible { + since: Instant, + elapsed: Duration, + }, + Fading { + started: Instant, + elapsed: Duration, + duration: Duration, + start_visibility: f32, + }, + Finished, +} + +struct SparkleFrame { + elapsed: Duration, + visibility: f32, + next: Duration, } impl Sparkle { fn enabled_foreground(&self) -> Option<(u8, u8, u8)> { - if !self.whimsy - || !self.animations - || !ASTRA_MODEL.is_match(&self.model) + if matches!(self.phase.get(), Phase::Finished) || effective_stdout_color_level() != StdoutColorLevel::TrueColor { return None; @@ -49,35 +76,209 @@ impl Sparkle { // Read the cached palette here: Windows may populate it after widget creation. default_fg() } + + fn interact(&self, now: Instant) { + match self.phase.get() { + Phase::Waiting => self.phase.set(Phase::Finished), + Phase::Visible { since, elapsed } if now >= since + IDLE_TIMEOUT => { + self.phase.set(Phase::Fading { + started: since + IDLE_TIMEOUT, + elapsed, + duration: IDLE_FADE, + start_visibility: 1.0, + }); + self.interact(now); + } + Phase::Visible { elapsed, .. } => self.phase.set(Phase::Fading { + started: now, + elapsed, + duration: INTERACTION_FADE, + start_visibility: 1.0, + }), + Phase::Fading { + started, + elapsed, + duration: IDLE_FADE, + start_visibility, + } => { + let remaining = IDLE_FADE.saturating_sub(now.saturating_duration_since(started)); + if remaining.is_zero() { + self.phase.set(Phase::Finished); + } else { + // Input during the timed fade finishes quickly without brightening the dots. + self.phase.set(Phase::Fading { + started: now, + elapsed, + duration: INTERACTION_FADE.min(remaining), + start_visibility: start_visibility * remaining.as_secs_f32() + / IDLE_FADE.as_secs_f32(), + }); + } + } + Phase::Fading { .. } | Phase::Finished => {} + } + } + + fn frame(&self, now: Instant) -> Option { + let elapsed = now.saturating_duration_since(self.started); + match self.phase.get() { + Phase::Waiting => { + self.phase.set(Phase::Visible { + since: now, + elapsed, + }); + Some(SparkleFrame { + elapsed, + visibility: 1.0, + next: FRAME_TICK, + }) + } + Phase::Visible { + since, + elapsed: last_visible_elapsed, + } => { + let remaining_idle = + IDLE_TIMEOUT.saturating_sub(now.saturating_duration_since(since)); + if remaining_idle.is_zero() { + // Keep the last rendered dots in place, even if this frame arrives late. + self.phase.set(Phase::Fading { + started: since + IDLE_TIMEOUT, + elapsed: last_visible_elapsed, + duration: IDLE_FADE, + start_visibility: 1.0, + }); + return self.frame(now); + } + self.phase.set(Phase::Visible { since, elapsed }); + Some(SparkleFrame { + elapsed, + visibility: 1.0, + next: FRAME_TICK.min(remaining_idle), + }) + } + Phase::Fading { + started, + elapsed, + duration, + start_visibility, + } => { + let remaining_fade = + duration.saturating_sub(now.saturating_duration_since(started)); + if remaining_fade.is_zero() { + self.phase.set(Phase::Finished); + return None; + } + // Fade the last visible stars so neither timeout nor typing introduces dots. + Some(SparkleFrame { + elapsed, + visibility: start_visibility * remaining_fade.as_secs_f32() + / duration.as_secs_f32(), + next: FADE_FRAME_TICK.min(remaining_fade), + }) + } + Phase::Finished => None, + } + } } impl BottomPane { pub(crate) fn set_astra_sparkle(&mut self, model: &str, settings: &Tui) { - let started = self - .composer - .astra_sparkle - .as_ref() - .map_or_else(Instant::now, |sparkle| sparkle.started); - self.composer.astra_sparkle = Some(Sparkle { - model: model.to_owned(), - whimsy: settings.whimsy, - animations: settings.animations, - started, - }); + let changed = if !settings.whimsy || !settings.animations || !ASTRA_MODEL.is_match(model) { + self.composer.astra_sparkle.take().is_some() + } else if self.composer.astra_sparkle.is_none() { + self.composer.astra_sparkle = Some(Sparkle { + started: Instant::now(), + phase: Cell::new(Phase::Waiting), + terminal_focused: true, + }); + true + } else { + false + }; + if changed && let Some(requester) = &self.composer.frame_requester { + requester.schedule_frame(); + } + } + + pub(crate) fn set_sparkle_terminal_focus(&mut self, focused: bool) { + if let Some(sparkle) = &mut self.composer.astra_sparkle { + sparkle.terminal_focused = focused; + } } } impl ChatComposer { - pub(super) fn render_sparkle(&self, area: Rect, cursor: Option<(u16, u16)>, buf: &mut Buffer) { - if area.is_empty() || !matches!(self.popups.active, ActivePopup::None) { + pub(super) fn interact_with_astra_sparkle(&self) { + if let Some(sparkle) = &self.astra_sparkle { + sparkle.interact(Instant::now()); + } + } + + pub(super) fn render_sparkle( + &self, + area: Rect, + textarea: Rect, + cursor: Option<(u16, u16)>, + buf: &mut Buffer, + ) { + self.render_sparkle_at(area, textarea, cursor, Instant::now(), buf); + } + + fn render_sparkle_at( + &self, + area: Rect, + textarea: Rect, + cursor: Option<(u16, u16)>, + now: Instant, + buf: &mut Buffer, + ) { + if (!self.is_empty() + || self.draft.paste_burst.is_active() + || !self.draft.input_enabled + || self.voice_strip.is_some() + || !matches!(self.popups.active, ActivePopup::None)) + && let Some(sparkle) = &self.astra_sparkle + { + sparkle.interact(now); + } + if !self.has_focus || area.height < 3 || textarea.is_empty() { return; } if let Some(sparkle) = &self.astra_sparkle + && sparkle.terminal_focused && let Some(foreground) = sparkle.enabled_foreground() + && let Some(frame) = sparkle.frame(now) { - render_stars(area, cursor, sparkle.started.elapsed(), foreground, buf); + let protected_area = if !self.is_empty() || self.draft.paste_burst.is_active() { + Rect::new(area.x, textarea.y, area.width, textarea.height) + } else { + let placeholder = if self.draft.input_enabled { + self.placeholder_text.as_str() + } else { + self.draft + .input_disabled_placeholder + .as_deref() + .unwrap_or("Input disabled.") + }; + // Spaces inside the placeholder are text too; the terminal can copy them. + Rect::new( + textarea.x, + textarea.y, + placeholder.width().min(usize::from(textarea.width)) as u16, + /*height*/ 1, + ) + }; + render_stars( + area, + cursor, + Some(protected_area), + frame.elapsed, + foreground, + frame.visibility, + buf, + ); if let Some(requester) = &self.frame_requester { - requester.schedule_frame_in(FRAME_TICK); + requester.schedule_frame_in(frame.next); } } } @@ -86,8 +287,10 @@ impl ChatComposer { fn render_stars( area: Rect, cursor: Option<(u16, u16)>, + protected_area: Option, elapsed: Duration, foreground: (u8, u8, u8), + visibility: f32, buf: &mut Buffer, ) { let time = elapsed.as_secs_f32(); @@ -104,6 +307,7 @@ fn render_stars( } // Preserve the cursor, selections, and pixels from the effort bursts. if cursor == Some((x, y)) + || protected_area.is_some_and(|protected| protected.contains(Position::new(x, y))) || !cell.modifier.is_empty() || cell.diff_option != CellDiffOption::None { @@ -122,7 +326,7 @@ fn render_stars( } let phase = (time / (4.0 + (hash % 31) as f32 / 10.0) + (hash % 997) as f32 / 997.0).fract(); - let brightness = (phase * std::f32::consts::PI).sin().powi(12) * 0.55; + let brightness = (phase * std::f32::consts::PI).sin().powi(12) * 0.55 * visibility; if brightness < 0.04 { continue; } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/sparkle_tests.rs b/codex-rs/tui/src/bottom_pane/chat_composer/sparkle_tests.rs index 81795f4844..acbf577ef4 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/sparkle_tests.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/sparkle_tests.rs @@ -7,15 +7,66 @@ use codex_protocol::openai_models::ReasoningEffort; use pretty_assertions::assert_eq; use ratatui::style::Style; +const SPARKLE_PALETTE: DefaultColors = DefaultColors { + fg: (230, 216, 255), + bg: (36, 27, 53), +}; + fn new_test_sparkle() -> Sparkle { Sparkle { - model: "astra".to_string(), - whimsy: true, - animations: true, started: Instant::now(), + phase: Cell::new(Phase::Waiting), + terminal_focused: true, } } +fn new_test_pane() -> BottomPane { + let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); + BottomPane::new(crate::bottom_pane::BottomPaneParams { + app_event_tx: crate::app_event_sender::AppEventSender::new(tx), + frame_requester: crate::tui::FrameRequester::test_dummy(), + has_input_focus: true, + enhanced_keys_supported: false, + placeholder_text: "Ask Codex to do anything".into(), + disable_paste_burst: true, + animations_enabled: true, + skills: None, + }) +} + +fn buffer_text(buffer: &Buffer) -> String { + buffer + .content + .chunks(usize::from(buffer.area.width)) + .map(|row| { + row.iter() + .map(ratatui::buffer::Cell::symbol) + .collect::() + .trim_end() + .to_owned() + }) + .collect::>() + .join("\n") +} + +fn rendered_sparkle( + composer: &ChatComposer, + area: Rect, + now: Instant, + baseline: &Buffer, +) -> Buffer { + let [surface, _, textarea, _] = composer.layout_areas(area); + let mut buffer = baseline.clone(); + composer.render_sparkle_at( + surface, + textarea, + composer.cursor_pos(area), + now, + &mut buffer, + ); + buffer +} + #[test] fn sparkle_keeps_the_existing_composer_layout() { with_test_default_colors( @@ -38,8 +89,10 @@ fn sparkle_keeps_the_existing_composer_layout() { render_stars( surface, composer.cursor_pos(area), + /*protected_area*/ None, Duration::ZERO, /*foreground*/ (230, 216, 255), + /*visibility*/ 1.0, &mut buffer, ); // Snapshot the composer layout without freezing the star distribution. @@ -59,6 +112,294 @@ fn sparkle_keeps_the_existing_composer_layout() { ); } +#[test] +fn sparkle_covers_the_untouched_composer_and_fades_after_interaction() { + with_test_default_colors(SPARKLE_PALETTE, || { + let width = 40; + let (mut composer, _rx) = new_test_composer(); + let area = Rect::new( + /*x*/ 0, + /*y*/ 0, + width, + composer.desired_height(width), + ); + let [_, _, textarea, _] = composer.layout_areas(area); + let mut baseline = Buffer::empty(area); + composer.render(area, &mut baseline); + let sparkle = new_test_sparkle(); + let started = sparkle.started; + composer.astra_sparkle = Some(sparkle); + + let idle = started + Duration::from_secs(63); + let active = rendered_sparkle(&composer, area, idle, &baseline); + let stars = active + .content + .iter() + .enumerate() + .filter_map(|(index, cell)| DOTS.contains(&cell.symbol()).then_some(index)) + .collect::>(); + let star_rows = stars + .iter() + .map(|index| active.pos_of(*index).1) + .collect::>(); + assert!(star_rows.iter().any(|y| *y < textarea.y)); + assert!( + star_rows + .iter() + .any(|y| (textarea.y..textarea.bottom()).contains(y)) + ); + assert!(star_rows.iter().any(|y| *y >= textarea.bottom())); + let cursor = composer.cursor_pos(area).unwrap(); + assert_eq!(active[cursor], baseline[cursor]); + + composer.astra_sparkle.as_ref().unwrap().interact(idle); + let fading = rendered_sparkle(&composer, area, idle + INTERACTION_FADE / 2, &baseline); + assert!(fading.content.iter().enumerate().all(|(index, cell)| { + !DOTS.contains(&cell.symbol()) || active.content[index].symbol() == cell.symbol() + })); + assert!(stars.iter().any(|index| { + let cell = &fading.content[*index]; + DOTS.contains(&cell.symbol()) && cell.fg != active.content[*index].fg + })); + + let finished = rendered_sparkle(&composer, area, idle + INTERACTION_FADE, &baseline); + assert_eq!(finished, baseline); + insta::assert_snapshot!( + format!("astra_sparkle_interaction_{width}"), + format!( + "idle\n{}\n\nfading\n{}\n\nfinished\n{}", + buffer_text(&active), + buffer_text(&fading), + buffer_text(&finished), + ) + ); + }); +} + +#[test] +fn sparkle_auto_fades_fifteen_seconds_after_its_first_visible_frame() { + with_test_default_colors(SPARKLE_PALETTE, || { + let (mut composer, _rx) = new_test_composer(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 8, + ); + let mut baseline = Buffer::empty(area); + composer.render(area, &mut baseline); + composer.astra_sparkle = Some(new_test_sparkle()); + let first_frame = composer.astra_sparkle.as_ref().unwrap().started + + Duration::from_secs(/*secs*/ 63); + let first = rendered_sparkle(&composer, area, first_frame, &baseline); + assert_ne!(first, baseline); + + let deadline = first_frame + IDLE_TIMEOUT; + let just_before = deadline - Duration::from_millis(/*millis*/ 50); + let active = rendered_sparkle(&composer, area, just_before, &baseline); + assert_ne!(active, baseline); + assert_eq!( + composer + .astra_sparkle + .as_ref() + .unwrap() + .frame(just_before) + .unwrap() + .next, + Duration::from_millis(/*millis*/ 50) + ); + + let sparkle = composer.astra_sparkle.as_ref().unwrap(); + assert_eq!( + [ + sparkle.frame(deadline).unwrap().visibility, + sparkle.frame(deadline + IDLE_FADE / 4).unwrap().visibility, + sparkle.frame(deadline + IDLE_FADE / 2).unwrap().visibility, + ], + [1.0, 0.75, 0.5] + ); + let fading = rendered_sparkle(&composer, area, deadline + IDLE_FADE / 2, &baseline); + assert!(fading.content.iter().enumerate().all(|(index, cell)| { + !DOTS.contains(&cell.symbol()) || active.content[index].symbol() == cell.symbol() + })); + let finished = rendered_sparkle(&composer, area, deadline + IDLE_FADE, &baseline); + assert_eq!(finished, baseline); + assert!(matches!( + composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Finished + )); + insta::assert_snapshot!( + "astra_idle_timeout", + format!( + "before timeout\n{}\n\nfading\n{}\n\nfinished\n{}", + buffer_text(&active), + buffer_text(&fading), + buffer_text(&finished), + ) + ); + }); +} + +#[test] +fn sparkle_preserves_placeholder_spaces_during_idle_animation() { + with_test_default_colors(SPARKLE_PALETTE, || { + let (mut composer, _rx) = new_test_composer(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 8, + ); + let [_, _, textarea, _] = composer.layout_areas(area); + let mut baseline = Buffer::empty(area); + composer.render(area, &mut baseline); + composer.astra_sparkle = Some(new_test_sparkle()); + let start = composer.astra_sparkle.as_ref().unwrap().started + + Duration::from_secs(/*secs*/ 63); + let end = textarea.x + composer.placeholder_text.width() as u16; + for second in 0..IDLE_TIMEOUT.as_secs() { + let active = rendered_sparkle( + &composer, + area, + start + Duration::from_secs(second), + &baseline, + ); + for x in textarea.x..end { + assert_eq!(active[(x, textarea.y)], baseline[(x, textarea.y)]); + } + assert_ne!(active, baseline); + if second == 2 { + insta::assert_snapshot!("astra_placeholder_spaces", buffer_text(&active)); + } + } + }); +} + +#[test] +fn idle_deadline_elapses_while_terminal_focus_is_elsewhere() { + with_test_default_colors(SPARKLE_PALETTE, || { + let (mut composer, _rx) = new_test_composer(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 8, + ); + let mut baseline = Buffer::empty(area); + composer.render(area, &mut baseline); + composer.astra_sparkle = Some(new_test_sparkle()); + let first_frame = composer.astra_sparkle.as_ref().unwrap().started; + let active = rendered_sparkle(&composer, area, first_frame, &baseline); + assert_ne!(active, baseline); + composer.astra_sparkle.as_mut().unwrap().terminal_focused = false; + let after_deadline = first_frame + IDLE_TIMEOUT + IDLE_FADE; + let hidden = rendered_sparkle(&composer, area, after_deadline, &baseline); + assert_eq!(hidden, baseline); + assert!(matches!( + composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Visible { .. } + )); + composer.astra_sparkle.as_mut().unwrap().terminal_focused = true; + assert_eq!( + rendered_sparkle(&composer, area, after_deadline, &baseline), + baseline + ); + assert!(matches!( + composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Finished + )); + }); +} + +#[test] +fn input_during_the_idle_fade_finishes_quickly_without_brightening() { + let sparkle = new_test_sparkle(); + let first_frame = sparkle.started; + sparkle.frame(first_frame).unwrap(); + sparkle + .frame(first_frame + IDLE_TIMEOUT - Duration::from_millis(/*millis*/ 50)) + .unwrap(); + let interaction = first_frame + IDLE_TIMEOUT + IDLE_FADE / 2; + let before = sparkle.frame(interaction).unwrap().visibility; + sparkle.interact(interaction); + assert_eq!(sparkle.frame(interaction).unwrap().visibility, before); + assert!(sparkle.frame(interaction + INTERACTION_FADE).is_none()); +} + +#[test] +fn typing_and_paste_start_one_fade_without_decorating_the_draft() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + + with_test_default_colors(SPARKLE_PALETTE, || { + for paste in [false, true] { + let (mut composer, _rx) = new_test_composer(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 60, /*height*/ 8, + ); + composer.astra_sparkle = Some(new_test_sparkle()); + let mut initial = Buffer::empty(area); + composer.render(area, &mut initial); + + if paste { + composer.handle_paste("first line\n中文 words".to_owned()); + } else { + composer.handle_key_event(KeyEvent::from(KeyCode::Char('a'))); + composer.handle_key_event(KeyEvent::from(KeyCode::Char(' '))); + } + let sparkle = composer.astra_sparkle.take().unwrap(); + let Phase::Fading { started, .. } = sparkle.phase.get() else { + panic!("first input should start fading"); + }; + let [surface, _, textarea, _] = composer.layout_areas(area); + let mut baseline = Buffer::empty(area); + composer.render(area, &mut baseline); + composer.astra_sparkle = Some(sparkle); + + let fading = + rendered_sparkle(&composer, area, started + INTERACTION_FADE / 2, &baseline); + for y in textarea.y..textarea.bottom() { + for x in surface.x..surface.right() { + assert_eq!(fading[(x, y)], baseline[(x, y)]); + } + } + assert_eq!( + rendered_sparkle(&composer, area, started + INTERACTION_FADE, &baseline), + baseline + ); + composer.set_text_content(String::new(), Vec::new(), Vec::new()); + assert!(matches!( + composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Finished + )); + } + }); +} + +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn sparkle_stops_scheduling_after_the_fade() { + let (mut composer, _rx) = new_test_composer(); + let (draw_tx, mut draw_rx) = tokio::sync::broadcast::channel(/*capacity*/ 4); + composer.set_frame_requester(crate::tui::FrameRequester::new(draw_tx)); + composer.astra_sparkle = Some(new_test_sparkle()); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 60, /*height*/ 3, + ); + let render = |composer: &ChatComposer| { + with_test_default_colors(SPARKLE_PALETTE, || { + composer.render(area, &mut Buffer::empty(area)) + }); + }; + render(&composer); + assert!( + tokio::time::timeout(FRAME_TICK * 2, draw_rx.recv()) + .await + .is_ok() + ); + composer + .astra_sparkle + .as_ref() + .expect("sparkle") + .interact(Instant::now() - INTERACTION_FADE); + render(&composer); + assert!( + tokio::time::timeout(FRAME_TICK * 2, draw_rx.recv()) + .await + .is_err() + ); +} + #[test] fn sparkle_preserves_content_cursor_and_background() { let area = Rect::new( @@ -78,8 +419,10 @@ fn sparkle_preserves_content_cursor_and_background() { render_stars( area, Some((3, 0)), + /*protected_area*/ None, FRAME_TICK * tick, /*foreground*/ (230, 216, 255), + /*visibility*/ 1.0, &mut after, ); assert_eq!( @@ -122,8 +465,10 @@ fn stars_fade_using_the_custom_terminal_foreground() { render_stars( area, /*cursor*/ None, + /*protected_area*/ None, FRAME_TICK * tick, colors.fg, + /*visibility*/ 1.0, &mut buffer, ); for (index, cell) in buffer.content.iter().enumerate() { @@ -149,18 +494,48 @@ fn stars_fade_using_the_custom_terminal_foreground() { } #[test] -fn model_changes_and_disable_setting_control_sparkle() { - let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); - let mut pane = BottomPane::new(crate::bottom_pane::BottomPaneParams { - app_event_tx: crate::app_event_sender::AppEventSender::new(tx), - frame_requester: crate::tui::FrameRequester::test_dummy(), - has_input_focus: true, - enhanced_keys_supported: false, - placeholder_text: String::new(), - disable_paste_burst: true, - animations_enabled: true, - skills: None, +fn offline_composer_keys_fade_stars() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyEventKind; + use crossterm::event::KeyModifiers; + + with_test_default_colors(SPARKLE_PALETTE, || { + for code in [KeyCode::Enter, KeyCode::Tab, KeyCode::Left] { + let mut pane = new_test_pane(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 6, + ); + pane.composer.astra_sparkle = Some(new_test_sparkle()); + pane.composer.render(area, &mut Buffer::empty(area)); + pane.handle_disconnected_key(KeyEvent::new(KeyCode::Null, KeyModifiers::NONE)); + assert!(matches!( + pane.composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Visible { .. } + )); + pane.handle_disconnected_key(KeyEvent::new_with_kind( + code, + KeyModifiers::NONE, + KeyEventKind::Release, + )); + assert!(matches!( + pane.composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Visible { .. } + )); + + pane.handle_disconnected_key(KeyEvent::new(code, KeyModifiers::NONE)); + assert!(matches!( + pane.composer.astra_sparkle.as_ref().unwrap().phase.get(), + Phase::Fading { .. } + )); + assert_eq!(pane.composer.current_text(), ""); + } }); +} + +#[test] +fn model_changes_and_disable_setting_control_sparkle() { + let mut pane = new_test_pane(); with_test_default_colors( DefaultColors { fg: (230, 216, 255), @@ -178,6 +553,8 @@ fn model_changes_and_disable_setting_control_sparkle() { .astra_sparkle .as_ref() .map(|sparkle| sparkle.started); + pane.set_sparkle_terminal_focus(/*focused*/ false); + pane.set_sparkle_terminal_focus(/*focused*/ true); for model in ["astra", "ASTRA-preview", "openai/astra-2026-09-01"] { pane.set_astra_sparkle(model, &settings); assert_eq!( @@ -195,6 +572,15 @@ fn model_changes_and_disable_setting_control_sparkle() { started ); } + pane.composer.interact_with_astra_sparkle(); + pane.set_astra_sparkle("gpt-6-astra", &settings); + assert_eq!( + pane.composer + .astra_sparkle + .as_ref() + .and_then(Sparkle::enabled_foreground), + None, + ); for model in [ "gpt-5.6-sol", "astral", @@ -230,7 +616,7 @@ fn model_changes_and_disable_setting_control_sparkle() { } #[test] -fn sparkle_renders_with_effort_bursts_and_pauses_for_popups() { +fn sparkle_yields_to_drafts_effort_bursts_and_popups() { with_test_default_colors( DefaultColors { fg: (230, 216, 255), @@ -266,7 +652,7 @@ fn sparkle_renders_with_effort_bursts_and_pauses_for_popups() { after .content .iter() - .any(|cell| DOTS.contains(&cell.symbol())) + .all(|cell| !DOTS.contains(&cell.symbol())) ); assert_eq!( composer.cursor_pos(area).map(|p| after[p].clone()), @@ -279,7 +665,8 @@ fn sparkle_renders_with_effort_bursts_and_pauses_for_popups() { composer.sync_popups(); assert!(!matches!(composer.popups.active, ActivePopup::None)); let mut buffer = Buffer::empty(area); - composer.render_sparkle(area, /*cursor*/ None, &mut buffer); + let [surface, _, textarea, _] = composer.layout_areas(area); + composer.render_sparkle(surface, textarea, /*cursor*/ None, &mut buffer); assert_eq!(buffer, Buffer::empty(area)); }, ); @@ -292,8 +679,9 @@ fn sparkle_waits_for_terminal_colors() { let area = Rect::new( /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 3, ); + let [surface, _, textarea, _] = composer.layout_areas(area); let mut buffer = Buffer::empty(area); - composer.render_sparkle(area, /*cursor*/ None, &mut buffer); + composer.render_sparkle(surface, textarea, /*cursor*/ None, &mut buffer); assert_eq!(buffer, Buffer::empty(area)); with_test_default_colors( DefaultColors { @@ -302,7 +690,7 @@ fn sparkle_waits_for_terminal_colors() { }, || { buffer.set_style(area, Style::default().bg(rgb_color((36, 27, 53)))); - composer.render_sparkle(area, /*cursor*/ None, &mut buffer); + composer.render_sparkle(surface, textarea, /*cursor*/ None, &mut buffer); assert!( buffer .content @@ -375,7 +763,7 @@ fn sparkle_preserves_voice_indicator_styles() { .collect::>(); assert_eq!(content, expected, "sparkle frame {tick}"); } - assert!(saw_stars); + assert!(!saw_stars); insta::assert_snapshot!( "astra_voice_composer", baseline diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index c630d43f41..7cef3753c5 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -464,6 +464,10 @@ impl ChatWidget { ) } + pub(crate) fn set_sparkle_terminal_focus(&mut self, focused: bool) { + self.bottom_pane.set_sparkle_terminal_focus(focused); + } + pub(super) fn refresh_model_display(&mut self) { let effective = self.effective_collaboration_mode(); self.session_header.set_model(effective.model()); diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index 9b76570dfc..daedcbc21b 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -11,6 +11,31 @@ use ratatui::Terminal; use ratatui::backend::TestBackend; use serial_test::serial; +#[tokio::test] +async fn astra_sparkle_pauses_and_resumes_with_terminal_focus() { + let (mut chat, _rx, _ops) = make_chatwidget_manual(Some("gpt-5.6-sol")).await; + chat.local_settings.tui.animations = true; + chat.local_settings.tui.whimsy = true; + chat.set_model("gpt-6-astra"); + crate::terminal_palette::with_test_default_colors( + crate::terminal_probe::DefaultColors { + fg: (230, 216, 255), + bg: (36, 27, 53), + }, + || { + let contains_stars = |text: &str| text.chars().any(|ch| "⠁⠂⠄⠈⠐⠠⡀⢀".contains(ch)); + chat.set_sparkle_terminal_focus(/*focused*/ false); + assert!(!contains_stars(&render_bottom_popup( + &chat, /*width*/ 80 + ))); + chat.set_sparkle_terminal_focus(/*focused*/ true); + assert!(contains_stars(&render_bottom_popup( + &chat, /*width*/ 80 + ))); + }, + ); +} + #[tokio::test] async fn voice_live_transcript_renders_beside_the_streamed_cell() { let (mut chat, _rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index b667cca1b9..0023bd8fd7 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -142,6 +142,8 @@ where current: usize, /// Whether the cursor is currently hidden pub hidden_cursor: bool, + /// Last cursor style sent successfully, so ordinary redraws do not repaint its repair anchor. + last_cursor_style: Option, /// Area of the viewport pub viewport_area: Rect, /// Last known size of the terminal. Used to detect if the internal buffers have to be resized. @@ -221,6 +223,7 @@ where buffers: [Buffer::empty(Rect::ZERO), Buffer::empty(Rect::ZERO)], current: 0, hidden_cursor: false, + last_cursor_style: None, viewport_area: Rect::new( /*x*/ 0, cursor_pos.y, @@ -291,6 +294,10 @@ where /// current backend for drawing. pub fn flush(&mut self) -> io::Result<()> { let updates = diff_buffers(self.previous_buffer(), self.current_buffer()); + self.flush_updates(updates) + } + + fn flush_updates(&mut self, updates: Vec) -> io::Result<()> { let last_put_command = updates.iter().rfind(|command| command.is_put()); if let Some(&DrawCommand::Put { x, y, .. }) = last_put_command { self.last_known_cursor_pos = Position { x, y }; @@ -309,6 +316,9 @@ where /// Sets the viewport area. pub fn set_viewport_area(&mut self, area: Rect) { + if self.viewport_area != area { + self.invalidate_cursor_state(); + } self.current_buffer_mut().resize(area); self.previous_buffer_mut().resize(area); self.viewport_area = area; @@ -428,11 +438,17 @@ where let cursor_position = frame.cursor_position; let cursor_style = frame.cursor_style; - // Draw to stdout - self.flush()?; + // Not every terminal or multiplexer hides intermediate cursor moves inside a + // synchronized update, especially when the frame spans multiple writes. + let updates = diff_buffers(self.previous_buffer(), self.current_buffer()); + if !updates.is_empty() && !self.hidden_cursor { + self.hide_cursor()?; + } + self.flush_updates(updates)?; match cursor_position { - None => self.hide_cursor()?, + None if !self.hidden_cursor => self.hide_cursor()?, + None => {} Some(position) => { self.set_cursor_style_with_repair(cursor_style)?; self.set_cursor_position(position)?; @@ -463,7 +479,9 @@ where /// Sets the visible terminal cursor style. pub fn set_cursor_style(&mut self, style: SetCursorStyle) -> io::Result<()> { - queue!(self.backend, style) + queue!(self.backend, style)?; + self.last_cursor_style = Some(style); + Ok(()) } /// Restores the user-configured terminal cursor style. diff --git a/codex-rs/tui/src/custom_terminal/cursor.rs b/codex-rs/tui/src/custom_terminal/cursor.rs index 34a99d3e4b..7505870af7 100644 --- a/codex-rs/tui/src/custom_terminal/cursor.rs +++ b/codex-rs/tui/src/custom_terminal/cursor.rs @@ -2,8 +2,8 @@ //! //! This helper issues cursor-style commands only at an owned, non-skipped glyph leader //! with positive width that fits within its row, then redraws it with the same style and -//! hyperlink. Without a safe anchor, it omits the command. The caller must restore the -//! requested cursor position afterward. +//! hyperlink. Unchanged styles do not repaint that anchor. Without a safe anchor, it omits the +//! command. The caller must restore the requested cursor position afterward. use std::io; use std::io::Write; @@ -22,12 +22,21 @@ impl Terminal where B: Backend + Write, { + pub(crate) fn invalidate_cursor_state(&mut self) { + self.last_cursor_style = None; + // An external program or screen switch can show a cursor that we believed was hidden. + self.hidden_cursor = false; + } + pub(super) fn set_cursor_style_with_repair( &mut self, cursor_style: SetCursorStyle, ) -> io::Result<()> { + if self.last_cursor_style == Some(cursor_style) { + return Ok(()); + } // JediTerm before 3.56 prints DECSCUSR's space intermediate at the cursor. - // Apply the style over an owned glyph, then repair it even on unchanged frames. + // Apply a changed style over an owned glyph, then repair it even on unchanged frames. // https://github.com/JetBrains/jediterm/commit/0c4524f2978bddae65a46c35f264bf89e2ed58fd let buffer = &self.buffers[self.current]; let anchor = (0..buffer.area.height).find_map(|row| { @@ -47,6 +56,9 @@ where }); // Empty and externally owned viewports have no cell we can safely repair. if let Some((anchor, cell)) = anchor { + if !self.hidden_cursor { + self.hide_cursor()?; + } self.set_cursor_position(anchor)?; self.set_cursor_style(cursor_style)?; let Position { x, y } = anchor; diff --git a/codex-rs/tui/src/custom_terminal/tests/cursor_tests.rs b/codex-rs/tui/src/custom_terminal/tests/cursor_tests.rs index 1d56c2a5e5..a6fd62b94b 100644 --- a/codex-rs/tui/src/custom_terminal/tests/cursor_tests.rs +++ b/codex-rs/tui/src/custom_terminal/tests/cursor_tests.rs @@ -2,6 +2,94 @@ use super::*; use crate::terminal_palette::rgb_color; use pretty_assertions::assert_eq; +#[test] +fn terminal_draw_keeps_intermediate_cursor_positions_hidden() { + let mut terminal = + Terminal::with_options(CaptureBackend::new(/*width*/ 12, /*height*/ 2)).expect("terminal"); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 12, /*height*/ 2, + ); + let cursor = (1, 4); + terminal.set_viewport_area(area); + let mut parser = + vt100::Parser::new(/*rows*/ 2, /*cols*/ 12, /*scrollback_len*/ 0); + + for (index, (marker, show_cursor)) in [ + ("x", true), + ("y", true), + ("y", true), + ("", false), + ("z", true), + ] + .into_iter() + .enumerate() + { + terminal.backend_mut().output.clear(); + terminal + .draw(|frame| { + let buffer = frame.buffer_mut(); + buffer.set_string(0, 0, "anchor", Style::default()); + if !marker.is_empty() { + buffer.set_string(10, 1, marker, Style::default()); + } + if show_cursor { + frame.set_cursor_position((cursor.1, cursor.0)); + } + }) + .expect("draw"); + + // Process one byte at a time to expose cursor motion even if a terminal displays a + // synchronized frame before all writes have arrived. + let output = terminal.backend().output(); + for byte in output.bytes() { + parser.process(&[byte]); + if index > 0 && !parser.screen().hide_cursor() { + assert_eq!(parser.screen().cursor_position(), cursor, "frame {index}"); + } + } + assert_eq!(parser.screen().hide_cursor(), !show_cursor, "frame {index}"); + assert_eq!( + parser.screen().cell(1, 10).expect("marker cell").contents(), + marker, + "frame {index}" + ); + } +} + +#[test] +fn terminal_draw_repaints_the_cursor_anchor_only_when_its_style_needs_restoring() { + let mut terminal = + Terminal::with_options(CaptureBackend::new(/*width*/ 12, /*height*/ 2)).expect("terminal"); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 12, /*height*/ 2, + ); + terminal.set_viewport_area(area); + let render = |terminal: &mut Terminal, marker| { + terminal.backend_mut().output.clear(); + terminal + .draw(|frame| { + let buffer = frame.buffer_mut(); + buffer.set_string(0, 0, "transcript", Style::default()); + buffer.set_string(10, 1, marker, Style::default()); + frame.set_cursor_position((4, 1)); + }) + .expect("draw"); + terminal.backend().output() + }; + + let first = render(&mut terminal, "x"); + assert!(first.contains("\x1b[1;1H")); + let next = render(&mut terminal, "y"); + assert!(next.contains("\x1b[2;11H")); + assert!(!next.contains("\x1b[1;1H")); + + terminal.invalidate_cursor_state(); + let restored = render(&mut terminal, "y"); + assert!(restored.contains("\x1b[1;1H")); + let unchanged = render(&mut terminal, "y"); + assert!(!unchanged.contains("\x1b[1;1H")); +} + #[test] fn terminal_draw_repairs_styled_anchor_on_cursor_only_frames() { let mut terminal = diff --git a/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_owned_wide_frames.snap b/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_owned_wide_frames.snap index f7a668b9c2..9994b559ed 100644 --- a/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_owned_wide_frames.snap +++ b/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_owned_wide_frames.snap @@ -1,6 +1,7 @@ --- source: tui/src/custom_terminal/tests/cursor_tests.rs +assertion_line: 158 expression: "frames.join(\"\\n\")" --- -\u{1b}[1;4H\u{1b}[39;48;5;4m\u{1b}]8;;https://example.com\u{7}界\u{1b}]8;;\u{7}\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;4H\u{1b}[0 q\u{1b}[1;4H\u{1b}[39;48;5;4m\u{1b}]8;;https://example.com\u{7}界\u{1b}]8;;\u{7}\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;6H\u{1b}[?25h -\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;4H\u{1b}[0 q\u{1b}[1;4H\u{1b}[39;48;5;4m\u{1b}]8;;https://example.com\u{7}界\u{1b}]8;;\u{7}\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;6H\u{1b}[?25h +\u{1b}[?25l\u{1b}[1;4H\u{1b}[39;48;5;4m\u{1b}]8;;https://example.com\u{7}界\u{1b}]8;;\u{7}\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;4H\u{1b}[0 q\u{1b}[1;4H\u{1b}[39;48;5;4m\u{1b}]8;;https://example.com\u{7}界\u{1b}]8;;\u{7}\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;6H\u{1b}[?25h +\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;6H\u{1b}[?25h diff --git a/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_single_column_frames.snap b/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_single_column_frames.snap index d962c8781b..904d9e1062 100644 --- a/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_single_column_frames.snap +++ b/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_single_column_frames.snap @@ -1,7 +1,8 @@ --- source: tui/src/custom_terminal/tests/cursor_tests.rs +assertion_line: 189 expression: "frames.join(\"\\n\")" --- -\u{1b}[1;1H\u{1b}[39;48;5;4mx\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[0 q\u{1b}[1;1H\u{1b}[39;48;5;4mx\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[?25h -\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[0 q\u{1b}[1;1H\u{1b}[39;48;5;4mx\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[?25h -\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[0 q\u{1b}[1;1H\u{1b}[39;48;5;4mx\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[?25h +\u{1b}[?25l\u{1b}[1;1H\u{1b}[39;48;5;4mx\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[0 q\u{1b}[1;1H\u{1b}[39;48;5;4mx\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[?25h +\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[?25h +\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[1;1H\u{1b}[?25h diff --git a/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_styled_frames.snap b/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_styled_frames.snap index 25841a4597..b41396a887 100644 --- a/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_styled_frames.snap +++ b/codex-rs/tui/src/custom_terminal/tests/snapshots/codex_tui__custom_terminal__tests__cursor__cursor_style_styled_frames.snap @@ -1,8 +1,9 @@ --- source: tui/src/custom_terminal/tests/cursor_tests.rs +assertion_line: 130 expression: "frames.join(\"\\n\")" --- -\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80mab cd ef \u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;1H\u{1b}[0 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;2H\u{1b}[?25h -\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;1H\u{1b}[0 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;5H\u{1b}[?25h -\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;1H\u{1b}[6 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;4H\u{1b}[?25h -\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;1H\u{1b}[2 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;2H\u{1b}[?25h +\u{1b}[?25l\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80mab cd ef \u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;1H\u{1b}[0 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;2H\u{1b}[?25h +\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;5H\u{1b}[?25h +\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[?25l\u{1b}[2;1H\u{1b}[6 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;4H\u{1b}[?25h +\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[?25l\u{1b}[2;1H\u{1b}[2 q\u{1b}[2;1H\u{1b}[1m\u{1b}[39;48;2;80;80;80ma\u{1b}[39m\u{1b}[49m\u{1b}[0m\u{1b}[2;2H\u{1b}[?25h diff --git a/codex-rs/tui/src/snapshots/codex_tui__tui__panic_tests__caught_panic_cursor_commands.snap b/codex-rs/tui/src/snapshots/codex_tui__tui__panic_tests__caught_panic_cursor_commands.snap new file mode 100644 index 0000000000..94e77246dd --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__tui__panic_tests__caught_panic_cursor_commands.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/tui_panic_tests.rs +expression: "phases.join(\"\\n\")" +--- +panic hook: \u{1b}[0 q \u{1b}[?25h +hidden repaint: \u{1b}[?25l \u{1b}[1;12H \u{1b}[2;2H \u{1b}[1;1H \u{1b}[2;1H +visible frame: \u{1b}[1;1H \u{1b}[6 q \u{1b}[1;1H \u{1b}[2;5H \u{1b}[?25h diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 2e3b272d66..223e285fce 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -69,6 +69,9 @@ mod input_boundary; #[cfg(unix)] mod job_control; mod keyboard_modes; +#[cfg(all(test, unix))] +#[path = "tui_panic_tests.rs"] +mod panic_tests; mod screen_size; mod scrollback; mod size_monitor; @@ -724,6 +727,7 @@ impl Tui { pub(crate) fn recover_after_caught_panic(&mut self) -> Result<()> { set_modes()?; self._stderr_guard.recover_after_caught_panic()?; + self.terminal.invalidate_cursor_state(); self.terminal.invalidate_viewport(); self.frame_requester().schedule_frame(); Ok(()) @@ -776,6 +780,7 @@ impl Tui { if let Err(err) = set_modes() { tracing::warn!("failed to re-enable terminal modes after external program: {err}"); } + self.terminal.invalidate_cursor_state(); // After the external program `f` finishes, reset terminal state and flush any buffered keypresses. flush_terminal_input_buffer(); @@ -841,6 +846,7 @@ impl Tui { return Ok(()); } let _ = execute!(self.terminal.backend_mut(), EnterAlternateScreen); + self.terminal.invalidate_cursor_state(); // Enable "alternate scroll" so terminals may translate wheel to arrows let _ = execute!(self.terminal.backend_mut(), EnableAlternateScroll); if let Ok(size) = self.terminal.size() { @@ -866,6 +872,7 @@ impl Tui { // Disable alternate scroll when leaving alt-screen let _ = execute!(self.terminal.backend_mut(), DisableAlternateScroll); let _ = execute!(self.terminal.backend_mut(), LeaveAlternateScreen); + self.terminal.invalidate_cursor_state(); if let Some(saved) = self.alt_saved_viewport.take() { self.terminal.set_viewport_area(saved); } @@ -1002,6 +1009,7 @@ impl Tui { stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { + self.terminal.invalidate_cursor_state(); prepared.apply(&mut self.terminal, screen_size)?; } @@ -1137,6 +1145,7 @@ impl Tui { stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { + self.terminal.invalidate_cursor_state(); prepared.apply(&mut self.terminal, screen_size)?; } diff --git a/codex-rs/tui/src/tui_panic_tests.rs b/codex-rs/tui/src/tui_panic_tests.rs new file mode 100644 index 0000000000..95dca4af55 --- /dev/null +++ b/codex-rs/tui/src/tui_panic_tests.rs @@ -0,0 +1,177 @@ +//! Exercise caught-panic recovery through a real terminal and the production panic hook. + +use std::fs::File; +use std::io::Read as _; +use std::io::Write as _; +use std::os::fd::FromRawFd; +use std::process::Command; +use std::process::Stdio; + +use crossterm::cursor::SetCursorStyle; +use ratatui::layout::Rect; +use ratatui::style::Style; + +const BEFORE_PANIC: &str = "\x1b]1337;codex-before-panic\x07"; +const BEFORE_RECOVERY: &str = "\x1b]1337;codex-before-recovery\x07"; +const BEFORE_VISIBLE_FRAME: &str = "\x1b]1337;codex-before-visible-frame\x07"; +const AFTER_VISIBLE_FRAME: &str = "\x1b]1337;codex-after-visible-frame\x07"; + +#[test] +fn caught_panic_rehides_cursor_before_repainting_and_restores_its_shape() { + let mut master = -1; + let mut slave = -1; + let mut size = libc::winsize { + ws_row: 24, + ws_col: 80, + ws_xpixel: 0, + ws_ypixel: 0, + }; + // SAFETY: openpty initializes both file descriptors and reads the supplied window size. + let result = unsafe { + libc::openpty( + &mut master, + &mut slave, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::addr_of_mut!(size), + ) + }; + assert!( + result == 0, + "open test PTY: {}", + std::io::Error::last_os_error() + ); + // SAFETY: a successful openpty call returned two owned file descriptors. + let mut master = unsafe { File::from_raw_fd(master) }; + // SAFETY: the slave descriptor is distinct from the master and is likewise newly owned. + let slave = unsafe { File::from_raw_fd(slave) }; + let child = Command::new(std::env::current_exe().expect("current test binary")) + .args([ + "--exact", + "tui::panic_tests::caught_panic_pty_child", + "--ignored", + "--nocapture", + ]) + .env("CODEX_TUI_CAUGHT_PANIC_PTY_CHILD", "1") + .stdin(Stdio::from(slave.try_clone().expect("clone PTY slave"))) + .stdout(Stdio::from(slave)) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn caught panic child"); + let reader = std::thread::spawn(move || { + let mut output = Vec::new(); + let mut buffer = [0; 4096]; + loop { + match master.read(&mut buffer) { + Ok(0) => break, + Ok(read) => output.extend_from_slice(&buffer[..read]), + Err(err) if err.kind() == std::io::ErrorKind::Interrupted => {} + // Linux reports EIO when the last PTY slave closes; other systems can return EOF. + Err(err) if err.raw_os_error() == Some(libc::EIO) => break, + Err(err) => panic!("read child PTY: {err}"), + } + } + output + }); + let child = child + .wait_with_output() + .expect("wait for caught panic child"); + let output = reader.join().expect("read caught panic terminal output"); + let output = String::from_utf8_lossy(&output); + assert!( + child.status.success(), + "child stderr: {}\nterminal: {output}", + String::from_utf8_lossy(&child.stderr) + ); + + let (_, output) = output.split_once(BEFORE_PANIC).expect("before panic"); + let (hook, output) = output.split_once(BEFORE_RECOVERY).expect("before recovery"); + let (hidden, output) = output + .split_once(BEFORE_VISIBLE_FRAME) + .expect("before visible frame"); + let (visible, _) = output + .split_once(AFTER_VISIBLE_FRAME) + .expect("after visible frame"); + + let cursor_move = regex_lite::Regex::new(r"\x1b\[[0-9]+;[0-9]+H").expect("cursor move regex"); + assert!( + hook.contains("\x1b[0 q") && hook.contains("\x1b[?25h"), + "panic hook did not reset and show the cursor" + ); + assert!( + hidden.find("\x1b[?25l").expect("hide after panic") + < cursor_move + .find(hidden) + .expect("repaint cursor movement") + .start(), + "repaint moved a visible cursor" + ); + assert!( + !hidden.contains("\x1b[?25h"), + "hidden frame exposed the cursor" + ); + assert!( + visible.find("\x1b[6 q").expect("restore steady bar") + < visible.find("\x1b[?25h").expect("show restored cursor") + ); + + let cursor_commands = + regex_lite::Regex::new(r"\x1b\[\?25[hl]|\x1b\[[0-9] q|\x1b\[[0-9]+;[0-9]+H") + .expect("cursor command regex"); + let phases = [ + ("panic hook", hook), + ("hidden repaint", hidden), + ("visible frame", visible), + ] + .map(|(name, bytes)| { + let commands = cursor_commands + .find_iter(bytes) + .map(|m| m.as_str().escape_debug().to_string()) + .collect::>(); + format!("{name}: {}", commands.join(" ")) + }); + insta::assert_snapshot!("caught_panic_cursor_commands", phases.join("\n")); +} + +#[tokio::test(flavor = "current_thread")] +#[ignore] +async fn caught_panic_pty_child() { + if std::env::var_os("CODEX_TUI_CAUGHT_PANIC_PTY_CHILD").is_none() { + return; + } + super::set_modes().expect("set terminal modes"); + super::set_panic_hook(); + let mut tui = super::test_support::make_test_tui().expect("create TUI"); + tui.terminal.set_viewport_area(Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 12, /*height*/ 2, + )); + + draw(&mut tui, "before panic", Some((4, 1))).expect("draw initial visible cursor"); + draw(&mut tui, "before panic", /*cursor*/ None).expect("hide initial cursor"); + mark(BEFORE_PANIC).expect("mark before panic"); + assert!(std::panic::catch_unwind(|| panic!("simulate caught startup panic")).is_err()); + mark(BEFORE_RECOVERY).expect("mark before recovery"); + tui.recover_after_caught_panic() + .expect("recover TUI after panic"); + draw(&mut tui, "after panic", /*cursor*/ None).expect("draw recovered hidden cursor"); + mark(BEFORE_VISIBLE_FRAME).expect("mark before visible frame"); + draw(&mut tui, "after panic", Some((4, 1))).expect("draw recovered visible cursor"); + mark(AFTER_VISIBLE_FRAME).expect("mark after visible frame"); + super::restore_after_exit().expect("restore terminal after test"); +} + +fn draw(tui: &mut super::Tui, text: &str, cursor: Option<(u16, u16)>) -> std::io::Result<()> { + tui.terminal.draw(|frame| { + frame.buffer_mut().set_string(0, 0, text, Style::default()); + if let Some(cursor) = cursor { + frame.set_cursor_style(SetCursorStyle::SteadyBar); + frame.set_cursor_position(cursor); + } + }) +} + +fn mark(marker: &str) -> std::io::Result<()> { + let mut stdout = std::io::stdout().lock(); + stdout.write_all(marker.as_bytes())?; + stdout.flush() +} diff --git a/codex-rs/tui/tests/suite/reconnect.rs b/codex-rs/tui/tests/suite/reconnect.rs index 3b530d463c..05f2eef87d 100644 --- a/codex-rs/tui/tests/suite/reconnect.rs +++ b/codex-rs/tui/tests/suite/reconnect.rs @@ -167,9 +167,20 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re _ => {} } } + // A PTY read can expose the history write before the same terminal update redraws the + // composer. Wait for both pieces in the same parsed screen before checking recovery. + let deadline = Instant::now() + Duration::from_secs(/*secs*/ 30); + while !(terminal.screen_contains("preserved-draft!") + && terminal.screen_contains("fresh-notification-after-reconnect")) + && Instant::now() < deadline + { + terminal.read_output(Duration::from_millis(/*millis*/ 20))?; + } ensure!( - terminal.screen_contains("preserved-draft!"), - "draft was lost after recovery" + terminal.screen_contains("preserved-draft!") + && terminal.screen_contains("fresh-notification-after-reconnect"), + "draft and notification did not remain visible after recovery; screen:\n{}", + terminal.screen_contents() ); drop(terminal); let methods = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), server).await???;