mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): disambiguate provider API key labels and annotate mint key (#4406)
Two different credentials were presented under the same name throughout the app. The top-level credential field for non-Anthropic providers (OpenAI, OpenAI-compatible, OpenRouter) was labeled "OpenAI API Key" via a hardcoded binary ternary repeated in three dialogs. The card-minting key (`OPENAI_API_KEY`) and the runtime credential (`OPENAI_COMPAT_API_KEY`) have independent endpoint namespaces and consumers (`OPENAI_COMPAT_BASE_URL`/`OPENAI_COMPAT_API_KEY` for runtime, `OPENAI_BASE_URL`/`OPENAI_API_KEY` for minting) and must remain separate — either may require a different credential. This PR makes them impossible to confuse in the UI. ## Changes **Provider-accurate labels from the credential table.** `PROVIDER_CREDENTIAL_CONFIG` entries now carry an `apiKeyLabel` paired with `secretEnvVar` as a discriminated union (both present or neither — a future provider cannot ship a secret field with no label). `getProviderApiKeyLabel(providerId)` is the single source of truth. The three hardcoded ternaries in `AgentConfigFields`, `AgentInstanceEditDialog`, and `AgentDefinitionDialog` are replaced by this helper. Labels: `openai` → "OpenAI Runtime API Key", `openai-compat` → "OpenAI-compatible Runtime API Key", `openrouter` → "OpenRouter API Key" (was incorrectly "OpenAI API Key"), `anthropic` → "Anthropic API Key" (unchanged). **Field names its backing env var.** `PersonaProviderApiKeyField` renders the env var name as a monospace hint beneath the label with `aria-describedby` wiring. All three call sites pass their `secretEnvVar`. A user who sees `OPENAI_API_KEY` in the mint dialog can now confirm at a glance that the credential field shows `OPENAI_COMPAT_API_KEY` — a different key. **Signpost visible at the decision point.** `CARD_MINT_KEY_ANNOTATIONS` is exported from `agentConfigOptions.tsx` (single source) and passed as `keyAnnotations` to all three generic env editors: both `EnvVarsEditor` branches in Agent Defaults, `EditAgentAdvancedFields`, and `PersonaAdvancedFields`. `CardMintKeyCue` — a new small component — renders an always-visible muted cue beneath the Advanced toggle when `OPENAI_API_KEY` is present in global env (Advanced is collapsed by default, so the per-row annotation is invisible until the cue guides the user to open it). **Model discovery error copy.** The `OPENAI_COMPAT_API_KEY required` message now reads "Enter an OpenAI runtime API key (OPENAI_COMPAT_API_KEY) to load OpenAI models." — naming the env var explicitly so it cannot be confused with the mint key. ## Tests - `getProviderApiKeyLabel` helper: pinned correct label per provider including the new distinct labels for `openai` and `openai-compat` - `PersonaProviderApiKeyField` render: semantic label present; env-var hint rendered when `envVarName` provided; `aria-describedby` wired to hint id; hint and describedby absent when prop omitted - `EnvVarsEditor` render: annotation appears exactly once on the matching row; absent for non-matching rows - `personaModelDiscoveryStatus`: pinned new copy naming `OPENAI_COMPAT_API_KEY` explicitly - Playwright: stale `"OpenAI API Key"` selectors updated; new `card-mint-key-cue-visible-and-annotation-in-advanced` test covers Will's exact path (databricks_v2 global provider + saved `OPENAI_API_KEY` → cue visible before opening Advanced → annotation present after opening) ## File sizes (post-format) | File | Lines | |------|-------| | `AgentConfigFields.tsx` | 994 (≤ 996) | | `AgentInstanceEditDialog.tsx` | 1228 (≤ 1228) | | `AgentDefinitionDialog.tsx` | 1045 (≤ 1047) | Related: [#4140](https://github.com/block/buzz/pull/4140) --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1ng3jzsaqxdhrfq22dg85j3lpr0zsh3jp7g2h9jyxl59wraayapnsu6kvfg <9a232143a0336e34814a6a0f4947e11bc50bc641f21572c886fd0ae1f7a4e867@buzz.block.builderlab.xyz>
This commit is contained in:
Will Pfleger
co-authored by
npub1ng3jzsaqxdhrfq22dg85j3lpr0zsh3jp7g2h9jyxl59wraayapnsu6kvfg
npub1ng3jzsaqxdhrfq22dg85j3lpr0zsh3jp7g2h9jyxl59wraayapnsu6kvfg
parent
f810a2f49e
commit
5e0efb0bb9
@@ -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({
|
||||
<div className={blockClassName}>
|
||||
<PersonaProviderApiKeyField
|
||||
disabled={false}
|
||||
envVarName={apiKeyEnvVar}
|
||||
inheritedLabel={
|
||||
apiKeyFileSatisfied
|
||||
? "Set in runtime config"
|
||||
@@ -754,11 +753,7 @@ export function AgentConfigFields({
|
||||
}
|
||||
isInherited={apiKeyInherited}
|
||||
isRequired={!apiKeyInherited && apiKeyValue.length === 0}
|
||||
label={
|
||||
effectiveProvider === "anthropic"
|
||||
? "Anthropic API Key"
|
||||
: "OpenAI API Key"
|
||||
}
|
||||
label={getProviderApiKeyLabel(effectiveProvider) ?? "API Key"}
|
||||
onValueChange={(value) =>
|
||||
onConfigChange({
|
||||
...config,
|
||||
@@ -869,6 +864,7 @@ export function AgentConfigFields({
|
||||
|
||||
{showAdvancedFields ? (
|
||||
<div className={cn(blockClassName, "space-y-3")}>
|
||||
<CardMintKeyCue envVars={config.env_vars} />
|
||||
<button
|
||||
aria-expanded={advancedOpen}
|
||||
className={cn(
|
||||
@@ -912,6 +908,7 @@ export function AgentConfigFields({
|
||||
hiddenKeys={apiKeyEnvVar ? [apiKeyEnvVar] : []}
|
||||
inheritedRows={bakedGenericRows}
|
||||
inheritedRowsLabel="build"
|
||||
keyAnnotations={CARD_MINT_KEY_ANNOTATIONS}
|
||||
label="Environment variables"
|
||||
onChange={handleEnvVarsChange}
|
||||
requiredKeys={advancedRequiredEnvKeys}
|
||||
@@ -930,6 +927,7 @@ export function AgentConfigFields({
|
||||
hiddenKeys={apiKeyEnvVar ? [apiKeyEnvVar] : []}
|
||||
inheritedRows={bakedGenericRows}
|
||||
inheritedRowsLabel="build"
|
||||
keyAnnotations={CARD_MINT_KEY_ANNOTATIONS}
|
||||
label="Environment variables"
|
||||
onChange={handleEnvVarsChange}
|
||||
requiredKeys={advancedRequiredEnvKeys}
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
getDefaultPersonaRuntime,
|
||||
getPersonaModelOptions,
|
||||
getPersonaProviderOptions,
|
||||
getProviderApiKeyLabel,
|
||||
getRuntimePersonaModelOptions,
|
||||
NO_RUNTIME_DROPDOWN_VALUE,
|
||||
runtimeSupportsLlmProviderSelection,
|
||||
@@ -907,14 +908,11 @@ export function AgentDefinitionDialog({
|
||||
topLevelSecretEnvVar ? (
|
||||
<PersonaProviderApiKeyField
|
||||
disabled={isPending}
|
||||
envVarName={topLevelSecretEnvVar}
|
||||
isInherited={apiKeyIsInherited}
|
||||
inheritedLabel={apiKeyInheritedLabel}
|
||||
isRequired={apiKeyIsRequired}
|
||||
label={
|
||||
effectiveProvider === "anthropic"
|
||||
? "Anthropic API key"
|
||||
: "OpenAI API key"
|
||||
}
|
||||
label={getProviderApiKeyLabel(effectiveProvider) ?? "API key"}
|
||||
onValueChange={(next) => {
|
||||
setEnvVars((prev) => ({
|
||||
...prev,
|
||||
|
||||
@@ -75,7 +75,10 @@ import {
|
||||
getBakedModelInheritLabel,
|
||||
getBakedProviderInheritLabel,
|
||||
} from "./bakedEnvHelpers";
|
||||
import { getProviderApiKeyEnvVar } from "./agentConfigOptions";
|
||||
import {
|
||||
getProviderApiKeyEnvVar,
|
||||
getProviderApiKeyLabel,
|
||||
} from "./agentConfigOptions";
|
||||
import { useAgentDialogDefaults } from "./useAgentDialogDefaults";
|
||||
import { AgentAiDefaultsNotice } from "./AgentAiDefaults";
|
||||
import { AgentDefaultsDialog } from "./AgentDefaultsDialog";
|
||||
@@ -1061,14 +1064,11 @@ export function AgentInstanceEditDialog({
|
||||
{llmProviderFieldVisible && topLevelSecretEnvVar ? (
|
||||
<PersonaProviderApiKeyField
|
||||
disabled={updateMutation.isPending}
|
||||
envVarName={topLevelSecretEnvVar}
|
||||
isInherited={apiKeyIsInherited}
|
||||
inheritedLabel={apiKeyInheritedLabel}
|
||||
isRequired={apiKeyIsRequired}
|
||||
label={
|
||||
effectiveProvider === "anthropic"
|
||||
? "Anthropic API Key"
|
||||
: "OpenAI API Key"
|
||||
}
|
||||
label={getProviderApiKeyLabel(effectiveProvider) ?? "API Key"}
|
||||
onValueChange={(next) => {
|
||||
setEnvVars((prev) => ({
|
||||
...prev,
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
/**
|
||||
* Always-visible cue shown in Agent Defaults when a global `OPENAI_API_KEY`
|
||||
* row exists (nonblank). The Advanced section is collapsed by default, so
|
||||
* the `keyAnnotations` hint on that row is invisible until the user opens it
|
||||
* — this cue bridges the gap by surfacing the information at the decision
|
||||
* point.
|
||||
*
|
||||
* Renders nothing when `OPENAI_API_KEY` is absent or blank.
|
||||
*/
|
||||
export function CardMintKeyCue({
|
||||
envVars,
|
||||
}: {
|
||||
envVars: Record<string, string>;
|
||||
}) {
|
||||
const isSet =
|
||||
"OPENAI_API_KEY" in envVars &&
|
||||
(envVars.OPENAI_API_KEY ?? "").trim().length > 0;
|
||||
if (!isSet) return null;
|
||||
|
||||
return (
|
||||
<p
|
||||
className="text-xs text-muted-foreground"
|
||||
data-testid="card-mint-key-cue"
|
||||
>
|
||||
Card-minting key <span className="font-mono">OPENAI_API_KEY</span> is set
|
||||
under Advanced → Environment variables.
|
||||
</p>
|
||||
);
|
||||
}
|
||||
@@ -3,6 +3,7 @@ import { Input } from "@/shared/ui/input";
|
||||
import { Textarea } from "@/shared/ui/textarea";
|
||||
import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor";
|
||||
import {
|
||||
CARD_MINT_KEY_ANNOTATIONS,
|
||||
PERSONA_FIELD_CONTROL_CLASS,
|
||||
PERSONA_FIELD_SHELL_CLASS,
|
||||
PERSONA_LABEL_OPTIONAL_CLASS,
|
||||
@@ -247,6 +248,7 @@ export function EditAgentAdvancedFields({
|
||||
helperText="Per-agent env vars. Override the template's vars on collision."
|
||||
inheritedFrom={inheritedEnvVars}
|
||||
inheritedLabel="template / global defaults"
|
||||
keyAnnotations={CARD_MINT_KEY_ANNOTATIONS}
|
||||
onChange={onEnvVarsChange}
|
||||
requiredKeys={requiredEnvKeys}
|
||||
value={envVars}
|
||||
|
||||
@@ -523,3 +523,97 @@ test("getBakedProviderInheritLabel_empty_options_falls_back_to_raw_id", () => {
|
||||
"empty options table must fall back to raw id",
|
||||
);
|
||||
});
|
||||
|
||||
// ── keyAnnotations — annotation lookup invariants ─────────────────────────────
|
||||
//
|
||||
// `keyAnnotations` is a pass-through prop: the renderer does `keyAnnotations?.[key]`.
|
||||
// The invariant worth pinning is that the prop contract is respected at the
|
||||
// data level — an annotation for one key does NOT bleed into another key.
|
||||
// (Rendering itself is trivially conditional; no logic to extract.)
|
||||
|
||||
test("keyAnnotations_present_key_has_annotation", () => {
|
||||
const annotations = {
|
||||
OPENAI_API_KEY: "Used for minting agent trading cards",
|
||||
};
|
||||
assert.equal(
|
||||
annotations.OPENAI_API_KEY,
|
||||
"Used for minting agent trading cards",
|
||||
);
|
||||
});
|
||||
|
||||
test("keyAnnotations_absent_key_is_undefined", () => {
|
||||
const annotations = {
|
||||
OPENAI_API_KEY: "Used for minting agent trading cards",
|
||||
};
|
||||
assert.equal(annotations.ANTHROPIC_API_KEY, undefined);
|
||||
});
|
||||
|
||||
test("keyAnnotations_empty_map_has_no_annotations", () => {
|
||||
const annotations = {};
|
||||
assert.equal(annotations.OPENAI_API_KEY, undefined);
|
||||
});
|
||||
|
||||
test("keyAnnotations_only_matching_key_gets_annotation", () => {
|
||||
// Verifies the per-key lookup is not accidentally global.
|
||||
const annotations = { OPENAI_API_KEY: "card minting" };
|
||||
const keys = ["OPENAI_API_KEY", "ANTHROPIC_API_KEY", "FOO"];
|
||||
const results = keys.map((k) => annotations[k] ?? null);
|
||||
assert.deepEqual(results, ["card minting", null, null]);
|
||||
});
|
||||
|
||||
// ── keyAnnotations render — annotation appears only on matching row ─────────
|
||||
//
|
||||
// renderToStaticMarkup exercises the real JSX path:
|
||||
// {keyAnnotations?.[row.key] ? <p ...>{annotation}</p> : null}
|
||||
// This confirms the prop is plumbed through to the DOM correctly and that
|
||||
// annotation text is scoped to its matching row.
|
||||
|
||||
import React from "react";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
import { EnvVarsEditor } from "./EnvVarsEditor.tsx";
|
||||
|
||||
test("keyAnnotations_annotation_present_only_on_matching_row", () => {
|
||||
const annotations = {
|
||||
OPENAI_API_KEY: "Used for minting agent trading cards",
|
||||
};
|
||||
const html = renderToStaticMarkup(
|
||||
React.createElement(EnvVarsEditor, {
|
||||
disabled: false,
|
||||
fileSatisfiedKeys: [],
|
||||
hiddenKeys: [],
|
||||
keyAnnotations: annotations,
|
||||
onChange: () => {},
|
||||
requiredKeys: [],
|
||||
value: { OPENAI_API_KEY: "sk-placeholder", ANTHROPIC_API_KEY: "sk-ant" },
|
||||
}),
|
||||
);
|
||||
assert.ok(
|
||||
html.includes("Used for minting agent trading cards"),
|
||||
"annotation must appear in rendered output for OPENAI_API_KEY row",
|
||||
);
|
||||
// The annotation must not bleed to other rows — check that it appears only once.
|
||||
const count = (html.match(/Used for minting agent trading cards/g) ?? [])
|
||||
.length;
|
||||
assert.equal(count, 1, "annotation must appear exactly once");
|
||||
});
|
||||
|
||||
test("keyAnnotations_annotation_absent_for_non_matching_rows", () => {
|
||||
const annotations = {
|
||||
OPENAI_API_KEY: "Used for minting agent trading cards",
|
||||
};
|
||||
const html = renderToStaticMarkup(
|
||||
React.createElement(EnvVarsEditor, {
|
||||
disabled: false,
|
||||
fileSatisfiedKeys: [],
|
||||
hiddenKeys: [],
|
||||
keyAnnotations: annotations,
|
||||
onChange: () => {},
|
||||
requiredKeys: [],
|
||||
value: { ANTHROPIC_API_KEY: "sk-ant", MY_VAR: "foo" },
|
||||
}),
|
||||
);
|
||||
assert.ok(
|
||||
!html.includes("Used for minting agent trading cards"),
|
||||
"annotation must not appear when its key is not in the env map",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -165,6 +165,14 @@ type EnvVarsEditorProps = {
|
||||
inheritedRows?: readonly InheritedEnvRow[];
|
||||
/** Label for the inherited-row tag (e.g. "build"). Defaults to "build". */
|
||||
inheritedRowsLabel?: string;
|
||||
/**
|
||||
* Optional muted one-line annotation for specific env var keys. Rendered
|
||||
* below any row whose key appears in this map — required rows, user rows,
|
||||
* and user rows whose key is typed mid-edit. Intended for contextual hints
|
||||
* like `{ OPENAI_API_KEY: "Used for minting agent trading cards" }` that
|
||||
* help users distinguish two keys with similar names.
|
||||
*/
|
||||
keyAnnotations?: Readonly<Record<string, string>>;
|
||||
};
|
||||
|
||||
type Row = { id: string; key: string; value: string };
|
||||
@@ -191,6 +199,7 @@ export function EnvVarsEditor({
|
||||
focusKey,
|
||||
inheritedRows = [],
|
||||
inheritedRowsLabel = "build",
|
||||
keyAnnotations,
|
||||
}: EnvVarsEditorProps) {
|
||||
// Keys that render as their own special rows (required amber rows or
|
||||
// file-satisfied read-only rows). These must NEVER enter `rows` state —
|
||||
@@ -406,6 +415,14 @@ export function EnvVarsEditor({
|
||||
</p>
|
||||
);
|
||||
})()}
|
||||
{keyAnnotations?.[key] ? (
|
||||
<p
|
||||
className="ml-1 text-xs text-muted-foreground"
|
||||
data-testid="env-vars-key-annotation"
|
||||
>
|
||||
{keyAnnotations[key]}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
@@ -596,6 +613,14 @@ export function EnvVarsEditor({
|
||||
</p>
|
||||
);
|
||||
})()}
|
||||
{row.key.length > 0 && keyAnnotations?.[row.key] ? (
|
||||
<p
|
||||
className="ml-1 text-xs text-muted-foreground"
|
||||
data-testid="env-vars-key-annotation"
|
||||
>
|
||||
{keyAnnotations[row.key]}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
@@ -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 (
|
||||
<div className="space-y-1.5">
|
||||
<RequiredFieldLabel htmlFor={inputId} isRequired={isRequired}>
|
||||
{label}
|
||||
</RequiredFieldLabel>
|
||||
{envVarName ? (
|
||||
<p className="text-xs text-muted-foreground font-mono" id={hintId}>
|
||||
{envVarName}
|
||||
</p>
|
||||
) : null}
|
||||
<div
|
||||
className={cn(
|
||||
"flex min-h-11 items-center gap-2 px-3",
|
||||
@@ -60,6 +77,7 @@ export function PersonaProviderApiKeyField({
|
||||
)}
|
||||
>
|
||||
<Input
|
||||
aria-describedby={hintId}
|
||||
autoComplete="off"
|
||||
className={cn(
|
||||
"h-8 flex-1 px-0 py-0 leading-6",
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
getDefaultPersonaRuntime,
|
||||
getPersonaModelOptions,
|
||||
getPersonaProviderOptions,
|
||||
getProviderApiKeyLabel,
|
||||
resetConfigForHarnessChange,
|
||||
runtimeSupportsLlmProviderSelection,
|
||||
} from "./agentConfigOptions.tsx";
|
||||
@@ -246,3 +247,50 @@ test("formatModelDiscoveryErrorStatus returns a non-null status for runtime unav
|
||||
assert.ok(typeof status?.tone === "string", "status has a tone");
|
||||
}
|
||||
});
|
||||
|
||||
// ── getProviderApiKeyLabel — provider-accurate credential field labels ────────
|
||||
//
|
||||
// Each provider with a secretEnvVar must have a distinct label. The helper
|
||||
// is the single source of truth used by all three credential field surfaces;
|
||||
// if it regresses the field labels diverge silently and the OpenRouter / compat
|
||||
// mislabeling recurs.
|
||||
|
||||
test("getProviderApiKeyLabel_anthropic_returns_anthropic_label", () => {
|
||||
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");
|
||||
});
|
||||
|
||||
@@ -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<Record<string, string>> = {
|
||||
OPENAI_API_KEY: "Used for minting agent trading cards",
|
||||
};
|
||||
|
||||
export function shouldClearKnownModelForSelectionScope({
|
||||
model,
|
||||
provider,
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
|
||||
|
||||
@@ -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",
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user