From d9589f8270b4e131addd245755bdb80b5d681c91 Mon Sep 17 00:00:00 2001 From: hefuc-oai Date: Wed, 24 Jun 2026 16:39:46 -0700 Subject: [PATCH] Honor CLI overrides for managed thread defaults --- .../request_processors/thread_processor.rs | 27 +++-- .../tests/common/test_app_server.rs | 12 +- .../app-server/tests/suite/v2/thread_start.rs | 109 +++++++++++++----- 3 files changed, 113 insertions(+), 35 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index d35aa3b40c..8e8eb5fe5b 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -1045,19 +1045,33 @@ impl ThreadRequestProcessor { ) -> Result<(), JSONRPCErrorError> { let thread_start_started_at = std::time::Instant::now(); let requested_cwd = typesafe_overrides.cwd.clone(); - let has_explicit_model = typesafe_overrides.model.is_some() + let current_cli_overrides = config_manager.current_cli_overrides(); + let has_cli_override = |key: &str| { + current_cli_overrides + .iter() + .any(|(path, _value)| path == key) + }; + let has_explicit_model = has_cli_override("model") + || typesafe_overrides.model.is_some() || config_overrides .as_ref() .is_some_and(|overrides| overrides.contains_key("model")); - let has_explicit_reasoning_effort = config_overrides - .as_ref() - .is_some_and(|overrides| overrides.contains_key("model_reasoning_effort")); - let has_explicit_service_tier = typesafe_overrides.service_tier.is_some() + let has_explicit_reasoning_effort = has_cli_override("model_reasoning_effort") + || config_overrides + .as_ref() + .is_some_and(|overrides| overrides.contains_key("model_reasoning_effort")); + let has_explicit_service_tier = has_cli_override("service_tier") + || typesafe_overrides.service_tier.is_some() || config_overrides .as_ref() .is_some_and(|overrides| overrides.contains_key("service_tier")); let mut config = config_manager - .load_with_overrides(config_overrides.clone(), typesafe_overrides.clone()) + .load_with_cli_overrides( + ¤t_cli_overrides, + config_overrides.clone(), + typesafe_overrides.clone(), + /*fallback_cwd*/ None, + ) .await .map_err(|err| config_load_error(&err))?; @@ -1080,7 +1094,6 @@ impl ThreadRequestProcessor { let trust_target = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &config.cwd) .await .unwrap_or_else(|| config.cwd.clone()); - let current_cli_overrides = config_manager.current_cli_overrides(); let cli_overrides_with_trust; let cli_overrides_for_reload = if let Err(err) = codex_core::config::set_project_trust_level( diff --git a/codex-rs/app-server/tests/common/test_app_server.rs b/codex-rs/app-server/tests/common/test_app_server.rs index fbe43e7689..c56eaa122f 100644 --- a/codex-rs/app-server/tests/common/test_app_server.rs +++ b/codex-rs/app-server/tests/common/test_app_server.rs @@ -160,6 +160,13 @@ impl TestAppServer { /// URL-based configuration, this helper rejects a `codex_home` containing /// that file. pub async fn new_with_auto_env(codex_home: &Path) -> anyhow::Result { + Self::new_with_auto_env_and_args(codex_home, &[]).await + } + + pub async fn new_with_auto_env_and_args( + codex_home: &Path, + args: &[&str], + ) -> anyhow::Result { let environments_toml = codex_home.join("environments.toml"); ensure!( !environments_toml @@ -182,7 +189,10 @@ impl TestAppServer { (CODEX_EXEC_SERVER_NOISE_AUTH_TOKEN_ENV_VAR, None), (CODEX_EXEC_SERVER_NOISE_CHATGPT_ACCOUNT_ID_ENV_VAR, None), ]; - let mut app_server = Self::new_with_env(codex_home, &env_overrides).await?; + let mut all_args = vec![DISABLE_PLUGIN_STARTUP_TASKS_ARG]; + all_args.extend_from_slice(args); + let mut app_server = + Self::new_with_env_and_args(codex_home, &env_overrides, &all_args).await?; app_server.auto_env = Some(auto_env); Ok(app_server) } diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index b3b913afb8..474c44c3c5 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -205,35 +205,14 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> { async fn thread_start_treats_managed_new_thread_model_settings_as_defaults() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?; - let managed_model = all_model_presets() - .iter() - .find(|preset| { - preset.id != "gpt-5.2" - && preset - .service_tiers - .iter() - .any(|tier| tier.id == "priority") - }) - .context("expected a model with the fast service tier")?; - std::fs::write( - codex_home.path().join("requirements.toml"), - format!( - r#" -[models.new_thread] -model = "{}" -model_reasoning_effort = "medium" -service_tier = "fast" -"#, - managed_model.id - ), - )?; + let managed_model = + create_config_with_managed_new_thread_defaults(codex_home.path(), &server.uri())?; - let mut mcp = TestAppServer::new(codex_home.path()).await?; + let mut mcp = TestAppServer::new_with_auto_env(codex_home.path()).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let default_request_id = mcp - .send_thread_start_request(ThreadStartParams::default()) + .send_thread_start_request_with_auto_env(ThreadStartParams::default()) .await?; let default_response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, @@ -249,14 +228,14 @@ service_tier = "fast" default_response.service_tier.as_deref(), ), ( - managed_model.id.as_str(), + managed_model.as_str(), Some(ReasoningEffort::Medium), Some("priority"), ) ); let explicit_request_id = mcp - .send_thread_start_request(ThreadStartParams { + .send_thread_start_request_with_auto_env(ThreadStartParams { model: Some("gpt-5.2".to_string()), service_tier: Some(Some(SERVICE_TIER_DEFAULT_REQUEST_VALUE.to_string())), config: Some(HashMap::from([( @@ -288,6 +267,51 @@ service_tier = "fast" Ok(()) } +#[tokio::test] +async fn thread_start_process_cli_overrides_managed_new_thread_defaults() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_with_managed_new_thread_defaults(codex_home.path(), &server.uri())?; + + let mut mcp = TestAppServer::new_with_auto_env_and_args( + codex_home.path(), + &[ + "-c", + "model=gpt-5.2", + "-c", + "model_reasoning_effort=low", + "-c", + "service_tier=default", + ], + ) + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_thread_start_request_with_auto_env(ThreadStartParams::default()) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: ThreadStartResponse = to_response(response)?; + + assert_eq!( + ( + response.model.as_str(), + response.reasoning_effort, + response.service_tier.as_deref(), + ), + ( + "gpt-5.2", + Some(ReasoningEffort::Low), + Some(SERVICE_TIER_DEFAULT_REQUEST_VALUE), + ) + ); + Ok(()) +} + #[tokio::test] async fn thread_start_accepts_absolute_runtime_workspace_roots() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; @@ -1276,6 +1300,37 @@ model_reasoning_effort = "high" Ok(()) } +fn create_config_with_managed_new_thread_defaults( + codex_home: &Path, + server_uri: &str, +) -> Result { + create_config_toml_without_approval_policy(codex_home, server_uri)?; + let managed_model = all_model_presets() + .iter() + .find(|preset| { + preset.id != "gpt-5.2" + && preset + .service_tiers + .iter() + .any(|tier| tier.id == "priority") + }) + .context("expected a model with the fast service tier")? + .id + .clone(); + std::fs::write( + codex_home.join("requirements.toml"), + format!( + r#" +[models.new_thread] +model = "{managed_model}" +model_reasoning_effort = "medium" +service_tier = "fast" +"# + ), + )?; + Ok(managed_model) +} + fn create_config_toml_without_approval_policy( codex_home: &Path, server_uri: &str,