From dbe72e9668a0210212610a671ccfff9a7592f2cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Tue, 19 May 2026 01:23:30 +0800 Subject: [PATCH] Prevent empty provider model slots from reaching runtime Provider presets and custom entries can leave haiku, sonnet, or opus blank when the main model is the intended fallback. Normalize those mappings at both the Settings UI and ProviderService boundaries so persisted provider config and generated runtime env stay consistent. Constraint: Users may configure only a main model for custom providers. Rejected: Keep blank secondary model slots | runtime env would still receive empty ANTHROPIC_DEFAULT_* values. Confidence: high Scope-risk: narrow Directive: Keep UI and ProviderService normalization aligned for provider model mapping changes. Tested: Not run per maintainer request. Not-tested: Local test suite skipped by request. --- .../src/__tests__/generalSettings.test.tsx | 56 +++++++++++++++++++ desktop/src/pages/Settings.tsx | 26 ++++++--- .../__tests__/conversation-service.test.ts | 3 + src/server/__tests__/providers.test.ts | 43 ++++++++++++++ src/server/services/providerService.ts | 14 ++++- 5 files changed, 133 insertions(+), 9 deletions(-) diff --git a/desktop/src/__tests__/generalSettings.test.tsx b/desktop/src/__tests__/generalSettings.test.tsx index 977ba0bc..19601308 100644 --- a/desktop/src/__tests__/generalSettings.test.tsx +++ b/desktop/src/__tests__/generalSettings.test.tsx @@ -878,6 +878,10 @@ describe('Settings > Providers tab', () => { MOCK_DELETE_PROVIDER.mockReset() MOCK_GET_SETTINGS.mockResolvedValue({}) MOCK_UPDATE_SETTINGS.mockResolvedValue({}) + useSettingsStore.setState({ + locale: 'en', + fetchAll: vi.fn().mockResolvedValue(undefined), + }) providerStoreState.providers = [ { id: 'provider-1', @@ -972,6 +976,58 @@ describe('Settings > Providers tab', () => { expect(within(dialog).getByText('Requests will be translated via the local proxy')).toBeInTheDocument() }) + it('normalizes blank model mappings to the main model when saving a provider', async () => { + providerStoreState.createProvider = vi.fn().mockResolvedValue({ + id: 'provider-new', + presetId: 'custom', + name: 'Custom', + apiKey: 'sk-test', + baseUrl: 'https://api.example.com/anthropic', + apiFormat: 'anthropic', + models: { + main: 'gpt-5.5', + haiku: 'gpt-5.5', + sonnet: 'gpt-5.5', + opus: 'gpt-5.5', + }, + }) + providerStoreState.presets = [ + { + id: 'custom', + name: 'Custom', + baseUrl: 'https://api.example.com/anthropic', + apiFormat: 'anthropic', + defaultModels: { + main: '', + haiku: '', + sonnet: '', + opus: '', + }, + needsApiKey: true, + websiteUrl: '', + }, + ] + + render() + + fireEvent.click(screen.getByRole('button', { name: /Add Provider|添加服务商/i })) + const dialog = screen.getByRole('dialog') + fireEvent.change(within(dialog).getByPlaceholderText('sk-...'), { target: { value: 'sk-test' } }) + fireEvent.change(within(dialog).getByLabelText(/Main Model|主模型/i), { target: { value: 'gpt-5.5' } }) + fireEvent.click(within(dialog).getByRole('button', { name: /Save|Add|保存|添加/i })) + + await waitFor(() => { + expect(providerStoreState.createProvider).toHaveBeenCalledWith(expect.objectContaining({ + models: { + main: 'gpt-5.5', + haiku: 'gpt-5.5', + sonnet: 'gpt-5.5', + opus: 'gpt-5.5', + }, + })) + }) + }) + it('hides the API key by default and reveals it from the eye button', () => { providerStoreState.presets = [ { diff --git a/desktop/src/pages/Settings.tsx b/desktop/src/pages/Settings.tsx index db7d2c93..3aee9958 100644 --- a/desktop/src/pages/Settings.tsx +++ b/desktop/src/pages/Settings.tsx @@ -513,6 +513,16 @@ function buildModelContextWindows( return windows } +function normalizeModelMapping(models: ModelMapping): ModelMapping { + const main = models.main.trim() + return { + main, + haiku: models.haiku.trim() || main, + sonnet: models.sonnet.trim() || main, + opus: models.opus.trim() || main, + } +} + function updateSettingsJsonAutoCompactWindow(raw: string, value: string): string { try { const parsed = JSON.parse(raw || '{}') as { env?: Record } @@ -685,6 +695,7 @@ function ProviderFormModal({ open, onClose, mode, provider, presets }: ProviderF const needsProxy = apiFormat !== 'anthropic' const autoCompactWindowEnv = autoCompactWindow.trim() const modelContextWindows = buildModelContextWindows(models, modelContextInputs) + const normalizedModels = normalizeModelMapping(models) const existingEnv = (settings.env as Record) || {} const cleanedEnv = stripProviderSettingsJsonEnv(existingEnv, presetDefaultEnvKeys) const merged = { @@ -699,10 +710,10 @@ function ProviderFormModal({ open, onClose, mode, provider, presets }: ProviderF : {}), ANTHROPIC_BASE_URL: needsProxy ? 'http://127.0.0.1:3456/proxy' : baseUrl, ...buildSettingsJsonAuthEnv(apiFormat, authStrategy, apiKey, selectedPreset), - ANTHROPIC_MODEL: models.main, - ANTHROPIC_DEFAULT_HAIKU_MODEL: models.haiku, - ANTHROPIC_DEFAULT_SONNET_MODEL: models.sonnet, - ANTHROPIC_DEFAULT_OPUS_MODEL: models.opus, + ANTHROPIC_MODEL: normalizedModels.main, + ANTHROPIC_DEFAULT_HAIKU_MODEL: normalizedModels.haiku, + ANTHROPIC_DEFAULT_SONNET_MODEL: normalizedModels.sonnet, + ANTHROPIC_DEFAULT_OPUS_MODEL: normalizedModels.opus, }, } setSettingsJson(JSON.stringify(merged, null, 2)) @@ -830,7 +841,7 @@ function ProviderFormModal({ open, onClose, mode, provider, presets }: ProviderF setModels(nextModels) setModelContextInputs(nextInputs) setSettingsJson((current) => updateSettingsJsonModelContextWindows( - updateSettingsJsonModels(current, nextModels), + updateSettingsJsonModels(current, normalizeModelMapping(nextModels)), buildModelContextWindows(nextModels, nextInputs), )) } @@ -858,6 +869,7 @@ function ProviderFormModal({ open, onClose, mode, provider, presets }: ProviderF const handleSubmit = async () => { if (!canSubmit) return + const normalizedModels = normalizeModelMapping(models) const parsedAutoCompactWindow = parseAutoCompactWindowInput(autoCompactWindow) const parsedModelContextWindows = buildModelContextWindows(models, modelContextInputs) setIsSubmitting(true) @@ -882,7 +894,7 @@ function ProviderFormModal({ open, onClose, mode, provider, presets }: ProviderF authStrategy, baseUrl: baseUrl.trim(), apiFormat, - models, + models: normalizedModels, ...(parsedAutoCompactWindow !== undefined && { autoCompactWindow: parsedAutoCompactWindow }), ...(Object.keys(parsedModelContextWindows).length > 0 && { modelContextWindows: parsedModelContextWindows }), notes: notes.trim() || undefined, @@ -893,7 +905,7 @@ function ProviderFormModal({ open, onClose, mode, provider, presets }: ProviderF baseUrl: baseUrl.trim(), authStrategy, apiFormat, - models, + models: normalizedModels, autoCompactWindow: parsedAutoCompactWindow ?? null, modelContextWindows: Object.keys(parsedModelContextWindows).length > 0 ? parsedModelContextWindows diff --git a/src/server/__tests__/conversation-service.test.ts b/src/server/__tests__/conversation-service.test.ts index 10dcf615..0f4eefe3 100644 --- a/src/server/__tests__/conversation-service.test.ts +++ b/src/server/__tests__/conversation-service.test.ts @@ -276,6 +276,9 @@ describe('ConversationService', () => { expect(env.ANTHROPIC_BASE_URL).toBe(`http://127.0.0.1:3456/proxy/providers/${provider.id}`) expect(env.ANTHROPIC_API_KEY).toBe('proxy-managed') expect(env.ANTHROPIC_MODEL).toBe('kimi-k2.6') + expect(env.ANTHROPIC_DEFAULT_HAIKU_MODEL).toBe('kimi-k2.6') + expect(env.ANTHROPIC_DEFAULT_SONNET_MODEL).toBe('kimi-k2.6') + expect(env.ANTHROPIC_DEFAULT_OPUS_MODEL).toBe('kimi-k2.6') expect(env.CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST).toBe('1') expect(env.CLAUDE_CODE_ENTRYPOINT).toBeUndefined() }) diff --git a/src/server/__tests__/providers.test.ts b/src/server/__tests__/providers.test.ts index b08e561c..40754b85 100644 --- a/src/server/__tests__/providers.test.ts +++ b/src/server/__tests__/providers.test.ts @@ -153,6 +153,28 @@ describe('ProviderService', () => { expect(provider.models.main).toBe('model-main') }) + test('should normalize empty model mappings to the main model when adding a provider', async () => { + const svc = new ProviderService() + const provider = await svc.addProvider(sampleInput({ + models: { + main: 'gpt-5.5', + haiku: '', + sonnet: ' ', + opus: '', + }, + })) + + expect(provider.models).toEqual({ + main: 'gpt-5.5', + haiku: 'gpt-5.5', + sonnet: 'gpt-5.5', + opus: 'gpt-5.5', + }) + + const config = await readProvidersConfig() + expect((config.providers as Array<{ models: unknown }>)[0]?.models).toEqual(provider.models) + }) + test('new providers should not be auto-activated', async () => { const svc = new ProviderService() const provider = await svc.addProvider(sampleInput()) @@ -361,6 +383,27 @@ describe('ProviderService', () => { expect(env.CLAUDE_CODE_AUTO_COMPACT_WINDOW).toBeUndefined() }) + test('should normalize empty model mappings before syncing settings', async () => { + const svc = new ProviderService() + const provider = await svc.addProvider(sampleInput({ + models: { + main: 'gpt-5.5', + haiku: '', + sonnet: '', + opus: '', + }, + })) + + await svc.activateProvider(provider.id) + + const settings = await readSettings() + const env = settings.env as Record + expect(env.ANTHROPIC_MODEL).toBe('gpt-5.5') + expect(env.ANTHROPIC_DEFAULT_HAIKU_MODEL).toBe('gpt-5.5') + expect(env.ANTHROPIC_DEFAULT_SONNET_MODEL).toBe('gpt-5.5') + expect(env.ANTHROPIC_DEFAULT_OPUS_MODEL).toBe('gpt-5.5') + }) + test('updating active provider should override and clear model context windows', async () => { const svc = new ProviderService() const added = await svc.addProvider(sampleInput({ diff --git a/src/server/services/providerService.ts b/src/server/services/providerService.ts index 2d4202f0..22ef7f80 100644 --- a/src/server/services/providerService.ts +++ b/src/server/services/providerService.ts @@ -85,6 +85,16 @@ function isSavedProvider(value: unknown): value is SavedProvider { ) } +function normalizeModelMapping(models: SavedProvider['models']): SavedProvider['models'] { + const main = models.main.trim() + return { + main, + haiku: models.haiku.trim() || main, + sonnet: models.sonnet.trim() || main, + opus: models.opus.trim() || main, + } +} + function normalizeProvidersIndex(value: unknown): ProvidersIndex | null { if (!isRecord(value) || !Array.isArray(value.providers)) { return null @@ -258,7 +268,7 @@ export class ProviderService { ...(input.authStrategy !== undefined && { authStrategy: input.authStrategy }), baseUrl: input.baseUrl, apiFormat: input.apiFormat ?? 'anthropic', - models: input.models, + models: normalizeModelMapping(input.models), ...(input.autoCompactWindow !== undefined && { autoCompactWindow: input.autoCompactWindow }), ...(input.modelContextWindows !== undefined && { modelContextWindows: input.modelContextWindows }), ...(input.notes !== undefined && { notes: input.notes }), @@ -282,7 +292,7 @@ export class ProviderService { ...(input.authStrategy !== undefined && { authStrategy: input.authStrategy }), ...(input.baseUrl !== undefined && { baseUrl: input.baseUrl }), ...(input.apiFormat !== undefined && { apiFormat: input.apiFormat }), - ...(input.models !== undefined && { models: input.models }), + ...(input.models !== undefined && { models: normalizeModelMapping(input.models) }), ...(typeof input.autoCompactWindow === 'number' && { autoCompactWindow: input.autoCompactWindow }), ...(input.modelContextWindows !== undefined && input.modelContextWindows !== null && { modelContextWindows: input.modelContextWindows }), ...(input.notes !== undefined && { notes: input.notes }),