Improve provider profile management in the external API (#3386)

Co-authored-by: John Richmond <5629+jr@users.noreply.github.com>
This commit is contained in:
Chris Estreich 2025-05-09 10:05:34 -07:00 committed by hannesrudolph
parent 49da2b4f03
commit a44481350f
7 changed files with 256 additions and 126 deletions

View file

@ -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<ClineProviderEvents> 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<ClineProviderEvents> 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<string | undefined> {
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<ClineProviderEvents> 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<ClineProviderEvents> 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<ClineProviderEvents> 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

View file

@ -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

View file

@ -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":

View file

@ -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<RooCodeEvents> 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<RooCodeEvents> 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<string | undefined> {
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<string | undefined> {
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<void> {
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<string | undefined> {
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()
}
}

View file

@ -113,13 +113,41 @@ export interface RooCodeAPI extends EventEmitter<RooCodeEvents> {
*/
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<string>
createProfile(name: string, profile?: ProviderSettings, activate?: boolean): Promise<string>
/**
* 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<string | undefined>
/**
* 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<string | undefined>
/**
* Deletes a profile by name
@ -139,5 +167,5 @@ export interface RooCodeAPI extends EventEmitter<RooCodeEvents> {
* @param name The name of the profile to activate
* @throws Error if the profile does not exist
*/
setActiveProfile(name: string): Promise<void>
setActiveProfile(name: string): Promise<string | undefined>
}

View file

@ -632,13 +632,38 @@ interface RooCodeAPI extends EventEmitter<RooCodeEvents> {
* @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<string>
createProfile(name: string, profile?: ProviderSettings, activate?: boolean): Promise<string>
/**
* 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<string | undefined>
/**
* 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<string | undefined>
/**
* Deletes a profile by name
* @param name The name of the profile to delete
@ -655,7 +680,7 @@ interface RooCodeAPI extends EventEmitter<RooCodeEvents> {
* @param name The name of the profile to activate
* @throws Error if the profile does not exist
*/
setActiveProfile(name: string): Promise<void>
setActiveProfile(name: string): Promise<string | undefined>
}
export {

View file

@ -11,7 +11,6 @@ export type AudioType = "notification" | "celebration" | "progress_loop"
export interface WebviewMessage {
type:
| "apiConfiguration"
| "deleteMultipleTasksWithIds"
| "currentApiConfigName"
| "saveApiConfiguration"