From f522aafb7f119918408375e4e9c67dc00d87419f Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 22 Oct 2025 16:55:06 +0100 Subject: [PATCH] chore: drop approve all (#5503) Not needed anymore --- codex-rs/core/src/config.rs | 26 +------- codex-rs/core/src/features.rs | 8 --- codex-rs/exec/src/lib.rs | 30 --------- codex-rs/exec/tests/suite/approve_all.rs | 81 ------------------------ codex-rs/exec/tests/suite/mod.rs | 1 - 5 files changed, 1 insertion(+), 145 deletions(-) delete mode 100644 codex-rs/exec/tests/suite/approve_all.rs diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 993a6832..0af01f68 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1220,7 +1220,7 @@ impl Config { } } } - let mut approval_policy = approval_policy_override + let approval_policy = approval_policy_override .or(config_profile.approval_policy) .or(cfg.approval_policy) .unwrap_or_else(|| { @@ -1328,10 +1328,6 @@ impl Config { .or(cfg.review_model) .unwrap_or_else(default_review_model); - if features.enabled(Feature::ApproveAll) { - approval_policy = AskForApproval::OnRequest; - } - let config = Self { model, review_model, @@ -1711,26 +1707,6 @@ trust_level = "trusted" Ok(()) } - #[test] - fn approve_all_feature_forces_on_request_policy() -> std::io::Result<()> { - let cfg = r#" -[features] -approve_all = true -"#; - let parsed = toml::from_str::(cfg) - .expect("TOML deserialization should succeed for approve_all feature"); - let temp_dir = TempDir::new()?; - let config = Config::load_from_base_config_with_overrides( - parsed, - ConfigOverrides::default(), - temp_dir.path().to_path_buf(), - )?; - - assert!(config.features.enabled(Feature::ApproveAll)); - assert_eq!(config.approval_policy, AskForApproval::OnRequest); - Ok(()) - } - #[test] fn config_defaults_to_auto_oauth_store_mode() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 84e203ec..647aac5f 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -39,8 +39,6 @@ pub enum Feature { ViewImageTool, /// Allow the model to request web searches. WebSearchRequest, - /// Automatically approve all approval requests from the harness. - ApproveAll, } impl Feature { @@ -238,10 +236,4 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: false, }, - FeatureSpec { - id: Feature::ApproveAll, - key: "approve_all", - stage: Stage::Experimental, - default_enabled: false, - }, ]; diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 1d68b85f..e470194c 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -18,7 +18,6 @@ use codex_core::NewConversation; use codex_core::auth::enforce_login_restrictions; use codex_core::config::Config; use codex_core::config::ConfigOverrides; -use codex_core::features::Feature; use codex_core::git_info::get_git_repo_root; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; @@ -192,7 +191,6 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any }; let config = Config::load_with_cli_overrides(cli_kv_overrides, overrides).await?; - let approve_all_enabled = config.features.enabled(Feature::ApproveAll); if let Err(err) = enforce_login_restrictions(&config).await { eprintln!("{err}"); @@ -366,34 +364,6 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any if matches!(event.msg, EventMsg::Error(_)) { error_seen = true; } - // Auto-approve requests when the approve_all feature is enabled. - if approve_all_enabled { - match &event.msg { - EventMsg::ExecApprovalRequest(_) => { - if let Err(e) = conversation - .submit(Op::ExecApproval { - id: event.id.clone(), - decision: codex_core::protocol::ReviewDecision::Approved, - }) - .await - { - error!("failed to auto-approve exec: {e}"); - } - } - EventMsg::ApplyPatchApprovalRequest(_) => { - if let Err(e) = conversation - .submit(Op::PatchApproval { - id: event.id.clone(), - decision: codex_core::protocol::ReviewDecision::Approved, - }) - .await - { - error!("failed to auto-approve patch: {e}"); - } - } - _ => {} - } - } let shutdown: CodexStatus = event_processor.process_event(event); match shutdown { CodexStatus::Running => continue, diff --git a/codex-rs/exec/tests/suite/approve_all.rs b/codex-rs/exec/tests/suite/approve_all.rs deleted file mode 100644 index ab5b407d..00000000 --- a/codex-rs/exec/tests/suite/approve_all.rs +++ /dev/null @@ -1,81 +0,0 @@ -#![cfg(not(target_os = "windows"))] -#![allow(clippy::expect_used, clippy::unwrap_used)] - -use anyhow::Result; -use core_test_support::responses; -use core_test_support::responses::ev_assistant_message; -use core_test_support::responses::ev_completed; -use core_test_support::responses::ev_function_call; -use core_test_support::responses::ev_response_created; -use core_test_support::responses::mount_sse_sequence; -use core_test_support::responses::sse; -use core_test_support::skip_if_no_network; -use core_test_support::test_codex_exec::test_codex_exec; -use serde_json::Value; -use serde_json::json; - -async fn run_exec_with_args(args: &[&str]) -> Result { - let test = test_codex_exec(); - - let call_id = "exec-approve"; - let exec_args = json!({ - "command": [ - if cfg!(windows) { "cmd.exe" } else { "/bin/sh" }, - if cfg!(windows) { "/C" } else { "-lc" }, - "echo approve-all-ok", - ], - "timeout_ms": 1500, - "with_escalated_permissions": true - }); - - let response_streams = vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&exec_args)?), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_assistant_message("msg-1", "done"), - ev_completed("resp-2"), - ]), - ]; - - let server = responses::start_mock_server().await; - let mock = mount_sse_sequence(&server, response_streams).await; - - test.cmd_with_server(&server).args(args).assert().success(); - - let requests = mock.requests(); - assert!(requests.len() >= 2, "expected at least two responses POSTs"); - let item = requests[1].function_call_output(call_id); - let output_str = item - .get("output") - .and_then(Value::as_str) - .expect("function_call_output.output should be a string"); - - Ok(output_str.to_string()) -} - -/// Setting `features.approve_all=true` should switch to auto-approvals. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn approve_all_auto_accepts_exec() -> Result<()> { - skip_if_no_network!(Ok(())); - - let output = run_exec_with_args(&[ - "--skip-git-repo-check", - "-c", - "features.approve_all=true", - "train", - ]) - .await?; - assert!( - output.contains("Exit code: 0"), - "expected Exit code: 0 in output: {output}" - ); - assert!( - output.contains("approve-all-ok"), - "expected command output in response: {output}" - ); - - Ok(()) -} diff --git a/codex-rs/exec/tests/suite/mod.rs b/codex-rs/exec/tests/suite/mod.rs index 3e1e39d6..052c43bf 100644 --- a/codex-rs/exec/tests/suite/mod.rs +++ b/codex-rs/exec/tests/suite/mod.rs @@ -1,6 +1,5 @@ // Aggregates all former standalone integration tests as modules. mod apply_patch; -mod approve_all; mod auth_env; mod originator; mod output_schema;