diff --git a/apps/desktop/src/app/settings/credential-key-ui.tsx b/apps/desktop/src/app/settings/credential-key-ui.tsx index ff017bde48..cd148fe07b 100644 --- a/apps/desktop/src/app/settings/credential-key-ui.tsx +++ b/apps/desktop/src/app/settings/credential-key-ui.tsx @@ -49,8 +49,10 @@ export function KeyField({ info, placeholder, rowProps, - varKey + varKey, + editKey = varKey }: { + editKey?: string expanded?: boolean info: EnvVarInfo placeholder?: string @@ -59,21 +61,21 @@ export function KeyField({ }) { const { t } = useI18n() const { edits, onClear, onSave, saving, setEdits } = rowProps - const editing = edits[varKey] !== undefined + const editing = edits[editKey] !== undefined // Bare (plain subtext) only while the group is collapsed and idle. Expanding // the card counts as "focused in", so it gets full input chrome too. const bare = !editing && !expanded - const draft = edits[varKey] ?? '' + const draft = edits[editKey] ?? '' const dirty = draft.trim().length > 0 const busy = saving === varKey const masked = info.redacted_value ?? '••••••••' - const startEdit = () => setEdits(c => ({ ...c, [varKey]: '' })) - const cancel = () => setEdits(c => withoutKey(c, varKey)) - const update = (e: ChangeEvent) => setEdits(c => ({ ...c, [varKey]: e.target.value })) + const startEdit = () => setEdits(c => ({ ...c, [editKey]: '' })) + const cancel = () => setEdits(c => withoutKey(c, editKey)) + const update = (e: ChangeEvent) => setEdits(c => ({ ...c, [editKey]: e.target.value })) const keydown = (e: KeyboardEvent) => { if (isSubmitEnter(e) && dirty) { - void onSave(varKey) + void onSave(varKey, editKey) } else if (e.key === 'Escape' && editing) { e.preventDefault() e.stopPropagation() @@ -120,7 +122,7 @@ export function KeyField({ aria-label={t.settings.credentials.remove} className="text-muted-foreground hover:text-destructive" disabled={busy} - onClick={() => void onClear(varKey)} + onClick={() => void onClear(varKey, editKey)} size="icon-xs" type="button" variant="ghost" @@ -129,7 +131,7 @@ export function KeyField({ )} {dirty && ( - @@ -323,6 +325,7 @@ export function ProviderKeyRows({ expanded, group, onExpand, onToggle, rowProps }} > withoutKey(c, key)) } - async function handleSave(key: string) { - const value = edits[key] + async function handleSave(key: string, editKey = key) { + const value = edits[editKey] if (!value) { return @@ -115,7 +115,7 @@ export function useEnvCredentials(profile?: string): UseEnvCredentials { try { await setEnvVar(key, value, profile) patchVar(key, { is_set: true, redacted_value: redactedValue(value) }) - clearLocalState(key) + clearLocalState(editKey) void queryClient.invalidateQueries({ queryKey: ['model-options'] }) notify({ kind: 'success', title: toolsets.savedTitle, message: toolsets.savedMessage(key) }) } catch (err) { @@ -154,7 +154,7 @@ export function useEnvCredentials(profile?: string): UseEnvCredentials { } } - async function handleClear(key: string) { + async function handleClear(key: string, editKey = key) { if (!(await confirm({ destructive: true, title: toolsets.removeConfirm(key) }))) { return } @@ -164,7 +164,7 @@ export function useEnvCredentials(profile?: string): UseEnvCredentials { try { await deleteEnvVar(key, profile) patchVar(key, { is_set: false, redacted_value: null }) - clearLocalState(key) + clearLocalState(editKey) void queryClient.invalidateQueries({ queryKey: ['model-options'] }) notify({ kind: 'success', title: toolsets.removedTitle, message: toolsets.removedMessage(key) }) } catch (err) { diff --git a/apps/desktop/src/app/settings/providers-settings.test.tsx b/apps/desktop/src/app/settings/providers-settings.test.tsx index 8983967182..9773ceab4d 100644 --- a/apps/desktop/src/app/settings/providers-settings.test.tsx +++ b/apps/desktop/src/app/settings/providers-settings.test.tsx @@ -239,6 +239,46 @@ describe('ProvidersSettings', () => { expect(await screen.findByText('WidgetAI')).toBeTruthy() }) + it('renders separate provider cards that share one credential env var', async () => { + getEnvVars.mockResolvedValue({ + DASHSCOPE_API_KEY: keyVar({ + provider: 'alibaba', + provider_label: 'Qwen Cloud', + provider_profiles: [ + { + description: 'International DashScope route', + primary: true, + provider: 'alibaba', + provider_label: 'Qwen Cloud', + url: 'https://modelstudio.console.alibabacloud.com/' + }, + { + description: 'Mainland-China DashScope route', + primary: true, + provider: 'alibaba-cn', + provider_label: 'Alibaba Cloud DashScope (China)', + url: 'https://bailian.console.aliyun.com/' + } + ] + }) + }) + listOAuthProviders.mockResolvedValue({ providers: [] }) + + const { ProvidersSettings } = await import('./providers-settings') + render() + + expect(await screen.findByText('Qwen Cloud')).toBeTruthy() + expect(screen.getByText('Alibaba Cloud DashScope (China)')).toBeTruthy() + const inputs = screen.getAllByPlaceholderText(/Paste .* key/) + expect(inputs).toHaveLength(2) + + fireEvent.focus(inputs[0]) + fireEvent.change(inputs[0], { target: { value: 'shared-secret' } }) + + expect(screen.getAllByDisplayValue('shared-secret')).toHaveLength(1) + expect((inputs[1] as HTMLInputElement).value).toBe('') + }) + it('orders API-key providers by priority then name, and filters them via search', async () => { // These three providers have no curated PROVIDER_GROUPS priority, so they // share the default priority and fall back to alphabetical among themselves diff --git a/apps/desktop/src/app/settings/providers-settings.tsx b/apps/desktop/src/app/settings/providers-settings.tsx index 3334bc2e2c..8ee97eacf1 100644 --- a/apps/desktop/src/app/settings/providers-settings.tsx +++ b/apps/desktop/src/app/settings/providers-settings.tsx @@ -76,21 +76,40 @@ function buildProviderKeyGroups(vars: Record): ProviderKeyGr continue } - // Prefer the backend-supplied provider label/id so the Keys tab groups by - // the same identity the CLI picker uses; fall back to the prefix guess. - const name = info.provider_label?.trim() || info.provider?.trim() || providerGroup(key) + // A shared credential (for example DASHSCOPE_API_KEY) can belong to more + // than one built-in route. Expand its provider profiles into card-scoped + // rows while keeping the same env-var key for save/remove operations. + const scopedInfos = info.provider_profiles?.length + ? info.provider_profiles.map(profile => ({ + ...info, + description: profile.description || info.description, + provider: profile.provider, + provider_label: profile.provider_label, + provider_primary: profile.primary, + url: profile.url ?? info.url + })) + : [info] - if (name === 'Other') { - continue + for (const scopedInfo of scopedInfos) { + // Prefer the backend-supplied provider label/id so the Keys tab groups by + // the same identity the CLI picker uses; fall back to the prefix guess. + const name = scopedInfo.provider_label?.trim() || scopedInfo.provider?.trim() || providerGroup(key) + + if (name === 'Other') { + continue + } + + buckets.set(name, [...(buckets.get(name) ?? []), [key, scopedInfo]]) } - - buckets.set(name, [...(buckets.get(name) ?? []), [key, info]]) } const groups: ProviderKeyGroup[] = [] for (const [name, entries] of buckets) { - const primary = entries.find(([k, i]) => !i.advanced && isKeyVar(k, i)) ?? entries.find(([k, i]) => isKeyVar(k, i)) + const primary = + entries.find(([k, i]) => i.provider_primary && isKeyVar(k, i)) ?? + entries.find(([k, i]) => !i.advanced && isKeyVar(k, i)) ?? + entries.find(([k, i]) => isKeyVar(k, i)) if (!primary) { continue diff --git a/apps/desktop/src/app/settings/types.ts b/apps/desktop/src/app/settings/types.ts index b8481bb47b..6452e91d5c 100644 --- a/apps/desktop/src/app/settings/types.ts +++ b/apps/desktop/src/app/settings/types.ts @@ -46,8 +46,8 @@ export interface EnvRowProps { revealed: Record saving: string | null setEdits: Dispatch>> - onSave: (key: string) => void - onClear: (key: string) => void + onSave: (key: string, editKey?: string) => void + onClear: (key: string, editKey?: string) => void onReveal: (key: string) => void compact?: boolean } diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 43b4fb19e1..0eeaed6e5f 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -191,11 +191,26 @@ export interface EnvVarInfo { // desktop-only env-var prefix guesses. Empty for non-provider env vars. provider?: string provider_label?: string + // A credential env var can be shared by multiple built-in routes. The + // singular fields above remain for compatibility; this list lets the Keys + // tab render every provider card without duplicating credential storage. + provider_profiles?: EnvProviderProfile[] + // Frontend-only hint copied from a provider profile while grouping a shared + // credential. It keeps the provider's first credential ahead of aliases. + provider_primary?: boolean redacted_value: null | string tools: string[] url: null | string } +export interface EnvProviderProfile { + description: string + primary: boolean + provider: string + provider_label: string + url: null | string +} + export type MemoryProviderFieldKind = 'bool' | 'json' | 'number' | 'secret' | 'select' | 'text' export interface MemoryProviderFieldOption { diff --git a/hermes_cli/web_routers/config_env.py b/hermes_cli/web_routers/config_env.py index b5001978a9..4afb594b28 100644 --- a/hermes_cli/web_routers/config_env.py +++ b/hermes_cli/web_routers/config_env.py @@ -178,8 +178,9 @@ def _catalog_provider_env_metadata() -> dict: Returns ``{env_var: {provider, provider_label, description, url, is_password, advanced}}`` for every API-key provider in the unified ``provider_catalog()`` - (the ``hermes model`` universe), so the desktop Keys tab renders a card even - for providers never hand-added to ``OPTIONAL_ENV_VARS``. Hand + (the ``hermes model`` universe). When multiple providers intentionally share + one env var, ``provider_profiles`` preserves every provider identity while + the legacy singular fields keep describing the first provider. Hand ``OPTIONAL_ENV_VARS`` prose is layered on top in the endpoint; this only supplies membership + grouping + fallbacks. """ @@ -197,17 +198,44 @@ def _catalog_provider_env_metadata() -> dict: } meta: dict = {} + + def _profile(entry: dict) -> dict: + """Return the provider-specific part of a shared credential row.""" + return { + "provider": entry["provider"], + "provider_label": entry["provider_label"], + "description": entry["description"], + "url": entry["url"], + "primary": bool(entry.get("provider_primary")), + } + + def _add_provider_env(env_var: str, entry: dict) -> None: + """Add one provider without discarding peers that share ``env_var``.""" + existing = meta.get(env_var) + if existing is None: + meta[env_var] = entry + return + if existing.get("provider") == entry.get("provider"): + return + profiles = existing.setdefault("provider_profiles", [_profile(existing)]) + if not any(profile.get("provider") == entry.get("provider") for profile in profiles): + profiles.append(_profile(entry)) + for d in provider_catalog(): if d.tab != "keys": continue # API-key vars: the first is the primary (password) field; aliases are # kept as additional password fields so users can clear them too. - for env_var in d.api_key_env_vars: + for index, env_var in enumerate(d.api_key_env_vars): if env_var in _non_provider_keys: continue # don't hijack a shared tool/messaging credential - meta.setdefault( + entry = _provider_card( + d, d.description, d.signup_url or None, is_password=True, advanced=False, + ) + entry["provider_primary"] = index == 0 + _add_provider_env( env_var, - _provider_card(d, d.description, d.signup_url or None, is_password=True, advanced=False), + entry, ) # Base-URL override is an advanced, non-secret field for the same card. if d.base_url_env_var: @@ -263,6 +291,10 @@ def _get_env_vars_sync(profile: Optional[str] = None): # by the SAME provider identity the CLI `hermes model` picker uses. "provider": cat_meta.get("provider", ""), "provider_label": cat_meta.get("provider_label", ""), + # One credential can intentionally serve multiple built-in routes. + # Preserve those identities so Desktop can render distinct cards + # that edit the same underlying env var. + "provider_profiles": cat_meta.get("provider_profiles", []), # True for a .env key in no catalog at all — an arbitrary/custom var # the user added directly, listed so the Keys page can manage it. "custom": custom, diff --git a/tests/hermes_cli/test_provider_parity.py b/tests/hermes_cli/test_provider_parity.py index 017c46b88b..cace2c3940 100644 --- a/tests/hermes_cli/test_provider_parity.py +++ b/tests/hermes_cli/test_provider_parity.py @@ -43,11 +43,18 @@ _DUAL_TAB = {"anthropic"} def _keys_tab_providers() -> set[str]: """Provider slugs that have at least one card on the desktop API-keys tab.""" data = client.get("/api/env", headers=HEADERS).json() - return { - info.get("provider") - for info in data.values() - if info.get("category") == "provider" and info.get("provider") - } + providers = set() + for info in data.values(): + if info.get("category") != "provider": + continue + if info.get("provider"): + providers.add(info["provider"]) + providers.update( + profile["provider"] + for profile in info.get("provider_profiles", []) + if profile.get("provider") + ) + return providers def _accounts_tab_providers() -> set[str]: @@ -86,3 +93,12 @@ def test_each_provider_lands_on_the_tab_its_auth_type_dictates(): assert d.slug in accounts, f"{d.slug} (accounts tab) missing from /api/providers/oauth" +def test_shared_api_key_preserves_each_provider_profile(): + """One env var must not collapse distinct built-in provider routes.""" + data = client.get("/api/env", headers=HEADERS).json() + profiles = data["DASHSCOPE_API_KEY"]["provider_profiles"] + by_provider = {profile["provider"]: profile for profile in profiles} + + assert {"alibaba", "alibaba-cn"} <= by_provider.keys() + assert by_provider["alibaba"]["primary"] is True + assert by_provider["alibaba-cn"]["primary"] is True