From 2f2ad409a76606dd9c54872dc518b0ecb72756af Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Fri, 10 Jul 2026 15:45:42 -0400 Subject: [PATCH] feat(desktop): thread baked build env into global agent config UI (#1722) Signed-off-by: Will Pfleger Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 --- desktop/scripts/check-file-sizes.mjs | 4 +- .../src-tauri/src/commands/agent_config.rs | 17 ++ .../features/agents/ui/EnvVarsEditor.test.mjs | 160 ++++++++++++++++++ .../src/features/agents/ui/EnvVarsEditor.tsx | 98 ++++++++++- .../src/features/agents/ui/bakedEnvHelpers.ts | 22 +++ .../ui/GlobalAgentConfigSettingsCard.tsx | 97 +++++------ 6 files changed, 348 insertions(+), 50 deletions(-) create mode 100644 desktop/src/features/agents/ui/bakedEnvHelpers.ts diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 2c0991459..7778db786 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -303,7 +303,9 @@ const overrides = new Map([ // global-agent-config: get_agent_config_surface / write_agent_config_field / // put_agent_session_config commands + GlobalAgentConfig serde types. New file // in this PR; queued to split with the command module refactor. - ["src-tauri/src/commands/agent_config.rs", 1002], + // +17: baked-env-global-unify: BUZZ_AGENT_THINKING_EFFORT added to + // is_safe_to_reveal allowlist + baked_env_thinking_effort_is_unmasked test. + ["src-tauri/src/commands/agent_config.rs", 1019], // draft-persistence predicate: submit-time `loadDraft` check + inline comment // + deps-array entry in submitMessage closes the never-persisted-boundary // defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index e056cbc18..71be269a9 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -275,11 +275,13 @@ pub struct BakedEnvEntry { /// /// Allowlist (case-insensitive): /// - `BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL` — agent runtime selection +/// - `BUZZ_AGENT_THINKING_EFFORT` — non-secret enum (none/minimal/low/medium/high/xhigh/max) /// - `DATABRICKS_HOST`, `DATABRICKS_MODEL` — Block non-secret defaults fn is_safe_to_reveal(key: &str) -> bool { const SAFE_KEYS: &[&str] = &[ "BUZZ_AGENT_PROVIDER", "BUZZ_AGENT_MODEL", + "BUZZ_AGENT_THINKING_EFFORT", "DATABRICKS_HOST", "DATABRICKS_MODEL", ]; @@ -973,6 +975,19 @@ mod tests { 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. @@ -980,6 +995,8 @@ mod tests { 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")); diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs b/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs index a1e09f54a..567195026 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs +++ b/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs @@ -8,6 +8,8 @@ * the emitted record (buildRecord merges required keys from value). * 3. Provider/runtime switch (skipKeys change) triggers a row reprojection * — the guard fires when skipKeys changes, even if value is unchanged. + * 4. inheritedRows: render/exclusion/override/no-serialize invariants. + * 5. getBakedProviderInheritLabel: label helper correctness. * * These are pure-logic tests — no React renderer needed. The transition tests * (Invariant 3) exercise the real exported `skipKeysEqual` guard that controls @@ -23,6 +25,7 @@ import { skipKeysEqual, isRequiredKeyMissing, } from "./EnvVarsEditor.tsx"; +import { getBakedProviderInheritLabel } from "./bakedEnvHelpers.ts"; // ── Invariant 1: toRows excludes skip keys ───────────────────────────────── @@ -352,3 +355,160 @@ test("isRequiredKeyMissing_keyExplicitlyEmpty_noInherited_missing", () => { "explicit empty local value with no inherited must be missing", ); }); + +// ── Invariant 4: inheritedRows — display/exclusion/override/no-serialize ─── +// +// These tests exercise the pure-logic invariants that must hold for the +// inherited build-defaults feature in EnvVarsEditor: +// +// (a) An inherited row with no matching local row IS visible (would render). +// (b) An inherited row whose key appears in `rows` is HIDDEN (local wins). +// (c) Serialization (buildRecord) NEVER includes inherited-only rows. +// (d) A local override row for a masked secret shows the masked value. +// +// Tests (a)–(d) operate on the same helpers used by the component render +// (toRows, toRecord, buildRecord) plus a simulated filter that mirrors the +// JSX `.filter((irow) => !rows.some((r) => r.key === irow.key))`. + +function simulateInheritedFilter(inheritedRows, rows) { + return inheritedRows.filter((irow) => !rows.some((r) => r.key === irow.key)); +} + +test("inheritedRows_no_local_row_row_is_visible", () => { + // DATABRICKS_HOST baked, no local row → inherited row shows. + const inherited = [ + { + key: "DATABRICKS_HOST", + value: "https://example.databricks.com/", + masked: false, + }, + ]; + const rows = toRows({}, new Set()); // no local env vars + const visible = simulateInheritedFilter(inherited, rows); + assert.equal( + visible.length, + 1, + "inherited row must be visible when no local override", + ); + assert.equal(visible[0].key, "DATABRICKS_HOST"); +}); + +test("inheritedRows_local_row_same_key_inherited_hidden", () => { + // User adds a local DATABRICKS_HOST row → inherited row must be hidden. + const inherited = [ + { + key: "DATABRICKS_HOST", + value: "https://baked.databricks.com/", + masked: false, + }, + ]; + const value = { DATABRICKS_HOST: "https://user.databricks.com/" }; + const rows = toRows(value, new Set()); + const visible = simulateInheritedFilter(inherited, rows); + assert.equal( + visible.length, + 0, + "inherited row must be hidden when local row has same key", + ); +}); + +test("inheritedRows_not_serialized_in_buildRecord", () => { + // buildRecord must never include keys that come only from inherited rows. + // Simulate: inherited has SECRET_KEY, local value does not. + const requiredKeys = []; + const value = { MY_VAR: "foo" }; + const rows = toRows(value, new Set(requiredKeys)); + // buildRecord reimplemented inline to match EnvVarsEditor's buildRecord: + const base = {}; + for (const key of requiredKeys) { + if (key in value) base[key] = value[key]; + } + const record = { ...base, ...toRecord(rows) }; + assert.equal( + "SECRET_KEY" in record, + false, + "inherited-only key must NOT appear in serialized record", + ); + assert.equal(record.MY_VAR, "foo", "non-inherited key preserved"); +}); + +test("inheritedRows_masked_secret_local_override_shows_masked_build_value", () => { + // Edge case: baked key is a masked secret (e.g. API_KEY → "••••••"), + // user types a local override → the hint would show the masked "••••••" value. + const inherited = [{ key: "API_KEY", value: "••••••", masked: true }]; + // The component finds override by: inheritedRows.find(irow => irow.key === row.key) + const row = { id: "r1", key: "API_KEY", value: "my-real-key" }; + const override = inherited.find((irow) => irow.key === row.key); + assert.ok(override, "override entry must be found for masked key"); + assert.equal( + override.value, + "••••••", + "masked baked value shown in override hint", + ); + assert.equal(override.masked, true, "masked flag preserved"); +}); + +test("inheritedRows_structured_keys_excluded_from_generic_rows", () => { + // BUZZ_AGENT_PROVIDER, BUZZ_AGENT_MODEL, BUZZ_AGENT_THINKING_EFFORT must + // be excluded from bakedGenericRows (they go to structured fields instead). + // This mirrors the BAKED_STRUCTURED_KEYS filter in GlobalAgentConfigSettingsCard. + const STRUCTURED = new Set([ + "BUZZ_AGENT_PROVIDER", + "BUZZ_AGENT_MODEL", + "BUZZ_AGENT_THINKING_EFFORT", + ]); + const allBaked = [ + { key: "BUZZ_AGENT_PROVIDER", value: "databricks_v2", masked: false }, + { key: "BUZZ_AGENT_MODEL", value: "goose-claude-opus-4-8", masked: false }, + { key: "BUZZ_AGENT_THINKING_EFFORT", value: "medium", masked: false }, + { + key: "DATABRICKS_HOST", + value: "https://example.databricks.com/", + masked: false, + }, + { key: "DATABRICKS_MODEL", value: "goose-claude-opus-4-8", masked: false }, + ]; + const generic = allBaked.filter((e) => !STRUCTURED.has(e.key)); + assert.equal( + generic.length, + 2, + "only non-structured keys go to generic rows", + ); + const genericKeys = generic.map((e) => e.key).sort(); + assert.deepEqual(genericKeys, ["DATABRICKS_HOST", "DATABRICKS_MODEL"]); +}); + +// ── Invariant 5: getBakedProviderInheritLabel — label helper ─────────────── + +test("getBakedProviderInheritLabel_known_provider_returns_friendly_name", () => { + const options = [ + { id: "anthropic", label: "Anthropic" }, + { id: "databricks_v2", label: "Databricks v2 (AI Gateway)" }, + { id: "openai", label: "OpenAI" }, + ]; + const label = getBakedProviderInheritLabel("databricks_v2", options); + assert.equal( + label, + "Databricks v2 (AI Gateway) (inherited from build)", + "known provider id must resolve to friendly label", + ); +}); + +test("getBakedProviderInheritLabel_unknown_provider_falls_back_to_raw_id", () => { + const options = [{ id: "anthropic", label: "Anthropic" }]; + const label = getBakedProviderInheritLabel("my-custom-provider", options); + assert.equal( + label, + "my-custom-provider (inherited from build)", + "unknown provider id must fall back to raw id", + ); +}); + +test("getBakedProviderInheritLabel_empty_options_falls_back_to_raw_id", () => { + const label = getBakedProviderInheritLabel("databricks_v2", []); + assert.equal( + label, + "databricks_v2 (inherited from build)", + "empty options table must fall back to raw id", + ); +}); diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.tsx b/desktop/src/features/agents/ui/EnvVarsEditor.tsx index e3ccf5e89..348eeba2e 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.tsx +++ b/desktop/src/features/agents/ui/EnvVarsEditor.tsx @@ -11,6 +11,18 @@ import { export type EnvVarsValue = Record; +/** + * A single baked / build-time env row passed as an inherited default. + * The value is already masked server-side for secrets (`masked === true`). + */ +export type InheritedEnvRow = { + key: string; + /** Display value — real value or `••••••` for masked keys. */ + value: string; + /** True when Rust replaced the real value with the mask placeholder. */ + masked: boolean; +}; + /** * Build a rows array from a value record, optionally skipping a set of keys. * Exported for unit tests. @@ -133,6 +145,21 @@ type EnvVarsEditorProps = { * it is set. Only acts on keys that appear in `requiredKeys`. */ focusKey?: string; + /** + * Read-only rows for baked / build-time inherited defaults. Displayed after + * required rows and before user-managed rows. A local row whose key matches + * an inherited row takes precedence — the inherited row is hidden and an + * "Overrides build default" hint is shown beneath the local row instead. + * + * **Invariant:** these rows are purely display; they never enter `rows` state + * and are never included in `buildRecord` / `onChange` output. Nothing baked + * is ever written into `global-agent-config.json`. + * + * Opt-in — agent create/edit dialogs do not pass this prop and are untouched. + */ + inheritedRows?: readonly InheritedEnvRow[]; + /** Label for the inherited-row tag (e.g. "build"). Defaults to "build". */ + inheritedRowsLabel?: string; }; type Row = { id: string; key: string; value: string }; @@ -156,6 +183,8 @@ export function EnvVarsEditor({ requiredKeys = [], fileSatisfiedKeys = [], focusKey, + inheritedRows = [], + inheritedRowsLabel = "build", }: EnvVarsEditorProps) { // Keys that render as their own special rows (required amber rows or // file-satisfied read-only rows). These must NEVER enter `rows` state — @@ -417,10 +446,64 @@ export function EnvVarsEditor({ ))} + {/* Inherited baked-build rows — read-only, visible value (pre-masked by Rust). + Hidden when a local user row has the same key (local wins). */} + {inheritedRows + .filter((irow) => !rows.some((r) => r.key === irow.key)) + .map((irow) => ( +
+
+
+ + + {irow.key} + + + Inherited from {inheritedRowsLabel} + +
+
+ + {irow.value} + +
+ {/* Spacer to align with the remove-button column */} +
+
+
+ ))} + {/* User-managed rows */} {rows.length === 0 && requiredKeys.length === 0 && - fileSatisfiedKeys.length === 0 ? ( + fileSatisfiedKeys.length === 0 && + inheritedRows.length === 0 ? (

No variables set.

@@ -494,6 +577,19 @@ export function EnvVarsEditor({

) : null} + {(() => { + if (row.key.length === 0) return null; + const override = inheritedRows.find( + (irow) => irow.key === row.key, + ); + if (!override) return null; + return ( +

+ Overrides {inheritedRowsLabel} default{" "} + {override.value} +

+ ); + })()}
); })} diff --git a/desktop/src/features/agents/ui/bakedEnvHelpers.ts b/desktop/src/features/agents/ui/bakedEnvHelpers.ts new file mode 100644 index 000000000..66a469074 --- /dev/null +++ b/desktop/src/features/agents/ui/bakedEnvHelpers.ts @@ -0,0 +1,22 @@ +/** + * Pure helper functions for displaying baked build env values in the global + * agent config card. Extracted into their own module so unit tests can import + * them without pulling in React, Tauri IPC, or TanStack Query. + */ + +/** + * Return the provider option label for the zero-value (inherit) option when a + * baked provider is present. Falls back to the raw provider id when the id + * doesn't appear in the options table. + * + * Used in GlobalAgentConfigSettingsCard to relabel the provider dropdown's + * empty-selection option when a baked build provider is set. + */ +export function getBakedProviderInheritLabel( + bakedProviderId: string, + options: readonly { id: string; label: string }[], +): string { + const match = options.find((o) => o.id === bakedProviderId); + const friendlyName = match ? match.label : bakedProviderId; + return `${friendlyName} (inherited from build)`; +} diff --git a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx index 565e302c3..43aa4d236 100644 --- a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx +++ b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx @@ -21,6 +21,8 @@ import { getBakedBuildEnv, type BakedEnvEntry } from "@/shared/api/tauri"; import { globalAgentConfigQueryKey } from "@/features/agents/useGlobalAgentConfig"; import { useAcpRuntimesQuery } from "@/features/agents/hooks"; import { EnvVarsEditor } from "@/features/agents/ui/EnvVarsEditor"; +import type { InheritedEnvRow } from "@/features/agents/ui/EnvVarsEditor"; +import { getBakedProviderInheritLabel } from "@/features/agents/ui/bakedEnvHelpers"; import { AUTO_PROVIDER_DROPDOWN_VALUE, CUSTOM_PROVIDER_DROPDOWN_VALUE, @@ -47,6 +49,17 @@ const EMPTY_CONFIG: GlobalAgentConfig = { model: null, }; +/** + * Baked env keys that map to structured fields (provider/model/effort dropdowns). + * These are routed to their own UI controls and must NOT appear as generic + * inherited rows in the env editor. + */ +const BAKED_STRUCTURED_KEYS = new Set([ + "BUZZ_AGENT_PROVIDER", + "BUZZ_AGENT_MODEL", + BUZZ_AGENT_THINKING_EFFORT, +]); + type SaveState = "idle" | "saving" | "saved" | "error"; export function GlobalAgentConfigSettingsCard() { @@ -100,6 +113,28 @@ export function GlobalAgentConfigSettingsCard() { }); }, []); + // Derive structured-field values and generic env rows from bakedEnv. + // Structured keys (BUZZ_AGENT_PROVIDER / BUZZ_AGENT_MODEL / BUZZ_AGENT_THINKING_EFFORT) + // route to their respective dropdown controls and are excluded from the + // generic inherited-rows list passed to EnvVarsEditor. + const bakedProvider = React.useMemo( + () => bakedEnv.find((e) => e.key === "BUZZ_AGENT_PROVIDER")?.value ?? null, + [bakedEnv], + ); + const bakedModel = React.useMemo( + () => bakedEnv.find((e) => e.key === "BUZZ_AGENT_MODEL")?.value ?? null, + [bakedEnv], + ); + const bakedEffort = React.useMemo( + () => + bakedEnv.find((e) => e.key === BUZZ_AGENT_THINKING_EFFORT)?.value ?? null, + [bakedEnv], + ); + const bakedGenericRows = React.useMemo( + () => bakedEnv.filter((e) => !BAKED_STRUCTURED_KEYS.has(e.key)), + [bakedEnv], + ); + // Resolve the buzz-agent runtime catalog entry for model discovery. // The card is always visible (open=true), so the query is always enabled. const runtimesQuery = useAcpRuntimesQuery(); @@ -213,6 +248,15 @@ export function GlobalAgentConfigSettingsCard() { ? CUSTOM_PROVIDER_DROPDOWN_VALUE : providerValue || AUTO_PROVIDER_DROPDOWN_VALUE; + // When a baked provider is present and no explicit global provider is set, + // relabel the zero-value option to surface the inherited-from-build value. + // When an explicit provider IS set, the zero-value option is still shown in + // the list but never selected — its label doesn't matter. + const providerZeroLabel = React.useMemo(() => { + if (!bakedProvider) return null; + return getBakedProviderInheritLabel(bakedProvider, providerOptions); + }, [bakedProvider, providerOptions]); + return (
- {opt.label} + {opt.id === "" ? (providerZeroLabel ?? opt.label) : opt.label} ))}