From ef1571054351cc4e1fd3fb71a06413a1d0b3753f Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Tue, 4 Aug 2026 20:29:41 -0400 Subject: [PATCH 01/10] feat(agent): advertise an OpenRouter model catalog so switch_model works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kind:24200 `switch_model` was live and OpenRouter was already a first-class inference provider, but every OpenRouter switch returned `UnsupportedModel`: `session/new` built a real `availableModels` catalog for Databricks only, and `_ => vec![configured model]` gave every other provider a single-entry list. Both switch paths validate against that list (`pool.rs:783` idle via `model_in_catalog`, `acp.rs:2149` via `resolve_model_switch_method`), so a one-entry catalog cannot represent any switch target. Adds `discover_openrouter_models` and wires `Provider::OpenRouter` into the `session/new` catalog alongside the Databricks arm. Queries `/models/user`, the ACCOUNT-scoped catalog, not the global `/models`. This is the substance of the change, not a detail: `/models` lists every model OpenRouter knows (338, of which 272 are tools-capable) while an account can only call the models on its eligibility allowlist (here 21, 13 tools-capable). Requesting an ineligible model returns HTTP 404 "No endpoints available matching your guardrail restrictions and data policy" — which reads as a privacy-settings problem and sends you looking in the wrong place. Verified against the live API: authenticating `/models` does NOT narrow it (338 either way), and `/models/user` contains none of three slugs confirmed uncallable on this account, including the undated `deepseek/deepseek-v4-flash` whose only eligible build is `-0731`. `/models` remains a degraded fallback for keys without account scope. Filters to models advertising `tools`: this catalog feeds an agent harness, so a model that cannot take tool calls only fails later and more confusingly. Mirrors the desktop's existing `filter_openrouter_models`. An all-parse-but-nothing-usable response is an error rather than an empty picker, since an empty list would make every switch fail validation with no indication why. Auth reuses `build_token_source`, which already returns a static source for `Provider::OpenRouter`; discovery failure degrades through the existing `discovery_failure_fallback` to the configured model. Verified: `cargo check -p buzz-agent` clean; 4 new catalog tests pass with the existing 15; `cargo test -p buzz-agent --lib` 385 passed. The 2 failures (auth::cache_path_includes_namespace_and_hash, hints::discover_skills_dedup_by_name) reproduce identically on clean HEAD with this change stashed — pre-existing, unrelated. Parser output checked against the live `/models/user` payload: 13 tools-capable models with correct display names. Co-Authored-By: Claude Opus 5 --- .claude/scheduled_tasks.lock | 1 + crates/buzz-agent/src/catalog.rs | 209 ++++++++++++++++++++++++++++++- crates/buzz-agent/src/lib.rs | 51 +++++--- 3 files changed, 241 insertions(+), 20 deletions(-) create mode 100644 .claude/scheduled_tasks.lock diff --git a/.claude/scheduled_tasks.lock b/.claude/scheduled_tasks.lock new file mode 100644 index 0000000000..f7efc643e6 --- /dev/null +++ b/.claude/scheduled_tasks.lock @@ -0,0 +1 @@ +{"sessionId":"06f65c49-467c-454b-9983-b2949cc6bd25","pid":65452,"procStart":"639213051333836130","acquiredAt":1785726080367} \ No newline at end of file diff --git a/crates/buzz-agent/src/catalog.rs b/crates/buzz-agent/src/catalog.rs index aa2a121c99..012b662672 100644 --- a/crates/buzz-agent/src/catalog.rs +++ b/crates/buzz-agent/src/catalog.rs @@ -1,8 +1,8 @@ -//! Databricks model catalog discovery. +//! Model catalog discovery. //! -//! Exposes [`discover_databricks_models`] — an async helper that lists -//! available models for the `databricks` and `databricks_v2` providers -//! without triggering a browser OAuth flow. Auth is acquired in-process via +//! Exposes [`discover_databricks_models`] and [`discover_openrouter_models`] — +//! async helpers that list available models for a provider without triggering a +//! browser OAuth flow. Auth is acquired in-process via //! [`build_token_source`](crate::llm::build_token_source): //! //! - Static bearer (`DATABRICKS_TOKEN`): returned immediately. @@ -129,6 +129,138 @@ pub async fn discover_databricks_models(cfg: &Config) -> Result, } } +// --------------------------------------------------------------------------- +// OpenRouter — api/v1/models/user (falls back to api/v1/models) +// --------------------------------------------------------------------------- + +/// Discover available models for [`Provider::OpenRouter`]. +/// +/// Queries `/models/user` — the **account-scoped** catalog — rather than the +/// global `/models`. This distinction is not cosmetic: `/models` lists every +/// model OpenRouter knows about (338 at time of writing, 272 tools-capable), +/// but a given account can only call the subset on its eligibility allowlist +/// (21, of which 13 are tools-capable). Calling an ineligible model returns +/// +/// > HTTP 404 "No endpoints available matching your guardrail restrictions and +/// > data policy" +/// +/// which reads like a privacy-settings problem and sends you looking in the +/// wrong place. Advertising the global list in a model picker therefore offers +/// hundreds of models that fail at request time, so `/models/user` is the +/// correct source and `/models` is only a degraded fallback for keys whose +/// account scope is unavailable. +/// +/// Returns a non-empty `Vec` on success. Returns +/// `Err(AgentError::LlmAuth)` when no token is available — callers degrade +/// gracefully via [`discovery_failure_fallback`]. +/// +/// # Panics +/// Never panics. +pub async fn discover_openrouter_models(cfg: &Config) -> Result, AgentError> { + if cfg.provider != Provider::OpenRouter { + return Err(AgentError::InvalidParams( + "discover_openrouter_models called for non-OpenRouter provider".into(), + )); + } + let token_source = build_token_source(cfg)?; + let bearer = token_source.bearer_no_browser().await?; + + let http = Client::new(); + let host = cfg.base_url.trim_end_matches('/'); + + // Account-scoped first; fall back to the global catalog only if that fails. + match fetch_openrouter_models(&http, &format!("{host}/models/user"), &bearer).await { + Ok(models) => Ok(models), + Err(scoped_err) => { + tracing::debug!( + error = %scoped_err, + "OpenRouter account-scoped model discovery failed; falling back to the global catalog (may list models this account cannot call)" + ); + fetch_openrouter_models(&http, &format!("{host}/models"), &bearer).await + } + } +} + +async fn fetch_openrouter_models( + http: &Client, + url: &str, + bearer: &str, +) -> Result, AgentError> { + let response = http + .get(url) + .bearer_auth(bearer) + .send() + .await + .map_err(|e| AgentError::Llm(format!("OpenRouter model discovery request failed: {e}")))?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AgentError::Llm(format!( + "OpenRouter model discovery HTTP {status}: {body}" + ))); + } + + let json: serde_json::Value = response.json().await.map_err(|e| { + AgentError::Llm(format!( + "OpenRouter model discovery response parse failed: {e}" + )) + })?; + + parse_openrouter_models(&json) +} + +/// Parse an OpenRouter `models` payload into selectable entries. +/// +/// Keeps only models advertising the `tools` parameter: this catalog feeds an +/// agent harness, and a model that cannot take tool calls cannot do the job, so +/// offering it in the picker only produces a confusing failure later. +pub(crate) fn parse_openrouter_models( + json: &serde_json::Value, +) -> Result, AgentError> { + let data = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { + AgentError::Llm( + "OpenRouter model discovery: unexpected response (missing 'data' array)".into(), + ) + })?; + + let models: Vec = data + .iter() + .filter_map(|entry| { + let id = entry.get("id")?.as_str()?.trim(); + if id.is_empty() { + return None; + } + let tools_capable = entry + .get("supported_parameters") + .and_then(|v| v.as_array()) + .is_some_and(|params| params.iter().any(|p| p.as_str() == Some("tools"))); + if !tools_capable { + return None; + } + // OpenRouter has no separate display name; `name` carries a vendor + // label, but the id is what the picker must round-trip. + let name = entry + .get("name") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .unwrap_or(id); + Some(ModelEntry { + id: id.to_string(), + name: name.to_string(), + }) + }) + .collect(); + + if models.is_empty() { + return Err(AgentError::Llm( + "OpenRouter model discovery returned no tools-capable models".into(), + )); + } + Ok(models) +} + // --------------------------------------------------------------------------- // v1 — api/2.0/serving-endpoints // --------------------------------------------------------------------------- @@ -397,6 +529,75 @@ pub(crate) fn parse_v2_endpoints_page( mod tests { use super::*; + #[test] + fn openrouter_parse_keeps_only_tools_capable_models() { + let json = serde_json::json!({ + "data": [ + // included: advertises tools + {"id": "openai/gpt-5.6-luna", "name": "OpenAI: GPT-5.6 Luna", + "supported_parameters": ["tools", "temperature"]}, + // included: tools among many params + {"id": "deepseek/deepseek-v4-flash-0731", + "supported_parameters": ["temperature", "tools"]}, + // excluded: no tools support — cannot serve an agent harness + {"id": "some/completion-only", "supported_parameters": ["temperature"]}, + // excluded: supported_parameters absent entirely + {"id": "some/unknown-caps"}, + // excluded: empty id + {"id": " ", "supported_parameters": ["tools"]}, + ] + }); + + let models = parse_openrouter_models(&json).unwrap(); + let ids: Vec<&str> = models.iter().map(|m| m.id.as_str()).collect(); + assert_eq!( + ids, + vec!["openai/gpt-5.6-luna", "deepseek/deepseek-v4-flash-0731"] + ); + // `name` is used when present, else the id round-trips as the label. + assert_eq!(models[0].name, "OpenAI: GPT-5.6 Luna"); + assert_eq!(models[1].name, "deepseek/deepseek-v4-flash-0731"); + } + + #[test] + fn openrouter_parse_errors_on_missing_data_array() { + let json = serde_json::json!({"endpoints": []}); + let err = parse_openrouter_models(&json).unwrap_err(); + assert!( + format!("{err}").contains("missing 'data' array"), + "unexpected error: {err}" + ); + } + + #[test] + fn openrouter_parse_errors_when_nothing_is_tools_capable() { + // A catalog that parses but offers nothing usable must be an error, not an + // empty picker: an empty list would make every switch_model request fail + // validation with no indication of why. + let json = serde_json::json!({ + "data": [{"id": "a/b", "supported_parameters": ["temperature"]}] + }); + let err = parse_openrouter_models(&json).unwrap_err(); + assert!( + format!("{err}").contains("no tools-capable models"), + "unexpected error: {err}" + ); + } + + #[test] + fn openrouter_fallback_is_the_configured_model() { + // Discovery failure must still leave the picker able to represent the + // model the agent is actually running. + let entries = discovery_failure_fallback(Provider::OpenRouter, "openai/gpt-5.6-luna"); + assert_eq!( + entries, + vec![ModelEntry { + id: "openai/gpt-5.6-luna".into(), + name: "openai/gpt-5.6-luna".into() + }] + ); + } + #[test] fn v1_parse_filters_ready_chat_endpoints() { let json = serde_json::json!({ diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index 9a45bf4c98..b3c64693b8 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -11,7 +11,9 @@ mod mcp; pub mod types; mod wire; -pub use catalog::{discover_databricks_models, ModelEntry, DATABRICKS_V2_KNOWN_MODELS}; +pub use catalog::{ + discover_databricks_models, discover_openrouter_models, ModelEntry, DATABRICKS_V2_KNOWN_MODELS, +}; pub use config::Provider; pub use types::AgentError; @@ -445,11 +447,16 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen ); drop(sessions); - // Build a models catalog for the `session/new` response. For Databricks - // providers this advertises available models so the desktop ModelPicker and - // pool can resolve `session/set_model` switches. For Anthropic/OpenAI we - // report only the configured model — live switching on those providers - // effectively requires respawn. + // Build a models catalog for the `session/new` response. For Databricks and + // OpenRouter providers this advertises available models so the desktop + // ModelPicker and pool can resolve `session/set_model` switches. For + // Anthropic/OpenAI we report only the configured model — live switching on + // those providers effectively requires respawn. + // + // Advertising a real catalog is what makes kind:24200 `switch_model` usable: + // both the idle and busy switch paths validate the requested model against + // this list, so a single-entry catalog makes every switch return + // `UnsupportedModel`. // // `models_cache` caches only a successful discovery result (`get_or_try_init` // leaves the cell empty on error so the next `session/new` call retries). On @@ -457,21 +464,33 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen // being written to the cell. let available_models: Vec = { use crate::config::Provider; - match app.cfg.provider { - Provider::Databricks | Provider::DatabricksV2 => { - let models = resolve_models_catalog( + let discovered = match app.cfg.provider { + Provider::Databricks | Provider::DatabricksV2 => Some( + resolve_models_catalog( &app.models_cache, app.cfg.provider, &app.cfg.model, discover_databricks_models(&app.cfg), ) - .await; - models - .iter() - .map(|m| json!({ "modelId": m.id, "name": m.name })) - .collect() - } - _ => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], + .await, + ), + Provider::OpenRouter => Some( + resolve_models_catalog( + &app.models_cache, + app.cfg.provider, + &app.cfg.model, + discover_openrouter_models(&app.cfg), + ) + .await, + ), + Provider::Anthropic | Provider::OpenAi => None, + }; + match discovered { + Some(models) => models + .iter() + .map(|m| json!({ "modelId": m.id, "name": m.name })) + .collect(), + None => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], } }; From 1f99db40b8e1928495bec8706b6f239b09ded430 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Tue, 4 Aug 2026 20:35:17 -0400 Subject: [PATCH 02/10] test(acp): guard that an OpenRouter catalog resolves a switch_model request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asserts the seam the previous commit opened: resolve_model_switch_method turns a model from buzz-agent's advertised OpenRouter catalog into a live SetModel switch, and refuses one that is absent. The negative case is the real gpt-5.6-terra situation — present in OpenRouter's global catalog but not on the account's eligibility allowlist, so a request for it returns HTTP 404. Refusing it at resolve time surfaces unsupported_model up front instead of failing mid-request. buzz-acp --lib: 648 passed, up from 647 on clean HEAD. The 20 failures are identical with this test stashed — pre-existing and unrelated. Co-Authored-By: Claude Opus 5 --- crates/buzz-acp/src/acp.rs | 44 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 700d5e8dcf..dbbfb3da69 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -2654,6 +2654,50 @@ mod tests { assert!(super::extract_model_config_options(&result).is_empty()); } + /// Regression guard for the OpenRouter model-catalog seam. + /// + /// `switch_model` (kind:24200) resolves the requested model against the + /// `availableModels` a provider advertises in `session/new`. buzz-agent used to + /// advertise a single-entry catalog for every non-Databricks provider, so an + /// OpenRouter switch could never resolve. The payload below is the real shape + /// buzz-agent now returns for `BUZZ_AGENT_PROVIDER=openrouter` (trimmed): an + /// account-eligible, tools-capable slate. + #[test] + fn openrouter_catalog_resolves_a_switch_and_rejects_an_ineligible_model() { + let session_new = serde_json::json!({ + "sessionId": "sess-openrouter", + "models": { + "currentModelId": "openai/gpt-5.6-luna", + "availableModels": [ + { "modelId": "openai/gpt-5.6-luna", "name": "OpenAI: GPT-5.6 Luna" }, + { "modelId": "openai/gpt-5.6-luna-pro", "name": "OpenAI: GPT-5.6 Luna Pro" }, + { "modelId": "z-ai/glm-5.2", "name": "Z.ai: GLM 5.2" }, + { "modelId": "deepseek/deepseek-v4-flash-0731", "name": "DeepSeek: DeepSeek V4 Flash 0731" }, + ] + } + }); + + // A model in the advertised catalog resolves to a live set_model switch. + let method = super::resolve_model_switch_method(&session_new, "z-ai/glm-5.2") + .expect("a catalog model must resolve"); + match method { + super::ModelSwitchMethod::SetModel { model_id } => { + assert_eq!(model_id, "z-ai/glm-5.2"); + } + other => panic!("expected SetModel, got {other:?}"), + } + + // A model absent from the catalog must NOT resolve. gpt-5.6-terra is the + // real case: it exists in OpenRouter's global catalog but is not on this + // account's eligibility allowlist, so requesting it returns HTTP 404. + // Refusing it here turns that into an up-front unsupported_model rather + // than a confusing mid-request failure. + assert!( + super::resolve_model_switch_method(&session_new, "openai/gpt-5.6-terra").is_none(), + "a model outside the advertised catalog must not resolve" + ); + } + #[test] fn extract_model_state_returns_models_object() { let result = serde_json::json!({ From 6bcace1d5060ad1cb6332bbf73d819e210b853bd Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 09:00:01 -0400 Subject: [PATCH 03/10] feat(acp): per-turn model routing, opt-in via BUZZ_ROUTING_POLICY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First half of putting a router inside buzz. Picks the model for an inbound turn instead of always using the agent's configured default. Applied through the EXISTING OwnedAgent::desired_model mechanism that switch_model already uses, so nothing new touches the ACP wire, the relay, or the trust boundary. In particular it needs no owner-signed kind:24200 control frame: the decision is made in-process by the harness already trusted to run the turn, which avoids handing an automated router the owner private key (control frames are owner-only — lib.rs:851 — and the NIP-OA delegation here covers relay membership only). Two stages, cheap first: - rules: deterministic case-insensitive matchers over the prompt. No network, no added latency. contains / contains_all. - classifier: optional LOCAL Ollama call, consulted only when no rule matched. Local by design — this code sees raw channel content, so shipping every turn's text to a hosted classifier in order to decide where to send it would leak exactly what a routing decision protects. gemma3:27b is the recommended model (a 176-call eval scored it 4/4 on the privacy class and reproduced its accuracy and confusion pattern exactly across three runs). Safety properties, each covered by a test: - OFF unless BUZZ_ROUTING_POLICY names a readable file with enabled:true, so dropping a file in place cannot silently start routing. - fails open everywhere: unreadable/unparseable policy, no rule match, classifier error or timeout, or an unknown label all resolve to "no opinion" and the turn proceeds on the agent's model. A router that can fail a turn is worse than none. - does NOT override an explicit live switch_model (model_overridden), so a human or the ModelPicker outranks the policy and the UI cannot be made to lie. - an empty needle list never matches, so "always route here" cannot be created by omission — that intent must be written as default_model. - a policy naming a model the provider does not advertise degrades to the agent default with a warning, via the existing catalog validation. SCOPE: this 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 — a dispatcher concern, and there is no dispatcher (selection is a p-tag mention with relay fan-out). Verified: 7 unit tests pass. Live classifier test against real Ollama (gemma3:27b) returns Decision { model: "db-model", reason: Classifier { label: "database" } } for a migration task in 24s; it SKIPs cleanly when BUZZ_ROUTING_LIVE_OLLAMA is unset, following the env-gated pattern in crates/buzz-test-client/tests/e2e_mesh_llm.rs. Co-Authored-By: Claude Opus 5 --- crates/buzz-acp/src/lib.rs | 1 + crates/buzz-acp/src/pool.rs | 28 ++ crates/buzz-acp/src/routing.rs | 458 +++++++++++++++++++++++++++++++++ 3 files changed, 487 insertions(+) create mode 100644 crates/buzz-acp/src/routing.rs diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 811253e4ac..7f65fe8b53 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -9,6 +9,7 @@ mod pool; mod pool_lifecycle; mod queue; mod relay; +mod routing; mod setup_mode; mod usage; diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 64edf68ee2..9d6b909723 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -1365,6 +1365,34 @@ pub async fn run_prompt_task( Some(b) => PromptSource::Channel(b.channel_id), None => PromptSource::Heartbeat, }; + + // Per-turn model routing (opt-in via BUZZ_ROUTING_POLICY; see routing.rs). + // + // Deliberately does NOT override a live `switch_model`: if the operator (or + // the desktop ModelPicker) explicitly pinned a model for this agent, + // `model_overridden` is set and a router silently changing it would make the + // UI lie about what is running. Explicit human choice outranks the policy. + // + // The decision is expressed as `desired_model`, which the existing + // session-creation path validates against the agent's advertised catalog and + // applies — so a policy naming a model the provider does not offer degrades + // to the agent default with a warning rather than failing the turn. + if !agent.model_overridden { + if let Some(policy) = crate::routing::Policy::from_env() { + let routed_text = prompt_text.as_deref().unwrap_or_default(); + if let Some(decision) = policy.decide(routed_text).await { + if agent.desired_model.as_deref() != Some(decision.model.as_str()) { + tracing::info!( + target: "acp::routing", + model = %decision.model, + reason = ?decision.reason, + "routing selected a model for this turn" + ); + agent.desired_model = Some(decision.model); + } + } + } + } let observer_channel_id = match &source { PromptSource::Channel(channel_id) => Some(*channel_id), PromptSource::Heartbeat => None, diff --git a/crates/buzz-acp/src/routing.rs b/crates/buzz-acp/src/routing.rs new file mode 100644 index 0000000000..6471dcf564 --- /dev/null +++ b/crates/buzz-acp/src/routing.rs @@ -0,0 +1,458 @@ +//! Per-turn model routing. +//! +//! Picks the model for an inbound turn instead of always using the agent's +//! configured default. The decision is applied through the EXISTING +//! [`OwnedAgent::desired_model`](crate::pool::OwnedAgent) mechanism that +//! `switch_model` already uses, so nothing new touches the ACP wire, the relay, +//! or the trust boundary — in particular this needs no owner-signed kind:24200 +//! control frame, because the decision is made in-process by the harness that is +//! already trusted to run the turn. +//! +//! # Opt-in, and fails open +//! +//! Routing is off unless `BUZZ_ROUTING_POLICY` names a readable policy file. A +//! missing file, unparseable JSON, `enabled: false`, no matching rule, or a +//! classifier that errors or times out all resolve to "no opinion" — the turn +//! proceeds on the agent's configured model exactly as before. Routing must never +//! be able to fail a turn; a router that can block work is worse than no router. +//! +//! # Two stages, cheap first +//! +//! 1. `rules` — deterministic substring/regex-free matchers over the prompt text. +//! No network, no latency. Most routing intent is expressible here. +//! 2. `classifier` — an optional local Ollama call, used only when no rule +//! matched. Local by design: this code sees raw channel content, so shipping +//! every turn's text to a hosted classifier to decide where to send it would +//! leak exactly what a routing decision is supposed to protect. +//! +//! # What this does NOT do +//! +//! It selects a MODEL, not a harness. One buzz-acp process serves one agent, so +//! routing a turn to opencode-vs-codex-vs-claude means choosing a different agent +//! — that is a dispatcher concern (there is none today: selection is a `p`-tag +//! mention with relay fan-out) and is deliberately out of scope here. + +use std::path::PathBuf; +use std::time::Duration; + +use serde::Deserialize; + +/// Env var naming the policy file. Absent => routing disabled. +pub const POLICY_ENV: &str = "BUZZ_ROUTING_POLICY"; + +/// How a rule matches the prompt text. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum MatchKind { + /// Any of `any` appears in the prompt, case-insensitively. + Contains, + /// Every one of `any` appears in the prompt, case-insensitively. + ContainsAll, +} + +impl Default for MatchKind { + fn default() -> Self { + Self::Contains + } +} + +/// One deterministic routing rule. +#[derive(Debug, Clone, Deserialize)] +pub struct Rule { + /// Human label, surfaced in the log line explaining a routing decision. + #[serde(default)] + pub name: Option, + #[serde(default)] + pub match_kind: MatchKind, + /// Needles to look for. An empty list never matches — a rule that matches + /// everything must be written as the policy's `default_model` instead, so + /// "always this model" cannot be created by accident. + #[serde(default)] + pub any: Vec, + /// Model id to use when this rule matches. Must be a model the agent's + /// provider actually advertises, or the existing apply step logs a miss and + /// falls back to the agent default. + pub model: String, +} + +impl Rule { + fn matches(&self, haystack_lower: &str) -> bool { + if self.any.is_empty() { + return false; + } + let hit = |needle: &String| { + let n = needle.trim().to_lowercase(); + !n.is_empty() && haystack_lower.contains(&n) + }; + match self.match_kind { + MatchKind::Contains => self.any.iter().any(hit), + MatchKind::ContainsAll => self.any.iter().all(hit), + } + } +} + +/// 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, +} + +/// 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, +} + +/// 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, + }) + } + + /// 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); + } + + #[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() + } + ); + } + + #[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). + } +} From 1de58aa94b28dc54510a0100f3bbba390e70c745 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 20:53:17 -0400 Subject: [PATCH 04/10] fix(agents): mount ModelPicker so switch_model is reachable from the UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModelPicker was dead code — nothing imported it — and it is the SOLE caller of switchManagedAgentModel. So kind:24200 switch_model had no UI entry point at all, and the OpenRouter catalog work (ef15710) gave the backend 13 switchable models that no screen could ask for. Mounted in both live agent cards in UnifiedAgentsSection: AgentPersonaCard (a picker when the persona has a ManagedAgent, the static label otherwise) and StandaloneAgentCard (always has one). AgentIdentityCard gains a `modelControl` slot that takes precedence over `modelLabel` and occupies the same position. It needs `pointer-events-auto` plus stopPropagation: the card's click target is an `absolute inset-0 z-10` button overlay and the label row is `pointer-events-none` so it cannot steal that click. Without both, the control is either unclickable or opens the profile panel instead of its own menu. Note: ManagedAgentRow/AgentGroupRows also render agents and were the first place tried — but that pair is itself orphaned (AgentGroupRows is referenced only by its own file), so mounting there would have been dead code inside dead code. Left untouched; whether to delete the pair is a separate pre-existing question. Verified in a browser (vite dev + ?e2e=mock#/agents), not just tsc: - the agent cards' label changed from "Default model" (agentCardModelLabel.ts:43) to "Auto" (ModelPicker.tsx:91), isolating the change to exactly those cards — the teams' "Auto" is TeamIdentityCard and is unaffected. - 3 triggers rendered, one per card, each aria-haspopup="menu". - clicking one: trigger data-state -> "open", role="menu" present, menu rendered its real empty state "This agent uses the runtime's default model." — i.e. the click path reaches fetchModels/getAgentModels and handles the response. tsc --noEmit exits 0. The mock agents have no running harness, so the populated 13-model list is not yet exercised end-to-end; that needs a seeded running OpenRouter agent. Co-Authored-By: Claude Opus 5 --- .../features/agents/ui/AgentIdentityCard.tsx | 20 ++++++++++++++++++- .../agents/ui/UnifiedAgentsSection.tsx | 10 ++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/desktop/src/features/agents/ui/AgentIdentityCard.tsx b/desktop/src/features/agents/ui/AgentIdentityCard.tsx index 6f13a84ae8..1caf1b7a7a 100644 --- a/desktop/src/features/agents/ui/AgentIdentityCard.tsx +++ b/desktop/src/features/agents/ui/AgentIdentityCard.tsx @@ -12,6 +12,16 @@ type AgentIdentityCardProps = { dataTestId: string; label: string; modelLabel?: string | null; + /** + * Interactive replacement for `modelLabel`, in the same slot. Takes precedence + * when both are supplied. + * + * The label row lives under `pointer-events-none` so it cannot steal clicks + * from the card's full-bleed button overlay, so anything interactive here is + * wrapped in `pointer-events-auto` and stops propagation — otherwise the + * control would either be unclickable or would also open the profile panel. + */ + modelControl?: ReactNode; onClick: () => void; /** Optional badge rendered below the label (e.g. "Restart required"). */ statusBadge?: ReactNode; @@ -25,6 +35,7 @@ export function AgentIdentityCard({ dataTestId, label, modelLabel, + modelControl, onClick, statusBadge, }: AgentIdentityCardProps) { @@ -72,7 +83,14 @@ export function AgentIdentityCard({ {label} - {modelLabel ? ( + {modelControl ? ( + event.stopPropagation()} + > + {modelControl} + + ) : modelLabel ? ( {modelLabel} diff --git a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx index 19a5ef1171..cbf3e78a54 100644 --- a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx +++ b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx @@ -25,6 +25,7 @@ import { IdentityCardSkeleton } from "@/shared/ui/identity-card-skeleton"; import { AgentIdentityCard } from "./AgentIdentityCard"; import { AgentRuntimeAvatarControl } from "./AgentRuntimeAvatarControl"; import { CreateIdentityCard } from "./CreateIdentityCard"; +import { ModelPicker } from "./ModelPicker"; import { PersonaActionsMenu } from "./PersonaActionsMenu"; import { buildUnifiedGroups, pickProfileAgent } from "./unifiedAgentGroups"; @@ -327,6 +328,12 @@ function AgentPersonaCard({ dataTestId={`persona-agent-row-${persona.id}`} label={title} modelLabel={modelLabel} + // A persona without a managed agent has nothing to switch, so it keeps the + // static label. With an agent, the same slot becomes a live picker: this is + // the only UI path to `switch_model` (kind:24200) — ModelPicker is its sole + // caller, and until now nothing rendered ModelPicker, so the backend could + // switch models that no screen could ask for. + modelControl={agent ? : undefined} onClick={() => { if (agent) { onOpenAgentProfile( @@ -406,6 +413,9 @@ function StandaloneAgentCard({ personaModel: null, defaultModel, })} + // Unknown agents are managed agents with no persona, so they always have a + // ManagedAgent to switch — unlike AgentPersonaCard, there is no undefined case. + modelControl={} onClick={() => { onOpenAgentProfile( agent.pubkey, From 514c77cb416bcce179e107f37b6ddd7e580e14fd Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 21:01:03 -0400 Subject: [PATCH 05/10] test(e2e): allow the mock to serve a populated get_agent_models catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_agent_models` returned a hardcoded empty list with supportsSwitching:false and had no override hook, so the ModelPicker could only ever be exercised in its "runtime default" empty state — the populated list, and therefore the whole model-selection path, was untestable in the browser harness. Its sibling `discover_agent_models` already had exactly this hook; this mirrors it. Verified against the real UI (vite dev + ?e2e=mock#/agents) by seeding 7 account-eligible OpenRouter models: - opening a picker rendered all 7 with their display names (OpenAI: GPT-5.6 Luna, ... Z.ai: GLM 5.2, MoonshotAI: Kimi K3) — previously the empty state. - selecting "Z.ai: GLM 5.2" ran the handler and the trigger label became z-ai/glm-5.2 while the other two agents' pickers stayed "Auto", so the selection persisted to exactly one agent. That closes the path from buzz-agent's session/new catalog (ef15710) through ModelPicker (1de58aa) to a click that changes an agent's model. Co-Authored-By: Claude Opus 5 --- desktop/src/testing/e2eBridge.ts | 37 +++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 355ccea9fc..9b7ab976b7 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -502,6 +502,25 @@ type E2eConfig = { agentDefaultModel?: string | null; selectedModel?: string | null; }; + /** + * Override for `get_agent_models` — the catalog the ModelPicker reads. + * + * Mirrors `discoverAgentModels`. Without it `get_agent_models` always + * returns an empty list with `supportsSwitching: false`, so the picker can + * only ever be exercised in its "runtime default" empty state and the + * populated list (and therefore the switch path) is untestable in the + * browser harness. + */ + agentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; /** * When set, `discover_agent_models` throws with this message instead of * returning a catalog. @@ -11447,7 +11466,22 @@ export function maybeInstallE2eTauriMocks() { return handleGetManagedAgentLog( payload as Parameters[0], ); - case "get_agent_models": + case "get_agent_models": { + const modelsOverride = activeConfig?.mock?.agentModels; + if (modelsOverride) { + return { + agentName: "mock-agent", + agentVersion: "0.0.0", + models: modelsOverride.models.map((model) => ({ + id: model.id, + name: model.name, + description: model.description ?? null, + })), + agentDefaultModel: modelsOverride.agentDefaultModel ?? null, + selectedModel: modelsOverride.selectedModel ?? null, + supportsSwitching: modelsOverride.supportsSwitching, + }; + } return { agentName: "mock-agent", agentVersion: "0.0.0", @@ -11456,6 +11490,7 @@ export function maybeInstallE2eTauriMocks() { selectedModel: null, supportsSwitching: false, }; + } case "discover_agent_models": { const discoverError = activeConfig?.mock?.discoverAgentModelsError; if (discoverError) { From 5ecbbdc496dafb804e71a68cf886f34f080df8be Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 22:46:02 -0400 Subject: [PATCH 06/10] chore(gitignore): ignore the Claude Code context tree Local tooling scratch that has no business in the repo. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index f26e74136c..69a484bacf 100644 --- a/.gitignore +++ b/.gitignore @@ -72,3 +72,6 @@ identity.key # Helm dependency tarballs — regenerable from Chart.lock via `helm dependency build` deploy/charts/*/charts/*.tgz + +# Claude Code context +.claude_context_tree From 9a2dd789e395204d75a0f645bc1337e718cc9868 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 22:46:19 -0400 Subject: [PATCH 07/10] feat(agents): read OpenCode's config file so its model is visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `opencode acp` takes no --model flag and reads no model env var, so its config file is the only tier that knows which model it runs. The config panel was blank for every OpenCode agent, and nothing in Buzz could tell the user why. Adding a config_file_path meant promoting OpenCode from PRESET_HARNESSES to KNOWN_ACP_RUNTIMES — presets have nowhere to hang one, and known_acp_runtime("opencode") returned None, so the config bridge saw no metadata at all. Builtins take their args from default_agent_args rather than a preset args list, so "opencode" is registered there too; without it the promotion would have silently launched the bare CLI instead of the ACP server. The reader handles JSONC (comments and trailing commas): OpenCode documents it as a first-class config format and its own docs use both, so a plain serde_json parse would reject real user configs. The comment stripper is string-aware because every one of these files carries a URL. `model` is written as provider_id/model_id and is split so the normalized provider and model fields each carry their own half. Co-Authored-By: Claude Opus 5 --- .../src/managed_agents/config_bridge/mod.rs | 1 + .../managed_agents/config_bridge/opencode.rs | 395 ++++++++++++++++++ .../managed_agents/config_bridge/reader.rs | 19 +- .../config_bridge/reader_tests.rs | 80 ++++ .../src-tauri/src/managed_agents/discovery.rs | 47 ++- .../src/managed_agents/discovery/presets.rs | 11 +- .../src/managed_agents/discovery/tests.rs | 30 ++ .../features/onboarding/ui/RuntimeIcon.tsx | 5 +- 8 files changed, 573 insertions(+), 15 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs index f8b045fc72..e13830abb3 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs @@ -2,6 +2,7 @@ mod buzz_agent; mod claude; mod codex; mod goose; +mod opencode; pub(crate) mod reader; mod schema_walker; pub(crate) mod types; diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs b/desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs new file mode 100644 index 0000000000..57d1a63d9a --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/config_bridge/opencode.rs @@ -0,0 +1,395 @@ +use std::path::PathBuf; + +use super::types::{ExtensionEntry, RuntimeFileConfig}; + +/// Read OpenCode config from `$OPENCODE_CONFIG`, else +/// `$XDG_CONFIG_HOME/opencode/opencode.json(c)`, else +/// `~/.config/opencode/opencode.json(c)`. +/// +/// This tier matters more for OpenCode than for the other harnesses: `opencode +/// acp` takes no `--model` flag and reads no model env var, so the config file +/// is the ONLY place its model is set. Without this reader the model field is +/// blank in the panel even when the harness is perfectly well configured. +pub(super) fn read_config_file() -> Option { + let raw = std::fs::read_to_string(opencode_config_path()?).ok()?; + parse_opencode_config(&raw) +} + +/// Canonical config path for display and for the reader. +/// +/// Returns the `.json` path even when nothing exists on disk yet — the panel +/// shows where the file *would* live, matching how `claude` reports +/// `~/.claude.json` unconditionally. +pub(crate) fn opencode_config_path() -> Option { + if let Some(explicit) = std::env::var_os("OPENCODE_CONFIG") { + let path = PathBuf::from(explicit); + if !path.as_os_str().is_empty() { + return Some(path); + } + } + + let dir = opencode_config_dir()?; + let json = dir.join("opencode.json"); + if json.exists() { + return Some(json); + } + let jsonc = dir.join("opencode.jsonc"); + if jsonc.exists() { + return Some(jsonc); + } + Some(json) +} + +fn opencode_config_dir() -> Option { + if let Some(xdg) = std::env::var_os("XDG_CONFIG_HOME") { + let base = PathBuf::from(xdg); + if !base.as_os_str().is_empty() { + return Some(base.join("opencode")); + } + } + Some(dirs::home_dir()?.join(".config").join("opencode")) +} + +fn parse_opencode_config(raw: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(&strip_jsonc(raw)).ok()?; + + // OpenCode writes the model as `provider_id/model_id`. Split it so the + // normalized provider and model fields each carry their own half rather + // than repeating the whole pair in both. + let (provider, model) = match json_string(&value, "model") { + Some(spec) => match spec.split_once('/') { + Some((p, m)) if !p.is_empty() && !m.is_empty() => { + (Some(p.to_string()), Some(m.to_string())) + } + // No slash (or a malformed one) — surface the value as written + // instead of guessing at a provider. + _ => (None, Some(spec)), + }, + None => (None, None), + }; + + let extensions = parse_mcp_servers(&value); + + // Config-driven extra fields — skip keys extracted into typed fields above. + let skip = &["model", "provider", "mcp"]; + let mut extra = super::schema_walker::extract_config_fields(&value, skip); + + // Custom providers from `provider.` — surface as + // "provider. = configured" rather than flattening their model tables, + // mirroring how the codex reader handles `model_providers`. + if let Some(serde_json::Value::Object(providers)) = value.get("provider") { + for name in providers.keys() { + extra.insert(format!("provider.{name}"), "configured".to_string()); + } + } + + Some(RuntimeFileConfig { + model, + provider, + // OpenCode has no single mode/effort/limit key: permissions live under + // `permission`, reasoning effort is per-model under + // `provider..models..options`. Both reach the panel via `extra`. + mode: None, + thinking_effort: None, + max_output_tokens: None, + context_limit: None, + // `instructions` is a list of file PATHS, not prompt text, so it is not + // a system prompt. The walker surfaces it in `extra`. + system_prompt: None, + extensions, + extra, + }) +} + +fn parse_mcp_servers(value: &serde_json::Value) -> Vec { + let Some(servers) = value.get("mcp").and_then(|v| v.as_object()) else { + return Vec::new(); + }; + + servers + .iter() + .map(|(name, config)| ExtensionEntry { + name: name.clone(), + kind: "mcp".to_string(), + // OpenCode runs an MCP server unless it explicitly opts out. + enabled: config + .get("enabled") + .and_then(serde_json::Value::as_bool) + .unwrap_or(true), + }) + .collect() +} + +fn json_string(value: &serde_json::Value, key: &str) -> Option { + value + .get(key)? + .as_str() + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) +} + +/// Make a JSONC document parseable by `serde_json`: drop comments, then drop +/// trailing commas. OpenCode documents `.jsonc` as a first-class config format +/// and its own docs use both, so a plain `serde_json` parse would reject real +/// user configs. +fn strip_jsonc(raw: &str) -> String { + strip_trailing_commas(&strip_comments(raw)) +} + +/// Remove `//` and `/* */` comments. String-aware: `//` inside a JSON string +/// must survive — every OpenCode config carries at least one URL (`$schema`). +/// Newlines inside block comments are preserved so line-based error offsets +/// still line up with the original file. +fn strip_comments(raw: &str) -> String { + let chars: Vec = raw.chars().collect(); + let mut out = String::with_capacity(raw.len()); + let mut i = 0; + let mut in_string = false; + + while i < chars.len() { + let c = chars[i]; + + if in_string { + out.push(c); + if c == '\\' && i + 1 < chars.len() { + out.push(chars[i + 1]); + i += 2; + continue; + } + if c == '"' { + in_string = false; + } + i += 1; + continue; + } + + match c { + '"' => { + in_string = true; + out.push(c); + i += 1; + } + '/' if chars.get(i + 1) == Some(&'/') => { + while i < chars.len() && chars[i] != '\n' { + i += 1; + } + } + '/' if chars.get(i + 1) == Some(&'*') => { + i += 2; + while i < chars.len() && !(chars[i] == '*' && chars.get(i + 1) == Some(&'/')) { + if chars[i] == '\n' { + out.push('\n'); + } + i += 1; + } + i = i.saturating_add(2).min(chars.len()); + } + _ => { + out.push(c); + i += 1; + } + } + } + + out +} + +/// Drop a `,` whose next significant character is `}` or `]`. Runs on +/// already-comment-free text, so "significant" only has to skip whitespace. +fn strip_trailing_commas(raw: &str) -> String { + let chars: Vec = raw.chars().collect(); + let mut out = String::with_capacity(raw.len()); + let mut i = 0; + let mut in_string = false; + + while i < chars.len() { + let c = chars[i]; + + if in_string { + out.push(c); + if c == '\\' && i + 1 < chars.len() { + out.push(chars[i + 1]); + i += 2; + continue; + } + if c == '"' { + in_string = false; + } + i += 1; + continue; + } + + if c == '"' { + in_string = true; + out.push(c); + i += 1; + continue; + } + + if c == ',' { + let next = chars[i + 1..].iter().find(|ch| !ch.is_whitespace()); + if matches!(next, Some('}') | Some(']')) { + i += 1; + continue; + } + } + + out.push(c); + i += 1; + } + + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn model_splits_into_provider_and_model() { + let cfg = parse_opencode_config(r#"{"model": "anthropic/claude-sonnet-4-5"}"#).unwrap(); + assert_eq!(cfg.provider.as_deref(), Some("anthropic")); + assert_eq!(cfg.model.as_deref(), Some("claude-sonnet-4-5")); + } + + #[test] + fn model_id_containing_slashes_keeps_everything_after_the_first() { + // `lmstudio/google/gemma-3n-e4b` — provider is the FIRST segment; the + // rest is the model id, which may itself contain slashes. + let cfg = parse_opencode_config(r#"{"model": "lmstudio/google/gemma-3n-e4b"}"#).unwrap(); + assert_eq!(cfg.provider.as_deref(), Some("lmstudio")); + assert_eq!(cfg.model.as_deref(), Some("google/gemma-3n-e4b")); + } + + #[test] + fn model_without_a_provider_prefix_is_surfaced_as_written() { + let cfg = parse_opencode_config(r#"{"model": "gpt-5"}"#).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("gpt-5")); + assert!(cfg.provider.is_none()); + } + + #[test] + fn mcp_servers_become_extensions_and_honor_enabled_false() { + let cfg = parse_opencode_config( + r#"{"mcp": { + "filesystem": {"type": "local", "command": ["npx", "-y", "fs"]}, + "sentry": {"type": "remote", "url": "https://x", "enabled": false} + }}"#, + ) + .unwrap(); + assert_eq!(cfg.extensions.len(), 2); + let sentry = cfg.extensions.iter().find(|e| e.name == "sentry").unwrap(); + assert!(!sentry.enabled); + let fs = cfg + .extensions + .iter() + .find(|e| e.name == "filesystem") + .unwrap(); + assert!(fs.enabled, "an mcp entry with no `enabled` key defaults on"); + } + + #[test] + fn custom_providers_are_summarized_not_flattened() { + let cfg = parse_opencode_config( + r#"{ + "model": "helicone/gpt-4o", + "provider": {"helicone": {"npm": "@ai-sdk/openai-compatible", "models": {"gpt-4o": {}}}} + }"#, + ) + .unwrap(); + assert_eq!( + cfg.extra.get("provider.helicone").map(String::as_str), + Some("configured") + ); + assert!( + !cfg.extra + .keys() + .any(|k| k.starts_with("provider.helicone.")), + "provider internals must not be flattened into extra" + ); + } + + #[test] + fn normalized_keys_are_not_duplicated_in_extra() { + let cfg = parse_opencode_config( + r#"{"model": "anthropic/x", "mcp": {"a": {}}, "theme": "opencode"}"#, + ) + .unwrap(); + assert!(!cfg.extra.contains_key("model")); + assert!(!cfg.extra.contains_key("mcp.a")); + assert_eq!(cfg.extra.get("theme").map(String::as_str), Some("opencode")); + } + + #[test] + fn unknown_future_fields_reach_extra() { + let cfg = parse_opencode_config(r#"{"some_new_opencode_field": "value"}"#).unwrap(); + assert_eq!( + cfg.extra.get("some_new_opencode_field").map(String::as_str), + Some("value") + ); + } + + #[test] + fn jsonc_comments_are_stripped_without_eating_urls() { + let raw = r#"{ + // the schema line is a comment magnet + "$schema": "https://opencode.ai/config.json", + "model": "openai/gpt-5" /* inline block */ + }"#; + let cfg = parse_opencode_config(raw).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("gpt-5")); + assert_eq!( + cfg.extra.get("$schema").map(String::as_str), + Some("https://opencode.ai/config.json"), + "a `//` inside a string must survive comment stripping" + ); + } + + #[test] + fn a_double_slash_inside_a_string_is_never_treated_as_a_comment() { + let stripped = strip_comments(r#"{"url": "http://localhost:8080/v1"}"#); + assert_eq!(stripped, r#"{"url": "http://localhost:8080/v1"}"#); + } + + #[test] + fn an_escaped_quote_does_not_end_the_string_scan() { + let cfg = parse_opencode_config(r#"{"username": "say \"hi\" // not a comment"}"#).unwrap(); + assert_eq!( + cfg.extra.get("username").map(String::as_str), + Some(r#"say "hi" // not a comment"#) + ); + } + + #[test] + fn trailing_commas_are_tolerated() { + let raw = r#"{ + "model": "openai/gpt-5", + "instructions": ["A.md", "B.md",], + }"#; + let cfg = parse_opencode_config(raw).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("gpt-5")); + } + + #[test] + fn a_comma_inside_a_string_is_not_mistaken_for_a_trailing_comma() { + let cfg = parse_opencode_config(r#"{"username": "last, first"}"#).unwrap(); + assert_eq!( + cfg.extra.get("username").map(String::as_str), + Some("last, first") + ); + } + + #[test] + fn empty_config_parses_to_an_empty_surface() { + let cfg = parse_opencode_config("{}").unwrap(); + assert!(cfg.model.is_none()); + assert!(cfg.provider.is_none()); + assert!(cfg.extensions.is_empty()); + } + + #[test] + fn unparseable_config_returns_none() { + assert!(parse_opencode_config("{{{{ not json").is_none()); + } +} diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index 372d2cfde1..1bef0da011 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -22,6 +22,7 @@ pub(crate) fn read_config_surface( "goose" => super::goose::read_config_file().map(|c| (c, true)), "claude" => super::claude::read_config_file().map(|c| (c, true)), "codex" => super::codex::read_config_file().map(|c| (c, true)), + "opencode" => super::opencode::read_config_file().map(|c| (c, true)), "buzz-agent" => super::buzz_agent::read_config_file().map(|c| (c, true)), _ => None, }) @@ -164,9 +165,17 @@ pub(crate) fn read_config_surface( }); } - let config_file_path = runtime_meta - .and_then(|m| m.config_file_path) - .map(resolve_tilde); + let config_file_path = match runtime_meta.map(|m| m.id) { + // OpenCode's config location moves with `$OPENCODE_CONFIG` and + // `$XDG_CONFIG_HOME`, so the static metadata path would name the wrong + // file on those setups. Ask the reader where it actually looked. + Some("opencode") => { + super::opencode::opencode_config_path().map(|path| path.to_string_lossy().into_owned()) + } + _ => runtime_meta + .and_then(|m| m.config_file_path) + .map(resolve_tilde), + }; let mcp_config_file_path = runtime_meta.and_then(mcp_config_file_path_for_runtime); let extensions = file_config.extensions.clone(); @@ -222,6 +231,10 @@ fn mcp_config_file_path_for_runtime(runtime: &KnownAcpRuntime) -> Option "codex" => { super::codex::codex_config_path().map(|path| path.to_string_lossy().into_owned()) } + // OpenCode declares MCP servers in the same file as everything else. + "opencode" => { + super::opencode::opencode_config_path().map(|path| path.to_string_lossy().into_owned()) + } _ => None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 4ee4ec79c3..c9e56820b0 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -175,6 +175,86 @@ fn goose_mcp_config_path_follows_path_root_override() { ); } +static OPENCODE_CONFIG_LOCK: Mutex<()> = Mutex::new(()); + +fn with_opencode_config(path: &Path, body: impl FnOnce() -> T) -> T { + let _guard = OPENCODE_CONFIG_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let prior = std::env::var_os("OPENCODE_CONFIG"); + std::env::set_var("OPENCODE_CONFIG", path); + let output = body(); + match prior { + Some(value) => std::env::set_var("OPENCODE_CONFIG", value), + None => std::env::remove_var("OPENCODE_CONFIG"), + } + output +} + +/// End-to-end wiring guard for the whole point of the OpenCode entry: the +/// harness takes no `--model` flag and reads no model env var, so unless the +/// bridge reaches its config file the model field is blank in the panel. +#[test] +fn opencode_surface_takes_its_model_from_the_config_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let config = dir.path().join("opencode.jsonc"); + std::fs::write( + &config, + r#"{ + // real OpenCode configs are JSONC with comments and trailing commas + "$schema": "https://opencode.ai/config.json", + "model": "anthropic/claude-sonnet-4-5", + "mcp": { "filesystem": { "type": "local" } }, + }"#, + ) + .expect("write config"); + + let record = test_record(); + let runtime = &KnownAcpRuntime { + id: "opencode", + label: "OpenCode", + commands: &["opencode"], + model_env_var: None, + provider_env_var: None, + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + config_file_path: Some("~/.config/opencode/opencode.json"), + config_file_format: Some("json"), + ..*test_runtime() + }; + + let surface = with_opencode_config(&config, || { + read_config_surface(&record, Some(runtime), None, None) + }); + + let model = surface.normalized.model.expect("model field"); + assert_eq!(model.value.as_deref(), Some("claude-sonnet-4-5")); + assert_eq!(model.origin, ConfigOrigin::ConfigFile); + // Nothing can write it back — no env var, no ACP model switching. + assert!(matches!(model.write_via, ConfigWriteMechanism::ReadOnly)); + + let provider = surface.normalized.provider.expect("provider field"); + assert_eq!(provider.value.as_deref(), Some("anthropic")); + + assert_eq!(surface.sources.config_file, ConfigTierStatus::Available); + assert_eq!( + surface.sources.config_file_path.as_deref().map(Path::new), + Some(config.as_path()), + "the reported path must be the file actually read, not the static default" + ); + assert_eq!( + surface + .extensions + .iter() + .map(|e| e.name.as_str()) + .collect::>(), + vec!["filesystem"] + ); +} + #[test] fn claude_surface_uses_mcp_config_path_not_settings_path() { let record = test_record(); diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 8d1b8a5013..cc69c91268 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -172,6 +172,51 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. auth_probe_args: Some(&["codex", "login", "status"]), }, + // Promoted from PRESET_HARNESSES so it can carry a `config_file_path`: + // `opencode acp` accepts no `--model` flag and reads no model env var, so + // its config file is the only tier that can tell Buzz which model it runs. + // A preset entry has nowhere to hang that, which left the config panel + // blank for every OpenCode agent. + KnownAcpRuntime { + id: "opencode", + label: "OpenCode", + commands: &["opencode"], + aliases: &[], + // Logo is bundled and keyed by id in the frontend (PRESET_LOGOS), so no + // remote avatar is fetched for this runtime. + avatar_url: "", + mcp_command: None, + mcp_hooks: false, + underlying_cli: None, + // Left empty deliberately: OpenCode is not auto-installable from Buzz, + // matching the behaviour it had as a preset. + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "https://opencode.ai/docs", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz talks to OpenCode through its CLI's ACP mode (opencode acp).", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + // No model/provider env var by design — see the note above the entry. + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + config_file_path: Some("~/.config/opencode/opencode.json"), + config_file_format: Some("json"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + // Model is required for OpenCode to run, but Buzz cannot set it — only + // the config file can. Marking it required would raise a readiness gap + // with no affordance to close it. + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + }, KnownAcpRuntime { id: "buzz-agent", label: "Buzz Agent", @@ -465,7 +510,7 @@ pub fn try_record_agent_command( fn default_agent_args(command: &str) -> Option> { match normalize_command_identity(command).as_str() { - "goose" => Some(vec!["acp".to_string()]), + "goose" | "opencode" => Some(vec!["acp".to_string()]), "codex" | "codex-acp" | "claude-agent-acp" | "claude-code-acp" | "claude-code" | "claudecode" | "buzz-agent" => Some(Vec::new()), _ => None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index 3622b21c4a..56bda27f8e 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -119,15 +119,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_hint: "Buzz talks to Grok Build through its CLI's agent stdio mode.", underlying_cli: None, }, - PresetHarness { - id: "opencode", - label: "OpenCode", - command: "opencode", - args: &["acp"], - install_instructions_url: "https://opencode.ai/docs", - install_hint: "Buzz talks to OpenCode through its CLI's ACP mode (opencode acp).", - underlying_cli: None, - }, + // OpenCode moved to KNOWN_ACP_RUNTIMES so it could carry a config_file_path + // — see the note on its entry in discovery.rs. PresetHarness { id: "kimi", label: "Kimi Code", diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..c4f15125ea 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -70,6 +70,36 @@ fn normalizes_claude_and_codex_args_to_empty() { ); } +/// OpenCode was promoted from a preset to a builtin runtime so it could carry +/// a `config_file_path` — its model lives only in its config file. Two things +/// had to survive that move, and both are silent if they break: the runtime +/// must resolve by command (or the config bridge sees no metadata at all), and +/// it must still spawn as `opencode acp` (builtins get their args from +/// `default_agent_args`, not from the preset's `args` list, so an omission here +/// would launch the bare CLI instead of the ACP server). +#[test] +fn opencode_resolves_as_a_builtin_and_keeps_its_acp_arg() { + let runtime = super::known_acp_runtime("opencode").expect("opencode should be a known runtime"); + assert_eq!(runtime.id, "opencode"); + assert_eq!( + runtime.config_file_path, + Some("~/.config/opencode/opencode.json") + ); + assert!( + runtime.model_env_var.is_none(), + "opencode has no model env var — that is why it needs the config file" + ); + + assert_eq!( + normalize_agent_args("opencode", Vec::new()), + vec!["acp".to_string()] + ); + assert_eq!( + normalize_agent_args("opencode", vec!["acp".into()]), + vec!["acp".to_string()] + ); +} + #[test] fn resolves_buzz_agent_avatar() { assert_eq!( diff --git a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx index 5b247c31f7..9ccf2c0ddb 100644 --- a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx +++ b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx @@ -14,8 +14,9 @@ const RUNTIME_LOGOS: Record = { claude: claudeLogoUrl, }; -// Public-path logos for bundled presets. Served from /harness-logos/ at runtime. -// Keys match the preset `id` values emitted by the backend PRESET_HARNESSES. +// Public-path logos for bundled harnesses. Served from /harness-logos/ at runtime. +// Keys match backend runtime ids (PRESET_HARNESSES, plus KNOWN_ACP_RUNTIMES +// entries such as `opencode` that ship no remote avatar). export const PRESET_LOGOS: Record = { devin: "/harness-logos/devin.svg", omp: "/harness-logos/omp.svg", From 5828c56a9f4d54779c52ab8486bccf703b363d5b Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 22:46:36 -0400 Subject: [PATCH 08/10] test(e2e): cover the agent ModelPicker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker was mounted last week and verified by hand in vite dev; the get_agent_models mock override landed with no spec able to reach it, because the catalog field existed only on the app-side E2eConfig and not on the test-side MockBridgeOptions. Targets the non-live branch (standalone agent, no active turns), where a pick persists through update_managed_agent. The live branch publishes a kind-24200 control frame and needs build_observer_control_event plus a relay to carry it — mock plumbing that does not exist yet. Both assertions were mutation-checked: dropping the catalog override fails the menu assertion, and asserting a different model id fails the payload assertion. Co-Authored-By: Claude Opus 5 --- desktop/playwright.config.ts | 1 + desktop/tests/e2e/agent-model-picker.spec.ts | 138 +++++++++++++++++++ desktop/tests/helpers/bridge.ts | 16 +++ 3 files changed, 155 insertions(+) create mode 100644 desktop/tests/e2e/agent-model-picker.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 7ce7f48389..c2b8882e28 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -126,6 +126,7 @@ export default defineConfig({ "**/agent-provider-dropdowns.spec.ts", "**/agent-lifecycle-feedback.spec.ts", "**/agent-access-warning.spec.ts", + "**/agent-model-picker.spec.ts", "**/inbox-live-update.spec.ts", "**/mesh-compute.spec.ts", "**/observer-archive-policy.spec.ts", diff --git a/desktop/tests/e2e/agent-model-picker.spec.ts b/desktop/tests/e2e/agent-model-picker.spec.ts new file mode 100644 index 0000000000..c51a828e7c --- /dev/null +++ b/desktop/tests/e2e/agent-model-picker.spec.ts @@ -0,0 +1,138 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +// A standalone agent: no persona, not running. That combination forces the +// ModelPicker down its non-live branch, where a pick persists the default via +// `update_managed_agent` instead of publishing a kind-24200 `switch_model` +// control frame (which the browser harness has no relay to carry). +const AGENT = TEST_IDENTITIES.tyler; +const AGENT_NAME = "Standalone Helper"; + +const CATALOG = { + models: [ + { id: "openrouter/auto", name: "Auto (OpenRouter)" }, + { id: "anthropic/claude-sonnet-4.5", name: "Claude Sonnet 4.5" }, + // A nameless entry proves the item falls back to the raw model id. + { id: "openai/gpt-5", name: null }, + ], + supportsSwitching: true, + agentDefaultModel: "openrouter/auto", +}; + +async function openAgentsView(page: Page) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + await expect(page.getByTestId("agents-library-personas")).toBeVisible({ + timeout: 10_000, + }); +} + +function commandCount(page: Page, command: string) { + return page.evaluate( + (name) => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (entry) => entry.command === name, + ).length, + command, + ); +} + +test("the picker loads its catalog on first open and persists the pick", async ({ + page, +}) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT.pubkey, + name: AGENT_NAME, + personaId: null, + status: "stopped", + }, + ], + agentModels: CATALOG, + }); + + await openAgentsView(page); + + const card = page.getByTestId(`managed-agent-${AGENT.pubkey}`); + await expect(card).toBeVisible(); + + // With no persisted model and no catalog yet, the trigger reads "Auto" and + // nothing has been fetched — the request is deferred to the first open. + const trigger = card.getByRole("button", { name: "Auto", exact: true }); + await expect(trigger).toBeVisible(); + expect(await commandCount(page, "get_agent_models")).toBe(0); + + await trigger.click(); + + await expect + .poll(() => commandCount(page, "get_agent_models")) + .toBeGreaterThan(0); + + // The seeded catalog renders, including the id fallback for a nameless model. + await expect( + page.getByRole("menuitemradio", { name: "Auto (OpenRouter)" }), + ).toBeVisible(); + await expect( + page.getByRole("menuitemradio", { name: "openai/gpt-5" }), + ).toBeVisible(); + const sonnet = page.getByRole("menuitemradio", { name: "Claude Sonnet 4.5" }); + await expect(sonnet).toBeVisible(); + + const commandsBeforePick = await page.evaluate( + () => window.__BUZZ_E2E_COMMAND_LOG__?.length ?? 0, + ); + await sonnet.click(); + + // The non-live path persists the chosen model as the agent's default. + await expect + .poll(async () => + page.evaluate((start) => { + const commands = window.__BUZZ_E2E_COMMAND_LOG__ ?? []; + return commands + .slice(start) + .some( + (entry) => + entry.command === "update_managed_agent" && + (entry.payload as { input?: { model?: string | null } })?.input + ?.model === "anthropic/claude-sonnet-4.5", + ); + }, commandsBeforePick), + ) + .toBe(true); + + // ...and the refetched agent drives the trigger label. + await expect( + card.getByRole("button", { + name: "anthropic/claude-sonnet-4.5", + exact: true, + }), + ).toBeVisible(); +}); + +test("a runtime that cannot switch models explains itself instead of listing", async ({ + page, +}) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT.pubkey, + name: AGENT_NAME, + personaId: null, + status: "stopped", + }, + ], + agentModels: { ...CATALOG, supportsSwitching: false }, + }); + + await openAgentsView(page); + + const card = page.getByTestId(`managed-agent-${AGENT.pubkey}`); + await card.getByRole("button", { name: "Auto", exact: true }).click(); + + await expect( + page.getByText("This agent uses the runtime's default model."), + ).toBeVisible(); + await expect(page.getByRole("menuitemradio")).toHaveCount(0); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 8a9ab2be11..15e408b24f 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -524,6 +524,22 @@ type MockBridgeOptions = { * returning a catalog. Exercises the discovery-failure UI path. */ discoverAgentModelsError?: string; + /** + * Override the `get_agent_models` mock response — the catalog the + * ModelPicker reads on first open. Without it the bridge always returns an + * empty list with `supportsSwitching: false`, so the populated menu (and the + * model-switch path behind it) is unreachable. + */ + agentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; }; type BridgeOptions = { From 06c78459cb94085bbbed2f4665d6310fd86390d6 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 5 Aug 2026 22:50:21 -0400 Subject: [PATCH 09/10] feat(agents): routing policy table in the agent editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buzz-acp has read a per-turn routing policy since 6bcace1, but nothing in Buzz could write one — the feature was reachable only by hand-editing JSON and setting BUZZ_ROUTING_POLICY yourself. The table lives in the edit dialog's Advanced block, instance-only: the policy file is keyed by pubkey, so there is nothing to edit on a definition that has no agent yet. Rules are name / any-of vs all-of / phrases / model, plus a default model and an enable switch. set_agent_routing_policy owns the file and returns its path; the UI points the env var at it rather than the backend patching env_vars, because the dialog replaces the whole env map on submit and would silently overwrite a backend-side write. Turning routing off with no rules deletes the file AND drops the env var, so nothing dormant is left pointing at a deleted policy. The types mirror buzz_acp::routing::Policy rather than importing it — buzz-acp is a sidecar the desktop talks to across a process boundary, not a library it links. Both sides now assert the same JSON document, so a rename fails a test instead of silently disabling routing (from_env swallows a parse failure by design). The classifier stage has no UI and is carried through opaquely so saving from the table cannot delete a classifier the user wrote by hand. Verified end to end in a browser: saving a rule writes the expected snake_case document and sets BUZZ_ROUTING_POLICY, and a saved policy rehydrates the table on reopen. Both assertions mutation-checked. Co-Authored-By: Claude Opus 5 --- crates/buzz-acp/src/routing.rs | 51 +++ desktop/playwright.config.ts | 1 + .../src/commands/agent_routing_policy.rs | 383 +++++++++++++++++ desktop/src-tauri/src/commands/mod.rs | 2 + desktop/src-tauri/src/lib.rs | 2 + .../agents/ui/AgentInstanceEditDialog.tsx | 1 + .../agents/ui/EditAgentAdvancedFields.tsx | 29 ++ .../agents/ui/RoutingPolicyEditor.tsx | 397 ++++++++++++++++++ desktop/src/shared/api/tauri.ts | 118 ++++++ desktop/src/testing/e2eBridge.ts | 33 ++ .../tests/e2e/agent-routing-policy.spec.ts | 165 ++++++++ 11 files changed, 1182 insertions(+) create mode 100644 desktop/src-tauri/src/commands/agent_routing_policy.rs create mode 100644 desktop/src/features/agents/ui/RoutingPolicyEditor.tsx create mode 100644 desktop/tests/e2e/agent-routing-policy.spec.ts diff --git a/crates/buzz-acp/src/routing.rs b/crates/buzz-acp/src/routing.rs index 6471dcf564..eefd4ea9b9 100644 --- a/crates/buzz-acp/src/routing.rs +++ b/crates/buzz-acp/src/routing.rs @@ -448,6 +448,57 @@ mod tests { ); } + /// 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()); diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index c2b8882e28..740e30d656 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -127,6 +127,7 @@ export default defineConfig({ "**/agent-lifecycle-feedback.spec.ts", "**/agent-access-warning.spec.ts", "**/agent-model-picker.spec.ts", + "**/agent-routing-policy.spec.ts", "**/inbox-live-update.spec.ts", "**/mesh-compute.spec.ts", "**/observer-archive-policy.spec.ts", diff --git a/desktop/src-tauri/src/commands/agent_routing_policy.rs b/desktop/src-tauri/src/commands/agent_routing_policy.rs new file mode 100644 index 0000000000..62b691846c --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_routing_policy.rs @@ -0,0 +1,383 @@ +//! Tauri commands for per-agent model routing policies. +//! +//! `buzz-acp` reads its routing policy from the file named by +//! `BUZZ_ROUTING_POLICY` (see `crates/buzz-acp/src/routing.rs`). These commands +//! own the other half of that contract: they write the file and report where it +//! lives, so the agent dialog can point the env var at it. +//! +//! Setting the env var is deliberately NOT done here. The edit dialog holds the +//! whole `env_vars` map in local state and replaces it wholesale on submit, so a +//! backend-side patch would be silently overwritten by the next save. The +//! frontend merges the returned path into that map instead. +//! +//! The types below mirror the serde shape of `buzz_acp::routing::Policy`. They +//! are duplicated rather than imported because `buzz-acp` is a sidecar the +//! desktop talks to across a process boundary, not a library it links. +//! `policy_shape_matches_the_harness_contract` pins the emitted JSON so a rename +//! on either side fails a test instead of silently disabling routing. + +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; +use tauri::AppHandle; + +use crate::managed_agents::managed_agents_base_dir; + +/// How a rule matches the prompt text. Mirrors `buzz_acp::routing::MatchKind`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "snake_case")] +pub enum MatchKind { + /// Any needle appears in the prompt. + #[default] + Contains, + /// Every needle appears in the prompt. + ContainsAll, +} + +/// One deterministic routing rule. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RoutingRule { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(default)] + pub match_kind: MatchKind, + #[serde(default)] + pub any: Vec, + pub model: String, +} + +/// Optional local classifier, consulted only when no rule matched. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RoutingClassifier { + pub url: String, + pub model: String, + #[serde(default)] + pub labels: Vec, + #[serde(default = "default_timeout_ms")] + pub timeout_ms: u64, +} + +fn default_timeout_ms() -> u64 { + 20_000 +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RoutingLabelTarget { + pub label: String, + pub model: String, +} + +/// A routing policy, in the exact shape `buzz-acp` deserializes. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct RoutingPolicy { + #[serde(default)] + pub enabled: bool, + #[serde(default)] + pub rules: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub classifier: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default_model: Option, +} + +/// Where an agent's policy lives, and what is currently in it. +/// +/// `path` is always populated — the UI needs it to set `BUZZ_ROUTING_POLICY` +/// even on the save that creates the file. `policy` is `None` when nothing has +/// been written yet, or when the file on disk is unreadable/unparseable, which +/// is the same thing the harness would conclude. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentRoutingPolicyFile { + pub path: String, + pub policy: Option, +} + +/// Read the routing policy for `pubkey`, if one has been written. +#[tauri::command] +pub fn get_agent_routing_policy( + pubkey: String, + app: AppHandle, +) -> Result { + let path = routing_policy_path(&app, &pubkey)?; + let policy = std::fs::read_to_string(&path) + .ok() + .and_then(|raw| serde_json::from_str::(&raw).ok()); + Ok(AgentRoutingPolicyFile { + path: path.to_string_lossy().into_owned(), + policy, + }) +} + +/// Write (or, with `policy: None`, delete) the routing policy for `pubkey`. +/// +/// Returns the path in both cases so the caller can set or clear +/// `BUZZ_ROUTING_POLICY` without recomputing it. +#[tauri::command] +pub fn set_agent_routing_policy( + pubkey: String, + policy: Option, + app: AppHandle, +) -> Result { + let path = routing_policy_path(&app, &pubkey)?; + + let Some(policy) = policy else { + match std::fs::remove_file(&path) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(format!("failed to delete routing policy: {error}")), + } + return Ok(AgentRoutingPolicyFile { + path: path.to_string_lossy().into_owned(), + policy: None, + }); + }; + + let policy = normalize_policy(policy)?; + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .map_err(|error| format!("failed to create routing policy dir: {error}"))?; + } + let json = serde_json::to_string_pretty(&policy) + .map_err(|error| format!("failed to serialize routing policy: {error}"))?; + std::fs::write(&path, json) + .map_err(|error| format!("failed to write routing policy: {error}"))?; + + Ok(AgentRoutingPolicyFile { + path: path.to_string_lossy().into_owned(), + policy: Some(policy), + }) +} + +fn routing_policy_path(app: &AppHandle, pubkey: &str) -> Result { + Ok(managed_agents_base_dir(app)? + .join("routing") + .join(format!("{}.json", validated_pubkey(pubkey)?))) +} + +/// The pubkey becomes a filename, so it must not be able to escape the routing +/// directory. Agent pubkeys are hex (or npub), both strictly alphanumeric — +/// rejecting everything else closes the traversal hole at the boundary rather +/// than trusting the caller. +fn validated_pubkey(pubkey: &str) -> Result<&str, String> { + if pubkey.is_empty() || pubkey.len() > 128 { + return Err("routing policy: agent pubkey has an invalid length".to_string()); + } + if !pubkey.chars().all(|c| c.is_ascii_alphanumeric()) { + return Err("routing policy: agent pubkey must be alphanumeric".to_string()); + } + Ok(pubkey) +} + +/// Trim user input and reject rules the harness would silently ignore. +/// +/// A rule with no needles never matches (`routing.rs` makes that explicit so +/// "always route here" cannot be created by omission), and a rule with no model +/// has nothing to route to. Both are user mistakes worth naming at save time +/// rather than discovering as a policy that quietly does nothing. +fn normalize_policy(mut policy: RoutingPolicy) -> Result { + for (index, rule) in policy.rules.iter_mut().enumerate() { + rule.name = rule + .name + .as_deref() + .map(str::trim) + .filter(|name| !name.is_empty()) + .map(str::to_string); + rule.model = rule.model.trim().to_string(); + rule.any = rule + .any + .iter() + .map(|needle| needle.trim().to_string()) + .filter(|needle| !needle.is_empty()) + .collect(); + + let label = rule + .name + .clone() + .unwrap_or_else(|| format!("rule {}", index + 1)); + if rule.model.is_empty() { + return Err(format!("{label} needs a model to route to.")); + } + if rule.any.is_empty() { + return Err(format!( + "{label} needs at least one phrase to match. To route everything, set a default model instead." + )); + } + } + + policy.default_model = policy + .default_model + .as_deref() + .map(str::trim) + .filter(|model| !model.is_empty()) + .map(str::to_string); + + if let Some(classifier) = policy.classifier.as_mut() { + classifier.url = classifier.url.trim().to_string(); + classifier.model = classifier.model.trim().to_string(); + classifier + .labels + .retain(|target| !target.label.trim().is_empty() && !target.model.trim().is_empty()); + if classifier.url.is_empty() || classifier.model.is_empty() { + return Err("The classifier needs both a URL and a model.".to_string()); + } + } + + Ok(policy) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rule(model: &str, any: &[&str]) -> RoutingRule { + RoutingRule { + name: None, + match_kind: MatchKind::Contains, + any: any.iter().map(|s| (*s).to_string()).collect(), + model: model.to_string(), + } + } + + /// Pins the on-disk contract with `crates/buzz-acp/src/routing.rs`. If a + /// field is renamed on either side, routing silently stops working — the + /// harness's `from_env` swallows a parse failure by design. This test is + /// the tripwire; its counterpart is + /// `routing::tests::a_desktop_written_policy_parses`. + #[test] + fn policy_shape_matches_the_harness_contract() { + let policy = RoutingPolicy { + enabled: true, + rules: vec![RoutingRule { + name: Some("db".to_string()), + match_kind: MatchKind::ContainsAll, + any: vec!["migration".to_string()], + model: "codex-model".to_string(), + }], + classifier: Some(RoutingClassifier { + url: "http://localhost:11434".to_string(), + model: "gemma3:27b".to_string(), + labels: vec![RoutingLabelTarget { + label: "database".to_string(), + model: "db-model".to_string(), + }], + timeout_ms: 20_000, + }), + default_model: Some("fallback".to_string()), + }; + + let json: serde_json::Value = serde_json::to_value(&policy).expect("serialize"); + assert_eq!( + json, + serde_json::json!({ + "enabled": true, + "rules": [{ + "name": "db", + "match_kind": "contains_all", + "any": ["migration"], + "model": "codex-model" + }], + "classifier": { + "url": "http://localhost:11434", + "model": "gemma3:27b", + "labels": [{ "label": "database", "model": "db-model" }], + "timeout_ms": 20000 + }, + "default_model": "fallback" + }) + ); + } + + #[test] + fn a_disabled_empty_policy_serializes_without_null_noise() { + let json = serde_json::to_value(RoutingPolicy::default()).expect("serialize"); + assert_eq!(json, serde_json::json!({ "enabled": false, "rules": [] })); + } + + #[test] + fn normalize_trims_and_drops_blank_needles() { + let policy = normalize_policy(RoutingPolicy { + enabled: true, + rules: vec![RoutingRule { + name: Some(" db ".to_string()), + any: vec![" migration ".to_string(), " ".to_string()], + model: " codex ".to_string(), + ..rule("x", &["y"]) + }], + default_model: Some(" ".to_string()), + ..Default::default() + }) + .expect("normalize"); + + assert_eq!(policy.rules[0].name.as_deref(), Some("db")); + assert_eq!(policy.rules[0].any, vec!["migration".to_string()]); + assert_eq!(policy.rules[0].model, "codex"); + assert_eq!( + policy.default_model, None, + "a whitespace-only default model is no default model" + ); + } + + #[test] + fn a_rule_with_no_needles_is_rejected_by_name() { + let err = normalize_policy(RoutingPolicy { + enabled: true, + rules: vec![RoutingRule { + name: Some("catch-all".to_string()), + ..rule("m", &[]) + }], + ..Default::default() + }) + .unwrap_err(); + assert!( + err.contains("catch-all"), + "error should name the rule: {err}" + ); + assert!( + err.contains("default model"), + "error should point at the fix: {err}" + ); + } + + #[test] + fn an_unnamed_bad_rule_is_reported_by_its_position() { + let err = normalize_policy(RoutingPolicy { + enabled: true, + rules: vec![rule("ok-model", &["x"]), rule("", &["y"])], + ..Default::default() + }) + .unwrap_err(); + assert!( + err.starts_with("rule 2"), + "expected a 1-based position: {err}" + ); + } + + #[test] + fn a_classifier_missing_its_url_is_rejected() { + let err = normalize_policy(RoutingPolicy { + enabled: true, + classifier: Some(RoutingClassifier { + url: " ".to_string(), + model: "gemma3:27b".to_string(), + labels: vec![], + timeout_ms: 20_000, + }), + ..Default::default() + }) + .unwrap_err(); + assert!(err.contains("classifier"), "{err}"); + } + + #[test] + fn a_pubkey_that_could_escape_the_routing_dir_is_rejected() { + for bad in ["../../etc/passwd", "a/b", "a\\b", "", "a.b"] { + assert!( + validated_pubkey(bad).is_err(), + "{bad:?} must not be accepted as a filename" + ); + } + assert!(validated_pubkey("deadbeef00").is_ok()); + } +} diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 66ef7ef17b..a2b06376eb 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -7,6 +7,7 @@ mod agent_model_process; mod agent_models; mod agent_models_env; mod agent_providers; +mod agent_routing_policy; mod agent_settings; mod agent_update_rollback; mod agents; @@ -68,6 +69,7 @@ pub use agent_logs::*; pub use agent_metric_archive::*; pub use agent_models::*; pub use agent_providers::*; +pub use agent_routing_policy::*; pub use agent_settings::*; pub use agents::*; pub use canvas::*; diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c4b733e3e0..94c2f09f78 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -819,6 +819,8 @@ pub fn run() { get_baked_build_env_keys, get_baked_build_env, put_agent_session_config, + get_agent_routing_policy, + set_agent_routing_policy, get_global_agent_config, set_global_agent_config, mesh_start_node, diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 79d1e9a790..4e33b9efdf 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -1200,6 +1200,7 @@ export function AgentInstanceEditDialog({ parallelism={parallelism} provider={effectiveProvider} requiredEnvKeys={advancedRequiredEnvKeys} + routingPolicyPubkey={agent.pubkey} systemPrompt={systemPrompt} onAcpCommandChange={setAcpCommand} onAgentArgsChange={setAgentArgs} diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index 8412792a2c..3c23cc0ecd 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -10,6 +10,10 @@ import { import type { AgentPersona } from "@/shared/api/types"; import { BuzzAgentModelTuningFields } from "./buzzAgentModelTuningFields"; import { isBuzzAgentRuntime } from "./buzzAgentConfig"; +import { + ROUTING_POLICY_ENV_KEY, + RoutingPolicyEditor, +} from "./RoutingPolicyEditor"; export function EditAgentAdvancedFields({ acpCommand, @@ -28,6 +32,7 @@ export function EditAgentAdvancedFields({ parallelism, provider, requiredEnvKeys, + routingPolicyPubkey, systemPrompt, onAcpCommandChange, onAgentArgsChange, @@ -61,6 +66,12 @@ export function EditAgentAdvancedFields({ /** Active LLM provider id — forwarded to BuzzAgentModelTuningFields for effort filtering. */ provider?: string; requiredEnvKeys: readonly string[]; + /** + * Agent pubkey, when this form edits a live agent instance. Enables the + * routing-policy table — the policy file is keyed by pubkey, so there is + * nothing to edit on a template/definition that has no agent yet. + */ + routingPolicyPubkey?: string; systemPrompt: string; onAcpCommandChange: (value: string) => void; onAgentArgsChange: (value: string) => void; @@ -270,6 +281,24 @@ export function EditAgentAdvancedFields({ provider={provider} /> ) : null} + + {/* Per-turn model routing — instance-only (the policy file is keyed by pubkey). */} + {routingPolicyPubkey ? ( + { + const next = { ...envVars }; + if (value === "") { + delete next[key]; + } else { + next[key] = value; + } + onEnvVarsChange(next); + }} + pubkey={routingPolicyPubkey} + /> + ) : null} ); } diff --git a/desktop/src/features/agents/ui/RoutingPolicyEditor.tsx b/desktop/src/features/agents/ui/RoutingPolicyEditor.tsx new file mode 100644 index 0000000000..4ec0f4cca4 --- /dev/null +++ b/desktop/src/features/agents/ui/RoutingPolicyEditor.tsx @@ -0,0 +1,397 @@ +import * as React from "react"; +import { Plus, X } from "lucide-react"; + +import { cn } from "@/shared/lib/cn"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; +import { Switch } from "@/shared/ui/switch"; +import { + getAgentRoutingPolicy, + setAgentRoutingPolicy, + type RoutingMatchKind, + type RoutingPolicy, +} from "@/shared/api/tauri"; +import { + PERSONA_FIELD_CONTROL_CLASS, + PERSONA_FIELD_SHELL_CLASS, + PERSONA_LABEL_OPTIONAL_CLASS, +} from "./agentConfigOptions"; + +/** Env var the harness reads the policy path from (`buzz-acp` routing.rs). */ +export const ROUTING_POLICY_ENV_KEY = "BUZZ_ROUTING_POLICY"; + +/** A rule row. `id` is local only — it keeps React keys stable while typing. */ +type RuleRow = { + id: string; + name: string; + matchKind: RoutingMatchKind; + /** Comma-separated in the UI; split on save. */ + phrases: string; + model: string; +}; + +function newRuleId(): string { + return crypto.randomUUID(); +} + +function toRows(policy: RoutingPolicy | null): RuleRow[] { + return (policy?.rules ?? []).map((rule) => ({ + id: newRuleId(), + name: rule.name ?? "", + matchKind: rule.matchKind, + phrases: rule.any.join(", "), + model: rule.model, + })); +} + +function splitPhrases(raw: string): string[] { + return raw + .split(",") + .map((phrase) => phrase.trim()) + .filter((phrase) => phrase.length > 0); +} + +/** + * Per-turn model routing for one agent. + * + * Writes the JSON policy file that `buzz-acp` reads, then points the agent's + * `BUZZ_ROUTING_POLICY` env var at it via `onEnvVarChange`. The env change is + * staged in the dialog's env map and lands with the dialog's own save — the + * file write happens immediately, because the file is not part of the agent + * record and has nothing to wait for. + * + * Routing is opt-in and fails open on the harness side: an unreadable or + * disabled policy means turns run on the agent's configured model, exactly as + * they did before. The UI mirrors that — turning routing off deletes the file + * and drops the env var rather than leaving a dormant one behind. + */ +export function RoutingPolicyEditor({ + disabled, + envValue, + pubkey, + onEnvVarChange, +}: { + disabled: boolean; + /** Current `BUZZ_ROUTING_POLICY` value in the dialog's env map, if any. */ + envValue: string | undefined; + pubkey: string; + onEnvVarChange: (key: string, value: string) => void; +}) { + const [loaded, setLoaded] = React.useState(false); + const [enabled, setEnabled] = React.useState(false); + const [rows, setRows] = React.useState([]); + const [defaultModel, setDefaultModel] = React.useState(""); + const [path, setPath] = React.useState(null); + const [classifier, setClassifier] = React.useState(undefined); + const [saving, setSaving] = React.useState(false); + const [error, setError] = React.useState(null); + const [savedAt, setSavedAt] = React.useState(null); + + React.useEffect(() => { + let cancelled = false; + void getAgentRoutingPolicy(pubkey) + .then((file) => { + if (cancelled) return; + setPath(file.path); + setEnabled(file.policy?.enabled ?? false); + setRows(toRows(file.policy)); + setDefaultModel(file.policy?.defaultModel ?? ""); + setClassifier(file.policy?.classifier); + setLoaded(true); + }) + .catch((loadError: unknown) => { + if (cancelled) return; + setError( + loadError instanceof Error ? loadError.message : String(loadError), + ); + setLoaded(true); + }); + return () => { + cancelled = true; + }; + }, [pubkey]); + + const updateRow = (id: string, patch: Partial) => { + setRows((current) => + current.map((row) => (row.id === id ? { ...row, ...patch } : row)), + ); + setSavedAt(null); + }; + + const handleSave = async () => { + setSaving(true); + setError(null); + try { + if (!enabled && rows.length === 0) { + // Nothing to route with. Delete the file and drop the env var so the + // agent is left in exactly the state it had before routing was touched. + const file = await setAgentRoutingPolicy(pubkey, null); + setPath(file.path); + onEnvVarChange(ROUTING_POLICY_ENV_KEY, ""); + setSavedAt(Date.now()); + return; + } + + const policy: RoutingPolicy = { + enabled, + rules: rows.map((row) => ({ + name: row.name.trim() ? row.name.trim() : null, + matchKind: row.matchKind, + any: splitPhrases(row.phrases), + model: row.model, + })), + defaultModel: defaultModel.trim() ? defaultModel.trim() : null, + classifier, + }; + const file = await setAgentRoutingPolicy(pubkey, policy); + setPath(file.path); + onEnvVarChange(ROUTING_POLICY_ENV_KEY, file.path); + setSavedAt(Date.now()); + } catch (saveError: unknown) { + setError( + saveError instanceof Error ? saveError.message : String(saveError), + ); + } finally { + setSaving(false); + } + }; + + const envPointsAtPolicy = !!envValue && !!path && envValue === path; + + return ( +
+
+
+

