diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 609565e33d..b9167c1cbf 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -9,7 +9,7 @@ import axios from "axios" import pWaitFor from "p-wait-for" import * as vscode from "vscode" -import type { GlobalState, ProviderName, ProviderSettings, RooCodeSettings } from "../../schemas" +import type { GlobalState, ProviderName, ProviderSettings, RooCodeSettings, ProviderSettingsEntry } from "../../schemas" import { t } from "../../i18n" import { setPanel } from "../../activate/registerCommands" import { requestyDefaultModelId, openRouterDefaultModelId, glamaDefaultModelId } from "../../shared/api" @@ -246,7 +246,7 @@ export class ClineProvider extends EventEmitter implements return false } - // check if there is a cline instance in the stack (if this provider has an active task) + // Check if there is a cline instance in the stack (if this provider has an active task) if (visibleProvider.getCurrentCline()) { return true } @@ -795,39 +795,127 @@ export class ClineProvider extends EventEmitter implements await this.postStateToWebview() } - async activateProviderProfile(args: { name: string } | { id: string }) { - const { name, ...providerSettings } = await this.providerSettingsManager.activateProfile(args) + // Provider Profile Management - await Promise.all([ - this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()), - this.contextProxy.setValue("currentApiConfigName", name), - ]) + getProviderProfileEntries(): ProviderSettingsEntry[] { + return this.contextProxy.getValues().listApiConfigMeta || [] + } + + getProviderProfileEntry(name: string): ProviderSettingsEntry | undefined { + return this.getProviderProfileEntries().find((profile) => profile.name === name) + } + + public hasProviderProfileEntry(name: string): boolean { + return !!this.getProviderProfileEntry(name) + } + + async upsertProviderProfile( + name: string, + providerSettings: ProviderSettings, + activate: boolean = true, + ): Promise { + try { + // TODO: Do we need to be calling `activateProfile`? It's not + // clear to me what the source of truth should be; in some cases + // we rely on the `ContextProxy`'s data store and in other cases + // we rely on the `ProviderSettingsManager`'s data store. It might + // be simpler to unify these two. + const id = await this.providerSettingsManager.saveConfig(name, providerSettings) + + if (activate) { + const { mode } = await this.getState() + + // These promises do the following: + // 1. Adds or updates the list of provider profiles. + // 2. Sets the current provider profile. + // 3. Sets the current mode's provider profile. + // 4. Copies the provider settings to the context. + // + // Note: 1, 2, and 4 can be done in one `ContextProxy` call: + // this.contextProxy.setValues({ ...providerSettings, listApiConfigMeta: ..., currentApiConfigName: ... }) + // We should probably switch to that and verify that it works. + // I left the original implementation in just to be safe. + await Promise.all([ + this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()), + this.updateGlobalState("currentApiConfigName", name), + this.providerSettingsManager.setModeConfig(mode, id), + this.contextProxy.setProviderSettings(providerSettings), + ]) + + // Change the provider for the current task. + // TODO: We should rename `buildApiHandler` for clarity (e.g. `getProviderClient`). + const task = this.getCurrentCline() + + if (task) { + task.api = buildApiHandler(providerSettings) + } + } else { + await this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()) + } + + await this.postStateToWebview() + return id + } catch (error) { + this.log( + `Error create new api configuration: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`, + ) + + vscode.window.showErrorMessage(t("common:errors.create_api_config")) + return undefined + } + } + + async deleteProviderProfile(profileToDelete: ProviderSettingsEntry) { + const globalSettings = this.contextProxy.getValues() + let profileToActivate: string | undefined = globalSettings.currentApiConfigName + + if (profileToDelete.name === profileToActivate) { + profileToActivate = this.getProviderProfileEntries().find(({ name }) => name !== profileToDelete.name)?.name + } + + if (!profileToActivate) { + throw new Error("You cannot delete the last profile") + } + + const entries = this.getProviderProfileEntries().filter(({ name }) => name !== profileToDelete.name) + + await this.contextProxy.setValues({ + ...globalSettings, + currentApiConfigName: profileToActivate, + listApiConfigMeta: entries, + }) - await this.updateApiConfiguration(providerSettings) await this.postStateToWebview() } - async updateApiConfiguration(providerSettings: ProviderSettings) { - // Update mode's default config. + async activateProviderProfile(args: { name: string } | { id: string }) { + const { name, id, ...providerSettings } = await this.providerSettingsManager.activateProfile(args) + + // See `upsertProviderProfile` for a description of what this is doing. + await Promise.all([ + this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()), + this.contextProxy.setValue("currentApiConfigName", name), + this.contextProxy.setProviderSettings(providerSettings), + ]) + const { mode } = await this.getState() - if (mode) { - const currentApiConfigName = this.getGlobalState("currentApiConfigName") - const listApiConfig = await this.providerSettingsManager.listConfig() - const config = listApiConfig?.find((c) => c.name === currentApiConfigName) - - if (config?.id) { - await this.providerSettingsManager.setModeConfig(mode, config.id) - } + if (id) { + await this.providerSettingsManager.setModeConfig(mode, id) } - await this.contextProxy.setProviderSettings(providerSettings) + // Change the provider for the current task. + const task = this.getCurrentCline() - if (this.getCurrentCline()) { - this.getCurrentCline()!.api = buildApiHandler(providerSettings) + if (task) { + task.api = buildApiHandler(providerSettings) } + + await this.postStateToWebview() } + // Task Management + async cancelTask() { const cline = this.getCurrentCline() @@ -943,7 +1031,7 @@ export class ClineProvider extends EventEmitter implements openRouterModelId: apiConfiguration?.openRouterModelId || openRouterDefaultModelId, } - await this.upsertApiConfiguration(currentApiConfigName, newConfiguration) + await this.upsertProviderProfile(currentApiConfigName, newConfiguration) } // Glama @@ -973,7 +1061,7 @@ export class ClineProvider extends EventEmitter implements glamaModelId: apiConfiguration?.glamaModelId || glamaDefaultModelId, } - await this.upsertApiConfiguration(currentApiConfigName, newConfiguration) + await this.upsertProviderProfile(currentApiConfigName, newConfiguration) } // Requesty @@ -988,29 +1076,7 @@ export class ClineProvider extends EventEmitter implements requestyModelId: apiConfiguration?.requestyModelId || requestyDefaultModelId, } - await this.upsertApiConfiguration(currentApiConfigName, newConfiguration) - } - - // Save configuration - - async upsertApiConfiguration(configName: string, apiConfiguration: ProviderSettings) { - try { - await this.providerSettingsManager.saveConfig(configName, apiConfiguration) - const listApiConfig = await this.providerSettingsManager.listConfig() - - await Promise.all([ - this.updateGlobalState("listApiConfigMeta", listApiConfig), - this.updateApiConfiguration(apiConfiguration), - this.updateGlobalState("currentApiConfigName", configName), - ]) - - await this.postStateToWebview() - } catch (error) { - this.log( - `Error create new api configuration: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`, - ) - vscode.window.showErrorMessage(t("common:errors.create_api_config")) - } + await this.upsertProviderProfile(currentApiConfigName, newConfiguration) } // Task history diff --git a/src/core/webview/__tests__/ClineProvider.test.ts b/src/core/webview/__tests__/ClineProvider.test.ts index 438f256ef0..3bd3a11db4 100644 --- a/src/core/webview/__tests__/ClineProvider.test.ts +++ b/src/core/webview/__tests__/ClineProvider.test.ts @@ -612,7 +612,7 @@ describe("ClineProvider", () => { expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "test-config") }) - test("saves current config when switching to mode without config", async () => { + it("saves current config when switching to mode without config", async () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0] @@ -633,15 +633,15 @@ describe("ClineProvider", () => { expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("architect", "current-id") }) - test("saves config as default for current mode when loading config", async () => { + it("saves config as default for current mode when loading config", async () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0] + const profile: ProviderSettingsEntry = { apiProvider: "anthropic", id: "new-id", name: "new-config" } + ;(provider as any).providerSettingsManager = { - activateProfile: jest - .fn() - .mockResolvedValue({ config: { apiProvider: "anthropic", id: "new-id" }, name: "new-config" }), - listConfig: jest.fn().mockResolvedValue([{ name: "new-config", id: "new-id", apiProvider: "anthropic" }]), + activateProfile: jest.fn().mockResolvedValue(profile), + listConfig: jest.fn().mockResolvedValue([profile]), setModeConfig: jest.fn(), getModeConfigId: jest.fn().mockResolvedValue(undefined), } as any @@ -656,18 +656,19 @@ describe("ClineProvider", () => { expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("architect", "new-id") }) - test("load API configuration by ID works and updates mode config", async () => { + it("load API configuration by ID works and updates mode config", async () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0] + const profile: ProviderSettingsEntry = { + name: "config-by-id", + id: "config-id-123", + apiProvider: "anthropic", + } + ;(provider as any).providerSettingsManager = { - activateProfile: jest.fn().mockResolvedValue({ - config: { apiProvider: "anthropic", id: "config-id-123" }, - name: "config-by-id", - }), - listConfig: jest - .fn() - .mockResolvedValue([{ name: "config-by-id", id: "config-id-123", apiProvider: "anthropic" }]), + activateProfile: jest.fn().mockResolvedValue(profile), + listConfig: jest.fn().mockResolvedValue([profile]), setModeConfig: jest.fn(), getModeConfigId: jest.fn().mockResolvedValue(undefined), } as any @@ -881,7 +882,7 @@ describe("ClineProvider", () => { }) }) - test("saves mode config when updating API configuration", async () => { + it("saves mode config when updating API configuration", async () => { // Setup mock context with mode and config name mockContext = { ...mockContext, @@ -907,12 +908,14 @@ describe("ClineProvider", () => { ;(provider as any).providerSettingsManager = { listConfig: jest.fn().mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + saveConfig: jest.fn().mockResolvedValue("test-id"), setModeConfig: jest.fn(), } as any // Update API configuration await messageHandler({ - type: "apiConfiguration", + type: "upsertApiConfiguration", + text: "test-config", apiConfiguration: { apiProvider: "anthropic" }, }) @@ -1675,14 +1678,11 @@ describe("ClineProvider", () => { currentApiConfigName: "test-config", } as any) - // Trigger updateApiConfiguration + // Trigger upsertApiConfiguration await messageHandler({ type: "upsertApiConfiguration", text: "test-config", - apiConfiguration: { - apiProvider: "anthropic", - apiKey: "test-key", - }, + apiConfiguration: { apiProvider: "anthropic", apiKey: "test-key" }, }) // Verify error was logged and user was notified diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index f85f2d2fb3..cde453d4fc 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -126,12 +126,6 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We // task. This essentially creates a fresh slate for the new task. await provider.initClineWithTask(message.text, message.images) break - case "apiConfiguration": - if (message.apiConfiguration) { - await provider.updateApiConfiguration(message.apiConfiguration) - } - await provider.postStateToWebview() - break case "customInstructions": await provider.updateCustomInstructions(message.text) break @@ -1068,7 +1062,7 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We break case "upsertApiConfiguration": if (message.text && message.apiConfiguration) { - await provider.upsertApiConfiguration(message.text, message.apiConfiguration) + await provider.upsertProviderProfile(message.text, message.apiConfiguration) } break case "renameApiConfiguration": diff --git a/src/exports/api.ts b/src/exports/api.ts index 4ee551bc08..ca30c0f3fb 100644 --- a/src/exports/api.ts +++ b/src/exports/api.ts @@ -6,7 +6,14 @@ import * as path from "path" import { getWorkspacePath } from "../utils/path" import { ClineProvider } from "../core/webview/ClineProvider" import { openClineInNewTab } from "../activate/registerCommands" -import { RooCodeSettings, RooCodeEvents, RooCodeEventName } from "../schemas" +import { + RooCodeSettings, + RooCodeEvents, + RooCodeEventName, + ProviderSettings, + ProviderSettingsEntry, + isSecretStateKey, +} from "../schemas" import { IpcOrigin, IpcMessageType, TaskCommandName, TaskEvent } from "../schemas/ipc" import { RooCodeAPI } from "./interface" @@ -246,8 +253,10 @@ export class API extends EventEmitter implements RooCodeAPI { // Global Settings Management - public getConfiguration() { - return this.sidebarProvider.getValues() + public getConfiguration(): RooCodeSettings { + return Object.fromEntries( + Object.entries(this.sidebarProvider.getValues()).filter(([key]) => !isSecretStateKey(key)), + ) } public async setConfiguration(values: RooCodeSettings) { @@ -258,77 +267,86 @@ export class API extends EventEmitter implements RooCodeAPI { // Provider Profile Management - private getProfilesMeta() { - return this.getConfiguration().listApiConfigMeta || [] + public getProfiles(): string[] { + return this.sidebarProvider.getProviderProfileEntries().map(({ name }) => name) } - public getProfiles() { - return this.getProfilesMeta().map((profile) => profile.name) + public getProfileEntry(name: string): ProviderSettingsEntry | undefined { + return this.sidebarProvider.getProviderProfileEntry(name) } - public hasProfile(name: string): boolean { - return !!(this.getConfiguration().listApiConfigMeta || []).find((profile) => profile.name === name) - } + public async createProfile(name: string, profile?: ProviderSettings, activate: boolean = true) { + const entry = this.getProfileEntry(name) - public async createProfile(name: string) { - if (!name || !name.trim()) { - throw new Error("Profile name cannot be empty") + if (entry) { + throw new Error(`Profile with name "${name}" already exists`) } - const currentSettings = this.getConfiguration() - const profiles = currentSettings.listApiConfigMeta || [] + const id = await this.sidebarProvider.upsertProviderProfile(name, profile ?? {}, activate) - if (profiles.some((profile) => profile.name === name)) { - throw new Error(`A profile with the name "${name}" already exists`) + if (!id) { + throw new Error(`Failed to create profile with name "${name}"`) } - const id = this.sidebarProvider.providerSettingsManager.generateId() - - await this.setConfiguration({ - ...currentSettings, - listApiConfigMeta: [ - ...profiles, - { - id, - name: name.trim(), - apiProvider: "openai" as const, - }, - ], - }) - return id } - public async deleteProfile(name: string) { - const currentSettings = this.getConfiguration() - const listApiConfigMeta = this.getProfilesMeta() - const targetIndex = listApiConfigMeta.findIndex((p) => p.name === name) + public async updateProfile( + name: string, + profile: ProviderSettings, + activate: boolean = true, + ): Promise { + const entry = this.getProfileEntry(name) - if (targetIndex === -1) { + if (!entry) { throw new Error(`Profile with name "${name}" does not exist`) } - const profileToDelete = listApiConfigMeta[targetIndex] - listApiConfigMeta.splice(targetIndex, 1) + const id = await this.sidebarProvider.upsertProviderProfile(name, profile, activate) - // If we're deleting the active profile, clear the currentApiConfigName. - const currentApiConfigName = - currentSettings.currentApiConfigName === profileToDelete.name - ? undefined - : currentSettings.currentApiConfigName + if (!id) { + throw new Error(`Failed to update profile with name "${name}"`) + } - await this.setConfiguration({ ...currentSettings, listApiConfigMeta, currentApiConfigName }) + return id + } + + public async upsertProfile( + name: string, + profile: ProviderSettings, + activate: boolean = true, + ): Promise { + const id = await this.sidebarProvider.upsertProviderProfile(name, profile, activate) + + if (!id) { + throw new Error(`Failed to upsert profile with name "${name}"`) + } + + return id + } + + public async deleteProfile(name: string): Promise { + const entry = this.getProfileEntry(name) + + if (!entry) { + throw new Error(`Profile with name "${name}" does not exist`) + } + + await this.sidebarProvider.deleteProviderProfile(entry) } public getActiveProfile(): string | undefined { return this.getConfiguration().currentApiConfigName } - public async setActiveProfile(name: string) { - if (!this.hasProfile(name)) { + public async setActiveProfile(name: string): Promise { + const entry = this.getProfileEntry(name) + + if (!entry) { throw new Error(`Profile with name "${name}" does not exist`) } await this.sidebarProvider.activateProviderProfile({ name }) + return this.getActiveProfile() } } diff --git a/src/exports/interface.ts b/src/exports/interface.ts index ed22f7e6d1..eb30ea1773 100644 --- a/src/exports/interface.ts +++ b/src/exports/interface.ts @@ -113,13 +113,41 @@ export interface RooCodeAPI extends EventEmitter { */ getProfiles(): string[] + /** + * Returns the profile entry for a given name + * @param name The name of the profile + * @returns The profile entry, or undefined if the profile does not exist + */ + getProfileEntry(name: string): ProviderSettingsEntry | undefined + /** * Creates a new API configuration profile * @param name The name of the profile + * @param profile The profile to create; defaults to an empty object + * @param activate Whether to activate the profile after creation; defaults to true * @returns The ID of the created profile * @throws Error if the profile already exists */ - createProfile(name: string, profile?: ProviderSettings): Promise + createProfile(name: string, profile?: ProviderSettings, activate?: boolean): Promise + + /** + * Updates an existing API configuration profile + * @param name The name of the profile + * @param profile The profile to update + * @param activate Whether to activate the profile after update; defaults to true + * @returns The ID of the updated profile + * @throws Error if the profile does not exist + */ + updateProfile(name: string, profile: ProviderSettings, activate?: boolean): Promise + + /** + * Creates a new API configuration profile or updates an existing one + * @param name The name of the profile + * @param profile The profile to create or update; defaults to an empty object + * @param activate Whether to activate the profile after upsert; defaults to true + * @returns The ID of the upserted profile + */ + upsertProfile(name: string, profile: ProviderSettings, activate?: boolean): Promise /** * Deletes a profile by name @@ -139,5 +167,5 @@ export interface RooCodeAPI extends EventEmitter { * @param name The name of the profile to activate * @throws Error if the profile does not exist */ - setActiveProfile(name: string): Promise + setActiveProfile(name: string): Promise } diff --git a/src/exports/roo-code.d.ts b/src/exports/roo-code.d.ts index 5535b94a5f..4727a1af4d 100644 --- a/src/exports/roo-code.d.ts +++ b/src/exports/roo-code.d.ts @@ -632,13 +632,38 @@ interface RooCodeAPI extends EventEmitter { * @returns Array of profile names */ getProfiles(): string[] + /** + * Returns the profile entry for a given name + * @param name The name of the profile + * @returns The profile entry, or undefined if the profile does not exist + */ + getProfileEntry(name: string): ProviderSettingsEntry | undefined /** * Creates a new API configuration profile * @param name The name of the profile + * @param profile The profile to create; defaults to an empty object + * @param activate Whether to activate the profile after creation; defaults to true * @returns The ID of the created profile * @throws Error if the profile already exists */ - createProfile(name: string, profile?: ProviderSettings): Promise + createProfile(name: string, profile?: ProviderSettings, activate?: boolean): Promise + /** + * Updates an existing API configuration profile + * @param name The name of the profile + * @param profile The profile to update + * @param activate Whether to activate the profile after update; defaults to true + * @returns The ID of the updated profile + * @throws Error if the profile does not exist + */ + updateProfile(name: string, profile: ProviderSettings, activate?: boolean): Promise + /** + * Creates a new API configuration profile or updates an existing one + * @param name The name of the profile + * @param profile The profile to create or update; defaults to an empty object + * @param activate Whether to activate the profile after upsert; defaults to true + * @returns The ID of the upserted profile + */ + upsertProfile(name: string, profile: ProviderSettings, activate?: boolean): Promise /** * Deletes a profile by name * @param name The name of the profile to delete @@ -655,7 +680,7 @@ interface RooCodeAPI extends EventEmitter { * @param name The name of the profile to activate * @throws Error if the profile does not exist */ - setActiveProfile(name: string): Promise + setActiveProfile(name: string): Promise } export { diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index c1dc3ffc19..22fe5c7d3e 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -11,7 +11,6 @@ export type AudioType = "notification" | "celebration" | "progress_loop" export interface WebviewMessage { type: - | "apiConfiguration" | "deleteMultipleTasksWithIds" | "currentApiConfigName" | "saveApiConfiguration"