Skip to content

Commit c24acec

Browse files
committed
feat: centralize API configuration in ProviderSettingsManager
- Added getCurrentProviderSettings(), getModeProviderSettings(), and updateCurrentProviderSettings() methods to ProviderSettingsManager - Updated ClineProvider to use ProviderSettingsManager instead of ContextProxy for API configuration - Modified Task class to fetch API configuration from ProviderSettingsManager with async initialization - Updated webview message handlers to use the centralized source - Removed getProviderSettings() and setProviderSettings() methods from ContextProxy - Updated all affected test files to reflect the architectural changes - Fixed test mocks to properly handle the new async API configuration flow This change makes ProviderSettingsManager the sole source of truth for API configuration data within the src/ directory, eliminating the dual-source confusion and potential synchronization issues.
1 parent 75f93c4 commit c24acec

File tree

13 files changed

+449
-259
lines changed

13 files changed

+449
-259
lines changed

src/core/config/ContextProxy.ts

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@ import * as vscode from "vscode"
22
import { ZodError } from "zod"
33

44
import {
5-
PROVIDER_SETTINGS_KEYS,
65
GLOBAL_SETTINGS_KEYS,
76
SECRET_STATE_KEYS,
87
GLOBAL_STATE_KEYS,
9-
type ProviderSettings,
108
type GlobalSettings,
119
type SecretState,
1210
type GlobalState,
1311
type RooCodeSettings,
14-
providerSettingsSchema,
1512
globalSettingsSchema,
1613
isSecretStateKey,
1714
} from "@roo-code/types"
@@ -186,48 +183,6 @@ export class ContextProxy {
186183
}
187184
}
188185

189-
/**
190-
* ProviderSettings
191-
*/
192-
193-
public getProviderSettings(): ProviderSettings {
194-
const values = this.getValues()
195-
196-
try {
197-
return providerSettingsSchema.parse(values)
198-
} catch (error) {
199-
if (error instanceof ZodError) {
200-
TelemetryService.instance.captureSchemaValidationError({ schemaName: "ProviderSettings", error })
201-
}
202-
203-
return PROVIDER_SETTINGS_KEYS.reduce((acc, key) => ({ ...acc, [key]: values[key] }), {} as ProviderSettings)
204-
}
205-
}
206-
207-
public async setProviderSettings(values: ProviderSettings) {
208-
// Explicitly clear out any old API configuration values before that
209-
// might not be present in the new configuration.
210-
// If a value is not present in the new configuration, then it is assumed
211-
// that the setting's value should be `undefined` and therefore we
212-
// need to remove it from the state cache if it exists.
213-
214-
// Ensure openAiHeaders is always an object even when empty
215-
// This is critical for proper serialization/deserialization through IPC
216-
if (values.openAiHeaders !== undefined) {
217-
// Check if it's empty or null
218-
if (!values.openAiHeaders || Object.keys(values.openAiHeaders).length === 0) {
219-
values.openAiHeaders = {}
220-
}
221-
}
222-
223-
await this.setValues({
224-
...PROVIDER_SETTINGS_KEYS.filter((key) => !isSecretStateKey(key))
225-
.filter((key) => !!this.stateCache[key])
226-
.reduce((acc, key) => ({ ...acc, [key]: undefined }), {} as ProviderSettings),
227-
...values,
228-
})
229-
}
230-
231186
/**
232187
* RooCodeSettings
233188
*/

src/core/config/ProviderSettingsManager.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { z, ZodError } from "zod"
33

44
import {
55
type ProviderSettingsEntry,
6+
type ProviderSettings,
67
providerSettingsSchema,
78
providerSettingsSchemaDiscriminated,
89
DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
@@ -373,6 +374,82 @@ export class ProviderSettingsManager {
373374
}
374375
}
375376

377+
/**
378+
* Get the current active provider settings.
379+
* This combines the profile data with the actual provider settings.
380+
*/
381+
public async getCurrentProviderSettings(): Promise<ProviderSettings> {
382+
try {
383+
return await this.lock(async () => {
384+
const providerProfiles = await this.load()
385+
const currentName = providerProfiles.currentApiConfigName
386+
387+
if (!currentName || !providerProfiles.apiConfigs[currentName]) {
388+
// Return default empty settings if no current config
389+
return {} as ProviderSettings
390+
}
391+
392+
const { id, ...settings } = providerProfiles.apiConfigs[currentName]
393+
return settings as ProviderSettings
394+
})
395+
} catch (error) {
396+
throw new Error(`Failed to get current provider settings: ${error}`)
397+
}
398+
}
399+
400+
/**
401+
* Get provider settings for a specific mode.
402+
* Returns the settings for the API config assigned to that mode.
403+
*/
404+
public async getModeProviderSettings(mode: Mode): Promise<ProviderSettings | undefined> {
405+
try {
406+
return await this.lock(async () => {
407+
const providerProfiles = await this.load()
408+
const configId = providerProfiles.modeApiConfigs?.[mode]
409+
410+
if (!configId) {
411+
return undefined
412+
}
413+
414+
// Find the config with this ID
415+
const config = Object.values(providerProfiles.apiConfigs).find((c) => c.id === configId)
416+
if (!config) {
417+
return undefined
418+
}
419+
420+
const { id, ...settings } = config
421+
return settings as ProviderSettings
422+
})
423+
} catch (error) {
424+
throw new Error(`Failed to get mode provider settings: ${error}`)
425+
}
426+
}
427+
428+
/**
429+
* Update the current active provider settings.
430+
* This updates the settings for the currently active profile.
431+
*/
432+
public async updateCurrentProviderSettings(settings: ProviderSettings): Promise<void> {
433+
try {
434+
return await this.lock(async () => {
435+
const providerProfiles = await this.load()
436+
const currentName = providerProfiles.currentApiConfigName
437+
438+
if (!currentName || !providerProfiles.apiConfigs[currentName]) {
439+
throw new Error("No active configuration to update")
440+
}
441+
442+
// Preserve the ID when updating
443+
const existingId = providerProfiles.apiConfigs[currentName].id
444+
providerProfiles.apiConfigs[currentName] = { ...settings, id: existingId }
445+
446+
await this.store(providerProfiles)
447+
})
448+
} catch (error) {
449+
throw new Error(`Failed to update current provider settings: ${error}`)
450+
}
451+
}
452+
376453
/**
377454
* Delete a config by name.
378455
*/

