diff --git a/codex-rs/tui/src/app/resize_reflow.rs b/codex-rs/tui/src/app/resize_reflow.rs index 74ed18d251..21fbf32aa9 100644 --- a/codex-rs/tui/src/app/resize_reflow.rs +++ b/codex-rs/tui/src/app/resize_reflow.rs @@ -472,7 +472,7 @@ impl App { }); } - let mut has_emitted_history_lines = false; + let mut has_emitted_history_lines = source_start > 0; let mut reflowed_lines = Vec::new(); for display in cell_displays { if !display.lines.is_empty() && !display.is_stream_continuation { diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 926fe38283..8f54882137 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -4073,6 +4073,29 @@ async fn capped_history_replay_excludes_preserved_header_from_source_render() { assert_eq!(rendered, expected); } +#[tokio::test] +async fn history_replay_preserves_separator_after_session_header_snapshot() { + let (mut app, _rx, _op_rx) = make_test_app_with_channels().await; + app.config.terminal_resize_reflow.max_rows = TerminalResizeReflowMaxRows::Disabled; + app.transcript_cells = vec![ + plain_line_cell("session header"), + plain_line_cell("first message"), + plain_line_cell("second message"), + ]; + + let replayed = + app.render_history_replay_lines(/*source_start*/ 1, /*terminal_width*/ 80); + let rendered = std::iter::once("session header".to_string()) + .chain(replayed.iter().map(rendered_line_text)) + .collect::>() + .join("\n"); + + assert_app_snapshot!( + "history_replay_preserves_separator_after_session_header", + rendered, + ); +} + #[tokio::test] async fn history_replay_uses_pet_reserved_width() { let (mut app, _rx, _op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__history_replay_preserves_separator_after_session_header.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__history_replay_preserves_separator_after_session_header.snap new file mode 100644 index 0000000000..e9cb121292 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__history_replay_preserves_separator_after_session_header.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/app/tests.rs +expression: rendered +--- +session header + +first message + +second message diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index c0a4e263ad..f2f7804262 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -59,8 +59,12 @@ mod frame_requester; #[cfg(unix)] mod job_control; mod keyboard_modes; +mod replay_clear; mod terminal_stderr; +use self::replay_clear::ReplayClearState; +use self::replay_clear::run_synchronized_draw; + /// Target frame interval for UI redraw scheduling. pub(crate) const TARGET_FRAME_INTERVAL: Duration = frame_rate_limiter::MIN_FRAME_INTERVAL; @@ -84,16 +88,6 @@ fn should_emit_notification(condition: NotificationCondition, terminal_focused: } } -fn run_synchronized_draw( - writer: &mut W, - operations: impl FnOnce(&mut W) -> Result, -) -> Result -where - W: Write, -{ - writer.sync_update(operations)? -} - impl Drop for Tui { fn drop(&mut self) { if let Err(err) = self.clear_ambient_pet_image() { @@ -107,12 +101,10 @@ mod tests { use std::io::Write as _; use super::clear_for_viewport_change; - use super::run_synchronized_draw; use super::should_emit_notification; use crate::custom_terminal::Terminal as CustomTerminal; use crate::test_backend::VT100Backend; use codex_config::types::NotificationCondition; - use pretty_assertions::assert_eq; use ratatui::layout::Position; use ratatui::layout::Rect; @@ -180,79 +172,6 @@ mod tests { "expected stale cells inside the new viewport to be cleared, rows: {rows:?}" ); } - - #[test] - fn synchronized_draw_wraps_replay_clear_and_repaint() { - let mut output = Vec::new(); - let mut replay_clear_pending = true; - let replay_clear_requested = replay_clear_pending; - - let value = run_synchronized_draw(&mut output, |writer| { - assert!(replay_clear_requested); - writer.write_all(b"clear")?; - writer.write_all(b"replay")?; - replay_clear_pending = false; - Ok(42) - }) - .expect("synchronized draw"); - - assert_eq!(value, 42); - assert!(!replay_clear_pending); - assert_eq!(output, b"\x1b[?2026hclearreplay\x1b[?2026l"); - } - - #[test] - fn synchronized_draw_retries_replay_clear_after_render_error() { - let mut output = Vec::new(); - let replay_clear_pending = true; - let replay_clear_requested = replay_clear_pending; - - let error = run_synchronized_draw(&mut output, |writer| { - assert!(replay_clear_requested); - writer.write_all(b"clear")?; - Err::<(), _>(std::io::Error::other("render failed")) - }) - .expect_err("draw should fail"); - - assert_eq!(error.kind(), std::io::ErrorKind::Other); - assert!(replay_clear_pending); - assert_eq!(output, b"\x1b[?2026hclear\x1b[?2026l"); - } - - #[test] - fn synchronized_draw_does_not_retry_clear_after_replay_is_committed() { - struct FailOnFlush { - output: Vec, - } - - impl std::io::Write for FailOnFlush { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - self.output.extend_from_slice(buf); - Ok(buf.len()) - } - - fn flush(&mut self) -> std::io::Result<()> { - Err(std::io::Error::other("flush failed")) - } - } - - let mut writer = FailOnFlush { output: Vec::new() }; - let mut replay_clear_pending = true; - let replay_clear_requested = replay_clear_pending; - - let error = run_synchronized_draw(&mut writer, |writer| { - assert!(replay_clear_requested); - writer.write_all(b"clear")?; - writer.write_all(b"replay")?; - replay_clear_pending = false; - Ok(()) - }) - .expect_err("sync flush should fail"); - - assert_eq!(error.kind(), std::io::ErrorKind::Other); - assert!(!replay_clear_pending); - assert_eq!(writer.output, b"\x1b[?2026hclearreplay\x1b[?2026l"); - } } pub fn set_modes() -> Result<()> { @@ -584,7 +503,7 @@ pub struct Tui { // True when overlay alt-screen UI is active alt_screen_active: Arc, // Rebuilds clear the terminal inside the next synchronized draw before replaying history. - terminal_replay_clear_pending: bool, + replay_clear: ReplayClearState, // True when terminal/tab is focused; updated internally from crossterm events terminal_focused: Arc, enhanced_keys_supported: bool, @@ -641,7 +560,7 @@ impl Tui { #[cfg(unix)] suspend_context: SuspendContext::new(), alt_screen_active: Arc::new(AtomicBool::new(false)), - terminal_replay_clear_pending: false, + replay_clear: ReplayClearState::default(), terminal_focused: Arc::new(AtomicBool::new(true)), enhanced_keys_supported, notification_backend: Some(detect_backend(NotificationMethod::default())), @@ -679,7 +598,7 @@ impl Tui { } pub(crate) fn request_terminal_replay_clear(&mut self) { - self.terminal_replay_clear_pending = true; + self.replay_clear.request(); } // Drop crossterm EventStream to avoid stdin conflicts with other processes. @@ -958,7 +877,7 @@ impl Tui { // Precompute any viewport updates that need a cursor-position query before entering // the synchronized update, to avoid racing with the event reader. - let mut pending_viewport_area = if self.terminal_replay_clear_pending { + let mut pending_viewport_area = if self.replay_clear.is_pending() { None } else { self.pending_viewport_area()? @@ -966,15 +885,14 @@ impl Tui { ensure_virtual_terminal_processing()?; - let mut replay_clear_pending = self.terminal_replay_clear_pending; - let replay_clear_requested = replay_clear_pending; + let mut replay_clear = self.replay_clear.begin_draw(); let result = run_synchronized_draw(&mut stdout(), |_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { prepared.apply(&mut self.terminal)?; } - if replay_clear_requested { + if replay_clear.requested() { self.clear_terminal_for_replay()?; } @@ -1008,9 +926,7 @@ impl Tui { &mut self.pending_history_lines, self.is_zellij, )?; - if replay_clear_requested { - replay_clear_pending = false; - } + replay_clear.commit_replay(); // Update the y position for suspending so Ctrl-Z can place the cursor correctly. #[cfg(unix)] @@ -1030,7 +946,7 @@ impl Tui { draw_fn(frame); }) }); - self.terminal_replay_clear_pending = replay_clear_pending; + self.replay_clear.finish_draw(replay_clear); result } @@ -1109,15 +1025,14 @@ impl Tui { ensure_virtual_terminal_processing()?; - let mut replay_clear_pending = self.terminal_replay_clear_pending; - let replay_clear_requested = replay_clear_pending; + let mut replay_clear = self.replay_clear.begin_draw(); let result = run_synchronized_draw(&mut stdout(), |_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { prepared.apply(&mut self.terminal)?; } - if replay_clear_requested { + if replay_clear.requested() { self.clear_terminal_for_replay()?; } @@ -1129,9 +1044,7 @@ impl Tui { &mut self.pending_history_lines, self.is_zellij, )?; - if replay_clear_requested { - replay_clear_pending = false; - } + replay_clear.commit_replay(); if needs_full_repaint { terminal.invalidate_viewport(); @@ -1155,7 +1068,7 @@ impl Tui { draw_fn(frame); }) }); - self.terminal_replay_clear_pending = replay_clear_pending; + self.replay_clear.finish_draw(replay_clear); result } diff --git a/codex-rs/tui/src/tui/replay_clear.rs b/codex-rs/tui/src/tui/replay_clear.rs new file mode 100644 index 0000000000..cb7c9ce5f0 --- /dev/null +++ b/codex-rs/tui/src/tui/replay_clear.rs @@ -0,0 +1,63 @@ +use std::io::Result; +use std::io::Write; + +use crossterm::SynchronizedUpdate; + +#[derive(Debug, Default)] +pub(super) struct ReplayClearState { + pending: bool, +} + +impl ReplayClearState { + pub(super) fn request(&mut self) { + self.pending = true; + } + + pub(super) fn is_pending(&self) -> bool { + self.pending + } + + pub(super) fn begin_draw(&self) -> ReplayClearDraw { + ReplayClearDraw { + requested: self.pending, + replay_committed: false, + } + } + + pub(super) fn finish_draw(&mut self, draw: ReplayClearDraw) { + if draw.replay_committed { + self.pending = false; + } + } +} + +pub(super) struct ReplayClearDraw { + requested: bool, + replay_committed: bool, +} + +impl ReplayClearDraw { + pub(super) fn requested(&self) -> bool { + self.requested + } + + pub(super) fn commit_replay(&mut self) { + if self.requested { + self.replay_committed = true; + } + } +} + +pub(super) fn run_synchronized_draw( + writer: &mut W, + operations: impl FnOnce(&mut W) -> Result, +) -> Result +where + W: Write, +{ + writer.sync_update(operations)? +} + +#[cfg(test)] +#[path = "replay_clear_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/tui/replay_clear_tests.rs b/codex-rs/tui/src/tui/replay_clear_tests.rs new file mode 100644 index 0000000000..6962a20565 --- /dev/null +++ b/codex-rs/tui/src/tui/replay_clear_tests.rs @@ -0,0 +1,85 @@ +use std::io::Write as _; + +use pretty_assertions::assert_eq; + +use super::ReplayClearState; +use super::run_synchronized_draw; + +#[test] +fn synchronized_draw_wraps_replay_clear_and_repaint() { + let mut output = Vec::new(); + let mut state = ReplayClearState::default(); + state.request(); + let mut draw = state.begin_draw(); + + let value = run_synchronized_draw(&mut output, |writer| { + assert!(draw.requested()); + writer.write_all(b"clear")?; + writer.write_all(b"replay")?; + draw.commit_replay(); + Ok(42) + }) + .expect("synchronized draw"); + state.finish_draw(draw); + + assert_eq!(value, 42); + assert!(!state.is_pending()); + assert_eq!(output, b"\x1b[?2026hclearreplay\x1b[?2026l"); +} + +#[test] +fn synchronized_draw_retries_replay_clear_after_render_error() { + let mut output = Vec::new(); + let mut state = ReplayClearState::default(); + state.request(); + let draw = state.begin_draw(); + + let error = run_synchronized_draw(&mut output, |writer| { + assert!(draw.requested()); + writer.write_all(b"clear")?; + Err::<(), _>(std::io::Error::other("render failed")) + }) + .expect_err("draw should fail"); + state.finish_draw(draw); + + assert_eq!(error.kind(), std::io::ErrorKind::Other); + assert!(state.is_pending()); + assert_eq!(output, b"\x1b[?2026hclear\x1b[?2026l"); +} + +#[test] +fn synchronized_draw_does_not_retry_clear_after_replay_is_committed() { + struct FailOnFlush { + output: Vec, + } + + impl std::io::Write for FailOnFlush { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.output.extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Err(std::io::Error::other("flush failed")) + } + } + + let mut writer = FailOnFlush { output: Vec::new() }; + let mut state = ReplayClearState::default(); + state.request(); + let mut draw = state.begin_draw(); + + let error = run_synchronized_draw(&mut writer, |writer| { + assert!(draw.requested()); + writer.write_all(b"clear")?; + writer.write_all(b"replay")?; + draw.commit_replay(); + Ok(()) + }) + .expect_err("sync flush should fail"); + state.finish_draw(draw); + + assert_eq!(error.kind(), std::io::ErrorKind::Other); + assert!(!state.is_pending()); + assert_eq!(writer.output, b"\x1b[?2026hclearreplay\x1b[?2026l"); +}