From 1fa5c7771c1fd4403394e45f22ab925b89c0ad6c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 25 Feb 2026 01:27:37 -0800 Subject: [PATCH 1/2] feat: record whether a skill script is approved for the session --- codex-rs/core/src/codex.rs | 3 ++ codex-rs/core/src/state/service.rs | 4 ++ .../tools/runtimes/shell/unix_escalation.rs | 52 +++++++++++++++++-- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0fe5f28523..9d31184f72 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1348,6 +1348,7 @@ impl Session { otel_manager, models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), + execve_session_approvals: RwLock::new(HashSet::new()), skills_manager, file_watcher, agent_control, @@ -8221,6 +8222,7 @@ mod tests { otel_manager: otel_manager.clone(), models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), + execve_session_approvals: RwLock::new(HashSet::new()), skills_manager, file_watcher, agent_control, @@ -8377,6 +8379,7 @@ mod tests { otel_manager: otel_manager.clone(), models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), + execve_session_approvals: RwLock::new(HashSet::new()), skills_manager, file_watcher, agent_control, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 2a541d38a9..0ec03b139e 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::sync::Arc; use crate::AuthManager; @@ -17,6 +18,7 @@ use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; use codex_hooks::Hooks; use codex_otel::OtelManager; +use codex_utils_absolute_path::AbsolutePathBuf; use std::path::PathBuf; use tokio::sync::Mutex; use tokio::sync::RwLock; @@ -42,6 +44,8 @@ pub(crate) struct SessionServices { pub(crate) models_manager: Arc, pub(crate) otel_manager: OtelManager, pub(crate) tool_approvals: Mutex, + #[cfg_attr(not(unix), allow(dead_code))] + pub(crate) execve_session_approvals: RwLock>, pub(crate) skills_manager: Arc, pub(crate) file_watcher: Arc, pub(crate) agent_control: AgentControl, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 6e8bd138fe..7485825ab1 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -166,6 +166,13 @@ struct CoreShellActionProvider { stopwatch: Stopwatch, } +enum DecisionSource { + SkillScript, + PrefixRule, + /// Often, this is `is_safe_command()`. + UnmatchedCommandFallback, +} + impl CoreShellActionProvider { fn decision_driven_by_policy(matched_rules: &[RuleMatch], decision: Decision) -> bool { matched_rules.iter().any(|rule_match| { @@ -233,6 +240,7 @@ impl CoreShellActionProvider { None } + #[allow(clippy::too_many_arguments)] async fn process_decision( &self, decision: Decision, @@ -241,6 +249,7 @@ impl CoreShellActionProvider { argv: &[String], workdir: &AbsolutePathBuf, additional_permissions: Option, + decision_source: DecisionSource, ) -> anyhow::Result { let action = match decision { Decision::Forbidden => EscalateAction::Deny { @@ -267,8 +276,26 @@ impl CoreShellActionProvider { .await? { ReviewDecision::Approved - | ReviewDecision::ApprovedExecpolicyAmendment { .. } - | ReviewDecision::ApprovedForSession => { + | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + if needs_escalation { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ReviewDecision::ApprovedForSession => { + // Currently, we only add session approvals for + // skill scripts because we are storing only the + // `program` whereas prefix rules may be restricted by a longer prefix. + if matches!(decision_source, DecisionSource::SkillScript) { + self.session + .services + .execve_session_approvals + .write() + .await + .insert(program.clone()); + } + if needs_escalation { EscalateAction::Escalate } else { @@ -337,14 +364,27 @@ impl EscalationPolicy for CoreShellActionProvider { // EscalateAction::Run case, rather than always escalating when a // skill matches. let needs_escalation = true; + let is_approved_for_session = self + .session + .services + .execve_session_approvals + .read() + .await + .contains(program); + let decision = if is_approved_for_session { + Decision::Allow + } else { + Decision::Prompt + }; return self .process_decision( - Decision::Prompt, + decision, needs_escalation, program, argv, workdir, skill.permission_profile.clone(), + DecisionSource::SkillScript, ) .await; } @@ -379,6 +419,11 @@ impl EscalationPolicy for CoreShellActionProvider { let needs_escalation = self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy; + let decision_source = if decision_driven_by_policy { + DecisionSource::PrefixRule + } else { + DecisionSource::UnmatchedCommandFallback + }; self.process_decision( evaluation.decision, needs_escalation, @@ -386,6 +431,7 @@ impl EscalationPolicy for CoreShellActionProvider { argv, workdir, None, + decision_source, ) .await } From a2c5b83e7711e371b95145d5ec7043ab5cf21645 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 25 Feb 2026 01:43:11 -0800 Subject: [PATCH 2/2] feat: include availableDecisions with command approvals --- .../src/protocol/common.rs | 1 + .../app-server-protocol/src/protocol/v2.rs | 29 ++- codex-rs/app-server-test-client/src/lib.rs | 4 + codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 6 + codex-rs/app-server/src/transport.rs | 2 + codex-rs/core/src/codex.rs | 38 +++ codex-rs/core/src/codex_delegate.rs | 4 +- codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/protocol/src/approvals.rs | 54 ++++ codex-rs/protocol/src/protocol.rs | 4 +- .../tui/src/app/pending_interactive_replay.rs | 2 + .../tui/src/bottom_pane/approval_overlay.rs | 243 +++++++++++------- codex-rs/tui/src/bottom_pane/mod.rs | 4 + codex-rs/tui/src/chatwidget.rs | 1 + codex-rs/tui/src/chatwidget/tests.rs | 8 + 16 files changed, 305 insertions(+), 98 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index b3d3b4ae7c..6d850d1282 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1396,6 +1396,7 @@ mod tests { }), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + available_decisions: None, }; let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms); assert_eq!( diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index d17b0772a1..867d6f455f 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -47,6 +47,7 @@ use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::protocol::ReadOnlyAccess as CoreReadOnlyAccess; use codex_protocol::protocol::RejectConfig as CoreRejectConfig; +use codex_protocol::protocol::ReviewDecision as CoreReviewDecision; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillDependencies as CoreSkillDependencies; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; @@ -681,7 +682,8 @@ pub struct ConfigEdit { pub enum CommandExecutionApprovalDecision { /// User approved the command. Accept, - /// User approved the command and future identical commands should run without prompting. + /// User approved the command and future prompts in the same session-scoped + /// approval cache should run without prompting. AcceptForSession, /// User approved the command, and wants to apply the proposed execpolicy amendment so future /// matching commands can run without prompting. @@ -698,6 +700,27 @@ pub enum CommandExecutionApprovalDecision { Cancel, } +impl From for CommandExecutionApprovalDecision { + fn from(value: CoreReviewDecision) -> Self { + match value { + CoreReviewDecision::Approved => Self::Accept, + CoreReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment, + } => Self::AcceptWithExecpolicyAmendment { + execpolicy_amendment: proposed_execpolicy_amendment.into(), + }, + CoreReviewDecision::ApprovedForSession => Self::AcceptForSession, + CoreReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => Self::ApplyNetworkPolicyAmendment { + network_policy_amendment: network_policy_amendment.into(), + }, + CoreReviewDecision::Denied => Self::Decline, + CoreReviewDecision::Abort => Self::Cancel, + } + } +} + v2_enum_from_core! { pub enum NetworkApprovalProtocol from CoreNetworkApprovalProtocol { Http, @@ -3509,6 +3532,10 @@ pub struct CommandExecutionRequestApprovalParams { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub proposed_network_policy_amendments: Option>, + /// Ordered list of decisions the client may present for this prompt. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional = nullable)] + pub available_decisions: Option>, } impl CommandExecutionRequestApprovalParams { diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index ae147d5b4e..688a2da88e 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1539,6 +1539,7 @@ impl CodexClient { additional_permissions, proposed_execpolicy_amendment, proposed_network_policy_amendments, + available_decisions, } = params; println!( @@ -1553,6 +1554,9 @@ impl CodexClient { if let Some(network_approval_context) = network_approval_context.as_ref() { println!("< network approval context: {network_approval_context:?}"); } + if let Some(available_decisions) = available_decisions.as_ref() { + println!("< available decisions: {available_decisions:?}"); + } if let Some(command) = command.as_deref() { println!("< command: {command}"); } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index df026596fa..070eed68dc 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -667,7 +667,7 @@ Certain actions (shell commands or modifying files) may require explicit user ap Order of messages: 1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action. -2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. +2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted. 3. Client response — for example `{ "decision": "accept" }`, `{ "decision": "acceptForSession" }`, `{ "decision": { "acceptWithExecpolicyAmendment": { "execpolicy_amendment": [...] } } }`, `{ "decision": { "applyNetworkPolicyAmendment": { "network_policy_amendment": { "host": "example.com", "action": "allow" } } } }`, `{ "decision": "decline" }`, or `{ "decision": "cancel" }`. 4. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 0b81a4c898..93117c127f 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -261,6 +261,11 @@ pub(crate) async fn apply_bespoke_event_handling( .note_permission_requested(&conversation_id.to_string()) .await; let approval_id_for_op = ev.effective_approval_id(); + let available_decisions = ev + .effective_available_decisions() + .into_iter() + .map(CommandExecutionApprovalDecision::from) + .collect::>(); let ExecApprovalRequestEvent { call_id, approval_id, @@ -357,6 +362,7 @@ pub(crate) async fn apply_bespoke_event_handling( additional_permissions, proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2, proposed_network_policy_amendments: proposed_network_policy_amendments_v2, + available_decisions: Some(available_decisions), }; let rx = outgoing .send_request(ServerRequestPayload::CommandExecutionRequestApproval( diff --git a/codex-rs/app-server/src/transport.rs b/codex-rs/app-server/src/transport.rs index 8c36df9822..6ac3e0d466 100644 --- a/codex-rs/app-server/src/transport.rs +++ b/codex-rs/app-server/src/transport.rs @@ -986,6 +986,7 @@ mod tests { ), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + available_decisions: None, }, }), }, @@ -1047,6 +1048,7 @@ mod tests { ), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + available_decisions: None, }, }), }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 9d31184f72..edafb082c0 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2573,6 +2573,35 @@ impl Session { network_approval_context: Option, proposed_execpolicy_amendment: Option, additional_permissions: Option, + ) -> ReviewDecision { + self.request_command_approval_with_options( + turn_context, + call_id, + approval_id, + command, + cwd, + reason, + network_approval_context, + proposed_execpolicy_amendment, + additional_permissions, + None, + ) + .await + } + + #[allow(clippy::too_many_arguments)] + pub async fn request_command_approval_with_options( + &self, + turn_context: &TurnContext, + call_id: String, + approval_id: Option, + command: Vec, + cwd: PathBuf, + reason: Option, + network_approval_context: Option, + proposed_execpolicy_amendment: Option, + additional_permissions: Option, + available_decisions: Option>, ) -> ReviewDecision { // command-level approvals use `call_id`. // `approval_id` is only present for subcommand callbacks (execve intercept) @@ -2606,6 +2635,14 @@ impl Session { }, ] }); + let available_decisions = available_decisions.unwrap_or_else(|| { + ExecApprovalRequestEvent::default_available_decisions( + network_approval_context.as_ref(), + proposed_execpolicy_amendment.as_ref(), + proposed_network_policy_amendments.as_deref(), + additional_permissions.as_ref(), + ) + }); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { call_id, approval_id, @@ -2617,6 +2654,7 @@ impl Session { proposed_execpolicy_amendment, proposed_network_policy_amendments, additional_permissions, + available_decisions: Some(available_decisions), parsed_cmd, }); self.send_event(turn_context, event).await; diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 9662830c8c..ec5596f5f1 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -321,10 +321,11 @@ async fn handle_exec_approval( network_approval_context, proposed_execpolicy_amendment, additional_permissions, + available_decisions, .. } = event; // Race approval with cancellation and timeout to avoid hangs. - let approval_fut = parent_session.request_command_approval( + let approval_fut = parent_session.request_command_approval_with_options( parent_ctx, call_id, approval_id, @@ -334,6 +335,7 @@ async fn handle_exec_approval( network_approval_context, proposed_execpolicy_amendment, additional_permissions, + available_decisions, ); let decision = await_approval_with_cancel( approval_fut, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index eff8d8e13e..7ec785ef7e 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -227,6 +227,7 @@ async fn run_codex_tool_session_inner( parsed_cmd, network_approval_context: _, additional_permissions: _, + available_decisions: _, } = ev; handle_exec_approval_request( command, diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index f742dc6328..49c0186e78 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -5,6 +5,7 @@ use crate::mcp::RequestId; use crate::models::PermissionProfile; use crate::parse_command::ParsedCommand; use crate::protocol::FileChange; +use crate::protocol::ReviewDecision; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -107,6 +108,13 @@ pub struct ExecApprovalRequestEvent { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub additional_permissions: Option, + /// Ordered list of decisions the client may present for this prompt. + /// + /// When absent, clients should derive the legacy default set from the + /// other fields on this request. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub available_decisions: Option>, pub parsed_cmd: Vec, } @@ -116,6 +124,52 @@ impl ExecApprovalRequestEvent { .clone() .unwrap_or_else(|| self.call_id.clone()) } + + pub fn default_available_decisions( + network_approval_context: Option<&NetworkApprovalContext>, + proposed_execpolicy_amendment: Option<&ExecPolicyAmendment>, + proposed_network_policy_amendments: Option<&[NetworkPolicyAmendment]>, + additional_permissions: Option<&PermissionProfile>, + ) -> Vec { + if network_approval_context.is_some() { + let mut decisions = vec![ReviewDecision::Approved, ReviewDecision::ApprovedForSession]; + if let Some(amendment) = proposed_network_policy_amendments.and_then(|amendments| { + amendments + .iter() + .find(|amendment| amendment.action == NetworkPolicyRuleAction::Allow) + }) { + decisions.push(ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: amendment.clone(), + }); + } + decisions.push(ReviewDecision::Abort); + return decisions; + } + + if additional_permissions.is_some() { + return vec![ReviewDecision::Approved, ReviewDecision::Abort]; + } + + let mut decisions = vec![ReviewDecision::Approved]; + if let Some(prefix) = proposed_execpolicy_amendment { + decisions.push(ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: prefix.clone(), + }); + } + decisions.push(ReviewDecision::Abort); + decisions + } + + pub fn effective_available_decisions(&self) -> Vec { + self.available_decisions.clone().unwrap_or_else(|| { + Self::default_available_decisions( + self.network_approval_context.as_ref(), + self.proposed_execpolicy_amendment.as_ref(), + self.proposed_network_policy_amendments.as_deref(), + self.additional_permissions.as_ref(), + ) + }) + } } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index a8770cc90a..4df089e30e 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -2765,8 +2765,8 @@ pub enum ReviewDecision { proposed_execpolicy_amendment: ExecPolicyAmendment, }, - /// User has approved this command and wants to automatically approve any - /// future identical instances (`command` and `cwd` match exactly) for the + /// User has approved this request and wants future prompts in the same + /// session-scoped approval cache to be automatically approved for the /// remainder of the session. ApprovedForSession, diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 14d9f13c77..63f605e9fe 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -382,6 +382,7 @@ mod tests { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: Vec::new(), }, ), @@ -524,6 +525,7 @@ mod tests { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: Vec::new(), }, ), diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index fd58baef46..80dbf74524 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -46,6 +46,7 @@ pub(crate) enum ApprovalRequest { id: String, command: Vec, reason: Option, + available_decisions: Vec, network_approval_context: Option, proposed_execpolicy_amendment: Option, proposed_network_policy_amendments: Option>, @@ -115,15 +116,13 @@ impl ApprovalOverlay { ) -> (Vec, SelectionViewParams) { let (options, title) = match &variant { ApprovalVariant::Exec { + available_decisions, network_approval_context, - proposed_execpolicy_amendment, - proposed_network_policy_amendments, additional_permissions, .. } => ( exec_options( - proposed_execpolicy_amendment.clone(), - proposed_network_policy_amendments.clone(), + available_decisions, network_approval_context.as_ref(), additional_permissions.as_ref(), ), @@ -365,6 +364,7 @@ impl From for ApprovalRequestState { id, command, reason, + available_decisions, network_approval_context, proposed_execpolicy_amendment, proposed_network_policy_amendments, @@ -397,6 +397,7 @@ impl From for ApprovalRequestState { variant: ApprovalVariant::Exec { id, command, + available_decisions, network_approval_context, proposed_execpolicy_amendment, proposed_network_policy_amendments, @@ -455,6 +456,7 @@ enum ApprovalVariant { Exec { id: String, command: Vec, + available_decisions: Vec, network_approval_context: Option, proposed_execpolicy_amendment: Option, proposed_network_policy_amendments: Option>, @@ -492,100 +494,93 @@ impl ApprovalOption { } fn exec_options( - proposed_execpolicy_amendment: Option, - proposed_network_policy_amendments: Option>, + available_decisions: &[ReviewDecision], network_approval_context: Option<&NetworkApprovalContext>, additional_permissions: Option<&PermissionProfile>, ) -> Vec { - if network_approval_context.is_some() { - let mut options = vec![ - ApprovalOption { - label: "Yes, just this once".to_string(), + available_decisions + .iter() + .filter_map(|decision| match decision { + ReviewDecision::Approved => Some(ApprovalOption { + label: if network_approval_context.is_some() { + "Yes, just this once".to_string() + } else { + "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 allow this host for this conversation".to_string(), + }), + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment, + } => { + let rendered_prefix = + strip_bash_lc_and_escape(proposed_execpolicy_amendment.command()); + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } + + Some(ApprovalOption { + label: format!( + "Yes, and don't ask again for commands that start with `{rendered_prefix}`" + ), + decision: ApprovalDecision::Review( + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: proposed_execpolicy_amendment.clone(), + }, + ), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + }) + } + ReviewDecision::ApprovedForSession => Some(ApprovalOption { + label: if network_approval_context.is_some() { + "Yes, and allow this host for this conversation".to_string() + } else if additional_permissions.is_some() { + "Yes, and allow these permissions for this session".to_string() + } else { + "Yes, and don't ask again for this command in this session".to_string() + }, decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], - }, - ]; - for amendment in proposed_network_policy_amendments.unwrap_or_default() { - let (label, shortcut) = match amendment.action { - NetworkPolicyRuleAction::Allow => ( - "Yes, and allow this host in the future".to_string(), - KeyCode::Char('p'), - ), - NetworkPolicyRuleAction::Deny => continue, - }; - options.push(ApprovalOption { - label, - decision: ApprovalDecision::Review(ReviewDecision::NetworkPolicyAmendment { - network_policy_amendment: amendment, - }), + }), + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => { + let (label, shortcut) = match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => ( + "Yes, and allow this host in the future".to_string(), + KeyCode::Char('p'), + ), + NetworkPolicyRuleAction::Deny => ( + "No, and block this host in the future".to_string(), + KeyCode::Char('d'), + ), + }; + Some(ApprovalOption { + label, + decision: ApprovalDecision::Review(ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: network_policy_amendment.clone(), + }), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(shortcut)], + }) + } + ReviewDecision::Denied => Some(ApprovalOption { + label: "No, continue without running it".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Denied), display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(shortcut)], - }); - } - options.push(ApprovalOption { - label: "No, and tell Codex what to do differently".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Abort), - display_shortcut: Some(key_hint::plain(KeyCode::Esc)), - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }); - return options; - } - - if additional_permissions.is_some() { - return 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 { + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('d'))], + }), + ReviewDecision::Abort => Some(ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Abort), display_shortcut: Some(key_hint::plain(KeyCode::Esc)), additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }, - ]; - } - - vec![ApprovalOption { - label: "Yes, proceed".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Approved), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }] - .into_iter() - .chain(proposed_execpolicy_amendment.and_then(|prefix| { - let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); - if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { - return None; - } - - Some(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), - display_shortcut: Some(key_hint::plain(KeyCode::Esc)), - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }]) - .collect() + .collect() } fn format_additional_permissions_rule( @@ -695,6 +690,7 @@ mod tests { id: "test".to_string(), command: vec!["echo".to_string(), "hi".to_string()], reason: Some("reason".to_string()), + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, @@ -740,6 +736,15 @@ mod tests { id: "test".to_string(), command: vec!["echo".to_string()], reason: None, + available_decisions: vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: ExecPolicyAmendment::new(vec![ + "echo".to_string(), + ]), + }, + ReviewDecision::Abort, + ], network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "echo".to_string(), @@ -781,6 +786,17 @@ mod tests { id: "test".to_string(), command: vec!["curl".to_string(), "https://example.com".to_string()], reason: None, + available_decisions: vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + }, + ReviewDecision::Abort, + ], network_approval_context: Some(NetworkApprovalContext { host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, @@ -818,6 +834,7 @@ mod tests { id: "test".into(), command, reason: None, + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, @@ -850,17 +867,17 @@ mod tests { protocol: NetworkApprovalProtocol::Https, }; let options = exec_options( - Some(ExecPolicyAmendment::new(vec!["curl".to_string()])), - Some(vec![ - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Allow, + &[ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, }, - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Deny, - }, - ]), + ReviewDecision::Abort, + ], Some(&network_context), None, ); @@ -877,6 +894,29 @@ mod tests { ); } + #[test] + fn generic_exec_options_can_offer_allow_for_session() { + let options = exec_options( + &[ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::Abort, + ], + None, + None, + ); + + let labels: Vec = options.into_iter().map(|option| option.label).collect(); + assert_eq!( + labels, + vec![ + "Yes, proceed".to_string(), + "Yes, and don't ask again for this command in this session".to_string(), + "No, and tell Codex what to do differently".to_string(), + ] + ); + } + #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { let additional_permissions = PermissionProfile { @@ -886,7 +926,11 @@ mod tests { }), ..Default::default() }; - let options = exec_options(None, None, None, Some(&additional_permissions)); + let options = exec_options( + &[ReviewDecision::Approved, ReviewDecision::Abort], + None, + Some(&additional_permissions), + ); let labels: Vec = options.into_iter().map(|option| option.label).collect(); assert_eq!( @@ -906,6 +950,7 @@ mod tests { id: "test".into(), command: vec!["cat".into(), "/tmp/readme.txt".into()], reason: None, + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, @@ -946,6 +991,7 @@ mod tests { id: "test".into(), command: vec!["cat".into(), "/tmp/readme.txt".into()], reason: Some("need filesystem access".into()), + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, @@ -973,6 +1019,17 @@ mod tests { id: "test".into(), command: vec!["curl".into(), "https://example.com".into()], reason: Some("network request blocked".into()), + available_decisions: vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + }, + ReviewDecision::Abort, + ], network_approval_context: Some(NetworkApprovalContext { host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e4c40dd45e..473f500e88 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1094,6 +1094,10 @@ mod tests { id: "1".to_string(), command: vec!["echo".into(), "ok".into()], reason: None, + available_decisions: vec![ + codex_protocol::protocol::ReviewDecision::Approved, + codex_protocol::protocol::ReviewDecision::Abort, + ], network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b182d814dc..1aa9b37d91 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2572,6 +2572,7 @@ impl ChatWidget { id: ev.effective_approval_id(), command: ev.command, reason: ev.reason, + available_decisions: ev.effective_available_decisions(), network_approval_context: ev.network_approval_context, proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment, proposed_network_policy_amendments: ev.proposed_network_policy_amendments, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 358a9615c2..3cb5afc23b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2783,6 +2783,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2832,6 +2833,7 @@ async fn exec_approval_uses_approval_id_when_present() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }), }); @@ -2868,6 +2870,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2922,6 +2925,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6717,6 +6721,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { ])), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6777,6 +6782,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { ])), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6824,6 +6830,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -7190,6 +7197,7 @@ async fn status_widget_and_approval_modal_snapshot() { ])), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event {