From 25dd24a11d961d731cd6903bbe6933cdf4cb6a0d Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Mon, 3 Aug 2026 21:30:18 -0400 Subject: [PATCH] feat(agents): canonical effort contract + native-key persistence + mutation contract (Phase 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - acceptedEffortValues on AcpRuntimeCatalogEntry: Goose carries canonical off|low|medium|high|max (source: goose thinking.rs:277-308); null for buzz-agent (per-model catalog). Single authority for UI, spawn bridge, and reader. - normalizeEffortValue() in buzzAgentConfig.ts: aliases none|disabled→off, med→medium, xhigh→max (case-insensitive); null acceptedValues = pass-through (buzz-agent path unchanged). - ALL_KNOWN_EFFORT_KEYS in buzzAgentConfig.ts mirrors Rust constant; used for record/persona runtime-transition cleanup. - agentConfigCore.ts: native key wins; legacy-only pre-migration save shown via effortValue fallback; legacyConsumedKey suppresses duplicate advanced row. optionSource harnessNative when acceptedEffortValues present. - AgentConfigFields.tsx: descriptor-driven global/onboarding path for harnessNative runtimes (isHarnessNativeEffort): canonical options from acceptedEffortValues, currentEffort from normalised descriptor value, effortValidForAutoClear from acceptedEffortValues. Unconditional BUZZ_AGENT_THINKING_EFFORT deletes guarded by !isHarnessNativeEffort (Delta 5: global-scope coexistence). allStructuredKeys includes legacyConsumedKey; effort clear deletes native + legacy; bakedStructuredKeys() replaces static set. - buzzAgentModelTuningFields.tsx: applyHarnessNativeEffortChange() exported production helper — single spec for the mutation contract. legacyKey=null at global/onboarding scope (Delta 5), legacyKey=BUZZ_AGENT_THINKING_EFFORT at record/persona scope. EffortSelectField gains canonicalValues prop to bypass Buzz 7-value catalog. HarnessNativeEffortFields calls production helper. - personaRuntimeModel.ts: resolveInheritedRuntimeSubmission gains excludePersonaEnvKeys param. Strips harness-native effort keys from persona layer on inherit-transition so the record never acquires a frozen copy of persona effort (effort inherits at spawn; it is not a credential). - AgentInstanceEditDialog.tsx: passes excludePersonaEnvKeys with runtime's thinkingEnvVar + ALL_KNOWN_EFFORT_KEYS when runtime has acceptedEffortValues. - bakedEnvHelpers.ts: getInheritedAgentDefaults accepts nativeEffortKey option; baked lookup is native-only for non-buzz-agent runtimes (Delta-4 boundary). bakedStructuredKeys() exported for AgentConfigFields. - useAgentDialogDefaults.ts: accepts nativeEffortKey; seeds inherited env under native key. - AgentDefinitionDialog + AgentInstanceEditDialog: pass prev/next native effort keys to selectionOnRuntimeChange; handleInheritHarnessChange clears all known effort keys on pin↔inherit toggle. - runtimeModelProviderSelection.ts: selectionOnRuntimeChange clears prev native + next native + legacy at record/persona scope; skip when native === legacy (buzz-agent same-key invariant). - agentConfigOptions.tsx: resetConfigForHarnessChange preserves all effort keys (global-scope Delta-5 rule; each runtime's descriptor strips foreign keys). - agent_config.rs: is_safe_to_reveal derives allowed effort keys from ALL_KNOWN_EFFORT_KEYS so GOOSE_THINKING_EFFORT is unmasked at baked tier. - AGENTS.md rule-2 'until PR 2.7' clause removed: native-key persistence lands in this commit. - Test coverage: applyHarnessNativeEffortChange record/persona + global scope pins; resolveInheritedRuntimeSubmission inherit-no-materialize (native + legacy), concrete-override-wins, non-effort-merged, full-submit-payload-clean; global Goose descriptor optionSource/value/invalid/no-legacy pins; runtimeModelProviderSelection useLiteralKeys cleanup. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src-tauri/src/commands/agent_config.rs | 26 ++ .../src/commands/agent_config_tests.rs | 18 + .../src-tauri/src/managed_agents/discovery.rs | 7 +- .../agents/lib/agentConfigCore.test.mjs | 304 ++++++++++++++- .../features/agents/lib/agentConfigCore.ts | 86 ++++- .../agents/lib/personaCatalogRelay.test.mjs | 4 +- .../features/agents/ui/AgentConfigFields.tsx | 159 ++++---- .../agents/ui/AgentDefinitionDialog.tsx | 30 +- .../agents/ui/AgentInstanceEditDialog.tsx | 88 ++--- .../agents/ui/EditAgentAdvancedFields.tsx | 50 ++- .../agents/ui/PersonaAdvancedFields.tsx | 55 ++- .../agents/ui/agentConfigOptions.test.mjs | 39 +- .../features/agents/ui/agentConfigOptions.tsx | 13 +- .../src/features/agents/ui/bakedEnvHelpers.ts | 84 ++++- .../agents/ui/buzzAgentConfig.test.mjs | 345 ++++++++++++++++++ .../src/features/agents/ui/buzzAgentConfig.ts | 42 +++ .../agents/ui/buzzAgentModelTuningFields.tsx | 206 ++++++++++- .../ui/createAgentLocalModeGate.test.mjs | 336 +++++++++++++++++ .../agents/ui/personaRuntimeModel.test.mjs | 141 +++++++ .../features/agents/ui/personaRuntimeModel.ts | 21 +- .../ui/runtimeModelProviderSelection.test.mjs | 99 +++++ .../ui/runtimeModelProviderSelection.ts | 46 +++ .../agents/ui/useAgentDialogDefaults.ts | 34 +- desktop/src/shared/api/tauri.ts | 7 +- desktop/src/shared/api/types.ts | 10 +- desktop/src/testing/e2eBridge.ts | 6 + 26 files changed, 2042 insertions(+), 214 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 2dc0ba0d69..7da48931ff 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -209,6 +209,32 @@ pub struct BakedEnvEntry { pub masked: bool, } +/// Returns `true` when a baked-env key is safe to display unmasked in the UI. +/// +/// This uses an explicit allowlist of keys that are known safe (non-secret). +/// Any key NOT in this set is masked — default-deny for a security surface. +/// +/// Allowlist (case-insensitive): +/// - `BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL` — agent runtime selection +/// - All known native thinking-effort keys (non-secret enum values) — derived +/// from `ALL_KNOWN_EFFORT_KEYS` so this list stays in sync with runtime +/// metadata declarations (`BUZZ_AGENT_THINKING_EFFORT`, `GOOSE_THINKING_EFFORT`). +/// - `DATABRICKS_HOST`, `DATABRICKS_MODEL` — Block non-secret defaults +fn is_safe_to_reveal(key: &str) -> bool { + use crate::managed_agents::config_bridge::ALL_KNOWN_EFFORT_KEYS; + const SAFE_KEYS: &[&str] = &[ + "BUZZ_AGENT_PROVIDER", + "BUZZ_AGENT_MODEL", + "DATABRICKS_HOST", + "DATABRICKS_MODEL", + ]; + let upper = key.to_ascii_uppercase(); + SAFE_KEYS.iter().any(|safe| upper == *safe) + || ALL_KNOWN_EFFORT_KEYS + .iter() + .any(|effort| upper == effort.to_ascii_uppercase()) +} + /// Expose the baked build env to the frontend with values shown, but any /// key not in the safe-to-reveal allowlist has its value replaced by `••••••`. /// diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 01a9801489..b5e6096055 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -605,6 +605,21 @@ fn baked_env_thinking_effort_is_unmasked() { assert!(!effort.masked); } +#[test] +fn baked_env_goose_thinking_effort_is_unmasked() { + // GOOSE_THINKING_EFFORT is a non-secret canonical enum (off/low/medium/high/max) — + // must be revealed so the native baked lookup in `bakedEnvHelpers.ts` can read + // it as inherited effort for Goose agents (plan v3 pass-3 ★ pin). + let entries = baked_env_from_map(&[("GOOSE_THINKING_EFFORT", "high")]); + assert_eq!(entries.len(), 1); + let effort = entries + .iter() + .find(|e| e.key == "GOOSE_THINKING_EFFORT") + .unwrap(); + assert_eq!(effort.value, "high"); + assert!(!effort.masked); +} + #[test] fn baked_env_allowlist_is_case_insensitive() { // Known-safe keys — case-insensitive match must allow them. @@ -614,6 +629,9 @@ fn baked_env_allowlist_is_case_insensitive() { assert!(super::is_safe_to_reveal("BUZZ_AGENT_MODEL")); assert!(super::is_safe_to_reveal("buzz_agent_thinking_effort")); assert!(super::is_safe_to_reveal("BUZZ_AGENT_THINKING_EFFORT")); + // Goose native effort key — derived from ALL_KNOWN_EFFORT_KEYS. + assert!(super::is_safe_to_reveal("goose_thinking_effort")); + assert!(super::is_safe_to_reveal("GOOSE_THINKING_EFFORT")); assert!(super::is_safe_to_reveal("databricks_host")); assert!(super::is_safe_to_reveal("DATABRICKS_HOST")); assert!(super::is_safe_to_reveal("databricks_model")); diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index dbb9a5bfb2..5d90b38438 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -586,10 +586,8 @@ pub fn clear_resolve_cache() { } // ── Adapter availability cache (Phase-2 badge fallback) ───────────────────── -// // Warmed by `discover_acp_runtimes`; invalidated by `clear_resolve_cache`. -// Allows `build_managed_agent_summary` to compare adapter availability without -// spawning a probe subprocess on every poll cycle. +// Allows `build_managed_agent_summary` to check adapter availability without re-probing. fn adapter_availability_cache() -> &'static std::sync::Mutex> { use std::sync::{Mutex, OnceLock}; @@ -1547,8 +1545,7 @@ pub fn discover_acp_runtimes_from( command, binary_path, default_args, - // Custom harnesses are plain ACP — no MCP sidecar, no env-var - // model switching, no thinking knobs. + // Custom harnesses are plain ACP — no MCP sidecar, no env-var model switching, no thinking knobs. mcp_command: None, model_env_var: None, provider_env_var: None, diff --git a/desktop/src/features/agents/lib/agentConfigCore.test.mjs b/desktop/src/features/agents/lib/agentConfigCore.test.mjs index 92159ff275..8300ba4a85 100644 --- a/desktop/src/features/agents/lib/agentConfigCore.test.mjs +++ b/desktop/src/features/agents/lib/agentConfigCore.test.mjs @@ -28,6 +28,7 @@ function runtime(id, metadata = {}) { modelEnvVar: null, providerEnvVar: null, thinkingEnvVar: null, + acceptedEffortValues: null, maxTokensEnvVar: null, contextLimitEnvVar: null, maxRoundsEnvVar: null, @@ -75,6 +76,34 @@ test("Goose exposes provider, model, and its real effort application key", () => modelEnvVar: "GOOSE_MODEL", providerEnvVar: "GOOSE_PROVIDER", thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "global", + }); + + // With acceptedEffortValues, optionSource is "harnessNative" (single metadata authority). + assert.equal(field(model, "effort").optionSource, "harnessNative"); + // Persistence key is the native key, not the legacy key. + assert.deepEqual(field(model, "effort").currentPersistence, { + kind: "envVar", + key: "GOOSE_THINKING_EFFORT", + }); + assert.deepEqual(field(model, "effort").targetApplication, { + kind: "envVar", + key: "GOOSE_THINKING_EFFORT", + }); +}); + +test("Goose without acceptedEffortValues falls back to legacyProviderModelCatalog", () => { + // When a Goose catalog entry predates acceptedEffortValues (null), the field + // falls back to legacyProviderModelCatalog — existing behavior preserved. + const model = deriveAgentConfigFieldModel({ + config, + runtime: runtime("goose", { + modelEnvVar: "GOOSE_MODEL", + providerEnvVar: "GOOSE_PROVIDER", + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: null, // explicit null → legacy path }), scope: "global", }); @@ -83,16 +112,47 @@ test("Goose exposes provider, model, and its real effort application key", () => field(model, "effort").optionSource, "legacyProviderModelCatalog", ); + // Native key is still used as persistence key (v3 Phase 2 always uses native key). assert.deepEqual(field(model, "effort").currentPersistence, { - kind: "envVar", - key: "BUZZ_AGENT_THINKING_EFFORT", - }); - assert.deepEqual(field(model, "effort").targetApplication, { kind: "envVar", key: "GOOSE_THINKING_EFFORT", }); }); +test("Goose reads value from native key, falls back to legacy pre-migration save", () => { + // With both keys present, native wins (global scope — native-wins applies at all tiers). + const modelBothKeys = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { + GOOSE_THINKING_EFFORT: "medium", + BUZZ_AGENT_THINKING_EFFORT: "high", + }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "global", + }); + assert.equal(field(modelBothKeys, "effort").value, "medium"); + + // With only legacy key, read-old fallback kicks in at record/persona tier. + // At global scope, legacy is excluded (Delta 4 / effort_tier_alias(global_tier=true)). + const modelLegacyOnly = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "instance", + }); + assert.equal(field(modelLegacyOnly, "effort").value, "high"); +}); + test("Claude models effort as a deferred native ACP option", () => { const model = deriveAgentConfigFieldModel({ config, @@ -405,34 +465,35 @@ test("structuredEnvKeys_per_agent_buzz_agent_includes_effort_and_numeric_keys", 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. +test("structuredEnvKeys_per_agent_goose_includes_effort_and_numeric_keys", () => { + // Per-agent Goose with acceptedEffortValues now renders an effort control — + // the native key (GOOSE_THINKING_EFFORT) must appear in the structured set. + // The legacy key (BUZZ_AGENT_THINKING_EFFORT) must NOT appear — it has no + // editor on this surface and must stay visible as a generic env row. const gooseModel = deriveAgentConfigFieldModel({ config, runtime: runtime("goose", { thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], 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 renderedDescriptors = gooseModel.fields.filter( + (f) => f.render === "control", ); + const keys = structuredEnvKeys(renderedDescriptors); - const keys = structuredEnvKeys(numericDescriptorsOnly); - + assert.ok( + keys.includes("GOOSE_THINKING_EFFORT"), + "Goose native effort key must be hidden (control renders)", + ); 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", + "legacy key must NOT be hidden — no editor owns it for Goose", ); assert.ok( keys.includes("GOOSE_MAX_TOKENS"), @@ -561,3 +622,212 @@ test("NUMERIC_KIND_MIN_contextLimit_is_1", () => { test("NUMERIC_KIND_MIN_maxRounds_is_0", () => { assert.equal(NUMERIC_KIND_MIN.maxRounds, 0); }); + +test("unconsumed_legacy_effort_row_stays_visible_for_goose", () => { + // An invalid-for-Goose legacy value in a Goose record (BUZZ_AGENT_THINKING_EFFORT=minimal) + // must remain visible as a generic advanced-env row — it is not consumed as structured + // effort and must not be hidden. Only the native key (GOOSE_THINKING_EFFORT) is in + // structuredEnvKeys for Goose. (plan v3 pass-3 ★ pin: unconsumed legacy rows stay visible.) + // Uses `minimal` — invalid for Goose's canonical vocabulary — to pin the normalization + // rejection path: the value is present but normalization returns null → not consumed. + const gooseModel = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "minimal" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "definition", + }); + const renderedDescriptors = gooseModel.fields.filter( + (f) => f.render === "control", + ); + const keys = structuredEnvKeys(renderedDescriptors); + // Legacy key is NOT in structured keys for Goose → stays visible as advanced row. + assert.equal( + keys.includes("BUZZ_AGENT_THINKING_EFFORT"), + false, + "legacy key must not be hidden for Goose — no structured control owns it", + ); + // Invalid legacy value is not consumed → effort control shows empty. + assert.equal( + field(gooseModel, "effort").value, + null, + "invalid legacy `minimal` must not be consumed — effort control is empty", + ); +}); + +test("goose_legacy_only_persona_scope_renders_canonical_value", () => { + // Legacy-only Goose persona (scope="definition") — the legacy fallback must + // apply at this scope and render the normalized canonical value. + // Exercises the legacy read path for the persona editor surface specifically. + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "definition", + }); + assert.equal( + field(model, "effort").value, + "high", + "valid legacy value at persona scope must be shown as canonical effort value", + ); + assert.equal( + field(model, "effort").legacyConsumedKey, + "BUZZ_AGENT_THINKING_EFFORT", + "legacyConsumedKey set at persona scope so the duplicate env row is hidden", + ); +}); + +test("goose_alias_none_normalizes_to_off_in_display_value", () => { + // Pre-migration Goose record with BUZZ_AGENT_THINKING_EFFORT=none. + // The display value must be normalized to "off" (canonical) not "none" (alias). + // Uses scope: "instance" — legacy is consumed only at record/persona tiers. + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "none" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "instance", + }); + assert.equal( + field(model, "effort").value, + "off", + "none (Buzz alias) must normalize to off (Goose canonical) for display", + ); +}); + +test("goose_alias_xhigh_normalizes_to_max_in_display_value", () => { + // Pre-migration Goose record with BUZZ_AGENT_THINKING_EFFORT=xhigh. + // The display value must be normalized to "max" (canonical). + // Uses scope: "instance" — legacy is consumed only at record/persona tiers. + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "xhigh" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "instance", + }); + assert.equal( + field(model, "effort").value, + "max", + "xhigh (Buzz alias) must normalize to max (Goose canonical) for display", + ); +}); + +test("goose_invalid_legacy_effort_shows_null_value_not_alias", () => { + // BUZZ_AGENT_THINKING_EFFORT=minimal is invalid for Goose (no alias). Display + // value must be null (skip-as-absent), not the raw invalid string. + // Uses scope: "instance" — legacy is consumed only at record/persona tiers. + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "minimal" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "instance", + }); + assert.equal( + field(model, "effort").value, + null, + "minimal (invalid for Goose) must yield null display value", + ); +}); + +test("goose_legacy_consumed_as_effort_reports_legacyConsumedKey", () => { + // When a legacy value is consumed as Goose effort at record/persona tier + // (scope "instance"), the descriptor must set legacyConsumedKey so callers + // can suppress the dup row. Legacy is NOT consumed at global scope. + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "instance", + }); + assert.equal( + field(model, "effort").legacyConsumedKey, + "BUZZ_AGENT_THINKING_EFFORT", + "legacyConsumedKey must be set when legacy key value is shown via fallback", + ); +}); + +test("goose_global_scope_legacy_not_consumed_effort_empty_and_row_visible", () => { + // Plan v3 Delta 4 pin: at global scope, BUZZ_AGENT_THINKING_EFFORT is + // buzz-agent's native key and must NEVER be consumed as Goose effort. + // The effort control must show empty; the legacy key must NOT appear in + // structuredEnvKeys (so it remains a visible advanced row). + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "global", + }); + assert.equal( + field(model, "effort").value, + null, + "Goose effort control must be empty at global scope with only legacy key", + ); + assert.equal( + field(model, "effort").legacyConsumedKey, + undefined, + "legacyConsumedKey must be undefined at global scope — legacy stays a visible row", + ); + // Legacy key must NOT be hidden (not in structuredEnvKeys). + const renderedDescriptors = model.fields.filter( + (f) => f.render === "control", + ); + const keys = structuredEnvKeys(renderedDescriptors); + assert.equal( + keys.includes("BUZZ_AGENT_THINKING_EFFORT"), + false, + "legacy key must remain a visible advanced row at global scope for Goose", + ); +}); + +test("goose_native_key_set_does_not_report_legacyConsumedKey", () => { + // When native key is present, legacyConsumedKey must be undefined (no dup row). + const model = deriveAgentConfigFieldModel({ + config: { + ...config, + env_vars: { GOOSE_THINKING_EFFORT: "medium" }, + }, + runtime: runtime("goose", { + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }), + scope: "global", + }); + assert.equal( + field(model, "effort").legacyConsumedKey, + undefined, + "legacyConsumedKey must be undefined when native key is set", + ); +}); diff --git a/desktop/src/features/agents/lib/agentConfigCore.ts b/desktop/src/features/agents/lib/agentConfigCore.ts index 5a8b8cb1c3..1337ed8b1a 100644 --- a/desktop/src/features/agents/lib/agentConfigCore.ts +++ b/desktop/src/features/agents/lib/agentConfigCore.ts @@ -2,7 +2,10 @@ import type { AcpRuntimeCatalogEntry, GlobalAgentConfig, } from "@/shared/api/types"; -import { BUZZ_AGENT_THINKING_EFFORT } from "../ui/buzzAgentConfig"; +import { + BUZZ_AGENT_THINKING_EFFORT, + normalizeEffortValue, +} from "../ui/buzzAgentConfig"; /** * Lifecycle status of the ACP runtime catalog query on a per-agent surface. @@ -81,6 +84,9 @@ export type AgentConfigFieldDescriptor = | { kind: "acpConfigOption"; id: string; category: string }; render: "control" | "deferredUntilNativeOptionsAvailable"; value: string | null; + /** Set when a legacy key's value is being shown via fallback; callers + * should add this key to hidden-keys to suppress the duplicate row. */ + legacyConsumedKey?: string; } | { kind: "maxOutputTokens" | "contextLimit" | "maxRounds"; @@ -117,6 +123,40 @@ function valueFromEnv(config: GlobalAgentConfig, key: string) { return config.env_vars[key]?.trim() || null; } +/** + * Single policy source for reading effort from a flat env-var map. + * + * Native key takes precedence; legacy key is the fallback only at + * record/persona tiers (caller must pass `legacyKey` conditionally — pass + * `null` at global/onboarding scope to enforce the tier boundary). + * + * Returns `{ value, legacyConsumed }` where: + * - `value` — the normalized canonical value, or `null` if absent/invalid + * - `legacyConsumed` — true iff `value` came from the legacy key + * + * Used by both `deriveAgentConfigFieldModel` (model layer) and + * `HarnessNativeEffortFields` (component layer) so neither duplicates the + * native-first / valid-legacy-fallback / normalize logic. + */ +export function resolveEffortFromEnv( + envVars: Record, + nativeKey: string, + legacyKey: string | null, + acceptedValues: readonly string[] | null, +): { value: string | null; legacyConsumed: boolean } { + const nativeRaw = envVars[nativeKey]?.trim() || null; + const legacyRaw = legacyKey ? envVars[legacyKey]?.trim() || null : null; + const nativeValue = nativeRaw + ? (normalizeEffortValue(nativeRaw, acceptedValues) ?? null) + : null; + const legacyValue = legacyRaw + ? (normalizeEffortValue(legacyRaw, acceptedValues) ?? null) + : null; + const value = nativeValue ?? legacyValue; + const legacyConsumed = !nativeValue && !!legacyValue; + return { value, legacyConsumed }; +} + /** * Derives the numeric descriptor set for a runtime from catalog fields. * @@ -204,19 +244,47 @@ export function deriveAgentConfigFieldModel({ }); if (runtime?.thinkingEnvVar) { + const nativeKey = runtime.thinkingEnvVar; + const isBuzzAgent = runtime.id === "buzz-agent"; + // Legacy fallback applies only at record/persona tiers (scope "definition" or + // "instance") — mirrors spawn's effort_tier_alias(global_tier=true) policy. + // At global/onboarding scope, legacy is buzz-agent's native key (Delta 5) and + // must never be consumed as Goose effort. + const isRecordOrPersonaScope = + scope === "definition" || scope === "instance"; + const legacyKey = + !isBuzzAgent && isRecordOrPersonaScope + ? BUZZ_AGENT_THINKING_EFFORT + : null; + const acceptedValues = isBuzzAgent + ? null + : (runtime.acceptedEffortValues ?? null); + // resolveEffortFromEnv is the single policy source for effort reads — + // normalizes, applies native-first / valid-legacy-fallback, and reports + // which key was consumed. HarnessNativeEffortFields calls the same fn. + const { value: effortValue, legacyConsumed } = resolveEffortFromEnv( + config.env_vars, + nativeKey, + legacyKey, + acceptedValues, + ); fields.push({ kind: "effort", - optionSource: - runtime.id === "buzz-agent" - ? "buzzAgentCatalog" + optionSource: isBuzzAgent + ? "buzzAgentCatalog" + : runtime.acceptedEffortValues + ? "harnessNative" : "legacyProviderModelCatalog", currentPersistence: { kind: "envVar", - key: BUZZ_AGENT_THINKING_EFFORT, + key: nativeKey, }, - targetApplication: { kind: "envVar", key: runtime.thinkingEnvVar }, + targetApplication: { kind: "envVar", key: nativeKey }, render: "control", - value: valueFromEnv(config, BUZZ_AGENT_THINKING_EFFORT), + value: effortValue, + legacyConsumedKey: legacyConsumed + ? BUZZ_AGENT_THINKING_EFFORT + : undefined, }); } else if (runtime?.id === "claude") { fields.push({ @@ -291,9 +359,7 @@ export function getRenderableEffortField( * 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) + * - Per-agent Goose: effort key (GOOSE_THINKING_EFFORT) + 2 numeric keys */ export function structuredEnvKeys( renderedDescriptors: AgentConfigFieldDescriptor[], diff --git a/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs b/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs index fbaf1f5274..f3727598c0 100644 --- a/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs +++ b/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs @@ -356,7 +356,7 @@ test("test_foreign_entry_with_no_local_copy_stays_unselected", () => { BOB, ); - assert.equal(personas[0].id, "catalog:" + ALICE + ":reviewer"); + assert.equal(personas[0].id, `catalog:${ALICE}:reviewer`); assert.equal(personas[0].isActive, false); }); @@ -377,7 +377,7 @@ test("test_catalog_source_match_is_scoped_to_the_publishing_owner", () => { ALICE, ); - assert.equal(personas[0].id, "catalog:" + BOB + ":reviewer"); + assert.equal(personas[0].id, `catalog:${BOB}:reviewer`); assert.equal(personas[0].isActive, false); }); diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 11f68e8a56..44d3516fcf 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -28,6 +28,7 @@ import { filterBakedGenericRows, } from "@/features/agents/lib/agentConfigCore"; import { + bakedStructuredKeys, getBakedProviderInheritLabel, getGlobalModelFallback, } from "@/features/agents/ui/bakedEnvHelpers"; @@ -56,6 +57,7 @@ import { EffortSelectField, NumericTuningFields, useEffortAutoClear, + applyHarnessNativeEffortChange, type NumericDescriptor, } from "@/features/agents/ui/buzzAgentModelTuningFields"; import { SettingsOptionGroup } from "@/features/settings/ui/SettingsOptionGroup"; @@ -70,12 +72,6 @@ export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { preferred_runtime: null, }; -const BAKED_STRUCTURED_KEYS = new Set([ - "BUZZ_AGENT_PROVIDER", - "BUZZ_AGENT_MODEL", - BUZZ_AGENT_THINKING_EFFORT, -]); - const PROGRESSIVE_FIELDS_TRANSITION = { duration: 0.22, ease: [0.23, 1, 0.32, 1], @@ -85,14 +81,7 @@ type AgentConfigDisclosure = | "onboarding-essential" | "progressive-defaults"; -// Canonical behaviors (PR 2 flag cleanup). These were per-surface props; -// onboarding's values won every call and are now the only behavior: -// - auto-select a valid model when the provider changes -// - keep the model select usable during discovery -// - preserve credential env vars across provider switches (the abandoned -// provider's key stays in env_vars — visible/deletable under Advanced) -// - require a provider before model/effort are editable (no saveable -// invalid state — design principle #4) +// Canonical behaviors (PR 2 flag cleanup — baked-in for all surfaces): const autoSelectModelOnProviderChange = true; const disableModelSelectDuringDiscovery = false; const preserveCredentialEnvVarsOnProviderChange = true; @@ -254,6 +243,7 @@ export function AgentConfigFields({ effortField?.currentPersistence.kind === "envVar" ? effortField.currentPersistence.key : null; + const isHarnessNativeEffort = effortField?.optionSource === "harnessNative"; const numericDescriptors = fieldModel.fields.filter( (d): d is NumericDescriptor => @@ -262,10 +252,15 @@ export function AgentConfigFields({ d.kind === "maxRounds") && d.render === "control", ); - const allStructuredKeys = structuredEnvKeys([ - ...(effortField ? [effortField] : []), - ...numericDescriptors, - ]); + const allStructuredKeys = React.useMemo(() => { + const keys = structuredEnvKeys([ + ...(effortField ? [effortField] : []), + ...numericDescriptors, + ]); + if (effortField?.legacyConsumedKey) + keys.push(effortField.legacyConsumedKey); + return keys; + }, [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, @@ -286,8 +281,6 @@ export function AgentConfigFields({ const modelField = fieldModel.fields.find( (field) => field.kind === "model" && field.render === "control", ); - // CLI-login harnesses apply this setting through ACP rather than an env var - // and provide their own default when no model override is persisted. const modelIsOptional = modelField?.targetApplication.kind === "acpNative"; const modelIsValid = modelIsOptional || @@ -295,16 +288,18 @@ export function AgentConfigFields({ fallbackModel !== null; const bakedEffort = React.useMemo( () => - bakedEnv.find((e) => e.key === BUZZ_AGENT_THINKING_EFFORT)?.value ?? null, - [bakedEnv], + bakedEnv.find( + (e) => e.key === (effortPersistenceKey ?? BUZZ_AGENT_THINKING_EFFORT), + )?.value ?? null, + [bakedEnv, effortPersistenceKey], ); const bakedGenericRows = React.useMemo( () => filterBakedGenericRows(bakedEnv, [ - ...BAKED_STRUCTURED_KEYS, + ...bakedStructuredKeys(effortPersistenceKey ?? undefined), ...allStructuredKeys, ]), - [bakedEnv, allStructuredKeys], + [bakedEnv, allStructuredKeys, effortPersistenceKey], ); const providerValue = providerFieldVisible ? (config.provider ?? "") : ""; @@ -383,20 +378,12 @@ export function AgentConfigFields({ showCustomModelOption, }); - // Mount-time healing policy: onboarding page 4 edits the root config during - // first-run (no higher layers to inherit from), so acting on open is safe - // and intentional there — it heals stale state and picks a valid model. - // Evergreen surfaces (Settings, dialogs) edit saved data that may pair with - // higher layers (see PR #2148 review thread), so they only act after the - // user explicitly edits the provider in this session. + // Mount-time healing (onboarding page 4 only). Evergreen surfaces wait for + // explicit provider edit (see PR #2148). const healOnMount = fieldModel.dependentValuePolicy.onCatalogMismatch === "onboardingCleanup"; const userEditedProviderRef = React.useRef(false); - // Advanced visibility is user-controlled. Provider changes can add required - // rows, but must not open this section without an explicit toggle click. const [advancedOpen, setAdvancedOpen] = React.useState(false); - // Read inside effects via ref so biome's exhaustive-deps stays honest: - // refs are stable, and healOnMount is captured at declaration. const mayMutateDependentFieldsRef = React.useRef(false); mayMutateDependentFieldsRef.current = healOnMount || userEditedProviderRef.current; @@ -437,15 +424,12 @@ export function AgentConfigFields({ const currentEffortForAutoClear = effortPersistenceKey ? (config.env_vars[effortPersistenceKey] ?? "") : ""; + const effortValidForAutoClear = isHarnessNativeEffort + ? (selectedRuntime?.acceptedEffortValues ?? []) + : null; - // When the selected harness changes outside this component (Back → setup - // page → choose a different harness → Next), the saved model can belong to - // the old harness. In onboarding, heal that stale value as soon as the new - // harness catalog proves it is unsupported; otherwise a Codex id like - // `gpt-5.5[low]` appears as a Claude Code custom model. - // Also clear when the Model control is omitted after a confirmed successful - // empty catalog — never while discovery failed/unavailable (transient - // failures must not erase saved model/effort). + // Stale-model heal: onboarding heals on mount; evergreen waits for provider edit. + // See PR #2148 for why evergreen uses a ref-gated mutation policy. React.useEffect(() => { if (!healOnMount) return; const currentModel = (config.model ?? "").trim(); @@ -462,7 +446,9 @@ export function AgentConfigFields({ if (!catalogMiss && !omittedAfterSuccessfulEmpty) return; const nextEnvVars = { ...config.env_vars }; - if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; + if (effortPersistenceKey && !isHarnessNativeEffort) + delete nextEnvVars[effortPersistenceKey]; + if (!isHarnessNativeEffort) delete nextEnvVars[BUZZ_AGENT_THINKING_EFFORT]; onCustomModelEditingChange(false); onConfigChange({ ...config, env_vars: nextEnvVars, model: null }); }, [ @@ -476,28 +462,24 @@ export function AgentConfigFields({ onCustomModelEditingChange, healOnMount, effortPersistenceKey, + isHarnessNativeEffort, ]); - // Orphan-model clearing follows the mount-time healing policy above: the - // backend resolves provider and model independently across layers - // (agent → definition → global), so a saved global model WITHOUT a global - // provider can be a deliberate, working pattern (provider supplied by a - // higher layer). Clearing it on page-open in evergreen surfaces silently - // breaks that agent on its next restart — see PR #2148 review thread. - // Onboarding heals on open by design (discriminating spec: "gates stale - // saved model and effort until provider selection"). + // Orphan-model clear: same ref-gated policy as mount-time heal (see PR #2148). React.useEffect(() => { if (!mayMutateDependentFieldsRef.current) return; if (!dependentFieldsDisabled) return; if ( (config.model ?? "").trim().length === 0 && - currentEffortForAutoClear.length === 0 + (currentEffortForAutoClear.length === 0 || isHarnessNativeEffort) ) { return; } const nextEnvVars = { ...config.env_vars }; - if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; + if (effortPersistenceKey && !isHarnessNativeEffort) + delete nextEnvVars[effortPersistenceKey]; + if (!isHarnessNativeEffort) delete nextEnvVars[BUZZ_AGENT_THINKING_EFFORT]; onCustomModelEditingChange(false); onConfigChange({ ...config, env_vars: nextEnvVars, model: null }); }, [ @@ -507,17 +489,20 @@ export function AgentConfigFields({ onConfigChange, onCustomModelEditingChange, effortPersistenceKey, + isHarnessNativeEffort, ]); - const { validValues: effortValidForAutoClear } = getProviderEffortConfig( + const { validValues: providerModelEffortValid } = getProviderEffortConfig( config.provider ?? "", config.model ?? "", ); useEffortAutoClear({ - currentEffort: currentEffortForAutoClear, - effortValid: effortValidForAutoClear, + currentEffort: isHarnessNativeEffort ? "" : currentEffortForAutoClear, + effortValid: effortValidForAutoClear ?? providerModelEffortValid, onClear: () => { const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; + if (!isHarnessNativeEffort) + delete nextEnvVars[BUZZ_AGENT_THINKING_EFFORT]; onConfigChange({ ...config, env_vars: nextEnvVars }); }, }); @@ -634,9 +619,11 @@ export function AgentConfigFields({ : implicitEffortProvider; const { validValues: effortValid, defaultValue: effortDefault } = getProviderEffortConfig(effortProvider, config.model ?? ""); - const currentEffort = effortPersistenceKey - ? (config.env_vars[effortPersistenceKey] ?? "") - : ""; + const currentEffort = isHarnessNativeEffort + ? (effortField?.value ?? "") + : effortPersistenceKey + ? (config.env_vars[effortPersistenceKey] ?? "") + : ""; const effortFieldVisible = showEffortField && effortField !== undefined; const progressiveDefaults = disclosure === "progressive-defaults"; @@ -848,42 +835,60 @@ export function AgentConfigFields({ {effortFieldVisible ? (
{ - const nextEnvVars = { ...config.env_vars }; - if (value === "") { - if (effortPersistenceKey) - delete nextEnvVars[effortPersistenceKey]; - } else { - if (effortPersistenceKey) - nextEnvVars[effortPersistenceKey] = value; + if (isHarnessNativeEffort) { + if (!effortPersistenceKey) return; + onConfigChange({ + ...config, + env_vars: applyHarnessNativeEffortChange( + config.env_vars, + effortPersistenceKey, + null, + value, + ), + }); + return; } - onConfigChange({ ...config, env_vars: nextEnvVars }); + const next = { ...config.env_vars }; + if (value === "") { + if (effortPersistenceKey) delete next[effortPersistenceKey]; + delete next[BUZZ_AGENT_THINKING_EFFORT]; + } else { + if (effortPersistenceKey) next[effortPersistenceKey] = value; + if (effortPersistenceKey !== BUZZ_AGENT_THINKING_EFFORT) + delete next[BUZZ_AGENT_THINKING_EFFORT]; + } + onConfigChange({ ...config, env_vars: next }); }} placeholderClassName={placeholderClassName} selectClassName={selectClassName} - showUnavailableOptions={showUnavailableEffortOptions} + showUnavailableOptions={ + isHarnessNativeEffort ? undefined : showUnavailableEffortOptions + } testId="global-agent-thinking-effort-select" useCustomSelect={useCustomSelect} /> diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 12702f45ac..1ccdbdbc03 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -154,18 +154,14 @@ export function AgentDefinitionDialog({ const [behaviorDraft, setBehaviorDraft] = React.useState( emptyPersonaBehaviorDraft, ); - // The seed the draft is diffed against at submit: an untouched quad - // submits no behavior group, keeping unrelated edits hash-quiet. + // Draft seed; untouched quad submits no behavior group (hash-quiet). const behaviorSeedRef = React.useRef(emptyPersonaBehaviorDraft); // Tracks when the runtime was auto-seeded by the default-runtime effect in // edit mode (i.e. the user never explicitly chose a runtime). Used to omit // the seeded runtime from the submit payload for builtin definitions whose // canonical runtime is null — the sync would revert it anyway. const isRuntimeAutoSeededRef = React.useRef(false); - // Guards the seeding effect so it fires at most once per dialog-open. - // Without this, clearing runtime back to "" via "No preference" would re- - // trigger the effect (the `runtime` dep would pass the length guard) and - // snap the dropdown back to the default — an edit-mode regression. + // Guards seeding effect; fires at most once per open (prevents re-trigger on clear). const hasSeededForOpenRef = React.useRef(false); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [isAvatarUploadPending, setIsAvatarUploadPending] = @@ -179,7 +175,12 @@ export function AgentDefinitionDialog({ model: inheritedModelDefault, }, inheritedEnvVars: inheritedEnvVarsForAdvanced, - } = useAgentDialogDefaults({ open }); + } = useAgentDialogDefaults({ + open, + nativeEffortKey: runtimes.find((r) => r.id === runtime)?.thinkingEnvVar, + acceptedEffortValues: + runtimes.find((r) => r.id === runtime)?.acceptedEffortValues ?? null, + }); const defaultRuntime = React.useMemo( () => getDefaultPersonaRuntime(runtimes, globalConfig.preferred_runtime), [globalConfig.preferred_runtime, runtimes], @@ -445,13 +446,10 @@ export function AgentDefinitionDialog({ runtimeFileConfig, ], ); - // requiredEnvKeys: the gate already handles baked-, global-, and file- - // satisfied keys so no further filtering is needed. + // requiredEnvKeys: gate handles baked/global/file-satisfied keys; no extra filtering. const { requiredEnvKeys } = localModeGate; const localModeSatisfied = localModeGate.satisfied; - // Effective provider: agent value → global fallback → file fallback. - // Mirrors the chain inside computeLocalModeGate so model-option scoping and - // model requiredness are consistent with the readiness gate. + // Effective provider: agent value → global fallback → file fallback (mirrors gate chain). const fileProvider = runtimeFileConfig?.provider?.trim() ?? ""; const effectiveProvider = trimmedProvider || inheritedProviderDefault.value || fileProvider; @@ -522,9 +520,7 @@ export function AgentDefinitionDialog({ isCustomProviderEditing, modelFieldVisible, open, - // Gate provider by runtime: runtimes that don't support LLM provider - // selection (codex, claude) must not inherit the global provider — doing - // so causes them to discover models from the wrong provider. + // Gate provider by runtime: codex/claude must not inherit global provider. provider: runtimeSupportsLlmProviderSelection(runtime) ? effectiveProvider : "", @@ -683,6 +679,10 @@ export function AgentDefinitionDialog({ nextRuntime.trim().length > 0 && runtimeSupportsLlmProviderSelection(nextRuntime), lockedRuntimeReset: "full", + previousRuntimeNativeEffortKey: + runtimes.find((r) => r.id === runtime)?.thinkingEnvVar ?? null, + nextRuntimeNativeEffortKey: + runtimes.find((r) => r.id === nextRuntime)?.thinkingEnvVar ?? null, }), ); } diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index adfb8182a8..1ea7aae391 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -68,6 +68,7 @@ import { useRequiredCredentialState } from "./useRequiredCredentialState"; import { CreateAgentRespondToField } from "./RespondToField"; import { RunOnSummarySection } from "./RunOnSummarySection"; import { PersonaDropdownField } from "./PersonaDropdownField"; +import { ALL_KNOWN_EFFORT_KEYS } from "./buzzAgentConfig"; import { MODEL_DISCOVERY_LOADING_VALUE, usePersonaModelDiscovery, @@ -266,10 +267,8 @@ export function AgentInstanceEditDialog({ return runtimeSupportsLlmProviderSelection(matched?.id ?? ""); }, [runtimes, originalAgentCommand]); - // 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. + // Post-submit effective runtime: from pinned selection, persona runtime, or command/id match. + // Feeds both block-save gate and submit so they always agree. const prospectiveRuntimeId = React.useMemo(() => { if (!inheritHarness) { return selectedRuntime?.id ?? selectedRuntimeId; @@ -356,6 +355,13 @@ export function AgentInstanceEditDialog({ personaModel: linkedPersona?.model ?? null, envVars, personaEnvVars: inheritedEnvVars, + // Strip harness-native effort keys from the persona layer on inherit-transition. + // Effort inherits at spawn (persona tier, not via record materialization — Delta 5). + // Gated on thinkingEnvVar, so applies to all runtimes including buzz-agent: a + // pinned→inherit transition no longer materializes native effort into the record. + excludePersonaEnvKeys: prospectiveRuntime?.thinkingEnvVar + ? [prospectiveRuntime.thinkingEnvVar, ...ALL_KNOWN_EFFORT_KEYS] + : undefined, }), [ inheritHarness, @@ -366,6 +372,7 @@ export function AgentInstanceEditDialog({ linkedPersona?.model, envVars, inheritedEnvVars, + prospectiveRuntime?.thinkingEnvVar, ], ); @@ -376,7 +383,12 @@ export function AgentInstanceEditDialog({ model: inheritedModelDefault, }, inheritedEnvVars: inheritedEnvVarsForAdvanced, - } = useAgentDialogDefaults({ inheritedEnvVars, open }); + } = useAgentDialogDefaults({ + inheritedEnvVars, + open, + nativeEffortKey: prospectiveRuntime?.thinkingEnvVar, + acceptedEffortValues: prospectiveRuntime?.acceptedEffortValues ?? null, + }); // 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. @@ -393,12 +405,8 @@ export function AgentInstanceEditDialog({ const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open }); - // Merge global env as the base layer so credential keys satisfied via global - // config (e.g. ANTHROPIC_API_KEY) are available to model discovery. Use - // `inheritedSubmission.envVars` (the same snapshot the credential gate - // validates) rather than raw `envVars`, so an inherit-transition that layers - // in persona env vars is reflected in discovery. Agent-local env takes - // precedence, matching the agent → global → file spawn-path precedence. + // Merge global env as base for model discovery. Uses the credential-gate snapshot + // so inherit-transitions reflect persona env vars. Agent-local env wins. const envVarsForDiscovery = React.useMemo( () => ({ ...globalConfig.env_vars, ...inheritedSubmission.envVars }), [globalConfig.env_vars, inheritedSubmission.envVars], @@ -421,11 +429,7 @@ export function AgentInstanceEditDialog({ selectedRuntime, }); - // D2: derive advancedRequiredEnvKeys for EnvVarsEditor display. - // The full requiredEnvKeys/requiredEnvKeyMissing continue driving Save gating. - // D2/D3: the top-level API key owns display, while the readiness gate keeps - // the complete required-key list. The effective snapshot covers persona - // inheritance during an instance inherit transition. + // API key env var for display (D2/D3). Save gate uses full requiredEnvKeys. const providerApiKeyEnvVar = getProviderApiKeyEnvVar(effectiveProvider); const personaSatisfied = providerApiKeyEnvVar != null && @@ -490,6 +494,18 @@ export function AgentInstanceEditDialog({ setEnvVars(next.envVars); } + /** Pin↔inherit toggle: clear all effort keys (Delta-3 record-scope clear). */ + function handleInheritHarnessChange(nextInherit: boolean) { + setInheritHarness(nextInherit); + if (prospectiveRuntime?.thinkingEnvVar) { + setEnvVars((prev: EnvVarsValue) => { + const next = { ...prev }; + for (const key of ALL_KNOWN_EFFORT_KEYS) delete next[key]; + return next; + }); + } + } + function handleRuntimeDropdownChange(nextValue: string) { const action = runtimeDropdownAction(nextValue); if (action.kind === "add-custom-harness") { @@ -539,6 +555,10 @@ export function AgentInstanceEditDialog({ nextRuntime?.id ?? nextRuntimeId, ), lockedRuntimeReset: "full", + previousRuntimeNativeEffortKey: + runtimes.find((r) => r.id === previousRuntimeId)?.thinkingEnvVar ?? + null, + nextRuntimeNativeEffortKey: nextRuntime?.thinkingEnvVar ?? null, }), ); } @@ -634,14 +654,9 @@ export function AgentInstanceEditDialog({ agentCommandOverride: agent.agentCommandOverride ?? null, }); - // Classify the effective post-submit runtime's provider capability as a - // tri-state: "capable" persists the provider, "locked" clears it (only - // when we KNOW it's provider-locked, e.g. Claude), "unknown" OMITS it so a - // transient/custom state never becomes a destructive write. Resolved - // STATICALLY (by id) so a not-yet-loaded catalog can't misclassify a known - // runtime as "unknown" — see resolveRuntimeProviderCapability. The runtime - // id is the shared prospectiveRuntimeId, so submit and the block-save gate - // always agree on which runtime is being saved. + // Provider capability: "capable" persists, "locked" clears, "unknown" omits. + // Resolved statically so unloaded catalog can't misclassify. Shared id ensures + // submit and block-save gate agree. const providerRuntimeCapability = resolveRuntimeProviderCapability( prospectiveRuntimeId, runtimeSupportsLlmProviderSelection(prospectiveRuntimeId), @@ -662,10 +677,7 @@ export function AgentInstanceEditDialog({ ? acpCommand.trim() : undefined, agentCommand: agentCommandUpdate, - // A non-inheriting selection is a deliberate pin — signal it so the - // backend preserves a Custom/runtime command even when it maps to the - // linked persona's own runtime (otherwise it would be dropped back to - // inherit). Omitted (falsy) when inheriting or on a name-only edit. + // Non-inheriting = explicit pin; preserves Custom commands vs linked persona. Omitted when inheriting. harnessOverride: agentCommandUpdate != null ? !inheritHarness : undefined, agentArgs: @@ -689,11 +701,7 @@ export function AgentInstanceEditDialog({ : normalizedModel !== (agent.model ?? null) ? normalizedModel : undefined, - // Tri-state provider persistence keyed on providerRuntimeCapability: - // "capable" → persist: value if changed, omit if unchanged. - // "locked" → clear: send null if provider was set, else omit. - // "unknown" → omit always (never send null for a transient state). - // llmProviderFieldVisible is for UX visibility only; not used here. + // Tri-state provider: "capable"→persist if changed, "locked"→clear, "unknown"→omit. provider: linkedPersona != null ? undefined @@ -710,12 +718,7 @@ export function AgentInstanceEditDialog({ ? undefined : submitEnvVars, respondTo: respondTo !== agent.respondTo ? respondTo : undefined, - // The allowlist is preserved across mode toggles in local UI state - // (so a user can flip away from allowlist and back without losing - // their entries), but we only send it on the wire when (a) it - // actually changed, AND (b) the saved mode will need it. Sending - // an allowlist while switching to a non-allowlist mode would be - // harmless server-side, but it's noise in the persisted record. + // Send allowlist only when changed AND saved mode is "allowlist". respondToAllowlist: respondTo === "allowlist" && respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") @@ -735,10 +738,7 @@ export function AgentInstanceEditDialog({ showAgentProfileSyncWarning(result.agent.name, result.profileSyncError); handleOpenChange(false); onUpdated?.(result.agent); - // The auto-restart policy deliberately never fires for a stopped or - // failing agent (a broken agent must not auto-loop), so an edit meant - // to FIX one silently waits for a manual start. Offer that start - // explicitly instead of relying on the user to know the policy. + // Auto-restart doesn't fire for stopped/failing agents; offer manual start. if (!isManagedAgentActive(result.agent)) { const startedName = result.agent.name; toast(`${startedName} saved while stopped.`, { @@ -1204,7 +1204,7 @@ export function AgentInstanceEditDialog({ onAgentArgsChange={setAgentArgs} onAutoRestartChange={setAutoRestartOnConfigChange} onEnvVarsChange={setEnvVars} - onInheritHarnessChange={setInheritHarness} + onInheritHarnessChange={handleInheritHarnessChange} onParallelismChange={setParallelism} onSystemPromptChange={setSystemPrompt} /> diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index 56685fd6c4..9ead4bb06c 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -13,6 +13,7 @@ import type { AgentPersona } from "@/shared/api/types"; import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; import { BuzzAgentModelTuningFields, + HarnessNativeEffortFields, NumericTuningFields, } from "./buzzAgentModelTuningFields"; import { @@ -24,6 +25,7 @@ import { deriveNumericDescriptors, structuredEnvKeys, type RuntimeCatalogStatus, + resolveEffortFromEnv, } from "../lib/agentConfigCore"; export function EditAgentAdvancedFields({ @@ -115,17 +117,50 @@ export function EditAgentAdvancedFields({ [catalogStatus, selectedRuntime], ); + // Harness-native effort: shown when the runtime declares a static canonical + // effort vocabulary (e.g. Goose). The native key is hidden from the generic + // env editor. The legacy key is hidden only when it was validly consumed. + const harnessNativeEffort = + catalogStatus === "ready" + ? (selectedRuntime?.acceptedEffortValues ?? null) + : null; + const harnessNativeEffortKey = selectedRuntime?.thinkingEnvVar ?? null; + // EditAgentAdvancedFields operates at "instance" scope — legacy fallback applies. + // resolveEffortFromEnv is the single policy source; the component reads the same fn. + const legacyEffortConsumed = React.useMemo(() => { + if (!harnessNativeEffort || !harnessNativeEffortKey) return false; + return resolveEffortFromEnv( + envVars, + harnessNativeEffortKey, + BUZZ_AGENT_THINKING_EFFORT, + harnessNativeEffort, + ).legacyConsumed; + }, [envVars, harnessNativeEffort, harnessNativeEffortKey]); + // Build the effective hidden-key list: caller's secrets + effort key (when - // rendered by BuzzAgentModelTuningFields) + numeric keys via structuredEnvKeys. + // rendered by BuzzAgentModelTuningFields or HarnessNativeEffortFields) + numeric keys. const effectiveHiddenKeys = React.useMemo( () => [ ...hiddenEnvKeys, ...(isBuzzAgentRuntime(modelTuningRuntimeId) ? [BUZZ_AGENT_THINKING_EFFORT] : []), + ...(harnessNativeEffort && harnessNativeEffortKey + ? [ + harnessNativeEffortKey, + ...(legacyEffortConsumed ? [BUZZ_AGENT_THINKING_EFFORT] : []), + ] + : []), ...structuredEnvKeys(numericDescriptors), ], - [hiddenEnvKeys, modelTuningRuntimeId, numericDescriptors], + [ + hiddenEnvKeys, + modelTuningRuntimeId, + harnessNativeEffort, + harnessNativeEffortKey, + legacyEffortConsumed, + numericDescriptors, + ], ); return ( @@ -334,7 +369,7 @@ export function EditAgentAdvancedFields({ /> ) : null} - {/* Effort-tuning knob — only shown for buzz-agent. */} + {/* Effort-tuning knob — buzz-agent (catalog-based) or harness-native. */} {isBuzzAgentRuntime(modelTuningRuntimeId) ? ( + ) : harnessNativeEffort && harnessNativeEffortKey ? ( + ) : null}
); diff --git a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx index 1ecd98b83f..27fdabde30 100644 --- a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx @@ -14,6 +14,7 @@ import { } from "../lib/agentParallelism"; import { BuzzAgentModelTuningFields, + HarnessNativeEffortFields, NumericTuningFields, } from "./buzzAgentModelTuningFields"; import { @@ -27,6 +28,7 @@ import { deriveNumericDescriptors, structuredEnvKeys, type RuntimeCatalogStatus, + resolveEffortFromEnv, } from "../lib/agentConfigCore"; export function PersonaAdvancedFields({ @@ -92,15 +94,55 @@ export function PersonaAdvancedFields({ [catalogStatus, selectedRuntime], ); + // Harness-native effort: shown when the runtime declares a static canonical + // effort vocabulary (e.g. Goose). The native key is hidden from the generic + // env editor. The legacy key is hidden only when it was validly consumed — + // i.e. the native key is absent, the legacy key is present, and its value + // normalizes to a canonical form. Invalid legacy values stay visible as + // advanced rows (plan v3 Delta 4 / ★ unconsumed-row visibility pin). + const harnessNativeEffort = + catalogStatus === "ready" + ? (selectedRuntime?.acceptedEffortValues ?? null) + : null; + const harnessNativeEffortKey = selectedRuntime?.thinkingEnvVar ?? null; + // PersonaAdvancedFields operates at "definition" scope — legacy fallback applies. + // resolveEffortFromEnv is the single policy source; the component reads the same fn. + const legacyEffortConsumed = React.useMemo(() => { + if (!harnessNativeEffort || !harnessNativeEffortKey) return false; + return resolveEffortFromEnv( + envVars, + harnessNativeEffortKey, + BUZZ_AGENT_THINKING_EFFORT, + harnessNativeEffort, + ).legacyConsumed; + }, [envVars, harnessNativeEffort, harnessNativeEffortKey]); + const effectiveHiddenKeys = React.useMemo( () => [ ...hiddenEnvKeys, ...(isBuzzAgentRuntime(modelTuningRuntimeId) ? [BUZZ_AGENT_THINKING_EFFORT] : []), + // When the harness-native effort control is shown, hide the native key + // from generic env editor. Also hide the legacy key when it was validly + // consumed (i.e. it provided the displayed effort value); leave it visible + // when invalid or not consumed. + ...(harnessNativeEffort && harnessNativeEffortKey + ? [ + harnessNativeEffortKey, + ...(legacyEffortConsumed ? [BUZZ_AGENT_THINKING_EFFORT] : []), + ] + : []), ...structuredEnvKeys(numericDescriptors), ], - [hiddenEnvKeys, modelTuningRuntimeId, numericDescriptors], + [ + hiddenEnvKeys, + modelTuningRuntimeId, + harnessNativeEffort, + harnessNativeEffortKey, + legacyEffortConsumed, + numericDescriptors, + ], ); return (
@@ -222,7 +264,7 @@ export function PersonaAdvancedFields({ /> ) : null} - {/* Effort-tuning knob — only shown for buzz-agent. */} + {/* Effort-tuning knob — buzz-agent (catalog-based) or harness-native. */} {isBuzzAgentRuntime(modelTuningRuntimeId) ? ( + ) : harnessNativeEffort && harnessNativeEffortKey ? ( + ) : null}
); diff --git a/desktop/src/features/agents/ui/agentConfigOptions.test.mjs b/desktop/src/features/agents/ui/agentConfigOptions.test.mjs index eb24d88084..2866f201ce 100644 --- a/desktop/src/features/agents/ui/agentConfigOptions.test.mjs +++ b/desktop/src/features/agents/ui/agentConfigOptions.test.mjs @@ -147,22 +147,55 @@ test("runtimeSupportsLlmProviderSelection is false for codex and claude", () => assert.equal(runtimeSupportsLlmProviderSelection("claude"), false); }); -test("resetConfigForHarnessChange clears harness-specific values", () => { +test("resetConfigForHarnessChange clears harness-specific values but preserves effort keys", () => { + // Delta-5 global-scope rule: ALL effort keys are preserved across runtime + // switches at global scope. Both BUZZ_AGENT_THINKING_EFFORT and + // GOOSE_THINKING_EFFORT survive a switch — each runtime reads only its own key. const config = { - env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high", KEEP_ME: "yes" }, + env_vars: { + BUZZ_AGENT_THINKING_EFFORT: "high", + GOOSE_THINKING_EFFORT: "medium", + KEEP_ME: "yes", + }, model: "claude-opus", preferred_runtime: "buzz-agent", provider: "anthropic", }; assert.deepEqual(resetConfigForHarnessChange(config, "claude"), { - env_vars: { KEEP_ME: "yes" }, + env_vars: { + BUZZ_AGENT_THINKING_EFFORT: "high", + GOOSE_THINKING_EFFORT: "medium", + KEEP_ME: "yes", + }, model: null, preferred_runtime: "claude", provider: null, }); }); +test("resetConfigForHarnessChange buzz-to-goose preserves both runtimes native effort keys", () => { + // Global switch: BUZZ_AGENT_THINKING_EFFORT and GOOSE_THINKING_EFFORT both + // survive. Each runtime descriptor uses only its own key; the other is a + // foreign key that its descriptor strips (Phase 3 invariant). At the global + // mutation layer, no key is deleted. + const config = { + env_vars: { + BUZZ_AGENT_THINKING_EFFORT: "high", + GOOSE_THINKING_EFFORT: "low", + }, + model: "gpt-4", + preferred_runtime: "buzz-agent", + provider: "openai", + }; + + const result = resetConfigForHarnessChange(config, "goose"); + assert.deepEqual(result.env_vars, { + BUZZ_AGENT_THINKING_EFFORT: "high", + GOOSE_THINKING_EFFORT: "low", + }); +}); + test("resetConfigForHarnessChange preserves compatible provider selection", () => { const config = { env_vars: { KEEP_ME: "yes" }, diff --git a/desktop/src/features/agents/ui/agentConfigOptions.tsx b/desktop/src/features/agents/ui/agentConfigOptions.tsx index 5c515a0507..9d670880b8 100644 --- a/desktop/src/features/agents/ui/agentConfigOptions.tsx +++ b/desktop/src/features/agents/ui/agentConfigOptions.tsx @@ -2,7 +2,6 @@ import type { AcpRuntimeCatalogEntry, GlobalAgentConfig, } from "@/shared/api/types"; -import { BUZZ_AGENT_THINKING_EFFORT } from "./buzzAgentConfig"; import type { RuntimeFileConfigSubset } from "@/shared/api/tauri"; // Dialogs import getDefaultPersonaRuntime via this re-export; lib code imports // directly from lib/resolvePersonaRuntime. @@ -204,13 +203,21 @@ export function runtimeSupportsLlmProviderSelection(runtimeId: string) { return runtimeId === "buzz-agent" || runtimeId === "goose"; } -/** Clears values whose meaning or support changes with the selected harness. */ +/** Clears values whose meaning or support changes with the selected harness. + * + * Used at **global and onboarding scope** — where the Delta-5 global-scope + * mutation rule applies: native effort keys from ALL runtimes are preserved + * (both BUZZ_AGENT_THINKING_EFFORT and GOOSE_THINKING_EFFORT survive a switch). + * Legacy-key deletion and native-key clearing happen only at record/persona + * tiers via the effort transition mutation in `selectionOnRuntimeChange`. + */ export function resetConfigForHarnessChange( config: GlobalAgentConfig, runtimeId: string, ): GlobalAgentConfig { + // Do NOT delete any effort keys: global scope preserves both runtimes' + // native keys and the legacy key (Delta-5 global-scope mutation rule). const nextEnvVars = { ...config.env_vars }; - delete nextEnvVars[BUZZ_AGENT_THINKING_EFFORT]; return { ...config, diff --git a/desktop/src/features/agents/ui/bakedEnvHelpers.ts b/desktop/src/features/agents/ui/bakedEnvHelpers.ts index 40b3328256..28320baf8c 100644 --- a/desktop/src/features/agents/ui/bakedEnvHelpers.ts +++ b/desktop/src/features/agents/ui/bakedEnvHelpers.ts @@ -4,6 +4,29 @@ * them without pulling in React, Tauri IPC, or TanStack Query. */ +import { + BUZZ_AGENT_THINKING_EFFORT, + normalizeEffortValue, +} from "./buzzAgentConfig"; + +const BAKED_STRUCTURED_KEYS_BASE = new Set([ + "BUZZ_AGENT_PROVIDER", + "BUZZ_AGENT_MODEL", +]); + +/** + * Returns the set of baked env keys hidden by structured controls. + * Only the current runtime's native effort key is hidden; a baked legacy key + * for a non-buzz-agent runtime remains visible as an advanced row (Delta-4). + */ +export function bakedStructuredKeys( + nativeEffortKey: string | undefined, +): Set { + const keys = new Set(BAKED_STRUCTURED_KEYS_BASE); + if (nativeEffortKey) keys.add(nativeEffortKey); + return keys; +} + /** * Return the provider option label for the zero-value (inherit) option when a * baked provider is present. Falls back to the raw provider id when the id @@ -143,11 +166,50 @@ export function getInheritedAgentDefaults( provider: string | null; }, bakedEnv: readonly BakedEnvEntry[] | undefined, + options?: { + /** + * The native effort env key for the current runtime (e.g. `GOOSE_THINKING_EFFORT`). + * When provided (and different from the legacy key), the native key is checked + * first and baked lookup uses native-only (no legacy fallback for baked tier). + * Defaults to `BUZZ_AGENT_THINKING_EFFORT` (buzz-agent behavior unchanged). + */ + nativeEffortKey?: string; + /** + * Canonical effort values for the current runtime (from `runtime.acceptedEffortValues`). + * Used to normalize the global native effort value before display. + * Pass `null` for buzz-agent (per-model catalog, no static vocabulary). + */ + acceptedEffortValues?: readonly string[] | null; + }, ): { effort: InheritedDefault; model: InheritedDefault; provider: InheritedDefault; } { + const nativeEffortKey = + options?.nativeEffortKey ?? BUZZ_AGENT_THINKING_EFFORT; + + // Effort value from global config: native key only. + // Global legacy (BUZZ_AGENT_THINKING_EFFORT for non-buzz-agent runtimes) is + // excluded — mirrors effort_tier_alias(global_tier=true) in Rust: spawn and + // the reader both exclude legacy at the global tier. For Goose, the legacy + // key is buzz-agent's native key (Delta 5); consuming it here would display + // buzz-agent's effort setting as Goose effort. Normalize the raw value so + // global GOOSE_THINKING_EFFORT=xhigh displays as canonical "max", not "xhigh" + // (acceptedEffortValues drives normalization; null = buzz-agent pass-through). + const globalNativeRaw = globalConfig.env_vars[nativeEffortKey]?.trim(); + const globalNativeEffort = globalNativeRaw + ? (normalizeEffortValue( + globalNativeRaw, + options?.acceptedEffortValues ?? null, + ) ?? null) + : null; + const globalEffortValue = globalNativeEffort || null; + + // Baked effort lookup: always use the native key (never the legacy key at + // baked tier, mirroring the global-tier native-only rule). + const bakedEffortKey = nativeEffortKey; + return { provider: resolveInheritedDefault( globalConfig.provider, @@ -175,10 +237,22 @@ export function getInheritedAgentDefaults( ? { source: "build", value: fallback } : { source: null, value: "" }; })(), - effort: resolveInheritedDefault( - globalConfig.env_vars.BUZZ_AGENT_THINKING_EFFORT, - bakedEnv, - "BUZZ_AGENT_THINKING_EFFORT", - ), + effort: (() => { + // Global config effort wins (native then legacy fallback). + if (globalEffortValue) { + return { + source: "global" as InheritedDefaultSource, + value: globalEffortValue, + }; + } + // Baked effort — native-only lookup for non-buzz-agent runtimes. + const baked = bakedEnv?.find( + (entry) => entry.key === bakedEffortKey && !entry.masked, + ); + const bakedValue = baked?.value.trim() ?? ""; + return bakedValue + ? { source: "build" as InheritedDefaultSource, value: bakedValue } + : { source: null, value: "" }; + })(), }; } diff --git a/desktop/src/features/agents/ui/buzzAgentConfig.test.mjs b/desktop/src/features/agents/ui/buzzAgentConfig.test.mjs index 4d702966b7..4395517732 100644 --- a/desktop/src/features/agents/ui/buzzAgentConfig.test.mjs +++ b/desktop/src/features/agents/ui/buzzAgentConfig.test.mjs @@ -9,7 +9,10 @@ import { BUZZ_AGENT_THINKING_EFFORT_VALUES, getProviderEffortConfig, isBuzzAgentRuntime, + normalizeEffortValue, } from "./buzzAgentConfig.ts"; +import { resolveEffortFromEnv } from "../lib/agentConfigCore.ts"; +import { applyHarnessNativeEffortChange } from "./buzzAgentModelTuningFields.tsx"; // --------------------------------------------------------------------------- // Thinking effort values @@ -621,3 +624,345 @@ test("effort none is invalid for anthropic manual-budget (should trigger auto-cl "none must not be in manual-budget set", ); }); + +// --------------------------------------------------------------------------- +// applyHarnessNativeEffortChange — production mutation contract +// --------------------------------------------------------------------------- +// +// These tests exercise the PRODUCTION helper imported from +// buzzAgentModelTuningFields.tsx — the same function called by +// HarnessNativeEffortFields.handleChange and AgentConfigFields' effort onChange. +// Two scope variants: +// record/persona scope: legacyKey = BUZZ_AGENT_THINKING_EFFORT (atomically deleted) +// global/onboarding scope: legacyKey = null (Delta 5: never delete foreign native key) +// +// Source: goose `crates/goose-provider-types/src/thinking.rs:277-308` +// Canonical: off|low|medium|high|max (exactly 5 values, no minimal/none/xhigh) + +const GOOSE_NATIVE_KEY = "GOOSE_THINKING_EFFORT"; +// Forward-declare here so mutation + normalization tests share one definition. +const GOOSE_VALUES = ["off", "low", "medium", "high", "max"]; + +test("harness_native_save_writes_native_key_and_deletes_legacy", () => { + // Legacy-only state (pre-migration Goose record): selecting a value must + // write the native key and atomically delete the legacy key. + const initial = { [BUZZ_AGENT_THINKING_EFFORT]: "high" }; + const result = applyHarnessNativeEffortChange( + initial, + GOOSE_NATIVE_KEY, + BUZZ_AGENT_THINKING_EFFORT, + "medium", + ); + assert.equal(result[GOOSE_NATIVE_KEY], "medium", "native key written"); + assert.equal( + Object.hasOwn(result, BUZZ_AGENT_THINKING_EFFORT), + false, + "legacy key deleted atomically on save", + ); +}); + +test("harness_native_clear_deletes_both_native_and_legacy_keys", () => { + // When clear is selected (value=""), both native and legacy keys are removed. + const initial = { + [GOOSE_NATIVE_KEY]: "high", + [BUZZ_AGENT_THINKING_EFFORT]: "xhigh", + }; + const result = applyHarnessNativeEffortChange( + initial, + GOOSE_NATIVE_KEY, + BUZZ_AGENT_THINKING_EFFORT, + "", + ); + assert.equal( + Object.hasOwn(result, GOOSE_NATIVE_KEY), + false, + "native key deleted on clear", + ); + assert.equal( + Object.hasOwn(result, BUZZ_AGENT_THINKING_EFFORT), + false, + "legacy key deleted on clear", + ); +}); + +test("harness_native_save_does_not_disturb_unrelated_env_vars", () => { + // Saving effort must not affect unrelated env keys. + const initial = { + [BUZZ_AGENT_THINKING_EFFORT]: "medium", + SOME_OTHER_KEY: "value", + GOOSE_MAX_TOKENS: "4096", + }; + const result = applyHarnessNativeEffortChange( + initial, + GOOSE_NATIVE_KEY, + BUZZ_AGENT_THINKING_EFFORT, + "off", + ); + assert.equal(result.SOME_OTHER_KEY, "value", "unrelated key preserved"); + assert.equal(result.GOOSE_MAX_TOKENS, "4096", "numeric key preserved"); + assert.equal(result[GOOSE_NATIVE_KEY], "off", "native key written"); +}); + +test("harness_native_clear_does_not_disturb_unrelated_env_vars", () => { + // Clearing effort must not affect unrelated env keys. + const initial = { + [GOOSE_NATIVE_KEY]: "high", + SOME_OTHER_KEY: "value", + GOOSE_MAX_TOKENS: "4096", + }; + const result = applyHarnessNativeEffortChange( + initial, + GOOSE_NATIVE_KEY, + BUZZ_AGENT_THINKING_EFFORT, + "", + ); + assert.equal( + result.SOME_OTHER_KEY, + "value", + "unrelated key preserved after clear", + ); + assert.equal( + result.GOOSE_MAX_TOKENS, + "4096", + "numeric key preserved after clear", + ); + assert.equal( + Object.hasOwn(result, GOOSE_NATIVE_KEY), + false, + "native key deleted", + ); +}); + +test("harness_native_global_scope_save_preserves_buzz_agent_effort", () => { + // At global/onboarding scope (legacyKey=null), saving Goose effort must NOT + // delete BUZZ_AGENT_THINKING_EFFORT — it is buzz-agent's own global native key + // (Delta 5: Goose and buzz-agent global keys coexist independently). + const initial = { + [BUZZ_AGENT_THINKING_EFFORT]: "high", // buzz-agent's independent global effort + SOME_OTHER_KEY: "value", + }; + const result = applyHarnessNativeEffortChange( + initial, + GOOSE_NATIVE_KEY, + null, // global scope: never delete foreign native key + "medium", + ); + assert.equal(result[GOOSE_NATIVE_KEY], "medium", "Goose native key written"); + assert.equal( + result[BUZZ_AGENT_THINKING_EFFORT], + "high", + "buzz-agent effort preserved (Delta 5: no foreign-key deletion)", + ); + assert.equal(result.SOME_OTHER_KEY, "value", "unrelated key preserved"); +}); + +test("harness_native_global_scope_clear_preserves_buzz_agent_effort", () => { + // At global/onboarding scope, clearing Goose effort (Inherit) must NOT + // delete BUZZ_AGENT_THINKING_EFFORT (Delta 5). + const initial = { + [GOOSE_NATIVE_KEY]: "medium", + [BUZZ_AGENT_THINKING_EFFORT]: "high", + }; + const result = applyHarnessNativeEffortChange( + initial, + GOOSE_NATIVE_KEY, + null, // global scope + "", + ); + assert.equal( + Object.hasOwn(result, GOOSE_NATIVE_KEY), + false, + "Goose native key cleared", + ); + assert.equal( + result[BUZZ_AGENT_THINKING_EFFORT], + "high", + "buzz-agent effort preserved on Goose clear", + ); +}); + +test("harness_native_five_canonical_choices_for_goose", () => { + // Goose's accepted effort vocabulary is exactly the 5 canonical values. + // The component offers these as the selectable options (plus "Inherit"). + const GOOSE_ACCEPTED = ["off", "low", "medium", "high", "max"]; + assert.equal( + GOOSE_ACCEPTED.length, + 5, + "exactly 5 canonical Goose effort values", + ); + assert.deepEqual( + GOOSE_ACCEPTED, + ["off", "low", "medium", "high", "max"], + "canonical order: off→low→medium→high→max", + ); + // Verify minimal is NOT a Goose value (buzz-agent only). + assert.equal( + GOOSE_ACCEPTED.includes("minimal"), + false, + "minimal is not a Goose value", + ); + // Verify none is NOT canonical (it is an alias for off). + assert.equal( + GOOSE_ACCEPTED.includes("none"), + false, + "none is not canonical (alias for off)", + ); +}); + +test("harness_native_legacy_to_native_migration_at_persona_scope_renders_value", () => { + // A legacy-only Goose persona (scope="definition") with a valid legacy value + // must display the normalized canonical value via the legacy fallback path. + // Exercises resolveEffortFromEnv — the same function HarnessNativeEffortFields + // calls — so this test pins the actual production read path. + + // Case 1: buzz-agent alias xhigh normalizes to max. + const result1 = resolveEffortFromEnv( + { [BUZZ_AGENT_THINKING_EFFORT]: "xhigh" }, + "GOOSE_THINKING_EFFORT", + BUZZ_AGENT_THINKING_EFFORT, + GOOSE_VALUES, + ); + assert.equal( + result1.value, + "max", + "xhigh alias normalized to max for display", + ); + assert.equal( + result1.legacyConsumed, + true, + "legacy key consumed when native absent", + ); + + // Case 2: canonical value passes through unchanged. + const result2 = resolveEffortFromEnv( + { [BUZZ_AGENT_THINKING_EFFORT]: "high" }, + "GOOSE_THINKING_EFFORT", + BUZZ_AGENT_THINKING_EFFORT, + GOOSE_VALUES, + ); + assert.equal(result2.value, "high", "canonical legacy value passes through"); + assert.equal( + result2.legacyConsumed, + true, + "legacy key consumed when native absent", + ); +}); + +test("harness_native_invalid_native_plus_valid_legacy_renders_legacy_value", () => { + // Invalid native value (e.g. "minimal" — not in Goose vocab) + valid legacy value: + // resolveEffortFromEnv must skip the invalid native and fall back to the valid legacy. + // This verifies that the component and deriveAgentConfigFieldModel agree on the read. + const result = resolveEffortFromEnv( + { + GOOSE_THINKING_EFFORT: "minimal", // invalid for Goose + [BUZZ_AGENT_THINKING_EFFORT]: "high", // valid legacy + }, + "GOOSE_THINKING_EFFORT", + BUZZ_AGENT_THINKING_EFFORT, + GOOSE_VALUES, + ); + assert.equal( + result.value, + "high", + "invalid native skipped; valid legacy provides the displayed value", + ); + assert.equal( + result.legacyConsumed, + true, + "legacyConsumed is true when native invalid and legacy valid", + ); +}); + +test("harness_native_invalid_legacy_renders_empty_and_row_stays_visible", () => { + // Invalid legacy value only (e.g. "minimal" — not in Goose vocab): + // resolveEffortFromEnv returns null (control shows Inherit/empty) and + // legacyConsumed is false (row stays visible as an advanced env row). + const result = resolveEffortFromEnv( + { [BUZZ_AGENT_THINKING_EFFORT]: "minimal" }, + "GOOSE_THINKING_EFFORT", + BUZZ_AGENT_THINKING_EFFORT, + GOOSE_VALUES, + ); + assert.equal( + result.value, + null, + "invalid legacy returns null — control shows Inherit", + ); + assert.equal( + result.legacyConsumed, + false, + "legacyConsumed is false — row stays visible as advanced env row", + ); +}); + +test("harness_native_global_scope_no_legacy_fallback", () => { + // At global scope the caller passes null for legacyKey — enforcing the tier boundary. + // Even if a legacy key exists, resolveEffortFromEnv must not consume it. + const result = resolveEffortFromEnv( + { + GOOSE_THINKING_EFFORT: "", + [BUZZ_AGENT_THINKING_EFFORT]: "high", + }, + "GOOSE_THINKING_EFFORT", + null, // global scope: no legacy fallback + GOOSE_VALUES, + ); + assert.equal( + result.value, + null, + "global scope: legacy key ignored when legacyKey=null", + ); + assert.equal( + result.legacyConsumed, + false, + "legacyConsumed is false at global scope", + ); +}); + +// --------------------------------------------------------------------------- +// normalizeEffortValue — Goose canonical alias normalization +// --------------------------------------------------------------------------- +// +// Source: goose `crates/goose-provider-types/src/thinking.rs:277-308` +// Canonical: off|low|medium|high|max +// Aliases: none|disabled→off, med→medium, xhigh→max (case-insensitive) +// (GOOSE_VALUES is defined above in the HarnessNativeEffortFields section.) + +test("normalizeEffortValue_canonical_values_pass_through", () => { + for (const v of GOOSE_VALUES) { + assert.equal(normalizeEffortValue(v, GOOSE_VALUES), v); + } +}); + +test("normalizeEffortValue_none_to_off", () => { + assert.equal(normalizeEffortValue("none", GOOSE_VALUES), "off"); +}); + +test("normalizeEffortValue_disabled_to_off", () => { + assert.equal(normalizeEffortValue("disabled", GOOSE_VALUES), "off"); +}); + +test("normalizeEffortValue_med_to_medium", () => { + assert.equal(normalizeEffortValue("med", GOOSE_VALUES), "medium"); +}); + +test("normalizeEffortValue_xhigh_to_max", () => { + assert.equal(normalizeEffortValue("xhigh", GOOSE_VALUES), "max"); +}); + +test("normalizeEffortValue_case_insensitive", () => { + assert.equal(normalizeEffortValue("HIGH", GOOSE_VALUES), "high"); + assert.equal(normalizeEffortValue("NONE", GOOSE_VALUES), "off"); + assert.equal(normalizeEffortValue("XHIGH", GOOSE_VALUES), "max"); +}); + +test("normalizeEffortValue_invalid_returns_null", () => { + assert.equal(normalizeEffortValue("minimal", GOOSE_VALUES), null); + assert.equal(normalizeEffortValue("unknown", GOOSE_VALUES), null); +}); + +test("normalizeEffortValue_null_acceptedValues_passthrough", () => { + // buzz-agent path: no static vocab, pass through raw value unchanged. + assert.equal(normalizeEffortValue("xhigh", null), "xhigh"); + assert.equal(normalizeEffortValue("minimal", null), "minimal"); +}); diff --git a/desktop/src/features/agents/ui/buzzAgentConfig.ts b/desktop/src/features/agents/ui/buzzAgentConfig.ts index be663c35cb..1eede40b93 100644 --- a/desktop/src/features/agents/ui/buzzAgentConfig.ts +++ b/desktop/src/features/agents/ui/buzzAgentConfig.ts @@ -8,6 +8,48 @@ /** Env var key for the thinking/effort level sent to the LLM. */ export const BUZZ_AGENT_THINKING_EFFORT = "BUZZ_AGENT_THINKING_EFFORT"; +/** + * All native thinking-effort env keys across all known runtimes. + * Used to strip foreign effort keys from a runtime's effective descriptor and + * to clear all effort-related keys when switching runtimes (record/persona scope). + * Must stay in sync with `ALL_KNOWN_EFFORT_KEYS` in `config_bridge/mod.rs`. + */ +export const ALL_KNOWN_EFFORT_KEYS: readonly string[] = [ + BUZZ_AGENT_THINKING_EFFORT, // buzz-agent native (= legacy key) + "GOOSE_THINKING_EFFORT", // Goose native +] as const; + +/** + * Normalize a raw effort value to its canonical form for a runtime with a + * static effort vocabulary (e.g. Goose). Returns `null` when the value is + * invalid for the given canonical set. + * + * Aliases (case-insensitive, from Goose `thinking.rs:277-297`): + * none|disabled → off, med → medium, xhigh → max + * Also case-folds (e.g. "HIGH" → "high"). + * + * Pass `null` for `acceptedValues` to skip normalization (buzz-agent path). + */ +export function normalizeEffortValue( + raw: string, + acceptedValues: readonly string[] | null, +): string | null { + if (!acceptedValues) return raw; // buzz-agent: pass through, no static vocab + const lower = raw.toLowerCase(); + // Canonical direct match after case-fold. + if (acceptedValues.includes(lower)) return lower; + // Known aliases (mirrors GOOSE_EFFORT_NORMALIZATION in runtime_metadata.rs). + const ALIASES: Record = { + none: "off", + disabled: "off", + med: "medium", + xhigh: "max", + }; + const mapped = ALIASES[lower]; + if (mapped && acceptedValues.includes(mapped)) return mapped; + return null; // invalid for this harness +} + /** Env var key for the maximum output token count per turn. */ export const BUZZ_AGENT_MAX_OUTPUT_TOKENS = "BUZZ_AGENT_MAX_OUTPUT_TOKENS"; diff --git a/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx b/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx index 7fa87c4e6b..152d5b9127 100644 --- a/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx +++ b/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx @@ -10,7 +10,10 @@ 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 { + numericTuningPlaceholder, + resolveEffortFromEnv, +} from "../lib/agentConfigCore"; import { AgentDropdownSelect, type AgentDropdownOption, @@ -19,6 +22,7 @@ import { BUZZ_AGENT_THINKING_EFFORT, BUZZ_AGENT_THINKING_EFFORT_VALUES, getProviderEffortConfig, + normalizeEffortValue, } from "./buzzAgentConfig"; /** @@ -35,6 +39,7 @@ import { * env-vars map pattern and the `setConfig` pattern). */ export function EffortSelectField({ + canonicalValues, currentEffort, disabled = false, emptyOptionLabel, @@ -53,6 +58,14 @@ export function EffortSelectField({ testId, useCustomSelect = false, }: { + /** + * When set, renders these values as the full option list (all valid, no + * greying, no default marking). Used by harness-native runtimes whose effort + * vocabulary is fixed and independent of the provider/model catalog. + * When set, `effortValid`, `effortDefault`, and `showUnavailableOptions` are + * ignored. + */ + canonicalValues?: readonly string[]; /** Current effort value from env vars ("" = inherit). */ currentEffort: string; /** Disable the dropdown. */ @@ -105,21 +118,26 @@ export function EffortSelectField({ : effortDefault === null ? "Inherit (default)" : (inheritFallbackLabel ?? "Inherit"); - const effortOptions: AgentDropdownOption[] = [ - { label: emptyOptionLabel ?? inheritLabel, value: "" }, - ...BUZZ_AGENT_THINKING_EFFORT_VALUES.flatMap((v) => { - const isValid = (effortValid as readonly string[]).includes(v); - if (!showUnavailableOptions && !isValid) return []; - const isDefault = v === effortDefault; - return [ - { - disabled: !isValid, - label: isDefault ? `${v} (default)` : v, - value: v, - }, + const effortOptions: AgentDropdownOption[] = canonicalValues + ? [ + { label: emptyOptionLabel ?? inheritLabel, value: "" }, + ...canonicalValues.map((v) => ({ label: v, value: v })), + ] + : [ + { label: emptyOptionLabel ?? inheritLabel, value: "" }, + ...BUZZ_AGENT_THINKING_EFFORT_VALUES.flatMap((v) => { + const isValid = (effortValid as readonly string[]).includes(v); + if (!showUnavailableOptions && !isValid) return []; + const isDefault = v === effortDefault; + return [ + { + disabled: !isValid, + label: isDefault ? `${v} (default)` : v, + value: v, + }, + ]; + }), ]; - }), - ]; return (
@@ -352,3 +370,161 @@ export function BuzzAgentModelTuningFields({
); } + +/** + * Pure mutation helper for harness-native effort controls. + * + * Write semantics (scope-aware): + * - Save (value non-empty): write `nativeKey`; delete `legacyKey` when supplied. + * - Clear (value = ""): delete `nativeKey`; delete `legacyKey` when supplied. + * + * `legacyKey` must be supplied at record/persona scope (definition/instance) to + * atomically delete `BUZZ_AGENT_THINKING_EFFORT` and prevent stale alias + * resurrection. At global/onboarding scope pass `null` — `BUZZ_AGENT_THINKING_EFFORT` + * is buzz-agent's own native key there (Delta 5: never delete a foreign runtime's key). + * + * This function is the single spec for the mutation contract. Both + * `HarnessNativeEffortFields.handleChange` and `AgentConfigFields`' effort + * `onChange` call it; tests import and exercise it directly. + */ +export function applyHarnessNativeEffortChange( + envVars: EnvVarsValue, + nativeKey: string, + /** Pass `BUZZ_AGENT_THINKING_EFFORT` at record/persona scope; `null` at global/onboarding. */ + legacyKey: string | null, + value: string, +): EnvVarsValue { + const next = { ...envVars }; + if (value === "") { + delete next[nativeKey]; + } else { + next[nativeKey] = value; + } + // Delete legacy key at record/persona scope to prevent stale alias resurrection. + // Omitted at global scope — legacy key is buzz-agent's own native key there. + if (legacyKey) delete next[legacyKey]; + return next; +} + +/** + * Effort-tuning knob for runtimes with a static canonical effort vocabulary + * (e.g. Goose: `off|low|medium|high|max`, driven by `runtime.acceptedEffortValues`). + * + * Semantics differ from `BuzzAgentModelTuningFields` in two ways: + * 1. The option list comes from `acceptedEffortValues` rather than the + * provider/model catalog — all listed values are valid, no greying out. + * 2. Saves write to the runtime's native key (`nativeEffortKey`) and + * **atomically deletes** the legacy key (`BUZZ_AGENT_THINKING_EFFORT`) + * to prevent stale alias resurrection. Clears both on empty selection. + * + * The caller controls whether this component renders; it is shown only when + * the selected runtime has `acceptedEffortValues !== null`. + */ +export function HarnessNativeEffortFields({ + acceptedEffortValues, + envVars, + inheritedEnvVars, + legacyEnvKey, + nativeEffortKey, + onEnvVarsChange, +}: { + /** Ordered canonical effort values from `runtime.acceptedEffortValues`. */ + acceptedEffortValues: readonly string[]; + envVars: EnvVarsValue; + /** + * Inherited defaults (global config env_vars) used to show the Inherit + * option label. Normalized to canonical before display. + */ + inheritedEnvVars?: EnvVarsValue; + /** + * Legacy effort key for pre-migration personas (e.g. `BUZZ_AGENT_THINKING_EFFORT`). + * When the native key is absent and the legacy key holds a valid canonical value, + * that value is displayed (the read path mirrors `resolveEffortFromEnv`). + * Pass only at record/persona scope (definition/instance) — omit or pass undefined + * at global/onboarding scope to enforce the tier boundary. + */ + legacyEnvKey?: string; + /** The native effort env key for this runtime (e.g. `GOOSE_THINKING_EFFORT`). */ + nativeEffortKey: string; + /** + * Replaces the whole envVars record atomically (native write + legacy delete). + * Mirrors the pattern used by EditAgentAdvancedFields for env mutations. + */ + onEnvVarsChange: (next: EnvVarsValue) => void; +}) { + // Read current effort via the shared policy source: native-first, valid-legacy + // fallback (when legacyEnvKey is supplied). This guarantees the component and + // deriveAgentConfigFieldModel agree on what value to display at any scope. + const { value: currentEffort } = resolveEffortFromEnv( + envVars, + nativeEffortKey, + legacyEnvKey ?? null, + acceptedEffortValues, + ); + + // Inherited effort from global config; normalize so xhigh shows as max, etc. + // NOTE: Native-key only — no legacy fallback on the inherited map. The merged + // inheritedEnvVars (global + persona env) cannot distinguish persona-sourced + // legacy (should display) from global-sourced legacy (must NOT per Delta 4). + // A naive legacy fallback here would over-report global legacy and violate the + // tier boundary. Native-only is intentional; persona-sourced legacy effort will + // be visible via the current-value read once the user opens a persona with + // legacy data (the control shows the legacy value via resolveEffortFromEnv above). + const rawInherited = inheritedEnvVars?.[nativeEffortKey] ?? ""; + const inheritedEffort = rawInherited + ? (normalizeEffortValue(rawInherited, acceptedEffortValues) ?? undefined) + : undefined; + + const inheritLabel = inheritedEffort + ? `Inherit (${inheritedEffort})` + : "Inherit"; + + const effortOptions: AgentDropdownOption[] = [ + { label: inheritLabel, value: "" }, + ...acceptedEffortValues.map((v) => ({ label: v, value: v })), + ]; + + function handleChange(value: string) { + // Record/persona scope: always pass the legacy key so it is atomically deleted. + onEnvVarsChange( + applyHarnessNativeEffortChange( + envVars, + nativeEffortKey, + legacyEnvKey ?? BUZZ_AGENT_THINKING_EFFORT, + value, + ), + ); + } + + return ( +
+

+ harness model tuning +

+
+
+ + +

+ Controls how much reasoning effort the LLM applies per turn. Leave + blank to inherit from the global or persona default. +

+
+
+
+ ); +} diff --git a/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs b/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs index 32fc7cf582..f5085c0ad5 100644 --- a/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs +++ b/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs @@ -36,11 +36,16 @@ import { countNonSecretInheritedEnvVars, getBakedModelInheritLabel, getAdvancedInheritedSummary, + bakedStructuredKeys, getGlobalModelFallback, getInheritedAgentDefaults, getBakedProviderInheritLabel, resolveInheritedDefault, } from "./bakedEnvHelpers.ts"; +import { + deriveAgentConfigFieldModel, + getRenderableEffortField, +} from "../lib/agentConfigCore.ts"; // ── Core predicate: provider-selection support ───────────────────────────── @@ -1442,3 +1447,334 @@ test("inherited defaults expose a provider-specific model fallback to agent dial value: "goose-claude-opus-4-8", }); }); + +// ── Baked effort: native-only for non-buzz-agent runtimes ───────────────── +// +// Plan v3 Delta 4: baked legacy key (BUZZ_AGENT_THINKING_EFFORT) must NOT be +// shown as Goose inherited effort. Only the native key is honoured at the baked tier. +// Paired with the ★ baked-unmasking pin from pass-3. + +test("baked_legacy_effort_excluded_for_goose_runtime", () => { + // Baked env has BUZZ_AGENT_THINKING_EFFORT=high but NOT GOOSE_THINKING_EFFORT. + // For a Goose runtime, the baked legacy key must not seed inherited effort. + const defaults = getInheritedAgentDefaults( + { env_vars: {}, provider: null, model: null }, + [{ key: "BUZZ_AGENT_THINKING_EFFORT", value: "high", masked: false }], + { nativeEffortKey: "GOOSE_THINKING_EFFORT" }, + ); + assert.deepEqual( + defaults.effort, + { source: null, value: "" }, + "baked legacy must not seed Goose inherited effort", + ); +}); + +test("baked_native_effort_included_for_goose_runtime", () => { + // Baked env has GOOSE_THINKING_EFFORT=medium. For a Goose runtime, the baked + // native key must seed inherited effort (★ baked unmasking pin: value usable, + // not ••••••). + const defaults = getInheritedAgentDefaults( + { env_vars: {}, provider: null, model: null }, + [{ key: "GOOSE_THINKING_EFFORT", value: "medium", masked: false }], + { nativeEffortKey: "GOOSE_THINKING_EFFORT" }, + ); + assert.deepEqual( + defaults.effort, + { source: "build", value: "medium" }, + "baked native must seed Goose inherited effort", + ); +}); + +test("baked_masked_native_effort_excluded_from_inherited_defaults", () => { + // A masked baked value (•••) must not be treated as a real inherited effort. + // (★ baked unmasking pin: masked ••••••must never read as a real inherited value.) + const defaults = getInheritedAgentDefaults( + { env_vars: {}, provider: null, model: null }, + [{ key: "GOOSE_THINKING_EFFORT", value: "••••••", masked: true }], + { nativeEffortKey: "GOOSE_THINKING_EFFORT" }, + ); + assert.deepEqual( + defaults.effort, + { source: null, value: "" }, + "masked baked value must not seed inherited effort", + ); +}); + +// ── bakedStructuredKeys: runtime-sensitive hidden-key set ───────────────── + +test("bakedStructuredKeys_with_native_effort_key_hides_both_fixed_and_native", () => { + const keys = bakedStructuredKeys("GOOSE_THINKING_EFFORT"); + assert.ok( + keys.has("BUZZ_AGENT_PROVIDER"), + "BUZZ_AGENT_PROVIDER always hidden", + ); + assert.ok(keys.has("BUZZ_AGENT_MODEL"), "BUZZ_AGENT_MODEL always hidden"); + assert.ok(keys.has("GOOSE_THINKING_EFFORT"), "native effort key hidden"); + assert.equal( + keys.has("BUZZ_AGENT_THINKING_EFFORT"), + false, + "legacy key NOT hidden for Goose (global legacy stays as advanced row)", + ); +}); + +test("bakedStructuredKeys_without_native_effort_key_hides_only_fixed", () => { + const keys = bakedStructuredKeys(undefined); + assert.ok(keys.has("BUZZ_AGENT_PROVIDER")); + assert.ok(keys.has("BUZZ_AGENT_MODEL")); + assert.equal(keys.has("GOOSE_THINKING_EFFORT"), false); + assert.equal(keys.has("BUZZ_AGENT_THINKING_EFFORT"), false); +}); + +// ── getInheritedAgentDefaults: global-tier legacy exclusion + normalization ─ +// +// Plan v3 Delta 4: global legacy is never consumed as structured effort. +// Plan v3 Delta 1: global native value normalized before display. + +test("global_legacy_effort_ignored_for_goose_in_inherited_defaults", () => { + // Global config has only the legacy key (BUZZ_AGENT_THINKING_EFFORT=high). + // For a Goose runtime, that key is buzz-agent's native key (Delta 5) and must + // NOT seed the inherited effort display — mirrors effort_tier_alias(global_tier=true). + const defaults = getInheritedAgentDefaults( + { + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + provider: null, + model: null, + }, + undefined, + { + nativeEffortKey: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }, + ); + assert.deepEqual( + defaults.effort, + { source: null, value: "" }, + "global legacy key must not seed Goose inherited effort", + ); +}); + +test("global_native_xhigh_normalizes_to_max_for_goose_in_inherited_defaults", () => { + // Global config has GOOSE_THINKING_EFFORT=xhigh (an accepted Goose alias). + // Must be normalized to canonical "max" before display, consistent with the + // Rust display path where xhigh → max via acceptedEffortValues (Delta 1). + const defaults = getInheritedAgentDefaults( + { + env_vars: { GOOSE_THINKING_EFFORT: "xhigh" }, + provider: null, + model: null, + }, + undefined, + { + nativeEffortKey: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + }, + ); + assert.deepEqual( + defaults.effort, + { source: "global", value: "max" }, + "global native xhigh must normalize to canonical max", + ); +}); + +// ── Global Goose effort: descriptor-driven canonical choices ──────────────── +// +// Plan v3 IMPORTANT 1: AgentConfigFields at global/onboarding scope for a +// harness-native runtime must use descriptor metadata for effort options and +// display, not the Buzz provider/model catalog (Delta 5 coexistence). + +const GOOSE_GLOBAL_RUNTIME = { + id: "goose", + thinkingEnvVar: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + providerEnvVar: "GOOSE_PROVIDER", + modelEnvVar: "GOOSE_MODEL", + maxTokensEnvVar: null, + contextLimitEnvVar: null, + maxRoundsEnvVar: null, + command: "goose", + defaultArgs: [], +}; + +test("global_goose_effort_field_optionSource_is_harnessNative", () => { + // At global scope with a Goose runtime, the effort field descriptor must report + // optionSource=harnessNative — the signal AgentConfigFields uses to branch to the + // metadata-driven canonical control instead of the Buzz provider/model catalog. + const model = deriveAgentConfigFieldModel({ + config: { env_vars: {}, provider: null, model: null }, + runtime: GOOSE_GLOBAL_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok( + effortField, + "effort field must be present for Goose at global scope", + ); + assert.equal( + effortField.optionSource, + "harnessNative", + "Goose global effort must be harnessNative (not legacyProviderModelCatalog or buzzAgentCatalog)", + ); +}); + +test("global_goose_effort_field_value_xhigh_normalized_to_max", () => { + // When global config has GOOSE_THINKING_EFFORT=xhigh, the descriptor value must + // be normalized to canonical "max" — the global control shows effortField.value. + const model = deriveAgentConfigFieldModel({ + config: { + env_vars: { GOOSE_THINKING_EFFORT: "xhigh" }, + provider: null, + model: null, + }, + runtime: GOOSE_GLOBAL_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok(effortField, "effort field must be present"); + assert.equal( + effortField.value, + "max", + "xhigh must normalize to canonical max via descriptor", + ); +}); + +test("global_goose_effort_field_invalid_value_resolves_to_null", () => { + // A global GOOSE_THINKING_EFFORT=minimal (invalid for Goose) must resolve to + // null via the descriptor — the control shows empty/Inherit, not the invalid value. + const model = deriveAgentConfigFieldModel({ + config: { + env_vars: { GOOSE_THINKING_EFFORT: "minimal" }, + provider: null, + model: null, + }, + runtime: GOOSE_GLOBAL_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok(effortField, "effort field must be present"); + assert.equal( + effortField.value, + null, + "invalid value must resolve to null (control shows Inherit)", + ); +}); + +test("global_goose_effort_field_no_legacy_fallback_at_global_scope", () => { + // At global scope, only the native GOOSE_THINKING_EFFORT is considered — not + // BUZZ_AGENT_THINKING_EFFORT (Delta 5: that is buzz-agent's own native key). + // Even if a global legacy key holds a valid value, the descriptor must report null. + const model = deriveAgentConfigFieldModel({ + config: { + env_vars: { + BUZZ_AGENT_THINKING_EFFORT: "high", // buzz-agent's own global native key + }, + provider: null, + model: null, + }, + runtime: GOOSE_GLOBAL_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok(effortField, "effort field must be present"); + assert.equal( + effortField.value, + null, + "global legacy BUZZ_AGENT_THINKING_EFFORT must not be consumed as Goose effort", + ); + assert.equal( + effortField.legacyConsumedKey, + undefined, + "no legacy key must be consumed at global scope", + ); +}); + +// ── Model-coupled mutation guard: harness-native effort survives heal/auto-clear +// +// Pins the isHarnessNativeEffort flag that gates heal-on-mount, orphan-clear, and +// useEffortAutoClear. All three paths skip the native effort key when this is true. +// Descriptor optionSource=harnessNative is the derivation; acceptedEffortValues≠null +// is the runtime signal. Tests verify the flag is set for Goose (any value, including +// aliases and invalid values) and NOT set for buzz-agent (provider/model-gated path). + +test("harness_native_xhigh_flag_set_so_model_heal_skips_key", () => { + // Global GOOSE_THINKING_EFFORT=xhigh: descriptor has optionSource=harnessNative and + // value=max. AgentConfigFields derives isHarnessNativeEffort from optionSource, which + // gates the heal-on-mount, orphan-clear, and useEffortAutoClear mutation paths. + // When true, all three skip the native key — xhigh survives and displays as max. + const model = deriveAgentConfigFieldModel({ + config: { + env_vars: { GOOSE_THINKING_EFFORT: "xhigh" }, + provider: null, + model: null, + }, + runtime: GOOSE_GLOBAL_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok(effortField, "effort field present"); + assert.equal( + effortField.optionSource, + "harnessNative", + "xhigh: optionSource=harnessNative gates model-heal skip (isHarnessNativeEffort=true)", + ); + assert.equal(effortField.value, "max", "xhigh normalizes to max for display"); +}); + +test("harness_native_invalid_minimal_flag_set_so_model_heal_skips_key", () => { + // Global GOOSE_THINKING_EFFORT=minimal (invalid for Goose): flag still set, + // heal/auto-clear skip the key. Reader shows Inherit (null), spawn skips it. + const model = deriveAgentConfigFieldModel({ + config: { + env_vars: { GOOSE_THINKING_EFFORT: "minimal" }, + provider: null, + model: null, + }, + runtime: GOOSE_GLOBAL_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok(effortField, "effort field present"); + assert.equal( + effortField.optionSource, + "harnessNative", + "minimal: optionSource=harnessNative gates model-heal skip", + ); + assert.equal( + effortField.value, + null, + "minimal is invalid — control shows Inherit, but key survives", + ); +}); + +test("buzz_agent_effort_optionSource_is_not_harnessNative_auto_clear_active", () => { + // buzz-agent uses the provider/model catalog path (optionSource=buzzAgentCatalog). + // isHarnessNativeEffort=false, so model-heal, orphan-clear, and useEffortAutoClear + // are all active for buzz-agent — behavior unchanged from main. + const BUZZ_AGENT_RUNTIME = { + id: "buzz-agent", + thinkingEnvVar: "BUZZ_AGENT_THINKING_EFFORT", + acceptedEffortValues: null, // buzz-agent: no static vocab, provider/model-gated + providerEnvVar: "BUZZ_AGENT_PROVIDER", + modelEnvVar: "BUZZ_AGENT_MODEL", + maxTokensEnvVar: null, + contextLimitEnvVar: null, + maxRoundsEnvVar: null, + command: null, + defaultArgs: [], + }; + const model = deriveAgentConfigFieldModel({ + config: { + env_vars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + provider: "anthropic", + model: "claude-3-5-sonnet", + }, + runtime: BUZZ_AGENT_RUNTIME, + scope: "global", + }); + const effortField = getRenderableEffortField(model); + assert.ok(effortField, "effort field present for buzz-agent"); + assert.equal( + effortField.optionSource, + "buzzAgentCatalog", + "buzz-agent effort is not harnessNative — auto-clear path active (behavior unchanged)", + ); +}); diff --git a/desktop/src/features/agents/ui/personaRuntimeModel.test.mjs b/desktop/src/features/agents/ui/personaRuntimeModel.test.mjs index 967bd3e129..1737401aa4 100644 --- a/desktop/src/features/agents/ui/personaRuntimeModel.test.mjs +++ b/desktop/src/features/agents/ui/personaRuntimeModel.test.mjs @@ -264,3 +264,144 @@ test("resolveInheritedRuntimeSubmission agent-local env overrides persona creden "agent-local credential must override persona credential on inherit-transition", ); }); + +// ── Linked Goose instance: effort persistence and inheritance ───────────────── +// +// When a linked Goose instance selects Inherit for effort, the record must NOT +// acquire the persona's effort key — inheritance happens at spawn time. The +// `excludePersonaEnvKeys` parameter strips harness-native effort keys from the +// persona layer on the inherit-transition so the record stays override-free. + +const GOOSE_EFFORT_KEY = "GOOSE_THINKING_EFFORT"; +const BUZZ_EFFORT_KEY = "BUZZ_AGENT_THINKING_EFFORT"; +// Keys stripped from persona layer for a Goose linked instance. +const GOOSE_EXCLUDE_EFFORT_KEYS = [GOOSE_EFFORT_KEY, BUZZ_EFFORT_KEY]; + +test("linked_goose_instance_inherit_does_not_materialize_persona_native_effort", () => { + // Persona has GOOSE_THINKING_EFFORT=high. User selects Inherit → local envVars + // has no effort key. Without excludePersonaEnvKeys the inherited-transition + // would persist GOOSE_THINKING_EFFORT=high into the record. + const result = resolveInheritedRuntimeSubmission({ + inheritHarness: true, + agentWasHarnessPinned: true, + provider: "", + personaProvider: "", + model: "", + personaModel: null, + envVars: {}, // user selected Inherit — no local effort override + personaEnvVars: { [GOOSE_EFFORT_KEY]: "high" }, + excludePersonaEnvKeys: GOOSE_EXCLUDE_EFFORT_KEYS, + }); + assert.equal( + Object.hasOwn(result.envVars, GOOSE_EFFORT_KEY), + false, + "persona native effort key must NOT materialize into record on Inherit", + ); +}); + +test("linked_goose_instance_inherit_does_not_materialize_persona_legacy_effort", () => { + // Same as above but persona has the legacy BUZZ_AGENT_THINKING_EFFORT key + // (pre-migration persona). Must also be excluded. + const result = resolveInheritedRuntimeSubmission({ + inheritHarness: true, + agentWasHarnessPinned: true, + provider: "", + personaProvider: "", + model: "", + personaModel: null, + envVars: {}, + personaEnvVars: { [BUZZ_EFFORT_KEY]: "high" }, + excludePersonaEnvKeys: GOOSE_EXCLUDE_EFFORT_KEYS, + }); + assert.equal( + Object.hasOwn(result.envVars, BUZZ_EFFORT_KEY), + false, + "persona legacy effort key must NOT materialize into record on Inherit", + ); +}); + +test("linked_goose_instance_concrete_effort_override_wins_over_persona", () => { + // When the user has a local effort override, it must win over persona — the + // exclude list should not strip it (local wins naturally via envVars layering). + const result = resolveInheritedRuntimeSubmission({ + inheritHarness: true, + agentWasHarnessPinned: true, + provider: "", + personaProvider: "", + model: "", + personaModel: null, + envVars: { [GOOSE_EFFORT_KEY]: "medium" }, // user selected a concrete value + personaEnvVars: { [GOOSE_EFFORT_KEY]: "high" }, + excludePersonaEnvKeys: GOOSE_EXCLUDE_EFFORT_KEYS, + }); + assert.equal( + result.envVars[GOOSE_EFFORT_KEY], + "medium", + "local effort override wins over persona", + ); +}); + +test("linked_goose_instance_non_effort_persona_env_still_merged", () => { + // excludePersonaEnvKeys must not affect non-effort keys — credentials and + // other persona env vars must still be included in the inherit-transition. + const result = resolveInheritedRuntimeSubmission({ + inheritHarness: true, + agentWasHarnessPinned: true, + provider: "", + personaProvider: "", + model: "", + personaModel: null, + envVars: {}, + personaEnvVars: { + [GOOSE_EFFORT_KEY]: "high", + SOME_OTHER_KEY: "persona_value", + }, + excludePersonaEnvKeys: GOOSE_EXCLUDE_EFFORT_KEYS, + }); + assert.equal( + Object.hasOwn(result.envVars, GOOSE_EFFORT_KEY), + false, + "effort key excluded from persona", + ); + assert.equal( + result.envVars.SOME_OTHER_KEY, + "persona_value", + "non-effort persona env vars still merged", + ); +}); + +test("linked_goose_instance_submit_payload_contains_no_inherited_effort_on_inherit", () => { + // Full scenario: harness-pinned Goose instance. Persona has GOOSE_THINKING_EFFORT=max. + // On inherit-transition (user selects Inherit, clears local effort), the submit + // payload must have neither native nor legacy effort key. + const result = resolveInheritedRuntimeSubmission({ + inheritHarness: true, + agentWasHarnessPinned: true, + provider: "", + personaProvider: "anthropic", + model: "", + personaModel: null, + envVars: { ANTHROPIC_API_KEY: "sk-agent" }, // credential is local, no effort + personaEnvVars: { + ANTHROPIC_API_KEY: "sk-persona", + [GOOSE_EFFORT_KEY]: "max", + }, + excludePersonaEnvKeys: GOOSE_EXCLUDE_EFFORT_KEYS, + }); + assert.equal( + Object.hasOwn(result.envVars, GOOSE_EFFORT_KEY), + false, + "no native effort key in submit payload on Inherit", + ); + assert.equal( + Object.hasOwn(result.envVars, BUZZ_EFFORT_KEY), + false, + "no legacy effort key in submit payload on Inherit", + ); + // Credential still present (agent's own layer wins over persona). + assert.equal( + result.envVars.ANTHROPIC_API_KEY, + "sk-agent", + "agent-local credential preserved", + ); +}); diff --git a/desktop/src/features/agents/ui/personaRuntimeModel.ts b/desktop/src/features/agents/ui/personaRuntimeModel.ts index 20d789c4eb..772c2aedba 100644 --- a/desktop/src/features/agents/ui/personaRuntimeModel.ts +++ b/desktop/src/features/agents/ui/personaRuntimeModel.ts @@ -175,6 +175,18 @@ export function resolveInheritedRuntimeSubmission(input: { envVars: Record; /** The persona's env vars, layered under the agent's own on transition. */ personaEnvVars: Record; + /** + * Env-var keys to strip from the persona layer on the inherit-transition. + * + * Used to prevent harness-native effort keys from materializing into the + * record when the user selects Inherit for effort. Effort on a linked + * instance is owned by the local record layer; the persona's effort value + * is inherited at spawn time without being persisted into the record. + * + * Pass the runtime's native effort key + `BUZZ_AGENT_THINKING_EFFORT` + * when the selected runtime has `acceptedEffortValues`. + */ + excludePersonaEnvKeys?: readonly string[]; }): { provider: string | null; model: string | null; @@ -192,12 +204,19 @@ export function resolveInheritedRuntimeSubmission(input: { input.agentWasHarnessPinned && localProvider.length === 0 ) { + const personaEnvVars = input.excludePersonaEnvKeys?.length + ? Object.fromEntries( + Object.entries(input.personaEnvVars).filter( + ([k]) => !input.excludePersonaEnvKeys?.includes(k), + ), + ) + : input.personaEnvVars; return { provider: input.personaProvider.trim() || null, // Fill an empty local model from the persona so a provider-backed runtime // isn't saved model-less; a deliberate local model still wins. model: localModel || input.personaModel?.trim() || null, - envVars: { ...input.personaEnvVars, ...input.envVars }, + envVars: { ...personaEnvVars, ...input.envVars }, }; } return { diff --git a/desktop/src/features/agents/ui/runtimeModelProviderSelection.test.mjs b/desktop/src/features/agents/ui/runtimeModelProviderSelection.test.mjs index 470b2312bc..77d32cc78d 100644 --- a/desktop/src/features/agents/ui/runtimeModelProviderSelection.test.mjs +++ b/desktop/src/features/agents/ui/runtimeModelProviderSelection.test.mjs @@ -228,3 +228,102 @@ test("auto-model selection clears the model; concrete selection sets it", () => ); assert.equal(concrete.model, "gpt-5"); }); + +// ── selectionOnRuntimeChange: effort-key cleanup ────────────────────────── +// +// At record/persona scope, a runtime change must clean up the previous +// runtime's native effort key AND the legacy key (BUZZ_AGENT_THINKING_EFFORT). +// The next runtime's native key is also cleared (stale leftover removal). +// Unrelated env keys are preserved. + +test("selectionOnRuntimeChange_goose_to_buzz_clears_goose_native_and_legacy", () => { + // Goose→buzz-agent: GOOSE_THINKING_EFFORT (prev native) + BUZZ_AGENT_THINKING_EFFORT (legacy) + // are deleted. Unrelated keys survive. + const current = { + ...base, + envVars: { + GOOSE_THINKING_EFFORT: "medium", + BUZZ_AGENT_THINKING_EFFORT: "high", + SOME_KEY: "value", + }, + }; + const next = selectionOnRuntimeChange(current, { + previousRuntime: "goose", + nextRuntime: "buzz-agent", + nextRuntimeCanChooseProvider: false, + lockedRuntimeReset: "full", + previousRuntimeNativeEffortKey: "GOOSE_THINKING_EFFORT", + nextRuntimeNativeEffortKey: "BUZZ_AGENT_THINKING_EFFORT", + }); + assert.equal( + next.envVars.GOOSE_THINKING_EFFORT, + undefined, + "prev native effort key must be cleared", + ); + assert.equal( + next.envVars.BUZZ_AGENT_THINKING_EFFORT, + undefined, + "legacy effort key must be cleared", + ); + assert.equal( + next.envVars.SOME_KEY, + "value", + "unrelated env key must survive", + ); +}); + +test("selectionOnRuntimeChange_buzz_to_goose_clears_goose_native_and_legacy", () => { + // buzz-agent→Goose: previousRuntimeNativeEffortKey is BUZZ_AGENT_THINKING_EFFORT + // which equals the legacy key — cleanup fires via the nextRuntimeNativeEffortKey path. + const current = { + ...base, + envVars: { + GOOSE_THINKING_EFFORT: "low", + BUZZ_AGENT_THINKING_EFFORT: "high", + OTHER: "keep", + }, + }; + const next = selectionOnRuntimeChange(current, { + previousRuntime: "buzz-agent", + nextRuntime: "goose", + nextRuntimeCanChooseProvider: true, + lockedRuntimeReset: "full", + previousRuntimeNativeEffortKey: "BUZZ_AGENT_THINKING_EFFORT", + nextRuntimeNativeEffortKey: "GOOSE_THINKING_EFFORT", + }); + // When prev native === legacy key, the else-if branch fires instead: + // it clears next native + legacy. + assert.equal( + next.envVars.GOOSE_THINKING_EFFORT, + undefined, + "next native effort key must be cleared", + ); + assert.equal( + next.envVars.BUZZ_AGENT_THINKING_EFFORT, + undefined, + "legacy effort key must be cleared", + ); + assert.equal(next.envVars.OTHER, "keep", "unrelated env key must survive"); +}); + +test("selectionOnRuntimeChange_buzz_to_buzz_preserves_effort_keys", () => { + // buzz-agent→buzz-agent: no effort cleanup (native === legacy key, + // both branches skip). Effort keys survive. + const current = { + ...base, + envVars: { BUZZ_AGENT_THINKING_EFFORT: "high" }, + }; + const next = selectionOnRuntimeChange(current, { + previousRuntime: "buzz-agent", + nextRuntime: "buzz-agent", + nextRuntimeCanChooseProvider: false, + lockedRuntimeReset: "full", + previousRuntimeNativeEffortKey: "BUZZ_AGENT_THINKING_EFFORT", + nextRuntimeNativeEffortKey: "BUZZ_AGENT_THINKING_EFFORT", + }); + assert.equal( + next.envVars.BUZZ_AGENT_THINKING_EFFORT, + "high", + "buzz-agent effort key preserved on same-runtime switch", + ); +}); diff --git a/desktop/src/features/agents/ui/runtimeModelProviderSelection.ts b/desktop/src/features/agents/ui/runtimeModelProviderSelection.ts index e98dc540df..27ab8d0db8 100644 --- a/desktop/src/features/agents/ui/runtimeModelProviderSelection.ts +++ b/desktop/src/features/agents/ui/runtimeModelProviderSelection.ts @@ -12,6 +12,7 @@ import { envVarsClearingManagedApiKey, envVarsWithoutKey, } from "./providerEnvVarUpdates"; +import { BUZZ_AGENT_THINKING_EFFORT } from "./buzzAgentConfig"; /** * Pure transition functions for the runtime -> LLM provider -> model dropdown @@ -41,6 +42,19 @@ export function selectionOnRuntimeChange( * only the provider selection ("provider-only"). */ lockedRuntimeReset: "full" | "provider-only"; + /** + * Native thinking-effort key for the PREVIOUS runtime, or `null` when the + * previous runtime has no effort key or it equals the legacy key. + * When provided, this key is deleted from envVars on runtime change. + */ + previousRuntimeNativeEffortKey?: string | null; + /** + * Native thinking-effort key for the NEXT runtime, or `null` when the + * next runtime has no effort key or it equals the legacy key. + * When provided, this key is deleted from envVars on runtime change + * (any stale leftover from a prior session is removed). + */ + nextRuntimeNativeEffortKey?: string | null; }, ): RuntimeModelProviderSelection { const next = { ...current }; @@ -73,6 +87,38 @@ export function selectionOnRuntimeChange( next.provider = ""; } + // Effort cleanup at record/persona scope: clear prev native, next native, + // and legacy keys when the runtime changes. Global scope is exempt (Delta-5 + // rule: global switches preserve both runtimes' native keys — that is handled + // by resetConfigForHarnessChange's no-op policy). + // Skip when native key equals the legacy key (buzz-agent: they are the same + // key so no aliasing applies; no destructive cleanup needed). + // + // Asymmetric case — prev==legacy + next==null (e.g. buzz→claude): neither + // branch fires, so BUZZ_AGENT_THINKING_EFFORT is preserved through the + // transition. This is intentional: claude has no effort env key so there is + // no stale-alias resurrection risk, and the value is safe to keep for a + // future buzz←claude switch. buzz→goose (next != legacy) DOES clear both + // keys because a stale BUZZ_AGENT_THINKING_EFFORT would be aliased into + // Goose effort on the next spawn. + if ( + params.previousRuntimeNativeEffortKey && + params.previousRuntimeNativeEffortKey !== BUZZ_AGENT_THINKING_EFFORT + ) { + const ev = { ...next.envVars }; + delete ev[params.previousRuntimeNativeEffortKey]; + delete ev[BUZZ_AGENT_THINKING_EFFORT]; + next.envVars = ev; + } else if ( + params.nextRuntimeNativeEffortKey && + params.nextRuntimeNativeEffortKey !== BUZZ_AGENT_THINKING_EFFORT + ) { + const ev = { ...next.envVars }; + delete ev[params.nextRuntimeNativeEffortKey]; + delete ev[BUZZ_AGENT_THINKING_EFFORT]; + next.envVars = ev; + } + return next; } diff --git a/desktop/src/features/agents/ui/useAgentDialogDefaults.ts b/desktop/src/features/agents/ui/useAgentDialogDefaults.ts index 5ede9558fe..59a6083677 100644 --- a/desktop/src/features/agents/ui/useAgentDialogDefaults.ts +++ b/desktop/src/features/agents/ui/useAgentDialogDefaults.ts @@ -2,28 +2,54 @@ import * as React from "react"; import { useBakedBuildEnvQuery } from "../hooks"; import { useGlobalAgentConfig } from "../useGlobalAgentConfig"; -import { BUZZ_AGENT_THINKING_EFFORT } from "./buzzAgentConfig"; import { getInheritedAgentDefaults } from "./bakedEnvHelpers"; export function useAgentDialogDefaults({ inheritedEnvVars = {}, open, + nativeEffortKey, + acceptedEffortValues, }: { inheritedEnvVars?: Record; open: boolean; + /** + * The native effort key for the current runtime + * (e.g. `runtime.thinkingEnvVar`). When absent, falls back to the + * buzz-agent legacy key — preserving existing behavior for callers that + * haven't yet passed runtime metadata. + */ + nativeEffortKey?: string | null; + /** + * Canonical effort values for normalization (from `runtime.acceptedEffortValues`). + * Passed through to `getInheritedAgentDefaults` so global native effort is + * normalized (e.g. xhigh→max) before display. Null for buzz-agent. + */ + acceptedEffortValues?: readonly string[] | null; }) { const { globalConfig } = useGlobalAgentConfig(); const { data: bakedEnv } = useBakedBuildEnvQuery({ enabled: open }); - const inheritedDefaults = getInheritedAgentDefaults(globalConfig, bakedEnv); + const inheritedDefaults = getInheritedAgentDefaults( + globalConfig, + bakedEnv, + nativeEffortKey + ? { nativeEffortKey, acceptedEffortValues: acceptedEffortValues ?? null } + : undefined, + ); + const resolvedEffortKey = nativeEffortKey ?? "BUZZ_AGENT_THINKING_EFFORT"; const effectiveInheritedEnvVars = React.useMemo( () => ({ ...globalConfig.env_vars, ...inheritedEnvVars, ...(inheritedDefaults.effort.value - ? { [BUZZ_AGENT_THINKING_EFFORT]: inheritedDefaults.effort.value } + ? { [resolvedEffortKey]: inheritedDefaults.effort.value } : {}), }), - [globalConfig.env_vars, inheritedDefaults.effort.value, inheritedEnvVars], + [ + globalConfig.env_vars, + inheritedDefaults.effort.value, + inheritedEnvVars, + resolvedEffortKey, + ], ); return { globalConfig, diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index c44fd3b1c0..daa3a3b0d1 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -62,8 +62,7 @@ type RawFeedItem = { created_at: number; channel_id: string | null; channel_name: string; - // Native FeedItemInfo.channel_type is Option: serde emits `null`, - // never omits the key. + // Native FeedItemInfo.channel_type is Option: serde emits `null`, never omits the key. channel_type: string | null; tags: string[][]; category: "mention" | "needs_action" | "activity" | "agent_activity"; @@ -161,7 +160,7 @@ export type RawManagedAgent = { auto_restart_on_config_change?: boolean; backend: ManagedAgentBackend; backend_agent_id: string | null; - // Pre-feature fixtures may omit these; mapped to "owner-only"/[] in fromRawManagedAgent. + // Optional in pre-feature mock fixtures; mapped to "owner-only" / [] in `fromRawManagedAgent`. respond_to?: ManagedAgent["respondTo"]; respond_to_allowlist?: string[]; }; @@ -190,6 +189,7 @@ export type RawAcpRuntimeCatalogEntry = { model_env_var?: string | null; provider_env_var?: string | null; thinking_env_var?: string | null; + accepted_effort_values?: string[] | null; max_tokens_env_var?: string | null; context_limit_env_var?: string | null; max_rounds_env_var?: string | null; @@ -749,6 +749,7 @@ export function fromRawAcpRuntimeCatalogEntry( modelEnvVar: entry.model_env_var ?? null, providerEnvVar: entry.provider_env_var ?? null, thinkingEnvVar: entry.thinking_env_var ?? null, + acceptedEffortValues: entry.accepted_effort_values ?? null, maxTokensEnvVar: entry.max_tokens_env_var ?? null, contextLimitEnvVar: entry.context_limit_env_var ?? null, maxRoundsEnvVar: entry.max_rounds_env_var ?? null, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 78f5d1aa3f..6bfe234039 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -310,11 +310,8 @@ export type ManagedAgent = { pubkey: string; name: string; personaId: string | null; - /** - * The record's harness/runtime id (e.g. "goose", "my-custom-harness"). - * `null` means the agent inherits its harness from the linked persona. - * Used to count agents referencing a harness definition (delete confirm). - */ + /** The record's harness/runtime id (`null` = inherits from persona). + * Used to count agents referencing a harness definition (delete confirm). */ runtime: string | null; teamId?: string | null; relayUrl: string; @@ -517,6 +514,9 @@ export type AcpRuntimeCatalogEntry = { providerEnvVar: string | null; /** Environment variable used to apply thinking effort, when supported. */ thinkingEnvVar: string | null; + /** Canonical effort values for runtimes with a static vocabulary (Goose: off|low|medium|high|max); + * `null` for buzz-agent (per-model catalog). Single authority for UI, spawn bridge, and reader. */ + acceptedEffortValues: string[] | null; maxTokensEnvVar: string | null; contextLimitEnvVar: string | null; maxRoundsEnvVar: string | null; diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index e430549d43..7f794d3644 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -7346,6 +7346,12 @@ function withMockRuntimeConfigMetadata( : runtime.id === "buzz-agent" ? "BUZZ_AGENT_MAX_ROUNDS" : null, + accepted_effort_values: + "accepted_effort_values" in runtime + ? runtime.accepted_effort_values + : runtime.id === "goose" + ? ["off", "low", "medium", "high", "max"] + : null, }; }