diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 8d632396b08..06ae4e64e97 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -80,6 +80,7 @@ use codex_otel::set_parent_from_context; use codex_otel::traceparent_context_from_env; use codex_protocol::SessionId; use codex_protocol::ThreadId; +use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::SandboxMode; use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::PermissionProfile; @@ -134,6 +135,7 @@ pub use exec_events::TurnStartedEvent; pub use exec_events::Usage; pub use exec_events::WebSearchItem; use serde_json::Value; +use std::future::Future; use std::io::IsTerminal; use std::io::Read; use std::path::Path; @@ -399,11 +401,11 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result None // No model specified, will use the default. }; - // Load configuration and determine approval policy let overrides = ConfigOverrides { model, review_model: None, - // Default to never ask for approvals in headless mode. Feature flags can override. + // Default to never ask for approvals in headless mode. Rebuild below if + // the fully resolved reviewer is AutoReview. approval_policy: Some(AskForApproval::Never), approvals_reviewer: None, sandbox_mode, @@ -428,14 +430,22 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result additional_writable_roots: add_dir, }; - let config = ConfigBuilder::default() - .cli_overrides(cli_kv_overrides) - .harness_overrides(overrides) - .loader_overrides(loader_overrides) - .strict_config(strict_config) - .cloud_requirements(cloud_requirements) - .build() - .await?; + let build_config = |overrides| { + ConfigBuilder::default() + .codex_home(codex_home.to_path_buf()) + .cli_overrides(cli_kv_overrides.clone()) + .harness_overrides(overrides) + .loader_overrides(loader_overrides.clone()) + .strict_config(strict_config) + .cloud_requirements(cloud_requirements.clone()) + .build() + }; + let config = build_exec_config( + overrides, + dangerously_bypass_approvals_and_sandbox || removed_full_auto, + build_config, + ) + .await?; #[allow(clippy::print_stderr)] match check_execpolicy_for_warnings(&config.config_layer_stack).await { @@ -561,6 +571,41 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result .await } +async fn build_exec_config( + overrides: ConfigOverrides, + preserve_headless_approval_policy: bool, + build_config: BuildConfig, +) -> std::io::Result +where + BuildConfig: Fn(ConfigOverrides) -> BuildFuture, + BuildFuture: Future>, +{ + let build_without_headless_approval_policy = || { + build_config(ConfigOverrides { + approval_policy: None, + ..overrides.clone() + }) + }; + match build_config(overrides.clone()).await { + Ok(config) + if config.approvals_reviewer == ApprovalsReviewer::AutoReview + && !preserve_headless_approval_policy => + { + build_without_headless_approval_policy().await + } + Ok(config) => Ok(config), + Err(headless_error) if !preserve_headless_approval_policy => { + match build_without_headless_approval_policy().await { + Ok(config) if config.approvals_reviewer == ApprovalsReviewer::AutoReview => { + Ok(config) + } + Ok(_) | Err(_) => Err(headless_error), + } + } + Err(headless_error) => Err(headless_error), + } +} + async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { let ExecRunArgs { in_process_start_args, diff --git a/codex-rs/exec/src/lib_tests.rs b/codex-rs/exec/src/lib_tests.rs index 79d07a0b761..37a7f7e0729 100644 --- a/codex-rs/exec/src/lib_tests.rs +++ b/codex-rs/exec/src/lib_tests.rs @@ -458,6 +458,92 @@ async fn thread_start_params_include_review_policy_when_auto_review_is_enabled() ); } +#[tokio::test] +async fn build_exec_config_retries_without_invalid_headless_policy_for_auto_review() { + let codex_home = tempdir().expect("create temp codex home"); + let cwd = tempdir().expect("create temp cwd"); + std::fs::write( + codex_home.path().join("config.toml"), + r#" +approval_policy = "on-request" +approvals_reviewer = "auto_review" +"#, + ) + .expect("write config"); + let requirements_path = codex_home.path().join("requirements.toml"); + std::fs::write( + &requirements_path, + r#" +allowed_approval_policies = ["never", "on-request"] +allowed_sandbox_modes = ["read-only", "workspace-write"] +"#, + ) + .expect("write requirements"); + let mut loader_overrides = LoaderOverrides::without_managed_config_for_tests(); + loader_overrides.system_requirements_path = Some(requirements_path); + let overrides = ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + approval_policy: Some(AskForApproval::Never), + sandbox_mode: Some(SandboxMode::DangerFullAccess), + ..Default::default() + }; + let build_config = |overrides| { + ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .loader_overrides(loader_overrides.clone()) + .harness_overrides(overrides) + .build() + }; + + let error = build_config(overrides.clone()) + .await + .expect_err("synthetic headless approval policy should fail"); + assert!( + error + .to_string() + .contains("`approval_policy = \"never\"` cannot be used") + ); + + let config = build_exec_config( + overrides, + /*preserve_headless_approval_policy*/ false, + build_config, + ) + .await + .expect("auto-review config should retry without the synthetic approval policy"); + + assert_eq!( + config.permissions.approval_policy.value(), + AskForApproval::OnRequest + ); + assert_eq!(config.approvals_reviewer, ApprovalsReviewer::AutoReview); +} + +#[tokio::test] +async fn build_exec_config_preserves_headless_error_when_retry_fails() { + let overrides = ConfigOverrides { + approval_policy: Some(AskForApproval::Never), + ..Default::default() + }; + + let error = build_exec_config( + overrides, + /*preserve_headless_approval_policy*/ false, + |overrides| async move { + let message = if overrides.approval_policy == Some(AskForApproval::Never) { + "headless error" + } else { + "retry error" + }; + Err(std::io::Error::other(message)) + }, + ) + .await + .expect_err("failed speculative retry should preserve the original error"); + + assert_eq!(error.to_string(), "headless error"); +} + #[tokio::test] async fn thread_start_params_include_user_thread_source() { let codex_home = tempdir().expect("create temp codex home"); diff --git a/codex-rs/exec/tests/suite/approval_policy.rs b/codex-rs/exec/tests/suite/approval_policy.rs new file mode 100644 index 00000000000..1097d608438 --- /dev/null +++ b/codex-rs/exec/tests/suite/approval_policy.rs @@ -0,0 +1,69 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::expect_used, clippy::unwrap_used)] + +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 { + let test = test_codex_exec(); + std::fs::write( + test.home_path().join("config.toml"), + r#" +approval_policy = "on-request" +approvals_reviewer = "auto_review" +"#, + )?; + + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + responses::ev_response_created("response_1"), + responses::ev_assistant_message("response_1", "done"), + responses::ev_completed("response_1"), + ]); + responses::mount_sse_once(&server, body).await; + + let mut cmd = test.cmd_with_server(&server); + let output = cmd + .arg("--skip-git-repo-check") + .args(extra_args) + .arg("check approval mode") + .output()?; + + assert!(output.status.success(), "exec run failed: {output:?}"); + + Ok(String::from_utf8(output.stderr)?) +} + +#[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?; + assert!( + stderr.contains("approval: on-request"), + "stderr missing preserved auto-review approval 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 = + run_exec_with_auto_review_config(&["--dangerously-bypass-approvals-and-sandbox"]).await?; + assert!( + stderr.contains("approval: never"), + "stderr missing bypass approval mode: {stderr}" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn exec_full_auto_preserves_never_for_auto_review_config() -> anyhow::Result<()> { + let stderr = run_exec_with_auto_review_config(&["--full-auto"]).await?; + assert!( + stderr.contains("approval: never"), + "stderr missing full-auto approval mode: {stderr}" + ); + + Ok(()) +} diff --git a/codex-rs/exec/tests/suite/mod.rs b/codex-rs/exec/tests/suite/mod.rs index c6fa0f9fde6..6f868563273 100644 --- a/codex-rs/exec/tests/suite/mod.rs +++ b/codex-rs/exec/tests/suite/mod.rs @@ -1,6 +1,7 @@ // Aggregates all former standalone integration tests as modules. mod add_dir; mod apply_patch; +mod approval_policy; mod auth_env; mod ephemeral; mod mcp_required_exit;