diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 96577a70e2..9d4f5ab7cb 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -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", diff --git a/codex-rs/deny.toml b/codex-rs/deny.toml index bb1651ddad..d785e1f2f3 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -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. diff --git a/codex-rs/ollama/Cargo.toml b/codex-rs/ollama/Cargo.toml index 42e32159f2..29c0852092 100644 --- a/codex-rs/ollama/Cargo.toml +++ b/codex-rs/ollama/Cargo.toml @@ -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 = [ diff --git a/codex-rs/ollama/src/client.rs b/codex-rs/ollama/src/client.rs index 232c146836..9c45a6ec1a 100644 --- a/codex-rs/ollama/src/client.rs +++ b/codex-rs/ollama/src/client.rs @@ -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 { 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 { + pub(crate) async fn try_from_provider( + provider: &ModelProviderInfo, + http_client_factory: HttpClientFactory, + ) -> io::Result { #![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) -> 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() { diff --git a/codex-rs/ollama/src/lib.rs b/codex-rs/ollama/src/lib.rs index d0de830bcc..8b8fb4af44 100644 --- a/codex-rs/ollama/src/lib.rs +++ b/codex-rs/ollama/src/lib.rs @@ -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() { diff --git a/codex-rs/utils/oss/src/lib.rs b/codex-rs/utils/oss/src/lib.rs index 4a56af083f..5367a49ed6 100644 --- a/codex-rs/utils/oss/src/lib.rs +++ b/codex-rs/utils/oss/src/lib.rs @@ -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}")))?; }