mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
parent
b4d9f92b4d
commit
4c32fdae55
3 changed files with 116 additions and 11 deletions
|
|
@ -207,17 +207,18 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ 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<SettingsViewRef, SettingsViewProps>(({ onDone, t
|
|||
}
|
||||
/>
|
||||
<ApiOptions
|
||||
key={currentApiConfigName}
|
||||
uriScheme={uriScheme}
|
||||
apiConfiguration={apiConfiguration}
|
||||
setApiConfigurationField={setApiConfigurationField}
|
||||
|
|
|
|||
|
|
@ -52,6 +52,20 @@ export const OpenAICompatible = ({
|
|||
return Object.entries(headers)
|
||||
})
|
||||
|
||||
// Sync local customHeaders state when apiConfiguration changes externally
|
||||
// (e.g., on profile switch). This mirrors the pattern in ApiOptions.tsx.
|
||||
useEffect(() => {
|
||||
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(
|
||||
<K extends keyof ProviderSettings, E>(
|
||||
|
|
|
|||
|
|
@ -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<ProviderSettings> = {
|
||||
openAiHeaders: { "X-Profile": "old-profile" },
|
||||
}
|
||||
|
||||
const { rerender } = render(
|
||||
<OpenAICompatible
|
||||
apiConfiguration={initialConfig as ProviderSettings}
|
||||
setApiConfigurationField={mockSetApiConfigurationField}
|
||||
organizationAllowList={mockOrganizationAllowList}
|
||||
/>,
|
||||
)
|
||||
|
||||
// Simulate profile switch: re-render with new headers
|
||||
const newConfig: Partial<ProviderSettings> = {
|
||||
openAiHeaders: { "X-Profile": "new-profile", Authorization: "Bearer token123" },
|
||||
}
|
||||
|
||||
rerender(
|
||||
<OpenAICompatible
|
||||
apiConfiguration={newConfig as ProviderSettings}
|
||||
setApiConfigurationField={mockSetApiConfigurationField}
|
||||
organizationAllowList={mockOrganizationAllowList}
|
||||
/>,
|
||||
)
|
||||
|
||||
// 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<string, string>
|
||||
expect(writtenHeaders).not.toEqual({ "X-Profile": "old-profile" })
|
||||
}
|
||||
})
|
||||
|
||||
it("should not write back headers when they match the current config", () => {
|
||||
const config: Partial<ProviderSettings> = {
|
||||
openAiHeaders: { "X-Custom": "value" },
|
||||
}
|
||||
|
||||
render(
|
||||
<OpenAICompatible
|
||||
apiConfiguration={config as ProviderSettings}
|
||||
setApiConfigurationField={mockSetApiConfigurationField}
|
||||
organizationAllowList={mockOrganizationAllowList}
|
||||
/>,
|
||||
)
|
||||
|
||||
// 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)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue