diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index 9103a17412..4d45285e80 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -1097,9 +1097,6 @@ async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() let codex_home = tempdir()?; app.config.codex_home = codex_home.path().to_path_buf().abs(); app.config.sqlite = SqliteConfig::new_for_testing(codex_home.path().abs()); - app.config - .web_search_mode - .set(codex_protocol::config_types::WebSearchMode::Live)?; std::fs::write( codex_home.path().join("config.toml"), "web_search = \"disabled\"\n", @@ -1169,7 +1166,7 @@ async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() assert_eq!(starts.len(), 2); for params in &starts { assert_eq!(params["dynamicTools"], serde_json::Value::Null); - assert_eq!(params["config"]["web_search"], "live"); + assert_eq!(params["config"].get("web_search"), None); let server = ¶ms["config"]["mcp_servers.codex_tui"]; assert!( server["url"] diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index fac4b63702..dceab5c168 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -14,6 +14,7 @@ mod provider_selection_tests; mod realtime; mod rollout_history; mod thread_list; +mod web_search; #[cfg(test)] #[path = "app_server_session/collaboration_catalog_tests.rs"] @@ -1831,6 +1832,7 @@ pub(crate) fn personality_opt_out_only(personality: Option) -> Opti fn config_request_overrides_from_config( config: &Config, + thread_params_mode: ThreadParamsMode, ) -> Option> { let mut session_config = toml::Value::Table(toml::Table::new()); for layer in config.config_layer_stack.layers_low_to_high() { @@ -1877,13 +1879,16 @@ fn config_request_overrides_from_config( "personality", personality_opt_out_only(config.personality).map(|personality| personality.to_string()), ); - insert( - "web_search", - Some(config.web_search_mode.value().to_string()), - ); // Only winning launch choices may replace server defaults or saved thread settings. let origins = config.config_layer_stack.origins(); let effective = config.config_layer_stack.effective_config(); + web_search::apply_launch_override( + config, + thread_params_mode, + &effective, + &origins, + &mut overrides, + ); for key in ["model_reasoning_summary", "model_verbosity"] { if origins.get(key).is_some_and(|origin| { matches!( @@ -1921,7 +1926,8 @@ fn remove_permission_config_overrides(config: &mut Option Option> { - let mut overrides = config_request_overrides_from_config(config).unwrap_or_default(); + let mut overrides = config_request_overrides_from_config(config, ThreadParamsMode::Embedded) + .unwrap_or_default(); let summary = config .model_reasoning_summary .unwrap_or(codex_protocol::config_types::ReasoningSummary::None); @@ -2084,7 +2090,9 @@ pub(crate) fn thread_start_params_from_config( permissions, config: match thread_params_mode { ThreadParamsMode::Embedded => new_thread_reasoning_overrides(config), - ThreadParamsMode::Remote => config_request_overrides_from_config(config), + ThreadParamsMode::Remote => { + config_request_overrides_from_config(config, thread_params_mode) + } }, ephemeral: Some(config.ephemeral), history_mode: (!config.ephemeral).then_some(ThreadHistoryMode::Paginated), @@ -2121,7 +2129,7 @@ fn thread_resume_params_from_config( ) }) .flatten(); - let mut config_overrides = config_request_overrides_from_config(&config); + let mut config_overrides = config_request_overrides_from_config(&config, thread_params_mode); if model_settings == ResumeModelSettings::RestoreFromThread && let Some(overrides) = config_overrides.as_mut() { @@ -2200,7 +2208,7 @@ fn thread_fork_params_from_config( approvals_reviewer: approvals_reviewer_override_from_config(&config), sandbox, permissions, - config: config_request_overrides_from_config(&config), + config: config_request_overrides_from_config(&config, thread_params_mode), base_instructions: config.base_instructions.clone().filter(|_| { !matches!( config.base_instructions_provenance, @@ -3425,7 +3433,6 @@ mod tests { let string = |value: &str| serde_json::Value::String(value.to_string()); let expected_config = HashMap::from([ ("model_reasoning_effort".to_string(), string("high")), - ("web_search".to_string(), string("disabled")), ("bypass_hook_trust".to_string(), true.into()), ]); let mut expected_start_config = expected_config.clone(); @@ -3440,34 +3447,54 @@ mod tests { } #[tokio::test] - async fn config_overrides_forward_explicit_summary_and_verbosity() -> Result<()> { + async fn config_overrides_forward_explicit_summary_verbosity_and_web_search() -> Result<()> { let home = tempfile::tempdir()?; let workspace = home.path().join("workspace"); std::fs::create_dir_all(workspace.join(".codex"))?; std::fs::write( home.path().join("config.toml"), format!( - "model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n[projects.{}]\ntrust_level = \"trusted\"\n", + "model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\nweb_search = \"disabled\"\n[projects.{}]\ntrust_level = \"trusted\"\n", toml::Value::String(workspace.to_string_lossy().into_owned()), ), )?; let profile = AbsolutePathBuf::from_absolute_path(home.path().join("work.config.toml"))?; std::fs::write( &profile, - "model_reasoning_summary = \"detailed\"\nmodel_verbosity = \"high\"\n", + "model_reasoning_summary = \"detailed\"\nmodel_verbosity = \"high\"\nweb_search = \"cached\"\n", )?; for (selected_profile, project, cli, expected) in [ - (false, false, false, [None, None]), - (true, false, false, [Some("detailed"), Some("high")]), - (false, false, true, [Some("auto"), Some("medium")]), - (true, false, true, [Some("auto"), Some("medium")]), - (true, true, false, [None, None]), - (true, true, true, [Some("auto"), Some("medium")]), + (false, false, false, [None, None, None]), + ( + true, + false, + false, + [Some("detailed"), Some("high"), Some("cached")], + ), + ( + false, + false, + true, + [Some("auto"), Some("medium"), Some("live")], + ), + ( + true, + false, + true, + [Some("auto"), Some("medium"), Some("live")], + ), + (true, true, false, [None, None, None]), + ( + true, + true, + true, + [Some("auto"), Some("medium"), Some("live")], + ), ] { std::fs::write( workspace.join(".codex/config.toml"), if project { - "model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n" + "model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\nweb_search = \"disabled\"\n" } else { "" }, @@ -3487,16 +3514,17 @@ mod tests { vec![ ("model_reasoning_summary".to_string(), "auto".into()), ("model_verbosity".to_string(), "medium".into()), + ("web_search".to_string(), "live".into()), ] } else { Vec::new() }) .build() .await?; - let overrides = - config_request_overrides_from_config(&config).expect("config overrides"); + let overrides = config_request_overrides_from_config(&config, ThreadParamsMode::Remote) + .expect("config overrides"); assert_eq!( - ["model_reasoning_summary", "model_verbosity"] + ["model_reasoning_summary", "model_verbosity", "web_search"] .map(|key| overrides.get(key).and_then(serde_json::Value::as_str)), expected, ); @@ -3525,13 +3553,7 @@ mod tests { assert_eq!(params.model, None); assert_eq!(params.model_provider, None); - assert_eq!( - params.config, - Some(HashMap::from([( - "web_search".to_string(), - serde_json::Value::String("cached".to_string()), - )])) - ); + assert_eq!(params.config, None); } #[tokio::test] @@ -3836,20 +3858,23 @@ mod tests { config.personality = None; let implicit_overrides = - config_request_overrides_from_config(&config).expect("config overrides"); + config_request_overrides_from_config(&config, ThreadParamsMode::Remote) + .expect("config overrides"); assert!(!implicit_overrides.contains_key("personality")); for personality in [Personality::Friendly, Personality::Pragmatic] { config.personality = Some(personality); let ordinary_overrides = - config_request_overrides_from_config(&config).expect("config overrides"); + config_request_overrides_from_config(&config, ThreadParamsMode::Remote) + .expect("config overrides"); assert!(!ordinary_overrides.contains_key("personality")); } config.personality = Some(Personality::None); let explicit_overrides = - config_request_overrides_from_config(&config).expect("config overrides"); + config_request_overrides_from_config(&config, ThreadParamsMode::Remote) + .expect("config overrides"); assert_eq!( explicit_overrides.get("personality"), diff --git a/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs b/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs index 3cb3cbdfba..057a51da65 100644 --- a/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs +++ b/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs @@ -3,8 +3,6 @@ use super::*; use crate::legacy_core::config::ConfigBuilder; use codex_app_server_protocol::ServerNotification; -use codex_protocol::config_types::ReasoningSummary; -use codex_protocol::config_types::Verbosity; use core_test_support::responses; use pretty_assertions::assert_eq; use serde_json::json; @@ -44,7 +42,7 @@ async fn reasoning_defaults_reach_responses() -> Result<()> { ThreadParamsMode::Embedded, ), ( - "model_reasoning_summary = 'detailed'\nmodel_verbosity = 'high'", + "model_reasoning_summary = 'detailed'\nmodel_verbosity = 'high'\nweb_search = 'live'", json!("detailed"), json!(null), ThreadParamsMode::Remote, @@ -75,16 +73,25 @@ stream_max_retries = 0 codex_protocol::config_types::TrustLevel::Trusted, ) .map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?; - let mut config = ConfigBuilder::default() + let server_config = ConfigBuilder::default() .codex_home(home.path().to_path_buf()) .build() .await?; - let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?; + let client_home = tempfile::tempdir()?; + let config = if mode == ThreadParamsMode::Remote { + std::fs::write( + client_home.path().join("config.toml"), + "model_reasoning_summary = 'concise'\nmodel_verbosity = 'low'\nweb_search = 'disabled'", + )?; + ConfigBuilder::default() + .codex_home(client_home.path().to_path_buf()) + .build() + .await? + } else { + server_config.clone() + }; + let mut app_server = crate::start_embedded_app_server_for_picker(&server_config).await?; app_server.thread_params_mode = mode; - if mode == ThreadParamsMode::Remote { - config.model_reasoning_summary = Some(ReasoningSummary::Concise); - config.model_verbosity = Some(Verbosity::Low); - } let started = app_server.start_thread(&config).await?; for resume in [false, true] { if resume { @@ -92,7 +99,7 @@ stream_max_retries = 0 break; } app_server.shutdown().await?; - app_server = crate::start_embedded_app_server_for_picker(&config).await?; + app_server = crate::start_embedded_app_server_for_picker(&server_config).await?; app_server.thread_params_mode = mode; app_server .resume_thread( @@ -156,6 +163,13 @@ stream_max_retries = 0 ); if mode == ThreadParamsMode::Remote { assert_eq!(body["text"]["verbosity"], json!("high")); + let web_search = body["tools"] + .as_array() + .expect("tools") + .iter() + .find(|tool| tool["type"] == "web_search") + .expect("server's web search tool"); + assert_eq!(web_search["external_web_access"], json!(true)); } let metadata: serde_json::Value = serde_json::from_str( body["client_metadata"]["x-codex-turn-metadata"] diff --git a/codex-rs/tui/src/app_server_session/web_search.rs b/codex-rs/tui/src/app_server_session/web_search.rs new file mode 100644 index 0000000000..8502790b1e --- /dev/null +++ b/codex-rs/tui/src/app_server_session/web_search.rs @@ -0,0 +1,100 @@ +//! Forward only winning launch search choices, preserving legacy precedence and embedded pins. + +use super::ThreadParamsMode; +use crate::legacy_core::config::Config; +use codex_config::ConfigLayerMetadata; +use codex_config::ConfigLayerSource; +use codex_features::Feature; +use std::collections::HashMap; + +pub(super) fn apply_launch_override( + config: &Config, + thread_params_mode: ThreadParamsMode, + effective: &toml::Value, + origins: &HashMap, + overrides: &mut HashMap, +) { + // Losing legacy flags must not independently change the server's search mode. + if let Some(features) = overrides + .get_mut("features") + .and_then(serde_json::Value::as_object_mut) + { + for key in ["web_search", "web_search_cached", "web_search_request"] { + features.remove(key); + } + if features.is_empty() { + overrides.remove("features"); + } + } + let is_launch = |key: &str| { + origins.get(key).is_some_and(|origin| { + matches!( + origin.name, + ConfigLayerSource::SessionFlags + | ConfigLayerSource::User { + profile: Some(_), + .. + } + ) + }) + }; + // A canonical setting always takes precedence, even when its origin is implicit. + if let Some(value) = effective.get("web_search") { + if is_launch("web_search") { + overrides.insert("web_search".to_string(), serde_json::json!(value)); + } + return; + } + let Some(features) = effective.get("features").and_then(toml::Value::as_table) else { + return; + }; + let request_key = if features.contains_key("web_search_request") { + "web_search_request" + } else { + "web_search" + }; + let cached = config.features.enabled(Feature::WebSearchCached); + let live = config.features.enabled(Feature::WebSearchRequest); + let active = [("web_search_cached", "cached"), (request_key, "live")] + .into_iter() + .find(|(key, mode)| { + features.get(*key).and_then(toml::Value::as_bool) == Some(true) + && (thread_params_mode == ThreadParamsMode::Remote + || if *mode == "cached" { + cached + } else { + live && !cached + }) + }); + let (key, mode) = match active { + Some(choice) => choice, + None => { + if thread_params_mode == ThreadParamsMode::Embedded && (cached || live) { + return; + } + // Explicitly disabling all legacy search features falls back to cached search. + let Some(key) = ["web_search_cached", request_key].into_iter().find(|key| { + features.get(*key).and_then(toml::Value::as_bool) == Some(false) + && is_launch(&format!("features.{key}")) + }) else { + return; + }; + (key, "cached") + } + }; + // Remote servers enforce their own search-mode requirements on this raw choice. + if is_launch(&format!("features.{key}")) + || (mode == "live" + && features + .get("web_search_cached") + .and_then(toml::Value::as_bool) + == Some(false) + && is_launch("features.web_search_cached")) + { + overrides.insert("web_search".to_string(), mode.into()); + } +} + +#[cfg(test)] +#[path = "web_search_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app_server_session/web_search_tests.rs b/codex-rs/tui/src/app_server_session/web_search_tests.rs new file mode 100644 index 0000000000..805f6a7098 --- /dev/null +++ b/codex-rs/tui/src/app_server_session/web_search_tests.rs @@ -0,0 +1,150 @@ +use super::super::ThreadParamsMode; +use super::super::config_request_overrides_from_config; +use crate::legacy_core::config::ConfigBuilder; +use color_eyre::Result; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn legacy_search_overrides_respect_launch_origins() -> Result<()> { + let home = tempfile::tempdir()?; + for (settings, flags, expected) in [ + ( + "web_search = 'disabled'", + "web_search_request = true\nweb_search_cached = true\nweb_search = true", + None, + ), + ("[features]\nweb_search_request = true", "", None), + ( + "[features]\nweb_search_cached = true\nweb_search_request = true", + "web_search_cached = false", + Some("live"), + ), + ] { + std::fs::write(home.path().join("config.toml"), settings)?; + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .cli_overrides( + toml::from_str::(&format!("[features]\nmulti_agent = true\n{flags}"))? + .into_iter() + .collect(), + ) + .loader_overrides(codex_config::LoaderOverrides::without_managed_config_for_tests()) + .build() + .await?; + let overrides = config_request_overrides_from_config(&config, ThreadParamsMode::Remote) + .expect("config overrides"); + assert_eq!( + overrides + .get("web_search") + .and_then(serde_json::Value::as_str), + expected, + ); + assert_eq!( + overrides.get("features"), + Some(&serde_json::json!({"multi_agent": true})), + ); + } + Ok(()) +} + +#[tokio::test] +async fn web_search_overrides_preserve_explicit_legacy_choices() -> Result<()> { + let home = tempfile::tempdir()?; + for (settings, expected) in [ + ("[features]\nweb_search_request = true", "live"), + ( + "[features]\nweb_search_cached = true\nweb_search_request = true", + "cached", + ), + ("[features]\nweb_search = true", "live"), + ( + "[features]\nweb_search = true\nweb_search_request = false", + "cached", + ), + ( + "web_search = 'disabled'\n[features]\nweb_search_request = true", + "disabled", + ), + ] { + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .cli_overrides( + toml::from_str::(settings)? + .into_iter() + .collect(), + ) + .loader_overrides(codex_config::LoaderOverrides::without_managed_config_for_tests()) + .build() + .await?; + assert_eq!( + config_request_overrides_from_config(&config, ThreadParamsMode::Remote), + Some(std::collections::HashMap::from([( + "web_search".to_string(), + serde_json::json!(expected), + )])), + "{settings}", + ); + } + Ok(()) +} + +#[tokio::test] +async fn embedded_legacy_search_honors_shared_feature_requirements() -> Result<()> { + let home = tempfile::tempdir()?; + let requirements_path = home.path().join("requirements.toml"); + for (requirements, flags, embedded_mode, remote_mode) in [ + ( + "web_search_request = false", + "web_search_request = true", + None, + "live", + ), + ( + "web_search_cached = true", + "web_search_request = true", + None, + "live", + ), + ( + "web_search_request = true", + "web_search_request = false", + None, + "cached", + ), + ( + "web_search_cached = false", + "web_search_cached = true\nweb_search_request = true", + Some("live"), + "cached", + ), + ] { + std::fs::write(&requirements_path, format!("[features]\n{requirements}"))?; + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .cli_overrides( + toml::from_str::(&format!("[features]\n{flags}"))? + .into_iter() + .collect(), + ) + .loader_overrides(codex_config::LoaderOverrides { + system_requirements_path: Some(requirements_path.clone()), + ..codex_config::LoaderOverrides::without_managed_config_for_tests() + }) + .build() + .await?; + for (mode, expected) in [ + (ThreadParamsMode::Embedded, embedded_mode), + (ThreadParamsMode::Remote, Some(remote_mode)), + ] { + let overrides = + config_request_overrides_from_config(&config, mode).expect("config overrides"); + assert_eq!( + overrides + .get("web_search") + .and_then(serde_json::Value::as_str), + expected + ); + } + } + Ok(()) +}