diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 3eb0519761..12c5d1497a 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -950,6 +950,7 @@ async fn on_exec_approval_response( .submit(Op::ExecApproval { id: event_turn_id, decision: response.decision, + allow_prefix: None, }) .await { @@ -1154,6 +1155,7 @@ async fn on_command_execution_request_approval_response( .submit(Op::ExecApproval { id: event_turn_id, decision, + allow_prefix: None, }) .await { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index e83deba8d1..ee7aa738a8 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -50,6 +50,7 @@ use mcp_types::RequestId; use serde_json; use serde_json::Value; use tokio::sync::Mutex; +use tokio::sync::OwnedRwLockReadGuard; use tokio::sync::RwLock; use tokio::sync::oneshot; use tokio_util::sync::CancellationToken; @@ -894,6 +895,14 @@ impl Session { Ok(()) } + pub(crate) async fn current_exec_policy(&self) -> OwnedRwLockReadGuard { + let exec_policy = { + let state = self.state.lock().await; + state.session_configuration.exec_policy.clone() + }; + exec_policy.read_owned().await + } + /// Emit an exec approval request event and await the user's decision. /// /// The request is keyed by `sub_id`/`call_id` so matching responses are delivered diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 0e1140febd..e4cf077785 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -285,7 +285,13 @@ async fn handle_exec_approval( ) .await; - let _ = codex.submit(Op::ExecApproval { id, decision }).await; + let _ = codex + .submit(Op::ExecApproval { + id, + decision, + allow_prefix: None, + }) + .await; } /// Handle an ApplyPatchApprovalRequest by consulting the parent session and replying. diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 44328de14a..70a835043c 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -10,6 +10,7 @@ use codex_execpolicy::Error as ExecPolicyRuleError; use codex_execpolicy::Evaluation; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; +use codex_execpolicy::RuleMatch; use codex_execpolicy::blocking_append_allow_prefix_rule; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; @@ -24,6 +25,8 @@ use crate::sandboxing::SandboxPermissions; use crate::tools::sandboxing::ApprovalRequirement; const FORBIDDEN_REASON: &str = "execpolicy forbids this command"; +const PROMPT_CONFLICT_REASON: &str = + "execpolicy requires approval for this command, but AskForApproval is set to Never"; const PROMPT_REASON: &str = "execpolicy requires approval for this command"; const POLICY_DIR_NAME: &str = "policy"; const POLICY_EXTENSION: &str = "codexpolicy"; @@ -129,49 +132,36 @@ pub(crate) async fn append_allow_prefix_rule_and_update( Ok(()) } -fn requirement_from_decision( - decision: Decision, - approval_policy: AskForApproval, -) -> ApprovalRequirement { - match decision { - Decision::Forbidden => ApprovalRequirement::Forbidden { - reason: FORBIDDEN_REASON.to_string(), - }, - Decision::Prompt => { - let reason = PROMPT_REASON.to_string(); - if matches!(approval_policy, AskForApproval::Never) { - ApprovalRequirement::Forbidden { reason } - } else { - ApprovalRequirement::NeedsApproval { - reason: Some(reason), - allow_prefix: None, +/// Return an allow-prefix option when a command needs approval and execpolicy did not drive the decision +fn allow_prefix_if_applicable(evaluation: &Evaluation, features: &Features) -> Option> { + if !features.enabled(Feature::ExecPolicy) { + return None; + } + + if evaluation.decision != Decision::Prompt { + return None; + } + + let mut first_prompt_from_heuristics: Option> = None; + for rule_match in &evaluation.matched_rules { + match rule_match { + RuleMatch::HeuristicsRuleMatch { command, decision } => { + if *decision == Decision::Prompt && first_prompt_from_heuristics.is_none() { + first_prompt_from_heuristics = Some(command.clone()); } } + _ if rule_match.decision() == Decision::Prompt => { + return None; + } + _ => {} } - Decision::Allow => ApprovalRequirement::Skip { - bypass_sandbox: true, - }, } + + first_prompt_from_heuristics } -/// Return an allow-prefix option when a single plain command needs approval without -/// any matching policy rule. We only surface the prefix opt-in when execpolicy did -/// not already drive the decision (NoMatch) and when the command is a single -/// unrolled command (multi-part scripts shouldn’t be whitelisted via prefix) and -/// when execpolicy feature is enabled. -fn allow_prefix_if_applicable( - commands: &[Vec], - features: &Features, -) -> Option> { - if features.enabled(Feature::ExecPolicy) && commands.len() == 1 { - Some(commands[0].clone()) - } else { - None - } -} - -pub(crate) async fn create_approval_requirement_for_command( - exec_policy: &Arc>, +pub(crate) fn create_approval_requirement_for_command( + policy: &Policy, features: &Features, command: &[String], approval_policy: AskForApproval, @@ -179,31 +169,55 @@ pub(crate) async fn create_approval_requirement_for_command( sandbox_permissions: SandboxPermissions, ) -> ApprovalRequirement { let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); - let policy = exec_policy.read().await; - let evaluation = policy.check_multiple(commands.iter()); + let heuristics_fallback = |cmd: &[String]| { + if requires_initial_appoval(approval_policy, sandbox_policy, cmd, sandbox_permissions) { + Decision::Prompt + } else { + Decision::Allow + } + }; + let evaluation = policy.check_multiple(commands.iter(), &heuristics_fallback); + let has_policy_allow = evaluation.matched_rules.iter().any(|rule_match| { + !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. }) + && rule_match.decision() == Decision::Allow + }); - match evaluation { - Evaluation::Match { decision, .. } => requirement_from_decision(decision, approval_policy), - Evaluation::NoMatch { .. } => { - if requires_initial_appoval( - approval_policy, - sandbox_policy, - command, - sandbox_permissions, - ) { - ApprovalRequirement::NeedsApproval { - reason: None, - allow_prefix: allow_prefix_if_applicable(&commands, features), + match evaluation.decision { + Decision::Forbidden => ApprovalRequirement::Forbidden { + reason: FORBIDDEN_REASON.to_string(), + }, + Decision::Prompt => { + let prompt_reason = derive_prompt_reason(&evaluation); + if matches!(approval_policy, AskForApproval::Never) { + ApprovalRequirement::Forbidden { + reason: PROMPT_CONFLICT_REASON.to_string(), } } else { - ApprovalRequirement::Skip { - bypass_sandbox: false, + ApprovalRequirement::NeedsApproval { + reason: prompt_reason, + allow_prefix: allow_prefix_if_applicable(&evaluation, features), } } } + Decision::Allow => ApprovalRequirement::Skip { + bypass_sandbox: has_policy_allow, + }, } } +/// Only return PROMPT_REASON when an execpolicy rule drove the prompt decision +fn derive_prompt_reason(evaluation: &Evaluation) -> Option { + evaluation.matched_rules.iter().find_map(|rule_match| { + if !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. }) + && rule_match.decision() == Decision::Prompt + { + Some(PROMPT_REASON.to_string()) + } else { + None + } + }) +} + async fn collect_policy_files(dir: &Path) -> Result, ExecPolicyError> { let mut read_dir = match fs::read_dir(dir).await { Ok(read_dir) => read_dir, @@ -273,10 +287,19 @@ mod tests { .expect("policy result"); let commands = [vec!["rm".to_string()]]; - assert!(matches!( - policy.read().await.check_multiple(commands.iter()), - Evaluation::NoMatch { .. } - )); + assert_eq!( + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::HeuristicsRuleMatch { + command: vec!["rm".to_string()], + decision: Decision::Allow + }], + }, + policy + .read() + .await + .check_multiple(commands.iter(), &|_| Decision::Allow) + ); assert!(!temp_dir.path().join(POLICY_DIR_NAME).exists()); } @@ -307,10 +330,19 @@ mod tests { .await .expect("policy result"); let command = [vec!["rm".to_string()]]; - assert!(matches!( - policy.read().await.check_multiple(command.iter()), - Evaluation::Match { .. } - )); + assert_eq!( + Evaluation { + decision: Decision::Forbidden, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["rm".to_string()], + decision: Decision::Forbidden + }], + }, + policy + .read() + .await + .check_multiple(command.iter(), &|_| Decision::Allow) + ); } #[tokio::test] @@ -326,14 +358,23 @@ mod tests { .await .expect("policy result"); let command = [vec!["ls".to_string()]]; - assert!(matches!( - policy.read().await.check_multiple(command.iter()), - Evaluation::NoMatch { .. } - )); + assert_eq!( + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::HeuristicsRuleMatch { + command: vec!["ls".to_string()], + decision: Decision::Allow + }], + }, + policy + .read() + .await + .check_multiple(command.iter(), &|_| Decision::Allow) + ); } - #[tokio::test] - async fn evaluates_bash_lc_inner_commands() { + #[test] + fn evaluates_bash_lc_inner_commands() { let policy_src = r#" prefix_rule(pattern=["rm"], decision="forbidden") "#; @@ -341,7 +382,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.codexpolicy", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = parser.build(); let forbidden_script = vec![ "bash".to_string(), @@ -356,8 +397,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::OnRequest, &SandboxPolicy::DangerFullAccess, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, @@ -367,14 +407,14 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[tokio::test] - async fn approval_requirement_prefers_execpolicy_match() { + #[test] + fn approval_requirement_prefers_execpolicy_match() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser .parse("test.codexpolicy", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = parser.build(); let command = vec!["rm".to_string()]; let requirement = create_approval_requirement_for_command( @@ -384,8 +424,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::OnRequest, &SandboxPolicy::DangerFullAccess, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, @@ -396,14 +435,14 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[tokio::test] - async fn approval_requirement_respects_approval_policy() { + #[test] + fn approval_requirement_respects_approval_policy() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser .parse("test.codexpolicy", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = parser.build(); let command = vec!["rm".to_string()]; let requirement = create_approval_requirement_for_command( @@ -413,22 +452,21 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::Never, &SandboxPolicy::DangerFullAccess, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, ApprovalRequirement::Forbidden { - reason: PROMPT_REASON.to_string() + reason: PROMPT_CONFLICT_REASON.to_string() } ); } - #[tokio::test] - async fn approval_requirement_falls_back_to_heuristics() { + #[test] + fn approval_requirement_falls_back_to_heuristics() { let command = vec!["python".to_string()]; - let empty_policy = Arc::new(RwLock::new(Policy::empty())); + let empty_policy = Policy::empty(); let requirement = create_approval_requirement_for_command( &empty_policy, &Features::with_defaults(), @@ -436,8 +474,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::UnlessTrusted, &SandboxPolicy::ReadOnly, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, @@ -448,6 +485,36 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } + #[test] + fn heuristics_apply_when_other_commands_match_policy() { + let policy_src = r#"prefix_rule(pattern=["apple"], decision="allow")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "apple | orange".to_string(), + ]; + + assert_eq!( + create_approval_requirement_for_command( + &policy, + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ), + ApprovalRequirement::NeedsApproval { + reason: None, + allow_prefix: Some(vec!["orange".to_string()]) + } + ); + } + #[tokio::test] async fn append_allow_prefix_rule_updates_policy_and_file() { let codex_home = tempdir().expect("create temp dir"); @@ -458,14 +525,13 @@ prefix_rule(pattern=["rm"], decision="forbidden") .await .expect("update policy"); - let evaluation = current_policy.read().await.check(&[ - "echo".to_string(), - "hello".to_string(), - "world".to_string(), - ]); + let evaluation = current_policy.read().await.check( + &["echo".to_string(), "hello".to_string(), "world".to_string()], + &|_| Decision::Allow, + ); assert!(matches!( evaluation, - Evaluation::Match { + Evaluation { decision: Decision::Allow, .. } @@ -497,11 +563,11 @@ prefix_rule(pattern=["rm"], decision="forbidden") )); } - #[tokio::test] - async fn allow_prefix_is_present_for_single_command_without_policy_match() { + #[test] + fn allow_prefix_is_present_for_single_command_without_policy_match() { let command = vec!["python".to_string()]; - let empty_policy = Arc::new(RwLock::new(Policy::empty())); + let empty_policy = Policy::empty(); let requirement = create_approval_requirement_for_command( &empty_policy, &Features::with_defaults(), @@ -509,8 +575,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::UnlessTrusted, &SandboxPolicy::ReadOnly, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, @@ -521,22 +586,21 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[tokio::test] - async fn allow_prefix_is_disabled_when_execpolicy_feature_disabled() { + #[test] + fn allow_prefix_is_disabled_when_execpolicy_feature_disabled() { let command = vec!["python".to_string()]; let mut features = Features::with_defaults(); features.disable(Feature::ExecPolicy); let requirement = create_approval_requirement_for_command( - &Arc::new(RwLock::new(Policy::empty())), + &Policy::empty(), &features, &command, AskForApproval::UnlessTrusted, &SandboxPolicy::ReadOnly, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, @@ -547,14 +611,14 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[tokio::test] - async fn allow_prefix_is_omitted_when_policy_prompts() { + #[test] + fn allow_prefix_is_omitted_when_policy_prompts() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser .parse("test.codexpolicy", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = parser.build(); let command = vec!["rm".to_string()]; let requirement = create_approval_requirement_for_command( @@ -564,8 +628,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::OnRequest, &SandboxPolicy::DangerFullAccess, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, @@ -576,28 +639,57 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[tokio::test] - async fn allow_prefix_is_omitted_for_multi_command_scripts() { + #[test] + fn allow_prefix_is_omitted_for_multi_command_scripts() { let command = vec![ "bash".to_string(), "-lc".to_string(), "python && echo ok".to_string(), ]; let requirement = create_approval_requirement_for_command( - &Arc::new(RwLock::new(Policy::empty())), + &Policy::empty(), &Features::with_defaults(), &command, AskForApproval::UnlessTrusted, &SandboxPolicy::ReadOnly, SandboxPermissions::UseDefault, - ) - .await; + ); assert_eq!( requirement, ApprovalRequirement::NeedsApproval { reason: None, - allow_prefix: None, + allow_prefix: Some(vec!["python".to_string()]), + } + ); + } + + #[test] + fn allow_prefix_uses_first_no_match_in_multi_command_scripts() { + let policy_src = r#"prefix_rule(pattern=["python"], decision="allow")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python && echo ok".to_string(), + ]; + + assert_eq!( + create_approval_requirement_for_command( + &policy, + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ), + ApprovalRequirement::Skip { + bypass_sandbox: true } ); } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 3be04b6c82..73e3c81cd5 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -232,15 +232,17 @@ impl ShellHandler { emitter.begin(event_ctx).await; let features = session.features().await; - let approval_requirement = create_approval_requirement_for_command( - &turn.exec_policy, - &features, - &exec_params.command, - turn.approval_policy, - &turn.sandbox_policy, - SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)), - ) - .await; + let approval_requirement = { + let exec_policy = session.current_exec_policy().await; + create_approval_requirement_for_command( + &exec_policy, + &features, + &exec_params.command, + turn.approval_policy, + &turn.sandbox_policy, + SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)), + ) + }; let req = ShellRequest { command: exec_params.command.clone(), cwd: exec_params.cwd.clone(), diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index fba8108103..abfb68837b 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -555,15 +555,17 @@ impl UnifiedExecSessionManager { let features = context.session.features().await; let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); - let approval_requirement = create_approval_requirement_for_command( - &context.turn.exec_policy, - &features, - command, - context.turn.approval_policy, - &context.turn.sandbox_policy, - SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)), - ) - .await; + let approval_requirement = { + let exec_policy = context.session.current_exec_policy().await; + create_approval_requirement_for_command( + &exec_policy, + &features, + command, + context.turn.approval_policy, + &context.turn.sandbox_policy, + SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)), + ) + }; let req = UnifiedExecToolRequest::new( command.to_vec(), cwd, diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index f5fe1a7df9..24d3a6e4cd 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -95,6 +95,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, + allow_prefix: None, }) .await .expect("submit exec approval"); diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index 8665d3a8ea..045c42e941 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -843,6 +843,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, + allow_prefix: None, }) .await .unwrap(); @@ -901,6 +902,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::ApprovedForSession, + allow_prefix: None, }) .await .unwrap(); @@ -959,6 +961,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Approved, + allow_prefix: None, }) .await .unwrap(); @@ -1017,6 +1020,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Denied, + allow_prefix: None, }) .await .unwrap(); @@ -1075,6 +1079,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::ApprovedForSession, + allow_prefix: None, }) .await .unwrap(); @@ -1134,6 +1139,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() { .submit(Op::ExecApproval { id: "0".into(), decision: ReviewDecision::Denied, + allow_prefix: None, }) .await .unwrap(); diff --git a/codex-rs/mcp-server/src/exec_approval.rs b/codex-rs/mcp-server/src/exec_approval.rs index 033523ac0d..e5b2e00f45 100644 --- a/codex-rs/mcp-server/src/exec_approval.rs +++ b/codex-rs/mcp-server/src/exec_approval.rs @@ -150,6 +150,7 @@ async fn on_exec_approval_response( .submit(Op::ExecApproval { id: event_id, decision: response.decision, + allow_prefix: None, }) .await { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 78cce24cd4..05c2837cc3 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -146,6 +146,9 @@ pub enum Op { id: String, /// The user's decision in response to the request. decision: ReviewDecision, + /// When set, persist this prefix to the execpolicy allow list. + #[serde(default, skip_serializing_if = "Option::is_none")] + allow_prefix: Option>, }, /// Approve a code patch diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 3defdbf9e3..82e8123f34 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -471,28 +471,23 @@ impl ApprovalOption { } fn exec_options(allow_prefix: Option>) -> Vec { - vec![ - ApprovalOption { - label: "Yes, proceed".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Approved), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }, - ApprovalOption { - label: "Yes, and don't ask again this session".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], - }, - ] - .into_iter() - .chain(allow_prefix.map(|prefix| ApprovalOption { - label: "Yes, and don't ask again for commands with this prefix".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix { - allow_prefix: prefix, - }), + vec![ApprovalOption { + label: "Yes, proceed".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Approved), display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], + }] + .into_iter() + .chain(allow_prefix.map(|prefix| { + let rendered_prefix = strip_bash_lc_and_escape(&prefix); + ApprovalOption { + label: format!("Yes, and don't ask again for `{rendered_prefix}` commands"), + decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix { + allow_prefix: 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(), @@ -687,7 +682,6 @@ mod tests { let (tx_raw, mut rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut view = ApprovalOverlay::new(make_exec_request(), tx); - view.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); view.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); assert!( @@ -702,6 +696,6 @@ mod tests { break; } } - assert_eq!(decision, Some(ReviewDecision::ApprovedForSession)); + assert_eq!(decision, Some(ReviewDecision::Approved)); } }