mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Preserve server web-search settings in the TUI (#49799)
## Why The TUI unconditionally forwarded its resolved `web_search` mode, allowing implicit client settings to replace server defaults or saved thread settings. ## What changed - Forward `web_search` on thread start, resume, and fork only when the winning choice comes from session flags or a selected profile. - Translate legacy search feature flags into a canonical `web_search` override while preserving precedence, and remove those flags from forwarded feature overrides. - Honor shared feature requirements for embedded sessions while letting remote servers enforce their own requirements on explicit search choices. ## Testing Add coverage for configuration origins, canonical and legacy search precedence, and embedded versus remote feature requirements. Extend the Responses integration test to verify that a remote server's live search setting survives thread start and resume when the client's configuration disables search. GitOrigin-RevId: c714f9cf0321aca9a5fa292a350720af5a2d02d4
This commit is contained in:
@@ -1097,9 +1097,6 @@ async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths()
|
|||||||
let codex_home = tempdir()?;
|
let codex_home = tempdir()?;
|
||||||
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
||||||
app.config.sqlite = SqliteConfig::new_for_testing(codex_home.path().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(
|
std::fs::write(
|
||||||
codex_home.path().join("config.toml"),
|
codex_home.path().join("config.toml"),
|
||||||
"web_search = \"disabled\"\n",
|
"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);
|
assert_eq!(starts.len(), 2);
|
||||||
for params in &starts {
|
for params in &starts {
|
||||||
assert_eq!(params["dynamicTools"], serde_json::Value::Null);
|
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"];
|
let server = ¶ms["config"]["mcp_servers.codex_tui"];
|
||||||
assert!(
|
assert!(
|
||||||
server["url"]
|
server["url"]
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ mod provider_selection_tests;
|
|||||||
mod realtime;
|
mod realtime;
|
||||||
mod rollout_history;
|
mod rollout_history;
|
||||||
mod thread_list;
|
mod thread_list;
|
||||||
|
mod web_search;
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
#[path = "app_server_session/collaboration_catalog_tests.rs"]
|
#[path = "app_server_session/collaboration_catalog_tests.rs"]
|
||||||
@@ -1831,6 +1832,7 @@ pub(crate) fn personality_opt_out_only(personality: Option<Personality>) -> Opti
|
|||||||
|
|
||||||
fn config_request_overrides_from_config(
|
fn config_request_overrides_from_config(
|
||||||
config: &Config,
|
config: &Config,
|
||||||
|
thread_params_mode: ThreadParamsMode,
|
||||||
) -> Option<HashMap<String, serde_json::Value>> {
|
) -> Option<HashMap<String, serde_json::Value>> {
|
||||||
let mut session_config = toml::Value::Table(toml::Table::new());
|
let mut session_config = toml::Value::Table(toml::Table::new());
|
||||||
for layer in config.config_layer_stack.layers_low_to_high() {
|
for layer in config.config_layer_stack.layers_low_to_high() {
|
||||||
@@ -1877,13 +1879,16 @@ fn config_request_overrides_from_config(
|
|||||||
"personality",
|
"personality",
|
||||||
personality_opt_out_only(config.personality).map(|personality| personality.to_string()),
|
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.
|
// Only winning launch choices may replace server defaults or saved thread settings.
|
||||||
let origins = config.config_layer_stack.origins();
|
let origins = config.config_layer_stack.origins();
|
||||||
let effective = config.config_layer_stack.effective_config();
|
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"] {
|
for key in ["model_reasoning_summary", "model_verbosity"] {
|
||||||
if origins.get(key).is_some_and(|origin| {
|
if origins.get(key).is_some_and(|origin| {
|
||||||
matches!(
|
matches!(
|
||||||
@@ -1921,7 +1926,8 @@ fn remove_permission_config_overrides(config: &mut Option<HashMap<String, serde_
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn new_thread_reasoning_overrides(config: &Config) -> Option<HashMap<String, serde_json::Value>> {
|
fn new_thread_reasoning_overrides(config: &Config) -> Option<HashMap<String, serde_json::Value>> {
|
||||||
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
|
let summary = config
|
||||||
.model_reasoning_summary
|
.model_reasoning_summary
|
||||||
.unwrap_or(codex_protocol::config_types::ReasoningSummary::None);
|
.unwrap_or(codex_protocol::config_types::ReasoningSummary::None);
|
||||||
@@ -2084,7 +2090,9 @@ pub(crate) fn thread_start_params_from_config(
|
|||||||
permissions,
|
permissions,
|
||||||
config: match thread_params_mode {
|
config: match thread_params_mode {
|
||||||
ThreadParamsMode::Embedded => new_thread_reasoning_overrides(config),
|
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),
|
ephemeral: Some(config.ephemeral),
|
||||||
history_mode: (!config.ephemeral).then_some(ThreadHistoryMode::Paginated),
|
history_mode: (!config.ephemeral).then_some(ThreadHistoryMode::Paginated),
|
||||||
@@ -2121,7 +2129,7 @@ fn thread_resume_params_from_config(
|
|||||||
)
|
)
|
||||||
})
|
})
|
||||||
.flatten();
|
.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
|
if model_settings == ResumeModelSettings::RestoreFromThread
|
||||||
&& let Some(overrides) = config_overrides.as_mut()
|
&& 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),
|
approvals_reviewer: approvals_reviewer_override_from_config(&config),
|
||||||
sandbox,
|
sandbox,
|
||||||
permissions,
|
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(|_| {
|
base_instructions: config.base_instructions.clone().filter(|_| {
|
||||||
!matches!(
|
!matches!(
|
||||||
config.base_instructions_provenance,
|
config.base_instructions_provenance,
|
||||||
@@ -3425,7 +3433,6 @@ mod tests {
|
|||||||
let string = |value: &str| serde_json::Value::String(value.to_string());
|
let string = |value: &str| serde_json::Value::String(value.to_string());
|
||||||
let expected_config = HashMap::from([
|
let expected_config = HashMap::from([
|
||||||
("model_reasoning_effort".to_string(), string("high")),
|
("model_reasoning_effort".to_string(), string("high")),
|
||||||
("web_search".to_string(), string("disabled")),
|
|
||||||
("bypass_hook_trust".to_string(), true.into()),
|
("bypass_hook_trust".to_string(), true.into()),
|
||||||
]);
|
]);
|
||||||
let mut expected_start_config = expected_config.clone();
|
let mut expected_start_config = expected_config.clone();
|
||||||
@@ -3440,34 +3447,54 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[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 home = tempfile::tempdir()?;
|
||||||
let workspace = home.path().join("workspace");
|
let workspace = home.path().join("workspace");
|
||||||
std::fs::create_dir_all(workspace.join(".codex"))?;
|
std::fs::create_dir_all(workspace.join(".codex"))?;
|
||||||
std::fs::write(
|
std::fs::write(
|
||||||
home.path().join("config.toml"),
|
home.path().join("config.toml"),
|
||||||
format!(
|
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()),
|
toml::Value::String(workspace.to_string_lossy().into_owned()),
|
||||||
),
|
),
|
||||||
)?;
|
)?;
|
||||||
let profile = AbsolutePathBuf::from_absolute_path(home.path().join("work.config.toml"))?;
|
let profile = AbsolutePathBuf::from_absolute_path(home.path().join("work.config.toml"))?;
|
||||||
std::fs::write(
|
std::fs::write(
|
||||||
&profile,
|
&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 [
|
for (selected_profile, project, cli, expected) in [
|
||||||
(false, false, false, [None, None]),
|
(false, false, false, [None, None, None]),
|
||||||
(true, false, false, [Some("detailed"), Some("high")]),
|
(
|
||||||
(false, false, true, [Some("auto"), Some("medium")]),
|
true,
|
||||||
(true, false, true, [Some("auto"), Some("medium")]),
|
false,
|
||||||
(true, true, false, [None, None]),
|
false,
|
||||||
(true, true, true, [Some("auto"), Some("medium")]),
|
[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(
|
std::fs::write(
|
||||||
workspace.join(".codex/config.toml"),
|
workspace.join(".codex/config.toml"),
|
||||||
if project {
|
if project {
|
||||||
"model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n"
|
"model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\nweb_search = \"disabled\"\n"
|
||||||
} else {
|
} else {
|
||||||
""
|
""
|
||||||
},
|
},
|
||||||
@@ -3487,16 +3514,17 @@ mod tests {
|
|||||||
vec![
|
vec![
|
||||||
("model_reasoning_summary".to_string(), "auto".into()),
|
("model_reasoning_summary".to_string(), "auto".into()),
|
||||||
("model_verbosity".to_string(), "medium".into()),
|
("model_verbosity".to_string(), "medium".into()),
|
||||||
|
("web_search".to_string(), "live".into()),
|
||||||
]
|
]
|
||||||
} else {
|
} else {
|
||||||
Vec::new()
|
Vec::new()
|
||||||
})
|
})
|
||||||
.build()
|
.build()
|
||||||
.await?;
|
.await?;
|
||||||
let overrides =
|
let overrides = config_request_overrides_from_config(&config, ThreadParamsMode::Remote)
|
||||||
config_request_overrides_from_config(&config).expect("config overrides");
|
.expect("config overrides");
|
||||||
assert_eq!(
|
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)),
|
.map(|key| overrides.get(key).and_then(serde_json::Value::as_str)),
|
||||||
expected,
|
expected,
|
||||||
);
|
);
|
||||||
@@ -3525,13 +3553,7 @@ mod tests {
|
|||||||
|
|
||||||
assert_eq!(params.model, None);
|
assert_eq!(params.model, None);
|
||||||
assert_eq!(params.model_provider, None);
|
assert_eq!(params.model_provider, None);
|
||||||
assert_eq!(
|
assert_eq!(params.config, None);
|
||||||
params.config,
|
|
||||||
Some(HashMap::from([(
|
|
||||||
"web_search".to_string(),
|
|
||||||
serde_json::Value::String("cached".to_string()),
|
|
||||||
)]))
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -3836,20 +3858,23 @@ mod tests {
|
|||||||
config.personality = None;
|
config.personality = None;
|
||||||
|
|
||||||
let implicit_overrides =
|
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"));
|
assert!(!implicit_overrides.contains_key("personality"));
|
||||||
|
|
||||||
for personality in [Personality::Friendly, Personality::Pragmatic] {
|
for personality in [Personality::Friendly, Personality::Pragmatic] {
|
||||||
config.personality = Some(personality);
|
config.personality = Some(personality);
|
||||||
let ordinary_overrides =
|
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"));
|
assert!(!ordinary_overrides.contains_key("personality"));
|
||||||
}
|
}
|
||||||
|
|
||||||
config.personality = Some(Personality::None);
|
config.personality = Some(Personality::None);
|
||||||
let explicit_overrides =
|
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!(
|
assert_eq!(
|
||||||
explicit_overrides.get("personality"),
|
explicit_overrides.get("personality"),
|
||||||
|
|||||||
@@ -3,8 +3,6 @@
|
|||||||
use super::*;
|
use super::*;
|
||||||
use crate::legacy_core::config::ConfigBuilder;
|
use crate::legacy_core::config::ConfigBuilder;
|
||||||
use codex_app_server_protocol::ServerNotification;
|
use codex_app_server_protocol::ServerNotification;
|
||||||
use codex_protocol::config_types::ReasoningSummary;
|
|
||||||
use codex_protocol::config_types::Verbosity;
|
|
||||||
use core_test_support::responses;
|
use core_test_support::responses;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
@@ -44,7 +42,7 @@ async fn reasoning_defaults_reach_responses() -> Result<()> {
|
|||||||
ThreadParamsMode::Embedded,
|
ThreadParamsMode::Embedded,
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
"model_reasoning_summary = 'detailed'\nmodel_verbosity = 'high'",
|
"model_reasoning_summary = 'detailed'\nmodel_verbosity = 'high'\nweb_search = 'live'",
|
||||||
json!("detailed"),
|
json!("detailed"),
|
||||||
json!(null),
|
json!(null),
|
||||||
ThreadParamsMode::Remote,
|
ThreadParamsMode::Remote,
|
||||||
@@ -75,16 +73,25 @@ stream_max_retries = 0
|
|||||||
codex_protocol::config_types::TrustLevel::Trusted,
|
codex_protocol::config_types::TrustLevel::Trusted,
|
||||||
)
|
)
|
||||||
.map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?;
|
.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())
|
.codex_home(home.path().to_path_buf())
|
||||||
.build()
|
.build()
|
||||||
.await?;
|
.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;
|
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?;
|
let started = app_server.start_thread(&config).await?;
|
||||||
for resume in [false, true] {
|
for resume in [false, true] {
|
||||||
if resume {
|
if resume {
|
||||||
@@ -92,7 +99,7 @@ stream_max_retries = 0
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
app_server.shutdown().await?;
|
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.thread_params_mode = mode;
|
||||||
app_server
|
app_server
|
||||||
.resume_thread(
|
.resume_thread(
|
||||||
@@ -156,6 +163,13 @@ stream_max_retries = 0
|
|||||||
);
|
);
|
||||||
if mode == ThreadParamsMode::Remote {
|
if mode == ThreadParamsMode::Remote {
|
||||||
assert_eq!(body["text"]["verbosity"], json!("high"));
|
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(
|
let metadata: serde_json::Value = serde_json::from_str(
|
||||||
body["client_metadata"]["x-codex-turn-metadata"]
|
body["client_metadata"]["x-codex-turn-metadata"]
|
||||||
|
|||||||
@@ -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<String, ConfigLayerMetadata>,
|
||||||
|
overrides: &mut HashMap<String, serde_json::Value>,
|
||||||
|
) {
|
||||||
|
// 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;
|
||||||
@@ -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::<toml::Table>(&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::<toml::Table>(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::<toml::Table>(&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(())
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user