diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 12e6983eea..24ab2c2f73 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -216,18 +216,21 @@ pub struct BakedEnvEntry { /// /// Allowlist (case-insensitive): /// - `BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL` — agent runtime selection -/// - `BUZZ_AGENT_THINKING_EFFORT` — non-secret enum (none/minimal/low/medium/high/xhigh/max) +/// - All known native thinking-effort keys (non-secret enum values) — derived +/// from runtime declarations via `all_known_effort_keys()` so this list stays +/// in sync automatically as runtimes are added. /// - `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", - "BUZZ_AGENT_THINKING_EFFORT", "DATABRICKS_HOST", "DATABRICKS_MODEL", ]; let upper = key.to_ascii_uppercase(); SAFE_KEYS.iter().any(|safe| upper == *safe) + || all_known_effort_keys().any(|effort| upper == effort.to_ascii_uppercase()) } /// Expose the baked build env to the frontend with values shown, but any diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 5519153578..fce2a580de 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -29,39 +29,7 @@ fn with_no_goose_config(body: impl FnOnce() -> T) -> T { } fn goose_runtime() -> &'static KnownAcpRuntime { - &KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - max_rounds_env_var: None, - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - } + crate::managed_agents::known_acp_runtime_exact("goose").expect("goose must be in catalog") } fn agent_record() -> ManagedAgentRecord { @@ -175,13 +143,15 @@ fn linked_stale_record_model_never_outranks_persona_model() { record.model = Some("stale-explicit-model".to_string()); let personas = vec![persona_with_model("persona-model")]; - let surface = resolve_config_surface( - record, - &personas, - Some(goose_runtime()), - None, - &Default::default(), - ); + let surface = with_no_goose_config(|| { + resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + None, + &Default::default(), + ) + }); let model = surface.normalized.model.as_ref().expect("model resolved"); assert_eq!(model.value.as_deref(), Some("persona-model")); @@ -205,7 +175,9 @@ fn linked_blank_definition_model_falls_through_to_global_default() { ..Default::default() }; - let surface = resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global); + let surface = with_no_goose_config(|| { + resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global) + }); let model = surface.normalized.model.as_ref().expect("model resolved"); assert_eq!(model.value.as_deref(), Some("global-model")); @@ -222,13 +194,15 @@ fn definition_less_explicit_record_model_keeps_buzz_explicit_origin() { record.model = Some("explicit-model".to_string()); let personas = vec![persona_with_model("persona-model")]; - let surface = resolve_config_surface( - record, - &personas, - Some(goose_runtime()), - None, - &Default::default(), - ); + let surface = with_no_goose_config(|| { + resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + None, + &Default::default(), + ) + }); let model = surface.normalized.model.as_ref().expect("model resolved"); assert_eq!(model.value.as_deref(), Some("explicit-model")); @@ -249,13 +223,15 @@ fn pending_pick_keeps_explicit_x_and_does_not_surface_live_y() { let personas: Vec = vec![]; let cache = session_cache("model-y", false); - let surface = resolve_config_surface( - record, - &personas, - Some(goose_runtime()), - Some(&cache), - &Default::default(), - ); + let surface = with_no_goose_config(|| { + resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + Some(&cache), + &Default::default(), + ) + }); let model = surface.normalized.model.expect("model resolved"); assert_eq!(model.value.as_deref(), Some("model-x")); @@ -277,13 +253,15 @@ fn genuine_explicit_live_switch_renders_y_over_x_buzz_explicit_secondary() { let personas: Vec = vec![]; let cache = session_cache("model-y", true); - let surface = resolve_config_surface( - record, - &personas, - Some(goose_runtime()), - Some(&cache), - &Default::default(), - ); + let surface = with_no_goose_config(|| { + resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + Some(&cache), + &Default::default(), + ) + }); let model = surface.normalized.model.expect("model resolved"); assert_eq!(model.value.as_deref(), Some("model-y")); @@ -340,13 +318,15 @@ fn persona_linked_live_switch_keeps_persona_default_secondary() { let personas = vec![persona_with_model("persona-model")]; let cache = session_cache("model-y", true); - let surface = resolve_config_surface( - record, - &personas, - Some(goose_runtime()), - Some(&cache), - &Default::default(), - ); + let surface = with_no_goose_config(|| { + resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + Some(&cache), + &Default::default(), + ) + }); let model = surface.normalized.model.expect("model resolved"); assert_eq!(model.value.as_deref(), Some("model-y")); @@ -375,13 +355,15 @@ fn global_default_live_switch_renders_global_model_as_secondary_global_default() ..Default::default() }; - let surface = resolve_config_surface( - record, - &personas, - Some(goose_runtime()), - Some(&cache), - &global, - ); + let surface = with_no_goose_config(|| { + resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + Some(&cache), + &global, + ) + }); let model = surface.normalized.model.expect("model resolved"); // Live model wins as primary. @@ -623,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. @@ -632,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 runtime declarations via 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/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 0eb024a86a..a40332048e 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -45,8 +45,7 @@ pub(crate) fn plan_adapter_install<'c>( { None } - // Codex adapter is outdated: uninstall the old package first so npm - // doesn't hit EEXIST on the shared `codex-acp` bin-link, then install. + // Codex adapter outdated: uninstall old package first (npm refuses to overwrite a bin from another pkg). Some(_) => Some(vec![ "npm uninstall -g @zed-industries/codex-acp", "npm install -g @agentclientprotocol/codex-acp", @@ -133,9 +132,8 @@ pub async fn save_custom_harness( std::fs::create_dir_all(&custom_dir) .map_err(|e| format!("failed to create custom_harnesses dir: {e}"))?; - // ── Phase 2+3: backup-swap write + rename (Windows-safe, rollback on failure) - // `save_and_warm` holds the persist mutex for the write + registry-warm pair - // so concurrent saves never produce a stale registry snapshot (B-6). + // Phase 2+3: backup-swap write + rename (Windows-safe, rollback on failure). + // `save_and_warm` holds the persist mutex so concurrent saves never produce a stale registry. custom_harnesses::save_and_warm(&custom_dir, &definition, rename_old_id.as_deref())?; // Resolve availability for the returned catalog entry. @@ -165,6 +163,8 @@ pub async fn save_custom_harness( model_env_var: None, provider_env_var: None, thinking_env_var: None, + accepted_effort_values: None, + effort_aliases: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index 9bb0f6230d..3aebd38f87 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -257,4 +257,69 @@ mod tests { assert_eq!(launch["policy_env"]["BUZZ_ACP_AGENTS"], "4"); assert_eq!(launch["owner_pubkey"], "owner-hex"); } + + /// Deploy parity (plan v3 Delta 4): for a legacy-only Goose record, the bridged + /// descriptor feeds `launch.env` with the native key, while the separately-merged + /// top-level `env_vars` retains the legacy key untouched. + /// + /// Contract: providers execute `launch`; top-level `env_vars` is compatibility + /// bookkeeping. This test pins that boundary. + #[test] + fn deploy_parity_launch_env_carries_native_goose_effort() { + use crate::managed_agents::known_acp_runtime_exact; + use crate::managed_agents::{ + global_config::GlobalAgentConfig, resolve_effective_agent_env, + }; + + // Goose record with only legacy BUZZ_AGENT_THINKING_EFFORT. + let record: ManagedAgentRecord = serde_json::from_value(serde_json::json!({ + "pubkey": "pk", + "name": "goose-agent", + "private_key_nsec": "", + "relay_url": "", + "acp_command": "goose-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 1, + "respond_to": "owner-only", + "respond_to_allowlist": [], + "env_vars": { "BUZZ_AGENT_THINKING_EFFORT": "high" }, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + })) + .unwrap(); + + let runtime = known_acp_runtime_exact("goose"); + let global = GlobalAgentConfig::default(); + // The descriptor (= what launch.env uses) comes from resolve_effective_agent_env. + let descriptor = resolve_effective_agent_env(&record, &[], runtime, &global); + + // launch.env: legacy key translated → native key. + assert_eq!( + descriptor + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("high"), + "launch.env must carry native GOOSE_THINKING_EFFORT" + ); + assert!( + !descriptor.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"), + "launch.env must not carry legacy key" + ); + + // top-level env_vars (unmodified raw input): legacy key is still there. + // (The deploy payload's `env_vars` field is merged_user_env of the raw record — + // the bridge only affects the descriptor/launch path.) + assert_eq!( + record + .env_vars + .get("BUZZ_AGENT_THINKING_EFFORT") + .map(String::as_str), + Some("high"), + "top-level env_vars retains legacy key as compatibility bookkeeping" + ); + } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs index f8b045fc72..26d8a472fe 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs @@ -8,6 +8,80 @@ pub(crate) mod types; pub(crate) use types::*; +/// The legacy effort env key written by pre-migration saves. +/// +/// Harnesses whose native `thinking_env_var` differs from this constant +/// (currently: Goose uses `GOOSE_THINKING_EFFORT`) need the alias resolver +/// below to translate old saves. buzz-agent's native key equals this constant, +/// so no aliasing applies there. +pub(crate) const LEGACY_THINKING_EFFORT_KEY: &str = "BUZZ_AGENT_THINKING_EFFORT"; + +/// Return all known native thinking-effort env keys across all runtimes. +/// +/// Derived from `KNOWN_ACP_RUNTIMES::thinking_env_var` so that adding a new +/// runtime automatically participates in foreign-key stripping without a +/// separate constant to update. +/// +/// Callers that need the slice for iterating (e.g. foreign-key stripping in +/// `apply_effort_bridge`) should call this function rather than maintaining +/// a parallel constant. +pub(crate) fn all_known_effort_keys() -> impl Iterator { + crate::managed_agents::discovery::KNOWN_ACP_RUNTIMES + .iter() + .filter_map(|rt| rt.thinking_env_var) +} + +/// Resolve the thinking-effort value for a single env-var tier map, with +/// within-tier legacy aliasing and normalization. +/// +/// Returns the **canonical** value (normalized via `norm`) for the tier, or +/// `None` when no usable candidate exists. +/// +/// Lookup order (applied independently per tier, not globally): +/// 1. Native key (`native_key`) — value normalized; invalid values skip as absent. +/// 2. Legacy key (`BUZZ_AGENT_THINKING_EFFORT`) — honoured only when: +/// (a) `native_key` differs from the legacy key (i.e. non-buzz-agent runtime), AND +/// (b) `allow_legacy_alias` is true (record and persona tiers only), AND +/// (c) the value normalizes to a canonical form. +/// An invalid legacy value is skipped so the next tier can supply a candidate. +/// +/// The `norm` function normalizes a raw value to canonical form; `None` = invalid. +/// +/// ## Per-tier `allow_legacy_alias` policy (plan v3) +/// +/// | Tier | `allow_legacy_alias` | Rationale | +/// |-------------|----------------------|--------------------------------------------------| +/// | record | `true` | Record-level legacy key migrated at save | +/// | persona | `true` | Persona-level legacy key migrated at save | +/// | global | `false` | Global legacy excluded end-to-end (Delta 2/5) | +/// | definition | `false` | Definition env is author-controlled; legacy alias| +/// | | | would silently conflate foreign effort | +/// | baked | `false` | Build floor; only native key is authoritative | +pub(crate) fn effort_tier_alias( + map: &std::collections::BTreeMap, + native_key: &str, + norm: impl Fn(&str) -> Option, + allow_legacy_alias: bool, +) -> Option { + // Native key first — normalize the value; invalid → skip. + if let Some(raw) = map.get(native_key) { + if let Some(canonical) = norm(raw) { + return Some(canonical); + } + // Invalid native value: skip-as-absent, fall through to legacy. + } + // Legacy alias — only when keys differ and this tier permits legacy consumption. + if allow_legacy_alias && native_key != LEGACY_THINKING_EFFORT_KEY { + if let Some(raw) = map.get(LEGACY_THINKING_EFFORT_KEY) { + if let Some(canonical) = norm(raw) { + return Some(canonical); + } + // Invalid legacy value for this harness: skip, fall through to next tier. + } + } + None +} + /// Read the goose harness config file (`~/.config/goose/config.yaml`). /// /// Used by readiness evaluation to silence requirements that are already @@ -16,3 +90,332 @@ pub(crate) use types::*; pub(crate) fn read_goose_file_config() -> Option { goose::read_config_file() } + +/// Apply the spawn-side legacy effort bridge to an already-merged effective env. +/// +/// For runtimes with a static effort vocabulary (`effort_normalization` is `Some`): +/// 1. Walk per-tier sanitized maps in tier-first precedence order and resolve the +/// canonical effort value. +/// 2. Strip all foreign known effort keys from `env` (runtime-scoped invariant). +/// 3. Remove any raw (possibly invalid/alias-form) entry for the native key. +/// 4. Insert the canonical value under the native key (if any tier resolved one). +/// +/// Tier order (spawn; ACP and file tiers absent): +/// record native → record legacy → persona native → persona legacy +/// → global native → definition native → baked native +/// +/// Global and definition legacy are excluded (plan v3 Delta 2). +/// Baked tier is native-only: build-floor values are already canonical; +/// applying the legacy alias there would silently consume a foreign key. +#[allow(clippy::too_many_arguments)] // baked tier is a required 8th param; grouping into a struct is premature +pub(crate) fn apply_effort_bridge( + env: &mut std::collections::BTreeMap, + runtime: Option<&crate::managed_agents::discovery::KnownAcpRuntime>, + record_env: &std::collections::BTreeMap, + personas: &[crate::managed_agents::types::AgentDefinition], + persona_id: Option<&str>, + global_env: &std::collections::BTreeMap, + harness_def: Option<&crate::managed_agents::custom_harnesses::HarnessDefinition>, + baked_env: &std::collections::BTreeMap, +) { + use std::collections::BTreeMap; + + let rt = match runtime { + Some(rt) => rt, + None => return, + }; + // Strip foreign known effort keys for any runtime that has a native effort key, + // regardless of whether it has an effort_normalization contract. + // This ensures GOOSE_THINKING_EFFORT is absent from buzz-agent descriptors and vice versa. + // Derived from runtime declarations — adding a new runtime automatically participates. + if let Some(native_key) = &rt.thinking_env_var { + for key in all_known_effort_keys() { + if key != *native_key { + env.remove(key); + } + } + } + // Effort tier resolution and alias normalization require effort_normalization. + let (norm, native_key) = match (&rt.effort_normalization, &rt.thinking_env_var) { + (Some(n), Some(k)) => (n, k), + _ => return, + }; + let norm_fn = |raw: &str| norm.normalize_str(raw); + let mue = crate::managed_agents::env_vars::merged_user_env; + let is_reserved = crate::managed_agents::env_vars::is_reserved_env_key; + let live_persona_env = crate::managed_agents::env_vars::live_persona_env; + + let s_record = mue(&BTreeMap::new(), record_env); + let s_persona = mue(&BTreeMap::new(), &live_persona_env(personas, persona_id)); + let s_global = mue(&BTreeMap::new(), global_env); + let s_def: BTreeMap = harness_def + .map(|d| { + d.env + .iter() + .filter(|(k, _)| !is_reserved(k)) + .map(|(k, v)| (k.clone(), v.clone())) + .collect() + }) + .unwrap_or_default(); + + // Baked tier: native-key only (no legacy alias — build floor is already + // canonical; allowing legacy here would silently conflate a foreign effort key). + // Only the native key is extracted to avoid carrying arbitrary baked keys. + let baked_native: BTreeMap = baked_env + .get(*native_key) + .map(|v| [(native_key.to_string(), v.clone())].into_iter().collect()) + .unwrap_or_default(); + + // Tier precedence (highest → lowest): + // record (legacy allowed) → persona (legacy allowed) → global (no legacy) + // → definition (no legacy) → baked (no legacy) + let canonical = None + .or_else(|| effort_tier_alias(&s_record, native_key, norm_fn, true)) + .or_else(|| effort_tier_alias(&s_persona, native_key, norm_fn, true)) + .or_else(|| effort_tier_alias(&s_global, native_key, norm_fn, false)) + .or_else(|| effort_tier_alias(&s_def, native_key, norm_fn, false)) + .or_else(|| effort_tier_alias(&baked_native, native_key, norm_fn, false)); + + // Remove raw native key (may be alias-form or invalid); canonical re-inserted below. + env.remove(*native_key); + if let Some(value) = canonical { + env.insert(native_key.to_string(), value); + } +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + + /// Goose runtime from the catalog — has static effort vocabulary and + /// native key `GOOSE_THINKING_EFFORT`. + fn goose_rt() -> &'static crate::managed_agents::discovery::KnownAcpRuntime { + crate::managed_agents::discovery::known_acp_runtime_exact("goose") + .expect("goose must be in catalog") + } + + fn empty_personas() -> Vec { + Vec::new() + } + + fn env_with(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + // ── baked tier tests ────────────────────────────────────────────────────── + + /// Baked `GOOSE_THINKING_EFFORT=high` with no higher-precedence tier + /// → canonical `high` survives in the effective env. + #[test] + fn baked_high_value_spawns_as_effort() { + let baked = env_with(&[("GOOSE_THINKING_EFFORT", "high")]); + let record = BTreeMap::new(); + let global = BTreeMap::new(); + let mut env = baked.clone(); // spawn starts with baked floor + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &record, + &empty_personas(), + None, + &global, + None, + &baked, + ); + + assert_eq!( + env.get("GOOSE_THINKING_EFFORT").map(String::as_str), + Some("high"), + "baked valid native value must survive to launch" + ); + } + + /// Baked `GOOSE_THINKING_EFFORT=xhigh` → normalized to canonical `max`. + #[test] + fn baked_xhigh_normalizes_to_max() { + let baked = env_with(&[("GOOSE_THINKING_EFFORT", "xhigh")]); + let mut env = baked.clone(); + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &BTreeMap::new(), + &empty_personas(), + None, + &BTreeMap::new(), + None, + &baked, + ); + + assert_eq!( + env.get("GOOSE_THINKING_EFFORT").map(String::as_str), + Some("max"), + "baked xhigh alias must normalize to canonical max" + ); + } + + /// Baked `GOOSE_THINKING_EFFORT=minimal` (invalid for Goose) → key absent from env. + #[test] + fn baked_invalid_minimal_skipped() { + let baked = env_with(&[("GOOSE_THINKING_EFFORT", "minimal")]); + let mut env = baked.clone(); + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &BTreeMap::new(), + &empty_personas(), + None, + &BTreeMap::new(), + None, + &baked, + ); + + assert!( + !env.contains_key("GOOSE_THINKING_EFFORT"), + "baked invalid value must be skipped (key absent from launch env)" + ); + } + + /// Baked env contains `BUZZ_AGENT_THINKING_EFFORT=high` (legacy key for Goose). + /// The baked tier is native-key-only — the legacy key must NOT be aliased. + #[test] + fn baked_legacy_key_not_aliased_in_baked_tier() { + // The baked env has only the legacy key — no Goose-native key. + let baked = env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "high")]); + let mut env = baked.clone(); + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &BTreeMap::new(), + &empty_personas(), + None, + &BTreeMap::new(), + None, + &baked, + ); + + // Legacy key should be stripped (foreign to Goose) and native key absent. + assert!( + !env.contains_key("GOOSE_THINKING_EFFORT"), + "baked legacy key must not produce a native effort value (no aliasing in baked tier)" + ); + // The legacy foreign key is also stripped by the foreign-key sweep. + assert!( + !env.contains_key("BUZZ_AGENT_THINKING_EFFORT"), + "foreign effort key must be stripped from Goose's env" + ); + } + + /// Record-level effort beats baked — record `max` wins over baked `high`. + #[test] + fn baked_beaten_by_record() { + let baked = env_with(&[("GOOSE_THINKING_EFFORT", "high")]); + let record = env_with(&[("GOOSE_THINKING_EFFORT", "max")]); + let mut env = { + let mut e = baked.clone(); + for (k, v) in &record { + e.insert(k.clone(), v.clone()); + } + e + }; + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &record, + &empty_personas(), + None, + &BTreeMap::new(), + None, + &baked, + ); + + assert_eq!( + env.get("GOOSE_THINKING_EFFORT").map(String::as_str), + Some("max"), + "record-level effort must beat baked floor" + ); + } + + // ── definition tier alias policy ────────────────────────────────────────── + + fn def_with_env( + pairs: &[(&str, &str)], + ) -> crate::managed_agents::custom_harnesses::HarnessDefinition { + crate::managed_agents::custom_harnesses::HarnessDefinition { + id: "test-def".to_string(), + label: "Test Definition".to_string(), + command: "goose".to_string(), + args: Vec::new(), + env: pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + install_instructions_url: String::new(), + install_hint: String::new(), + } + } + + /// Definition env containing only the legacy key `BUZZ_AGENT_THINKING_EFFORT=high` + /// for a Goose agent → the bridge must NOT alias it; native key remains absent. + /// Mirrors reader.rs: definition tier uses `allow_legacy_alias=false`. + #[test] + fn definition_legacy_key_excluded_spawn() { + let harness_def = def_with_env(&[("BUZZ_AGENT_THINKING_EFFORT", "high")]); + let mut env = env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "high")]); + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &BTreeMap::new(), + &empty_personas(), + None, + &BTreeMap::new(), + Some(&harness_def), + &BTreeMap::new(), + ); + + assert!( + !env.contains_key("GOOSE_THINKING_EFFORT"), + "definition-tier legacy key must NOT be aliased to the native effort key" + ); + // Legacy key is also stripped by the foreign-key sweep. + assert!( + !env.contains_key("BUZZ_AGENT_THINKING_EFFORT"), + "foreign effort key must be stripped from Goose's env" + ); + } + + /// Definition env with the native key `GOOSE_THINKING_EFFORT=medium` + /// → the bridge accepts it (native key at definition tier is fine). + #[test] + fn definition_native_key_accepted_spawn() { + let harness_def = def_with_env(&[("GOOSE_THINKING_EFFORT", "medium")]); + let mut env = env_with(&[("GOOSE_THINKING_EFFORT", "medium")]); + + apply_effort_bridge( + &mut env, + Some(goose_rt()), + &BTreeMap::new(), + &empty_personas(), + None, + &BTreeMap::new(), + Some(&harness_def), + &BTreeMap::new(), + ); + + assert_eq!( + env.get("GOOSE_THINKING_EFFORT").map(String::as_str), + Some("medium"), + "definition native key must be accepted and reinserted as canonical" + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index c51f325cf3..a774c68753 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -1,4 +1,5 @@ -use crate::managed_agents::discovery::KnownAcpRuntime; +use crate::managed_agents::config_bridge::effort_tier_alias; +use crate::managed_agents::discovery::{EffortNormalization, KnownAcpRuntime}; use crate::managed_agents::types::ManagedAgentRecord; use super::types::*; @@ -35,6 +36,7 @@ pub(crate) fn read_config_surface( let provider_env_var = runtime_meta.and_then(|m| m.provider_env_var); let provider_locked = runtime_meta.is_some_and(|m| m.provider_locked); let thinking_env_var = runtime_meta.and_then(|m| m.thinking_env_var); + let effort_norm = runtime_meta.and_then(|m| m.effort_normalization); let supports_acp_native = runtime_meta.is_some_and(|m| m.supports_acp_native_config); let required_fields: &[&str] = runtime_meta .map(|m| m.required_normalized_fields) @@ -49,7 +51,12 @@ pub(crate) fn read_config_surface( .or_else(|| find_config_option_value(c, "model")) }); let acp_mode = session_cache.and_then(|c| find_config_option_value(c, "mode")); - let acp_effort = session_cache.and_then(|c| find_config_option_value(c, "effort")); + // Effort: find by real ACP category `thought_level` (Goose, claude-agent-acp); fall back to + // the legacy invented category `effort` for transition compatibility. Retain the matched + // entry's actual config_id for write-back routing. + let acp_effort_opt = session_cache.and_then(find_effort_config_option); + let acp_effort = acp_effort_opt.as_ref().map(|(v, _)| v.clone()); + let acp_effort_config_id = acp_effort_opt.map(|(_, id)| id); let model_overridden = session_cache.is_some_and(|c| c.model_overridden); @@ -79,7 +86,9 @@ pub(crate) fn read_config_surface( record, &file_config.thinking_effort, &acp_effort, + acp_effort_config_id.as_deref(), thinking_env_var, + effort_norm, is_pre_spawn, session_cache, tiers, @@ -486,37 +495,164 @@ fn build_thinking_field( record: &ManagedAgentRecord, file_effort: &Option, acp_effort: &Option, + acp_effort_config_id: Option<&str>, thinking_env_var: Option<&str>, + effort_norm: Option<&'static EffortNormalization>, is_pre_spawn: bool, - session_cache: Option<&SessionConfigCache>, + _session_cache: Option<&SessionConfigCache>, tiers: &InheritedConfigTiers, ) -> Option { // Tier ordering: record env > ACP > persona env > global env > definition env > config file. - let [rec_env, pers_env, glob_env, def_env] = thinking_env_var - .map(|k| { - env_candidates( - k, - &record.env_vars, - &tiers.persona_env, - &tiers.global_env, - &tiers.definition_env, - ) - }) - .unwrap_or([None, None, None, None]); + // + // When the runtime declares an effort normalization contract (e.g. Goose), each of the + // record and persona tiers is resolved via `effort_tier_alias`, which applies within-tier + // legacy aliasing with normalization: if the native key is absent or invalid, fall back to + // BUZZ_AGENT_THINKING_EFFORT when the value normalizes to a canonical form. + // Global env uses the same resolver but with `allow_legacy_alias=false` — legacy alias excluded. + // Definition env also uses `allow_legacy_alias=false` semantics (no legacy alias there). + + // Resolve per-tier effort values with optional normalization. + // Owned values kept in locals to outlive the &str borrows below. + let _rec_o: Option; + let _per_o: Option; + let _glo_o: Option; + let _def_o: Option; + let _file_o: Option; + + let rec_env: Option<&str>; + let pers_env: Option<&str>; + let glob_env: Option<&str>; + let def_env: Option<&str>; + let file_env: Option<&str>; + + if let Some((native, norm)) = thinking_env_var.zip(effort_norm) { + let nf = |v: &str| norm.normalize_str(v); + _rec_o = effort_tier_alias(&record.env_vars, native, nf, true); + _per_o = effort_tier_alias(&tiers.persona_env, native, nf, true); + _glo_o = effort_tier_alias(&tiers.global_env, native, nf, false); + _def_o = effort_tier_alias(&tiers.definition_env, native, nf, false); + // Normalize file effort too for consistent B comparison. + _file_o = file_effort.as_deref().and_then(|v| norm.normalize_str(v)); + rec_env = _rec_o.as_deref(); + pers_env = _per_o.as_deref(); + glob_env = _glo_o.as_deref(); + def_env = _def_o.as_deref(); + file_env = _file_o.as_deref(); + } else { + _rec_o = None; + _per_o = None; + _glo_o = None; + _def_o = None; + _file_o = None; + let [re, pe, ge, de] = thinking_env_var + .map(|k| { + env_candidates( + k, + &record.env_vars, + &tiers.persona_env, + &tiers.global_env, + &tiers.definition_env, + ) + }) + .unwrap_or([None, None, None, None]); + rec_env = re; + pers_env = pe; + glob_env = ge; + def_env = de; + file_env = file_effort.as_deref(); + } - let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[ + // Effort same-value collapse (B heuristic): when ACP would be the winner and its value + // exactly equals what inheritance already resolves to (after normalization), fall through to + // non-ACP resolution so the panel shows the true baseline origin ("Global default") rather + // than "Runtime override (this session only)". The live session is almost certainly echoing + // what spawn injected. + // + // Effort-only: model has an explicit `model_overridden` signal; effort has no equivalent, + // so we apply the heuristic unconditionally for effort alone (plan v3 Phase 1). + let without_acp: &[(Option<&str>, ConfigOrigin)] = &[ (rec_env, ConfigOrigin::BuzzExplicit), - (acp_effort.as_deref(), ConfigOrigin::AcpConfigOption), (pers_env, ConfigOrigin::PersonaDefault), (glob_env, ConfigOrigin::GlobalDefault), (def_env, ConfigOrigin::HarnessDefault), - (file_effort.as_deref(), ConfigOrigin::ConfigFile), + (file_env, ConfigOrigin::ConfigFile), ]; + + // Normalize the raw ACP effort value (plan v3 Delta 1: every candidate normalized + // before validity, precedence, override tracking, and B equality). + // When the runtime declares a normalization contract (`effort_norm`), canonicalize + // the live ACP value; if it is an alias (`none`→`off`, `xhigh`→`max`, case-fold) + // the normalized form is used for comparison and display. If the ACP value is + // invalid for this runtime (e.g. `minimal` or garbage), treat it as absent so + // lower tiers win — skip-as-absent applies to the output map, not just winner + // selection. The matched `config_id` is preserved for `write_via` regardless. + let _acp_norm_o: Option; + let acp_effort_normalized: Option<&str> = + if let (Some(raw), Some(norm)) = (acp_effort.as_deref(), effort_norm) { + match norm.normalize_str(raw) { + Some(canonical) => { + _acp_norm_o = Some(canonical.to_string()); + _acp_norm_o.as_deref() + } + None => { + // ACP value is invalid for this runtime — skip as absent. + _acp_norm_o = None; + None + } + } + } else { + // No normalization contract (buzz-agent legacy path): pass through raw. + _acp_norm_o = None; + acp_effort.as_deref() + }; + + let tiers_list: &[(Option<&str>, ConfigOrigin)]; + let with_acp_storage; + if rec_env.is_none() { + if let Some(acp) = acp_effort_normalized { + let baseline_value = without_acp + .iter() + .find(|(v, _)| v.is_some()) + .and_then(|(v, _)| *v); + if baseline_value == Some(acp) { + // Equal-value (after normalization): fall through to non-ACP resolution. + tiers_list = without_acp; + } else { + // Genuine divergence: ACP wins between rec_env and pers_env. + with_acp_storage = [ + (rec_env, ConfigOrigin::BuzzExplicit), + (Some(acp), ConfigOrigin::AcpConfigOption), + (pers_env, ConfigOrigin::PersonaDefault), + (glob_env, ConfigOrigin::GlobalDefault), + (def_env, ConfigOrigin::HarnessDefault), + (file_env, ConfigOrigin::ConfigFile), + ]; + tiers_list = &with_acp_storage; + } + } else { + tiers_list = without_acp; + } + } else { + // rec_env is Some: record-level env always wins over ACP regardless. + // Insert normalized ACP (or None if invalid) to preserve tier structure. + with_acp_storage = [ + (rec_env, ConfigOrigin::BuzzExplicit), + (acp_effort_normalized, ConfigOrigin::AcpConfigOption), + (pers_env, ConfigOrigin::PersonaDefault), + (glob_env, ConfigOrigin::GlobalDefault), + (def_env, ConfigOrigin::HarnessDefault), + (file_env, ConfigOrigin::ConfigFile), + ]; + tiers_list = &with_acp_storage; + } let (value, origin, overridden_value, overridden_origin) = resolve_with_override(tiers_list)?; - let write_via = if !is_pre_spawn && has_config_option(session_cache, "effort") { + // Write route: use the matched entry's actual config_id (not a hardcoded constant). + // `thought_level` category entries advertise `thinking_effort` as their id (Goose); + // claude-agent-acp uses its own id. Never hardcode `"effort"`. + let write_via = if let (false, Some(config_id)) = (is_pre_spawn, acp_effort_config_id) { ConfigWriteMechanism::AcpSetConfigOption { - config_id: "effort".to_string(), + config_id: config_id.to_string(), } } else if let Some(env_key) = thinking_env_var { ConfigWriteMechanism::RespawnWithEnvVar { @@ -694,6 +830,37 @@ fn find_model_config_id(cache: Option<&SessionConfigCache>) -> Option { }) } +/// Find the live effort ACP option, returning `(current_value, config_id)`. +/// +/// Searches by ACP category `thought_level` first — the schema constant used by +/// Goose (`acp/response_builder.rs:286-323`) and claude-agent-acp. Falls back to +/// the legacy invented category `effort` for transition compatibility with old +/// test fixtures and adapters that were built before the canonical category name +/// was established. `thought_level` always wins when both categories are present. +fn find_effort_config_option(cache: &SessionConfigCache) -> Option<(String, String)> { + // Primary: real ACP category emitted by Goose and claude-agent-acp. + if let Some(entry) = cache + .config_options + .iter() + .find(|e| e.category.as_deref() == Some("thought_level")) + { + if let Some(v) = &entry.current_value { + return Some((v.clone(), entry.config_id.clone())); + } + } + // Fallback: legacy invented category (transition / test fixtures). + if let Some(entry) = cache + .config_options + .iter() + .find(|e| e.category.as_deref() == Some("effort")) + { + if let Some(v) = &entry.current_value { + return Some((v.clone(), entry.config_id.clone())); + } + } + None +} + fn resolve_tilde(path: &str) -> String { if let Some(rest) = path.strip_prefix("~/") { if let Some(home) = dirs::home_dir() { diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 62caffeb2e..c43798932e 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -28,39 +28,7 @@ fn with_goose_path_root(value: Option<&str>, body: impl FnOnce() -> T) -> T { } fn test_runtime() -> &'static KnownAcpRuntime { - &KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - max_rounds_env_var: None, - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - } + crate::managed_agents::known_acp_runtime_exact("goose").expect("goose must be in catalog") } fn test_record() -> ManagedAgentRecord { @@ -617,39 +585,8 @@ fn extra_env_var_skipped_when_already_in_file_config_extra() { // them in the advanced tier. fn buzz_agent_runtime() -> &'static KnownAcpRuntime { - &KnownAcpRuntime { - id: "buzz-agent", - label: "Buzz Agent", - commands: &["buzz-agent"], - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: true, - model_env_var: Some("BUZZ_AGENT_MODEL"), - provider_env_var: Some("BUZZ_AGENT_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: None, - config_file_format: None, - supports_acp_native_config: false, - thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), - max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), - context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), - max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - } + crate::managed_agents::known_acp_runtime_exact("buzz-agent") + .expect("buzz-agent must be in catalog") } #[test] diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs index 8613124f25..32c0656714 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs @@ -256,3 +256,540 @@ fn reserved_key_absent_from_definition_env_falls_through() { assert_eq!(model.value.as_deref(), Some("persona-struct-model")); assert_eq!(model.origin, ConfigOrigin::PersonaDefault); } + +// ── Phase 1: thought_level ACP category + alias normalization + B-collapse ─── +// +// Plan v3 Phase 1: Live Goose effort is identified by ACP category +// `thought_level` (not the invented `effort` category). The matched entry's +// real `config_id` is used for AcpSetConfigOption write-back. All candidates +// are normalized before comparison so aliases compare equal to canonical +// forms (none↔off, xhigh↔max). B-collapse applies after normalization. + +/// Goose real ACP shape: `category="thought_level"`, `id="thinking_effort"`, +/// canonical current value `high` → effort surfaces as `AcpConfigOption` with +/// write_via `AcpSetConfigOption { config_id: "thinking_effort" }`. +/// This pins that live Goose effort is actually read (the old `effort` category +/// would miss it entirely on a real Goose session). +#[test] +fn goose_real_acp_shape_thought_level_surfaces_and_routes_write_via_thinking_effort() { + let record = test_record(); + let runtime = test_runtime(); // Goose with effort_normalization + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("high".to_string()), + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("Goose thought_level effort must surface"); + assert_eq!(effort.value.as_deref(), Some("high")); + assert_eq!(effort.origin, ConfigOrigin::AcpConfigOption); + assert!( + matches!( + &effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { config_id } + if config_id == "thinking_effort" + ), + "write_via must be AcpSetConfigOption with config_id=\"thinking_effort\", got {:?}", + effort.write_via + ); +} + +/// ★ Both ACP categories present → `thought_level` wins, `effort` is ignored. +/// Prevents hardcoding or first-match bugs that would pick the wrong entry. +#[test] +fn thought_level_category_wins_over_effort_category_when_both_present() { + let record = test_record(); + let runtime = test_runtime(); // Goose + let cache = SessionConfigCache { + config_options: vec![ + // Legacy category comes first in the vec — must not win. + AcpConfigOptionEntry { + config_id: "effort".to_string(), + category: Some("effort".to_string()), + display_name: Some("Effort (legacy)".to_string()), + current_value: Some("low".to_string()), + options: vec![], + }, + AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("high".to_string()), + options: vec![], + }, + ], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface from thought_level category"); + assert_eq!( + effort.value.as_deref(), + Some("high"), + "thought_level value must win" + ); + assert!( + matches!( + &effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { config_id } + if config_id == "thinking_effort" + ), + "write_via must use thought_level entry's config_id" + ); +} + +/// B-collapse alias case — `none` and `off` are the same Goose effort after +/// normalization. ACP emits canonical `off`; record env has legacy `none`. +/// After normalization both equal `off` → B-collapse applies and the panel +/// shows the true baseline origin (BuzzExplicit), not AcpConfigOption. +/// write_via must still use AcpSetConfigOption (live session) even when +/// display origin falls through. +#[test] +fn b_collapse_none_and_off_are_equal_after_normalization() { + let mut record = test_record(); + // Record env carries the alias `none` (legacy write). + record + .env_vars + .insert("GOOSE_THINKING_EFFORT".to_string(), "none".to_string()); + let runtime = test_runtime(); // Goose with effort_normalization + // Live ACP emits canonical `off`. + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("off".to_string()), + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface"); + // Normalized `none` == canonical `off` — record env wins (above ACP in tier order). + // Record env is above ACP, so it wins regardless of B-collapse — BuzzExplicit. + assert_eq!( + effort.value.as_deref(), + Some("off"), + "alias normalized to canonical" + ); + assert_eq!(effort.origin, ConfigOrigin::BuzzExplicit); +} + +/// B-collapse alias case — `xhigh` and `max` are the same Goose effort after +/// normalization. ACP emits canonical `max`; global env has alias `xhigh`. +/// After normalization both equal `max` → B-collapse: ACP falls through to +/// the non-ACP resolution, showing GlobalDefault as origin. +/// write_via stays AcpSetConfigOption (live session has an effort option). +#[test] +fn b_collapse_xhigh_and_max_are_equal_after_normalization() { + let record = test_record(); + let runtime = test_runtime(); // Goose with effort_normalization + // Global env has alias `xhigh`. + let tiers = global_env_tiers("GOOSE_THINKING_EFFORT", "xhigh"); + // Live ACP emits canonical `max`. + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("max".to_string()), + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &tiers) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface"); + // B-collapse: ACP `max` == normalized `xhigh` (`max`) → fall through to non-ACP. + // Non-ACP resolution: global env `xhigh` normalizes to `max`, origin GlobalDefault. + assert_eq!( + effort.value.as_deref(), + Some("max"), + "alias normalized to canonical" + ); + assert_eq!( + effort.origin, + ConfigOrigin::GlobalDefault, + "B-collapse: equal-value ACP falls through to true baseline origin" + ); + // write_via stays ACP-backed even when display provenance falls through. + assert!( + matches!( + &effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { config_id } + if config_id == "thinking_effort" + ), + "equal-value collapse must retain ACP write_via with thinking_effort id" + ); +} + +/// Alias normalization in record env: `GOOSE_THINKING_EFFORT=none` is +/// normalized to canonical `off` before being surfaced. The panel sees +/// the canonical form, never the raw alias. +#[test] +fn goose_record_env_alias_none_normalized_to_canonical_off() { + let mut record = test_record(); + record + .env_vars + .insert("GOOSE_THINKING_EFFORT".to_string(), "none".to_string()); + let runtime = test_runtime(); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), None, &no_tiers()) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface"); + assert_eq!( + effort.value.as_deref(), + Some("off"), + "none must normalize to off" + ); + assert_eq!(effort.origin, ConfigOrigin::BuzzExplicit); +} + +/// Legacy fallback: `category="effort"` is still honoured when no `thought_level` +/// entry exists (e.g. older adapters or test fixtures that predate the real category). +#[test] +fn legacy_effort_category_fallback_used_when_no_thought_level_present() { + let record = test_record(); + let runtime = test_runtime(); // Goose + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "effort_legacy".to_string(), + category: Some("effort".to_string()), + display_name: Some("Effort".to_string()), + current_value: Some("medium".to_string()), + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("legacy effort category must surface as fallback"); + assert_eq!(effort.value.as_deref(), Some("medium")); + assert_eq!(effort.origin, ConfigOrigin::AcpConfigOption); + // write_via uses the actual config_id from the matched entry. + assert!( + matches!( + &effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { config_id } + if config_id == "effort_legacy" + ), + "fallback must retain entry's own config_id, not a hardcoded value" + ); +} + +// ── ACP effort normalization tests ──────────────────────────────────────────── +// +// Plan v3 Delta 1: every candidate — native, legacy, ACP, file — normalized +// before validity, precedence, override tracking, and B equality. +// ACP effort must be canonicalized through `effort_norm` before comparison; +// aliases (`none`→`off`, `xhigh`→`max`, case-fold) must collapse; invalid +// values (e.g. `minimal`) must be treated as absent so lower tiers win. + +/// ACP `HIGH` (wrong case) against global `high` — must normalize to `high` and B-collapse. +#[test] +fn acp_effort_case_alias_collapses_against_global_canonical() { + let record = test_record(); + let runtime = test_runtime(); // Goose with effort_normalization + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("HIGH".to_string()), // wrong case + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + // Global env has canonical `high`. + let mut tiers = no_tiers(); + tiers + .global_env + .insert("GOOSE_THINKING_EFFORT".to_string(), "high".to_string()); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &tiers) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface"); + // ACP `HIGH` normalizes to `high` == global `high` → B-collapse: falls through to + // non-ACP resolution, origin is GlobalDefault (not AcpConfigOption). + assert_eq!( + effort.value.as_deref(), + Some("high"), + "case-folded ACP value must collapse to canonical" + ); + assert_eq!( + effort.origin, + ConfigOrigin::GlobalDefault, + "B-collapse must report GlobalDefault, not AcpConfigOption" + ); +} + +/// ACP `none` against global `off` — alias-equal after normalization, must B-collapse. +#[test] +fn acp_effort_alias_none_collapses_against_global_off() { + let record = test_record(); + let runtime = test_runtime(); + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("none".to_string()), // alias for `off` + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let mut tiers = no_tiers(); + tiers + .global_env + .insert("GOOSE_THINKING_EFFORT".to_string(), "off".to_string()); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &tiers) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface"); + assert_eq!( + effort.value.as_deref(), + Some("off"), + "ACP `none` must normalize to `off` and B-collapse with global `off`" + ); + assert_eq!(effort.origin, ConfigOrigin::GlobalDefault); +} + +/// ACP `xhigh` against persona `max` — alias-equal after normalization, must B-collapse. +#[test] +fn acp_effort_alias_xhigh_collapses_against_persona_max() { + let record = test_record(); + let runtime = test_runtime(); + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("xhigh".to_string()), // alias for `max` + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let mut tiers = no_tiers(); + tiers + .persona_env + .insert("GOOSE_THINKING_EFFORT".to_string(), "max".to_string()); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &tiers) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface"); + assert_eq!( + effort.value.as_deref(), + Some("max"), + "ACP `xhigh` must normalize to `max` and B-collapse with persona `max`" + ); + assert_eq!(effort.origin, ConfigOrigin::PersonaDefault); +} + +/// ACP value invalid for Goose (`minimal`) — must be skipped; lower valid candidate wins. +#[test] +fn acp_effort_invalid_for_runtime_skipped_lower_tier_wins() { + let record = test_record(); + let runtime = test_runtime(); + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "thinking_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Thinking Effort".to_string()), + current_value: Some("minimal".to_string()), // Goose does not accept "minimal" + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + // Global env has a valid Goose effort — should win because ACP `minimal` is skipped. + let mut tiers = no_tiers(); + tiers + .global_env + .insert("GOOSE_THINKING_EFFORT".to_string(), "medium".to_string()); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), Some(&cache), &tiers) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface from global tier"); + assert_eq!( + effort.value.as_deref(), + Some("medium"), + "invalid ACP `minimal` must be skipped; global `medium` must win" + ); + assert_eq!( + effort.origin, + ConfigOrigin::GlobalDefault, + "origin must be GlobalDefault after ACP skip" + ); + // write_via stays AcpSetConfigOption — the route targets the live option, + // not its current value; an invalid live value changes display resolution + // but not writability (plan v3 Phase 1 ruling). + assert!( + matches!( + &effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { config_id } + if config_id == "thinking_effort" + ), + "write_via must remain AcpSetConfigOption{{thinking_effort}} even when ACP value is invalid; got {:?}", + effort.write_via + ); +} + +// ── Definition-tier effort alias policy (reader) ────────────────────────────── +// +// plan v3: legacy alias (`BUZZ_AGENT_THINKING_EFFORT`) is consumed only at +// record and persona tiers; definition and global tiers use native-key-only +// lookup. These tests verify the reader enforces the same tier boundary as +// the spawn bridge. + +/// Definition env with legacy key `BUZZ_AGENT_THINKING_EFFORT=high` for Goose +/// → reader must NOT surface it as effort (definition tier excludes legacy alias). +#[test] +fn definition_legacy_key_excluded_reader() { + let record = test_record(); + let runtime = test_runtime(); // Goose + let mut tiers = no_tiers(); + tiers + .definition_env + .insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string()); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), None, &tiers) + }); + + assert!( + surface.normalized.thinking_effort.is_none(), + "definition-tier legacy key must NOT be surfaced as effort for Goose" + ); +} + +/// Definition env with native key `GOOSE_THINKING_EFFORT=medium` for Goose +/// → reader surfaces it correctly (native key at definition tier is accepted). +#[test] +fn definition_native_key_accepted_reader() { + let record = test_record(); + let runtime = test_runtime(); // Goose + let mut tiers = no_tiers(); + tiers + .definition_env + .insert("GOOSE_THINKING_EFFORT".to_string(), "medium".to_string()); + + let surface = with_goose_path_root(Some("/nonexistent"), || { + read_config_surface(&record, Some(runtime), None, &tiers) + }); + + let effort = surface + .normalized + .thinking_effort + .expect("definition native key must surface as effort"); + assert_eq!(effort.value.as_deref(), Some("medium")); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 2cccccb95e..26a40f251b 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -15,7 +15,9 @@ mod runtime_metadata; mod windows_install; use presets::{preset_catalog_entry, PRESET_HARNESSES}; pub(crate) use presets::{preset_harness_definitions, preset_harness_ids}; -pub(crate) use runtime_metadata::KnownAcpRuntime; +pub(crate) use runtime_metadata::{ + EffortNormalization, KnownAcpRuntime, GOOSE_EFFORT_NORMALIZATION, +}; const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; @@ -72,7 +74,7 @@ fn common_binary_paths() -> &'static [PathBuf] { }) } -const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ +pub(crate) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ KnownAcpRuntime { id: "goose", label: "Goose", @@ -101,6 +103,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ config_file_format: Some("yaml"), supports_acp_native_config: true, thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + effort_normalization: Some(&GOOSE_EFFORT_NORMALIZATION), max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), max_rounds_env_var: None, @@ -134,6 +137,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ config_file_format: Some("json"), supports_acp_native_config: false, thinking_env_var: None, + effort_normalization: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, @@ -167,6 +171,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ config_file_format: Some("toml"), supports_acp_native_config: false, thinking_env_var: None, + effort_normalization: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, @@ -201,6 +206,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ config_file_format: None, supports_acp_native_config: false, thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), + effort_normalization: None, // buzz-agent: per-model catalog; see getProviderEffortConfig() in TS max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), @@ -596,14 +602,8 @@ pub fn clear_resolve_cache() { } // ── Adapter availability cache (Phase-2 badge fallback) ───────────────────── -// -// `build_managed_agent_summary` needs to compare the spawn-time adapter -// availability against the *current* availability without triggering a live -// `probe_codex_acp_version` subprocess on every poll cycle. This cache -// stores the last availability status of the codex-acp binary at its resolved -// path. It is warmed by `discover_acp_runtimes` (which already probes), so -// the badge path reads warm data, and is invalidated by `clear_resolve_cache` -// (called on every Doctor install and every `discover_acp_providers` call). +// Warmed by `discover_acp_runtimes`; invalidated by `clear_resolve_cache`. +// 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}; @@ -732,10 +732,7 @@ fn resolve_command_uncached(command: &str) -> Option { } } - // Check nvm's default Node.js bin directory — nvm initializes via - // ~/.zshrc (interactive) which is not loaded by a login shell, so - // `node`, `npm`, and npm-global shims installed there are otherwise - // invisible. + // Check nvm's default Node.js bin directory (not on PATH in login shells). if let Some(home) = dirs::home_dir() { if let Some(nvm_bin) = find_nvm_default_bin(&home) { for basename in &basenames { @@ -1406,6 +1403,15 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr model_env_var: runtime.model_env_var.map(str::to_string), provider_env_var: runtime.provider_env_var.map(str::to_string), thinking_env_var: runtime.thinking_env_var.map(str::to_string), + accepted_effort_values: runtime + .effort_normalization + .map(|n| n.canonical_values().iter().map(|s| s.to_string()).collect()), + effort_aliases: runtime.effort_normalization.map(|n| { + n.aliases + .iter() + .map(|(a, c)| (a.to_string(), c.to_string())) + .collect() + }), max_tokens_env_var: runtime.max_tokens_env_var.map(str::to_string), context_limit_env_var: runtime.context_limit_env_var.map(str::to_string), max_rounds_env_var: runtime.max_rounds_env_var.map(str::to_string), @@ -1554,19 +1560,19 @@ pub fn discover_acp_runtimes_from( entries.push(AcpRuntimeCatalogEntry { id: def.id.clone(), label: def.label.clone(), - // F1 security fix: never copy user-supplied avatar URL into the catalog. - // All icons are bundled assets; customs fall back to TerminalSquare in the UI. + // Security: never copy user-supplied avatar URL; all icons are bundled assets. avatar_url: String::new(), availability, 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, thinking_env_var: None, + accepted_effort_values: None, + effort_aliases: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, @@ -1581,23 +1587,16 @@ pub fn discover_acp_runtimes_from( auth_status: AuthStatus::NotApplicable, login_hint: None, source: HarnessSource::Custom, - // Carry definition env into the catalog so the edit form can - // read it back — prevents silently erasing env on save. + // Carry definition env — prevents silently erasing env on save. definition_env: def.env.clone(), }); } } - // Publish the loaded-harness registry from a FRESH directory read under the - // persist mutex — never from the snapshot taken before the auth probes ran. - // A save/delete landing during Phase 2 already re-warmed the registry; a - // stale-snapshot publish here would clobber it (the just-saved harness - // would become unresolvable at spawn until the next discovery). - // - // This exact line is pinned by `discovery_publish_path_survives_mid_flight_save` - // / `..._drops_mid_flight_delete` (discovery tests), which land a save/delete - // through the pre-publish test hook below and red if this reverts to - // publishing a stale snapshot. + // Publish from a FRESH directory read under the persist mutex — never from + // the snapshot taken before auth probes ran. A save/delete during Phase 2 + // already re-warmed the registry; a stale publish here would clobber it. + // (Pinned by discovery_publish_path_survives_mid_flight_save and _delete.) #[cfg(test)] pre_publish_test_hook::run(); crate::managed_agents::custom_harnesses::warm_harness_registry_locked(custom_harnesses_dir); diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index bcc4288005..c25822913f 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -67,6 +67,8 @@ pub(super) fn preset_catalog_entry( model_env_var: None, provider_env_var: None, thinking_env_var: None, + accepted_effort_values: None, + effort_aliases: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index 34edecdcd9..fc143eca65 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -1,3 +1,62 @@ +/// Canonicalization contract for a harness's thinking-effort env var. +/// +/// The authority for UI choices, spawn bridge, and reader. All candidates +/// are normalized through `normalize()` before any validity, precedence, +/// override, or B-equality check. +/// +/// Source for Goose: `crates/goose-provider-types/src/thinking.rs` +/// • `FromStr` (aliases, case-insensitive): `off|disabled|none`, `low`, +/// `medium|med`, `high`, `max|xhigh` +/// • `Display` (canonical): `off`, `low`, `medium`, `high`, `max` +/// • Live ACP emits Display values via `response_builder.rs:326-337`. +pub(crate) struct EffortNormalization { + /// Canonical values in UI display order (drive choices, persistence, ACP comparison). + pub canonical: &'static [&'static str], + /// `(alias, canonical)` pairs, case-insensitive. Only aliases that differ + /// from their canonical form are listed. + pub aliases: &'static [(&'static str, &'static str)], +} + +/// Goose thinking-effort canonicalization contract. +/// +/// Source: `crates/goose-provider-types/src/thinking.rs` at Goose `2db0e31fe`. +/// Canonical Display values: `off`, `low`, `medium`, `high`, `max`. +/// Aliases (case-insensitive): `none|disabled→off`, `med→medium`, `xhigh→max`. +/// `minimal` (Buzz-only) is invalid — skipped as absent at every tier. +pub(crate) static GOOSE_EFFORT_NORMALIZATION: EffortNormalization = EffortNormalization { + canonical: &["off", "low", "medium", "high", "max"], + aliases: &[ + ("none", "off"), + ("disabled", "off"), + ("med", "medium"), + ("xhigh", "max"), + ], +}; + +impl EffortNormalization { + /// Normalize `raw` to a `String` canonical form. + /// `None` → invalid for this harness; caller must treat as absent (skip-as-absent policy). + pub fn normalize_str(&self, raw: &str) -> Option { + let lower = raw.to_lowercase(); + // Check direct canonical match first. + if self.canonical.contains(&lower.as_str()) { + return Some(lower); + } + // Check aliases. + for &(alias, canon) in self.aliases { + if lower == alias { + return Some(canon.to_string()); + } + } + None + } + + /// Return the canonical values slice (for UI choices). + pub fn canonical_values(&self) -> &'static [&'static str] { + self.canonical + } +} + /// Static capabilities and installation metadata for a known ACP runtime. pub(crate) struct KnownAcpRuntime { pub id: &'static str, @@ -47,6 +106,20 @@ pub(crate) struct KnownAcpRuntime { pub config_file_format: Option<&'static str>, pub supports_acp_native_config: bool, // tier 1a: config/read+write pub thinking_env_var: Option<&'static str>, + /// Canonicalization contract for `thinking_env_var` on this harness. + /// + /// `Some(contract)` — harness uses a finite, static effort vocabulary. + /// All candidates (native env, legacy env, ACP tier, file tier) are + /// normalized through this contract before validity checks, precedence + /// resolution, override tracking, and B-equality comparison. + /// + /// `None` — harness accepts any provider/model-specific value via its own + /// catalog (buzz-agent); see `getProviderEffortConfig()` in TS for that path. + /// + /// The single canonical authority shared by UI choices, spawn bridge, and + /// reader. No value-authority logic may live outside this struct for + /// harnesses that declare one. + pub effort_normalization: Option<&'static EffortNormalization>, /// Env var for normalizing `max_output_tokens`. `None` when the harness /// does not have a first-class env var for this field (config-file only). pub max_tokens_env_var: Option<&'static str>, diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 26902ae8de..3a9f85f66f 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -24,19 +24,8 @@ //! //! ## Env-assembly precedence (mirrors `spawn_agent_child`) //! -//! 1. Baked build defaults (`baked_build_env()`) — injected first so the -//! layers above can override them. -//! 2. Runtime metadata env vars (`runtime_metadata_env_vars`) — provider / -//! model env keys derived from the record's `model`/`provider` fields and -//! the runtime's `model_env_var`/`provider_env_var`. -//! 3. Merged user env (`merged_user_env`) — live persona env under the -//! record's `env_vars` overrides, after reserved-key and malformed-key -//! filtering. Last-wins on collision. -//! -//! The config-file tier (Goose `~/.config/goose/config.yaml`) is tracked -//! separately because it is not part of the process env — the harness reads -//! it at startup. We do not evaluate it here; it is exposed for future -//! UI display only. +//! Baked build defaults → runtime metadata env → merged user env. +//! Config-file tier (Goose `~/.config/goose/config.yaml`) tracked separately. use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; @@ -70,8 +59,6 @@ pub(crate) struct EffectiveAgentEnv { /// The process-env map the spawned harness would receive. pub env: BTreeMap, /// Harness config file path, if any (e.g. `~/.config/goose/config.yaml`). - // Not read yet; kept for the unified-agent-record rewrite (chunk A) which - // replaces this resolution path wholesale. #[allow(dead_code)] pub config_file_path: Option<&'static str>, /// The resolved harness binary name (e.g. `"buzz-agent"`, `"goose"`). @@ -242,8 +229,7 @@ fn resolve_effective_agent_env_with_def( } // Layer 2b: definition env — the harness author's defaults (e.g. CURSOR_ACP=1). - // Applied as a floor below global so user env always wins on collision. - // Reserved keys are stripped by the shared `is_reserved_env_key` predicate. + // Applied as a floor below global; reserved keys stripped by `is_reserved_env_key`. if let Some(ref def) = harness_def { for (key, value) in &def.env { if !super::env_vars::is_reserved_env_key(key) { @@ -252,17 +238,12 @@ fn resolve_effective_agent_env_with_def( } } - // Layer 3a: global env vars — the lowest user-settable layer. - // Injected before persona/agent so per-agent values win on collision. - // `merged_user_env` with an empty "lower" map applies reserved/malformed-key - // filtering to the global map for free. + // Layer 3a: global env vars (lowest user-settable layer). + // `merged_user_env` with empty lower map applies reserved/malformed-key filtering. let global_env = merged_user_env(&BTreeMap::new(), &global.env_vars); env.extend(global_env); - // Layer 3b: merged user env — live persona env under the record's own - // overrides (last-wins), after reserved/malformed-key filtering. Reading - // the persona live is what makes persona credential edits refresh on the - // next spawn instead of being frozen into the record. + // Layer 3b: merged user env — persona env under record overrides, filtered. let user_env = merged_user_env( &super::env_vars::live_persona_env(personas, record.persona_id.as_deref()), &record.env_vars, @@ -278,6 +259,18 @@ fn resolve_effective_agent_env_with_def( effective_model.as_deref(), ); + // Phase 3: translate legacy effort key → native key, strip foreign effort keys. + crate::managed_agents::config_bridge::apply_effort_bridge( + &mut env, + runtime, + &record.env_vars, + personas, + record.persona_id.as_deref(), + &global.env_vars, + harness_def.as_deref(), + &baked_build_env(), + ); + EffectiveAgentEnv { env, config_file_path: runtime.and_then(|r| r.config_file_path), @@ -1049,6 +1042,7 @@ mod tests { default_env: &[], supports_acp_native_config: false, thinking_env_var: None, + effort_normalization: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, @@ -1241,6 +1235,7 @@ mod tests { default_env: &[], supports_acp_native_config: false, thinking_env_var: None, + effort_normalization: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, @@ -1739,3 +1734,8 @@ mod tests { #[cfg(test)] #[path = "readiness_goose_file_config_tests.rs"] mod goose_file_config_tests; + +// Phase 3 effort-bridge spawn tests live in a sibling file. +#[cfg(test)] +#[path = "readiness_effort_bridge_tests.rs"] +mod effort_bridge_tests; diff --git a/desktop/src-tauri/src/managed_agents/readiness_effort_bridge_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_effort_bridge_tests.rs new file mode 100644 index 0000000000..fbe94baebc --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_effort_bridge_tests.rs @@ -0,0 +1,364 @@ +//! Spawn-side legacy effort bridge tests (Phase 3). +//! +//! Tests for `resolve_effective_agent_env` when the runtime has `effort_normalization`. +//! Verifies: tier-first precedence, alias normalization, foreign-key stripping, +//! invalid-value skip-as-absent, and global-scope invariants. +//! +//! Included from `readiness.rs` via `#[path]`; `super::*` resolves against that module. + +use std::collections::BTreeMap; + +use super::*; +use crate::managed_agents::discovery::known_acp_runtime_exact; + +fn goose_runtime() -> Option<&'static KnownAcpRuntime> { + known_acp_runtime_exact("goose") +} + +fn empty_global() -> crate::managed_agents::global_config::GlobalAgentConfig { + Default::default() +} + +fn make_record( + env_vars: BTreeMap, +) -> crate::managed_agents::types::ManagedAgentRecord { + crate::managed_agents::types::ManagedAgentRecord { + pubkey: "test-pubkey".to_string(), + name: "test-agent".to_string(), + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: "goose-acp".to_string(), + agent_command: "goose".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars, + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + } +} + +fn make_persona( + id: &str, + env_vars: BTreeMap, +) -> crate::managed_agents::types::AgentDefinition { + serde_json::from_value(serde_json::json!({ + "id": id, + "display_name": "test-persona", + "system_prompt": "", + "env_vars": env_vars, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + })) + .unwrap() +} + +fn env_with(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} + +// ── Bridge activation ─────────────────────────────────────────────────────── + +#[test] +fn goose_record_native_effort_is_translated_and_foreign_key_stripped() { + // Record has GOOSE_THINKING_EFFORT — bridge uses it. No legacy key present. + let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "high")])); + let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global()); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("high") + ); + // No legacy key in output. + assert!(!effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT")); +} + +#[test] +fn goose_record_legacy_effort_migrated_to_native_key() { + // Record has only BUZZ_AGENT_THINKING_EFFORT — bridge translates it. + let record = make_record(env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "medium")])); + let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global()); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("medium"), + "legacy key must be translated to GOOSE_THINKING_EFFORT" + ); + assert!( + !effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"), + "legacy key must be stripped from the descriptor" + ); +} + +// ── Tier-first precedence ──────────────────────────────────────────────────── + +#[test] +fn goose_record_native_beats_persona_native() { + // record GOOSE_THINKING_EFFORT=high, persona GOOSE_THINKING_EFFORT=low → high wins. + let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "high")])); + let persona = make_persona("p1", env_with(&[("GOOSE_THINKING_EFFORT", "low")])); + let mut record_with_persona = record; + record_with_persona.persona_id = Some("p1".to_string()); + let effective = resolve_effective_agent_env( + &record_with_persona, + &[persona], + goose_runtime(), + &empty_global(), + ); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("high") + ); +} + +#[test] +fn goose_record_legacy_beats_persona_native_tier_first() { + // record BUZZ_AGENT_THINKING_EFFORT=high, persona GOOSE_THINKING_EFFORT=low + // → tier-first: record legacy wins over persona native. + let record = make_record(env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "high")])); + let persona = make_persona("p1", env_with(&[("GOOSE_THINKING_EFFORT", "low")])); + let mut r2 = record; + r2.persona_id = Some("p1".to_string()); + let effective = resolve_effective_agent_env(&r2, &[persona], goose_runtime(), &empty_global()); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("high"), + "record legacy must beat persona native under tier-first rule" + ); +} + +#[test] +fn goose_global_native_excluded_from_legacy_fallback() { + // Only global has BUZZ_AGENT_THINKING_EFFORT (legacy at global tier). + // Global legacy is excluded end-to-end → no effort key in output. + let mut global = empty_global(); + global + .env_vars + .insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "low".to_string()); + let record = make_record(BTreeMap::new()); + let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &global); + assert!( + !effective.env.contains_key("GOOSE_THINKING_EFFORT"), + "global legacy must not seed Goose effort (excluded end-to-end)" + ); + assert!( + !effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"), + "global legacy must be stripped from Goose descriptor" + ); +} + +// ── Alias normalization ────────────────────────────────────────────────────── + +#[test] +fn goose_alias_none_is_normalized_to_off() { + let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "none")])); + let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global()); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("off"), + "none→off alias normalization" + ); +} + +#[test] +fn goose_alias_xhigh_is_normalized_to_max() { + let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "xhigh")])); + let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global()); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("max"), + "xhigh→max alias normalization" + ); +} + +#[test] +fn goose_invalid_value_skip_as_absent_allows_lower_tier_to_win() { + // Record has invalid "minimal" — skipped. Persona native "low" wins. + let record = make_record(env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "minimal")])); + let persona = make_persona("p1", env_with(&[("GOOSE_THINKING_EFFORT", "low")])); + let mut r2 = record; + r2.persona_id = Some("p1".to_string()); + let effective = resolve_effective_agent_env(&r2, &[persona], goose_runtime(), &empty_global()); + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("low"), + "invalid record legacy must be skipped, persona native wins" + ); +} + +// ── Foreign-key stripping (global-scope coexist invariant) ───────────────── + +#[test] +fn buzz_agent_global_effort_survives_when_goose_record_effort_migrated() { + // Global config has both BUZZ_AGENT_THINKING_EFFORT (buzz-agent's native key) + // and GOOSE_THINKING_EFFORT (Goose's native key). A Goose agent's effective + // descriptor must strip BUZZ_AGENT_THINKING_EFFORT (foreign to Goose) while + // honouring GOOSE_THINKING_EFFORT from the global tier. + // + // Note: global GOOSE_THINKING_EFFORT is treated as a global-native tier (not + // legacy), so it IS honoured. Global legacy (BUZZ_AGENT_THINKING_EFFORT for + // Goose) is excluded. Only the native key appears in the Goose descriptor. + let mut global = empty_global(); + global + .env_vars + .insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string()); + global + .env_vars + .insert("GOOSE_THINKING_EFFORT".to_string(), "low".to_string()); + let record = make_record(BTreeMap::new()); + let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &global); + // Global native (GOOSE_THINKING_EFFORT=low) wins. + assert_eq!( + effective + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("low"), + "global native GOOSE_THINKING_EFFORT must reach Goose descriptor" + ); + // Foreign key must be stripped from Goose descriptor. + assert!( + !effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"), + "BUZZ_AGENT_THINKING_EFFORT must be stripped from Goose descriptor" + ); +} + +// ── Bidirectional global coexist: each runtime sees only its own key ──────── + +#[test] +fn buzz_agent_global_effort_descriptor_strips_goose_native_key() { + // Both BUZZ_AGENT_THINKING_EFFORT and GOOSE_THINKING_EFFORT are in global + // config. A buzz-agent descriptor must contain only BUZZ_AGENT_THINKING_EFFORT; + // GOOSE_THINKING_EFFORT (foreign to buzz-agent) must be stripped. + // Paired with buzz_agent_global_effort_survives_when_goose_record_effort_migrated + // to assert the global coexist invariant from both directions. + use crate::managed_agents::discovery::known_acp_runtime_exact; + let buzz_agent_runtime = known_acp_runtime_exact("buzz-agent"); + let mut global = empty_global(); + global + .env_vars + .insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string()); + global + .env_vars + .insert("GOOSE_THINKING_EFFORT".to_string(), "low".to_string()); + let record = make_record(std::collections::BTreeMap::new()); + let effective = resolve_effective_agent_env(&record, &[], buzz_agent_runtime, &global); + // buzz-agent's native key must be present. + assert_eq!( + effective + .env + .get("BUZZ_AGENT_THINKING_EFFORT") + .map(String::as_str), + Some("high"), + "buzz-agent native key must survive in global coexist scenario" + ); + // Foreign key must be stripped. + assert!( + !effective.env.contains_key("GOOSE_THINKING_EFFORT"), + "GOOSE_THINKING_EFFORT must be stripped from buzz-agent descriptor" + ); +} + +#[test] +fn global_coexist_both_runtimes_retain_own_key_independently() { + // Using the same global config, build descriptors for both Goose and buzz-agent + // and verify each sees only its own key. Proves the invariant is symmetric. + use crate::managed_agents::discovery::known_acp_runtime_exact; + let goose = goose_runtime(); + let buzz = known_acp_runtime_exact("buzz-agent"); + let mut global = empty_global(); + global.env_vars.insert( + "BUZZ_AGENT_THINKING_EFFORT".to_string(), + "medium".to_string(), + ); + global + .env_vars + .insert("GOOSE_THINKING_EFFORT".to_string(), "high".to_string()); + let record = make_record(std::collections::BTreeMap::new()); + + let goose_eff = resolve_effective_agent_env(&record, &[], goose, &global); + let buzz_eff = resolve_effective_agent_env(&record, &[], buzz, &global); + + // Goose sees its own key. + assert_eq!( + goose_eff + .env + .get("GOOSE_THINKING_EFFORT") + .map(String::as_str), + Some("high") + ); + assert!(!goose_eff.env.contains_key("BUZZ_AGENT_THINKING_EFFORT")); + + // buzz-agent sees its own key. + assert_eq!( + buzz_eff + .env + .get("BUZZ_AGENT_THINKING_EFFORT") + .map(String::as_str), + Some("medium") + ); + assert!(!buzz_eff.env.contains_key("GOOSE_THINKING_EFFORT")); +} diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 255c1aae32..fb7690b3ff 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -642,6 +642,11 @@ pub struct AcpRuntimeCatalogEntry { pub provider_env_var: Option, /// Environment variable used to apply thinking effort, when supported. pub thinking_env_var: Option, + /// Canonical effort values (`off|low|medium|high|max` for Goose; `None` for per-model harnesses). + pub accepted_effort_values: Option>, + /// Effort aliases for TS normalization: `(alias, canonical)` pairs from + /// `EffortNormalization::aliases`. `None` when runtime has no static vocabulary. + pub effort_aliases: Option>, pub max_tokens_env_var: Option, pub context_limit_env_var: Option, pub max_rounds_env_var: Option, @@ -664,12 +669,7 @@ pub struct AcpRuntimeCatalogEntry { /// JSON file in `custom_harnesses/`. The UI uses this to decide editability. pub source: HarnessSource, /// Definition-level environment variables for `source: custom` entries. - /// - /// Populated from `HarnessDefinition.env` so the edit form can read them - /// back and the user doesn't silently lose env vars when saving. Always - /// empty for `builtin` and `preset` entries (those env values come from the - /// runtime metadata path, not user-editable JSON). - /// + /// Populated from `HarnessDefinition.env`; always empty for builtin/preset entries. /// Skipped in serialization when empty to keep the catalog payload compact. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub definition_env: BTreeMap, diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index f2eb7f285c..1ff73b861e 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -30,13 +30,16 @@ with a TypeScript lookup table or an id comparison in a component. belongs in `deriveAgentConfigFieldModel` (once, with a named reason), never in a component. Components ask the field model what exists (`hasRenderableAgentConfigField`, `getRenderableEffortField`). -2. **Effort reads/writes go through the descriptor.** Use the effort - descriptor's `currentPersistence` key — never a raw - `BUZZ_AGENT_THINKING_EFFORT` literal in UI code. `currentPersistence` is - where the value lives *today*; `targetApplication` is how the harness - *should* receive it. They intentionally differ until PR 2.7 migrates - Goose/Claude — do not "fix" one to match the other without doing the - migration work. +2. **Effort reads/writes go through the native persistence key.** For + harness-native runtimes (e.g. Goose), `runtime.thinkingEnvVar` is the + single persistence key. Use `resolveEffortFromEnv` for reads (normalizes, + applies native-first / valid-legacy-fallback at record/persona tiers) and + `applyHarnessNativeEffortChange` for writes. Legacy alias + (`BUZZ_AGENT_THINKING_EFFORT`) is consumed only at record and persona + tiers; definition and global tiers are native-key-only (plan v3 Delta 2). + For global/onboarding scope, pass `legacyEnvKey=null` to + `HarnessNativeEffortFields` so the write path never silently deletes the + legacy key at a scope where it must not be touched. 3. **Field absence has a named reason, not a boolean.** Codex effort is `ownedByModelId`; Claude effort is `deferredUntilNativeOptionsAvailable`. New absences get new named reasons in `AgentConfigOmission` / 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..38a659ae52 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,41 @@ 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, + effortAliases?: ReadonlyArray | 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, effortAliases) ?? null) + : null; + const legacyValue = legacyRaw + ? (normalizeEffortValue(legacyRaw, acceptedValues, effortAliases) ?? 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 +245,49 @@ 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); + const effortAliases = isBuzzAgent ? null : (runtime.effortAliases ?? 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, + effortAliases, + ); 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 +362,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[], @@ -348,3 +417,23 @@ export function numericTuningPlaceholder( ? `Inherit (${inheritedValue})` : "Inherit (agent default)"; } + +/** + * Return all known native thinking-effort env keys from the runtime catalog. + * + * Derived from `runtime.thinkingEnvVar` declarations rather than a manually + * maintained constant — adding a new runtime to the catalog automatically + * participates in foreign-key stripping and transition cleanup. + * + * Use this in place of the static `ALL_KNOWN_EFFORT_KEYS` constant wherever + * the runtime catalog is available. + */ +export function allKnownEffortKeys( + runtimes: readonly Pick[], +): string[] { + const keys = new Set(); + for (const rt of runtimes) { + if (rt.thinkingEnvVar) keys.add(rt.thinkingEnvVar); + } + return [...keys]; +} 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 (isHarnessNativeEffort) { + if (!effortPersistenceKey) return; + onConfigChange({ + ...config, + env_vars: applyHarnessNativeEffortChange( + config.env_vars, + effortPersistenceKey, + null, + value, + ), + }); + return; + } + const next = { ...config.env_vars }; if (value === "") { - if (effortPersistenceKey) - delete nextEnvVars[effortPersistenceKey]; + if (effortPersistenceKey) delete next[effortPersistenceKey]; + delete next[BUZZ_AGENT_THINKING_EFFORT]; } else { - if (effortPersistenceKey) - nextEnvVars[effortPersistenceKey] = value; + if (effortPersistenceKey) next[effortPersistenceKey] = value; + if (effortPersistenceKey !== BUZZ_AGENT_THINKING_EFFORT) + delete next[BUZZ_AGENT_THINKING_EFFORT]; } - onConfigChange({ ...config, env_vars: nextEnvVars }); + 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..c5eec7c3b0 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 { allKnownEffortKeys } from "../lib/agentConfigCore"; 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, ...allKnownEffortKeys(runtimes)] + : undefined, }), [ inheritHarness, @@ -366,9 +372,10 @@ export function AgentInstanceEditDialog({ linkedPersona?.model, envVars, inheritedEnvVars, + prospectiveRuntime?.thinkingEnvVar, + runtimes, ], ); - const { globalConfig, inheritedDefaults: { @@ -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 allKnownEffortKeys(runtimes)) 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..26bbb0c3c8 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,58 @@ 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; + const harnessNativeEffortAliases = + catalogStatus === "ready" ? (selectedRuntime?.effortAliases ?? null) : 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, + harnessNativeEffortAliases, + ).legacyConsumed; + }, [ + envVars, + harnessNativeEffort, + harnessNativeEffortKey, + harnessNativeEffortAliases, + ]); + // 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 +377,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..48c270fe01 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,63 @@ 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; + const harnessNativeEffortAliases = + catalogStatus === "ready" ? (selectedRuntime?.effortAliases ?? null) : 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, + harnessNativeEffortAliases, + ).legacyConsumed; + }, [ + envVars, + harnessNativeEffort, + harnessNativeEffortKey, + harnessNativeEffortAliases, + ]); + 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 +272,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..edbe7b2f58 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,57 @@ 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 alias pairs from `runtime.effortAliases` — the single normalization authority. + * Passed to `normalizeEffortValue` so the TS alias table derives from Rust metadata. + * `null`/`undefined` falls back to the built-in Goose alias table (backward compat). + */ + effortAliases?: ReadonlyArray | 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, + options?.effortAliases, + ) ?? 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 +244,30 @@ 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. + // Normalize through the same contract as global (alias→canonical, invalid→absent). + const baked = bakedEnv?.find( + (entry) => entry.key === bakedEffortKey && !entry.masked, + ); + const bakedRaw = baked?.value.trim() ?? ""; + const bakedValue = bakedRaw + ? (normalizeEffortValue( + bakedRaw, + options?.acceptedEffortValues ?? null, + options?.effortAliases, + ) ?? "") + : ""; + 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..fe81921b59 100644 --- a/desktop/src/features/agents/ui/buzzAgentConfig.ts +++ b/desktop/src/features/agents/ui/buzzAgentConfig.ts @@ -8,6 +8,44 @@ /** Env var key for the thinking/effort level sent to the LLM. */ export const BUZZ_AGENT_THINKING_EFFORT = "BUZZ_AGENT_THINKING_EFFORT"; +/** + * 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. + * + * Alias resolution uses the runtime-supplied `effortAliases` descriptor when + * provided (single source of truth from Rust `EffortNormalization::aliases`). + * Falls back to a built-in table for backward compatibility with fixtures that + * predate the IPC field. + * + * Pass `null` for `acceptedValues` to skip normalization (buzz-agent path). + */ +export function normalizeEffortValue( + raw: string, + acceptedValues: readonly string[] | null, + effortAliases?: ReadonlyArray | 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; + // Alias resolution: use the descriptor-supplied table when available, otherwise + // fall back to the built-in Goose alias table (for older fixtures/tests). + const aliasTable: ReadonlyArray = + effortAliases ?? + // Built-in fallback: mirrors GOOSE_EFFORT_NORMALIZATION in runtime_metadata.rs. + ([ + ["none", "off"], + ["disabled", "off"], + ["med", "medium"], + ["xhigh", "max"], + ] as const); + for (const [alias, canon] of aliasTable) { + if (lower === alias && acceptedValues.includes(canon)) return canon; + } + 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..3ce9b49cbb 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,171 @@ 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, + effortAliases, + envVars, + inheritedEnvVars, + legacyEnvKey, + nativeEffortKey, + onEnvVarsChange, +}: { + /** Ordered canonical effort values from `runtime.acceptedEffortValues`. */ + acceptedEffortValues: readonly string[]; + /** + * Effort alias pairs from `runtime.effortAliases` — the single normalization authority. + * Passed to `resolveEffortFromEnv` for canonical alias resolution. Falls back to the + * built-in table when absent (backward compat for call sites without catalog access). + */ + effortAliases?: ReadonlyArray | null; + 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 `BUZZ_AGENT_THINKING_EFFORT` at record/persona scope (definition/instance); + * pass `null` at global/onboarding scope to enforce the tier boundary and prevent + * the write path from silently deleting the legacy key. + */ + legacyEnvKey: string | null; + /** 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 non-null). This guarantees the component and + // deriveAgentConfigFieldModel agree on what value to display at any scope. + const { value: currentEffort } = resolveEffortFromEnv( + envVars, + nativeEffortKey, + legacyEnvKey, + acceptedEffortValues, + effortAliases, + ); + + // 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. + // At global/onboarding scope legacyEnvKey is null — the write path skips deletion. + onEnvVarsChange( + applyHarnessNativeEffortChange( + envVars, + nativeEffortKey, + legacyEnvKey, + 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..210ad0bf57 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,391 @@ 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", + ); +}); + +// ── Baked effort: alias normalization and invalid-value skip ────────────── +// +// The baked tier must normalize values through the same contract as every +// other tier — xhigh→max and invalid values produce absent inherited effort. +// Paired with Rust `baked_xhigh_normalizes_to_max` and `baked_invalid_minimal_skipped` +// in config_bridge/mod.rs which verify the spawn side; these verify the display side. + +test("baked_xhigh_normalizes_to_max_in_inherited_defaults", () => { + // Baked env has GOOSE_THINKING_EFFORT=xhigh (alias for max). + // getInheritedAgentDefaults must normalize it to canonical "max" before display. + const gooseAliases = [ + ["none", "off"], + ["disabled", "off"], + ["med", "medium"], + ["xhigh", "max"], + ]; + const defaults = getInheritedAgentDefaults( + { env_vars: {}, provider: null, model: null }, + [{ key: "GOOSE_THINKING_EFFORT", value: "xhigh", masked: false }], + { + nativeEffortKey: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + effortAliases: gooseAliases, + }, + ); + assert.deepEqual( + defaults.effort, + { source: "build", value: "max" }, + "baked xhigh alias must normalize to canonical max for display", + ); +}); + +test("baked_invalid_minimal_absent_in_inherited_defaults", () => { + // Baked env has GOOSE_THINKING_EFFORT=minimal (invalid for Goose static vocab). + // getInheritedAgentDefaults must skip it — UI shows no inherited effort value. + const gooseAliases = [ + ["none", "off"], + ["disabled", "off"], + ["med", "medium"], + ["xhigh", "max"], + ]; + const defaults = getInheritedAgentDefaults( + { env_vars: {}, provider: null, model: null }, + [{ key: "GOOSE_THINKING_EFFORT", value: "minimal", masked: false }], + { + nativeEffortKey: "GOOSE_THINKING_EFFORT", + acceptedEffortValues: ["off", "low", "medium", "high", "max"], + effortAliases: gooseAliases, + }, + ); + assert.deepEqual( + defaults.effort, + { source: null, value: "" }, + "baked invalid value must be skipped — no inherited effort displayed", + ); +}); + +// ── 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..d1843bbda5 100644 --- a/desktop/src/features/agents/ui/useAgentDialogDefaults.ts +++ b/desktop/src/features/agents/ui/useAgentDialogDefaults.ts @@ -2,28 +2,65 @@ 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, + effortAliases, }: { 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; + /** + * Effort alias pairs from `runtime.effortAliases` — the single normalization authority. + * Passed through to `getInheritedAgentDefaults` for canonical alias resolution. + * Falls back to the built-in table when absent (backward compat for older call sites). + */ + effortAliases?: ReadonlyArray | 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, + effortAliases: effortAliases ?? 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 bb56bc18e5..15d2332486 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"; @@ -158,8 +157,7 @@ export type RawManagedAgent = { auto_restart_on_config_change?: boolean; backend: ManagedAgentBackend; backend_agent_id: string | null; - // Optional: pre-feature mock 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[]; }; @@ -188,6 +186,8 @@ export type RawAcpRuntimeCatalogEntry = { model_env_var?: string | null; provider_env_var?: string | null; thinking_env_var?: string | null; + accepted_effort_values?: string[] | null; + effort_aliases?: Array<[string, string]> | null; max_tokens_env_var?: string | null; context_limit_env_var?: string | null; max_rounds_env_var?: string | null; @@ -749,6 +749,8 @@ 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, + effortAliases: entry.effort_aliases ?? null, maxTokensEnvVar: entry.max_tokens_env_var ?? null, contextLimitEnvVar: entry.context_limit_env_var ?? null, maxRoundsEnvVar: entry.max_rounds_env_var ?? null, @@ -761,8 +763,7 @@ export function fromRawAcpRuntimeCatalogEntry( authStatus: entry.auth_status, loginHint: entry.login_hint ?? null, source: entry.source, - // Map definition_env (snake_case from Rust) to definitionEnv (camelCase). - // Absent when empty (Rust serialization skips empty BTreeMap) — default to {}. + // Absent when empty (Rust skips empty BTreeMap) — default to {}. definitionEnv: entry.definition_env ?? {}, }; } @@ -1156,8 +1157,7 @@ export async function applyCommunity( }); } -// Validate a candidate repos dir without mutating the filesystem. Rejects -// with a human-readable reason; resolves for a valid or empty path. +// Validate a candidate repos dir (rejects with human-readable reason; resolves for a valid or empty path). export async function validateReposDir(dir: string): Promise { await invokeTauri("validate_repos_dir", { dir }); } diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index fd2c71bced..576314ed63 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -308,11 +308,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; `null` for buzz-agent. */ + acceptedEffortValues: string[] | null; + effortAliases: Array<[string, 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 b15358d9d3..49f697c96d 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -7331,6 +7331,23 @@ 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, + effort_aliases: + "effort_aliases" in runtime + ? runtime.effort_aliases + : runtime.id === "goose" + ? [ + ["none", "off"], + ["disabled", "off"], + ["med", "medium"], + ["xhigh", "max"], + ] + : null, }; }