jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-27 12:40:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(config-bridge): add harness-definition env tier and fix equal-value model override (#3580)

Browse Source 
All seven normalized config fields resolve through sanitized
`InheritedConfigTiers` passed wholesale to `read_config_surface`. The
reader's precedence tiers now match spawn's Layer 2b exactly — including
harness-definition env — and the equal-value model-override regression
is fixed.

## Changes

**`config_bridge/types.rs`** — add `InheritedConfigTiers`: persona env,
global env, harness definition env, structured model/provider/prompt for
both tiers. Add `HarnessDefault` `ConfigOrigin` variant for
harness-definition env values.

**`commands/agent_config.rs`** — `build_inherited_tiers` now resolves
the harness definition env using the same lookup path as spawn
(`record.runtime` → `persona.runtime` → empty string) and applies
`sanitize_inherited_env` to it. `resolve_config_surface` is unchanged in
shape — tiers passed to the reader now include `definition_env`.

**`config_bridge/reader.rs`** — `env_candidates` extended to 4-element
return (record, persona, global, definition). All five field builders
that use env candidates now include the definition-env slot below global
env and above the structured block, matching spawn Layer 2b. Magic
`configured[..6]` slice replaced with `configured[..configured.len()-1]`
(named split: all non-file candidates). Equal-value model-override arm
falls through to the normal resolve path instead of early-returning
`RuntimeOverride`, so the panel shows the baseline origin (e.g.
`BuzzExplicit`) rather than a spurious "Live override" label for a no-op
switch.

**`config_bridge/reader_tests_ext.rs`** — three new Layer 2b tests:
definition env beats structured persona model, global env beats
definition env, reserved-key-absent fallthrough.

**`commands/agent_config_tests.rs`** —
`genuine_explicit_live_switch_to_same_model_yields_clean_field` updated
to assert `origin == BuzzExplicit` (not `RuntimeOverride`); wrapped in
`with_no_goose_config` for hermeticity. New
`reserved_key_in_definition_env_shaped_map_is_stripped_by_sanitize` test
pins the shared sanitization contract.

**`AgentConfigPanel.tsx` / `types.ts`** — `HarnessDefault` origin
variant wired end-to-end: TS union type and provenance sentence
("Inherited from harness definition").

---------

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
Will Pfleger
2026-08-03 11:04:49 -04:00
committed by GitHub
parent 857e63c4dd
commit be95a8a986
10 changed files with 1676 additions and 1059 deletions
Download Patch File Download Diff File Expand all files Collapse all files

765
desktop/src-tauri/src/commands/agent_config.rs
View File

@@ -8,14 +8,14 @@ use crate::{
read_goose_file_config,
reader::read_config_surface,
types::{
AcpConfigOptionEntry, AcpConfigOptionValue, AcpModelEntry, ConfigOrigin,
NormalizedField, RuntimeConfigSurface, SessionConfigCache,
AcpConfigOptionEntry, AcpConfigOptionValue, AcpModelEntry, InheritedConfigTiers,
RuntimeConfigSurface, SessionConfigCache,
},
},
current_instance_id, known_acp_runtime, load_managed_agents, load_personas,
resolve_effective_prompt_model_provider, save_managed_agents, sync_managed_agent_processes,
current_instance_id, is_reserved_env_key, is_well_formed_env_key, known_acp_runtime,
load_managed_agents, load_personas, save_managed_agents, sync_managed_agent_processes,
AgentDefinition, GlobalAgentConfig, KnownAcpRuntime, ManagedAgentRecord,
ManagedAgentRuntimeKey,
ManagedAgentRuntimeKey, MAX_ENV_VALUE_BYTES,
},
};
@@ -36,28 +36,86 @@ pub struct RuntimeFileConfigSubset {
pub satisfied_env_keys: Vec<String>,
}
/// Resolve the config surface with persona and global default values applied.
/// Sanitize a raw env map from an inherited tier (persona or global) with the
/// same rules `merged_user_env` applies at spawn time: reserved keys, malformed
/// keys, NUL-byte values, and oversize values are stripped silently.
fn sanitize_inherited_env(
raw: &std::collections::BTreeMap<String, String>,
) -> std::collections::BTreeMap<String, String> {
raw.iter()
.filter(|(k, v)| {
!is_reserved_env_key(k)
&& is_well_formed_env_key(k)
&& !v.contains('\0')
&& v.len() <= MAX_ENV_VALUE_BYTES
})
.map(|(k, v)| (k.clone(), v.clone()))
.collect()
}
/// Normalize a structured field value: blank/whitespace-only collapses to
/// `None`, matching `effective_config`'s `non_blank` helper.
fn non_blank(v: Option<&str>) -> Option<String> {
v.filter(|s| !s.trim().is_empty()).map(str::to_owned)
}
/// Build a sanitized `InheritedConfigTiers` snapshot at the command boundary.
///
/// Linked instances are definition-authoritative: the record's own
/// system_prompt/model/provider are cleared before applying, so a stale
/// materialized snapshot can never shadow the persona's current values or a
/// blank-definition fallthrough to global defaults (mirrors
/// `effective_config::resolve_linked`). Definition-less instances keep their
/// own explicit values.
/// Persona env, global env, and harness definition env are sanitized with
/// spawn-equivalent rules. Structured fields are normalized (blank → None).
/// A missing persona (orphaned link) yields empty persona tiers — the panel
/// still renders from record/global while spawn independently refuses.
fn build_inherited_tiers(
record_persona_id: Option<&str>,
record_runtime: Option<&str>,
personas: &[AgentDefinition],
global: &GlobalAgentConfig,
) -> InheritedConfigTiers {
let persona = record_persona_id.and_then(|pid| personas.iter().find(|p| p.id == pid));
let persona_env = persona
.map(|p| sanitize_inherited_env(&p.env_vars))
.unwrap_or_default();
let global_env = sanitize_inherited_env(&global.env_vars);
// Definition env: same resolution as spawn (record.runtime → persona.runtime → "").
// Reserved keys stripped; no malformed-key / NUL / oversize check needed because
// harness definitions are local admin-authored JSON, not user-provided data — but
// we apply `sanitize_inherited_env` for defense-in-depth (same rules as the other tiers).
let definition_env = {
let runtime_id = record_runtime
.or_else(|| persona.and_then(|p| p.runtime.as_deref()))
.unwrap_or("");
crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id)
.map(|def| sanitize_inherited_env(&def.env))
.unwrap_or_default()
};
let persona_model = persona.and_then(|p| non_blank(p.model.as_deref()));
let persona_provider = persona.and_then(|p| non_blank(p.provider.as_deref()));
let persona_prompt = persona.and_then(|p| non_blank(Some(&p.system_prompt)));
let global_model = non_blank(global.model.as_deref());
let global_provider = non_blank(global.provider.as_deref());
InheritedConfigTiers {
persona_env,
global_env,
definition_env,
persona_model,
persona_provider,
persona_prompt,
global_model,
global_provider,
}
}
/// Resolve the config surface with inherited persona and global tiers 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
/// Buzz keeps `had_* == true` and is never re-tagged.
/// Persona-linked instances have their system_prompt/model/provider cleared
/// first (definition-authoritative): stale materialized snapshots can never
/// shadow live persona values. The reader then resolves each field through its
/// full candidate list (record env > ACP > persona env > global env > structured
/// persona/global > config file) via `resolve_with_override`.
fn resolve_config_surface(
mut record: ManagedAgentRecord,
personas: &[AgentDefinition],
@@ -65,146 +123,23 @@ fn resolve_config_surface(
session_cache: Option<&SessionConfigCache>,
global: &GlobalAgentConfig,
) -> RuntimeConfigSurface {
// Linked instances are definition-authoritative (mirrors
// `effective_config::resolve_linked`): the record's own
// system_prompt/model/provider fields are, at best, a stale materialized
// snapshot from the last `apply_persona_snapshot` — never a legitimate
// live override, since `update_managed_agent` blocks writing these three
// fields for linked instances. Clear them before computing `had_*` below
// so a stale byte can never masquerade as BuzzExplicit and suppress
// definition/global injection. Env var overrides (set via the advanced
// env-vars editor) are untouched — those remain a legitimate
// per-instance override regardless of link status.
// Linked instances are definition-authoritative: clear stale materialized
// model/provider/prompt so they can never masquerade as BuzzExplicit and
// shadow definition values. Env var overrides are untouched.
if record.persona_id.is_some() {
record.system_prompt = None;
record.model = None;
record.provider = None;
}
let had_prompt =
record.system_prompt.is_some() || record.env_vars.contains_key("BUZZ_ACP_SYSTEM_PROMPT");
let had_model = record.model.is_some();
let provider_env_key = runtime_meta.and_then(|m| m.provider_env_var).unwrap_or("");
let had_provider = record.env_vars.contains_key(provider_env_key);
let (persona_prompt, persona_model, persona_provider) = resolve_effective_prompt_model_provider(
let tiers = build_inherited_tiers(
record.persona_id.as_deref(),
record.runtime.as_deref(),
personas,
record.system_prompt.clone(),
record.model.clone(),
record.provider.clone(),
global,
);
// Build the baseline the reader overrides a live model against, paired with
// its true origin so the secondary is tagged correctly. Two sources:
// - persona-linked, no explicit record model: the persona model is the
// baseline (PersonaDefault).
// - genuine-explicit (record had its own model) that live-switched: the
// record's own model is the baseline (BuzzExplicit). Gated behind
// `model_overridden` so a persona edited mid-life (override flag false)
// never synthesizes a baseline and false-positives an override.
// An explicit pick with no live switch has no baseline to override.
let model_overridden = session_cache.is_some_and(|c| c.model_overridden);
let baseline = if had_model {
if model_overridden {
record
.model
.clone()
.map(|m| (m, ConfigOrigin::BuzzExplicit))
} else {
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.
if !had_prompt {
if let Some(p) = persona_prompt {
record
.env_vars
.insert("BUZZ_ACP_SYSTEM_PROMPT".to_string(), p);
}
}
if !had_model {
record.model = persona_model.clone();
}
if !had_provider && !provider_env_key.is_empty() {
if let Some(prov) = persona_provider {
record.env_vars.insert(provider_env_key.to_string(), prov);
}
}
// 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,
session_cache,
baseline.as_ref().map(|(m, o)| (m.as_str(), o.clone())),
);
// Re-tag persona-sourced fields from BuzzExplicit to PersonaDefault.
if !had_prompt {
retag_persona_default(&mut surface.normalized.system_prompt);
}
if !had_model && !inject_global_model {
retag_persona_default(&mut surface.normalized.model);
}
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);
}
surface
}
/// Re-tag a field's origin from `BuzzExplicit` to `PersonaDefault`, leaving any
/// other origin untouched. No-op when the field is absent.
fn retag_persona_default(field: &mut Option<NormalizedField>) {
if let Some(field) = field {
if field.origin == ConfigOrigin::BuzzExplicit {
field.origin = ConfigOrigin::PersonaDefault;
}
}
read_config_surface(&record, runtime_meta, session_cache, &tiers)
}
/// Get the file-layer config for a runtime — used by the Create/Edit/Persona
@@ -327,16 +262,6 @@ pub fn get_baked_build_env() -> Vec<BakedEnvEntry> {
.collect()
}
/// Re-tag a field's origin from `BuzzExplicit` to `GlobalDefault`, leaving any
/// other origin untouched. No-op when the field is absent.
fn retag_global_default(field: &mut Option<NormalizedField>) {
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.
@@ -601,511 +526,5 @@ fn parse_models(raw: Option<&serde_json::Value>) -> (Vec<AcpModelEntry>, Option<
}
#[cfg(test)]
mod tests {
use super::*;
use crate::managed_agents::{BackendKind, RespondTo};
fn goose_runtime() -> &'static KnownAcpRuntime {
&KnownAcpRuntime {
id: "goose",
label: "Goose",
commands: &["goose"],
aliases: &[],
avatar_url: "",
mcp_command: None,
mcp_hooks: false,
underlying_cli: None,
cli_install_commands: &[],
cli_install_commands_windows: &[],
adapter_install_commands: &[],
cli_install_instructions_url: "",
adapter_install_instructions_url: "",
cli_install_hint: "",
adapter_install_hint: "",
skill_dir: None,
supports_acp_model_switching: false,
model_env_var: Some("GOOSE_MODEL"),
provider_env_var: Some("GOOSE_PROVIDER"),
provider_locked: false,
default_env: &[],
config_file_path: Some("~/.config/goose/config.yaml"),
config_file_format: Some("yaml"),
supports_acp_native_config: true,
thinking_env_var: Some("GOOSE_THINKING_EFFORT"),
max_tokens_env_var: Some("GOOSE_MAX_TOKENS"),
context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"),
required_normalized_fields: &["model", "provider"],
login_hint: None,
auth_probe_args: None,
}
}
fn agent_record() -> ManagedAgentRecord {
ManagedAgentRecord {
pubkey: "agent".to_string(),
name: "Agent".to_string(),
persona_id: Some("persona-1".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_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,
env_vars: Default::default(),
start_on_app_launch: false,
auto_restart_on_config_change: true,
runtime_pid: None,
backend: BackendKind::Local,
backend_agent_id: None,
provider_binary_path: None,
team_id: 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,
last_error_code: None,
respond_to: RespondTo::OwnerOnly,
respond_to_allowlist: vec![],
display_name: None,
slug: None,
runtime: None,
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
shared: false,
source_team: None,
source_team_persona_slug: None,
catalog_source: None,
definition_respond_to: None,
definition_respond_to_allowlist: Vec::new(),
definition_parallelism: None,
relay_mesh: None,
agent_command_override: None,
persona_source_version: None,
provider: None,
}
}
fn persona_with_model(model: &str) -> AgentDefinition {
AgentDefinition {
id: "persona-1".to_string(),
display_name: "Persona".to_string(),
avatar_url: None,
system_prompt: "You are a persona.".to_string(),
runtime: None,
model: Some(model.to_string()),
provider: None,
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
shared: false,
source_team: None,
source_team_persona_slug: None,
catalog_source: None,
env_vars: Default::default(),
respond_to: None,
respond_to_allowlist: Vec::new(),
parallelism: None,
created_at: "".to_string(),
updated_at: "".to_string(),
}
}
/// A post-spawn session cache whose live model is `current_model` and whose
/// `model_overridden` flag records whether a `SwitchModel` control signal set
/// it (the live-switch signal).
fn session_cache(current_model: &str, model_overridden: bool) -> SessionConfigCache {
SessionConfigCache {
config_options: vec![],
available_modes: vec![],
available_models: vec![],
current_model: Some(current_model.to_string()),
model_overridden,
goose_native_config: None,
captured_at: "".to_string(),
}
}
/// Definition-authoritative: a stale materialized `record.model` on a
/// linked instance must never outrank (or even be consulted against) the
/// linked persona's model. `update_managed_agent` already blocks writing
/// model/provider/prompt for linked instances, so a non-`None` value here
/// can only be leftover snapshot bytes from before a persona edit — the
/// panel must report the persona's current model, tagged `PersonaDefault`,
/// not the stale byte as `BuzzExplicit`.
#[test]
fn linked_stale_record_model_never_outranks_persona_model() {
let mut record = agent_record();
record.model = Some("stale-explicit-model".to_string());
let personas = vec![persona_with_model("persona-model")];
let surface = resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
None,
&Default::default(),
);
let model = surface.normalized.model.as_ref().expect("model resolved");
assert_eq!(model.value.as_deref(), Some("persona-model"));
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
}
/// Definition-authoritative, blank-definition case: a linked instance
/// whose persona has no model of its own must fall through to the global
/// default, tagged `GlobalDefault` — mirroring
/// `effective_config::resolve_linked`'s `None => global` arm. A stale
/// materialized record model must not shadow this fallthrough either.
#[test]
fn linked_blank_definition_model_falls_through_to_global_default() {
let mut record = agent_record();
record.model = Some("stale-explicit-model".to_string());
let mut persona = persona_with_model("unused");
persona.model = None;
let personas = vec![persona];
let global = crate::managed_agents::GlobalAgentConfig {
model: Some("global-model".to_string()),
..Default::default()
};
let surface =
resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global);
let model = surface.normalized.model.as_ref().expect("model resolved");
assert_eq!(model.value.as_deref(), Some("global-model"));
assert_eq!(model.origin, ConfigOrigin::GlobalDefault);
}
/// A definition-less (no `persona_id`) instance's own explicit model IS
/// authoritative — the stale-record clearing above is scoped to linked
/// instances only.
#[test]
fn definition_less_explicit_record_model_keeps_buzz_explicit_origin() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("explicit-model".to_string());
let personas = vec![persona_with_model("persona-model")];
let surface = resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
None,
&Default::default(),
);
let model = surface.normalized.model.as_ref().expect("model resolved");
assert_eq!(model.value.as_deref(), Some("explicit-model"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
}
/// Part A — pending-pick: a genuine-explicit pick X with a divergent live
/// model Y but `model_overridden == false` (the live switch is not yet
/// applied — a restart is pending) must keep X as the primary and must NOT
/// surface Y as an override row. The live `acp_model` does not win. This
/// FAILS against a let-live-acp-win variant (one that dropped the
/// `model_overridden` gate), so it is not vacuous.
#[test]
fn pending_pick_keeps_explicit_x_and_does_not_surface_live_y() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("model-x".to_string());
let personas: Vec<AgentDefinition> = vec![];
let cache = session_cache("model-y", false);
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"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
assert_ne!(model.origin, ConfigOrigin::RuntimeOverride);
assert_ne!(model.overridden_value.as_deref(), Some("model-y"));
}
/// W2 — genuine-explicit live switch: record.model = X, no persona,
/// `model_overridden == true`, live model = Y. The live Y must render as the
/// primary with a `RuntimeOverride` origin and X as the secondary tagged
/// `BuzzExplicit` (its true source — NOT `PersonaDefault`). FAILS against the
/// shipped no-persona early-return, which left X as primary and Y struck.
#[test]
fn genuine_explicit_live_switch_renders_y_over_x_buzz_explicit_secondary() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("model-x".to_string());
let personas: Vec<AgentDefinition> = vec![];
let cache = session_cache("model-y", true);
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"));
assert_eq!(model.origin, ConfigOrigin::RuntimeOverride);
assert_eq!(model.overridden_value.as_deref(), Some("model-x"));
assert_eq!(model.overridden_origin, Some(ConfigOrigin::BuzzExplicit));
}
/// Y==X collision: a genuine-explicit agent live-switches to the SAME value
/// it already had. There is no real divergence, so the field must be a clean
/// single value with NO secondary row. FAILS against a naive `return base`
/// that would leak the `AcpConfigOption` row `build_model_field` populates.
#[test]
fn genuine_explicit_live_switch_to_same_model_yields_clean_field() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("model-x".to_string());
let personas: Vec<AgentDefinition> = vec![];
let cache = session_cache("model-x", true);
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"));
assert_eq!(model.overridden_value, None);
assert_eq!(model.overridden_origin, None);
}
/// Persona parity (regression): a persona-linked agent with no explicit
/// record model that live-switches still renders the persona model as the
/// secondary tagged `PersonaDefault` — the typed-baseline change must NOT
/// regress the persona arm to a different origin.
#[test]
fn persona_linked_live_switch_keeps_persona_default_secondary() {
let record = agent_record();
let personas = vec![persona_with_model("persona-model")];
let cache = session_cache("model-y", true);
let surface = resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
Some(&cache),
&Default::default(),
);
let model = surface.normalized.model.expect("model resolved");
assert_eq!(model.value.as_deref(), Some("model-y"));
assert_eq!(model.origin, ConfigOrigin::RuntimeOverride);
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<AgentDefinition> = 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"
);
}
// ── get_baked_build_env / is_secret_key tests ──────────────────────────
/// Build a `BakedEnvEntry` vec from a synthetic map, mirroring what
/// `get_baked_build_env()` does. Used to test masking without relying on
/// compile-time `option_env!` vars (OSS builds have empty `baked_build_env`).
fn baked_env_from_map(map: &[(&str, &str)]) -> Vec<BakedEnvEntry> {
map.iter()
.filter(|(_, v)| !v.is_empty())
.map(|(k, v)| {
let masked = !super::is_safe_to_reveal(k);
BakedEnvEntry {
key: k.to_string(),
value: if masked {
"\u{2022}\u{2022}\u{2022}\u{2022}\u{2022}\u{2022}".to_string()
} else {
v.to_string()
},
masked,
}
})
.collect()
}
#[test]
fn baked_env_non_secret_key_shows_real_value() {
let entries = baked_env_from_map(&[("BUZZ_AGENT_PROVIDER", "databricks_v2")]);
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].key, "BUZZ_AGENT_PROVIDER");
assert_eq!(entries[0].value, "databricks_v2");
assert!(!entries[0].masked);
}
#[test]
fn baked_env_api_key_is_masked() {
let entries = baked_env_from_map(&[("ANTHROPIC_API_KEY", "sk-secret")]);
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].value, "••••••");
assert!(entries[0].masked);
}
#[test]
fn baked_env_token_key_is_masked() {
let entries = baked_env_from_map(&[("GITHUB_TOKEN", "ghp_secret")]);
assert_eq!(entries.len(), 1);
assert!(entries[0].masked);
}
#[test]
fn baked_env_secret_key_is_masked() {
let entries = baked_env_from_map(&[("MY_DB_SECRET", "s3cr3t")]);
assert_eq!(entries.len(), 1);
assert!(entries[0].masked);
}
#[test]
fn baked_env_password_key_is_masked() {
let entries = baked_env_from_map(&[("DB_PASSWORD", "hunter2")]);
assert_eq!(entries.len(), 1);
assert!(entries[0].masked);
}
#[test]
fn baked_env_empty_value_filtered_out() {
let entries = baked_env_from_map(&[("BUZZ_AGENT_PROVIDER", "")]);
assert!(entries.is_empty());
}
#[test]
fn baked_env_mixed_keys_correct_masking() {
let entries = baked_env_from_map(&[
("BUZZ_AGENT_PROVIDER", "databricks_v2"),
("BUZZ_AGENT_MODEL", "goose-claude-opus-4-8"),
("DATABRICKS_HOST", "https://example.com"),
("DATABRICKS_TOKEN", "dapi-secret"),
]);
assert_eq!(entries.len(), 4);
let provider = entries
.iter()
.find(|e| e.key == "BUZZ_AGENT_PROVIDER")
.unwrap();
assert_eq!(provider.value, "databricks_v2");
assert!(!provider.masked);
let model = entries
.iter()
.find(|e| e.key == "BUZZ_AGENT_MODEL")
.unwrap();
assert_eq!(model.value, "goose-claude-opus-4-8");
assert!(!model.masked);
let host = entries.iter().find(|e| e.key == "DATABRICKS_HOST").unwrap();
assert_eq!(host.value, "https://example.com");
assert!(!host.masked);
let token = entries
.iter()
.find(|e| e.key == "DATABRICKS_TOKEN")
.unwrap();
assert_eq!(token.value, "••••••");
assert!(token.masked);
}
#[test]
fn baked_env_thinking_effort_is_unmasked() {
// BUZZ_AGENT_THINKING_EFFORT is a non-secret enum — must not be masked.
let entries = baked_env_from_map(&[("BUZZ_AGENT_THINKING_EFFORT", "medium")]);
assert_eq!(entries.len(), 1);
let effort = entries
.iter()
.find(|e| e.key == "BUZZ_AGENT_THINKING_EFFORT")
.unwrap();
assert_eq!(effort.value, "medium");
assert!(!effort.masked);
}
#[test]
fn baked_env_allowlist_is_case_insensitive() {
// Known-safe keys — case-insensitive match must allow them.
assert!(super::is_safe_to_reveal("buzz_agent_provider"));
assert!(super::is_safe_to_reveal("BUZZ_AGENT_PROVIDER"));
assert!(super::is_safe_to_reveal("buzz_agent_model"));
assert!(super::is_safe_to_reveal("BUZZ_AGENT_MODEL"));
assert!(super::is_safe_to_reveal("buzz_agent_thinking_effort"));
assert!(super::is_safe_to_reveal("BUZZ_AGENT_THINKING_EFFORT"));
assert!(super::is_safe_to_reveal("databricks_host"));
assert!(super::is_safe_to_reveal("DATABRICKS_HOST"));
assert!(super::is_safe_to_reveal("databricks_model"));
assert!(super::is_safe_to_reveal("DATABRICKS_MODEL"));
// Keys NOT in the allowlist — masked regardless of naming pattern.
assert!(!super::is_safe_to_reveal("my_api_key"));
assert!(!super::is_safe_to_reveal("GITHUB_TOKEN"));
assert!(!super::is_safe_to_reveal("DB_SECRET"));
assert!(!super::is_safe_to_reveal("DB_PASSWORD"));
// Bare names that old heuristic (contains("_TOKEN") etc.) would have missed.
assert!(!super::is_safe_to_reveal("APIKEY"));
assert!(!super::is_safe_to_reveal("TOKEN"));
assert!(!super::is_safe_to_reveal("SECRET"));
assert!(!super::is_safe_to_reveal("PASSWORD"));
assert!(!super::is_safe_to_reveal("PRIVATE_KEY"));
// Unknown key → masked by default.
assert!(!super::is_safe_to_reveal("SOME_UNKNOWN_KEY"));
}
}
#[path = "agent_config_tests.rs"]
mod tests;

651
desktop/src-tauri/src/commands/agent_config_tests.rs Normal file
View File

@@ -0,0 +1,651 @@
//! Unit tests for `commands/agent_config.rs` (split to keep `agent_config.rs`
//! under the 1000-line file-size ratchet).
//!
//! Included via `#[path = "agent_config_tests.rs"] mod tests;` at the bottom of
//! `agent_config.rs`, so `use super::*` gives access to all items in that module.
use super::*;
use crate::managed_agents::config_bridge::types::ConfigOrigin;
use crate::managed_agents::{BackendKind, RespondTo};
use std::sync::Mutex;
static GOOSE_PATH_ROOT_LOCK: Mutex<()> = Mutex::new(());
/// Run a test body with GOOSE_PATH_ROOT set to a non-existent path so that the
/// goose config file read returns `None`. Restores the prior value on exit.
fn with_no_goose_config<T>(body: impl FnOnce() -> T) -> T {
let _guard = GOOSE_PATH_ROOT_LOCK
.lock()
.unwrap_or_else(|err| err.into_inner());
let prior = std::env::var_os("GOOSE_PATH_ROOT");
std::env::set_var("GOOSE_PATH_ROOT", "/nonexistent-buzz-test-path");
let output = body();
match prior {
Some(value) => std::env::set_var("GOOSE_PATH_ROOT", value),
None => std::env::remove_var("GOOSE_PATH_ROOT"),
}
output
}
fn goose_runtime() -> &'static KnownAcpRuntime {
&KnownAcpRuntime {
id: "goose",
label: "Goose",
commands: &["goose"],
aliases: &[],
avatar_url: "",
mcp_command: None,
mcp_hooks: false,
underlying_cli: None,
cli_install_commands: &[],
cli_install_commands_windows: &[],
adapter_install_commands: &[],
cli_install_instructions_url: "",
adapter_install_instructions_url: "",
cli_install_hint: "",
adapter_install_hint: "",
skill_dir: None,
supports_acp_model_switching: false,
model_env_var: Some("GOOSE_MODEL"),
provider_env_var: Some("GOOSE_PROVIDER"),
provider_locked: false,
default_env: &[],
config_file_path: Some("~/.config/goose/config.yaml"),
config_file_format: Some("yaml"),
supports_acp_native_config: true,
thinking_env_var: Some("GOOSE_THINKING_EFFORT"),
max_tokens_env_var: Some("GOOSE_MAX_TOKENS"),
context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"),
required_normalized_fields: &["model", "provider"],
login_hint: None,
auth_probe_args: None,
}
}
fn agent_record() -> ManagedAgentRecord {
ManagedAgentRecord {
pubkey: "agent".to_string(),
name: "Agent".to_string(),
persona_id: Some("persona-1".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_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,
env_vars: Default::default(),
start_on_app_launch: false,
auto_restart_on_config_change: true,
runtime_pid: None,
backend: BackendKind::Local,
backend_agent_id: None,
provider_binary_path: None,
team_id: 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,
last_error_code: None,
respond_to: RespondTo::OwnerOnly,
respond_to_allowlist: vec![],
display_name: None,
slug: None,
runtime: None,
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
shared: false,
source_team: None,
source_team_persona_slug: None,
catalog_source: None,
definition_respond_to: None,
definition_respond_to_allowlist: Vec::new(),
definition_parallelism: None,
relay_mesh: None,
agent_command_override: None,
persona_source_version: None,
provider: None,
}
}
fn persona_with_model(model: &str) -> AgentDefinition {
AgentDefinition {
id: "persona-1".to_string(),
display_name: "Persona".to_string(),
avatar_url: None,
system_prompt: "You are a persona.".to_string(),
runtime: None,
model: Some(model.to_string()),
provider: None,
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
shared: false,
source_team: None,
source_team_persona_slug: None,
catalog_source: None,
env_vars: Default::default(),
respond_to: None,
respond_to_allowlist: Vec::new(),
parallelism: None,
created_at: "".to_string(),
updated_at: "".to_string(),
}
}
/// A post-spawn session cache whose live model is `current_model` and whose
/// `model_overridden` flag records whether a `SwitchModel` control signal set
/// it (the live-switch signal).
fn session_cache(current_model: &str, model_overridden: bool) -> SessionConfigCache {
SessionConfigCache {
config_options: vec![],
available_modes: vec![],
available_models: vec![],
current_model: Some(current_model.to_string()),
model_overridden,
goose_native_config: None,
captured_at: "".to_string(),
}
}
/// Definition-authoritative: a stale materialized `record.model` on a
/// linked instance must never outrank (or even be consulted against) the
/// linked persona's model. `update_managed_agent` already blocks writing
/// model/provider/prompt for linked instances, so a non-`None` value here
/// can only be leftover snapshot bytes from before a persona edit — the
/// panel must report the persona's current model, tagged `PersonaDefault`,
/// not the stale byte as `BuzzExplicit`.
#[test]
fn linked_stale_record_model_never_outranks_persona_model() {
let mut record = agent_record();
record.model = Some("stale-explicit-model".to_string());
let personas = vec![persona_with_model("persona-model")];
let surface = resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
None,
&Default::default(),
);
let model = surface.normalized.model.as_ref().expect("model resolved");
assert_eq!(model.value.as_deref(), Some("persona-model"));
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
}
/// Definition-authoritative, blank-definition case: a linked instance
/// whose persona has no model of its own must fall through to the global
/// default, tagged `GlobalDefault` — mirroring
/// `effective_config::resolve_linked`'s `None => global` arm. A stale
/// materialized record model must not shadow this fallthrough either.
#[test]
fn linked_blank_definition_model_falls_through_to_global_default() {
let mut record = agent_record();
record.model = Some("stale-explicit-model".to_string());
let mut persona = persona_with_model("unused");
persona.model = None;
let personas = vec![persona];
let global = crate::managed_agents::GlobalAgentConfig {
model: Some("global-model".to_string()),
..Default::default()
};
let surface = resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global);
let model = surface.normalized.model.as_ref().expect("model resolved");
assert_eq!(model.value.as_deref(), Some("global-model"));
assert_eq!(model.origin, ConfigOrigin::GlobalDefault);
}
/// A definition-less (no `persona_id`) instance's own explicit model IS
/// authoritative — the stale-record clearing above is scoped to linked
/// instances only.
#[test]
fn definition_less_explicit_record_model_keeps_buzz_explicit_origin() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("explicit-model".to_string());
let personas = vec![persona_with_model("persona-model")];
let surface = resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
None,
&Default::default(),
);
let model = surface.normalized.model.as_ref().expect("model resolved");
assert_eq!(model.value.as_deref(), Some("explicit-model"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
}
/// Part A — pending-pick: a genuine-explicit pick X with a divergent live
/// model Y but `model_overridden == false` (the live switch is not yet
/// applied — a restart is pending) must keep X as the primary and must NOT
/// surface Y as an override row. The live `acp_model` does not win. This
/// FAILS against a let-live-acp-win variant (one that dropped the
/// `model_overridden` gate), so it is not vacuous.
#[test]
fn pending_pick_keeps_explicit_x_and_does_not_surface_live_y() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("model-x".to_string());
let personas: Vec<AgentDefinition> = vec![];
let cache = session_cache("model-y", false);
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"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
assert_ne!(model.origin, ConfigOrigin::RuntimeOverride);
assert_ne!(model.overridden_value.as_deref(), Some("model-y"));
}
/// W2 — genuine-explicit live switch: record.model = X, no persona,
/// `model_overridden == true`, live model = Y. The live Y must render as the
/// primary with a `RuntimeOverride` origin and X as the secondary tagged
/// `BuzzExplicit` (its true source — NOT `PersonaDefault`). FAILS against the
/// shipped no-persona early-return, which left X as primary and Y struck.
#[test]
fn genuine_explicit_live_switch_renders_y_over_x_buzz_explicit_secondary() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("model-x".to_string());
let personas: Vec<AgentDefinition> = vec![];
let cache = session_cache("model-y", true);
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"));
assert_eq!(model.origin, ConfigOrigin::RuntimeOverride);
assert_eq!(model.overridden_value.as_deref(), Some("model-x"));
assert_eq!(model.overridden_origin, Some(ConfigOrigin::BuzzExplicit));
}
/// Y==X collision: a genuine-explicit agent live-switches to the SAME value
/// it already had. There is no real divergence, so the field must be a clean
/// single value with NO secondary row and origin matching the baseline (not
/// RuntimeOverride). FAILS against a naive `return base` that would leak the
/// `AcpConfigOption` row `build_model_field` populates, and against the
/// prior implementation that stamped `RuntimeOverride` on the equal-value arm.
///
/// `with_no_goose_config` suppresses the goose config file read so that the
/// fall-through to normal resolution cannot pick up a local `~/.config/goose/config.yaml`
/// model as a spurious secondary — the test is about tier precedence, not the
/// local developer's goose install.
#[test]
fn genuine_explicit_live_switch_to_same_model_yields_clean_field() {
let mut record = agent_record();
record.persona_id = None;
record.model = Some("model-x".to_string());
let personas: Vec<AgentDefinition> = vec![];
let cache = session_cache("model-x", true);
let surface = with_no_goose_config(|| {
resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
Some(&cache),
&Default::default(),
)
});
let model = surface.normalized.model.expect("model resolved");
assert_eq!(model.value.as_deref(), Some("model-x"));
// Equal-value switch must NOT stamp RuntimeOverride — baseline origin wins.
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
assert_ne!(model.origin, ConfigOrigin::RuntimeOverride);
assert_eq!(model.overridden_value, None);
assert_eq!(model.overridden_origin, None);
}
/// Persona parity (regression): a persona-linked agent with no explicit
/// record model that live-switches still renders the persona model as the
/// secondary tagged `PersonaDefault` — the typed-baseline change must NOT
/// regress the persona arm to a different origin.
#[test]
fn persona_linked_live_switch_keeps_persona_default_secondary() {
let record = agent_record();
let personas = vec![persona_with_model("persona-model")];
let cache = session_cache("model-y", true);
let surface = resolve_config_surface(
record,
&personas,
Some(goose_runtime()),
Some(&cache),
&Default::default(),
);
let model = surface.normalized.model.expect("model resolved");
assert_eq!(model.value.as_deref(), Some("model-y"));
assert_eq!(model.origin, ConfigOrigin::RuntimeOverride);
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<AgentDefinition> = 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"
);
}
// ── Snapshot constructor tests (build_inherited_tiers) ──────────────────────
//
// These test the sanitized snapshot constructor — the command-boundary
// function that builds InheritedConfigTiers from raw persona/global data.
/// Orphaned persona link: a record whose persona_id references a non-existent
/// persona should produce empty persona tiers (not a panic), and the panel
/// still renders from the record and global tiers.
#[test]
fn orphaned_persona_link_yields_empty_persona_tiers() {
let mut record = agent_record();
record.persona_id = Some("missing-persona".to_string());
// No personas in the list — dangling link.
let personas: Vec<AgentDefinition> = vec![];
let global = crate::managed_agents::GlobalAgentConfig {
model: Some("global-model".to_string()),
..Default::default()
};
let tiers = build_inherited_tiers(record.persona_id.as_deref(), None, &personas, &global);
// Persona tier is empty — the orphan yields no persona inheritance.
assert!(tiers.persona_env.is_empty());
assert!(tiers.persona_model.is_none());
assert!(tiers.persona_provider.is_none());
assert!(tiers.persona_prompt.is_none());
// Global tiers are unaffected.
assert_eq!(tiers.global_model.as_deref(), Some("global-model"));
}
/// Reserved key in persona env is stripped by sanitization — it must never
/// reach the reader or the display surface.
#[test]
fn reserved_key_in_inherited_persona_env_is_stripped() {
let mut persona = persona_with_model("model");
// BUZZ_PRIVATE_KEY is a reserved key — must be stripped.
persona
.env_vars
.insert("BUZZ_PRIVATE_KEY".to_string(), "nsec-secret".to_string());
// A safe key — must survive.
persona
.env_vars
.insert("GOOSE_MODEL".to_string(), "persona-model".to_string());
let personas = vec![persona];
let global = crate::managed_agents::GlobalAgentConfig::default();
let tiers = build_inherited_tiers(Some("persona-1"), None, &personas, &global);
assert!(
!tiers.persona_env.contains_key("BUZZ_PRIVATE_KEY"),
"reserved key must be stripped from persona env tier"
);
assert!(
tiers.persona_env.contains_key("GOOSE_MODEL"),
"safe key must survive sanitization"
);
}
/// `sanitize_inherited_env` strips reserved keys from a definition-env-shaped
/// map. This pins the shared sanitization contract for definition_env — the
/// same function is applied to all three env tiers (persona, global, definition)
/// at the command boundary.
#[test]
fn reserved_key_in_definition_env_shaped_map_is_stripped_by_sanitize() {
// Exercise sanitize_inherited_env directly with a definition-env-shaped map.
let mut raw = std::collections::BTreeMap::new();
raw.insert("BUZZ_PRIVATE_KEY".to_string(), "nsec-secret".to_string());
raw.insert("GOOSE_MODEL".to_string(), "harness-model".to_string());
let sanitized = sanitize_inherited_env(&raw);
assert!(
!sanitized.contains_key("BUZZ_PRIVATE_KEY"),
"reserved key must be stripped by sanitize_inherited_env"
);
assert!(
sanitized.contains_key("GOOSE_MODEL"),
"safe key must survive sanitize_inherited_env"
);
}
/// Malformed key in global env is stripped by sanitization — keys must be
/// POSIX-shaped (`[A-Za-z_][A-Za-z0-9_]*`).
#[test]
fn malformed_key_in_inherited_global_env_is_stripped() {
let mut global = crate::managed_agents::GlobalAgentConfig::default();
// Key with an `=` — would bypass env-var security if passed to spawn.
global
.env_vars
.insert("BAD=KEY".to_string(), "value".to_string());
// A valid key — must survive.
global
.env_vars
.insert("GOOSE_PROVIDER".to_string(), "anthropic".to_string());
let tiers = build_inherited_tiers(None, None, &[], &global);
assert!(
!tiers.global_env.contains_key("BAD=KEY"),
"malformed key must be stripped from global env tier"
);
assert!(
tiers.global_env.contains_key("GOOSE_PROVIDER"),
"valid key must survive sanitization"
);
}
// ── get_baked_build_env / is_secret_key tests ──────────────────────────
/// Build a `BakedEnvEntry` vec from a synthetic map, mirroring what
/// `get_baked_build_env()` does. Used to test masking without relying on
/// compile-time `option_env!` vars (OSS builds have empty `baked_build_env`).
fn baked_env_from_map(map: &[(&str, &str)]) -> Vec<BakedEnvEntry> {
map.iter()
.filter(|(_, v)| !v.is_empty())
.map(|(k, v)| {
let masked = !super::is_safe_to_reveal(k);
BakedEnvEntry {
key: k.to_string(),
value: if masked {
"\u{2022}\u{2022}\u{2022}\u{2022}\u{2022}\u{2022}".to_string()
} else {
v.to_string()
},
masked,
}
})
.collect()
}
#[test]
fn baked_env_non_secret_key_shows_real_value() {
let entries = baked_env_from_map(&[("BUZZ_AGENT_PROVIDER", "databricks_v2")]);
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].key, "BUZZ_AGENT_PROVIDER");
assert_eq!(entries[0].value, "databricks_v2");
assert!(!entries[0].masked);
}
#[test]
fn baked_env_api_key_is_masked() {
let entries = baked_env_from_map(&[("ANTHROPIC_API_KEY", "sk-secret")]);
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].value, "••••••");
assert!(entries[0].masked);
}
#[test]
fn baked_env_token_key_is_masked() {
let entries = baked_env_from_map(&[("GITHUB_TOKEN", "ghp_secret")]);
assert_eq!(entries.len(), 1);
assert!(entries[0].masked);
}
#[test]
fn baked_env_secret_key_is_masked() {
let entries = baked_env_from_map(&[("MY_DB_SECRET", "s3cr3t")]);
assert_eq!(entries.len(), 1);
assert!(entries[0].masked);
}
#[test]
fn baked_env_password_key_is_masked() {
let entries = baked_env_from_map(&[("DB_PASSWORD", "hunter2")]);
assert_eq!(entries.len(), 1);
assert!(entries[0].masked);
}
#[test]
fn baked_env_empty_value_filtered_out() {
let entries = baked_env_from_map(&[("BUZZ_AGENT_PROVIDER", "")]);
assert!(entries.is_empty());
}
#[test]
fn baked_env_mixed_keys_correct_masking() {
let entries = baked_env_from_map(&[
("BUZZ_AGENT_PROVIDER", "databricks_v2"),
("BUZZ_AGENT_MODEL", "goose-claude-opus-4-8"),
("DATABRICKS_HOST", "https://example.com"),
("DATABRICKS_TOKEN", "dapi-secret"),
]);
assert_eq!(entries.len(), 4);
let provider = entries
.iter()
.find(|e| e.key == "BUZZ_AGENT_PROVIDER")
.unwrap();
assert_eq!(provider.value, "databricks_v2");
assert!(!provider.masked);
let model = entries
.iter()
.find(|e| e.key == "BUZZ_AGENT_MODEL")
.unwrap();
assert_eq!(model.value, "goose-claude-opus-4-8");
assert!(!model.masked);
let host = entries.iter().find(|e| e.key == "DATABRICKS_HOST").unwrap();
assert_eq!(host.value, "https://example.com");
assert!(!host.masked);
let token = entries
.iter()
.find(|e| e.key == "DATABRICKS_TOKEN")
.unwrap();
assert_eq!(token.value, "••••••");
assert!(token.masked);
}
#[test]
fn baked_env_thinking_effort_is_unmasked() {
// BUZZ_AGENT_THINKING_EFFORT is a non-secret enum — must not be masked.
let entries = baked_env_from_map(&[("BUZZ_AGENT_THINKING_EFFORT", "medium")]);
assert_eq!(entries.len(), 1);
let effort = entries
.iter()
.find(|e| e.key == "BUZZ_AGENT_THINKING_EFFORT")
.unwrap();
assert_eq!(effort.value, "medium");
assert!(!effort.masked);
}
#[test]
fn baked_env_allowlist_is_case_insensitive() {
// Known-safe keys — case-insensitive match must allow them.
assert!(super::is_safe_to_reveal("buzz_agent_provider"));
assert!(super::is_safe_to_reveal("BUZZ_AGENT_PROVIDER"));
assert!(super::is_safe_to_reveal("buzz_agent_model"));
assert!(super::is_safe_to_reveal("BUZZ_AGENT_MODEL"));
assert!(super::is_safe_to_reveal("buzz_agent_thinking_effort"));
assert!(super::is_safe_to_reveal("BUZZ_AGENT_THINKING_EFFORT"));
assert!(super::is_safe_to_reveal("databricks_host"));
assert!(super::is_safe_to_reveal("DATABRICKS_HOST"));
assert!(super::is_safe_to_reveal("databricks_model"));
assert!(super::is_safe_to_reveal("DATABRICKS_MODEL"));
// Keys NOT in the allowlist — masked regardless of naming pattern.
assert!(!super::is_safe_to_reveal("my_api_key"));
assert!(!super::is_safe_to_reveal("GITHUB_TOKEN"));
assert!(!super::is_safe_to_reveal("DB_SECRET"));
assert!(!super::is_safe_to_reveal("DB_PASSWORD"));
// Bare names that old heuristic (contains("_TOKEN") etc.) would have missed.
assert!(!super::is_safe_to_reveal("APIKEY"));
assert!(!super::is_safe_to_reveal("TOKEN"));
assert!(!super::is_safe_to_reveal("SECRET"));
assert!(!super::is_safe_to_reveal("PASSWORD"));
assert!(!super::is_safe_to_reveal("PRIVATE_KEY"));
// Unknown key → masked by default.
assert!(!super::is_safe_to_reveal("SOME_UNKNOWN_KEY"));
}

