diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 7039501635..11009dd90d 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -990,6 +990,9 @@ async fn cli_main( let root_remote = remote.remote; let root_remote_auth_token_env = remote.remote_auth_token_env; let root_strict_config = interactive.strict_config; + interactive + .shared + .take_auto_review_config_overrides(&mut root_config_overrides); reject_root_strict_config_for_subcommand(root_strict_config, &subcommand)?; if let Some(subcommand) = subcommand.as_ref() { profile_v2_for_subcommand(&interactive, subcommand)?; @@ -2570,13 +2573,19 @@ fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli) approval_policy, web_search, prompt, - config_overrides, + mut config_overrides, .. } = subcommand_cli; + let subcommand_auto_review = shared.auto_review; interactive .shared .apply_subcommand_overrides(shared.into_inner()); - if let Some(approval) = approval_policy { + interactive + .shared + .take_auto_review_config_overrides(&mut config_overrides); + if subcommand_auto_review { + interactive.approval_policy = None; + } else if let Some(approval) = approval_policy { interactive.approval_policy = Some(approval); } if web_search { @@ -2698,12 +2707,15 @@ mod tests { fn finalize_resume_from_args(args: &[&str]) -> TuiCli { let cli = MultitoolCli::try_parse_from(args).expect("parse"); let MultitoolCli { - interactive, - config_overrides: root_overrides, + mut interactive, + config_overrides: mut root_overrides, subcommand, feature_toggles: _, remote: _, } = cli; + interactive + .shared + .take_auto_review_config_overrides(&mut root_overrides); let Subcommand::Resume(ResumeCommand { session_id, @@ -2732,12 +2744,15 @@ mod tests { fn finalize_fork_from_args(args: &[&str]) -> TuiCli { let cli = MultitoolCli::try_parse_from(args).expect("parse"); let MultitoolCli { - interactive, - config_overrides: root_overrides, + mut interactive, + config_overrides: mut root_overrides, subcommand, feature_toggles: _, remote: _, } = cli; + interactive + .shared + .take_auto_review_config_overrides(&mut root_overrides); let Subcommand::Fork(ForkCommand { session_id, @@ -2754,6 +2769,22 @@ mod tests { finalize_fork_interactive(interactive, root_overrides, session_id, last, all, fork_cli) } + fn finalize_exec_from_args(args: &[&str]) -> ExecCli { + let mut cli = MultitoolCli::try_parse_from(args).expect("parse"); + cli.interactive + .shared + .take_auto_review_config_overrides(&mut cli.config_overrides); + let Some(Subcommand::Exec(mut exec)) = cli.subcommand else { + panic!("expected exec subcommand"); + }; + exec.shared + .inherit_exec_root_options(&cli.interactive.shared); + prepend_config_flags(&mut exec.config_overrides, cli.config_overrides); + exec.shared + .take_auto_review_config_overrides(&mut exec.config_overrides); + exec + } + fn finalize_archive_from_args(args: &[&str]) -> (String, TuiCli, InteractiveRemoteOptions) { let cli = MultitoolCli::try_parse_from(args).expect("parse"); let MultitoolCli { @@ -2921,6 +2952,157 @@ mod tests { assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); } + #[test] + fn approve_for_me_configures_interactive_mode() { + for flag in ["--approve-for-me", "--not-so-yolo"] { + let mut cli = MultitoolCli::try_parse_from(["codex", flag]).expect("parse flag"); + + assert!(cli.interactive.auto_review); + cli.interactive + .shared + .take_auto_review_config_overrides(&mut cli.interactive.config_overrides); + assert_eq!( + cli.interactive.config_overrides.raw_overrides, + vec![ + r#"approvals_reviewer="auto_review""#.to_string(), + r#"approval_policy="on-request""#.to_string(), + r#"sandbox_mode="workspace-write""#.to_string(), + ] + ); + assert!(!cli.interactive.auto_review); + } + } + + #[test] + fn not_so_yolo_alias_is_hidden_from_help() { + for args in [&["codex", "--help"][..], &["codex", "exec", "--help"][..]] { + let help = help_from_args(args); + + assert!(!help.contains("--not-so-yolo"), "{help}"); + } + } + + #[test] + fn approve_for_me_defaults_propagate_from_root_to_exec() { + let exec = finalize_exec_from_args(&["codex", "--approve-for-me", "exec", "summarize"]); + + assert_eq!( + exec.config_overrides.raw_overrides, + vec![ + r#"approvals_reviewer="auto_review""#.to_string(), + r#"approval_policy="on-request""#.to_string(), + r#"sandbox_mode="workspace-write""#.to_string(), + ] + ); + assert!(exec.sandbox_mode.is_none()); + } + + #[test] + fn later_exec_sandbox_partially_overrides_approve_for_me() { + let exec = finalize_exec_from_args(&[ + "codex", + "--approve-for-me", + "exec", + "--sandbox", + "read-only", + ]); + + assert_matches!( + exec.sandbox_mode, + Some(codex_utils_cli::SandboxModeCliArg::ReadOnly) + ); + assert_eq!( + exec.config_overrides.raw_overrides, + vec![ + r#"approvals_reviewer="auto_review""#.to_string(), + r#"approval_policy="on-request""#.to_string(), + r#"sandbox_mode="workspace-write""#.to_string(), + ] + ); + } + + #[test] + fn later_approve_for_me_overrides_root_exec_sandbox() { + let exec = finalize_exec_from_args(&[ + "codex", + "--sandbox", + "read-only", + "exec", + "--approve-for-me", + ]); + + assert!(exec.sandbox_mode.is_none()); + assert_eq!( + exec.config_overrides.raw_overrides, + vec![ + r#"approvals_reviewer="auto_review""#.to_string(), + r#"approval_policy="on-request""#.to_string(), + r#"sandbox_mode="workspace-write""#.to_string(), + ] + ); + } + + #[test] + fn later_resume_approval_policy_partially_overrides_approve_for_me() { + let interactive = finalize_resume_from_args(&[ + "codex", + "--approve-for-me", + "resume", + "--ask-for-approval", + "never", + ]); + + assert_matches!( + interactive.approval_policy, + Some(codex_utils_cli::ApprovalModeCliArg::Never) + ); + assert_eq!( + interactive.config_overrides.raw_overrides, + vec![ + r#"approvals_reviewer="auto_review""#.to_string(), + r#"approval_policy="on-request""#.to_string(), + r#"sandbox_mode="workspace-write""#.to_string(), + ] + ); + } + + #[test] + fn later_approve_for_me_overrides_root_tui_approval_policy() { + let interactive = finalize_resume_from_args(&[ + "codex", + "--ask-for-approval", + "never", + "resume", + "--approve-for-me", + ]); + + assert!(interactive.approval_policy.is_none()); + assert_eq!( + interactive.config_overrides.raw_overrides, + vec![ + r#"approvals_reviewer="auto_review""#.to_string(), + r#"approval_policy="on-request""#.to_string(), + r#"sandbox_mode="workspace-write""#.to_string(), + ] + ); + } + + #[test] + fn approve_for_me_conflicts_with_explicit_interactive_permissions() { + for conflicting_args in [ + vec!["--sandbox", "read-only"], + vec!["--ask-for-approval", "on-request"], + vec!["--dangerously-bypass-approvals-and-sandbox"], + ] { + let mut args = vec!["codex", "--approve-for-me"]; + args.extend(conflicting_args); + + let error = + MultitoolCli::try_parse_from(args).expect_err("permission flags should conflict"); + assert_eq!(error.kind(), clap::error::ErrorKind::ArgumentConflict); + } + } + fn app_server_from_args(args: &[&str]) -> AppServerCommand { let cli = MultitoolCli::try_parse_from(args).expect("parse"); let Subcommand::AppServer(app_server) = cli.subcommand.expect("app-server present") else { diff --git a/codex-rs/exec/src/cli_tests.rs b/codex-rs/exec/src/cli_tests.rs index f17707bf77..cfd8542d5d 100644 --- a/codex-rs/exec/src/cli_tests.rs +++ b/codex-rs/exec/src/cli_tests.rs @@ -73,3 +73,27 @@ fn parses_config_isolation_flags() { assert!(cli.ignore_user_config); assert!(cli.ignore_rules); } + +#[test] +fn approve_for_me_flag_applies_to_resume_when_passed_at_exec_root() { + for flag in ["--approve-for-me", "--not-so-yolo"] { + let cli = Cli::parse_from(["codex-exec", flag, "resume", "--last"]); + + assert!(cli.auto_review); + } +} + +#[test] +fn approve_for_me_flag_conflicts_with_other_sandbox_modes() { + for conflicting_args in [ + vec!["--sandbox", "read-only"], + vec!["--dangerously-bypass-approvals-and-sandbox"], + ] { + let mut args = vec!["codex-exec", "--approve-for-me"]; + args.extend(conflicting_args); + args.push("summarize"); + + let error = Cli::try_parse_from(args).expect_err("flags should conflict"); + assert_eq!(error.kind(), clap::error::ErrorKind::ArgumentConflict); + } +} diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 962dba866a..097816cda9 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -255,9 +255,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result json: json_mode, prompt, output_schema: output_schema_path, - config_overrides, + mut config_overrides, } = cli; - let shared = shared.into_inner(); + let mut shared = shared.into_inner(); + shared.take_auto_review_config_overrides(&mut config_overrides); let SharedCliOptions { images, model: model_cli_arg, @@ -265,6 +266,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result oss_provider, config_profile_v2, sandbox_mode: sandbox_mode_cli_arg, + auto_review: _, dangerously_bypass_approvals_and_sandbox, bypass_hook_trust, cwd, diff --git a/codex-rs/exec/tests/suite/approval_policy.rs b/codex-rs/exec/tests/suite/approval_policy.rs index de650ef978..5efb6d0bda 100644 --- a/codex-rs/exec/tests/suite/approval_policy.rs +++ b/codex-rs/exec/tests/suite/approval_policy.rs @@ -4,15 +4,9 @@ use core_test_support::responses; use core_test_support::test_codex_exec::test_codex_exec; -async fn run_exec_with_auto_review_config(extra_args: &[&str]) -> anyhow::Result { +async fn run_exec_with_config(config_toml: &str, extra_args: &[&str]) -> anyhow::Result { let test = test_codex_exec(); - std::fs::write( - test.home_path().join("config.toml"), - r#" -approval_policy = "on-request" -approvals_reviewer = "auto_review" -"#, - )?; + std::fs::write(test.home_path().join("config.toml"), config_toml)?; let server = responses::start_mock_server().await; let body = responses::sse(vec![ @@ -34,6 +28,17 @@ approvals_reviewer = "auto_review" Ok(String::from_utf8(output.stderr)?) } +async fn run_exec_with_auto_review_config(extra_args: &[&str]) -> anyhow::Result { + run_exec_with_config( + r#" +approval_policy = "on-request" +approvals_reviewer = "auto_review" +"#, + extra_args, + ) + .await +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn exec_preserves_on_request_for_auto_review_config() -> anyhow::Result<()> { let stderr = run_exec_with_auto_review_config(&[]).await?; @@ -45,6 +50,21 @@ async fn exec_preserves_on_request_for_auto_review_config() -> anyhow::Result<() Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn exec_approve_for_me_flag_sets_approval_mode_and_sandbox() -> anyhow::Result<()> { + let stderr = run_exec_with_config("", &["--approve-for-me"]).await?; + assert!( + stderr.contains("approval: on-request"), + "stderr missing --approve-for-me approval mode: {stderr}" + ); + assert!( + stderr.contains("sandbox: workspace-write"), + "stderr missing --approve-for-me sandbox mode: {stderr}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn exec_bypass_preserves_never_for_auto_review_config() -> anyhow::Result<()> { let stderr = diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index 13ddffb14a..2273973bbf 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -136,4 +136,5 @@ fn mark_tui_args(cmd: clap::Command) -> clap::Command { cmd.mut_arg("dangerously_bypass_approvals_and_sandbox", |arg| { arg.conflicts_with("approval_policy") }) + .mut_arg("auto_review", |arg| arg.conflicts_with("approval_policy")) } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index ccc0e4c451..8aef050cc2 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -924,6 +924,9 @@ pub async fn run_main( ) }; + cli.shared + .take_auto_review_config_overrides(&mut cli.config_overrides); + // Map the legacy --search flag to the canonical web_search mode. if cli.web_search { cli.config_overrides diff --git a/codex-rs/utils/cli/src/shared_options.rs b/codex-rs/utils/cli/src/shared_options.rs index 839b32ccda..c6c3e05e65 100644 --- a/codex-rs/utils/cli/src/shared_options.rs +++ b/codex-rs/utils/cli/src/shared_options.rs @@ -1,5 +1,6 @@ //! Shared command-line flags used by both interactive and non-interactive Codex entry points. +use crate::CliConfigOverrides; use crate::SandboxModeCliArg; use clap::Args; use codex_protocol::config_types::ProfileV2Name; @@ -39,6 +40,15 @@ pub struct SharedCliOptions { #[arg(long = "sandbox", short = 's')] pub sandbox_mode: Option, + /// Route approval requests through automatic review using the workspace-write sandbox. + #[arg( + long = "approve-for-me", + alias = "not-so-yolo", + default_value_t = false, + conflicts_with_all = ["sandbox_mode", "dangerously_bypass_approvals_and_sandbox"] + )] + pub auto_review: bool, + /// Skip all confirmation prompts and execute commands without sandboxing. /// EXTREMELY DANGEROUS. Intended solely for running in environments that are externally sandboxed. #[arg( @@ -63,9 +73,25 @@ pub struct SharedCliOptions { } impl SharedCliOptions { + pub fn take_auto_review_config_overrides(&mut self, overrides: &mut CliConfigOverrides) { + if self.auto_review { + overrides + .raw_overrides + .push(r#"approvals_reviewer="auto_review""#.to_string()); + overrides + .raw_overrides + .push(r#"approval_policy="on-request""#.to_string()); + overrides + .raw_overrides + .push(r#"sandbox_mode="workspace-write""#.to_string()); + self.auto_review = false; + } + } + pub fn inherit_exec_root_options(&mut self, root: &Self) { - let self_selected_sandbox_mode = - self.sandbox_mode.is_some() || self.dangerously_bypass_approvals_and_sandbox; + let self_selected_sandbox_mode = self.sandbox_mode.is_some() + || self.auto_review + || self.dangerously_bypass_approvals_and_sandbox; let Self { images, model, @@ -73,6 +99,7 @@ impl SharedCliOptions { oss_provider, config_profile_v2, sandbox_mode, + auto_review, dangerously_bypass_approvals_and_sandbox, bypass_hook_trust, cwd, @@ -85,6 +112,7 @@ impl SharedCliOptions { oss_provider: root_oss_provider, config_profile_v2: root_config_profile_v2, sandbox_mode: root_sandbox_mode, + auto_review: root_auto_review, dangerously_bypass_approvals_and_sandbox: root_dangerously_bypass_approvals_and_sandbox, bypass_hook_trust: root_bypass_hook_trust, cwd: root_cwd, @@ -103,10 +131,9 @@ impl SharedCliOptions { if config_profile_v2.is_none() { config_profile_v2.clone_from(root_config_profile_v2); } - if sandbox_mode.is_none() { - *sandbox_mode = *root_sandbox_mode; - } if !self_selected_sandbox_mode { + *sandbox_mode = *root_sandbox_mode; + *auto_review = *root_auto_review; *dangerously_bypass_approvals_and_sandbox = *root_dangerously_bypass_approvals_and_sandbox; } @@ -130,6 +157,7 @@ impl SharedCliOptions { pub fn apply_subcommand_overrides(&mut self, subcommand: Self) { let subcommand_selected_sandbox_mode = subcommand.sandbox_mode.is_some() + || subcommand.auto_review || subcommand.dangerously_bypass_approvals_and_sandbox; let Self { images, @@ -138,6 +166,7 @@ impl SharedCliOptions { oss_provider, config_profile_v2, sandbox_mode, + auto_review, dangerously_bypass_approvals_and_sandbox, bypass_hook_trust, cwd, @@ -158,6 +187,7 @@ impl SharedCliOptions { } if subcommand_selected_sandbox_mode { self.sandbox_mode = sandbox_mode; + self.auto_review = auto_review; self.dangerously_bypass_approvals_and_sandbox = dangerously_bypass_approvals_and_sandbox; }