Move approvals to use ListSelectionView (#4275)
Unify selection menus: - Move approvals to the vertical menu `ListSelectionView` - Add header section to `ListSelectionView` <img width="502" height="214" alt="image" src="https://github.com/user-attachments/assets/f4b43ddf-3549-403c-ad9e-a523688714e4" /> <img width="748" height="214" alt="image" src="https://github.com/user-attachments/assets/f94ac7b5-dc94-4dc0-a1df-7a8e3ba2453b" /> --------- Co-authored-by: pakrym-oai <pakrym@openai.com>
This commit is contained in:
Ahmed Ibrahim
GitHub
559
codex-rs/tui/src/bottom_pane/approval_overlay.rs
Normal file
559
codex-rs/tui/src/bottom_pane/approval_overlay.rs
Normal file
@@ -0,0 +1,559 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::bottom_pane::BottomPaneView;
|
||||
use crate::bottom_pane::CancellationEvent;
|
||||
use crate::bottom_pane::list_selection_view::HeaderLine;
|
||||
use crate::bottom_pane::list_selection_view::ListSelectionView;
|
||||
use crate::bottom_pane::list_selection_view::SelectionItem;
|
||||
use crate::bottom_pane::list_selection_view::SelectionViewParams;
|
||||
use crate::exec_command::strip_bash_lc_and_escape;
|
||||
use crate::history_cell;
|
||||
use crate::text_formatting::truncate_text;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::ReviewDecision;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyEventKind;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Stylize;
|
||||
use ratatui::text::Line;
|
||||
use ratatui::text::Span;
|
||||
|
||||
/// Request coming from the agent that needs user approval.
|
||||
pub(crate) enum ApprovalRequest {
|
||||
Exec {
|
||||
id: String,
|
||||
command: Vec<String>,
|
||||
reason: Option<String>,
|
||||
},
|
||||
ApplyPatch {
|
||||
id: String,
|
||||
reason: Option<String>,
|
||||
grant_root: Option<PathBuf>,
|
||||
},
|
||||
}
|
||||
|
||||
/// Modal overlay asking the user to approve or deny one or more requests.
|
||||
pub(crate) struct ApprovalOverlay {
|
||||
current: Option<ApprovalRequestState>,
|
||||
queue: Vec<ApprovalRequest>,
|
||||
app_event_tx: AppEventSender,
|
||||
list: ListSelectionView,
|
||||
options: Vec<ApprovalOption>,
|
||||
current_complete: bool,
|
||||
done: bool,
|
||||
}
|
||||
|
||||
impl ApprovalOverlay {
|
||||
pub fn new(request: ApprovalRequest, app_event_tx: AppEventSender) -> Self {
|
||||
let mut view = Self {
|
||||
current: Some(ApprovalRequestState::from(request)),
|
||||
queue: Vec::new(),
|
||||
app_event_tx: app_event_tx.clone(),
|
||||
list: ListSelectionView::new(
|
||||
SelectionViewParams {
|
||||
title: String::new(),
|
||||
..Default::default()
|
||||
},
|
||||
app_event_tx,
|
||||
),
|
||||
options: Vec::new(),
|
||||
current_complete: false,
|
||||
done: false,
|
||||
};
|
||||
let (options, params) = view.build_options();
|
||||
view.options = options;
|
||||
view.list = ListSelectionView::new(params, view.app_event_tx.clone());
|
||||
view
|
||||
}
|
||||
|
||||
pub fn enqueue_request(&mut self, req: ApprovalRequest) {
|
||||
self.queue.push(req);
|
||||
}
|
||||
|
||||
fn set_current(&mut self, request: ApprovalRequest) {
|
||||
self.current = Some(ApprovalRequestState::from(request));
|
||||
self.current_complete = false;
|
||||
let (options, params) = self.build_options();
|
||||
self.options = options;
|
||||
self.list = ListSelectionView::new(params, self.app_event_tx.clone());
|
||||
}
|
||||
|
||||
fn build_options(&self) -> (Vec<ApprovalOption>, SelectionViewParams) {
|
||||
let Some(state) = self.current.as_ref() else {
|
||||
return (
|
||||
Vec::new(),
|
||||
SelectionViewParams {
|
||||
title: String::new(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
};
|
||||
let (options, title) = match &state.variant {
|
||||
ApprovalVariant::Exec { .. } => (exec_options(), "Allow command?".to_string()),
|
||||
ApprovalVariant::ApplyPatch { .. } => (patch_options(), "Apply changes?".to_string()),
|
||||
};
|
||||
|
||||
let items = options
|
||||
.iter()
|
||||
.map(|opt| SelectionItem {
|
||||
name: opt.label.clone(),
|
||||
description: Some(opt.description.clone()),
|
||||
is_current: false,
|
||||
actions: Vec::new(),
|
||||
dismiss_on_select: false,
|
||||
search_value: None,
|
||||
})
|
||||
.collect();
|
||||
|
||||
let params = SelectionViewParams {
|
||||
title,
|
||||
footer_hint: Some("Press Enter to confirm or Esc to cancel".to_string()),
|
||||
items,
|
||||
header: state.header.clone(),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
(options, params)
|
||||
}
|
||||
|
||||
fn apply_selection(&mut self, actual_idx: usize) {
|
||||
if self.current_complete {
|
||||
return;
|
||||
}
|
||||
let Some(option) = self.options.get(actual_idx) else {
|
||||
return;
|
||||
};
|
||||
if let Some(state) = self.current.as_ref() {
|
||||
match (&state.variant, option.decision) {
|
||||
(ApprovalVariant::Exec { id, command }, decision) => {
|
||||
self.handle_exec_decision(id, command, decision);
|
||||
}
|
||||
(ApprovalVariant::ApplyPatch { id, .. }, decision) => {
|
||||
self.handle_patch_decision(id, decision);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self.current_complete = true;
|
||||
self.advance_queue();
|
||||
}
|
||||
|
||||
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
|
||||
if let Some(lines) = build_exec_history_lines(command.to_vec(), decision) {
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_user_approval_decision(lines),
|
||||
)));
|
||||
}
|
||||
self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval {
|
||||
id: id.to_string(),
|
||||
decision,
|
||||
}));
|
||||
}
|
||||
|
||||
fn handle_patch_decision(&self, id: &str, decision: ReviewDecision) {
|
||||
self.app_event_tx.send(AppEvent::CodexOp(Op::PatchApproval {
|
||||
id: id.to_string(),
|
||||
decision,
|
||||
}));
|
||||
}
|
||||
|
||||
fn advance_queue(&mut self) {
|
||||
if let Some(next) = self.queue.pop() {
|
||||
self.set_current(next);
|
||||
} else {
|
||||
self.done = true;
|
||||
}
|
||||
}
|
||||
|
||||
fn try_handle_shortcut(&mut self, key_event: &KeyEvent) -> bool {
|
||||
if key_event.kind != KeyEventKind::Press {
|
||||
return false;
|
||||
}
|
||||
let KeyEvent {
|
||||
code: KeyCode::Char(c),
|
||||
modifiers,
|
||||
..
|
||||
} = key_event
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
if modifiers.contains(KeyModifiers::CONTROL) || modifiers.contains(KeyModifiers::ALT) {
|
||||
return false;
|
||||
}
|
||||
let lower = c.to_ascii_lowercase();
|
||||
if let Some(idx) = self
|
||||
.options
|
||||
.iter()
|
||||
.position(|opt| opt.shortcut.map(|s| s == lower).unwrap_or(false))
|
||||
{
|
||||
self.apply_selection(idx);
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl BottomPaneView for ApprovalOverlay {
|
||||
fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
if self.try_handle_shortcut(&key_event) {
|
||||
return;
|
||||
}
|
||||
self.list.handle_key_event(key_event);
|
||||
if let Some(idx) = self.list.take_last_selected_index() {
|
||||
self.apply_selection(idx);
|
||||
}
|
||||
}
|
||||
|
||||
fn on_ctrl_c(&mut self) -> CancellationEvent {
|
||||
if self.done {
|
||||
return CancellationEvent::Handled;
|
||||
}
|
||||
if !self.current_complete
|
||||
&& let Some(state) = self.current.as_ref()
|
||||
{
|
||||
match &state.variant {
|
||||
ApprovalVariant::Exec { id, command } => {
|
||||
self.handle_exec_decision(id, command, ReviewDecision::Abort);
|
||||
}
|
||||
ApprovalVariant::ApplyPatch { id, .. } => {
|
||||
self.handle_patch_decision(id, ReviewDecision::Abort);
|
||||
}
|
||||
}
|
||||
}
|
||||
self.queue.clear();
|
||||
self.done = true;
|
||||
CancellationEvent::Handled
|
||||
}
|
||||
|
||||
fn is_complete(&self) -> bool {
|
||||
self.done
|
||||
}
|
||||
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
self.list.desired_height(width)
|
||||
}
|
||||
|
||||
fn render(&self, area: Rect, buf: &mut Buffer) {
|
||||
self.list.render(area, buf);
|
||||
}
|
||||
|
||||
fn try_consume_approval_request(
|
||||
&mut self,
|
||||
request: ApprovalRequest,
|
||||
) -> Option<ApprovalRequest> {
|
||||
self.enqueue_request(request);
|
||||
None
|
||||
}
|
||||
|
||||
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
|
||||
self.list.cursor_pos(area)
|
||||
}
|
||||
}
|
||||
|
||||
struct ApprovalRequestState {
|
||||
variant: ApprovalVariant,
|
||||
header: Vec<HeaderLine>,
|
||||
}
|
||||
|
||||
impl From<ApprovalRequest> for ApprovalRequestState {
|
||||
fn from(value: ApprovalRequest) -> Self {
|
||||
match value {
|
||||
ApprovalRequest::Exec {
|
||||
id,
|
||||
command,
|
||||
reason,
|
||||
} => {
|
||||
let mut header = Vec::new();
|
||||
if let Some(reason) = reason
|
||||
&& !reason.is_empty()
|
||||
{
|
||||
header.push(HeaderLine::Text {
|
||||
text: reason,
|
||||
italic: true,
|
||||
});
|
||||
header.push(HeaderLine::Spacer);
|
||||
}
|
||||
let command_snippet = exec_snippet(&command);
|
||||
if !command_snippet.is_empty() {
|
||||
header.push(HeaderLine::Text {
|
||||
text: format!("Command: {command_snippet}"),
|
||||
italic: false,
|
||||
});
|
||||
header.push(HeaderLine::Spacer);
|
||||
}
|
||||
Self {
|
||||
variant: ApprovalVariant::Exec { id, command },
|
||||
header,
|
||||
}
|
||||
}
|
||||
ApprovalRequest::ApplyPatch {
|
||||
id,
|
||||
reason,
|
||||
grant_root,
|
||||
} => {
|
||||
let mut header = Vec::new();
|
||||
if let Some(reason) = reason
|
||||
&& !reason.is_empty()
|
||||
{
|
||||
header.push(HeaderLine::Text {
|
||||
text: reason,
|
||||
italic: true,
|
||||
});
|
||||
header.push(HeaderLine::Spacer);
|
||||
}
|
||||
if let Some(root) = grant_root {
|
||||
header.push(HeaderLine::Text {
|
||||
text: format!(
|
||||
"Grant write access to {} for the remainder of this session.",
|
||||
root.display()
|
||||
),
|
||||
italic: false,
|
||||
});
|
||||
header.push(HeaderLine::Spacer);
|
||||
}
|
||||
Self {
|
||||
variant: ApprovalVariant::ApplyPatch { id },
|
||||
header,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
enum ApprovalVariant {
|
||||
Exec { id: String, command: Vec<String> },
|
||||
ApplyPatch { id: String },
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct ApprovalOption {
|
||||
label: String,
|
||||
description: String,
|
||||
decision: ReviewDecision,
|
||||
shortcut: Option<char>,
|
||||
}
|
||||
|
||||
fn exec_options() -> Vec<ApprovalOption> {
|
||||
vec![
|
||||
ApprovalOption {
|
||||
label: "Approve and run now".to_string(),
|
||||
description: "(Y) Run this command one time".to_string(),
|
||||
decision: ReviewDecision::Approved,
|
||||
shortcut: Some('y'),
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Always approve this session".to_string(),
|
||||
description: "(A) Automatically approve this command for the rest of the session"
|
||||
.to_string(),
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
shortcut: Some('a'),
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Cancel".to_string(),
|
||||
description: "(N) Do not run the command".to_string(),
|
||||
decision: ReviewDecision::Abort,
|
||||
shortcut: Some('n'),
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
fn patch_options() -> Vec<ApprovalOption> {
|
||||
vec![
|
||||
ApprovalOption {
|
||||
label: "Approve".to_string(),
|
||||
description: "(Y) Apply the proposed changes".to_string(),
|
||||
decision: ReviewDecision::Approved,
|
||||
shortcut: Some('y'),
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Cancel".to_string(),
|
||||
description: "(N) Do not apply the changes".to_string(),
|
||||
decision: ReviewDecision::Abort,
|
||||
shortcut: Some('n'),
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
fn build_exec_history_lines(
|
||||
command: Vec<String>,
|
||||
decision: ReviewDecision,
|
||||
) -> Option<Vec<Line<'static>>> {
|
||||
use ReviewDecision::*;
|
||||
|
||||
let (symbol, summary): (Span<'static>, Vec<Span<'static>>) = match decision {
|
||||
Approved => {
|
||||
let snippet = Span::from(exec_snippet(&command)).dim();
|
||||
(
|
||||
"✔ ".green(),
|
||||
vec![
|
||||
"You ".into(),
|
||||
"approved".bold(),
|
||||
" codex to run ".into(),
|
||||
snippet,
|
||||
" this time".bold(),
|
||||
],
|
||||
)
|
||||
}
|
||||
ApprovedForSession => {
|
||||
let snippet = Span::from(exec_snippet(&command)).dim();
|
||||
(
|
||||
"✔ ".green(),
|
||||
vec![
|
||||
"You ".into(),
|
||||
"approved".bold(),
|
||||
" codex to run ".into(),
|
||||
snippet,
|
||||
" every time this session".bold(),
|
||||
],
|
||||
)
|
||||
}
|
||||
Denied => {
|
||||
let snippet = Span::from(exec_snippet(&command)).dim();
|
||||
(
|
||||
"✗ ".red(),
|
||||
vec![
|
||||
"You ".into(),
|
||||
"did not approve".bold(),
|
||||
" codex to run ".into(),
|
||||
snippet,
|
||||
],
|
||||
)
|
||||
}
|
||||
Abort => {
|
||||
let snippet = Span::from(exec_snippet(&command)).dim();
|
||||
(
|
||||
"✗ ".red(),
|
||||
vec![
|
||||
"You ".into(),
|
||||
"canceled".bold(),
|
||||
" the request to run ".into(),
|
||||
snippet,
|
||||
],
|
||||
)
|
||||
}
|
||||
};
|
||||
|
||||
let mut lines = Vec::new();
|
||||
let mut spans = Vec::new();
|
||||
spans.push(symbol);
|
||||
spans.extend(summary);
|
||||
lines.push(Line::from(spans));
|
||||
Some(lines)
|
||||
}
|
||||
|
||||
fn truncate_exec_snippet(full_cmd: &str) -> String {
|
||||
let mut snippet = match full_cmd.split_once('\n') {
|
||||
Some((first, _)) => format!("{first} ..."),
|
||||
None => full_cmd.to_string(),
|
||||
};
|
||||
snippet = truncate_text(&snippet, 80);
|
||||
snippet
|
||||
}
|
||||
|
||||
fn exec_snippet(command: &[String]) -> String {
|
||||
let full_cmd = strip_bash_lc_and_escape(command);
|
||||
truncate_exec_snippet(&full_cmd)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::app_event::AppEvent;
|
||||
use tokio::sync::mpsc::unbounded_channel;
|
||||
|
||||
fn make_exec_request() -> ApprovalRequest {
|
||||
ApprovalRequest::Exec {
|
||||
id: "test".to_string(),
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
reason: Some("reason".to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ctrl_c_aborts_and_clears_queue() {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = ApprovalOverlay::new(make_exec_request(), tx);
|
||||
view.enqueue_request(make_exec_request());
|
||||
assert_eq!(CancellationEvent::Handled, view.on_ctrl_c());
|
||||
assert!(view.queue.is_empty());
|
||||
assert!(view.is_complete());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shortcut_triggers_selection() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = ApprovalOverlay::new(make_exec_request(), tx);
|
||||
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.
|
||||
let mut saw_op = false;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if matches!(ev, AppEvent::CodexOp(_)) {
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(saw_op, "expected approval decision to emit an op");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn header_includes_command_snippet() {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let command = vec!["echo".into(), "hello".into(), "world".into()];
|
||||
let exec_request = ApprovalRequest::Exec {
|
||||
id: "test".into(),
|
||||
command,
|
||||
reason: None,
|
||||
};
|
||||
|
||||
let view = ApprovalOverlay::new(exec_request, tx);
|
||||
let mut buf = Buffer::empty(Rect::new(0, 0, 80, 6));
|
||||
view.render(Rect::new(0, 0, 80, 6), &mut buf);
|
||||
|
||||
let rendered: Vec<String> = (0..buf.area.height)
|
||||
.map(|row| {
|
||||
(0..buf.area.width)
|
||||
.map(|col| buf[(col, row)].symbol().to_string())
|
||||
.collect()
|
||||
})
|
||||
.collect();
|
||||
assert!(
|
||||
rendered
|
||||
.iter()
|
||||
.any(|line| line.contains("Command: echo hello world")),
|
||||
"expected header to include command snippet, got {rendered:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enter_sets_last_selected_index_without_dismissing() {
|
||||
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
|
||||
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));
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
assert!(
|
||||
view.is_complete(),
|
||||
"exec approval should complete without queued requests"
|
||||
);
|
||||
|
||||
let mut decision = None;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if let AppEvent::CodexOp(Op::ExecApproval { decision: d, .. }) = ev {
|
||||
decision = Some(d);
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert_eq!(decision, Some(ReviewDecision::ApprovedForSession));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user