feat(agents): model-tuning parity in global Agent Defaults editor (#4578)

## 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 `<input min>`.
- **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<string, string>` 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 (<value>)` 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 <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>

desktop/src-tauri/src/commands/agent_config.rs

@@ -31,8 +31,7 @@ pub struct RuntimeFileConfigSubset {
     pub provider: Option<String>,
     /// Model set in the harness config file, if any.
     pub model: Option<String>,
-    /// 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<String>,
 }
 

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,

desktop/src-tauri/src/commands/agent_discovery.rs

@@ -21,25 +21,13 @@ fn active_installs() -> &'static std::sync::Mutex<std::collections::HashSet<Stri
 }
 
 /// Returns the adapter install commands that `install_acp_runtime_blocking` would
-/// run for `runtime_id` given a resolved adapter binary at `adapter_path` (or
-/// `None` if none was found).
+/// run for `runtime_id` given a resolved adapter binary at `adapter_path` (or `None` if not found).
+/// Returns `None` when no install is needed; `Some(cmds)` when adapter is missing or outdated.
 ///
-/// Returns `None` when no install is needed (adapter is present and current).
-/// Returns `Some(cmds)` when the adapter is missing or (for codex) below its
-/// minimum supported version.
-///
-/// For the codex **outdated** case the returned sequence is a two-step
-/// reinstall: first uninstall the old `@zed-industries/codex-acp` package
-/// (idempotent — exit 0 when absent), then install the new
-/// `@agentclientprotocol/codex-acp`.  This is required because both packages
-/// install a global binary named `codex-acp`, and npm ≥7 refuses to overwrite
-/// a bin file owned by a different package with `EEXIST`.
-///
-/// For the **missing** case the catalog's `adapter_install_commands` are used
-/// as-is (no prior package to remove).
-///
-/// This is a pure planning function: it never spawns a process.  Tests use it to
-/// assert the correct install command is selected without touching real npm.
+/// For the codex **outdated** case, returns a two-step reinstall: uninstall `@zed-industries/codex-acp`
+/// then install `@agentclientprotocol/codex-acp` (npm ≥7 refuses to overwrite a bin from another pkg).
+/// For the **missing** case, catalog's `adapter_install_commands` are used as-is.
+/// Pure planning function: never spawns a process. Tests use it to assert commands without real npm.
 pub(crate) fn plan_adapter_install<'c>(
     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,

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,

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:<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:<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:<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.

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,

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").

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,

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<String>,
     /// Environment variable used to apply thinking effort, when supported.
     pub thinking_env_var: Option<String>,
+    pub max_tokens_env_var: Option<String>,
+    pub context_limit_env_var: Option<String>,
+    pub max_rounds_env_var: Option<String>,
     pub install_hint: String,
     pub install_instructions_url: String,
     /// true when at least one automated install step is available