From cbb59a47d04877a81f482f7c698780c4b1a6d167 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 29 May 2026 12:05:33 +0900 Subject: [PATCH] fix: guard manual rewind application --- crates/pod/src/controller.rs | 9 +++- crates/tui/src/app.rs | 61 +++++++++++++++++++----- crates/tui/src/main.rs | 89 +++++++++++++++++++++++++++++++++++- crates/tui/src/ui.rs | 20 ++++---- 4 files changed, 154 insertions(+), 25 deletions(-) diff --git a/crates/pod/src/controller.rs b/crates/pod/src/controller.rs index 1b2ee670..8e4f5a9b 100644 --- a/crates/pod/src/controller.rs +++ b/crates/pod/src/controller.rs @@ -797,7 +797,7 @@ async fn controller_loop( target, expected_head_entries, } => match shared_state.get_status() { - PodStatus::Idle | PodStatus::Paused => { + PodStatus::Idle => { if apply_rewind(&mut pod, &event_tx, target, expected_head_entries) { shared_state.set_status(PodStatus::Idle); let _ = event_tx.send(Event::Status { @@ -805,6 +805,13 @@ async fn controller_loop( }); } } + PodStatus::Paused => { + let _ = event_tx.send(Event::Error { + code: ErrorCode::InvalidRequest, + message: "Cannot apply rewind while the Pod is paused; resume or wait for idle first" + .into(), + }); + } PodStatus::Running => { let _ = event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index e4c791e1..fb473154 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -51,14 +51,33 @@ impl CompletionState { pub const MAX_VISIBLE: usize = 6; } +#[derive(Debug, Clone, Default)] +pub struct RewindPickerScroll { + pub top_offset: usize, + pub total_lines: usize, + pub area_height: u16, + pub tail_top_offset: usize, +} + #[derive(Debug, Clone)] pub struct RewindPickerState { pub head_entries: usize, pub targets: Vec, pub selected: usize, + pub scroll: RewindPickerScroll, } impl RewindPickerState { + pub fn new(head_entries: usize, targets: Vec) -> Self { + let selected = targets.iter().position(|t| t.eligible).unwrap_or(0); + Self { + head_entries, + targets, + selected, + scroll: RewindPickerScroll::default(), + } + } + pub fn selected_target(&self) -> Option<&RewindTarget> { self.targets.get(self.selected) } @@ -947,13 +966,7 @@ impl App { } => { if self.rewind_request_pending { self.rewind_request_pending = false; - let selected = targets.iter().position(|t| t.eligible).unwrap_or(0); - self.rewind_picker = Some(RewindPickerState { - head_entries, - targets, - selected, - }); - self.scroll = Scroll::default(); + self.rewind_picker = Some(RewindPickerState::new(head_entries, targets)); } } Event::RewindApplied { @@ -964,14 +977,26 @@ impl App { if let Some(greeting) = self.greeting.clone() { self.restore_snapshot(&entries, greeting); } - self.input.replace_with_segments(&input); + let restored_composer = if self.input.is_empty() { + self.input.replace_with_segments(&input); + true + } else { + false + }; self.completion = None; self.close_rewind_picker(); self.reset_run_state(self.pod_status); - let mut message = format!( - "Rewound session: discarded {} log entries; restored selected input to composer.", - summary.discarded_entries - ); + let mut message = if restored_composer { + format!( + "Rewound session: discarded {} log entries; restored selected input to composer.", + summary.discarded_entries + ) + } else { + format!( + "Rewound session: discarded {} log entries. Rewind applied; composer not overwritten because it was not empty.", + summary.discarded_entries + ) + }; if summary.tool_side_effect_warning { message.push_str( " History suffix was discarded; tool side effects were not undone.", @@ -1324,6 +1349,18 @@ impl App { } pub fn submit_rewind_picker(&mut self) -> Option { + if self.paused { + self.push_command_diagnostic( + "cannot apply rewind while the Pod is paused; resume or wait for idle first", + ); + return None; + } + if !self.input.is_empty() { + self.push_command_diagnostic( + "cannot apply rewind while composer is not empty; clear it before restoring rewind input", + ); + return None; + } let Some(picker) = self.rewind_picker.as_ref() else { return None; }; diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index db690998..8346206b 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -1101,7 +1101,7 @@ fn handle_pause_or_quit(app: &mut App) -> Option { #[cfg(test)] mod tests { use super::*; - use protocol::{Event, Segment}; + use protocol::{Event, RewindTarget, RewindTargetId, Segment}; #[test] fn parse_pod_name_mode() { @@ -1650,6 +1650,93 @@ mod tests { assert!(has_alert(&app, "tool side effects")); } + #[test] + fn rewind_applied_keeps_non_empty_composer() { + let mut app = App::new("agent".to_string()); + app.handle_pod_event(Event::Snapshot { + greeting: test_greeting(), + entries: vec![], + status: PodStatus::Idle, + }); + type_keys(&mut app, "draft"); + + app.handle_pod_event(Event::RewindApplied { + entries: vec![], + input: vec![Segment::Text { + content: "retry this".into(), + }], + summary: protocol::RewindSummary { + truncated_to_entries: 0, + discarded_entries: 2, + tool_side_effect_warning: false, + }, + }); + + assert_eq!(input_text(&app), "draft"); + assert!(has_alert( + &app, + "composer not overwritten because it was not empty" + )); + } + + #[test] + fn rewind_apply_rejects_non_empty_composer_and_paused_status() { + let mut app = App::new("agent".to_string()); + app.rewind_picker = Some(crate::app::RewindPickerState::new(1, vec![rewind_target()])); + type_keys(&mut app, "draft"); + assert!(app.submit_rewind_picker().is_none()); + assert!(has_alert(&app, "composer is not empty")); + + let mut app = App::new("agent".to_string()); + app.rewind_picker = Some(crate::app::RewindPickerState::new(1, vec![rewind_target()])); + app.set_pod_status(PodStatus::Paused); + assert!(app.submit_rewind_picker().is_none()); + assert!(has_alert( + &app, + "cannot apply rewind while the Pod is paused" + )); + } + + #[test] + fn rewind_picker_draw_does_not_overwrite_history_scroll_state() { + let mut app = App::new("agent".to_string()); + app.scroll.top_offset = 3; + app.scroll.turn_starts = vec![0, 5, 9]; + app.scroll.total_lines = 42; + app.rewind_picker = Some(crate::app::RewindPickerState::new(1, vec![rewind_target()])); + let original_top_offset = app.scroll.top_offset; + let original_turn_starts = app.scroll.turn_starts.clone(); + let original_total_lines = app.scroll.total_lines; + + let backend = ratatui::backend::TestBackend::new(80, 24); + let mut terminal = ratatui::Terminal::new(backend).unwrap(); + terminal + .draw(|frame| crate::ui::draw(frame, &mut app)) + .unwrap(); + app.close_rewind_picker(); + + assert_eq!(app.scroll.top_offset, original_top_offset); + assert_eq!(app.scroll.turn_starts, original_turn_starts); + assert_eq!(app.scroll.total_lines, original_total_lines); + } + + fn rewind_target() -> RewindTarget { + RewindTarget { + id: RewindTargetId { + segment_id: uuid::Uuid::nil(), + user_input_entry_index: 0, + }, + expected_head_entries: 1, + truncate_entries: 0, + turn_index: 1, + timestamp_ms: Some(1), + preview: "retry this".into(), + eligible: true, + disabled_reason: None, + warning: None, + } + } + fn test_greeting() -> protocol::Greeting { protocol::Greeting { pod_name: "agent".into(), diff --git a/crates/tui/src/ui.rs b/crates/tui/src/ui.rs index a2f9c33e..1400f5fd 100644 --- a/crates/tui/src/ui.rs +++ b/crates/tui/src/ui.rs @@ -419,8 +419,8 @@ fn draw_history(frame: &mut Frame, app: &mut App, area: Rect) { return; } - if let Some(picker) = app.rewind_picker.clone() { - draw_rewind_picker(frame, app, history_area, inner, outer_block, &picker); + if let Some(picker) = app.rewind_picker.as_mut() { + draw_rewind_picker(frame, history_area, inner, outer_block, picker); return; } @@ -454,11 +454,10 @@ fn draw_history(frame: &mut Frame, app: &mut App, area: Rect) { fn draw_rewind_picker( frame: &mut Frame, - app: &mut App, history_area: Rect, inner: Rect, outer_block: UiBlock<'_>, - picker: &crate::app::RewindPickerState, + picker: &mut crate::app::RewindPickerState, ) { let mut logical: Vec> = Vec::new(); logical.push(Line::from(vec![ @@ -534,14 +533,13 @@ fn draw_rewind_picker( } let tail_top = lines.len().saturating_sub(inner.height as usize); - app.scroll.area_height = inner.height; - app.scroll.total_lines = lines.len(); - app.scroll.tail_top_offset = tail_top; - app.scroll.turn_starts.clear(); - app.scroll.top_offset = app.scroll.top_offset.min(tail_top); + picker.scroll.area_height = inner.height; + picker.scroll.total_lines = lines.len(); + picker.scroll.tail_top_offset = tail_top; + picker.scroll.top_offset = picker.scroll.top_offset.min(tail_top); - let end = (app.scroll.top_offset + inner.height as usize).min(lines.len()); - let visible = lines[app.scroll.top_offset..end].to_vec(); + let end = (picker.scroll.top_offset + inner.height as usize).min(lines.len()); + let visible = lines[picker.scroll.top_offset..end].to_vec(); Paragraph::new(visible) .block(outer_block) .render(history_area, frame.buffer_mut());