From 289b9a4937b9636d56a023edd664dbdb99212316 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 1 Jul 2026 18:37:10 -0400 Subject: [PATCH 01/16] feat(desktop): add global agent configuration defaults layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a global agent config record (global-agent-config.json) that applies to ALL managed agents as the lowest user-settable precedence layer. Per-agent and persona configs always win on collision. Precedence: baked build floor < global < persona < per-agent. - New global_config module: GlobalAgentConfig struct (env_vars, provider, model), load/save with atomic_write_json_restricted (0600), validate_global_config (strips empty values, rejects reserved and derived-provider-model keys), strip_empty_env_vars helper. - Tauri commands get_global_agent_config / set_global_agent_config with save-time validation; registered in invoke_handler. - ConfigOrigin::GlobalDefault added; resolve_config_surface re-tags injected global fields via retag_global_default (mirrors PersonaDefault pattern), so AgentConfigPanel shows 'Inherited from global defaults'. - Merge seams: resolve_effective_agent_env (readiness.rs — fixes readiness gate + nudge in one place), spawn_agent_child (runtime.rs — replaces empty lower-map with global env), build_deploy_payload (agents.rs — global < persona < agent for env and model/provider fallback chain). Global config is live-resolved at spawn/readiness/deploy — no delete+respawn required when the global record changes. - tauriGlobalAgentConfig.ts: getGlobalAgentConfig / setGlobalAgentConfig invokeTauri wrappers. - useGlobalAgentConfig hook: loads once on mount, fails safe (no global config is not an error state for callers). - GlobalAgentConfigSettingsCard: Settings screen card with provider picker (reuses getPersonaProviderOptions), model input, EnvVarsEditor, and save-with-feedback button. Added to Settings → Agents section. - computeLocalModeGate: accepts optional globalEnvVars param; keys present in global count as satisfied so Create dialog does not flag a key missing that global already provides. - CreateAgentDialog: passes globalConfig.env_vars as inheritedFrom to EnvVarsEditor (shows 'global defaults' hint row). - EditAgentDialog: merges global + persona into inheritedWithGlobal for the EnvVarsEditor inherited hint so both sources are visible. - AgentConfigPanel: provenanceSentence handles new GlobalDefault case. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 9 +- .../src-tauri/src/commands/agent_config.rs | 95 ++++++- desktop/src-tauri/src/commands/agents.rs | 33 ++- .../src/commands/global_agent_config.rs | 35 +++ desktop/src-tauri/src/commands/mod.rs | 2 + desktop/src-tauri/src/lib.rs | 2 + .../src/managed_agents/config_bridge/types.rs | 5 + .../src/managed_agents/global_config/mod.rs | 148 +++++++++++ .../src/managed_agents/global_config/tests.rs | 139 ++++++++++ desktop/src-tauri/src/managed_agents/mod.rs | 4 + .../src-tauri/src/managed_agents/readiness.rs | 53 +++- .../src-tauri/src/managed_agents/runtime.rs | 19 +- .../features/agents/ui/AgentConfigPanel.tsx | 2 + .../features/agents/ui/CreateAgentDialog.tsx | 4 + .../features/agents/ui/EditAgentDialog.tsx | 14 +- .../ui/createAgentLocalModeGate.test.mjs | 69 +++++ .../agents/ui/personaDialogPickers.tsx | 57 +++-- .../features/agents/useGlobalAgentConfig.ts | 46 ++++ .../ui/GlobalAgentConfigSettingsCard.tsx | 238 ++++++++++++++++++ .../features/settings/ui/SettingsPanels.tsx | 10 +- .../src/shared/api/tauriGlobalAgentConfig.ts | 25 ++ desktop/src/shared/api/types.ts | 20 ++ 22 files changed, 967 insertions(+), 62 deletions(-) create mode 100644 desktop/src-tauri/src/commands/global_agent_config.rs create mode 100644 desktop/src-tauri/src/managed_agents/global_config/mod.rs create mode 100644 desktop/src-tauri/src/managed_agents/global_config/tests.rs create mode 100644 desktop/src/features/agents/useGlobalAgentConfig.ts create mode 100644 desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx create mode 100644 desktop/src/shared/api/tauriGlobalAgentConfig.ts diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 93ae68ffa..e5cd02a8b 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -60,7 +60,10 @@ const overrides = new Map([ // config-bridge: get_agent_config_surface/write_agent_config_field/put_agent_session_config // commands add ~40 lines. Queued to split. // branch cut; override bumped to cover the merged total. Queued to split. - ["src-tauri/src/commands/agents.rs", 1437], + // global-agent-config: build_deploy_payload threads global config fallback + // for provider/model/env_vars (+4 lines). cargo fmt reflowed 2 more lines. + // Queued to split. + ["src-tauri/src/commands/agents.rs", 1443], // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS // const + build_thread_replies_filter helper, mirroring the channel sibling so // the two p-gate filters can't drift) plus two guard unit tests. The file was @@ -84,7 +87,9 @@ const overrides = new Map([ // activity-feed threads avatar_url into build_managed_agent_summary for the // assistant-bubble pinned snapshot. // +1 for agent_pubkey field in setup payload (config-nudge card wire). - ["src-tauri/src/managed_agents/runtime.rs", 2208], + // global-agent-config: spawn_agent_child loads global config and merges as + // lowest env layer (+8 lines). Queued to split. + ["src-tauri/src/managed_agents/runtime.rs", 2216], // config-bridge setup-payload env-boundary fix adds readiness wiring in // spawn_agent_child; load-bearing security fix, queued to split. ["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016], diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 59547764d..818dd4ca8 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -14,7 +14,8 @@ use crate::{ }, current_instance_id, effective_agent_command, known_acp_runtime, load_managed_agents, load_personas, resolve_effective_prompt_model_provider, save_managed_agents, - sync_managed_agent_processes, KnownAcpRuntime, ManagedAgentRecord, PersonaRecord, + sync_managed_agent_processes, GlobalAgentConfig, KnownAcpRuntime, ManagedAgentRecord, + PersonaRecord, }, }; @@ -36,13 +37,17 @@ pub struct RuntimeFileConfigSubset { pub satisfied_env_keys: Vec, } -/// Resolve the config surface with persona values applied. +/// Resolve the config surface with persona and global default values applied. /// /// The pipeline: resolve the linked persona's prompt/model/provider, inject /// each into the record only where the record lacks its own value, let /// `read_config_surface` tag those injected fields `BuzzExplicit`, then re-tag /// exactly the injected fields to `PersonaDefault`. /// +/// Global defaults fill in when neither the record nor the linked persona +/// provides a value. They are re-tagged to `GlobalDefault` so the UI can +/// display "inherited from global defaults". +/// /// The re-tag is triple-gated — a field is re-tagged only when (a) the record /// did not already have it (`!had_*`), (b) the surface produced the field, and /// (c) the reader tagged it `BuzzExplicit`. A value the user set explicitly in @@ -52,6 +57,7 @@ fn resolve_config_surface( personas: &[PersonaRecord], runtime_meta: Option<&KnownAcpRuntime>, session_cache: Option<&SessionConfigCache>, + global: &GlobalAgentConfig, ) -> RuntimeConfigSurface { let had_prompt = record.system_prompt.is_some() || record.env_vars.contains_key("BUZZ_ACP_SYSTEM_PROMPT"); @@ -109,6 +115,24 @@ fn resolve_config_surface( } } + // Inject global defaults where neither the record nor the persona had a value. + // Track injection so we can re-tag to GlobalDefault after the reader. + let inject_global_model = !had_model && record.model.is_none(); + let inject_global_provider = !had_provider + && !provider_env_key.is_empty() + && !record.env_vars.contains_key(provider_env_key); + + if inject_global_model { + record.model = global.model.clone(); + } + if inject_global_provider { + if let Some(ref gprov) = global.provider { + record + .env_vars + .insert(provider_env_key.to_string(), gprov.clone()); + } + } + let mut surface = read_config_surface( &record, runtime_meta, @@ -120,13 +144,21 @@ fn resolve_config_surface( if !had_prompt { retag_persona_default(&mut surface.normalized.system_prompt); } - if !had_model { + if !had_model && !inject_global_model { retag_persona_default(&mut surface.normalized.model); } - if !had_provider && !provider_env_key.is_empty() { + if !had_provider && !provider_env_key.is_empty() && !inject_global_provider { retag_persona_default(&mut surface.normalized.provider); } + // Re-tag global-sourced fields from BuzzExplicit to GlobalDefault. + if inject_global_model { + retag_global_default(&mut surface.normalized.model); + } + if inject_global_provider { + retag_global_default(&mut surface.normalized.provider); + } + // Re-tag persona-snapshotted model from BuzzExplicit to PersonaDefault. // Persona-created agents have record.model set at create time from the // persona snapshot — had_model is true, but the model came from the persona, @@ -184,6 +216,15 @@ pub fn get_runtime_file_config(runtime_id: String) -> Option) { + if let Some(field) = field { + if field.origin == ConfigOrigin::BuzzExplicit { + field.origin = ConfigOrigin::GlobalDefault; + } + } +} /// Get the full config surface for a managed agent. /// /// Returns normalized + advanced config from all available tiers. @@ -227,12 +268,14 @@ pub async fn get_agent_config_surface( ); let runtime_meta = known_acp_runtime(&effective_cmd); let session_cache = state.get_session_cache(&pubkey); + let global = crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); Ok(resolve_config_surface( record, &personas, runtime_meta, session_cache.as_ref(), + &global, )) } @@ -544,7 +587,13 @@ mod tests { 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); + let surface = 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")); @@ -565,8 +614,13 @@ mod tests { let personas: Vec = vec![]; let cache = session_cache("model-y", false); - let surface = - resolve_config_surface(record, &personas, Some(goose_runtime()), Some(&cache)); + let surface = 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")); @@ -588,8 +642,13 @@ mod tests { let personas: Vec = vec![]; let cache = session_cache("model-y", true); - let surface = - resolve_config_surface(record, &personas, Some(goose_runtime()), Some(&cache)); + let surface = 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")); @@ -610,8 +669,13 @@ mod tests { let personas: Vec = vec![]; let cache = session_cache("model-x", true); - let surface = - resolve_config_surface(record, &personas, Some(goose_runtime()), Some(&cache)); + let surface = 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")); @@ -629,8 +693,13 @@ mod tests { 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)); + let surface = 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")); diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index c7c21e6a8..f00f11676 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -315,14 +315,21 @@ fn build_deploy_payload( return Err(err); } - // Merge persona env_vars + agent env_vars for provider deploy. Provider + // Merge global + persona + agent env_vars for provider deploy. Provider // deploy re-reads live persona env vars so remote agents receive current // credentials; local spawn uses only pinned record.env_vars for determinism - // across restarts. Without this, provider-backed agents wouldn't receive - // credentials saved on the persona or the agent itself. + // across restarts. Global env vars are the lowest user-settable layer: + // global < persona < agent (last-wins on key collision). + let global_env = crate::managed_agents::load_global_agent_config(app) + .unwrap_or_default() + .env_vars; let persona_env = crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?; - let merged_env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars); + // Merge: global < persona (persona wins over global). + let global_persona_merged = crate::managed_agents::merged_user_env(&global_env, &persona_env); + // Merge: global+persona < agent (agent wins over everything). + let merged_env = + crate::managed_agents::merged_user_env(&global_persona_merged, &record.env_vars); // Resolve the persona's structured provider/model so the remote provider // receives the same authoritative values that local spawn derives from @@ -331,8 +338,9 @@ fn build_deploy_payload( // imported personas whose derived keys were filtered at import time). // // Precedence mirrors local spawn: persona structured model is authoritative - // when present; the agent record's `model` is a fallback for personas that - // don't specify one (or when no persona is linked). + // when present; the agent record's model is a fallback; global model is the + // last resort (when no persona or agent specifies one). + let global_config = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let (effective_model, effective_provider) = if let Some(pid) = record.persona_id.as_deref() { let personas = load_personas(app).map_err(|e| { format!( @@ -343,11 +351,18 @@ fn build_deploy_payload( .into_iter() .find(|p| p.id == pid) .ok_or_else(|| format!("persona `{pid}` not found while building deploy payload"))?; - let model = persona.model.clone().or(record.model.clone()); - let provider = persona.provider; + let model = persona + .model + .clone() + .or(record.model.clone()) + .or(global_config.model.clone()); + let provider = persona.provider.or(global_config.provider.clone()); (model, provider) } else { - (record.model.clone(), None) + ( + record.model.clone().or(global_config.model.clone()), + global_config.provider.clone(), + ) }; Ok(serde_json::json!({ diff --git a/desktop/src-tauri/src/commands/global_agent_config.rs b/desktop/src-tauri/src/commands/global_agent_config.rs new file mode 100644 index 000000000..e3277d104 --- /dev/null +++ b/desktop/src-tauri/src/commands/global_agent_config.rs @@ -0,0 +1,35 @@ +//! Tauri commands for global agent configuration defaults. +//! +//! `get_global_agent_config` / `set_global_agent_config` — simple load/save +//! around the `global_config` module with the standard save-time validation. + +use tauri::AppHandle; + +use crate::managed_agents::{ + load_global_agent_config, save_global_agent_config, validate_global_config, GlobalAgentConfig, +}; + +/// Read the current global agent configuration. +/// +/// Returns the default (empty) config if `global-agent-config.json` has not +/// been written yet. +#[tauri::command] +pub fn get_global_agent_config(app: AppHandle) -> Result { + load_global_agent_config(&app) +} + +/// Validate and persist a new global agent configuration. +/// +/// Strips empty env values before writing (empty = "inherit" semantics), then +/// applies standard validation: POSIX key shape, reserved-key reject, +/// derived-provider-model-key reject, NUL/size caps. +#[tauri::command] +pub fn set_global_agent_config( + config: GlobalAgentConfig, + app: AppHandle, +) -> Result { + validate_global_config(&config)?; + save_global_agent_config(&app, &config)?; + // Re-read from disk so the returned value reflects the strip-on-write pass. + load_global_agent_config(&app) +} diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 7fea4604a..1419d68d8 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -10,6 +10,7 @@ mod channels; mod dms; mod engrams; mod export_util; +mod global_agent_config; mod identity; mod identity_archive; mod legacy_storage; @@ -43,6 +44,7 @@ pub use channel_templates::*; pub use channels::*; pub use dms::*; pub use engrams::*; +pub use global_agent_config::*; pub use identity::*; pub use identity_archive::*; pub use legacy_storage::*; diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c43bcd146..c4c9b2cfe 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -527,6 +527,8 @@ pub fn run() { get_agent_config_surface, get_runtime_file_config, put_agent_session_config, + get_global_agent_config, + set_global_agent_config, mesh_availability, mesh_start_node, mesh_ensure_client_node, diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs index c201141df..17589bad5 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs @@ -21,6 +21,11 @@ pub enum ConfigOrigin { /// resolved before calling the reader, then the surface is post-processed to /// re-tag injected fields from `BuzzExplicit` to `PersonaDefault`. PersonaDefault, + /// Value inherited from global agent configuration defaults. + /// The lowest user-settable layer — active when neither the agent record nor + /// the linked persona specifies a value. Re-tagged from `BuzzExplicit` by the + /// `resolve_config_surface` call site, analogously to `PersonaDefault`. + GlobalDefault, /// Live runtime model override applied via the ModelPicker (Phase 3). /// The ACP session's current model diverges from the persona model because /// the user picked a different model on the running instance. Runtime-only — diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs new file mode 100644 index 000000000..56c02cbf2 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -0,0 +1,148 @@ +//! Global agent configuration defaults. +//! +//! A single `global-agent-config.json` record that applies to ALL managed +//! agents. Per-agent config always wins; global provides the lowest +//! user-settable layer below persona. +//! +//! # Precedence (low → high) +//! +//! ```text +//! baked build env < GLOBAL < persona < per-agent < Buzz-identity +//! ``` +//! +//! # Semantics +//! +//! Unlike per-agent/persona env (snapshotted at create time), global config is +//! **live-resolved at spawn/readiness/deploy** — change a global key and every +//! agent picks it up on the next restart, with no delete+respawn required. +//! +//! # Storage +//! +//! `/agents/global-agent-config.json`, written `0o600` via +//! `atomic_write_json_restricted` (same as the agent store). + +use std::collections::BTreeMap; + +use serde::{Deserialize, Serialize}; +use tauri::AppHandle; + +use crate::managed_agents::env_vars::{validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS}; +use crate::managed_agents::storage::{atomic_write_json_restricted, managed_agents_base_dir}; + +/// The global agent configuration record. +/// +/// Shape mirrors the per-agent/persona trio (`env_vars` + `provider` + `model`) +/// so the config vocabulary is consistent across all three tiers. +/// +/// `env_vars` is the lowest user-settable env layer — global < persona < agent. +/// `provider` / `model` are fallback defaults: effective provider/model = +/// `agent → persona → global → None`. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct GlobalAgentConfig { + /// Global env vars injected into ALL agents unconditionally. + /// + /// Lowest user-settable layer — per-agent and persona values win on any + /// key collision. Reserved and derived keys are rejected at save time and + /// stripped at spawn time. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub env_vars: BTreeMap, + + /// Global fallback provider (e.g. `"databricks_v2"`, `"anthropic"`). + /// + /// Used only when neither the agent record nor the linked persona specifies + /// a provider. `None` = no global default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider: Option, + + /// Global fallback model identifier. + /// + /// Used only when neither the agent record nor the linked persona specifies + /// a model. `None` = no global default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub model: Option, +} + +/// Validate a `GlobalAgentConfig` before persisting it. +/// +/// Rules beyond `validate_user_env_keys`: +/// - `DERIVED_PROVIDER_MODEL_ENV_KEYS` (`GOOSE_PROVIDER`, `GOOSE_MODEL`, …) +/// must NOT be set as global env vars — they would shadow the structured +/// `provider`/`model` fields and break provider/model resolution. Users +/// must use the structured fields instead. +/// - Empty per-key values are stripped before validation so a caller that +/// passes `KEY=""` does not accidentally shadow a real global value. +pub fn validate_global_config(config: &GlobalAgentConfig) -> Result<(), String> { + // Strip empty values first — they mean "inherit" and must not be stored. + let non_empty: BTreeMap = config + .env_vars + .iter() + .filter(|(_, v)| !v.is_empty()) + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); + + // Standard env-var key validation (POSIX shape, reserved-key check, NUL/size caps). + validate_user_env_keys(&non_empty)?; + + // Reject derived provider/model keys in global env_vars. + let derived: Vec<&str> = non_empty + .keys() + .filter(|k| { + DERIVED_PROVIDER_MODEL_ENV_KEYS + .iter() + .any(|d| d.eq_ignore_ascii_case(k.as_str())) + }) + .map(String::as_str) + .collect(); + if !derived.is_empty() { + return Err(format!( + "the following keys must be set via the structured provider/model fields, \ + not as env vars: {}", + derived.join(", ") + )); + } + + Ok(()) +} + +/// Strip empty values from `env_vars`. +/// +/// Empty per-agent/persona values mean "no value"; if stored they would shadow +/// a real global default with an empty string. Strip them at save time so a +/// caller that clears a row cannot accidentally shadow global. +pub fn strip_empty_env_vars(config: &mut GlobalAgentConfig) { + config.env_vars.retain(|_, v| !v.is_empty()); +} + +fn global_config_path(app: &AppHandle) -> Result { + Ok(managed_agents_base_dir(app)?.join("global-agent-config.json")) +} + +/// Load the global agent config from disk. +/// +/// Returns the default (all-empty) config if the file does not exist yet. +pub fn load_global_agent_config(app: &AppHandle) -> Result { + let path = global_config_path(app)?; + if !path.exists() { + return Ok(GlobalAgentConfig::default()); + } + let content = std::fs::read_to_string(&path) + .map_err(|e| format!("failed to read global agent config: {e}"))?; + serde_json::from_str(&content).map_err(|e| format!("failed to parse global agent config: {e}")) +} + +/// Save the global agent config to disk. +/// +/// Strips empty env values before writing (empty = "inherit" semantics). +/// Written `0o600` — same protection as `managed-agents.json`. +pub fn save_global_agent_config(app: &AppHandle, config: &GlobalAgentConfig) -> Result<(), String> { + let mut config = config.clone(); + strip_empty_env_vars(&mut config); + + let path = global_config_path(app)?; + let payload = serde_json::to_vec_pretty(&config) + .map_err(|e| format!("failed to serialize global agent config: {e}"))?; + atomic_write_json_restricted(&path, &payload) +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs new file mode 100644 index 000000000..381b5b80d --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -0,0 +1,139 @@ +use std::collections::BTreeMap; + +use super::{strip_empty_env_vars, validate_global_config, GlobalAgentConfig}; + +fn config_with_env(pairs: &[(&str, &str)]) -> GlobalAgentConfig { + GlobalAgentConfig { + env_vars: pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + ..Default::default() + } +} + +// ── validate_global_config ──────────────────────────────────────────────────── + +#[test] +fn validate_accepts_valid_env_vars() { + let config = config_with_env(&[("ANTHROPIC_API_KEY", "sk-test"), ("MY_CUSTOM_KEY", "value")]); + assert!(validate_global_config(&config).is_ok()); +} + +#[test] +fn validate_rejects_reserved_key() { + let config = config_with_env(&[("BUZZ_PRIVATE_KEY", "should-not-be-settable")]); + let err = validate_global_config(&config).unwrap_err(); + assert!( + err.contains("reserved"), + "expected reserved-key error, got: {err}" + ); +} + +#[test] +fn validate_rejects_derived_provider_model_key_goose_provider() { + let config = config_with_env(&[("GOOSE_PROVIDER", "anthropic")]); + let err = validate_global_config(&config).unwrap_err(); + assert!( + err.contains("structured provider/model fields"), + "expected derived-key error, got: {err}" + ); +} + +#[test] +fn validate_rejects_derived_key_goose_model() { + let config = config_with_env(&[("GOOSE_MODEL", "claude-opus-4")]); + let err = validate_global_config(&config).unwrap_err(); + assert!( + err.contains("structured provider/model fields"), + "got: {err}" + ); +} + +#[test] +fn validate_rejects_derived_key_buzz_agent_provider() { + let config = config_with_env(&[("BUZZ_AGENT_PROVIDER", "anthropic")]); + let err = validate_global_config(&config).unwrap_err(); + assert!( + err.contains("structured provider/model fields"), + "got: {err}" + ); +} + +#[test] +fn validate_rejects_malformed_key() { + let config = config_with_env(&[("has spaces", "val")]); + let err = validate_global_config(&config).unwrap_err(); + assert!( + err.contains("must match"), + "expected malformed-key error, got: {err}" + ); +} + +#[test] +fn validate_ignores_empty_values_for_reserved_key_check() { + // A reserved key with an EMPTY value is a no-op (stripped at save time). + // validate_global_config skips empty-value entries so it does not reject + // an empty clear for a key that happens to share a name with a reserved key. + let config = config_with_env(&[("BUZZ_PRIVATE_KEY", "")]); + // Strip is done inside validate — empty values are stripped before checking. + assert!( + validate_global_config(&config).is_ok(), + "empty value for reserved key should be treated as unset" + ); +} + +// ── strip_empty_env_vars ────────────────────────────────────────────────────── + +#[test] +fn strip_removes_empty_values_only() { + let mut config = config_with_env(&[("KEY_A", "value"), ("KEY_B", ""), ("KEY_C", "other")]); + strip_empty_env_vars(&mut config); + assert_eq!(config.env_vars.len(), 2); + assert!(config.env_vars.contains_key("KEY_A")); + assert!( + !config.env_vars.contains_key("KEY_B"), + "empty value must be stripped" + ); + assert!(config.env_vars.contains_key("KEY_C")); +} + +#[test] +fn strip_is_idempotent_on_all_non_empty() { + let mut config = config_with_env(&[("KEY_A", "v1"), ("KEY_B", "v2")]); + let original = config.env_vars.clone(); + strip_empty_env_vars(&mut config); + assert_eq!(config.env_vars, original); +} + +// ── GlobalAgentConfig defaults ──────────────────────────────────────────────── + +#[test] +fn default_config_is_all_none_empty() { + let config = GlobalAgentConfig::default(); + assert!(config.env_vars.is_empty()); + assert!(config.provider.is_none()); + assert!(config.model.is_none()); +} + +#[test] +fn roundtrip_serialization() { + let config = GlobalAgentConfig { + env_vars: BTreeMap::from([("ANTHROPIC_API_KEY".to_string(), "sk-test".to_string())]), + provider: Some("anthropic".to_string()), + model: Some("claude-opus-4".to_string()), + }; + let json = serde_json::to_string(&config).expect("serialize"); + let back: GlobalAgentConfig = serde_json::from_str(&json).expect("deserialize"); + assert_eq!(config, back); +} + +#[test] +fn empty_env_vars_omitted_from_serialization() { + let config = GlobalAgentConfig::default(); + let json = serde_json::to_string(&config).expect("serialize"); + // With all-default/empty, the JSON should be compact. + assert!(!json.contains("env_vars"), "empty env_vars must be omitted"); + assert!(!json.contains("provider"), "None provider must be omitted"); + assert!(!json.contains("model"), "None model must be omitted"); +} diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 06d8a8bc7..2447e9481 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -5,6 +5,7 @@ mod backend; pub(crate) mod config_bridge; mod discovery; mod env_vars; +pub(crate) mod global_config; mod nest; mod persona_avatars; mod persona_card; @@ -28,6 +29,9 @@ mod types; pub use backend::*; pub use discovery::*; pub use env_vars::*; +pub(crate) use global_config::{ + load_global_agent_config, save_global_agent_config, validate_global_config, GlobalAgentConfig, +}; pub use nest::*; pub use persona_card::*; pub use personas::*; diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 5b60914b2..7572cabcc 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -47,6 +47,7 @@ use crate::managed_agents::{ discovery::{known_acp_runtime, KnownAcpRuntime}, effective_agent_command, env_vars::merged_user_env, + global_config::GlobalAgentConfig, types::{ManagedAgentRecord, PersonaRecord}, }; @@ -71,17 +72,21 @@ pub(crate) struct EffectiveAgentEnv { pub effective_command: String, } -/// Assemble the effective agent env from a record, personas, and optional -/// known-runtime metadata — without an `AppHandle` so it is unit-testable. +/// Assemble the effective agent env from a record, personas, optional +/// known-runtime metadata, and the global agent config defaults — without an +/// `AppHandle` so it is fully unit-testable. /// /// # Arguments /// * `record` — the managed agent record (model/provider/env_vars/…) /// * `personas` — all current persona records (for persona-backed resolution) /// * `runtime` — the `KnownAcpRuntime` for the effective command, if any +/// * `global` — global agent config defaults (lowest user layer; pass +/// `&GlobalAgentConfig::default()` in tests that don't need global config) pub(crate) fn resolve_effective_agent_env( record: &ManagedAgentRecord, personas: &[PersonaRecord], runtime: Option<&KnownAcpRuntime>, + global: &GlobalAgentConfig, ) -> EffectiveAgentEnv { let effective_command = effective_agent_command( record.persona_id.as_deref(), @@ -93,9 +98,34 @@ pub(crate) fn resolve_effective_agent_env( let mut env = baked_build_env(); // Layer 2: runtime metadata env vars (model / provider keys derived from - // the record's structured fields). - let effective_model = record.model.as_deref(); - let effective_provider = record.provider.as_deref(); + // the record's structured fields, with global as fallback). + // + // Structured-field fallback: effective provider/model = agent → persona → global → None. + // This means a global provider/model is used by readiness evaluation and spawn + // when neither the agent record nor the linked persona specifies one. + let persona_model = record.persona_id.as_deref().and_then(|pid| { + personas + .iter() + .find(|p| p.id == pid) + .and_then(|p| p.model.clone()) + }); + let persona_provider = record.persona_id.as_deref().and_then(|pid| { + personas + .iter() + .find(|p| p.id == pid) + .and_then(|p| p.provider.clone()) + }); + let effective_model = record + .model + .as_deref() + .or(persona_model.as_deref()) + .or(global.model.as_deref()); + let effective_provider = record + .provider + .as_deref() + .or(persona_provider.as_deref()) + .or(global.provider.as_deref()); + if let Some(rt) = runtime { for (key, value) in super::runtime::runtime_metadata_env_vars( rt.model_env_var, @@ -108,8 +138,15 @@ pub(crate) fn resolve_effective_agent_env( } } - // Layer 3: merged user env (agent env_vars after reserved/malformed-key - // filtering; last-wins on collision). + // 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. + let global_env = merged_user_env(&BTreeMap::new(), &global.env_vars); + env.extend(global_env); + + // Layer 3b: merged user env (agent env_vars after reserved/malformed-key + // filtering; last-wins on collision, so agent beats global). let user_env = merged_user_env(&BTreeMap::new(), &record.env_vars); env.extend(user_env); @@ -943,7 +980,7 @@ mod tests { }; let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); // User env_vars must be present in the output (last-write-wins). assert_eq!( diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index dbf3bc230..91a7ad51f 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1768,7 +1768,8 @@ pub fn spawn_agent_child( agent_readiness, resolve_effective_agent_env, AgentReadiness, Requirement, }; - let effective = resolve_effective_agent_env(record, &personas, runtime_meta); + let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + let effective = resolve_effective_agent_env(record, &personas, runtime_meta, &global); // Compute the optional payload before touching the command. let setup_payload_json = if let AgentReadiness::NotReady { requirements } = agent_readiness(&effective) { @@ -1969,18 +1970,22 @@ pub fn spawn_agent_child( // (snapshotted at create) already merged under the agent's own overrides. // We read it directly and never look up the live persona, so credential // edits on the persona reach the agent only via delete+respawn (which - // rewrites the snapshot), not on a plain restart. `merged_user_env` with an - // empty persona map still applies the reserved-key / malformed-key / NUL - // filtering as defense-in-depth for older on-disk records. + // rewrites the snapshot), not on a plain restart. `merged_user_env` with a + // global-config lower map applies reserved-key / malformed-key / NUL + // filtering to both layers as defense-in-depth. + // + // Precedence: baked floor < Buzz-set env above < GLOBAL < per-agent. + // Global is the lower-precedence map; agent env_vars win on collision. // // These writes go LAST so user-provided values win over every Buzz-set env // above — EXCEPT reserved keys (BUZZ_PRIVATE_KEY, NOSTR_PRIVATE_KEY, // BUZZ_AUTH_TAG, BUZZ_API_TOKEN, BUZZ_ACP_PRIVATE_KEY, BUZZ_ACP_API_TOKEN), // which `merged_user_env` strips. Those carry Buzz's identity and must // never be GUI-overridable. - for (key, value) in - super::env_vars::merged_user_env(&std::collections::BTreeMap::new(), &record.env_vars) - { + let global_env_for_spawn = crate::managed_agents::load_global_agent_config(app) + .unwrap_or_default() + .env_vars; + for (key, value) in super::env_vars::merged_user_env(&global_env_for_spawn, &record.env_vars) { command.env(key, value); } diff --git a/desktop/src/features/agents/ui/AgentConfigPanel.tsx b/desktop/src/features/agents/ui/AgentConfigPanel.tsx index 7492d7893..2e988a349 100644 --- a/desktop/src/features/agents/ui/AgentConfigPanel.tsx +++ b/desktop/src/features/agents/ui/AgentConfigPanel.tsx @@ -133,6 +133,8 @@ function provenanceSentence( case "acpConfigOption": case "acpNativeRead": return "From ACP session"; + case "globalDefault": + return "Inherited from global defaults"; } } diff --git a/desktop/src/features/agents/ui/CreateAgentDialog.tsx b/desktop/src/features/agents/ui/CreateAgentDialog.tsx index 3f31d9715..1ce917d47 100644 --- a/desktop/src/features/agents/ui/CreateAgentDialog.tsx +++ b/desktop/src/features/agents/ui/CreateAgentDialog.tsx @@ -54,6 +54,7 @@ import { AgentProviderField, } from "./personaProviderModelFields"; import { usePersonaModelDiscovery } from "./usePersonaModelDiscovery"; +import { useGlobalAgentConfig } from "@/features/agents/useGlobalAgentConfig"; export function CreateAgentDialog({ open, @@ -69,6 +70,7 @@ export function CreateAgentDialog({ const allProvidersQuery = useAcpRuntimesQuery({ enabled: open }); const backendProvidersQuery = useBackendProvidersQuery({ enabled: open }); const { lastRuntimeId, setLastRuntime } = useLastRuntime(); + const { globalConfig } = useGlobalAgentConfig(); const [acpCommand, setAcpCommand] = React.useState("buzz-acp"); const [agentCommand, setAgentCommand] = React.useState("buzz-agent"); const [agentArgs, setAgentArgs] = React.useState("acp"); @@ -836,6 +838,8 @@ export function CreateAgentDialog({ disabled={createMutation.isPending} fileSatisfiedKeys={fileSatisfiedEnvKeys} helperText="Injected at spawn. Overrides the persona's env vars on collision." + inheritedFrom={globalConfig.env_vars} + inheritedLabel="global defaults" onChange={setEnvVars} requiredKeys={requiredEnvKeys} value={envVars} diff --git a/desktop/src/features/agents/ui/EditAgentDialog.tsx b/desktop/src/features/agents/ui/EditAgentDialog.tsx index e4155a5be..aa3298dce 100644 --- a/desktop/src/features/agents/ui/EditAgentDialog.tsx +++ b/desktop/src/features/agents/ui/EditAgentDialog.tsx @@ -45,6 +45,7 @@ import { import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor"; import { CreateAgentRespondToField } from "./RespondToField"; import { usePersonaModelDiscovery } from "./usePersonaModelDiscovery"; +import { useGlobalAgentConfig } from "@/features/agents/useGlobalAgentConfig"; export function EditAgentDialog({ agent, @@ -324,6 +325,15 @@ export function EditAgentDialog({ selectedRuntime, }); + const { globalConfig } = useGlobalAgentConfig(); + + // Merge global + persona env for the inherited display hint in EnvVarsEditor. + // Persona wins over global on collision (higher precedence), so persona keys + // shadow global for display consistency. + const inheritedWithGlobal = React.useMemo(() => { + return { ...globalConfig.env_vars, ...inheritedEnvVars }; + }, [globalConfig.env_vars, inheritedEnvVars]); + // When the provider scope changes and the current model is no longer valid // for the new scope, clear it (mirrors Persona's useEffect for the same). React.useEffect(() => { @@ -756,8 +766,8 @@ export function EditAgentDialog({ disabled={updateMutation.isPending} fileSatisfiedKeys={fileSatisfiedEnvKeys} helperText="Per-agent env vars. Override the persona's vars on collision." - inheritedFrom={inheritedEnvVars} - inheritedLabel="persona" + inheritedFrom={inheritedWithGlobal} + inheritedLabel="persona / global defaults" onChange={setEnvVars} requiredKeys={requiredEnvKeys} value={envVars} diff --git a/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs b/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs index e8e24370e..a1d471d1e 100644 --- a/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs +++ b/desktop/src/features/agents/ui/createAgentLocalModeGate.test.mjs @@ -410,3 +410,72 @@ test("localMode_goose_envPlusFileConfig_bothEmpty_stillRequired", () => { ); assert.equal(result.satisfied, false, "gate must not be satisfied"); }); + +// ── Global env vars satisfy required credential keys ───────────────────── + +test("localMode_globalEnvVars_satisfies_missing_env_key", () => { + // A required key present in globalEnvVars must not appear in missingEnvKeys. + const result = computeLocalModeGate({ + envVars: {}, + globalEnvVars: { ANTHROPIC_API_KEY: "sk-global" }, + isProviderMode: false, + model: "claude-3-5-sonnet-20241022", + provider: "anthropic", + runtimeId: "buzz-agent", + useMesh: false, + }); + + assert.equal( + result.satisfied, + true, + "global ANTHROPIC_API_KEY must satisfy the gate", + ); + assert.equal( + result.missingEnvKeys.includes("ANTHROPIC_API_KEY"), + false, + "ANTHROPIC_API_KEY in globalEnvVars must not appear in missingEnvKeys", + ); +}); + +test("localMode_perAgentEnvVar_wins_over_globalEnvVars_for_gate", () => { + // If the per-agent envVars has the key, globalEnvVars is redundant but + // the gate must remain satisfied (per-agent wins, both paths satisfy). + const result = computeLocalModeGate({ + envVars: { ANTHROPIC_API_KEY: "sk-per-agent" }, + globalEnvVars: { ANTHROPIC_API_KEY: "sk-global" }, + isProviderMode: false, + model: "claude-3-5-sonnet-20241022", + provider: "anthropic", + runtimeId: "buzz-agent", + useMesh: false, + }); + + assert.equal( + result.satisfied, + true, + "per-agent key must satisfy the gate regardless of global", + ); +}); + +test("localMode_globalEnvVars_empty_still_fails_gate", () => { + // No global and no per-agent env → gate must still surface the missing key. + const result = computeLocalModeGate({ + envVars: {}, + globalEnvVars: {}, + isProviderMode: false, + model: "claude-3-5-sonnet-20241022", + provider: "anthropic", + runtimeId: "buzz-agent", + useMesh: false, + }); + + assert.equal( + result.satisfied, + false, + "empty global and per-agent env must leave gate unsatisfied", + ); + assert.ok( + result.missingEnvKeys.includes("ANTHROPIC_API_KEY"), + "ANTHROPIC_API_KEY must be in missingEnvKeys when neither source provides it", + ); +}); diff --git a/desktop/src/features/agents/ui/personaDialogPickers.tsx b/desktop/src/features/agents/ui/personaDialogPickers.tsx index 574dabff7..875c287c5 100644 --- a/desktop/src/features/agents/ui/personaDialogPickers.tsx +++ b/desktop/src/features/agents/ui/personaDialogPickers.tsx @@ -369,6 +369,9 @@ export function getDefaultPersonaRuntime(runtimes: AcpRuntimeCatalogEntry[]) { */ export function computeLocalModeGate({ envVars, + globalEnvVars = {}, + globalProvider = "", + globalModel = "", isProviderMode, model, provider, @@ -377,6 +380,21 @@ export function computeLocalModeGate({ useMesh, }: { envVars: Record; + /** + * Global agent config env vars. Required credential keys satisfied here + * are excluded from `missingEnvKeys` so global config silences the gate. + */ + globalEnvVars?: Record; + /** + * Global fallback provider. When the agent's own provider is empty but a + * global provider is set, the provider normalized-field gate is satisfied. + */ + globalProvider?: string; + /** + * Global fallback model. When the agent's own model is empty but a global + * model is set, the model normalized-field gate is satisfied. + */ + globalModel?: string; isProviderMode: boolean; model: string; provider: string; @@ -407,42 +425,41 @@ export function computeLocalModeGate({ const needsProviderSelection = runtimeSupportsLlmProviderSelection(runtimeId); - // A normalized field is satisfied by the runtime file config when the file - // provides the value (provider or model). The file layer silences the - // requirement; the value is not injected into the Buzz env. + // File-layer values for goose-style runtimes. These silence requirements + // when the runtime config file provides the value — the file layer is the + // lowest precedence fallback: env → global → file. const fileProvider = runtimeFileConfig?.provider?.trim() ?? ""; const fileModel = runtimeFileConfig?.model?.trim() ?? ""; const fileSatisfiedKeys = new Set(runtimeFileConfig?.satisfiedEnvKeys ?? []); + // Effective provider/model: agent value → global fallback → file fallback. + const effectiveProvider = + provider.trim() || (globalProvider ?? "").trim() || fileProvider; + const effectiveModel = + model.trim() || (globalModel ?? "").trim() || fileModel; + const missingNormalizedFields: string[] = []; if (needsProviderSelection) { - if (provider.trim().length === 0 && fileProvider.length === 0) { - missingNormalizedFields.push("provider"); - } - if (model.trim().length === 0 && fileModel.length === 0) { - missingNormalizedFields.push("model"); - } + if (effectiveProvider.length === 0) missingNormalizedFields.push("provider"); + if (effectiveModel.length === 0) missingNormalizedFields.push("model"); } // Credential keys depend on the selected provider (empty provider → no keys // required beyond the normalized field gate above). - // Use the file provider as fallback when the env provider is empty, so - // credential requirements are computed correctly for file-config runtimes. - const effectiveProviderForKeys = needsProviderSelection - ? provider.trim() || fileProvider - : ""; - const providerForKeys = needsProviderSelection - ? effectiveProviderForKeys - : ""; + // Use the effective provider (env → global → file) so credential + // requirements are computed correctly for all config sources. + const providerForKeys = needsProviderSelection ? effectiveProvider : ""; const requiredKeys = requiredCredentialEnvKeys(runtimeId, providerForKeys); const missingEnvKeys: string[] = []; const fileSatisfiedEnvKeys: string[] = []; for (const key of requiredKeys) { - if ((envVars[key] ?? "").length > 0) { - // Set in Buzz env — satisfied, no action. + const agentValue = envVars[key] ?? ""; + const globalValue = (globalEnvVars ?? {})[key] ?? ""; + if (agentValue.length > 0 || globalValue.length > 0) { + // Set in Buzz env or global defaults — satisfied, no action. } else if (fileSatisfiedKeys.has(key)) { - // Not in Buzz env but present in the runtime config file — silenced. + // Not in Buzz env or global but present in the runtime config file. fileSatisfiedEnvKeys.push(key); } else { missingEnvKeys.push(key); diff --git a/desktop/src/features/agents/useGlobalAgentConfig.ts b/desktop/src/features/agents/useGlobalAgentConfig.ts new file mode 100644 index 000000000..bf9ef8e8d --- /dev/null +++ b/desktop/src/features/agents/useGlobalAgentConfig.ts @@ -0,0 +1,46 @@ +/** + * React hook: load the global agent configuration defaults. + * + * Fetches once on mount and exposes the config. Error is swallowed and + * treated as no global config (safe — the absence of a global config is + * not an error state for callers). + */ +import * as React from "react"; + +import { getGlobalAgentConfig } from "@/shared/api/tauriGlobalAgentConfig"; +import type { GlobalAgentConfig } from "@/shared/api/types"; + +const EMPTY_CONFIG: GlobalAgentConfig = { + env_vars: {}, + provider: null, + model: null, +}; + +export function useGlobalAgentConfig(): { + globalConfig: GlobalAgentConfig; + isLoading: boolean; +} { + const [globalConfig, setGlobalConfig] = + React.useState(EMPTY_CONFIG); + const [isLoading, setIsLoading] = React.useState(true); + + React.useEffect(() => { + let cancelled = false; + getGlobalAgentConfig() + .then((config) => { + if (!cancelled) { + setGlobalConfig(config); + setIsLoading(false); + } + }) + .catch(() => { + // Treat load failure as no global config — never block the dialog. + if (!cancelled) setIsLoading(false); + }); + return () => { + cancelled = true; + }; + }, []); + + return { globalConfig, isLoading }; +} diff --git a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx new file mode 100644 index 000000000..4b8f368ed --- /dev/null +++ b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx @@ -0,0 +1,238 @@ +/** + * Settings card for global agent configuration defaults. + * + * Lets the user set env vars, provider, and model that apply to ALL local + * agents as the lowest-precedence user layer. Per-agent and persona configs + * always win on collision. + * + * Precedence: baked floor < GLOBAL (this card) < persona < per-agent. + */ +import { AlertCircle, Check, Loader } from "lucide-react"; +import * as React from "react"; + +import { + getGlobalAgentConfig, + setGlobalAgentConfig, +} from "@/shared/api/tauriGlobalAgentConfig"; +import type { GlobalAgentConfig } from "@/shared/api/types"; +import { EnvVarsEditor } from "@/features/agents/ui/EnvVarsEditor"; +import { + AUTO_PROVIDER_DROPDOWN_VALUE, + CUSTOM_PROVIDER_DROPDOWN_VALUE, + getPersonaProviderOptions, +} from "@/features/agents/ui/personaDialogPickers"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; +import { SettingsSectionHeader } from "./SettingsSectionHeader"; +import { SettingsOptionGroup } from "./SettingsOptionGroup"; + +const EMPTY_CONFIG: GlobalAgentConfig = { + env_vars: {}, + provider: null, + model: null, +}; + +type SaveState = "idle" | "saving" | "saved" | "error"; + +export function GlobalAgentConfigSettingsCard() { + const [config, setConfig] = React.useState(EMPTY_CONFIG); + const [dirty, setDirty] = React.useState(false); + const [saveState, setSaveState] = React.useState("idle"); + const [saveError, setSaveError] = React.useState(null); + const [isLoading, setIsLoading] = React.useState(true); + const [isCustomProvider, setIsCustomProvider] = React.useState(false); + const savedTimerRef = React.useRef | null>( + null, + ); + + // Load on mount. + React.useEffect(() => { + let cancelled = false; + getGlobalAgentConfig() + .then((loaded) => { + if (!cancelled) { + setConfig(loaded); + setIsLoading(false); + } + }) + .catch(() => { + if (!cancelled) setIsLoading(false); + }); + return () => { + cancelled = true; + }; + }, []); + + function handleEnvVarsChange(next: Record) { + setConfig((prev) => ({ ...prev, env_vars: next })); + setDirty(true); + setSaveState("idle"); + setSaveError(null); + } + + function handleProviderChange(value: string) { + if (value === CUSTOM_PROVIDER_DROPDOWN_VALUE) { + setIsCustomProvider(true); + return; + } + if (value === AUTO_PROVIDER_DROPDOWN_VALUE || value === "") { + setIsCustomProvider(false); + setConfig((prev) => ({ ...prev, provider: null })); + } else { + setIsCustomProvider(false); + setConfig((prev) => ({ ...prev, provider: value })); + } + setDirty(true); + setSaveState("idle"); + setSaveError(null); + } + + function handleCustomProviderInput(value: string) { + setConfig((prev) => ({ ...prev, provider: value || null })); + setDirty(true); + setSaveState("idle"); + setSaveError(null); + } + + function handleModelChange(value: string) { + setConfig((prev) => ({ ...prev, model: value || null })); + setDirty(true); + setSaveState("idle"); + setSaveError(null); + } + + async function handleSave() { + setSaveState("saving"); + setSaveError(null); + try { + const saved = await setGlobalAgentConfig(config); + setConfig(saved); + setDirty(false); + setSaveState("saved"); + if (savedTimerRef.current) clearTimeout(savedTimerRef.current); + savedTimerRef.current = setTimeout(() => setSaveState("idle"), 2500); + } catch (err) { + setSaveState("error"); + setSaveError(typeof err === "string" ? err : "Failed to save."); + } + } + + const providerValue = config.provider ?? ""; + const providerOptions = getPersonaProviderOptions(providerValue, "goose"); + const providerSelectValue = isCustomProvider + ? CUSTOM_PROVIDER_DROPDOWN_VALUE + : providerValue || AUTO_PROVIDER_DROPDOWN_VALUE; + + return ( +
+ + + {isLoading ? ( +
+ + Loading… +
+ ) : ( + + {/* Provider field */} +
+ + + {isCustomProvider ? ( + handleCustomProviderInput(e.target.value)} + placeholder="Custom provider ID" + value={providerValue} + /> + ) : null} +

+ Applies to all agents that don't have a per-agent provider set. +

+
+ + {/* Model field */} +
+ + handleModelChange(e.target.value)} + placeholder="e.g. claude-opus-4 (leave blank for no global default)" + value={config.model ?? ""} + /> +

+ Applies to all agents that don't have a per-agent model set. +

+
+ + {/* Env vars */} +
+ +
+ + )} + + {/* Save bar */} + {!isLoading && ( +
+ + {saveState === "saved" && ( + + + Saved + + )} + {saveState === "error" && saveError && ( + + + {saveError} + + )} +
+ )} +
+ ); +} diff --git a/desktop/src/features/settings/ui/SettingsPanels.tsx b/desktop/src/features/settings/ui/SettingsPanels.tsx index 944b32246..826bde4fa 100644 --- a/desktop/src/features/settings/ui/SettingsPanels.tsx +++ b/desktop/src/features/settings/ui/SettingsPanels.tsx @@ -43,6 +43,7 @@ import { MeshComputeSettingsCard } from "@/features/mesh-compute/ui/MeshComputeS import { MobilePairingCard } from "./MobilePairingCard"; import { NotificationSettingsCard } from "./NotificationSettingsCard"; import { PreventSleepSettingsCard } from "./PreventSleepSettingsCard"; +import { GlobalAgentConfigSettingsCard } from "./GlobalAgentConfigSettingsCard"; import { ProfileSettingsCard } from "./ProfileSettingsCard"; import { UpdateChecker } from "../UpdateChecker"; import { SettingsSectionHeader } from "./SettingsSectionHeader"; @@ -373,7 +374,14 @@ export function renderSettingsSection( case "experimental": return ; case "agents": - return ; + return ( + <> + +
+ +
+ + ); case "channel-templates": return ; case "compute": diff --git a/desktop/src/shared/api/tauriGlobalAgentConfig.ts b/desktop/src/shared/api/tauriGlobalAgentConfig.ts new file mode 100644 index 000000000..a079c342a --- /dev/null +++ b/desktop/src/shared/api/tauriGlobalAgentConfig.ts @@ -0,0 +1,25 @@ +import { invokeTauri } from "@/shared/api/tauri"; +import type { GlobalAgentConfig } from "@/shared/api/types"; + +/** + * Read the current global agent configuration defaults. + * + * Returns an empty default if the file has not been written yet. + */ +export async function getGlobalAgentConfig(): Promise { + return invokeTauri("get_global_agent_config"); +} + +/** + * Validate and persist a new global agent configuration. + * + * The backend strips empty env values (empty = "inherit"), validates key + * shape and reserved-key rules, and returns the saved config. + * + * Throws a string error message on validation failure. + */ +export async function setGlobalAgentConfig( + config: GlobalAgentConfig, +): Promise { + return invokeTauri("set_global_agent_config", { config }); +} diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index e2a7da326..12001e83c 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -512,6 +512,7 @@ export type ConfigOrigin = | "envVar" | "configFile" | "personaDefault" + | "globalDefault" | "runtimeOverride" | "harnessConstraint"; @@ -844,3 +845,22 @@ export type ChannelMessagesPageResponse = { /** Present only when a full page was returned — pass back to fetch the next (older) page. */ nextCursor: ChannelPageCursor | null; }; + +// ── Global agent configuration ──────────────────────────────────────────────── + +/** + * Global agent configuration defaults applied to ALL agents. + * + * Lowest user-settable layer — per-agent and persona values win on any key + * collision. Mirrors the Rust `GlobalAgentConfig` struct. + * + * Precedence: baked floor < global < persona < per-agent. + */ +export type GlobalAgentConfig = { + /** Global env vars injected into all agents unconditionally. */ + env_vars: Record; + /** Global fallback provider (e.g. "anthropic", "databricks_v2"). Null = no global default. */ + provider: string | null; + /** Global fallback model identifier. Null = no global default. */ + model: string | null; +}; From 1313e6cc790d82e020bf1eaaece00432d36eeb62 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 1 Jul 2026 19:12:56 -0400 Subject: [PATCH 02/16] fix(desktop): resolve readiness/spawn divergence and isMissing for global defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs found by Thufir in pass-1 review; all verified red-before/green-after. Fix 1 (CRITICAL — readiness/spawn divergence): extract `resolve_effective_model_provider(record, personas, global)` in `global_config/mod.rs` encoding the agent→persona→global→None precedence chain, and use it in both `resolve_effective_agent_env` (readiness) and `spawn_agent_child` (runtime). Before the fix, readiness used the full fallback chain but the spawn path read only `record.model`/`record.provider`, so a global-default-only agent reported Ready but spawned without provider/model env. Also fixes the same drift in `build_deploy_payload`. Hoists the global config load out of the readiness scoped block in runtime.rs to reuse it for the env-var merge, eliminating the duplicate load. Fix 2 (IMPORTANT — override baseline for global-default agents): in `resolve_config_surface`, when `!had_model && persona_model.is_none() && model_overridden`, use `(global.model, ConfigOrigin::GlobalDefault)` as the baseline. Before the fix, baseline was None in this case, so a live model switch on a global-default-only agent had no secondary row to show. Fix 3 (IMPORTANT — required rows): extract `isRequiredKeyMissing` from `EnvVarsEditor.tsx` and teach it to check `inheritedFrom?.[key]` in addition to the agent-local value. Before the fix, a globally-satisfied required key still rendered the amber "Required" badge because `isMissing` only checked the local map. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src-tauri/src/commands/agent_config.rs | 57 +++++ desktop/src-tauri/src/commands/agents.rs | 39 +-- .../src/managed_agents/global_config/mod.rs | 41 ++++ .../src/managed_agents/global_config/tests.rs | 230 +++++++++++++++++- desktop/src-tauri/src/managed_agents/mod.rs | 3 +- .../src-tauri/src/managed_agents/readiness.rs | 29 +-- .../src-tauri/src/managed_agents/runtime.rs | 30 +-- .../src/features/agents/ui/EnvVarsEditor.tsx | 21 +- .../agents/ui/envVarsEditorMissing.test.mjs | 105 ++++++++ 9 files changed, 480 insertions(+), 75 deletions(-) create mode 100644 desktop/src/features/agents/ui/envVarsEditorMissing.test.mjs diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 818dd4ca8..8baabd20b 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -93,9 +93,21 @@ fn resolve_config_surface( None } } else { + // Prefer persona as baseline, fall back to global when persona has none + // and the model was overridden mid-session (global-default agent). persona_model .clone() .map(|m| (m, ConfigOrigin::PersonaDefault)) + .or_else(|| { + if model_overridden { + global + .model + .clone() + .map(|m| (m, ConfigOrigin::GlobalDefault)) + } else { + None + } + }) }; // Inject resolved persona values into the record where absent. @@ -707,4 +719,49 @@ mod tests { assert_eq!(model.overridden_value.as_deref(), Some("persona-model")); assert_eq!(model.overridden_origin, Some(ConfigOrigin::PersonaDefault)); } + + /// Fix 2 regression: a global-default-only agent (no record model, no + /// persona model, but global has a model) that live-switches mid-session + /// must render the global model as the secondary tagged `GlobalDefault`. + /// Before the fix, `baseline` was `None` in the `!had_model` arm when + /// persona has no model, so `read_config_surface` had no secondary to + /// surface. Fails against pre-fix code where the baseline arm returned + /// `None` when `!had_model && persona_model.is_none() && model_overridden`. + #[test] + fn global_default_live_switch_renders_global_model_as_secondary_global_default() { + // Record has no model, no persona, global provides the model. + let mut record = agent_record(); + record.persona_id = None; + // record.model = None (set by agent_record()) + let personas: Vec = vec![]; + let cache = session_cache("model-y", true); + let global = crate::managed_agents::GlobalAgentConfig { + model: Some("global-model".to_string()), + ..Default::default() + }; + + let surface = resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + Some(&cache), + &global, + ); + let model = surface.normalized.model.expect("model resolved"); + + // Live model wins as primary. + assert_eq!(model.value.as_deref(), Some("model-y")); + assert_eq!(model.origin, ConfigOrigin::RuntimeOverride); + // Global model surfaces as secondary, tagged GlobalDefault. + assert_eq!( + model.overridden_value.as_deref(), + Some("global-model"), + "global model must be the override baseline secondary" + ); + assert_eq!( + model.overridden_origin, + Some(ConfigOrigin::GlobalDefault), + "override baseline origin must be GlobalDefault, not PersonaDefault or BuzzExplicit" + ); + } } diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index f00f11676..514d53897 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -333,37 +333,16 @@ fn build_deploy_payload( // Resolve the persona's structured provider/model so the remote provider // receives the same authoritative values that local spawn derives from - // `runtime_metadata_env_vars`. Without this, remote deploy would rely on - // stale derived env copies in `env_vars` (or have no provider at all for - // imported personas whose derived keys were filtered at import time). - // - // Precedence mirrors local spawn: persona structured model is authoritative - // when present; the agent record's model is a fallback; global model is the - // last resort (when no persona or agent specifies one). + // `runtime_metadata_env_vars`. Uses the shared resolver for consistent + // agent → persona → global → None precedence. let global_config = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); - let (effective_model, effective_provider) = if let Some(pid) = record.persona_id.as_deref() { - let personas = load_personas(app).map_err(|e| { - format!( - "failed to load personas while building deploy payload for persona `{pid}`: {e}" - ) - })?; - let persona = personas - .into_iter() - .find(|p| p.id == pid) - .ok_or_else(|| format!("persona `{pid}` not found while building deploy payload"))?; - let model = persona - .model - .clone() - .or(record.model.clone()) - .or(global_config.model.clone()); - let provider = persona.provider.or(global_config.provider.clone()); - (model, provider) - } else { - ( - record.model.clone().or(global_config.model.clone()), - global_config.provider.clone(), - ) - }; + let personas = load_personas(app).unwrap_or_default(); + let (effective_model, effective_provider) = + crate::managed_agents::resolve_effective_model_provider(record, &personas, &global_config); + let (effective_model, effective_provider) = ( + effective_model.map(|s| s.to_string()), + effective_provider.map(|s| s.to_string()), + ); Ok(serde_json::json!({ "name": &record.name, diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 56c02cbf2..2b07f87d0 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -28,6 +28,7 @@ use tauri::AppHandle; use crate::managed_agents::env_vars::{validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS}; use crate::managed_agents::storage::{atomic_write_json_restricted, managed_agents_base_dir}; +use crate::managed_agents::types::{ManagedAgentRecord, PersonaRecord}; /// The global agent configuration record. /// @@ -144,5 +145,45 @@ pub fn save_global_agent_config(app: &AppHandle, config: &GlobalAgentConfig) -> atomic_write_json_restricted(&path, &payload) } +/// Resolve the effective model and provider for an agent using the +/// precedence chain: `agent record → linked persona → global defaults → None`. +/// +/// This is the single source of truth used by readiness evaluation, spawn, +/// and deploy-payload construction. All three paths must use this function so +/// they agree on what model/provider the agent will actually run with. +/// +/// # Arguments +/// * `record` — the `ManagedAgentRecord` (may have `None` for model/provider) +/// * `personas` — all current persona records (looked up by `record.persona_id`) +/// * `global` — global agent config defaults +/// +/// # Returns +/// `(effective_model, effective_provider)` — both `Option<&str>`. +pub(crate) fn resolve_effective_model_provider<'a>( + record: &'a ManagedAgentRecord, + personas: &'a [PersonaRecord], + global: &'a GlobalAgentConfig, +) -> (Option<&'a str>, Option<&'a str>) { + let (persona_model, persona_provider) = record + .persona_id + .as_deref() + .and_then(|pid| personas.iter().find(|p| p.id == pid)) + .map(|p| (p.model.as_deref(), p.provider.as_deref())) + .unwrap_or((None, None)); + + let effective_model = record + .model + .as_deref() + .or(persona_model) + .or(global.model.as_deref()); + let effective_provider = record + .provider + .as_deref() + .or(persona_provider) + .or(global.provider.as_deref()); + + (effective_model, effective_provider) +} + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 381b5b80d..2e1b91160 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -1,6 +1,10 @@ use std::collections::BTreeMap; -use super::{strip_empty_env_vars, validate_global_config, GlobalAgentConfig}; +use super::{ + resolve_effective_model_provider, strip_empty_env_vars, validate_global_config, + GlobalAgentConfig, +}; +use crate::managed_agents::{BackendKind, ManagedAgentRecord, PersonaRecord, RespondTo}; fn config_with_env(pairs: &[(&str, &str)]) -> GlobalAgentConfig { GlobalAgentConfig { @@ -137,3 +141,227 @@ fn empty_env_vars_omitted_from_serialization() { assert!(!json.contains("provider"), "None provider must be omitted"); assert!(!json.contains("model"), "None model must be omitted"); } + +// ── resolve_effective_model_provider ───────────────────────────────────────── + +fn bare_record() -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: "agent".to_string(), + name: "Agent".to_string(), + persona_id: None, + private_key_nsec: "".to_string(), + auth_tag: None, + relay_url: "ws://localhost:3000".to_string(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: "goose".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: "".to_string(), + turn_timeout_seconds: 300, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + mcp_toolsets: None, + env_vars: BTreeMap::new(), + start_on_app_launch: false, + runtime_pid: None, + backend: BackendKind::Local, + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: "".to_string(), + updated_at: "".to_string(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: RespondTo::OwnerOnly, + respond_to_allowlist: vec![], + relay_mesh: None, + } +} + +fn persona(id: &str, model: Option<&str>, provider: Option<&str>) -> PersonaRecord { + PersonaRecord { + id: id.to_string(), + display_name: "Test Persona".to_string(), + avatar_url: None, + system_prompt: "".to_string(), + runtime: None, + model: model.map(str::to_string), + provider: provider.map(str::to_string), + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + created_at: "".to_string(), + updated_at: "".to_string(), + } +} + +/// Tier 1 — agent record wins: record has explicit model/provider; they must +/// outrank both the linked persona and the global defaults. Fails against any +/// implementation that prefers global or persona over the record. +#[test] +fn resolve_agent_record_wins_over_persona_and_global() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + record.model = Some("record-model".to_string()); + record.provider = Some("record-provider".to_string()); + let personas = vec![persona( + "p1", + Some("persona-model"), + Some("persona-provider"), + )]; + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + ..Default::default() + }; + + let (model, provider) = resolve_effective_model_provider(&record, &personas, &global); + + assert_eq!(model, Some("record-model"), "record model must win"); + assert_eq!( + provider, + Some("record-provider"), + "record provider must win" + ); +} + +/// Tier 2 — persona fallback: record has no model/provider; the linked +/// persona's values must be used. Fails against an implementation that skips +/// persona lookup and returns global or None directly. +#[test] +fn resolve_persona_fallback_when_record_has_none() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + // record.model and record.provider are None + let personas = vec![persona( + "p1", + Some("persona-model"), + Some("persona-provider"), + )]; + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + ..Default::default() + }; + + let (model, provider) = resolve_effective_model_provider(&record, &personas, &global); + + assert_eq!( + model, + Some("persona-model"), + "persona model must be used when record has none" + ); + assert_eq!( + provider, + Some("persona-provider"), + "persona provider must be used when record has none" + ); +} + +/// Tier 3 — global fallback: record and persona both have no model/provider; +/// global defaults must fill in. This is the core bug Fix 1 addresses — a +/// global-only agent was Ready per readiness but spawned without model/provider. +/// Fails against the pre-fix runtime.rs spawn path that read only record.model. +#[test] +fn resolve_global_fallback_when_record_and_persona_have_none() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + // record.model / provider = None; persona.model / provider = None + let personas = vec![persona("p1", None, None)]; + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + ..Default::default() + }; + + let (model, provider) = resolve_effective_model_provider(&record, &personas, &global); + + assert_eq!( + model, + Some("global-model"), + "global model must be used when record and persona have none" + ); + assert_eq!( + provider, + Some("global-provider"), + "global provider must be used when record and persona have none" + ); +} + +/// Tier 4 — no persona linked: record.persona_id is None, record has no +/// model/provider; global defaults must still fill in (persona lookup skipped). +#[test] +fn resolve_global_fallback_when_no_persona_linked() { + let record = bare_record(); // persona_id = None, model/provider = None + let personas: Vec = vec![]; + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + ..Default::default() + }; + + let (model, provider) = resolve_effective_model_provider(&record, &personas, &global); + + assert_eq!(model, Some("global-model")); + assert_eq!(provider, Some("global-provider")); +} + +/// All-None: no source provides model/provider → both must be None. +/// Guards against a resolver that synthesizes phantom defaults. +#[test] +fn resolve_all_none_when_no_source_provides_values() { + let record = bare_record(); // persona_id = None, model/provider = None + let personas: Vec = vec![]; + let global = GlobalAgentConfig::default(); // model/provider = None + + let (model, provider) = resolve_effective_model_provider(&record, &personas, &global); + + assert_eq!( + model, None, + "must return None when no source provides a model" + ); + assert_eq!( + provider, None, + "must return None when no source provides a provider" + ); +} + +/// Partial tier — record has model but not provider; persona has provider but +/// not model; global has both. Each field resolves independently through the +/// three-tier chain. +#[test] +fn resolve_each_field_resolves_independently_through_tiers() { + let mut record = bare_record(); + record.persona_id = Some("p1".to_string()); + record.model = Some("record-model".to_string()); + // record.provider = None → falls through to persona + let personas = vec![persona("p1", None, Some("persona-provider"))]; + // persona.model = None → global fills model if record also had none, but + // record has model here so global is not needed for model. + let global = GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-provider".to_string()), + ..Default::default() + }; + + let (model, provider) = resolve_effective_model_provider(&record, &personas, &global); + + assert_eq!(model, Some("record-model"), "record wins for model"); + assert_eq!( + provider, + Some("persona-provider"), + "persona wins for provider when record has none" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 2447e9481..e7ff7ccc5 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -30,7 +30,8 @@ pub use backend::*; pub use discovery::*; pub use env_vars::*; pub(crate) use global_config::{ - load_global_agent_config, save_global_agent_config, validate_global_config, GlobalAgentConfig, + load_global_agent_config, resolve_effective_model_provider, save_global_agent_config, + validate_global_config, GlobalAgentConfig, }; pub use nest::*; pub use persona_card::*; diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 7572cabcc..cce28381a 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -100,31 +100,10 @@ pub(crate) fn resolve_effective_agent_env( // Layer 2: runtime metadata env vars (model / provider keys derived from // the record's structured fields, with global as fallback). // - // Structured-field fallback: effective provider/model = agent → persona → global → None. - // This means a global provider/model is used by readiness evaluation and spawn - // when neither the agent record nor the linked persona specifies one. - let persona_model = record.persona_id.as_deref().and_then(|pid| { - personas - .iter() - .find(|p| p.id == pid) - .and_then(|p| p.model.clone()) - }); - let persona_provider = record.persona_id.as_deref().and_then(|pid| { - personas - .iter() - .find(|p| p.id == pid) - .and_then(|p| p.provider.clone()) - }); - let effective_model = record - .model - .as_deref() - .or(persona_model.as_deref()) - .or(global.model.as_deref()); - let effective_provider = record - .provider - .as_deref() - .or(persona_provider.as_deref()) - .or(global.provider.as_deref()); + // Uses the shared resolver to guarantee readiness and spawn agree on the + // effective model/provider: agent → persona → global → None. + let (effective_model, effective_provider) = + super::global_config::resolve_effective_model_provider(record, personas, global); if let Some(rt) = runtime { for (key, value) in super::runtime::runtime_metadata_env_vars( diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 91a7ad51f..c824983a9 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1665,6 +1665,9 @@ pub fn spawn_agent_child( // command, so we recompute them from the effective value rather than the // frozen record snapshot. Mirrors the model resolution below. let personas = super::load_personas(app).unwrap_or_default(); + // Load global config once; used for runtime_metadata_env_vars (model/provider fallback) + // and for the env-var merge at spawn time. + let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let effective_command = super::effective_agent_command( record.persona_id.as_deref(), &personas, @@ -1768,7 +1771,6 @@ pub fn spawn_agent_child( agent_readiness, resolve_effective_agent_env, AgentReadiness, Requirement, }; - let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let effective = resolve_effective_agent_env(record, &personas, runtime_meta, &global); // Compute the optional payload before touching the command. let setup_payload_json = @@ -1865,23 +1867,20 @@ pub fn spawn_agent_child( command.env("BUZZ_ACP_PERSONA_NAME", persona_name); } - // System prompt, model, and provider come from the record snapshot — the - // record is the authoritative spawn source. For persona-created agents the - // snapshot was pinned at create (see `create_managed_agent`); for others - // these are the user-supplied values. Reading the record (never the live - // persona) is what keeps a running agent pinned across restarts: a persona - // edit reaches the agent only via delete+respawn, which rewrites the - // snapshot. + // System prompt comes from the record snapshot (pinned at create for + // persona-created agents, keeping a running agent stable across restarts). + // Model and provider use the shared resolver: agent → persona → global → None, + // so a global-default-only agent spawns with the correct provider/model env. let effective_prompt = record.system_prompt.clone(); - let effective_model = record.model.clone(); - let effective_provider = record.provider.clone(); + let (effective_model, effective_provider) = + crate::managed_agents::resolve_effective_model_provider(record, &personas, &global); if let Some(prompt) = &effective_prompt { command.env("BUZZ_ACP_SYSTEM_PROMPT", prompt); } else { command.env_remove("BUZZ_ACP_SYSTEM_PROMPT"); } - if let Some(model) = &effective_model { + if let Some(model) = effective_model { command.env("BUZZ_ACP_MODEL", model); } else { command.env_remove("BUZZ_ACP_MODEL"); @@ -1895,8 +1894,8 @@ pub fn spawn_agent_child( meta.model_env_var, meta.provider_env_var, meta.provider_locked, - effective_model.as_deref(), - effective_provider.as_deref(), + effective_model, + effective_provider, ) { command.env(key, value); } @@ -1982,10 +1981,7 @@ pub fn spawn_agent_child( // BUZZ_AUTH_TAG, BUZZ_API_TOKEN, BUZZ_ACP_PRIVATE_KEY, BUZZ_ACP_API_TOKEN), // which `merged_user_env` strips. Those carry Buzz's identity and must // never be GUI-overridable. - let global_env_for_spawn = crate::managed_agents::load_global_agent_config(app) - .unwrap_or_default() - .env_vars; - for (key, value) in super::env_vars::merged_user_env(&global_env_for_spawn, &record.env_vars) { + for (key, value) in super::env_vars::merged_user_env(&global.env_vars, &record.env_vars) { command.env(key, value); } diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.tsx b/desktop/src/features/agents/ui/EnvVarsEditor.tsx index 68a1d42c0..63e2d2ac7 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.tsx +++ b/desktop/src/features/agents/ui/EnvVarsEditor.tsx @@ -11,6 +11,23 @@ import { export type EnvVarsValue = Record; +/** + * Returns true when a required env key is unsatisfied — neither the agent-local + * value nor the inherited (global / persona) value provides it. + * + * Used by `EnvVarsEditor` to render the amber "Required" badge on unfilled rows. + * Exported for unit testing. + */ +export function isRequiredKeyMissing( + key: string, + localValue: EnvVarsValue, + inheritedFrom: EnvVarsValue | undefined, +): boolean { + const local = localValue[key] ?? ""; + const inherited = inheritedFrom?.[key] ?? ""; + return local.length === 0 && inherited.length === 0; +} + type EnvVarsEditorProps = { /** The current key/value map. */ value: EnvVarsValue; @@ -119,7 +136,9 @@ export function EnvVarsEditor({ {/* Required credential rows — shown first, key is read-only */} {requiredKeys.map((key) => { const currentValue = value[key] ?? ""; - const isMissing = currentValue.length === 0; + // A required key is only "missing" if neither the agent-local value + // nor the inherited (global / persona) value provides it. + const isMissing = isRequiredKeyMissing(key, value, inheritedFrom); return (
diff --git a/desktop/src/features/agents/ui/envVarsEditorMissing.test.mjs b/desktop/src/features/agents/ui/envVarsEditorMissing.test.mjs new file mode 100644 index 000000000..6784a0662 --- /dev/null +++ b/desktop/src/features/agents/ui/envVarsEditorMissing.test.mjs @@ -0,0 +1,105 @@ +/** + * Unit tests for isRequiredKeyMissing in EnvVarsEditor. + * + * Fix 3 regression: a required env key satisfied by an inherited (global / + * persona) value must NOT render as missing. Before the fix, isMissing was + * computed from the agent-local `value[key]` only, so a globally-satisfied key + * still rendered the amber "Required" badge even though the key was provided. + * + * isRequiredKeyMissing is the extracted pure helper that gates the badge. + */ + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { isRequiredKeyMissing } from "./EnvVarsEditor.tsx"; + +// ── Happy paths — key is satisfied ─────────────────────────────────────────── + +test("envVarsMissing_localValueSet_notMissing", () => { + // The agent has set the key directly — it is satisfied by the local value. + assert.equal( + isRequiredKeyMissing( + "ANTHROPIC_API_KEY", + { ANTHROPIC_API_KEY: "sk-local" }, + undefined, + ), + false, + "key present in local value must not be missing", + ); +}); + +test("envVarsMissing_inheritedValueSet_notMissing", () => { + // The key is NOT in the agent-local value but IS in inheritedFrom (global). + // This is the core Fix 3 scenario — the row must NOT show "Required". + assert.equal( + isRequiredKeyMissing( + "ANTHROPIC_API_KEY", + {}, + { ANTHROPIC_API_KEY: "sk-global" }, + ), + false, + "key satisfied by inheritedFrom (global) must not be missing", + ); +}); + +test("envVarsMissing_bothLocalAndInheritedSet_notMissing", () => { + // Both sources have the key — local value takes precedence in the UI, + // but either alone is sufficient for the missing predicate. + assert.equal( + isRequiredKeyMissing( + "ANTHROPIC_API_KEY", + { ANTHROPIC_API_KEY: "sk-local" }, + { ANTHROPIC_API_KEY: "sk-global" }, + ), + false, + "key present in both sources must not be missing", + ); +}); + +// ── Missing paths — key is genuinely absent ─────────────────────────────────── + +test("envVarsMissing_neitherLocalNorInherited_isMissing", () => { + // Neither source provides the key — the badge must show. + assert.equal( + isRequiredKeyMissing("ANTHROPIC_API_KEY", {}, undefined), + true, + "key absent from both local and inherited must be missing", + ); +}); + +test("envVarsMissing_inheritedEmpty_isMissing", () => { + // inheritedFrom exists but the specific key has an empty string value — + // empty is treated as absent for the missing predicate. + assert.equal( + isRequiredKeyMissing("ANTHROPIC_API_KEY", {}, { ANTHROPIC_API_KEY: "" }), + true, + "empty inherited value must still be treated as missing", + ); +}); + +test("envVarsMissing_localEmpty_inheritedUndefined_isMissing", () => { + // Local value exists but is empty string; no inheritedFrom. + assert.equal( + isRequiredKeyMissing( + "ANTHROPIC_API_KEY", + { ANTHROPIC_API_KEY: "" }, + undefined, + ), + true, + "empty local value with no inherited must be missing", + ); +}); + +test("envVarsMissing_differentKey_notInherited_isMissing", () => { + // inheritedFrom has a different key, not the required one. + assert.equal( + isRequiredKeyMissing( + "ANTHROPIC_API_KEY", + {}, + { OPENAI_API_KEY: "sk-openai" }, + ), + true, + "inherited key for a different provider must not satisfy this key", + ); +}); From 93341ed32ae5a0c659d9a004238aaa5231f5d43a Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 1 Jul 2026 19:29:42 -0400 Subject: [PATCH 03/16] fix(desktop): restore live-persona-first deploy resolver; fix inherited copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_deploy_payload now uses a dedicated deploy-specific resolver (live-persona → record → global) instead of the shared readiness/spawn resolver (record → persona → global). Provider start does not re-snapshot the linked persona onto the record before deploy, so the record may hold a stale snapshot while the live persona has moved on. Deploy needs live-persona-first to ensure remote agents receive the current config after a persona update, without requiring delete+recreate. Also fix EnvVarsEditor hint copy: rows where the local value is empty but an inherited (global/persona) value satisfies the key now read 'Inherited from {label} value …' instead of 'Overrides {label} value …'. Regression tests added: - deploy_resolver_uses_live_persona_over_stale_record_snapshot - deploy_resolver_falls_back_to_record_when_persona_has_none - deploy_resolver_falls_back_to_global_when_persona_and_record_have_none Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/commands/agents.rs | 59 +++++--- .../src-tauri/src/commands/agents_tests.rs | 135 ++++++++++++++++++ .../src/features/agents/ui/EnvVarsEditor.tsx | 8 +- 3 files changed, 183 insertions(+), 19 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 514d53897..9829136f3 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -11,8 +11,8 @@ use crate::{ save_managed_agents, start_managed_agent_process, stop_managed_agent_process, sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentLogResponse, - ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, - DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, + ManagedAgentRecord, ManagedAgentSummary, PersonaRecord, RelayMeshConfig, + DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -287,13 +287,44 @@ async fn start_local_agent_with_preflight( build_managed_agent_summary(app, record, &runtimes, &personas) } +/// Resolve the deploy-specific structured model/provider for a managed agent. +/// +/// Deploy uses **live-persona-first** precedence so remote agents receive +/// current config after a persona update, without requiring delete+recreate. +/// Unlike local spawn (which re-snapshots the persona onto `record` at the +/// start of every spawn), provider start does not re-snapshot — so the +/// record may hold a stale snapshot while the linked persona has moved on. +/// +/// Precedence: live-persona → record (snapshot fallback) → global. +/// Symmetric for both model and provider. +/// +/// Exported `pub(crate)` for unit testing. +pub(crate) fn resolve_deploy_model_provider<'a>( + record: &'a ManagedAgentRecord, + personas: &'a [PersonaRecord], + global: &'a crate::managed_agents::GlobalAgentConfig, +) -> (Option<&'a str>, Option<&'a str>) { + let live_persona = record + .persona_id + .as_deref() + .and_then(|pid| personas.iter().find(|p| p.id == pid)); + let model = live_persona + .and_then(|p| p.model.as_deref()) + .or(record.model.as_deref()) + .or(global.model.as_deref()); + let provider = live_persona + .and_then(|p| p.provider.as_deref()) + .or(record.provider.as_deref()) + .or(global.provider.as_deref()); + (model, provider) +} + /// Build the standard agent JSON payload for provider deploy calls. /// /// Unlike local spawn (which uses only pinned `record.env_vars` for /// determinism), provider deploy re-reads live persona env vars and -/// structured model/provider so remote agents receive current credentials -/// and the same authoritative values that local spawn derives from -/// `runtime_metadata_env_vars`. The only field still pinned is +/// structured model/provider so remote agents receive current credentials. +/// The only field still pinned is /// `agent_command`/`agent_args` — those were captured at create time. /// The only read-time resolution is `relay_url`: a blank pin resolves to /// the active workspace relay here, matching the create-path contract. @@ -320,9 +351,8 @@ fn build_deploy_payload( // credentials; local spawn uses only pinned record.env_vars for determinism // across restarts. Global env vars are the lowest user-settable layer: // global < persona < agent (last-wins on key collision). - let global_env = crate::managed_agents::load_global_agent_config(app) - .unwrap_or_default() - .env_vars; + let global_config = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + let global_env = global_config.env_vars.clone(); let persona_env = crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?; // Merge: global < persona (persona wins over global). @@ -331,17 +361,14 @@ fn build_deploy_payload( let merged_env = crate::managed_agents::merged_user_env(&global_persona_merged, &record.env_vars); - // Resolve the persona's structured provider/model so the remote provider - // receives the same authoritative values that local spawn derives from - // `runtime_metadata_env_vars`. Uses the shared resolver for consistent - // agent → persona → global → None precedence. - let global_config = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + // Resolve the deploy-specific structured provider/model. Uses the deploy + // resolver with live-persona → record → global precedence. let personas = load_personas(app).unwrap_or_default(); let (effective_model, effective_provider) = - crate::managed_agents::resolve_effective_model_provider(record, &personas, &global_config); + resolve_deploy_model_provider(record, &personas, &global_config); let (effective_model, effective_provider) = ( - effective_model.map(|s| s.to_string()), - effective_provider.map(|s| s.to_string()), + effective_model.map(str::to_string), + effective_provider.map(str::to_string), ); Ok(serde_json::json!({ diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index f141c6ae5..fd6d9f724 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -1,5 +1,140 @@ use super::*; +fn bare_agent_record( + persona_id: Option<&str>, + model: Option<&str>, + provider: Option<&str>, +) -> ManagedAgentRecord { + use crate::managed_agents::{BackendKind, RespondTo}; + use std::collections::BTreeMap; + ManagedAgentRecord { + pubkey: "agent".to_string(), + name: "Agent".to_string(), + persona_id: persona_id.map(str::to_string), + private_key_nsec: "".to_string(), + auth_tag: None, + relay_url: "ws://localhost:3000".to_string(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: "goose".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: "".to_string(), + turn_timeout_seconds: 300, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: model.map(str::to_string), + provider: provider.map(str::to_string), + persona_source_version: None, + mcp_toolsets: None, + env_vars: BTreeMap::new(), + start_on_app_launch: false, + runtime_pid: None, + backend: BackendKind::Local, + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: "".to_string(), + updated_at: "".to_string(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + respond_to: RespondTo::OwnerOnly, + respond_to_allowlist: vec![], + relay_mesh: None, + } +} + +fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> PersonaRecord { + use std::collections::BTreeMap; + PersonaRecord { + id: id.to_string(), + display_name: "Test Persona".to_string(), + avatar_url: None, + system_prompt: "".to_string(), + runtime: None, + model: model.map(str::to_string), + provider: provider.map(str::to_string), + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + created_at: "".to_string(), + updated_at: "".to_string(), + } +} + +/// Deploy-path regression for Fix 1 of Thufir pass-2: a persona-linked +/// provider agent with a stale record snapshot must use the live persona +/// model/provider in the deploy payload, not the stale record values. +/// +/// Scenario: agent was created with persona at model="old-model"/provider="old-prov". +/// The persona was subsequently updated to "new-model"/"new-prov" but the record +/// was NOT re-snapshotted (provider start skips re-snapshot; local spawn does it). +/// The deploy resolver must use the current persona values. +/// +/// Fails against `resolve_effective_model_provider` (record-first precedence), +/// which would return "old-model"/"old-prov" from the stale record. +#[test] +fn deploy_resolver_uses_live_persona_over_stale_record_snapshot() { + // Record holds the stale snapshot (created when persona had old values). + let record = bare_agent_record(Some("p1"), Some("old-model"), Some("old-prov")); + // Live persona has been updated since the record was snapshotted. + let personas = vec![persona_record("p1", Some("new-model"), Some("new-prov"))]; + let global = crate::managed_agents::GlobalAgentConfig::default(); + + let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global); + + assert_eq!( + model, + Some("new-model"), + "deploy must use live persona model, not stale record snapshot" + ); + assert_eq!( + provider, + Some("new-prov"), + "deploy must use live persona provider, not stale record snapshot" + ); +} + +/// Deploy resolver falls back to record when persona has no model/provider +/// (persona without structured model — fallback to record snapshot). +#[test] +fn deploy_resolver_falls_back_to_record_when_persona_has_none() { + let record = bare_agent_record(Some("p1"), Some("record-model"), Some("record-prov")); + // Persona exists but has no model/provider. + let personas = vec![persona_record("p1", None, None)]; + let global = crate::managed_agents::GlobalAgentConfig::default(); + + let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global); + + assert_eq!(model, Some("record-model")); + assert_eq!(provider, Some("record-prov")); +} + +/// Deploy resolver falls back to global when both persona and record have none. +#[test] +fn deploy_resolver_falls_back_to_global_when_persona_and_record_have_none() { + let record = bare_agent_record(Some("p1"), None, None); + let personas = vec![persona_record("p1", None, None)]; + let global = crate::managed_agents::GlobalAgentConfig { + model: Some("global-model".to_string()), + provider: Some("global-prov".to_string()), + ..Default::default() + }; + + let (model, provider) = resolve_deploy_model_provider(&record, &personas, &global); + + assert_eq!(model, Some("global-model")); + assert_eq!(provider, Some("global-prov")); +} + #[test] fn normalize_relay_mesh_rejects_empty_model_ref() { let config = RelayMeshConfig { diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.tsx b/desktop/src/features/agents/ui/EnvVarsEditor.tsx index 63e2d2ac7..4619a9284 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.tsx +++ b/desktop/src/features/agents/ui/EnvVarsEditor.tsx @@ -193,14 +193,16 @@ export function EnvVarsEditor({
{(() => { const inheritedValue = inheritedFrom?.[key]; - return inheritedValue !== undefined ? ( + if (inheritedValue === undefined) return null; + const verb = currentValue ? "Overrides" : "Inherited from"; + return (

- Overrides {inheritedLabel} value{" "} + {verb} {inheritedLabel} value{" "} {maskInherited(inheritedValue)}

- ) : null; + ); })()}
); From ab23df87c4098b49467d7b2418af82f76372d0e9 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 1 Jul 2026 19:40:02 -0400 Subject: [PATCH 04/16] chore(desktop): bump agents.rs file-size exception for deploy resolver resolve_deploy_model_provider fn + doc comment adds 6 lines on top of the prior global-agent-config bump. Queued to split with the rest of the file. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index e5cd02a8b..28c5e30e8 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -62,8 +62,9 @@ const overrides = new Map([ // branch cut; override bumped to cover the merged total. Queued to split. // global-agent-config: build_deploy_payload threads global config fallback // for provider/model/env_vars (+4 lines). cargo fmt reflowed 2 more lines. + // deploy-resolver: resolve_deploy_model_provider fn + doc comment (+6 lines). // Queued to split. - ["src-tauri/src/commands/agents.rs", 1443], + ["src-tauri/src/commands/agents.rs", 1449], // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS // const + build_thread_replies_filter helper, mirroring the channel sibling so // the two p-gate filters can't drift) plus two guard unit tests. The file was From b15296a70ab21753de06f66962691d10532c3982 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 2 Jul 2026 12:20:40 -0400 Subject: [PATCH 05/16] fix(desktop): always emit all GlobalAgentConfig fields; move card to AgentsView IPC contract fix: drop skip_serializing_if on GlobalAgentConfig env_vars, provider, and model so the Tauri command always returns {"env_vars":{},"provider":null,"model":null} instead of {}. The TS type declares all three non-optional; a missing field caused an Object.entries crash in GlobalAgentConfigSettingsCard. Move GlobalAgentConfigSettingsCard out of SettingsPanels agents-case and into AgentsView (between TeamsSection and RelayDirectorySection) for contextual proximity to the agent list. Add an explicit loadError state so config-load failures show an inline error instead of silently leaving the card empty. Gate the save bar on !loadError so it does not render over an error message. Fix personaDialogPickers.tsx useOptionalChain biome lint warning. Reformat readiness.rs via cargo fmt (whitespace only). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 3 +- .../src/managed_agents/global_config/mod.rs | 6 +-- .../src/managed_agents/global_config/tests.rs | 44 ++++++++++++++++--- desktop/src/features/agents/ui/AgentsView.tsx | 3 ++ .../agents/ui/personaDialogPickers.tsx | 5 ++- .../ui/GlobalAgentConfigSettingsCard.tsx | 13 +++++- .../features/settings/ui/SettingsPanels.tsx | 10 +---- 7 files changed, 62 insertions(+), 22 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 28c5e30e8..5e3bdfcb9 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -100,7 +100,8 @@ const overrides = new Map([ // New file in this PR; queued to split. // +2 readiness integration tests for flat-DATABRICKS_HOST canonicalization fix. // +1 cargo fmt whitespace reformat (readiness.rs closures inline after rebase). - ["src-tauri/src/managed_agents/readiness.rs", 1150], + // +16: resolve_effective_agent_env + global-config readiness wiring (#1448 base). + ["src-tauri/src/managed_agents/readiness.rs", 1166], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 2b07f87d0..75a0475cd 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -45,21 +45,21 @@ pub struct GlobalAgentConfig { /// Lowest user-settable layer — per-agent and persona values win on any /// key collision. Reserved and derived keys are rejected at save time and /// stripped at spawn time. - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + #[serde(default)] pub env_vars: BTreeMap, /// Global fallback provider (e.g. `"databricks_v2"`, `"anthropic"`). /// /// Used only when neither the agent record nor the linked persona specifies /// a provider. `None` = no global default. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub provider: Option, /// Global fallback model identifier. /// /// Used only when neither the agent record nor the linked persona specifies /// a model. `None` = no global default. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub model: Option, } diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 2e1b91160..c5aa89c85 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -133,13 +133,25 @@ fn roundtrip_serialization() { } #[test] -fn empty_env_vars_omitted_from_serialization() { +fn default_global_config_serializes_all_fields() { + // IPC contract: the frontend TS type declares env_vars/provider/model as + // non-optional. A bare `{}` (old skip_serializing_if behaviour) caused an + // `Object.entries` crash on the undefined value. All three fields must + // always be present in the serialized form. let config = GlobalAgentConfig::default(); let json = serde_json::to_string(&config).expect("serialize"); - // With all-default/empty, the JSON should be compact. - assert!(!json.contains("env_vars"), "empty env_vars must be omitted"); - assert!(!json.contains("provider"), "None provider must be omitted"); - assert!(!json.contains("model"), "None model must be omitted"); + assert!( + json.contains("\"env_vars\""), + "serialized JSON must always include env_vars; got: {json}" + ); + assert!( + json.contains("\"provider\""), + "serialized JSON must always include provider; got: {json}" + ); + assert!( + json.contains("\"model\""), + "serialized JSON must always include model; got: {json}" + ); } // ── resolve_effective_model_provider ───────────────────────────────────────── @@ -365,3 +377,25 @@ fn resolve_each_field_resolves_independently_through_tiers() { "persona wins for provider when record has none" ); } + +// ── IPC serialization ───────────────────────────────────────────────────────── + +/// A fully-populated `GlobalAgentConfig` must round-trip through JSON without +/// loss. +#[test] +fn populated_global_config_round_trips() { + let original = GlobalAgentConfig { + env_vars: [("ANTHROPIC_API_KEY".to_string(), "sk-test".to_string())] + .into_iter() + .collect(), + provider: Some("anthropic".to_string()), + model: Some("claude-opus-4-5".to_string()), + }; + let json = serde_json::to_string(&original).expect("serialization must not fail"); + let decoded: GlobalAgentConfig = + serde_json::from_str(&json).expect("deserialization must not fail"); + assert_eq!( + decoded, original, + "populated config must round-trip losslessly" + ); +} diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index c1c0bdbbe..25fc7457f 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -23,6 +23,7 @@ import { useManagedAgentActions } from "./useManagedAgentActions"; import { usePersonaActions } from "./usePersonaActions"; import { useTeamActions } from "./useTeamActions"; import { useProfilePanel } from "@/shared/context/ProfilePanelContext"; +import { GlobalAgentConfigSettingsCard } from "@/features/settings/ui/GlobalAgentConfigSettingsCard"; export function AgentsView() { const { openPersonaProfilePanel, openProfilePanel } = useProfilePanel(); @@ -149,6 +150,8 @@ export function AgentsView() { teams={teamActions.teams} /> + + 0 || globalValue.length > 0) { // Set in Buzz env or global defaults — satisfied, no action. } else if (fileSatisfiedKeys.has(key)) { diff --git a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx index 4b8f368ed..166807cfe 100644 --- a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx +++ b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx @@ -40,6 +40,7 @@ export function GlobalAgentConfigSettingsCard() { const [saveState, setSaveState] = React.useState("idle"); const [saveError, setSaveError] = React.useState(null); const [isLoading, setIsLoading] = React.useState(true); + const [loadError, setLoadError] = React.useState(false); const [isCustomProvider, setIsCustomProvider] = React.useState(false); const savedTimerRef = React.useRef | null>( null, @@ -56,7 +57,10 @@ export function GlobalAgentConfigSettingsCard() { } }) .catch(() => { - if (!cancelled) setIsLoading(false); + if (!cancelled) { + setIsLoading(false); + setLoadError(true); + } }); return () => { cancelled = true; @@ -135,6 +139,11 @@ export function GlobalAgentConfigSettingsCard() { Loading… + ) : loadError ? ( +
+ + Failed to load agent defaults. Restart the app to try again. +
) : ( {/* Provider field */} @@ -207,7 +216,7 @@ export function GlobalAgentConfigSettingsCard() { )} {/* Save bar */} - {!isLoading && ( + {!isLoading && !loadError && (