diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index eace45389..b256ccd20 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -340,6 +340,11 @@ pub(crate) struct App { // Esc-backtracking state grouped pub(crate) backtrack: crate::app_backtrack::BacktrackState, + /// When set, the next draw re-renders the transcript into terminal scrollback once. + /// + /// This is used after a confirmed thread rollback to ensure scrollback reflects the trimmed + /// transcript cells. + pub(crate) backtrack_render_pending: bool, pub(crate) feedback: codex_feedback::CodexFeedback, /// Set when the user confirms an update; propagated on exit. pub(crate) pending_update_action: Option, @@ -362,6 +367,8 @@ pub(crate) struct App { impl App { async fn shutdown_current_thread(&mut self) { if let Some(thread_id) = self.chat_widget.thread_id() { + // Clear any in-flight rollback guard when switching threads. + self.backtrack.pending_rollback = None; self.suppress_shutdown_complete = true; self.chat_widget.submit_op(Op::Shutdown); self.server.remove_thread(&thread_id).await; @@ -502,6 +509,7 @@ impl App { has_emitted_history_lines: false, commit_anim_running: Arc::new(AtomicBool::new(false)), backtrack: BacktrackState::default(), + backtrack_render_pending: false, feedback: feedback.clone(), pending_update_action: None, suppress_shutdown_complete: false, @@ -622,6 +630,10 @@ impl App { self.chat_widget.handle_paste(pasted); } TuiEvent::Draw => { + if self.backtrack_render_pending { + self.backtrack_render_pending = false; + self.render_transcript_once(tui); + } self.chat_widget.maybe_post_pending_notification(tui); if self .chat_widget @@ -870,6 +882,9 @@ impl App { return Ok(AppRunControl::Continue); } self.handle_codex_event_now(event); + if self.backtrack_render_pending { + tui.frame_requester().schedule_frame(); + } } AppEvent::ExternalApprovalRequest { thread_id, event } => { self.handle_external_approval_request(thread_id, event); @@ -1416,6 +1431,7 @@ impl App { let errors = errors_for_cwd(&cwd, response); emit_skill_load_warnings(&self.app_event_tx, &errors); } + self.handle_backtrack_event(&event.msg); self.chat_widget.handle_codex_event(event); } @@ -1740,6 +1756,7 @@ mod tests { enhanced_keys_supported: false, commit_anim_running: Arc::new(AtomicBool::new(false)), backtrack: BacktrackState::default(), + backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), pending_update_action: None, suppress_shutdown_complete: false, @@ -1782,6 +1799,7 @@ mod tests { enhanced_keys_supported: false, commit_anim_running: Arc::new(AtomicBool::new(false)), backtrack: BacktrackState::default(), + backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), pending_update_action: None, suppress_shutdown_complete: false, diff --git a/codex-rs/tui/src/app_backtrack.rs b/codex-rs/tui/src/app_backtrack.rs index 43dacb573..73d238f9a 100644 --- a/codex-rs/tui/src/app_backtrack.rs +++ b/codex-rs/tui/src/app_backtrack.rs @@ -3,6 +3,18 @@ //! This file owns backtrack mode (Esc/Enter navigation in the transcript overlay) and also //! mediates a key rendering boundary for the transcript overlay. //! +//! Overall goal: keep the main chat view and the transcript overlay in sync while allowing +//! users to "rewind" to an earlier user message. We stage a rollback request, wait for core to +//! confirm it, then trim the local transcript to the matching history boundary. This avoids UI +//! state diverging from the agent if a rollback fails or targets a different thread. +//! +//! Backtrack operates as a small state machine: +//! - The first `Esc` in the main view "primes" the feature and captures a base thread id. +//! - A subsequent `Esc` opens the transcript overlay (`Ctrl+T`) and highlights a user message. +//! - `Enter` requests a rollback from core and records a `pending_rollback` guard. +//! - Only after receiving `EventMsg::ThreadRolledBack` do we trim local transcript state and +//! schedule a one-time scrollback refresh. +//! //! The transcript overlay (`Ctrl+T`) renders committed transcript cells plus a render-only live //! tail derived from the current in-flight `ChatWidget.active_cell`. //! @@ -20,6 +32,9 @@ use crate::history_cell::UserHistoryCell; use crate::pager_overlay::Overlay; use crate::tui; use crate::tui::TuiEvent; +use codex_core::protocol::CodexErrorInfo; +use codex_core::protocol::ErrorEvent; +use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_protocol::ThreadId; use color_eyre::eyre::Result; @@ -33,24 +48,53 @@ pub(crate) struct BacktrackState { /// True when Esc has primed backtrack mode in the main view. pub(crate) primed: bool, /// Session id of the base thread to rollback. + /// + /// If the current thread changes, backtrack selections become invalid and must be ignored. pub(crate) base_id: Option, - /// Index in the transcript of the last user message. + /// Index of the currently highlighted user message. + /// + /// This is an index into the filtered "user messages since the last session start" view, + /// not an index into `transcript_cells`. `usize::MAX` indicates "no selection". pub(crate) nth_user_message: usize, /// True when the transcript overlay is showing a backtrack preview. pub(crate) overlay_preview_active: bool, + /// Pending rollback request awaiting confirmation from core. + /// + /// This acts as a guardrail: once we request a rollback, we block additional backtrack + /// submissions until core responds with either a success or failure event. + pub(crate) pending_rollback: Option, } +/// A user-visible backtrack choice that can be confirmed into a rollback request. #[derive(Debug, Clone)] pub(crate) struct BacktrackSelection { + /// The selected user message, counted from the most recent session start. + /// + /// This value is used both to compute the rollback depth and to trim the local transcript + /// after core confirms the rollback. pub(crate) nth_user_message: usize, + /// Composer prefill derived from the selected user message. + /// + /// This is applied immediately on selection confirmation; if the rollback fails, the prefill + /// remains as a convenience so the user can retry or edit. pub(crate) prefill: String, } +/// An in-flight rollback requested from core. +/// +/// We keep enough information to apply the corresponding local trim only if the response targets +/// the same active thread we issued the request for. +#[derive(Debug, Clone)] +pub(crate) struct PendingBacktrackRollback { + pub(crate) selection: BacktrackSelection, + pub(crate) thread_id: Option, +} + impl App { - /// Route overlay events when transcript overlay is active. - /// - If backtrack preview is active: Esc steps selection; Enter confirms. - /// - Otherwise: Esc begins preview; all other events forward to overlay. - /// interactions (Esc to step target, Enter to confirm) and overlay lifecycle. + /// Route overlay events while the transcript overlay is active. + /// + /// If backtrack preview is active, Esc steps the selection and Enter confirms it. + /// Otherwise, Esc begins preview mode and all other events are forwarded to the overlay. pub(crate) async fn handle_backtrack_overlay_event( &mut self, tui: &mut tui::Tui, @@ -112,22 +156,38 @@ impl App { } /// Stage a backtrack and request thread history from the agent. + /// + /// We send the rollback request immediately, but we only mutate the transcript after core + /// confirms success so the UI cannot get ahead of the actual thread state. + /// + /// The composer prefill is applied immediately as a UX convenience; it does not imply that + /// core has accepted the rollback. pub(crate) fn apply_backtrack_rollback(&mut self, selection: BacktrackSelection) { let user_total = user_count(&self.transcript_cells); if user_total == 0 { return; } + if self.backtrack.pending_rollback.is_some() { + self.chat_widget + .add_error_message("Backtrack rollback already in progress.".to_string()); + return; + } + let num_turns = user_total.saturating_sub(selection.nth_user_message); let num_turns = u32::try_from(num_turns).unwrap_or(u32::MAX); if num_turns == 0 { return; } + let prefill = selection.prefill.clone(); + self.backtrack.pending_rollback = Some(PendingBacktrackRollback { + selection, + thread_id: self.chat_widget.thread_id(), + }); self.chat_widget.submit_op(Op::ThreadRollback { num_turns }); - self.trim_transcript_for_backtrack(selection.nth_user_message); - if !selection.prefill.is_empty() { - self.chat_widget.set_composer_text(selection.prefill); + if !prefill.is_empty() { + self.chat_widget.set_composer_text(prefill); } } @@ -290,7 +350,6 @@ impl App { self.close_transcript_overlay(tui); if let Some(selection) = selection { self.apply_backtrack_rollback(selection); - self.render_transcript_once(tui); tui.frame_requester().schedule_frame(); } } @@ -328,10 +387,39 @@ impl App { selection: BacktrackSelection, ) { self.apply_backtrack_rollback(selection); - self.render_transcript_once(tui); tui.frame_requester().schedule_frame(); } + pub(crate) fn handle_backtrack_event(&mut self, event: &EventMsg) { + match event { + EventMsg::ThreadRolledBack(_) => self.finish_pending_backtrack(), + EventMsg::Error(ErrorEvent { + codex_error_info: Some(CodexErrorInfo::ThreadRollbackFailed), + .. + }) => { + // Core rejected the rollback; clear the guard so the user can retry. + self.backtrack.pending_rollback = None; + } + _ => {} + } + } + + /// Finish a pending rollback by applying the local trim and scheduling a scrollback refresh. + /// + /// We ignore events that do not correspond to the currently active thread to avoid applying + /// stale updates after a session switch. + fn finish_pending_backtrack(&mut self) { + let Some(pending) = self.backtrack.pending_rollback.take() else { + return; + }; + if pending.thread_id != self.chat_widget.thread_id() { + // Ignore rollbacks targeting a prior thread. + return; + } + self.trim_transcript_for_backtrack(pending.selection.nth_user_message); + self.backtrack_render_pending = true; + } + fn backtrack_selection(&self, nth_user_message: usize) -> Option { let base_id = self.backtrack.base_id?; if self.chat_widget.thread_id() != Some(base_id) { @@ -350,7 +438,7 @@ impl App { }) } - /// Trim transcript_cells to preserve only content up to the selected user message. + /// Trim `transcript_cells` to preserve only content before the selected user message. fn trim_transcript_for_backtrack(&mut self, nth_user_message: usize) { trim_transcript_cells_to_nth_user(&mut self.transcript_cells, nth_user_message); } diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 1a406264e..6cc764c4a 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -406,6 +406,11 @@ pub(crate) struct App { // Esc-backtracking state grouped pub(crate) backtrack: crate::app_backtrack::BacktrackState, + /// When set, the next draw re-renders the transcript into terminal scrollback once. + /// + /// This is used after a confirmed conversation rollback to ensure scrollback reflects the + /// trimmed transcript cells. + pub(crate) backtrack_render_pending: bool, pub(crate) feedback: codex_feedback::CodexFeedback, /// Set when the user confirms an update; propagated on exit. pub(crate) pending_update_action: Option, @@ -420,6 +425,8 @@ pub(crate) struct App { impl App { async fn shutdown_current_conversation(&mut self) { if let Some(conversation_id) = self.chat_widget.conversation_id() { + // Clear any in-flight rollback guard when switching conversations. + self.backtrack.pending_rollback = None; self.suppress_shutdown_complete = true; self.chat_widget.submit_op(Op::Shutdown); self.server.remove_thread(&conversation_id).await; @@ -587,6 +594,7 @@ impl App { scroll_config, scroll_state: MouseScrollState::default(), backtrack: BacktrackState::default(), + backtrack_render_pending: false, feedback: feedback.clone(), pending_update_action: None, suppress_shutdown_complete: false, @@ -711,6 +719,10 @@ impl App { self.chat_widget.handle_paste(pasted); } TuiEvent::Draw => { + if self.backtrack_render_pending { + self.backtrack_render_pending = false; + self.render_transcript_once(tui); + } self.chat_widget.maybe_post_pending_notification(tui); if self .chat_widget @@ -1634,7 +1646,11 @@ impl App { let errors = errors_for_cwd(&cwd, response); emit_skill_load_warnings(&self.app_event_tx, &errors); } + self.handle_backtrack_event(&event.msg); self.chat_widget.handle_codex_event(event); + if self.backtrack_render_pending { + tui.frame_requester().schedule_frame(); + } } AppEvent::Exit(mode) => match mode { ExitMode::ShutdownFirst => self.chat_widget.submit_op(Op::Shutdown), @@ -2363,6 +2379,7 @@ mod tests { scroll_config: ScrollConfig::default(), scroll_state: MouseScrollState::default(), backtrack: BacktrackState::default(), + backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), pending_update_action: None, suppress_shutdown_complete: false, @@ -2416,6 +2433,7 @@ mod tests { scroll_config: ScrollConfig::default(), scroll_state: MouseScrollState::default(), backtrack: BacktrackState::default(), + backtrack_render_pending: false, feedback: codex_feedback::CodexFeedback::new(), pending_update_action: None, suppress_shutdown_complete: false, diff --git a/codex-rs/tui2/src/app_backtrack.rs b/codex-rs/tui2/src/app_backtrack.rs index 1f5ec3e45..195dc79b3 100644 --- a/codex-rs/tui2/src/app_backtrack.rs +++ b/codex-rs/tui2/src/app_backtrack.rs @@ -3,6 +3,18 @@ //! This file owns backtrack mode (Esc/Enter navigation in the transcript overlay) and also //! mediates a key rendering boundary for the transcript overlay. //! +//! Overall goal: keep the main chat view and the transcript overlay in sync while allowing +//! users to "rewind" to an earlier user message. We stage a rollback request, wait for core to +//! confirm it, then trim the local transcript to the matching history boundary. This avoids UI +//! state diverging from the agent if a rollback fails or targets a different thread. +//! +//! Backtrack operates as a small state machine: +//! - The first `Esc` in the main view "primes" the feature and captures a base conversation id. +//! - A subsequent `Esc` opens the transcript overlay (`Ctrl+T`) and highlights a user message. +//! - `Enter` requests a rollback from core and records a `pending_rollback` guard. +//! - Only after receiving `EventMsg::ThreadRolledBack` do we trim local transcript state and +//! schedule a one-time scrollback refresh. +//! //! The transcript overlay (`Ctrl+T`) renders committed transcript cells plus a render-only live //! tail derived from the current in-flight `ChatWidget.active_cell`. //! @@ -20,6 +32,9 @@ use crate::history_cell::UserHistoryCell; use crate::pager_overlay::Overlay; use crate::tui; use crate::tui::TuiEvent; +use codex_core::protocol::CodexErrorInfo; +use codex_core::protocol::ErrorEvent; +use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_protocol::ThreadId; use color_eyre::eyre::Result; @@ -32,25 +47,55 @@ use crossterm::event::KeyEventKind; pub(crate) struct BacktrackState { /// True when Esc has primed backtrack mode in the main view. pub(crate) primed: bool, - /// Session id of the base thread to rollback. + /// Session id of the base conversation to rollback. + /// + /// If the current conversation changes, backtrack selections become invalid and must be + /// ignored. pub(crate) base_id: Option, - /// Index in the transcript of the last user message. + /// Index of the currently highlighted user message. + /// + /// This is an index into the filtered "user messages since the last session start" view, + /// not an index into `transcript_cells`. `usize::MAX` indicates "no selection". pub(crate) nth_user_message: usize, /// True when the transcript overlay is showing a backtrack preview. pub(crate) overlay_preview_active: bool, + /// Pending rollback request awaiting confirmation from core. + /// + /// This acts as a guardrail: once we request a rollback, we block additional backtrack + /// submissions until core responds with either a success or failure event. + pub(crate) pending_rollback: Option, } +/// A user-visible backtrack choice that can be confirmed into a rollback request. #[derive(Debug, Clone)] pub(crate) struct BacktrackSelection { + /// The selected user message, counted from the most recent session start. + /// + /// This value is used both to compute the rollback depth and to trim the local transcript + /// after core confirms the rollback. pub(crate) nth_user_message: usize, + /// Composer prefill derived from the selected user message. + /// + /// This is applied immediately on selection confirmation; if the rollback fails, the prefill + /// remains as a convenience so the user can retry or edit. pub(crate) prefill: String, } +/// An in-flight rollback requested from core. +/// +/// We keep enough information to apply the corresponding local trim only if the response targets +/// the same active conversation we issued the request for. +#[derive(Debug, Clone)] +pub(crate) struct PendingBacktrackRollback { + pub(crate) selection: BacktrackSelection, + pub(crate) thread_id: Option, +} + impl App { - /// Route overlay events when transcript overlay is active. - /// - If backtrack preview is active: Esc steps selection; Enter confirms. - /// - Otherwise: Esc begins preview; all other events forward to overlay. - /// interactions (Esc to step target, Enter to confirm) and overlay lifecycle. + /// Route overlay events while the transcript overlay is active. + /// + /// If backtrack preview is active, Esc steps the selection and Enter confirms it. + /// Otherwise, Esc begins preview mode and all other events are forwarded to the overlay. pub(crate) async fn handle_backtrack_overlay_event( &mut self, tui: &mut tui::Tui, @@ -111,22 +156,39 @@ impl App { } } + /// Stage a backtrack and request thread history from the agent. + /// + /// We send the rollback request immediately, but we only mutate the transcript after core + /// confirms success so the UI cannot get ahead of the actual thread state. + /// + /// The composer prefill is applied immediately as a UX convenience; it does not imply that + /// core has accepted the rollback. pub(crate) fn apply_backtrack_rollback(&mut self, selection: BacktrackSelection) { let user_total = user_count(&self.transcript_cells); if user_total == 0 { return; } + if self.backtrack.pending_rollback.is_some() { + self.chat_widget + .add_error_message("Backtrack rollback already in progress.".to_string()); + return; + } + let num_turns = user_total.saturating_sub(selection.nth_user_message); let num_turns = u32::try_from(num_turns).unwrap_or(u32::MAX); if num_turns == 0 { return; } + let prefill = selection.prefill.clone(); + self.backtrack.pending_rollback = Some(PendingBacktrackRollback { + selection, + thread_id: self.chat_widget.conversation_id(), + }); self.chat_widget.submit_op(Op::ThreadRollback { num_turns }); - self.trim_transcript_for_backtrack(selection.nth_user_message); - if !selection.prefill.is_empty() { - self.chat_widget.set_composer_text(selection.prefill); + if !prefill.is_empty() { + self.chat_widget.set_composer_text(prefill); } } @@ -311,7 +373,6 @@ impl App { self.close_transcript_overlay(tui); if let Some(selection) = selection { self.apply_backtrack_rollback(selection); - self.render_transcript_once(tui); tui.frame_requester().schedule_frame(); } } @@ -349,9 +410,38 @@ impl App { selection: BacktrackSelection, ) { self.apply_backtrack_rollback(selection); - self.render_transcript_once(tui); tui.frame_requester().schedule_frame(); } + + pub(crate) fn handle_backtrack_event(&mut self, event: &EventMsg) { + match event { + EventMsg::ThreadRolledBack(_) => self.finish_pending_backtrack(), + EventMsg::Error(ErrorEvent { + codex_error_info: Some(CodexErrorInfo::ThreadRollbackFailed), + .. + }) => { + // Core rejected the rollback; clear the guard so the user can retry. + self.backtrack.pending_rollback = None; + } + _ => {} + } + } + + /// Finish a pending rollback by applying the local trim and scheduling a scrollback refresh. + /// + /// We ignore events that do not correspond to the currently active conversation to avoid + /// applying stale updates after a session switch. + fn finish_pending_backtrack(&mut self) { + let Some(pending) = self.backtrack.pending_rollback.take() else { + return; + }; + if pending.thread_id != self.chat_widget.conversation_id() { + // Ignore rollbacks targeting a prior thread. + return; + } + self.trim_transcript_for_backtrack(pending.selection.nth_user_message); + self.backtrack_render_pending = true; + } fn backtrack_selection(&self, nth_user_message: usize) -> Option { let base_id = self.backtrack.base_id?; if self.chat_widget.conversation_id() != Some(base_id) { @@ -370,7 +460,7 @@ impl App { }) } - /// Trim transcript_cells to preserve only content up to the selected user message. + /// Trim `transcript_cells` to preserve only content before the selected user message. fn trim_transcript_for_backtrack(&mut self, nth_user_message: usize) { trim_transcript_cells_to_nth_user(&mut self.transcript_cells, nth_user_message); }