mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Reuse the HTTP connection pool for remote plugin requests (#49100)
## Why Each call to `PluginsConfigInput::remote_plugin_service_config` created a new HTTP client pool, preventing connection reuse across service configurations. ## What changed Share one lazily initialized, route-aware pool across a `PluginsConfigInput` and its clones. Newly constructed inputs retain separate pools, while service configurations continue to use the current endpoint and product SKU and requests use the supplied authentication. ## Testing Add tests for lazy initialization, pool sharing across repeated calls and early clones, isolation between new inputs, and requests using updated endpoints, product SKUs, and authentication with the shared pool. GitOrigin-RevId: 3e929c34052798779a4f153e4db03b3f75b6f87d
This commit is contained in:
committed by
copyberry
parent
1260716393
commit
bfdb157178
@@ -14,6 +14,7 @@ use super::LoadedPlugin;
|
||||
use super::PluginLoadOutcome;
|
||||
use crate::PluginGitMode;
|
||||
use crate::app_mcp_routing::apply_app_mcp_routing_policy;
|
||||
use crate::http_client_selector::HttpClientSelector;
|
||||
use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack;
|
||||
use crate::is_openai_curated_marketplace_name;
|
||||
use crate::loaded_cache_metrics;
|
||||
@@ -58,6 +59,7 @@ use crate::marketplace_upgrade::ConfigLayerReload;
|
||||
use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError;
|
||||
use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome;
|
||||
use crate::marketplace_upgrade::upgrade_configured_git_marketplaces_with_mode;
|
||||
use crate::remote::CODEX_PRODUCT_SKU;
|
||||
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use crate::remote::RecommendedPluginsMode;
|
||||
use crate::remote::RemoteInstalledPlugin;
|
||||
@@ -98,7 +100,9 @@ use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use codex_connectors::ConnectorSnapshot;
|
||||
use codex_connectors::PluginConnectorSource;
|
||||
use codex_hooks::plugin_hook_declarations;
|
||||
use codex_http_client::ClientRouteClass;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::RouteAwareClientPool;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_plugin::AppConnectorId;
|
||||
@@ -128,6 +132,7 @@ use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
@@ -157,6 +162,7 @@ pub struct PluginsConfigInput {
|
||||
pub chatgpt_base_url: String,
|
||||
pub product_sku: Option<String>,
|
||||
http_client_factory: HttpClientFactory,
|
||||
remote_http_clients: Arc<OnceLock<Arc<dyn HttpClientSelector>>>,
|
||||
}
|
||||
|
||||
impl PluginsConfigInput {
|
||||
@@ -177,16 +183,29 @@ impl PluginsConfigInput {
|
||||
chatgpt_base_url,
|
||||
http_client_factory,
|
||||
product_sku,
|
||||
remote_http_clients: Arc::new(OnceLock::new()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds route-aware service state for remote plugin requests.
|
||||
/// Shares a lazy route-aware connection pool across this input and its clones.
|
||||
/// Request metadata remains current; new config inputs own a new pool.
|
||||
pub fn remote_plugin_service_config(&self) -> RemotePluginServiceConfig {
|
||||
RemotePluginServiceConfig::new(
|
||||
self.chatgpt_base_url.clone(),
|
||||
self.http_client_factory.clone(),
|
||||
self.product_sku.clone(),
|
||||
)
|
||||
let http_clients = self.remote_http_clients.get_or_init(|| {
|
||||
Arc::new(
|
||||
RouteAwareClientPool::with_chatgpt_cloudflare_cookies_without_request_logging(
|
||||
self.http_client_factory.clone(),
|
||||
ClientRouteClass::Api,
|
||||
),
|
||||
)
|
||||
});
|
||||
RemotePluginServiceConfig {
|
||||
chatgpt_base_url: self.chatgpt_base_url.clone(),
|
||||
product_sku: self
|
||||
.product_sku
|
||||
.clone()
|
||||
.unwrap_or_else(|| CODEX_PRODUCT_SKU.to_string()),
|
||||
http_clients: Arc::clone(http_clients),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3811,3 +3830,7 @@ impl PluginUninstallError {
|
||||
#[cfg(test)]
|
||||
#[path = "manager_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "plugins_config_input_tests.rs"]
|
||||
mod plugins_config_input_tests;
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
//! Verify connection reuse without retaining request metadata or crossing config lifetimes.
|
||||
|
||||
use super::PluginsConfigInput;
|
||||
use crate::remote::fetch_recommended_plugins;
|
||||
use crate::test_support::test_http_client_factory;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_login::CodexAuth;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::header;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
#[test]
|
||||
fn repeated_service_configs_and_early_clones_share_one_lazy_pool() {
|
||||
let input = PluginsConfigInput::new(
|
||||
ConfigLayerStack::default(),
|
||||
"openai".to_string(),
|
||||
/*plugins_enabled*/ true,
|
||||
/*remote_plugin_enabled*/ true,
|
||||
"https://chatgpt.com/backend-api".to_string(),
|
||||
test_http_client_factory(),
|
||||
/*product_sku*/ None,
|
||||
);
|
||||
let clone = input.clone();
|
||||
assert!(input.remote_http_clients.get().is_none());
|
||||
let first = input.remote_plugin_service_config();
|
||||
let reuse: Vec<_> = (0..220)
|
||||
.map(|_| {
|
||||
Arc::ptr_eq(
|
||||
&first.http_clients,
|
||||
&clone.remote_plugin_service_config().http_clients,
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let new_input = PluginsConfigInput::new(
|
||||
input.config_layer_stack.clone(),
|
||||
input.model_provider_id.clone(),
|
||||
input.plugins_enabled,
|
||||
input.remote_plugin_enabled,
|
||||
input.chatgpt_base_url.clone(),
|
||||
input.http_client_factory.clone(),
|
||||
input.product_sku,
|
||||
);
|
||||
assert_eq!(
|
||||
(
|
||||
reuse,
|
||||
Arc::ptr_eq(
|
||||
&first.http_clients,
|
||||
&new_input.remote_plugin_service_config().http_clients,
|
||||
),
|
||||
),
|
||||
(vec![true; 220], false)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reused_pool_uses_current_endpoint_product_and_authentication() {
|
||||
let first_server = MockServer::start().await;
|
||||
let second_server = MockServer::start().await;
|
||||
for (server, token, account, product_sku) in [
|
||||
(&first_server, "header.e30.first", "first-account", "codex"),
|
||||
(
|
||||
&second_server,
|
||||
"header.e30.second",
|
||||
"second-account",
|
||||
"updated",
|
||||
),
|
||||
] {
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/suggested/codex"))
|
||||
.and(header("authorization", format!("Bearer {token}")))
|
||||
.and(header("chatgpt-account-id", account))
|
||||
.and(header("oai-product-sku", product_sku))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"enabled": true,
|
||||
"plugins": [],
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(server)
|
||||
.await;
|
||||
}
|
||||
let mut input = PluginsConfigInput::new(
|
||||
ConfigLayerStack::default(),
|
||||
"openai".to_string(),
|
||||
/*plugins_enabled*/ true,
|
||||
/*remote_plugin_enabled*/ true,
|
||||
format!("{}/backend-api", first_server.uri()),
|
||||
test_http_client_factory(),
|
||||
/*product_sku*/ None,
|
||||
);
|
||||
let first = input.remote_plugin_service_config();
|
||||
let first_auth = CodexAuth::from_external_chatgpt_tokens(
|
||||
"header.e30.first",
|
||||
"first-account",
|
||||
/*chatgpt_plan_type*/ None,
|
||||
)
|
||||
.unwrap();
|
||||
let first_result = fetch_recommended_plugins(&first, Some(&first_auth))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
input.chatgpt_base_url = format!("{}/backend-api", second_server.uri());
|
||||
input.product_sku = Some("updated".to_string());
|
||||
let second = input.remote_plugin_service_config();
|
||||
let second_auth = CodexAuth::from_external_chatgpt_tokens(
|
||||
"header.e30.second",
|
||||
"second-account",
|
||||
/*chatgpt_plan_type*/ None,
|
||||
)
|
||||
.unwrap();
|
||||
let second_result = fetch_recommended_plugins(&second, Some(&second_auth))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
(
|
||||
first_result,
|
||||
Arc::ptr_eq(&first.http_clients, &second.http_clients),
|
||||
),
|
||||
(second_result, true)
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user