From 739d9cb27614c001731a0e71068905123bfb7e36 Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Wed, 17 Jun 2026 18:50:15 +1000 Subject: [PATCH] Fix custom OpenAI provider dropping port when URL scheme is omitted (#9730) Signed-off-by: Michael Neale Signed-off-by: Douwe M Osinga Co-authored-by: Douwe M Osinga --- crates/goose/src/providers/api_client.rs | 4 ++ crates/goose/src/providers/openai.rs | 78 +++++++++++++++++++++++- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/goose/src/providers/api_client.rs b/crates/goose/src/providers/api_client.rs index f6bc62d47..940a0493c 100644 --- a/crates/goose/src/providers/api_client.rs +++ b/crates/goose/src/providers/api_client.rs @@ -313,6 +313,10 @@ impl ApiClient { }) } + pub fn host(&self) -> &str { + &self.host + } + fn rebuild_client(&mut self) -> Result<()> { let mut client_builder = Client::builder() .timeout(self.timeout) diff --git a/crates/goose/src/providers/openai.rs b/crates/goose/src/providers/openai.rs index 1b4d29e4d..1a28a6718 100644 --- a/crates/goose/src/providers/openai.rs +++ b/crates/goose/src/providers/openai.rs @@ -85,7 +85,37 @@ struct ParsedBaseUrl { from_base_url: bool, } +/// Ensure a base URL has an explicit scheme. +/// +/// Users frequently enter hosts like `localhost:1234` without a scheme. The +/// `url` crate parses such input as `scheme="localhost"`, `path="1234"`, +/// silently dropping both the host and the port. When no `://` is present we +/// prepend a sensible scheme (`http://` for local hosts, `https://` +/// otherwise) so the host and port survive parsing. +pub(crate) fn ensure_url_scheme(raw_url: &str) -> String { + let trimmed = raw_url.trim(); + if trimmed.contains("://") { + return trimmed.to_string(); + } + + let host_part = trimmed.split(['/', '?']).next().unwrap_or(trimmed); + let bare_host = if let Some(rest) = host_part.strip_prefix('[') { + rest.split(']').next().unwrap_or(rest) + } else { + host_part.split(':').next().unwrap_or(host_part) + }; + let is_local = bare_host == "localhost" + || bare_host == "127.0.0.1" + || bare_host == "0.0.0.0" + || bare_host == "::1"; + + let scheme = if is_local { "http" } else { "https" }; + format!("{}://{}", scheme, trimmed) +} + pub(crate) fn parse_openai_base_url(raw_url: &str) -> Result { + let raw_url = ensure_url_scheme(raw_url); + let raw_url = raw_url.as_str(); let parsed = url::Url::parse(raw_url) .map_err(|e| anyhow::anyhow!("Invalid OPENAI_BASE_URL '{}': {}", raw_url, e))?; @@ -397,7 +427,8 @@ impl OpenAiProvider { let global_config = crate::config::Config::global(); let api_key = Self::resolve_api_key(&config, &|key| global_config.get_secret(key))?; - let url = url::Url::parse(&config.base_url) + let normalized_base_url = ensure_url_scheme(&config.base_url); + let url = url::Url::parse(&normalized_base_url) .map_err(|e| anyhow::anyhow!("Invalid base URL '{}': {}", config.base_url, e))?; let host = if let Some(port) = url.port() { @@ -1421,6 +1452,51 @@ mod tests { } } + #[test] + fn ensure_url_scheme_adds_http_for_local_hosts() { + assert_eq!(ensure_url_scheme("localhost:1234"), "http://localhost:1234"); + assert_eq!( + ensure_url_scheme("127.0.0.1:8080/v1"), + "http://127.0.0.1:8080/v1" + ); + assert_eq!(ensure_url_scheme("0.0.0.0:3000"), "http://0.0.0.0:3000"); + assert_eq!(ensure_url_scheme("[::1]:1234"), "http://[::1]:1234"); + } + + #[test] + fn ensure_url_scheme_adds_https_for_remote_hosts() { + assert_eq!( + ensure_url_scheme("api.example.com:8443/v1"), + "https://api.example.com:8443/v1" + ); + assert_eq!(ensure_url_scheme("example.com"), "https://example.com"); + } + + #[test] + fn ensure_url_scheme_preserves_existing_scheme() { + assert_eq!( + ensure_url_scheme("http://localhost:1234"), + "http://localhost:1234" + ); + assert_eq!( + ensure_url_scheme("https://api.openai.com/v1"), + "https://api.openai.com/v1" + ); + } + + #[test] + fn from_custom_config_preserves_port_without_scheme() { + let mut config = + base_declarative_config(vec![ModelInfo::new("m1".to_string(), 128000)], None); + config.base_url = "localhost:1234".to_string(); + + let provider = + OpenAiProvider::from_custom_config(ModelConfig::new_or_fail("m1"), config).unwrap(); + + assert_eq!(provider.api_client.host(), "http://localhost:1234"); + assert_eq!(provider.base_path, "v1/chat/completions"); + } + #[tokio::test] async fn fetch_supported_models_static_only_skips_api() { // Any request to the mock returns 500 — if the fix calls the API, the test fails.