src/core/config/__tests__/ContextProxy.spec.ts

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -291,67 +291,6 @@ describe("ContextProxy", () => {
291291
})
292292
})
293293

294-
describe("setProviderSettings", () => {
295-
it("should clear old API configuration values and set new ones", async () => {
296-
// Set up initial API configuration values
297-
await proxy.updateGlobalState("apiModelId", "old-model")
298-
await proxy.updateGlobalState("openAiBaseUrl", "https://old-url.com")
299-
await proxy.updateGlobalState("modelTemperature", 0.7)
300-
301-
// Spy on setValues
302-
const setValuesSpy = vi.spyOn(proxy, "setValues")
303-
304-
// Call setProviderSettings with new configuration
305-
await proxy.setProviderSettings({
306-
apiModelId: "new-model",
307-
apiProvider: "anthropic",
308-
// Note: openAiBaseUrl is not included in the new config
309-
})
310-
311-
// Verify setValues was called with the correct parameters
312-
// It should include undefined for openAiBaseUrl (to clear it)
313-
// and the new values for apiModelId and apiProvider
314-
expect(setValuesSpy).toHaveBeenCalledWith(
315-
expect.objectContaining({
316-
apiModelId: "new-model",
317-
apiProvider: "anthropic",
318-
openAiBaseUrl: undefined,
319-
modelTemperature: undefined,
320-
}),
321-
)
322-
323-
// Verify the state cache has been updated correctly
324-
expect(proxy.getGlobalState("apiModelId")).toBe("new-model")
325-
expect(proxy.getGlobalState("apiProvider")).toBe("anthropic")
326-
expect(proxy.getGlobalState("openAiBaseUrl")).toBeUndefined()
327-
expect(proxy.getGlobalState("modelTemperature")).toBeUndefined()
328-
})
329-
330-
it("should handle empty API configuration", async () => {
331-
// Set up initial API configuration values
332-
await proxy.updateGlobalState("apiModelId", "old-model")
333-
await proxy.updateGlobalState("openAiBaseUrl", "https://old-url.com")
334-
335-
// Spy on setValues
336-
const setValuesSpy = vi.spyOn(proxy, "setValues")
337-
338-
// Call setProviderSettings with empty configuration
339-
await proxy.setProviderSettings({})
340-
341-
// Verify setValues was called with undefined for all existing API config keys
342-
expect(setValuesSpy).toHaveBeenCalledWith(
343-
expect.objectContaining({
344-
apiModelId: undefined,
345-
openAiBaseUrl: undefined,
346-
}),
347-
)
348-
349-
// Verify the state cache has been cleared
350-
expect(proxy.getGlobalState("apiModelId")).toBeUndefined()
351-
expect(proxy.getGlobalState("openAiBaseUrl")).toBeUndefined()
352-
})
353-
})
354-
355294
describe("resetAllState", () => {
356295
it("should clear all in-memory caches", async () => {
357296
// Setup initial state in caches

src/core/config/__tests__/importExport.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ describe("importExport", () => {
8181
setValues: vi.fn(),
8282
setValue: vi.fn(),
8383
export: vi.fn().mockImplementation(() => Promise.resolve({})),
84-
setProviderSettings: vi.fn(),
8584
} as unknown as ReturnType<typeof vi.mocked<ContextProxy>>
8685

8786
mockCustomModesManager = { updateCustomMode: vi.fn() } as unknown as ReturnType<

src/core/config/importExport.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,18 +74,10 @@ export async function importSettingsFromPath(
7474
await providerSettingsManager.import(providerProfiles)
7575
await contextProxy.setValues(globalSettings)
7676

77-
// Set the current provider.
77+
// Set the current provider name in context
7878
const currentProviderName = providerProfiles.currentApiConfigName
79-
const currentProvider = providerProfiles.apiConfigs[currentProviderName]
8079
contextProxy.setValue("currentApiConfigName", currentProviderName)
8180

82-
// TODO: It seems like we don't need to have the provider settings in
83-
// the proxy; we can just use providerSettingsManager as the source of
84-
// truth.
85-
if (currentProvider) {
86-
contextProxy.setProviderSettings(currentProvider)
87-
}
88-
8981
contextProxy.setValue("listApiConfigMeta", await providerSettingsManager.listConfig())
9082

9183
return { providerProfiles, globalSettings, success: true }

0 commit comments

Comments
 (0)