From 985cdcc6eac33ccd77bc50c26e22c701d07eda4e Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 3 Aug 2026 18:09:24 -0400 Subject: [PATCH] feat(agents): model-tuning parity in global Agent Defaults editor (#4578) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Overview The global Agent Defaults surface (Settings card, defaults modal, onboarding) exposed structured controls for Effort but left Max Output Tokens, Context Limit, and Max Rounds as raw env vars. Per-agent dialogs had structured numeric fields but only for `isBuzzAgentRuntime` — incorrectly excluding Goose. This PR unifies numeric-tuning capability across all surfaces, fixes a pre-existing dual-editor defect, and adds full test coverage. ## What changed ### Phase 1 — Catalog projection - Add `max_rounds_env_var` to `KnownAcpRuntime` in `runtime_metadata.rs` (`Some("BUZZ_AGENT_MAX_ROUNDS")` for buzz-agent, `None` elsewhere). - Project all three numeric env-var fields (`max_tokens_env_var`, `context_limit_env_var`, `max_rounds_env_var`) end-to-end: `AcpRuntimeCatalogEntry` Rust struct, TS `types.ts`, `RawAcpRuntimeCatalogEntry` + `fromRawAcpRuntimeCatalogEntry` in `tauri.ts`, and the e2e mock bridge (`withMockRuntimeConfigMetadata`). ### Phase 2 — Field model - `deriveAgentConfigFieldModel` now derives `maxOutputTokens` / `contextLimit` / `maxRounds` descriptors from catalog-projected fields. - `structuredEnvKeys(descriptors)` — exported helper that takes the **rendered** descriptor set (not the whole model). Hidden keys follow what is actually rendered per surface: global hides effort + all three numeric keys for buzz-agent / two for Goose; per-agent buzz-agent hides effort + three numeric keys; per-agent Goose hides only its two numeric keys. `BUZZ_AGENT_THINKING_EFFORT` stays a visible generic env row per-agent because no effort control renders there. ### Phase 3 — UI - Extract `NumericTuningFields` from `buzzAgentModelTuningFields.tsx` as a shared descriptor-driven component (`descriptors`, `envVars`, `inheritedEnvVars`, `onEnvVarChange`). Kind-specific minima: `NUMERIC_KIND_MIN` map (`maxOutputTokens`/`contextLimit`: 1, `maxRounds`: 0) applied to ``. - **Global surface** (`AgentConfigFields.tsx`): deduplicate the previously duplicated Advanced env-editor block; render `NumericTuningFields` below the env editor when descriptors exist; `hiddenKeys` and `bakedGenericRows` exclusions use `structuredEnvKeys` so structured keys are never double-rendered. Under 1000 lines. - **Per-agent surfaces** (`EditAgentAdvancedFields`, `PersonaAdvancedFields`): replace `isBuzzAgentRuntime` as the numeric-field gate with `deriveNumericDescriptors(selectedRuntime)` from `agentConfigCore`; hidden keys come from `structuredEnvKeys(numericDescriptors)` — the same rendered descriptor set, no local rebuilding (fixes pre-existing dual-editor defect). Catalog status carried as `RuntimeCatalogStatus` (`loading | ready | error`); both error and loading withhold structured controls and leave saved values visible as generic rows, making error distinguishable from "runtime not capable" (`ready` + no runtime). - **Dialogs** (`AgentDefinitionDialog`, `AgentInstanceEditDialog`, callers): `AgentDefinitionDialog` accepts `runtimeCatalogStatus?: "loading" | "ready" | "error"` (replaces separate `runtimesLoading`/`runtimesError` booleans); all call sites — `AgentManagementDialogs`, `AgentsView`, `RequestedAgentCreateDialogs`, `UserProfilePersonaDialogs` — compute and pass the status. ### Phase 4 — Tests - `buildRecord` exported from `EnvVarsEditor.tsx` as a pure `(nextRows, value, requiredKeys, hiddenKeys) => Record` helper for isolation testing. - **17 new node tests** in `agentConfigCore.test.mjs`: `deriveNumericDescriptors` (all three fields, partial, undefined runtime, matches field-model subset); `structuredEnvKeys` per surface including discriminating Goose per-agent effort-key invariant; `NUMERIC_KIND_MIN` values. - **4 new node tests** in `EnvVarsEditor.test.mjs`: hidden tuning key preserved through generic row edits; runtime-switch then generic edit (derives both descriptor sets, asserts new-runtime hidden key survives `buildRecord` via `hiddenKeys` and old-runtime key survives via generic rows); baked numeric key excluded via `filterBakedGenericRows` with `numericTuningPlaceholder` assertion; clearing a structured override — `numericTuningPlaceholder` verifies placeholder text. - **5 new Playwright tests** in `agent-numeric-tuning.spec.ts` (added to smoke project `testMatch`): global numeric fields visible for buzz-agent; global: non-capable runtime hides numeric controls; Goose per-agent shows `Inherit (16384)` after saving global value through the UI; delayed catalog: saved values visible as generic rows while loading then structured controls appear after settle; failed catalog: saved values remain visible as generic rows (never the "unsupported" empty state). ## Result - buzz-agent global defaults: Max output tokens, Context limit, Max rounds as structured inputs with `Inherit (N)` placeholders from baked env. - Goose global defaults: Max output tokens, Context limit as structured inputs. - A Goose global value surfaces as `Inherit ()` in the per-agent Goose edit dialog. - No structured key is editable in two places on any surface; no persisted key has zero editors. - No `runtime.id === "buzz-agent"` comparison decides numeric-field visibility anywhere — capability flows catalog → `AcpRuntimeCatalogEntry` → field model → UI. Signed-off-by: Will Pfleger Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 --- desktop/playwright.config.ts | 1 + .../src-tauri/src/commands/agent_config.rs | 3 +- .../src/commands/agent_config_tests.rs | 1 + .../src-tauri/src/commands/agent_discovery.rs | 27 +- .../config_bridge/reader_tests.rs | 2 + .../src-tauri/src/managed_agents/discovery.rs | 39 +- .../src/managed_agents/discovery/presets.rs | 3 + .../discovery/runtime_metadata.rs | 2 + .../src-tauri/src/managed_agents/readiness.rs | 12 +- desktop/src-tauri/src/managed_agents/types.rs | 12 +- .../agents/lib/agentConfigCore.test.mjs | 411 +++++++++++++++++- .../features/agents/lib/agentConfigCore.ts | 157 +++++++ .../features/agents/ui/AgentConfigFields.tsx | 126 +++--- .../agents/ui/AgentDefinitionDialog.tsx | 13 +- .../src/features/agents/ui/AgentDialog.tsx | 8 +- .../agents/ui/AgentInstanceEditDialog.tsx | 38 +- .../agents/ui/AgentManagementDialogs.tsx | 4 +- desktop/src/features/agents/ui/AgentsView.tsx | 16 +- .../agents/ui/EditAgentAdvancedFields.tsx | 85 +++- .../features/agents/ui/EnvVarsEditor.test.mjs | 308 +++++++++++++ .../src/features/agents/ui/EnvVarsEditor.tsx | 39 +- .../agents/ui/PersonaAdvancedFields.tsx | 79 +++- .../agents/ui/RequestedAgentCreateDialogs.tsx | 8 +- .../agents/ui/buzzAgentModelTuningFields.tsx | 196 ++++----- .../src/features/agents/useAgentManagement.ts | 6 +- .../features/profile/ui/UserProfilePanel.tsx | 1 + .../profile/ui/UserProfilePersonaDialogs.tsx | 9 +- desktop/src/shared/api/tauri.ts | 26 +- desktop/src/shared/api/types.ts | 10 +- desktop/src/testing/e2eBridge.ts | 56 ++- .../tests/e2e/agent-numeric-tuning.spec.ts | 372 ++++++++++++++++ desktop/tests/helpers/bridge.ts | 21 +- 32 files changed, 1769 insertions(+), 322 deletions(-) create mode 100644 desktop/tests/e2e/agent-numeric-tuning.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 796fdede1..f5c1e34a5 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -136,6 +136,7 @@ export default defineConfig({ "**/inline-custom-harness.spec.ts", "**/where-to-run-config.spec.ts", "**/huddle-transcription.spec.ts", + "**/agent-numeric-tuning.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 7aded7959..12e6983ee 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -31,8 +31,7 @@ pub struct RuntimeFileConfigSubset { pub provider: Option, /// Model set in the harness config file, if any. pub model: Option, - /// Flat credential env keys found in the harness config file's `extra` map - /// (e.g. `DATABRICKS_HOST`). Only non-empty values are included. + /// Flat credential env keys in the harness config file's `extra` map (e.g. `DATABRICKS_HOST`); only non-empty values included. pub satisfied_env_keys: Vec, } diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index f3667cff4..551915357 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -57,6 +57,7 @@ fn goose_runtime() -> &'static KnownAcpRuntime { thinking_env_var: Some("GOOSE_THINKING_EFFORT"), max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + max_rounds_env_var: None, required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 4abf53ee9..0eb024a86 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -21,25 +21,13 @@ fn active_installs() -> &'static std::sync::Mutex( runtime_id: &str, adapter_path: Option<&std::path::Path>, @@ -177,6 +165,9 @@ pub async fn save_custom_harness( model_env_var: None, provider_env_var: None, thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, install_hint: definition.install_hint, install_instructions_url: definition.install_instructions_url, can_auto_install: false, 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 153db1bbd..62caffeb2 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 @@ -56,6 +56,7 @@ fn test_runtime() -> &'static KnownAcpRuntime { thinking_env_var: Some("GOOSE_THINKING_EFFORT"), max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + max_rounds_env_var: None, required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, @@ -644,6 +645,7 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 248625ce9..2cccccb95 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -103,6 +103,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ thinking_env_var: Some("GOOSE_THINKING_EFFORT"), max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + max_rounds_env_var: None, required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, @@ -135,6 +136,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ thinking_env_var: None, max_tokens_env_var: None, context_limit_env_var: None, + max_rounds_env_var: None, required_normalized_fields: &[], login_hint: Some("Run the Claude CLI to complete authentication."), auth_probe_args: Some(&["claude", "auth", "status"]), @@ -167,6 +169,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ thinking_env_var: None, max_tokens_env_var: None, context_limit_env_var: None, + max_rounds_env_var: None, required_normalized_fields: &[], login_hint: Some("Run `codex login` to authenticate."), // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. @@ -200,6 +203,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, @@ -278,11 +282,8 @@ pub(crate) fn known_acp_runtime_exact(id: &str) -> Option<&'static KnownAcpRunti /// The agent command a freshly-created agent defaults to when the create /// request supplies none. Resolves the bundled `buzz-agent` from the catalog so /// the default cannot drift from the provider definition. Falls back to the id -/// if the catalog entry is missing. -/// -/// The previous default was the bare global `goose`, which is not on PATH on a -/// stock Windows install: every worker failed with `program not found`. The -/// bundled `buzz-agent` ships with the app and resolves on every platform. +/// if the catalog entry is missing. (Previous default was bare `goose`, which +/// is not on PATH on a stock Windows install; buzz-agent ships with the app.) pub fn default_agent_command() -> String { known_acp_runtime_exact("buzz-agent") .and_then(|p| p.commands.first().copied()) @@ -375,10 +376,8 @@ pub use overrides::{apply_agent_command_update, create_time_agent_command_overri /// Prefix of the typed dangling-harness error produced by /// `try_record_agent_command` / `resolve_effective_harness_descriptor`. -/// -/// This sentinel is an internal Rust contract: user-facing surfaces must -/// convert it to a sentence via [`user_facing_harness_error`] (spawn) or to -/// the missing id via [`dangling_harness_id`] (summary) — never show it raw. +/// Internal Rust contract: surfaces must convert it via [`user_facing_harness_error`] or +/// [`dangling_harness_id`] — never show it raw. pub(crate) const DANGLING_HARNESS_PREFIX: &str = "DANGLING_HARNESS_ID:"; /// Extract the missing harness id from a `DANGLING_HARNESS_ID:` error. @@ -398,22 +397,16 @@ pub(crate) fn user_facing_harness_error(error: &str) -> String { } } -/// Summary-row display for a dangling harness id: shows the *missing* id so -/// the agent list tells the same story as spawn (which refuses with the -/// sentence above), rather than silently falling back to the default command -/// as if the agent were healthy. +/// Summary-row display for a dangling harness id: shows the *missing* id so the agent list +/// tells the same story as spawn rather than silently falling back to the default command. pub(crate) fn dangling_harness_display(id: &str) -> String { format!("harness (deleted): {id}") } /// Spawn-time variant of `record_agent_command` that returns a typed error when -/// a record's `runtime` id or its persona's `runtime` id is set but cannot be -/// resolved (i.e. the definition was deleted after the agent was created). -/// -/// Returns `Err("DANGLING_HARNESS_ID:")` so callers can surface the error -/// without falling through to `buzz-agent`. When there is no runtime id at all -/// the fallback to `default_agent_command()` is intentional (legacy agents -/// pre-date the unified harness model). +/// a record's `runtime` id or persona's `runtime` id is set but unresolvable +/// (definition deleted after agent was created). Returns `Err("DANGLING_HARNESS_ID:")`. +/// When there is no runtime id at all, falls through to `default_agent_command()` intentionally. pub fn try_record_agent_command( record: &crate::managed_agents::types::ManagedAgentRecord, personas: &[crate::managed_agents::types::AgentDefinition], @@ -1413,6 +1406,9 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr model_env_var: runtime.model_env_var.map(str::to_string), provider_env_var: runtime.provider_env_var.map(str::to_string), thinking_env_var: runtime.thinking_env_var.map(str::to_string), + max_tokens_env_var: runtime.max_tokens_env_var.map(str::to_string), + context_limit_env_var: runtime.context_limit_env_var.map(str::to_string), + max_rounds_env_var: runtime.max_rounds_env_var.map(str::to_string), install_hint, install_instructions_url: install_instructions_url.to_string(), can_auto_install, @@ -1571,6 +1567,9 @@ pub fn discover_acp_runtimes_from( model_env_var: None, provider_env_var: None, thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, install_hint: def.install_hint.clone(), install_instructions_url: def.install_instructions_url.clone(), // Security line: custom definitions carry no install scripts. diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index 3622b21c4..bcc428800 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -67,6 +67,9 @@ pub(super) fn preset_catalog_entry( model_env_var: None, provider_env_var: None, thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, install_hint: def.install_hint.to_string(), install_instructions_url: def.install_instructions_url.to_string(), can_auto_install: false, diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index fdfe9b8be..34edecdcd 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -52,6 +52,8 @@ pub(crate) struct KnownAcpRuntime { pub max_tokens_env_var: Option<&'static str>, /// Env var for normalizing `context_limit`. `None` when not applicable. pub context_limit_env_var: Option<&'static str>, + /// Env var for normalizing `max_rounds`. `None` when not applicable. + pub max_rounds_env_var: Option<&'static str>, /// Normalized field keys that must be set for this harness to function. /// Used by the config bridge to mark fields as required in the UI. /// Keys match the camelCase names used in `NormalizedConfig` (e.g. "model", "provider"). diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fa8eb36fa..26902ae8d 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1051,19 +1051,16 @@ mod tests { thinking_env_var: None, max_tokens_env_var: None, context_limit_env_var: None, + max_rounds_env_var: None, required_normalized_fields: &[], login_hint: None, auth_probe_args: None, } } - /// Returns the absolute path of the currently-running test binary as a - /// `&'static str`. Host-portable stand-in for a "present" binary: - /// the path is absolute so `find_command` resolves it via `path.exists()` - /// rather than searching `PATH`, and the file always exists on the host. - /// - /// The tiny allocation is intentionally leaked — this runs at most once per - /// test process and the process exits immediately after tests complete. + /// Returns the absolute path of the currently-running test binary as a `&'static str`. + /// Host-portable stand-in for a "present" binary: absolute path so `find_command` resolves + /// it via `path.exists()`. Leaked allocation is intentional — process exits after tests. fn present_binary_str() -> &'static str { let path = std::env::current_exe().expect("current_exe must be available in tests"); Box::leak(path.to_string_lossy().into_owned().into_boxed_str()) @@ -1246,6 +1243,7 @@ mod tests { thinking_env_var: None, max_tokens_env_var: None, context_limit_env_var: None, + max_rounds_env_var: None, required_normalized_fields: &[], login_hint: None, auth_probe_args: None, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index fcd8b13fc..255c1aae3 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -594,10 +594,8 @@ pub enum AcpAvailabilityStatus { NotInstalled, } -/// Authentication/login status for a CLI-based ACP runtime. -/// -/// Serializes as a tagged union `{ status: "...", diagnostic?: "..." }` so -/// the TypeScript side can exhaustively switch on `status`. +/// Authentication/login status for a CLI-based ACP runtime. Serializes as a tagged union +/// `{ status: "...", diagnostic?: "..." }` so the TypeScript side can exhaustively switch on `status`. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case", tag = "status")] pub enum AuthStatus { @@ -616,8 +614,7 @@ pub enum AuthStatus { Unknown, } -/// Origin of an ACP runtime catalog entry. Serializes as a lowercase string -/// so the TypeScript consumer can switch on it without numeric comparisons. +/// Origin of an ACP runtime catalog entry. Serializes as a lowercase string so the TypeScript consumer can switch on it without numeric comparisons. #[derive(Debug, Clone, Serialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum HarnessSource { @@ -645,6 +642,9 @@ pub struct AcpRuntimeCatalogEntry { pub provider_env_var: Option, /// Environment variable used to apply thinking effort, when supported. pub thinking_env_var: Option, + pub max_tokens_env_var: Option, + pub context_limit_env_var: Option, + pub max_rounds_env_var: Option, pub install_hint: String, pub install_instructions_url: String, /// true when at least one automated install step is available diff --git a/desktop/src/features/agents/lib/agentConfigCore.test.mjs b/desktop/src/features/agents/lib/agentConfigCore.test.mjs index 62d8a61a6..92159ff27 100644 --- a/desktop/src/features/agents/lib/agentConfigCore.test.mjs +++ b/desktop/src/features/agents/lib/agentConfigCore.test.mjs @@ -1,7 +1,12 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { deriveAgentConfigFieldModel } from "./agentConfigCore.ts"; +import { + deriveAgentConfigFieldModel, + deriveNumericDescriptors, + structuredEnvKeys, +} from "./agentConfigCore.ts"; +import { NUMERIC_KIND_MIN } from "../ui/buzzAgentModelTuningFields.tsx"; const config = { env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, @@ -23,6 +28,9 @@ function runtime(id, metadata = {}) { modelEnvVar: null, providerEnvVar: null, thinkingEnvVar: null, + maxTokensEnvVar: null, + contextLimitEnvVar: null, + maxRoundsEnvVar: null, installHint: "", installInstructionsUrl: "", canAutoInstall: false, @@ -152,3 +160,404 @@ test("catalog mismatch cleanup is named and restricted to onboarding", () => { onCatalogMismatch: "explainOnly", }); }); + +// ── Numeric descriptor derivation per runtime ───────────────────────────── +// +// The catalog-projected fields (maxTokensEnvVar, contextLimitEnvVar, +// maxRoundsEnvVar) determine which numeric descriptors appear in the field +// model. Capability facts flow catalog → descriptor → UI; no runtime-ID +// comparison decides numeric-field visibility. + +test("buzz-agent derives three numeric descriptors from catalog fields", () => { + const model = deriveAgentConfigFieldModel({ + config, + runtime: runtime("buzz-agent", { + modelEnvVar: "BUZZ_AGENT_MODEL", + providerEnvVar: "BUZZ_AGENT_PROVIDER", + thinkingEnvVar: "BUZZ_AGENT_THINKING_EFFORT", + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }), + scope: "global", + }); + + const numericKinds = model.fields + .filter((f) => + ["maxOutputTokens", "contextLimit", "maxRounds"].includes(f.kind), + ) + .map((f) => f.kind); + assert.deepEqual(numericKinds, [ + "maxOutputTokens", + "contextLimit", + "maxRounds", + ]); + + const maxOutput = field(model, "maxOutputTokens"); + assert.equal(maxOutput.render, "control"); + assert.deepEqual(maxOutput.currentPersistence, { + kind: "envVar", + key: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + }); + assert.deepEqual(maxOutput.targetApplication, { + kind: "envVar", + key: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + }); + + const ctx = field(model, "contextLimit"); + assert.deepEqual(ctx.currentPersistence, { + kind: "envVar", + key: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + }); + + const rounds = field(model, "maxRounds"); + assert.deepEqual(rounds.currentPersistence, { + kind: "envVar", + key: "BUZZ_AGENT_MAX_ROUNDS", + }); +}); + +test("Goose derives two numeric descriptors and no maxRounds", () => { + const model = deriveAgentConfigFieldModel({ + config, + runtime: runtime("goose", { + modelEnvVar: "GOOSE_MODEL", + providerEnvVar: "GOOSE_PROVIDER", + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + maxTokensEnvVar: "GOOSE_MAX_TOKENS", + contextLimitEnvVar: "GOOSE_CONTEXT_LIMIT", + maxRoundsEnvVar: null, // Goose has no max-rounds env var + }), + scope: "global", + }); + + const numericKinds = model.fields + .filter((f) => + ["maxOutputTokens", "contextLimit", "maxRounds"].includes(f.kind), + ) + .map((f) => f.kind); + assert.deepEqual(numericKinds, ["maxOutputTokens", "contextLimit"]); + assert.equal( + field(model, "maxRounds"), + undefined, + "maxRounds must be absent for Goose", + ); + + assert.deepEqual(field(model, "maxOutputTokens").currentPersistence, { + kind: "envVar", + key: "GOOSE_MAX_TOKENS", + }); + assert.deepEqual(field(model, "contextLimit").currentPersistence, { + kind: "envVar", + key: "GOOSE_CONTEXT_LIMIT", + }); +}); + +test("Claude derives no numeric descriptors", () => { + const model = deriveAgentConfigFieldModel({ + config, + runtime: runtime("claude"), + scope: "global", + }); + + const hasNumeric = model.fields.some((f) => + ["maxOutputTokens", "contextLimit", "maxRounds"].includes(f.kind), + ); + assert.equal(hasNumeric, false, "Claude must have no numeric descriptors"); +}); + +test("Codex derives no numeric descriptors", () => { + const model = deriveAgentConfigFieldModel({ + config, + runtime: runtime("codex"), + scope: "global", + }); + + const hasNumeric = model.fields.some((f) => + ["maxOutputTokens", "contextLimit", "maxRounds"].includes(f.kind), + ); + assert.equal(hasNumeric, false, "Codex must have no numeric descriptors"); +}); + +test("numeric descriptor value is read from env_vars when set", () => { + const cfgWithTuning = { + env_vars: { + BUZZ_AGENT_MAX_OUTPUT_TOKENS: "8192", + BUZZ_AGENT_MAX_CONTEXT_TOKENS: "100000", + BUZZ_AGENT_MAX_ROUNDS: "25", + }, + model: "test-model", + preferred_runtime: null, + provider: "anthropic", + }; + const model = deriveAgentConfigFieldModel({ + config: cfgWithTuning, + runtime: runtime("buzz-agent", { + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }), + scope: "global", + }); + + assert.equal(field(model, "maxOutputTokens").value, "8192"); + assert.equal(field(model, "contextLimit").value, "100000"); + assert.equal(field(model, "maxRounds").value, "25"); +}); + +test("numeric descriptor value is null when env var is absent", () => { + const cfgEmpty = { + env_vars: {}, + model: "test-model", + preferred_runtime: null, + provider: null, + }; + const model = deriveAgentConfigFieldModel({ + config: cfgEmpty, + runtime: runtime("buzz-agent", { + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }), + scope: "global", + }); + + assert.equal(field(model, "maxOutputTokens").value, null); + assert.equal(field(model, "contextLimit").value, null); + assert.equal(field(model, "maxRounds").value, null); +}); + +// ── structuredEnvKeys: rendered-descriptor ownership ───────────────────── +// +// structuredEnvKeys accepts the descriptors a surface ACTUALLY renders and +// returns the env-var keys that surface owns. Keys only appear in the output +// when a first-class control for them renders — a persisted value must never +// have zero editors. +// +// Critical invariant: per-agent Goose passes only its two numeric descriptors +// (no effort descriptor, because no effort control renders there). The effort +// key (BUZZ_AGENT_THINKING_EFFORT) must NOT appear in the output — it must +// stay a visible generic env row where any saved value can be edited. + +test("structuredEnvKeys_global_includes_effort_key_and_numeric_keys", () => { + // Global surface renders effort + all numeric descriptors. + const buzzAgentModel = deriveAgentConfigFieldModel({ + config, + runtime: runtime("buzz-agent", { + modelEnvVar: "BUZZ_AGENT_MODEL", + providerEnvVar: "BUZZ_AGENT_PROVIDER", + thinkingEnvVar: "BUZZ_AGENT_THINKING_EFFORT", + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }), + scope: "global", + }); + + // Global renders all renderable descriptors. + const renderedDescriptors = buzzAgentModel.fields.filter( + (f) => f.render === "control", + ); + const keys = structuredEnvKeys(renderedDescriptors); + + assert.ok( + keys.includes("BUZZ_AGENT_THINKING_EFFORT"), + "effort key must be hidden on global (effort control renders)", + ); + assert.ok( + keys.includes("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + "maxOutputTokens key must be hidden on global", + ); + assert.ok( + keys.includes("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + "contextLimit key must be hidden on global", + ); + assert.ok( + keys.includes("BUZZ_AGENT_MAX_ROUNDS"), + "maxRounds key must be hidden on global", + ); +}); + +test("structuredEnvKeys_per_agent_buzz_agent_includes_effort_and_numeric_keys", () => { + // Per-agent buzz-agent renders effort + all 3 numeric descriptors. + const buzzAgentModel = deriveAgentConfigFieldModel({ + config, + runtime: runtime("buzz-agent", { + thinkingEnvVar: "BUZZ_AGENT_THINKING_EFFORT", + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }), + scope: "definition", + }); + + const renderedDescriptors = buzzAgentModel.fields.filter( + (f) => f.render === "control", + ); + const keys = structuredEnvKeys(renderedDescriptors); + + assert.ok(keys.includes("BUZZ_AGENT_THINKING_EFFORT"), "effort key present"); + assert.ok(keys.includes("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), "maxTokens present"); + assert.ok( + keys.includes("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + "contextLimit present", + ); + assert.ok(keys.includes("BUZZ_AGENT_MAX_ROUNDS"), "maxRounds present"); +}); + +test("structuredEnvKeys_per_agent_goose_excludes_effort_key_discriminating_invariant", () => { + // Per-agent Goose: effort migration is out of scope, so no effort control + // renders on the per-agent surface for Goose. Only the 2 numeric descriptors + // are passed as the rendered set. The effort persistence key + // (BUZZ_AGENT_THINKING_EFFORT) must NOT appear in the output — any saved + // value must remain visible and editable as a generic env row. + const gooseModel = deriveAgentConfigFieldModel({ + config, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + maxTokensEnvVar: "GOOSE_MAX_TOKENS", + contextLimitEnvVar: "GOOSE_CONTEXT_LIMIT", + }), + scope: "definition", + }); + + // Simulate per-agent surface: only the numeric descriptors render (no effort + // control for Goose per-agent — effort migration is out of scope). + const numericDescriptorsOnly = gooseModel.fields.filter((f) => + ["maxOutputTokens", "contextLimit", "maxRounds"].includes(f.kind), + ); + + const keys = structuredEnvKeys(numericDescriptorsOnly); + + assert.equal( + keys.includes("BUZZ_AGENT_THINKING_EFFORT"), + false, + "effort persistence key must NOT be hidden for Goose per-agent — no editor would replace it", + ); + assert.ok( + keys.includes("GOOSE_MAX_TOKENS"), + "maxTokens key must be present (control renders)", + ); + assert.ok( + keys.includes("GOOSE_CONTEXT_LIMIT"), + "contextLimit key must be present (control renders)", + ); +}); + +test("structuredEnvKeys_deferred_effort_excluded_from_result", () => { + // A deferred effort descriptor (render !== "control") must not contribute + // its key to the hidden set — the value has no editor on this surface. + const claudeModel = deriveAgentConfigFieldModel({ + config, + runtime: runtime("claude"), + scope: "global", + }); + + const allDescriptors = claudeModel.fields; // includes deferred effort + const keys = structuredEnvKeys(allDescriptors); + + // Claude's deferred effort has currentPersistence.kind === "unavailable" + // and render === "deferredUntilNativeOptionsAvailable"; no key emitted. + assert.equal( + keys.length, + 0, + "deferred effort and model descriptors must not contribute hidden keys", + ); +}); + +// ── deriveNumericDescriptors: standalone helper ─────────────────────────── +// +// The same logic that populates the numeric portion of deriveAgentConfigFieldModel +// is available as a standalone helper for per-agent surfaces that don't need +// the full field model. + +test("deriveNumericDescriptors_undefined_runtime_returns_empty", () => { + const ds = deriveNumericDescriptors(undefined); + assert.deepEqual(ds, []); +}); + +test("deriveNumericDescriptors_runtime_with_all_three_fields", () => { + const ds = deriveNumericDescriptors( + runtime("buzz-agent", { + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }), + ); + assert.deepEqual( + ds.map((d) => d.kind), + ["maxOutputTokens", "contextLimit", "maxRounds"], + ); + for (const d of ds) { + assert.equal(d.render, "control"); + assert.equal(d.currentPersistence.kind, "envVar"); + assert.equal(d.value, null, "standalone helper returns null values"); + } +}); + +test("deriveNumericDescriptors_partial_fields_match_catalog_projection", () => { + // Goose: two numeric fields, no maxRounds. + const ds = deriveNumericDescriptors( + runtime("goose", { + maxTokensEnvVar: "GOOSE_MAX_TOKENS", + contextLimitEnvVar: "GOOSE_CONTEXT_LIMIT", + maxRoundsEnvVar: null, + }), + ); + assert.deepEqual( + ds.map((d) => d.kind), + ["maxOutputTokens", "contextLimit"], + ); +}); + +test("deriveNumericDescriptors_matches_deriveAgentConfigFieldModel_numeric_subset", () => { + // The standalone helper must produce the same descriptor set (without values) + // that deriveAgentConfigFieldModel embeds, so surfaces that call the helper + // directly get a consistent policy with the full field model. + const runtimeEntry = runtime("buzz-agent", { + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_MAX_CONTEXT_TOKENS", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + }); + + const standalone = deriveNumericDescriptors(runtimeEntry); + const fromModel = deriveAgentConfigFieldModel({ + config, + runtime: runtimeEntry, + scope: "global", + }).fields.filter((f) => + ["maxOutputTokens", "contextLimit", "maxRounds"].includes(f.kind), + ); + + // Kinds and keys must match; values differ (standalone returns null, model + // reads from config). + assert.deepEqual( + standalone.map((d) => d.kind), + fromModel.map((d) => d.kind), + "descriptor kinds must match", + ); + for (let i = 0; i < standalone.length; i++) { + assert.deepEqual( + standalone[i].currentPersistence, + fromModel[i].currentPersistence, + `persistence must match for descriptor ${i}`, + ); + } +}); + +// ── NUMERIC_KIND_MIN: kind-specific input minima ────────────────────────── +// +// max output tokens and context limit must have min=1 (buzz-agent rejects 0). +// max rounds allows 0 (meaning unlimited). + +test("NUMERIC_KIND_MIN_maxOutputTokens_is_1", () => { + assert.equal(NUMERIC_KIND_MIN.maxOutputTokens, 1); +}); + +test("NUMERIC_KIND_MIN_contextLimit_is_1", () => { + assert.equal(NUMERIC_KIND_MIN.contextLimit, 1); +}); + +test("NUMERIC_KIND_MIN_maxRounds_is_0", () => { + assert.equal(NUMERIC_KIND_MIN.maxRounds, 0); +}); diff --git a/desktop/src/features/agents/lib/agentConfigCore.ts b/desktop/src/features/agents/lib/agentConfigCore.ts index 5827aedfa..5a8b8cb1c 100644 --- a/desktop/src/features/agents/lib/agentConfigCore.ts +++ b/desktop/src/features/agents/lib/agentConfigCore.ts @@ -4,6 +4,18 @@ import type { } from "@/shared/api/types"; import { BUZZ_AGENT_THINKING_EFFORT } from "../ui/buzzAgentConfig"; +/** + * Lifecycle status of the ACP runtime catalog query on a per-agent surface. + * + * - `loading` — query in flight; structured controls are withheld and env-var + * keys are not hidden (saved values remain visible as generic rows). + * - `ready` — query resolved; descriptors derived from `selectedRuntime`. + * - `error` — query failed; same gate as loading: no structured controls, + * keys not hidden, saved values stay visible. Distinguishable from + * "runtime not capable" (which is `ready` + no selectedRuntime). + */ +export type RuntimeCatalogStatus = "loading" | "ready" | "error"; + export type AgentConfigScope = | "onboarding" | "global" @@ -69,6 +81,13 @@ export type AgentConfigFieldDescriptor = | { kind: "acpConfigOption"; id: string; category: string }; render: "control" | "deferredUntilNativeOptionsAvailable"; value: string | null; + } + | { + kind: "maxOutputTokens" | "contextLimit" | "maxRounds"; + currentPersistence: EnvVarPersistence; + targetApplication: { kind: "envVar"; key: string }; + render: "control"; + value: string | null; }; export type AgentConfigOmission = { @@ -76,6 +95,18 @@ export type AgentConfigOmission = { reason: "ownedByModelId" | "unsupportedByHarness"; }; +/** + * A numeric tuning descriptor: one of the three env-var-backed number fields + * (max output tokens, context limit, max rounds). + * + * Defined here so both the field model derivation and the rendering surfaces + * share a single type — avoids the type being redefined in UI layers. + */ +export type NumericDescriptor = Extract< + AgentConfigFieldDescriptor, + { kind: "maxOutputTokens" | "contextLimit" | "maxRounds" } +>; + export type AgentConfigFieldModel = { fields: AgentConfigFieldDescriptor[]; omissions: AgentConfigOmission[]; @@ -86,6 +117,51 @@ function valueFromEnv(config: GlobalAgentConfig, key: string) { return config.env_vars[key]?.trim() || null; } +/** + * Derives the numeric descriptor set for a runtime from catalog fields. + * + * The returned descriptors drive `NumericTuningFields` on any surface that + * renders numeric knobs. Surfaces pass the same descriptor set to both the + * renderer and `structuredEnvKeys()` — one policy, no local rebuilding. + * + * Returns `[]` when `runtime` is undefined (catalog not yet settled, or the + * runtime has no numeric env-var fields). + */ +export function deriveNumericDescriptors( + runtime: AcpRuntimeCatalogEntry | undefined, +): NumericDescriptor[] { + if (!runtime) return []; + const ds: NumericDescriptor[] = []; + if (runtime.maxTokensEnvVar) { + ds.push({ + kind: "maxOutputTokens", + currentPersistence: { kind: "envVar", key: runtime.maxTokensEnvVar }, + targetApplication: { kind: "envVar", key: runtime.maxTokensEnvVar }, + render: "control", + value: null, + }); + } + if (runtime.contextLimitEnvVar) { + ds.push({ + kind: "contextLimit", + currentPersistence: { kind: "envVar", key: runtime.contextLimitEnvVar }, + targetApplication: { kind: "envVar", key: runtime.contextLimitEnvVar }, + render: "control", + value: null, + }); + } + if (runtime.maxRoundsEnvVar) { + ds.push({ + kind: "maxRounds", + currentPersistence: { kind: "envVar", key: runtime.maxRoundsEnvVar }, + targetApplication: { kind: "envVar", key: runtime.maxRoundsEnvVar }, + render: "control", + value: null, + }); + } + return ds; +} + /** * Derives the harness-scoped field model consumed by agent config renderers. * @@ -163,6 +239,16 @@ export function deriveAgentConfigFieldModel({ }); } + // Numeric fields — derived from the shared helper, then value-populated + // from config. Any surface needing only the descriptor structure (without + // saved values) calls deriveNumericDescriptors(runtime) directly. + for (const d of deriveNumericDescriptors(runtime)) { + fields.push({ + ...d, + value: valueFromEnv(config, d.currentPersistence.key), + }); + } + return { fields, omissions, @@ -191,3 +277,74 @@ export function getRenderableEffortField( field.kind === "effort" && field.render === "control", ); } + +/** + * Returns the env-var keys owned by the rendered descriptors on a surface. + * + * Pass only the descriptors that **actually render controls** on the surface — + * the resulting key set should be used as `EnvVarsEditor.hiddenKeys` and to + * exclude keys from baked-row generic display. + * + * Invariant: a key appears in the output only when a first-class control for + * it renders on the surface — a persisted value must never have zero editors. + * + * Per-surface consequences (assuming standard descriptor sets): + * - Global: effort key + numeric keys rendered by the descriptors + * - Per-agent buzz-agent: effort key + 3 numeric keys + * - Per-agent Goose: 2 numeric keys only — Goose effort (BUZZ_AGENT_THINKING_EFFORT) + * stays a visible generic env row because no effort control renders per-agent + * for Goose (effort migration is out of scope) + */ +export function structuredEnvKeys( + renderedDescriptors: AgentConfigFieldDescriptor[], +): string[] { + const keys: string[] = []; + for (const d of renderedDescriptors) { + if (d.render !== "control") continue; + if (d.kind === "effort" && d.currentPersistence.kind === "envVar") { + keys.push(d.currentPersistence.key); + } else if ( + d.kind === "maxOutputTokens" || + d.kind === "contextLimit" || + d.kind === "maxRounds" + ) { + keys.push(d.currentPersistence.key); + } + } + return keys; +} + +/** + * Filters a baked-env row array to exclude keys already covered by structured + * controls, preventing double-editing. The result is the set of baked rows + * that the generic env-vars editor should display. + * + * Call with the union of always-structured keys (provider/model/effort set) + * and numeric structured keys derived from `structuredEnvKeys()`. + * + * Pure — suitable for Node-layer unit tests without a component renderer. + */ +export function filterBakedGenericRows( + bakedEnv: readonly T[], + excludeKeys: ReadonlySet | readonly string[], +): T[] { + const exclude = + excludeKeys instanceof Set ? excludeKeys : new Set(excludeKeys); + return bakedEnv.filter((e) => !exclude.has(e.key)); +} + +/** + * Returns the placeholder string for a numeric tuning input. + * + * When an inherited value is present, the field shows `"Inherit ()"`. + * When absent (no global setting), the field shows `"Inherit (agent default)"`. + * + * Pure — used by NumericTuningFields and testable without a component renderer. + */ +export function numericTuningPlaceholder( + inheritedValue: string | null | undefined, +): string { + return inheritedValue + ? `Inherit (${inheritedValue})` + : "Inherit (agent default)"; +} diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index ce3d25220..11f68e8a5 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -24,6 +24,8 @@ import { deriveAgentConfigFieldModel, getRenderableEffortField, hasRenderableAgentConfigField, + structuredEnvKeys, + filterBakedGenericRows, } from "@/features/agents/lib/agentConfigCore"; import { getBakedProviderInheritLabel, @@ -52,7 +54,9 @@ import { } from "@/features/agents/ui/buzzAgentConfig"; import { EffortSelectField, + NumericTuningFields, useEffortAutoClear, + type NumericDescriptor, } from "@/features/agents/ui/buzzAgentModelTuningFields"; import { SettingsOptionGroup } from "@/features/settings/ui/SettingsOptionGroup"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; @@ -66,7 +70,6 @@ export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { preferred_runtime: null, }; -/** Baked env keys that route to structured controls, not the generic env editor. */ const BAKED_STRUCTURED_KEYS = new Set([ "BUZZ_AGENT_PROVIDER", "BUZZ_AGENT_MODEL", @@ -103,12 +106,7 @@ export const CANONICAL_CONFIG_BEHAVIORS = { requireProviderForModelAndEffort, } as const; -/** - * Disclosure preset → the eight visibility decisions it owns. Full and - * progressive defaults expose the same controls; the progressive preset - * changes only when those controls are revealed. Exported for the contract - * test. - */ +/** Disclosure preset → the eight visibility decisions it owns. Exported for the contract test. */ export function resolveDisclosure(disclosure: AgentConfigDisclosure) { const full = disclosure !== "onboarding-essential"; return { @@ -139,14 +137,7 @@ export function shouldRevealDependentConfigFields({ ); } -/** - * Determines whether the status line beneath the Model field should render. - * - * Discovery warnings bypass the `onboarding-essential` preset so that a - * first-run failure is never silently invisible. On the happy path - * (`status === null`) the status line stays hidden in onboarding, keeping - * the page clean. - */ +/** Whether the status line under the Model field renders. Discovery warnings bypass onboarding-essential so first-run failures are never invisible. */ export function shouldShowModelStatusMessage( showDescriptions: boolean, status: { message: string; tone: string } | null, @@ -155,14 +146,8 @@ export function shouldShowModelStatusMessage( } /** - * Whether the Model control should render given discovery state. - * - * Optional-model harnesses (Claude Code / Codex, `acpNative`) omit the control - * while discovery is in flight and after a **confirmed successful empty** - * catalog (IPC resolved, no usable options) — there is nothing useful to pick. - * Discovery failures / unavailable runtimes keep the control so #2246 failure - * UI can render. Full disclosure still shows the control when Custom model is - * available. Required-model harnesses always render the control. + * Renders the Model control given discovery state. Optional-model harnesses omit it while + * discovery is loading or after confirmed successful empty; failures keep it for the #2246 UI. */ export function shouldRenderModelControl({ discoveredModelOptions, @@ -269,6 +254,19 @@ export function AgentConfigFields({ effortField?.currentPersistence.kind === "envVar" ? effortField.currentPersistence.key : null; + + const numericDescriptors = fieldModel.fields.filter( + (d): d is NumericDescriptor => + (d.kind === "maxOutputTokens" || + d.kind === "contextLimit" || + d.kind === "maxRounds") && + d.render === "control", + ); + const allStructuredKeys = structuredEnvKeys([ + ...(effortField ? [effortField] : []), + ...numericDescriptors, + ]); + const bakedEnvMap = Object.fromEntries(bakedEnv.map((e) => [e.key, e.value])); const bakedProvider = React.useMemo( () => bakedEnv.find((e) => e.key === "BUZZ_AGENT_PROVIDER")?.value ?? null, [bakedEnv], @@ -301,8 +299,12 @@ export function AgentConfigFields({ [bakedEnv], ); const bakedGenericRows = React.useMemo( - () => bakedEnv.filter((e) => !BAKED_STRUCTURED_KEYS.has(e.key)), - [bakedEnv], + () => + filterBakedGenericRows(bakedEnv, [ + ...BAKED_STRUCTURED_KEYS, + ...allStructuredKeys, + ]), + [bakedEnv, allStructuredKeys], ); const providerValue = providerFieldVisible ? (config.provider ?? "") : ""; @@ -573,16 +575,15 @@ export function AgentConfigFields({ } function handleEnvVarsChange(next: Record) { - const effort = effortPersistenceKey - ? config.env_vars[effortPersistenceKey] - : undefined; - const merged = { ...next }; - if (effortPersistenceKey && effort !== undefined) { - merged[effortPersistenceKey] = effort; - } - onConfigChange({ ...config, env_vars: merged }); + onConfigChange({ ...config, env_vars: next }); } + const handleNumericEnvVarChange = (key: string, value: string) => { + const next = { ...config.env_vars, [key]: value }; + if (value === "") delete next[key]; + onConfigChange({ ...config, env_vars: next }); + }; + // On internal Block builds, BUZZ_AGENT_PROVIDER is baked in and a boot // migration rewrites v1→v2. Hide the legacy v1 option so it is not offered // for new selections; OSS builds show it. @@ -739,6 +740,33 @@ export function AgentConfigFields({ ) : null; + const advancedEditorBlock = ( + <> + + {numericDescriptors.length > 0 ? ( + + ) : null} + + ); + const dependentContent = ( <> {providerFieldVisible && apiKeyEnvVar ? ( @@ -903,40 +931,12 @@ export function AgentConfigFields({ : PROGRESSIVE_FIELDS_TRANSITION } > - k !== BUZZ_AGENT_THINKING_EFFORT, - ), - )} - /> + {advancedEditorBlock} ) : null} ) : advancedOpen ? ( - k !== BUZZ_AGENT_THINKING_EFFORT, - ), - )} - /> + advancedEditorBlock ) : null} ) : null} diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 1916b5706..12702f45a 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -101,7 +101,7 @@ type AgentDefinitionDialogProps = { error: Error | null; isPending: boolean; runtimes: AcpRuntimeCatalogEntry[]; - runtimesLoading?: boolean; + runtimeCatalogStatus?: "loading" | "ready" | "error"; onOpenChange: (open: boolean) => void; onSubmit: ( input: CreatePersonaInput | UpdatePersonaInput, @@ -128,13 +128,14 @@ export function AgentDefinitionDialog({ error, isPending, runtimes, - runtimesLoading = false, + runtimeCatalogStatus = "ready" as const, onOpenChange, onSubmit, publishCatalogUpdatesOnSave = false, createRunSection, createSubmitBlocked = false, }: AgentDefinitionDialogProps) { + const runtimesLoading = runtimeCatalogStatus === "loading"; const [displayName, setDisplayName] = React.useState(""); const [aiDefaultsOpen, setAiDefaultsOpen] = React.useState(false); const aiDefaultsTriggerRef = React.useRef(null); @@ -394,11 +395,7 @@ export function AgentDefinitionDialog({ (runtime.trim().length > 0 && runtimeCanChooseLlmProvider) || blankRuntimeModelProviderEditable; const trimmedProvider = provider.trim(); - // Required credential env keys for this runtime + provider combination. - // Used to show required markers on the LLM provider label and amber - // locked rows in the env vars editor. - // File-layer config for the selected runtime (e.g. goose config.yaml). - // Used to silence requirements already satisfied there. + // Required credential env keys and file-layer config; silences requirements satisfied in the file layer. const { data: runtimeFileConfig } = useRuntimeFileConfigQuery(runtime, { enabled: open, }); @@ -1016,6 +1013,8 @@ export function AgentDefinitionDialog({ model={model} modelTuningRuntimeId={runtime} namePoolText={namePoolText} + catalogStatus={runtimeCatalogStatus} + selectedRuntime={selectedRuntime} onBehaviorDraftChange={(nextBehaviorDraft) => { setHasUserChanges(true); setBehaviorDraft(nextBehaviorDraft); diff --git a/desktop/src/features/agents/ui/AgentDialog.tsx b/desktop/src/features/agents/ui/AgentDialog.tsx index dc608da48..a875770b3 100644 --- a/desktop/src/features/agents/ui/AgentDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDialog.tsx @@ -34,7 +34,7 @@ type AgentDialogCreateProps = { definitionError: Error | null; isDefinitionPending: boolean; runtimes: AcpRuntimeCatalogEntry[]; - runtimesLoading: boolean; + runtimeCatalogStatus: "loading" | "ready" | "error"; onSubmitDefinition: ( input: CreatePersonaInput | UpdatePersonaInput, intent: AgentCreateIntent, @@ -68,7 +68,7 @@ type AgentDialogDefinitionEditProps = { error: Error | null; isPending: boolean; runtimes: AcpRuntimeCatalogEntry[]; - runtimesLoading?: boolean; + runtimeCatalogStatus?: "loading" | "ready" | "error"; onOpenChange: (open: boolean) => void; onSubmit: ( input: CreatePersonaInput | UpdatePersonaInput, @@ -125,7 +125,7 @@ function AgentCreateDialogRouter({ definitionError, isDefinitionPending, runtimes, - runtimesLoading, + runtimeCatalogStatus, onSubmitDefinition, }: AgentDialogCreateProps) { const [runDraft, setRunDraft] = React.useState(emptyWhereToRunDraft); @@ -166,7 +166,7 @@ function AgentCreateDialogRouter({ }} open runtimes={runtimes} - runtimesLoading={runtimesLoading} + runtimeCatalogStatus={runtimeCatalogStatus} submitLabel={copy.submitLabel} title={copy.title} /> diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index d717d773e..adfb8182a 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -266,16 +266,10 @@ export function AgentInstanceEditDialog({ return runtimeSupportsLlmProviderSelection(matched?.id ?? ""); }, [runtimes, originalAgentCommand]); - // The runtime id that will actually be active after submit. When inheriting, - // resolve from the LINKED PERSONA's runtime — that is what will run once the - // override is cleared. Deriving from agent.agentCommand here is wrong for a - // pinned agent that just toggled "Inherit runtime from template": the override - // (e.g. a Claude pin) is still present on the record, so it would resolve to - // the old pin instead of the persona's runtime, hiding required credentials. - // Fall back to the agent.agentCommand dual-match (command path, then id) only - // when there is no linked persona or its runtime is unset. This single - // prospective id feeds BOTH the block-save gate (requiredEnvKeys) and the - // submit path so they never disagree on which runtime is being saved. + // The runtime id active after submit. Inheriting resolves from the LINKED PERSONA's runtime + // (that is what runs once the override is cleared, not the current override). + // Falls back to dual-match (command path, then id) when no persona or its runtime is unset. + // This single prospective id feeds BOTH the block-save gate and submit so they always agree. const prospectiveRuntimeId = React.useMemo(() => { if (!inheritHarness) { return selectedRuntime?.id ?? selectedRuntimeId; @@ -307,6 +301,15 @@ export function AgentInstanceEditDialog({ const llmProviderFieldVisible = runtimeSupportsLlmProviderSelection(prospectiveRuntimeId); + const prospectiveRuntime = runtimes.find( + (r) => r.id === prospectiveRuntimeId, + ); + const runtimeCatalogStatus = runtimesQuery.isLoading + ? ("loading" as const) + : runtimesQuery.isError + ? ("error" as const) + : ("ready" as const); + // One-shot focus: when the dialog opens from a card deep-link, scroll and // focus the relevant field. The effect re-runs when `llmProviderFieldVisible` // changes so a provider-field focus request fires once the field materializes. @@ -339,9 +342,8 @@ export function AgentInstanceEditDialog({ return () => cancelAnimationFrame(id); }, [open, initialFocus, agent.pubkey, llmProviderFieldVisible]); - // Provider + env to PERSIST on submit — also fed to the credential gate so - // gate, saved record, and spawn snapshot all agree on one resolved value. - // See resolveInheritedRuntimeSubmission for the inherit/transition contract. + // Provider + env to PERSIST on submit — also fed to the credential gate so gate, saved record, + // and spawn snapshot all agree on one resolved value. See resolveInheritedRuntimeSubmission. const inheritedSubmission = React.useMemo( () => resolveInheritedRuntimeSubmission({ @@ -376,12 +378,8 @@ export function AgentInstanceEditDialog({ inheritedEnvVars: inheritedEnvVarsForAdvanced, } = useAgentDialogDefaults({ inheritedEnvVars, open }); - // Runtime/provider-required credential state, derived from the PROSPECTIVE - // post-submit runtime — see the hook for the inherit-transition rationale. - // Pass globalProvider so the hook uses it as a fallback when the per-agent - // provider is empty (global-provider-only configs must surface required keys). - // Pass globalEnvVars so keys satisfied by global config are excluded from - // requiredEnvKeys and do not block Save (display and gate agree). + // Runtime/provider-required credential state for the PROSPECTIVE post-submit runtime. + // globalProvider/globalEnvVars: fallback for empty per-agent provider; keys satisfied globally don't block Save. const { requiredEnvKeys, fileSatisfiedEnvKeys, requiredEnvKeyMissing } = useRequiredCredentialState({ open, @@ -1199,6 +1197,8 @@ export function AgentInstanceEditDialog({ parallelism={parallelism} provider={effectiveProvider} requiredEnvKeys={advancedRequiredEnvKeys} + catalogStatus={runtimeCatalogStatus} + selectedRuntime={prospectiveRuntime} systemPrompt={systemPrompt} onAcpCommandChange={setAcpCommand} onAgentArgsChange={setAgentArgs} diff --git a/desktop/src/features/agents/ui/AgentManagementDialogs.tsx b/desktop/src/features/agents/ui/AgentManagementDialogs.tsx index 0d01cbbcd..b72669e5f 100644 --- a/desktop/src/features/agents/ui/AgentManagementDialogs.tsx +++ b/desktop/src/features/agents/ui/AgentManagementDialogs.tsx @@ -22,7 +22,7 @@ export function AgentManagementDialogs() { }} onSubmitDefinition={management.submitCreate} runtimes={management.runtimes} - runtimesLoading={management.runtimesLoading} + runtimeCatalogStatus={management.runtimeCatalogStatus} /> ) : null} {management.createdAgent ? ( @@ -51,7 +51,7 @@ export function AgentManagementDialogs() { onSubmit={management.submitUpdate} open runtimes={management.runtimes} - runtimesLoading={management.runtimesLoading} + runtimeCatalogStatus={management.runtimeCatalogStatus} submitLabel="Save changes" title="Edit agent" /> diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index 3d1673c36..720d6e62a 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -319,7 +319,13 @@ export function AgentsView() { }} onSubmitDefinition={personas.handleSubmit} runtimes={personas.acpRuntimesQuery.data ?? []} - runtimesLoading={personas.acpRuntimesQuery.isLoading} + runtimeCatalogStatus={ + personas.acpRuntimesQuery.isLoading + ? "loading" + : personas.acpRuntimesQuery.isError + ? "error" + : "ready" + } /> ) : null} {agents.agentToAddToChannel ? ( @@ -368,7 +374,13 @@ export function AgentsView() { isPending={personas.isPending} mode="definition-edit" runtimes={personas.acpRuntimesQuery.data ?? []} - runtimesLoading={personas.acpRuntimesQuery.isLoading} + runtimeCatalogStatus={ + personas.acpRuntimesQuery.isLoading + ? "loading" + : personas.acpRuntimesQuery.isError + ? "error" + : "ready" + } onOpenChange={(open) => { if (!open) { personas.setPersonaDialogState(null); diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index 972c4e287..56685fd6c 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -1,3 +1,4 @@ +import * as React from "react"; import { cn } from "@/shared/lib/cn"; import { Input } from "@/shared/ui/input"; import { Textarea } from "@/shared/ui/textarea"; @@ -9,9 +10,21 @@ import { PERSONA_LABEL_OPTIONAL_CLASS, } from "./agentConfigOptions"; import type { AgentPersona } from "@/shared/api/types"; -import { BuzzAgentModelTuningFields } from "./buzzAgentModelTuningFields"; -import { isBuzzAgentRuntime } from "./buzzAgentConfig"; +import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; +import { + BuzzAgentModelTuningFields, + NumericTuningFields, +} from "./buzzAgentModelTuningFields"; +import { + isBuzzAgentRuntime, + BUZZ_AGENT_THINKING_EFFORT, +} from "./buzzAgentConfig"; import { EDIT_AGENT_PARALLELISM_HELP } from "../lib/agentParallelism"; +import { + deriveNumericDescriptors, + structuredEnvKeys, + type RuntimeCatalogStatus, +} from "../lib/agentConfigCore"; export function EditAgentAdvancedFields({ acpCommand, @@ -30,6 +43,8 @@ export function EditAgentAdvancedFields({ parallelism, provider, requiredEnvKeys, + catalogStatus = "ready", + selectedRuntime, systemPrompt, onAcpCommandChange, onAgentArgsChange, @@ -55,7 +70,7 @@ export function EditAgentAdvancedFields({ model?: string; /** * The actual/prospective runtime id used to decide whether to show the - * buzz-agent model-tuning fields. Uses `prospectiveRuntimeId` from + * buzz-agent effort-tuning field. Uses `prospectiveRuntimeId` from * EditAgentDialog — the resolved runtime, not the "inherit"/"custom" sentinel. */ modelTuningRuntimeId: string; @@ -63,6 +78,24 @@ export function EditAgentAdvancedFields({ /** Active LLM provider id — forwarded to BuzzAgentModelTuningFields for effort filtering. */ provider?: string; requiredEnvKeys: readonly string[]; + /** + * Lifecycle status of the runtime catalog query. Controls the numeric-tuning + * gate and hidden-key behaviour: + * - `loading` or `error`: no structured controls; keys not hidden — saved + * values stay visible as generic rows. + * - `ready`: descriptors derived from `selectedRuntime` (empty when the + * runtime has no numeric env-var fields). + * + * Defaults to `"ready"` so existing callers without the catalog query do not + * need to change. + */ + catalogStatus?: RuntimeCatalogStatus; + /** + * The catalog entry for the prospective runtime. Drives descriptor-based + * numeric tuning fields (max output tokens / context limit / max rounds). + * When undefined after the catalog has settled, no numeric controls render. + */ + selectedRuntime?: AcpRuntimeCatalogEntry; systemPrompt: string; onAcpCommandChange: (value: string) => void; onAgentArgsChange: (value: string) => void; @@ -72,6 +105,29 @@ export function EditAgentAdvancedFields({ onAutoRestartChange: (value: boolean) => void; onSystemPromptChange: (value: string) => void; }) { + // Numeric tuning descriptors — gate on catalog status so that loading/error + // never collapses to "no controls": keys stay visible as generic rows. + const numericDescriptors = React.useMemo( + () => + catalogStatus === "ready" + ? deriveNumericDescriptors(selectedRuntime) + : [], + [catalogStatus, selectedRuntime], + ); + + // Build the effective hidden-key list: caller's secrets + effort key (when + // rendered by BuzzAgentModelTuningFields) + numeric keys via structuredEnvKeys. + const effectiveHiddenKeys = React.useMemo( + () => [ + ...hiddenEnvKeys, + ...(isBuzzAgentRuntime(modelTuningRuntimeId) + ? [BUZZ_AGENT_THINKING_EFFORT] + : []), + ...structuredEnvKeys(numericDescriptors), + ], + [hiddenEnvKeys, modelTuningRuntimeId, numericDescriptors], + ); + return (
{/* Inherit runtime from template */} @@ -248,7 +304,7 @@ export function EditAgentAdvancedFields({ - {/* Tier-1 buzz-agent model-tuning knobs — only shown for buzz-agent. */} + {/* Descriptor-driven numeric tuning knobs — shown when the catalog has settled + and the runtime exposes numeric env-var fields. */} + {numericDescriptors.length > 0 ? ( + { + const next = { ...envVars }; + if (value === "") { + delete next[key]; + } else { + next[key] = value; + } + onEnvVarsChange(next); + }} + /> + ) : null} + + {/* Effort-tuning knob — only shown for buzz-agent. */} {isBuzzAgentRuntime(modelTuningRuntimeId) ? ( { "annotation must not appear when its key is not in the env map", ); }); + +// ── buildRecord with hiddenKeys: structured-field preservation ──────────── +// +// These tests exercise the exported buildRecord(nextRows, value, requiredKeys, +// hiddenKeys) using the real implementation. hiddenKeys are structured-field +// env vars (e.g. BUZZ_AGENT_MAX_ROUNDS) that are owned by first-class controls +// outside the editor — they must survive onChange cycles even though they +// never appear as generic rows. +// +// Four scenarios from rev 4: +// 1. Edit an unrelated generic row → hidden (tuning) key is unchanged. +// 2. Runtime switch then generic edit: after switching to a new runtime, +// the new runtime's hidden keys survive; old runtime keys appear as +// generic rows and survive via toRecord, not hidden-key preservation. +// 3. Baked numeric key excluded via real descriptor/helper path: uses the +// production deriveAgentConfigFieldModel + structuredEnvKeys helpers to +// derive the hidden set, then verifies toRows excludes the numeric key. +// 4. Clearing a structured override → placeholder returns: after the user +// clears a structured field (key absent from value), buildRecord must +// not reintroduce it, leaving the structured field free to show the +// Inherit placeholder. + +test("buildRecord_hidden_tuning_key_unchanged_when_generic_row_edited", () => { + // Structured field set BUZZ_AGENT_MAX_ROUNDS to "50"; it lives in value + // as a hiddenKey. User then edits a generic env var via the row editor. + // The tuning key must survive the buildRecord emit cycle unchanged. + const value = { BUZZ_AGENT_MAX_ROUNDS: "50", MY_VAR: "old" }; + const nextRows = [{ id: "r1", key: "MY_VAR", value: "new" }]; + const record = buildRecordUtil( + nextRows, + value, + [], + ["BUZZ_AGENT_MAX_ROUNDS"], + ); + + assert.equal( + record.BUZZ_AGENT_MAX_ROUNDS, + "50", + "hidden tuning key must survive when an unrelated generic row is edited", + ); + assert.equal(record.MY_VAR, "new", "generic row edit applied"); +}); + +test("buildRecord_runtime_switch_new_hiddenKeys_then_generic_edit", () => { + // Scenario 2: runtime switch then generic edit. + // + // Before switch: agent is buzz-agent with BUZZ_AGENT_MAX_ROUNDS = "50" stored + // in value (set via the numeric tuning control). After switching to Goose, + // the buzz-agent key is no longer hidden — it becomes a visible generic row. + // The test verifies: + // (a) After the switch, the old buzz-agent key appears as a generic row + // (toRows with the new Goose hidden set projects it). + // (b) After a generic-row edit, buildRecord preserves BOTH the old-runtime + // key (now a generic row) and the new-runtime hidden key. + // (c) An unset new-runtime hidden key is not introduced. + + // Derive both descriptor sets from real runtime objects. + const buzzAgentRuntime = { + id: "buzz-agent", + label: "Buzz Agent", + avatarUrl: "", + availability: "available", + command: "buzz-agent", + binaryPath: "buzz-agent", + defaultArgs: [], + mcpCommand: null, + modelEnvVar: "BUZZ_AGENT_MODEL", + providerEnvVar: "BUZZ_AGENT_PROVIDER", + thinkingEnvVar: "BUZZ_AGENT_THINKING_EFFORT", + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: "BUZZ_AGENT_CONTEXT_LIMIT", + maxRoundsEnvVar: "BUZZ_AGENT_MAX_ROUNDS", + installHint: "", + installInstructionsUrl: "", + canAutoInstall: false, + underlyingCliPath: null, + nodeRequired: false, + authStatus: { status: "not_applicable" }, + loginHint: null, + }; + const gooseRuntime = { + id: "goose", + label: "Goose", + avatarUrl: "", + availability: "available", + command: "goose", + binaryPath: "goose", + defaultArgs: ["acp"], + mcpCommand: null, + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + maxTokensEnvVar: "GOOSE_MAX_TOKENS", + contextLimitEnvVar: "GOOSE_CONTEXT_LIMIT", + maxRoundsEnvVar: null, + installHint: "", + installInstructionsUrl: "", + canAutoInstall: true, + underlyingCliPath: null, + nodeRequired: false, + authStatus: { status: "not_applicable" }, + loginHint: null, + }; + + const buzzDescriptors = deriveNumericDescriptors(buzzAgentRuntime); + const gooseDescriptors = deriveNumericDescriptors(gooseRuntime); + const buzzHiddenKeys = structuredEnvKeys(buzzDescriptors); + const gooseHiddenKeys = structuredEnvKeys(gooseDescriptors); + + // Sanity-check that BUZZ_AGENT_MAX_ROUNDS is hidden under buzz-agent but not + // under Goose — that contrast is what makes it become a generic row. + assert.ok( + buzzHiddenKeys.includes("BUZZ_AGENT_MAX_ROUNDS"), + "BUZZ_AGENT_MAX_ROUNDS must be hidden under buzz-agent descriptors", + ); + assert.equal( + gooseHiddenKeys.includes("BUZZ_AGENT_MAX_ROUNDS"), + false, + "BUZZ_AGENT_MAX_ROUNDS must not be hidden under Goose descriptors", + ); + + // Pre-switch value: buzz-agent max-rounds was set, GOOSE_MAX_TOKENS was + // already set (e.g. user configured it before switching back), plus a + // generic user var. GOOSE_MAX_TOKENS is a hidden key under the Goose + // descriptor set, so it must survive buildRecord() via hiddenKeys. + const valueBeforeSwitch = { + BUZZ_AGENT_MAX_ROUNDS: "50", + GOOSE_MAX_TOKENS: "16384", + USER_VAR: "original", + }; + + // After the switch to Goose, toRows is reproj with the new (Goose) hidden + // set. BUZZ_AGENT_MAX_ROUNDS is no longer hidden → appears as a generic row. + // GOOSE_MAX_TOKENS IS hidden under Goose → must not appear in generic rows. + const rowsAfterSwitch = toRows(valueBeforeSwitch, new Set(gooseHiddenKeys)); + assert.ok( + rowsAfterSwitch.some((r) => r.key === "BUZZ_AGENT_MAX_ROUNDS"), + "old-runtime key must become a generic row after the switch", + ); + assert.equal( + rowsAfterSwitch.some((r) => r.key === "GOOSE_MAX_TOKENS"), + false, + "new-runtime hidden key must not appear as a generic row after the switch", + ); + + // User edits the generic USER_VAR row. + const editedRows = rowsAfterSwitch.map((r) => + r.key === "USER_VAR" ? { ...r, value: "updated" } : r, + ); + + // buildRecord: old-runtime key survives via toRecord (it's now a generic + // row); new-runtime Goose hidden key survives via hiddenKeys (carried + // through from value). An unset Goose key must not be introduced. + const record = buildRecordUtil( + editedRows, + valueBeforeSwitch, + [], + gooseHiddenKeys, + ); + + assert.equal( + record.BUZZ_AGENT_MAX_ROUNDS, + "50", + "old-runtime key must survive as a generic row value after switch", + ); + assert.equal( + record.GOOSE_MAX_TOKENS, + "16384", + "new-runtime hidden key must survive buildRecord via hiddenKeys", + ); + assert.equal(record.USER_VAR, "updated", "generic row edit applied"); + assert.equal( + "GOOSE_CONTEXT_LIMIT" in record, + false, + "unset new-runtime hidden key must not be introduced", + ); +}); + +test("filterBakedGenericRows_numeric_baked_key_excluded_and_placeholder_shown", () => { + // Scenario 3: baked numeric key excluded via the real production helper. + // + // The global baked env contains BUZZ_AGENT_MAX_OUTPUT_TOKENS = "4096" + // (the baked value shipped with the agent). The production + // filterBakedGenericRows path must exclude this key from the generic + // baked-row display so it isn't editable twice, while the structured + // numeric input shows the inherited placeholder via numericTuningPlaceholder. + const buzzAgentRuntime = { + id: "buzz-agent", + label: "Buzz Agent", + avatarUrl: "", + availability: "available", + command: "buzz-agent", + binaryPath: "buzz-agent", + defaultArgs: [], + mcpCommand: null, + modelEnvVar: "BUZZ_AGENT_MODEL", + providerEnvVar: "BUZZ_AGENT_PROVIDER", + thinkingEnvVar: "BUZZ_AGENT_THINKING_EFFORT", + maxTokensEnvVar: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", + contextLimitEnvVar: null, + maxRoundsEnvVar: null, + installHint: "", + installInstructionsUrl: "", + canAutoInstall: false, + underlyingCliPath: null, + nodeRequired: false, + authStatus: { status: "not_applicable" }, + loginHint: null, + }; + + const numericDescriptors = deriveNumericDescriptors(buzzAgentRuntime); + const numericStructuredKeys = structuredEnvKeys(numericDescriptors); + + assert.ok( + numericStructuredKeys.includes("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + "numeric key must appear in structured keys via production helpers", + ); + + // Simulate the baked env: BUZZ_AGENT_MAX_OUTPUT_TOKENS is baked, plus a + // non-structured baked var. + const bakedEnv = [ + { key: "BUZZ_AGENT_MAX_OUTPUT_TOKENS", value: "4096" }, + { key: "SOME_OTHER_BAKED_VAR", value: "hello" }, + ]; + + // filterBakedGenericRows must exclude the numeric key. + const genericRows = filterBakedGenericRows(bakedEnv, numericStructuredKeys); + + assert.equal( + genericRows.some((r) => r.key === "BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + false, + "baked numeric key must be excluded from generic baked rows", + ); + assert.ok( + genericRows.some((r) => r.key === "SOME_OTHER_BAKED_VAR"), + "non-structured baked var must remain in generic rows", + ); + + // The structured numeric input shows the inherited placeholder for the + // baked value via numericTuningPlaceholder. + const bakedValue = "4096"; + assert.equal( + numericTuningPlaceholder(bakedValue), + "Inherit (4096)", + "structured placeholder must reflect the baked value", + ); + assert.equal( + numericTuningPlaceholder(undefined), + "Inherit (agent default)", + "structured placeholder without baked value shows agent-default text", + ); +}); + +test("buildRecord_clearing_structured_field_allows_placeholder_to_return", () => { + // Scenario 4: clearing a structured override → placeholder returns. + // + // Step 1: value has BUZZ_AGENT_MAX_ROUNDS = "50" (user set it via the + // structured field). BUZZ_AGENT_MAX_ROUNDS is in hiddenKeys. + // Step 2: user clears the structured field → onEnvVarChange(key, "") + // removes the key from value (value no longer contains it). + // Step 3: after the clear, buildRecord must not reintroduce the key. + // Step 4: with the key absent from value, numericTuningPlaceholder over + // the (now-empty) inheritedEnvVars shows "Inherit (agent default)" + // — the numeric field's empty-state placeholder. + + // After the clear, value no longer contains BUZZ_AGENT_MAX_ROUNDS. + const valueAfterClear = { MY_VAR: "foo" }; + const nextRows = [{ id: "r1", key: "MY_VAR", value: "updated" }]; + + const record = buildRecordUtil( + nextRows, + valueAfterClear, + [], + ["BUZZ_AGENT_MAX_ROUNDS"], + ); + + assert.equal( + "BUZZ_AGENT_MAX_ROUNDS" in record, + false, + "cleared structured key must not be reintroduced by buildRecord", + ); + assert.equal(record.MY_VAR, "updated"); + + // With the key cleared, the inherited value is also absent (not set + // globally). numericTuningPlaceholder returns the agent-default text — + // the placeholder that renders in the structured input. + const inheritedAfterClear = undefined; + assert.equal( + numericTuningPlaceholder(inheritedAfterClear), + "Inherit (agent default)", + "numeric input must show Inherit (agent default) after clear when no global override", + ); + + // If a global override IS set, the placeholder shows that value instead. + const inheritedGlobal = "25"; + assert.equal( + numericTuningPlaceholder(inheritedGlobal), + "Inherit (25)", + "numeric input must show Inherit () when a global override exists", + ); +}); diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.tsx b/desktop/src/features/agents/ui/EnvVarsEditor.tsx index 91e5b0762..08496eece 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.tsx +++ b/desktop/src/features/agents/ui/EnvVarsEditor.tsx @@ -44,7 +44,9 @@ export function toRows( * Collapse an ordered row list back to a record, skipping rows with empty * keys. Exported for unit tests. */ -export function toRecord(rows: Row[]): EnvVarsValue { +export function toRecord( + rows: readonly { key: string; value: string }[], +): EnvVarsValue { const out: EnvVarsValue = {}; for (const row of rows) { // Empty key = user is mid-edit; skip it so we don't poison the record. @@ -177,6 +179,27 @@ type EnvVarsEditorProps = { type Row = { id: string; key: string; value: string }; +/** + * Pure record builder: merges `toRecord(nextRows)` with the current values of + * `requiredKeys` and `hiddenKeys` from `value`. Required and hidden keys are + * excluded from the row state (`skipKeys`), so this merge is the only place + * their current values survive an `onChange` emit cycle. + * + * Exported for unit testing. `EnvVarsEditor` calls this internally. + */ +export function buildRecord( + nextRows: readonly { key: string; value: string }[], + value: EnvVarsValue, + requiredKeys: readonly string[], + hiddenKeys: readonly string[], +): EnvVarsValue { + const base: EnvVarsValue = {}; + for (const key of [...requiredKeys, ...hiddenKeys]) { + if (key in value) base[key] = value[key]; + } + return { ...base, ...toRecord(nextRows) }; +} + /** * A flat key/value editor for environment variables. * @@ -241,18 +264,6 @@ export function EnvVarsEditor({ } }, [value, skipKeys]); - // Build the emitted record: normal rows + required-key values preserved - // from `value`. Required keys are never in `rows`, so `toRecord(rows)` - // would silently drop any required secret the user just typed unless we - // merge them back explicitly. - function buildRecord(nextRows: Row[]): EnvVarsValue { - const base: EnvVarsValue = {}; - for (const key of [...requiredKeys, ...hiddenKeys]) { - if (key in value) base[key] = value[key]; - } - return { ...base, ...toRecord(nextRows) }; - } - // Ref map: key → required-value Input element. Populated via callback refs // on each required-key row's value Input so focus can be dispatched directly // without any DOM walking through presentation classes. @@ -294,7 +305,7 @@ export function EnvVarsEditor({ function emit(next: Row[]) { setRows(next); - const record = buildRecord(next); + const record = buildRecord(next, value, requiredKeys, hiddenKeys); lastEmitted.current = record; onChange(record); } diff --git a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx index b7d190378..1ecd98b83 100644 --- a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx @@ -1,20 +1,33 @@ +import * as React from "react"; import { Input } from "@/shared/ui/input"; import { cn } from "@/shared/lib/cn"; import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor"; import { CreateAgentRespondToField } from "./RespondToField"; import type { PersonaBehaviorDraft } from "./personaBehaviorDraft"; -import { isBuzzAgentRuntime } from "./buzzAgentConfig"; +import { + isBuzzAgentRuntime, + BUZZ_AGENT_THINKING_EFFORT, +} from "./buzzAgentConfig"; import { AGENT_PARALLELISM_HELP, AGENT_PARALLELISM_PLACEHOLDER, } from "../lib/agentParallelism"; -import { BuzzAgentModelTuningFields } from "./buzzAgentModelTuningFields"; +import { + BuzzAgentModelTuningFields, + NumericTuningFields, +} from "./buzzAgentModelTuningFields"; import { CARD_MINT_KEY_ANNOTATIONS, PERSONA_FIELD_CONTROL_CLASS, PERSONA_FIELD_SHELL_CLASS, PERSONA_LABEL_OPTIONAL_CLASS, } from "./agentConfigOptions"; +import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; +import { + deriveNumericDescriptors, + structuredEnvKeys, + type RuntimeCatalogStatus, +} from "../lib/agentConfigCore"; export function PersonaAdvancedFields({ behaviorDraft, @@ -31,6 +44,8 @@ export function PersonaAdvancedFields({ requiredEnvKeys = [], fileSatisfiedEnvKeys = [], hiddenEnvKeys = [], + catalogStatus = "ready" as RuntimeCatalogStatus, + selectedRuntime, }: { behaviorDraft: PersonaBehaviorDraft; disabled: boolean; @@ -40,7 +55,7 @@ export function PersonaAdvancedFields({ inheritedEnvVars?: EnvVarsValue; /** Active LLM model — forwarded to BuzzAgentModelTuningFields for effort filtering. */ model?: string; - /** Runtime id for the buzz-agent tuning knobs visibility gate. */ + /** Runtime id for the buzz-agent effort-tuning knob visibility gate. */ modelTuningRuntimeId?: string; namePoolText: string; onBehaviorDraftChange: (value: PersonaBehaviorDraft) => void; @@ -51,7 +66,42 @@ export function PersonaAdvancedFields({ requiredEnvKeys?: readonly string[]; fileSatisfiedEnvKeys?: readonly string[]; hiddenEnvKeys?: readonly string[]; + /** + * Lifecycle status of the runtime catalog query. Controls the numeric-tuning + * gate and hidden-key behaviour: + * - `loading` or `error`: no structured controls; keys not hidden — saved + * values stay visible as generic rows. + * - `ready`: descriptors derived from `selectedRuntime` (empty when the + * runtime has no numeric env-var fields). + */ + catalogStatus?: RuntimeCatalogStatus; + /** + * The catalog entry for the selected runtime. Drives descriptor-based + * numeric tuning fields. When undefined after the catalog has settled, + * no numeric controls render. + */ + selectedRuntime?: AcpRuntimeCatalogEntry; }) { + // Numeric tuning descriptors — gate on catalog status so that loading/error + // never collapses to "no controls": keys stay visible as generic rows. + const numericDescriptors = React.useMemo( + () => + catalogStatus === "ready" + ? deriveNumericDescriptors(selectedRuntime) + : [], + [catalogStatus, selectedRuntime], + ); + + const effectiveHiddenKeys = React.useMemo( + () => [ + ...hiddenEnvKeys, + ...(isBuzzAgentRuntime(modelTuningRuntimeId) + ? [BUZZ_AGENT_THINKING_EFFORT] + : []), + ...structuredEnvKeys(numericDescriptors), + ], + [hiddenEnvKeys, modelTuningRuntimeId, numericDescriptors], + ); return (
- {/* Tier-1 buzz-agent model-tuning knobs — only shown for buzz-agent. */} + {/* Descriptor-driven numeric tuning knobs — shown when catalog has settled + and the runtime exposes numeric env-var fields. */} + {numericDescriptors.length > 0 ? ( + { + const next = { ...envVars }; + if (value === "") { + delete next[key]; + } else { + next[key] = value; + } + onEnvVarsChange(next); + }} + /> + ) : null} + + {/* Effort-tuning knob — only shown for buzz-agent. */} {isBuzzAgentRuntime(modelTuningRuntimeId) ? ( ) : null} {personas.createdAgent ? ( diff --git a/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx b/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx index 938d5edf5..7fa87c4e6 100644 --- a/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx +++ b/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx @@ -9,14 +9,13 @@ import * as React from "react"; import { Input } from "@/shared/ui/input"; import { cn } from "@/shared/lib/cn"; import type { EnvVarsValue } from "./EnvVarsEditor"; +import type { NumericDescriptor } from "../lib/agentConfigCore"; +import { numericTuningPlaceholder } from "../lib/agentConfigCore"; import { AgentDropdownSelect, type AgentDropdownOption, } from "./agentConfigControls"; import { - BUZZ_AGENT_MAX_CONTEXT_TOKENS, - BUZZ_AGENT_MAX_OUTPUT_TOKENS, - BUZZ_AGENT_MAX_ROUNDS, BUZZ_AGENT_THINKING_EFFORT, BUZZ_AGENT_THINKING_EFFORT_VALUES, getProviderEffortConfig, @@ -201,6 +200,98 @@ export function useEffortAutoClear({ }, [effortValid, currentEffort]); } +export type { NumericDescriptor }; + +const NUMERIC_KIND_LABELS: Record = { + maxOutputTokens: "Max output tokens", + contextLimit: "Context limit", + maxRounds: "Max rounds", +}; + +const NUMERIC_KIND_DESCRIPTIONS: Record = { + maxOutputTokens: + "Maximum tokens the LLM may generate per response. Leave blank to inherit.", + contextLimit: + "Maximum context window tokens tracked before a handoff. Leave blank to inherit.", + maxRounds: + "Maximum LLM + tool-call rounds per turn. 0 = unlimited. Leave blank to inherit.", +}; + +const NUMERIC_KIND_TEST_IDS: Record = { + maxOutputTokens: "numeric-max-output-tokens-input", + contextLimit: "numeric-context-limit-input", + maxRounds: "numeric-max-rounds-input", +}; + +/** + * Input `min` attribute per numeric kind. + * + * - `maxOutputTokens` / `contextLimit`: minimum 1 — the buzz-agent runtime + * rejects 0 for these fields (crates/buzz-agent/src/config.rs:921-928). + * - `maxRounds`: 0 is valid (means unlimited). + */ +export const NUMERIC_KIND_MIN: Record = { + maxOutputTokens: 1, + contextLimit: 1, + maxRounds: 0, +}; + +/** + * Descriptor-driven numeric tuning inputs. + * + * Renders a grid of number inputs for every numeric descriptor in `descriptors`. + * Label and help text are keyed by descriptor kind — the same copy renders on + * both the global defaults surface and per-agent dialogs. + */ +export function NumericTuningFields({ + descriptors, + envVars, + inheritedEnvVars, + onEnvVarChange, +}: { + /** Numeric descriptors to render. Empty array → renders nothing. */ + descriptors: NumericDescriptor[]; + envVars: EnvVarsValue; + inheritedEnvVars: EnvVarsValue; + onEnvVarChange: (key: string, value: string) => void; +}) { + if (descriptors.length === 0) return null; + return ( +
+ {descriptors.map((d) => { + const key = d.currentPersistence.key; + const label = NUMERIC_KIND_LABELS[d.kind]; + const description = NUMERIC_KIND_DESCRIPTIONS[d.kind]; + const testId = NUMERIC_KIND_TEST_IDS[d.kind]; + const inheritedVal = inheritedEnvVars[key]; + return ( +
+ + onEnvVarChange(key, event.target.value)} + placeholder={numericTuningPlaceholder(inheritedVal)} + step="1" + type="number" + value={envVars[key] ?? ""} + /> +

+ {description} +

+
+ ); + })} +
+ ); +} + export function BuzzAgentModelTuningFields({ envVars, inheritedEnvVars, @@ -257,105 +348,6 @@ export function BuzzAgentModelTuningFields({ blank to inherit from the global or persona default.

- - {/* Max Rounds */} -
- - - onEnvVarChange(BUZZ_AGENT_MAX_ROUNDS, event.target.value) - } - placeholder={ - inheritedEnvVars[BUZZ_AGENT_MAX_ROUNDS] - ? `Inherit (${inheritedEnvVars[BUZZ_AGENT_MAX_ROUNDS]})` - : "Inherit (agent default)" - } - step="1" - type="number" - value={envVars[BUZZ_AGENT_MAX_ROUNDS] ?? ""} - /> -

- Maximum LLM + tool-call rounds per turn. 0 = unlimited. Leave blank - to inherit. -

-
- - {/* Max Output Tokens */} -
- - - onEnvVarChange(BUZZ_AGENT_MAX_OUTPUT_TOKENS, event.target.value) - } - placeholder={ - inheritedEnvVars[BUZZ_AGENT_MAX_OUTPUT_TOKENS] - ? `Inherit (${inheritedEnvVars[BUZZ_AGENT_MAX_OUTPUT_TOKENS]})` - : "Inherit (agent default)" - } - step="1" - type="number" - value={envVars[BUZZ_AGENT_MAX_OUTPUT_TOKENS] ?? ""} - /> -

- Maximum tokens the LLM may generate per response. Leave blank to - inherit. -

-
- - {/* Context Limit */} -
- - - onEnvVarChange(BUZZ_AGENT_MAX_CONTEXT_TOKENS, event.target.value) - } - placeholder={ - inheritedEnvVars[BUZZ_AGENT_MAX_CONTEXT_TOKENS] - ? `Inherit (${inheritedEnvVars[BUZZ_AGENT_MAX_CONTEXT_TOKENS]})` - : "Inherit (agent default)" - } - step="1" - type="number" - value={envVars[BUZZ_AGENT_MAX_CONTEXT_TOKENS] ?? ""} - /> -

- Maximum context window tokens buzz-agent tracks before a handoff. - Leave blank to inherit. -

-
); diff --git a/desktop/src/features/agents/useAgentManagement.ts b/desktop/src/features/agents/useAgentManagement.ts index f4cdb895e..066f7949a 100644 --- a/desktop/src/features/agents/useAgentManagement.ts +++ b/desktop/src/features/agents/useAgentManagement.ts @@ -301,7 +301,11 @@ export function useAgentManagement() { ...createdAgentAttachment, isPending, runtimes: runtimesQuery.data ?? [], - runtimesLoading: runtimesQuery.isLoading, + runtimeCatalogStatus: runtimesQuery.isLoading + ? ("loading" as const) + : runtimesQuery.isError + ? ("error" as const) + : ("ready" as const), submitCreate, submitUpdate, dismiss, diff --git a/desktop/src/features/profile/ui/UserProfilePanel.tsx b/desktop/src/features/profile/ui/UserProfilePanel.tsx index af30728d6..cb188dd00 100644 --- a/desktop/src/features/profile/ui/UserProfilePanel.tsx +++ b/desktop/src/features/profile/ui/UserProfilePanel.tsx @@ -957,6 +957,7 @@ export function UserProfilePanel({ personaToExportSnapshot={personaToExportSnapshot} resolvedPersona={resolvedPersona} runtimes={acpRuntimesQuery.data ?? []} + runtimesError={acpRuntimesQuery.isError} runtimesLoading={acpRuntimesQuery.isLoading} updateError={ updatePersonaMutation.error instanceof Error diff --git a/desktop/src/features/profile/ui/UserProfilePersonaDialogs.tsx b/desktop/src/features/profile/ui/UserProfilePersonaDialogs.tsx index 2de2e2f71..9fae1bd88 100644 --- a/desktop/src/features/profile/ui/UserProfilePersonaDialogs.tsx +++ b/desktop/src/features/profile/ui/UserProfilePersonaDialogs.tsx @@ -29,6 +29,7 @@ export function UserProfilePersonaDialogs({ resolvedPersona, runtimes, runtimesLoading, + runtimesError = false, updateError, onCloseCardMint, onCloseDelete, @@ -50,6 +51,7 @@ export function UserProfilePersonaDialogs({ resolvedPersona: AgentPersona | undefined; runtimes: AcpRuntimeCatalogEntry[]; runtimesLoading: boolean; + runtimesError?: boolean; updateError: Error | null; onCloseCardMint: () => void; onCloseDelete: () => void; @@ -59,6 +61,11 @@ export function UserProfilePersonaDialogs({ onExportSnapshot: (persona: AgentPersona) => void; onSubmit: (input: CreatePersonaInput | UpdatePersonaInput) => Promise; }) { + const runtimeCatalogStatus = runtimesLoading + ? "loading" + : runtimesError + ? "error" + : ("ready" as const); return ( <> { if (!open) { onCloseDialog(); diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 69e2e455e..bb56bc18e 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -188,6 +188,9 @@ export type RawAcpRuntimeCatalogEntry = { model_env_var?: string | null; provider_env_var?: string | null; thinking_env_var?: string | null; + max_tokens_env_var?: string | null; + context_limit_env_var?: string | null; + max_rounds_env_var?: string | null; install_hint: string; install_instructions_url: string; can_auto_install: boolean; @@ -199,10 +202,7 @@ export type RawAcpRuntimeCatalogEntry = { auth_status: AuthStatus; login_hint?: string; source: "builtin" | "preset" | "custom"; - /** - * Definition-level env vars for `source: custom` entries. - * Omitted/absent for builtin and preset — skipped in Rust serialization when empty. - */ + /** Definition-level env vars for `source: custom` entries; absent for builtin/preset. */ definition_env?: Record; }; @@ -749,6 +749,9 @@ export function fromRawAcpRuntimeCatalogEntry( modelEnvVar: entry.model_env_var ?? null, providerEnvVar: entry.provider_env_var ?? null, thinkingEnvVar: entry.thinking_env_var ?? null, + maxTokensEnvVar: entry.max_tokens_env_var ?? null, + contextLimitEnvVar: entry.context_limit_env_var ?? null, + maxRoundsEnvVar: entry.max_rounds_env_var ?? null, installHint: entry.install_hint, installInstructionsUrl: entry.install_instructions_url, canAutoInstall: entry.can_auto_install, @@ -1024,9 +1027,8 @@ export type RuntimeFileConfigSubset = { }; /** - * Get the file-layer config for a runtime so dialogs can show - * "Set in goose config" instead of surfacing a false required-field marker. - * Returns `null` when the runtime has no config file or it cannot be parsed. + * Get the file-layer config for a runtime so dialogs can show "Set in goose config" instead of + * surfacing a false required-field marker. Returns `null` when unavailable or unparseable. */ export async function getRuntimeFileConfig( runtimeId: string, @@ -1040,13 +1042,9 @@ export async function getRuntimeFileConfig( } /** - * Return the key names of all non-empty baked build env vars. - * - * Internal (Block) builds bake provider credentials into the binary at compile - * time. This returns the *key names only* — never the values — so dialogs can - * treat them as satisfied without exposing secrets to the frontend. - * - * OSS builds return an empty array (no baked env). + * Return the key names of all non-empty baked build env vars. Internal (Block) builds bake + * provider credentials into the binary at compile time; this returns *key names only* (never + * values) so dialogs treat them as satisfied without exposing secrets. OSS builds return []. */ export async function getBakedBuildEnvKeys(): Promise { return invokeTauri("get_baked_build_env_keys"); diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index d0e8ee004..fd2c71bce 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -517,6 +517,9 @@ export type AcpRuntimeCatalogEntry = { providerEnvVar: string | null; /** Environment variable used to apply thinking effort, when supported. */ thinkingEnvVar: string | null; + maxTokensEnvVar: string | null; + contextLimitEnvVar: string | null; + maxRoundsEnvVar: string | null; installHint: string; installInstructionsUrl: string; canAutoInstall: boolean; @@ -529,12 +532,7 @@ export type AcpRuntimeCatalogEntry = { authStatus: AuthStatus; /** Hint for completing authentication; null when not applicable or already logged in. */ loginHint: string | null; - /** - * Whether this entry is compiled into the app ("builtin"), a bundled preset - * ("preset" — PATH-probed, not editable/deletable), or loaded from a user - * JSON file in `custom_harnesses/` ("custom"). Controls editability in the - * UI — only "custom" entries can be edited or deleted. - */ + /** "builtin" (compiled in), "preset" (PATH-probed, not editable), or "custom" (user JSON). Controls UI editability. */ source: "builtin" | "preset" | "custom"; /** * Definition-level environment variables for `source: custom` entries. diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 6520dd760..9c6618c83 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -74,7 +74,7 @@ type MockCommandAvailability = { resolvedPath?: string | null; }; -type MockManagedAgentSeed = { +export type MockManagedAgentSeed = { pubkey: string; name: string; avatarUrl?: string | null; @@ -91,6 +91,8 @@ type MockManagedAgentSeed = { autoRestartOnConfigChange?: boolean; respondTo?: RawManagedAgent["respond_to"]; respondToAllowlist?: string[]; + /** Per-agent env vars seeded into the mock store. */ + envVars?: Record; }; type MockManagedAgentRuntimeSeed = { @@ -214,6 +216,8 @@ type E2eConfig = { /** Catalog responses for successive discovery calls. The final response repeats. */ acpRuntimesCatalogSequence?: RawAcpRuntimeCatalogEntry[][]; acpRuntimesDelayMs?: number; + /** When true, the catalog discovery call throws — simulates a failed query. */ + acpRuntimesError?: boolean; acpAuthMethods?: Record; acpAuthMethodsErrors?: Record; acpAuthMethodsError?: string; @@ -2086,6 +2090,24 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { const now = new Date().toISOString(); const status = seed.status ?? "stopped"; + // Resolve agent_command and agent_args from the well-known default catalog + // so the fixture mirrors real wire shape. Hardcoding ["acp"] for all runtimes + // is incorrect: buzz-agent ships with no default args. + const DEFAULT_RUNTIME_COMMAND: Record< + string, + { command: string; args: string[] } + > = { + goose: { command: "goose", args: ["acp"] }, + "buzz-agent": { command: "buzz-agent", args: [] }, + claude: { command: "claude", args: [] }, + codex: { command: "codex", args: [] }, + }; + const catalogEntry = seed.runtime + ? DEFAULT_RUNTIME_COMMAND[seed.runtime] + : undefined; + const agentCommand = catalogEntry?.command ?? seed.runtime ?? "goose"; + const agentArgs = catalogEntry?.args ?? ["acp"]; + return { pubkey: seed.pubkey, name: seed.name, @@ -2095,8 +2117,8 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { runtime: seed.runtime ?? null, relay_url: DEFAULT_RELAY_WS_URL, acp_command: "buzz-acp", - agent_command: "goose", - agent_args: ["acp"], + agent_command: agentCommand, + agent_args: agentArgs, mcp_command: "", turn_timeout_seconds: 320, idle_timeout_seconds: null, @@ -2105,7 +2127,7 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { system_prompt: null, avatar_url: seed.avatarUrl ?? null, model: null, - env_vars: {}, + env_vars: { ...(seed.envVars ?? {}) }, status, pid: status === "running" ? 42000 + mockManagedAgents.length : null, created_at: now, @@ -7165,6 +7187,28 @@ function withMockRuntimeConfigMetadata( : runtime.id === "goose" ? "GOOSE_THINKING_EFFORT" : null, + max_tokens_env_var: + "max_tokens_env_var" in runtime + ? runtime.max_tokens_env_var + : runtime.id === "buzz-agent" + ? "BUZZ_AGENT_MAX_OUTPUT_TOKENS" + : runtime.id === "goose" + ? "GOOSE_MAX_TOKENS" + : null, + context_limit_env_var: + "context_limit_env_var" in runtime + ? runtime.context_limit_env_var + : runtime.id === "buzz-agent" + ? "BUZZ_AGENT_MAX_CONTEXT_TOKENS" + : runtime.id === "goose" + ? "GOOSE_CONTEXT_LIMIT" + : null, + max_rounds_env_var: + "max_rounds_env_var" in runtime + ? runtime.max_rounds_env_var + : runtime.id === "buzz-agent" + ? "BUZZ_AGENT_MAX_ROUNDS" + : null, }; } @@ -7182,6 +7226,10 @@ async function handleDiscoverAcpRuntimes( }); } + if (config?.mock?.acpRuntimesError) { + throw new Error("Mocked catalog discovery failure"); + } + const afterInstallSequence = config?.mock?.acpRuntimesCatalogAfterInstallSequence; if (mockInstallCompleted && afterInstallSequence?.length) { diff --git a/desktop/tests/e2e/agent-numeric-tuning.spec.ts b/desktop/tests/e2e/agent-numeric-tuning.spec.ts new file mode 100644 index 000000000..dc70c8b12 --- /dev/null +++ b/desktop/tests/e2e/agent-numeric-tuning.spec.ts @@ -0,0 +1,372 @@ +/** + * Playwright regression tests for the numeric tuning fields (max output tokens, + * context limit, max rounds) on both the global Agent Defaults surface and the + * per-agent Advanced section. + * + * Covers: + * 1. Global defaults Advanced shows numeric inputs for buzz-agent. + * 2. Global defaults Advanced hides numeric inputs for non-capable runtimes. + * 3. Per-agent Goose: saving a max-tokens value globally surfaces as + * Inherit () placeholder in the per-agent edit dialog. + * 4. Delayed catalog: while loading, saved tuning env vars stay visible + * as generic rows (not silently dropped); structured controls appear + * once the catalog settles. + * 5. Failed catalog: when discovery errors, saved tuning env vars remain + * visible as generic rows (never the "unsupported" empty state). + */ + +import { expect, test } from "@playwright/test"; +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +// ── Helpers ──────────────────────────────────────────────────────────────── + +async function openAiDefaultsSettings(page: import("@playwright/test").Page) { + await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.getByTestId("open-settings").click(); + await page.getByTestId("profile-popover-settings").click(); + await expect(page.getByTestId("settings-view")).toBeVisible(); + await page.getByTestId("settings-nav-agents").click(); + await expect(page.getByTestId("settings-global-agent-config")).toBeVisible({ + timeout: 10_000, + }); + await expect(page.locator(".animate-spin").first()).not.toBeVisible({ + timeout: 5_000, + }); +} + +async function openEditAgentDialog( + page: import("@playwright/test").Page, + agentName: string, +) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + + const agentButton = page.getByRole("button", { + name: `${agentName} 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, + }); +} + +// ── Tests ────────────────────────────────────────────────────────────────── + +test("global_advanced_buzz_agent_shows_all_numeric_controls", async ({ + page, +}) => { + // The mock bridge's withMockRuntimeConfigMetadata injects the numeric env var + // fields for buzz-agent. When buzz-agent is selected and Advanced is opened, + // all three numeric inputs must be visible. + await installMockBridge(page, { + acpRuntimesCatalog: [ + { + id: "buzz-agent", + label: "Buzz Agent", + avatar_url: "", + availability: "available", + command: "buzz-agent", + binary_path: "/usr/local/bin/buzz-agent", + default_args: [], + mcp_command: null, + install_hint: "Ships with the Buzz desktop app.", + install_instructions_url: "https://github.com/block/buzz", + can_auto_install: false, + underlying_cli_path: null, + auth_status: { status: "not_applicable" }, + }, + ], + globalAgentConfig: { + env_vars: {}, + provider: "anthropic", + model: null, + preferred_runtime: "buzz-agent", + }, + }); + + await openAiDefaultsSettings(page); + + // Open the Advanced section. The settings card uses disclosure="full" (no + // animation wrapper), so we click the toggle and wait for content directly. + await page.getByTestId("global-agent-advanced-toggle").click(); + + // All three numeric inputs must be present for buzz-agent. + await expect(page.getByTestId("numeric-max-output-tokens-input")).toBeVisible( + { timeout: 5_000 }, + ); + await expect(page.getByTestId("numeric-context-limit-input")).toBeVisible(); + await expect(page.getByTestId("numeric-max-rounds-input")).toBeVisible(); +}); + +test("global_advanced_non_capable_runtime_hides_numeric_controls", async ({ + page, +}) => { + // Claude has no numeric tuning env vars (contextLimitEnvVar = null etc.). + // After selecting Claude, the Advanced section must show no numeric inputs. + await installMockBridge(page, { + acpRuntimesCatalog: [ + { + id: "claude", + label: "Claude Code", + avatar_url: "", + availability: "available", + command: "/usr/local/bin/claude-agent", + binary_path: "/usr/local/bin/claude-agent", + default_args: ["acp"], + mcp_command: null, + install_hint: "Install via npm.", + install_instructions_url: "https://example.com", + can_auto_install: true, + underlying_cli_path: "/usr/local/bin/claude", + auth_status: { status: "logged_in" }, + }, + ], + globalAgentConfig: { + env_vars: {}, + provider: null, + model: null, + preferred_runtime: "claude", + }, + }); + + await openAiDefaultsSettings(page); + + await page.getByTestId("global-agent-advanced-toggle").click(); + + // No numeric inputs must render for a non-capable runtime. + await expect(page.getByTestId("numeric-max-output-tokens-input")).toHaveCount( + 0, + ); + await expect(page.getByTestId("numeric-context-limit-input")).toHaveCount(0); + await expect(page.getByTestId("numeric-max-rounds-input")).toHaveCount(0); +}); + +test("goose_per_agent_advanced_max_tokens_shows_inherited_global_placeholder", async ({ + page, +}) => { + // Save GOOSE_MAX_TOKENS = 16384 in the global Agent Defaults settings via + // the UI, then open a Goose agent's edit dialog. The max-output-tokens input + // must show "Inherit (16384)" — the globally-saved value surfaced via the + // inherited placeholder. + await installMockBridge(page, { + globalAgentConfig: { + env_vars: { ANTHROPIC_API_KEY: "sk-ant-test-key" }, + provider: "anthropic", + model: "claude-opus-4-5", + preferred_runtime: "goose", + }, + managedAgents: [ + { + pubkey: TEST_IDENTITIES.tyler.pubkey, + name: "Tyler Agent", + runtime: "goose", + status: "stopped", + channelNames: ["agents"], + }, + ], + }); + + // Step 1: open global defaults, expand Advanced, enter the max-tokens value. + await openAiDefaultsSettings(page); + await page.getByTestId("global-agent-advanced-toggle").click(); + await expect(page.getByTestId("numeric-max-output-tokens-input")).toBeVisible( + { timeout: 5_000 }, + ); + await page.getByTestId("numeric-max-output-tokens-input").click(); + await page + .getByTestId("numeric-max-output-tokens-input") + .pressSequentially("16384"); + // Blur to ensure React's change event fires for the number input. + await page.keyboard.press("Tab"); + + // Step 2: save the global defaults. + await expect(page.getByRole("button", { name: "Save defaults" })).toBeEnabled( + { timeout: 5_000 }, + ); + await page.getByRole("button", { name: "Save defaults" }).click(); + // Wait for the save to complete: the button returns to disabled (dirty resets). + await expect( + page.getByRole("button", { name: "Save defaults" }), + ).toBeDisabled({ timeout: 5_000 }); + + // Step 3: navigate back and open the per-agent edit dialog for the Goose + // agent. We use the app's Back link rather than page.goto("/") to preserve + // the in-memory mock state (page.goto causes a full reload that resets it). + await page.getByRole("button", { name: "Back to app" }).click(); + await page.getByTestId("open-agents-view").click(); + const agentButton = page.getByRole("button", { + name: "Tyler Agent 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, + }); + + // Wait for the provider field — signals the catalog and dialog have settled. + await expect(page.locator("#edit-agent-llm-provider")).toBeVisible({ + timeout: 10_000, + }); + + // Open the Advanced section. + await page.getByRole("button", { name: "Advanced", exact: true }).click(); + await expect(page.getByTestId("numeric-max-output-tokens-input")).toBeVisible( + { timeout: 5_000 }, + ); + + // The placeholder must reflect the globally-saved value. + await expect( + page.getByTestId("numeric-max-output-tokens-input"), + ).toHaveAttribute("placeholder", "Inherit (16384)"); +}); + +test("delayed_catalog_per_agent_saved_tuning_values_visible_then_structured_controls_appear", async ({ + page, +}) => { + // Scenario: catalog takes 5 seconds to respond (simulates slow discovery). + // The per-agent edit dialog opens. While the catalog is still in flight, + // saved tuning env vars must not be dropped from view — they appear as + // generic env rows (no hiddenKeys applied yet). Once the catalog settles, + // the structured numeric controls replace the generic rows. + await installMockBridge(page, { + acpRuntimesCatalog: [ + { + id: "buzz-agent", + label: "Buzz Agent", + avatar_url: "", + availability: "available", + command: "buzz-agent", + binary_path: "/usr/local/bin/buzz-agent", + default_args: [], + mcp_command: null, + install_hint: "Ships with the Buzz desktop app.", + install_instructions_url: "https://github.com/block/buzz", + can_auto_install: false, + underlying_cli_path: null, + auth_status: { status: "not_applicable" }, + }, + ], + // 5-second delay: generous enough that the dialog opens and Advanced is + // expanded while the catalog query is still in-flight (navigation takes + // ~1-2 s), but short enough to keep the test under 30 s. + acpRuntimesDelayMs: 5000, + globalAgentConfig: { + env_vars: {}, + provider: "anthropic", + model: null, + preferred_runtime: "buzz-agent", + }, + managedAgents: [ + { + pubkey: TEST_IDENTITIES.tyler.pubkey, + name: "Tyler Agent", + runtime: "buzz-agent", + status: "stopped", + channelNames: ["agents"], + envVars: { + BUZZ_AGENT_MAX_OUTPUT_TOKENS: "4096", + BUZZ_AGENT_MAX_ROUNDS: "25", + }, + }, + ], + }); + + await openEditAgentDialog(page, "Tyler Agent"); + + // Open Advanced before the catalog has settled (the dialog opens quickly; + // the catalog query fires when the dialog opens and takes ~5 seconds). + await page.getByRole("button", { name: "Advanced", exact: true }).click(); + + // While loading: structured numeric controls must NOT be visible yet — + // the catalog-settling gate withholds them. + await expect(page.getByTestId("numeric-max-output-tokens-input")).toHaveCount( + 0, + ); + + // The saved tuning env vars must be visible as generic rows (not hidden) + // while the catalog hasn't settled: BUZZ_AGENT_MAX_OUTPUT_TOKENS and + // BUZZ_AGENT_MAX_ROUNDS should appear in the env-vars editor. + await expect( + page.locator( + 'input[data-testid="env-vars-key"][value="BUZZ_AGENT_MAX_OUTPUT_TOKENS"]', + ), + ).toBeVisible(); + await expect( + page.locator( + 'input[data-testid="env-vars-key"][value="BUZZ_AGENT_MAX_ROUNDS"]', + ), + ).toBeVisible(); + + // After the catalog settles (allow up to 8 s — 5 s delay + margin): + // structured controls appear, replacing the generic rows. + await expect(page.getByTestId("numeric-max-output-tokens-input")).toBeVisible( + { timeout: 8_000 }, + ); + await expect(page.getByTestId("numeric-max-rounds-input")).toBeVisible({ + timeout: 8_000, + }); +}); + +test("failed_catalog_per_agent_saved_tuning_values_remain_visible_as_generic_rows", async ({ + page, +}) => { + // Scenario: catalog discovery fails (network error / IPC rejection). + // The per-agent edit dialog opens. The saved tuning env vars must remain + // visible as generic rows — the error state must never produce the + // "unsupported" no-controls state that would hide persisted values. + await installMockBridge(page, { + acpRuntimesError: true, + globalAgentConfig: { + env_vars: {}, + provider: "anthropic", + model: null, + preferred_runtime: "buzz-agent", + }, + managedAgents: [ + { + pubkey: TEST_IDENTITIES.tyler.pubkey, + name: "Tyler Agent", + runtime: "buzz-agent", + status: "stopped", + channelNames: ["agents"], + envVars: { + BUZZ_AGENT_MAX_OUTPUT_TOKENS: "8192", + BUZZ_AGENT_MAX_ROUNDS: "10", + }, + }, + ], + }); + + await openEditAgentDialog(page, "Tyler Agent"); + + // Open Advanced after a brief wait (query has had time to fail). + await page.waitForTimeout(500); + await page.getByRole("button", { name: "Advanced", exact: true }).click(); + + // Structured numeric controls must NOT render (catalog errored — no runtime). + await expect(page.getByTestId("numeric-max-output-tokens-input")).toHaveCount( + 0, + ); + + // Saved tuning values must still be visible as generic env rows — the error + // state must never hide persisted values with no editor to replace them. + await expect( + page.locator( + 'input[data-testid="env-vars-key"][value="BUZZ_AGENT_MAX_OUTPUT_TOKENS"]', + ), + ).toBeVisible({ timeout: 5_000 }); + await expect( + page.locator( + 'input[data-testid="env-vars-key"][value="BUZZ_AGENT_MAX_ROUNDS"]', + ), + ).toBeVisible(); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 830a82879..45766d25b 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -1,5 +1,6 @@ import type { Page } from "@playwright/test"; import type { ChannelTemplate, RelayEvent } from "../../src/shared/api/types"; +import type { MockManagedAgentSeed } from "../../src/testing/e2eBridge"; import { FEATURE_OVERRIDES_STORAGE_KEY, PREVIEW_FEATURE_IDS } from "./features"; export const TEST_IDENTITIES = { @@ -43,24 +44,6 @@ type MockCommandAvailability = { resolvedPath?: string | null; }; -type MockManagedAgentSeed = { - pubkey: string; - name: string; - personaId?: string | null; - status?: "running" | "stopped" | "deployed" | "not_deployed"; - channelNames?: string[]; - channelIds?: string[]; - backend?: - | { type: "local" } - | { type: "provider"; id: string; config: Record }; - lastError?: string | null; - lastErrorCode?: number | null; - needsRestart?: boolean; - autoRestartOnConfigChange?: boolean; - respondTo?: "owner-only" | "allowlist" | "anyone"; - respondToAllowlist?: string[]; -}; - type MockSearchProfileSeed = { pubkey: string; displayName: string | null; @@ -199,6 +182,8 @@ type MockBridgeOptions = { /** Catalog responses for successive discovery calls. The final response repeats. */ acpRuntimesCatalogSequence?: Record[][]; acpRuntimesDelayMs?: number; + /** When true, the mock catalog discovery command throws an error. */ + acpRuntimesError?: boolean; acpAuthMethods?: Record[] }>; acpAuthMethodsError?: string; /** When set, the `delete_custom_harness` mock command throws with this message. */