From 4c32fdae55d8e8a1480f007e1bfc6d110243b2f8 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 13 Feb 2026 15:29:12 +0000 Subject: [PATCH] fix: refresh provider settings on profile switch in Settings view When switching Configuration Profiles within the Settings/Providers tab, provider settings were not refreshing because: 1. The cachedState update happened asynchronously (useEffect) after the render cycle, causing child components to mount with stale config 2. ApiOptions and provider components retained local state from the previous profile since they were not remounted Three fixes applied: - Changed profile-switch useEffect to synchronous render-time state update (React "adjusting state when a prop changes" pattern) - Added key={currentApiConfigName} on ApiOptions to force full remount on profile switch, resetting all local state in provider components - Added sync effects and write-back guards in OpenAICompatible.tsx to prevent stale header overwrites Closes #11450 --- .../src/components/settings/SettingsView.tsx | 18 ++-- .../settings/providers/OpenAICompatible.tsx | 24 +++++- .../__tests__/OpenAICompatible.spec.tsx | 85 +++++++++++++++++++ 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 47e087615e3..5d538f482ff 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -207,17 +207,18 @@ const SettingsView = forwardRef(({ onDone, t const apiConfiguration = useMemo(() => cachedState.apiConfiguration ?? {}, [cachedState.apiConfiguration]) - useEffect(() => { - // Update only when currentApiConfigName is changed. - // Expected to be triggered by loadApiConfiguration/upsertApiConfiguration. - if (prevApiConfigName.current === currentApiConfigName) { - return - } - + // Synchronous state update during render when profile changes. + // This follows the React pattern for "adjusting state when a prop changes" + // and ensures cachedState is updated BEFORE child components (like ApiOptions) + // render, so they receive the correct apiConfiguration immediately. + // Using useEffect here would cause a timing issue: the key-based remount of + // ApiOptions would occur before cachedState is updated, initializing local + // state from the stale (old profile) config. + if (prevApiConfigName.current !== currentApiConfigName) { setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState })) prevApiConfigName.current = currentApiConfigName setChangeDetected(false) - }, [currentApiConfigName, extensionState]) + } // Bust the cache when settings are imported. useEffect(() => { @@ -767,6 +768,7 @@ const SettingsView = forwardRef(({ onDone, t } /> { + const propHeaders = apiConfiguration?.openAiHeaders || {} + if (JSON.stringify(customHeaders) !== JSON.stringify(Object.entries(propHeaders))) { + setCustomHeaders(Object.entries(propHeaders)) + } + }, [apiConfiguration?.openAiHeaders]) // eslint-disable-line react-hooks/exhaustive-deps + + // Sync azureApiVersionSelected when apiConfiguration changes externally + useEffect(() => { + setAzureApiVersionSelected(!!apiConfiguration?.azureApiVersion) + }, [apiConfiguration?.azureApiVersion]) + const handleAddCustomHeader = useCallback(() => { // Only update the local state to show the new row in the UI. setCustomHeaders((prev) => [...prev, ["", ""]]) @@ -88,15 +102,19 @@ export const OpenAICompatible = ({ // Helper to convert array of tuples to object - // Add effect to update the parent component's state when local headers change + // Debounced write-back: update the parent config when local headers change. + // Guard against no-op writes to prevent stale overwrites on profile switch. useEffect(() => { const timer = setTimeout(() => { + const currentConfigHeaders = apiConfiguration?.openAiHeaders || {} const headerObject = convertHeadersToObject(customHeaders) - setApiConfigurationField("openAiHeaders", headerObject, false) + if (JSON.stringify(currentConfigHeaders) !== JSON.stringify(headerObject)) { + setApiConfigurationField("openAiHeaders", headerObject, false) + } }, 300) return () => clearTimeout(timer) - }, [customHeaders, setApiConfigurationField]) + }, [customHeaders, apiConfiguration?.openAiHeaders, setApiConfigurationField]) const handleInputChange = useCallback( ( diff --git a/webview-ui/src/components/settings/providers/__tests__/OpenAICompatible.spec.tsx b/webview-ui/src/components/settings/providers/__tests__/OpenAICompatible.spec.tsx index aba81ec2191..a819ef62316 100644 --- a/webview-ui/src/components/settings/providers/__tests__/OpenAICompatible.spec.tsx +++ b/webview-ui/src/components/settings/providers/__tests__/OpenAICompatible.spec.tsx @@ -313,3 +313,88 @@ describe("OpenAICompatible Component - includeMaxTokens checkbox", () => { }) }) }) + +describe("OpenAICompatible Component - Profile Switch Sync", () => { + const mockSetApiConfigurationField = vi.fn() + const mockOrganizationAllowList = { + allowAll: true, + providers: {}, + } + + beforeEach(() => { + vi.clearAllMocks() + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("should sync custom headers when apiConfiguration changes (profile switch)", () => { + const initialConfig: Partial = { + openAiHeaders: { "X-Profile": "old-profile" }, + } + + const { rerender } = render( + , + ) + + // Simulate profile switch: re-render with new headers + const newConfig: Partial = { + openAiHeaders: { "X-Profile": "new-profile", Authorization: "Bearer token123" }, + } + + rerender( + , + ) + + // After the debounced write-back fires, it should not overwrite the new headers + // with old ones. Advance timers to trigger the debounced write-back. + vi.advanceTimersByTime(350) + + // The write-back should be guarded: since the local state is synced to new headers, + // and the new headers match the config, no write should occur (no-op guard) + const writeBackCalls = mockSetApiConfigurationField.mock.calls.filter( + (call: any[]) => call[0] === "openAiHeaders", + ) + // If there are write-back calls, they should be writing the NEW headers, not the old ones + for (const call of writeBackCalls) { + const writtenHeaders = call[1] as Record + expect(writtenHeaders).not.toEqual({ "X-Profile": "old-profile" }) + } + }) + + it("should not write back headers when they match the current config", () => { + const config: Partial = { + openAiHeaders: { "X-Custom": "value" }, + } + + render( + , + ) + + // Clear initial calls + mockSetApiConfigurationField.mockClear() + + // Advance past the debounce timer + vi.advanceTimersByTime(350) + + // The write-back should detect that the headers are unchanged and skip the write + const headerWriteCalls = mockSetApiConfigurationField.mock.calls.filter( + (call: any[]) => call[0] === "openAiHeaders", + ) + expect(headerWriteCalls).toHaveLength(0) + }) +})