diff --git a/.claude/scheduled_tasks.lock b/.claude/scheduled_tasks.lock new file mode 100644 index 0000000000..f7efc643e6 --- /dev/null +++ b/.claude/scheduled_tasks.lock @@ -0,0 +1 @@ +{"sessionId":"06f65c49-467c-454b-9983-b2949cc6bd25","pid":65452,"procStart":"639213051333836130","acquiredAt":1785726080367} \ No newline at end of file diff --git a/.gitignore b/.gitignore index f26e74136c..69a484bacf 100644 --- a/.gitignore +++ b/.gitignore @@ -72,3 +72,6 @@ identity.key # Helm dependency tarballs — regenerable from Chart.lock via `helm dependency build` deploy/charts/*/charts/*.tgz + +# Claude Code context +.claude_context_tree diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 700d5e8dcf..dbbfb3da69 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -2654,6 +2654,50 @@ mod tests { assert!(super::extract_model_config_options(&result).is_empty()); } + /// Regression guard for the OpenRouter model-catalog seam. + /// + /// `switch_model` (kind:24200) resolves the requested model against the + /// `availableModels` a provider advertises in `session/new`. buzz-agent used to + /// advertise a single-entry catalog for every non-Databricks provider, so an + /// OpenRouter switch could never resolve. The payload below is the real shape + /// buzz-agent now returns for `BUZZ_AGENT_PROVIDER=openrouter` (trimmed): an + /// account-eligible, tools-capable slate. + #[test] + fn openrouter_catalog_resolves_a_switch_and_rejects_an_ineligible_model() { + let session_new = serde_json::json!({ + "sessionId": "sess-openrouter", + "models": { + "currentModelId": "openai/gpt-5.6-luna", + "availableModels": [ + { "modelId": "openai/gpt-5.6-luna", "name": "OpenAI: GPT-5.6 Luna" }, + { "modelId": "openai/gpt-5.6-luna-pro", "name": "OpenAI: GPT-5.6 Luna Pro" }, + { "modelId": "z-ai/glm-5.2", "name": "Z.ai: GLM 5.2" }, + { "modelId": "deepseek/deepseek-v4-flash-0731", "name": "DeepSeek: DeepSeek V4 Flash 0731" }, + ] + } + }); + + // A model in the advertised catalog resolves to a live set_model switch. + let method = super::resolve_model_switch_method(&session_new, "z-ai/glm-5.2") + .expect("a catalog model must resolve"); + match method { + super::ModelSwitchMethod::SetModel { model_id } => { + assert_eq!(model_id, "z-ai/glm-5.2"); + } + other => panic!("expected SetModel, got {other:?}"), + } + + // A model absent from the catalog must NOT resolve. gpt-5.6-terra is the + // real case: it exists in OpenRouter's global catalog but is not on this + // account's eligibility allowlist, so requesting it returns HTTP 404. + // Refusing it here turns that into an up-front unsupported_model rather + // than a confusing mid-request failure. + assert!( + super::resolve_model_switch_method(&session_new, "openai/gpt-5.6-terra").is_none(), + "a model outside the advertised catalog must not resolve" + ); + } + #[test] fn extract_model_state_returns_models_object() { let result = serde_json::json!({ diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 811253e4ac..7f65fe8b53 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -9,6 +9,7 @@ mod pool; mod pool_lifecycle; mod queue; mod relay; +mod routing; mod setup_mode; mod usage; diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 64edf68ee2..9d6b909723 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -1365,6 +1365,34 @@ pub async fn run_prompt_task( Some(b) => PromptSource::Channel(b.channel_id), None => PromptSource::Heartbeat, }; + + // Per-turn model routing (opt-in via BUZZ_ROUTING_POLICY; see routing.rs). + // + // Deliberately does NOT override a live `switch_model`: if the operator (or + // the desktop ModelPicker) explicitly pinned a model for this agent, + // `model_overridden` is set and a router silently changing it would make the + // UI lie about what is running. Explicit human choice outranks the policy. + // + // The decision is expressed as `desired_model`, which the existing + // session-creation path validates against the agent's advertised catalog and + // applies — so a policy naming a model the provider does not offer degrades + // to the agent default with a warning rather than failing the turn. + if !agent.model_overridden { + if let Some(policy) = crate::routing::Policy::from_env() { + let routed_text = prompt_text.as_deref().unwrap_or_default(); + if let Some(decision) = policy.decide(routed_text).await { + if agent.desired_model.as_deref() != Some(decision.model.as_str()) { + tracing::info!( + target: "acp::routing", + model = %decision.model, + reason = ?decision.reason, + "routing selected a model for this turn" + ); + agent.desired_model = Some(decision.model); + } + } + } + } let observer_channel_id = match &source { PromptSource::Channel(channel_id) => Some(*channel_id), PromptSource::Heartbeat => None, diff --git a/crates/buzz-acp/src/routing.rs b/crates/buzz-acp/src/routing.rs new file mode 100644 index 0000000000..f427dd1b02 --- /dev/null +++ b/crates/buzz-acp/src/routing.rs @@ -0,0 +1,707 @@ +//! Per-turn model routing. +//! +//! Picks the model for an inbound turn instead of always using the agent's +//! configured default. The decision is applied through the EXISTING +//! [`OwnedAgent::desired_model`](crate::pool::OwnedAgent) mechanism that +//! `switch_model` already uses, so nothing new touches the ACP wire, the relay, +//! or the trust boundary — in particular this needs no owner-signed kind:24200 +//! control frame, because the decision is made in-process by the harness that is +//! already trusted to run the turn. +//! +//! # Opt-in, and fails open +//! +//! Routing is off unless `BUZZ_ROUTING_POLICY` names a readable policy file. A +//! missing file, unparseable JSON, `enabled: false`, no matching rule, or a +//! classifier that errors or times out all resolve to "no opinion" — the turn +//! proceeds on the agent's configured model exactly as before. Routing must never +//! be able to fail a turn; a router that can block work is worse than no router. +//! +//! # Two stages, cheap first +//! +//! 1. `rules` — deterministic substring/regex-free matchers over the prompt text. +//! No network, no latency. Most routing intent is expressible here. +//! 2. `classifier` — an optional local Ollama call, used only when no rule +//! matched. Local by design: this code sees raw channel content, so shipping +//! every turn's text to a hosted classifier to decide where to send it would +//! leak exactly what a routing decision is supposed to protect. +//! +//! # What this does NOT do +//! +//! It selects a MODEL, not a harness. One buzz-acp process serves one agent, so +//! routing a turn to opencode-vs-codex-vs-claude means choosing a different agent +//! — that is a dispatcher concern (there is none today: selection is a `p`-tag +//! mention with relay fan-out) and is deliberately out of scope here. + +use std::path::PathBuf; +use std::time::Duration; + +use serde::Deserialize; + +/// Env var naming the policy file. Absent => routing disabled. +pub const POLICY_ENV: &str = "BUZZ_ROUTING_POLICY"; + +/// How a rule matches the prompt text. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum MatchKind { + /// Any of `any` appears in the prompt, case-insensitively. + Contains, + /// Every one of `any` appears in the prompt, case-insensitively. + ContainsAll, +} + +impl Default for MatchKind { + fn default() -> Self { + Self::Contains + } +} + +/// One deterministic routing rule. +#[derive(Debug, Clone, Deserialize)] +pub struct Rule { + /// Human label, surfaced in the log line explaining a routing decision. + #[serde(default)] + pub name: Option, + #[serde(default)] + pub match_kind: MatchKind, + /// Needles to look for. An empty list never matches — a rule that matches + /// everything must be written as the policy's `default_model` instead, so + /// "always this model" cannot be created by accident. + #[serde(default)] + pub any: Vec, + /// Model id to use when this rule matches. Must be a model the agent's + /// provider actually advertises, or the existing apply step logs a miss and + /// falls back to the agent default. + pub model: String, +} + +/// Shared needle check for both the model [`Rule`] and the [`HarnessRule`], so +/// the two matchers cannot drift. An empty `any` never matches — a rule that +/// matches everything must be expressed as a `default_*`, not by omission. +fn any_contains(kind: MatchKind, any: &[String], haystack_lower: &str) -> bool { + if any.is_empty() { + return false; + } + let hit = |needle: &String| { + let n = needle.trim().to_lowercase(); + !n.is_empty() && haystack_lower.contains(&n) + }; + match kind { + MatchKind::Contains => any.iter().any(hit), + MatchKind::ContainsAll => any.iter().all(hit), + } +} + +impl Rule { + fn matches(&self, haystack_lower: &str) -> bool { + any_contains(self.match_kind, &self.any, haystack_lower) + } +} + +/// Optional local classifier, consulted only when no rule matched. +#[derive(Debug, Clone, Deserialize)] +pub struct Classifier { + /// Ollama base url, e.g. `http://localhost:11434`. + pub url: String, + /// Ollama model id doing the classifying, e.g. `gemma3:27b`. + pub model: String, + /// Map from a classifier label to the model to run the turn on. + #[serde(default)] + pub labels: Vec, + #[serde(default = "default_timeout_ms")] + pub timeout_ms: u64, +} + +fn default_timeout_ms() -> u64 { + 20_000 +} + +#[derive(Debug, Clone, Deserialize)] +pub struct LabelTarget { + pub label: String, + pub model: String, +} + +/// Harness-class routing — an optional sibling of the model-routing fields. +/// +/// Where the model router picks a *model* for one agent's process, this picks a +/// *harness class* (claude / opencode / codex) that should own a turn. It is +/// consumed by an ingress "decline gate": each harness-agent runs the same +/// deterministic decision and simply skips a turn another class owns, since the +/// relay already delivered that turn to every subscribed process. It therefore +/// mutates nothing and emits no wire frame — strictly less privileged than the +/// model router. +/// +/// Deterministic rules ONLY, by design: the decision is distributed across +/// independent processes, so it must be reproducible in each one. A non- +/// deterministic classifier could make two processes disagree and yield zero +/// handlers (a dropped turn), which violates the fail-open contract. Semantic +/// harness routing needs a single decision authority and is out of scope here — +/// hence there is deliberately no `classifier` field. +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct HarnessRouting { + /// This process's own harness class. `None` => the caller supplies the + /// class derived from the agent's command, so one shared harness block is + /// portable across every agent in a group. + #[serde(default)] + pub self_class: Option, + /// Class that owns any turn no rule matched. `None` => never decline on a + /// no-match (the process handles it — fail-open). + #[serde(default)] + pub default_class: Option, + /// Deterministic prompt -> class rules, reusing [`MatchKind`]. + #[serde(default)] + pub rules: Vec, +} + +/// A [`Rule`] whose target is a harness `class` rather than a `model`. +#[derive(Debug, Clone, Deserialize)] +pub struct HarnessRule { + #[serde(default)] + pub name: Option, + #[serde(default)] + pub match_kind: MatchKind, + #[serde(default)] + pub any: Vec, + pub class: String, +} + +impl HarnessRule { + fn matches(&self, haystack_lower: &str) -> bool { + any_contains(self.match_kind, &self.any, haystack_lower) + } +} + +/// Why a harness class was chosen — the analogue of [`Reason`], carried into +/// the log so a decline is never silent. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum HarnessReason { + Rule(String), + Default, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HarnessDecision { + pub class: String, + pub reason: HarnessReason, +} + +/// A routing policy, loaded from `$BUZZ_ROUTING_POLICY`. +#[derive(Debug, Clone, Deserialize)] +pub struct Policy { + /// Off by default so dropping a file in place cannot silently start routing. + #[serde(default)] + pub enabled: bool, + #[serde(default)] + pub rules: Vec, + #[serde(default)] + pub classifier: Option, + /// Model used when nothing matched. `None` => leave the agent's default alone. + #[serde(default)] + pub default_model: Option, + /// Optional harness-class routing. Absent => the harness decline gate is off + /// (fail-open), identical to today's behavior. + #[serde(default)] + pub harness: Option, +} + +/// Why a model was chosen — carried into the log so a routing decision is never +/// silent. An operator debugging "why did this turn use that model" needs the +/// reason, not just the outcome. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Reason { + Rule(String), + Classifier { label: String }, + Default, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Decision { + pub model: String, + pub reason: Reason, +} + +impl Policy { + /// Load from `$BUZZ_ROUTING_POLICY`. Returns `None` when the var is unset, + /// the file is unreadable, or the JSON does not parse — routing is a + /// convenience, so a broken policy degrades to "no routing" rather than + /// preventing the harness from starting. + pub fn from_env() -> Option { + let path = PathBuf::from(std::env::var_os(POLICY_ENV)?); + match std::fs::read_to_string(&path) { + Ok(raw) => match serde_json::from_str::(&raw) { + Ok(policy) => Some(policy), + Err(e) => { + tracing::warn!( + target: "acp::routing", + path = %path.display(), + error = %e, + "routing policy failed to parse — routing disabled for this process" + ); + None + } + }, + Err(e) => { + tracing::warn!( + target: "acp::routing", + path = %path.display(), + error = %e, + "routing policy unreadable — routing disabled for this process" + ); + None + } + } + } + + /// Deterministic stage. Pure: no IO, so it is fully testable and adds no + /// latency to a turn. + pub fn decide_static(&self, prompt: &str) -> Option { + if !self.enabled { + return None; + } + let lower = prompt.to_lowercase(); + for (i, rule) in self.rules.iter().enumerate() { + if rule.matches(&lower) { + return Some(Decision { + model: rule.model.clone(), + reason: Reason::Rule( + rule.name.clone().unwrap_or_else(|| format!("rules[{i}]")), + ), + }); + } + } + None + } + + /// The fallback applied when neither a rule nor the classifier decided. + pub fn fallback(&self) -> Option { + if !self.enabled { + return None; + } + self.default_model.as_ref().map(|m| Decision { + model: m.clone(), + reason: Reason::Default, + }) + } + + /// Which harness class should own this turn. `None` => no opinion + /// (fail-open). Deterministic and IO-free, mirroring [`decide_static`], so + /// every process reaches the same answer and exactly one handles the turn. + /// + /// [`decide_static`]: Policy::decide_static + pub fn decide_harness(&self, prompt: &str) -> Option { + if !self.enabled { + return None; + } + let h = self.harness.as_ref()?; + let lower = prompt.to_lowercase(); + for (i, rule) in h.rules.iter().enumerate() { + if rule.matches(&lower) { + return Some(HarnessDecision { + class: rule.class.clone(), + reason: HarnessReason::Rule( + rule.name + .clone() + .unwrap_or_else(|| format!("harness.rules[{i}]")), + ), + }); + } + } + h.default_class.clone().map(|class| HarnessDecision { + class, + reason: HarnessReason::Default, + }) + } + + /// Should the process whose class is `self_class` DECLINE this turn? + /// + /// Returns `Some(target)` only when a *different* class owns the turn — the + /// caller then skips its local enqueue and a matching-class agent takes it. + /// Returns `None` in every fail-open case: routing disabled, no harness + /// block, no rule matched with no `default_class`, or the target equals this + /// process's class. The harness block's `self_class` overrides the passed + /// `self_class`, so one shared block is portable across a group. + pub fn harness_decline(&self, prompt: &str, self_class: &str) -> Option { + let self_class = self + .harness + .as_ref() + .and_then(|h| h.self_class.as_deref()) + .unwrap_or(self_class); + match self.decide_harness(prompt) { + Some(d) if !d.class.eq_ignore_ascii_case(self_class) => Some(d), + _ => None, + } + } + + /// Full decision for a turn: rules, then classifier, then default. + /// + /// Never returns an error. Any classifier failure is logged and treated as + /// "no opinion", so the turn falls through to `default_model` or the agent's + /// own configured model. + pub async fn decide(&self, prompt: &str) -> Option { + if !self.enabled || prompt.trim().is_empty() { + return None; + } + if let Some(d) = self.decide_static(prompt) { + return Some(d); + } + if let Some(classifier) = self.classifier.as_ref() { + match classify_ollama(classifier, prompt).await { + Ok(Some(label)) => { + if let Some(target) = classifier + .labels + .iter() + .find(|l| l.label.eq_ignore_ascii_case(label.trim())) + { + return Some(Decision { + model: target.model.clone(), + reason: Reason::Classifier { + label: target.label.clone(), + }, + }); + } + tracing::debug!( + target: "acp::routing", + label = %label, + "classifier returned a label with no configured target — using default" + ); + } + Ok(None) => {} + Err(e) => tracing::warn!( + target: "acp::routing", + error = %e, + "classifier call failed — using default" + ), + } + } + self.fallback() + } +} + +/// Ask a local Ollama model to pick one label. Returns `Ok(None)` when the reply +/// is unusable — an unparseable classification is not an error worth failing a +/// turn over. +async fn classify_ollama(cfg: &Classifier, prompt: &str) -> Result, String> { + if cfg.labels.is_empty() { + return Ok(None); + } + let labels: Vec<&str> = cfg.labels.iter().map(|l| l.label.as_str()).collect(); + // Ask for a bare label rather than JSON: there is exactly one field wanted, + // and a one-word reply cannot be half-parsed the way a JSON object can. + let instruction = format!( + "Classify the task below into exactly one of these categories: {}.\n\ + Reply with ONLY the category word. No punctuation, no explanation.\n\n\ + TASK:\n{}", + labels.join(", "), + prompt + ); + let body = serde_json::json!({ + "model": cfg.model, + "stream": false, + "options": { "temperature": 0 }, + "messages": [{ "role": "user", "content": instruction }], + }); + let http = reqwest::Client::new(); + let resp = http + .post(format!("{}/api/chat", cfg.url.trim_end_matches('/'))) + .timeout(Duration::from_millis(cfg.timeout_ms)) + .json(&body) + .send() + .await + .map_err(|e| format!("ollama request failed: {e}"))?; + if !resp.status().is_success() { + return Err(format!("ollama HTTP {}", resp.status())); + } + let v: serde_json::Value = resp + .json() + .await + .map_err(|e| format!("ollama response parse failed: {e}"))?; + let text = v + .get("message") + .and_then(|m| m.get("content")) + .and_then(|c| c.as_str()) + .unwrap_or_default() + .trim() + .to_string(); + if text.is_empty() { + return Ok(None); + } + // Small models sometimes answer in a sentence. Accept the first configured + // label that appears anywhere in the reply rather than discarding it. + let lower = text.to_lowercase(); + for l in &labels { + if lower.contains(&l.to_lowercase()) { + return Ok(Some((*l).to_string())); + } + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn policy(json: serde_json::Value) -> Policy { + serde_json::from_value(json).expect("policy should parse") + } + + #[test] + fn disabled_policy_never_decides() { + let p = policy(serde_json::json!({ + "enabled": false, + "rules": [{ "any": ["migration"], "model": "m1" }], + "default_model": "fallback" + })); + assert_eq!(p.decide_static("a migration"), None); + assert_eq!(p.fallback(), None); + } + + #[test] + fn first_matching_rule_wins_and_carries_its_name() { + let p = policy(serde_json::json!({ + "enabled": true, + "rules": [ + { "name": "db", "any": ["migration", "schema"], "model": "codex-model" }, + { "name": "ui", "any": ["button"], "model": "ui-model" } + ] + })); + let d = p.decide_static("Add a Postgres MIGRATION for members").unwrap(); + assert_eq!(d.model, "codex-model"); + assert_eq!(d.reason, Reason::Rule("db".into())); + // Case-insensitive, and the later rule still reachable. + assert_eq!(p.decide_static("fix the Button").unwrap().model, "ui-model"); + // No match => no opinion, NOT the first rule. + assert_eq!(p.decide_static("unrelated text"), None); + } + + #[test] + fn contains_all_requires_every_needle() { + let p = policy(serde_json::json!({ + "enabled": true, + "rules": [{ + "name": "both", "match_kind": "contains_all", + "any": ["relay", "membership"], "model": "m" + }] + })); + assert!(p.decide_static("relay membership check").is_some()); + assert!(p.decide_static("relay only").is_none()); + } + + #[test] + fn an_empty_needle_list_never_matches() { + // Guards against "always route here" being created by omission — that + // intent must be expressed as default_model. + let p = policy(serde_json::json!({ + "enabled": true, + "rules": [{ "any": [], "model": "everything" }], + "default_model": "fallback" + })); + assert_eq!(p.decide_static("literally anything"), None); + assert_eq!(p.fallback().unwrap().model, "fallback"); + } + + #[test] + fn absent_default_model_leaves_the_agent_alone() { + let p = policy(serde_json::json!({ "enabled": true, "rules": [] })); + assert_eq!(p.fallback(), None); + } + + #[test] + fn harness_rules_pick_a_class_and_an_absent_block_is_fail_open() { + let p = policy(serde_json::json!({ + "enabled": true, + "rules": [], + "harness": { + "default_class": "claude", + "rules": [ + { "name": "db", "match_kind": "contains_all", "any": ["migration"], "class": "codex" }, + { "name": "ui", "any": ["button"], "class": "opencode" } + ] + } + })); + + // A matched rule names the owning class, case-insensitively. + let d = p.decide_harness("write the MIGRATION").unwrap(); + assert_eq!(d.class, "codex"); + assert_eq!(d.reason, HarnessReason::Rule("db".into())); + + // Same turn: the codex process owns it => no decline; the claude process + // declines toward codex. + assert!(p.harness_decline("write the migration", "codex").is_none()); + assert_eq!( + p.harness_decline("write the migration", "claude").unwrap().class, + "codex" + ); + + // An unmatched turn falls to default_class; a codex process declines + // toward claude, a claude process handles it. + assert_eq!( + p.harness_decline("just chatting", "codex").unwrap().class, + "claude" + ); + assert!(p.harness_decline("just chatting", "claude").is_none()); + + // self_class in the block overrides the passed identity. + let owned = policy(serde_json::json!({ + "enabled": true, "rules": [], + "harness": { "self_class": "codex", "default_class": "claude" } + })); + assert_eq!( + owned.harness_decline("anything", "opencode").unwrap().class, + "claude" + ); + + // No harness block => never declines (unchanged behavior). + let bare = policy(serde_json::json!({ "enabled": true, "rules": [] })); + assert!(bare.harness_decline("anything", "codex").is_none()); + assert_eq!(bare.decide_harness("anything"), None); + + // Disabled policy => no opinion even with a harness block. + let off = policy(serde_json::json!({ + "enabled": false, + "harness": { "default_class": "codex" } + })); + assert!(off.harness_decline("x", "claude").is_none()); + } + + #[test] + fn a_harness_block_with_a_classifier_field_is_rejected() { + // Guards R2: a distributed decline cannot use a non-deterministic + // classifier, so the field must not exist. deny_unknown_fields makes the + // mistake loud rather than silently ignoring it. + let err = serde_json::from_value::(serde_json::json!({ + "enabled": true, + "harness": { + "default_class": "codex", + "classifier": { "url": "http://x", "model": "m" } + } + })); + assert!(err.is_err(), "a classifier inside harness must not parse"); + } + + #[tokio::test] + async fn empty_prompt_and_unreachable_classifier_both_degrade_to_default() { + let p = policy(serde_json::json!({ + "enabled": true, + "rules": [], + // Port 1 is reserved and never listening, so this exercises the + // failure path without depending on a live Ollama. + "classifier": { + "url": "http://127.0.0.1:1", "model": "gemma3:27b", "timeout_ms": 500, + "labels": [{ "label": "code", "model": "code-model" }] + }, + "default_model": "fallback" + })); + assert_eq!(p.decide("").await, None, "empty prompt must not route"); + let d = p.decide("some real task text").await.unwrap(); + assert_eq!(d.model, "fallback"); + assert_eq!(d.reason, Reason::Default); + } + + /// Live classifier check against a real Ollama. Skips unless + /// `BUZZ_ROUTING_LIVE_OLLAMA` names a base url, matching the existing + /// env-gated pattern in `crates/buzz-test-client/tests/e2e_mesh_llm.rs` — + /// the unit tests above cover the logic, but only a live model proves the + /// prompt actually elicits a usable one-word label. + /// + /// BUZZ_ROUTING_LIVE_OLLAMA=http://localhost:11434 \ + /// cargo test -p buzz-acp --lib -- routing::tests::live_ollama --nocapture + #[tokio::test] + async fn live_ollama_classifier_returns_a_configured_label() { + let Ok(url) = std::env::var("BUZZ_ROUTING_LIVE_OLLAMA") else { + eprintln!("SKIP: BUZZ_ROUTING_LIVE_OLLAMA not set — needs a live Ollama endpoint"); + return; + }; + let model = + std::env::var("BUZZ_ROUTING_LIVE_MODEL").unwrap_or_else(|_| "gemma3:27b".to_string()); + let p = policy(serde_json::json!({ + "enabled": true, + "rules": [], + "classifier": { + "url": url, "model": model, "timeout_ms": 120000, + "labels": [ + { "label": "database", "model": "db-model" }, + { "label": "frontend", "model": "ui-model" } + ] + }, + "default_model": "fallback" + })); + + let d = p + .decide("Write the Postgres migration adding a unique index on relay_members.") + .await + .expect("a decision"); + eprintln!("live classifier -> {:?}", d); + assert_eq!( + d.model, "db-model", + "a migration task should classify as database, got {:?}", + d.reason + ); + assert_eq!( + d.reason, + Reason::Classifier { + label: "database".into() + } + ); + } + + /// The other half of the desktop contract. Buzz Desktop writes this file + /// (`commands/agent_routing_policy.rs`) and points `BUZZ_ROUTING_POLICY` at + /// it; `from_env` swallows a parse failure by design, so a field rename + /// would disable routing silently. This document is byte-for-byte what + /// `policy_shape_matches_the_harness_contract` asserts the desktop emits — + /// if one side is renamed, one of the two tests fails. + #[test] + fn a_desktop_written_policy_parses() { + let raw = r#"{ + "enabled": true, + "rules": [ + { + "name": "db", + "match_kind": "contains_all", + "any": ["migration"], + "model": "codex-model" + } + ], + "classifier": { + "url": "http://localhost:11434", + "model": "gemma3:27b", + "labels": [{ "label": "database", "model": "db-model" }], + "timeout_ms": 20000 + }, + "default_model": "fallback" + }"#; + + let p: Policy = serde_json::from_str(raw).expect("desktop-written policy must parse"); + assert!(p.enabled); + assert_eq!(p.rules[0].match_kind, MatchKind::ContainsAll); + assert_eq!(p.rules[0].model, "codex-model"); + assert_eq!(p.default_model.as_deref(), Some("fallback")); + let classifier = p.classifier.as_ref().expect("classifier"); + assert_eq!(classifier.timeout_ms, 20_000); + assert_eq!(classifier.labels[0].model, "db-model"); + + // A rule the desktop saved must actually route. + let decision = p.decide_static("write the migration").expect("a decision"); + assert_eq!(decision.model, "codex-model"); + } + + /// The desktop omits `classifier` and `default_model` when unset + /// (`skip_serializing_if`). That minimal document must still load. + #[test] + fn a_minimal_desktop_policy_parses() { + let p: Policy = serde_json::from_str(r#"{"enabled": false, "rules": []}"#).expect("parse"); + assert!(!p.enabled); + assert!(p.classifier.is_none()); + assert!(p.default_model.is_none()); + } + + #[test] + fn a_broken_policy_file_disables_routing_rather_than_erroring() { + assert!(serde_json::from_str::("{ not json").is_err()); + // from_env's contract: unparseable => None (verified by the type above; + // from_env itself is exercised by the runtime probe, not here, because it + // reads process env). + } +} diff --git a/crates/buzz-agent/src/catalog.rs b/crates/buzz-agent/src/catalog.rs index aa2a121c99..012b662672 100644 --- a/crates/buzz-agent/src/catalog.rs +++ b/crates/buzz-agent/src/catalog.rs @@ -1,8 +1,8 @@ -//! Databricks model catalog discovery. +//! Model catalog discovery. //! -//! Exposes [`discover_databricks_models`] — an async helper that lists -//! available models for the `databricks` and `databricks_v2` providers -//! without triggering a browser OAuth flow. Auth is acquired in-process via +//! Exposes [`discover_databricks_models`] and [`discover_openrouter_models`] — +//! async helpers that list available models for a provider without triggering a +//! browser OAuth flow. Auth is acquired in-process via //! [`build_token_source`](crate::llm::build_token_source): //! //! - Static bearer (`DATABRICKS_TOKEN`): returned immediately. @@ -129,6 +129,138 @@ pub async fn discover_databricks_models(cfg: &Config) -> Result, } } +// --------------------------------------------------------------------------- +// OpenRouter — api/v1/models/user (falls back to api/v1/models) +// --------------------------------------------------------------------------- + +/// Discover available models for [`Provider::OpenRouter`]. +/// +/// Queries `/models/user` — the **account-scoped** catalog — rather than the +/// global `/models`. This distinction is not cosmetic: `/models` lists every +/// model OpenRouter knows about (338 at time of writing, 272 tools-capable), +/// but a given account can only call the subset on its eligibility allowlist +/// (21, of which 13 are tools-capable). Calling an ineligible model returns +/// +/// > HTTP 404 "No endpoints available matching your guardrail restrictions and +/// > data policy" +/// +/// which reads like a privacy-settings problem and sends you looking in the +/// wrong place. Advertising the global list in a model picker therefore offers +/// hundreds of models that fail at request time, so `/models/user` is the +/// correct source and `/models` is only a degraded fallback for keys whose +/// account scope is unavailable. +/// +/// Returns a non-empty `Vec` on success. Returns +/// `Err(AgentError::LlmAuth)` when no token is available — callers degrade +/// gracefully via [`discovery_failure_fallback`]. +/// +/// # Panics +/// Never panics. +pub async fn discover_openrouter_models(cfg: &Config) -> Result, AgentError> { + if cfg.provider != Provider::OpenRouter { + return Err(AgentError::InvalidParams( + "discover_openrouter_models called for non-OpenRouter provider".into(), + )); + } + let token_source = build_token_source(cfg)?; + let bearer = token_source.bearer_no_browser().await?; + + let http = Client::new(); + let host = cfg.base_url.trim_end_matches('/'); + + // Account-scoped first; fall back to the global catalog only if that fails. + match fetch_openrouter_models(&http, &format!("{host}/models/user"), &bearer).await { + Ok(models) => Ok(models), + Err(scoped_err) => { + tracing::debug!( + error = %scoped_err, + "OpenRouter account-scoped model discovery failed; falling back to the global catalog (may list models this account cannot call)" + ); + fetch_openrouter_models(&http, &format!("{host}/models"), &bearer).await + } + } +} + +async fn fetch_openrouter_models( + http: &Client, + url: &str, + bearer: &str, +) -> Result, AgentError> { + let response = http + .get(url) + .bearer_auth(bearer) + .send() + .await + .map_err(|e| AgentError::Llm(format!("OpenRouter model discovery request failed: {e}")))?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AgentError::Llm(format!( + "OpenRouter model discovery HTTP {status}: {body}" + ))); + } + + let json: serde_json::Value = response.json().await.map_err(|e| { + AgentError::Llm(format!( + "OpenRouter model discovery response parse failed: {e}" + )) + })?; + + parse_openrouter_models(&json) +} + +/// Parse an OpenRouter `models` payload into selectable entries. +/// +/// Keeps only models advertising the `tools` parameter: this catalog feeds an +/// agent harness, and a model that cannot take tool calls cannot do the job, so +/// offering it in the picker only produces a confusing failure later. +pub(crate) fn parse_openrouter_models( + json: &serde_json::Value, +) -> Result, AgentError> { + let data = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { + AgentError::Llm( + "OpenRouter model discovery: unexpected response (missing 'data' array)".into(), + ) + })?; + + let models: Vec = data + .iter() + .filter_map(|entry| { + let id = entry.get("id")?.as_str()?.trim(); + if id.is_empty() { + return None; + } + let tools_capable = entry + .get("supported_parameters") + .and_then(|v| v.as_array()) + .is_some_and(|params| params.iter().any(|p| p.as_str() == Some("tools"))); + if !tools_capable { + return None; + } + // OpenRouter has no separate display name; `name` carries a vendor + // label, but the id is what the picker must round-trip. + let name = entry + .get("name") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .unwrap_or(id); + Some(ModelEntry { + id: id.to_string(), + name: name.to_string(), + }) + }) + .collect(); + + if models.is_empty() { + return Err(AgentError::Llm( + "OpenRouter model discovery returned no tools-capable models".into(), + )); + } + Ok(models) +} + // --------------------------------------------------------------------------- // v1 — api/2.0/serving-endpoints // --------------------------------------------------------------------------- @@ -397,6 +529,75 @@ pub(crate) fn parse_v2_endpoints_page( mod tests { use super::*; + #[test] + fn openrouter_parse_keeps_only_tools_capable_models() { + let json = serde_json::json!({ + "data": [ + // included: advertises tools + {"id": "openai/gpt-5.6-luna", "name": "OpenAI: GPT-5.6 Luna", + "supported_parameters": ["tools", "temperature"]}, + // included: tools among many params + {"id": "deepseek/deepseek-v4-flash-0731", + "supported_parameters": ["temperature", "tools"]}, + // excluded: no tools support — cannot serve an agent harness + {"id": "some/completion-only", "supported_parameters": ["temperature"]}, + // excluded: supported_parameters absent entirely + {"id": "some/unknown-caps"}, + // excluded: empty id + {"id": " ", "supported_parameters": ["tools"]}, + ] + }); + + let models = parse_openrouter_models(&json).unwrap(); + let ids: Vec<&str> = models.iter().map(|m| m.id.as_str()).collect(); + assert_eq!( + ids, + vec!["openai/gpt-5.6-luna", "deepseek/deepseek-v4-flash-0731"] + ); + // `name` is used when present, else the id round-trips as the label. + assert_eq!(models[0].name, "OpenAI: GPT-5.6 Luna"); + assert_eq!(models[1].name, "deepseek/deepseek-v4-flash-0731"); + } + + #[test] + fn openrouter_parse_errors_on_missing_data_array() { + let json = serde_json::json!({"endpoints": []}); + let err = parse_openrouter_models(&json).unwrap_err(); + assert!( + format!("{err}").contains("missing 'data' array"), + "unexpected error: {err}" + ); + } + + #[test] + fn openrouter_parse_errors_when_nothing_is_tools_capable() { + // A catalog that parses but offers nothing usable must be an error, not an + // empty picker: an empty list would make every switch_model request fail + // validation with no indication of why. + let json = serde_json::json!({ + "data": [{"id": "a/b", "supported_parameters": ["temperature"]}] + }); + let err = parse_openrouter_models(&json).unwrap_err(); + assert!( + format!("{err}").contains("no tools-capable models"), + "unexpected error: {err}" + ); + } + + #[test] + fn openrouter_fallback_is_the_configured_model() { + // Discovery failure must still leave the picker able to represent the + // model the agent is actually running. + let entries = discovery_failure_fallback(Provider::OpenRouter, "openai/gpt-5.6-luna"); + assert_eq!( + entries, + vec![ModelEntry { + id: "openai/gpt-5.6-luna".into(), + name: "openai/gpt-5.6-luna".into() + }] + ); + } + #[test] fn v1_parse_filters_ready_chat_endpoints() { let json = serde_json::json!({ diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index 9a45bf4c98..b3c64693b8 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -11,7 +11,9 @@ mod mcp; pub mod types; mod wire; -pub use catalog::{discover_databricks_models, ModelEntry, DATABRICKS_V2_KNOWN_MODELS}; +pub use catalog::{ + discover_databricks_models, discover_openrouter_models, ModelEntry, DATABRICKS_V2_KNOWN_MODELS, +}; pub use config::Provider; pub use types::AgentError; @@ -445,11 +447,16 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen ); drop(sessions); - // Build a models catalog for the `session/new` response. For Databricks - // providers this advertises available models so the desktop ModelPicker and - // pool can resolve `session/set_model` switches. For Anthropic/OpenAI we - // report only the configured model — live switching on those providers - // effectively requires respawn. + // Build a models catalog for the `session/new` response. For Databricks and + // OpenRouter providers this advertises available models so the desktop + // ModelPicker and pool can resolve `session/set_model` switches. For + // Anthropic/OpenAI we report only the configured model — live switching on + // those providers effectively requires respawn. + // + // Advertising a real catalog is what makes kind:24200 `switch_model` usable: + // both the idle and busy switch paths validate the requested model against + // this list, so a single-entry catalog makes every switch return + // `UnsupportedModel`. // // `models_cache` caches only a successful discovery result (`get_or_try_init` // leaves the cell empty on error so the next `session/new` call retries). On @@ -457,21 +464,33 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen // being written to the cell. let available_models: Vec = { use crate::config::Provider; - match app.cfg.provider { - Provider::Databricks | Provider::DatabricksV2 => { - let models = resolve_models_catalog( + let discovered = match app.cfg.provider { + Provider::Databricks | Provider::DatabricksV2 => Some( + resolve_models_catalog( &app.models_cache, app.cfg.provider, &app.cfg.model, discover_databricks_models(&app.cfg), ) - .await; - models - .iter() - .map(|m| json!({ "modelId": m.id, "name": m.name })) - .collect() - } - _ => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], + .await, + ), + Provider::OpenRouter => Some( + resolve_models_catalog( + &app.models_cache, + app.cfg.provider, + &app.cfg.model, + discover_openrouter_models(&app.cfg), + ) + .await, + ), + Provider::Anthropic | Provider::OpenAi => None, + }; + match discovered { + Some(models) => models + .iter() + .map(|m| json!({ "modelId": m.id, "name": m.name })) + .collect(), + None => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], } }; diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 7ce7f48389..740e30d656 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -126,6 +126,8 @@ export default defineConfig({ "**/agent-provider-dropdowns.spec.ts", "**/agent-lifecycle-feedback.spec.ts", "**/agent-access-warning.spec.ts", + "**/agent-model-picker.spec.ts", + "**/agent-routing-policy.spec.ts", "**/inbox-live-update.spec.ts", "**/mesh-compute.spec.ts", "**/observer-archive-policy.spec.ts", diff --git a/desktop/src-tauri/src/commands/agent_routing_policy.rs b/desktop/src-tauri/src/commands/agent_routing_policy.rs new file mode 100644 index 0000000000..62b691846c --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_routing_policy.rs @@ -0,0 +1,383 @@ +//! Tauri commands for per-agent model routing policies. +//! +//! `buzz-acp` reads its routing policy from the file named by +//! `BUZZ_ROUTING_POLICY` (see `crates/buzz-acp/src/routing.rs`). These commands +//! own the other half of that contract: they write the file and report where it +//! lives, so the agent dialog can point the env var at it. +//! +//! Setting the env var is deliberately NOT done here. The edit dialog holds the +//! whole `env_vars` map in local state and replaces it wholesale on submit, so a +//! backend-side patch would be silently overwritten by the next save. The +//! frontend merges the returned path into that map instead. +//! +//! The types below mirror the serde shape of `buzz_acp::routing::Policy`. They +//! are duplicated rather than imported because `buzz-acp` is a sidecar the +//! desktop talks to across a process boundary, not a library it links. +//! `policy_shape_matches_the_harness_contract` pins the emitted JSON so a rename +//! on either side fails a test instead of silently disabling routing. + +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; +use tauri::AppHandle; + +use crate::managed_agents::managed_agents_base_dir; + +/// How a rule matches the prompt text. Mirrors `buzz_acp::routing::MatchKind`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "snake_case")] +pub enum MatchKind { + /// Any needle appears in the prompt. + #[default] + Contains, + /// Every needle appears in the prompt. + ContainsAll, +} + +/// One deterministic routing rule. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RoutingRule { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(default)] + pub match_kind: MatchKind, + #[serde(default)] + pub any: Vec, + pub model: String, +} + +/// Optional local classifier, consulted only when no rule matched. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RoutingClassifier { + pub url: String, + pub model: String, + #[serde(default)] + pub labels: Vec, + #[serde(default = "default_timeout_ms")] + pub timeout_ms: u64, +} + +fn default_timeout_ms() -> u64 { + 20_000 +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RoutingLabelTarget { + pub label: String, + pub model: String, +} + +/// A routing policy, in the exact shape `buzz-acp` deserializes. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct RoutingPolicy { + #[serde(default)] + pub enabled: bool, + #[serde(default)] + pub rules: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub classifier: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default_model: Option, +} + +/// Where an agent's policy lives, and what is currently in it. +/// +/// `path` is always populated — the UI needs it to set `BUZZ_ROUTING_POLICY` +/// even on the save that creates the file. `policy` is `None` when nothing has +/// been written yet, or when the file on disk is unreadable/unparseable, which +/// is the same thing the harness would conclude. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentRoutingPolicyFile { + pub path: String, + pub policy: Option, +} + +/// Read the routing policy for `pubkey`, if one has been written. +#[tauri::command] +pub fn get_agent_routing_policy( + pubkey: String, + app: AppHandle, +) -> Result { + let path = routing_policy_path(&app, &pubkey)?; + let policy = std::fs::read_to_string(&path) + .ok() + .and_then(|raw| serde_json::from_str::(&raw).ok()); + Ok(AgentRoutingPolicyFile { + path: path.to_string_lossy().into_owned(), + policy, + }) +} + +/// Write (or, with `policy: None`, delete) the routing policy for `pubkey`. +/// +/// Returns the path in both cases so the caller can set or clear +/// `BUZZ_ROUTING_POLICY` without recomputing it. +#[tauri::command] +pub fn set_agent_routing_policy( + pubkey: String, + policy: Option, + app: AppHandle, +) -> Result { + let path = routing_policy_path(&app, &pubkey)?; + + let Some(policy) = policy else { + match std::fs::remove_file(&path) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(format!("failed to delete routing policy: {error}")), + } + return Ok(AgentRoutingPolicyFile { + path: path.to_string_lossy().into_owned(), + policy: None, + }); + }; + + let policy = normalize_policy(policy)?; + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .map_err(|error| format!("failed to create routing policy dir: {error}"))?; + } + let json = serde_json::to_string_pretty(&policy) + .map_err(|error| format!("failed to serialize routing policy: {error}"))?; + std::fs::write(&path, json) + .map_err(|error| format!("failed to write routing policy: {error}"))?; + + Ok(AgentRoutingPolicyFile { + path: path.to_string_lossy().into_owned(), + policy: Some(policy), + }) +} + +fn routing_policy_path(app: &AppHandle, pubkey: &str) -> Result { + Ok(managed_agents_base_dir(app)? + .join("routing") + .join(format!("{}.json", validated_pubkey(pubkey)?))) +} + +/// The pubkey becomes a filename, so it must not be able to escape the routing +/// directory. Agent pubkeys are hex (or npub), both strictly alphanumeric — +/// rejecting everything else closes the traversal hole at the boundary rather +/// than trusting the caller. +fn validated_pubkey(pubkey: &str) -> Result<&str, String> { + if pubkey.is_empty() || pubkey.len() > 128 { + return Err("routing policy: agent pubkey has an invalid length".to_string()); + } + if !pubkey.chars().all(|c| c.is_ascii_alphanumeric()) { + return Err("routing policy: agent pubkey must be alphanumeric".to_string()); + } + Ok(pubkey) +} + +/// Trim user input and reject rules the harness would silently ignore. +/// +/// A rule with no needles never matches (`routing.rs` makes that explicit so +/// "always route here" cannot be created by omission), and a rule with no model +/// has nothing to route to. Both are user mistakes worth naming at save time +/// rather than discovering as a policy that quietly does nothing. +fn normalize_policy(mut policy: RoutingPolicy) -> Result { + for (index, rule) in policy.rules.iter_mut().enumerate() { + rule.name = rule + .name + .as_deref() + .map(str::trim) + .filter(|name| !name.is_empty()) + .map(str::to_string); + rule.model = rule.model.trim().to_string(); + rule.any = rule + .any + .iter() + .map(|needle| needle.trim().to_string()) + .filter(|needle| !needle.is_empty()) + .collect(); + + let label = rule + .name + .clone() + .unwrap_or_else(|| format!("rule {}", index + 1)); + if rule.model.is_empty() { + return Err(format!("{label} needs a model to route to.")); + } + if rule.any.is_empty() { + return Err(format!( + "{label} needs at least one phrase to match. To route everything, set a default model instead." + )); + } + } + + policy.default_model = policy + .default_model + .as_deref() + .map(str::trim) + .filter(|model| !model.is_empty()) + .map(str::to_string); + + if let Some(classifier) = policy.classifier.as_mut() { + classifier.url = classifier.url.trim().to_string(); + classifier.model = classifier.model.trim().to_string(); + classifier + .labels + .retain(|target| !target.label.trim().is_empty() && !target.model.trim().is_empty()); + if classifier.url.is_empty() || classifier.model.is_empty() { + return Err("The classifier needs both a URL and a model.".to_string()); + } + } + + Ok(policy) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rule(model: &str, any: &[&str]) -> RoutingRule { + RoutingRule { + name: None, + match_kind: MatchKind::Contains, + any: any.iter().map(|s| (*s).to_string()).collect(), + model: model.to_string(), + } + } + + /// Pins the on-disk contract with `crates/buzz-acp/src/routing.rs`. If a + /// field is renamed on either side, routing silently stops working — the + /// harness's `from_env` swallows a parse failure by design. This test is + /// the tripwire; its counterpart is + /// `routing::tests::a_desktop_written_policy_parses`. + #[test] + fn policy_shape_matches_the_harness_contract() { + let policy = RoutingPolicy { + enabled: true, + rules: vec![RoutingRule { + name: Some("db".to_string()), + match_kind: MatchKind::ContainsAll, + any: vec!["migration".to_string()], + model: "codex-model".to_string(), + }], + classifier: Some(RoutingClassifier { + url: "http://localhost:11434".to_string(), + model: "gemma3:27b".to_string(), + labels: vec![RoutingLabelTarget { + label: "database".to_string(), + model: "db-model".to_string(), + }], + timeout_ms: 20_000, + }), + default_model: Some("fallback".to_string()), + }; + + let json: serde_json::Value = serde_json::to_value(&policy).expect("serialize"); + assert_eq!( + json, + serde_json::json!({ + "enabled": true, + "rules": [{ + "name": "db", + "match_kind": "contains_all", + "any": ["migration"], + "model": "codex-model" + }], + "classifier": { + "url": "http://localhost:11434", + "model": "gemma3:27b", + "labels": [{ "label": "database", "model": "db-model" }], + "timeout_ms": 20000 + }, + "default_model": "fallback" + }) + ); + } + + #[test] + fn a_disabled_empty_policy_serializes_without_null_noise() { + let json = serde_json::to_value(RoutingPolicy::default()).expect("serialize"); + assert_eq!(json, serde_json::json!({ "enabled": false, "rules": [] })); + } + + #[test] + fn normalize_trims_and_drops_blank_needles() { + let policy = normalize_policy(RoutingPolicy { + enabled: true, + rules: vec![RoutingRule { + name: Some(" db ".to_string()), + any: vec![" migration ".to_string(), " ".to_string()], + model: " codex ".to_string(), + ..rule("x", &["y"]) + }], + default_model: Some(" ".to_string()), + ..Default::default() + }) + .expect("normalize"); + + assert_eq!(policy.rules[0].name.as_deref(), Some("db")); + assert_eq!(policy.rules[0].any, vec!["migration".to_string()]); + assert_eq!(policy.rules[0].model, "codex"); + assert_eq!( + policy.default_model, None, + "a whitespace-only default model is no default model" + ); + } + + #[test] + fn a_rule_with_no_needles_is_rejected_by_name() { + let err = normalize_policy(RoutingPolicy { + enabled: true, + rules: vec![RoutingRule { + name: Some("catch-all".to_string()), + ..rule("m", &[]) + }], + ..Default::default() + }) + .unwrap_err(); + assert!( + err.contains("catch-all"), + "error should name the rule: {err}" + ); + assert!( + err.contains("default model"), + "error should point at the fix: {err}" + ); + } + + #[test] + fn an_unnamed_bad_rule_is_reported_by_its_position() { + let err = normalize_policy(RoutingPolicy { + enabled: true, + rules: vec![rule("ok-model", &["x"]), rule("", &["y"])], + ..Default::default() + }) + .unwrap_err(); + assert!( + err.starts_with("rule 2"), + "expected a 1-based position: {err}" + ); + } + + #[test] + fn a_classifier_missing_its_url_is_rejected() { + let err = normalize_policy(RoutingPolicy { + enabled: true, + classifier: Some(RoutingClassifier { + url: " ".to_string(), + model: "gemma3:27b".to_string(), + labels: vec![], + timeout_ms: 20_000, + }), + ..Default::default() + }) + .unwrap_err(); + assert!(err.contains("classifier"), "{err}"); + } + + #[test] + fn a_pubkey_that_could_escape_the_routing_dir_is_rejected() { + for bad in ["../../etc/passwd", "a/b", "a\\b", "", "a.b"] { + assert!( + validated_pubkey(bad).is_err(), + "{bad:?} must not be accepted as a filename" + ); + } + assert!(validated_pubkey("deadbeef00").is_ok()); + } +} diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 66ef7ef17b..a2b06376eb 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -7,6 +7,7 @@ mod agent_model_process; mod agent_models; mod agent_models_env; mod agent_providers; +mod agent_routing_policy; mod agent_settings; mod agent_update_rollback; mod agents; @@ -68,6 +69,7 @@ pub use agent_logs::*; pub use agent_metric_archive::*; pub use agent_models::*; pub use agent_providers::*; +pub use agent_routing_policy::*; pub use agent_settings::*; pub use agents::*; pub use canvas::*; diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c4b733e3e0..94c2f09f78 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -819,6 +819,8 @@ pub fn run() { get_baked_build_env_keys, get_baked_build_env, put_agent_session_config, + get_agent_routing_policy, + set_agent_routing_policy, get_global_agent_config, set_global_agent_config, mesh_start_node, diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs index f8b045fc72..e13830abb3 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs @@ -2,6 +2,7 @@ mod buzz_agent; mod claude; mod codex; mod goose; +mod opencode; pub(crate) mod reader; mod schema_walker; pub(crate) mod types; diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs b/desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs new file mode 100644 index 0000000000..57d1a63d9a --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs @@ -0,0 +1,395 @@ +use std::path::PathBuf; + +use super::types::{ExtensionEntry, RuntimeFileConfig}; + +/// Read OpenCode config from `$OPENCODE_CONFIG`, else +/// `$XDG_CONFIG_HOME/opencode/opencode.json(c)`, else +/// `~/.config/opencode/opencode.json(c)`. +/// +/// This tier matters more for OpenCode than for the other harnesses: `opencode +/// acp` takes no `--model` flag and reads no model env var, so the config file +/// is the ONLY place its model is set. Without this reader the model field is +/// blank in the panel even when the harness is perfectly well configured. +pub(super) fn read_config_file() -> Option { + let raw = std::fs::read_to_string(opencode_config_path()?).ok()?; + parse_opencode_config(&raw) +} + +/// Canonical config path for display and for the reader. +/// +/// Returns the `.json` path even when nothing exists on disk yet — the panel +/// shows where the file *would* live, matching how `claude` reports +/// `~/.claude.json` unconditionally. +pub(crate) fn opencode_config_path() -> Option { + if let Some(explicit) = std::env::var_os("OPENCODE_CONFIG") { + let path = PathBuf::from(explicit); + if !path.as_os_str().is_empty() { + return Some(path); + } + } + + let dir = opencode_config_dir()?; + let json = dir.join("opencode.json"); + if json.exists() { + return Some(json); + } + let jsonc = dir.join("opencode.jsonc"); + if jsonc.exists() { + return Some(jsonc); + } + Some(json) +} + +fn opencode_config_dir() -> Option { + if let Some(xdg) = std::env::var_os("XDG_CONFIG_HOME") { + let base = PathBuf::from(xdg); + if !base.as_os_str().is_empty() { + return Some(base.join("opencode")); + } + } + Some(dirs::home_dir()?.join(".config").join("opencode")) +} + +fn parse_opencode_config(raw: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(&strip_jsonc(raw)).ok()?; + + // OpenCode writes the model as `provider_id/model_id`. Split it so the + // normalized provider and model fields each carry their own half rather + // than repeating the whole pair in both. + let (provider, model) = match json_string(&value, "model") { + Some(spec) => match spec.split_once('/') { + Some((p, m)) if !p.is_empty() && !m.is_empty() => { + (Some(p.to_string()), Some(m.to_string())) + } + // No slash (or a malformed one) — surface the value as written + // instead of guessing at a provider. + _ => (None, Some(spec)), + }, + None => (None, None), + }; + + let extensions = parse_mcp_servers(&value); + + // Config-driven extra fields — skip keys extracted into typed fields above. + let skip = &["model", "provider", "mcp"]; + let mut extra = super::schema_walker::extract_config_fields(&value, skip); + + // Custom providers from `provider.` — surface as + // "provider. = configured" rather than flattening their model tables, + // mirroring how the codex reader handles `model_providers`. + if let Some(serde_json::Value::Object(providers)) = value.get("provider") { + for name in providers.keys() { + extra.insert(format!("provider.{name}"), "configured".to_string()); + } + } + + Some(RuntimeFileConfig { + model, + provider, + // OpenCode has no single mode/effort/limit key: permissions live under + // `permission`, reasoning effort is per-model under + // `provider..models..options`. Both reach the panel via `extra`. + mode: None, + thinking_effort: None, + max_output_tokens: None, + context_limit: None, + // `instructions` is a list of file PATHS, not prompt text, so it is not + // a system prompt. The walker surfaces it in `extra`. + system_prompt: None, + extensions, + extra, + }) +} + +fn parse_mcp_servers(value: &serde_json::Value) -> Vec { + let Some(servers) = value.get("mcp").and_then(|v| v.as_object()) else { + return Vec::new(); + }; + + servers + .iter() + .map(|(name, config)| ExtensionEntry { + name: name.clone(), + kind: "mcp".to_string(), + // OpenCode runs an MCP server unless it explicitly opts out. + enabled: config + .get("enabled") + .and_then(serde_json::Value::as_bool) + .unwrap_or(true), + }) + .collect() +} + +fn json_string(value: &serde_json::Value, key: &str) -> Option { + value + .get(key)? + .as_str() + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) +} + +/// Make a JSONC document parseable by `serde_json`: drop comments, then drop +/// trailing commas. OpenCode documents `.jsonc` as a first-class config format +/// and its own docs use both, so a plain `serde_json` parse would reject real +/// user configs. +fn strip_jsonc(raw: &str) -> String { + strip_trailing_commas(&strip_comments(raw)) +} + +/// Remove `//` and `/* */` comments. String-aware: `//` inside a JSON string +/// must survive — every OpenCode config carries at least one URL (`$schema`). +/// Newlines inside block comments are preserved so line-based error offsets +/// still line up with the original file. +fn strip_comments(raw: &str) -> String { + let chars: Vec = raw.chars().collect(); + let mut out = String::with_capacity(raw.len()); + let mut i = 0; + let mut in_string = false; + + while i < chars.len() { + let c = chars[i]; + + if in_string { + out.push(c); + if c == '\\' && i + 1 < chars.len() { + out.push(chars[i + 1]); + i += 2; + continue; + } + if c == '"' { + in_string = false; + } + i += 1; + continue; + } + + match c { + '"' => { + in_string = true; + out.push(c); + i += 1; + } + '/' if chars.get(i + 1) == Some(&'/') => { + while i < chars.len() && chars[i] != '\n' { + i += 1; + } + } + '/' if chars.get(i + 1) == Some(&'*') => { + i += 2; + while i < chars.len() && !(chars[i] == '*' && chars.get(i + 1) == Some(&'/')) { + if chars[i] == '\n' { + out.push('\n'); + } + i += 1; + } + i = i.saturating_add(2).min(chars.len()); + } + _ => { + out.push(c); + i += 1; + } + } + } + + out +} + +/// Drop a `,` whose next significant character is `}` or `]`. Runs on +/// already-comment-free text, so "significant" only has to skip whitespace. +fn strip_trailing_commas(raw: &str) -> String { + let chars: Vec = raw.chars().collect(); + let mut out = String::with_capacity(raw.len()); + let mut i = 0; + let mut in_string = false; + + while i < chars.len() { + let c = chars[i]; + + if in_string { + out.push(c); + if c == '\\' && i + 1 < chars.len() { + out.push(chars[i + 1]); + i += 2; + continue; + } + if c == '"' { + in_string = false; + } + i += 1; + continue; + } + + if c == '"' { + in_string = true; + out.push(c); + i += 1; + continue; + } + + if c == ',' { + let next = chars[i + 1..].iter().find(|ch| !ch.is_whitespace()); + if matches!(next, Some('}') | Some(']')) { + i += 1; + continue; + } + } + + out.push(c); + i += 1; + } + + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn model_splits_into_provider_and_model() { + let cfg = parse_opencode_config(r#"{"model": "anthropic/claude-sonnet-4-5"}"#).unwrap(); + assert_eq!(cfg.provider.as_deref(), Some("anthropic")); + assert_eq!(cfg.model.as_deref(), Some("claude-sonnet-4-5")); + } + + #[test] + fn model_id_containing_slashes_keeps_everything_after_the_first() { + // `lmstudio/google/gemma-3n-e4b` — provider is the FIRST segment; the + // rest is the model id, which may itself contain slashes. + let cfg = parse_opencode_config(r#"{"model": "lmstudio/google/gemma-3n-e4b"}"#).unwrap(); + assert_eq!(cfg.provider.as_deref(), Some("lmstudio")); + assert_eq!(cfg.model.as_deref(), Some("google/gemma-3n-e4b")); + } + + #[test] + fn model_without_a_provider_prefix_is_surfaced_as_written() { + let cfg = parse_opencode_config(r#"{"model": "gpt-5"}"#).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("gpt-5")); + assert!(cfg.provider.is_none()); + } + + #[test] + fn mcp_servers_become_extensions_and_honor_enabled_false() { + let cfg = parse_opencode_config( + r#"{"mcp": { + "filesystem": {"type": "local", "command": ["npx", "-y", "fs"]}, + "sentry": {"type": "remote", "url": "https://x", "enabled": false} + }}"#, + ) + .unwrap(); + assert_eq!(cfg.extensions.len(), 2); + let sentry = cfg.extensions.iter().find(|e| e.name == "sentry").unwrap(); + assert!(!sentry.enabled); + let fs = cfg + .extensions + .iter() + .find(|e| e.name == "filesystem") + .unwrap(); + assert!(fs.enabled, "an mcp entry with no `enabled` key defaults on"); + } + + #[test] + fn custom_providers_are_summarized_not_flattened() { + let cfg = parse_opencode_config( + r#"{ + "model": "helicone/gpt-4o", + "provider": {"helicone": {"npm": "@ai-sdk/openai-compatible", "models": {"gpt-4o": {}}}} + }"#, + ) + .unwrap(); + assert_eq!( + cfg.extra.get("provider.helicone").map(String::as_str), + Some("configured") + ); + assert!( + !cfg.extra + .keys() + .any(|k| k.starts_with("provider.helicone.")), + "provider internals must not be flattened into extra" + ); + } + + #[test] + fn normalized_keys_are_not_duplicated_in_extra() { + let cfg = parse_opencode_config( + r#"{"model": "anthropic/x", "mcp": {"a": {}}, "theme": "opencode"}"#, + ) + .unwrap(); + assert!(!cfg.extra.contains_key("model")); + assert!(!cfg.extra.contains_key("mcp.a")); + assert_eq!(cfg.extra.get("theme").map(String::as_str), Some("opencode")); + } + + #[test] + fn unknown_future_fields_reach_extra() { + let cfg = parse_opencode_config(r#"{"some_new_opencode_field": "value"}"#).unwrap(); + assert_eq!( + cfg.extra.get("some_new_opencode_field").map(String::as_str), + Some("value") + ); + } + + #[test] + fn jsonc_comments_are_stripped_without_eating_urls() { + let raw = r#"{ + // the schema line is a comment magnet + "$schema": "https://opencode.ai/config.json", + "model": "openai/gpt-5" /* inline block */ + }"#; + let cfg = parse_opencode_config(raw).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("gpt-5")); + assert_eq!( + cfg.extra.get("$schema").map(String::as_str), + Some("https://opencode.ai/config.json"), + "a `//` inside a string must survive comment stripping" + ); + } + + #[test] + fn a_double_slash_inside_a_string_is_never_treated_as_a_comment() { + let stripped = strip_comments(r#"{"url": "http://localhost:8080/v1"}"#); + assert_eq!(stripped, r#"{"url": "http://localhost:8080/v1"}"#); + } + + #[test] + fn an_escaped_quote_does_not_end_the_string_scan() { + let cfg = parse_opencode_config(r#"{"username": "say \"hi\" // not a comment"}"#).unwrap(); + assert_eq!( + cfg.extra.get("username").map(String::as_str), + Some(r#"say "hi" // not a comment"#) + ); + } + + #[test] + fn trailing_commas_are_tolerated() { + let raw = r#"{ + "model": "openai/gpt-5", + "instructions": ["A.md", "B.md",], + }"#; + let cfg = parse_opencode_config(raw).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("gpt-5")); + } + + #[test] + fn a_comma_inside_a_string_is_not_mistaken_for_a_trailing_comma() { + let cfg = parse_opencode_config(r#"{"username": "last, first"}"#).unwrap(); + assert_eq!( + cfg.extra.get("username").map(String::as_str), + Some("last, first") + ); + } + + #[test] + fn empty_config_parses_to_an_empty_surface() { + let cfg = parse_opencode_config("{}").unwrap(); + assert!(cfg.model.is_none()); + assert!(cfg.provider.is_none()); + assert!(cfg.extensions.is_empty()); + } + + #[test] + fn unparseable_config_returns_none() { + assert!(parse_opencode_config("{{{{ not json").is_none()); + } +} diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index 372d2cfde1..1bef0da011 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -22,6 +22,7 @@ pub(crate) fn read_config_surface( "goose" => super::goose::read_config_file().map(|c| (c, true)), "claude" => super::claude::read_config_file().map(|c| (c, true)), "codex" => super::codex::read_config_file().map(|c| (c, true)), + "opencode" => super::opencode::read_config_file().map(|c| (c, true)), "buzz-agent" => super::buzz_agent::read_config_file().map(|c| (c, true)), _ => None, }) @@ -164,9 +165,17 @@ pub(crate) fn read_config_surface( }); } - let config_file_path = runtime_meta - .and_then(|m| m.config_file_path) - .map(resolve_tilde); + let config_file_path = match runtime_meta.map(|m| m.id) { + // OpenCode's config location moves with `$OPENCODE_CONFIG` and + // `$XDG_CONFIG_HOME`, so the static metadata path would name the wrong + // file on those setups. Ask the reader where it actually looked. + Some("opencode") => { + super::opencode::opencode_config_path().map(|path| path.to_string_lossy().into_owned()) + } + _ => runtime_meta + .and_then(|m| m.config_file_path) + .map(resolve_tilde), + }; let mcp_config_file_path = runtime_meta.and_then(mcp_config_file_path_for_runtime); let extensions = file_config.extensions.clone(); @@ -222,6 +231,10 @@ fn mcp_config_file_path_for_runtime(runtime: &KnownAcpRuntime) -> Option "codex" => { super::codex::codex_config_path().map(|path| path.to_string_lossy().into_owned()) } + // OpenCode declares MCP servers in the same file as everything else. + "opencode" => { + super::opencode::opencode_config_path().map(|path| path.to_string_lossy().into_owned()) + } _ => None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 4ee4ec79c3..c9e56820b0 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -175,6 +175,86 @@ fn goose_mcp_config_path_follows_path_root_override() { ); } +static OPENCODE_CONFIG_LOCK: Mutex<()> = Mutex::new(()); + +fn with_opencode_config(path: &Path, body: impl FnOnce() -> T) -> T { + let _guard = OPENCODE_CONFIG_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let prior = std::env::var_os("OPENCODE_CONFIG"); + std::env::set_var("OPENCODE_CONFIG", path); + let output = body(); + match prior { + Some(value) => std::env::set_var("OPENCODE_CONFIG", value), + None => std::env::remove_var("OPENCODE_CONFIG"), + } + output +} + +/// End-to-end wiring guard for the whole point of the OpenCode entry: the +/// harness takes no `--model` flag and reads no model env var, so unless the +/// bridge reaches its config file the model field is blank in the panel. +#[test] +fn opencode_surface_takes_its_model_from_the_config_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let config = dir.path().join("opencode.jsonc"); + std::fs::write( + &config, + r#"{ + // real OpenCode configs are JSONC with comments and trailing commas + "$schema": "https://opencode.ai/config.json", + "model": "anthropic/claude-sonnet-4-5", + "mcp": { "filesystem": { "type": "local" } }, + }"#, + ) + .expect("write config"); + + let record = test_record(); + let runtime = &KnownAcpRuntime { + id: "opencode", + label: "OpenCode", + commands: &["opencode"], + model_env_var: None, + provider_env_var: None, + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + config_file_path: Some("~/.config/opencode/opencode.json"), + config_file_format: Some("json"), + ..*test_runtime() + }; + + let surface = with_opencode_config(&config, || { + read_config_surface(&record, Some(runtime), None, None) + }); + + let model = surface.normalized.model.expect("model field"); + assert_eq!(model.value.as_deref(), Some("claude-sonnet-4-5")); + assert_eq!(model.origin, ConfigOrigin::ConfigFile); + // Nothing can write it back — no env var, no ACP model switching. + assert!(matches!(model.write_via, ConfigWriteMechanism::ReadOnly)); + + let provider = surface.normalized.provider.expect("provider field"); + assert_eq!(provider.value.as_deref(), Some("anthropic")); + + assert_eq!(surface.sources.config_file, ConfigTierStatus::Available); + assert_eq!( + surface.sources.config_file_path.as_deref().map(Path::new), + Some(config.as_path()), + "the reported path must be the file actually read, not the static default" + ); + assert_eq!( + surface + .extensions + .iter() + .map(|e| e.name.as_str()) + .collect::>(), + vec!["filesystem"] + ); +} + #[test] fn claude_surface_uses_mcp_config_path_not_settings_path() { let record = test_record(); diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 8d1b8a5013..cc69c91268 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -172,6 +172,51 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. auth_probe_args: Some(&["codex", "login", "status"]), }, + // Promoted from PRESET_HARNESSES so it can carry a `config_file_path`: + // `opencode acp` accepts no `--model` flag and reads no model env var, so + // its config file is the only tier that can tell Buzz which model it runs. + // A preset entry has nowhere to hang that, which left the config panel + // blank for every OpenCode agent. + KnownAcpRuntime { + id: "opencode", + label: "OpenCode", + commands: &["opencode"], + aliases: &[], + // Logo is bundled and keyed by id in the frontend (PRESET_LOGOS), so no + // remote avatar is fetched for this runtime. + avatar_url: "", + mcp_command: None, + mcp_hooks: false, + underlying_cli: None, + // Left empty deliberately: OpenCode is not auto-installable from Buzz, + // matching the behaviour it had as a preset. + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "https://opencode.ai/docs", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz talks to OpenCode through its CLI's ACP mode (opencode acp).", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + // No model/provider env var by design — see the note above the entry. + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + config_file_path: Some("~/.config/opencode/opencode.json"), + config_file_format: Some("json"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + // Model is required for OpenCode to run, but Buzz cannot set it — only + // the config file can. Marking it required would raise a readiness gap + // with no affordance to close it. + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + }, KnownAcpRuntime { id: "buzz-agent", label: "Buzz Agent", @@ -465,7 +510,7 @@ pub fn try_record_agent_command( fn default_agent_args(command: &str) -> Option> { match normalize_command_identity(command).as_str() { - "goose" => Some(vec!["acp".to_string()]), + "goose" | "opencode" => Some(vec!["acp".to_string()]), "codex" | "codex-acp" | "claude-agent-acp" | "claude-code-acp" | "claude-code" | "claudecode" | "buzz-agent" => Some(Vec::new()), _ => None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index 3622b21c4a..56bda27f8e 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -119,15 +119,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_hint: "Buzz talks to Grok Build through its CLI's agent stdio mode.", underlying_cli: None, }, - PresetHarness { - id: "opencode", - label: "OpenCode", - command: "opencode", - args: &["acp"], - install_instructions_url: "https://opencode.ai/docs", - install_hint: "Buzz talks to OpenCode through its CLI's ACP mode (opencode acp).", - underlying_cli: None, - }, + // OpenCode moved to KNOWN_ACP_RUNTIMES so it could carry a config_file_path + // — see the note on its entry in discovery.rs. PresetHarness { id: "kimi", label: "Kimi Code", diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..c4f15125ea 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -70,6 +70,36 @@ fn normalizes_claude_and_codex_args_to_empty() { ); } +/// OpenCode was promoted from a preset to a builtin runtime so it could carry +/// a `config_file_path` — its model lives only in its config file. Two things +/// had to survive that move, and both are silent if they break: the runtime +/// must resolve by command (or the config bridge sees no metadata at all), and +/// it must still spawn as `opencode acp` (builtins get their args from +/// `default_agent_args`, not from the preset's `args` list, so an omission here +/// would launch the bare CLI instead of the ACP server). +#[test] +fn opencode_resolves_as_a_builtin_and_keeps_its_acp_arg() { + let runtime = super::known_acp_runtime("opencode").expect("opencode should be a known runtime"); + assert_eq!(runtime.id, "opencode"); + assert_eq!( + runtime.config_file_path, + Some("~/.config/opencode/opencode.json") + ); + assert!( + runtime.model_env_var.is_none(), + "opencode has no model env var — that is why it needs the config file" + ); + + assert_eq!( + normalize_agent_args("opencode", Vec::new()), + vec!["acp".to_string()] + ); + assert_eq!( + normalize_agent_args("opencode", vec!["acp".into()]), + vec!["acp".to_string()] + ); +} + #[test] fn resolves_buzz_agent_avatar() { assert_eq!( diff --git a/desktop/src/features/agents/ui/AgentIdentityCard.tsx b/desktop/src/features/agents/ui/AgentIdentityCard.tsx index 6f13a84ae8..1caf1b7a7a 100644 --- a/desktop/src/features/agents/ui/AgentIdentityCard.tsx +++ b/desktop/src/features/agents/ui/AgentIdentityCard.tsx @@ -12,6 +12,16 @@ type AgentIdentityCardProps = { dataTestId: string; label: string; modelLabel?: string | null; + /** + * Interactive replacement for `modelLabel`, in the same slot. Takes precedence + * when both are supplied. + * + * The label row lives under `pointer-events-none` so it cannot steal clicks + * from the card's full-bleed button overlay, so anything interactive here is + * wrapped in `pointer-events-auto` and stops propagation — otherwise the + * control would either be unclickable or would also open the profile panel. + */ + modelControl?: ReactNode; onClick: () => void; /** Optional badge rendered below the label (e.g. "Restart required"). */ statusBadge?: ReactNode; @@ -25,6 +35,7 @@ export function AgentIdentityCard({ dataTestId, label, modelLabel, + modelControl, onClick, statusBadge, }: AgentIdentityCardProps) { @@ -72,7 +83,14 @@ export function AgentIdentityCard({ {label} - {modelLabel ? ( + {modelControl ? ( + event.stopPropagation()} + > + {modelControl} + + ) : modelLabel ? ( {modelLabel} diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 79d1e9a790..4e33b9efdf 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -1200,6 +1200,7 @@ export function AgentInstanceEditDialog({ parallelism={parallelism} provider={effectiveProvider} requiredEnvKeys={advancedRequiredEnvKeys} + routingPolicyPubkey={agent.pubkey} systemPrompt={systemPrompt} onAcpCommandChange={setAcpCommand} onAgentArgsChange={setAgentArgs} diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index 8412792a2c..3c23cc0ecd 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -10,6 +10,10 @@ import { import type { AgentPersona } from "@/shared/api/types"; import { BuzzAgentModelTuningFields } from "./buzzAgentModelTuningFields"; import { isBuzzAgentRuntime } from "./buzzAgentConfig"; +import { + ROUTING_POLICY_ENV_KEY, + RoutingPolicyEditor, +} from "./RoutingPolicyEditor"; export function EditAgentAdvancedFields({ acpCommand, @@ -28,6 +32,7 @@ export function EditAgentAdvancedFields({ parallelism, provider, requiredEnvKeys, + routingPolicyPubkey, systemPrompt, onAcpCommandChange, onAgentArgsChange, @@ -61,6 +66,12 @@ export function EditAgentAdvancedFields({ /** Active LLM provider id — forwarded to BuzzAgentModelTuningFields for effort filtering. */ provider?: string; requiredEnvKeys: readonly string[]; + /** + * Agent pubkey, when this form edits a live agent instance. Enables the + * routing-policy table — the policy file is keyed by pubkey, so there is + * nothing to edit on a template/definition that has no agent yet. + */ + routingPolicyPubkey?: string; systemPrompt: string; onAcpCommandChange: (value: string) => void; onAgentArgsChange: (value: string) => void; @@ -270,6 +281,24 @@ export function EditAgentAdvancedFields({ provider={provider} /> ) : null} + + {/* Per-turn model routing — instance-only (the policy file is keyed by pubkey). */} + {routingPolicyPubkey ? ( + { + const next = { ...envVars }; + if (value === "") { + delete next[key]; + } else { + next[key] = value; + } + onEnvVarsChange(next); + }} + pubkey={routingPolicyPubkey} + /> + ) : null} ); } diff --git a/desktop/src/features/agents/ui/RoutingPolicyEditor.tsx b/desktop/src/features/agents/ui/RoutingPolicyEditor.tsx new file mode 100644 index 0000000000..4ec0f4cca4 --- /dev/null +++ b/desktop/src/features/agents/ui/RoutingPolicyEditor.tsx @@ -0,0 +1,397 @@ +import * as React from "react"; +import { Plus, X } from "lucide-react"; + +import { cn } from "@/shared/lib/cn"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; +import { Switch } from "@/shared/ui/switch"; +import { + getAgentRoutingPolicy, + setAgentRoutingPolicy, + type RoutingMatchKind, + type RoutingPolicy, +} from "@/shared/api/tauri"; +import { + PERSONA_FIELD_CONTROL_CLASS, + PERSONA_FIELD_SHELL_CLASS, + PERSONA_LABEL_OPTIONAL_CLASS, +} from "./agentConfigOptions"; + +/** Env var the harness reads the policy path from (`buzz-acp` routing.rs). */ +export const ROUTING_POLICY_ENV_KEY = "BUZZ_ROUTING_POLICY"; + +/** A rule row. `id` is local only — it keeps React keys stable while typing. */ +type RuleRow = { + id: string; + name: string; + matchKind: RoutingMatchKind; + /** Comma-separated in the UI; split on save. */ + phrases: string; + model: string; +}; + +function newRuleId(): string { + return crypto.randomUUID(); +} + +function toRows(policy: RoutingPolicy | null): RuleRow[] { + return (policy?.rules ?? []).map((rule) => ({ + id: newRuleId(), + name: rule.name ?? "", + matchKind: rule.matchKind, + phrases: rule.any.join(", "), + model: rule.model, + })); +} + +function splitPhrases(raw: string): string[] { + return raw + .split(",") + .map((phrase) => phrase.trim()) + .filter((phrase) => phrase.length > 0); +} + +/** + * Per-turn model routing for one agent. + * + * Writes the JSON policy file that `buzz-acp` reads, then points the agent's + * `BUZZ_ROUTING_POLICY` env var at it via `onEnvVarChange`. The env change is + * staged in the dialog's env map and lands with the dialog's own save — the + * file write happens immediately, because the file is not part of the agent + * record and has nothing to wait for. + * + * Routing is opt-in and fails open on the harness side: an unreadable or + * disabled policy means turns run on the agent's configured model, exactly as + * they did before. The UI mirrors that — turning routing off deletes the file + * and drops the env var rather than leaving a dormant one behind. + */ +export function RoutingPolicyEditor({ + disabled, + envValue, + pubkey, + onEnvVarChange, +}: { + disabled: boolean; + /** Current `BUZZ_ROUTING_POLICY` value in the dialog's env map, if any. */ + envValue: string | undefined; + pubkey: string; + onEnvVarChange: (key: string, value: string) => void; +}) { + const [loaded, setLoaded] = React.useState(false); + const [enabled, setEnabled] = React.useState(false); + const [rows, setRows] = React.useState([]); + const [defaultModel, setDefaultModel] = React.useState(""); + const [path, setPath] = React.useState(null); + const [classifier, setClassifier] = React.useState(undefined); + const [saving, setSaving] = React.useState(false); + const [error, setError] = React.useState(null); + const [savedAt, setSavedAt] = React.useState(null); + + React.useEffect(() => { + let cancelled = false; + void getAgentRoutingPolicy(pubkey) + .then((file) => { + if (cancelled) return; + setPath(file.path); + setEnabled(file.policy?.enabled ?? false); + setRows(toRows(file.policy)); + setDefaultModel(file.policy?.defaultModel ?? ""); + setClassifier(file.policy?.classifier); + setLoaded(true); + }) + .catch((loadError: unknown) => { + if (cancelled) return; + setError( + loadError instanceof Error ? loadError.message : String(loadError), + ); + setLoaded(true); + }); + return () => { + cancelled = true; + }; + }, [pubkey]); + + const updateRow = (id: string, patch: Partial) => { + setRows((current) => + current.map((row) => (row.id === id ? { ...row, ...patch } : row)), + ); + setSavedAt(null); + }; + + const handleSave = async () => { + setSaving(true); + setError(null); + try { + if (!enabled && rows.length === 0) { + // Nothing to route with. Delete the file and drop the env var so the + // agent is left in exactly the state it had before routing was touched. + const file = await setAgentRoutingPolicy(pubkey, null); + setPath(file.path); + onEnvVarChange(ROUTING_POLICY_ENV_KEY, ""); + setSavedAt(Date.now()); + return; + } + + const policy: RoutingPolicy = { + enabled, + rules: rows.map((row) => ({ + name: row.name.trim() ? row.name.trim() : null, + matchKind: row.matchKind, + any: splitPhrases(row.phrases), + model: row.model, + })), + defaultModel: defaultModel.trim() ? defaultModel.trim() : null, + classifier, + }; + const file = await setAgentRoutingPolicy(pubkey, policy); + setPath(file.path); + onEnvVarChange(ROUTING_POLICY_ENV_KEY, file.path); + setSavedAt(Date.now()); + } catch (saveError: unknown) { + setError( + saveError instanceof Error ? saveError.message : String(saveError), + ); + } finally { + setSaving(false); + } + }; + + const envPointsAtPolicy = !!envValue && !!path && envValue === path; + + return ( +
+
+
+

