fix(desktop): preserve shared provider credential cards
This commit is contained in:
@@ -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<HTMLInputElement>) => setEdits(c => ({ ...c, [varKey]: e.target.value }))
|
||||
const startEdit = () => setEdits(c => ({ ...c, [editKey]: '' }))
|
||||
const cancel = () => setEdits(c => withoutKey(c, editKey))
|
||||
const update = (e: ChangeEvent<HTMLInputElement>) => setEdits(c => ({ ...c, [editKey]: e.target.value }))
|
||||
|
||||
const keydown = (e: KeyboardEvent<HTMLInputElement>) => {
|
||||
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({
|
||||
</Button>
|
||||
)}
|
||||
{dirty && (
|
||||
<Button className="h-8" disabled={busy} onClick={() => void onSave(varKey)} size="sm">
|
||||
<Button className="h-8" disabled={busy} onClick={() => void onSave(varKey, editKey)} size="sm">
|
||||
{busy ? <Loader2 className="animate-spin" /> : <Save />}
|
||||
{busy ? t.settings.credentials.saving : t.common.save}
|
||||
</Button>
|
||||
@@ -323,6 +325,7 @@ export function ProviderKeyRows({ expanded, group, onExpand, onToggle, rowProps
|
||||
}}
|
||||
>
|
||||
<KeyField
|
||||
editKey={`${group.name}:${group.primary[0]}`}
|
||||
expanded={expanded}
|
||||
info={group.primary[1]}
|
||||
placeholder={t.settings.credentials.pasteLabelKey(group.name)}
|
||||
@@ -348,6 +351,7 @@ export function ProviderKeyRows({ expanded, group, onExpand, onToggle, rowProps
|
||||
<ListRow
|
||||
action={
|
||||
<KeyField
|
||||
editKey={`${group.name}:${key}`}
|
||||
expanded={expanded}
|
||||
info={info}
|
||||
placeholder={credentialPlaceholder(key, info, fieldLabel)}
|
||||
|
||||
@@ -103,8 +103,8 @@ export function useEnvCredentials(profile?: string): UseEnvCredentials {
|
||||
setRevealed(c => 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) {
|
||||
|
||||
@@ -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(<ProvidersSettings onClose={vi.fn()} onViewChange={vi.fn()} view="keys" />)
|
||||
|
||||
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
|
||||
|
||||
@@ -76,21 +76,40 @@ function buildProviderKeyGroups(vars: Record<string, EnvVarInfo>): 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
|
||||
|
||||
@@ -46,8 +46,8 @@ export interface EnvRowProps {
|
||||
revealed: Record<string, string>
|
||||
saving: string | null
|
||||
setEdits: Dispatch<SetStateAction<Record<string, string>>>
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user