From c24acec6427b341a928884bc50187b16acf01365 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 30 Jul 2025 17:41:02 +0000 Subject: [PATCH] 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. --- src/core/config/ContextProxy.ts | 45 ---- src/core/config/ProviderSettingsManager.ts | 77 ++++++ .../config/__tests__/ContextProxy.spec.ts | 61 ----- .../config/__tests__/importExport.spec.ts | 1 - src/core/config/importExport.ts | 10 +- src/core/task/Task.ts | 68 +++-- src/core/task/__tests__/Task.spec.ts | 109 ++++---- src/core/webview/ClineProvider.ts | 50 ++-- .../webview/__tests__/ClineProvider.spec.ts | 235 ++++++++++++++---- .../ClineProvider.sticky-mode.spec.ts | 6 - .../__tests__/webviewMessageHandler.spec.ts | 29 +++ src/core/webview/webviewMessageHandler.ts | 16 +- .../__tests__/autoImportSettings.spec.ts | 1 - 13 files changed, 449 insertions(+), 259 deletions(-) diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index 5535cd2ff4..309746546b 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -2,16 +2,13 @@ import * as vscode from "vscode" import { ZodError } from "zod" import { - PROVIDER_SETTINGS_KEYS, GLOBAL_SETTINGS_KEYS, SECRET_STATE_KEYS, GLOBAL_STATE_KEYS, - type ProviderSettings, type GlobalSettings, type SecretState, type GlobalState, type RooCodeSettings, - providerSettingsSchema, globalSettingsSchema, isSecretStateKey, } from "@roo-code/types" @@ -186,48 +183,6 @@ export class ContextProxy { } } - /** - * ProviderSettings - */ - - public getProviderSettings(): ProviderSettings { - const values = this.getValues() - - try { - return providerSettingsSchema.parse(values) - } catch (error) { - if (error instanceof ZodError) { - TelemetryService.instance.captureSchemaValidationError({ schemaName: "ProviderSettings", error }) - } - - return PROVIDER_SETTINGS_KEYS.reduce((acc, key) => ({ ...acc, [key]: values[key] }), {} as ProviderSettings) - } - } - - public async setProviderSettings(values: ProviderSettings) { - // Explicitly clear out any old API configuration values before that - // might not be present in the new configuration. - // If a value is not present in the new configuration, then it is assumed - // that the setting's value should be `undefined` and therefore we - // need to remove it from the state cache if it exists. - - // Ensure openAiHeaders is always an object even when empty - // This is critical for proper serialization/deserialization through IPC - if (values.openAiHeaders !== undefined) { - // Check if it's empty or null - if (!values.openAiHeaders || Object.keys(values.openAiHeaders).length === 0) { - values.openAiHeaders = {} - } - } - - await this.setValues({ - ...PROVIDER_SETTINGS_KEYS.filter((key) => !isSecretStateKey(key)) - .filter((key) => !!this.stateCache[key]) - .reduce((acc, key) => ({ ...acc, [key]: undefined }), {} as ProviderSettings), - ...values, - }) - } - /** * RooCodeSettings */ diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 350c8136f2..935381078e 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -3,6 +3,7 @@ import { z, ZodError } from "zod" import { type ProviderSettingsEntry, + type ProviderSettings, providerSettingsSchema, providerSettingsSchemaDiscriminated, DEFAULT_CONSECUTIVE_MISTAKE_LIMIT, @@ -373,6 +374,82 @@ export class ProviderSettingsManager { } } + /** + * Get the current active provider settings. + * This combines the profile data with the actual provider settings. + */ + public async getCurrentProviderSettings(): Promise { + try { + return await this.lock(async () => { + const providerProfiles = await this.load() + const currentName = providerProfiles.currentApiConfigName + + if (!currentName || !providerProfiles.apiConfigs[currentName]) { + // Return default empty settings if no current config + return {} as ProviderSettings + } + + const { id, ...settings } = providerProfiles.apiConfigs[currentName] + return settings as ProviderSettings + }) + } catch (error) { + throw new Error(`Failed to get current provider settings: ${error}`) + } + } + + /** + * Get provider settings for a specific mode. + * Returns the settings for the API config assigned to that mode. + */ + public async getModeProviderSettings(mode: Mode): Promise { + try { + return await this.lock(async () => { + const providerProfiles = await this.load() + const configId = providerProfiles.modeApiConfigs?.[mode] + + if (!configId) { + return undefined + } + + // Find the config with this ID + const config = Object.values(providerProfiles.apiConfigs).find((c) => c.id === configId) + if (!config) { + return undefined + } + + const { id, ...settings } = config + return settings as ProviderSettings + }) + } catch (error) { + throw new Error(`Failed to get mode provider settings: ${error}`) + } + } + + /** + * Update the current active provider settings. + * This updates the settings for the currently active profile. + */ + public async updateCurrentProviderSettings(settings: ProviderSettings): Promise { + try { + return await this.lock(async () => { + const providerProfiles = await this.load() + const currentName = providerProfiles.currentApiConfigName + + if (!currentName || !providerProfiles.apiConfigs[currentName]) { + throw new Error("No active configuration to update") + } + + // Preserve the ID when updating + const existingId = providerProfiles.apiConfigs[currentName].id + providerProfiles.apiConfigs[currentName] = { ...settings, id: existingId } + + await this.store(providerProfiles) + }) + } catch (error) { + throw new Error(`Failed to update current provider settings: ${error}`) + } + } + /** * Delete a config by name. */ diff --git a/src/core/config/__tests__/ContextProxy.spec.ts b/src/core/config/__tests__/ContextProxy.spec.ts index 86b7bbef30..ea31f39992 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -291,67 +291,6 @@ describe("ContextProxy", () => { }) }) - describe("setProviderSettings", () => { - it("should clear old API configuration values and set new ones", async () => { - // Set up initial API configuration values - await proxy.updateGlobalState("apiModelId", "old-model") - await proxy.updateGlobalState("openAiBaseUrl", "https://old-url.com") - await proxy.updateGlobalState("modelTemperature", 0.7) - - // Spy on setValues - const setValuesSpy = vi.spyOn(proxy, "setValues") - - // Call setProviderSettings with new configuration - await proxy.setProviderSettings({ - apiModelId: "new-model", - apiProvider: "anthropic", - // Note: openAiBaseUrl is not included in the new config - }) - - // Verify setValues was called with the correct parameters - // It should include undefined for openAiBaseUrl (to clear it) - // and the new values for apiModelId and apiProvider - expect(setValuesSpy).toHaveBeenCalledWith( - expect.objectContaining({ - apiModelId: "new-model", - apiProvider: "anthropic", - openAiBaseUrl: undefined, - modelTemperature: undefined, - }), - ) - - // Verify the state cache has been updated correctly - expect(proxy.getGlobalState("apiModelId")).toBe("new-model") - expect(proxy.getGlobalState("apiProvider")).toBe("anthropic") - expect(proxy.getGlobalState("openAiBaseUrl")).toBeUndefined() - expect(proxy.getGlobalState("modelTemperature")).toBeUndefined() - }) - - it("should handle empty API configuration", async () => { - // Set up initial API configuration values - await proxy.updateGlobalState("apiModelId", "old-model") - await proxy.updateGlobalState("openAiBaseUrl", "https://old-url.com") - - // Spy on setValues - const setValuesSpy = vi.spyOn(proxy, "setValues") - - // Call setProviderSettings with empty configuration - await proxy.setProviderSettings({}) - - // Verify setValues was called with undefined for all existing API config keys - expect(setValuesSpy).toHaveBeenCalledWith( - expect.objectContaining({ - apiModelId: undefined, - openAiBaseUrl: undefined, - }), - ) - - // Verify the state cache has been cleared - expect(proxy.getGlobalState("apiModelId")).toBeUndefined() - expect(proxy.getGlobalState("openAiBaseUrl")).toBeUndefined() - }) - }) - describe("resetAllState", () => { it("should clear all in-memory caches", async () => { // Setup initial state in caches diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 361d6b23b0..1299c3828d 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -81,7 +81,6 @@ describe("importExport", () => { setValues: vi.fn(), setValue: vi.fn(), export: vi.fn().mockImplementation(() => Promise.resolve({})), - setProviderSettings: vi.fn(), } as unknown as ReturnType> mockCustomModesManager = { updateCustomMode: vi.fn() } as unknown as ReturnType< diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index c3d6f9c215..4ab3073ca8 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -74,18 +74,10 @@ export async function importSettingsFromPath( await providerSettingsManager.import(providerProfiles) await contextProxy.setValues(globalSettings) - // Set the current provider. + // Set the current provider name in context const currentProviderName = providerProfiles.currentApiConfigName - const currentProvider = providerProfiles.apiConfigs[currentProviderName] contextProxy.setValue("currentApiConfigName", currentProviderName) - // TODO: It seems like we don't need to have the provider settings in - // the proxy; we can just use providerSettingsManager as the source of - // truth. - if (currentProvider) { - contextProxy.setProviderSettings(currentProvider) - } - contextProxy.setValue("listApiConfigMeta", await providerSettingsManager.listConfig()) return { providerProfiles, globalSettings, success: true } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index edbde32ea7..2868d2b1f3 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -112,7 +112,6 @@ export type ClineEvents = { export type TaskOptions = { provider: ClineProvider - apiConfiguration: ProviderSettings enableDiff?: boolean enableCheckpoints?: boolean fuzzyMatchThreshold?: number @@ -192,7 +191,7 @@ export class Task extends EventEmitter { private pauseInterval: NodeJS.Timeout | undefined // API - readonly apiConfiguration: ProviderSettings + private _apiConfiguration?: ProviderSettings api: ApiHandler private static lastGlobalApiRequestTime?: number private consecutiveAutoApprovedRequestsCount: number = 0 @@ -234,7 +233,7 @@ export class Task extends EventEmitter { // Tool Use consecutiveMistakeCount: number = 0 - consecutiveMistakeLimit: number + consecutiveMistakeLimit: number = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT consecutiveMistakeCountForApplyDiff: Map = new Map() toolUsage: ToolUsage = {} @@ -258,11 +257,10 @@ export class Task extends EventEmitter { constructor({ provider, - apiConfiguration, enableDiff = false, enableCheckpoints = true, fuzzyMatchThreshold = 1.0, - consecutiveMistakeLimit = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT, + consecutiveMistakeLimit, task, images, historyItem, @@ -294,19 +292,29 @@ export class Task extends EventEmitter { console.error("Failed to initialize RooIgnoreController:", error) }) - this.apiConfiguration = apiConfiguration - this.api = buildApiHandler(apiConfiguration) + // API configuration will be loaded asynchronously + this.api = buildApiHandler({} as ProviderSettings) // Temporary, will be updated in initializeApiConfiguration this.urlContentFetcher = new UrlContentFetcher(provider.context) this.browserSession = new BrowserSession(provider.context) this.diffEnabled = enableDiff this.fuzzyMatchThreshold = fuzzyMatchThreshold - this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT this.providerRef = new WeakRef(provider) this.globalStoragePath = provider.context.globalStorageUri.fsPath this.diffViewProvider = new DiffViewProvider(this.cwd, this) this.enableCheckpoints = enableCheckpoints + // Set initial consecutiveMistakeLimit, will be updated when API config is loaded + this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT + + // Initialize API configuration + this.initializeApiConfiguration().then((apiConfig) => { + // Update consecutiveMistakeLimit with API config value if not explicitly provided + if (consecutiveMistakeLimit === undefined && apiConfig.consecutiveMistakeLimit !== undefined) { + this.consecutiveMistakeLimit = apiConfig.consecutiveMistakeLimit + } + }) + this.rootTask = rootTask this.parentTask = parentTask this.taskNumber = taskNumber @@ -357,6 +365,33 @@ export class Task extends EventEmitter { } } + /** + * Initialize API configuration from ProviderSettingsManager. + * This ensures the Task always uses the current API configuration. + */ + private async initializeApiConfiguration(): Promise { + const provider = this.providerRef.deref() + if (!provider) { + throw new Error("Provider reference lost during API configuration initialization") + } + + const apiConfiguration = await provider.providerSettingsManager.getCurrentProviderSettings() + this._apiConfiguration = apiConfiguration + this.api = buildApiHandler(apiConfiguration) + return apiConfiguration + } + + /** + * Get the current API configuration, loading it if necessary. + * This ensures we always have the latest configuration from ProviderSettingsManager. + */ + public async getApiConfiguration(): Promise { + if (!this._apiConfiguration) { + return await this.initializeApiConfiguration() + } + return this._apiConfiguration + } + /** * Initialize the task mode from the provider state. * This method handles async initialization with proper error handling. @@ -1387,16 +1422,12 @@ export class Task extends EventEmitter { // take a few seconds. For the best UX we show a placeholder api_req_started // message with a loading spinner as this happens. - // Determine API protocol based on provider and model - const modelId = getModelId(this.apiConfiguration) - const apiProtocol = getApiProtocol(this.apiConfiguration.apiProvider, modelId) - await this.say( "api_req_started", JSON.stringify({ request: userContent.map((block) => formatContentBlockToMarkdown(block)).join("\n\n") + "\n\nLoading...", - apiProtocol, + apiProtocol: "anthropic", // Temporary, will be updated after loading environment details }), ) @@ -1419,6 +1450,11 @@ export class Task extends EventEmitter { maxReadFileLine, }) + // Get API configuration for model info + const apiConfig = await this.getApiConfiguration() + const modelId = getModelId(apiConfig) + const apiProtocol = getApiProtocol(apiConfig.apiProvider, modelId) + const environmentDetails = await getEnvironmentDetails(this, includeFileDetails) // Add environment details as its own text block, separate from tool @@ -1848,7 +1884,6 @@ export class Task extends EventEmitter { public async *attemptApiRequest(retryAttempt: number = 0): ApiStream { const state = await this.providerRef.deref()?.getState() const { - apiConfiguration, autoApprovalEnabled, alwaysApproveResubmit, requestDelaySeconds, @@ -1858,6 +1893,9 @@ export class Task extends EventEmitter { profileThresholds = {}, } = state ?? {} + // Get API configuration from ProviderSettingsManager + const apiConfiguration = await this.getApiConfiguration() + // Get condensing configuration for automatic triggers const customCondensingPrompt = state?.customCondensingPrompt const condensingApiConfigId = state?.condensingApiConfigId @@ -1913,7 +1951,7 @@ export class Task extends EventEmitter { const maxTokens = getModelMaxOutputTokens({ modelId: this.api.getModel().id, model: modelInfo, - settings: this.apiConfiguration, + settings: apiConfiguration, }) const contextWindow = modelInfo.contextWindow diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 9aa5a8d7a8..c4a70a3834 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -262,6 +262,9 @@ describe("Cline", () => { // Mock provider methods mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + mockProvider.providerSettingsManager = { + getCurrentProviderSettings: vi.fn().mockResolvedValue(mockApiConfig), + } mockProvider.getTaskWithId = vi.fn().mockImplementation(async (id) => ({ historyItem: { id, @@ -295,7 +298,6 @@ describe("Cline", () => { it("should respect provided settings", async () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, fuzzyMatchThreshold: 0.95, task: "test task", startTask: false, @@ -307,7 +309,6 @@ describe("Cline", () => { it("should use default fuzzy match threshold when not provided", async () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, enableDiff: true, fuzzyMatchThreshold: 0.95, task: "test task", @@ -323,7 +324,6 @@ describe("Cline", () => { it("should use default consecutiveMistakeLimit when not provided", () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "test task", startTask: false, }) @@ -334,7 +334,6 @@ describe("Cline", () => { it("should respect provided consecutiveMistakeLimit", () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, consecutiveMistakeLimit: 5, task: "test task", startTask: false, @@ -346,7 +345,6 @@ describe("Cline", () => { it("should keep consecutiveMistakeLimit of 0 as 0 for unlimited", () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, consecutiveMistakeLimit: 0, task: "test task", startTask: false, @@ -358,7 +356,6 @@ describe("Cline", () => { it("should pass 0 to ToolRepetitionDetector for unlimited mode", () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, consecutiveMistakeLimit: 0, task: "test task", startTask: false, @@ -373,7 +370,6 @@ describe("Cline", () => { it("should pass consecutiveMistakeLimit to ToolRepetitionDetector", () => { const cline = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, consecutiveMistakeLimit: 5, task: "test task", startTask: false, @@ -386,7 +382,7 @@ describe("Cline", () => { it("should require either task or historyItem", () => { expect(() => { - new Task({ provider: mockProvider, apiConfiguration: mockApiConfig }) + new Task({ provider: mockProvider }) }).toThrow("Either historyItem or task/images must be provided") }) }) @@ -397,7 +393,6 @@ describe("Cline", () => { // Cline.create will now use our mocked getEnvironmentDetails const [cline, task] = Task.create({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "test task", }) @@ -504,9 +499,9 @@ describe("Cline", () => { ] // Test with model that supports images + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(configWithImages) const [clineWithImages, taskWithImages] = Task.create({ provider: mockProvider, - apiConfiguration: configWithImages, task: "test task", }) @@ -527,9 +522,9 @@ describe("Cline", () => { clineWithImages.apiConversationHistory = conversationHistory // Test with model that doesn't support images + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(configWithoutImages) const [clineWithoutImages, taskWithoutImages] = Task.create({ provider: mockProvider, - apiConfiguration: configWithoutImages, task: "test task", }) @@ -626,7 +621,6 @@ describe("Cline", () => { it.skip("should handle API retry with countdown", async () => { const [cline, task] = Task.create({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "test task", }) @@ -751,7 +745,6 @@ describe("Cline", () => { it.skip("should not apply retry delay twice", async () => { const [cline, task] = Task.create({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "test task", }) @@ -876,7 +869,6 @@ describe("Cline", () => { it("should process mentions in task and feedback tags", async () => { const [cline, task] = Task.create({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "test task", }) @@ -970,13 +962,14 @@ describe("Cline", () => { context: { globalStorageUri: { fsPath: "/test/storage" }, }, - getState: vi.fn().mockResolvedValue({ - apiConfiguration: mockApiConfig, - }), + getState: vi.fn().mockResolvedValue({}), say: vi.fn(), postStateToWebview: vi.fn().mockResolvedValue(undefined), postMessageToWebview: vi.fn().mockResolvedValue(undefined), updateTaskHistory: vi.fn().mockResolvedValue(undefined), + providerSettingsManager: { + getCurrentProviderSettings: vi.fn().mockResolvedValue(mockApiConfig), + }, } // Get the mocked delay function @@ -996,11 +989,13 @@ describe("Cline", () => { // Create parent task const parent = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "parent task", startTask: false, }) + // Wait for API configuration to be initialized + await parent.getApiConfiguration() + // Mock the API stream response const mockStream = { async *[Symbol.asyncIterator]() { @@ -1030,13 +1025,15 @@ describe("Cline", () => { // Create a subtask immediately after const child = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "child task", parentTask: parent, rootTask: parent, startTask: false, }) + // Wait for API configuration to be initialized + await child.getApiConfiguration() + // Mock the child's API stream const childMockStream = { async *[Symbol.asyncIterator]() { @@ -1069,11 +1066,13 @@ describe("Cline", () => { // Create parent task const parent = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "parent task", startTask: false, }) + // Wait for API configuration to be initialized + await parent.getApiConfiguration() + // Mock the API stream response const mockStream = { async *[Symbol.asyncIterator]() { @@ -1105,13 +1104,15 @@ describe("Cline", () => { // Create a subtask after time has passed const child = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "child task", parentTask: parent, rootTask: parent, startTask: false, }) + // Wait for API configuration to be initialized + await child.getApiConfiguration() + vi.spyOn(child.api, "createMessage").mockReturnValue(mockStream) // Make an API request with the child task @@ -1129,11 +1130,13 @@ describe("Cline", () => { // Create parent task const parent = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "parent task", startTask: false, }) + // Wait for API configuration to be initialized + await parent.getApiConfiguration() + // Mock the API stream response const mockStream = { async *[Symbol.asyncIterator]() { @@ -1160,13 +1163,15 @@ describe("Cline", () => { // Create first subtask const child1 = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "child task 1", parentTask: parent, rootTask: parent, startTask: false, }) + // Wait for API configuration to be initialized + await child1.getApiConfiguration() + vi.spyOn(child1.api, "createMessage").mockReturnValue(mockStream) // Make an API request with the first child task @@ -1183,13 +1188,15 @@ describe("Cline", () => { // Create second subtask immediately after const child2 = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "child task 2", parentTask: parent, rootTask: parent, startTask: false, }) + // Wait for API configuration to be initialized + await child2.getApiConfiguration() + vi.spyOn(child2.api, "createMessage").mockReturnValue(mockStream) // Make an API request with the second child task @@ -1203,18 +1210,18 @@ describe("Cline", () => { it("should handle rate limiting with zero rate limit", async () => { // Update config to have zero rate limit mockApiConfig.rateLimitSeconds = 0 - mockProvider.getState.mockResolvedValue({ - apiConfiguration: mockApiConfig, - }) + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(mockApiConfig) // Create parent task const parent = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "parent task", startTask: false, }) + // Wait for API configuration to be initialized + await parent.getApiConfiguration() + // Mock the API stream response const mockStream = { async *[Symbol.asyncIterator]() { @@ -1241,13 +1248,15 @@ describe("Cline", () => { // Create a subtask const child = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "child task", parentTask: parent, rootTask: parent, startTask: false, }) + // Wait for API configuration to be initialized + await child.getApiConfiguration() + vi.spyOn(child.api, "createMessage").mockReturnValue(mockStream) // Make an API request with the child task @@ -1262,11 +1271,13 @@ describe("Cline", () => { // Create task const task = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, task: "test task", startTask: false, }) + // Wait for API configuration to be initialized + await task.getApiConfiguration() + // Mock the API stream response const mockStream = { async *[Symbol.asyncIterator]() { @@ -1314,6 +1325,9 @@ describe("Cline", () => { globalStorageUri: { fsPath: "/test/storage" }, }, getState: vi.fn(), + providerSettingsManager: { + getCurrentProviderSettings: vi.fn().mockResolvedValue(mockApiConfig), + }, } }) @@ -1326,7 +1340,6 @@ describe("Cline", () => { const task = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, enableDiff: true, task: "test task", startTask: false, @@ -1346,7 +1359,6 @@ describe("Cline", () => { const task = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, enableDiff: true, task: "test task", startTask: false, @@ -1368,7 +1380,6 @@ describe("Cline", () => { const task = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, enableDiff: true, task: "test task", startTask: false, @@ -1388,7 +1399,6 @@ describe("Cline", () => { it("should not create diff strategy when enableDiff is false", async () => { const task = new Task({ provider: mockProvider, - apiConfiguration: mockApiConfig, enableDiff: false, task: "test task", startTask: false, @@ -1407,40 +1417,45 @@ describe("Cline", () => { apiProvider: "anthropic" as const, apiModelId: "gpt-4", } + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(anthropicConfig) const anthropicTask = new Task({ provider: mockProvider, - apiConfiguration: anthropicConfig, task: "test task", startTask: false, }) // Should use anthropic protocol even with non-claude model - expect(anthropicTask.apiConfiguration.apiProvider).toBe("anthropic") + const apiConfig = await anthropicTask.getApiConfiguration() + expect(apiConfig.apiProvider).toBe("anthropic") // Test with OpenRouter provider and Claude model const openrouterClaudeConfig = { apiProvider: "openrouter" as const, openRouterModelId: "anthropic/claude-3-opus", } + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue( + openrouterClaudeConfig, + ) const openrouterClaudeTask = new Task({ provider: mockProvider, - apiConfiguration: openrouterClaudeConfig, task: "test task", startTask: false, }) - expect(openrouterClaudeTask.apiConfiguration.apiProvider).toBe("openrouter") + const openrouterApiConfig = await openrouterClaudeTask.getApiConfiguration() + expect(openrouterApiConfig.apiProvider).toBe("openrouter") // Test with OpenRouter provider and non-Claude model const openrouterGptConfig = { apiProvider: "openrouter" as const, openRouterModelId: "openai/gpt-4", } + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(openrouterGptConfig) const openrouterGptTask = new Task({ provider: mockProvider, - apiConfiguration: openrouterGptConfig, task: "test task", startTask: false, }) - expect(openrouterGptTask.apiConfiguration.apiProvider).toBe("openrouter") + const gptApiConfig = await openrouterGptTask.getApiConfiguration() + expect(gptApiConfig.apiProvider).toBe("openrouter") // Test with various Claude model formats const claudeModelFormats = [ @@ -1456,9 +1471,9 @@ describe("Cline", () => { apiProvider: "openai" as const, openAiModelId: modelId, } + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(config) const task = new Task({ provider: mockProvider, - apiConfiguration: config, task: "test task", startTask: false, }) @@ -1472,25 +1487,29 @@ describe("Cline", () => { const undefinedProviderConfig = { apiModelId: "claude-3-opus", } + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue( + undefinedProviderConfig, + ) const undefinedProviderTask = new Task({ provider: mockProvider, - apiConfiguration: undefinedProviderConfig, task: "test task", startTask: false, }) - expect(undefinedProviderTask.apiConfiguration.apiProvider).toBeUndefined() + const undefinedApiConfig = await undefinedProviderTask.getApiConfiguration() + expect(undefinedApiConfig.apiProvider).toBeUndefined() // Test with no model ID const noModelConfig = { apiProvider: "openai" as const, } + mockProvider.providerSettingsManager.getCurrentProviderSettings.mockResolvedValue(noModelConfig) const noModelTask = new Task({ provider: mockProvider, - apiConfiguration: noModelConfig, task: "test task", startTask: false, }) - expect(noModelTask.apiConfiguration.apiProvider).toBe("openai") + const noModelApiConfig = await noModelTask.getApiConfiguration() + expect(noModelApiConfig.apiProvider).toBe("openai") }) }) }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 280ab61a06..c75ef1fe6c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -537,7 +537,6 @@ export class ClineProvider > = {}, ) { const { - apiConfiguration, organizationAllowList, diffEnabled: enableDiff, enableCheckpoints, @@ -545,13 +544,15 @@ export class ClineProvider experiments, } = await this.getState() + // Get API configuration from ProviderSettingsManager to check organization allowlist + const apiConfiguration = await this.providerSettingsManager.getCurrentProviderSettings() + if (!ProfileValidator.isProfileAllowed(apiConfiguration, organizationAllowList)) { throw new OrganizationAllowListViolationError(t("common:errors.violated_organization_allowlist")) } const cline = new Task({ provider: this, - apiConfiguration, enableDiff, enableCheckpoints, fuzzyMatchThreshold, @@ -621,17 +622,13 @@ export class ClineProvider } } - const { - apiConfiguration, - diffEnabled: enableDiff, - enableCheckpoints, - fuzzyMatchThreshold, - experiments, - } = await this.getState() + const { diffEnabled: enableDiff, enableCheckpoints, fuzzyMatchThreshold, experiments } = await this.getState() + + // Get API configuration from ProviderSettingsManager for consecutiveMistakeLimit + const apiConfiguration = await this.providerSettingsManager.getCurrentProviderSettings() const cline = new Task({ provider: this, - apiConfiguration, enableDiff, enableCheckpoints, fuzzyMatchThreshold, @@ -953,7 +950,7 @@ export class ClineProvider this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()), this.updateGlobalState("currentApiConfigName", name), this.providerSettingsManager.setModeConfig(mode, id), - this.contextProxy.setProviderSettings(providerSettings), + this.providerSettingsManager.updateCurrentProviderSettings(providerSettings), ]) // Change the provider for the current task. @@ -1009,7 +1006,7 @@ export class ClineProvider await Promise.all([ this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()), this.contextProxy.setValue("currentApiConfigName", name), - this.contextProxy.setProviderSettings(providerSettings), + this.providerSettingsManager.updateCurrentProviderSettings(providerSettings), ]) const { mode } = await this.getState() @@ -1113,7 +1110,8 @@ export class ClineProvider // OpenRouter async handleOpenRouterCallback(code: string) { - let { apiConfiguration, currentApiConfigName } = await this.getState() + const { currentApiConfigName } = await this.getState() + const apiConfiguration = await this.providerSettingsManager.getCurrentProviderSettings() let apiKey: string try { @@ -1161,7 +1159,8 @@ export class ClineProvider throw error } - const { apiConfiguration, currentApiConfigName } = await this.getState() + const { currentApiConfigName } = await this.getState() + const apiConfiguration = await this.providerSettingsManager.getCurrentProviderSettings() const newConfiguration: ProviderSettings = { ...apiConfiguration, @@ -1176,7 +1175,8 @@ export class ClineProvider // Requesty async handleRequestyCallback(code: string) { - let { apiConfiguration, currentApiConfigName } = await this.getState() + const { currentApiConfigName } = await this.getState() + const apiConfiguration = await this.providerSettingsManager.getCurrentProviderSettings() const newConfiguration: ProviderSettings = { ...apiConfiguration, @@ -1433,7 +1433,6 @@ export class ClineProvider async getStateToPostToWebview() { const { - apiConfiguration, lastShownAnnouncementId, customInstructions, alwaysAllowReadOnly, @@ -1527,9 +1526,12 @@ export class ClineProvider const currentMode = mode ?? defaultModeSlug const hasSystemPromptOverride = await this.hasFileBasedSystemPromptOverride(currentMode) + // Get current provider settings from ProviderSettingsManager + const currentProviderSettings = await this.providerSettingsManager.getCurrentProviderSettings() + return { version: this.context.extension?.packageJSON?.version ?? "", - apiConfiguration, + apiConfiguration: currentProviderSettings, customInstructions, alwaysAllowReadOnly: alwaysAllowReadOnly ?? false, alwaysAllowReadOnlyOutsideWorkspace: alwaysAllowReadOnlyOutsideWorkspace ?? false, @@ -1652,15 +1654,12 @@ export class ClineProvider const stateValues = this.contextProxy.getValues() const customModes = await this.customModesManager.getCustomModes() - // Determine apiProvider with the same logic as before. - const apiProvider: ProviderName = stateValues.apiProvider ? stateValues.apiProvider : "anthropic" + // Get provider settings from ProviderSettingsManager + const providerSettings = await this.providerSettingsManager.getCurrentProviderSettings() - // Build the apiConfiguration object combining state values and secrets. - const providerSettings = this.contextProxy.getProviderSettings() - - // Ensure apiProvider is set properly if not already in state + // Ensure apiProvider is set properly if not already in settings if (!providerSettings.apiProvider) { - providerSettings.apiProvider = apiProvider + providerSettings.apiProvider = stateValues.apiProvider || "anthropic" } let organizationAllowList = ORGANIZATION_ALLOW_ALL @@ -1927,7 +1926,8 @@ export class ClineProvider * like the current mode, API provider, git repository information, etc. */ public async getTelemetryProperties(): Promise { - const { mode, apiConfiguration, language } = await this.getState() + const { mode, language } = await this.getState() + const apiConfiguration = await this.providerSettingsManager.getCurrentProviderSettings() const task = this.getCurrentCline() const packageJSON = this.context.extension?.packageJSON diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 2e70f80f99..61610e82eb 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -46,6 +46,32 @@ vi.mock("axios", () => ({ vi.mock("../../../utils/safeWriteJson") +vi.mock("../../config/ProviderSettingsManager", () => ({ + ProviderSettingsManager: vi.fn().mockImplementation(() => ({ + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "openrouter", + openRouterApiKey: "test-key", + openRouterModelId: "test-model", + }), + getModeProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "openrouter", + openRouterApiKey: "test-key", + openRouterModelId: "test-model", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), + getModeConfigId: vi.fn().mockResolvedValue(undefined), + listConfig: vi.fn().mockResolvedValue([]), + activateProfile: vi.fn().mockResolvedValue({ + name: "test-config", + id: "test-id", + apiProvider: "anthropic", + }), + setModeConfig: vi.fn().mockResolvedValue(undefined), + saveConfig: vi.fn().mockResolvedValue("test-id"), + resetAllConfigs: vi.fn().mockResolvedValue(undefined), + })), +})) + vi.mock("@modelcontextprotocol/sdk/types.js", () => ({ CallToolResultSchema: {}, ListResourcesResultSchema: {}, @@ -198,24 +224,22 @@ vi.mock("../../../integrations/workspace/WorkspaceTracker", () => { }) vi.mock("../../task/Task", () => ({ - Task: vi - .fn() - .mockImplementation( - (_provider, _apiConfiguration, _customInstructions, _diffEnabled, _fuzzyMatchThreshold, _task, taskId) => ({ - api: undefined, - abortTask: vi.fn(), - handleWebviewAskResponse: vi.fn(), - clineMessages: [], - apiConversationHistory: [], - overwriteClineMessages: vi.fn(), - overwriteApiConversationHistory: vi.fn(), - getTaskNumber: vi.fn().mockReturnValue(0), - setTaskNumber: vi.fn(), - setParentTask: vi.fn(), - setRootTask: vi.fn(), - taskId: taskId || "test-task-id", - }), - ), + Task: vi.fn().mockImplementation((options) => ({ + api: undefined, + abortTask: vi.fn(), + handleWebviewAskResponse: vi.fn(), + clineMessages: [], + apiConversationHistory: [], + overwriteClineMessages: vi.fn(), + overwriteApiConversationHistory: vi.fn(), + getTaskNumber: vi.fn().mockReturnValue(0), + setTaskNumber: vi.fn(), + setParentTask: vi.fn(), + setRootTask: vi.fn(), + taskId: options?.taskId || "test-task-id", + initializeApiConfiguration: vi.fn().mockResolvedValue(undefined), + getApiConfiguration: vi.fn().mockReturnValue({ apiProvider: "openrouter" }), + })), })) vi.mock("../../../integrations/misc/extract-text", () => ({ @@ -414,9 +438,6 @@ describe("ClineProvider", () => { defaultTaskOptions = { provider, - apiConfiguration: { - apiProvider: "openrouter", - }, } // @ts-ignore - Access private property for testing @@ -876,6 +897,11 @@ describe("ClineProvider", () => { listConfig: vi.fn().mockResolvedValue([profile]), activateProfile: vi.fn().mockResolvedValue(profile), setModeConfig: vi.fn(), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // Switch to architect mode @@ -897,6 +923,11 @@ describe("ClineProvider", () => { .fn() .mockResolvedValue([{ name: "current-config", id: "current-id", apiProvider: "anthropic" }]), setModeConfig: vi.fn(), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any provider.setValue("currentApiConfigName", "current-config") @@ -919,6 +950,11 @@ describe("ClineProvider", () => { listConfig: vi.fn().mockResolvedValue([profile]), setModeConfig: vi.fn(), getModeConfigId: vi.fn().mockResolvedValue(undefined), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // First set the mode @@ -946,6 +982,11 @@ describe("ClineProvider", () => { listConfig: vi.fn().mockResolvedValue([profile]), setModeConfig: vi.fn(), getModeConfigId: vi.fn().mockResolvedValue(undefined), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // First set the mode @@ -1146,6 +1187,11 @@ describe("ClineProvider", () => { listConfig: vi.fn().mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), saveConfig: vi.fn().mockResolvedValue("test-id"), setModeConfig: vi.fn(), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // Update API configuration @@ -1608,6 +1654,11 @@ describe("ClineProvider", () => { listConfig: vi.fn().mockResolvedValue([profile]), activateProfile: vi.fn().mockResolvedValue(profile), setModeConfig: vi.fn(), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // Switch to architect mode @@ -1632,6 +1683,11 @@ describe("ClineProvider", () => { .fn() .mockResolvedValue([{ name: "current-config", id: "current-id", apiProvider: "anthropic" }]), setModeConfig: vi.fn(), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // Mock the ContextProxy's getValue method to return the current config name @@ -1689,6 +1745,11 @@ describe("ClineProvider", () => { ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } // Spy on log method to verify warning was logged @@ -1758,6 +1819,11 @@ describe("ClineProvider", () => { activateProfile: vi .fn() .mockResolvedValue({ name: "test-config", id: "config-id", apiProvider: "anthropic" }), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } // Spy on log method to verify no warning was logged @@ -1813,6 +1879,11 @@ describe("ClineProvider", () => { ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } // Create history item with built-in mode @@ -1844,6 +1915,11 @@ describe("ClineProvider", () => { ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } // Create history item without mode @@ -1891,6 +1967,11 @@ describe("ClineProvider", () => { .fn() .mockResolvedValue([{ name: "test-config", id: "config-id", apiProvider: "anthropic" }]), activateProfile: vi.fn().mockRejectedValue(new Error("Failed to load config")), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } // Spy on log method @@ -1990,6 +2071,11 @@ describe("ClineProvider", () => { listConfig: vi .fn() .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // Mock getState to provide necessary data @@ -2022,6 +2108,11 @@ describe("ClineProvider", () => { listConfig: vi .fn() .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any const testApiConfig = { @@ -2065,6 +2156,11 @@ describe("ClineProvider", () => { listConfig: vi .fn() .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any // Setup Task instance with auto-mock from the top of the file @@ -2106,6 +2202,11 @@ describe("ClineProvider", () => { listConfig: vi .fn() .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "anthropic", + anthropicApiKey: "test-key", + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), } as any const testApiConfig = { @@ -2434,9 +2535,6 @@ describe("getTelemetryProperties", () => { defaultTaskOptions = { provider, - apiConfiguration: { - apiProvider: "openrouter", - }, } // Setup Task instance with mocked getModel method @@ -2628,17 +2726,29 @@ describe("ClineProvider - Router Models", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - // Mock getState to return API configuration - vi.spyOn(provider, "getState").mockResolvedValue({ - apiConfiguration: { + // Mock providerSettingsManager to return API configuration + ;(provider as any).providerSettingsManager = { + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "openrouter", openRouterApiKey: "openrouter-key", requestyApiKey: "requesty-key", glamaApiKey: "glama-key", unboundApiKey: "unbound-key", litellmApiKey: "litellm-key", litellmBaseUrl: "http://localhost:4000", - }, - } as any) + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), + getModeConfigId: vi.fn().mockResolvedValue(undefined), + listConfig: vi.fn().mockResolvedValue([]), + activateProfile: vi.fn().mockResolvedValue({ + name: "test-config", + id: "test-id", + apiProvider: "anthropic", + }), + setModeConfig: vi.fn().mockResolvedValue(undefined), + saveConfig: vi.fn().mockResolvedValue("test-id"), + resetAllConfigs: vi.fn().mockResolvedValue(undefined), + } const mockModels = { "model-1": { @@ -2690,16 +2800,29 @@ describe("ClineProvider - Router Models", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - vi.spyOn(provider, "getState").mockResolvedValue({ - apiConfiguration: { + // Mock providerSettingsManager to return API configuration + ;(provider as any).providerSettingsManager = { + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "openrouter", openRouterApiKey: "openrouter-key", requestyApiKey: "requesty-key", glamaApiKey: "glama-key", unboundApiKey: "unbound-key", litellmApiKey: "litellm-key", litellmBaseUrl: "http://localhost:4000", - }, - } as any) + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), + getModeConfigId: vi.fn().mockResolvedValue(undefined), + listConfig: vi.fn().mockResolvedValue([]), + activateProfile: vi.fn().mockResolvedValue({ + name: "test-config", + id: "test-id", + apiProvider: "anthropic", + }), + setModeConfig: vi.fn().mockResolvedValue(undefined), + saveConfig: vi.fn().mockResolvedValue("test-id"), + resetAllConfigs: vi.fn().mockResolvedValue(undefined), + } const mockModels = { "model-1": { maxTokens: 4096, contextWindow: 8192, description: "Test model", supportsPromptCache: false }, @@ -2764,16 +2887,28 @@ describe("ClineProvider - Router Models", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - // Mock state without LiteLLM config - vi.spyOn(provider, "getState").mockResolvedValue({ - apiConfiguration: { + // Mock providerSettingsManager without LiteLLM config + ;(provider as any).providerSettingsManager = { + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "openrouter", openRouterApiKey: "openrouter-key", requestyApiKey: "requesty-key", glamaApiKey: "glama-key", unboundApiKey: "unbound-key", // No litellm config - }, - } as any) + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), + getModeConfigId: vi.fn().mockResolvedValue(undefined), + listConfig: vi.fn().mockResolvedValue([]), + activateProfile: vi.fn().mockResolvedValue({ + name: "test-config", + id: "test-id", + apiProvider: "anthropic", + }), + setModeConfig: vi.fn().mockResolvedValue(undefined), + saveConfig: vi.fn().mockResolvedValue("test-id"), + resetAllConfigs: vi.fn().mockResolvedValue(undefined), + } const mockModels = { "model-1": { maxTokens: 4096, contextWindow: 8192, description: "Test model", supportsPromptCache: false }, @@ -2801,15 +2936,28 @@ describe("ClineProvider - Router Models", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - vi.spyOn(provider, "getState").mockResolvedValue({ - apiConfiguration: { + // Mock providerSettingsManager without LiteLLM config + ;(provider as any).providerSettingsManager = { + getCurrentProviderSettings: vi.fn().mockResolvedValue({ + apiProvider: "openrouter", openRouterApiKey: "openrouter-key", requestyApiKey: "requesty-key", glamaApiKey: "glama-key", unboundApiKey: "unbound-key", // No litellm config - }, - } as any) + }), + updateCurrentProviderSettings: vi.fn().mockResolvedValue(undefined), + getModeConfigId: vi.fn().mockResolvedValue(undefined), + listConfig: vi.fn().mockResolvedValue([]), + activateProfile: vi.fn().mockResolvedValue({ + name: "test-config", + id: "test-id", + apiProvider: "anthropic", + }), + setModeConfig: vi.fn().mockResolvedValue(undefined), + saveConfig: vi.fn().mockResolvedValue("test-id"), + resetAllConfigs: vi.fn().mockResolvedValue(undefined), + } const mockModels = { "model-1": { maxTokens: 4096, contextWindow: 8192, description: "Test model", supportsPromptCache: false }, @@ -2916,9 +3064,6 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { defaultTaskOptions = { provider, - apiConfiguration: { - apiProvider: "openrouter", - }, } // Mock getMcpHub method diff --git a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts index 6b19b47a38..4b694299f6 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts @@ -265,7 +265,6 @@ describe("ClineProvider - Sticky Mode", () => { // Create a mock task const mockTask = new Task({ provider, - apiConfiguration: { apiProvider: "openrouter" }, }) // Get the actual taskId from the mock @@ -355,7 +354,6 @@ describe("ClineProvider - Sticky Mode", () => { // Create a mock task with history const mockTask = new Task({ provider, - apiConfiguration: { apiProvider: "openrouter" }, }) // Get the actual taskId from the mock @@ -478,7 +476,6 @@ describe("ClineProvider - Sticky Mode", () => { // Create a mock task const mockTask = new Task({ provider, - apiConfiguration: { apiProvider: "openrouter" }, }) // Get the actual taskId from the mock @@ -531,7 +528,6 @@ describe("ClineProvider - Sticky Mode", () => { // Create parent task const parentTask = new Task({ provider, - apiConfiguration: { apiProvider: "openrouter" }, }) // Get the actual taskId from the mock @@ -580,7 +576,6 @@ describe("ClineProvider - Sticky Mode", () => { // Create a subtask (simulating new_task tool behavior) const subtask = new Task({ provider, - apiConfiguration: { apiProvider: "openrouter" }, parentTask: parentTask, }) const subtaskId = (subtask as any).taskId || "subtask-id" @@ -616,7 +611,6 @@ describe("ClineProvider - Sticky Mode", () => { // Create a mock task that throws on save const mockTask = new Task({ provider, - apiConfiguration: { apiProvider: "openrouter" }, }) vi.spyOn(mockTask as any, "saveClineMessages").mockRejectedValue(new Error("Save failed")) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 284ee98944..d404bb279a 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -30,6 +30,10 @@ const mockClineProvider = { setValue: vi.fn(), getValue: vi.fn(), }, + providerSettingsManager: { + getCurrentProviderSettings: vi.fn(), + updateCurrentProviderSettings: vi.fn(), + }, log: vi.fn(), postStateToWebview: vi.fn(), getCurrentCline: vi.fn(), @@ -107,6 +111,15 @@ describe("webviewMessageHandler - requestRouterModels", () => { litellmBaseUrl: "http://localhost:4000", }, }) + // Mock providerSettingsManager to return the same API configuration + vi.mocked(mockClineProvider.providerSettingsManager.getCurrentProviderSettings).mockResolvedValue({ + openRouterApiKey: "openrouter-key", + requestyApiKey: "requesty-key", + glamaApiKey: "glama-key", + unboundApiKey: "unbound-key", + litellmApiKey: "litellm-key", + litellmBaseUrl: "http://localhost:4000", + } as any) }) it("successfully fetches models from all providers", async () => { @@ -167,6 +180,14 @@ describe("webviewMessageHandler - requestRouterModels", () => { // Missing litellm config }, }) + // Mock providerSettingsManager to return the same configuration without litellm + vi.mocked(mockClineProvider.providerSettingsManager.getCurrentProviderSettings).mockResolvedValue({ + openRouterApiKey: "openrouter-key", + requestyApiKey: "requesty-key", + glamaApiKey: "glama-key", + unboundApiKey: "unbound-key", + // Missing litellm config + } as any) const mockModels: ModelRecord = { "model-1": { @@ -205,6 +226,14 @@ describe("webviewMessageHandler - requestRouterModels", () => { // Missing litellm config }, }) + // Mock providerSettingsManager to return the same configuration without litellm + vi.mocked(mockClineProvider.providerSettingsManager.getCurrentProviderSettings).mockResolvedValue({ + openRouterApiKey: "openrouter-key", + requestyApiKey: "requesty-key", + glamaApiKey: "glama-key", + unboundApiKey: "unbound-key", + // Missing litellm config + } as any) const mockModels: ModelRecord = { "model-1": { diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 763e118125..cb76b43698 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -237,7 +237,7 @@ export const webviewMessageHandler = async ( if (listApiConfig.length === 1) { // Check if first time init then sync with exist config. if (!checkExistKey(listApiConfig[0])) { - const { apiConfiguration } = await provider.getState() + const apiConfiguration = await provider.providerSettingsManager.getCurrentProviderSettings() await provider.providerSettingsManager.saveConfig( listApiConfig[0].name ?? "default", @@ -512,7 +512,7 @@ export const webviewMessageHandler = async ( await flushModels(routerNameFlush) break case "requestRouterModels": - const { apiConfiguration } = await provider.getState() + const apiConfiguration = await provider.providerSettingsManager.getCurrentProviderSettings() const routerModels: Partial> = { openrouter: {}, @@ -611,7 +611,7 @@ export const webviewMessageHandler = async ( break case "requestOllamaModels": { // Specific handler for Ollama models only - const { apiConfiguration: ollamaApiConfig } = await provider.getState() + const ollamaApiConfig = await provider.providerSettingsManager.getCurrentProviderSettings() try { // Flush cache first to ensure fresh models await flushModels("ollama") @@ -635,7 +635,7 @@ export const webviewMessageHandler = async ( } case "requestLmStudioModels": { // Specific handler for LM Studio models only - const { apiConfiguration: lmStudioApiConfig } = await provider.getState() + const lmStudioApiConfig = await provider.providerSettingsManager.getCurrentProviderSettings() try { // Flush cache first to ensure fresh models await flushModels("lmstudio") @@ -1335,11 +1335,11 @@ export const webviewMessageHandler = async ( case "enhancePrompt": if (message.text) { try { - const { apiConfiguration, customSupportPrompts, listApiConfigMeta, enhancementApiConfigId } = + const { customSupportPrompts, listApiConfigMeta, enhancementApiConfigId } = await provider.getState() // Try to get enhancement config first, fall back to current config. - let configToUse: ProviderSettings = apiConfiguration + let configToUse: ProviderSettings if (enhancementApiConfigId && !!listApiConfigMeta.find(({ id }) => id === enhancementApiConfigId)) { const { name: _, ...providerSettings } = await provider.providerSettingsManager.getProfile({ @@ -1348,7 +1348,11 @@ export const webviewMessageHandler = async ( if (providerSettings.apiProvider) { configToUse = providerSettings + } else { + configToUse = await provider.providerSettingsManager.getCurrentProviderSettings() } + } else { + configToUse = await provider.providerSettingsManager.getCurrentProviderSettings() } const enhancedPrompt = await singleCompletionHandler( diff --git a/src/utils/__tests__/autoImportSettings.spec.ts b/src/utils/__tests__/autoImportSettings.spec.ts index 2b9b42293f..458d0cdac5 100644 --- a/src/utils/__tests__/autoImportSettings.spec.ts +++ b/src/utils/__tests__/autoImportSettings.spec.ts @@ -91,7 +91,6 @@ describe("autoImportSettings", () => { mockContextProxy = { setValues: vi.fn().mockResolvedValue(undefined), setValue: vi.fn().mockResolvedValue(undefined), - setProviderSettings: vi.fn().mockResolvedValue(undefined), } // Mock custom modes manager