diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 7a8a64948..768fe030d 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -16,6 +16,8 @@ use crate::key_hint::KeyBinding; use crate::render::highlight::highlight_bash_to_lines; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; +use codex_core::features::Feature; +use codex_core::features::Features; use codex_core::protocol::ElicitationAction; use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::FileChange; @@ -69,10 +71,11 @@ pub(crate) struct ApprovalOverlay { options: Vec, current_complete: bool, done: bool, + features: Features, } impl ApprovalOverlay { - pub fn new(request: ApprovalRequest, app_event_tx: AppEventSender) -> Self { + pub fn new(request: ApprovalRequest, app_event_tx: AppEventSender, features: Features) -> Self { let mut view = Self { current_request: None, current_variant: None, @@ -82,6 +85,7 @@ impl ApprovalOverlay { options: Vec::new(), current_complete: false, done: false, + features, }; view.set_current(request); view @@ -96,7 +100,7 @@ impl ApprovalOverlay { let ApprovalRequestState { variant, header } = ApprovalRequestState::from(request); self.current_variant = Some(variant.clone()); self.current_complete = false; - let (options, params) = Self::build_options(variant, header); + let (options, params) = Self::build_options(variant, header, &self.features); self.options = options; self.list = ListSelectionView::new(params, self.app_event_tx.clone()); } @@ -104,13 +108,14 @@ impl ApprovalOverlay { fn build_options( variant: ApprovalVariant, header: Box, + features: &Features, ) -> (Vec, SelectionViewParams) { let (options, title) = match &variant { ApprovalVariant::Exec { proposed_execpolicy_amendment, .. } => ( - exec_options(proposed_execpolicy_amendment.clone()), + exec_options(proposed_execpolicy_amendment.clone(), features), "Would you like to run the following command?".to_string(), ), ApprovalVariant::ApplyPatch { .. } => ( @@ -474,30 +479,36 @@ impl ApprovalOption { } } -fn exec_options(proposed_execpolicy_amendment: Option) -> Vec { - vec![ - ApprovalOption { - label: "Yes, proceed".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Approved), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }, - ApprovalOption { - label: "Yes, and don't ask again this session".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], - }, - ] - .into_iter() - .chain(proposed_execpolicy_amendment.map(|prefix| ApprovalOption { - label: "Yes, and don't ask again for commands with this prefix".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedExecpolicyAmendment { - proposed_execpolicy_amendment: prefix, - }), +fn exec_options( + proposed_execpolicy_amendment: Option, + features: &Features, +) -> Vec { + vec![ApprovalOption { + label: "Yes, proceed".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Approved), display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - })) + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], + }] + .into_iter() + .chain( + proposed_execpolicy_amendment + .filter(|_| features.enabled(Feature::ExecPolicy)) + .map(|prefix| { + let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); + ApprovalOption { + label: format!( + "Yes, and don't ask again for commands that start with `{rendered_prefix}`" + ), + decision: ApprovalDecision::Review( + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: prefix, + }, + ), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + } + }), + ) .chain([ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Abort), @@ -568,7 +579,7 @@ mod tests { fn ctrl_c_aborts_and_clears_queue() { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new(make_exec_request(), tx); + let mut view = ApprovalOverlay::new(make_exec_request(), tx, Features::with_defaults()); view.enqueue_request(make_exec_request()); assert_eq!(CancellationEvent::Handled, view.on_ctrl_c()); assert!(view.queue.is_empty()); @@ -579,7 +590,7 @@ mod tests { fn shortcut_triggers_selection() { let (tx, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); - let mut view = ApprovalOverlay::new(make_exec_request(), tx); + let mut view = ApprovalOverlay::new(make_exec_request(), tx, Features::with_defaults()); assert!(!view.is_complete()); view.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); // We expect at least one CodexOp message in the queue. @@ -608,6 +619,7 @@ mod tests { ])), }, tx, + Features::with_defaults(), ); view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE)); let mut saw_op = false; @@ -631,6 +643,33 @@ mod tests { ); } + #[test] + fn exec_prefix_option_hidden_when_execpolicy_disabled() { + let (tx, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let mut view = ApprovalOverlay::new( + ApprovalRequest::Exec { + id: "test".to_string(), + command: vec!["echo".to_string()], + reason: None, + risk: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ + "echo".to_string(), + ])), + }, + tx, + { + let mut features = Features::with_defaults(); + features.disable(Feature::ExecPolicy); + features + }, + ); + assert_eq!(view.options.len(), 2); + view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE)); + assert!(!view.is_complete()); + assert!(rx.try_recv().is_err()); + } + #[test] fn header_includes_command_snippet() { let (tx, _rx) = unbounded_channel::(); @@ -644,7 +683,7 @@ mod tests { proposed_execpolicy_amendment: None, }; - let view = ApprovalOverlay::new(exec_request, tx); + let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults()); let mut buf = Buffer::empty(Rect::new(0, 0, 80, view.desired_height(80))); view.render(Rect::new(0, 0, 80, view.desired_height(80)), &mut buf); @@ -694,8 +733,7 @@ mod tests { fn enter_sets_last_selected_index_without_dismissing() { let (tx_raw, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); - let mut view = ApprovalOverlay::new(make_exec_request(), tx); - view.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + let mut view = ApprovalOverlay::new(make_exec_request(), tx, Features::with_defaults()); view.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); assert!( @@ -710,6 +748,6 @@ mod tests { break; } } - assert_eq!(decision, Some(ReviewDecision::ApprovedForSession)); + assert_eq!(decision, Some(ReviewDecision::Approved)); } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5ebb492a2..8a4336f6f 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -8,6 +8,7 @@ use crate::render::renderable::Renderable; use crate::render::renderable::RenderableItem; use crate::tui::FrameRequester; use bottom_pane_view::BottomPaneView; +use codex_core::features::Features; use codex_core::skills::model::SkillMetadata; use codex_file_search::FileMatch; use crossterm::event::KeyCode; @@ -409,7 +410,7 @@ impl BottomPane { } /// Called when the agent requests user approval. - pub fn push_approval_request(&mut self, request: ApprovalRequest) { + pub fn push_approval_request(&mut self, request: ApprovalRequest, features: &Features) { let request = if let Some(view) = self.view_stack.last_mut() { match view.try_consume_approval_request(request) { Some(request) => request, @@ -423,7 +424,7 @@ impl BottomPane { }; // Otherwise create a new approval modal overlay. - let modal = ApprovalOverlay::new(request, self.app_event_tx.clone()); + let modal = ApprovalOverlay::new(request, self.app_event_tx.clone(), features.clone()); self.pause_status_timer_for_modal(); self.push_view(Box::new(modal)); } @@ -578,6 +579,7 @@ mod tests { fn ctrl_c_on_modal_consumes_and_shows_quit_hint() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); let mut pane = BottomPane::new(BottomPaneParams { app_event_tx: tx, frame_requester: FrameRequester::test_dummy(), @@ -588,7 +590,7 @@ mod tests { animations_enabled: true, skills: Some(Vec::new()), }); - pane.push_approval_request(exec_request()); + pane.push_approval_request(exec_request(), &features); assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c()); assert!(pane.ctrl_c_quit_hint_visible()); assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c()); @@ -600,6 +602,7 @@ mod tests { fn overlay_not_shown_above_approval_modal() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); let mut pane = BottomPane::new(BottomPaneParams { app_event_tx: tx, frame_requester: FrameRequester::test_dummy(), @@ -612,7 +615,7 @@ mod tests { }); // Create an approval modal (active view). - pane.push_approval_request(exec_request()); + pane.push_approval_request(exec_request(), &features); // Render and verify the top row does not include an overlay. let area = Rect::new(0, 0, 60, 6); @@ -633,6 +636,7 @@ mod tests { fn composer_shown_after_denied_while_task_running() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); let mut pane = BottomPane::new(BottomPaneParams { app_event_tx: tx, frame_requester: FrameRequester::test_dummy(), @@ -648,7 +652,7 @@ mod tests { pane.set_task_running(true); // Push an approval modal (e.g., command approval) which should hide the status view. - pane.push_approval_request(exec_request()); + pane.push_approval_request(exec_request(), &features); // Simulate pressing 'n' (No) on the modal. use crossterm::event::KeyCode; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 2684fa9ff..8f9db3b9d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1076,7 +1076,8 @@ impl ChatWidget { risk: ev.risk, proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment, }; - self.bottom_pane.push_approval_request(request); + self.bottom_pane + .push_approval_request(request, &self.config.features); self.request_redraw(); } @@ -1093,7 +1094,8 @@ impl ChatWidget { changes: ev.changes.clone(), cwd: self.config.cwd.clone(), }; - self.bottom_pane.push_approval_request(request); + self.bottom_pane + .push_approval_request(request, &self.config.features); self.request_redraw(); self.notify(Notification::EditApprovalRequested { cwd: self.config.cwd.clone(), @@ -1113,7 +1115,8 @@ impl ChatWidget { request_id: ev.id, message: ev.message, }; - self.bottom_pane.push_approval_request(request); + self.bottom_pane + .push_approval_request(request, &self.config.features); self.request_redraw(); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap index eaf0fed3a..ca093f271 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap @@ -4,14 +4,12 @@ expression: terminal.backend().vt100().screen().contents() --- Would you like to run the following command? - Reason: this is a test reason such as one that would be produced by the - model + Reason: this is a test reason such as one that would be produced by the model $ echo hello world › 1. Yes, proceed (y) - 2. Yes, and don't ask again this session (a) - 3. Yes, and don't ask again for commands with this prefix (p) - 4. No, and tell Codex what to do differently (esc) + 2. Yes, and don't ask again for commands that start with `echo hello world` (p) + 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/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap index ce2277ce6..2bbe9aefc 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap @@ -7,8 +7,7 @@ expression: terminal.backend().vt100().screen().contents() $ echo hello world › 1. Yes, proceed (y) - 2. Yes, and don't ask again this session (a) - 3. Yes, and don't ask again for commands with this prefix (p) - 4. No, and tell Codex what to do differently (esc) + 2. Yes, and don't ask again for commands that start with `echo hello world` (p) + 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/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap index ff70d7d49..1c6a3ef13 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap @@ -3,7 +3,7 @@ source: tui/src/chatwidget/tests.rs expression: "format!(\"{buf:?}\")" --- Buffer { - area: Rect { x: 0, y: 0, width: 80, height: 14 }, + area: Rect { x: 0, y: 0, width: 80, height: 13 }, content: [ " ", " ", @@ -15,8 +15,7 @@ Buffer { " $ echo hello world ", " ", "› 1. Yes, proceed (y) ", - " 2. Yes, and don't ask again this session (a) ", - " 3. No, and tell Codex what to do differently (esc) ", + " 2. No, and tell Codex what to do differently (esc) ", " ", " Press enter to confirm or esc to cancel ", ], @@ -30,10 +29,8 @@ Buffer { x: 7, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, x: 0, y: 9, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, x: 21, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 44, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, - x: 45, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 48, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, - x: 51, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 2, y: 13, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 48, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 51, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 12, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, ] } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap index 5ce388c87..5e6e33dec 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap @@ -2,18 +2,16 @@ source: tui/src/chatwidget/tests.rs expression: terminal.backend() --- -" " -" " -" Would you like to run the following command? " -" " -" Reason: this is a test reason such as one that would be produced by the " -" model " -" " -" $ echo 'hello world' " -" " -"› 1. Yes, proceed (y) " -" 2. Yes, and don't ask again this session (a) " -" 3. Yes, and don't ask again for commands with this prefix (p) " -" 4. No, and tell Codex what to do differently (esc) " -" " -" Press enter to confirm or esc to cancel " +" " +" " +" Would you like to run the following command? " +" " +" Reason: this is a test reason such as one that would be produced by the model " +" " +" $ echo 'hello world' " +" " +"› 1. Yes, proceed (y) " +" 2. Yes, and don't ask again for commands that start with `echo 'hello world'` (p) " +" 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/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index abce9d428..ef1de1fde 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2007,11 +2007,12 @@ fn approval_modal_exec_snapshot() { }); // Render to a fixed-size test terminal and snapshot. // Call desired_height first and use that exact height for rendering. - let height = chat.desired_height(80); + let width = 100; + let height = chat.desired_height(width); let mut terminal = - crate::custom_terminal::Terminal::with_options(VT100Backend::new(80, height)) + crate::custom_terminal::Terminal::with_options(VT100Backend::new(width, height)) .expect("create terminal"); - let viewport = Rect::new(0, 0, 80, height); + let viewport = Rect::new(0, 0, width, height); terminal.set_viewport_area(viewport); terminal @@ -2057,10 +2058,11 @@ fn approval_modal_exec_without_reason_snapshot() { msg: EventMsg::ExecApprovalRequest(ev), }); - let height = chat.desired_height(80); + let width = 100; + let height = chat.desired_height(width); let mut terminal = - ratatui::Terminal::new(VT100Backend::new(80, height)).expect("create terminal"); - terminal.set_viewport_area(Rect::new(0, 0, 80, height)); + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); terminal .draw(|f| chat.render(f.area(), f.buffer_mut())) .expect("draw approval modal (no reason)"); @@ -2275,9 +2277,11 @@ fn status_widget_and_approval_modal_snapshot() { }); // Render at the widget's desired height and snapshot. - let height = chat.desired_height(80); - let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + let width: u16 = 100; + let height = chat.desired_height(width); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(width, height)) .expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); terminal .draw(|f| chat.render(f.area(), f.buffer_mut())) .expect("draw status + approval modal");