fix: guard manual rewind application
This commit is contained in:
parent
f8881f7289
commit
cbb59a47d0
|
|
@ -797,7 +797,7 @@ async fn controller_loop<C, St>(
|
||||||
target,
|
target,
|
||||||
expected_head_entries,
|
expected_head_entries,
|
||||||
} => match shared_state.get_status() {
|
} => match shared_state.get_status() {
|
||||||
PodStatus::Idle | PodStatus::Paused => {
|
PodStatus::Idle => {
|
||||||
if apply_rewind(&mut pod, &event_tx, target, expected_head_entries) {
|
if apply_rewind(&mut pod, &event_tx, target, expected_head_entries) {
|
||||||
shared_state.set_status(PodStatus::Idle);
|
shared_state.set_status(PodStatus::Idle);
|
||||||
let _ = event_tx.send(Event::Status {
|
let _ = event_tx.send(Event::Status {
|
||||||
|
|
@ -805,6 +805,13 @@ async fn controller_loop<C, St>(
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
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 => {
|
PodStatus::Running => {
|
||||||
let _ = event_tx.send(Event::Error {
|
let _ = event_tx.send(Event::Error {
|
||||||
code: ErrorCode::AlreadyRunning,
|
code: ErrorCode::AlreadyRunning,
|
||||||
|
|
|
||||||
|
|
@ -51,14 +51,33 @@ impl CompletionState {
|
||||||
pub const MAX_VISIBLE: usize = 6;
|
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)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct RewindPickerState {
|
pub struct RewindPickerState {
|
||||||
pub head_entries: usize,
|
pub head_entries: usize,
|
||||||
pub targets: Vec<RewindTarget>,
|
pub targets: Vec<RewindTarget>,
|
||||||
pub selected: usize,
|
pub selected: usize,
|
||||||
|
pub scroll: RewindPickerScroll,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl RewindPickerState {
|
impl RewindPickerState {
|
||||||
|
pub fn new(head_entries: usize, targets: Vec<RewindTarget>) -> 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> {
|
pub fn selected_target(&self) -> Option<&RewindTarget> {
|
||||||
self.targets.get(self.selected)
|
self.targets.get(self.selected)
|
||||||
}
|
}
|
||||||
|
|
@ -947,13 +966,7 @@ impl App {
|
||||||
} => {
|
} => {
|
||||||
if self.rewind_request_pending {
|
if self.rewind_request_pending {
|
||||||
self.rewind_request_pending = false;
|
self.rewind_request_pending = false;
|
||||||
let selected = targets.iter().position(|t| t.eligible).unwrap_or(0);
|
self.rewind_picker = Some(RewindPickerState::new(head_entries, targets));
|
||||||
self.rewind_picker = Some(RewindPickerState {
|
|
||||||
head_entries,
|
|
||||||
targets,
|
|
||||||
selected,
|
|
||||||
});
|
|
||||||
self.scroll = Scroll::default();
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Event::RewindApplied {
|
Event::RewindApplied {
|
||||||
|
|
@ -964,14 +977,26 @@ impl App {
|
||||||
if let Some(greeting) = self.greeting.clone() {
|
if let Some(greeting) = self.greeting.clone() {
|
||||||
self.restore_snapshot(&entries, greeting);
|
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.completion = None;
|
||||||
self.close_rewind_picker();
|
self.close_rewind_picker();
|
||||||
self.reset_run_state(self.pod_status);
|
self.reset_run_state(self.pod_status);
|
||||||
let mut message = format!(
|
let mut message = if restored_composer {
|
||||||
"Rewound session: discarded {} log entries; restored selected input to composer.",
|
format!(
|
||||||
summary.discarded_entries
|
"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 {
|
if summary.tool_side_effect_warning {
|
||||||
message.push_str(
|
message.push_str(
|
||||||
" History suffix was discarded; tool side effects were not undone.",
|
" History suffix was discarded; tool side effects were not undone.",
|
||||||
|
|
@ -1324,6 +1349,18 @@ impl App {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn submit_rewind_picker(&mut self) -> Option<Method> {
|
pub fn submit_rewind_picker(&mut self) -> Option<Method> {
|
||||||
|
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 {
|
let Some(picker) = self.rewind_picker.as_ref() else {
|
||||||
return None;
|
return None;
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -1101,7 +1101,7 @@ fn handle_pause_or_quit(app: &mut App) -> Option<Method> {
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use protocol::{Event, Segment};
|
use protocol::{Event, RewindTarget, RewindTargetId, Segment};
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_pod_name_mode() {
|
fn parse_pod_name_mode() {
|
||||||
|
|
@ -1650,6 +1650,93 @@ mod tests {
|
||||||
assert!(has_alert(&app, "tool side effects"));
|
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 {
|
fn test_greeting() -> protocol::Greeting {
|
||||||
protocol::Greeting {
|
protocol::Greeting {
|
||||||
pod_name: "agent".into(),
|
pod_name: "agent".into(),
|
||||||
|
|
|
||||||
|
|
@ -419,8 +419,8 @@ fn draw_history(frame: &mut Frame, app: &mut App, area: Rect) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Some(picker) = app.rewind_picker.clone() {
|
if let Some(picker) = app.rewind_picker.as_mut() {
|
||||||
draw_rewind_picker(frame, app, history_area, inner, outer_block, &picker);
|
draw_rewind_picker(frame, history_area, inner, outer_block, picker);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -454,11 +454,10 @@ fn draw_history(frame: &mut Frame, app: &mut App, area: Rect) {
|
||||||
|
|
||||||
fn draw_rewind_picker(
|
fn draw_rewind_picker(
|
||||||
frame: &mut Frame,
|
frame: &mut Frame,
|
||||||
app: &mut App,
|
|
||||||
history_area: Rect,
|
history_area: Rect,
|
||||||
inner: Rect,
|
inner: Rect,
|
||||||
outer_block: UiBlock<'_>,
|
outer_block: UiBlock<'_>,
|
||||||
picker: &crate::app::RewindPickerState,
|
picker: &mut crate::app::RewindPickerState,
|
||||||
) {
|
) {
|
||||||
let mut logical: Vec<Line<'static>> = Vec::new();
|
let mut logical: Vec<Line<'static>> = Vec::new();
|
||||||
logical.push(Line::from(vec![
|
logical.push(Line::from(vec![
|
||||||
|
|
@ -534,14 +533,13 @@ fn draw_rewind_picker(
|
||||||
}
|
}
|
||||||
|
|
||||||
let tail_top = lines.len().saturating_sub(inner.height as usize);
|
let tail_top = lines.len().saturating_sub(inner.height as usize);
|
||||||
app.scroll.area_height = inner.height;
|
picker.scroll.area_height = inner.height;
|
||||||
app.scroll.total_lines = lines.len();
|
picker.scroll.total_lines = lines.len();
|
||||||
app.scroll.tail_top_offset = tail_top;
|
picker.scroll.tail_top_offset = tail_top;
|
||||||
app.scroll.turn_starts.clear();
|
picker.scroll.top_offset = picker.scroll.top_offset.min(tail_top);
|
||||||
app.scroll.top_offset = app.scroll.top_offset.min(tail_top);
|
|
||||||
|
|
||||||
let end = (app.scroll.top_offset + inner.height as usize).min(lines.len());
|
let end = (picker.scroll.top_offset + inner.height as usize).min(lines.len());
|
||||||
let visible = lines[app.scroll.top_offset..end].to_vec();
|
let visible = lines[picker.scroll.top_offset..end].to_vec();
|
||||||
Paragraph::new(visible)
|
Paragraph::new(visible)
|
||||||
.block(outer_block)
|
.block(outer_block)
|
||||||
.render(history_area, frame.buffer_mut());
|
.render(history_area, frame.buffer_mut());
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user