mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Honor system proxy settings for standalone web search (#47142)
## Why Standalone web search used a default HTTP client, bypassing the configured system proxy route when `respect_system_proxy` was enabled. ## What changed Pass the configured `HttpClientFactory` to the web search tool and create its client asynchronously for the provider's search URL using the API route policy. Export and reuse `SEARCH_ENDPOINT` so route selection and search requests use the same endpoint. ## Testing Add an isolated regression test with environment proxies removed and a mock system proxy. Verify that standalone web search reaches the proxy and returns its result in the follow-up model request. GitOrigin-RevId: 3b64803a777b9d1eb0ccf694e06658b0fd562129
This commit is contained in:
Generated
+1
@@ -5114,6 +5114,7 @@ dependencies = [
|
||||
"codex-core",
|
||||
"codex-extension-api",
|
||||
"codex-extension-items",
|
||||
"codex-http-client",
|
||||
"codex-login",
|
||||
"codex-model-provider",
|
||||
"codex-model-provider-info",
|
||||
|
||||
@@ -30,4 +30,5 @@ pub use responses_websocket::ResponsesWebsocketClient;
|
||||
pub use responses_websocket::ResponsesWebsocketClose;
|
||||
pub use responses_websocket::ResponsesWebsocketConnection;
|
||||
pub use responses_websocket::ResponsesWebsocketProbe;
|
||||
pub use search::SEARCH_ENDPOINT;
|
||||
pub use search::SearchClient;
|
||||
|
||||
@@ -11,6 +11,9 @@ use http::Method;
|
||||
use serde_json::to_value;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// The provider-relative endpoint for standalone web search.
|
||||
pub const SEARCH_ENDPOINT: &str = "alpha/search";
|
||||
|
||||
pub struct SearchClient<T: HttpTransport> {
|
||||
session: EndpointSession<T>,
|
||||
}
|
||||
@@ -28,10 +31,6 @@ impl<T: HttpTransport> SearchClient<T> {
|
||||
}
|
||||
}
|
||||
|
||||
fn path() -> &'static str {
|
||||
"alpha/search"
|
||||
}
|
||||
|
||||
pub async fn search(
|
||||
&self,
|
||||
request: &SearchRequest,
|
||||
@@ -41,7 +40,7 @@ impl<T: HttpTransport> SearchClient<T> {
|
||||
.map_err(|e| ApiError::Stream(format!("failed to encode search request: {e}")))?;
|
||||
let resp = self
|
||||
.session
|
||||
.execute(Method::POST, Self::path(), extra_headers, Some(body))
|
||||
.execute(Method::POST, SEARCH_ENDPOINT, extra_headers, Some(body))
|
||||
.await?;
|
||||
serde_json::from_slice(&resp.body)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to decode search response: {e}")))
|
||||
|
||||
@@ -70,6 +70,7 @@ pub use crate::endpoint::ResponsesWebsocketClient;
|
||||
pub use crate::endpoint::ResponsesWebsocketClose;
|
||||
pub use crate::endpoint::ResponsesWebsocketConnection;
|
||||
pub use crate::endpoint::ResponsesWebsocketProbe;
|
||||
pub use crate::endpoint::SEARCH_ENDPOINT;
|
||||
pub use crate::endpoint::SearchClient;
|
||||
pub use crate::endpoint::session_update_session_json;
|
||||
pub use crate::error::ApiError;
|
||||
|
||||
@@ -219,6 +219,8 @@ mod user_notification;
|
||||
mod user_shell_cmd;
|
||||
mod view_image;
|
||||
mod web_search;
|
||||
#[path = "web_search_system_proxy_tests.rs"]
|
||||
mod web_search_system_proxy;
|
||||
mod websocket_fallback;
|
||||
mod window_headers;
|
||||
#[cfg(target_os = "windows")]
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
//! Standalone web search must use the configured system proxy route.
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::config::Config;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_features::Feature;
|
||||
use codex_http_client::cache_system_proxy_route_for_test;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_models_manager::bundled_models_response;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_web_search_extension::install as install_web_search_extension;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use tokio::process::Command;
|
||||
use tokio::time::timeout;
|
||||
use wiremock::Mock;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
const TEST_NAME: &str =
|
||||
"suite::web_search_system_proxy::standalone_web_search_honors_respect_system_proxy";
|
||||
const TEST_SUBPROCESS_ENV_VAR: &str = "CODEX_WEB_SEARCH_SYSTEM_PROXY_TEST_SUBPROCESS";
|
||||
const API_BASE_URL: &str = "http://web-search-proxy.invalid/v1";
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn standalone_web_search_honors_respect_system_proxy() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if std::env::var_os(TEST_SUBPROCESS_ENV_VAR).is_none() {
|
||||
// Isolate the proxy cache and remove environment proxies so only the configured
|
||||
// system proxy route can reach the fake API origin.
|
||||
let mut command = Command::new(std::env::current_exe()?);
|
||||
command.arg("--exact").arg(TEST_NAME);
|
||||
for key in codex_network_proxy::PROXY_ENV_KEYS {
|
||||
command.env_remove(key);
|
||||
}
|
||||
let output = timeout(
|
||||
Duration::from_secs(120),
|
||||
command
|
||||
.kill_on_drop(true)
|
||||
.env(TEST_SUBPROCESS_ENV_VAR, "1")
|
||||
.output(),
|
||||
)
|
||||
.await??;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"subprocess test `{TEST_NAME}` failed\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let proxy = responses::start_mock_server().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/v1/alpha/search"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"output": "Search result through system proxy",
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&proxy)
|
||||
.await;
|
||||
responses::mount_sse_once(
|
||||
&proxy,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
"web-run-1",
|
||||
"web",
|
||||
"run",
|
||||
&json!({"search_query": [{"q": "standalone web search"}]}).to_string(),
|
||||
),
|
||||
responses::ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let follow_up_mock = responses::mount_sse_once(
|
||||
&proxy,
|
||||
responses::sse(vec![
|
||||
responses::ev_assistant_message("msg-1", "done"),
|
||||
responses::ev_completed("resp-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let auth = CodexAuth::from_api_key("dummy");
|
||||
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth.clone());
|
||||
let mut extension_builder = ExtensionRegistryBuilder::<Config>::new();
|
||||
install_web_search_extension(&mut extension_builder, auth_manager);
|
||||
let mut builder = test_codex()
|
||||
.with_auth(auth)
|
||||
.with_extensions(Arc::new(extension_builder.build()))
|
||||
.with_config(|config| {
|
||||
config.model_catalog =
|
||||
Some(bundled_models_response().expect("bundled models.json should parse"));
|
||||
config.model_provider.base_url = Some(API_BASE_URL.to_string());
|
||||
for feature in [Feature::RespectSystemProxy, Feature::StandaloneWebSearch] {
|
||||
config
|
||||
.features
|
||||
.enable(feature)
|
||||
.expect("test config should allow feature update");
|
||||
}
|
||||
config.respect_system_proxy = true;
|
||||
config
|
||||
.web_search_mode
|
||||
.set(WebSearchMode::Live)
|
||||
.expect("web search mode should be accepted");
|
||||
});
|
||||
let test = builder.build_with_auto_env(&proxy).await?;
|
||||
|
||||
for endpoint in ["responses", "alpha/search"] {
|
||||
cache_system_proxy_route_for_test(&format!("{API_BASE_URL}/{endpoint}"), proxy.uri());
|
||||
}
|
||||
test.submit_turn("search the web through the system proxy")
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
follow_up_mock
|
||||
.single_request()
|
||||
.function_call_output_content_and_success("web-run-1"),
|
||||
Some((Some("Search result through system proxy".to_string()), None))
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -17,6 +17,7 @@ codex-api = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
codex-extension-api = { workspace = true }
|
||||
codex-extension-items = { workspace = true }
|
||||
codex-http-client = { workspace = true }
|
||||
codex-login = { workspace = true }
|
||||
codex-model-provider = { workspace = true }
|
||||
codex-model-provider-info = { workspace = true }
|
||||
|
||||
@@ -17,6 +17,7 @@ use codex_extension_api::ThreadLifecycleContributor;
|
||||
use codex_extension_api::ThreadOriginator;
|
||||
use codex_extension_api::ThreadStartInput;
|
||||
use codex_extension_api::ToolContributor;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_login::AuthManager;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
@@ -33,6 +34,7 @@ struct WebSearchExtension {
|
||||
#[derive(Clone)]
|
||||
struct WebSearchExtensionConfig {
|
||||
available: bool,
|
||||
http_client_factory: HttpClientFactory,
|
||||
provider: ModelProviderInfo,
|
||||
settings: SearchSettings,
|
||||
}
|
||||
@@ -46,6 +48,7 @@ impl From<&Config> for WebSearchExtensionConfig {
|
||||
|| config.model_provider.uses_openai_actor_authorization()
|
||||
|| config.model_provider.supports_standalone_web_search)
|
||||
&& web_search_mode != WebSearchMode::Disabled,
|
||||
http_client_factory: config.http_client_factory(),
|
||||
provider: config.model_provider.clone(),
|
||||
settings: search_settings(config, web_search_mode),
|
||||
}
|
||||
@@ -133,6 +136,7 @@ impl ToolContributor for WebSearchExtension {
|
||||
|
||||
vec![Arc::new(WebSearchTool {
|
||||
session_id: session_store.level_id().to_string(),
|
||||
http_client_factory: config.http_client_factory.clone(),
|
||||
provider: create_model_provider(
|
||||
config.provider.clone(),
|
||||
Some(self.auth_manager.clone()),
|
||||
@@ -157,6 +161,8 @@ mod tests {
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_extension_api::ToolName;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -203,6 +209,7 @@ mod tests {
|
||||
let thread_store = ExtensionData::new("11111111-1111-4111-8111-111111111111");
|
||||
thread_store.insert(WebSearchExtensionConfig {
|
||||
available: true,
|
||||
http_client_factory: HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
provider: ModelProviderInfo::create_openai_provider(/*base_url*/ None),
|
||||
settings: Default::default(),
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use codex_api::ReqwestTransport;
|
||||
use codex_api::SEARCH_ENDPOINT;
|
||||
use codex_api::SearchClient;
|
||||
use codex_api::SearchCommands;
|
||||
use codex_api::SearchQuery;
|
||||
@@ -18,8 +19,11 @@ use codex_extension_api::parse_tool_input_schema_without_compaction;
|
||||
use codex_extension_items::ExtensionItem;
|
||||
use codex_extension_items::web_search::WebSearchAction;
|
||||
use codex_extension_items::web_search::WebSearchItem;
|
||||
use codex_http_client::ClientRouteClass;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_login::default_client::ClientRedirectPolicy;
|
||||
use codex_login::default_client::add_originator_header;
|
||||
use codex_login::default_client::create_client;
|
||||
use codex_login::default_client::create_client_for_route_async;
|
||||
use codex_model_provider::SharedModelProvider;
|
||||
use codex_protocol::models::WebSearchAction as CoreWebSearchAction;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -44,6 +48,7 @@ const RESULTS_PAYLOAD_BYTES_METRIC: &str = "codex.web_search.results.payload_byt
|
||||
|
||||
pub(crate) struct WebSearchTool {
|
||||
pub(crate) session_id: String,
|
||||
pub(crate) http_client_factory: HttpClientFactory,
|
||||
pub(crate) provider: SharedModelProvider,
|
||||
pub(crate) settings: SearchSettings,
|
||||
pub(crate) originator: Option<String>,
|
||||
@@ -108,8 +113,16 @@ impl WebSearchTool {
|
||||
.api_auth()
|
||||
.await
|
||||
.map_err(|err| FunctionCallError::Fatal(err.to_string()))?;
|
||||
let http_client = create_client_for_route_async(
|
||||
self.http_client_factory.clone(),
|
||||
provider.url_for_path(SEARCH_ENDPOINT),
|
||||
ClientRouteClass::Api,
|
||||
ClientRedirectPolicy::Default,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| FunctionCallError::Fatal(err.to_string()))?;
|
||||
let client = SearchClient::new(
|
||||
ReqwestTransport::from_http_client(create_client()),
|
||||
ReqwestTransport::from_http_client(http_client),
|
||||
provider,
|
||||
auth,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user