From fce9943c740a39ea5e6281142c3c0c86505a871f Mon Sep 17 00:00:00 2001 From: Juniper Bevensee Date: Wed, 29 Jul 2026 10:35:15 +1200 Subject: [PATCH] fix(keys): unify the provider list so the harness add-key menu offers custom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "add key" dropdown existed twice: KEY_PROVIDERS in app/(dashboard)/keys/page.tsx and a second, unrelated const of the same name in harnesses/[id]/page.tsx. Nothing linked them, so they drifted in both directions — the harness-scoped menu lacked `google` and `custom`, while the global menu lacked the twelve service providers (zai, signal, mattermost, aws-bedrock, google-cloud, helius, coingecko, dehashed, opencorporates, capsolver, open-measures, pexels). The reported symptom was the missing `custom`: a harness could not be given a key for any provider HSM doesn't know by name. Nothing server-side was stopping it — POST /api/keys does no provider validation and KeyInput.provider is a plain string — the option simply was not in that array. Extract both into lib/key-providers.ts as the single list, plus providerOptions() carrying over the unknown-provider escape hatch the keys page had (a ?request= prefill would otherwise vanish from the harness dropdown). `custom` also needs the env-var field, which the harness form never had: with no PROVIDER_TO_VAR entry, resolveEnvVar falls back to the key's name and then to a useless CUSTOM_API_KEY, so a custom key added from a harness would have landed in the wrong variable. The field now renders there too, clears when the provider changes away from custom, and is only sent for custom. MODEL_PROVIDERS is deliberately untouched — that drives the model cascade editor, not credential storage. Tests pin the parity itself, since a redeclared local list is how this broke: they fail if either page declares its own KEY_PROVIDERS, stops importing the shared module, or drops the custom env-var field. Verified red against a reintroduced local const. Full suite green (874). --- app/(dashboard)/harnesses/[id]/page.tsx | 34 +++++++++---- app/(dashboard)/keys/page.tsx | 14 +---- lib/__tests__/key-providers.test.ts | 68 +++++++++++++++++++++++++ lib/key-providers.ts | 42 +++++++++++++++ 4 files changed, 136 insertions(+), 22 deletions(-) create mode 100644 lib/__tests__/key-providers.test.ts create mode 100644 lib/key-providers.ts diff --git a/app/(dashboard)/harnesses/[id]/page.tsx b/app/(dashboard)/harnesses/[id]/page.tsx index 2338b96..daa5333 100644 --- a/app/(dashboard)/harnesses/[id]/page.tsx +++ b/app/(dashboard)/harnesses/[id]/page.tsx @@ -13,6 +13,7 @@ import { RiskBar } from '@/components/shared/risk-bar' import { TierMix } from '@/components/shared/tier-mix' import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs' import type { Harness, HabitatTier, Tool, Key, MemoryScope, Surface } from '@/lib/types' +import { providerOptions } from '@/lib/key-providers' import { SignalSetupDialog } from '@/components/surfaces/signal-setup-dialog' import { TelegramSetupDialog } from '@/components/surfaces/telegram-setup-dialog' import { MattermostSetupDialog } from '@/components/surfaces/mattermost-setup-dialog' @@ -73,13 +74,6 @@ const SURFACE_STATUS_STYLES: Record = { const MODEL_PROVIDERS = ['anthropic', 'openrouter', 'ollama', 'custom', 'gemini', 'nous', 'bedrock', 'zai'] as const -const KEY_PROVIDERS = [ - 'anthropic', 'openai', 'openrouter', 'zai', 'notion', 'github', 'telegram', 'signal', - 'mattermost', 'aws', 'aws-bedrock', 'google-cloud', 'brave', - 'helius', 'coingecko', 'dehashed', 'opencorporates', 'capsolver', - 'open-measures', 'pexels', -] - type FallbackProviderEntry = { provider: string; model: string; base_url?: string } type ModelConfig = { provider: string; primary: string; models: string[]; fallbackProviders?: FallbackProviderEntry[] } @@ -193,6 +187,7 @@ function HermesHarnessDetail({ params }: { params: Promise<{ id: string }> }) { const [newKeyName, setNewKeyName] = useState('') const [newKeyValue, setNewKeyValue] = useState('') const [newKeyBudget, setNewKeyBudget] = useState('') + const [newKeyEnvVar, setNewKeyEnvVar] = useState('') const [keySaving, setKeySaving] = useState(false) const [showAssignKey, setShowAssignKey] = useState(false) @@ -238,6 +233,9 @@ function HermesHarnessDetail({ params }: { params: Promise<{ id: string }> }) { value: newKeyValue, assignedTo: [harness.id], ...(newKeyName ? { name: newKeyName } : {}), + // Only meaningful for custom/unknown providers — resolveEnvVar ignores + // the hint for mapped ones, but don't send a stale value regardless. + ...(newKeyEnvVar && newKeyProvider === 'custom' ? { envVar: newKeyEnvVar } : {}), ...(newKeyBudget ? { budgetUsd: parseFloat(newKeyBudget) } : {}), }), }) @@ -252,6 +250,7 @@ function HermesHarnessDetail({ params }: { params: Promise<{ id: string }> }) { setNewKeyName('') setNewKeyValue('') setNewKeyBudget('') + setNewKeyEnvVar('') // Recreate (not 'quick') so the new env_file value actually loads — a plain // restart keeps the old creation-time env. (POST /api/keys also recreates // server-side; this is the belt-and-suspenders client trigger.) @@ -1285,11 +1284,14 @@ function HermesHarnessDetail({ params }: { params: Promise<{ id: string }> }) { @@ -1315,6 +1317,18 @@ function HermesHarnessDetail({ params }: { params: Promise<{ id: string }> }) { className="w-full mt-1 text-sm border border-[var(--border)] rounded-md px-2 py-1.5 bg-[var(--surface)]" /> + {newKeyProvider === 'custom' && ( +
+ + setNewKeyEnvVar(e.target.value)} + placeholder="e.g. SANTIMENT_API_KEY — else derived from name" + className="w-full mt-1 text-sm font-mono border border-[var(--border)] rounded-md px-2 py-1.5 bg-[var(--surface)]" + /> +
+ )}
}) { diff --git a/app/(dashboard)/keys/page.tsx b/app/(dashboard)/keys/page.tsx index 7968043..25550f9 100644 --- a/app/(dashboard)/keys/page.tsx +++ b/app/(dashboard)/keys/page.tsx @@ -9,17 +9,10 @@ import { Button } from '@/components/ui/button' import { toast } from 'sonner' import { filterKeys } from '@/lib/keys-filter' import { parseKeyRequestParams } from '@/lib/keys-request' +import { providerOptions } from '@/lib/key-providers' import type { Key, Harness } from '@/lib/types' import type { HabitatTier, HarnessStatus } from '@/lib/types' -const KEY_PROVIDERS = [ - // Model/inference providers first. `openrouter` maps to OPENROUTER_API_KEY on - // the write path (see KeysService.resolveEnvVar) — the fleet's GLM-5.2 chat - // primary AND its cheap-metered rung ([intelligent-routing-cost]) both ride - // OpenRouter, so neither can be provisioned without it here. - 'anthropic', 'openai', 'google', 'openrouter', 'aws', 'github', 'brave', 'notion', 'telegram', 'custom', -] - function keyHealthToStatus(health: Key['health']): HarnessStatus { if (health === 'good') return 'running' if (health === 'warning') return 'idle' @@ -256,10 +249,7 @@ function KeysPageContent() { className="w-full rounded-md border border-[var(--border)] bg-background px-3 py-1.5 text-sm" > - {(newProvider && !KEY_PROVIDERS.includes(newProvider) - ? [...KEY_PROVIDERS, newProvider] - : KEY_PROVIDERS - ).map((p) => ( + {providerOptions(newProvider).map((p) => ( ))} diff --git a/lib/__tests__/key-providers.test.ts b/lib/__tests__/key-providers.test.ts new file mode 100644 index 0000000..7a7ba2c --- /dev/null +++ b/lib/__tests__/key-providers.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect } from 'vitest' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { KEY_PROVIDERS, providerOptions } from '@/lib/key-providers' + +const repoRoot = join(__dirname, '..', '..') +const read = (p: string) => readFileSync(join(repoRoot, p), 'utf8') + +const KEYS_PAGE = 'app/(dashboard)/keys/page.tsx' +const HARNESS_PAGE = 'app/(dashboard)/harnesses/[id]/page.tsx' + +describe('KEY_PROVIDERS', () => { + it('has no duplicates', () => { + expect(new Set(KEY_PROVIDERS).size).toBe(KEY_PROVIDERS.length) + }) + + it('offers custom — the escape hatch both add-key menus must expose', () => { + expect(KEY_PROVIDERS).toContain('custom') + }) + + it('retains every provider each menu had before they were unified', () => { + // Regression guard for the drift this module fixed: the harness-scoped menu + // and the global keys menu each carried providers the other lacked. + const onceGlobalOnly = ['google', 'custom'] + const onceHarnessOnly = [ + 'zai', 'signal', 'mattermost', 'aws-bedrock', 'google-cloud', 'helius', + 'coingecko', 'dehashed', 'opencorporates', 'capsolver', 'open-measures', 'pexels', + ] + for (const p of [...onceGlobalOnly, ...onceHarnessOnly]) { + expect(KEY_PROVIDERS, `${p} went missing`).toContain(p) + } + }) +}) + +describe('providerOptions', () => { + it('returns the canonical list for a known or empty provider', () => { + expect(providerOptions('')).toEqual(KEY_PROVIDERS) + expect(providerOptions('anthropic')).toEqual(KEY_PROVIDERS) + }) + + it('appends an unknown provider so a prefill is never silently dropped', () => { + const opts = providerOptions('hedra') + expect(opts).toContain('hedra') + expect(opts).toHaveLength(KEY_PROVIDERS.length + 1) + }) +}) + +describe('both add-key menus stay in parity', () => { + // The whole point of lib/key-providers: a locally-redeclared list is how the + // two menus diverged in the first place. Fail loudly if one reappears. + it.each([KEYS_PAGE, HARNESS_PAGE])('%s declares no local provider list', (page) => { + expect(read(page)).not.toMatch(/const\s+KEY_PROVIDERS\s*=/) + }) + + it.each([KEYS_PAGE, HARNESS_PAGE])('%s sources its options from the shared module', (page) => { + const src = read(page) + expect(src).toContain("from '@/lib/key-providers'") + expect(src).toMatch(/providerOptions\(/) + }) + + it.each([KEYS_PAGE, HARNESS_PAGE])('%s renders the env-var field custom keys need', (page) => { + // Without it a custom key falls back to the name-derived var, or the + // useless CUSTOM_API_KEY — see KeysService.resolveEnvVar. + const src = read(page) + expect(src).toMatch(/===\s*'custom'\s*&&/) + expect(src).toMatch(/envVar/) + }) +}) diff --git a/lib/key-providers.ts b/lib/key-providers.ts new file mode 100644 index 0000000..ee767fa --- /dev/null +++ b/lib/key-providers.ts @@ -0,0 +1,42 @@ +// Single source of truth for the "add an API key" provider dropdown. +// +// This list used to be duplicated: one copy in app/(dashboard)/keys/page.tsx and +// a second, unrelated copy in app/(dashboard)/harnesses/[id]/page.tsx. They drifted +// in BOTH directions — the harness-scoped menu was missing `google` and `custom`, +// the global menu was missing the twelve service providers below `brave`. Anything +// added to only one menu is invisible from the other, so keep this the only copy. +// +// Not to be confused with MODEL_PROVIDERS in harnesses/[id]/page.tsx — that drives +// the model cascade editor (inference backends), not credential storage. +// +// Ordering is presentational: model/inference first, then platform/surface, then +// service APIs, with `custom` last as the escape hatch. +// +// `custom` is special on the write path: KeysService.resolveEnvVar has no +// PROVIDER_TO_VAR entry for it, so it resolves via the explicit `envVar` hint, +// else the key's name, else the useless CUSTOM_API_KEY fallback. Any form that +// offers `custom` must therefore also offer the env-var field. +export const KEY_PROVIDERS = [ + // Model / inference. `openrouter` maps to OPENROUTER_API_KEY on the write path + // (see KeysService.resolveEnvVar) — the fleet's GLM-5.2 chat primary AND its + // cheap-metered rung ([intelligent-routing-cost]) both ride OpenRouter, so + // neither can be provisioned without it here. + 'anthropic', 'openai', 'google', 'google-cloud', 'openrouter', 'zai', + 'aws', 'aws-bedrock', + // Platforms / surfaces + 'github', 'notion', 'telegram', 'signal', 'mattermost', + // Service APIs + 'brave', 'helius', 'coingecko', 'dehashed', 'opencorporates', + 'capsolver', 'open-measures', 'pexels', + // Escape hatch — requires the env-var field, see above. + 'custom', +] + +// A provider may reach a form from outside the list (a ?request= prefill via +// lib/keys-request, or a key discovered from an existing .env). Appending it +// keeps the select from silently dropping the caller's choice. +export function providerOptions(current: string): string[] { + return current && !KEY_PROVIDERS.includes(current) + ? [...KEY_PROVIDERS, current] + : KEY_PROVIDERS +}