diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index c89e352f78..19b175f030 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -23,9 +23,13 @@ use crate::app_server_session::AppServerStartedThread; use crate::app_server_session::TurnPermissionsOverride; use crate::app_server_session::app_server_rate_limit_snapshots; use crate::bottom_pane::AppLinkViewParams; +use crate::bottom_pane::ApplyPatchApprovalRequest; use crate::bottom_pane::ApprovalRequest; +use crate::bottom_pane::ExecApprovalRequest; use crate::bottom_pane::FeedbackAudience; +use crate::bottom_pane::McpElicitationApprovalRequest; use crate::bottom_pane::McpServerElicitationFormRequest; +use crate::bottom_pane::PermissionsApprovalRequest; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::bottom_pane::popup_consts::standard_popup_hint_line; diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index f1dc58fd51..8305719187 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -2091,18 +2091,18 @@ impl App { self.chat_widget.handle_manage_skills_closed(); } AppEvent::FullScreenApprovalRequest(request) => match request { - ApprovalRequest::ApplyPatch { cwd, changes, .. } => { + ApprovalRequest::ApplyPatch(request) => { let _ = tui.enter_alt_screen(); - let diff_summary = DiffSummary::new(changes, cwd); + let diff_summary = DiffSummary::new(request.changes, request.cwd); self.overlay = Some(Overlay::new_static_with_renderables( vec![diff_summary.into()], "P A T C H".to_string(), self.keymap.pager.clone(), )); } - ApprovalRequest::Exec { command, .. } => { + ApprovalRequest::Exec(request) => { let _ = tui.enter_alt_screen(); - let full_cmd = strip_bash_lc_and_escape(&command); + let full_cmd = strip_bash_lc_and_escape(&request.command); let full_cmd_lines = highlight_bash_to_lines(&full_cmd); self.overlay = Some(Overlay::new_static_with_lines( full_cmd_lines, @@ -2110,27 +2110,22 @@ impl App { self.keymap.pager.clone(), )); } - ApprovalRequest::Permissions { - environment_id, - permissions, - reason, - .. - } => { + ApprovalRequest::Permissions(request) => { let _ = tui.enter_alt_screen(); let mut lines = Vec::new(); - if let Some(environment_id) = environment_id { + if let Some(environment_id) = request.environment_id { lines.push(Line::from(vec![ "Environment: ".into(), environment_id.bold(), ])); lines.push(Line::from("")); } - if let Some(reason) = reason { + if let Some(reason) = request.reason { lines.push(Line::from(vec!["Reason: ".into(), reason.italic()])); lines.push(Line::from("")); } if let Some(rule_line) = - crate::bottom_pane::format_requested_permissions_rule(&permissions) + crate::bottom_pane::format_requested_permissions_rule(&request.permissions) { lines.push(Line::from(vec![ "Permission rule: ".into(), @@ -2143,16 +2138,12 @@ impl App { self.keymap.pager.clone(), )); } - ApprovalRequest::McpElicitation { - server_name, - message, - .. - } => { + ApprovalRequest::McpElicitation(request) => { let _ = tui.enter_alt_screen(); let paragraph = Paragraph::new(vec![ - Line::from(vec!["Server: ".into(), server_name.bold()]), + Line::from(vec!["Server: ".into(), request.server_name.bold()]), Line::from(""), - Line::from(message), + Line::from(request.message), ]) .wrap(Wrap { trim: false }); self.overlay = Some(Overlay::new_static_with_renderables( diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 68c4f8dd87..764733fd0e 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2543,12 +2543,7 @@ async fn inactive_thread_exec_approval_preserves_context() { action: AppServerNetworkPolicyRuleAction::Allow, }]); - let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec { - available_decisions, - network_approval_context, - additional_permissions, - .. - })) = app + let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec(approval))) = app .interactive_request_for_thread_request(thread_id, &request) .await .expect("valid localized paths") @@ -2557,14 +2552,14 @@ async fn inactive_thread_exec_approval_preserves_context() { }; assert_eq!( - network_approval_context, + approval.network_approval_context, Some(AppServerNetworkApprovalContext { host: "example.com".to_string(), protocol: AppServerNetworkApprovalProtocol::Socks5Tcp, }) ); assert_eq!( - additional_permissions, + approval.additional_permissions, Some(AdditionalPermissionProfile { network: Some(AdditionalNetworkPermissions { enabled: Some(true), @@ -2578,7 +2573,7 @@ async fn inactive_thread_exec_approval_preserves_context() { }) ); assert_eq!( - available_decisions, + approval.available_decisions, vec![ codex_app_server_protocol::CommandExecutionApprovalDecision::Accept, codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptForSession, @@ -2610,7 +2605,7 @@ async fn inactive_thread_exec_approval_splits_shell_wrapped_command() { params.command = Some(shlex::try_join(["/bin/zsh", "-lc", script]).expect("round-trippable shell wrapper")); - let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec { command, .. })) = app + let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec(approval))) = app .interactive_request_for_thread_request(thread_id, &request) .await .expect("valid localized paths") @@ -2619,7 +2614,7 @@ async fn inactive_thread_exec_approval_splits_shell_wrapped_command() { }; assert_eq!( - command, + approval.command, vec![ "/bin/zsh".to_string(), "-lc".to_string(), @@ -2670,14 +2665,11 @@ async fn inactive_thread_file_change_approval_recovers_buffered_changes() { .expect("valid localized paths") .expect("expected file change approval request"); - let ThreadInteractiveRequest::Approval(ApprovalRequest::ApplyPatch { - changes, reason, .. - }) = &request - else { + let ThreadInteractiveRequest::Approval(ApprovalRequest::ApplyPatch(approval)) = &request else { panic!("expected apply-patch approval request"); }; assert_eq!( - changes, + &approval.changes, &HashMap::from([( PathBuf::from("README.md"), FileChange::Add { @@ -2686,7 +2678,7 @@ async fn inactive_thread_file_change_approval_recovers_buffered_changes() { )]) ); assert_eq!( - reason, + &approval.reason, &Some("command failed; retry without sandbox?".to_string()) ); @@ -2728,11 +2720,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions( }, }; - let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Permissions { - environment_id, - permissions, - .. - })) = app + let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Permissions(approval))) = app .interactive_request_for_thread_request(thread_id, &request) .await .expect("valid localized paths") @@ -2740,9 +2728,9 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions( panic!("expected permissions approval request"); }; - assert_eq!(environment_id.as_deref(), Some("remote")); + assert_eq!(approval.environment_id.as_deref(), Some("remote")); assert_eq!( - permissions, + approval.permissions, RequestPermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 4fae845fc1..671a3ce560 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -219,7 +219,7 @@ impl App { let proposed_execpolicy_amendment = params.proposed_execpolicy_amendment.clone(); let proposed_network_policy_amendments = params.proposed_network_policy_amendments.clone(); - Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec { + let approval = ExecApprovalRequest { thread_id, thread_label, id: params @@ -243,10 +243,14 @@ impl App { }), network_approval_context, additional_permissions, - })) + }; + Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec( + approval, + ))) } ServerRequest::FileChangeRequestApproval { params, .. } => Some( - ThreadInteractiveRequest::Approval(ApprovalRequest::ApplyPatch { + ThreadInteractiveRequest::Approval(ApprovalRequest::ApplyPatch( + ApplyPatchApprovalRequest { thread_id, thread_label, id: params.item_id.clone(), @@ -260,7 +264,8 @@ impl App { .await .map(crate::app_server_approval_conversions::file_update_changes_to_display) .unwrap_or_default(), - }), + }, + )), ), ServerRequest::McpServerElicitationRequest { request_id, params } => { if let Some(params) = AppLinkViewParams::from_url_app_server_request( @@ -284,13 +289,13 @@ impl App { message, .. } => Some(ThreadInteractiveRequest::Approval( - ApprovalRequest::McpElicitation { + ApprovalRequest::McpElicitation(McpElicitationApprovalRequest { thread_id, thread_label, server_name: params.server_name.clone(), request_id: request_id.clone(), message: message.clone(), - }, + }), )), codex_app_server_protocol::McpServerElicitationRequest::OpenAiForm { .. @@ -319,14 +324,14 @@ impl App { ) })?; Some(ThreadInteractiveRequest::Approval( - ApprovalRequest::Permissions { + ApprovalRequest::Permissions(PermissionsApprovalRequest { thread_id, thread_label, call_id: params.item_id.clone(), environment_id: params.environment_id.clone(), reason: params.reason.clone(), permissions, - }, + }), )) } _ => None, @@ -350,20 +355,17 @@ impl App { } fn render_inactive_patch_preview(&mut self, request: &ApprovalRequest) { - let ApprovalRequest::ApplyPatch { - thread_label, - cwd, - changes, - .. - } = request - else { + let ApprovalRequest::ApplyPatch(request) = request else { return; }; - if thread_label.is_none() || changes.is_empty() { + if request.thread_label.is_none() || request.changes.is_empty() { return; } self.chat_widget - .add_to_history(history_cell::new_patch_event(changes.clone(), cwd)); + .add_to_history(history_cell::new_patch_event( + request.changes.clone(), + &request.cwd, + )); } pub(super) async fn pending_inactive_thread_requests(&self) -> Vec<(ThreadId, ServerRequest)> { diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 672c19d439..ccab67ef05 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -71,86 +71,97 @@ use ratatui::widgets::Wrap; /// Request coming from the agent that needs user approval. #[derive(Clone, Debug)] pub(crate) enum ApprovalRequest { - Exec { - thread_id: ThreadId, - thread_label: Option, - id: String, - environment_id: Option, - command: Vec, - reason: Option, - available_decisions: Vec, - network_approval_context: Option, - additional_permissions: Option, - }, - Permissions { - thread_id: ThreadId, - thread_label: Option, - call_id: String, - environment_id: Option, - reason: Option, - permissions: RequestPermissionProfile, - }, - ApplyPatch { - thread_id: ThreadId, - thread_label: Option, - id: String, - reason: Option, - cwd: AbsolutePathBuf, - changes: HashMap, - }, - McpElicitation { - thread_id: ThreadId, - thread_label: Option, - server_name: String, - request_id: RequestId, - message: String, - }, + Exec(ExecApprovalRequest), + Permissions(PermissionsApprovalRequest), + ApplyPatch(ApplyPatchApprovalRequest), + McpElicitation(McpElicitationApprovalRequest), +} + +#[derive(Clone, Debug)] +pub(crate) struct ExecApprovalRequest { + pub thread_id: ThreadId, + pub thread_label: Option, + pub id: String, + pub environment_id: Option, + pub command: Vec, + pub reason: Option, + pub available_decisions: Vec, + pub network_approval_context: Option, + pub additional_permissions: Option, +} + +#[derive(Clone, Debug)] +pub(crate) struct PermissionsApprovalRequest { + pub thread_id: ThreadId, + pub thread_label: Option, + pub call_id: String, + pub environment_id: Option, + pub reason: Option, + pub permissions: RequestPermissionProfile, +} + +#[derive(Clone, Debug)] +pub(crate) struct ApplyPatchApprovalRequest { + pub thread_id: ThreadId, + pub thread_label: Option, + pub id: String, + pub reason: Option, + pub cwd: AbsolutePathBuf, + pub changes: HashMap, +} + +#[derive(Clone, Debug)] +pub(crate) struct McpElicitationApprovalRequest { + pub thread_id: ThreadId, + pub thread_label: Option, + pub server_name: String, + pub request_id: RequestId, + pub message: String, } impl ApprovalRequest { fn thread_id(&self) -> ThreadId { match self { - ApprovalRequest::Exec { thread_id, .. } - | ApprovalRequest::Permissions { thread_id, .. } - | ApprovalRequest::ApplyPatch { thread_id, .. } - | ApprovalRequest::McpElicitation { thread_id, .. } => *thread_id, + ApprovalRequest::Exec(request) => request.thread_id, + ApprovalRequest::Permissions(request) => request.thread_id, + ApprovalRequest::ApplyPatch(request) => request.thread_id, + ApprovalRequest::McpElicitation(request) => request.thread_id, } } fn thread_label(&self) -> Option<&str> { match self { - ApprovalRequest::Exec { thread_label, .. } - | ApprovalRequest::Permissions { thread_label, .. } - | ApprovalRequest::ApplyPatch { thread_label, .. } - | ApprovalRequest::McpElicitation { thread_label, .. } => thread_label.as_deref(), + ApprovalRequest::Exec(request) => request.thread_label.as_deref(), + ApprovalRequest::Permissions(request) => request.thread_label.as_deref(), + ApprovalRequest::ApplyPatch(request) => request.thread_label.as_deref(), + ApprovalRequest::McpElicitation(request) => request.thread_label.as_deref(), } } pub(super) fn matches_resolved_request(&self, request: &ResolvedAppServerRequest) -> bool { match (self, request) { ( - ApprovalRequest::Exec { id, .. }, + ApprovalRequest::Exec(request), ResolvedAppServerRequest::ExecApproval { id: resolved_id }, - ) => id == resolved_id, + ) => request.id == *resolved_id, ( - ApprovalRequest::Permissions { call_id, .. }, + ApprovalRequest::Permissions(request), ResolvedAppServerRequest::PermissionsApproval { id }, - ) => call_id == id, + ) => request.call_id == *id, ( - ApprovalRequest::ApplyPatch { id, .. }, + ApprovalRequest::ApplyPatch(request), ResolvedAppServerRequest::FileChangeApproval { id: resolved_id }, - ) => id == resolved_id, + ) => request.id == *resolved_id, ( - ApprovalRequest::McpElicitation { - server_name, - request_id, - .. - }, + ApprovalRequest::McpElicitation(request), ResolvedAppServerRequest::McpElicitation { server_name: resolved_server_name, request_id: resolved_request_id, }, - ) => server_name == resolved_server_name && request_id == resolved_request_id, + ) => { + request.server_name == *resolved_server_name + && request.request_id == *resolved_request_id + } _ => false, } } @@ -239,19 +250,14 @@ impl ApprovalOverlay { list_keymap: &ListKeymap, ) -> (Vec, SelectionViewParams) { let (options, title) = match request { - ApprovalRequest::Exec { - available_decisions, - network_approval_context, - additional_permissions, - .. - } => ( + ApprovalRequest::Exec(request) => ( exec_options( - available_decisions, - network_approval_context.as_ref(), - additional_permissions.as_ref(), + &request.available_decisions, + request.network_approval_context.as_ref(), + request.additional_permissions.as_ref(), approval_keymap, ), - network_approval_context.as_ref().map_or_else( + request.network_approval_context.as_ref().map_or_else( || "Would you like to run the following command?".to_string(), |network_approval_context| { format!( @@ -261,17 +267,17 @@ impl ApprovalOverlay { }, ), ), - ApprovalRequest::Permissions { .. } => ( + ApprovalRequest::Permissions(_) => ( permissions_options(approval_keymap), "Would you like to grant these permissions?".to_string(), ), - ApprovalRequest::ApplyPatch { .. } => ( + ApprovalRequest::ApplyPatch(_) => ( patch_options(approval_keymap), "Would you like to make the following edits?".to_string(), ), - ApprovalRequest::McpElicitation { server_name, .. } => ( + ApprovalRequest::McpElicitation(request) => ( elicitation_options(approval_keymap), - format!("{server_name} needs your approval."), + format!("{} needs your approval.", request.server_name), ), }; @@ -310,35 +316,31 @@ impl ApprovalOverlay { }; if let Some(request) = self.current_request.as_ref() { match (request, &option.decision) { - ( - ApprovalRequest::Exec { id, command, .. }, - ApprovalDecision::Command(decision), - ) => { - self.handle_exec_decision(id, command, decision.clone()); + (ApprovalRequest::Exec(request), ApprovalDecision::Command(decision)) => { + self.handle_exec_decision(&request.id, &request.command, decision.clone()); } ( - ApprovalRequest::Permissions { - call_id, - permissions, - .. - }, + ApprovalRequest::Permissions(request), ApprovalDecision::Permissions(decision), - ) => self.handle_permissions_decision(call_id, permissions, *decision), - ( - ApprovalRequest::ApplyPatch { id, .. }, - ApprovalDecision::FileChange(decision), ) => { - self.handle_patch_decision(id, decision.clone()); + self.handle_permissions_decision( + &request.call_id, + &request.permissions, + *decision, + ); + } + (ApprovalRequest::ApplyPatch(request), ApprovalDecision::FileChange(decision)) => { + self.handle_patch_decision(&request.id, decision.clone()); } ( - ApprovalRequest::McpElicitation { - server_name, - request_id, - .. - }, + ApprovalRequest::McpElicitation(request), ApprovalDecision::McpElicitation(decision), ) => { - self.handle_elicitation_decision(server_name, request_id, *decision); + self.handle_elicitation_decision( + &request.server_name, + &request.request_id, + *decision, + ); } _ => {} } @@ -358,22 +360,22 @@ impl ApprovalOverlay { return; }; if request.thread_label().is_none() { - let subject = match request { - ApprovalRequest::Exec { - network_approval_context: Some(network_approval_context), - .. - } => history_cell::ApprovalDecisionSubject::NetworkAccess { + let network_approval_context = match request { + ApprovalRequest::Exec(request) => request.network_approval_context.as_ref(), + ApprovalRequest::Permissions(_) + | ApprovalRequest::ApplyPatch(_) + | ApprovalRequest::McpElicitation(_) => None, + }; + let subject = if let Some(network_approval_context) = network_approval_context { + history_cell::ApprovalDecisionSubject::NetworkAccess { target: network_approval_target(network_approval_context, command), - }, - _ => { - if let Some(target) = network_approval_command_target(command) { - history_cell::ApprovalDecisionSubject::NetworkAccess { - target: target.to_string(), - } - } else { - history_cell::ApprovalDecisionSubject::Command(command.to_vec()) - } } + } else if let Some(target) = network_approval_command_target(command) { + history_cell::ApprovalDecisionSubject::NetworkAccess { + target: target.to_string(), + } + } else { + history_cell::ApprovalDecisionSubject::Command(command.to_vec()) }; let cell = history_cell::new_approval_decision_cell( subject, @@ -488,35 +490,27 @@ impl ApprovalOverlay { && let Some(request) = self.current_request.as_ref() { match request { - ApprovalRequest::Exec { id, command, .. } => { + ApprovalRequest::Exec(request) => { self.handle_exec_decision( - id, - command, + &request.id, + &request.command, CommandExecutionApprovalDecision::Cancel, ); } - ApprovalRequest::Permissions { - call_id, - permissions, - .. - } => { + ApprovalRequest::Permissions(request) => { self.handle_permissions_decision( - call_id, - permissions, + &request.call_id, + &request.permissions, PermissionsDecision::Deny, ); } - ApprovalRequest::ApplyPatch { id, .. } => { - self.handle_patch_decision(id, FileChangeApprovalDecision::Cancel); + ApprovalRequest::ApplyPatch(request) => { + self.handle_patch_decision(&request.id, FileChangeApprovalDecision::Cancel); } - ApprovalRequest::McpElicitation { - server_name, - request_id, - .. - } => { + ApprovalRequest::McpElicitation(request) => { self.handle_elicitation_decision( - server_name, - request_id, + &request.server_name, + &request.request_id, McpServerElicitationAction::Cancel, ); } @@ -675,35 +669,27 @@ fn network_approval_command_target(command: &[String]) -> Option<&str> { fn build_header(request: &ApprovalRequest) -> Box { match request { - ApprovalRequest::Exec { - thread_label, - environment_id, - reason, - command, - network_approval_context, - additional_permissions, - .. - } => { + ApprovalRequest::Exec(request) => { let mut header: Vec> = Vec::new(); - if let Some(thread_label) = thread_label { + if let Some(thread_label) = &request.thread_label { header.push(Line::from(vec![ "Thread: ".into(), thread_label.clone().bold(), ])); header.push(Line::from("")); } - if let Some(environment_id) = environment_id { + if let Some(environment_id) = &request.environment_id { header.push(Line::from(vec![ "Environment: ".into(), environment_id.clone().bold(), ])); header.push(Line::from("")); } - if let Some(reason) = reason { + if let Some(reason) = &request.reason { header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()])); header.push(Line::from("")); } - if let Some(additional_permissions) = additional_permissions + if let Some(additional_permissions) = &request.additional_permissions && let Some(rule_line) = format_additional_permissions_rule(additional_permissions) { header.push(Line::from(vec![ @@ -712,43 +698,37 @@ fn build_header(request: &ApprovalRequest) -> Box { ])); header.push(Line::from("")); } - let full_cmd = strip_bash_lc_and_escape(command); + let full_cmd = strip_bash_lc_and_escape(&request.command); let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd); if let Some(first) = full_cmd_lines.first_mut() { first.spans.insert(0, Span::from("$ ")); } - if network_approval_context.is_none() { + if request.network_approval_context.is_none() { header.extend(full_cmd_lines); } Box::new(Paragraph::new(header).wrap(Wrap { trim: false })) } - ApprovalRequest::Permissions { - thread_label, - environment_id, - reason, - permissions, - .. - } => { + ApprovalRequest::Permissions(request) => { let mut header: Vec> = Vec::new(); - if let Some(thread_label) = thread_label { + if let Some(thread_label) = &request.thread_label { header.push(Line::from(vec![ "Thread: ".into(), thread_label.clone().bold(), ])); header.push(Line::from("")); } - if let Some(environment_id) = environment_id { + if let Some(environment_id) = &request.environment_id { header.push(Line::from(vec![ "Environment: ".into(), environment_id.clone().bold(), ])); header.push(Line::from("")); } - if let Some(reason) = reason { + if let Some(reason) = &request.reason { header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()])); header.push(Line::from("")); } - if let Some(rule_line) = format_requested_permissions_rule(permissions) { + if let Some(rule_line) = format_requested_permissions_rule(&request.permissions) { header.push(Line::from(vec![ "Permission rule: ".into(), rule_line.cyan(), @@ -756,19 +736,15 @@ fn build_header(request: &ApprovalRequest) -> Box { } Box::new(Paragraph::new(header).wrap(Wrap { trim: false })) } - ApprovalRequest::ApplyPatch { - thread_label, - reason, - .. - } => { + ApprovalRequest::ApplyPatch(request) => { let mut header: Vec> = Vec::new(); - if let Some(thread_label) = thread_label { + if let Some(thread_label) = &request.thread_label { header.push(Box::new(Line::from(vec![ "Thread: ".into(), thread_label.clone().bold(), ]))); } - if let Some(reason) = reason + if let Some(reason) = &request.reason && !reason.is_empty() { if !header.is_empty() { @@ -784,14 +760,9 @@ fn build_header(request: &ApprovalRequest) -> Box { } Box::new(ColumnRenderable::with(header)) } - ApprovalRequest::McpElicitation { - thread_label, - server_name, - message, - .. - } => { + ApprovalRequest::McpElicitation(request) => { let mut lines = Vec::new(); - if let Some(thread_label) = thread_label { + if let Some(thread_label) = &request.thread_label { lines.push(Line::from(vec![ "Thread: ".into(), thread_label.clone().bold(), @@ -799,9 +770,9 @@ fn build_header(request: &ApprovalRequest) -> Box { lines.push(Line::from("")); } lines.extend([ - Line::from(vec!["Server: ".into(), server_name.clone().bold()]), + Line::from(vec!["Server: ".into(), request.server_name.clone().bold()]), Line::from(""), - Line::from(message.clone()), + Line::from(request.message.clone()), ]); let header = Paragraph::new(lines).wrap(Wrap { trim: false }); Box::new(header) @@ -1226,7 +1197,7 @@ mod tests { } fn make_exec_request() -> ApprovalRequest { - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1239,11 +1210,11 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - } + }) } fn make_permissions_request() -> ApprovalRequest { - ApprovalRequest::Permissions { + ApprovalRequest::Permissions(PermissionsApprovalRequest { thread_id: ThreadId::new(), thread_label: None, call_id: "test".to_string(), @@ -1258,17 +1229,17 @@ mod tests { Some(vec![absolute_path("/tmp/out.txt")]), )), }, - } + }) } fn make_elicitation_request() -> ApprovalRequest { - ApprovalRequest::McpElicitation { + ApprovalRequest::McpElicitation(McpElicitationApprovalRequest { thread_id: ThreadId::new(), thread_label: None, server_name: "test-server".to_string(), request_id: RequestId::String("request-1".to_string()), message: "Need more information".to_string(), - } + }) } #[test] @@ -1367,7 +1338,7 @@ mod tests { let (tx, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let mut view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1380,7 +1351,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); @@ -1411,7 +1382,7 @@ mod tests { action: NetworkPolicyRuleAction::Deny, }; let mut view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1429,7 +1400,7 @@ mod tests { protocol: NetworkApprovalProtocol::Https, }), additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); @@ -1486,7 +1457,7 @@ mod tests { let tx = AppEventSender::new(tx); let thread_id = ThreadId::new(); let mut view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id, thread_label: Some("Robie [explorer]".to_string()), id: "test".to_string(), @@ -1499,7 +1470,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); @@ -1521,7 +1492,7 @@ mod tests { let mut keymap = crate::keymap::RuntimeKeymap::defaults(); keymap.approval.open_thread = vec![key_hint::plain(KeyCode::Char('x'))]; let mut view = make_overlay_with_keymap( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id, thread_label: Some("Robie [explorer]".to_string()), id: "test".to_string(), @@ -1534,7 +1505,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }, + }), tx, Features::with_defaults(), keymap.approval, @@ -1560,7 +1531,7 @@ mod tests { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: Some("Robie [explorer]".to_string()), id: "test".to_string(), @@ -1573,7 +1544,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); @@ -1589,7 +1560,7 @@ mod tests { let (tx, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let mut view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1607,7 +1578,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); @@ -1642,7 +1613,7 @@ mod tests { let (tx, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let mut view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1665,7 +1636,7 @@ mod tests { protocol: NetworkApprovalProtocol::Https, }), additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); @@ -1682,7 +1653,7 @@ mod tests { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let command = vec!["echo".into(), "hello".into(), "world".into()]; - let exec_request = ApprovalRequest::Exec { + let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -1695,7 +1666,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }; + }); let view = make_overlay(exec_request, tx, Features::with_defaults()); let mut buf = Buffer::empty(Rect::new(0, 0, 80, view.desired_height(/*width*/ 80))); @@ -1983,7 +1954,7 @@ mod tests { fn additional_permissions_prompt_shows_permission_rule_line() { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); - let exec_request = ApprovalRequest::Exec { + let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2007,7 +1978,7 @@ mod tests { .into(), ), }), - }; + }); let view = make_overlay(exec_request, tx, Features::with_defaults()); let mut buf = Buffer::empty(Rect::new(0, 0, 100, view.desired_height(/*width*/ 100))); @@ -2040,7 +2011,7 @@ mod tests { fn additional_permissions_prompt_snapshot() { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); - let exec_request = ApprovalRequest::Exec { + let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2064,7 +2035,7 @@ mod tests { .into(), ), }), - }; + }); let view = make_overlay(exec_request, tx, Features::with_defaults()); assert_snapshot!( @@ -2095,14 +2066,14 @@ mod tests { content: "one\ntwo\nthree\n".to_string(), }, ); - let request = ApprovalRequest::ApplyPatch { + let request = ApprovalRequest::ApplyPatch(ApplyPatchApprovalRequest { thread_id: ThreadId::new(), thread_label: Some("Banach [worker]".to_string()), id: "test".to_string(), reason: None, cwd: absolute_path("/tmp"), changes, - }; + }); let keymap = crate::keymap::RuntimeKeymap::defaults(); let view = ApprovalOverlay::new( request, @@ -2121,7 +2092,7 @@ mod tests { fn network_exec_prompt_title_includes_host() { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); - let exec_request = ApprovalRequest::Exec { + let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2144,7 +2115,7 @@ mod tests { protocol: NetworkApprovalProtocol::Https, }), additional_permissions: None, - }; + }); let view = make_overlay(exec_request, tx, Features::with_defaults()); let mut buf = Buffer::empty(Rect::new(0, 0, 100, view.desired_height(/*width*/ 100))); @@ -2258,7 +2229,7 @@ mod tests { let (tx_raw, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut view = make_overlay( - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2274,7 +2245,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - }, + }), tx, Features::with_defaults(), ); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 73046a41d8..f8f2936d1e 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -66,8 +66,12 @@ pub(crate) use app_link_view::AppLinkElicitationTarget; pub(crate) use app_link_view::AppLinkSuggestionType; pub(crate) use app_link_view::AppLinkView; pub(crate) use app_link_view::AppLinkViewParams; +pub(crate) use approval_overlay::ApplyPatchApprovalRequest; pub(crate) use approval_overlay::ApprovalOverlay; pub(crate) use approval_overlay::ApprovalRequest; +pub(crate) use approval_overlay::ExecApprovalRequest; +pub(crate) use approval_overlay::McpElicitationApprovalRequest; +pub(crate) use approval_overlay::PermissionsApprovalRequest; pub(crate) use approval_overlay::format_requested_permissions_rule; pub(crate) use mcp_server_elicitation::McpServerElicitationFormRequest; pub(crate) use mcp_server_elicitation::McpServerElicitationOverlay; @@ -1906,7 +1910,7 @@ mod tests { } fn exec_request() -> ApprovalRequest { - ApprovalRequest::Exec { + ApprovalRequest::Exec(ExecApprovalRequest { thread_id: codex_protocol::ThreadId::new(), thread_label: None, id: "1".to_string(), @@ -1919,7 +1923,7 @@ mod tests { ], network_approval_context: None, additional_permissions: None, - } + }) } #[derive(Default)] diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 424e8310f8..becd52eaf1 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -263,6 +263,7 @@ use crate::app_event::WindowsSandboxEnableMode; use crate::app_event_sender::AppEventSender; use crate::auto_review_denials; use crate::auto_review_denials::RecentAutoReviewDenials; +use crate::bottom_pane::ApplyPatchApprovalRequest; use crate::bottom_pane::ApprovalRequest; use crate::bottom_pane::BottomPane; use crate::bottom_pane::BottomPaneParams; @@ -270,15 +271,18 @@ use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::CollaborationModeIndicator; use crate::bottom_pane::ColumnWidthMode; use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED; +use crate::bottom_pane::ExecApprovalRequest; use crate::bottom_pane::ExperimentalFeatureItem; use crate::bottom_pane::ExperimentalFeaturesView; use crate::bottom_pane::GoalStatusIndicator; use crate::bottom_pane::HistoryEntry; use crate::bottom_pane::InputResult; use crate::bottom_pane::LocalImageAttachment; +use crate::bottom_pane::McpElicitationApprovalRequest; use crate::bottom_pane::McpServerElicitationFormRequest; use crate::bottom_pane::MemoriesSettingsView; use crate::bottom_pane::MentionBinding; +use crate::bottom_pane::PermissionsApprovalRequest; use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT; use crate::bottom_pane::QueuedInputAction; use crate::bottom_pane::SelectionAction; diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index babfd3fb03..2afe8265f1 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -287,7 +287,7 @@ impl ChatWidget { self.notify(Notification::ExecApprovalRequested { command }); let available_decisions = ev.effective_available_decisions(); - let request = ApprovalRequest::Exec { + let request = ApprovalRequest::Exec(ExecApprovalRequest { thread_id: self.thread_id.unwrap_or_default(), thread_label: None, id: ev.effective_approval_id(), @@ -297,7 +297,7 @@ impl ChatWidget { available_decisions, network_approval_context: ev.network_approval_context, additional_permissions: ev.additional_permissions, - }; + }); self.bottom_pane .push_approval_request(request, &self.config.features); self.set_ambient_pet_notification( @@ -310,14 +310,14 @@ impl ChatWidget { pub(crate) fn handle_apply_patch_approval_now(&mut self, ev: ApplyPatchApprovalRequestEvent) { self.flush_answer_stream_with_separator(); - let request = ApprovalRequest::ApplyPatch { + let request = ApprovalRequest::ApplyPatch(ApplyPatchApprovalRequest { thread_id: self.thread_id.unwrap_or_default(), thread_label: None, id: ev.call_id, reason: ev.reason, changes: ev.changes.clone(), cwd: self.config.cwd.clone(), - }; + }); self.bottom_pane .push_approval_request(request, &self.config.features); self.set_ambient_pet_notification( @@ -361,13 +361,13 @@ impl ChatWidget { } else { match params.request { McpServerElicitationRequest::Form { message, .. } => { - let request = ApprovalRequest::McpElicitation { + let request = ApprovalRequest::McpElicitation(McpElicitationApprovalRequest { thread_id, thread_label: None, server_name: params.server_name, request_id, message, - }; + }); self.bottom_pane .push_approval_request(request, &self.config.features); } @@ -434,14 +434,14 @@ impl ChatWidget { pub(crate) fn handle_request_permissions_now(&mut self, ev: RequestPermissionsEvent) { self.flush_answer_stream_with_separator(); - let request = ApprovalRequest::Permissions { + let request = ApprovalRequest::Permissions(PermissionsApprovalRequest { thread_id: self.thread_id.unwrap_or_default(), thread_label: None, call_id: ev.call_id, environment_id: ev.environment_id, reason: ev.reason, permissions: ev.permissions, - }; + }); self.bottom_pane .push_approval_request(request, &self.config.features); self.set_ambient_pet_notification(