479
desktop/src-tauri/src/managed_agents/config_bridge/reader.rs
View File

@@ -3,15 +3,17 @@ use crate::managed_agents::types::ManagedAgentRecord;
use super::types::*;
/// Build the full config surface for an agent, merging all four tiers.
/// Build the full config surface for an agent, merging all tiers.
///
/// Pre-spawn (no session cache): tiers 2a (env vars / record) and 2b (config files).
/// Post-spawn (session cache present): adds tiers 1a (ACP native) and 1b (ACP configOptions).
/// Inherited values flow through `tiers` — a sanitized snapshot of the
/// persona and global tiers assembled at the command boundary. Each field
/// builder constructs its own candidate list and resolves via
/// `resolve_with_override`.
pub(crate) fn read_config_surface(
record: &ManagedAgentRecord,
runtime_meta: Option<&KnownAcpRuntime>,
session_cache: Option<&SessionConfigCache>,
baseline: Option<(&str, ConfigOrigin)>,
tiers: &InheritedConfigTiers,
) -> RuntimeConfigSurface {
let is_pre_spawn = session_cache.is_none();
@@ -27,14 +29,7 @@ pub(crate) fn read_config_surface(
})
.unwrap_or_else(|| (RuntimeFileConfig::default(), false));
// Tier 2a: record-level values (Buzz-explicit).
let record_model = record.model.clone();
let record_provider = record
.env_vars
.get(runtime_meta.and_then(|m| m.provider_env_var).unwrap_or(""))
.cloned()
.or_else(|| record.provider.clone()); // structured provider field as fallback
// Runtime-specific env var keys.
let supports_acp_model = runtime_meta.is_some_and(|m| m.supports_acp_model_switching);
let model_env_var = runtime_meta.and_then(|m| m.model_env_var);
let provider_env_var = runtime_meta.and_then(|m| m.provider_env_var);
@@ -48,10 +43,6 @@ pub(crate) fn read_config_surface(
let context_limit_env_var = runtime_meta.and_then(|m| m.context_limit_env_var);
// Tier 1b: ACP configOptions from session cache.
// For unstable/switchable agents, current_model comes from the `models`
// field. For stable agents that only report model via configOptions
// (category="model", current_value), fall back to find_config_option_value
// so their current model is surfaced in the panel.
let acp_model = session_cache.and_then(|c| {
c.current_model
.clone()
@@ -59,61 +50,53 @@ pub(crate) fn read_config_surface(
});
let acp_mode = session_cache.and_then(|c| find_config_option_value(c, "mode"));
let acp_effort = session_cache.and_then(|c| find_config_option_value(c, "effort"));
let record_effort = thinking_env_var
.and_then(|k| record.env_vars.get(k))
.cloned();
let model_overridden = session_cache.is_some_and(|c| c.model_overridden);
let normalized = NormalizedConfig {
model: Some(apply_runtime_override(
build_model_field(
&record_model,
&file_config.model,
&acp_model,
model_env_var,
supports_acp_model,
is_pre_spawn,
session_cache,
required_fields.contains(&"model"),
),
acp_model.as_deref(),
baseline,
model: Some(build_model_field(
record,
&file_config.model,
&acp_model,
model_env_var,
supports_acp_model,
is_pre_spawn,
session_cache,
required_fields.contains(&"model"),
model_overridden,
tiers,
)),
provider: build_provider_field(
&record_provider,
record,
&file_config.provider,
provider_env_var,
provider_locked,
required_fields.contains(&"provider"),
tiers,
),
mode: build_mode_field(&file_config.mode, &acp_mode, is_pre_spawn, session_cache),
thinking_effort: build_thinking_field(
&record_effort,
record,
&file_config.thinking_effort,
&acp_effort,
thinking_env_var,
is_pre_spawn,
session_cache,
tiers,
),
max_output_tokens: build_numeric_env_field(
max_tokens_env_var,
&record.env_vars,
record,
&file_config.max_output_tokens,
tiers,
),
context_limit: build_numeric_env_field(
context_limit_env_var,
&record.env_vars,
record,
&file_config.context_limit,
tiers,
),
system_prompt: build_system_prompt_field(
&record
.system_prompt
.clone()
.or_else(|| record.env_vars.get("BUZZ_ACP_SYSTEM_PROMPT").cloned()),
&file_config.system_prompt,
),
system_prompt: build_system_prompt_field(record, &file_config.system_prompt, tiers),
};
// Advanced fields from config file extras.
@@ -130,7 +113,7 @@ pub(crate) fn read_config_surface(
})
.collect();
// Collect the env var keys already covered by normalized fields so we don't double-surface them.
// Collect the env var keys already covered by normalized fields.
let normalized_env_keys: Vec<&str> = [
model_env_var,
provider_env_var,
@@ -144,15 +127,13 @@ pub(crate) fn read_config_surface(
.collect();
// Tier 2a: remaining env vars not covered by normalized fields.
// Env var wins over config file for the same key (tier 2a > 2b), so skip
// keys already present in file_config.extra.
let mut advanced = advanced;
for (k, v) in &record.env_vars {
if normalized_env_keys.contains(&k.as_str()) {
continue;
}
if file_config.extra.contains_key(k) {
continue; // config file already surfaced this key
continue;
}
advanced.push(ConfigField {
key: k.clone(),
@@ -178,8 +159,6 @@ pub(crate) fn read_config_surface(
{
ConfigTierStatus::Available
} else {
// Post-spawn without native config data is also Pending — it arrives
// asynchronously after the session/new response.
ConfigTierStatus::Pending
}
} else {
@@ -226,9 +205,27 @@ fn mcp_config_file_path_for_runtime(runtime: &KnownAcpRuntime) -> Option<String>
}
}
/// Extract an env-backed candidate value for `env_key` from each tier in
/// spawn precedence: record env > persona env > global env > definition env.
/// Returns `[record, persona, global, definition]` — `None` when key is absent.
fn env_candidates<'a>(
env_key: &str,
record_env: &'a std::collections::BTreeMap<String, String>,
persona_env: &'a std::collections::BTreeMap<String, String>,
global_env: &'a std::collections::BTreeMap<String, String>,
definition_env: &'a std::collections::BTreeMap<String, String>,
) -> [Option<&'a str>; 4] {
[
record_env.get(env_key).map(String::as_str),
persona_env.get(env_key).map(String::as_str),
global_env.get(env_key).map(String::as_str),
definition_env.get(env_key).map(String::as_str),
]
}
#[allow(clippy::too_many_arguments)]
fn build_model_field(
record_model: &Option<String>,
record: &ManagedAgentRecord,
file_model: &Option<String>,
acp_model: &Option<String>,
model_env_var: Option<&str>,
@@ -236,30 +233,109 @@ fn build_model_field(
is_pre_spawn: bool,
session_cache: Option<&SessionConfigCache>,
is_required: bool,
model_overridden: bool,
tiers: &InheritedConfigTiers,
) -> NormalizedField {
// Precedence: Buzz-explicit > ACP current > config file
let (value, origin) = if let Some(ref m) = record_model {
(Some(m.clone()), ConfigOrigin::BuzzExplicit)
} else if let Some(ref m) = acp_model {
(Some(m.clone()), ConfigOrigin::AcpConfigOption)
} else if let Some(ref m) = file_model {
(Some(m.clone()), ConfigOrigin::ConfigFile)
} else {
// No value from any tier. EnvVar is the sentinel origin for "no value
// resolved" — there is no dedicated None-origin variant. The panel
// renders this as an empty/absent field.
(None, ConfigOrigin::EnvVar)
};
let [rec_env, pers_env, glob_env, def_env] = model_env_var
.map(|k| {
env_candidates(
k,
&record.env_vars,
&tiers.persona_env,
&tiers.global_env,
&tiers.definition_env,
)
})
.unwrap_or([None, None, None, None]);
// The secondary expresses ONLY the static record-vs-file precedence: a
// Buzz-explicit model shadowing a config-file model. The live-session
// override (acp vs record/persona) is exclusively `apply_runtime_override`'s
// job, gated on `model_overridden`. Surfacing `acp_model` here would leak an
// override row even when no live switch has been applied.
let (overridden_value, overridden_origin) = if record_model.is_some() && file_model.is_some() {
(file_model.clone(), Some(ConfigOrigin::ConfigFile))
// Structured record model (definition-less only; linked cleared upstream).
let struct_record = record.model.as_deref();
let struct_persona = tiers.persona_model.as_deref();
let struct_global = tiers.global_model.as_deref();
// Configured candidates in spawn order: record env > persona env > global env >
// definition env > struct record > struct persona > struct global > file.
// The file entry is always last; everything before it is a "configured" candidate
// that gates whether ACP participates as a fallback (see any_configured below).
let configured: &[(Option<&str>, ConfigOrigin)] = &[
(rec_env, ConfigOrigin::BuzzExplicit),
(pers_env, ConfigOrigin::PersonaDefault),
(glob_env, ConfigOrigin::GlobalDefault),
(def_env, ConfigOrigin::HarnessDefault),
(struct_record, ConfigOrigin::BuzzExplicit),
(struct_persona, ConfigOrigin::PersonaDefault),
(struct_global, ConfigOrigin::GlobalDefault),
(file_model.as_deref(), ConfigOrigin::ConfigFile),
];
// "Configured" = any non-file candidate. The file entry is always last, so
// slicing to len()-1 is equivalent to the old magic `[..6]` and stays correct
// if the array ever grows again.
let any_configured = configured[..configured.len() - 1]
.iter()
.any(|(v, _)| v.is_some());
// When model_overridden is true and ACP is present, ACP is the live winner.
// The top configured candidate becomes the secondary (the overridden baseline).
// Equal-value case: ACP == baseline → fall through to normal resolution so
// the field carries the correct baseline origin rather than RuntimeOverride.
if model_overridden {
if let Some(acp) = acp_model.as_deref() {
let baseline = configured.iter().find(|(v, _)| v.is_some());
match baseline {
Some((Some(baseline_value), _)) if acp == *baseline_value => {
// Equal-value switch: no real divergence.
// Fall through to the normal resolve path below — it will
// return the same value with its true baseline origin, with
// no secondary row.
}
Some((Some(baseline_value), baseline_origin)) => {
return NormalizedField {
value: Some(acp.to_string()),
origin: ConfigOrigin::RuntimeOverride,
write_via: model_write_mechanism(
is_pre_spawn,
supports_acp_model,
session_cache,
model_env_var,
),
overridden_value: Some(baseline_value.to_string()),
overridden_origin: Some(baseline_origin.clone()),
is_required,
};
}
_ => {
// No configured baseline — ACP is the only source.
return NormalizedField {
value: Some(acp.to_string()),
origin: ConfigOrigin::RuntimeOverride,
write_via: model_write_mechanism(
is_pre_spawn,
supports_acp_model,
session_cache,
model_env_var,
),
overridden_value: None,
overridden_origin: None,
is_required,
};
}
}
}
}
let (value, origin, overridden_value, overridden_origin) = if !any_configured {
// No configured candidate: ACP participates as AcpConfigOption fallback.
let full: &[(Option<&str>, ConfigOrigin)] = &[
(acp_model.as_deref(), ConfigOrigin::AcpConfigOption),
(file_model.as_deref(), ConfigOrigin::ConfigFile),
];
resolve_with_override(full).unwrap_or((None, ConfigOrigin::EnvVar, None, None))
} else {
(None, None)
// ACP excluded: a configured value is pending and wins over live ACP.
match resolve_with_override(configured) {
Some(r) => r,
None => (None, ConfigOrigin::EnvVar, None, None),
}
};
let write_via = model_write_mechanism(
@@ -280,7 +356,6 @@ fn build_model_field(
}
/// Resolve how the model field is written back to the runtime.
/// Prefer ACP `set_config_option`/`set_model` post-spawn, else env-var respawn.
fn model_write_mechanism(
is_pre_spawn: bool,
supports_acp_model: bool,
@@ -301,67 +376,13 @@ fn model_write_mechanism(
}
}
/// Re-key the model field as a live runtime override when the harness signals
/// that a `SwitchModel` control signal set the model (Phase 3c).
///
/// The override-active signal is `model_overridden` from the
/// `session_config_captured` payload — NOT `acp_model != persona_model`, which
/// would false-positive when a persona model is edited mid-life while the
/// session is stale on the old model.
///
/// `baseline` is the value the live model overrides, paired with its true
/// origin — `(persona_model, PersonaDefault)` for a persona-linked agent, or
/// `(record_model, BuzzExplicit)` for a genuine-explicit agent that live-
/// switched. It is `Some` only when there is such a baseline to override
/// against; otherwise the field passes through unchanged. Carrying the origin
/// in the pair (rather than hardcoding it) lets the secondary be tagged by its
/// real source instead of always reading `PersonaDefault`.
///
/// The `acp == baseline_value` short-circuit keeps a live pick of the baseline
/// model itself from rendering a no-op "override of X with X". It yields a
/// CLEAN single-value field — `overridden_value`/`overridden_origin` cleared —
/// rather than passing `base` through, because `build_model_field` already
/// populates `base`'s secondary with an `AcpConfigOption` row for the
/// record-model-plus-live-session case; returning `base` would leak that
/// spurious row. The override preserves the base field's write mechanism — only
/// the displayed value, origin, and secondary change.
fn apply_runtime_override(
base: NormalizedField,
acp_model: Option<&str>,
baseline: Option<(&str, ConfigOrigin)>,
model_overridden: bool,
) -> NormalizedField {
if !model_overridden {
return base;
}
let (Some(acp), Some((baseline_value, baseline_origin))) = (acp_model, baseline) else {
return base;
};
if acp == baseline_value {
// Live pick equals the baseline — no real divergence. Strip any
// secondary `build_model_field` may have produced so the panel shows a
// single clean value rather than "X overridden by X".
return NormalizedField {
overridden_value: None,
overridden_origin: None,
..base
};
}
NormalizedField {
value: Some(acp.to_string()),
origin: ConfigOrigin::RuntimeOverride,
overridden_value: Some(baseline_value.to_string()),
overridden_origin: Some(baseline_origin),
..base
}
}
fn build_provider_field(
record_provider: &Option<String>,
record: &ManagedAgentRecord,
file_provider: &Option<String>,
provider_env_var: Option<&str>,
provider_locked: bool,
is_required: bool,
tiers: &InheritedConfigTiers,
) -> Option<NormalizedField> {
if provider_locked {
return Some(NormalizedField {
@@ -374,15 +395,43 @@ fn build_provider_field(
});
}
let tiers: &[(Option<&str>, ConfigOrigin)] = &[
(record_provider.as_deref(), ConfigOrigin::BuzzExplicit),
let [rec_env, pers_env, glob_env, def_env] = provider_env_var
.map(|k| {
env_candidates(
k,
&record.env_vars,
&tiers.persona_env,
&tiers.global_env,
&tiers.definition_env,
)
})
.unwrap_or([None, None, None, None]);
let struct_record = record.provider.as_deref();
let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[
(rec_env, ConfigOrigin::BuzzExplicit),
(pers_env, ConfigOrigin::PersonaDefault),
(glob_env, ConfigOrigin::GlobalDefault),
(def_env, ConfigOrigin::HarnessDefault),
(struct_record, ConfigOrigin::BuzzExplicit),
(
tiers.persona_provider.as_deref(),
ConfigOrigin::PersonaDefault,
),
(
tiers.global_provider.as_deref(),
ConfigOrigin::GlobalDefault,
),
(file_provider.as_deref(), ConfigOrigin::ConfigFile),
];
let (value, origin, overridden_value, overridden_origin) = match resolve_with_override(tiers) {
Some(resolved) => resolved,
None if is_required => (None, ConfigOrigin::EnvVar, None, None),
None => return None,
};
let (value, origin, overridden_value, overridden_origin) =
match resolve_with_override(tiers_list) {
Some(resolved) => resolved,
None if is_required => (None, ConfigOrigin::EnvVar, None, None),
None => return None,
};
let write_via = if let Some(env_key) = provider_env_var {
ConfigWriteMechanism::RespawnWithEnvVar {
@@ -432,20 +481,38 @@ fn build_mode_field(
})
}
#[allow(clippy::too_many_arguments)]
fn build_thinking_field(
record_effort: &Option<String>,
record: &ManagedAgentRecord,
file_effort: &Option<String>,
acp_effort: &Option<String>,
thinking_env_var: Option<&str>,
is_pre_spawn: bool,
session_cache: Option<&SessionConfigCache>,
tiers: &InheritedConfigTiers,
) -> Option<NormalizedField> {
let tiers: &[(Option<&str>, ConfigOrigin)] = &[
(record_effort.as_deref(), ConfigOrigin::BuzzExplicit),
// Tier ordering: record env > ACP > persona env > global env > definition env > config file.
let [rec_env, pers_env, glob_env, def_env] = thinking_env_var
.map(|k| {
env_candidates(
k,
&record.env_vars,
&tiers.persona_env,
&tiers.global_env,
&tiers.definition_env,
)
})
.unwrap_or([None, None, None, None]);
let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[
(rec_env, ConfigOrigin::BuzzExplicit),
(acp_effort.as_deref(), ConfigOrigin::AcpConfigOption),
(pers_env, ConfigOrigin::PersonaDefault),
(glob_env, ConfigOrigin::GlobalDefault),
(def_env, ConfigOrigin::HarnessDefault),
(file_effort.as_deref(), ConfigOrigin::ConfigFile),
];
let (value, origin, overridden_value, overridden_origin) = resolve_with_override(tiers)?;
let (value, origin, overridden_value, overridden_origin) = resolve_with_override(tiers_list)?;
let write_via = if !is_pre_spawn && has_config_option(session_cache, "effort") {
ConfigWriteMechanism::AcpSetConfigOption {
@@ -469,67 +536,105 @@ fn build_thinking_field(
})
}
/// Numeric fields (max_output_tokens, context_limit) — env-var tier wins over
/// config-file tier. When an env var key is given and present in the record's
/// env_vars map the field is BuzzExplicit + RespawnWithEnvVar; otherwise if the
/// config file supplied a value it is ConfigFile + ReadOnly; otherwise None.
/// Numeric fields (max_output_tokens, context_limit).
/// Tier ordering: record env > persona env > global env > config file.
fn build_numeric_env_field(
env_var: Option<&'static str>,
record_env: &std::collections::BTreeMap<String, String>,
record: &ManagedAgentRecord,
file_value: &Option<String>,
tiers: &InheritedConfigTiers,
) -> Option<NormalizedField> {
if let Some(key) = env_var {
if let Some(v) = record_env.get(key) {
return Some(NormalizedField {
value: Some(v.clone()),
origin: ConfigOrigin::BuzzExplicit,
write_via: ConfigWriteMechanism::RespawnWithEnvVar {
env_key: key.to_string(),
},
overridden_value: file_value.clone(),
overridden_origin: file_value.as_ref().map(|_| ConfigOrigin::ConfigFile),
is_required: false,
});
let [rec_env, pers_env, glob_env, def_env] = env_var
.map(|k| {
env_candidates(
k,
&record.env_vars,
&tiers.persona_env,
&tiers.global_env,
&tiers.definition_env,
)
})
.unwrap_or([None, None, None, None]);
let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[
(rec_env, ConfigOrigin::BuzzExplicit),
(pers_env, ConfigOrigin::PersonaDefault),
(glob_env, ConfigOrigin::GlobalDefault),
(def_env, ConfigOrigin::HarnessDefault),
(file_value.as_deref(), ConfigOrigin::ConfigFile),
];
let (value, origin, overridden_value, overridden_origin) = resolve_with_override(tiers_list)?;
let write_via = if let Some(key) = env_var {
ConfigWriteMechanism::RespawnWithEnvVar {
env_key: key.to_string(),
}
}
file_value.as_ref().map(|v| NormalizedField {
value: Some(v.clone()),
origin: ConfigOrigin::ConfigFile,
write_via: ConfigWriteMechanism::ReadOnly,
overridden_value: None,
overridden_origin: None,
} else {
ConfigWriteMechanism::ReadOnly
};
Some(NormalizedField {
value,
origin,
write_via,
overridden_value,
overridden_origin,
is_required: false,
})
}
/// Record/env prompt wins (BuzzExplicit, respawnable); a config-file prompt it
/// shadows is reported as the overridden secondary. A config-file-only prompt
/// — no record/env value to shadow it — is surfaced directly (read-only)
/// instead of being dropped: a prompt that drives the agent should always be
/// visible somewhere in the panel.
/// System prompt field.
///
/// Tier ordering per v3 plan: record env > persona env > global env >
/// struct record > struct persona > config file.
///
/// Env tiers sit above structured per spawn contract: `descriptor.env` is
/// written last (after the structured prompt), so env wins on collision.
/// `GlobalAgentConfig` has no structured system_prompt, so the global tier
/// is env-only. `BUZZ_ACP_SYSTEM_PROMPT` is not reserved and is therefore
/// a real global env tier.
fn build_system_prompt_field(
record_prompt: &Option<String>,
record: &ManagedAgentRecord,
file_prompt: &Option<String>,
tiers: &InheritedConfigTiers,
) -> Option<NormalizedField> {
if let Some(v) = record_prompt {
return Some(NormalizedField {
value: Some(v.clone()),
origin: ConfigOrigin::BuzzExplicit,
write_via: ConfigWriteMechanism::RespawnWithEnvVar {
env_key: "BUZZ_ACP_SYSTEM_PROMPT".to_string(),
},
overridden_value: file_prompt.clone(),
overridden_origin: file_prompt.as_ref().map(|_| ConfigOrigin::ConfigFile),
is_required: false,
});
}
const PROMPT_ENV_KEY: &str = "BUZZ_ACP_SYSTEM_PROMPT";
file_prompt.as_ref().map(|v| NormalizedField {
value: Some(v.clone()),
origin: ConfigOrigin::ConfigFile,
write_via: ConfigWriteMechanism::ReadOnly,
overridden_value: None,
overridden_origin: None,
let [rec_env, pers_env, glob_env, def_env] = env_candidates(
PROMPT_ENV_KEY,
&record.env_vars,
&tiers.persona_env,
&tiers.global_env,
&tiers.definition_env,
);
// Structured record prompt (definition-less only; linked cleared upstream).
let struct_record = record.system_prompt.as_deref();
let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[
(rec_env, ConfigOrigin::BuzzExplicit), // record env
(pers_env, ConfigOrigin::PersonaDefault), // persona env
(glob_env, ConfigOrigin::GlobalDefault), // global env
(def_env, ConfigOrigin::HarnessDefault), // definition env
(struct_record, ConfigOrigin::BuzzExplicit), // struct record
(
tiers.persona_prompt.as_deref(),
ConfigOrigin::PersonaDefault,
), // struct persona
(file_prompt.as_deref(), ConfigOrigin::ConfigFile),
];
let (value, origin, overridden_value, overridden_origin) = resolve_with_override(tiers_list)?;
Some(NormalizedField {
value,
origin,
write_via: ConfigWriteMechanism::RespawnWithEnvVar {
env_key: PROMPT_ENV_KEY.to_string(),
},
overridden_value,
overridden_origin,
is_required: false,
})
}

499
desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs
View File

@@ -120,11 +120,53 @@ fn test_record() -> ManagedAgentRecord {
}
}
/// Default empty tiers: no persona or global inheritance.
fn no_tiers() -> InheritedConfigTiers {
InheritedConfigTiers::default()
}
/// Tiers with only global env set (for AC-1 style tests).
fn global_env_tiers(key: &str, val: &str) -> InheritedConfigTiers {
let mut global_env = BTreeMap::new();
global_env.insert(key.to_string(), val.to_string());
InheritedConfigTiers {
global_env,
..Default::default()
}
}
/// Tiers with only persona env set.
fn persona_env_tiers(key: &str, val: &str) -> InheritedConfigTiers {
let mut persona_env = BTreeMap::new();
persona_env.insert(key.to_string(), val.to_string());
InheritedConfigTiers {
persona_env,
..Default::default()
}
}
/// Tiers with both persona and global env set for the same key.
fn persona_and_global_env_tiers(
key: &str,
persona_val: &str,
global_val: &str,
) -> InheritedConfigTiers {
let mut persona_env = BTreeMap::new();
persona_env.insert(key.to_string(), persona_val.to_string());
let mut global_env = BTreeMap::new();
global_env.insert(key.to_string(), global_val.to_string());
InheritedConfigTiers {
persona_env,
global_env,
..Default::default()
}
}
#[test]
fn pre_spawn_surface_reports_pending_acp_tiers() {
let record = test_record();
let runtime = test_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
assert!(surface.is_pre_spawn);
assert_eq!(surface.sources.acp_native, ConfigTierStatus::Pending);
@@ -140,7 +182,7 @@ fn surface_reports_mcp_specific_config_path() {
let record = test_record();
let runtime = test_runtime();
let surface = with_goose_path_root(None, || {
read_config_surface(&record, Some(runtime), None, None)
read_config_surface(&record, Some(runtime), None, &no_tiers())
});
let path = surface
@@ -159,7 +201,7 @@ fn goose_mcp_config_path_follows_path_root_override() {
let record = test_record();
let runtime = test_runtime();
let surface = with_goose_path_root(Some("/tmp/buzz-goose-root"), || {
read_config_surface(&record, Some(runtime), None, None)
read_config_surface(&record, Some(runtime), None, &no_tiers())
});
let expected_path = Path::new("/tmp/buzz-goose-root")
@@ -183,7 +225,7 @@ fn claude_surface_uses_mcp_config_path_not_settings_path() {
config_file_path: Some("~/.claude/settings.json"),
..*test_runtime()
};
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
assert!(surface
.sources
@@ -203,7 +245,7 @@ fn record_model_overrides_file_model() {
record.model = Some("explicit-model".to_string());
let runtime = test_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("explicit-model"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
@@ -216,7 +258,7 @@ fn provider_locked_shows_locked() {
provider_locked: true,
..*test_runtime()
};
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let provider = surface.normalized.provider.unwrap();
assert_eq!(provider.value.as_deref(), Some("Anthropic (locked)"));
assert_eq!(provider.origin, ConfigOrigin::HarnessConstraint);
@@ -242,7 +284,7 @@ fn post_spawn_with_model_config_option_uses_acp() {
captured_at: "".to_string(),
};
let surface = read_config_surface(&record, Some(runtime), Some(&cache), None);
let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers());
assert!(!surface.is_pre_spawn);
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("claude-opus-4"));
@@ -266,53 +308,86 @@ fn acp_model_overrides_file_model_with_override_tracking() {
captured_at: "".to_string(),
};
let surface = read_config_surface(&record, Some(runtime), Some(&cache), None);
let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers());
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("acp-model"));
assert_eq!(model.origin, ConfigOrigin::AcpConfigOption);
// The goose config file might have a model too — since we can't control
// the actual file in a unit test, just verify the override fields are populated
// when we manually construct the scenario via build_model_field.
}
// ── Persona resolution integration tests ────────────────────────────
// ── Persona / global tier integration tests ──────────────────────────────────
//
// These simulate the call-site pattern in agent_config.rs:
// 1. Inject persona-resolved values into the record (as if absent)
// 2. Call read_config_surface (reader tags them BuzzExplicit)
// 3. Re-tag injected fields to PersonaDefault
//
// This exercises the same logic path as get_agent_config_surface without
// requiring Tauri AppHandle/State infrastructure.
// These exercise the tiers-based candidate resolution for model, provider, and
// system_prompt via `InheritedConfigTiers` — replacing the old inject+retag
// simulation tests.
#[test]
fn persona_model_injection_produces_persona_default_origin() {
let mut record = test_record();
// Simulate: record has no model, persona provides one.
// The call-site injects it before calling the reader.
record.model = Some("persona-model".to_string());
fn persona_model_tier_produces_persona_default_origin() {
let record = test_record(); // no record.model
let runtime = test_runtime();
let tiers = InheritedConfigTiers {
persona_model: Some("persona-model".to_string()),
..Default::default()
};
let mut surface = read_config_surface(&record, Some(runtime), None, None);
// Reader sees injected model as BuzzExplicit.
let model = surface.normalized.model.as_ref().unwrap();
assert_eq!(model.value.as_deref(), Some("persona-model"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
// Call-site re-tags (simulating had_model == false).
if let Some(ref mut field) = surface.normalized.model {
if field.origin == ConfigOrigin::BuzzExplicit {
field.origin = ConfigOrigin::PersonaDefault;
}
}
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("persona-model"));
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
}
// ── Runtime override (Phase 3c) ──────────────────────────────────────
#[test]
fn global_model_tier_produces_global_default_origin() {
let record = test_record();
let runtime = test_runtime();
let tiers = InheritedConfigTiers {
global_model: Some("global-model".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("global-model"));
assert_eq!(model.origin, ConfigOrigin::GlobalDefault);
}
#[test]
fn persona_provider_tier_produces_persona_default_origin() {
let record = test_record();
let runtime = test_runtime();
let tiers = InheritedConfigTiers {
persona_provider: Some("anthropic".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let provider = surface.normalized.provider.unwrap();
assert_eq!(provider.value.as_deref(), Some("anthropic"));
assert_eq!(provider.origin, ConfigOrigin::PersonaDefault);
}
#[test]
fn persona_prompt_tier_produces_persona_default_origin() {
let record = test_record();
let runtime = test_runtime();
let tiers = InheritedConfigTiers {
persona_prompt: Some("You are a helpful assistant.".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let prompt = surface.normalized.system_prompt.unwrap();
assert_eq!(
prompt.value.as_deref(),
Some("You are a helpful assistant.")
);
assert_eq!(prompt.origin, ConfigOrigin::PersonaDefault);
}
// ── Runtime override (model_overridden gate) ──────────────────────────────────
//
// A live ModelPicker switch is signalled by `model_overridden: true` in the
// `session_config_captured` payload. The reader keys the override-active
@@ -321,7 +396,7 @@ fn persona_model_injection_produces_persona_default_origin() {
#[test]
fn runtime_override_wins_display_when_model_overridden_is_true() {
// Persona-linked agent (record.model == None); persona == "persona-model".
// Persona-linked agent (record.model == None); persona model via tiers.
// A live switch pushed "live-model" to the session and set model_overridden.
let record = test_record();
let runtime = test_runtime();
@@ -334,29 +409,27 @@ fn runtime_override_wins_display_when_model_overridden_is_true() {
goose_native_config: None,
captured_at: "".to_string(),
};
let tiers = InheritedConfigTiers {
persona_model: Some("persona-model".to_string()),
..Default::default()
};
let surface = read_config_surface(
&record,
Some(runtime),
Some(&cache),
Some(("persona-model", ConfigOrigin::PersonaDefault)),
);
let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers);
let model = surface.normalized.model.unwrap();
// Override wins the display value with a runtime-override origin.
assert_eq!(model.value.as_deref(), Some("live-model"));
assert_eq!(model.origin, ConfigOrigin::RuntimeOverride);
// Persona is the secondary value (not struck through — the UI keys off
// the RuntimeOverride origin to suppress strikethrough).
// Persona is the secondary value.
assert_eq!(model.overridden_value.as_deref(), Some("persona-model"));
assert_eq!(model.overridden_origin, Some(ConfigOrigin::PersonaDefault));
}
#[test]
fn no_runtime_override_when_model_overridden_is_false() {
// At spawn the session's current_model == persona model (BUZZ_ACP_MODEL
// is set to the persona model) and model_overridden is false. No override;
// the field falls through to normal precedence.
// At spawn the session's current_model == persona model and
// model_overridden is false. No override; field falls through to normal
// precedence.
let record = test_record();
let runtime = test_runtime();
let cache = SessionConfigCache {
@@ -368,17 +441,15 @@ fn no_runtime_override_when_model_overridden_is_false() {
goose_native_config: None,
captured_at: "".to_string(),
};
let tiers = InheritedConfigTiers {
persona_model: Some("persona-model".to_string()),
..Default::default()
};
let surface = read_config_surface(
&record,
Some(runtime),
Some(&cache),
Some(("persona-model", ConfigOrigin::PersonaDefault)),
);
let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers);
let model = surface.normalized.model.unwrap();
// model_overridden is false => the override branch is not taken: origin
// is the normal precedence result, never RuntimeOverride.
// model_overridden is false => the override branch is not taken.
assert_ne!(model.origin, ConfigOrigin::RuntimeOverride);
assert_eq!(model.value.as_deref(), Some("persona-model"));
assert_ne!(model.overridden_origin, Some(ConfigOrigin::PersonaDefault));
@@ -402,106 +473,51 @@ fn no_false_positive_override_when_persona_edited_mid_life() {
goose_native_config: None,
captured_at: "".to_string(),
};
let tiers = InheritedConfigTiers {
persona_model: Some("new-persona-model".to_string()),
..Default::default()
};
let surface = read_config_surface(
&record,
Some(runtime),
Some(&cache),
Some(("new-persona-model", ConfigOrigin::PersonaDefault)),
);
let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers);
let model = surface.normalized.model.unwrap();
// model_overridden is false => no RuntimeOverride, even though
// acp_model != persona_model. The old divergence-based signal would
// have false-positived here. The persona is never surfaced as the
// overridden secondary (that marker is exclusive to a real override).
// have false-positived here.
assert_ne!(model.origin, ConfigOrigin::RuntimeOverride);
assert_ne!(model.overridden_origin, Some(ConfigOrigin::PersonaDefault));
}
#[test]
fn persona_provider_injection_produces_persona_default_origin() {
let mut record = test_record();
// Simulate: record has no provider env var, persona provides one.
// The call-site injects it as GOOSE_PROVIDER before calling the reader.
record
.env_vars
.insert("GOOSE_PROVIDER".to_string(), "anthropic".to_string());
let runtime = test_runtime();
let mut surface = read_config_surface(&record, Some(runtime), None, None);
// Reader sees injected provider as BuzzExplicit.
let provider = surface.normalized.provider.as_ref().unwrap();
assert_eq!(provider.value.as_deref(), Some("anthropic"));
assert_eq!(provider.origin, ConfigOrigin::BuzzExplicit);
// Call-site re-tags (simulating had_provider == false).
if let Some(ref mut field) = surface.normalized.provider {
if field.origin == ConfigOrigin::BuzzExplicit {
field.origin = ConfigOrigin::PersonaDefault;
}
}
let provider = surface.normalized.provider.unwrap();
assert_eq!(provider.value.as_deref(), Some("anthropic"));
assert_eq!(provider.origin, ConfigOrigin::PersonaDefault);
}
// ── system_prompt builder unit tests ─────────────────────────────────────────
#[test]
fn persona_system_prompt_injection_produces_persona_default_origin() {
let mut record = test_record();
// Simulate: record has no system_prompt, persona provides one via env var.
// The call-site injects it as BUZZ_ACP_SYSTEM_PROMPT before calling the reader.
record.env_vars.insert(
"BUZZ_ACP_SYSTEM_PROMPT".to_string(),
"You are a helpful assistant.".to_string(),
);
let runtime = test_runtime();
let mut surface = read_config_surface(&record, Some(runtime), None, None);
// Reader sees injected prompt as BuzzExplicit.
let prompt = surface.normalized.system_prompt.as_ref().unwrap();
assert_eq!(
prompt.value.as_deref(),
Some("You are a helpful assistant.")
);
assert_eq!(prompt.origin, ConfigOrigin::BuzzExplicit);
// Call-site re-tags (simulating had_prompt == false).
if let Some(ref mut field) = surface.normalized.system_prompt {
if field.origin == ConfigOrigin::BuzzExplicit {
field.origin = ConfigOrigin::PersonaDefault;
}
}
let prompt = surface.normalized.system_prompt.unwrap();
assert_eq!(
prompt.value.as_deref(),
Some("You are a helpful assistant.")
);
assert_eq!(prompt.origin, ConfigOrigin::PersonaDefault);
}
#[test]
fn config_file_only_system_prompt_surfaces_as_read_only_config_file_field() {
// Record/env has no prompt; the config file does. It must NOT be
// dropped — it should surface with ConfigFile origin, read-only.
let field = build_system_prompt_field(&None, &Some("File-driven prompt.".to_string())).unwrap();
fn config_file_only_system_prompt_surfaces_as_config_file_origin() {
// Record/env has no prompt; the config file does. Must surface with
// ConfigFile origin. Write mechanism is always RespawnWithEnvVar for
// system_prompt — the UI writes back via BUZZ_ACP_SYSTEM_PROMPT.
let record = test_record();
let field = build_system_prompt_field(
&record,
&Some("File-driven prompt.".to_string()),
&no_tiers(),
)
.unwrap();
assert_eq!(field.value.as_deref(), Some("File-driven prompt."));
assert_eq!(field.origin, ConfigOrigin::ConfigFile);
assert!(matches!(field.write_via, ConfigWriteMechanism::ReadOnly));
assert!(matches!(
field.write_via,
ConfigWriteMechanism::RespawnWithEnvVar { ref env_key }
if env_key == "BUZZ_ACP_SYSTEM_PROMPT"
));
assert!(field.overridden_value.is_none());
}
#[test]
fn record_system_prompt_shadows_config_file_prompt_as_secondary() {
let field = build_system_prompt_field(
&Some("Record prompt.".to_string()),
&Some("File prompt.".to_string()),
)
.unwrap();
let mut record = test_record();
record.system_prompt = Some("Record prompt.".to_string());
let field =
build_system_prompt_field(&record, &Some("File prompt.".to_string()), &no_tiers()).unwrap();
assert_eq!(field.value.as_deref(), Some("Record prompt."));
assert_eq!(field.origin, ConfigOrigin::BuzzExplicit);
assert_eq!(field.overridden_value.as_deref(), Some("File prompt."));
@@ -510,19 +526,19 @@ fn record_system_prompt_shadows_config_file_prompt_as_secondary() {
#[test]
fn no_system_prompt_from_any_tier_yields_none() {
assert!(build_system_prompt_field(&None, &None).is_none());
let record = test_record();
assert!(build_system_prompt_field(&record, &None, &no_tiers()).is_none());
}
#[test]
fn explicit_record_model_not_retagged_when_already_present() {
let mut record = test_record();
// Record already has its own model — persona resolution should NOT re-tag.
// Record already has its own model — origin stays BuzzExplicit.
record.model = Some("explicit-model".to_string());
let runtime = test_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
// had_model == true, so no re-tagging occurs. Origin stays BuzzExplicit.
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("explicit-model"));
assert_eq!(model.origin, ConfigOrigin::BuzzExplicit);
@@ -544,7 +560,7 @@ fn extra_env_vars_appear_in_advanced_as_buzz_explicit() {
.insert("SPROUT_ACP_MEMORY".to_string(), "mem-value".to_string());
let runtime = test_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect();
assert!(
@@ -575,21 +591,15 @@ fn extra_env_vars_appear_in_advanced_as_buzz_explicit() {
#[test]
fn extra_env_var_skipped_when_already_in_file_config_extra() {
// If a key is in both record.env_vars and file_config.extra, the config
// file entry wins (it was already added to advanced). The env var must
// not produce a second entry.
//
// We can't inject into file_config.extra directly in a unit test (it
// comes from disk), so we verify the dedup logic via the normalized-key
// path: GOOSE_THINKING_EFFORT is a normalized key and must not appear
// in advanced even if set in env_vars.
// If a key is normalized, it must not appear in advanced even if set
// in env_vars.
let mut record = test_record();
record
.env_vars
.insert("GOOSE_THINKING_EFFORT".to_string(), "high".to_string());
let runtime = test_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect();
assert!(
@@ -598,7 +608,7 @@ fn extra_env_var_skipped_when_already_in_file_config_extra() {
);
}
// ── buzz-agent normalized env-var field tests ───────────────────────────────
// ── buzz-agent normalized env-var field tests ─────────────────────────────────
//
// buzz-agent uses env vars (not a config file) for max_output_tokens and
// context_limit. build_numeric_env_field must surface these as BuzzExplicit
@@ -649,7 +659,7 @@ fn buzz_agent_max_output_tokens_from_env_is_buzz_explicit() {
);
let runtime = buzz_agent_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let field = surface.normalized.max_output_tokens.unwrap();
assert_eq!(field.value.as_deref(), Some("8192"));
@@ -670,7 +680,7 @@ fn buzz_agent_context_limit_from_env_is_buzz_explicit() {
);
let runtime = buzz_agent_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let field = surface.normalized.context_limit.unwrap();
assert_eq!(field.value.as_deref(), Some("100000"));
@@ -688,7 +698,7 @@ fn buzz_agent_max_tokens_absent_when_no_env_var_or_file() {
let record = test_record();
let runtime = buzz_agent_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
assert!(
surface.normalized.max_output_tokens.is_none(),
@@ -713,7 +723,7 @@ fn buzz_agent_max_tokens_env_var_not_double_surfaced_in_advanced() {
);
let runtime = buzz_agent_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect();
assert!(
@@ -734,7 +744,7 @@ fn buzz_agent_thinking_effort_from_env_is_buzz_explicit() {
.insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string());
let runtime = buzz_agent_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let field = surface.normalized.thinking_effort.unwrap();
assert_eq!(field.value.as_deref(), Some("high"));
@@ -755,7 +765,7 @@ fn buzz_agent_thinking_effort_env_var_not_double_surfaced_in_advanced() {
);
let runtime = buzz_agent_runtime();
let surface = read_config_surface(&record, Some(runtime), None, None);
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect();
assert!(
@@ -764,10 +774,20 @@ fn buzz_agent_thinking_effort_env_var_not_double_surfaced_in_advanced() {
);
}
// ── provider builder unit tests ───────────────────────────────────────────────
#[test]
fn missing_required_provider_still_returns_dropdown_field() {
let provider = build_provider_field(&None, &None, Some("GOOSE_PROVIDER"), false, true)
.expect("required provider field should be surfaced even when empty");
let record = test_record();
let provider = build_provider_field(
&record,
&None,
Some("GOOSE_PROVIDER"),
false,
true,
&no_tiers(),
)
.expect("required provider field should be surfaced even when empty");
assert_eq!(provider.value, None);
assert_eq!(provider.origin, ConfigOrigin::EnvVar);
@@ -776,5 +796,156 @@ fn missing_required_provider_still_returns_dropdown_field() {
#[test]
fn missing_optional_provider_stays_hidden() {
assert!(build_provider_field(&None, &None, Some("GOOSE_PROVIDER"), false, false).is_none());
let record = test_record();
assert!(build_provider_field(
&record,
&None,
Some("GOOSE_PROVIDER"),
false,
false,
&no_tiers()
)
.is_none());
}
// ── thinking_effort persona/global tier tests (AC-1..5) ──────────────────────
//
// The plan's acceptance criteria for effort tier resolution.
// Tier ordering: record env > ACP > persona env > global env > config file.
fn buzz_agent_rt() -> &'static KnownAcpRuntime {
crate::managed_agents::discovery::known_acp_runtime_exact("buzz-agent")
.expect("buzz-agent must be in catalog")
}
/// AC-1: no record effort, global env has effort → GlobalDefault.
/// Real-world case: global-agent-config has BUZZ_AGENT_THINKING_EFFORT=high,
/// per-agent record has no env_vars → effort must surface with GlobalDefault origin.
#[test]
fn global_effort_surfaces_as_global_default_when_record_has_none() {
let record = test_record();
let runtime = buzz_agent_rt();
let tiers = global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "high");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let effort = surface
.normalized
.thinking_effort
.expect("effort must surface from global tier");
assert_eq!(effort.value.as_deref(), Some("high"));
assert_eq!(effort.origin, ConfigOrigin::GlobalDefault);
}
/// AC-2: persona env has effort, global also has effort → PersonaDefault wins, shadows global.
#[test]
fn persona_effort_shadows_global_and_tags_persona_default() {
let record = test_record();
let runtime = buzz_agent_rt();
let tiers = persona_and_global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "medium", "high");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let effort = surface
.normalized
.thinking_effort
.expect("effort must surface from persona tier");
assert_eq!(effort.value.as_deref(), Some("medium"));
assert_eq!(effort.origin, ConfigOrigin::PersonaDefault);
// global is the overridden baseline
assert_eq!(effort.overridden_value.as_deref(), Some("high"));
assert_eq!(effort.overridden_origin, Some(ConfigOrigin::GlobalDefault));
}
/// AC-3: record-level effort wins over persona and global, stays BuzzExplicit.
#[test]
fn record_effort_outranks_persona_and_global_keeps_buzz_explicit() {
let mut record = test_record();
record.env_vars.insert(
"BUZZ_AGENT_THINKING_EFFORT".to_string(),
"xhigh".to_string(),
);
let runtime = buzz_agent_rt();
let tiers = persona_and_global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "medium", "high");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let effort = surface
.normalized
.thinking_effort
.expect("effort must surface from record tier");
assert_eq!(effort.value.as_deref(), Some("xhigh"));
assert_eq!(effort.origin, ConfigOrigin::BuzzExplicit);
}
/// AC-4: no effort from any tier → thinking_effort field is absent.
#[test]
fn no_effort_anywhere_yields_no_thinking_effort_field() {
let record = test_record();
let runtime = buzz_agent_rt();
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
assert!(
surface.normalized.thinking_effort.is_none(),
"thinking_effort must be None when no tier has a value"
);
}
/// AC-5 (conflicting-ACP): inherited effort set (global=high) + live ACP effort=low
/// → ACP wins as primary (AcpConfigOption), global is the overridden secondary.
#[test]
fn acp_effort_wins_over_inherited_global_effort_as_secondary() {
let record = test_record();
let runtime = buzz_agent_rt();
let cache = SessionConfigCache {
config_options: vec![AcpConfigOptionEntry {
config_id: "effort".to_string(),
category: Some("effort".to_string()),
display_name: Some("Effort".to_string()),
current_value: Some("low".to_string()),
options: vec![],
}],
available_modes: vec![],
available_models: vec![],
current_model: None,
model_overridden: false,
goose_native_config: None,
captured_at: "".to_string(),
};
let tiers = global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "high");
let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers);
let effort = surface
.normalized
.thinking_effort
.expect("effort must surface from ACP tier");
// Live ACP value wins.
assert_eq!(effort.value.as_deref(), Some("low"));
assert_eq!(effort.origin, ConfigOrigin::AcpConfigOption);
// Global is surfaced as the overridden baseline.
assert_eq!(effort.overridden_value.as_deref(), Some("high"));
assert_eq!(effort.overridden_origin, Some(ConfigOrigin::GlobalDefault));
}
// ── Numerics inheritance tests ────────────────────────────────────────────────
//
// max_output_tokens and context_limit gain persona/global tiers.
#[test]
fn numeric_max_tokens_inherits_from_global_env() {
let record = test_record();
let runtime = buzz_agent_runtime();
let tiers = global_env_tiers("BUZZ_AGENT_MAX_OUTPUT_TOKENS", "16384");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let field = surface.normalized.max_output_tokens.unwrap();
assert_eq!(field.value.as_deref(), Some("16384"));
assert_eq!(field.origin, ConfigOrigin::GlobalDefault);
}
// ── Extended tests (split file to respect line-count ratchet) ────────────────
#[path = "reader_tests_ext.rs"]
mod ext;

258
desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs Normal file
View File

@@ -0,0 +1,258 @@
//! Additional tests for `config_bridge/reader.rs` — split out to keep
//! `reader_tests.rs` under the 1000-line file-size ratchet.
//!
//! Included as `mod ext` inside `reader_tests.rs`, so `use super::*` gives
//! access to all helpers and types from that module.
use super::*;
// ── Numerics inheritance tests ────────────────────────────────────────────────
//
// max_output_tokens and context_limit gain persona/global tiers.
#[test]
fn numeric_context_limit_inherits_from_persona_env() {
let record = test_record();
let runtime = buzz_agent_runtime();
let tiers = persona_env_tiers("BUZZ_AGENT_MAX_CONTEXT_TOKENS", "200000");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let field = surface.normalized.context_limit.unwrap();
assert_eq!(field.value.as_deref(), Some("200000"));
assert_eq!(field.origin, ConfigOrigin::PersonaDefault);
}
#[test]
fn record_max_tokens_overrides_global_env_with_secondary() {
let mut record = test_record();
record.env_vars.insert(
"BUZZ_AGENT_MAX_OUTPUT_TOKENS".to_string(),
"8192".to_string(),
);
let runtime = buzz_agent_runtime();
let tiers = global_env_tiers("BUZZ_AGENT_MAX_OUTPUT_TOKENS", "16384");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let field = surface.normalized.max_output_tokens.unwrap();
assert_eq!(field.value.as_deref(), Some("8192"));
assert_eq!(field.origin, ConfigOrigin::BuzzExplicit);
// Global value is the overridden secondary.
assert_eq!(field.overridden_value.as_deref(), Some("16384"));
assert_eq!(field.overridden_origin, Some(ConfigOrigin::GlobalDefault));
}
// ── Env-vs-structured collision tests (plan v3, Phase 2) ─────────────────────
/// Collision test 1: persona structured prompt + global env BUZZ_ACP_SYSTEM_PROMPT
/// → global env wins (env block sits entirely above structured).
#[test]
fn global_env_prompt_wins_over_persona_structured_prompt() {
let record = test_record();
let runtime = test_runtime();
let tiers = InheritedConfigTiers {
global_env: {
let mut m = BTreeMap::new();
m.insert(
"BUZZ_ACP_SYSTEM_PROMPT".to_string(),
"global-env-prompt".to_string(),
);
m
},
persona_prompt: Some("persona-structured-prompt".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let prompt = surface.normalized.system_prompt.unwrap();
assert_eq!(prompt.value.as_deref(), Some("global-env-prompt"));
assert_eq!(prompt.origin, ConfigOrigin::GlobalDefault);
}
/// Collision test 2: structured persona/record model + higher user-env value at
/// the runtime's model key → env value wins.
#[test]
fn persona_env_model_wins_over_persona_structured_model() {
let record = test_record(); // no record.model
let runtime = test_runtime(); // GOOSE_MODEL
let tiers = InheritedConfigTiers {
persona_env: {
let mut m = BTreeMap::new();
m.insert("GOOSE_MODEL".to_string(), "env-model".to_string());
m
},
persona_model: Some("struct-persona-model".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
// persona env outranks persona struct because env candidates precede struct
assert_eq!(model.value.as_deref(), Some("env-model"));
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
}
/// Collision test 3: no env representation → structured persona/record/global
/// fallback and provenance remain intact.
#[test]
fn structured_fallback_intact_when_no_env_representation() {
let record = test_record(); // no record.model, no env vars
let runtime = test_runtime();
let tiers = InheritedConfigTiers {
persona_model: Some("struct-persona-model".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("struct-persona-model"));
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
}
// ── Post-sanitization fallthrough test ───────────────────────────────────────
//
// Sanitization itself happens at the command boundary in `build_inherited_tiers`
// (a value with a NUL byte or an oversize value is dropped from the tier) and is
// pinned by the tests in `commands/agent_config_tests.rs`. The reader only ever
// sees the sanitized result, so what it must guarantee is the downstream half:
// a key stripped from one tier falls through to the next.
/// A key absent from the global env tier — the shape the reader sees after the
/// command boundary strips an invalid value — falls through to the persona tier.
#[test]
fn post_sanitization_empty_global_env_falls_through_to_persona_tier() {
let record = test_record();
let runtime = buzz_agent_rt();
// No global env (stripped); persona provides the valid fallback.
let tiers = persona_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "medium");
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
// Persona value surfaces instead of the stripped global value.
let effort = surface.normalized.thinking_effort.unwrap();
assert_eq!(effort.value.as_deref(), Some("medium"));
assert_eq!(effort.origin, ConfigOrigin::PersonaDefault);
}
// ── Pass-3 prompt collision test ─────────────────────────────────────────────
//
// From Thufir's pass-3 verdict MINOR clarification (promoted to required):
// definition-less record with both structured and env prompt — env wins.
/// Pass-3 clarification: record.system_prompt = A + record env
/// BUZZ_ACP_SYSTEM_PROMPT = B → B wins as BuzzExplicit.
/// The env block sits above the struct block per v3 candidate-preparation
/// contract; current reader semantics (struct before env) would be wrong.
#[test]
fn record_env_prompt_wins_over_record_struct_prompt_as_buzz_explicit() {
let mut record = test_record();
record.system_prompt = Some("struct-prompt-A".to_string());
record.env_vars.insert(
"BUZZ_ACP_SYSTEM_PROMPT".to_string(),
"env-prompt-B".to_string(),
);
let runtime = test_runtime();
let surface = read_config_surface(&record, Some(runtime), None, &no_tiers());
let prompt = surface.normalized.system_prompt.unwrap();
assert_eq!(prompt.value.as_deref(), Some("env-prompt-B"));
assert_eq!(prompt.origin, ConfigOrigin::BuzzExplicit);
// Struct prompt is the secondary.
assert_eq!(prompt.overridden_value.as_deref(), Some("struct-prompt-A"));
assert_eq!(prompt.overridden_origin, Some(ConfigOrigin::BuzzExplicit));
}
// ── Definition env tier tests (Layer 2b) ─────────────────────────────────────
//
// The harness definition's `env` block sits below global env and above
// structured values in spawn's precedence (Layer 2b). These tests exercise
// the reader's mapping of that tier to `HarnessDefault` origin.
/// Definition env wins over structured persona model when no user-env or
/// global-env candidate is present.
#[test]
fn definition_env_beats_structured_persona_model() {
let record = test_record(); // no record.model, no record.env_vars
let runtime = test_runtime(); // model_env_var = "GOOSE_MODEL"
let tiers = InheritedConfigTiers {
definition_env: {
let mut m = BTreeMap::new();
m.insert("GOOSE_MODEL".to_string(), "harness-model".to_string());
m
},
persona_model: Some("persona-struct-model".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("harness-model"));
assert_eq!(model.origin, ConfigOrigin::HarnessDefault);
// Structured persona model is the overridden secondary.
assert_eq!(
model.overridden_value.as_deref(),
Some("persona-struct-model")
);
assert_eq!(model.overridden_origin, Some(ConfigOrigin::PersonaDefault));
}
/// Global env beats definition env — user-settable tiers always win over the
/// harness author's defaults.
#[test]
fn global_env_beats_definition_env() {
let record = test_record();
let runtime = test_runtime(); // model_env_var = "GOOSE_MODEL"
let tiers = InheritedConfigTiers {
global_env: {
let mut m = BTreeMap::new();
m.insert("GOOSE_MODEL".to_string(), "global-model".to_string());
m
},
definition_env: {
let mut m = BTreeMap::new();
m.insert("GOOSE_MODEL".to_string(), "harness-model".to_string());
m
},
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
assert_eq!(model.value.as_deref(), Some("global-model"));
assert_eq!(model.origin, ConfigOrigin::GlobalDefault);
// Harness default is the overridden secondary.
assert_eq!(model.overridden_value.as_deref(), Some("harness-model"));
assert_eq!(model.overridden_origin, Some(ConfigOrigin::HarnessDefault));
}
/// A reserved key in the definition env is stripped by sanitization and must
/// not reach the reader. This test exercises the reader's contract (a key
/// absent from the tier falls through) — sanitization itself is pinned in
/// the `agent_config_tests.rs` constructor tests.
#[test]
fn reserved_key_absent_from_definition_env_falls_through() {
let record = test_record();
let runtime = test_runtime(); // model_env_var = "GOOSE_MODEL"
// definition_env contains only an unrelated key — the env map here is what
// the command boundary would produce after stripping a reserved key; the
// reader must fall through to the next tier (persona structured model).
let tiers = InheritedConfigTiers {
definition_env: BTreeMap::new(), // stripped — nothing survives
persona_model: Some("persona-struct-model".to_string()),
..Default::default()
};
let surface = read_config_surface(&record, Some(runtime), None, &tiers);
let model = surface.normalized.model.unwrap();
// Falls through to persona structured model.
assert_eq!(model.value.as_deref(), Some("persona-struct-model"));
assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
}

48
desktop/src-tauri/src/managed_agents/config_bridge/types.rs
View File

@@ -2,6 +2,41 @@ use std::collections::BTreeMap;
use serde::{Deserialize, Serialize};
/// Sanitized inherited config tiers passed to the reader.
///
/// Built at the `agent_config` command boundary with spawn-equivalent
/// sanitization: reserved, malformed, NUL-value, and oversize-value env keys
/// are stripped (matching `merged_user_env`). Structured fields are
/// normalized: blank/whitespace-only values collapse to `None`.
///
/// Orphaned persona links (persona_id references a missing persona) produce
/// an empty persona env tier and `None` for all structured persona fields —
/// the panel still renders from record/global. This diverges deliberately from
/// spawn's `OrphanedInstance` refusal, which is a spawn-safety property the
/// display surface does not need to enforce.
#[derive(Debug, Clone, Default)]
pub struct InheritedConfigTiers {
/// Sanitized env vars from the linked persona definition.
pub persona_env: BTreeMap<String, String>,
/// Sanitized env vars from the global agent config.
pub global_env: BTreeMap<String, String>,
/// Sanitized env vars from the resolved harness definition (`HarnessDefinition::env`).
/// Sits below global env and above structured values, matching spawn Layer 2b.
/// Empty for preset harnesses (all shipped presets have `env: {}`); only
/// user-authored custom harness JSONs with a non-empty `env` block contribute here.
pub definition_env: BTreeMap<String, String>,
/// Structured model from the linked persona (non-blank only).
pub persona_model: Option<String>,
/// Structured provider from the linked persona (non-blank only).
pub persona_provider: Option<String>,
/// Structured system_prompt from the linked persona (non-blank only).
pub persona_prompt: Option<String>,
/// Structured model from global config (non-blank only).
pub global_model: Option<String>,
/// Structured provider from global config (non-blank only).
pub global_provider: Option<String>,
}
/// Where a config value came from — determines precedence and UI annotations.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
@@ -17,14 +52,12 @@ pub enum ConfigOrigin {
/// Read from harness config file on disk (tier 2b, lowest precedence).
ConfigFile,
/// Value inherited from persona defaults.
/// Populated by the `get_agent_config_surface` call site: persona values are
/// resolved before calling the reader, then the surface is post-processed to
/// re-tag injected fields from `BuzzExplicit` to `PersonaDefault`.
/// Populated when a persona's env var or structured field wins for this
/// field in the reader's candidate resolution.
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`.
/// the linked persona specifies a value.
GlobalDefault,
/// Live runtime model override applied via the ModelPicker (Phase 3).
/// The ACP session's current model diverges from the persona model because
@@ -35,6 +68,11 @@ pub enum ConfigOrigin {
/// env var. E.g. Claude Code only supports Anthropic as a provider; the
/// "locked" display is synthesized by the config bridge, not read from disk.
HarnessConstraint,
/// Value comes from a custom harness definition's `env` block.
/// Sits below global env and above structured persona/global values,
/// matching spawn Layer 2b. Only reachable for user-authored custom harness
/// JSONs with a non-empty `env` block; preset harnesses always have empty env.
HarnessDefault,
}
/// How a config field can be written back to the runtime.

3
desktop/src-tauri/src/managed_agents/runtime.rs
View File

@@ -22,8 +22,7 @@ pub(crate) use path::should_use_inherited;
mod metadata;
pub(crate) use metadata::{
resolve_effective_prompt_model_provider, resolve_session_title, runtime_metadata_env_vars,
SESSION_TITLE_ENV_VAR,
resolve_session_title, runtime_metadata_env_vars, SESSION_TITLE_ENV_VAR,
};
mod stop;

26
desktop/src-tauri/src/managed_agents/runtime/metadata.rs
View File

@@ -57,32 +57,6 @@ pub(crate) fn resolve_session_title(display_name: Option<&str>, name: &str) -> O
.find(|value| !value.is_empty())
}
/// Resolve effective prompt/model/provider using definition-authoritative
/// semantics for linked instances.
///
/// Used by `agent_config.rs` to inject persona defaults into the config surface
/// before running the reader.
pub(crate) fn resolve_effective_prompt_model_provider(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::AgentDefinition],
record_prompt: Option<String>,
record_model: Option<String>,
record_provider: Option<String>,
) -> (Option<String>, Option<String>, Option<String>) {
match persona_id.and_then(|pid| personas.iter().find(|p| p.id == pid)) {
Some(p) => {
fn non_blank(v: Option<&str>) -> Option<String> {
v.filter(|s| !s.trim().is_empty()).map(str::to_owned)
}
let prompt = non_blank(Some(&p.system_prompt));
let model = non_blank(p.model.as_deref());
let provider = non_blank(p.provider.as_deref());
(prompt, model, provider)
}
None => (record_prompt, record_model, record_provider),
}
}
#[cfg(test)]
mod tests {
use super::resolve_session_title;

2
desktop/src/features/agents/ui/AgentConfigPanel.tsx
View File

@@ -137,6 +137,8 @@ function provenanceSentence(
return "From ACP session";
case "globalDefault":
return "Inherited from global defaults";
case "harnessDefault":
return "Inherited from harness definition";
}
}

4
desktop/src/shared/api/types.ts
View File

@@ -602,7 +602,6 @@ export type AgentModelInfo = {
};
// ── Config bridge types ──────────────────────────────────────────────────────
export type ConfigOrigin =
| "buzzExplicit"
| "acpNativeRead"
@@ -612,7 +611,8 @@ export type ConfigOrigin =
| "personaDefault"
| "globalDefault"
| "runtimeOverride"
| "harnessConstraint";
| "harnessConstraint"
| "harnessDefault";
export type ConfigWriteMechanism =
| { type: "respawnWithEnvVar"; envKey: string }