mirror of
https://github.com/openai/codex.git
synced 2026-05-05 13:51:29 +03:00
feat: approval for sub-agent in the TUI (#12995)
<img width="766" height="290" alt="Screenshot 2026-02-27 at 10 50 48" src="https://github.com/user-attachments/assets/3bc96cd9-ed2c-4d67-a317-8f7b60abbbb1" />
This commit is contained in:
@@ -17,6 +17,7 @@ use crate::render::highlight::highlight_bash_to_lines;
|
||||
use crate::render::renderable::ColumnRenderable;
|
||||
use crate::render::renderable::Renderable;
|
||||
use codex_core::features::Features;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::mcp::RequestId;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::ElicitationAction;
|
||||
@@ -41,6 +42,8 @@ use ratatui::widgets::Wrap;
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) enum ApprovalRequest {
|
||||
Exec {
|
||||
thread_id: ThreadId,
|
||||
thread_label: Option<String>,
|
||||
id: String,
|
||||
command: Vec<String>,
|
||||
reason: Option<String>,
|
||||
@@ -49,18 +52,40 @@ pub(crate) enum ApprovalRequest {
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
},
|
||||
ApplyPatch {
|
||||
thread_id: ThreadId,
|
||||
thread_label: Option<String>,
|
||||
id: String,
|
||||
reason: Option<String>,
|
||||
cwd: PathBuf,
|
||||
changes: HashMap<PathBuf, FileChange>,
|
||||
},
|
||||
McpElicitation {
|
||||
thread_id: ThreadId,
|
||||
thread_label: Option<String>,
|
||||
server_name: String,
|
||||
request_id: RequestId,
|
||||
message: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl ApprovalRequest {
|
||||
fn thread_id(&self) -> ThreadId {
|
||||
match self {
|
||||
ApprovalRequest::Exec { thread_id, .. }
|
||||
| ApprovalRequest::ApplyPatch { thread_id, .. }
|
||||
| ApprovalRequest::McpElicitation { thread_id, .. } => *thread_id,
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_label(&self) -> Option<&str> {
|
||||
match self {
|
||||
ApprovalRequest::Exec { thread_label, .. }
|
||||
| ApprovalRequest::ApplyPatch { thread_label, .. }
|
||||
| ApprovalRequest::McpElicitation { thread_label, .. } => thread_label.as_deref(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Modal overlay asking the user to approve or deny one or more requests.
|
||||
pub(crate) struct ApprovalOverlay {
|
||||
current_request: Option<ApprovalRequest>,
|
||||
@@ -158,13 +183,7 @@ impl ApprovalOverlay {
|
||||
.collect();
|
||||
|
||||
let params = SelectionViewParams {
|
||||
footer_hint: Some(Line::from(vec![
|
||||
"Press ".into(),
|
||||
key_hint::plain(KeyCode::Enter).into(),
|
||||
" to confirm or ".into(),
|
||||
key_hint::plain(KeyCode::Esc).into(),
|
||||
" to cancel".into(),
|
||||
])),
|
||||
footer_hint: Some(approval_footer_hint(request)),
|
||||
items,
|
||||
header,
|
||||
..Default::default()
|
||||
@@ -207,20 +226,39 @@ impl ApprovalOverlay {
|
||||
}
|
||||
|
||||
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
|
||||
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision.clone());
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
|
||||
self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval {
|
||||
id: id.to_string(),
|
||||
turn_id: None,
|
||||
decision,
|
||||
}));
|
||||
let Some(request) = self.current_request.as_ref() else {
|
||||
return;
|
||||
};
|
||||
if request.thread_label().is_none() {
|
||||
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision.clone());
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
|
||||
}
|
||||
let thread_id = request.thread_id();
|
||||
self.app_event_tx.send(AppEvent::SubmitThreadOp {
|
||||
thread_id,
|
||||
op: Op::ExecApproval {
|
||||
id: id.to_string(),
|
||||
turn_id: None,
|
||||
decision,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
fn handle_patch_decision(&self, id: &str, decision: ReviewDecision) {
|
||||
self.app_event_tx.send(AppEvent::CodexOp(Op::PatchApproval {
|
||||
id: id.to_string(),
|
||||
decision,
|
||||
}));
|
||||
let Some(thread_id) = self
|
||||
.current_request
|
||||
.as_ref()
|
||||
.map(ApprovalRequest::thread_id)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
self.app_event_tx.send(AppEvent::SubmitThreadOp {
|
||||
thread_id,
|
||||
op: Op::PatchApproval {
|
||||
id: id.to_string(),
|
||||
decision,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
fn handle_elicitation_decision(
|
||||
@@ -229,12 +267,21 @@ impl ApprovalOverlay {
|
||||
request_id: &RequestId,
|
||||
decision: ElicitationAction,
|
||||
) {
|
||||
self.app_event_tx
|
||||
.send(AppEvent::CodexOp(Op::ResolveElicitation {
|
||||
let Some(thread_id) = self
|
||||
.current_request
|
||||
.as_ref()
|
||||
.map(ApprovalRequest::thread_id)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
self.app_event_tx.send(AppEvent::SubmitThreadOp {
|
||||
thread_id,
|
||||
op: Op::ResolveElicitation {
|
||||
server_name: server_name.to_string(),
|
||||
request_id: request_id.clone(),
|
||||
decision,
|
||||
}));
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
fn advance_queue(&mut self) {
|
||||
@@ -261,6 +308,23 @@ impl ApprovalOverlay {
|
||||
false
|
||||
}
|
||||
}
|
||||
KeyEvent {
|
||||
kind: KeyEventKind::Press,
|
||||
code: KeyCode::Char('o'),
|
||||
..
|
||||
} => {
|
||||
if let Some(request) = self.current_request.as_ref() {
|
||||
if request.thread_label().is_some() {
|
||||
self.app_event_tx
|
||||
.send(AppEvent::SelectAgentThread(request.thread_id()));
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
e => {
|
||||
if let Some(idx) = self
|
||||
.options
|
||||
@@ -347,9 +411,28 @@ impl Renderable for ApprovalOverlay {
|
||||
}
|
||||
}
|
||||
|
||||
fn approval_footer_hint(request: &ApprovalRequest) -> Line<'static> {
|
||||
let mut spans = vec![
|
||||
"Press ".into(),
|
||||
key_hint::plain(KeyCode::Enter).into(),
|
||||
" to confirm or ".into(),
|
||||
key_hint::plain(KeyCode::Esc).into(),
|
||||
" to cancel".into(),
|
||||
];
|
||||
if request.thread_label().is_some() {
|
||||
spans.extend([
|
||||
" or ".into(),
|
||||
key_hint::plain(KeyCode::Char('o')).into(),
|
||||
" to open thread".into(),
|
||||
]);
|
||||
}
|
||||
Line::from(spans)
|
||||
}
|
||||
|
||||
fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
|
||||
match request {
|
||||
ApprovalRequest::Exec {
|
||||
thread_label,
|
||||
reason,
|
||||
command,
|
||||
network_approval_context,
|
||||
@@ -357,6 +440,13 @@ fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
|
||||
..
|
||||
} => {
|
||||
let mut header: Vec<Line<'static>> = Vec::new();
|
||||
if let Some(thread_label) = thread_label {
|
||||
header.push(Line::from(vec![
|
||||
"Thread: ".into(),
|
||||
thread_label.clone().bold(),
|
||||
]));
|
||||
header.push(Line::from(""));
|
||||
}
|
||||
if let Some(reason) = reason {
|
||||
header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()]));
|
||||
header.push(Line::from(""));
|
||||
@@ -381,12 +471,20 @@ fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
|
||||
Box::new(Paragraph::new(header).wrap(Wrap { trim: false }))
|
||||
}
|
||||
ApprovalRequest::ApplyPatch {
|
||||
thread_label,
|
||||
reason,
|
||||
cwd,
|
||||
changes,
|
||||
..
|
||||
} => {
|
||||
let mut header: Vec<Box<dyn Renderable>> = Vec::new();
|
||||
if let Some(thread_label) = thread_label {
|
||||
header.push(Box::new(Line::from(vec![
|
||||
"Thread: ".into(),
|
||||
thread_label.clone().bold(),
|
||||
])));
|
||||
header.push(Box::new(Line::from("")));
|
||||
}
|
||||
if let Some(reason) = reason
|
||||
&& !reason.is_empty()
|
||||
{
|
||||
@@ -403,16 +501,25 @@ fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
|
||||
Box::new(ColumnRenderable::with(header))
|
||||
}
|
||||
ApprovalRequest::McpElicitation {
|
||||
thread_label,
|
||||
server_name,
|
||||
message,
|
||||
..
|
||||
} => {
|
||||
let header = Paragraph::new(vec![
|
||||
let mut lines = Vec::new();
|
||||
if let Some(thread_label) = thread_label {
|
||||
lines.push(Line::from(vec![
|
||||
"Thread: ".into(),
|
||||
thread_label.clone().bold(),
|
||||
]));
|
||||
lines.push(Line::from(""));
|
||||
}
|
||||
lines.extend([
|
||||
Line::from(vec!["Server: ".into(), server_name.clone().bold()]),
|
||||
Line::from(""),
|
||||
Line::from(message.clone()),
|
||||
])
|
||||
.wrap(Wrap { trim: false });
|
||||
]);
|
||||
let header = Paragraph::new(lines).wrap(Wrap { trim: false });
|
||||
Box::new(header)
|
||||
}
|
||||
}
|
||||
@@ -652,6 +759,8 @@ mod tests {
|
||||
|
||||
fn make_exec_request() -> ApprovalRequest {
|
||||
ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".to_string(),
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
reason: Some("reason".to_string()),
|
||||
@@ -679,10 +788,10 @@ mod tests {
|
||||
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.
|
||||
// We expect at least one thread-scoped approval op message in the queue.
|
||||
let mut saw_op = false;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if matches!(ev, AppEvent::CodexOp(_)) {
|
||||
if matches!(ev, AppEvent::SubmitThreadOp { .. }) {
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
@@ -690,12 +799,68 @@ mod tests {
|
||||
assert!(saw_op, "expected approval decision to emit an op");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn o_opens_source_thread_for_cross_thread_approval() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let thread_id = ThreadId::new();
|
||||
let mut view = ApprovalOverlay::new(
|
||||
ApprovalRequest::Exec {
|
||||
thread_id,
|
||||
thread_label: Some("Robie [explorer]".to_string()),
|
||||
id: "test".to_string(),
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
reason: None,
|
||||
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
|
||||
network_approval_context: None,
|
||||
additional_permissions: None,
|
||||
},
|
||||
tx,
|
||||
Features::with_defaults(),
|
||||
);
|
||||
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Char('o'), KeyModifiers::NONE));
|
||||
|
||||
let event = rx.try_recv().expect("expected select-agent-thread event");
|
||||
assert_eq!(
|
||||
matches!(event, AppEvent::SelectAgentThread(id) if id == thread_id),
|
||||
true
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cross_thread_footer_hint_mentions_o_shortcut() {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let view = ApprovalOverlay::new(
|
||||
ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: Some("Robie [explorer]".to_string()),
|
||||
id: "test".to_string(),
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
reason: None,
|
||||
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
|
||||
network_approval_context: None,
|
||||
additional_permissions: None,
|
||||
},
|
||||
tx,
|
||||
Features::with_defaults(),
|
||||
);
|
||||
|
||||
assert_snapshot!(
|
||||
"approval_overlay_cross_thread_prompt",
|
||||
render_overlay_lines(&view, 80)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_prefix_option_emits_execpolicy_amendment() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = ApprovalOverlay::new(
|
||||
ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".to_string(),
|
||||
command: vec!["echo".to_string()],
|
||||
reason: None,
|
||||
@@ -717,7 +882,11 @@ mod tests {
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE));
|
||||
let mut saw_op = false;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if let AppEvent::CodexOp(Op::ExecApproval { decision, .. }) = ev {
|
||||
if let AppEvent::SubmitThreadOp {
|
||||
op: Op::ExecApproval { decision, .. },
|
||||
..
|
||||
} = ev
|
||||
{
|
||||
assert_eq!(
|
||||
decision,
|
||||
ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
@@ -742,6 +911,8 @@ mod tests {
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = ApprovalOverlay::new(
|
||||
ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".to_string(),
|
||||
command: vec!["curl".to_string(), "https://example.com".to_string()],
|
||||
reason: None,
|
||||
@@ -779,6 +950,8 @@ mod tests {
|
||||
let tx = AppEventSender::new(tx);
|
||||
let command = vec!["echo".into(), "hello".into(), "world".into()];
|
||||
let exec_request = ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".into(),
|
||||
command,
|
||||
reason: None,
|
||||
@@ -893,6 +1066,8 @@ mod tests {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let exec_request = ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".into(),
|
||||
command: vec!["cat".into(), "/tmp/readme.txt".into()],
|
||||
reason: None,
|
||||
@@ -932,6 +1107,8 @@ mod tests {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let exec_request = ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".into(),
|
||||
command: vec!["cat".into(), "/tmp/readme.txt".into()],
|
||||
reason: Some("need filesystem access".into()),
|
||||
@@ -958,6 +1135,8 @@ mod tests {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let exec_request = ApprovalRequest::Exec {
|
||||
thread_id: ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "test".into(),
|
||||
command: vec!["curl".into(), "https://example.com".into()],
|
||||
reason: Some("network request blocked".into()),
|
||||
@@ -1049,7 +1228,11 @@ mod tests {
|
||||
|
||||
let mut decision = None;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if let AppEvent::CodexOp(Op::ExecApproval { decision: d, .. }) = ev {
|
||||
if let AppEvent::SubmitThreadOp {
|
||||
op: Op::ExecApproval { decision: d, .. },
|
||||
..
|
||||
} = ev
|
||||
{
|
||||
decision = Some(d);
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -812,6 +812,11 @@ impl BottomPane {
|
||||
self.is_task_running
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn has_active_view(&self) -> bool {
|
||||
!self.view_stack.is_empty()
|
||||
}
|
||||
|
||||
/// Return true when the pane is in the regular composer state without any
|
||||
/// overlays or popups and not running a task. This is the safe context to
|
||||
/// use Esc-Esc for backtracking from the main view.
|
||||
@@ -1115,6 +1120,8 @@ mod tests {
|
||||
|
||||
fn exec_request() -> ApprovalRequest {
|
||||
ApprovalRequest::Exec {
|
||||
thread_id: codex_protocol::ThreadId::new(),
|
||||
thread_label: None,
|
||||
id: "1".to_string(),
|
||||
command: vec!["echo".into(), "ok".into()],
|
||||
reason: None,
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/approval_overlay.rs
|
||||
expression: "render_overlay_lines(&view, 80)"
|
||||
---
|
||||
Would you like to run the following command?
|
||||
|
||||
Thread: Robie [explorer]
|
||||
|
||||
$ echo hi
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel or o to open thread
|
||||
Reference in New Issue
Block a user