diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 1bd8af8976..ce3d252203 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -32,9 +32,11 @@ import { import { AUTO_PROVIDER_DROPDOWN_VALUE, BLOCK_BUILD_HIDDEN_PROVIDER_IDS, + CARD_MINT_KEY_ANNOTATIONS, CUSTOM_PROVIDER_DROPDOWN_VALUE, getPersonaProviderOptions, getProviderApiKeyEnvVar, + getProviderApiKeyLabel, runtimeSupportsLlmProviderSelection, } from "@/features/agents/ui/agentConfigOptions"; import { @@ -54,6 +56,7 @@ import { } from "@/features/agents/ui/buzzAgentModelTuningFields"; import { SettingsOptionGroup } from "@/features/settings/ui/SettingsOptionGroup"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; +import { CardMintKeyCue } from "./CardMintKeyCue"; import { getGlobalAgentCredentialState } from "./globalAgentCredentialState"; export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { @@ -74,7 +77,6 @@ const PROGRESSIVE_FIELDS_TRANSITION = { duration: 0.22, ease: [0.23, 1, 0.32, 1], } as const; - type AgentConfigDisclosure = | "full" | "onboarding-essential" @@ -85,13 +87,9 @@ type AgentConfigDisclosure = // - auto-select a valid model when the provider changes // - keep the model select usable during discovery // - preserve credential env vars across provider switches (the abandoned -// provider's key stays in env_vars — visible/deletable under Advanced — -// so flipping back never loses a typed key; spawned agents may therefore -// see credentials for providers they don't use) +// provider's key stays in env_vars — visible/deletable under Advanced) // - require a provider before model/effort are editable (no saveable -// invalid state — design principle #4). Note: legacy configs saved with -// a model but no provider are cleared by the pre-existing orphan-model -// effect on next edit — deliberate data healing, documented in PR. +// invalid state — design principle #4) const autoSelectModelOnProviderChange = true; const disableModelSelectDuringDiscovery = false; const preserveCredentialEnvVarsOnProviderChange = true; @@ -747,6 +745,7 @@ export function AgentConfigFields({
onConfigChange({ ...config, @@ -869,6 +864,7 @@ export function AgentConfigFields({ {showAdvancedFields ? (
+
); })} @@ -596,6 +613,14 @@ export function EnvVarsEditor({

); })()} + {row.key.length > 0 && keyAnnotations?.[row.key] ? ( +

+ {keyAnnotations[row.key]} +

+ ) : null}
); })} diff --git a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx index 99cf3e97a8..01485dd9eb 100644 --- a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx @@ -6,6 +6,7 @@ import type { PersonaBehaviorDraft } from "./personaBehaviorDraft"; import { isBuzzAgentRuntime } from "./buzzAgentConfig"; import { BuzzAgentModelTuningFields } from "./buzzAgentModelTuningFields"; import { + CARD_MINT_KEY_ANNOTATIONS, PERSONA_FIELD_CONTROL_CLASS, PERSONA_FIELD_SHELL_CLASS, PERSONA_LABEL_OPTIONAL_CLASS, @@ -142,6 +143,7 @@ export function PersonaAdvancedFields({ disabled={disabled} fileSatisfiedKeys={fileSatisfiedEnvKeys} hiddenKeys={hiddenEnvKeys} + keyAnnotations={CARD_MINT_KEY_ANNOTATIONS} onChange={onEnvVarsChange} requiredKeys={requiredEnvKeys} value={envVars} diff --git a/desktop/src/features/agents/ui/PersonaProviderApiKeyField.test.mjs b/desktop/src/features/agents/ui/PersonaProviderApiKeyField.test.mjs new file mode 100644 index 0000000000..62a6ff416c --- /dev/null +++ b/desktop/src/features/agents/ui/PersonaProviderApiKeyField.test.mjs @@ -0,0 +1,170 @@ +/** + * Behavioral tests for PersonaProviderApiKeyField. + * + * Tests the rendering invariants that matter for the disambiguation story: + * - semantic label is present in the rendered output + * - envVarName hint is rendered when the prop is present + * - hint id is wired to the input via aria-describedby + * - hint is absent when envVarName is omitted + * - two simultaneous instances produce unique IDs (no duplicate-ID collision + * in the nested AgentInstanceEditDialog + AgentDefaultsDialog path) + */ + +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import React from "react"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { PersonaProviderApiKeyField } from "./PersonaProviderApiKeyField.tsx"; + +function makeProps(overrides = {}) { + return { + disabled: false, + isInherited: false, + inheritedLabel: "Set in global defaults", + isRequired: false, + label: "OpenAI Runtime API Key", + onValueChange: () => {}, + value: "", + ...overrides, + }; +} + +/** Extract the value of the first attribute matching `name="..."` in html. */ +function extractAttr(html, attrName) { + const re = new RegExp(`${attrName}="([^"]+)"`); + const m = re.exec(html); + return m ? m[1] : null; +} + +/** Extract ALL values of an attribute from html, in document order. */ +function extractAllAttrs(html, attrName) { + const re = new RegExp(`${attrName}="([^"]+)"`, "g"); + return Array.from(html.matchAll(re), (m) => m[1]); +} + +test("PersonaProviderApiKeyField_renders_semantic_label", () => { + const html = renderToStaticMarkup( + React.createElement(PersonaProviderApiKeyField, makeProps()), + ); + assert.ok( + html.includes("OpenAI Runtime API Key"), + "semantic label must appear in rendered output", + ); +}); + +test("PersonaProviderApiKeyField_renders_env_var_hint_when_envVarName_present", () => { + const html = renderToStaticMarkup( + React.createElement( + PersonaProviderApiKeyField, + makeProps({ envVarName: "OPENAI_COMPAT_API_KEY" }), + ), + ); + assert.ok( + html.includes("OPENAI_COMPAT_API_KEY"), + "env-var hint must appear when envVarName is provided", + ); +}); + +test("PersonaProviderApiKeyField_wires_hint_id_via_aria_describedby", () => { + const html = renderToStaticMarkup( + React.createElement( + PersonaProviderApiKeyField, + makeProps({ envVarName: "OPENAI_COMPAT_API_KEY" }), + ), + ); + // Extract the dynamically-generated hint id from the rendered paragraph. + const hintId = extractAttr(html, "id"); + assert.ok(hintId, "hint paragraph must have an id"); + assert.ok( + hintId.startsWith("persona-provider-api-key-hint-"), + `hint id must follow the expected prefix, got: ${hintId}`, + ); + // The input's aria-describedby must point at the same id. + const describedBy = extractAttr(html, "aria-describedby"); + assert.equal( + describedBy, + hintId, + "input aria-describedby must reference the hint's id", + ); +}); + +test("PersonaProviderApiKeyField_omits_hint_when_envVarName_absent", () => { + const html = renderToStaticMarkup( + React.createElement(PersonaProviderApiKeyField, makeProps()), + ); + assert.ok( + !html.includes("aria-describedby"), + "no aria-describedby when envVarName is omitted", + ); + assert.ok( + !html.includes("persona-provider-api-key-hint"), + "hint id must not appear when envVarName is omitted", + ); +}); + +test("PersonaProviderApiKeyField_two_instances_have_unique_ids_and_each_aria_describedby_resolves_to_own_hint", () => { + // Render BOTH instances in a single renderToStaticMarkup call — this + // mirrors the real nested-dialog DOM where AgentInstanceEditDialog's + // credential field and the nested AgentDefaultsDialog's field are + // simultaneously mounted under the same React root. A shared root is what + // makes React.useId() guarantee uniqueness; two separate renderToStaticMarkup + // calls each reset the counter and would produce the same ID. + const combined = renderToStaticMarkup( + React.createElement( + React.Fragment, + null, + React.createElement( + PersonaProviderApiKeyField, + makeProps({ + label: "Anthropic API Key", + envVarName: "ANTHROPIC_API_KEY", + }), + ), + React.createElement( + PersonaProviderApiKeyField, + makeProps({ + label: "OpenAI Runtime API Key", + envVarName: "OPENAI_COMPAT_API_KEY", + }), + ), + ), + ); + + // Two hint paragraph ids must be present and distinct. + const allHintIds = extractAllAttrs(combined, "id").filter((id) => + id.startsWith("persona-provider-api-key-hint-"), + ); + assert.equal(allHintIds.length, 2, "exactly two hint ids must be present"); + const [hintIdA, hintIdB] = allHintIds; + assert.notEqual(hintIdA, hintIdB, "two instances must not share a hint id"); + + // Each input's aria-describedby must match its own hint id (same order). + const allDescribedBy = extractAllAttrs(combined, "aria-describedby"); + assert.equal( + allDescribedBy.length, + 2, + "exactly two aria-describedby attributes must be present", + ); + assert.equal( + allDescribedBy[0], + hintIdA, + "instance A: aria-describedby must reference instance A's own hint", + ); + assert.equal( + allDescribedBy[1], + hintIdB, + "instance B: aria-describedby must reference instance B's own hint", + ); + + // Confirm each instance names its own env var in the rendered output. + assert.ok( + combined.includes("ANTHROPIC_API_KEY"), + "combined output must name ANTHROPIC_API_KEY", + ); + assert.ok( + combined.includes("OPENAI_COMPAT_API_KEY"), + "combined output must name OPENAI_COMPAT_API_KEY", + ); +}); diff --git a/desktop/src/features/agents/ui/PersonaProviderApiKeyField.tsx b/desktop/src/features/agents/ui/PersonaProviderApiKeyField.tsx index 17f9e2e826..2be1f1c28d 100644 --- a/desktop/src/features/agents/ui/PersonaProviderApiKeyField.tsx +++ b/desktop/src/features/agents/ui/PersonaProviderApiKeyField.tsx @@ -25,6 +25,7 @@ import { */ export function PersonaProviderApiKeyField({ disabled, + envVarName, isInherited, inheritedLabel, isRequired, @@ -33,6 +34,13 @@ export function PersonaProviderApiKeyField({ value, }: { disabled: boolean; + /** + * The backing environment variable name, e.g. `OPENAI_COMPAT_API_KEY`. + * Rendered as a monospace hint beneath the label so users can distinguish + * this field from other keys with similar names (e.g. `OPENAI_API_KEY`). + * When present, the input's `aria-describedby` points at the hint element. + */ + envVarName?: string; /** True when the key is satisfied by an inherited layer. */ isInherited: boolean; /** Human-readable source of the inherited value. */ @@ -46,13 +54,22 @@ export function PersonaProviderApiKeyField({ value: string; }) { const [showValue, setShowValue] = React.useState(false); - const inputId = "persona-provider-api-key"; + const uid = React.useId(); + const inputId = `persona-provider-api-key-${uid}`; + const hintId = envVarName + ? `persona-provider-api-key-hint-${uid}` + : undefined; return (
{label} + {envVarName ? ( +

+ {envVarName} +

+ ) : null}
{ + assert.equal(getProviderApiKeyLabel("anthropic"), "Anthropic API Key"); +}); + +test("getProviderApiKeyLabel_openai_returns_openai_runtime_label", () => { + assert.equal(getProviderApiKeyLabel("openai"), "OpenAI Runtime API Key"); +}); + +test("getProviderApiKeyLabel_openai_compat_returns_distinct_label", () => { + // openai and openai-compat must have distinct labels — both use + // OPENAI_COMPAT_API_KEY but carry different semantic identities. + assert.equal( + getProviderApiKeyLabel("openai-compat"), + "OpenAI-compatible Runtime API Key", + ); +}); + +test("getProviderApiKeyLabel_openrouter_returns_openrouter_label", () => { + // Key fix: OpenRouter was mislabeled "OpenAI API Key" before this change. + assert.equal(getProviderApiKeyLabel("openrouter"), "OpenRouter API Key"); +}); + +test("getProviderApiKeyLabel_databricks_returns_null", () => { + // Databricks uses OAuth PKCE — no typed-secret label. + assert.equal(getProviderApiKeyLabel("databricks"), null); +}); + +test("getProviderApiKeyLabel_databricks_v2_returns_null", () => { + assert.equal(getProviderApiKeyLabel("databricks_v2"), null); +}); + +test("getProviderApiKeyLabel_unknown_provider_returns_null", () => { + assert.equal(getProviderApiKeyLabel("some-unknown-provider"), null); +}); + +test("getProviderApiKeyLabel_provider_id_trimmed_and_lowercased", () => { + // Mirrors getProviderApiKeyEnvVar normalisation behaviour. + assert.equal(getProviderApiKeyLabel(" Anthropic "), "Anthropic API Key"); +}); diff --git a/desktop/src/features/agents/ui/agentConfigOptions.tsx b/desktop/src/features/agents/ui/agentConfigOptions.tsx index d51c970f29..38865ca148 100644 --- a/desktop/src/features/agents/ui/agentConfigOptions.tsx +++ b/desktop/src/features/agents/ui/agentConfigOptions.tsx @@ -63,19 +63,30 @@ export type PersonaDropdownOption = { * * `requiredEnvKeys`: keys that must be present in the agent's effective env for * the provider to work (surfaced as amber required rows in EnvVarsEditor). - * `secretEnvVar`: the one env key that holds a user-typed secret (API key). - * Only set for providers where the credential is a plaintext secret the user - * pastes in. Cleared automatically when the user switches away from the - * provider. Databricks uses OAuth PKCE (no typed secret), so it has no - * secretEnvVar. + * `secretEnvVar` + `apiKeyLabel`: paired — either both are present or neither + * is. `secretEnvVar` is the env key holding the user-typed secret; clearing + * it when the user switches providers ensures no orphaned credentials remain. + * Databricks uses OAuth PKCE (no typed secret), so it carries neither field. + * `apiKeyLabel` is the human-readable label shown in the credential field; + * derived by `getProviderApiKeyLabel` — single source of truth for all UI + * surfaces so they never drift. * * Mirrors the Rust `readiness::buzz_agent_requirements` / * `readiness::goose_requirements` logic — keep in sync. */ -export type ProviderCredentialConfig = { - requiredEnvKeys: readonly string[]; - secretEnvVar?: string; -}; +export type ProviderCredentialConfig = + | { + requiredEnvKeys: readonly string[]; + secretEnvVar?: undefined; + apiKeyLabel?: undefined; + } + | { + requiredEnvKeys: readonly string[]; + /** The env key holding the user-typed API secret. */ + secretEnvVar: string; + /** Display label for the credential input field, e.g. "Anthropic API Key". */ + apiKeyLabel: string; + }; /** * Unified provider credential config table. Single source of truth for both @@ -87,20 +98,23 @@ const PROVIDER_CREDENTIAL_CONFIG: Partial< anthropic: { requiredEnvKeys: ["ANTHROPIC_API_KEY"], secretEnvVar: "ANTHROPIC_API_KEY", + apiKeyLabel: "Anthropic API Key", }, openai: { requiredEnvKeys: ["OPENAI_COMPAT_API_KEY"], secretEnvVar: "OPENAI_COMPAT_API_KEY", + apiKeyLabel: "OpenAI Runtime API Key", }, "openai-compat": { requiredEnvKeys: ["OPENAI_COMPAT_API_KEY"], secretEnvVar: "OPENAI_COMPAT_API_KEY", + apiKeyLabel: "OpenAI-compatible Runtime API Key", }, databricks: { // DATABRICKS_TOKEN is NOT required — OAuth PKCE is the normal path. requiredEnvKeys: ["DATABRICKS_HOST"], - // No secretEnvVar: DATABRICKS_HOST is a URL, not a secret credential, and - // is not cleared on provider switch (unlike API keys). + // No secretEnvVar / apiKeyLabel: DATABRICKS_HOST is a URL, not a secret + // credential, and is not cleared on provider switch (unlike API keys). }, databricks_v2: { // DATABRICKS_TOKEN is NOT required — OAuth PKCE is the normal path. @@ -113,6 +127,7 @@ const PROVIDER_CREDENTIAL_CONFIG: Partial< openrouter: { requiredEnvKeys: ["OPENROUTER_API_KEY"], secretEnvVar: "OPENROUTER_API_KEY", + apiKeyLabel: "OpenRouter API Key", }, }; @@ -402,6 +417,31 @@ export function getProviderApiKeyEnvVar(providerId: string): string | null { ); } +/** + * Returns the display label for the provider's API key field, if any. + * Derived from PROVIDER_CREDENTIAL_CONFIG.apiKeyLabel — single source of truth + * for all credential field labels so every surface stays in sync. + * + * Returns null when the provider has no typed-secret credential (e.g., + * Databricks, which uses OAuth PKCE). + */ +export function getProviderApiKeyLabel(providerId: string): string | null { + return ( + PROVIDER_CREDENTIAL_CONFIG[providerId.trim().toLowerCase()]?.apiKeyLabel ?? + null + ); +} + +/** + * Muted contextual hint for the `OPENAI_API_KEY` row in env editors. + * Pass as `keyAnnotations` to every `EnvVarsEditor` that may surface this key + * (Agent Defaults, agent edit dialog, persona definition dialog). Exported + * so the constant is defined once and never duplicated across surfaces. + */ +export const CARD_MINT_KEY_ANNOTATIONS: Readonly> = { + OPENAI_API_KEY: "Used for minting agent trading cards", +}; + export function shouldClearKnownModelForSelectionScope({ model, provider, diff --git a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs index 23d79aaf7f..548c9ccc0a 100644 --- a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs +++ b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs @@ -21,7 +21,8 @@ test("model discovery status names missing OpenAI-compatible credentials", () => ); assert.equal(status?.tone, "warning"); - assert.match(status?.message ?? "", /OpenAI API key/); + assert.match(status?.message ?? "", /OpenAI runtime API key/); + assert.match(status?.message ?? "", /OPENAI_COMPAT_API_KEY/); assert.match(status?.message ?? "", /OpenAI models/); }); diff --git a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts index c6991fdd01..b943f6455c 100644 --- a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts +++ b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts @@ -110,7 +110,8 @@ export function formatModelDiscoveryErrorStatus( if (message.includes("OPENAI_COMPAT_API_KEY required")) { return { - message: "Enter an OpenAI API key to load OpenAI models.", + message: + "Enter an OpenAI runtime API key (OPENAI_COMPAT_API_KEY) to load OpenAI models.", tone: "warning", }; } diff --git a/desktop/tests/e2e/global-agent-config-screenshots.spec.ts b/desktop/tests/e2e/global-agent-config-screenshots.spec.ts index 7ec1daa236..4c72b72f75 100644 --- a/desktop/tests/e2e/global-agent-config-screenshots.spec.ts +++ b/desktop/tests/e2e/global-agent-config-screenshots.spec.ts @@ -980,4 +980,44 @@ test.describe("global agent config screenshots", () => { path: `${SHOTS}/11-edit-runtime-less-provider-required-save-blocked.png`, }); }); + + // Will's exact stuck path: databricks_v2 global provider + saved global + // OPENAI_API_KEY. The cue must be visible without opening Advanced; once + // Advanced is opened the annotation must appear on the matching row. + test("card-mint-key-cue-visible-and-annotation-in-advanced", async ({ + page, + }) => { + await installMockBridge(page, { + globalAgentConfig: { + provider: "databricks_v2", + model: null, + preferred_runtime: "buzz-agent", + env_vars: { OPENAI_API_KEY: "sk-placeholder" }, + }, + }); + + await openAiDefaultsSettings(page); + + const card = page.getByTestId("settings-global-agent-config"); + + // The cue must be visible without the user opening Advanced. + await expect(card.getByTestId("card-mint-key-cue")).toBeVisible(); + await expect(card.getByTestId("card-mint-key-cue")).toContainText( + "OPENAI_API_KEY", + ); + await expect(card.getByTestId("card-mint-key-cue")).toContainText( + "Advanced → Environment variables", + ); + + // Advanced is collapsed at this point. + const advancedToggle = card.getByTestId("global-agent-advanced-toggle"); + await expect(advancedToggle).toHaveAttribute("aria-expanded", "false"); + + // Open Advanced — the OPENAI_API_KEY row's annotation must be visible. + await advancedToggle.click(); + await expect(advancedToggle).toHaveAttribute("aria-expanded", "true"); + await expect( + card.getByText("Used for minting agent trading cards"), + ).toBeVisible(); + }); }); diff --git a/desktop/tests/e2e/persona-env-vars.spec.ts b/desktop/tests/e2e/persona-env-vars.spec.ts index 1e9b077a82..60a8e888b2 100644 --- a/desktop/tests/e2e/persona-env-vars.spec.ts +++ b/desktop/tests/e2e/persona-env-vars.spec.ts @@ -329,7 +329,7 @@ test("persona model options follow the selected LLM provider", async ({ await selectDropdownOption(page, llmProvider, "OpenAI"); const dialog = page.getByRole("dialog"); - await expect(dialog.getByLabel("OpenAI API Key")).toBeVisible(); + await expect(dialog.getByLabel("OpenAI Runtime API Key")).toBeVisible(); await expect( dialog.getByRole("button", { name: "Advanced", exact: true }), ).toHaveAttribute("aria-expanded", "false"); @@ -343,7 +343,7 @@ test("persona model options follow the selected LLM provider", async ({ await selectDropdownOption(page, llmProvider, "Anthropic"); await expect(dialog.getByLabel("Anthropic API Key")).toBeVisible(); - await expect(dialog.getByLabel("OpenAI API Key")).not.toBeVisible(); + await expect(dialog.getByLabel("OpenAI Runtime API Key")).not.toBeVisible(); await expect(model).toBeVisible(); // Switch back to inherited defaults — per-agent provider, credential, and