diff --git a/ui/desktop/src/components/onboarding/ProviderConfigForm.tsx b/ui/desktop/src/components/onboarding/ProviderConfigForm.tsx index 07af91a9a..4874f59c4 100644 --- a/ui/desktop/src/components/onboarding/ProviderConfigForm.tsx +++ b/ui/desktop/src/components/onboarding/ProviderConfigForm.tsx @@ -200,8 +200,14 @@ function ApiKeyForm({ const toSubmit = Object.fromEntries( Object.entries(configValues) - .filter(([, entry]) => !!entry.value) - .map(([k, entry]) => [k, entry.value || '']) + .filter( + ([, entry]) => + !!entry.value || (entry.serverValue != null && typeof entry.serverValue === 'string') + ) + .map(([k, entry]) => [ + k, + entry.value ?? (typeof entry.serverValue === 'string' ? entry.serverValue : ''), + ]) ); setIsSubmitting(true); diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.test.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.test.tsx index 8df9ef6e6..c4a406abd 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.test.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.test.tsx @@ -165,6 +165,40 @@ describe('CustomProviderForm transitions', () => { expect(screen.queryByText(/Failed to save provider/)).not.toBeInTheDocument(); }); + it.each([ + ['anthropic_compatible', 'anthropic_compatible'], + ['ollama_compatible', 'ollama_compatible'], + ['openai_compatible', 'openai_compatible'], + ['anthropic', 'anthropic_compatible'], + ['ollama', 'ollama_compatible'], + ['openai', 'openai_compatible'], + ])('saves a provider stored as %s with engine %s', async (engine, expectedEngine) => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + render( + , + { wrapper: IntlTestWrapper } + ); + + await user.click(screen.getByRole('button', { name: 'Update Provider' })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledOnce()); + expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ engine: expectedEngine })); + }); + it('clears form validation when returning to the setup choice', async () => { const user = userEvent.setup(); renderForm(); diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx index 1c0b656ab..ea2ba77b3 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx @@ -227,6 +227,20 @@ const i18n = defineMessages({ type Step = 'choice' | 'catalog' | 'form'; +type ProviderEngine = 'openai_compatible' | 'anthropic_compatible' | 'ollama_compatible'; + +const ENGINE_ALIASES: Record = { + openai: 'openai_compatible', + openai_compatible: 'openai_compatible', + anthropic: 'anthropic_compatible', + anthropic_compatible: 'anthropic_compatible', + ollama: 'ollama_compatible', + ollama_compatible: 'ollama_compatible', +}; + +const normalizeEngine = (engine: string): ProviderEngine => + ENGINE_ALIASES[engine.trim().toLowerCase()] ?? 'openai_compatible'; + interface CustomProviderFormProps { onSubmit: (data: UpdateCustomProviderRequest) => void | Promise; onCancel: () => void; @@ -245,7 +259,12 @@ export default function CustomProviderForm({ isEditable, }: CustomProviderFormProps) { const intl = useIntl(); - const [engine, setEngine] = useState('openai_compatible'); + const engineOptions: { value: ProviderEngine; label: string }[] = [ + { value: 'openai_compatible', label: intl.formatMessage(i18n.openaiCompatible) }, + { value: 'anthropic_compatible', label: intl.formatMessage(i18n.anthropicCompatible) }, + { value: 'ollama_compatible', label: intl.formatMessage(i18n.ollamaCompatible) }, + ]; + const [engine, setEngine] = useState('openai_compatible'); const [displayName, setDisplayName] = useState(''); const [apiUrl, setApiUrl] = useState(''); const [basePath, setBasePath] = useState(''); @@ -284,12 +303,7 @@ export default function CustomProviderForm({ useEffect(() => { if (initialData) { - const engineMap: Record = { - openai: 'openai_compatible', - anthropic: 'anthropic_compatible', - ollama: 'ollama_compatible', - }; - setEngine(engineMap[initialData.engine] || 'openai_compatible'); + setEngine(normalizeEngine(initialData.engine)); setDisplayName(initialData.display_name); setApiUrl(initialData.api_url); setBasePath(initialData.base_path ?? ''); @@ -320,12 +334,7 @@ export default function CustomProviderForm({ setSupportsStreaming(template.supportsStreaming); setRequiresAuth(true); - const formatToEngine: Record = { - openai: 'openai_compatible', - anthropic: 'anthropic_compatible', - ollama: 'ollama_compatible', - }; - setEngine(formatToEngine[template.format] || 'openai_compatible'); + setEngine(normalizeEngine(template.format)); const templateModels = template.models.filter((m) => !m.deprecated).map((m) => m.id); setModels(templateModels.join(', ')); @@ -633,25 +642,10 @@ export default function CustomProviderForm({ id="provider-select" aria-invalid={!!validationErrors.providerType} aria-describedby={validationErrors.providerType ? 'provider-select-error' : undefined} - options={[ - { value: 'openai_compatible', label: intl.formatMessage(i18n.openaiCompatible) }, - { - value: 'anthropic_compatible', - label: intl.formatMessage(i18n.anthropicCompatible), - }, - { value: 'ollama_compatible', label: intl.formatMessage(i18n.ollamaCompatible) }, - ]} - value={{ - value: engine, - label: - engine === 'openai_compatible' - ? intl.formatMessage(i18n.openaiCompatible) - : engine === 'anthropic_compatible' - ? intl.formatMessage(i18n.anthropicCompatible) - : intl.formatMessage(i18n.ollamaCompatible), - }} + options={engineOptions} + value={engineOptions.find((option) => option.value === engine)} onChange={(option: unknown) => { - const selectedOption = option as { value: string; label: string } | null; + const selectedOption = option as { value: ProviderEngine } | null; if (selectedOption) setEngine(selectedOption.value); }} isSearchable={false} diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx index dd6a155c7..0974d2758 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx @@ -102,10 +102,13 @@ export default function DefaultProviderSetupForm({ const values: { [k: string]: ConfigInput } = {}; let fields: Awaited> = []; + let readFailed = false; try { fields = await acpReadProviderConfig(provider.name); } catch { - // Provider may not be in the registry yet; fall back to defaults below. + // A failed read cannot be distinguished from "nothing is stored", so + // seeding defaults here would submit them over values already on disk. + readFailed = true; } const fieldByKey = new Map(fields.map((field) => [field.key, field])); @@ -119,7 +122,7 @@ export default function DefaultProviderSetupForm({ ? { maskedValue: field.value } : field.value; values[parameter.name] = { serverValue }; - } else if (parameter.default !== undefined && parameter.default !== null) { + } else if (!readFailed && parameter.default !== undefined && parameter.default !== null) { values[parameter.name] = { value: parameter.default }; } } diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.test.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.test.tsx new file mode 100644 index 000000000..8c724d0cf --- /dev/null +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.test.tsx @@ -0,0 +1,66 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { acpSaveProviderConfig } from '../../../../../../acp/providers'; +import { providerConfigSubmitHandler } from './DefaultSubmitHandler'; + +vi.mock('../../../../../../acp/providers', () => ({ + acpSaveProviderConfig: vi.fn(), +})); + +const litellm = { + name: 'litellm', + metadata: { + config_keys: [ + { name: 'LITELLM_API_KEY' }, + { name: 'LITELLM_HOST', default: 'http://localhost:4000' }, + { name: 'LITELLM_BASE_PATH', default: 'v1/chat/completions' }, + { name: 'LITELLM_TIMEOUT', default: '600' }, + ], + }, +}; + +function savedKeys() { + const [, fields] = vi.mocked(acpSaveProviderConfig).mock.calls[0]; + return Object.fromEntries(fields.map(({ key, value }) => [key, value])); +} + +describe('providerConfigSubmitHandler', () => { + beforeEach(() => { + vi.mocked(acpSaveProviderConfig).mockClear(); + }); + + it('never writes a metadata default for a field the user did not supply', async () => { + await providerConfigSubmitHandler(litellm, { LITELLM_API_KEY: 'rotated-key' }); + + expect(savedKeys()).toEqual({ LITELLM_API_KEY: 'rotated-key' }); + }); + + it('submits only the values it was given', async () => { + await providerConfigSubmitHandler(litellm, { + LITELLM_API_KEY: 'rotated-key', + LITELLM_HOST: 'http://192.168.1.50:4000', + }); + + expect(savedKeys()).toEqual({ + LITELLM_API_KEY: 'rotated-key', + LITELLM_HOST: 'http://192.168.1.50:4000', + }); + }); + + it('skips empty values rather than falling back to the default', async () => { + await providerConfigSubmitHandler(litellm, { + LITELLM_API_KEY: 'rotated-key', + LITELLM_HOST: '', + }); + + expect(savedKeys()).toEqual({ LITELLM_API_KEY: 'rotated-key' }); + }); + + it('ignores values for keys the provider does not declare', async () => { + await providerConfigSubmitHandler(litellm, { + LITELLM_API_KEY: 'rotated-key', + SOME_OTHER_KEY: 'ignored', + }); + + expect(savedKeys()).toEqual({ LITELLM_API_KEY: 'rotated-key' }); + }); +}); diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx index 6cb5ec273..e347443e7 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx @@ -11,18 +11,18 @@ export const providerConfigSubmitHandler = async ( provider: { name: string; metadata: { - config_keys?: Array<{ name: string; default?: unknown }>; + config_keys?: Array<{ name: string }>; }; }, configValues: Record ) => { const fields: { key: string; value: string }[] = []; - for (const { name, default: defaultValue } of provider.metadata.config_keys ?? []) { - const value = configValues[name] ?? defaultValue; - if (value === undefined || value === null || value === '') { + for (const { name } of provider.metadata.config_keys ?? []) { + const value = configValues[name]; + if (value === undefined || value === '') { continue; } - fields.push({ key: name, value: String(value) }); + fields.push({ key: name, value }); } await acpSaveProviderConfig(provider.name, fields);