feat(ollama): align provider defaults with codex semantics
This commit is contained in:
6
agents.md
Normal file
6
agents.md
Normal file
@@ -0,0 +1,6 @@
|
||||
# Agents Upgrade Plan
|
||||
|
||||
- test: rebuild ollama integration suite to exercise local, cloud, search, and quota error flows
|
||||
- refactor: simplify provider/session coupling so compute_web_search_settings shares normalization utilities
|
||||
- docs: clarify cloud vs local env configuration (api key precedence, insecure toggle) in configuration guide
|
||||
- ci: add targeted regression job for ollama providers covering chat, streaming, and tool execution paths
|
||||
@@ -2053,10 +2053,14 @@ fn normalize_base_url(
|
||||
}
|
||||
|
||||
let path = url.path().trim_end_matches('/');
|
||||
if path == "/api" {
|
||||
url.set_path("/");
|
||||
} else if !path.is_empty() && path != "/" {
|
||||
return Err("Ollama base URLs must not include additional path segments".to_string());
|
||||
match path {
|
||||
"" | "/" => {}
|
||||
"/api" | "/v1" => {
|
||||
url.set_path("/");
|
||||
}
|
||||
_ => {
|
||||
return Err("Ollama base URLs must not include additional path segments".to_string());
|
||||
}
|
||||
}
|
||||
|
||||
if mode_hint == OllamaMode::Cloud {
|
||||
@@ -2163,6 +2167,18 @@ mod tests {
|
||||
assert_eq!(url, "https://ollama.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_base_url_accepts_v1_path_for_local() {
|
||||
let url = normalize_base_url(Some("http://localhost:11434/v1"), OllamaMode::Local).unwrap();
|
||||
assert_eq!(url, "http://localhost:11434");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_base_url_accepts_v1_path_for_cloud() {
|
||||
let url = normalize_base_url(Some("https://api.ollama.com/v1"), OllamaMode::Cloud).unwrap();
|
||||
assert_eq!(url, "https://ollama.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_base_url_canonicalises_api_hostname() {
|
||||
let url = normalize_base_url(Some("https://api.ollama.com"), OllamaMode::Cloud).unwrap();
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use std::sync::Arc;
|
||||
use std::{sync::Arc, time::Duration};
|
||||
|
||||
use owlen_core::tools::WEB_SEARCH_TOOL_NAME;
|
||||
use owlen_core::types::{ChatParameters, Role};
|
||||
use owlen_core::types::{ChatParameters, ChatResponse, Role};
|
||||
use owlen_core::{
|
||||
Config, Provider,
|
||||
providers::OllamaProvider,
|
||||
@@ -84,6 +84,19 @@ async fn create_session(
|
||||
(session, temp_dir)
|
||||
}
|
||||
|
||||
async fn send_prompt(session: &mut SessionController, message: &str) -> ChatResponse {
|
||||
match session
|
||||
.send_message(message.to_string(), ChatParameters::default())
|
||||
.await
|
||||
{
|
||||
Ok(SessionOutcome::Complete(response)) => response,
|
||||
Ok(SessionOutcome::Streaming { .. }) => {
|
||||
panic!("expected complete outcome, got streaming response")
|
||||
}
|
||||
Err(err) => panic!("send_message failed: {err:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
fn configure_local(base_url: &str) -> Config {
|
||||
let mut config = Config::default();
|
||||
config.general.default_provider = "ollama_local".into();
|
||||
@@ -100,7 +113,7 @@ fn configure_local(base_url: &str) -> Config {
|
||||
config
|
||||
}
|
||||
|
||||
fn configure_cloud(base_url: &str) -> Config {
|
||||
fn configure_cloud(base_url: &str, search_endpoint: &str) -> Config {
|
||||
let mut config = Config::default();
|
||||
config.general.default_provider = "ollama_cloud".into();
|
||||
config.general.default_model = Some("llama3:8b-cloud".into());
|
||||
@@ -119,7 +132,7 @@ fn configure_cloud(base_url: &str) -> Config {
|
||||
cloud.api_key = Some("test-key".into());
|
||||
cloud.extra.insert(
|
||||
"web_search_endpoint".into(),
|
||||
Value::String("/v1/web/search".into()),
|
||||
Value::String(search_endpoint.into()),
|
||||
);
|
||||
cloud.extra.insert(
|
||||
owlen_core::config::OLLAMA_CLOUD_ENDPOINT_KEY.into(),
|
||||
@@ -130,47 +143,32 @@ fn configure_cloud(base_url: &str) -> Config {
|
||||
config
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn local_provider_happy_path_records_usage() {
|
||||
async fn run_local_completion(base_suffix: &str) {
|
||||
let server = MockServer::start().await;
|
||||
let raw_base = format!("{}{}", server.uri(), base_suffix);
|
||||
|
||||
let tags = load_fixture("ollama_tags");
|
||||
let completion = load_fixture("ollama_local_completion");
|
||||
|
||||
let base_url = server.uri();
|
||||
|
||||
let tags_template = ResponseTemplate::new(200).set_body_json(tags);
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/api/tags"))
|
||||
.respond_with(tags_template.clone())
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(tags))
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/chat"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(completion))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let config = configure_local(&base_url);
|
||||
let config = configure_local(&raw_base);
|
||||
let provider: Arc<dyn Provider> =
|
||||
Arc::new(OllamaProvider::new(base_url.clone()).expect("local provider"));
|
||||
Arc::new(OllamaProvider::new(raw_base.clone()).expect("local provider"));
|
||||
|
||||
let (mut session, _tmp) = create_session(provider, config).await;
|
||||
|
||||
let outcome = session
|
||||
.send_message(
|
||||
"Summarise the local status.".to_string(),
|
||||
ChatParameters::default(),
|
||||
)
|
||||
.await
|
||||
.expect("local completion");
|
||||
|
||||
let response = match outcome {
|
||||
SessionOutcome::Complete(response) => response,
|
||||
_ => panic!("expected complete outcome"),
|
||||
};
|
||||
|
||||
let response = send_prompt(&mut session, "Summarise the local status.").await;
|
||||
assert_eq!(response.message.content, "Local response complete.");
|
||||
|
||||
let snapshot = session
|
||||
@@ -182,34 +180,41 @@ async fn local_provider_happy_path_records_usage() {
|
||||
assert_eq!(snapshot.weekly.total_tokens, 36);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn cloud_tool_call_flows_through_web_search() {
|
||||
async fn run_cloud_tool_flow(base_suffix: &str) {
|
||||
let server = MockServer::start().await;
|
||||
let raw_base = format!("{}{}", server.uri(), base_suffix);
|
||||
let search_endpoint = if base_suffix.is_empty() {
|
||||
"/v1/web/search"
|
||||
} else {
|
||||
"/web/search"
|
||||
};
|
||||
|
||||
let tags = load_fixture("ollama_tags");
|
||||
let tool_call = load_fixture("ollama_cloud_tool_call");
|
||||
let final_chunk = load_fixture("ollama_cloud_final");
|
||||
|
||||
let base_url = server.uri();
|
||||
|
||||
let tags_template = ResponseTemplate::new(200).set_body_json(tags);
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/api/tags"))
|
||||
.respond_with(tags_template.clone())
|
||||
.and(header("authorization", "Bearer test-key"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(tags))
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/chat"))
|
||||
.and(header("authorization", "Bearer test-key"))
|
||||
.and(BodySubstringMatcher::not_contains("\"role\":\"tool\""))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(tool_call))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/chat"))
|
||||
.and(header("authorization", "Bearer test-key"))
|
||||
.and(BodySubstringMatcher::contains("\"role\":\"tool\""))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(final_chunk))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
@@ -229,38 +234,36 @@ async fn cloud_tool_call_flows_through_web_search() {
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let config = configure_cloud(&base_url);
|
||||
let config = configure_cloud(&raw_base, search_endpoint);
|
||||
let cloud_cfg = config
|
||||
.providers
|
||||
.get("ollama_cloud")
|
||||
.expect("cloud provider config")
|
||||
.clone();
|
||||
assert_eq!(cloud_cfg.api_key.as_deref(), Some("test-key"));
|
||||
assert_eq!(cloud_cfg.base_url.as_deref(), Some(base_url.as_str()));
|
||||
assert_eq!(cloud_cfg.base_url.as_deref(), Some(raw_base.as_str()));
|
||||
assert_eq!(cloud_cfg.api_key.as_deref(), Some("test-key"));
|
||||
assert_eq!(cloud_cfg.base_url.as_deref(), Some(raw_base.as_str()));
|
||||
|
||||
let provider: Arc<dyn Provider> = Arc::new(
|
||||
OllamaProvider::from_config("ollama_cloud", &cloud_cfg, Some(&config.general))
|
||||
.expect("cloud provider"),
|
||||
);
|
||||
|
||||
let (mut session, _tmp) = create_session(provider, config).await;
|
||||
let search_url = format!(
|
||||
"{}/{}",
|
||||
raw_base.trim_end_matches('/'),
|
||||
search_endpoint.trim_start_matches('/')
|
||||
);
|
||||
|
||||
session.grant_consent(
|
||||
WEB_SEARCH_TOOL_NAME,
|
||||
vec!["network".into()],
|
||||
vec![format!("{}/v1/web/search", base_url)],
|
||||
vec![search_url],
|
||||
);
|
||||
|
||||
let outcome = session
|
||||
.send_message(
|
||||
"What is new in Rust today?".to_string(),
|
||||
ChatParameters::default(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let response = match outcome {
|
||||
Ok(SessionOutcome::Complete(response)) => response,
|
||||
Ok(_) => panic!("expected complete outcome"),
|
||||
Err(err) => panic!("cloud completion: {err:?}"),
|
||||
};
|
||||
let response = send_prompt(&mut session, "What is new in Rust today?").await;
|
||||
assert_eq!(
|
||||
response.message.content,
|
||||
"Rust 1.85 shipped today. Summarising the highlights now."
|
||||
@@ -283,114 +286,126 @@ async fn cloud_tool_call_flows_through_web_search() {
|
||||
.await
|
||||
.expect("usage snapshot");
|
||||
assert_eq!(snapshot.provider, "ollama_cloud");
|
||||
assert_eq!(snapshot.hourly.total_tokens, 112); // 64 prompt + 48 completion
|
||||
assert_eq!(snapshot.hourly.total_tokens, 112);
|
||||
assert_eq!(snapshot.weekly.total_tokens, 112);
|
||||
}
|
||||
|
||||
async fn run_cloud_error(
|
||||
base_suffix: &str,
|
||||
status: u16,
|
||||
error_body: Value,
|
||||
prompt: &str,
|
||||
) -> String {
|
||||
let server = MockServer::start().await;
|
||||
let raw_base = format!("{}{}", server.uri(), base_suffix);
|
||||
let search_endpoint = if base_suffix.is_empty() {
|
||||
"/v1/web/search"
|
||||
} else {
|
||||
"/web/search"
|
||||
};
|
||||
|
||||
let tags = load_fixture("ollama_tags");
|
||||
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/api/tags"))
|
||||
.and(header("authorization", "Bearer test-key"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(tags))
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/chat"))
|
||||
.and(header("authorization", "Bearer test-key"))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(status)
|
||||
.set_body_json(error_body)
|
||||
.set_delay(Duration::from_millis(5)),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let config = configure_cloud(&raw_base, search_endpoint);
|
||||
let cloud_cfg = config
|
||||
.providers
|
||||
.get("ollama_cloud")
|
||||
.expect("cloud provider config")
|
||||
.clone();
|
||||
let provider: Arc<dyn Provider> = Arc::new(
|
||||
OllamaProvider::from_config("ollama_cloud", &cloud_cfg, Some(&config.general))
|
||||
.expect("cloud provider"),
|
||||
);
|
||||
|
||||
let (mut session, _tmp) = create_session(provider, config).await;
|
||||
|
||||
let err_text = match session
|
||||
.send_message(prompt.to_string(), ChatParameters::default())
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("expected error status {status} but request succeeded"),
|
||||
Err(err) => err.to_string(),
|
||||
};
|
||||
|
||||
let snapshot = session
|
||||
.current_usage_snapshot()
|
||||
.await
|
||||
.expect("usage snapshot");
|
||||
assert_eq!(snapshot.hourly.total_tokens, 0);
|
||||
assert_eq!(snapshot.weekly.total_tokens, 0);
|
||||
|
||||
err_text
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn local_provider_happy_path_records_usage() {
|
||||
run_local_completion("").await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn local_provider_accepts_v1_base_url() {
|
||||
run_local_completion("/v1").await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn cloud_tool_call_flows_through_web_search() {
|
||||
run_cloud_tool_flow("").await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn cloud_tool_call_accepts_v1_base_url() {
|
||||
run_cloud_tool_flow("/v1").await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn cloud_unauthorized_degrades_without_usage() {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let tags = load_fixture("ollama_tags");
|
||||
let tags_template = ResponseTemplate::new(200).set_body_json(tags);
|
||||
let base_url = server.uri();
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/api/tags"))
|
||||
.respond_with(tags_template.clone())
|
||||
.mount(&server)
|
||||
for suffix in ["", "/v1"] {
|
||||
let err_text = run_cloud_error(
|
||||
suffix,
|
||||
401,
|
||||
json!({ "error": "unauthorized" }),
|
||||
"Switch to cloud",
|
||||
)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/chat"))
|
||||
.respond_with(ResponseTemplate::new(401).set_body_json(json!({
|
||||
"error": "unauthorized"
|
||||
})))
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let config = configure_cloud(&base_url);
|
||||
let cloud_cfg = config
|
||||
.providers
|
||||
.get("ollama_cloud")
|
||||
.expect("cloud provider config")
|
||||
.clone();
|
||||
let provider: Arc<dyn Provider> = Arc::new(
|
||||
OllamaProvider::from_config("ollama_cloud", &cloud_cfg, Some(&config.general))
|
||||
.expect("cloud provider"),
|
||||
);
|
||||
|
||||
let (mut session, _tmp) = create_session(provider, config).await;
|
||||
|
||||
let err_text = match session
|
||||
.send_message("Switch to cloud".to_string(), ChatParameters::default())
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("expected unauthorized error, but request succeeded"),
|
||||
Err(err) => err.to_string(),
|
||||
};
|
||||
assert!(
|
||||
err_text.contains("unauthorized") || err_text.contains("API key"),
|
||||
"error should surface unauthorized detail, got: {err_text}"
|
||||
);
|
||||
|
||||
let snapshot = session
|
||||
.current_usage_snapshot()
|
||||
.await
|
||||
.expect("usage snapshot");
|
||||
assert_eq!(snapshot.hourly.total_tokens, 0);
|
||||
assert_eq!(snapshot.weekly.total_tokens, 0);
|
||||
assert!(
|
||||
err_text.contains("unauthorized") || err_text.contains("API key"),
|
||||
"error should surface unauthorized detail for base '{suffix}', got: {err_text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn cloud_rate_limit_returns_error_without_usage() {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let tags = load_fixture("ollama_tags");
|
||||
let tags_template = ResponseTemplate::new(200).set_body_json(tags);
|
||||
let base_url = server.uri();
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/api/tags"))
|
||||
.respond_with(tags_template.clone())
|
||||
.mount(&server)
|
||||
for suffix in ["", "/v1"] {
|
||||
let err_text = run_cloud_error(
|
||||
suffix,
|
||||
429,
|
||||
json!({ "error": "too many requests" }),
|
||||
"Hit rate limit",
|
||||
)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/chat"))
|
||||
.respond_with(ResponseTemplate::new(429).set_body_json(json!({
|
||||
"error": "too many requests"
|
||||
})))
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let config = configure_cloud(&base_url);
|
||||
let cloud_cfg = config
|
||||
.providers
|
||||
.get("ollama_cloud")
|
||||
.expect("cloud provider config")
|
||||
.clone();
|
||||
let provider: Arc<dyn Provider> = Arc::new(
|
||||
OllamaProvider::from_config("ollama_cloud", &cloud_cfg, Some(&config.general))
|
||||
.expect("cloud provider"),
|
||||
);
|
||||
|
||||
let (mut session, _tmp) = create_session(provider, config).await;
|
||||
|
||||
let err_text = match session
|
||||
.send_message("Hit rate limit".to_string(), ChatParameters::default())
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("expected rate-limit error, but request succeeded"),
|
||||
Err(err) => err.to_string(),
|
||||
};
|
||||
assert!(
|
||||
err_text.contains("rate limited") || err_text.contains("429"),
|
||||
"error should mention rate limiting, got: {err_text}"
|
||||
);
|
||||
|
||||
let snapshot = session
|
||||
.current_usage_snapshot()
|
||||
.await
|
||||
.expect("usage snapshot");
|
||||
assert_eq!(snapshot.hourly.total_tokens, 0);
|
||||
assert_eq!(snapshot.weekly.total_tokens, 0);
|
||||
assert!(
|
||||
err_text.contains("rate limited") || err_text.contains("429"),
|
||||
"error should mention rate limiting for base '{suffix}', got: {err_text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user