diff --git a/.gitignore b/.gitignore index f26e74136c0..de8d3f48990 100644 --- a/.gitignore +++ b/.gitignore @@ -67,8 +67,14 @@ identity.key # Claude Code worktrees .claude/worktrees/ +# Claude Code runtime state (machine-local locks, not durable) +.claude/scheduled_tasks.lock + # mesh-llm build cache .cache/ # 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 0d87bca028c..4279eb979aa 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -2750,6 +2750,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/config.rs b/crates/buzz-acp/src/config.rs index 5244ef5537a..32f5fe991e0 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -728,6 +728,22 @@ pub(crate) fn normalize_agent_command_identity(command: &str) -> String { .collect() } +/// Canonical harness *class* for a spawn command — the coarse bucket a harness +/// dispatcher compares against, stable across binary-name variants. +/// UNWIRED (decline-gate caller reverted 2026-08-08); kept for the dispatcher. +#[allow(dead_code)] +pub(crate) fn harness_class(command: &str) -> String { + let id = normalize_agent_command_identity(command); + match id.as_str() { + "codex" | "codex-acp" => "codex".to_string(), + "opencode" | "opencode-acp" => "opencode".to_string(), + "claude-agent-acp" | "claude-code-acp" | "claude-code" | "claudecode" => { + "claude".to_string() + } + _ => id, // goose, buzz-agent, hermes, custom -> class == identity + } +} + fn default_agent_args(command: &str) -> Option> { match normalize_agent_command_identity(command).as_str() { "goose" => Some(vec!["acp".to_string()]), @@ -1675,6 +1691,14 @@ mod tests { assert_eq!(normalize_agent_command_identity("///"), ""); } + #[test] + fn harness_class_folds_variants() { + assert_eq!(harness_class("codex-acp"), "codex"); + assert_eq!(harness_class("/usr/local/bin/opencode"), "opencode"); + assert_eq!(harness_class("claude-code-acp"), "claude"); + assert_eq!(harness_class("goose"), "goose"); + } + #[test] fn default_agent_env_recognizes_hermes_identities() { for command in [ diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 68b2df4d607..0e973a47ba9 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 6e3a9b24fa5..14960a51471 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -1780,6 +1780,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 00000000000..8a0a0448d88 --- /dev/null +++ b/crates/buzz-acp/src/routing.rs @@ -0,0 +1,717 @@ +//! 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, Default)] +#[serde(rename_all = "snake_case")] +pub enum MatchKind { + /// Any of `any` appears in the prompt, case-insensitively. + #[default] + Contains, + /// Every one of `any` appears in the prompt, case-insensitively. + ContainsAll, +} + +/// 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`]. + /// + /// UNWIRED. The ingress decline-gate that consumed this was reverted after + /// adversarial review (2026-08-08): a per-class *decline* fails closed + /// system-wide when no sibling of the target class is subscribed, and it + /// duplicates the existing per-pubkey `require_mention` selector in the + /// targeted case. Kept as a staged primitive for a real dispatcher (one + /// authority that *assigns* a turn and guarantees delivery/fallback) — which + /// must NOT reuse these decline semantics. Do not rewire as-is. + /// + /// [`decide_static`]: Policy::decide_static + #[allow(dead_code)] + 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. + /// + /// UNWIRED — see [`Policy::decide_harness`]; the ingress caller was reverted. + #[allow(dead_code)] + 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 69714b145c5..ff7d9bb75fa 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. @@ -137,6 +137,138 @@ async fn discover_databricks_models_with_token_source( } } +// --------------------------------------------------------------------------- +// 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 // --------------------------------------------------------------------------- @@ -480,6 +612,61 @@ mod tests { assert_eq!(requests.load(Ordering::SeqCst), 2); } + #[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 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 98fa99ca5bf..146619eb41c 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -12,7 +12,7 @@ pub mod model_capabilities; pub mod types; mod wire; -pub use catalog::{discover_databricks_models, ModelEntry}; +pub use catalog::{discover_databricks_models, discover_openrouter_models, ModelEntry}; pub use config::Provider; pub use types::AgentError; @@ -351,6 +351,24 @@ fn configured_model_fallback(model: &str) -> Vec { vec![ModelEntry { id: model, name }] } +/// Return the configured OpenRouter model as a one-entry catalog for this response. +/// +/// Deliberately separate from [`configured_model_fallback`]: that one resolves its +/// label against the Databricks manifest, which would be the wrong registry for an +/// OpenRouter id. OpenRouter ids are already human-readable (`vendor/model`), so the +/// configured value serves as both id and label. +/// +/// Like the Databricks fallback, this is never written to `models_cache` — a failed +/// discovery must be retried by the next session rather than pinning degraded state +/// for the process lifetime. +fn configured_openrouter_fallback(model: &str) -> Vec { + let model = model.trim().to_string(); + vec![ModelEntry { + id: model.clone(), + name: model, + }] +} + async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSender) { let p: SessionNewParams = match decode(params, "session/new") { Ok(p) => p, @@ -452,6 +470,36 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen .map(|m| json!({ "modelId": m.id, "name": m.name })) .collect() } + // OpenRouter authenticates with a required static `OPENROUTER_API_KEY` + // (`Config::from_env` fails without one) and has no interactive OAuth + // path, so an auth failure here can never be recovered by a later + // `session/prompt`. That makes this the static-credential case the + // Databricks arm rejects on, with no OAuth branch to fall back to. + Provider::OpenRouter => { + let models = match resolve_models_catalog( + &app.models_cache, + discover_openrouter_models(&app.cfg), + ) + .await + { + Ok(models) => models, + Err(error @ AgentError::LlmAuth(_)) => { + return reject(wire_tx, id, error.json_rpc_code(), &error.to_string()) + .await; + } + Err(error) => { + tracing::warn!( + error = %error, + "OpenRouter model catalog unavailable; using configured model" + ); + configured_openrouter_fallback(&app.cfg.model) + } + }; + models + .iter() + .map(|m| json!({ "modelId": m.id, "name": m.name })) + .collect() + } _ => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], } }; @@ -1012,6 +1060,20 @@ mod tests { assert_eq!(cache.get(), Some(&discovered)); } + /// Discovery failure must still leave the picker able to represent the model + /// the agent is actually running. Moved here from `catalog.rs` when the + /// provider-aware `discovery_failure_fallback` was removed upstream. + #[test] + fn openrouter_fallback_is_the_configured_model() { + assert_eq!( + crate::configured_openrouter_fallback(" openai/gpt-5.6-luna "), + vec![ModelEntry { + id: "openai/gpt-5.6-luna".into(), + name: "openai/gpt-5.6-luna".into(), + }] + ); + } + #[test] fn configured_model_fallback_is_trimmed_and_singular() { // Unknown id: trimmed, and the raw id passes through as the name. diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 7d06c4da91b..2318f8feab2 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -137,6 +137,8 @@ export default defineConfig({ "**/agent-lifecycle-feedback.spec.ts", "**/agent-access-warning.spec.ts", "**/edit-agent-run-on.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 00000000000..62b691846ca --- /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 761bee9cd32..bb2f2121ac9 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -8,6 +8,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; @@ -76,6 +77,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 31d4ff37133..09f98d0f84f 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -50,13 +50,7 @@ use builderlab::*; #[doc(hidden)] pub use commands::print_agent_access_owner_only_probe_if_requested; use commands::*; -use deep_link::{ - acknowledge_pending_community_deep_link, acknowledge_pending_entity_deep_link, - acknowledge_pending_navigation_deep_link, clear_pending_navigation_deep_links, - handle_deep_link_url, take_pending_community_deep_link, take_pending_entity_deep_link, - take_pending_navigation_deep_link, PendingCommunityDeepLinks, PendingEntityDeepLinks, - PendingNavigationDeepLinks, -}; +use deep_link::*; use huddle::{ add_agent_to_huddle, audio_output::{get_audio_output_device, list_audio_output_devices, set_audio_output_device}, @@ -785,6 +779,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, persist_agent_effort_level, get_global_agent_config, set_global_agent_config, 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 f8b045fc72f..e13830abb3d 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 00000000000..57d1a63d9a7 --- /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 93827635e90..bb606b15f3e 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -29,6 +29,7 @@ pub(crate) fn read_config_surface( "goose" => super::goose::read_config_file().map(|c| (c, true)), "claude" => super::claude::read_config_file(claude_config_dir).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, }) @@ -227,6 +228,13 @@ fn config_file_path_for_runtime( return Some(dir.join("settings.json").to_string_lossy().into_owned()); } } + // 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. + if runtime.id == "opencode" { + return super::opencode::opencode_config_path() + .map(|path| path.to_string_lossy().into_owned()); + } runtime.config_file_path.map(resolve_tilde) } @@ -255,6 +263,10 @@ fn mcp_config_file_path_for_runtime( "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 36b6022b53b..ba112d445b3 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 @@ -956,3 +956,6 @@ fn numeric_max_tokens_inherits_from_global_env() { // ── Extended tests (split file to respect line-count ratchet) ──────────────── #[path = "reader_tests_ext.rs"] mod ext; + +#[path = "reader_tests_opencode.rs"] +mod opencode_tests; diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_opencode.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_opencode.rs new file mode 100644 index 00000000000..d06b59f7fa2 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_opencode.rs @@ -0,0 +1,87 @@ +//! OpenCode config-bridge tests — split out of `reader_tests.rs` to keep it +//! under the 1000-line file-size ratchet. +//! +//! Included as `mod opencode_tests` inside `reader_tests.rs`, so `use super::*` +//! gives access to all helpers and types from that module. + +use super::*; + +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, &no_tiers(), 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"] + ); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index bc0e3a6cdae..5f3d634ddb7 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -13,6 +13,10 @@ mod presets; mod runtime_metadata; #[macro_use] mod windows_install; +// Declared after windows_install so the macro_use macros its entries call are +// already in scope. +mod known_runtimes; +use known_runtimes::KNOWN_ACP_RUNTIMES; pub(crate) use presets::{ canonical_harness_command, command_for_runtime_id, preset_harness_definitions, preset_harness_ids, @@ -75,144 +79,6 @@ fn common_binary_paths() -> &'static [PathBuf] { }) } -const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ - KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: GOOSE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("goose"), - cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], - // Goose's stable release currently publishes only the Unix installer; - // its official Windows instructions intentionally point at this main-branch script. - cli_install_commands_windows: &[windows_install_command!("goose", "https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1", "$env:CONFIGURE='false'; ")], - adapter_install_commands: &[], - cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", - adapter_install_instructions_url: "", - cli_install_hint: "Buzz talks to Goose through the Goose CLI.", - adapter_install_hint: "", - skill_dir: Some(".goose/skills"), - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[("GOOSE_MODE", "auto")], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - max_rounds_env_var: None, - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, - KnownAcpRuntime { - id: "claude", - label: "Claude Code", - commands: &["claude-agent-acp", "claude-code-acp"], - aliases: &["claude-code", "claudecode"], - avatar_url: CLAUDE_CODE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("claude"), - cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], - cli_install_commands_windows: &[windows_install_command!("claude", "https://claude.ai/install.ps1")], - adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], - cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", - cli_install_hint: "Buzz talks to Claude Code through the Claude Code CLI.", - adapter_install_hint: "Buzz talks to the Claude Code CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/claude-agent-acp.", - skill_dir: Some(".claude/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: true, - default_env: &[], - config_file_path: Some("~/.claude/settings.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, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run the Claude CLI to complete authentication."), - auth_probe_args: Some(&["claude", "auth", "status"]), - }, - KnownAcpRuntime { - id: "codex", - label: "Codex", - commands: &["codex-acp"], - aliases: &[], - avatar_url: CODEX_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: false, - underlying_cli: Some("codex"), - cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], - cli_install_commands_windows: &[windows_install_command!("codex", "https://chatgpt.com/codex/install.ps1")], - adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], - cli_install_instructions_url: "https://developers.openai.com/codex/cli/", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", - cli_install_hint: "Buzz talks to Codex through the Codex CLI.", - adapter_install_hint: "Buzz talks to the Codex CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/codex-acp.", - skill_dir: Some(".codex/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.codex/config.toml"), - config_file_format: Some("toml"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run `codex login` to authenticate."), - // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. - auth_probe_args: Some(&["codex", "login", "status"]), - }, - KnownAcpRuntime { - id: "buzz-agent", - label: "Buzz Agent", - commands: &["buzz-agent"], - aliases: &[], - avatar_url: BUZZ_AGENT_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: true, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "https://github.com/block/buzz", - adapter_install_instructions_url: "https://github.com/block/buzz", - cli_install_hint: "Ships with the Buzz desktop app.", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: true, - model_env_var: Some("BUZZ_AGENT_MODEL"), - provider_env_var: Some("BUZZ_AGENT_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: None, - config_file_format: None, - supports_acp_native_config: false, - thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), - max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), - context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), - max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, -]; - /// Skill discovery directories declared by known runtimes. pub(crate) fn known_skill_dirs() -> impl Iterator { KNOWN_ACP_RUNTIMES.iter().filter_map(|p| p.skill_dir) @@ -442,7 +308,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, @@ -1623,3 +1489,6 @@ pub fn managed_agent_avatar_url(command: &str) -> Option { #[cfg(test)] mod tests; + +#[cfg(test)] +mod opencode_tests; diff --git a/desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs b/desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs new file mode 100644 index 00000000000..1b141cb88cf --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs @@ -0,0 +1,192 @@ +//! The `KNOWN_ACP_RUNTIMES` table — the static description of every ACP +//! runtime Buzz knows how to discover, probe and configure. +//! +//! Split out of `discovery.rs` to keep that file under the repo's file-size +//! ratchet. The table is unchanged by the move. + +use super::runtime_metadata::KnownAcpRuntime; +use super::{BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL}; + +pub(super) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ + KnownAcpRuntime { + id: "goose", + label: "Goose", + commands: &["goose"], + aliases: &[], + avatar_url: GOOSE_AVATAR_URL, + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("goose"), + cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], + // Goose's stable release currently publishes only the Unix installer; + // its official Windows instructions intentionally point at this main-branch script. + cli_install_commands_windows: &[windows_install_command!("goose", "https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1", "$env:CONFIGURE='false'; ")], + adapter_install_commands: &[], + cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz talks to Goose through the Goose CLI.", + adapter_install_hint: "", + skill_dir: Some(".goose/skills"), + supports_acp_model_switching: false, + model_env_var: Some("GOOSE_MODEL"), + provider_env_var: Some("GOOSE_PROVIDER"), + provider_locked: false, + default_env: &[("GOOSE_MODE", "auto")], + config_file_path: Some("~/.config/goose/config.yaml"), + config_file_format: Some("yaml"), + supports_acp_native_config: true, + thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), + context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + max_rounds_env_var: None, + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + }, + KnownAcpRuntime { + id: "claude", + label: "Claude Code", + commands: &["claude-agent-acp", "claude-code-acp"], + aliases: &["claude-code", "claudecode"], + avatar_url: CLAUDE_CODE_AVATAR_URL, + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("claude"), + cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], + cli_install_commands_windows: &[windows_install_command!("claude", "https://claude.ai/install.ps1")], + adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], + cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", + cli_install_hint: "Buzz talks to Claude Code through the Claude Code CLI.", + adapter_install_hint: "Buzz talks to the Claude Code CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/claude-agent-acp.", + skill_dir: Some(".claude/skills"), + supports_acp_model_switching: false, + model_env_var: None, + provider_env_var: None, + provider_locked: true, + default_env: &[], + config_file_path: Some("~/.claude/settings.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, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run the Claude CLI to complete authentication."), + auth_probe_args: Some(&["claude", "auth", "status"]), + }, + KnownAcpRuntime { + id: "codex", + label: "Codex", + commands: &["codex-acp"], + aliases: &[], + avatar_url: CODEX_AVATAR_URL, + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: false, + underlying_cli: Some("codex"), + cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], + cli_install_commands_windows: &[windows_install_command!("codex", "https://chatgpt.com/codex/install.ps1")], + adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], + cli_install_instructions_url: "https://developers.openai.com/codex/cli/", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", + cli_install_hint: "Buzz talks to Codex through the Codex CLI.", + adapter_install_hint: "Buzz talks to the Codex CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/codex-acp.", + skill_dir: Some(".codex/skills"), + supports_acp_model_switching: false, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + config_file_path: Some("~/.codex/config.toml"), + config_file_format: Some("toml"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run `codex login` to authenticate."), + // 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, + max_rounds_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", + commands: &["buzz-agent"], + aliases: &[], + avatar_url: BUZZ_AGENT_AVATAR_URL, + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: true, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "https://github.com/block/buzz", + adapter_install_instructions_url: "https://github.com/block/buzz", + cli_install_hint: "Ships with the Buzz desktop app.", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: true, + model_env_var: Some("BUZZ_AGENT_MODEL"), + provider_env_var: Some("BUZZ_AGENT_PROVIDER"), + provider_locked: false, + default_env: &[], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), + max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + }, +]; diff --git a/desktop/src-tauri/src/managed_agents/discovery/opencode_tests.rs b/desktop/src-tauri/src/managed_agents/discovery/opencode_tests.rs new file mode 100644 index 00000000000..2c9d6853d73 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/opencode_tests.rs @@ -0,0 +1,34 @@ +//! OpenCode discovery tests — split out of `tests.rs` to keep it under the +//! repo's file-size ratchet. + +use super::normalize_agent_args; + +/// 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()] + ); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index d86e5f33f05..7f08f961213 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -126,15 +126,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/features/agents/ui/AgentIdentityCard.tsx b/desktop/src/features/agents/ui/AgentIdentityCard.tsx index b0668616aeb..18510e0abbb 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,21 @@ export function AgentIdentityCard({ {label} - {modelLabel ? ( + {modelControl ? ( + // Not a control: a propagation boundary. `modelControl` supplies its own + // interactive element; this span only stops the click from also reaching + // the card's full-bleed button overlay. It is never focused itself, and + // keyboard activation of the child fires a click that this same handler + // stops, so a key handler here would be dead code. + // biome-ignore lint/a11y/noStaticElementInteractions: see above + // biome-ignore lint/a11y/useKeyWithClickEvents: see above + event.stopPropagation()} + > + {modelControl} + + ) : modelLabel ? ( {modelLabel} diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 62d85d385d4..c43af068cba 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -587,10 +587,6 @@ export function AgentInstanceEditDialog({ ); } - function handleOpenChange(next: boolean) { - onOpenChange(next); - } - const providerValid = isEditAgentProviderSaveValid({ llmProviderFieldVisible, currentProvider: provider, @@ -735,7 +731,7 @@ export function AgentInstanceEditDialog({ ); } showAgentProfileSyncWarning(result.agent.name, result.profileSyncError); - handleOpenChange(false); + onOpenChange(false); onUpdated?.(result.agent); // The auto-restart policy deliberately never fires for a stopped or // failing agent (a broken agent must not auto-loop), so an edit meant @@ -846,7 +842,7 @@ export function AgentInstanceEditDialog({ : ADVANCED_FIELDS_MOTION_TRANSITION; return ( - + + + ))} + + + + +
+ +
+ { + 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 d0ff2e2738a..7d8aa477f09 100644 --- a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx +++ b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx @@ -19,6 +19,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 } from "./unifiedAgentGroups"; @@ -313,6 +314,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={() => { // The card's main click always opens the PERSONA target, never an // explicit pubkey. A pubkey target is durable in the panel, so a pick @@ -400,6 +407,9 @@ function StandaloneAgentCard({ provider: agent.provider, 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 5b247c31f73..9ccf2c0ddbf 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/features/onboarding/ui/presetLogos.test.mjs b/desktop/src/features/onboarding/ui/presetLogos.test.mjs index 7a5df620dab..b3d0e1c5b83 100644 --- a/desktop/src/features/onboarding/ui/presetLogos.test.mjs +++ b/desktop/src/features/onboarding/ui/presetLogos.test.mjs @@ -38,6 +38,36 @@ const presetIds = [...presetBlock[1].matchAll(/^\s{8}id: "([^"]+)",$/gm)].map( (match) => match[1], ); +// A bundled logo is also legitimate for a KNOWN_ACP_RUNTIMES entry. `opencode` +// lives there rather than in PRESET_HARNESSES so it can carry a +// `config_file_path`, but it still ships a mark keyed by id in PRESET_LOGOS. +// Only the reverse direction consults this list — every *preset* must still have +// a logo, while a known runtime is free to use a remote avatar instead. +const KNOWN_RUNTIMES_RS = + "src-tauri/src/managed_agents/discovery/known_runtimes.rs"; +const knownRuntimesRs = readFileSync( + path.join(desktopRoot, KNOWN_RUNTIMES_RS), + "utf8", +); + +// `pub(super)` is optional in the pattern: the table carries it today because +// it lives in a submodule of `discovery`, and requiring the bare `const` form +// is what silently broke this guard when the table was moved out of +// discovery.rs. +const runtimeBlock = knownRuntimesRs.match( + /(?:pub(?:\([^)]*\))?\s+)?const KNOWN_ACP_RUNTIMES: &\[KnownAcpRuntime\] = &\[([\s\S]*?)\n\];/, +); +assert.ok( + runtimeBlock, + `could not locate KNOWN_ACP_RUNTIMES in ${KNOWN_RUNTIMES_RS}`, +); + +const knownRuntimeIds = [ + ...runtimeBlock[1].matchAll(/^\s{8}id: "([^"]+)",$/gm), +].map((match) => match[1]); + +const logoOwnerIds = [...new Set([...presetIds, ...knownRuntimeIds])]; + test("PRESET_HARNESSES parse found the preset ids", () => { // Guards the regex itself: a struct-field rename would otherwise silently // yield zero ids and make every assertion below vacuously pass. @@ -70,14 +100,15 @@ for (const id of presetIds) { }); } -test("PRESET_LOGOS has no entries for unknown presets", () => { +test("PRESET_LOGOS has no entries for unknown harnesses", () => { const unknown = Object.keys(PRESET_LOGOS).filter( - (id) => !presetIds.includes(id), + (id) => !logoOwnerIds.includes(id), ); assert.deepEqual( unknown, [], - `PRESET_LOGOS maps ids the backend does not emit as presets: ${unknown.join(", ")}`, + `PRESET_LOGOS maps ids the backend emits as neither a preset nor a known ` + + `ACP runtime: ${unknown.join(", ")}`, ); }); diff --git a/desktop/src/shared/api/routingPolicy.ts b/desktop/src/shared/api/routingPolicy.ts new file mode 100644 index 00000000000..8438ab597d1 --- /dev/null +++ b/desktop/src/shared/api/routingPolicy.ts @@ -0,0 +1,119 @@ +import { invokeTauri } from "./tauri"; + +// ── 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, + }), + ); +} diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 1aa98ca4a7f..d08e9c003e2 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -589,6 +589,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. @@ -7795,6 +7814,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; @@ -10266,6 +10291,7 @@ export function maybeInstallE2eTauriMocks() { mockGlobalAgentConfig = config.mock?.globalAgentConfig ? { ...config.mock.globalAgentConfig } : null; + mockRoutingPolicies.clear(); resetMockRelayMembers(config); resetMockRelayAgents(config); resetMockManagedAgents(config); @@ -12646,7 +12672,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", @@ -12655,6 +12696,7 @@ export function maybeInstallE2eTauriMocks() { selectedModel: null, supportsSwitching: false, }; + } case "discover_agent_models": { const discoverError = activeConfig?.mock?.discoverAgentModelsError; if (discoverError) { @@ -12763,6 +12805,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 00000000000..c51a828e7cd --- /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 00000000000..d037cef1b43 --- /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 815c362fc10..2ec5c287305 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -588,6 +588,22 @@ type MockBridgeOptions = { * (`applyProbeResult` in whereToRunIntent.test.mjs). */ backendProviderProbeDelayMs?: number; + /** + * 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 = {