RomanNum3ral
/
claw-code
mirror of https://github.com/instructkr/claw-code.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: Ollama API compatibility — allow bare model names and smart openai/ prefix handling on wire 

fix: Ollama API compatibility — allow bare model names and smart openai/ prefix handling on wire
This commit is contained in:
Copilot 2026-05-27 19:45:03 +02:00 committed by GitHub
parent 87b7e74770
commit f6401edc85
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 341 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

184
rust/crates/api/src/providers/openai_compat.rs
View File

@ -932,6 +932,48 @@ fn strip_routing_prefix(model: &str) -> &str {
}
}
/// Normalize a base URL for comparison purposes.
///
/// Strips any trailing slashes and a trailing `/chat/completions` path
/// component so that the following variants are all treated as equivalent:
/// - `https://api.openai.com/v1`
/// - `https://api.openai.com/v1/`
/// - `https://api.openai.com/v1/chat/completions`
fn normalize_base_url(url: &str) -> &str {
let url = url.trim_end_matches('/');
url.strip_suffix("/chat/completions")
.map(|s| s.trim_end_matches('/'))
.unwrap_or(url)
}
/// Extract the host (without port) from a URL string.
/// Returns an empty string if the URL cannot be parsed.
fn url_host(url: &str) -> &str {
// Strip scheme ("https://", "http://", etc.)
let rest = match url.split_once("://") {
Some((_, r)) => r,
None => return "",
};
// Isolate the authority (before the first '/', '?', or '#')
let authority = rest.split(['/', '?', '#']).next().unwrap_or("");
// Strip optional userinfo (e.g. "user:pass@" in "user:pass@localhost:11434")
let authority = match authority.rsplit_once('@') {
Some((_, host_port)) => host_port,
None => authority,
};
if authority.starts_with('[') {
// IPv6 literal: host is between '[' and ']'
authority
.split(']')
.next()
.unwrap_or("")
.trim_start_matches('[')
} else {
// IPv4 or hostname: strip optional port
authority.split(':').next().unwrap_or("")
}
}
fn wire_model_for_base_url<'a>(
model: &'a str,
config: OpenAiCompatConfig,
@ -944,21 +986,24 @@ fn wire_model_for_base_url<'a>(
let lowered_prefix = prefix.to_ascii_lowercase();
if lowered_prefix == "openai" {
let trimmed_base_url = base_url.trim_end_matches('/');
let default_openai = DEFAULT_OPENAI_BASE_URL.trim_end_matches('/');
if matches!(
lowered_prefix.as_str(),
"xai" | "grok" | "kimi" | "gemini" | "gemma"
) {
// The `openai/` prefix is a claw-code routing hint. Whether to strip it
// depends on the target endpoint:
//
// - Default OpenAI endpoint: strip (it is only a routing prefix here).
// - Known-local endpoints (localhost / 127.0.0.1 / ::1, e.g. Ollama,
// LM Studio): strip because local servers use bare model names.
// - Custom non-local endpoints (OpenRouter, other gateways): preserve
// the full slug so the gateway receives the model ID it expects
// (e.g. `openai/gpt-4.1-mini` for OpenRouter).
let is_default_url = normalize_base_url(base_url)
.eq_ignore_ascii_case(normalize_base_url(config.default_base_url));
let host = url_host(base_url);
let is_local_url =
host.eq_ignore_ascii_case("localhost") || matches!(host, "127.0.0.1" | "::1");
if is_default_url || is_local_url {
return Cow::Borrowed(&model[pos + 1..]);
}
if config.provider_name == "OpenAI" && trimmed_base_url != default_openai {
// Only preserve the full slug if it's NOT a model we want to strip
if !model.contains("gemini") && !model.contains("gemma") {
return Cow::Borrowed(model);
}
}
return Cow::Borrowed(&model[pos + 1..]);
return Cow::Borrowed(model);
}
if matches!(lowered_prefix.as_str(), "xai" | "grok" | "qwen" | "kimi") {
@ -2730,4 +2775,117 @@ mod tests {
assert_eq!(super::strip_routing_prefix("kimi-k2.5"), "kimi-k2.5"); // no prefix, unchanged
assert_eq!(super::strip_routing_prefix("kimi/kimi-k1.5"), "kimi-k1.5");
}
#[test]
fn wire_model_strips_openai_prefix_for_custom_base_url() {
// Issue #3123: Ollama models with openai/ prefix should have prefix
// stripped for the default OpenAI endpoint and for known-local endpoints,
// but preserved for custom non-local gateways (e.g. OpenRouter).
use std::borrow::Cow;
let ollama_url = "http://localhost:11434/v1";
let openrouter_url = "https://openrouter.ai/api/v1";
let config = super::OpenAiCompatConfig::openai();
// openai/ prefix stripped for known-local URL (Ollama)
assert_eq!(
super::wire_model_for_base_url("openai/qwen2.5-coder:7b", config, ollama_url),
Cow::Borrowed("qwen2.5-coder:7b")
);
// openai/ prefix stripped for 127.0.0.1
assert_eq!(
super::wire_model_for_base_url("openai/llama3.2", config, "http://127.0.0.1:11434/v1"),
Cow::Borrowed("llama3.2")
);
// openai/ prefix stripped for IPv6 loopback
assert_eq!(
super::wire_model_for_base_url("openai/llama3.2", config, "http://[::1]:11434/v1"),
Cow::Borrowed("llama3.2")
);
// openai/ prefix stripped for default OpenAI URL
assert_eq!(
super::wire_model_for_base_url("openai/gpt-4o", config, super::DEFAULT_OPENAI_BASE_URL),
Cow::Borrowed("gpt-4o")
);
// openai/ prefix preserved for custom non-local gateway (OpenRouter)
assert_eq!(
super::wire_model_for_base_url("openai/gpt-4.1-mini", config, openrouter_url),
Cow::Borrowed("openai/gpt-4.1-mini")
);
// openai/ prefix preserved for a domain that contains "localhost" as a substring
// (false-positive guard: must match the host exactly, not via substring)
assert_eq!(
super::wire_model_for_base_url(
"openai/gpt-4.1-mini",
config,
"https://not-localhost.example.com/v1"
),
Cow::Borrowed("openai/gpt-4.1-mini")
);
// Bare model names (no slash) pass through unchanged
assert_eq!(
super::wire_model_for_base_url("qwen2.5-coder:7b", config, ollama_url),
Cow::Borrowed("qwen2.5-coder:7b")
);
// xai/ prefix stripped
let xai_config = super::OpenAiCompatConfig::xai();
assert_eq!(
super::wire_model_for_base_url("xai/grok-3", xai_config, super::DEFAULT_XAI_BASE_URL),
Cow::Borrowed("grok-3")
);
// Regression: trailing slash on the default OpenAI URL must still strip openai/
assert_eq!(
super::wire_model_for_base_url("openai/gpt-4o", config, "https://api.openai.com/v1/"),
Cow::Borrowed("gpt-4o")
);
// Regression: full chat/completions path as base URL must still strip openai/
assert_eq!(
super::wire_model_for_base_url(
"openai/gpt-4o",
config,
"https://api.openai.com/v1/chat/completions"
),
Cow::Borrowed("gpt-4o")
);
// Regression: host matching is case-insensitive for default OpenAI URL
assert_eq!(
super::wire_model_for_base_url("openai/gpt-4o", config, "https://API.OPENAI.COM/v1"),
Cow::Borrowed("gpt-4o")
);
// Regression: host matching is case-insensitive for known-local URLs
assert_eq!(
super::wire_model_for_base_url("openai/llama3.2", config, "http://LOCALHOST:11434/v1"),
Cow::Borrowed("llama3.2")
);
// Regression: URLs with userinfo should still be recognized as local
assert_eq!(
super::wire_model_for_base_url(
"openai/llama3.2",
config,
"http://user:pass@localhost:11434/v1"
),
Cow::Borrowed("llama3.2")
);
// Regression: URLs with userinfo for non-local gateways should preserve prefix
assert_eq!(
super::wire_model_for_base_url(
"openai/gpt-4.1-mini",
config,
"https://user:pass@openrouter.ai/api/v1"
),
Cow::Borrowed("openai/gpt-4.1-mini")
);
}
}

43
rust/crates/api/tests/openai_compat_integration.rs
View File

@ -162,7 +162,7 @@ async fn send_message_preserves_deepseek_reasoning_content_before_text() {
}
#[tokio::test]
async fn custom_openai_gateway_preserves_slash_model_ids_and_extra_body_params() {
async fn custom_openai_gateway_strips_openai_prefix_and_preserves_extra_body_params() {
let state = Arc::new(Mutex::new(Vec::<CapturedRequest>::new()));
let body = concat!(
"{",
@ -206,7 +206,7 @@ async fn custom_openai_gateway_preserves_slash_model_ids_and_extra_body_params()
let captured = state.lock().await;
let request = captured.first().expect("captured request");
let body: serde_json::Value = serde_json::from_str(&request.body).expect("json body");
assert_eq!(body["model"], json!("openai/gpt-4.1-mini"));
assert_eq!(body["model"], json!("gpt-4.1-mini"));
assert_eq!(
body["web_search_options"],
json!({"search_context_size": "low"})
@ -214,6 +214,45 @@ async fn custom_openai_gateway_preserves_slash_model_ids_and_extra_body_params()
assert_eq!(body["parallel_tool_calls"], json!(false));
}
#[tokio::test]
async fn custom_openai_gateway_preserves_non_routing_slash_model_ids() {
let state = Arc::new(Mutex::new(Vec::<CapturedRequest>::new()));
let body = concat!(
"{",
"\"id\":\"chatcmpl_non_routing_slash\",",
"\"model\":\"my-org/my-fine-tuned-model\",",
"\"choices\":[{",
"\"message\":{\"role\":\"assistant\",\"content\":\"Custom model reply\",\"tool_calls\":[]},",
"\"finish_reason\":\"stop\"",
"}],",
"\"usage\":{\"prompt_tokens\":4,\"completion_tokens\":3}",
"}"
);
let server = spawn_server(
state.clone(),
vec![http_response("200 OK", "application/json", body)],
)
.await;
let client = OpenAiCompatClient::new("openai-test-key", OpenAiCompatConfig::openai())
.with_base_url(server.base_url());
let response = client
.send_message(&MessageRequest {
model: "my-org/my-fine-tuned-model".to_string(),
..sample_request(false)
})
.await
.expect("gateway request should succeed");
assert_eq!(response.model, "my-org/my-fine-tuned-model");
assert_eq!(response.total_tokens(), 7);
let captured = state.lock().await;
let request = captured.first().expect("captured request");
let body: serde_json::Value = serde_json::from_str(&request.body).expect("json body");
assert_eq!(body["model"], json!("my-org/my-fine-tuned-model"));
}
#[tokio::test]
async fn send_message_blocks_oversized_xai_requests_before_the_http_call() {
let state = Arc::new(Mutex::new(Vec::<CapturedRequest>::new()));

138
rust/crates/rusty-claude-cli/src/main.rs
View File

@ -1926,9 +1926,41 @@ fn resolve_model_alias_with_config(model: &str) -> String {
resolve_model_alias(trimmed).to_string()
}
/// Validate model syntax at parse time.
/// Accepts: known aliases (opus, sonnet, haiku) or provider/model pattern.
/// Rejects: empty, whitespace-only, strings with spaces, or invalid chars.
/// Returns true if the given base URL points to a loopback/local host
/// (localhost, 127.0.0.1, or ::1).
fn is_local_base_url(url: &str) -> bool {
let rest = match url.split_once("://") {
Some((_, r)) => r,
None => return false,
};
let authority = rest.split(['/', '?', '#']).next().unwrap_or("");
// Strip optional userinfo (e.g. "user:pass@")
let authority = match authority.rsplit_once('@') {
Some((_, host_port)) => host_port,
None => authority,
};
let host = if authority.starts_with('[') {
// IPv6 literal
authority
.split(']')
.next()
.unwrap_or("")
.trim_start_matches('[')
} else {
authority.split(':').next().unwrap_or("")
};
host.eq_ignore_ascii_case("localhost") || matches!(host, "127.0.0.1" | "::1")
}
/// Validate model syntax at parse time. Callers must resolve model aliases
/// (e.g. "opus" → "anthropic/claude-opus-4-6") before calling this function;
/// raw aliases are rejected.
///
/// Accepts: `provider/model` format (e.g. "anthropic/claude-opus-4-6"),
/// or bare model names when `OPENAI_BASE_URL` points to a loopback host
/// (for local providers like Ollama, LM Studio, vLLM — e.g. "qwen2.5-coder:7b").
/// Rejects: empty or whitespace-only strings, strings containing spaces,
/// and bare model names when no loopback `OPENAI_BASE_URL` is configured.
fn validate_model_syntax(model: &str) -> Result<(), String> {
let trimmed = model.trim();
if trimmed.is_empty() {
@ -1944,6 +1976,19 @@ fn validate_model_syntax(model: &str) -> Result<(), String> {
// Check provider/model format: provider_id/model_id
let parts: Vec<&str> = trimmed.split('/').collect();
if parts.len() != 2 || parts[0].is_empty() || parts[1].is_empty() {
// When OPENAI_BASE_URL points to a loopback/local endpoint
// (Ollama, LM Studio, vLLM, etc.), allow bare model names
// (e.g. "qwen2.5-coder:7b", "llama3:8b") that don't follow the
// provider/model convention. Only bypass for local hosts to avoid
// masking errors for non-local gateways (e.g. OpenRouter) where
// a slash-containing slug is typically required.
if parts.len() == 1 {
if let Ok(base_url) = std::env::var("OPENAI_BASE_URL") {
if is_local_base_url(&base_url) {
return Ok(());
}
}
}
// #154: hint if the model looks like it belongs to a different provider
let mut err_msg = format!(
"invalid model syntax: '{}'.\nExpected provider/model (e.g., anthropic/claude-opus-4-7)",
@ -11334,6 +11379,15 @@ fn print_help(output_format: CliOutputFormat) -> Result<(), Box<dyn std::error::
Ok(())
}
#[cfg(test)]
fn env_lock() -> std::sync::MutexGuard<'static, ()> {
use std::sync::{Mutex, OnceLock};
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
LOCK.get_or_init(|| Mutex::new(()))
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}
#[cfg(test)]
mod tests {
use super::{
@ -11620,11 +11674,8 @@ mod tests {
);
}
fn env_lock() -> MutexGuard<'static, ()> {
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
LOCK.get_or_init(|| Mutex::new(()))
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
fn env_lock() -> std::sync::MutexGuard<'static, ()> {
super::env_lock()
}
fn with_current_dir<T>(cwd: &Path, f: impl FnOnce() -> T) -> T {
@ -16447,7 +16498,37 @@ mod dump_manifests_tests {
#[cfg(test)]
mod alias_resolution_tests {
use super::{resolve_model_alias_with_config, validate_model_syntax};
use std::ffi::OsString;
use super::{env_lock, resolve_model_alias_with_config, validate_model_syntax};
struct ScopedEnvVar {
key: &'static str,
previous: Option<OsString>,
}
impl ScopedEnvVar {
fn set(key: &'static str, value: &str) -> Self {
let previous = std::env::var_os(key);
std::env::set_var(key, value);
Self { key, previous }
}
fn remove(key: &'static str) -> Self {
let previous = std::env::var_os(key);
std::env::remove_var(key);
Self { key, previous }
}
}
impl Drop for ScopedEnvVar {
fn drop(&mut self) {
match &self.previous {
Some(value) => std::env::set_var(self.key, value),
None => std::env::remove_var(self.key),
}
}
}
#[test]
fn test_alias_resolution_builtin() {
@ -16468,6 +16549,9 @@ mod alias_resolution_tests {
#[test]
fn test_alias_resolution_syntax_validation() {
let _guard = env_lock();
let _url = ScopedEnvVar::remove("OPENAI_BASE_URL");
// Resolved aliases should pass syntax validation
let resolved = resolve_model_alias_with_config("opus");
assert!(validate_model_syntax(&resolved).is_ok());
@ -16478,6 +16562,9 @@ mod alias_resolution_tests {
#[test]
fn test_unknown_alias_fails_validation() {
let _guard = env_lock();
let _url = ScopedEnvVar::remove("OPENAI_BASE_URL");
// Unknown aliases resolve to themselves
let resolved = resolve_model_alias_with_config("unknown-alias");
assert_eq!(resolved, "unknown-alias");
@ -16490,9 +16577,42 @@ mod alias_resolution_tests {
#[test]
fn test_direct_provider_model_passes() {
let _guard = env_lock();
let _url = ScopedEnvVar::remove("OPENAI_BASE_URL");
// Direct provider/model strings should remain unchanged and pass
let model = "openai/gpt-4o";
assert_eq!(resolve_model_alias_with_config(model), model);
assert!(validate_model_syntax(model).is_ok());
}
#[test]
fn test_bare_model_name_passes_when_openai_base_url_set() {
// Issue #3123: Ollama-style bare model names (no provider/ prefix)
// should pass validation when OPENAI_BASE_URL is set, indicating
// a local OpenAI-compatible endpoint is configured.
let _guard = env_lock();
let _url = ScopedEnvVar::set("OPENAI_BASE_URL", "http://localhost:11434/v1");
assert!(validate_model_syntax("qwen2.5-coder:7b").is_ok());
assert!(validate_model_syntax("llama3:8b").is_ok());
assert!(validate_model_syntax("mistral").is_ok());
}
#[test]
fn test_bare_model_name_rejected_for_non_local_openai_base_url() {
// Bare model names should be rejected when OPENAI_BASE_URL points to
// a non-local gateway (e.g. OpenRouter), since those typically require
// a slash-containing slug like `openai/gpt-4.1-mini`.
let _guard = env_lock();
let _url = ScopedEnvVar::set("OPENAI_BASE_URL", "https://openrouter.ai/api/v1");
assert!(validate_model_syntax("mistral").is_err());
assert!(validate_model_syntax("qwen2.5-coder:7b").is_err());
}
#[test]
fn test_bare_model_name_passes_for_127_0_0_1() {
let _guard = env_lock();
let _url = ScopedEnvVar::set("OPENAI_BASE_URL", "http://127.0.0.1:11434/v1");
assert!(validate_model_syntax("llama3:8b").is_ok());
}
}