From ad65567e3ed928615737ec77b12050dfdf4ea22b Mon Sep 17 00:00:00 2001 From: PARTH J ROHIT Date: Fri, 11 Sep 2026 20:10:08 +0100 Subject: [PATCH] fix(settings): don't send a stale base URL to a fixed-endpoint AI provider Saving Ollama and then selecting Google Gemini answered "AI provider destination is not permitted." with nothing on screen to correct. The base URL is loaded into form state from the saved configuration and was never cleared when the provider changed, so the Ollama URL was still being sent. A fixed-destination provider rejects any base URL at all (`ai_egress.validate_config`: `if config.base_url is not None: raise _deny()`), so the request was denied -- and because those providers show no base URL field, the offending value was invisible. There was no sequence of UI actions that could recover: reloading re-reads the saved Ollama config and puts the URL straight back. Selecting a provider that does not take a base URL now clears it, and the save and test requests omit the field entirely for such a provider whatever the state happens to hold. A provider that does require one is unaffected. --- .../settings/hooks/useSettings.test.ts | 49 +++++++++++++++++++ .../features/settings/hooks/useSettings.ts | 23 +++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/apps/frontend/src/features/settings/hooks/useSettings.test.ts b/apps/frontend/src/features/settings/hooks/useSettings.test.ts index 6dde595..fb0604d 100644 --- a/apps/frontend/src/features/settings/hooks/useSettings.test.ts +++ b/apps/frontend/src/features/settings/hooks/useSettings.test.ts @@ -136,3 +136,52 @@ describe('useSettings tab deep-linking', () => { expect(result.current.activeTab).toBe('Notifications'); }); }); + +describe('useSettings base URL handling across providers', () => { + const OLLAMA_SAVED: AiProviderPublicConfig = { + provider: 'ollama', + model: 'llama3.2', + baseUrl: 'http://localhost:11434', + hasApiKey: false, + apiKeyLast4: null, + }; + + it('does not carry a saved Ollama base URL into a fixed-endpoint provider', async () => { + // The reported sequence: Ollama is saved, the user switches to a hosted + // provider, and the backend answers "AI provider destination is not + // permitted." because a fixed destination rejects any base URL -- while + // the form shows no base URL field to clear, so the value is invisible. + vi.mocked(aiService.getConfig).mockResolvedValue(OLLAMA_SAVED); + vi.mocked(aiService.testConfig).mockResolvedValue({ ok: true, message: 'Connected.' }); + vi.mocked(aiService.saveConfig).mockResolvedValue(EMPTY_CONFIG); + + const { result } = renderHook(() => useSettings(), { wrapper: wrapper() }); + await waitFor(() => expect(result.current.baseUrl).toBe('http://localhost:11434')); + + act(() => result.current.setProvider('openai')); + expect(result.current.baseUrl).toBe(''); + + await act(async () => { + await result.current.testAiConfig(); + }); + expect(vi.mocked(aiService.testConfig).mock.lastCall?.[0].baseUrl).toBeUndefined(); + + await act(async () => { + await result.current.saveAiConfig(); + }); + expect(vi.mocked(aiService.saveConfig).mock.lastCall?.[0].baseUrl).toBeUndefined(); + }); + + it('still sends the base URL for a provider that requires one', async () => { + vi.mocked(aiService.getConfig).mockResolvedValue(OLLAMA_SAVED); + vi.mocked(aiService.saveConfig).mockResolvedValue(OLLAMA_SAVED); + + const { result } = renderHook(() => useSettings(), { wrapper: wrapper() }); + await waitFor(() => expect(result.current.baseUrl).toBe('http://localhost:11434')); + + await act(async () => { + await result.current.saveAiConfig(); + }); + expect(vi.mocked(aiService.saveConfig).mock.lastCall?.[0].baseUrl).toBe('http://localhost:11434'); + }); +}); diff --git a/apps/frontend/src/features/settings/hooks/useSettings.ts b/apps/frontend/src/features/settings/hooks/useSettings.ts index 83c97d3..43fd119 100644 --- a/apps/frontend/src/features/settings/hooks/useSettings.ts +++ b/apps/frontend/src/features/settings/hooks/useSettings.ts @@ -88,12 +88,27 @@ export function useSettings() { (nextProvider: AiProvider) => { setProvider(nextProvider); setModel(capabilityByProvider.get(nextProvider)?.defaultModel ?? ''); + // A provider with a fixed endpoint has no base URL field, so a value + // left over from a previously selected provider would be invisible here + // and still be sent -- and the backend denies any base URL on a fixed + // destination, which surfaced as "AI provider destination is not + // permitted." with nothing on screen to correct. + if (!capabilityByProvider.get(nextProvider)?.requiresBaseUrl) { + setBaseUrl(''); + } setStatusMessage(null); setError(null); }, [capabilityByProvider], ); + // Belt and braces: never send a base URL for a provider that does not take + // one, whatever the field happens to hold. + const baseUrlForRequest = useCallback(() => { + if (!capabilityByProvider.get(provider)?.requiresBaseUrl) return undefined; + return baseUrl.trim() || undefined; + }, [baseUrl, capabilityByProvider, provider]); + const saveAiConfig = useCallback(async () => { setLoading(true); setError(null); @@ -104,7 +119,7 @@ export function useSettings() { provider, apiKey: apiKey.trim() || undefined, model: model.trim() || defaultModel, - baseUrl: baseUrl.trim() || undefined, + baseUrl: baseUrlForRequest(), }); setAiConfig(config); setApiKey(''); @@ -114,7 +129,7 @@ export function useSettings() { } finally { setLoading(false); } - }, [apiKey, baseUrl, capabilityByProvider, model, provider]); + }, [apiKey, baseUrlForRequest, capabilityByProvider, model, provider]); const testAiConfig = useCallback(async () => { setTesting(true); @@ -126,7 +141,7 @@ export function useSettings() { provider, apiKey: apiKey.trim() || undefined, model: model.trim() || defaultModel, - baseUrl: baseUrl.trim() || undefined, + baseUrl: baseUrlForRequest(), }); setStatusMessage(response.message); } catch (caught) { @@ -134,7 +149,7 @@ export function useSettings() { } finally { setTesting(false); } - }, [apiKey, baseUrl, capabilityByProvider, model, provider]); + }, [apiKey, baseUrlForRequest, capabilityByProvider, model, provider]); return { tabs: settingsTabs,