Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion desktop/scripts/check-file-sizes.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions desktop/src-tauri/src/commands/agent_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
];
Expand Down Expand Up @@ -973,13 +975,28 @@ 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.
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"));
Expand Down
160 changes: 160 additions & 0 deletions desktop/src/features/agents/ui/EnvVarsEditor.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,6 +25,7 @@ import {
skipKeysEqual,
isRequiredKeyMissing,
} from "./EnvVarsEditor.tsx";
import { getBakedProviderInheritLabel } from "./bakedEnvHelpers.ts";

// ── Invariant 1: toRows excludes skip keys ─────────────────────────────────

Expand Down Expand Up @@ -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",
);
});
98 changes: 97 additions & 1 deletion desktop/src/features/agents/ui/EnvVarsEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@ import {

export type EnvVarsValue = Record<string, string>;

/**
* 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.
Expand Down Expand Up @@ -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 };
Expand All @@ -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 —
Expand Down Expand Up @@ -417,10 +446,64 @@ export function EnvVarsEditor({
</div>
))}

{/* 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) => (
<div key={irow.key} className="space-y-1">
<div className="flex items-center gap-2">
<div
className={cn(
"flex min-h-11 flex-1 items-center gap-1.5 px-3",
PERSONA_FIELD_SHELL_CLASS,
"border-muted-foreground/20 bg-muted/20",
)}
>
<Lock
className="h-3 w-3 shrink-0 text-muted-foreground/40"
aria-hidden
/>
<span
className="font-mono text-sm leading-6 text-foreground/60"
data-testid="env-vars-inherited-key"
>
{irow.key}
</span>
<span className="ml-1 rounded-sm bg-muted px-1 py-0.5 text-2xs font-medium text-muted-foreground">
Inherited from {inheritedRowsLabel}
</span>
</div>
<div
className={cn(
"flex min-h-11 flex-[2] items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
"opacity-60",
)}
>
<span
className={cn(
"font-mono text-sm",
irow.masked
? "text-muted-foreground/50"
: "text-foreground/70",
)}
data-testid="env-vars-inherited-value"
>
{irow.value}
</span>
</div>
{/* Spacer to align with the remove-button column */}
<div className="h-9 w-9 shrink-0" aria-hidden />
</div>
</div>
))}

{/* User-managed rows */}
{rows.length === 0 &&
requiredKeys.length === 0 &&
fileSatisfiedKeys.length === 0 ? (
fileSatisfiedKeys.length === 0 &&
inheritedRows.length === 0 ? (
<p className="text-xs italic text-muted-foreground">
No variables set.
</p>
Expand Down Expand Up @@ -494,6 +577,19 @@ export function EnvVarsEditor({
</span>
</p>
) : null}
{(() => {
if (row.key.length === 0) return null;
const override = inheritedRows.find(
(irow) => irow.key === row.key,
);
if (!override) return null;
return (
<p className="ml-1 text-xs text-muted-foreground">
Overrides {inheritedRowsLabel} default{" "}
<span className="font-mono">{override.value}</span>
</p>
);
})()}
</div>
);
})}
Expand Down
22 changes: 22 additions & 0 deletions desktop/src/features/agents/ui/bakedEnvHelpers.ts
Original file line number Diff line number Diff line change
@@ -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)`;
}
Loading
Loading