mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Route Ollama through the shared HTTP client (#36078)
## Why Ollama requests need to honor Codex's configured outbound proxy policy and custom CA handling. ## What changed - Replace Ollama's direct `reqwest` client with a route-aware client created by `codex-http-client`. - Preserve the five-second connection timeout and legacy system-root fallback for default-routed requests. - Reuse one Ollama client for connectivity, version, model discovery, and model-pull checks. - Surface HTTP transport initialization errors instead of replacing them with the generic Ollama connection error. ## Testing - Cover system-proxy routing and invalid `CODEX_CA_CERTIFICATE` and `SSL_CERT_FILE` values under both outbound proxy policies. - Verify that version and model checks reuse the existing Ollama client. GitOrigin-RevId: c7cc36845a9bceb57ce5524b9e1b3cbe317897a3
This commit is contained in:
2
codex-rs/Cargo.lock
generated
2
codex-rs/Cargo.lock
generated
@@ -3732,11 +3732,11 @@ dependencies = [
|
||||
"async-stream",
|
||||
"bytes",
|
||||
"codex-core",
|
||||
"codex-http-client",
|
||||
"codex-model-provider-info",
|
||||
"futures",
|
||||
"memchr",
|
||||
"pretty_assertions",
|
||||
"reqwest 0.12.28",
|
||||
"semver",
|
||||
"serde_json",
|
||||
"tokio",
|
||||
|
||||
@@ -243,7 +243,6 @@ deny = [
|
||||
"codex-responses-api-proxy",
|
||||
# Temporary migration exceptions.
|
||||
"codex-app-server",
|
||||
"codex-ollama",
|
||||
"codex-otel",
|
||||
# Third-party crates that own their reqwest integration. These are not part of the
|
||||
# first-party migration count above.
|
||||
|
||||
@@ -16,10 +16,10 @@ workspace = true
|
||||
async-stream = { workspace = true }
|
||||
bytes = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
codex-http-client = { workspace = true }
|
||||
codex-model-provider-info = { workspace = true }
|
||||
futures = { workspace = true }
|
||||
memchr = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
semver = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
tokio = { workspace = true, features = [
|
||||
|
||||
@@ -4,6 +4,7 @@ use semver::Version;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::collections::VecDeque;
|
||||
use std::io;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::line_buffer::LineBuffer;
|
||||
use crate::parser::pull_events_from_value;
|
||||
@@ -12,6 +13,12 @@ use crate::pull::PullProgressReporter;
|
||||
use crate::url::base_url_to_host_root;
|
||||
use crate::url::is_openai_compatible_base_url;
|
||||
use codex_core::config::Config;
|
||||
use codex_http_client::ClientRouteClass;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
#[cfg(test)]
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_http_client::RouteAwareClientPool;
|
||||
use codex_http_client::RouteAwareRequestError;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_model_provider_info::OLLAMA_OSS_PROVIDER_ID;
|
||||
#[cfg(test)]
|
||||
@@ -20,10 +27,11 @@ use codex_model_provider_info::WireApi;
|
||||
use codex_model_provider_info::create_oss_provider_with_base_url;
|
||||
|
||||
const OLLAMA_CONNECTION_ERROR: &str = "No running Ollama server detected. Start it with: `ollama serve` (after installing). Install instructions: https://github.com/ollama/ollama?tab=readme-ov-file#ollama";
|
||||
const OLLAMA_CONNECTION_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
|
||||
/// Client for interacting with a local Ollama instance.
|
||||
pub struct OllamaClient {
|
||||
client: reqwest::Client,
|
||||
client: RouteAwareClientPool,
|
||||
host_root: String,
|
||||
uses_openai_compat: bool,
|
||||
}
|
||||
@@ -46,17 +54,24 @@ impl OllamaClient {
|
||||
)
|
||||
})?;
|
||||
|
||||
Self::try_from_provider(provider).await
|
||||
Self::try_from_provider(provider, config.http_client_factory()).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
async fn try_from_provider_with_base_url(base_url: &str) -> io::Result<Self> {
|
||||
let provider = create_oss_provider_with_base_url(base_url, WireApi::Responses);
|
||||
Self::try_from_provider(&provider).await
|
||||
Self::try_from_provider(
|
||||
&provider,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Build a client from a provider definition and verify the server is reachable.
|
||||
pub(crate) async fn try_from_provider(provider: &ModelProviderInfo) -> io::Result<Self> {
|
||||
pub(crate) async fn try_from_provider(
|
||||
provider: &ModelProviderInfo,
|
||||
http_client_factory: HttpClientFactory,
|
||||
) -> io::Result<Self> {
|
||||
#![expect(clippy::expect_used)]
|
||||
let base_url = provider
|
||||
.base_url
|
||||
@@ -64,10 +79,12 @@ impl OllamaClient {
|
||||
.expect("oss provider must have a base_url");
|
||||
let uses_openai_compat = is_openai_compatible_base_url(base_url);
|
||||
let host_root = base_url_to_host_root(base_url);
|
||||
let client = reqwest::Client::builder()
|
||||
.connect_timeout(std::time::Duration::from_secs(5))
|
||||
.build()
|
||||
.unwrap_or_else(|_| reqwest::Client::new());
|
||||
let client = RouteAwareClientPool::with_connect_timeout(
|
||||
http_client_factory,
|
||||
ClientRouteClass::Other,
|
||||
OLLAMA_CONNECTION_TIMEOUT,
|
||||
)
|
||||
.with_legacy_custom_ca_fallback();
|
||||
let client = Self {
|
||||
client,
|
||||
host_root,
|
||||
@@ -84,10 +101,21 @@ impl OllamaClient {
|
||||
} else {
|
||||
format!("{}/api/tags", self.host_root.trim_end_matches('/'))
|
||||
};
|
||||
let resp = self.client.get(url).send().await.map_err(|err| {
|
||||
tracing::warn!("Failed to connect to Ollama server: {err:?}");
|
||||
io::Error::other(OLLAMA_CONNECTION_ERROR)
|
||||
})?;
|
||||
let resp = self
|
||||
.client
|
||||
.get(url)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|error| match error {
|
||||
RouteAwareRequestError::Route(error) => {
|
||||
tracing::warn!(error = %error, "Failed to initialize Ollama HTTP transport");
|
||||
io::Error::other(error)
|
||||
}
|
||||
error => {
|
||||
tracing::warn!(error = ?error, "Failed to connect to Ollama server");
|
||||
io::Error::other(OLLAMA_CONNECTION_ERROR)
|
||||
}
|
||||
})?;
|
||||
if resp.status().is_success() {
|
||||
Ok(())
|
||||
} else {
|
||||
@@ -247,10 +275,11 @@ impl OllamaClient {
|
||||
/// Low-level constructor given a raw host root, e.g. "http://localhost:11434".
|
||||
#[cfg(test)]
|
||||
fn from_host_root(host_root: impl Into<String>) -> Self {
|
||||
let client = reqwest::Client::builder()
|
||||
.connect_timeout(std::time::Duration::from_secs(5))
|
||||
.build()
|
||||
.unwrap_or_else(|_| reqwest::Client::new());
|
||||
let client = RouteAwareClientPool::with_connect_timeout(
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
ClientRouteClass::Other,
|
||||
OLLAMA_CONNECTION_TIMEOUT,
|
||||
);
|
||||
Self {
|
||||
client,
|
||||
host_root: host_root.into(),
|
||||
@@ -438,6 +467,146 @@ mod tests {
|
||||
.expect("client should be created when probe succeeds");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_try_from_provider_preserves_outbound_proxy_policy() {
|
||||
if std::env::var(codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
tracing::info!(
|
||||
"{} set; skipping test_try_from_provider_preserves_outbound_proxy_policy",
|
||||
codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let proxy = wiremock::MockServer::start().await;
|
||||
let base_url = "http://ollama-proxy.invalid";
|
||||
let request_url = format!("{base_url}/api/tags");
|
||||
codex_http_client::cache_system_proxy_route_for_test(&request_url, proxy.uri());
|
||||
|
||||
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||
.and(wiremock::matchers::path("/api/tags"))
|
||||
.respond_with(wiremock::ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&proxy)
|
||||
.await;
|
||||
|
||||
let provider = create_oss_provider_with_base_url(base_url, WireApi::Responses);
|
||||
let client = OllamaClient::try_from_provider(
|
||||
&provider,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::RespectSystemProxy),
|
||||
)
|
||||
.await
|
||||
.expect("client should preserve the configured outbound proxy policy");
|
||||
|
||||
assert_eq!(
|
||||
client.client.outbound_proxy_policy(),
|
||||
OutboundProxyPolicy::RespectSystemProxy
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_try_from_provider_handles_invalid_custom_ca_by_proxy_policy() {
|
||||
const CHILD_POLICY_ENV: &str = "CODEX_OLLAMA_INVALID_CA_TEST_POLICY";
|
||||
|
||||
if std::env::var(codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
tracing::info!(
|
||||
"{} set; skipping test_try_from_provider_handles_invalid_custom_ca_by_proxy_policy",
|
||||
codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let Ok(policy_name) = std::env::var(CHILD_POLICY_ENV) else {
|
||||
let invalid_ca_path = std::env::temp_dir().join(format!(
|
||||
"codex-ollama-invalid-ca-{}.pem",
|
||||
std::process::id()
|
||||
));
|
||||
std::fs::write(&invalid_ca_path, "not a PEM certificate")
|
||||
.expect("invalid CA fixture should be written");
|
||||
|
||||
for ca_env in ["CODEX_CA_CERTIFICATE", "SSL_CERT_FILE"] {
|
||||
for policy_name in ["reqwest-default", "respect-system-proxy"] {
|
||||
let output = std::process::Command::new(
|
||||
std::env::current_exe().expect("test executable should be available"),
|
||||
)
|
||||
.arg("--exact")
|
||||
.arg("client::tests::test_try_from_provider_handles_invalid_custom_ca_by_proxy_policy")
|
||||
.arg("--nocapture")
|
||||
.env_remove("CODEX_CA_CERTIFICATE")
|
||||
.env_remove("SSL_CERT_FILE")
|
||||
.env(ca_env, &invalid_ca_path)
|
||||
.env(CHILD_POLICY_ENV, policy_name)
|
||||
.output()
|
||||
.expect("isolated CA subprocess should run");
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"{policy_name} failed with invalid {ca_env}\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
std::fs::remove_file(invalid_ca_path).expect("invalid CA fixture should be removed");
|
||||
return;
|
||||
};
|
||||
|
||||
let outbound_proxy_policy = match policy_name.as_str() {
|
||||
"reqwest-default" => OutboundProxyPolicy::ReqwestDefault,
|
||||
"respect-system-proxy" => OutboundProxyPolicy::RespectSystemProxy,
|
||||
_ => panic!("unexpected test proxy policy: {policy_name}"),
|
||||
};
|
||||
let server = wiremock::MockServer::start().await;
|
||||
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||
.and(wiremock::matchers::path("/api/tags"))
|
||||
.respond_with(wiremock::ResponseTemplate::new(200))
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let provider = create_oss_provider_with_base_url(&server.uri(), WireApi::Responses);
|
||||
let result = OllamaClient::try_from_provider(
|
||||
&provider,
|
||||
HttpClientFactory::new(outbound_proxy_policy),
|
||||
)
|
||||
.await;
|
||||
|
||||
match outbound_proxy_policy {
|
||||
OutboundProxyPolicy::ReqwestDefault => {
|
||||
result.expect("default-routed Ollama should fall back to system roots");
|
||||
assert_eq!(
|
||||
server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("mock server should report requests")
|
||||
.len(),
|
||||
1
|
||||
);
|
||||
}
|
||||
OutboundProxyPolicy::RespectSystemProxy => {
|
||||
let error = result
|
||||
.err()
|
||||
.expect("system-proxy Ollama should reject invalid custom CAs");
|
||||
let ca_env = if std::env::var_os("CODEX_CA_CERTIFICATE").is_some() {
|
||||
"CODEX_CA_CERTIFICATE"
|
||||
} else {
|
||||
"SSL_CERT_FILE"
|
||||
};
|
||||
assert!(
|
||||
error.to_string().contains(ca_env),
|
||||
"expected actionable {ca_env} error, got: {error}"
|
||||
);
|
||||
assert_ne!(error.to_string(), OLLAMA_CONNECTION_ERROR);
|
||||
assert!(
|
||||
server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("mock server should report requests")
|
||||
.is_empty()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_try_from_oss_provider_err_when_server_missing() {
|
||||
if std::env::var(codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
|
||||
@@ -6,7 +6,6 @@ mod url;
|
||||
|
||||
pub use client::OllamaClient;
|
||||
use codex_core::config::Config;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
pub use pull::CliProgressReporter;
|
||||
pub use pull::PullEvent;
|
||||
pub use pull::PullProgressReporter;
|
||||
@@ -20,24 +19,19 @@ pub const DEFAULT_OSS_MODEL: &str = "gpt-oss:20b";
|
||||
///
|
||||
/// - Ensures a local Ollama server is reachable.
|
||||
/// - Checks if the model exists locally and pulls it if missing.
|
||||
pub async fn ensure_oss_ready(config: &Config) -> std::io::Result<()> {
|
||||
pub async fn ensure_oss_ready(config: &Config, client: &OllamaClient) -> std::io::Result<()> {
|
||||
// Only download when the requested model is the default OSS model (or when -m is not provided).
|
||||
let model = match config.model.as_ref() {
|
||||
Some(model) => model,
|
||||
None => DEFAULT_OSS_MODEL,
|
||||
};
|
||||
|
||||
// Verify local Ollama is reachable.
|
||||
let ollama_client = crate::OllamaClient::try_from_oss_provider(config).await?;
|
||||
|
||||
// If the model is not present locally, pull it.
|
||||
match ollama_client.fetch_models().await {
|
||||
match client.fetch_models().await {
|
||||
Ok(models) => {
|
||||
if !models.iter().any(|m| m == model) {
|
||||
let mut reporter = crate::CliProgressReporter::new();
|
||||
ollama_client
|
||||
.pull_with_reporter(model, &mut reporter)
|
||||
.await?;
|
||||
client.pull_with_reporter(model, &mut reporter).await?;
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
@@ -60,8 +54,7 @@ fn supports_responses(version: &Version) -> bool {
|
||||
/// Ensure the running Ollama server is new enough to support the Responses API.
|
||||
///
|
||||
/// Returns `Ok(())` when the version endpoint is missing or unparsable.
|
||||
pub async fn ensure_responses_supported(provider: &ModelProviderInfo) -> std::io::Result<()> {
|
||||
let client = crate::OllamaClient::try_from_provider(provider).await?;
|
||||
pub async fn ensure_responses_supported(client: &OllamaClient) -> std::io::Result<()> {
|
||||
let Some(version) = client.fetch_version().await? else {
|
||||
return Ok(());
|
||||
};
|
||||
@@ -79,6 +72,60 @@ pub async fn ensure_responses_supported(provider: &ModelProviderInfo) -> std::io
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_model_provider_info::WireApi;
|
||||
use codex_model_provider_info::create_oss_provider_with_base_url;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn version_check_reuses_existing_ollama_client() {
|
||||
if std::env::var(codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
tracing::info!(
|
||||
"{} set; skipping version_check_reuses_existing_ollama_client",
|
||||
codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let server = wiremock::MockServer::start().await;
|
||||
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||
.and(wiremock::matchers::path("/api/tags"))
|
||||
.respond_with(
|
||||
wiremock::ResponseTemplate::new(200)
|
||||
.set_body_json(serde_json::json!({"models": [{"name": "gpt-oss:20b"}]})),
|
||||
)
|
||||
.expect(2)
|
||||
.mount(&server)
|
||||
.await;
|
||||
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||
.and(wiremock::matchers::path("/api/version"))
|
||||
.respond_with(
|
||||
wiremock::ResponseTemplate::new(200)
|
||||
.set_body_json(serde_json::json!({"version": "0.14.1"})),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let provider = create_oss_provider_with_base_url(&server.uri(), WireApi::Responses);
|
||||
let client = OllamaClient::try_from_provider(
|
||||
&provider,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await
|
||||
.expect("create Ollama client");
|
||||
|
||||
ensure_responses_supported(&client)
|
||||
.await
|
||||
.expect("version check should reuse the existing client");
|
||||
assert_eq!(
|
||||
client.fetch_models().await.expect("fetch models"),
|
||||
vec!["gpt-oss:20b"]
|
||||
);
|
||||
|
||||
server.verify().await;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn supports_responses_for_dev_zero() {
|
||||
|
||||
@@ -25,8 +25,9 @@ pub async fn ensure_oss_provider_ready(
|
||||
.map_err(|e| std::io::Error::other(format!("OSS setup failed: {e}")))?;
|
||||
}
|
||||
OLLAMA_OSS_PROVIDER_ID => {
|
||||
codex_ollama::ensure_responses_supported(&config.model_provider).await?;
|
||||
codex_ollama::ensure_oss_ready(config)
|
||||
let client = codex_ollama::OllamaClient::try_from_oss_provider(config).await?;
|
||||
codex_ollama::ensure_responses_supported(&client).await?;
|
||||
codex_ollama::ensure_oss_ready(config, &client)
|
||||
.await
|
||||
.map_err(|e| std::io::Error::other(format!("OSS setup failed: {e}")))?;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user