From 7be3b319cf8d49cef8dc1fbbbcdf008df7b65fe6 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Fri, 15 May 2026 13:26:10 -0700 Subject: [PATCH] Remove exec approval prompt flag --- codex-rs/cli/src/main.rs | 69 +++++++++++++--------------------- codex-rs/exec/src/cli.rs | 10 ----- codex-rs/exec/src/lib.rs | 10 +---- codex-rs/exec/src/lib_tests.rs | 17 --------- 4 files changed, 28 insertions(+), 78 deletions(-) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 9c2c557bc2..b7f0cb8dd7 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -843,6 +843,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { let root_remote_auth_token_env = remote.remote_auth_token_env; let root_strict_config = interactive.strict_config; reject_root_strict_config_for_subcommand(root_strict_config, &subcommand)?; + reject_root_approval_policy_for_exec(interactive.approval_policy, &subcommand)?; if let Some(subcommand) = subcommand.as_ref() { profile_v2_for_subcommand(&interactive, subcommand)?; } @@ -868,7 +869,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { root_remote_auth_token_env.as_deref(), "exec", )?; - apply_exec_root_options(&mut exec_cli, &interactive)?; + apply_exec_root_options(&mut exec_cli, &interactive); prepend_config_flags( &mut exec_cli.config_overrides, root_config_overrides.clone(), @@ -1738,17 +1739,20 @@ fn prepend_config_flags( subcommand_config_overrides.prepend_root_overrides(cli_config_overrides); } -fn apply_exec_root_options(exec_cli: &mut ExecCli, interactive: &TuiCli) -> anyhow::Result<()> { +fn apply_exec_root_options(exec_cli: &mut ExecCli, interactive: &TuiCli) { exec_cli .shared .inherit_exec_root_options(&interactive.shared); exec_cli.strict_config |= interactive.strict_config; - if exec_cli.approval_policy.is_none() { - exec_cli.approval_policy = interactive.approval_policy; - } - if exec_cli.dangerously_bypass_approvals_and_sandbox && exec_cli.approval_policy.is_some() { +} + +fn reject_root_approval_policy_for_exec( + approval_policy: Option, + subcommand: &Option, +) -> anyhow::Result<()> { + if approval_policy.is_some() && matches!(subcommand, Some(Subcommand::Exec(_))) { anyhow::bail!( - "--dangerously-bypass-approvals-and-sandbox cannot be used with --ask-for-approval" + "`--ask-for-approval` is only supported for interactive TUI commands, not `codex exec`" ); } Ok(()) @@ -2311,22 +2315,15 @@ mod tests { } #[test] - fn exec_accepts_approval_policy_after_subcommand() { - let cli = MultitoolCli::try_parse_from(["codex", "exec", "-a", "untrusted", "hi"]) - .expect("parse should succeed"); + fn exec_rejects_approval_policy_after_subcommand() { + let err = MultitoolCli::try_parse_from(["codex", "exec", "-a", "untrusted", "hi"]) + .expect_err("exec should not accept approval prompts"); - let Some(Subcommand::Exec(exec)) = cli.subcommand else { - panic!("expected exec subcommand"); - }; - - assert_matches!( - exec.approval_policy, - Some(codex_utils_cli::ApprovalModeCliArg::Untrusted) - ); + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); } #[test] - fn exec_accepts_root_approval_policy() { + fn exec_rejects_root_approval_policy() { let cli = MultitoolCli::try_parse_from(["codex", "-a", "untrusted", "exec", "hi"]) .expect("parse should succeed"); let MultitoolCli { @@ -2338,31 +2335,17 @@ mod tests { panic!("expected exec subcommand"); }; - assert_matches!( - exec.approval_policy, - Some(codex_utils_cli::ApprovalModeCliArg::Untrusted) + apply_exec_root_options(&mut exec, &interactive); + let err = reject_root_approval_policy_for_exec( + interactive.approval_policy, + &Some(Subcommand::Exec(exec)), + ) + .expect_err("root approval prompts should be rejected for exec"); + + assert_eq!( + err.to_string(), + "`--ask-for-approval` is only supported for interactive TUI commands, not `codex exec`" ); - apply_exec_root_options(&mut exec, &interactive).expect("root options apply"); - - assert_matches!( - exec.approval_policy, - Some(codex_utils_cli::ApprovalModeCliArg::Untrusted) - ); - } - - #[test] - fn exec_rejects_subcommand_approval_policy_with_bypass() { - let err = MultitoolCli::try_parse_from([ - "codex", - "exec", - "--dangerously-bypass-approvals-and-sandbox", - "-a", - "untrusted", - "hi", - ]) - .expect_err("conflicting permission flags should be rejected"); - - assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); } fn app_server_from_args(args: &[&str]) -> AppServerCommand { diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index f317cf214c..3a5ebfd1ba 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -2,7 +2,6 @@ use clap::Args; use clap::FromArgMatches; use clap::Parser; use clap::ValueEnum; -use codex_utils_cli::ApprovalModeCliArg; use codex_utils_cli::CliConfigOverrides; use codex_utils_cli::SharedCliOptions; use std::path::PathBuf; @@ -24,15 +23,6 @@ pub struct Cli { #[clap(flatten)] pub shared: ExecSharedCliOptions, - /// Configure when the model requires human approval before executing a command. - #[arg( - long = "ask-for-approval", - short = 'a', - global = true, - conflicts_with = "dangerously_bypass_approvals_and_sandbox" - )] - pub approval_policy: Option, - /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", global = true, default_value_t = false)] pub skip_git_repo_check: bool, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index a5206818cd..6015a4b59f 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -93,7 +93,6 @@ use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_existing_preserving_symlinks; -use codex_utils_cli::ApprovalModeCliArg; use codex_utils_cli::SharedCliOptions; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; @@ -239,13 +238,10 @@ fn cli_overrides_include_approval_policy(cli_kv_overrides: &[(String, T)]) -> fn exec_approval_policy_override( dangerously_bypass_approvals_and_sandbox: bool, - approval_policy: Option, cli_kv_overrides: &[(String, T)], ) -> Option { if dangerously_bypass_approvals_and_sandbox { Some(AskForApproval::Never) - } else if let Some(approval_policy) = approval_policy { - Some(approval_policy.into()) } else if cli_overrides_include_approval_policy(cli_kv_overrides) { None } else { @@ -267,7 +263,6 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result command, strict_config, shared, - approval_policy, skip_git_repo_check, ephemeral, ignore_user_config, @@ -434,11 +429,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result model, review_model: None, config_profile, - // Default to never ask for approvals in headless mode unless the caller - // explicitly selected an approval policy. + // Default to never ask for approvals in headless mode unless a config + // override explicitly selected an approval policy. approval_policy: exec_approval_policy_override( dangerously_bypass_approvals_and_sandbox, - approval_policy, &cli_kv_overrides, ), approvals_reviewer: None, diff --git a/codex-rs/exec/src/lib_tests.rs b/codex-rs/exec/src/lib_tests.rs index c70a0e359e..3de15c3988 100644 --- a/codex-rs/exec/src/lib_tests.rs +++ b/codex-rs/exec/src/lib_tests.rs @@ -90,27 +90,12 @@ fn exec_approval_policy_override_defaults_to_never() { assert_eq!( exec_approval_policy_override( /*dangerously_bypass_approvals_and_sandbox*/ false, - /*approval_policy*/ None, &cli_kv_overrides ), Some(AskForApproval::Never) ); } -#[test] -fn exec_approval_policy_override_uses_cli_approval_policy() { - let cli_kv_overrides: Vec<(String, ())> = Vec::new(); - - assert_eq!( - exec_approval_policy_override( - /*dangerously_bypass_approvals_and_sandbox*/ false, - Some(ApprovalModeCliArg::Untrusted), - &cli_kv_overrides - ), - Some(AskForApproval::UnlessTrusted) - ); -} - #[test] fn exec_approval_policy_override_allows_config_approval_policy() { let cli_kv_overrides = vec![("approval_policy".to_string(), ())]; @@ -118,7 +103,6 @@ fn exec_approval_policy_override_allows_config_approval_policy() { assert_eq!( exec_approval_policy_override( /*dangerously_bypass_approvals_and_sandbox*/ false, - /*approval_policy*/ None, &cli_kv_overrides ), None @@ -132,7 +116,6 @@ fn exec_approval_policy_override_yolo_forces_never() { assert_eq!( exec_approval_policy_override( /*dangerously_bypass_approvals_and_sandbox*/ true, - Some(ApprovalModeCliArg::Untrusted), &cli_kv_overrides ), Some(AskForApproval::Never)