+ Model routing + optional +

+

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

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

Loading routing policy…

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

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

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

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

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

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

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

+ {error} +

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

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

+ ) : null} + + )} +
+ ); +} diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 69e2e455ec..ffcce40391 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -1013,6 +1013,124 @@ export async function putAgentSessionConfig( return invokeTauri("put_agent_session_config", { pubkey, payload }); } +// ── Per-turn model routing ──────────────────────────────────────────────────── +// +// The harness reads its policy from the file named by `BUZZ_ROUTING_POLICY` +// (crates/buzz-acp/src/routing.rs). These commands own the file; pointing the +// env var at the returned `path` is the caller's job, because the edit dialog +// replaces the whole env map on submit and would overwrite a backend patch. + +export type RoutingMatchKind = "contains" | "contains_all"; + +export type RoutingRule = { + name?: string | null; + matchKind: RoutingMatchKind; + /** Needles to look for. An empty list is rejected at save time. */ + any: string[]; + model: string; +}; + +export type RoutingPolicy = { + enabled: boolean; + rules: RoutingRule[]; + defaultModel?: string | null; + /** + * The optional local-Ollama classifier stage. Buzz has no UI for it, so it is + * carried through opaquely — saving from the rules table must not silently + * delete a classifier the user wrote into the file by hand. + */ + classifier?: unknown; +}; + +export type AgentRoutingPolicyFile = { + /** Where the policy lives — set `BUZZ_ROUTING_POLICY` to this. */ + path: string; + /** `null` when nothing has been written yet. */ + policy: RoutingPolicy | null; +}; + +/** + * Wire shape. The Rust side mirrors `buzz_acp::routing::Policy` verbatim, which + * is snake_case, so these fields are NOT camelCase like the rest of our API — + * the file has to be readable by the harness, not by us. + */ +type RawRoutingPolicy = { + enabled: boolean; + rules: { + name?: string | null; + match_kind: RoutingMatchKind; + any: string[]; + model: string; + }[]; + default_model?: string | null; + classifier?: unknown; +}; + +type RawAgentRoutingPolicyFile = { + path: string; + policy: RawRoutingPolicy | null; +}; + +function fromRawRoutingPolicy(raw: RawRoutingPolicy): RoutingPolicy { + return { + enabled: raw.enabled, + rules: (raw.rules ?? []).map((rule) => ({ + name: rule.name ?? null, + matchKind: rule.match_kind ?? "contains", + any: rule.any ?? [], + model: rule.model, + })), + defaultModel: raw.default_model ?? null, + }; +} + +function toRawRoutingPolicy(policy: RoutingPolicy): RawRoutingPolicy { + return { + enabled: policy.enabled, + rules: policy.rules.map((rule) => ({ + name: rule.name?.trim() ? rule.name.trim() : null, + match_kind: rule.matchKind, + any: rule.any, + model: rule.model, + })), + default_model: policy.defaultModel?.trim() + ? policy.defaultModel.trim() + : null, + }; +} + +function fromRawRoutingPolicyFile( + raw: RawAgentRoutingPolicyFile, +): AgentRoutingPolicyFile { + return { + path: raw.path, + policy: raw.policy ? fromRawRoutingPolicy(raw.policy) : null, + }; +} + +export async function getAgentRoutingPolicy( + pubkey: string, +): Promise { + return fromRawRoutingPolicyFile( + await invokeTauri("get_agent_routing_policy", { + pubkey, + }), + ); +} + +/** Pass `null` to delete the policy file. */ +export async function setAgentRoutingPolicy( + pubkey: string, + policy: RoutingPolicy | null, +): Promise { + return fromRawRoutingPolicyFile( + await invokeTauri("set_agent_routing_policy", { + pubkey, + policy: policy ? toRawRoutingPolicy(policy) : null, + }), + ); +} + /** File-layer config for a runtime (e.g. `~/.config/goose/config.yaml`). */ export type RuntimeFileConfigSubset = { /** Provider set in the harness config file. */ diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 9b7ab976b7..17c4a1951e 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -7353,6 +7353,12 @@ let mockGlobalAgentConfig: { model: string | null; preferred_runtime?: string | null; } | null = null; +/** + * Routing policies written through `set_agent_routing_policy`, keyed by pubkey. + * Opaque on purpose: the shape is the harness's, and the mock only has to hand + * back what it was given. + */ +const mockRoutingPolicies = new Map(); // Per-page get_nsec call counter for sequenced error testing. let nsecCallCount = 0; @@ -9630,6 +9636,7 @@ export function maybeInstallE2eTauriMocks() { mockGlobalAgentConfig = config.mock?.globalAgentConfig ? { ...config.mock.globalAgentConfig } : null; + mockRoutingPolicies.clear(); resetMockRelayMembers(config); resetMockRelayAgents(config); resetMockManagedAgents(config); @@ -11599,6 +11606,32 @@ export function maybeInstallE2eTauriMocks() { const configArgs = payload as { pubkey: string }; return buildMockConfigSurface(configArgs.pubkey); } + // Per-turn model routing. Kept in memory so the edit dialog's routing + // table round-trips inside a test the way it does against the real + // command — without a mock the editor's mount-time read would throw into + // its own error state and every Advanced-panel test would see it. + case "get_agent_routing_policy": { + const { pubkey } = payload as { pubkey: string }; + return { + path: `/mock/agents/routing/${pubkey}.json`, + policy: mockRoutingPolicies.get(pubkey) ?? null, + }; + } + case "set_agent_routing_policy": { + const { pubkey, policy } = payload as { + pubkey: string; + policy: unknown; + }; + if (policy === null || policy === undefined) { + mockRoutingPolicies.delete(pubkey); + } else { + mockRoutingPolicies.set(pubkey, policy); + } + return { + path: `/mock/agents/routing/${pubkey}.json`, + policy: mockRoutingPolicies.get(pubkey) ?? null, + }; + } case "get_runtime_file_config": { const runtimeId = (payload as { runtimeId?: string } | null | undefined) ?.runtimeId; diff --git a/desktop/tests/e2e/agent-routing-policy.spec.ts b/desktop/tests/e2e/agent-routing-policy.spec.ts new file mode 100644 index 0000000000..d037cef1b4 --- /dev/null +++ b/desktop/tests/e2e/agent-routing-policy.spec.ts @@ -0,0 +1,165 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +// Per-turn model routing (`crates/buzz-acp/src/routing.rs`) is opt-in through +// the `BUZZ_ROUTING_POLICY` env var, which must name a policy file. This spec +// pins the UI half of that contract: the routing table writes the policy AND +// points the env var at the returned path. Either half alone does nothing — +// a policy file nothing references never gets read, and an env var pointing at +// a missing file makes the harness fail open and route nothing. + +const AGENT_PUBKEY = TEST_IDENTITIES.tyler.pubkey; +const AGENT_NAME = "Tyler Agent"; + +async function openAdvanced(page: import("@playwright/test").Page) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + + const agentButton = page.getByRole("button", { + name: `${AGENT_NAME} agent profile`, + }); + await expect(agentButton).toBeVisible({ timeout: 10_000 }); + await agentButton.click(); + + await expect(page.getByTestId("user-profile-panel")).toBeVisible({ + timeout: 10_000, + }); + await page.getByTestId("user-profile-edit-agent").click(); + await expect(page.getByTestId("edit-agent-dialog")).toBeVisible({ + timeout: 10_000, + }); + await page.getByRole("button", { name: "Advanced" }).click(); + await expect(page.getByTestId("routing-policy-editor")).toBeVisible({ + timeout: 10_000, + }); +} + +test.describe("agent routing policy", () => { + test("saving a rule writes the policy and points BUZZ_ROUTING_POLICY at it", async ({ + page, + }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped", + channelNames: ["agents"], + }, + ], + }); + + await openAdvanced(page); + + // Until routing is switched on and saved, the agent carries no policy var. + await expect(page.getByTestId("routing-policy-save")).toBeVisible(); + + await page.getByTestId("routing-policy-enabled").click(); + await page.getByTestId("routing-rule-add").click(); + + await page.getByTestId("routing-rule-name").fill("db"); + await page + .getByTestId("routing-rule-match-kind") + .selectOption("contains_all"); + await page.getByTestId("routing-rule-phrases").fill("migration, schema"); + await page.getByTestId("routing-rule-model").fill("codex-model"); + await page.getByTestId("routing-default-model").fill("fallback-model"); + + const commandsBefore = await page.evaluate( + () => window.__BUZZ_E2E_COMMAND_LOG__?.length ?? 0, + ); + await page.getByTestId("routing-policy-save").click(); + + // The policy reaching the backend is the load-bearing half. Assert the + // snake_case wire shape, not our camelCase view model — the harness parses + // this document, so a rename here silently disables routing. + await expect + .poll(async () => + page.evaluate( + (start) => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []) + .slice(start) + .find((entry) => entry.command === "set_agent_routing_policy") + ?.payload ?? null, + commandsBefore, + ), + ) + .toEqual({ + pubkey: AGENT_PUBKEY, + policy: { + enabled: true, + rules: [ + { + name: "db", + match_kind: "contains_all", + any: ["migration", "schema"], + model: "codex-model", + }, + ], + default_model: "fallback-model", + }, + }); + + await expect(page.getByTestId("routing-policy-error")).toHaveCount(0); + + // ...and the other half: the env var now names the saved file. Read the + // live input values — React controlled inputs do not mirror `value` into a + // DOM attribute, so an attribute selector would pass vacuously. + await expect + .poll(async () => { + const keys = await page + .getByTestId("env-vars-key") + .evaluateAll((nodes) => + nodes.map((node) => (node as HTMLInputElement).value), + ); + const values = await page + .getByTestId("env-vars-value") + .evaluateAll((nodes) => + nodes.map((node) => (node as HTMLInputElement).value), + ); + const index = keys.indexOf("BUZZ_ROUTING_POLICY"); + return index === -1 ? null : values[index]; + }) + .toBe(`/mock/agents/routing/${AGENT_PUBKEY}.json`); + }); + + test("a saved policy is read back when the dialog is reopened", async ({ + page, + }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped", + channelNames: ["agents"], + }, + ], + }); + + await openAdvanced(page); + await page.getByTestId("routing-policy-enabled").click(); + await page.getByTestId("routing-rule-add").click(); + await page.getByTestId("routing-rule-name").fill("ui"); + await page.getByTestId("routing-rule-phrases").fill("button"); + await page.getByTestId("routing-rule-model").fill("ui-model"); + await page.getByTestId("routing-policy-save").click(); + await expect(page.getByTestId("routing-policy-error")).toHaveCount(0); + + // Close and reopen: the table is hydrated from the stored policy, not from + // component state that happened to survive. + await page.keyboard.press("Escape"); + await expect(page.getByTestId("edit-agent-dialog")).not.toBeVisible(); + await page.getByTestId("user-profile-edit-agent").click(); + await page.getByRole("button", { name: "Advanced" }).click(); + + await expect(page.getByTestId("routing-rule-name")).toHaveValue("ui"); + await expect(page.getByTestId("routing-rule-phrases")).toHaveValue( + "button", + ); + await expect(page.getByTestId("routing-rule-model")).toHaveValue( + "ui-model", + ); + }); +}); From 556fb203f949eaee6181bff956270fabe3a53962 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Thu, 6 Aug 2026 21:15:48 -0400 Subject: [PATCH 10/10] =?UTF-8?q?feat(acp):=20harness-class=20decline=20ga?= =?UTF-8?q?te=20=E2=80=94=20data=20model=20+=20decision=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends per-turn routing with an optional `harness` block that picks a harness *class* (claude/opencode/codex) for a turn, distinct from the model the existing router selects. Consumed by an ingress decline gate (not wired yet): each harness-agent runs the same deterministic decision and skips a turn another class owns, since the relay already delivered it to every subscribed process. Mutates nothing, emits no wire frame — less privileged than the model router. Deterministic rules only and no `classifier` field (deny_unknown_fields): the decision is distributed across independent processes and must be reproducible so exactly one handles the turn. Fail-open throughout — absent block, no match, or self-owned turn all leave behavior unchanged. This is slice 1 of the design: pure `routing.rs` logic + unit tests, no ingress wiring. Back-compatible via #[serde(default)]. Verify: cargo test -p buzz-acp --lib -- routing::tests Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QR3RWXAuVDN3RSwHhsi3DH --- crates/buzz-acp/src/routing.rs | 220 +++++++++++++++++++++++++++++++-- 1 file changed, 209 insertions(+), 11 deletions(-) diff --git a/crates/buzz-acp/src/routing.rs b/crates/buzz-acp/src/routing.rs index eefd4ea9b9..f427dd1b02 100644 --- a/crates/buzz-acp/src/routing.rs +++ b/crates/buzz-acp/src/routing.rs @@ -75,19 +75,26 @@ pub struct Rule { 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 { - if self.any.is_empty() { - return false; - } - let hit = |needle: &String| { - let n = needle.trim().to_lowercase(); - !n.is_empty() && haystack_lower.contains(&n) - }; - match self.match_kind { - MatchKind::Contains => self.any.iter().any(hit), - MatchKind::ContainsAll => self.any.iter().all(hit), - } + any_contains(self.match_kind, &self.any, haystack_lower) } } @@ -115,6 +122,71 @@ pub struct LabelTarget { 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 { @@ -128,6 +200,10 @@ pub struct Policy { /// 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 @@ -209,6 +285,55 @@ impl Policy { }) } + /// Which harness class should own this turn. `None` => no opinion + /// (fail-open). Deterministic and IO-free, mirroring [`decide_static`], so + /// every process reaches the same answer and exactly one handles the turn. + /// + /// [`decide_static`]: Policy::decide_static + pub fn decide_harness(&self, prompt: &str) -> Option { + if !self.enabled { + return None; + } + let h = self.harness.as_ref()?; + let lower = prompt.to_lowercase(); + for (i, rule) in h.rules.iter().enumerate() { + if rule.matches(&lower) { + return Some(HarnessDecision { + class: rule.class.clone(), + reason: HarnessReason::Rule( + rule.name + .clone() + .unwrap_or_else(|| format!("harness.rules[{i}]")), + ), + }); + } + } + h.default_class.clone().map(|class| HarnessDecision { + class, + reason: HarnessReason::Default, + }) + } + + /// Should the process whose class is `self_class` DECLINE this turn? + /// + /// Returns `Some(target)` only when a *different* class owns the turn — the + /// caller then skips its local enqueue and a matching-class agent takes it. + /// Returns `None` in every fail-open case: routing disabled, no harness + /// block, no rule matched with no `default_class`, or the target equals this + /// process's class. The harness block's `self_class` overrides the passed + /// `self_class`, so one shared block is portable across a group. + pub fn harness_decline(&self, prompt: &str, self_class: &str) -> Option { + let self_class = self + .harness + .as_ref() + .and_then(|h| h.self_class.as_deref()) + .unwrap_or(self_class); + match self.decide_harness(prompt) { + Some(d) if !d.class.eq_ignore_ascii_case(self_class) => Some(d), + _ => None, + } + } + /// Full decision for a turn: rules, then classifier, then default. /// /// Never returns an error. Any classifier failure is logged and treated as @@ -382,6 +507,79 @@ mod tests { 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!({