diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index c442f48793..43ebfd2454 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -867,6 +867,8 @@ impl App { app_server: &mut AppServerSession, event: TuiEvent, ) -> Result { + self.transcript_view + .sync_patch_approval(self.chat_widget.current_patch_approval()); tui.link_hover.observe(&event); self.refresh_link_hover(tui)?; self.invalidate_right_click_paste(&event); diff --git a/codex-rs/tui/src/app/owned_transcript.rs b/codex-rs/tui/src/app/owned_transcript.rs index 76432f6ec9..fa4c42782f 100644 --- a/codex-rs/tui/src/app/owned_transcript.rs +++ b/codex-rs/tui/src/app/owned_transcript.rs @@ -395,7 +395,7 @@ impl App { Ok(bottom_area) } - /// Keep modal input ownership while allowing selection and copying in the visible transcript. + /// Keep modal input ownership while allowing selection, copying, and disclosure in the visible transcript. pub(super) fn handle_owned_transcript_event( &mut self, tui: &mut tui::Tui, @@ -670,9 +670,13 @@ impl App { TuiEvent::Key(key) => self .transcript_view .handle_key(*key, &self.transcript_cells), - TuiEvent::Mouse(mouse) if modal_transcript_mouse => self - .transcript_view - .handle_selection_mouse(*mouse, &self.transcript_cells), + TuiEvent::Mouse(mouse) if modal_transcript_mouse => { + self.transcript_view.handle_selection_mouse( + *mouse, + &self.transcript_cells, + self.chat_widget.current_patch_approval(), + ) + } TuiEvent::Mouse(mouse) => self .transcript_view .handle_mouse(*mouse, &self.transcript_cells), diff --git a/codex-rs/tui/src/app/tests/patch_approval_tests.rs b/codex-rs/tui/src/app/tests/patch_approval_tests.rs index c5dee23417..283be96d30 100644 --- a/codex-rs/tui/src/app/tests/patch_approval_tests.rs +++ b/codex-rs/tui/src/app/tests/patch_approval_tests.rs @@ -2,8 +2,14 @@ use super::*; use crate::app::app_server_requests::AppServerRequestResolution; +use crate::app::owned_transcript::tests::buffer_text; +use crate::chatwidget::tests::helpers::normalize_snapshot_paths; +use crate::custom_terminal::test_support::last_rendered_buffer; use codex_app_server_protocol::FileChangeApprovalDecision; use codex_app_server_protocol::PatchApplyStatus; +use crossterm::event::MouseButton; +use crossterm::event::MouseEvent; +use crossterm::event::MouseEventKind; use pretty_assertions::assert_eq; use ratatui::layout::Rect; use tokio::sync::mpsc::UnboundedReceiver; @@ -16,7 +22,7 @@ fn changes() -> Vec { FileUpdateChange { path: "a-added.txt".to_string(), kind: PatchChangeKind::Add, - diff: "alpha\nbeta\n".to_string(), + diff: "alpha\nbeta\ngamma\ndelta\n".to_string(), }, FileUpdateChange { path: "m-updated.txt".to_string(), @@ -36,7 +42,7 @@ fn expected_changes() -> HashMap { ( PathBuf::from("a-added.txt"), FileChange::Add { - content: "alpha\nbeta\n".to_string(), + content: "alpha\nbeta\ngamma\ndelta\n".to_string(), }, ), ( @@ -170,6 +176,42 @@ async fn active_patch_approval_pager_preserves_changes_and_accepts_once() -> Res enqueue_pending_patch(&mut app, thread_id).await?; drain_pending_patch(&mut app, &mut tui).await?; + tui.set_owned_screen(/*owned*/ true)?; + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + app.insert_history_cell(&mut tui, cell); + } + } + let size = tui.terminal.size()?; + app.render_owned_transcript(&mut tui, size)?; + let collapsed = buffer_text(last_rendered_buffer(&tui.terminal)); + assert!(!collapsed.contains("+delta")); + let (column, row) = collapsed + .lines() + .enumerate() + .find_map(|(row, line)| line.find("+ Show details").map(|column| (column, row))) + .expect("visible patch disclosure"); + for kind in [ + MouseEventKind::Down(MouseButton::Left), + MouseEventKind::Up(MouseButton::Left), + ] { + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Mouse(MouseEvent { + kind, + column: column as u16, + row: row as u16, + modifiers: KeyModifiers::NONE, + }), + ) + .await?; + } + app.render_owned_transcript(&mut tui, size)?; + let expanded = buffer_text(last_rendered_buffer(&tui.terminal)); + insta::with_settings!({snapshot_path => "../../snapshots"}, { + insta::assert_snapshot!("patch_approval_expanded_inline", normalize_snapshot_paths(expanded)); + }); let request = open_patch(&mut app, &mut rx); assert_eq!(request.changes, expected_changes()); assert_eq!(request.id, ITEM_ID); @@ -208,15 +250,34 @@ async fn active_patch_approval_pager_preserves_changes_and_accepts_once() -> Res app.pending_app_server_requests .contains_server_request(&self::request(thread_id)) ); - assert_eq!(open_patch(&mut app, &mut rx).changes, expected_changes()); + let mut next_patch = open_patch(&mut app, &mut rx); + assert_eq!(next_patch.changes, expected_changes()); + next_patch.id = "next-patch".into(); app.chat_widget - .handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); + .push_approval_request(ApprovalRequest::ApplyPatch(next_patch)); + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)), + ) + .await?; assert_decision( &mut app, &mut rx, thread_id, FileChangeApprovalDecision::Accept, ); + assert_eq!(open_patch(&mut app, &mut rx).id, "next-patch"); + let mut output = vec!["more output".into(); usize::from(size.height)]; + output.push("after approval".into()); + app.insert_history_cell( + &mut tui, + Box::new(crate::history_cell::PlainHistoryCell::new(output)), + ); + app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Draw) + .await?; + assert!(buffer_text(last_rendered_buffer(&tui.terminal)).contains("after approval")); + tui.set_owned_screen(/*owned*/ false)?; Ok(()) } diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 0fee868894..46fd8ef491 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -617,6 +617,13 @@ impl BottomPaneView for ApprovalOverlay { self.done } + fn current_patch_approval(&self) -> Option<&ApplyPatchApprovalRequest> { + let ApprovalRequest::ApplyPatch(patch) = self.current_request.as_ref()? else { + return None; + }; + Some(patch) + } + fn try_consume_approval_request( &mut self, request: ApprovalRequest, diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index 57a66255c5..de5e68d71d 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -182,6 +182,11 @@ pub(crate) trait BottomPaneView: Renderable { false } + /// The patch whose inline review should end when this prompt is answered. + fn current_patch_approval(&self) -> Option<&super::ApplyPatchApprovalRequest> { + None + } + /// Try to handle approval request; return the original value if not /// consumed. fn try_consume_approval_request( diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index d38dd154ac..aef7c535b1 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -752,6 +752,13 @@ impl BottomPane { self.view_stack.last().map(std::convert::AsRef::as_ref) } + pub(crate) fn current_patch_approval(&self) -> Option<&ApplyPatchApprovalRequest> { + self.view_stack + .iter() + .rev() + .find_map(|view| view.current_patch_approval()) + } + fn push_view(&mut self, view: Box) { self.key_chord_reset_requested = true; self.view_stack.push(view); diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index 94f4fe228d..617253a793 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -15,6 +15,11 @@ pub(crate) enum KeyEventAction { } impl ChatWidget { + pub(crate) fn current_patch_approval(&self) -> Option<(ThreadId, &str)> { + let patch = self.bottom_pane.current_patch_approval()?; + Some((patch.thread_id, &patch.id)) + } + pub(crate) fn clear_composer_selection(&mut self) { self.bottom_pane.clear_composer_selection(); } diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_expanded_inline.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_expanded_inline.snap new file mode 100644 index 0000000000..596fac2327 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_expanded_inline.snap @@ -0,0 +1,28 @@ +--- +source: tui/src/app/tests/patch_approval_tests.rs +expression: normalize_snapshot_paths(expanded) +--- +• Edited 3 files (+5 -2) + └ a-added.txt (+4 -0) + 1 +alpha + 2 +beta + 3 +gamma + 4 +delta + + └ m-updated.txt (+1 -1) + 1 -old + + + Would you like to make the following edits? + + Description: Apply proposed file edits + Destination: /tmp/project/a-added.txt + Destination: /tmp/project/m-updated.txt + Destination: /tmp/project/z-deleted.txt + + +› 1. Yes, proceed (y) + 2. Yes, and don't ask again for these files (a) + 3. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_pager_top.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_pager_top.snap index 5f07ffbae4..e32daebd60 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_pager_top.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__patch_approval_tests__patch_approval_pager_top.snap @@ -1,18 +1,17 @@ --- source: tui/src/app/tests/patch_approval_tests.rs -assertion_line: 184 expression: "format!(\"{first:?}\")" --- Buffer { area: Rect { x: 0, y: 0, width: 48, height: 11 }, content: [ "/ P A T C H / / / / / / / / / / / / / / / / / / ", - "a-added.txt (+2 -0) ", + "a-added.txt (+4 -0) ", " ", " 1 +alpha ", " 2 +beta ", - " ", - "m-updated.txt (+1 -1) ", + " 3 +gamma ", + " 4 +delta ", "─────────────────────────────────────────── 0% ─", " ↑/↓ to scroll · pgup/pgdn to page · home/end to", " q close ", @@ -31,10 +30,12 @@ Buffer { x: 2, y: 4, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, x: 4, y: 4, fg: Green, bg: Reset, underline: Reset, modifier: NONE, x: 9, y: 4, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 15, y: 6, fg: Green, bg: Reset, underline: Reset, modifier: NONE, - x: 17, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 18, y: 6, fg: Red, bg: Reset, underline: Reset, modifier: NONE, - x: 20, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 4, y: 5, fg: Green, bg: Reset, underline: Reset, modifier: NONE, + x: 10, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 4, y: 6, fg: Green, bg: Reset, underline: Reset, modifier: NONE, + x: 10, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, x: 0, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, x: 1, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, x: 4, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, diff --git a/codex-rs/tui/src/transcript_view.rs b/codex-rs/tui/src/transcript_view.rs index 0f895886a6..9911b8c439 100644 --- a/codex-rs/tui/src/transcript_view.rs +++ b/codex-rs/tui/src/transcript_view.rs @@ -458,6 +458,7 @@ impl TranscriptView { } pub(crate) fn jump_to_latest(&mut self) { + self.disclosure.resume_after_approval = None; self.cancel_search(); self.cancel_beginning(); self.position = Position::Latest; @@ -471,6 +472,7 @@ impl TranscriptView { pub(crate) fn scroll(&mut self, cells: &[Arc], rows: isize) { if rows != 0 { + self.disclosure.resume_after_approval = None; self.last_click = None; self.cancel_beginning(); } diff --git a/codex-rs/tui/src/transcript_view/disclosure.rs b/codex-rs/tui/src/transcript_view/disclosure.rs index d22138770d..5429eaa7c2 100644 --- a/codex-rs/tui/src/transcript_view/disclosure.rs +++ b/codex-rs/tui/src/transcript_view/disclosure.rs @@ -8,6 +8,7 @@ use crate::bottom_pane::TranscriptFooter; use crate::key_hint::KeyBindingListExt; use crate::keymap::ListAction; use crate::keymap::RuntimeKeymap; +use codex_protocol::ThreadId; use crossterm::event::KeyEvent; use ratatui::style::Modifier; use ratatui::style::Style; @@ -17,6 +18,7 @@ use super::*; pub(super) struct Disclosure { pub(super) expanded: HashSet, pub(super) focused: Option>, + pub(super) resume_after_approval: Option<(ThreadId, String)>, pub(super) live_ids: Vec, pub(super) keymap: RuntimeKeymap, } @@ -26,6 +28,7 @@ impl Default for Disclosure { Self { expanded: HashSet::new(), focused: None, + resume_after_approval: None, live_ids: Vec::new(), keymap: RuntimeKeymap::defaults(), } @@ -39,6 +42,14 @@ impl Disclosure { } impl TranscriptView { + pub(crate) fn sync_patch_approval(&mut self, current: Option<(ThreadId, &str)>) { + if let Some((thread_id, id)) = &self.disclosure.resume_after_approval + && current != Some((*thread_id, id.as_str())) + { + self.jump_to_latest(); + } + } + pub(crate) fn is_activity_focused(&self) -> bool { self.disclosure.focused.is_some() && !self.detailed && self.mode == HistoryRenderMode::Rich } diff --git a/codex-rs/tui/src/transcript_view/footer_tests.rs b/codex-rs/tui/src/transcript_view/footer_tests.rs index 309403b832..60167612cd 100644 --- a/codex-rs/tui/src/transcript_view/footer_tests.rs +++ b/codex-rs/tui/src/transcript_view/footer_tests.rs @@ -435,6 +435,50 @@ impl HistoryCell for ActivityCell { } } +#[test] +fn patch_disclosure_preserves_manual_scrolling() { + let cells: Vec> = vec![ + Arc::new(PlainHistoryCell::new(vec!["older output".into(); 20])), + Arc::new(ActivityCell::default()), + Arc::new(PlainHistoryCell::new(vec!["tail".into()])), + ]; + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 32, /*height*/ 10, + ); + for scroll_before_click in [false, true] { + let mut view = TranscriptView::default(); + let mut buf = Buffer::empty(area); + view.render(area, &mut buf, &cells); + if scroll_before_click { + view.scroll(&cells, /*rows*/ -1); + view.render(area, &mut buf, &cells); + } + let (column, row) = crate::transcript_view::tests::text(&buf) + .lines() + .enumerate() + .find_map(|(row, line)| line.find("+ Show details").map(|column| (column, row))) + .expect("activity disclosure"); + view.handle_selection_mouse( + MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: column as u16, + row: row as u16, + modifiers: KeyModifiers::NONE, + }, + &cells, + Some((codex_protocol::ThreadId::new(), "patch")), + ); + assert!(view.is_activity_focused()); + if !scroll_before_click { + view.scroll(&cells, /*rows*/ -1); + } + let position = view.position; + view.sync_patch_approval(/*current*/ None); + assert_eq!(view.position, position); + assert!(!view.is_following()); + } +} + #[test] fn activity_focus_keeps_controls_without_passive_hints() { let mut cells: Vec> = Vec::new(); diff --git a/codex-rs/tui/src/transcript_view/input.rs b/codex-rs/tui/src/transcript_view/input.rs index 1ce29ba3af..59183c42ac 100644 --- a/codex-rs/tui/src/transcript_view/input.rs +++ b/codex-rs/tui/src/transcript_view/input.rs @@ -7,6 +7,7 @@ //! Wheel input accumulates fractional rows and discards the remainder when direction reverses. use crate::key_hint::KeyBindingListExt; +use codex_protocol::ThreadId; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -29,9 +30,9 @@ pub(crate) enum ViewAction { } #[derive(Clone, Copy, PartialEq, Eq)] -enum MouseMode { +enum MouseMode<'a> { Interactive, - Selection, + Selection(Option<(ThreadId, &'a str)>), } /// Transcript jumps shared with input routing so legacy terminals preserve their Alt modifier. @@ -251,16 +252,17 @@ impl TranscriptView { cells: &[Arc], ) -> Option { if self.select_mode.is_some() { - return self.handle_selection_mouse(event, cells); + return self.handle_selection_mouse(event, cells, /*patch_approval*/ None); } self.handle_mouse_with_mode(event, cells, MouseMode::Interactive) } - /// Handles selection and scrolling without activating transcript content. + /// Modals allow transcript disclosure, while select mode keeps clicks for text selection. pub(crate) fn handle_selection_mouse( &mut self, event: MouseEvent, cells: &[Arc], + patch_approval: Option<(ThreadId, &str)>, ) -> Option { let endpoint_changed = matches!( event.kind, @@ -272,12 +274,16 @@ impl TranscriptView { .selection .as_ref() .map(|selection| (selection.start, selection.end, selection.unit())); - let action = self.handle_mouse_with_mode(event, cells, MouseMode::Selection); + let action = + self.handle_mouse_with_mode(event, cells, MouseMode::Selection(patch_approval)); let after = self .selection .as_ref() .map(|selection| (selection.start, selection.end, selection.unit())); if endpoint_changed && before != after { + if self.has_selection_range() { + self.disclosure.resume_after_approval = None; + } self.adopt_select_mouse_range(); } action @@ -287,7 +293,7 @@ impl TranscriptView { &mut self, event: MouseEvent, cells: &[Arc], - mode: MouseMode, + mode: MouseMode<'_>, ) -> Option { if mode == MouseMode::Interactive && let Some(action) = self.handle_follow_control_mouse(event) @@ -498,12 +504,17 @@ impl TranscriptView { &mut self, event: MouseEvent, cells: &[Arc], - mode: MouseMode, + mode: MouseMode<'_>, ) -> Option { - if mode == MouseMode::Interactive + let was_following = self.is_following(); + if !self.is_select_mode() && event.modifiers.is_empty() && self.toggle_disclosure_at(cells, event.column, event.row) { + // Keep the patch anchored for inspection, then follow again when the prompt closes. + if was_following && let MouseMode::Selection(Some((thread_id, id))) = mode { + self.disclosure.resume_after_approval = Some((thread_id, id.to_owned())); + } return Some(ViewAction::Changed); } self.disclosure.focused = None; diff --git a/codex-rs/tui/src/transcript_view/input_tests.rs b/codex-rs/tui/src/transcript_view/input_tests.rs index 829240298e..802f45d962 100644 --- a/codex-rs/tui/src/transcript_view/input_tests.rs +++ b/codex-rs/tui/src/transcript_view/input_tests.rs @@ -319,7 +319,7 @@ fn selection_mouse_does_not_open_links() { ) }; assert!(matches!( - view.handle_selection_mouse(down, &cells), + view.handle_selection_mouse(down, &cells, /*patch_approval*/ None), Some(ViewAction::Changed) )); let area = view.area; @@ -331,6 +331,7 @@ fn selection_mouse_does_not_open_links() { ..down }, &cells, + /*patch_approval*/ None, ), Some(ViewAction::Changed) ));