diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 5519153578..01a9801489 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. diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 0eb024a86a..5801cb3055 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", @@ -165,6 +164,7 @@ pub async fn save_custom_harness( model_env_var: None, provider_env_var: None, thinking_env_var: None, + accepted_effort_values: 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/config_bridge/mod.rs b/desktop/src-tauri/src/managed_agents/config_bridge/mod.rs index f8b045fc72..536abbce04 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,63 @@ 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"; + +/// The set of all known native thinking-effort env keys across all runtimes. +/// Used to strip foreign effort keys from a runtime's effective descriptor. +/// Must stay in sync with `KnownAcpRuntime::thinking_env_var` declarations. +#[allow(dead_code)] // used in Phase 3 (spawn foreign-key stripping in readiness.rs) +pub(crate) const ALL_KNOWN_EFFORT_KEYS: &[&str] = &[ + LEGACY_THINKING_EFFORT_KEY, // buzz-agent native + "GOOSE_THINKING_EFFORT", // Goose native +]; + +/// 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) `global_tier` is false (legacy alias excluded from global tier), 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. +pub(crate) fn effort_tier_alias( + map: &std::collections::BTreeMap, + native_key: &str, + norm: impl Fn(&str) -> Option, + global_tier: 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 is not the global tier. + if !global_tier && 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 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..ea87446164 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 `global_tier=true` — legacy alias excluded. + // Definition env also uses `global_tier=true` semantics (no legacy alias there). - let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[ + // 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, false); + _per_o = effort_tier_alias(&tiers.persona_env, native, nf, false); + _glo_o = effort_tier_alias(&tiers.global_env, native, nf, true); + _def_o = effort_tier_alias(&tiers.definition_env, native, nf, true); + // 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(); + } + + // 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..3be03dfe2b 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,490 @@ 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 + ); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index fafcb2589d..dbb9a5bfb2 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -18,7 +18,9 @@ pub(crate) use presets::{ preset_harness_ids, }; use presets::{preset_catalog_entry, PRESET_HARNESSES}; -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"; @@ -104,6 +106,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, @@ -137,6 +140,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, @@ -170,6 +174,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, @@ -204,6 +209,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"), @@ -581,13 +587,9 @@ 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 compare adapter availability without +// spawning a probe subprocess on every poll cycle. fn adapter_availability_cache() -> &'static std::sync::Mutex> { use std::sync::{Mutex, OnceLock}; @@ -716,10 +718,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 { @@ -1390,6 +1389,9 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr model_env_var: runtime.model_env_var.map(str::to_string), provider_env_var: runtime.provider_env_var.map(str::to_string), thinking_env_var: runtime.thinking_env_var.map(str::to_string), + accepted_effort_values: runtime + .effort_normalization + .map(|n| n.canonical_values().iter().map(|s| s.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), @@ -1551,6 +1553,7 @@ pub fn discover_acp_runtimes_from( model_env_var: None, provider_env_var: None, thinking_env_var: None, + accepted_effort_values: 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/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index b2d8a14ef0..65e20ce172 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -67,6 +67,7 @@ pub(super) fn preset_catalog_entry( model_env_var: None, provider_env_var: None, thinking_env_var: None, + accepted_effort_values: 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 c072448ff1..740216acd6 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -70,8 +70,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"`). @@ -1049,6 +1047,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 +1240,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, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index c5bb6173d1..c2a54e4ba6 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -642,6 +642,8 @@ 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>, pub max_tokens_env_var: Option, pub context_limit_env_var: Option, pub max_rounds_env_var: Option, @@ -664,12 +666,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,