feat(acp): harness-class decline gate — data model + decision logic - #5131
Draft
mfethe1 wants to merge 10 commits into
Draft
feat(acp): harness-class decline gate — data model + decision logic#5131mfethe1 wants to merge 10 commits into
mfethe1 wants to merge 10 commits into
Conversation
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 <noreply@anthropic.com>
…equest 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
`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 <noreply@anthropic.com>
Local tooling scratch that has no business in the repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QR3RWXAuVDN3RSwHhsi3DH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Slice 1 of a harness-class dispatcher: extends the existing per-turn routing (
crates/buzz-acp/src/routing.rs) with an optionalharnessblock that picks a harness class (claude / opencode / codex) for a turn — distinct from the model the current router already selects.It is designed as an in-process decline gate, not a re-router. Because one process = one agent keypair = one harness (frozen at spawn), the relay fan-out already delivers a turn to every subscribed process. Each harness-agent runs the same deterministic decision and simply skips turns another class owns. The gate mutates nothing and emits no wire frame — strictly less privileged than the model router: no
desired_modelwrite, no owner-signed control frame, no re-addressing.Why deterministic-only (no classifier here)
The decline is distributed across independent processes, so it must be reproducible — every process must reach the same target class so exactly one handles the turn. A non-deterministic classifier could make two processes disagree and yield zero handlers (a dropped turn), violating the fail-open contract. So
HarnessRoutinghas deterministic rules +default_classonly, and#[serde(deny_unknown_fields)]rejects a strayclassifier. Semantic harness routing needs a single decision authority and is deferred to a later mesh-level dispatcher.Scope of this PR
Pure
routing.rslogic + unit tests. No ingress wiring yet —decide_harness/harness_declineare landed as testable units the waydecide_staticwas landed independently of its apply site. Back-compatible via#[serde(default)]: every existing desktop-written policy still parses.Follow-on slices: (2) wire the gate at the ingress
queue.pushpoint + integration test; (3) desktop mirror types + contract test; (4) editor UI; (5, only if needed) the semantic re-mention dispatcher.Fail-open audit
harness_declinereturnsNone(→ handle the turn, unchanged behavior) for: routing disabled, noharnessblock, no rule matched with nodefault_class, or target class == this process's class. A turn drops only if a rule/default_classnames a class no running agent has — operator misconfiguration, analogous to naming a model the provider doesn't advertise.Test
12 passed (2 new:
harness_rules_pick_a_class_and_an_absent_block_is_fail_open,a_harness_block_with_a_classifier_field_is_rejected).🤖 Generated with Claude Code