Remove exec approval prompt flag

This commit is contained in:
Adrian Bravo
2026-05-15 13:26:10 -07:00
parent 04bea35425
commit 7be3b319cf
4 changed files with 28 additions and 78 deletions

View File

@@ -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<codex_utils_cli::ApprovalModeCliArg>,
subcommand: &Option<Subcommand>,
) -> 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 {

View File

@@ -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<ApprovalModeCliArg>,
/// 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,

View File

@@ -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<T>(cli_kv_overrides: &[(String, T)]) ->
fn exec_approval_policy_override<T>(
dangerously_bypass_approvals_and_sandbox: bool,
approval_policy: Option<ApprovalModeCliArg>,
cli_kv_overrides: &[(String, T)],
) -> Option<AskForApproval> {
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,

View File

@@ -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)