+ Model routing + optional +

+

+ Send a turn to a different model based on what it says. Rules are + checked in order; the first match wins. +

+
+ { + setEnabled(next); + setSavedAt(null); + }} + /> +
+ + {!loaded ? ( +

Loading routing policy…

+ ) : ( + <> +
+ {rows.length === 0 ? ( +

+ No rules yet. Without rules, every turn falls back to the + default model below — or to the agent's own model if that is + blank too. +

+ ) : null} + + {rows.map((row, index) => ( +
+
+ + updateRow(row.id, { name: event.target.value }) + } + placeholder="name" + value={row.name} + /> +
+ +
+ + updateRow(row.id, { phrases: event.target.value }) + } + placeholder="migration, schema" + value={row.phrases} + /> +
+
+ + updateRow(row.id, { model: event.target.value }) + } + placeholder="model id" + value={row.model} + /> +
+ +
+ ))} + + +
+ +
+ +
+ { + setDefaultModel(event.target.value); + setSavedAt(null); + }} + placeholder="Leave blank to use the agent's own model" + value={defaultModel} + /> +
+
+ + {classifier ? ( +

+ This policy also has a local classifier configured in the file. + Buzz keeps it as-is — edit it in{" "} + {path}. +

+ ) : null} + +
+ + {savedAt !== null && !error ? ( +

+ Saved. Save the agent to apply — the harness reads the policy at + start-up. +

+ ) : null} +
+ + {error ? ( +

+ {error} +

+ ) : null} + + {enabled && !envPointsAtPolicy ? ( +

+ Routing is not active yet: {ROUTING_POLICY_ENV_KEY} does not point + at this policy. Save the routing policy to set it. +

+ ) : null} + + )} +
+ ); +} diff --git a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx index 19a5ef1171..cbf3e78a54 100644 --- a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx +++ b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx @@ -25,6 +25,7 @@ import { IdentityCardSkeleton } from "@/shared/ui/identity-card-skeleton"; import { AgentIdentityCard } from "./AgentIdentityCard"; import { AgentRuntimeAvatarControl } from "./AgentRuntimeAvatarControl"; import { CreateIdentityCard } from "./CreateIdentityCard"; +import { ModelPicker } from "./ModelPicker"; import { PersonaActionsMenu } from "./PersonaActionsMenu"; import { buildUnifiedGroups, pickProfileAgent } from "./unifiedAgentGroups"; @@ -327,6 +328,12 @@ function AgentPersonaCard({ dataTestId={`persona-agent-row-${persona.id}`} label={title} modelLabel={modelLabel} + // A persona without a managed agent has nothing to switch, so it keeps the + // static label. With an agent, the same slot becomes a live picker: this is + // the only UI path to `switch_model` (kind:24200) — ModelPicker is its sole + // caller, and until now nothing rendered ModelPicker, so the backend could + // switch models that no screen could ask for. + modelControl={agent ? : undefined} onClick={() => { if (agent) { onOpenAgentProfile( @@ -406,6 +413,9 @@ function StandaloneAgentCard({ personaModel: null, defaultModel, })} + // Unknown agents are managed agents with no persona, so they always have a + // ManagedAgent to switch — unlike AgentPersonaCard, there is no undefined case. + modelControl={} onClick={() => { onOpenAgentProfile( agent.pubkey, diff --git a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx index 5b247c31f7..9ccf2c0ddb 100644 --- a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx +++ b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx @@ -14,8 +14,9 @@ const RUNTIME_LOGOS: Record = { claude: claudeLogoUrl, }; -// Public-path logos for bundled presets. Served from /harness-logos/ at runtime. -// Keys match the preset `id` values emitted by the backend PRESET_HARNESSES. +// Public-path logos for bundled harnesses. Served from /harness-logos/ at runtime. +// Keys match backend runtime ids (PRESET_HARNESSES, plus KNOWN_ACP_RUNTIMES +// entries such as `opencode` that ship no remote avatar). export const PRESET_LOGOS: Record = { devin: "/harness-logos/devin.svg", omp: "/harness-logos/omp.svg", diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 69e2e455ec..ffcce40391 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -1013,6 +1013,124 @@ export async function putAgentSessionConfig( return invokeTauri("put_agent_session_config", { pubkey, payload }); } +// ── Per-turn model routing ──────────────────────────────────────────────────── +// +// The harness reads its policy from the file named by `BUZZ_ROUTING_POLICY` +// (crates/buzz-acp/src/routing.rs). These commands own the file; pointing the +// env var at the returned `path` is the caller's job, because the edit dialog +// replaces the whole env map on submit and would overwrite a backend patch. + +export type RoutingMatchKind = "contains" | "contains_all"; + +export type RoutingRule = { + name?: string | null; + matchKind: RoutingMatchKind; + /** Needles to look for. An empty list is rejected at save time. */ + any: string[]; + model: string; +}; + +export type RoutingPolicy = { + enabled: boolean; + rules: RoutingRule[]; + defaultModel?: string | null; + /** + * The optional local-Ollama classifier stage. Buzz has no UI for it, so it is + * carried through opaquely — saving from the rules table must not silently + * delete a classifier the user wrote into the file by hand. + */ + classifier?: unknown; +}; + +export type AgentRoutingPolicyFile = { + /** Where the policy lives — set `BUZZ_ROUTING_POLICY` to this. */ + path: string; + /** `null` when nothing has been written yet. */ + policy: RoutingPolicy | null; +}; + +/** + * Wire shape. The Rust side mirrors `buzz_acp::routing::Policy` verbatim, which + * is snake_case, so these fields are NOT camelCase like the rest of our API — + * the file has to be readable by the harness, not by us. + */ +type RawRoutingPolicy = { + enabled: boolean; + rules: { + name?: string | null; + match_kind: RoutingMatchKind; + any: string[]; + model: string; + }[]; + default_model?: string | null; + classifier?: unknown; +}; + +type RawAgentRoutingPolicyFile = { + path: string; + policy: RawRoutingPolicy | null; +}; + +function fromRawRoutingPolicy(raw: RawRoutingPolicy): RoutingPolicy { + return { + enabled: raw.enabled, + rules: (raw.rules ?? []).map((rule) => ({ + name: rule.name ?? null, + matchKind: rule.match_kind ?? "contains", + any: rule.any ?? [], + model: rule.model, + })), + defaultModel: raw.default_model ?? null, + }; +} + +function toRawRoutingPolicy(policy: RoutingPolicy): RawRoutingPolicy { + return { + enabled: policy.enabled, + rules: policy.rules.map((rule) => ({ + name: rule.name?.trim() ? rule.name.trim() : null, + match_kind: rule.matchKind, + any: rule.any, + model: rule.model, + })), + default_model: policy.defaultModel?.trim() + ? policy.defaultModel.trim() + : null, + }; +} + +function fromRawRoutingPolicyFile( + raw: RawAgentRoutingPolicyFile, +): AgentRoutingPolicyFile { + return { + path: raw.path, + policy: raw.policy ? fromRawRoutingPolicy(raw.policy) : null, + }; +} + +export async function getAgentRoutingPolicy( + pubkey: string, +): Promise { + return fromRawRoutingPolicyFile( + await invokeTauri("get_agent_routing_policy", { + pubkey, + }), + ); +} + +/** Pass `null` to delete the policy file. */ +export async function setAgentRoutingPolicy( + pubkey: string, + policy: RoutingPolicy | null, +): Promise { + return fromRawRoutingPolicyFile( + await invokeTauri("set_agent_routing_policy", { + pubkey, + policy: policy ? toRawRoutingPolicy(policy) : null, + }), + ); +} + /** File-layer config for a runtime (e.g. `~/.config/goose/config.yaml`). */ export type RuntimeFileConfigSubset = { /** Provider set in the harness config file. */ diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 355ccea9fc..17c4a1951e 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -502,6 +502,25 @@ type E2eConfig = { agentDefaultModel?: string | null; selectedModel?: string | null; }; + /** + * Override for `get_agent_models` — the catalog the ModelPicker reads. + * + * Mirrors `discoverAgentModels`. Without it `get_agent_models` always + * returns an empty list with `supportsSwitching: false`, so the picker can + * only ever be exercised in its "runtime default" empty state and the + * populated list (and therefore the switch path) is untestable in the + * browser harness. + */ + agentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; /** * When set, `discover_agent_models` throws with this message instead of * returning a catalog. @@ -7334,6 +7353,12 @@ let mockGlobalAgentConfig: { model: string | null; preferred_runtime?: string | null; } | null = null; +/** + * Routing policies written through `set_agent_routing_policy`, keyed by pubkey. + * Opaque on purpose: the shape is the harness's, and the mock only has to hand + * back what it was given. + */ +const mockRoutingPolicies = new Map(); // Per-page get_nsec call counter for sequenced error testing. let nsecCallCount = 0; @@ -9611,6 +9636,7 @@ export function maybeInstallE2eTauriMocks() { mockGlobalAgentConfig = config.mock?.globalAgentConfig ? { ...config.mock.globalAgentConfig } : null; + mockRoutingPolicies.clear(); resetMockRelayMembers(config); resetMockRelayAgents(config); resetMockManagedAgents(config); @@ -11447,7 +11473,22 @@ export function maybeInstallE2eTauriMocks() { return handleGetManagedAgentLog( payload as Parameters[0], ); - case "get_agent_models": + case "get_agent_models": { + const modelsOverride = activeConfig?.mock?.agentModels; + if (modelsOverride) { + return { + agentName: "mock-agent", + agentVersion: "0.0.0", + models: modelsOverride.models.map((model) => ({ + id: model.id, + name: model.name, + description: model.description ?? null, + })), + agentDefaultModel: modelsOverride.agentDefaultModel ?? null, + selectedModel: modelsOverride.selectedModel ?? null, + supportsSwitching: modelsOverride.supportsSwitching, + }; + } return { agentName: "mock-agent", agentVersion: "0.0.0", @@ -11456,6 +11497,7 @@ export function maybeInstallE2eTauriMocks() { selectedModel: null, supportsSwitching: false, }; + } case "discover_agent_models": { const discoverError = activeConfig?.mock?.discoverAgentModelsError; if (discoverError) { @@ -11564,6 +11606,32 @@ export function maybeInstallE2eTauriMocks() { const configArgs = payload as { pubkey: string }; return buildMockConfigSurface(configArgs.pubkey); } + // Per-turn model routing. Kept in memory so the edit dialog's routing + // table round-trips inside a test the way it does against the real + // command — without a mock the editor's mount-time read would throw into + // its own error state and every Advanced-panel test would see it. + case "get_agent_routing_policy": { + const { pubkey } = payload as { pubkey: string }; + return { + path: `/mock/agents/routing/${pubkey}.json`, + policy: mockRoutingPolicies.get(pubkey) ?? null, + }; + } + case "set_agent_routing_policy": { + const { pubkey, policy } = payload as { + pubkey: string; + policy: unknown; + }; + if (policy === null || policy === undefined) { + mockRoutingPolicies.delete(pubkey); + } else { + mockRoutingPolicies.set(pubkey, policy); + } + return { + path: `/mock/agents/routing/${pubkey}.json`, + policy: mockRoutingPolicies.get(pubkey) ?? null, + }; + } case "get_runtime_file_config": { const runtimeId = (payload as { runtimeId?: string } | null | undefined) ?.runtimeId; diff --git a/desktop/tests/e2e/agent-model-picker.spec.ts b/desktop/tests/e2e/agent-model-picker.spec.ts new file mode 100644 index 0000000000..c51a828e7c --- /dev/null +++ b/desktop/tests/e2e/agent-model-picker.spec.ts @@ -0,0 +1,138 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +// A standalone agent: no persona, not running. That combination forces the +// ModelPicker down its non-live branch, where a pick persists the default via +// `update_managed_agent` instead of publishing a kind-24200 `switch_model` +// control frame (which the browser harness has no relay to carry). +const AGENT = TEST_IDENTITIES.tyler; +const AGENT_NAME = "Standalone Helper"; + +const CATALOG = { + models: [ + { id: "openrouter/auto", name: "Auto (OpenRouter)" }, + { id: "anthropic/claude-sonnet-4.5", name: "Claude Sonnet 4.5" }, + // A nameless entry proves the item falls back to the raw model id. + { id: "openai/gpt-5", name: null }, + ], + supportsSwitching: true, + agentDefaultModel: "openrouter/auto", +}; + +async function openAgentsView(page: Page) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + await expect(page.getByTestId("agents-library-personas")).toBeVisible({ + timeout: 10_000, + }); +} + +function commandCount(page: Page, command: string) { + return page.evaluate( + (name) => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === name, + ).length, + command, + ); +} + +test("the picker loads its catalog on first open and persists the pick", async ({ + page, +}) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT.pubkey, + name: AGENT_NAME, + personaId: null, + status: "stopped", + }, + ], + agentModels: CATALOG, + }); + + await openAgentsView(page); + + const card = page.getByTestId(`managed-agent-${AGENT.pubkey}`); + await expect(card).toBeVisible(); + + // With no persisted model and no catalog yet, the trigger reads "Auto" and + // nothing has been fetched — the request is deferred to the first open. + const trigger = card.getByRole("button", { name: "Auto", exact: true }); + await expect(trigger).toBeVisible(); + expect(await commandCount(page, "get_agent_models")).toBe(0); + + await trigger.click(); + + await expect + .poll(() => commandCount(page, "get_agent_models")) + .toBeGreaterThan(0); + + // The seeded catalog renders, including the id fallback for a nameless model. + await expect( + page.getByRole("menuitemradio", { name: "Auto (OpenRouter)" }), + ).toBeVisible(); + await expect( + page.getByRole("menuitemradio", { name: "openai/gpt-5" }), + ).toBeVisible(); + const sonnet = page.getByRole("menuitemradio", { name: "Claude Sonnet 4.5" }); + await expect(sonnet).toBeVisible(); + + const commandsBeforePick = await page.evaluate( + () => window.__BUZZ_E2E_COMMAND_LOG__?.length ?? 0, + ); + await sonnet.click(); + + // The non-live path persists the chosen model as the agent's default. + await expect + .poll(async () => + page.evaluate((start) => { + const commands = window.__BUZZ_E2E_COMMAND_LOG__ ?? []; + return commands + .slice(start) + .some( + (entry) => + entry.command === "update_managed_agent" && + (entry.payload as { input?: { model?: string | null } })?.input + ?.model === "anthropic/claude-sonnet-4.5", + ); + }, commandsBeforePick), + ) + .toBe(true); + + // ...and the refetched agent drives the trigger label. + await expect( + card.getByRole("button", { + name: "anthropic/claude-sonnet-4.5", + exact: true, + }), + ).toBeVisible(); +}); + +test("a runtime that cannot switch models explains itself instead of listing", async ({ + page, +}) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT.pubkey, + name: AGENT_NAME, + personaId: null, + status: "stopped", + }, + ], + agentModels: { ...CATALOG, supportsSwitching: false }, + }); + + await openAgentsView(page); + + const card = page.getByTestId(`managed-agent-${AGENT.pubkey}`); + await card.getByRole("button", { name: "Auto", exact: true }).click(); + + await expect( + page.getByText("This agent uses the runtime's default model."), + ).toBeVisible(); + await expect(page.getByRole("menuitemradio")).toHaveCount(0); +}); diff --git a/desktop/tests/e2e/agent-routing-policy.spec.ts b/desktop/tests/e2e/agent-routing-policy.spec.ts new file mode 100644 index 0000000000..d037cef1b4 --- /dev/null +++ b/desktop/tests/e2e/agent-routing-policy.spec.ts @@ -0,0 +1,165 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +// Per-turn model routing (`crates/buzz-acp/src/routing.rs`) is opt-in through +// the `BUZZ_ROUTING_POLICY` env var, which must name a policy file. This spec +// pins the UI half of that contract: the routing table writes the policy AND +// points the env var at the returned path. Either half alone does nothing — +// a policy file nothing references never gets read, and an env var pointing at +// a missing file makes the harness fail open and route nothing. + +const AGENT_PUBKEY = TEST_IDENTITIES.tyler.pubkey; +const AGENT_NAME = "Tyler Agent"; + +async function openAdvanced(page: import("@playwright/test").Page) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + + const agentButton = page.getByRole("button", { + name: `${AGENT_NAME} agent profile`, + }); + await expect(agentButton).toBeVisible({ timeout: 10_000 }); + await agentButton.click(); + + await expect(page.getByTestId("user-profile-panel")).toBeVisible({ + timeout: 10_000, + }); + await page.getByTestId("user-profile-edit-agent").click(); + await expect(page.getByTestId("edit-agent-dialog")).toBeVisible({ + timeout: 10_000, + }); + await page.getByRole("button", { name: "Advanced" }).click(); + await expect(page.getByTestId("routing-policy-editor")).toBeVisible({ + timeout: 10_000, + }); +} + +test.describe("agent routing policy", () => { + test("saving a rule writes the policy and points BUZZ_ROUTING_POLICY at it", async ({ + page, + }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped", + channelNames: ["agents"], + }, + ], + }); + + await openAdvanced(page); + + // Until routing is switched on and saved, the agent carries no policy var. + await expect(page.getByTestId("routing-policy-save")).toBeVisible(); + + await page.getByTestId("routing-policy-enabled").click(); + await page.getByTestId("routing-rule-add").click(); + + await page.getByTestId("routing-rule-name").fill("db"); + await page + .getByTestId("routing-rule-match-kind") + .selectOption("contains_all"); + await page.getByTestId("routing-rule-phrases").fill("migration, schema"); + await page.getByTestId("routing-rule-model").fill("codex-model"); + await page.getByTestId("routing-default-model").fill("fallback-model"); + + const commandsBefore = await page.evaluate( + () => window.__BUZZ_E2E_COMMAND_LOG__?.length ?? 0, + ); + await page.getByTestId("routing-policy-save").click(); + + // The policy reaching the backend is the load-bearing half. Assert the + // snake_case wire shape, not our camelCase view model — the harness parses + // this document, so a rename here silently disables routing. + await expect + .poll(async () => + page.evaluate( + (start) => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []) + .slice(start) + .find((entry) => entry.command === "set_agent_routing_policy") + ?.payload ?? null, + commandsBefore, + ), + ) + .toEqual({ + pubkey: AGENT_PUBKEY, + policy: { + enabled: true, + rules: [ + { + name: "db", + match_kind: "contains_all", + any: ["migration", "schema"], + model: "codex-model", + }, + ], + default_model: "fallback-model", + }, + }); + + await expect(page.getByTestId("routing-policy-error")).toHaveCount(0); + + // ...and the other half: the env var now names the saved file. Read the + // live input values — React controlled inputs do not mirror `value` into a + // DOM attribute, so an attribute selector would pass vacuously. + await expect + .poll(async () => { + const keys = await page + .getByTestId("env-vars-key") + .evaluateAll((nodes) => + nodes.map((node) => (node as HTMLInputElement).value), + ); + const values = await page + .getByTestId("env-vars-value") + .evaluateAll((nodes) => + nodes.map((node) => (node as HTMLInputElement).value), + ); + const index = keys.indexOf("BUZZ_ROUTING_POLICY"); + return index === -1 ? null : values[index]; + }) + .toBe(`/mock/agents/routing/${AGENT_PUBKEY}.json`); + }); + + test("a saved policy is read back when the dialog is reopened", async ({ + page, + }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped", + channelNames: ["agents"], + }, + ], + }); + + await openAdvanced(page); + await page.getByTestId("routing-policy-enabled").click(); + await page.getByTestId("routing-rule-add").click(); + await page.getByTestId("routing-rule-name").fill("ui"); + await page.getByTestId("routing-rule-phrases").fill("button"); + await page.getByTestId("routing-rule-model").fill("ui-model"); + await page.getByTestId("routing-policy-save").click(); + await expect(page.getByTestId("routing-policy-error")).toHaveCount(0); + + // Close and reopen: the table is hydrated from the stored policy, not from + // component state that happened to survive. + await page.keyboard.press("Escape"); + await expect(page.getByTestId("edit-agent-dialog")).not.toBeVisible(); + await page.getByTestId("user-profile-edit-agent").click(); + await page.getByRole("button", { name: "Advanced" }).click(); + + await expect(page.getByTestId("routing-rule-name")).toHaveValue("ui"); + await expect(page.getByTestId("routing-rule-phrases")).toHaveValue( + "button", + ); + await expect(page.getByTestId("routing-rule-model")).toHaveValue( + "ui-model", + ); + }); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 8a9ab2be11..15e408b24f 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -524,6 +524,22 @@ type MockBridgeOptions = { * returning a catalog. Exercises the discovery-failure UI path. */ discoverAgentModelsError?: string; + /** + * Override the `get_agent_models` mock response — the catalog the + * ModelPicker reads on first open. Without it the bridge always returns an + * empty list with `supportsSwitching: false`, so the populated menu (and the + * model-switch path behind it) is unreachable. + */ + agentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; }; type BridgeOptions = {