fix(desktop): stop provider save from overwriting stored config with defaults (#11517)
Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -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);
|
||||
|
||||
+34
@@ -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(
|
||||
<CustomProviderForm
|
||||
initialData={{
|
||||
engine,
|
||||
display_name: 'Existing Provider',
|
||||
api_url: 'https://existing.example.com',
|
||||
api_key: '',
|
||||
models: ['model-a'],
|
||||
supports_streaming: true,
|
||||
requires_auth: true,
|
||||
}}
|
||||
isEditable
|
||||
onSubmit={onSubmit}
|
||||
onCancel={vi.fn()}
|
||||
/>,
|
||||
{ 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();
|
||||
|
||||
+25
-31
@@ -227,6 +227,20 @@ const i18n = defineMessages({
|
||||
|
||||
type Step = 'choice' | 'catalog' | 'form';
|
||||
|
||||
type ProviderEngine = 'openai_compatible' | 'anthropic_compatible' | 'ollama_compatible';
|
||||
|
||||
const ENGINE_ALIASES: Record<string, ProviderEngine> = {
|
||||
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<void>;
|
||||
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<ProviderEngine>('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<string, string> = {
|
||||
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<string, string> = {
|
||||
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}
|
||||
|
||||
+5
-2
@@ -102,10 +102,13 @@ export default function DefaultProviderSetupForm({
|
||||
const values: { [k: string]: ConfigInput } = {};
|
||||
|
||||
let fields: Awaited<ReturnType<typeof acpReadProviderConfig>> = [];
|
||||
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 };
|
||||
}
|
||||
}
|
||||
|
||||
+66
@@ -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' });
|
||||
});
|
||||
});
|
||||
+5
-5
@@ -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<string, string>
|
||||
) => {
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user