diff --git a/crates/buzz-agent/README.md b/crates/buzz-agent/README.md index a2504db451..f138e4a4f1 100644 --- a/crates/buzz-agent/README.md +++ b/crates/buzz-agent/README.md @@ -21,9 +21,9 @@ HTTPS │ ▼ - Anthropic Messages API - or any OpenAI-compat - (vLLM, llama.cpp, OpenRouter, + Anthropic Messages API, + OpenRouter, or any OpenAI-compat + (vLLM, llama.cpp, Databricks, Block Gateway, Ollama, …) ``` @@ -50,6 +50,12 @@ OPENAI_COMPAT_MODEL=gpt-5 \ OPENAI_COMPAT_BASE_URL=https://api.openai.com/v1 \ ./target/release/buzz-agent +# Or OpenRouter +BUZZ_AGENT_PROVIDER=openrouter \ +OPENROUTER_API_KEY=sk-or-v1-... \ +OPENROUTER_MODEL=anthropic/claude-sonnet-4.5 \ + ./target/release/buzz-agent + # Or Databricks model serving via OAuth 2.0 PKCE BUZZ_AGENT_PROVIDER=databricks \ DATABRICKS_HOST=https://dbc-...cloud.databricks.com \ @@ -129,15 +135,18 @@ Everything is environment variables. No flags, no config files. (We are a subpro | Variable | Default | Notes | |---|---|---| -| `BUZZ_AGENT_PROVIDER` | — | Required. `anthropic`, `openai`, `databricks`, or `databricks_v2`. No implicit fallback — the agent errors at startup when this is unset. | +| `BUZZ_AGENT_PROVIDER` | — | Required. `anthropic`, `openai`, `openrouter`, `databricks`, or `databricks_v2`. No implicit fallback — the agent errors at startup when this is unset. | | `ANTHROPIC_API_KEY` | — | Required when provider=anthropic. | | `ANTHROPIC_MODEL` | — | Required when provider=anthropic. | | `ANTHROPIC_BASE_URL` | `https://api.anthropic.com` | | | `ANTHROPIC_API_VERSION` | `2023-06-01` | | | `OPENAI_COMPAT_API_KEY` | — | Required when provider=openai. | | `OPENAI_COMPAT_MODEL` | — | Required when provider=openai. | -| `OPENAI_COMPAT_BASE_URL` | `https://api.openai.com/v1` | Point at vLLM, llama.cpp, OpenRouter, Ollama, etc. | +| `OPENAI_COMPAT_BASE_URL` | `https://api.openai.com/v1` | Point at vLLM, llama.cpp, Ollama, etc. | | `OPENAI_COMPAT_API` | `auto` | `auto` \| `chat` \| `responses`. `auto` picks Responses for `*.openai.com`, Chat Completions everywhere else. | +| `OPENROUTER_API_KEY` | — | Required when provider=openrouter. | +| `OPENROUTER_MODEL` | — | Required when provider=openrouter. Use OpenRouter's `vendor/model` id, e.g. `anthropic/claude-sonnet-4.5`. | +| `OPENROUTER_BASE_URL` | `https://openrouter.ai/api/v1` | | | `DATABRICKS_HOST` | — | Required when provider=databricks or provider=databricks_v2. | | `DATABRICKS_MODEL` | — | Required when provider=databricks or provider=databricks_v2. | | `DATABRICKS_TOKEN` | — | Optional static bearer escape hatch. If unset, Databricks uses browser OAuth + refresh cache. | @@ -167,17 +176,24 @@ Everything is environment variables. No flags, no config files. (We are a subpro | vLLM | `openai` | `POST {base}/chat/completions` | any tool-calling model | | llama.cpp | `openai` | `POST {base}/chat/completions` | any tool-calling GGUF | | Ollama | `openai` | `POST {base}/chat/completions` | llama3.1, qwen2.5-coder | -| OpenRouter | `openai` | `POST {base}/chat/completions` | anything they route | | Block Gateway | `openai` | `POST {base}/chat/completions` | gpt-5, claude | +| OpenRouter | `openrouter` | `POST {base}/chat/completions` | anything they route (extended-thinking replay, provider-agnostic tool calling) | | Databricks | `databricks` | `POST {host}/serving-endpoints/{model}/invocations` | goose-claude-4-6-sonnet | | Databricks AI Gateway v2 | `databricks_v2` | `POST {host}/ai-gateway/{provider}/v1/...` | databricks-gpt-5-5, databricks-claude-opus-4-7 | -If `BUZZ_AGENT_PROVIDER=anthropic` is selected without `ANTHROPIC_API_KEY`, or `BUZZ_AGENT_PROVIDER=openai` is selected without `OPENAI_COMPAT_API_KEY`, the agent returns an error — there is no implicit fallback to another provider. +If `BUZZ_AGENT_PROVIDER=anthropic` is selected without `ANTHROPIC_API_KEY`, `BUZZ_AGENT_PROVIDER=openai` is selected without `OPENAI_COMPAT_API_KEY`, or `BUZZ_AGENT_PROVIDER=openrouter` is selected without `OPENROUTER_API_KEY`, the agent returns an error — there is no implicit fallback to another provider. `provider=openai` speaks two HTTP dialects: the [Responses API](https://platform.openai.com/docs/api-reference/responses) (`/v1/responses`, required for GPT-5 / o-series tool-calling on OpenAI's own service) and the [Chat Completions API](https://platform.openai.com/docs/api-reference/chat) (`/chat/completions`, the broadly-supported OpenAI-compatible wire format). By default (`OPENAI_COMPAT_API=auto`) the agent picks **Responses** when `OPENAI_COMPAT_BASE_URL` points at an `*.openai.com` host and **Chat Completions** everywhere else. Pin the choice explicitly with `OPENAI_COMPAT_API=chat` or `OPENAI_COMPAT_API=responses` for providers that diverge from the default (e.g. a Responses-compatible self-hosted gateway). +`provider=openrouter` is first-class, not routed through `provider=openai`: it speaks OpenAI's Chat Completions wire format but with OpenRouter-specific extensions layered on top — + +- `reasoning.effort` is set on the request when reasoning effort is configured. The request deliberately carries no `provider.require_parameters` filter: that filter routes only to endpoints advertising every parameter in the body, and 83 of 274 tools-capable OpenRouter models do not advertise `reasoning`, so it turns an effort setting into a hard 404 on a valid model id. A model that cannot reason answers without reasoning instead. +- The response's `reasoning_details` array (opaque extended-thinking payload) is captured and replayed byte-for-byte on the next turn's assistant message, so multi-turn tool use keeps the model's chain-of-thought. +- `anthropic/*` models get Anthropic-style `cache_control` breakpoints injected on the system message and the last two user messages. +- Retryable statuses (429 and typed `provider_overloaded` 503) honor the documented `Retry-After` header (clamped to a small ceiling — see `RETRY_AFTER_CAP_SECS` in `llm.rs` — since the sleep happens outside `BUZZ_AGENT_LLM_TIMEOUT_SECS`); 502 and untyped 503 retry with jittered backoff instead. `401` is treated as an expired/invalid key and refreshed once, while `402` (no credits) and `403` (guardrail/moderation/permission) fail immediately without retry. + `Provider` is a Rust `enum` with one `match` in `Llm::complete`. There is no trait, no `Box`, no async-trait. Adding a provider is a `match` arm and one `body`/`parse` pair in `llm.rs`. ## MCP Servers diff --git a/crates/buzz-agent/src/agent.rs b/crates/buzz-agent/src/agent.rs index ce4d2db4b3..48d4ea3b02 100644 --- a/crates/buzz-agent/src/agent.rs +++ b/crates/buzz-agent/src/agent.rs @@ -256,6 +256,7 @@ impl RunCtx<'_> { self.history.push(HistoryItem::Assistant { text: response.text, tool_calls: Vec::new(), + reasoning_details: response.reasoning_details.clone(), }); let stop = map_stop(response.stop); // Only gate genuine end_turn — don't override max_tokens/refusal. @@ -292,6 +293,7 @@ impl RunCtx<'_> { self.history.push(HistoryItem::Assistant { text: response.text, tool_calls: calls.clone(), + reasoning_details: response.reasoning_details, }); if let Some(stop) = self.execute_calls(&calls).await { @@ -726,6 +728,7 @@ pub(crate) fn push_hook_outputs_as_tool_results( // preserve. provider_extra: Default::default(), }], + reasoning_details: None, }); history.push(HistoryItem::ToolResult(ToolResult { provider_id, @@ -790,3 +793,76 @@ fn map_stop(p: ProviderStop) -> StopReason { ProviderStop::Refusal => StopReason::Refusal, } } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + /// A9 regression: `reasoning_details` contributes real bytes to + /// `estimated_bytes` (see `types.rs::HistoryItem::size_with`), so a + /// history item carrying a large opaque reasoning array must actually + /// drive `truncate_history` eviction — not be silently invisible to the + /// sizing gate that decides what survives a turn. + #[test] + fn truncate_history_evicts_oldest_turn_with_reasoning_details() { + let big_reasoning = json!([{ "type": "reasoning.text", "text": "x".repeat(400) }]); + let mut history = vec![ + HistoryItem::User("first question".into()), + HistoryItem::Assistant { + text: "first answer".into(), + tool_calls: vec![], + reasoning_details: Some(big_reasoning), + }, + HistoryItem::User("second question".into()), + HistoryItem::Assistant { + text: "second answer".into(), + tool_calls: vec![], + reasoning_details: None, + }, + ]; + + let total_before: usize = history.iter().map(HistoryItem::estimated_bytes).sum(); + // Budget below the total but above the second (smaller) turn alone, + // so only the oldest user+assistant pair — the one carrying + // reasoning_details — must be dropped. + let max_bytes = total_before - 100; + assert!( + max_bytes > 0, + "test fixture must leave room to evict only one turn" + ); + + truncate_history(&mut history, max_bytes); + + assert_eq!( + history.len(), + 2, + "the oldest user+assistant turn (with reasoning_details) must be evicted" + ); + assert!(matches!(&history[0], HistoryItem::User(s) if s == "second question")); + assert!( + matches!(&history[1], HistoryItem::Assistant { text, .. } if text == "second answer") + ); + let total_after: usize = history.iter().map(HistoryItem::estimated_bytes).sum(); + assert!(total_after <= max_bytes); + } + + #[test] + fn truncate_history_noop_when_under_budget() { + let mut history = vec![ + HistoryItem::User("hi".into()), + HistoryItem::Assistant { + text: "hello".into(), + tool_calls: vec![], + reasoning_details: None, + }, + ]; + let original_len = history.len(); + truncate_history(&mut history, 1_000_000); + assert_eq!( + history.len(), + original_len, + "under budget must not evict anything" + ); + } +} diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index 037b67b3cb..a0e64f1a9d 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -671,6 +671,8 @@ pub enum Provider { /// Databricks AI Gateway v2. Routes by model family through the gateway's /// OpenAI Responses, Anthropic Messages, or MLflow Chat Completions paths. DatabricksV2, + /// OpenRouter multi-provider gateway. Routes to `{base_url}/chat/completions` with bearer auth. Wire format is OpenAI-chat-compatible. + OpenRouter, } /// Which OpenAI-family HTTP API to call. Set via `OPENAI_COMPAT_API` @@ -740,10 +742,11 @@ pub struct Config { pub thinking_effort: Option, /// Emit Anthropic `cache_control` breakpoints on the stable prefix /// (tools + system prompt) and the rolling conversation tail. Default on; - /// disable with `BUZZ_AGENT_PROMPT_CACHING=0`. Only consulted on Anthropic - /// Messages routes (first-party Anthropic and the DatabricksV2 Claude - /// route) — the Databricks gateway does not auto-cache, so without this the - /// surfaced `cache_read_input_tokens` is structurally always 0. + /// disable with `BUZZ_AGENT_PROMPT_CACHING=0`. Consulted on every route that + /// speaks the Anthropic caching dialect: first-party Anthropic, the + /// DatabricksV2 Claude route, and OpenRouter's `anthropic/*` models. The + /// Databricks gateway does not auto-cache, so without this the surfaced + /// `cache_read_input_tokens` is structurally always 0. pub prompt_caching: bool, } @@ -755,6 +758,7 @@ impl Config { env("BUZZ_AGENT_PROVIDER").as_deref(), env("ANTHROPIC_API_KEY").as_deref(), env("OPENAI_COMPAT_API_KEY").as_deref(), + env("OPENROUTER_API_KEY").as_deref(), )?; // Universal model override — takes priority over provider-specific model @@ -797,6 +801,16 @@ impl Config { databricks_host.ok_or_else(|| "config: DATABRICKS_HOST required".to_string())?, OpenAiApi::Chat, // only read by OpenAI/legacy Databricks dispatch ), + Provider::OpenRouter => ( + req("OPENROUTER_API_KEY")?, + resolve_model( + buzz_agent_model.as_deref(), + env("OPENROUTER_MODEL").as_deref(), + ) + .ok_or_else(|| "config: OPENROUTER_MODEL required".to_string())?, + env_or("OPENROUTER_BASE_URL", "https://openrouter.ai/api/v1"), + OpenAiApi::Chat, // OpenRouter uses Chat Completions only + ), }; let system_prompt = match (env("BUZZ_AGENT_SYSTEM_PROMPT"), env("BUZZ_AGENT_SYSTEM_PROMPT_FILE")) { (Some(_), Some(_)) => return Err( @@ -1002,6 +1016,7 @@ fn resolve_provider( requested: Option<&str>, anthropic_key: Option<&str>, openai_key: Option<&str>, + openrouter_key: Option<&str>, ) -> Result { match requested.map(str::trim).filter(|s| !s.is_empty()) { Some(raw) => { @@ -1017,6 +1032,8 @@ fn resolve_provider( ), "databricks" => Ok(Provider::Databricks), "databricks_v2" | "databricks-v2" => Ok(Provider::DatabricksV2), + "openrouter" if present_nonempty(openrouter_key) => Ok(Provider::OpenRouter), + "openrouter" => Err("config: OPENROUTER_API_KEY required".into()), _ => Err(format!( "config: BUZZ_AGENT_PROVIDER={raw} not supported" )), @@ -1234,11 +1251,11 @@ mod tests { #[test] fn resolve_provider_keeps_requested_provider_when_token_present() { assert_eq!( - resolve_provider(Some("anthropic"), Some("sk-ant"), None,).unwrap(), + resolve_provider(Some("anthropic"), Some("sk-ant"), None, None).unwrap(), Provider::Anthropic ); assert_eq!( - resolve_provider(Some("openai"), None, Some("sk-openai"),).unwrap(), + resolve_provider(Some("openai"), None, Some("sk-openai"), None).unwrap(), Provider::OpenAi ); } @@ -1246,17 +1263,17 @@ mod tests { #[test] fn resolve_provider_errors_when_requested_provider_key_missing() { // No fallback — missing key returns an error regardless of Databricks availability. - let err = resolve_provider(Some("anthropic"), None, None).unwrap_err(); + let err = resolve_provider(Some("anthropic"), None, None, None).unwrap_err(); assert!(err.contains("ANTHROPIC_API_KEY required"), "{err}"); - let err = resolve_provider(Some("openai-compat"), None, Some(" ")).unwrap_err(); + let err = resolve_provider(Some("openai-compat"), None, Some(" "), None).unwrap_err(); assert!(err.contains("OPENAI_COMPAT_API_KEY required"), "{err}"); } #[test] fn resolve_provider_errors_when_provider_env_absent() { // No implicit inference — absent BUZZ_AGENT_PROVIDER is an error. - let err = resolve_provider(None, None, None).unwrap_err(); + let err = resolve_provider(None, None, None, None).unwrap_err(); assert!(err.contains("BUZZ_AGENT_PROVIDER is required"), "{err}"); } @@ -1266,19 +1283,19 @@ mod tests { // When BUZZ_AGENT_PROVIDER=databricks, resolve_provider succeeds regardless // of DATABRICKS_HOST/MODEL (those are validated later in from_env()). assert_eq!( - resolve_provider(Some("databricks"), None, None).unwrap(), + resolve_provider(Some("databricks"), None, None, None).unwrap(), Provider::Databricks ); // Missing key for other providers still errors — no Databricks fallback. - let err = resolve_provider(Some("openai"), None, None).unwrap_err(); + let err = resolve_provider(Some("openai"), None, None, None).unwrap_err(); assert!(err.contains("OPENAI_COMPAT_API_KEY required"), "{err}"); - let err = resolve_provider(None, None, None).unwrap_err(); + let err = resolve_provider(None, None, None, None).unwrap_err(); assert!(err.contains("BUZZ_AGENT_PROVIDER is required"), "{err}"); } #[test] fn resolve_provider_unsupported_error_preserves_user_casing() { - let err = resolve_provider(Some("OpenAIish"), None, None).unwrap_err(); + let err = resolve_provider(Some("OpenAIish"), None, None, None).unwrap_err(); assert!(err.contains("BUZZ_AGENT_PROVIDER=OpenAIish")); } @@ -2666,6 +2683,9 @@ mod tests { if p == "databricks" { return openai_result(&m); } + if p == "openrouter" { + return (ALL_7.to_vec(), Some("medium")); + } // openai-compat, unknown, empty → all-7, default medium. (ALL_7.to_vec(), Some("medium")) } @@ -2717,4 +2737,18 @@ mod tests { ); } } + + #[test] + fn resolve_provider_openrouter_with_key() { + assert_eq!( + resolve_provider(Some("openrouter"), None, None, Some("sk-or-123")).unwrap(), + Provider::OpenRouter + ); + } + + #[test] + fn resolve_provider_openrouter_missing_key() { + let err = resolve_provider(Some("openrouter"), None, None, None).unwrap_err(); + assert!(err.contains("OPENROUTER_API_KEY")); + } } diff --git a/crates/buzz-agent/src/handoff.rs b/crates/buzz-agent/src/handoff.rs index 9c27c6606d..3b0feefecf 100644 --- a/crates/buzz-agent/src/handoff.rs +++ b/crates/buzz-agent/src/handoff.rs @@ -259,7 +259,11 @@ fn push_history_snippet(out: &mut String, item: &HistoryItem) { out.push_str(s); out.push('\n'); } - HistoryItem::Assistant { text, tool_calls } => { + HistoryItem::Assistant { + text, + tool_calls, + reasoning_details: _, + } => { out.push_str("[assistant] "); if !text.is_empty() { out.push_str(text); diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 98b881eb45..f595a165e5 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -146,6 +146,18 @@ impl Llm { .await?; parse_anthropic(v) } + Provider::OpenRouter => { + let mut body = + openai_body(cfg, system_prompt, history, tools, effective_model, None); + apply_openrouter_mutations( + &mut body, + cfg.thinking_effort, + effective_model, + cfg.prompt_caching, + ); + let v = self.post_openrouter(cfg, &body).await?; + parse_openai_with_reasoning_details(v) + } Provider::OpenAi | Provider::Databricks => { self.openai_request( cfg, @@ -248,6 +260,16 @@ impl Llm { }); Ok(parse_anthropic(self.post_anthropic(cfg, &body).await?)?.text) } + Provider::OpenRouter => { + let body = openrouter_summary_body( + effective_model, + system_prompt, + user_prompt, + max_output_tokens, + ); + let v = self.post_openrouter(cfg, &body).await?; + Ok(parse_openai(v)?.text) + } Provider::OpenAi | Provider::Databricks => { let r = self .openai_request( @@ -652,6 +674,32 @@ impl Llm { } } + async fn post_openrouter(&self, cfg: &Config, body: &Value) -> Result { + let url = format!("{}/chat/completions", cfg.base_url.trim_end_matches('/')); + let mut bearer = self.auth.bearer().await?; + let mut refreshed = false; + loop { + match openrouter_post(&self.http, &url, body, &bearer).await { + Err(AgentError::LlmAuth(_)) if !refreshed => { + refreshed = true; + let new_bearer = self.auth.refresh_now(&bearer).await?; + // A static key refreshes to itself — a byte-identical retry + // would be a guaranteed duplicate request against a key the + // server just rejected. Fail terminal immediately; the retry + // is only meaningful when the source can actually mint a + // distinct token (e.g., a PKCE OAuth source). + if new_bearer == bearer { + return Err(AgentError::LlmAuth( + "401: static key rejected — update key in agent settings".into(), + )); + } + bearer = new_bearer; + } + result => return result, + } + } + } + /// If `err` names `/v1/responses` / "use the Responses API", latch a /// sticky upgrade so subsequent OpenAI calls hit Responses. Logged once. fn try_upgrade(&self, err: &AgentError) -> bool { @@ -695,7 +743,11 @@ fn anthropic_body( messages.push(json!({ "role": "user", "content": [{ "type": "text", "text": text }] })); } - HistoryItem::Assistant { text, tool_calls } => { + HistoryItem::Assistant { + text, + tool_calls, + reasoning_details: _, + } => { flush(&mut messages, &mut pending); let mut content: Vec = Vec::new(); if !text.is_empty() { @@ -839,11 +891,18 @@ fn openai_body( flush_images(&mut messages, &mut pending_images); messages.push(json!({ "role": "user", "content": text })); } - HistoryItem::Assistant { text, tool_calls } => { + HistoryItem::Assistant { + text, + tool_calls, + reasoning_details, + } => { flush_images(&mut messages, &mut pending_images); let mut msg = serde_json::Map::new(); msg.insert("role".into(), json!("assistant")); msg.insert("content".into(), json!(text.as_str())); + if let Some(details) = reasoning_details { + msg.insert("reasoning_details".into(), details.clone()); + } if !tool_calls.is_empty() { let calls: Vec = tool_calls .iter() @@ -951,7 +1010,11 @@ fn responses_body( "role": "user", "content": [{ "type": "input_text", "text": text }], })), - HistoryItem::Assistant { text, tool_calls } => { + HistoryItem::Assistant { + text, + tool_calls, + reasoning_details: _, + } => { if !text.is_empty() { input.push(json!({ "role": "assistant", @@ -1207,6 +1270,7 @@ fn parse_responses(v: Value) -> Result { output_tokens, total_tokens, reasoning, + reasoning_details: None, }) } @@ -1442,10 +1506,44 @@ fn parse_anthropic(v: Value) -> Result { // total from them. Always None for this provider. total_tokens: None, reasoning, + reasoning_details: None, }) } fn parse_openai(v: Value) -> Result { + // A5: error-inside-200 check — choice-level `finish_reason == "error"` + if let Some(choice) = v + .get("choices") + .and_then(Value::as_array) + .and_then(|a| a.first()) + { + if choice.get("finish_reason").and_then(Value::as_str) == Some("error") { + let err = choice.get("error").cloned().unwrap_or(Value::Null); + // OpenRouter's `error.code` is numeric; other OpenAI-compat hosts + // may send a string. Accept either rather than discarding the + // typed code as "unknown". + let code = err + .get("code") + .and_then(|c| { + c.as_str() + .map(str::to_string) + .or_else(|| c.as_i64().map(|n| n.to_string())) + }) + .unwrap_or_else(|| "unknown".into()); + let message = err + .get("message") + .and_then(Value::as_str) + .unwrap_or("provider error in 200 response"); + let error_type = err + .get("metadata") + .and_then(|m| m.get("error_type")) + .and_then(Value::as_str); + return Err(AgentError::Llm(match error_type { + Some(et) => format!("provider error ({code}, {et}): {message}"), + None => format!("provider error ({code}): {message}"), + })); + } + } let choice = v .get("choices") .and_then(Value::as_array) @@ -1517,9 +1615,24 @@ fn parse_openai(v: Value) -> Result { output_tokens, total_tokens, reasoning, + reasoning_details: None, }) } +fn parse_openai_with_reasoning_details(v: Value) -> Result { + let reasoning_details = v + .get("choices") + .and_then(Value::as_array) + .and_then(|a| a.first()) + .and_then(|c| c.get("message")) + .and_then(|m| m.get("reasoning_details")) + .filter(|rd| rd.is_array()) + .cloned(); + let mut response = parse_openai(v)?; + response.reasoning_details = reasoning_details; + Ok(response) +} + fn make_tool_call( id: String, name: String, @@ -1809,7 +1922,7 @@ where /// flow; subsequent requests use the cache + refresh transparently. pub(crate) fn build_token_source(cfg: &Config) -> Result, AgentError> { match cfg.provider { - Provider::Anthropic | Provider::OpenAi => { + Provider::Anthropic | Provider::OpenAi | Provider::OpenRouter => { Ok(Arc::new(StaticTokenSource::new(cfg.api_key.clone()))) } Provider::Databricks | Provider::DatabricksV2 => { @@ -1835,6 +1948,29 @@ pub(crate) fn build_token_source(cfg: &Config) -> Result, A } } +/// Build the request body for `Llm::summarize` on `Provider::OpenRouter`. +/// Extracted so tests can assert on the actual wire shape instead of a +/// hand-rolled literal — summaries never carry `reasoning` (see +/// `apply_openrouter_mutations`, which the summary path never calls). +/// It spells the token limit `max_tokens` directly for the same reason: the +/// mutation that renames it is never applied here. +fn openrouter_summary_body( + effective_model: &str, + system_prompt: &str, + user_prompt: &str, + max_output_tokens: u32, +) -> Value { + json!({ + "model": effective_model, + "stream": false, + "max_tokens": max_output_tokens, + "messages": [ + { "role": "system", "content": system_prompt }, + { "role": "user", "content": user_prompt }, + ], + }) +} + /// Return a clone of `body` with any top-level `"model"` field removed. /// Used for Databricks model-serving, which encodes the model in the URL /// path and rejects the field in the body. @@ -1849,12 +1985,353 @@ fn strip_model(body: &Value) -> Value { } } +#[derive(Debug)] +enum OpenRouterErrorClass { + Retryable(Option), + Unknown, +} + +/// Ceiling applied to the server-supplied `Retry-After` header before we +/// sleep on it. OpenRouter can advertise waits up to an hour, but +/// `openrouter_post`'s per-attempt sleep happens *outside* +/// `Client::timeout` (`cfg.llm_timeout`, default 240s) — an unclamped hint +/// could keep a single turn alive for up to two full-duration sleeps across +/// `MAX_RETRIES`. Clamping (never rejecting) keeps us honoring the server's +/// backoff signal while bounding worst-case turn latency to a value smaller +/// than the connect/response timeout. +const RETRY_AFTER_CAP_SECS: u64 = 60; + +fn parse_retry_after_header(headers: &reqwest::header::HeaderMap) -> Option { + let val = headers.get(reqwest::header::RETRY_AFTER)?.to_str().ok()?; + let secs: u64 = val.trim().parse().ok()?; + (secs > 0).then(|| std::time::Duration::from_secs(secs.min(RETRY_AFTER_CAP_SECS))) +} + +fn classify_openrouter_error( + status: u16, + body: &str, + header_retry_after: Option, +) -> OpenRouterErrorClass { + let parsed: Option = serde_json::from_str(body).ok(); + let error_type = parsed + .as_ref() + .and_then(|v| v.get("error")) + .and_then(|e| e.get("metadata")) + .and_then(|m| m.get("error_type")) + .and_then(Value::as_str); + // OpenRouter's documented retry hint is the HTTP `Retry-After` header + // (see https://openrouter.ai/docs/api_reference/errors-and-debugging); + // no current doc specifies a body-level retry field, so we don't parse one. + let retry_after = header_retry_after; + + match (status, error_type) { + (429, _) => OpenRouterErrorClass::Retryable(retry_after), + (502, _) => OpenRouterErrorClass::Retryable(None), + (503, Some("provider_overloaded")) => OpenRouterErrorClass::Retryable(retry_after), + _ => OpenRouterErrorClass::Unknown, + } +} + +/// The one place the parameter-routing failure is worded. OpenRouter reports it +/// two ways — a 404 `No endpoints found ...` (what a `require_parameters`-style +/// or unsupported-parameter body actually returns) and an untyped 503 — and both +/// mean the same thing to the user: the model id is fine, the request shape is +/// not serveable by any endpoint behind it. +fn openrouter_parameter_routing_error(error_body: &str) -> AgentError { + AgentError::Llm(format!( + "no OpenRouter endpoint supports the requested parameters — \ + check model, effort, and tool requirements: {error_body}" + )) +} + +async fn openrouter_post( + http: &Client, + url: &str, + body: &Value, + bearer: &str, +) -> Result { + let body_bytes = + serde_json::to_vec(body).map_err(|e| AgentError::Llm(format!("serialize: {e}")))?; + let call_start = std::time::Instant::now(); + for attempt in 0..MAX_RETRIES { + let resp = match http + .post(url) + .header("content-type", "application/json") + .header("HTTP-Referer", "https://github.com/block/buzz") + .header("X-OpenRouter-Title", "Buzz") + .bearer_auth(bearer) + .body(body_bytes.clone()) + .send() + .await + { + Ok(r) => r, + Err(e) => { + if attempt + 1 < MAX_RETRIES && is_retryable_transport_error(&e) { + tracing::warn!( + attempt = attempt + 1, + max_attempts = MAX_RETRIES, + error = %e, + "llm: openrouter transport error, retrying" + ); + backoff_with_jitter(attempt).await; + continue; + } + return Err(terminal_llm_error( + call_start.elapsed(), + attempt + 1, + &format!("transport: {e}"), + )); + } + }; + let status = resp.status(); + // Unlike the generic `post` path, OpenRouter's static-key auth makes + // 401 and 403 distinguishable: 401 is an invalid/expired key (worth + // one refresh-and-retry), while OpenRouter documents 403 as a + // guardrail/moderation/permission rejection the same key will always + // reproduce. Refreshing a static key returns the identical key, so + // classifying 403 as `LlmAuth` would just waste a duplicate request + // and surface Desktop's unrelated "access denied" copy. + if status == 401 { + return Err(AgentError::LlmAuth(read_error_body(resp).await)); + } + if status == 403 { + return Err(AgentError::Llm(format!( + "{status}: {}", + read_error_body(resp).await + ))); + } + if status == 402 { + return Err(AgentError::Llm( + "OpenRouter credits exhausted — check https://openrouter.ai/credits".into(), + )); + } + if status == 404 { + // OpenRouter overloads 404: a genuinely unknown/unavailable model id + // and a valid model whose parameter set no endpoint can serve both + // land here. Discriminate on the full parameter-routing phrase rather + // than a prefix — "No endpoints found for "-style bodies are + // about the model, and reporting a parameter problem as + // `LlmModelNotFound` (or vice versa) sends the user to the wrong fix. + let error_body = read_error_body(resp).await; + if error_body.contains("No endpoints found that can handle the requested parameters") { + return Err(openrouter_parameter_routing_error(&error_body)); + } + return Err(AgentError::LlmModelNotFound(format!( + "{status}: {error_body}" + ))); + } + // A6: status+error_type retry matrix + // 499 (Client Closed Request) is included: OpenRouter may emit it when a + // turn times out mid-stream. Will added 499 to the shared `post()` path in + // #2175 for the same reason; OpenRouter must match to avoid silently opting + // out of that stall-surfacing recovery. + if status.is_server_error() || status == 429 || status.as_u16() == 499 { + let header_retry_after = parse_retry_after_header(resp.headers()); + let error_body = read_error_body(resp).await; + let should_retry = if attempt + 1 < MAX_RETRIES { + match classify_openrouter_error(status.as_u16(), &error_body, header_retry_after) { + OpenRouterErrorClass::Retryable(delay) => { + if let Some(d) = delay { + tokio::time::sleep(d).await; + } else { + backoff_with_jitter(attempt).await; + } + true + } + OpenRouterErrorClass::Unknown => { + backoff_with_jitter(attempt).await; + true + } + } + } else { + false + }; + if should_retry { + continue; + } + // Terminal: classify for the user + return if status == 429 { + Err(terminal_llm_error( + call_start.elapsed(), + attempt + 1, + &format!("rate limited: {error_body}"), + )) + } else { + let parsed: Option = serde_json::from_str(&error_body).ok(); + let has_error_type = parsed + .as_ref() + .and_then(|v| v.get("error")) + .and_then(|e| e.get("metadata")) + .and_then(|m| m.get("error_type")) + .and_then(Value::as_str) + .is_some(); + Err(if !has_error_type && status.as_u16() == 503 { + openrouter_parameter_routing_error(&error_body) + } else { + terminal_llm_error( + call_start.elapsed(), + attempt + 1, + &format!("exhausted retries: {status}: {error_body}"), + ) + }) + }; + } + if !status.is_success() { + return Err(AgentError::Llm(format!( + "{status}: {}", + read_error_body(resp).await + ))); + } + if let Some(len) = resp.content_length() { + if len as usize > MAX_LLM_RESPONSE_BYTES { + return Err(AgentError::Llm(format!( + "response too large: {len} > {MAX_LLM_RESPONSE_BYTES}" + ))); + } + } + let mut buf: Vec = Vec::new(); + let mut stream = resp; + loop { + match stream.chunk().await { + Ok(Some(chunk)) => { + if buf.len() + chunk.len() > MAX_LLM_RESPONSE_BYTES { + return Err(AgentError::Llm(format!( + "response exceeded {MAX_LLM_RESPONSE_BYTES} bytes" + ))); + } + buf.extend_from_slice(&chunk); + } + Ok(None) => break, + Err(e) => { + return Err(terminal_llm_error( + call_start.elapsed(), + attempt + 1, + &format!("body read: {e}"), + )) + } + } + } + return serde_json::from_slice(&buf).map_err(|e| AgentError::Llm(format!("json: {e}"))); + } + Err(terminal_llm_error( + call_start.elapsed(), + MAX_RETRIES, + "exhausted retries", + )) +} + +fn apply_openrouter_mutations( + body: &mut Value, + effort: Option, + effective_model: &str, + prompt_caching: bool, +) { + if let Some(obj) = body.as_object_mut() { + // OpenRouter's Chat Completions API spells the output cap `max_tokens`; + // `max_completion_tokens` is OpenAI-native and only 53 of 274 + // tools-capable OpenRouter models advertise it. Sending the OpenAI + // spelling risks the cap being dropped on the floor by the endpoint we + // route to, so translate it here rather than at the shared `openai_body` + // (which OpenAI and Databricks also use, where the OpenAI spelling is + // correct). + if let Some(max_tokens) = obj.remove("max_completion_tokens") { + obj.insert("max_tokens".into(), max_tokens); + } + + // A2/A3: Add OpenRouter reasoning object when effort is configured. + // Deliberately NOT paired with `provider.require_parameters`: that filter + // routes only to endpoints advertising every parameter in the body, and + // 83 of 274 tools-capable models do not advertise `reasoning`, so it turns + // an opt-in effort setting into a hard 404 ("No endpoints found that can + // handle the requested parameters") on a model id that is perfectly + // valid. OpenRouter already best-effort routes on `tools`/`max_tokens` + // without the filter, so we accept a request served without reasoning + // over a request that cannot be served at all. + if let Some(e) = effort { + obj.insert( + "reasoning".into(), + json!({ "effort": e.openai_effort_str() }), + ); + } + + // A7: Anthropic cache_control injection for anthropic/* models. Gated on + // `prompt_caching` (`BUZZ_AGENT_PROMPT_CACHING`) for the same reason as + // the native Anthropic route: these are Anthropic-dialect breakpoints on + // an Anthropic model, so the documented kill switch must reach them too. + if prompt_caching && effective_model.starts_with("anthropic/") { + apply_anthropic_cache_control(obj); + } + } +} + +fn apply_anthropic_cache_control(body: &mut serde_json::Map) { + if let Some(messages) = body.get_mut("messages").and_then(Value::as_array_mut) { + // Cache the system message + if let Some(system_msg) = messages + .iter_mut() + .find(|m| m.get("role").and_then(Value::as_str) == Some("system")) + { + if let Some(content) = system_msg.get("content").and_then(Value::as_str) { + let content_str = content.to_string(); + if let Some(obj) = system_msg.as_object_mut() { + obj.insert( + "content".into(), + json!([{ + "type": "text", + "text": content_str, + "cache_control": { "type": "ephemeral" } + }]), + ); + } + } + } + + // Cache last 2 user messages (skip image-only ones — A7 mixed-content regression) + let mut user_count = 0; + for msg in messages.iter_mut().rev() { + if msg.get("role").and_then(Value::as_str) != Some("user") { + continue; + } + // Only cache string content (plain text user messages), not array content + // (image batches from tool results). This prevents corrupting image-only + // user messages by converting them to text cache breakpoints. + if let Some(content) = msg.get("content").and_then(Value::as_str) { + let content_str = content.to_string(); + if let Some(obj) = msg.as_object_mut() { + obj.insert( + "content".into(), + json!([{ + "type": "text", + "text": content_str, + "cache_control": { "type": "ephemeral" } + }]), + ); + } + user_count += 1; + } + if user_count >= 2 { + break; + } + } + } + // Cache the last tool definition + if let Some(tools) = body.get_mut("tools").and_then(Value::as_array_mut) { + if let Some(last_tool) = tools.last_mut() { + if let Some(function) = last_tool.get_mut("function").and_then(Value::as_object_mut) { + function.insert("cache_control".into(), json!({ "type": "ephemeral" })); + } + } + } +} + #[cfg(test)] mod tests { use super::*; use crate::config::{Config, HookServers, OpenAiApi, Provider}; use crate::types::{HistoryItem, ToolCall, ToolResult, ToolResultContent}; + use std::collections::VecDeque; use std::time::Duration; + use tokio::sync::Mutex; use tracing_subscriber::layer::SubscriberExt; fn cfg(provider: Provider) -> Config { @@ -2492,6 +2969,7 @@ mod tests { arguments: serde_json::json!({"source":"x.png"}), provider_extra: Default::default(), }], + reasoning_details: None, }, HistoryItem::ToolResult(ToolResult { provider_id: "toolu_1".into(), @@ -2542,6 +3020,7 @@ mod tests { arguments: serde_json::json!({"command": "ls"}), provider_extra: Default::default(), }], + reasoning_details: None, }, HistoryItem::ToolResult(ToolResult { provider_id: "call_abc".into(), @@ -2641,6 +3120,7 @@ mod tests { arguments: serde_json::json!({}), provider_extra: Default::default(), }], + reasoning_details: None, }, ]; let body = responses_body(&cfg_responses(), "system", &history, &[], "model", None); @@ -2927,6 +3407,7 @@ mod tests { provider_extra: Default::default(), }, ], + reasoning_details: None, }, HistoryItem::ToolResult(ToolResult { provider_id: "toolu_a".into(), @@ -2988,6 +3469,7 @@ mod tests { HistoryItem::Assistant { text: "hi".into(), tool_calls: vec![], + reasoning_details: None, }, HistoryItem::User("more".into()), ], @@ -4003,6 +4485,7 @@ mod tests { let history = vec![HistoryItem::Assistant { text: r.text.clone(), tool_calls: r.tool_calls.clone(), + reasoning_details: None, }]; let body = openai_body( &cfg(Provider::DatabricksV2), @@ -4597,4 +5080,1476 @@ mod tests { }); assert_eq!(parse_responses(v).unwrap().output_tokens, None); } + + // ---- A3: OpenRouter body-shape tests ---- + + fn tools_vec() -> Vec { + vec![ToolDef { + name: "dev__shell".into(), + description: "run a shell command".into(), + input_schema: serde_json::json!({ + "type": "object", + "properties": {"command": {"type": "string"}}, + }), + }] + } + + #[test] + fn openrouter_body_tools_with_effort() { + let mut c = cfg(Provider::OpenRouter); + c.thinking_effort = Some(ThinkingEffort::High); + let mut body = openai_body( + &c, + "system", + &[HistoryItem::User("hi".into())], + &tools_vec(), + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations( + &mut body, + c.thinking_effort, + "anthropic/claude-opus-4-7", + true, + ); + assert_eq!(body["reasoning"]["effort"], "high"); + // `openai_body` is always called with `effort=None` on the OpenRouter + // path (line 151): OpenRouter uses its own `reasoning` object, not the + // OpenAI-style `reasoning_effort` field. Verify the field is structurally + // absent — not merely removed by a no-op cleanup. + assert!( + body.get("reasoning_effort").is_none(), + "reasoning_effort must never appear on the OpenRouter body path: \ + openai_body is called with effort=None, so the field is never emitted" + ); + assert!( + body.get("provider").is_none(), + "provider.require_parameters hard-404s models that do not advertise \ + every parameter we send" + ); + assert!(!body["tools"].as_array().unwrap().is_empty()); + assert_eq!( + body["max_tokens"], 1024, + "token limit must carry openai_body's value under OpenRouter's spelling" + ); + assert!( + body.get("max_completion_tokens").is_none(), + "max_completion_tokens is the OpenAI-native spelling; OpenRouter reads max_tokens" + ); + } + + #[test] + fn openrouter_body_tools_no_effort() { + let c = cfg(Provider::OpenRouter); + let mut body = openai_body( + &c, + "system", + &[HistoryItem::User("hi".into())], + &tools_vec(), + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations(&mut body, None, "anthropic/claude-opus-4-7", true); + assert!( + body.get("reasoning").is_none(), + "reasoning must be absent when effort is None" + ); + assert!( + body.get("reasoning_effort").is_none(), + "reasoning_effort must never appear on the OpenRouter body path" + ); + assert!( + body.get("provider").is_none(), + "tools alone must not add a provider routing filter" + ); + } + + #[test] + fn openrouter_body_empty_tools_with_effort() { + let mut c = cfg(Provider::OpenRouter); + c.thinking_effort = Some(ThinkingEffort::Medium); + let mut body = openai_body( + &c, + "system", + &[HistoryItem::User("hi".into())], + &[], + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations( + &mut body, + c.thinking_effort, + "anthropic/claude-opus-4-7", + true, + ); + assert_eq!(body["reasoning"]["effort"], "medium"); + assert!( + body.get("provider").is_none(), + "83 of 274 tools-capable models do not advertise `reasoning`; filtering on \ + it would 404 them instead of answering without reasoning" + ); + } + + #[test] + fn openrouter_body_empty_tools_no_effort() { + let c = cfg(Provider::OpenRouter); + let mut body = openai_body( + &c, + "system", + &[HistoryItem::User("hi".into())], + &[], + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations(&mut body, None, "anthropic/claude-opus-4-7", true); + assert!(body.get("reasoning").is_none()); + assert!( + body.get("provider").is_none(), + "no body shape adds a provider routing filter" + ); + } + + /// `BUZZ_AGENT_PROMPT_CACHING=0` must reach the OpenRouter `anthropic/*` + /// route too, not just the native Anthropic Messages routes. The switch and + /// this route landed in separate changes, so nothing but this test stops the + /// gate from being dropped and the kill switch silently becoming a no-op on + /// a route that emits Anthropic-dialect breakpoints. + #[test] + fn openrouter_body_caching_disabled_emits_no_cache_control() { + let c = cfg(Provider::OpenRouter); + let mut body = openai_body( + &c, + "system", + &[HistoryItem::User("hi".into())], + &tools_vec(), + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations(&mut body, None, "anthropic/claude-opus-4-7", false); + assert!( + !body.to_string().contains("cache_control"), + "BUZZ_AGENT_PROMPT_CACHING=0 must suppress every breakpoint: {body}" + ); + // The switch is scoped to caching — the routing-contract mutations that + // make the request serveable at all must still be applied. + assert_eq!( + body.get("max_tokens").and_then(Value::as_u64), + Some(u64::from(c.max_output_tokens)), + "the max_tokens rename is not part of the caching gate" + ); + } + + /// The paired positive case: with caching on, the same body does carry + /// breakpoints. Without this twin, a mutation that hard-disabled caching + /// outright would still leave the test above green. + #[test] + fn openrouter_body_caching_enabled_emits_cache_control() { + let c = cfg(Provider::OpenRouter); + let mut body = openai_body( + &c, + "system", + &[HistoryItem::User("hi".into())], + &tools_vec(), + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations(&mut body, None, "anthropic/claude-opus-4-7", true); + assert!( + body.to_string().contains("cache_control"), + "caching on must still emit breakpoints: {body}" + ); + } + + /// The rename moves an existing value; it never invents a token limit for a + /// body that did not carry one. + #[test] + fn openrouter_body_without_token_limit_gains_none() { + let mut body = json!({ "model": "vendor/model", "messages": [] }); + apply_openrouter_mutations(&mut body, None, "vendor/model", true); + assert!(body.get("max_tokens").is_none()); + assert!(body.get("max_completion_tokens").is_none()); + } + + #[test] + fn openrouter_summary_carries_neither_reasoning_nor_provider() { + let body = openrouter_summary_body( + "anthropic/claude-opus-4-7", + "summarize", + "text to summarize", + 1024, + ); + assert_eq!(body["model"], "anthropic/claude-opus-4-7"); + assert_eq!(body["messages"][0]["role"], "system"); + assert_eq!(body["messages"][1]["content"], "text to summarize"); + assert_eq!(body["max_tokens"], 1024); + assert!( + body.get("max_completion_tokens").is_none(), + "summary body must use OpenRouter's token-limit spelling" + ); + assert!( + body.get("reasoning").is_none(), + "summary body must not carry reasoning" + ); + assert!( + body.get("provider").is_none(), + "summary body must not carry provider" + ); + } + + /// The token-limit rename belongs to `apply_openrouter_mutations`, not to + /// `openai_body` — which OpenAI and Databricks also use, and where + /// `max_completion_tokens` is the correct spelling. + #[test] + fn openai_body_keeps_max_completion_tokens_when_unmutated() { + let body = openai_body( + &cfg(Provider::OpenAi), + "system", + &[HistoryItem::User("hi".into())], + &[], + "model", + None, + ); + assert_eq!(body["max_completion_tokens"], 1024); + assert!(body.get("max_tokens").is_none()); + } + + // ---- A5: error-inside-200 ---- + + #[test] + fn parse_openai_error_inside_200_returns_error() { + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "error", + "error": { + "code": 503, + "message": "No endpoints found that support tool use" + } + }] + }); + let err = parse_openai(v).unwrap_err(); + match &err { + AgentError::Llm(s) => { + assert!(s.contains("provider error (503)"), "got: {s}"); + assert!(s.contains("No endpoints found"), "got: {s}"); + } + _ => panic!("expected AgentError::Llm, got: {err:?}"), + } + } + + #[test] + fn parse_openai_error_inside_200_accepts_string_code() { + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "error", + "error": { + "code": "insufficient_quota", + "message": "quota exceeded" + } + }] + }); + let err = parse_openai(v).unwrap_err(); + match &err { + AgentError::Llm(s) => assert!( + s.contains("provider error (insufficient_quota)"), + "got: {s}" + ), + _ => panic!("expected AgentError::Llm, got: {err:?}"), + } + } + + #[test] + fn parse_openai_error_inside_200_surfaces_error_type() { + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "error", + "error": { + "code": 429, + "message": "Rate limit exceeded", + "metadata": { "error_type": "rate_limit_exceeded" } + } + }] + }); + let err = parse_openai(v).unwrap_err(); + match &err { + AgentError::Llm(s) => { + assert!(s.contains("429"), "got: {s}"); + assert!(s.contains("rate_limit_exceeded"), "got: {s}"); + } + _ => panic!("expected AgentError::Llm, got: {err:?}"), + } + } + + #[test] + fn parse_openai_normal_stop_not_affected_by_error_check() { + let v = serde_json::json!({ + "choices": [{"finish_reason": "stop", "message": {"content": "hello"}}], + "usage": {"prompt_tokens": 10, "completion_tokens": 5} + }); + let r = parse_openai(v).unwrap(); + assert_eq!(r.text, "hello"); + assert_eq!(r.stop, ProviderStop::EndTurn); + } + + #[test] + fn parse_openai_tool_calls_not_affected_by_error_check() { + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "tool_calls", + "message": { + "content": "", + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": {"name": "test", "arguments": "{}"} + }] + } + }] + }); + let r = parse_openai(v).unwrap(); + assert_eq!(r.stop, ProviderStop::ToolUse); + assert_eq!(r.tool_calls.len(), 1); + } + + // ---- A6: OpenRouter retry classification ---- + + #[test] + fn classify_429_rate_limit_body_retry_after_is_ignored() { + // M1: no documented body-level retry field, so a `retry_after` key + // inside `error.metadata` is inert — only the HTTP header (passed + // separately) can produce a delay. + let body = + r#"{"error":{"metadata":{"error_type":"rate_limit_exceeded","retry_after":2.5}}}"#; + match classify_openrouter_error(429, body, None) { + OpenRouterErrorClass::Retryable(None) => {} + other => panic!("expected Retryable(None), got: {other:?}"), + } + } + + #[test] + fn classify_429_rate_limit_without_retry_after() { + let body = r#"{"error":{"metadata":{"error_type":"rate_limit_exceeded"}}}"#; + match classify_openrouter_error(429, body, None) { + OpenRouterErrorClass::Retryable(None) => {} + other => panic!("expected Retryable(None), got: {other:?}"), + } + } + + #[test] + fn classify_429_prefers_http_header_over_body() { + // Body-level retry hints are no longer parsed (M1: undocumented field); + // the HTTP header is the only source, and it's honored when present. + let body = r#"{"error":{"metadata":{"error_type":"rate_limit_exceeded"}}}"#; + let header = Some(Duration::from_secs(3)); + match classify_openrouter_error(429, body, header) { + OpenRouterErrorClass::Retryable(Some(d)) => { + assert_eq!(d, Duration::from_secs(3), "HTTP header must be honored"); + } + other => panic!("expected Retryable with header delay, got: {other:?}"), + } + } + + #[test] + fn classify_502_provider_unavailable() { + let body = r#"{"error":{"metadata":{"error_type":"provider_unavailable"}}}"#; + match classify_openrouter_error(502, body, None) { + OpenRouterErrorClass::Retryable(None) => {} + other => panic!("expected Retryable(None) for 502, got: {other:?}"), + } + } + + #[test] + fn classify_503_provider_overloaded_with_retry_after() { + // No documented body-level retry field (M1); the header is the only + // source `classify_openrouter_error` consults. + let body = r#"{"error":{"metadata":{"error_type":"provider_overloaded"}}}"#; + let header = Some(Duration::from_secs(5)); + match classify_openrouter_error(503, body, header) { + OpenRouterErrorClass::Retryable(Some(d)) => { + assert_eq!(d, Duration::from_secs(5)); + } + other => panic!("expected Retryable with delay, got: {other:?}"), + } + } + + #[test] + fn classify_503_untyped_is_unknown() { + let body = r#"{"error":{"message":"No endpoints found"}}"#; + match classify_openrouter_error(503, body, None) { + OpenRouterErrorClass::Unknown => {} + other => panic!("expected Unknown for untyped 503, got: {other:?}"), + } + } + + #[test] + fn classify_500_untyped_is_unknown() { + match classify_openrouter_error(500, r#"{"error":{"message":"internal"}}"#, None) { + OpenRouterErrorClass::Unknown => {} + other => panic!("expected Unknown for 500, got: {other:?}"), + } + } + + #[test] + fn parse_retry_after_header_valid() { + let mut headers = reqwest::header::HeaderMap::new(); + headers.insert(reqwest::header::RETRY_AFTER, "5".parse().unwrap()); + assert_eq!( + parse_retry_after_header(&headers), + Some(Duration::from_secs(5)) + ); + } + + #[test] + fn parse_retry_after_header_zero_rejected() { + let mut headers = reqwest::header::HeaderMap::new(); + headers.insert(reqwest::header::RETRY_AFTER, "0".parse().unwrap()); + assert_eq!(parse_retry_after_header(&headers), None); + } + + #[test] + fn parse_retry_after_header_over_cap_clamped() { + let mut headers = reqwest::header::HeaderMap::new(); + headers.insert(reqwest::header::RETRY_AFTER, "3601".parse().unwrap()); + assert_eq!( + parse_retry_after_header(&headers), + Some(Duration::from_secs(RETRY_AFTER_CAP_SECS)), + "over-cap hints clamp to the ceiling rather than being dropped" + ); + } + + #[test] + fn parse_retry_after_header_at_cap_unclamped() { + let mut headers = reqwest::header::HeaderMap::new(); + headers.insert( + reqwest::header::RETRY_AFTER, + RETRY_AFTER_CAP_SECS.to_string().parse().unwrap(), + ); + assert_eq!( + parse_retry_after_header(&headers), + Some(Duration::from_secs(RETRY_AFTER_CAP_SECS)) + ); + } + + #[test] + fn parse_retry_after_header_missing() { + let headers = reqwest::header::HeaderMap::new(); + assert_eq!(parse_retry_after_header(&headers), None); + } + + #[test] + fn parse_retry_after_header_non_numeric_ignored() { + let mut headers = reqwest::header::HeaderMap::new(); + headers.insert( + reqwest::header::RETRY_AFTER, + "Wed, 21 Oct 2026 07:28:00 GMT".parse().unwrap(), + ); + assert_eq!(parse_retry_after_header(&headers), None); + } + + // ---- A7: Anthropic cache_control with mixed content ---- + + #[test] + fn anthropic_cache_control_mixed_text_tool_image_history() { + let history = vec![ + HistoryItem::User("first question".into()), + HistoryItem::Assistant { + text: String::new(), + tool_calls: vec![ToolCall { + provider_id: "toolu_1".into(), + name: "dev__view_image".into(), + arguments: serde_json::json!({"source": "x.png"}), + provider_extra: Default::default(), + }], + reasoning_details: None, + }, + HistoryItem::ToolResult(ToolResult { + provider_id: "toolu_1".into(), + content: vec![ + ToolResultContent::Text("10×10 image".into()), + ToolResultContent::Image { + data: "aW1n".into(), + mime_type: "image/png".into(), + }, + ], + is_error: false, + }), + HistoryItem::User("second question about the image".into()), + HistoryItem::User("third question".into()), + ]; + let mut body = openai_body( + &cfg(Provider::OpenRouter), + "system", + &history, + &tools_vec(), + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations(&mut body, None, "anthropic/claude-opus-4-7", true); + let messages = body["messages"].as_array().unwrap(); + + // System message should have cache_control + let system = &messages[0]; + assert_eq!(system["content"][0]["cache_control"]["type"], "ephemeral"); + + // Image-batch user messages (containing image_url blocks) must NOT have cache_control + let image_user_msgs: Vec<_> = messages + .iter() + .filter(|m| { + m.get("role").and_then(Value::as_str) == Some("user") + && m.get("content") + .and_then(Value::as_array) + .map(|a| { + a.iter() + .any(|b| b.get("type").and_then(Value::as_str) == Some("image_url")) + }) + .unwrap_or(false) + }) + .collect(); + assert!( + !image_user_msgs.is_empty(), + "should have image user messages" + ); + for img_msg in &image_user_msgs { + let content = img_msg["content"].as_array().unwrap(); + for block in content { + assert!( + block.get("cache_control").is_none(), + "image-only user message must not receive cache_control" + ); + } + } + + // Exactly 2 text user messages should have cache_control (skipping image-only ones) + let cached_text_count = messages + .iter() + .filter(|m| { + m.get("role").and_then(Value::as_str) == Some("user") + && m.get("content") + .and_then(Value::as_array) + .map(|a| { + a.iter().any(|b| { + b.get("type").and_then(Value::as_str) == Some("text") + && b.get("cache_control").is_some() + }) + }) + .unwrap_or(false) + }) + .count(); + assert_eq!( + cached_text_count, 2, + "exactly 2 text user messages should have cache_control" + ); + + // Last tool def should have cache_control + let tools = body["tools"].as_array().unwrap(); + let last_tool = tools.last().unwrap(); + assert_eq!(last_tool["function"]["cache_control"]["type"], "ephemeral"); + } + + #[test] + fn anthropic_cache_control_image_only_user_does_not_consume_slot() { + // An image-only user message between two text user messages must not + // consume a cache breakpoint slot — both text messages should get cached. + let history = vec![ + HistoryItem::User("text message one".into()), + HistoryItem::Assistant { + text: String::new(), + tool_calls: vec![ToolCall { + provider_id: "toolu_1".into(), + name: "dev__view_image".into(), + arguments: serde_json::json!({"source": "x.png"}), + provider_extra: Default::default(), + }], + reasoning_details: None, + }, + HistoryItem::ToolResult(ToolResult { + provider_id: "toolu_1".into(), + content: vec![ToolResultContent::Image { + data: "aW1n".into(), + mime_type: "image/png".into(), + }], + is_error: false, + }), + HistoryItem::User("text message two".into()), + HistoryItem::User("text message three".into()), + ]; + let mut body = openai_body( + &cfg(Provider::OpenRouter), + "system", + &history, + &[], + "anthropic/claude-opus-4-7", + None, + ); + apply_openrouter_mutations(&mut body, None, "anthropic/claude-opus-4-7", true); + let messages = body["messages"].as_array().unwrap(); + + // Count text user messages that got cache_control + let cached_text_count = messages + .iter() + .filter(|m| { + m.get("role").and_then(Value::as_str) == Some("user") + && m.get("content") + .and_then(Value::as_array) + .map(|a| a.iter().any(|b| b.get("cache_control").is_some())) + .unwrap_or(false) + }) + .count(); + assert_eq!( + cached_text_count, 2, + "image-only user messages must not consume a cache breakpoint slot" + ); + } + + // ---- A9: reasoning_details round-trip ---- + + #[test] + fn parse_openai_with_reasoning_details_captures_array() { + let details = serde_json::json!([ + {"type": "thinking", "content": "Let me consider..."}, + {"type": "thinking", "content": "The answer is 42."} + ]); + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "tool_calls", + "message": { + "content": "", + "reasoning_details": details, + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": {"name": "test", "arguments": "{}"} + }] + } + }], + "usage": {"prompt_tokens": 10, "completion_tokens": 5} + }); + let r = parse_openai_with_reasoning_details(v).unwrap(); + assert_eq!(r.reasoning_details, Some(details)); + } + + #[test] + fn parse_openai_with_reasoning_details_none_when_absent() { + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "stop", + "message": {"content": "hello"} + }] + }); + let r = parse_openai_with_reasoning_details(v).unwrap(); + assert!( + r.reasoning_details.is_none(), + "reasoning_details must be None when not in response" + ); + } + + /// M2: a malformed `reasoning_details` shape (null or a bare object, + /// rather than the documented array) must be omitted at the parse + /// boundary, not stored and later replayed into the next request. + #[test] + fn parse_openai_with_reasoning_details_omits_non_array_shapes() { + let null_shape = serde_json::json!({ + "choices": [{ + "finish_reason": "stop", + "message": {"content": "hello", "reasoning_details": null} + }] + }); + let r = parse_openai_with_reasoning_details(null_shape).unwrap(); + assert!( + r.reasoning_details.is_none(), + "null reasoning_details must be omitted, not stored as Some(Null)" + ); + + let object_shape = serde_json::json!({ + "choices": [{ + "finish_reason": "stop", + "message": { + "content": "hello", + "reasoning_details": {"type": "thinking", "content": "not an array"} + } + }] + }); + let r = parse_openai_with_reasoning_details(object_shape).unwrap(); + assert!( + r.reasoning_details.is_none(), + "a bare-object reasoning_details must be omitted, not stored as Some(object)" + ); + } + + #[test] + fn parse_openai_plain_never_captures_reasoning_details() { + let v = serde_json::json!({ + "choices": [{ + "finish_reason": "stop", + "message": { + "content": "hello", + "reasoning_details": [{"type": "thinking", "content": "hmm"}] + } + }] + }); + let r = parse_openai(v).unwrap(); + assert!( + r.reasoning_details.is_none(), + "plain parse_openai must never capture reasoning_details (OpenAI/Databricks regression)" + ); + } + + #[test] + fn reasoning_details_two_request_round_trip() { + let details = serde_json::json!([ + {"type": "thinking", "content": "Step 1: analyze the request."}, + {"type": "thinking", "content": "Step 2: call the tool."} + ]); + // Request 1: model returns a tool call with reasoning_details + let response1 = serde_json::json!({ + "choices": [{ + "finish_reason": "tool_calls", + "message": { + "content": "", + "reasoning_details": details, + "tool_calls": [{ + "id": "call_abc", + "type": "function", + "function": {"name": "dev__shell", "arguments": "{\"command\":\"ls\"}"} + }] + } + }], + "usage": {"prompt_tokens": 50, "completion_tokens": 20} + }); + let r1 = parse_openai_with_reasoning_details(response1).unwrap(); + assert_eq!(r1.reasoning_details, Some(details.clone())); + + // Build history as the agent would: assistant turn with reasoning_details, + // followed by a tool result. + let history = vec![ + HistoryItem::User("run ls".into()), + HistoryItem::Assistant { + text: String::new(), + tool_calls: r1.tool_calls, + reasoning_details: r1.reasoning_details, + }, + HistoryItem::ToolResult(ToolResult { + provider_id: "call_abc".into(), + content: vec![ToolResultContent::Text("file.txt".into())], + is_error: false, + }), + ]; + + // Request 2: build the body for the continuation + let body = openai_body( + &cfg(Provider::OpenRouter), + "system", + &history, + &[], + "anthropic/claude-opus-4-7", + None, + ); + let messages = body["messages"].as_array().unwrap(); + + // The assistant message must carry the identical reasoning_details array + let assistant_msg = messages + .iter() + .find(|m| m.get("role").and_then(Value::as_str) == Some("assistant")) + .expect("assistant message must exist"); + assert_eq!( + assistant_msg["reasoning_details"], details, + "reasoning_details must be replayed byte-for-byte on the assistant message" + ); + + // The assistant message must appear BEFORE the tool result + let assistant_idx = messages + .iter() + .position(|m| m.get("role").and_then(Value::as_str) == Some("assistant")) + .unwrap(); + let tool_idx = messages + .iter() + .position(|m| m.get("role").and_then(Value::as_str) == Some("tool")) + .unwrap(); + assert!( + assistant_idx < tool_idx, + "assistant with reasoning_details must precede tool result" + ); + } + + #[test] + fn reasoning_details_none_emits_no_field_in_body() { + let history = vec![ + HistoryItem::User("hello".into()), + HistoryItem::Assistant { + text: "hi back".into(), + tool_calls: Vec::new(), + reasoning_details: None, + }, + ]; + let body = openai_body( + &cfg(Provider::OpenRouter), + "system", + &history, + &[], + "anthropic/claude-opus-4-7", + None, + ); + let messages = body["messages"].as_array().unwrap(); + let assistant_msg = messages + .iter() + .find(|m| m.get("role").and_then(Value::as_str) == Some("assistant")) + .expect("assistant message must exist"); + assert!( + assistant_msg.get("reasoning_details").is_none(), + "assistant with None reasoning_details must not emit the field" + ); + } + + #[test] + fn reasoning_details_charged_to_estimated_bytes() { + let details = serde_json::json!([ + {"type": "thinking", "content": "A long chain of reasoning tokens here."} + ]); + let with = HistoryItem::Assistant { + text: "text".into(), + tool_calls: Vec::new(), + reasoning_details: Some(details.clone()), + }; + let without = HistoryItem::Assistant { + text: "text".into(), + tool_calls: Vec::new(), + reasoning_details: None, + }; + assert!( + with.estimated_bytes() > without.estimated_bytes(), + "reasoning_details must contribute to estimated_bytes" + ); + assert!( + with.context_pressure_bytes() > without.context_pressure_bytes(), + "reasoning_details must contribute to context_pressure_bytes" + ); + let details_size = serde_json::to_vec(&details).unwrap().len(); + assert_eq!( + with.estimated_bytes() - without.estimated_bytes(), + details_size, + "reasoning_details contribution must equal its serialized size" + ); + } + + #[test] + fn reasoning_details_not_replayed_in_anthropic_body() { + let history = vec![ + HistoryItem::User("hi".into()), + HistoryItem::Assistant { + text: "ok".into(), + tool_calls: Vec::new(), + reasoning_details: Some( + serde_json::json!([{"type": "thinking", "content": "hmm"}]), + ), + }, + ]; + let body = anthropic_body( + &cfg(Provider::Anthropic), + "system", + &history, + &[], + "claude-opus-4-7", + None, + ); + let messages = body["messages"].as_array().unwrap(); + let assistant = messages + .iter() + .find(|m| m.get("role").and_then(Value::as_str) == Some("assistant")) + .unwrap(); + assert!( + assistant.get("reasoning_details").is_none(), + "anthropic_body must not replay reasoning_details" + ); + } + + #[test] + fn reasoning_details_not_replayed_in_responses_body() { + let history = vec![ + HistoryItem::User("hi".into()), + HistoryItem::Assistant { + text: "ok".into(), + tool_calls: Vec::new(), + reasoning_details: Some( + serde_json::json!([{"type": "thinking", "content": "hmm"}]), + ), + }, + ]; + let body = responses_body(&cfg_responses(), "system", &history, &[], "model", None); + let body_str = serde_json::to_string(&body).unwrap(); + assert!( + !body_str.contains("reasoning_details"), + "responses_body must not replay reasoning_details" + ); + } + + // ---- T4: openrouter_post transport-level regressions ---- + // + // These stub an HTTP server directly and drive `openrouter_post` (not the + // classifier in isolation), proving the retry/attempt-accounting and + // header behavior the classifier-only tests above cannot see. + + /// One canned response: status, body, and any extra headers (e.g. + /// `Retry-After`) to send back for a single request. + struct CannedResponse { + status: u16, + body: String, + extra_headers: Vec<(String, String)>, + } + + impl CannedResponse { + fn new(status: u16, body: &str) -> Self { + Self { + status, + body: body.into(), + extra_headers: Vec::new(), + } + } + + fn with_header(mut self, name: &str, value: &str) -> Self { + self.extra_headers.push((name.into(), value.into())); + self + } + } + + fn status_line(status: u16) -> &'static str { + match status { + 200 => "200 OK", + 401 => "401 Unauthorized", + 402 => "402 Payment Required", + 403 => "403 Forbidden", + 404 => "404 Not Found", + 429 => "429 Too Many Requests", + 499 => "499 Client Closed Request", + 500 => "500 Internal Server Error", + 502 => "502 Bad Gateway", + 503 => "503 Service Unavailable", + _ => panic!("unsupported status {status} in test stub"), + } + } + + /// Spawns a stub HTTP server that pops one `CannedResponse` per request + /// (repeating the last one once the queue is exhausted, so an + /// over-budget attempt count is visible rather than hanging), and + /// captures each request's raw header block for header-attribution + /// assertions. Returns (url, captured_header_blocks, attempt_counter). + async fn spawn_openrouter_stub( + responses: Vec, + ) -> ( + String, + Arc>>, + Arc, + ) { + use std::sync::atomic::{AtomicU32, Ordering}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + let queue = Arc::new(Mutex::new(VecDeque::from(responses))); + let captured: Arc>> = Arc::new(Mutex::new(Vec::new())); + let attempts = Arc::new(AtomicU32::new(0)); + let captured_clone = captured.clone(); + let attempts_clone = attempts.clone(); + tokio::spawn(async move { + loop { + let (mut sock, _) = match listener.accept().await { + Ok(p) => p, + Err(_) => return, + }; + let queue = queue.clone(); + let captured = captured_clone.clone(); + let attempts = attempts_clone.clone(); + tokio::spawn(async move { + let mut buf = Vec::new(); + let mut tmp = [0u8; 4096]; + while !buf.windows(4).any(|w| w == b"\r\n\r\n") { + match sock.read(&mut tmp).await { + Ok(0) | Err(_) => return, + Ok(n) => buf.extend_from_slice(&tmp[..n]), + } + if buf.len() > 1_000_000 { + return; + } + } + let header_end = buf.windows(4).position(|w| w == b"\r\n\r\n").unwrap() + 4; + let header_str = String::from_utf8_lossy(&buf[..header_end]).into_owned(); + // Drain any remaining body per Content-Length so `Connection: + // close` doesn't race the client's write. + let content_length: usize = header_str + .lines() + .find_map(|line| { + line.to_ascii_lowercase() + .strip_prefix("content-length:") + .and_then(|v| v.trim().parse().ok()) + }) + .unwrap_or(0); + let mut body_len = buf.len() - header_end; + while body_len < content_length { + match sock.read(&mut tmp).await { + Ok(0) | Err(_) => break, + Ok(n) => body_len += n, + } + } + captured.lock().await.push(header_str); + attempts.fetch_add(1, Ordering::SeqCst); + + let mut q = queue.lock().await; + let canned = if q.len() > 1 { + q.pop_front().unwrap() + } else { + // Repeat the final canned response so a test bug that + // over-retries produces a visible extra attempt + // instead of a hung connection. + let last = q.front().unwrap(); + CannedResponse { + status: last.status, + body: last.body.clone(), + extra_headers: last.extra_headers.clone(), + } + }; + drop(q); + + let mut resp = format!( + "HTTP/1.1 {}\r\nContent-Type: application/json\r\nContent-Length: {}\r\n", + status_line(canned.status), + canned.body.len() + ); + for (name, value) in &canned.extra_headers { + resp.push_str(&format!("{name}: {value}\r\n")); + } + resp.push_str("Connection: close\r\n\r\n"); + resp.push_str(&canned.body); + let _ = sock.write_all(resp.as_bytes()).await; + let _ = sock.shutdown().await; + }); + } + }); + (url, captured, attempts) + } + + /// A 403 (guardrail/moderation/permission rejection, per OpenRouter docs) + /// must NOT be classified as `LlmAuth`: refreshing a static key returns + /// the identical key, so retrying would just waste a duplicate request. + /// Exactly one attempt, plain `AgentError::Llm` with the body preserved. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_403_single_attempt_not_auth_error() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 403, + r#"{"error":{"message":"model flagged by moderation"}}"#, + )]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::Llm(s) if s.contains("403") && s.contains("model flagged by moderation")), + "403 must surface as AgentError::Llm with status+body, not LlmAuth: got {err:?}" + ); + assert_eq!( + attempts.load(std::sync::atomic::Ordering::SeqCst), + 1, + "403 must not be retried (a refreshed static key is identical)" + ); + } + + /// A 402 short-circuits on the first attempt: no retry, one request, + /// the actionable credits-exhausted message. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_402_single_attempt_short_circuit() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 402, + r#"{"error":{"message":"payment required"}}"#, + )]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::Llm(s) if s.contains("credits exhausted")), + "got {err:?}" + ); + assert_eq!( + attempts.load(std::sync::atomic::Ordering::SeqCst), + 1, + "402 must not be retried" + ); + } + + /// A 404 whose body is OpenRouter's parameter-routing rejection is NOT a + /// missing model: the id is valid and no endpoint behind it can serve the + /// request shape. It must surface the actionable routing message rather than + /// `LlmModelNotFound`, which sends the user hunting a model-name typo. + /// Body text is the one OpenRouter actually returned in the live probe run. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_404_no_endpoints_found_is_parameter_routing_error() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 404, + r#"{"error":{"message":"No endpoints found that can handle the requested parameters. To learn more about provider routing, visit: https://openrouter.ai/docs/guides/routing/provider-selection","code":404}}"#, + )]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::Llm(s) if s.contains("no OpenRouter endpoint supports")), + "parameter-routing 404 must not be reported as a missing model: got {err:?}" + ); + assert_eq!( + attempts.load(std::sync::atomic::Ordering::SeqCst), + 1, + "404 must not be retried" + ); + } + + /// Every other 404 still maps to `LlmModelNotFound`, including one that + /// shares the `No endpoints found` prefix but is about the model rather than + /// the parameters — the discriminator is narrow enough that a genuinely + /// unavailable model keeps its own error kind (Desktop renders + /// model-not-found differently from a generic LLM failure). + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_404_unknown_model_stays_model_not_found() { + let (url, _captured, _attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 404, + r#"{"error":{"message":"No endpoints found for vendor/nonexistent-model.","code":404}}"#, + )]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::LlmModelNotFound(s) if s.contains("404") && s.contains("vendor/nonexistent-model")), + "a model-level 404 must stay LlmModelNotFound: got {err:?}" + ); + } + + /// A 429 with `Retry-After: 1` sleeps for that duration before the retry + /// succeeds — proving the header value is actually honored, not just + /// classified. + #[tokio::test(flavor = "current_thread")] + async fn openrouter_post_429_honors_retry_after_header() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![ + CannedResponse::new(429, r#"{"error":{"message":"rate limited"}}"#) + .with_header("Retry-After", "1"), + CannedResponse::new(200, r#"{"choices":[{"message":{"content":"ok"}}]}"#), + ]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(30)) + .build() + .unwrap(); + let before = std::time::Instant::now(); + let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .expect("second attempt succeeds"); + assert_eq!(out["choices"][0]["message"]["content"], "ok"); + assert!( + before.elapsed() >= Duration::from_secs(1), + "must sleep at least the Retry-After hint" + ); + assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 2); + } + + /// A `Retry-After` far beyond `RETRY_AFTER_CAP_SECS` must not stall the + /// retry loop for anywhere near its advertised duration — proving the + /// cap is enforced end-to-end in `openrouter_post`'s actual sleep, not + /// merely in the isolated `parse_retry_after_header` unit tests above. + /// Runs on a paused clock so a real 999999s wait would hang the test + /// instead of silently passing. + #[tokio::test(start_paused = true)] + async fn openrouter_post_429_retry_sleep_capped_despite_huge_retry_after() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![ + CannedResponse::new(429, r#"{"error":{"message":"rate limited"}}"#) + .with_header("Retry-After", "999999"), + CannedResponse::new(200, r#"{"choices":[{"message":{"content":"ok"}}]}"#), + ]) + .await; + let http = Client::builder().build().unwrap(); + let before = tokio::time::Instant::now(); + let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .expect("second attempt succeeds"); + assert_eq!(out["choices"][0]["message"]["content"], "ok"); + assert!( + before.elapsed() <= Duration::from_secs(RETRY_AFTER_CAP_SECS + 5), + "retry sleep must be clamped to RETRY_AFTER_CAP_SECS ({RETRY_AFTER_CAP_SECS}s), \ + not the header's 999999s: elapsed {:?}", + before.elapsed() + ); + assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 2); + } + + /// An untyped 503 (no `error.metadata.error_type`) exhausts all + /// `MAX_RETRIES` attempts, then returns the actionable routing message — + /// proving attempt accounting terminates rather than retrying forever. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_untyped_503_exhausts_retries_into_actionable_message() { + let canned = CannedResponse::new(503, r#"{"error":{"message":"no capacity"}}"#); + let (url, _captured, attempts) = spawn_openrouter_stub(vec![canned]).await; + let http = Client::builder() + .timeout(Duration::from_secs(30)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::Llm(s) if s.contains("no OpenRouter endpoint supports")), + "got {err:?}" + ); + assert_eq!( + attempts.load(std::sync::atomic::Ordering::SeqCst), + MAX_RETRIES, + "must exhaust exactly MAX_RETRIES attempts, no more" + ); + } + + /// Attribution headers (`HTTP-Referer`, `X-OpenRouter-Title`) are on the + /// actual wire request, not merely asserted against a body fixture. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_sends_attribution_headers() { + let (url, captured, _attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 200, + r#"{"choices":[{"message":{"content":"ok"}}]}"#, + )]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .expect("200 succeeds"); + let headers = captured.lock().await; + let header_str = headers + .first() + .expect("one request captured") + .to_lowercase(); + assert!( + header_str.contains("http-referer: https://github.com/block/buzz"), + "got: {header_str}" + ); + assert!( + header_str.contains("x-openrouter-title: buzz"), + "got: {header_str}" + ); + } + + /// A 499 response is retried and the call succeeds on the second attempt, + /// mirroring the shared `post()` path (#2175). + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_retries_499_then_succeeds() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![ + CannedResponse::new(499, ""), + CannedResponse::new(200, r#"{"choices":[{"message":{"content":"ok"}}]}"#), + ]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(30)) + .build() + .unwrap(); + let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .expect("retry after 499 should succeed"); + assert_eq!(out["choices"][0]["message"]["content"], "ok"); + assert_eq!( + attempts.load(std::sync::atomic::Ordering::SeqCst), + 2, + "exactly one 499 retry" + ); + } + + /// A 502 without `provider_unavailable` still retries (the classifier's + /// unconditional-retry branch), then succeeds on attempt 2 — proving the + /// generic 502 path isn't accidentally routed to `Unknown`/terminal. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_502_retries_then_succeeds() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![ + CannedResponse::new(502, r#"{"error":{"message":"bad gateway"}}"#), + CannedResponse::new(200, r#"{"choices":[{"message":{"content":"ok"}}]}"#), + ]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(30)) + .build() + .unwrap(); + let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .expect("retry succeeds"); + assert_eq!(out["choices"][0]["message"]["content"], "ok"); + assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 2); + } + + /// A 200 response whose body is truncated mid-stream (connection closed + /// before the full Content-Length is delivered) must surface the error + /// through `terminal_llm_error`, not a bare `AgentError::Llm("read: …")` — + /// the caller needs cumulative duration and attempt-count context to diagnose + /// an upstream that silently drops connections. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_truncated_200_body_wraps_in_terminal_error() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + // Spawn a stub that sends a 200 with Content-Length > actual body, + // then closes the connection — reqwest sees a truncated stream. + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + tokio::spawn(async move { + if let Ok((mut sock, _)) = listener.accept().await { + let mut buf = [0u8; 4096]; + // Read until end-of-headers, ignore body + loop { + match sock.read(&mut buf).await { + Ok(0) | Err(_) => break, + Ok(n) => { + if buf[..n].windows(4).any(|w| w == b"\r\n\r\n") { + break; + } + } + } + } + // Claim 1 MB of body, send only 10 bytes, then close. + let _ = sock + .write_all( + b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\ + Content-Length: 1048576\r\nConnection: close\r\n\r\n\ + {truncated", + ) + .await; + let _ = sock.shutdown().await; + } + }); + + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::Llm(s) if s.contains("body read")), + "truncated body must surface as AgentError::Llm with 'body read': got {err:?}" + ); + assert!( + matches!(&err, AgentError::Llm(s) if s.contains("cumulative")), + "body-read error must include terminal_llm_error's cumulative context: got {err:?}" + ); + } + + /// A `TokenSource` whose `refresh_now` always returns the identical token — + /// models a static API key whose bytes never change on refresh. + struct StaticAuth { + token: String, + } + + #[async_trait::async_trait] + impl TokenSource for StaticAuth { + async fn bearer(&self) -> Result { + Ok(self.token.clone()) + } + async fn refresh_now(&self, _rejected: &str) -> Result { + Ok(self.token.clone()) // static: same bytes every time + } + } + + /// A `TokenSource` that returns a stale token from `bearer()` and a + /// distinct fresh token from `refresh_now()`, modelling a PKCE OAuth source. + struct MintingAuth { + stale: String, + fresh: String, + refreshes: std::sync::atomic::AtomicU32, + } + + #[async_trait::async_trait] + impl TokenSource for MintingAuth { + async fn bearer(&self) -> Result { + Ok(self.stale.clone()) + } + async fn refresh_now(&self, _rejected: &str) -> Result { + self.refreshes + .fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(self.fresh.clone()) + } + } + + /// A static key 401: `refresh_now` returns the same bytes — the second + /// wire request would be byte-identical, so the retry must be skipped. + /// Exactly one wire request reaches the server. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn post_openrouter_static_key_401_single_attempt_no_retry() { + use std::sync::atomic::Ordering; + + let (url, _captured, attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 401, + r#"{"error":{"message":"invalid api key"}}"#, + )]) + .await; + let auth = Arc::new(StaticAuth { + token: "static-key".into(), + }); + let llm = llm_with(auth); + let mut c = cfg(Provider::OpenRouter); + c.base_url = url; + + let err = llm.post_openrouter(&c, &json!({})).await.unwrap_err(); + assert!( + matches!(&err, AgentError::LlmAuth(s) if s.contains("static key rejected")), + "static 401 must surface as LlmAuth with 'static key rejected': got {err:?}" + ); + assert_eq!( + attempts.load(Ordering::SeqCst), + 1, + "exactly one wire request — no duplicate retry for a static key" + ); + } + + /// A minting-source 401: `refresh_now` produces a distinct fresh token, so + /// the retry is legitimate. The stub accepts the fresh token's second + /// request with 200 and exactly two wire requests reach the server. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn post_openrouter_minting_source_401_retries_with_fresh_token() { + use std::sync::atomic::Ordering; + + // Stub: always 401 for bearer "stale", 200 for anything else. + // We repurpose `spawn_auth_stub` here: it rejects `Bearer stale`, + // accepts `Bearer fresh`. + let always_401 = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let base = spawn_auth_stub(always_401, 401).await; + + let auth = Arc::new(MintingAuth { + stale: "stale".into(), + fresh: "fresh".into(), + refreshes: std::sync::atomic::AtomicU32::new(0), + }); + let llm = llm_with(auth.clone()); + let mut c = cfg(Provider::OpenRouter); + c.base_url = base; + + let result = llm.post_openrouter(&c, &json!({})).await; + // `spawn_auth_stub` returns `{"ok":true}` on success. + assert!( + result.is_ok(), + "minting-source retry should succeed: {result:?}" + ); + assert_eq!( + auth.refreshes.load(Ordering::SeqCst), + 1, + "exactly one refresh" + ); + } } diff --git a/crates/buzz-agent/src/types.rs b/crates/buzz-agent/src/types.rs index 31172def6d..343a75bf72 100644 --- a/crates/buzz-agent/src/types.rs +++ b/crates/buzz-agent/src/types.rs @@ -62,6 +62,7 @@ pub enum HistoryItem { Assistant { text: String, tool_calls: Vec, + reasoning_details: Option, }, ToolResult(ToolResult), } @@ -83,7 +84,11 @@ impl HistoryItem { fn size_with(&self, content_size: fn(&ToolResultContent) -> usize) -> usize { match self { Self::User(s) => s.len(), - Self::Assistant { text, tool_calls } => { + Self::Assistant { + text, + tool_calls, + reasoning_details, + } => { text.len() + tool_calls .iter() @@ -102,6 +107,11 @@ impl HistoryItem { .unwrap_or(0) }) .sum::() + + reasoning_details + .as_ref() + .and_then(|v| serde_json::to_vec(v).ok()) + .map(|b| b.len()) + .unwrap_or(0) } Self::ToolResult(r) => { r.provider_id.len() + r.content.iter().map(content_size).sum::() @@ -188,6 +198,10 @@ pub struct LlmResponse { /// /// Empty string when the provider returned no reasoning content. pub reasoning: String, + /// Raw `reasoning_details` array from an OpenRouter response, if present. + /// Replayed on subsequent turns so the model can continue its chain-of-thought. + /// `None` for all non-OpenRouter providers. + pub reasoning_details: Option, } #[derive(Debug, Clone, Copy, PartialEq)] @@ -481,6 +495,7 @@ mod tests { arguments: Value::Null, provider_extra: extra, }], + reasoning_details: None, }; let without_extra = HistoryItem::Assistant { text: String::new(), @@ -490,6 +505,7 @@ mod tests { arguments: Value::Null, provider_extra: Map::new(), }], + reasoning_details: None, }; assert!(with_extra.estimated_bytes() > without_extra.estimated_bytes() + 500); assert_eq!( diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index ca1fe9bdf6..7ce03b140b 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -99,6 +99,17 @@ pub async fn get_agent_models( // so a build-provided provider still gets live discovery. let effective_provider = effective_discovery_provider(saved_provider.as_deref(), provider_env_var, &merged_env); + if let Some(models) = discover_openrouter_models( + &state.http_client, + &effective_provider, + &merged_env, + persisted_model.clone(), + ) + .await? + { + return Ok(models); + } + if let Some(models) = discover_openai_compatible_models( &state.http_client, &effective_provider, @@ -154,69 +165,11 @@ fn model_discovery_error(pubkey: &str, error: &str) -> String { ) } -/// Everything `get_agent_models` needs from the record + context, resolved in -/// one pure step so the linked-agent regression test can bind the exact values -/// the command consumes. -#[derive(Debug, PartialEq, Eq)] -struct AgentModelDiscoveryConfig { - /// Effective harness command (descriptor-resolved), for `resolve_command`. - command: String, - /// Effective harness args (descriptor-resolved). - args: Vec, - /// Model from the authoritative resolver spawn uses — linked instances - /// read their definition, never stale `record.model` bytes. - model: Option, - /// Provider from the same authoritative resolver — never stale - /// `record.provider` bytes for linked instances. - provider: Option, - /// The runtime's provider env var (e.g. `GOOSE_PROVIDER`), so discovery - /// can recover the provider from the env when the resolver yields none. - /// `None` for runtimes that do not take a provider, or an unknown command. - provider_env_var: Option<&'static str>, - /// The descriptor's fully layered env (definition/persona/global/agent). - env: BTreeMap, -} - -/// Resolve the model-discovery config for a saved agent — the descriptor-backed -/// successor to the old `saved_agent_model_discovery_config`. -/// -/// Command/args/env come from `resolve_effective_harness_descriptor` (the same -/// resolver as `spawn_agent_child`); model/provider come from -/// `resolve_effective_model_provider` (#1968's definition-authoritative -/// contract) — linked instances read their definition, never a stale -/// materialized `record.model`/`record.provider`, so discovery cannot query a -/// provider this agent will not actually launch with. Definition-less -/// instances keep their own record values, matching spawn's -/// `resolve_definition_less` arm. When the resolver yields no provider, -/// `effective_discovery_provider` recovers the provider the agent will -/// actually launch with from the runtime's own provider env var, read out of -/// the descriptor env (which already layers definition/persona/global values -/// the same way spawn does). -/// -/// Returns `Err("DANGLING_HARNESS_ID:")` from the descriptor resolver when -/// the harness id no longer exists; the caller routes it through -/// `model_discovery_error`. -fn agent_model_discovery_config( - record: &crate::managed_agents::ManagedAgentRecord, - personas: &[crate::managed_agents::AgentDefinition], - global: &crate::managed_agents::GlobalAgentConfig, -) -> Result { - let descriptor = - crate::managed_agents::resolve_effective_harness_descriptor(record, personas, global)?; - let (model, provider) = - crate::managed_agents::resolve_effective_model_provider(record, personas, global); - let provider_env_var = - known_acp_runtime(&descriptor.command).and_then(|meta| meta.provider_env_var); - - Ok(AgentModelDiscoveryConfig { - command: descriptor.command, - args: descriptor.args, - model, - provider, - provider_env_var, - env: descriptor.env, - }) -} +#[path = "agent_models_discovery_config.rs"] +mod discovery_config; +use discovery_config::{ + agent_model_discovery_config, draft_agent_model_discovery_env, AgentModelDiscoveryConfig, +}; #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -269,31 +222,12 @@ pub async fn discover_agent_models( .unwrap_or_else(|| agent_command.to_string()); let runtime_meta = known_acp_runtime(agent_command); - let mut derived_env = BTreeMap::new(); - if let Some(meta) = runtime_meta { - let provider = input - .provider - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()); - if !meta.provider_locked { - if let (Some(env_key), Some(provider)) = (meta.provider_env_var, provider) { - derived_env.insert(env_key.to_string(), provider.to_string()); - } - } - } - // Layer definition_env below user env_vars so user overrides always win. - // Reserved keys are stripped, matching the same filter applied at spawn. - let mut filtered_definition_env = BTreeMap::new(); - for (key, value) in &input.definition_env { - if !crate::managed_agents::is_reserved_env_key(key) { - filtered_definition_env.insert(key.clone(), value.clone()); - } - } - // Merge: derived (metadata) → definition env → user env_vars. - let merged_with_def = - crate::managed_agents::merged_user_env(&derived_env, &filtered_definition_env); - let merged_env = crate::managed_agents::merged_user_env(&merged_with_def, &input.env_vars); + let merged_env = draft_agent_model_discovery_env( + agent_command, + input.provider.as_deref(), + &input.definition_env, + &input.env_vars, + ); let merged_env = discovery_env_with_baked_floor(merged_env); // Recover a build-provided provider when the form has none, so the create // dialog discovers live models instead of falling through to the subprocess. @@ -348,6 +282,13 @@ pub async fn discover_agent_models( return Err("Buzz shared compute is not available in this build".to_string()); } + if let Some(models) = + discover_openrouter_models(&state.http_client, &effective_provider, &merged_env, None) + .await? + { + return Ok(models); + } + if let Some(models) = discover_openai_compatible_models( &state.http_client, &effective_provider, @@ -388,6 +329,15 @@ struct OpenAiModelListItem { created: Option, } +#[path = "agent_models_openrouter.rs"] +mod openrouter; +use openrouter::discover_openrouter_models; +#[cfg(test)] +use openrouter::{ + filter_openrouter_models, is_openrouter_provider, openrouter_models_url, + OpenRouterModelListItem, OpenRouterModelListResponse, +}; + fn is_openai_compatible_provider(provider: Option<&str>) -> bool { matches!( provider diff --git a/desktop/src-tauri/src/commands/agent_models_discovery_config.rs b/desktop/src-tauri/src/commands/agent_models_discovery_config.rs new file mode 100644 index 0000000000..e43f09495b --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_discovery_config.rs @@ -0,0 +1,112 @@ +//! Model-discovery configuration resolution for the agent-models commands. +//! +//! Two entry points, one per call shape: [`agent_model_discovery_config`] +//! resolves a *saved* agent through the same descriptor/model resolvers spawn +//! uses, and [`draft_agent_model_discovery_env`] derives the env for an unsaved +//! form. Both are pure so the regression tests can bind the exact values the +//! commands consume. +//! +//! Included from `agent_models.rs` via `#[path]`, so `super::*` resolves +//! against that module (the `agent_models_tests.rs` convention). + +use std::collections::BTreeMap; + +use crate::managed_agents::known_acp_runtime; + +/// Everything `get_agent_models` needs from the record + context, resolved in +/// one pure step so the linked-agent regression test can bind the exact values +/// the command consumes. +#[derive(Debug, PartialEq, Eq)] +pub(super) struct AgentModelDiscoveryConfig { + /// Effective harness command (descriptor-resolved), for `resolve_command`. + pub(super) command: String, + /// Effective harness args (descriptor-resolved). + pub(super) args: Vec, + /// Model from the authoritative resolver spawn uses — linked instances + /// read their definition, never stale `record.model` bytes. + pub(super) model: Option, + /// Provider from the same authoritative resolver — never stale + /// `record.provider` bytes for linked instances. + pub(super) provider: Option, + /// The runtime's provider env var (e.g. `GOOSE_PROVIDER`), so discovery + /// can recover the provider from the env when the resolver yields none. + /// `None` for runtimes that do not take a provider, or an unknown command. + pub(super) provider_env_var: Option<&'static str>, + /// The descriptor's fully layered env (definition/persona/global/agent). + pub(super) env: BTreeMap, +} + +/// Resolve the model-discovery config for a saved agent — the descriptor-backed +/// successor to the old `saved_agent_model_discovery_config`. +/// +/// Command/args/env come from `resolve_effective_harness_descriptor` (the same +/// resolver as `spawn_agent_child`); model/provider come from +/// `resolve_effective_model_provider` (#1968's definition-authoritative +/// contract) — linked instances read their definition, never a stale +/// materialized `record.model`/`record.provider`, so discovery cannot query a +/// provider this agent will not actually launch with. Definition-less +/// instances keep their own record values, matching spawn's +/// `resolve_definition_less` arm. When the resolver yields no provider, +/// `effective_discovery_provider` recovers the provider the agent will +/// actually launch with from the runtime's own provider env var, read out of +/// the descriptor env (which already layers definition/persona/global values +/// the same way spawn does). +/// +/// Returns `Err("DANGLING_HARNESS_ID:")` from the descriptor resolver when +/// the harness id no longer exists; the caller routes it through +/// `model_discovery_error`. +pub(super) fn agent_model_discovery_config( + record: &crate::managed_agents::ManagedAgentRecord, + personas: &[crate::managed_agents::AgentDefinition], + global: &crate::managed_agents::GlobalAgentConfig, +) -> Result { + let descriptor = + crate::managed_agents::resolve_effective_harness_descriptor(record, personas, global)?; + let (model, provider) = + crate::managed_agents::resolve_effective_model_provider(record, personas, global); + let provider_env_var = + known_acp_runtime(&descriptor.command).and_then(|meta| meta.provider_env_var); + + Ok(AgentModelDiscoveryConfig { + command: descriptor.command, + args: descriptor.args, + model, + provider, + provider_env_var, + env: descriptor.env, + }) +} + +/// Derive the discovery env for an unsaved ("draft") agent configuration. +/// +/// Mirrors the layering `agent_model_discovery_config` takes from the harness +/// descriptor, but sources the provider from form input: runtime-derived +/// provider env var → definition env → user env vars, so user overrides always +/// win. Extracted so the draft path has the same tested seam as the saved one. +pub(super) fn draft_agent_model_discovery_env( + agent_command: &str, + provider: Option<&str>, + definition_env: &BTreeMap, + env_vars: &BTreeMap, +) -> BTreeMap { + let mut derived_env = BTreeMap::new(); + if let Some(meta) = known_acp_runtime(agent_command) { + let provider = provider.map(str::trim).filter(|value| !value.is_empty()); + if !meta.provider_locked { + if let (Some(env_key), Some(provider)) = (meta.provider_env_var, provider) { + derived_env.insert(env_key.to_string(), provider.to_string()); + } + } + } + // Reserved keys are stripped from definition env, matching the same filter + // applied at spawn. + let mut filtered_definition_env = BTreeMap::new(); + for (key, value) in definition_env { + if !crate::managed_agents::is_reserved_env_key(key) { + filtered_definition_env.insert(key.clone(), value.clone()); + } + } + let merged_with_def = + crate::managed_agents::merged_user_env(&derived_env, &filtered_definition_env); + crate::managed_agents::merged_user_env(&merged_with_def, env_vars) +} diff --git a/desktop/src-tauri/src/commands/agent_models_openrouter.rs b/desktop/src-tauri/src/commands/agent_models_openrouter.rs new file mode 100644 index 0000000000..be6dd2cf26 --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_openrouter.rs @@ -0,0 +1,112 @@ +use std::collections::BTreeMap; + +use serde::Deserialize; + +use crate::managed_agents::{AgentModelInfo, AgentModelsResponse}; + +#[cfg(test)] +use super::env_value; +use super::{env_or_process_value, redaction_env_with_value, DiscoveryProvider}; + +#[derive(Debug, Deserialize)] +#[cfg_attr(test, derive(Clone))] +pub(super) struct OpenRouterModelListResponse { + pub data: Vec, +} + +#[derive(Debug, Deserialize)] +#[cfg_attr(test, derive(Clone))] +pub(super) struct OpenRouterModelListItem { + pub id: String, + #[serde(default)] + pub supported_parameters: Vec, +} + +pub(super) fn is_openrouter_provider(provider: Option<&str>) -> bool { + matches!( + provider + .map(str::trim) + .map(str::to_ascii_lowercase) + .as_deref(), + Some("openrouter") + ) +} + +#[cfg(test)] +pub(super) fn openrouter_models_url(env: &BTreeMap) -> String { + let base_url = env_value(env, "OPENROUTER_BASE_URL") + .unwrap_or_else(|| "https://openrouter.ai/api/v1".to_string()); + format!("{}/models", base_url.trim_end_matches('/')) +} + +fn openrouter_models_url_for_discovery(env: &BTreeMap) -> String { + let base_url = env_or_process_value(env, "OPENROUTER_BASE_URL") + .unwrap_or_else(|| "https://openrouter.ai/api/v1".to_string()); + format!("{}/models", base_url.trim_end_matches('/')) +} + +pub(super) async fn discover_openrouter_models( + client: &reqwest::Client, + provider: &DiscoveryProvider, + env: &BTreeMap, + selected_model: Option, +) -> Result, String> { + if !is_openrouter_provider(provider.as_deref()) { + return Ok(None); + } + + let api_key = match provider.required_env(env, "OPENROUTER_API_KEY")? { + Some(api_key) => api_key, + None => return Ok(None), + }; + let redaction_env = redaction_env_with_value(env, "OPENROUTER_API_KEY", &api_key); + let url = openrouter_models_url_for_discovery(env); + let response = client + .get(&url) + .bearer_auth(&api_key) + .send() + .await + .map_err(|error| format!("OpenRouter model discovery request failed: {error}"))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + let body = crate::managed_agents::redact_env_values_in(&body, &redaction_env); + return Err(format!("OpenRouter model discovery HTTP {status}: {body}")); + } + + let response = response + .json::() + .await + .map_err(|error| format!("OpenRouter model discovery response parse failed: {error}"))?; + + filter_openrouter_models(response, selected_model) +} + +pub(super) fn filter_openrouter_models( + response: OpenRouterModelListResponse, + selected_model: Option, +) -> Result, String> { + let models: Vec = response + .data + .into_iter() + .filter(|m| m.supported_parameters.iter().any(|p| p == "tools")) + .map(|m| AgentModelInfo { + id: m.id.clone(), + name: Some(m.id), + description: None, + }) + .collect(); + + if models.is_empty() { + return Err("OpenRouter model discovery returned no tools-capable models".to_string()); + } + + Ok(Some(AgentModelsResponse { + agent_name: "openrouter".to_string(), + agent_version: "models-api".to_string(), + models, + agent_default_model: None, + selected_model, + supports_switching: true, + })) +} diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index b65f240900..14c981d730 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -590,3 +590,294 @@ fn model_discovery_error_converts_dangling_sentinel_to_sentence() { let plain = model_discovery_error("agent-pk", "plain failure"); assert_eq!(plain, "cannot discover models for agent-pk: plain failure"); } + +// --------------------------------------------------------------------------- +// OpenRouter provider +// --------------------------------------------------------------------------- + +#[test] +fn is_openrouter_provider_matches() { + assert!(is_openrouter_provider(Some("openrouter"))); + assert!(is_openrouter_provider(Some(" OpenRouter "))); + assert!(!is_openrouter_provider(Some("openai"))); + assert!(!is_openrouter_provider(Some("anthropic"))); + assert!(!is_openrouter_provider(None)); +} + +#[test] +fn openrouter_models_url_uses_default_base_url() { + assert_eq!( + openrouter_models_url(&BTreeMap::new()), + "https://openrouter.ai/api/v1/models" + ); +} + +#[test] +fn openrouter_models_url_respects_custom_base_url() { + let env = BTreeMap::from([( + "OPENROUTER_BASE_URL".to_string(), + "https://eu.openrouter.ai/api/v1".to_string(), + )]); + assert_eq!( + openrouter_models_url(&env), + "https://eu.openrouter.ai/api/v1/models" + ); +} + +#[test] +fn openrouter_models_url_strips_trailing_slash() { + let env = BTreeMap::from([( + "OPENROUTER_BASE_URL".to_string(), + "https://proxy.example.com/api/v1/".to_string(), + )]); + assert_eq!( + openrouter_models_url(&env), + "https://proxy.example.com/api/v1/models" + ); +} + +#[test] +fn openrouter_filter_keeps_tools_capable_models() { + let response = OpenRouterModelListResponse { + data: vec![ + OpenRouterModelListItem { + id: "anthropic/claude-opus-4-7".to_string(), + supported_parameters: vec!["tools".to_string(), "reasoning".to_string()], + }, + OpenRouterModelListItem { + id: "openai/gpt-5.5-pro".to_string(), + supported_parameters: vec!["tools".to_string()], + }, + OpenRouterModelListItem { + id: "meta-llama/llama-no-tools".to_string(), + supported_parameters: vec!["temperature".to_string()], + }, + ], + }; + let result = filter_openrouter_models(response, None).unwrap().unwrap(); + let ids: Vec<_> = result.models.iter().map(|m| m.id.as_str()).collect(); + assert_eq!(ids, vec!["anthropic/claude-opus-4-7", "openai/gpt-5.5-pro"]); +} + +#[test] +fn openrouter_filter_excludes_absent_supported_parameters() { + let response: OpenRouterModelListResponse = + serde_json::from_str(r#"{"data": [{"id": "model-no-params"}]}"#).unwrap(); + assert!( + response.data[0].supported_parameters.is_empty(), + "absent supported_parameters must default to empty vec" + ); + let result = filter_openrouter_models(response, None); + assert!( + result.is_err(), + "models with no supported_parameters must be excluded" + ); + assert!( + result.unwrap_err().contains("no tools-capable models"), + "error must indicate no tools-capable models" + ); +} + +#[test] +fn openrouter_filter_excludes_empty_supported_parameters() { + let response = OpenRouterModelListResponse { + data: vec![OpenRouterModelListItem { + id: "model-empty-params".to_string(), + supported_parameters: Vec::new(), + }], + }; + let result = filter_openrouter_models(response, None); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("no tools-capable models")); +} + +#[test] +fn openrouter_filter_empty_result_returns_error() { + let response = OpenRouterModelListResponse { data: Vec::new() }; + let result = filter_openrouter_models(response, None); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("no tools-capable models")); +} + +#[test] +fn openrouter_filter_preserves_selected_model() { + let response = OpenRouterModelListResponse { + data: vec![OpenRouterModelListItem { + id: "openai/gpt-5.5-pro".to_string(), + supported_parameters: vec!["tools".to_string()], + }], + }; + let result = filter_openrouter_models(response, Some("openai/gpt-5.5-pro".to_string())) + .unwrap() + .unwrap(); + assert_eq!(result.selected_model.as_deref(), Some("openai/gpt-5.5-pro")); +} + +#[test] +fn openrouter_credential_redaction_env_records_key() { + let env = BTreeMap::from([( + "OPENROUTER_API_KEY".to_string(), + "sk-or-v1-secret-key-12345".to_string(), + )]); + let redaction = + redaction_env_with_value(&env, "OPENROUTER_API_KEY", "sk-or-v1-secret-key-12345"); + assert_eq!( + redaction.get("OPENROUTER_API_KEY").map(String::as_str), + Some("sk-or-v1-secret-key-12345"), + "redaction env must record the API key for error body redaction" + ); +} + +#[test] +fn openrouter_saved_agent_model_discovery_resolves_provider() { + let record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "abcd1234", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "buzz-agent", + "agent_command_override": "buzz-agent", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "model": "anthropic/claude-sonnet-4", + "provider": "openrouter", + "env_vars": { + "OPENROUTER_API_KEY": "sk-or-test-key", + "BUZZ_PRIVATE_KEY": "must-not-leak" + }, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("sample openrouter managed agent record"); + + let discovery = agent_model_discovery_config( + &record, + &[], + &crate::managed_agents::GlobalAgentConfig::default(), + ) + .expect("discovery config should resolve for an openrouter record"); + assert_eq!(discovery.provider.as_deref(), Some("openrouter")); + assert_eq!( + discovery.model.as_deref(), + Some("anthropic/claude-sonnet-4") + ); + assert_eq!( + discovery.env.get("OPENROUTER_API_KEY").map(String::as_str), + Some("sk-or-test-key") + ); + assert!(!discovery.env.contains_key("BUZZ_PRIVATE_KEY")); +} + +/// B5/T4: unsaved-agent ("draft") discovery mirrors the saved-agent path — +/// `draft_agent_model_discovery_env` must derive the provider env var from +/// form input the same way `agent_model_discovery_config` derives it from a +/// persisted record's harness descriptor, and preserve caller-supplied env +/// (including the OpenRouter API key) unmodified. +#[test] +fn openrouter_draft_agent_model_discovery_derives_provider_env() { + let env_vars = BTreeMap::from([( + "OPENROUTER_API_KEY".to_string(), + "sk-or-draft-key".to_string(), + )]); + + let merged = draft_agent_model_discovery_env( + "buzz-agent", + Some("openrouter"), + &BTreeMap::new(), + &env_vars, + ); + + assert_eq!( + merged.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("openrouter"), + "provider env var must be derived from form input for a known ACP runtime" + ); + assert_eq!( + merged.get("OPENROUTER_API_KEY").map(String::as_str), + Some("sk-or-draft-key"), + "caller-supplied env vars must survive the merge" + ); +} + +#[test] +fn draft_agent_model_discovery_env_omits_provider_when_absent() { + let merged = + draft_agent_model_discovery_env("buzz-agent", None, &BTreeMap::new(), &BTreeMap::new()); + assert!( + !merged.contains_key("BUZZ_AGENT_PROVIDER"), + "no provider must be derived when the caller supplies none" + ); +} + +/// The three-tier precedence this merge exists to preserve: main's inline +/// `derived → definition_env → env_vars` layering was folded into +/// `draft_agent_model_discovery_env`, so pin the order at every collision +/// boundary rather than trusting the two single-tier tests above. +/// +/// `SHARED` collides across all three tiers, so the user value proves the +/// full chain; the pairwise keys prove each adjacent boundary independently +/// (a merge that dropped only the middle tier would still satisfy `SHARED`). +/// `BUZZ_PRIVATE_KEY` proves a reserved key cannot ride in on a harness +/// definition, which is the tier a user never types. +#[test] +fn draft_agent_model_discovery_env_layers_all_three_tiers_in_order() { + // Tier 2 (middle): harness definition env — overlays the runtime-derived + // floor, loses to user env. + let definition_env = BTreeMap::from([ + ("SHARED".to_string(), "from-definition".to_string()), + // Collides with tier 1: `buzz-agent`'s own provider env var, which the + // `provider` argument derives below. + ("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()), + ("USER_OVER_DEF".to_string(), "from-definition".to_string()), + ("DEFINITION_ONLY".to_string(), "from-definition".to_string()), + // Reserved: must never reach the child, even from a definition. + ("BUZZ_PRIVATE_KEY".to_string(), "must-not-leak".to_string()), + ]); + // Tier 3 (top): user-entered env — wins over everything. + let env_vars = BTreeMap::from([ + ("SHARED".to_string(), "from-user".to_string()), + ("USER_OVER_DEF".to_string(), "from-user".to_string()), + ("USER_ONLY".to_string(), "from-user".to_string()), + ]); + + // Tier 1 (floor): `Some("openrouter")` derives BUZZ_AGENT_PROVIDER. + let merged = draft_agent_model_discovery_env( + "buzz-agent", + Some("openrouter"), + &definition_env, + &env_vars, + ); + + let expected: &[(&str, Option<&str>)] = &[ + // Collides in all three tiers — the top tier wins. + ("SHARED", Some("from-user")), + // Tier 2 over tier 1: the definition's value survives, proving the + // derived provider is the floor and not layered on top. + ("BUZZ_AGENT_PROVIDER", Some("openai")), + // Tier 3 over tier 2. + ("USER_OVER_DEF", Some("from-user")), + // Single-tier keys pass through untouched. + ("DEFINITION_ONLY", Some("from-definition")), + ("USER_ONLY", Some("from-user")), + // Reserved keys never survive the definition tier. Doubly enforced — + // the explicit `is_reserved_env_key` filter here and `merged_user_env`'s + // own `retain` — so this pins the contract, not either mechanism. + ("BUZZ_PRIVATE_KEY", None), + ]; + for (key, want) in expected { + assert_eq!( + merged.get(*key).map(String::as_str), + *want, + "env key `{key}` must resolve to {want:?} after three-tier layering" + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c053d933c5..fa8eb36fa1 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -481,6 +481,7 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { } Some("anthropic") => Some("ANTHROPIC_MODEL"), Some("openai") | Some("openai-compat") => Some("OPENAI_COMPAT_MODEL"), + Some("openrouter") => Some("OPENROUTER_MODEL"), _ => None, }; let model_present = effective @@ -523,6 +524,12 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { key: "DATABRICKS_HOST".to_string(), }); } + Some("openrouter") + if env_key_missing("OPENROUTER_API_KEY") => { + missing.push(Requirement::EnvKey { + key: "OPENROUTER_API_KEY".to_string(), + }); + } _ => { // Unknown provider or no provider yet — only the NormalizedField // requirement above captures this gap. @@ -630,6 +637,13 @@ fn goose_requirements( key: "DATABRICKS_HOST".to_string(), }); } + Some("openrouter") + if env_key_missing("OPENROUTER_API_KEY") && !file_key_present("OPENROUTER_API_KEY") => + { + missing.push(Requirement::EnvKey { + key: "OPENROUTER_API_KEY".to_string(), + }); + } _ => {} } @@ -1668,195 +1682,62 @@ mod tests { field: "model".to_string() })); } -} - -// ── goose file-config–aware requirement tests ───────────────────────────── -// -// These tests call `goose_requirements` directly, injecting a synthetic -// `RuntimeFileConfig` so there is no disk I/O and tests are deterministic. - -#[cfg(test)] -mod goose_file_config_tests { - use std::collections::BTreeMap; - - use super::*; - use crate::managed_agents::config_bridge::RuntimeFileConfig; - - fn empty_env() -> EffectiveAgentEnv { - EffectiveAgentEnv { - env: BTreeMap::new(), - config_file_path: Some("~/.config/goose/config.yaml"), - effective_command: "goose".to_string(), - } - } - fn env_with(pairs: &[(&str, &str)]) -> EffectiveAgentEnv { - EffectiveAgentEnv { - env: pairs - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect(), - config_file_path: Some("~/.config/goose/config.yaml"), - effective_command: "goose".to_string(), - } - } - - fn databricks_file_config() -> RuntimeFileConfig { - let mut extra = BTreeMap::new(); - extra.insert( - "DATABRICKS_HOST".to_string(), - "https://dbc.example.com".to_string(), - ); - RuntimeFileConfig { - provider: Some("databricks_v2".to_string()), - model: Some("goose-claude-4-6-opus".to_string()), - extra, - ..Default::default() - } - } + // ── OpenRouter readiness ───────────────────────────────────────────── #[test] - fn goose_file_config_silences_databricks_host_requirement() { - // File has provider, model, and DATABRICKS_HOST — all requirements silenced. - let env = empty_env(); - let cfg = databricks_file_config(); - let result = goose_requirements(&env, Some(&cfg)); - assert!( - result.is_empty(), - "all requirements should be silenced by goose file config; \ - got: {:?}", - result - ); - } - - #[test] - fn goose_env_empty_file_absent_still_not_ready() { - // No env, no file config → provider and model both required. - let env = empty_env(); - let result = goose_requirements(&env, None); - assert!( - result.contains(&Requirement::NormalizedField { - field: "provider".to_string() - }), - "provider must be required when absent from both env and file" - ); - assert!( - result.contains(&Requirement::NormalizedField { - field: "model".to_string() - }), - "model must be required when absent from both env and file" - ); - } - - #[test] - fn goose_file_config_silences_provider_and_model_but_not_anthropic_key() { - // File has provider=anthropic and model, but ANTHROPIC_API_KEY is not - // in the file's `extra` map — it must still be required. - let cfg = RuntimeFileConfig { - provider: Some("anthropic".to_string()), - model: Some("claude-opus-4-5".to_string()), - extra: BTreeMap::new(), - ..Default::default() - }; - let env = empty_env(); - let result = goose_requirements(&env, Some(&cfg)); - // Provider and model silenced. - assert!( - !result.contains(&Requirement::NormalizedField { - field: "provider".to_string() - }), - "provider silenced by file config" - ); - assert!( - !result.contains(&Requirement::NormalizedField { - field: "model".to_string() - }), - "model silenced by file config" + fn buzz_agent_openrouter_with_all_fields_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openrouter"), + ("BUZZ_AGENT_MODEL", "anthropic/claude-sonnet-4"), + ("OPENROUTER_API_KEY", "sk-or-test-key"), + ]), ); - // ANTHROPIC_API_KEY not in file extra → still required. + let result = agent_readiness(&env); assert!( - result.contains(&Requirement::EnvKey { - key: "ANTHROPIC_API_KEY".to_string() - }), - "ANTHROPIC_API_KEY must remain required when not in file extra" + result.is_ready(), + "openrouter with all fields should be ready" ); } #[test] - fn goose_env_provider_wins_over_file_provider_for_cred_check() { - // Env has GOOSE_PROVIDER=anthropic (different from file's databricks_v2). - // The env provider must win for credential checking. - let env = env_with(&[ - ("GOOSE_PROVIDER", "anthropic"), - ("GOOSE_MODEL", "claude-opus-4-5"), - ]); - let cfg = databricks_file_config(); // has provider=databricks_v2 - let result = goose_requirements(&env, Some(&cfg)); - // anthropic requires ANTHROPIC_API_KEY, not DATABRICKS_HOST. - assert!( - result.contains(&Requirement::EnvKey { - key: "ANTHROPIC_API_KEY".to_string() - }), - "env provider=anthropic must require ANTHROPIC_API_KEY" - ); - assert!( - !result.contains(&Requirement::EnvKey { - key: "DATABRICKS_HOST".to_string() - }), - "env provider=anthropic must NOT require DATABRICKS_HOST" + fn buzz_agent_openrouter_missing_key_returns_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openrouter"), + ("BUZZ_AGENT_MODEL", "anthropic/claude-sonnet-4"), + ]), ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "OPENROUTER_API_KEY".to_string() + })); } #[test] - fn goose_flat_databricks_host_in_file_config_silences_requirement() { - // Will's typical goose config: flat DATABRICKS_HOST at the top level, - // no active_provider — provider inferred as "databricks". - // The parser must store extra["DATABRICKS_HOST"] = value (canonical key), - // and goose_requirements must then silence the DATABRICKS_HOST requirement. - let mut extra = BTreeMap::new(); - extra.insert( - "DATABRICKS_HOST".to_string(), - "https://block.cloud.databricks.com".to_string(), - ); - let cfg = RuntimeFileConfig { - provider: Some("databricks".to_string()), - model: Some("goose-claude-4-5".to_string()), - extra, - ..Default::default() - }; - let env = empty_env(); - let result = goose_requirements(&env, Some(&cfg)); - // All requirements silenced — provider (file), model (file), DATABRICKS_HOST (file). - assert!( - result.is_empty(), - "flat DATABRICKS_HOST in file config must silence all requirements; \ - got: {:?}", - result + fn buzz_agent_openrouter_with_provider_model_fallback_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openrouter"), + ("OPENROUTER_MODEL", "google/gemini-2.5-flash"), + ("OPENROUTER_API_KEY", "sk-or-test-key"), + ]), ); - } - - #[test] - fn goose_goose_provider_databricks_flat_host_silences_databricks_host() { - // GOOSE_PROVIDER=databricks (not active_provider) + flat DATABRICKS_HOST. - // The parser canonicalizes to extra["DATABRICKS_HOST"]; readiness must silence it. - let mut extra = BTreeMap::new(); - extra.insert( - "DATABRICKS_HOST".to_string(), - "https://dbc.example.com".to_string(), - ); - let cfg = RuntimeFileConfig { - provider: Some("databricks".to_string()), - model: Some("some-model".to_string()), - extra, - ..Default::default() - }; - let env = empty_env(); - let result = goose_requirements(&env, Some(&cfg)); + let result = agent_readiness(&env); assert!( - !result.contains(&Requirement::EnvKey { - key: "DATABRICKS_HOST".to_string() - }), - "DATABRICKS_HOST must be silenced when canonical key is in file extra" + result.is_ready(), + "OPENROUTER_MODEL fallback should satisfy model requirement" ); } } + +// Goose file-config-aware requirement tests live in a sibling file so this +// module stays under the desktop file-size ratchet. +#[cfg(test)] +#[path = "readiness_goose_file_config_tests.rs"] +mod goose_file_config_tests; diff --git a/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs new file mode 100644 index 0000000000..46d0e4c7a7 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs @@ -0,0 +1,190 @@ +//! Goose file-config-aware requirement tests. +//! +//! These tests call `goose_requirements` directly, injecting a synthetic +//! `RuntimeFileConfig` so there is no disk I/O and tests are deterministic. +//! +//! Included from `readiness.rs` via `#[path]`; `super::*` therefore resolves +//! against that module, matching the `storage_tests.rs` convention. + +use std::collections::BTreeMap; + +use super::*; +use crate::managed_agents::config_bridge::RuntimeFileConfig; + +fn empty_env() -> EffectiveAgentEnv { + EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: Some("~/.config/goose/config.yaml"), + effective_command: "goose".to_string(), + } +} + +fn env_with(pairs: &[(&str, &str)]) -> EffectiveAgentEnv { + EffectiveAgentEnv { + env: pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + config_file_path: Some("~/.config/goose/config.yaml"), + effective_command: "goose".to_string(), + } +} + +fn databricks_file_config() -> RuntimeFileConfig { + let mut extra = BTreeMap::new(); + extra.insert( + "DATABRICKS_HOST".to_string(), + "https://dbc.example.com".to_string(), + ); + RuntimeFileConfig { + provider: Some("databricks_v2".to_string()), + model: Some("goose-claude-4-6-opus".to_string()), + extra, + ..Default::default() + } +} + +#[test] +fn goose_file_config_silences_databricks_host_requirement() { + // File has provider, model, and DATABRICKS_HOST — all requirements silenced. + let env = empty_env(); + let cfg = databricks_file_config(); + let result = goose_requirements(&env, Some(&cfg)); + assert!( + result.is_empty(), + "all requirements should be silenced by goose file config; \ + got: {:?}", + result + ); +} + +#[test] +fn goose_env_empty_file_absent_still_not_ready() { + // No env, no file config → provider and model both required. + let env = empty_env(); + let result = goose_requirements(&env, None); + assert!( + result.contains(&Requirement::NormalizedField { + field: "provider".to_string() + }), + "provider must be required when absent from both env and file" + ); + assert!( + result.contains(&Requirement::NormalizedField { + field: "model".to_string() + }), + "model must be required when absent from both env and file" + ); +} + +#[test] +fn goose_file_config_silences_provider_and_model_but_not_anthropic_key() { + // File has provider=anthropic and model, but ANTHROPIC_API_KEY is not + // in the file's `extra` map — it must still be required. + let cfg = RuntimeFileConfig { + provider: Some("anthropic".to_string()), + model: Some("claude-opus-4-5".to_string()), + extra: BTreeMap::new(), + ..Default::default() + }; + let env = empty_env(); + let result = goose_requirements(&env, Some(&cfg)); + // Provider and model silenced. + assert!( + !result.contains(&Requirement::NormalizedField { + field: "provider".to_string() + }), + "provider silenced by file config" + ); + assert!( + !result.contains(&Requirement::NormalizedField { + field: "model".to_string() + }), + "model silenced by file config" + ); + // ANTHROPIC_API_KEY not in file extra → still required. + assert!( + result.contains(&Requirement::EnvKey { + key: "ANTHROPIC_API_KEY".to_string() + }), + "ANTHROPIC_API_KEY must remain required when not in file extra" + ); +} + +#[test] +fn goose_env_provider_wins_over_file_provider_for_cred_check() { + // Env has GOOSE_PROVIDER=anthropic (different from file's databricks_v2). + // The env provider must win for credential checking. + let env = env_with(&[ + ("GOOSE_PROVIDER", "anthropic"), + ("GOOSE_MODEL", "claude-opus-4-5"), + ]); + let cfg = databricks_file_config(); // has provider=databricks_v2 + let result = goose_requirements(&env, Some(&cfg)); + // anthropic requires ANTHROPIC_API_KEY, not DATABRICKS_HOST. + assert!( + result.contains(&Requirement::EnvKey { + key: "ANTHROPIC_API_KEY".to_string() + }), + "env provider=anthropic must require ANTHROPIC_API_KEY" + ); + assert!( + !result.contains(&Requirement::EnvKey { + key: "DATABRICKS_HOST".to_string() + }), + "env provider=anthropic must NOT require DATABRICKS_HOST" + ); +} + +#[test] +fn goose_flat_databricks_host_in_file_config_silences_requirement() { + // Will's typical goose config: flat DATABRICKS_HOST at the top level, + // no active_provider — provider inferred as "databricks". + // The parser must store extra["DATABRICKS_HOST"] = value (canonical key), + // and goose_requirements must then silence the DATABRICKS_HOST requirement. + let mut extra = BTreeMap::new(); + extra.insert( + "DATABRICKS_HOST".to_string(), + "https://block.cloud.databricks.com".to_string(), + ); + let cfg = RuntimeFileConfig { + provider: Some("databricks".to_string()), + model: Some("goose-claude-4-5".to_string()), + extra, + ..Default::default() + }; + let env = empty_env(); + let result = goose_requirements(&env, Some(&cfg)); + // All requirements silenced — provider (file), model (file), DATABRICKS_HOST (file). + assert!( + result.is_empty(), + "flat DATABRICKS_HOST in file config must silence all requirements; \ + got: {:?}", + result + ); +} + +#[test] +fn goose_goose_provider_databricks_flat_host_silences_databricks_host() { + // GOOSE_PROVIDER=databricks (not active_provider) + flat DATABRICKS_HOST. + // The parser canonicalizes to extra["DATABRICKS_HOST"]; readiness must silence it. + let mut extra = BTreeMap::new(); + extra.insert( + "DATABRICKS_HOST".to_string(), + "https://dbc.example.com".to_string(), + ); + let cfg = RuntimeFileConfig { + provider: Some("databricks".to_string()), + model: Some("some-model".to_string()), + extra, + ..Default::default() + }; + let env = empty_env(); + let result = goose_requirements(&env, Some(&cfg)); + assert!( + !result.contains(&Requirement::EnvKey { + key: "DATABRICKS_HOST".to_string() + }), + "DATABRICKS_HOST must be silenced when canonical key is in file extra" + ); +} diff --git a/desktop/src/features/agents/ui/agentConfigOptions.tsx b/desktop/src/features/agents/ui/agentConfigOptions.tsx index 1313d2cec4..d51c970f29 100644 --- a/desktop/src/features/agents/ui/agentConfigOptions.tsx +++ b/desktop/src/features/agents/ui/agentConfigOptions.tsx @@ -42,6 +42,7 @@ const KNOWN_LLM_PROVIDER_IDS = [ "databricks_v2", "openai", "openai-compat", + "openrouter", ] as const; type PersonaLlmProviderId = (typeof KNOWN_LLM_PROVIDER_IDS)[number]; @@ -109,6 +110,10 @@ const PROVIDER_CREDENTIAL_CONFIG: Partial< "databricks-v2": { requiredEnvKeys: ["DATABRICKS_HOST"], }, + openrouter: { + requiredEnvKeys: ["OPENROUTER_API_KEY"], + secretEnvVar: "OPENROUTER_API_KEY", + }, }; const DEFAULT_MODEL_OPTION: PersonaModelOption = { @@ -120,6 +125,7 @@ export const PERSONA_LLM_PROVIDER_OPTIONS: readonly PersonaModelOption[] = [ { id: "anthropic", label: "Anthropic" }, { id: "openai", label: "OpenAI" }, { id: "openai-compat", label: "OpenAI-compatible" }, + { id: "openrouter", label: "OpenRouter" }, { id: "relay-mesh", label: "Buzz shared compute" }, { id: "databricks", label: "Databricks" }, { id: "databricks_v2", label: "Databricks v2" }, @@ -279,7 +285,8 @@ export function providerRequiresExplicitModel( return ( trimmedProvider === "anthropic" || trimmedProvider === "openai" || - trimmedProvider === "openai-compat" + trimmedProvider === "openai-compat" || + trimmedProvider === "openrouter" ); } diff --git a/desktop/src/features/agents/ui/buzzAgentConfig.ts b/desktop/src/features/agents/ui/buzzAgentConfig.ts index a0271fec0f..be663c35cb 100644 --- a/desktop/src/features/agents/ui/buzzAgentConfig.ts +++ b/desktop/src/features/agents/ui/buzzAgentConfig.ts @@ -128,6 +128,9 @@ export function getProviderEffortConfig( // databricks v1 uses OpenAI Chat Completions wire format. return openaiConfig(m); } + if (provider === "openrouter") { + return { validValues: ALL_VALUES, defaultValue: "medium" }; + } // openai-compat, unknown, empty — all values, default medium. return { validValues: ALL_VALUES, defaultValue: "medium" }; } diff --git a/desktop/src/features/agents/ui/effortTable.fixture.json b/desktop/src/features/agents/ui/effortTable.fixture.json index defb1f86de..d097bc995f 100644 --- a/desktop/src/features/agents/ui/effortTable.fixture.json +++ b/desktop/src/features/agents/ui/effortTable.fixture.json @@ -209,6 +209,13 @@ "validValues": ["none", "minimal", "low", "medium", "high", "xhigh", "max"], "defaultValue": "medium" }, + { + "note": "openrouter: all-7 with medium default", + "provider": "openrouter", + "model": "", + "validValues": ["none", "minimal", "low", "medium", "high", "xhigh", "max"], + "defaultValue": "medium" + }, { "note": "empty provider: all-7 with medium default", "provider": "",