From 4fe2eefeb9cfbace40dcb8f359c1e8ca2aafd86c Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Thu, 6 Feb 2025 21:22:07 -0700 Subject: [PATCH 01/15] Update McpView.tsx Moved the restart button to the top --- webview-ui/src/components/mcp/McpView.tsx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index 54b7c29fa7..b5dd59c828 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/webview-ui/src/components/mcp/McpView.tsx @@ -202,6 +202,13 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
e.stopPropagation()}> + + +
- - - {server.status === "connecting" ? "Restarting..." : "Restart Server"} -
) )} From 4b3ea070339c8561ed6479cd5c8f55712a27e9d1 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 7 Feb 2025 03:06:01 -0800 Subject: [PATCH 02/15] Prevent provider client instantiations from throwing --- src/api/providers/__tests__/deepseek.test.ts | 2 +- src/api/providers/__tests__/gemini.test.ts | 2 +- src/api/providers/deepseek.ts | 5 +--- src/api/providers/gemini.ts | 5 +--- src/api/providers/glama.ts | 7 +++--- src/api/providers/openai-native.ts | 25 ++++++++------------ src/api/providers/openai.ts | 11 ++++----- src/api/providers/openrouter.ts | 18 +++++++------- src/api/providers/unbound.ts | 7 +++--- src/api/providers/vertex.ts | 4 ++-- 10 files changed, 37 insertions(+), 49 deletions(-) diff --git a/src/api/providers/__tests__/deepseek.test.ts b/src/api/providers/__tests__/deepseek.test.ts index e510b19edc..fe5fa7787e 100644 --- a/src/api/providers/__tests__/deepseek.test.ts +++ b/src/api/providers/__tests__/deepseek.test.ts @@ -84,7 +84,7 @@ describe("DeepSeekHandler", () => { expect(handler.getModel().id).toBe(mockOptions.apiModelId) }) - it("should throw error if API key is missing", () => { + it.skip("should throw error if API key is missing", () => { expect(() => { new DeepSeekHandler({ ...mockOptions, diff --git a/src/api/providers/__tests__/gemini.test.ts b/src/api/providers/__tests__/gemini.test.ts index e57ba2ea78..1e536eaecf 100644 --- a/src/api/providers/__tests__/gemini.test.ts +++ b/src/api/providers/__tests__/gemini.test.ts @@ -33,7 +33,7 @@ describe("GeminiHandler", () => { expect(handler["options"].apiModelId).toBe("gemini-2.0-flash-thinking-exp-1219") }) - it("should throw if API key is missing", () => { + it.skip("should throw if API key is missing", () => { expect(() => { new GeminiHandler({ apiModelId: "gemini-2.0-flash-thinking-exp-1219", diff --git a/src/api/providers/deepseek.ts b/src/api/providers/deepseek.ts index 96be435840..1c7186d48c 100644 --- a/src/api/providers/deepseek.ts +++ b/src/api/providers/deepseek.ts @@ -4,12 +4,9 @@ import { deepSeekModels, deepSeekDefaultModelId } from "../../shared/api" export class DeepSeekHandler extends OpenAiHandler { constructor(options: ApiHandlerOptions) { - if (!options.deepSeekApiKey) { - throw new Error("DeepSeek API key is required. Please provide it in the settings.") - } super({ ...options, - openAiApiKey: options.deepSeekApiKey, + openAiApiKey: options.deepSeekApiKey ?? "not-provided", openAiModelId: options.apiModelId ?? deepSeekDefaultModelId, openAiBaseUrl: options.deepSeekBaseUrl ?? "https://api.deepseek.com/v1", openAiStreamingEnabled: true, diff --git a/src/api/providers/gemini.ts b/src/api/providers/gemini.ts index 0f6392b6b3..0577a021e6 100644 --- a/src/api/providers/gemini.ts +++ b/src/api/providers/gemini.ts @@ -10,11 +10,8 @@ export class GeminiHandler implements ApiHandler, SingleCompletionHandler { private client: GoogleGenerativeAI constructor(options: ApiHandlerOptions) { - if (!options.geminiApiKey) { - throw new Error("API key is required for Google Gemini") - } this.options = options - this.client = new GoogleGenerativeAI(options.geminiApiKey) + this.client = new GoogleGenerativeAI(options.geminiApiKey ?? "not-provided") } async *createMessage(systemPrompt: string, messages: Anthropic.Messages.MessageParam[]): ApiStream { diff --git a/src/api/providers/glama.ts b/src/api/providers/glama.ts index 1e8c721faa..95b806f27c 100644 --- a/src/api/providers/glama.ts +++ b/src/api/providers/glama.ts @@ -13,10 +13,9 @@ export class GlamaHandler implements ApiHandler, SingleCompletionHandler { constructor(options: ApiHandlerOptions) { this.options = options - this.client = new OpenAI({ - baseURL: "https://glama.ai/api/gateway/openai/v1", - apiKey: this.options.glamaApiKey, - }) + const baseURL = "https://glama.ai/api/gateway/openai/v1" + const apiKey = this.options.glamaApiKey ?? "not-provided" + this.client = new OpenAI({ baseURL, apiKey }) } async *createMessage(systemPrompt: string, messages: Anthropic.Messages.MessageParam[]): ApiStream { diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index f1b5bcebd3..e4883b7a98 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -17,9 +17,8 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler constructor(options: ApiHandlerOptions) { this.options = options - this.client = new OpenAI({ - apiKey: this.options.openAiNativeApiKey, - }) + const apiKey = this.options.openAiNativeApiKey ?? "not-provided" + this.client = new OpenAI({ apiKey }) } async *createMessage(systemPrompt: string, messages: Anthropic.Messages.MessageParam[]): ApiStream { @@ -41,7 +40,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler private async *handleO1FamilyMessage( modelId: string, systemPrompt: string, - messages: Anthropic.Messages.MessageParam[] + messages: Anthropic.Messages.MessageParam[], ): ApiStream { // o1 supports developer prompt with formatting // o1-preview and o1-mini only support user messages @@ -63,7 +62,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler private async *handleO3FamilyMessage( modelId: string, systemPrompt: string, - messages: Anthropic.Messages.MessageParam[] + messages: Anthropic.Messages.MessageParam[], ): ApiStream { const stream = await this.client.chat.completions.create({ model: "o3-mini", @@ -85,7 +84,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler private async *handleDefaultModelMessage( modelId: string, systemPrompt: string, - messages: Anthropic.Messages.MessageParam[] + messages: Anthropic.Messages.MessageParam[], ): ApiStream { const stream = await this.client.chat.completions.create({ model: modelId, @@ -98,9 +97,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler yield* this.handleStreamResponse(stream) } - private async *yieldResponseData( - response: OpenAI.Chat.Completions.ChatCompletion - ): ApiStream { + private async *yieldResponseData(response: OpenAI.Chat.Completions.ChatCompletion): ApiStream { yield { type: "text", text: response.choices[0]?.message.content || "", @@ -112,9 +109,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler } } - private async *handleStreamResponse( - stream: AsyncIterable - ): ApiStream { + private async *handleStreamResponse(stream: AsyncIterable): ApiStream { for await (const chunk of stream) { const delta = chunk.choices[0]?.delta if (delta?.content) { @@ -168,7 +163,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler private getO1CompletionOptions( modelId: string, - prompt: string + prompt: string, ): OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming { return { model: modelId, @@ -178,7 +173,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler private getO3CompletionOptions( modelId: string, - prompt: string + prompt: string, ): OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming { return { model: "o3-mini", @@ -189,7 +184,7 @@ export class OpenAiNativeHandler implements ApiHandler, SingleCompletionHandler private getDefaultCompletionOptions( modelId: string, - prompt: string + prompt: string, ): OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming { return { model: modelId, diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index acfbe43d79..408f4e5cc3 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -19,6 +19,8 @@ export class OpenAiHandler implements ApiHandler, SingleCompletionHandler { constructor(options: ApiHandlerOptions) { this.options = options + const baseURL = this.options.openAiBaseUrl ?? "https://api.openai.com/v1" + const apiKey = this.options.openAiApiKey ?? "not-provided" let urlHost: string try { @@ -33,15 +35,12 @@ export class OpenAiHandler implements ApiHandler, SingleCompletionHandler { // Azure API shape slightly differs from the core API shape: // https://github.com/openai/openai-node?tab=readme-ov-file#microsoft-azure-openai this.client = new AzureOpenAI({ - baseURL: this.options.openAiBaseUrl, - apiKey: this.options.openAiApiKey, + baseURL, + apiKey, apiVersion: this.options.azureApiVersion || azureOpenAiDefaultApiVersion, }) } else { - this.client = new OpenAI({ - baseURL: this.options.openAiBaseUrl, - apiKey: this.options.openAiApiKey, - }) + this.client = new OpenAI({ baseURL, apiKey }) } } diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index 43ed56c7f1..0e23c5d35d 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -27,14 +27,16 @@ export class OpenRouterHandler implements ApiHandler, SingleCompletionHandler { constructor(options: ApiHandlerOptions) { this.options = options - this.client = new OpenAI({ - baseURL: this.options.openRouterBaseUrl || "https://openrouter.ai/api/v1", - apiKey: this.options.openRouterApiKey, - defaultHeaders: { - "HTTP-Referer": "https://github.com/RooVetGit/Roo-Cline", - "X-Title": "Roo Code", - }, - }) + + const baseURL = this.options.openRouterBaseUrl || "https://openrouter.ai/api/v1" + const apiKey = this.options.openRouterApiKey ?? "not-provided" + + const defaultHeaders = { + "HTTP-Referer": "https://github.com/RooVetGit/Roo-Cline", + "X-Title": "Roo Code", + } + + this.client = new OpenAI({ baseURL, apiKey, defaultHeaders }) } async *createMessage( diff --git a/src/api/providers/unbound.ts b/src/api/providers/unbound.ts index 2bc3d82822..305bd282ad 100644 --- a/src/api/providers/unbound.ts +++ b/src/api/providers/unbound.ts @@ -16,10 +16,9 @@ export class UnboundHandler implements ApiHandler, SingleCompletionHandler { constructor(options: ApiHandlerOptions) { this.options = options - this.client = new OpenAI({ - baseURL: "https://api.getunbound.ai/v1", - apiKey: this.options.unboundApiKey, - }) + const baseURL = "https://api.getunbound.ai/v1" + const apiKey = this.options.unboundApiKey ?? "not-provided" + this.client = new OpenAI({ baseURL, apiKey }) } async *createMessage(systemPrompt: string, messages: Anthropic.Messages.MessageParam[]): ApiStream { diff --git a/src/api/providers/vertex.ts b/src/api/providers/vertex.ts index d997135e1c..1ea68eaa4e 100644 --- a/src/api/providers/vertex.ts +++ b/src/api/providers/vertex.ts @@ -12,9 +12,9 @@ export class VertexHandler implements ApiHandler, SingleCompletionHandler { constructor(options: ApiHandlerOptions) { this.options = options this.client = new AnthropicVertex({ - projectId: this.options.vertexProjectId, + projectId: this.options.vertexProjectId ?? "not-provided", // https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models/use-claude#regions - region: this.options.vertexRegion, + region: this.options.vertexRegion ?? "us-east5", }) } From b2b5e1ffa64166fe757cc6a7cf2e7f63c94bd466 Mon Sep 17 00:00:00 2001 From: MuriloFP Date: Fri, 7 Feb 2025 15:51:30 -0300 Subject: [PATCH 03/15] fix: Implement singleton pattern for MCP server management to prevent multiple instances Problem: - Multiple instances of the Roo Code application were launching separate MCP server instances - This led to unnecessary resource consumption and potential conflicts between instances Solution: 1. Created new McpServerManager singleton class to manage MCP server instances: - Static getInstance() method ensures only one McpHub instance exists - Tracks registered ClineProvider instances - Handles cleanup on extension deactivation 2. Modified ClineProvider class: - Changed mcpHub from private to protected - Added getMcpHub() public getter method - Updated initialization to use McpServerManager - Added unregister logic in dispose() 3. Updated extension.ts to handle cleanup: - Added McpServerManager cleanup in deactivate() Technical Implementation: - Uses WeakRef for provider tracking to allow proper garbage collection - Maintains global state to track instance IDs - Implements proper cleanup of resources on disposal - Ensures backward compatibility with existing code This change significantly improves resource usage and prevents potential conflicts between multiple instances of the application while maintaining all existing functionality. --- src/core/Cline.ts | 10 +++-- src/core/webview/ClineProvider.ts | 21 +++++++++- src/extension.ts | 9 ++++- src/services/mcp/McpServerManager.ts | 60 ++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 src/services/mcp/McpServerManager.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 2c36dd5495..0f2e796132 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -832,7 +832,7 @@ export class Cline { this.lastApiRequestTime = Date.now() if (mcpEnabled ?? true) { - mcpHub = this.providerRef.deref()?.mcpHub + mcpHub = this.providerRef.deref()?.getMcpHub() if (!mcpHub) { throw new Error("MCP hub not available") } @@ -1013,7 +1013,7 @@ export class Cline { // (have to do this for partial and complete since sending content in thinking tags to markdown renderer will automatically be removed) // Remove end substrings of (with optional line break after) and (with optional line break before) // - Needs to be separate since we dont want to remove the line break before the first tag // - Needs to happen before the xml parsing below @@ -2267,7 +2267,8 @@ export class Cline { await this.say("mcp_server_request_started") // same as browser_action_result const toolResult = await this.providerRef .deref() - ?.mcpHub?.callTool(server_name, tool_name, parsedArguments) + ?.getMcpHub() + ?.callTool(server_name, tool_name, parsedArguments) // TODO: add progress indicator and ability to parse images and non-text responses const toolResultPretty = @@ -2335,7 +2336,8 @@ export class Cline { await this.say("mcp_server_request_started") const resourceResult = await this.providerRef .deref() - ?.mcpHub?.readResource(server_name, uri) + ?.getMcpHub() + ?.readResource(server_name, uri) const resourceResultPretty = resourceResult?.contents .map((item) => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 9326c83350..7b4d4b2047 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -36,6 +36,7 @@ import { EXPERIMENT_IDS, experiments as Experiments, experimentDefault, Experime import { CustomSupportPrompts, supportPrompt } from "../../shared/support-prompt" import { ACTION_NAMES } from "../CodeActionProvider" +import { McpServerManager } from "../../services/mcp/McpServerManager" /* https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts @@ -136,7 +137,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { private isViewLaunched = false private cline?: Cline private workspaceTracker?: WorkspaceTracker - mcpHub?: McpHub + protected mcpHub?: McpHub // Change from private to protected private latestAnnouncementId = "jan-21-2025-custom-modes" // update to some unique identifier when we add a new announcement configManager: ConfigManager customModesManager: CustomModesManager @@ -148,11 +149,19 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.outputChannel.appendLine("ClineProvider instantiated") ClineProvider.activeInstances.add(this) this.workspaceTracker = new WorkspaceTracker(this) - this.mcpHub = new McpHub(this) this.configManager = new ConfigManager(this.context) this.customModesManager = new CustomModesManager(this.context, async () => { await this.postStateToWebview() }) + + // Initialize MCP Hub through the singleton manager + McpServerManager.getInstance(this.context, this) + .then((hub) => { + this.mcpHub = hub + }) + .catch((error) => { + this.outputChannel.appendLine(`Failed to initialize MCP Hub: ${error}`) + }) } /* @@ -181,6 +190,9 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.customModesManager?.dispose() this.outputChannel.appendLine("Disposed all disposables") ClineProvider.activeInstances.delete(this) + + // Unregister from McpServerManager + McpServerManager.unregisterProvider(this) } public static getVisibleInstance(): ClineProvider | undefined { @@ -2538,4 +2550,9 @@ export class ClineProvider implements vscode.WebviewViewProvider { get messages() { return this.cline?.clineMessages || [] } + + // Add public getter + public getMcpHub(): McpHub | undefined { + return this.mcpHub + } } diff --git a/src/extension.ts b/src/extension.ts index 8ca7f3312c..a05afa4651 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -6,6 +6,7 @@ import "./utils/path" // Necessary to have access to String.prototype.toPosix. import { CodeActionProvider } from "./core/CodeActionProvider" import { DIFF_VIEW_URI_SCHEME } from "./integrations/editor/DiffViewProvider" import { handleUri, registerCommands, registerCodeActions, registerTerminalActions } from "./activate" +import { McpServerManager } from "./services/mcp/McpServerManager" /** * Built using https://github.com/microsoft/vscode-webview-ui-toolkit @@ -16,10 +17,12 @@ import { handleUri, registerCommands, registerCodeActions, registerTerminalActio */ let outputChannel: vscode.OutputChannel +let extensionContext: vscode.ExtensionContext // This method is called when your extension is activated. // Your extension is activated the very first time the command is executed. export function activate(context: vscode.ExtensionContext) { + extensionContext = context outputChannel = vscode.window.createOutputChannel("Roo-Code") context.subscriptions.push(outputChannel) outputChannel.appendLine("Roo-Code extension activated") @@ -83,7 +86,9 @@ export function activate(context: vscode.ExtensionContext) { return createClineAPI(outputChannel, sidebarProvider) } -// This method is called when your extension is deactivated. -export function deactivate() { +// This method is called when your extension is deactivated +export async function deactivate() { outputChannel.appendLine("Roo-Code extension deactivated") + // Clean up MCP server manager + await McpServerManager.cleanup(extensionContext) } diff --git a/src/services/mcp/McpServerManager.ts b/src/services/mcp/McpServerManager.ts new file mode 100644 index 0000000000..20ed2b8322 --- /dev/null +++ b/src/services/mcp/McpServerManager.ts @@ -0,0 +1,60 @@ +import * as vscode from "vscode" +import { McpHub } from "./McpHub" +import { ClineProvider } from "../../core/webview/ClineProvider" + +/** + * Singleton manager for MCP server instances. + * Ensures only one set of MCP servers runs across all webviews. + */ +export class McpServerManager { + private static instance: McpHub | null = null + private static readonly GLOBAL_STATE_KEY = "mcpHubInstanceId" + private static providers: Set = new Set() + + /** + * Get the singleton McpHub instance. + * Creates a new instance if one doesn't exist. + */ + static async getInstance(context: vscode.ExtensionContext, provider: ClineProvider): Promise { + // Register the provider + this.providers.add(provider) + + if (!this.instance) { + this.instance = new McpHub(provider) + // Store a unique identifier in global state to track the primary instance + await context.globalState.update(this.GLOBAL_STATE_KEY, Date.now().toString()) + } + return this.instance + } + + /** + * Remove a provider from the tracked set. + * This is called when a webview is disposed. + */ + static unregisterProvider(provider: ClineProvider): void { + this.providers.delete(provider) + } + + /** + * Notify all registered providers of server state changes. + */ + static notifyProviders(message: any): void { + this.providers.forEach((provider) => { + provider.postMessageToWebview(message).catch((error) => { + console.error("Failed to notify provider:", error) + }) + }) + } + + /** + * Clean up the singleton instance and all its resources. + */ + static async cleanup(context: vscode.ExtensionContext): Promise { + if (this.instance) { + await this.instance.dispose() + this.instance = null + await context.globalState.update(this.GLOBAL_STATE_KEY, undefined) + } + this.providers.clear() + } +} From ef95562dfe5166b2c66c5c55a4fe264e12e57ffd Mon Sep 17 00:00:00 2001 From: MuriloFP Date: Fri, 7 Feb 2025 16:57:21 -0300 Subject: [PATCH 04/15] fix: ensure MCP server list appears in settings when server starts first When the MCP server was initialized before opening RooCode, the server list would not appear in the settings. This was fixed by: 1. Adding proper server list initialization in ClineProvider.ts when the webview launches, checking if mcpHub exists and sending its current servers to the webview: ```typescript if (this.mcpHub) { this.postMessageToWebview({ type: "mcpServers", mcpServers: this.mcpHub.getServers() }) } ``` 2. Using the public getServers() method from McpHub instead of relying on internal state updates, ensuring consistent server list state across the application. The fix maintains clean separation of concerns and follows existing patterns for state management between the extension and webview. --- src/core/webview/ClineProvider.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 7b4d4b2047..23346d945c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -613,6 +613,15 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.postMessageToWebview({ type: "openRouterModels", openRouterModels }) } }) + + // If MCP Hub is already initialized, update the webview with current server list + if (this.mcpHub) { + this.postMessageToWebview({ + type: "mcpServers", + mcpServers: this.mcpHub.getServers(), + }) + } + // gui relies on model info to be up-to-date to provide the most accurate pricing, so we need to fetch the latest details on launch. // we do this for all users since many users switch between api providers and if they were to switch back to openrouter it would be showing outdated model info if we hadn't retrieved the latest at this point // (see normalizeApiConfiguration > openrouter) @@ -2115,6 +2124,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { autoApprovalEnabled: autoApprovalEnabled ?? false, customModes: await this.customModesManager.getCustomModes(), experiments: experiments ?? experimentDefault, + mcpServers: this.mcpHub?.getServers() ?? [], } } From c9be470319704782523d913a0b2444b687a17e95 Mon Sep 17 00:00:00 2001 From: MuriloFP Date: Fri, 7 Feb 2025 17:12:39 -0300 Subject: [PATCH 05/15] fix: prevent race conditions in McpServerManager singleton initialization Added thread-safe initialization to the McpServerManager singleton pattern to prevent potential race conditions when getInstance is called concurrently. The changes include: 1. Added initializationPromise to track ongoing initialization 2. Implemented double-checked locking pattern: - First check: Return existing instance if available - Second check: Wait for any ongoing initialization - Third check: Double-check inside initialization block 3. Added proper cleanup in finally block to prevent deadlocks This ensures that: - Only one McpHub instance is ever created - Concurrent calls wait for initialization to complete - Resources are properly initialized and cleaned up - No memory leaks from incomplete initialization The fix maintains the existing functionality while making it safe for concurrent access in VS Code's multi-window environment. --- src/services/mcp/McpServerManager.ts | 33 +++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/services/mcp/McpServerManager.ts b/src/services/mcp/McpServerManager.ts index 20ed2b8322..e15f9db0a7 100644 --- a/src/services/mcp/McpServerManager.ts +++ b/src/services/mcp/McpServerManager.ts @@ -10,21 +10,44 @@ export class McpServerManager { private static instance: McpHub | null = null private static readonly GLOBAL_STATE_KEY = "mcpHubInstanceId" private static providers: Set = new Set() + private static initializationPromise: Promise | null = null /** * Get the singleton McpHub instance. * Creates a new instance if one doesn't exist. + * Thread-safe implementation using a promise-based lock. */ static async getInstance(context: vscode.ExtensionContext, provider: ClineProvider): Promise { // Register the provider this.providers.add(provider) - if (!this.instance) { - this.instance = new McpHub(provider) - // Store a unique identifier in global state to track the primary instance - await context.globalState.update(this.GLOBAL_STATE_KEY, Date.now().toString()) + // If we already have an instance, return it + if (this.instance) { + return this.instance } - return this.instance + + // If initialization is in progress, wait for it + if (this.initializationPromise) { + return this.initializationPromise + } + + // Create a new initialization promise + this.initializationPromise = (async () => { + try { + // Double-check instance in case it was created while we were waiting + if (!this.instance) { + this.instance = new McpHub(provider) + // Store a unique identifier in global state to track the primary instance + await context.globalState.update(this.GLOBAL_STATE_KEY, Date.now().toString()) + } + return this.instance + } finally { + // Clear the initialization promise after completion or error + this.initializationPromise = null + } + })() + + return this.initializationPromise } /** From 0bc9e17c1aa888d57343ab24d6e461dd9c2f54c2 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 7 Feb 2025 12:25:52 -0800 Subject: [PATCH 06/15] Better model picker --- webview-ui/jest.config.cjs | 2 +- webview-ui/package-lock.json | 196 ++------ webview-ui/package.json | 6 +- .../history/__tests__/HistoryView.test.tsx | 68 ++- .../src/components/settings/ApiOptions.tsx | 151 +----- .../components/settings/GlamaModelPicker.tsx | 424 +---------------- .../settings/ModelDescriptionMarkdown.tsx | 90 ++++ .../src/components/settings/ModelInfoView.tsx | 124 +++++ .../src/components/settings/ModelPicker.tsx | 130 +++++ .../components/settings/OpenAiModelPicker.tsx | 188 +------- .../settings/OpenRouterModelPicker.tsx | 446 +----------------- .../settings/__tests__/ModelPicker.test.tsx | 86 ++++ webview-ui/src/components/settings/styles.ts | 80 ++++ webview-ui/src/components/ui/button.tsx | 7 +- webview-ui/src/components/ui/command.tsx | 10 +- webview-ui/src/components/ui/popover.tsx | 2 +- webview-ui/src/index.css | 22 + webview-ui/src/utils/formatPrice.ts | 8 + 18 files changed, 687 insertions(+), 1353 deletions(-) create mode 100644 webview-ui/src/components/settings/ModelDescriptionMarkdown.tsx create mode 100644 webview-ui/src/components/settings/ModelInfoView.tsx create mode 100644 webview-ui/src/components/settings/ModelPicker.tsx create mode 100644 webview-ui/src/components/settings/__tests__/ModelPicker.test.tsx create mode 100644 webview-ui/src/components/settings/styles.ts create mode 100644 webview-ui/src/utils/formatPrice.ts diff --git a/webview-ui/jest.config.cjs b/webview-ui/jest.config.cjs index 69ed93166e..6ee94dda39 100644 --- a/webview-ui/jest.config.cjs +++ b/webview-ui/jest.config.cjs @@ -6,7 +6,7 @@ module.exports = { moduleFileExtensions: ["ts", "tsx", "js", "jsx", "json", "node"], transform: { "^.+\\.(ts|tsx)$": ["ts-jest", { tsconfig: { jsx: "react-jsx" } }] }, testMatch: ["/src/**/__tests__/**/*.{js,jsx,ts,tsx}", "/src/**/*.{spec,test}.{js,jsx,ts,tsx}"], - setupFilesAfterEnv: ["/src/setupTests.ts", "@testing-library/jest-dom/extend-expect"], + setupFilesAfterEnv: ["/src/setupTests.ts"], moduleNameMapper: { "\\.(css|less|scss|sass)$": "identity-obj-proxy", "^vscrui$": "/src/__mocks__/vscrui.ts", diff --git a/webview-ui/package-lock.json b/webview-ui/package-lock.json index 6590035968..b44ade2851 100644 --- a/webview-ui/package-lock.json +++ b/webview-ui/package-lock.json @@ -42,9 +42,9 @@ "@storybook/react": "^8.5.2", "@storybook/react-vite": "^8.5.2", "@storybook/test": "^8.5.2", - "@testing-library/jest-dom": "^5.17.0", - "@testing-library/react": "^13.4.0", - "@testing-library/user-event": "^13.5.0", + "@testing-library/jest-dom": "^6.6.3", + "@testing-library/react": "^16.2.0", + "@testing-library/user-event": "^14.6.1", "@types/jest": "^27.5.2", "@types/node": "^18.0.0", "@types/react": "^18.3.18", @@ -5498,24 +5498,22 @@ } }, "node_modules/@testing-library/jest-dom": { - "version": "5.17.0", - "resolved": "https://registry.npmjs.org/@testing-library/jest-dom/-/jest-dom-5.17.0.tgz", - "integrity": "sha512-ynmNeT7asXyH3aSVv4vvX4Rb+0qjOhdNHnO/3vuZNqPmhDpV/+rCSGwQ7bLcmU2cJ4dvoheIO85LQj0IbJHEtg==", + "version": "6.6.3", + "resolved": "https://registry.npmjs.org/@testing-library/jest-dom/-/jest-dom-6.6.3.tgz", + "integrity": "sha512-IteBhl4XqYNkM54f4ejhLRJiZNqcSCoXUOG2CPK7qbD322KjQozM4kHQOfkG2oln9b9HTYqs+Sae8vBATubxxA==", "dev": true, "license": "MIT", "dependencies": { - "@adobe/css-tools": "^4.0.1", - "@babel/runtime": "^7.9.2", - "@types/testing-library__jest-dom": "^5.9.1", + "@adobe/css-tools": "^4.4.0", "aria-query": "^5.0.0", "chalk": "^3.0.0", "css.escape": "^1.5.1", - "dom-accessibility-api": "^0.5.6", - "lodash": "^4.17.15", + "dom-accessibility-api": "^0.6.3", + "lodash": "^4.17.21", "redent": "^3.0.0" }, "engines": { - "node": ">=8", + "node": ">=14", "npm": ">=6", "yarn": ">=1" } @@ -5534,66 +5532,49 @@ "node": ">=8" } }, + "node_modules/@testing-library/jest-dom/node_modules/dom-accessibility-api": { + "version": "0.6.3", + "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.6.3.tgz", + "integrity": "sha512-7ZgogeTnjuHbo+ct10G9Ffp0mif17idi0IyWNVA/wcwcm7NPOD/WEHVP3n7n3MhXqxoIYm8d6MuZohYWIZ4T3w==", + "dev": true, + "license": "MIT" + }, "node_modules/@testing-library/react": { - "version": "13.4.0", - "resolved": "https://registry.npmjs.org/@testing-library/react/-/react-13.4.0.tgz", - "integrity": "sha512-sXOGON+WNTh3MLE9rve97ftaZukN3oNf2KjDy7YTx6hcTO2uuLHuCGynMDhFwGw/jYf4OJ2Qk0i4i79qMNNkyw==", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/runtime": "^7.12.5", - "@testing-library/dom": "^8.5.0", - "@types/react-dom": "^18.0.0" - }, - "engines": { - "node": ">=12" - }, - "peerDependencies": { - "react": "^18.0.0", - "react-dom": "^18.0.0" - } - }, - "node_modules/@testing-library/react/node_modules/@testing-library/dom": { - "version": "8.20.1", - "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-8.20.1.tgz", - "integrity": "sha512-/DiOQ5xBxgdYRC8LNk7U+RWat0S3qRLeIw3ZIkMQ9kkVlRmwD/Eg8k8CqIpD6GW7u20JIUOfMKbxtiLutpjQ4g==", - "dev": true, - "license": "MIT", - "dependencies": { - "@babel/code-frame": "^7.10.4", - "@babel/runtime": "^7.12.5", - "@types/aria-query": "^5.0.1", - "aria-query": "5.1.3", - "chalk": "^4.1.0", - "dom-accessibility-api": "^0.5.9", - "lz-string": "^1.5.0", - "pretty-format": "^27.0.2" - }, - "engines": { - "node": ">=12" - } - }, - "node_modules/@testing-library/react/node_modules/aria-query": { - "version": "5.1.3", - "resolved": "https://registry.npmjs.org/aria-query/-/aria-query-5.1.3.tgz", - "integrity": "sha512-R5iJ5lkuHybztUfuOAznmboyjWq8O6sqNqtK7CLOqdydi54VNbORp49mb14KbWgG1QD3JFO9hJdZ+y4KutfdOQ==", - "dev": true, - "license": "Apache-2.0", - "dependencies": { - "deep-equal": "^2.0.5" - } - }, - "node_modules/@testing-library/user-event": { - "version": "13.5.0", - "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-13.5.0.tgz", - "integrity": "sha512-5Kwtbo3Y/NowpkbRuSepbyMFkZmHgD+vPzYB/RJ4oxt5Gj/avFFBYjhw27cqSVPVw/3a67NK1PbiIr9k4Gwmdg==", + "version": "16.2.0", + "resolved": "https://registry.npmjs.org/@testing-library/react/-/react-16.2.0.tgz", + "integrity": "sha512-2cSskAvA1QNtKc8Y9VJQRv0tm3hLVgxRGDB+KYhIaPQJ1I+RHbhIXcM+zClKXzMes/wshsMVzf4B9vS4IZpqDQ==", "dev": true, "license": "MIT", "dependencies": { "@babel/runtime": "^7.12.5" }, "engines": { - "node": ">=10", + "node": ">=18" + }, + "peerDependencies": { + "@testing-library/dom": "^10.0.0", + "@types/react": "^18.0.0 || ^19.0.0", + "@types/react-dom": "^18.0.0 || ^19.0.0", + "react": "^18.0.0 || ^19.0.0", + "react-dom": "^18.0.0 || ^19.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "@types/react-dom": { + "optional": true + } + } + }, + "node_modules/@testing-library/user-event": { + "version": "14.6.1", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-14.6.1.tgz", + "integrity": "sha512-vq7fv0rnt+QTXgPxr5Hjc210p6YKq2kmdziLgnsZGgLJ9e6VAShx1pACLuRjd/AS/sr7phAR58OIIpf0LlmQNw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12", "npm": ">=6" }, "peerDependencies": { @@ -8255,39 +8236,6 @@ "node": ">=6" } }, - "node_modules/deep-equal": { - "version": "2.2.3", - "resolved": "https://registry.npmjs.org/deep-equal/-/deep-equal-2.2.3.tgz", - "integrity": "sha512-ZIwpnevOurS8bpT4192sqAowWM76JDKSHYzMLty3BZGSswgq6pBaH3DhCSW5xVAZICZyKdOBPjwww5wfgT/6PA==", - "dev": true, - "license": "MIT", - "dependencies": { - "array-buffer-byte-length": "^1.0.0", - "call-bind": "^1.0.5", - "es-get-iterator": "^1.1.3", - "get-intrinsic": "^1.2.2", - "is-arguments": "^1.1.1", - "is-array-buffer": "^3.0.2", - "is-date-object": "^1.0.5", - "is-regex": "^1.1.4", - "is-shared-array-buffer": "^1.0.2", - "isarray": "^2.0.5", - "object-is": "^1.1.5", - "object-keys": "^1.1.1", - "object.assign": "^4.1.4", - "regexp.prototype.flags": "^1.5.1", - "side-channel": "^1.0.4", - "which-boxed-primitive": "^1.0.2", - "which-collection": "^1.0.1", - "which-typed-array": "^1.1.13" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/deep-is": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/deep-is/-/deep-is-0.1.4.tgz", @@ -8638,27 +8586,6 @@ "node": ">= 0.4" } }, - "node_modules/es-get-iterator": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/es-get-iterator/-/es-get-iterator-1.1.3.tgz", - "integrity": "sha512-sPZmqHBe6JIiTfN5q2pEi//TwxmAFHwj/XEuYjTuse78i8KxaqMTTzxPoFKuzRpDpTJ+0NAbpfenkmH2rePtuw==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.2", - "get-intrinsic": "^1.1.3", - "has-symbols": "^1.0.3", - "is-arguments": "^1.1.1", - "is-map": "^2.0.2", - "is-set": "^2.0.2", - "is-string": "^1.0.7", - "isarray": "^2.0.5", - "stop-iteration-iterator": "^1.0.0" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/es-iterator-helpers": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/es-iterator-helpers/-/es-iterator-helpers-1.2.1.tgz", @@ -13201,23 +13128,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/object-is": { - "version": "1.1.6", - "resolved": "https://registry.npmjs.org/object-is/-/object-is-1.1.6.tgz", - "integrity": "sha512-F8cZ+KfGlSGi09lJT7/Nd6KJZ9ygtvYC0/UYYLI9nmQKLMnydpB9yvbv9K1uSkEu7FU9vYPmVwLg328tX+ot3Q==", - "dev": true, - "license": "MIT", - "dependencies": { - "call-bind": "^1.0.7", - "define-properties": "^1.2.1" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/object-keys": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/object-keys/-/object-keys-1.1.1.tgz", @@ -14923,20 +14833,6 @@ "stacktrace-gps": "^3.0.4" } }, - "node_modules/stop-iteration-iterator": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/stop-iteration-iterator/-/stop-iteration-iterator-1.1.0.tgz", - "integrity": "sha512-eLoXW/DHyl62zxY4SCaIgnRhuMr6ri4juEYARS8E6sCEqzKpOiE521Ucofdx+KnDZl5xmvGYaaKCk5FEOxJCoQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "es-errors": "^1.3.0", - "internal-slot": "^1.1.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/storybook": { "version": "8.5.2", "resolved": "https://registry.npmjs.org/storybook/-/storybook-8.5.2.tgz", diff --git a/webview-ui/package.json b/webview-ui/package.json index 077bff5609..d7a5765690 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -48,9 +48,9 @@ "@storybook/react": "^8.5.2", "@storybook/react-vite": "^8.5.2", "@storybook/test": "^8.5.2", - "@testing-library/jest-dom": "^5.17.0", - "@testing-library/react": "^13.4.0", - "@testing-library/user-event": "^13.5.0", + "@testing-library/jest-dom": "^6.6.3", + "@testing-library/react": "^16.2.0", + "@testing-library/user-event": "^14.6.1", "@types/jest": "^27.5.2", "@types/node": "^18.0.0", "@types/react": "^18.3.18", diff --git a/webview-ui/src/components/history/__tests__/HistoryView.test.tsx b/webview-ui/src/components/history/__tests__/HistoryView.test.tsx index 1f1e4c7daf..d47c5e460b 100644 --- a/webview-ui/src/components/history/__tests__/HistoryView.test.tsx +++ b/webview-ui/src/components/history/__tests__/HistoryView.test.tsx @@ -1,13 +1,13 @@ -import React from "react" -import { render, screen, fireEvent, within, waitFor } from "@testing-library/react" -import userEvent from "@testing-library/user-event" +// cd webview-ui && npx jest src/components/history/__tests__/HistoryView.test.ts + +import { render, screen, fireEvent, within, act } from "@testing-library/react" import HistoryView from "../HistoryView" import { useExtensionState } from "../../../context/ExtensionStateContext" import { vscode } from "../../../utils/vscode" -// Mock dependencies jest.mock("../../../context/ExtensionStateContext") jest.mock("../../../utils/vscode") + jest.mock("react-virtuoso", () => ({ Virtuoso: ({ data, itemContent }: any) => (
@@ -41,21 +41,21 @@ const mockTaskHistory = [ ] describe("HistoryView", () => { - beforeEach(() => { - // Reset all mocks before each test - jest.clearAllMocks() + beforeAll(() => { jest.useFakeTimers() + }) - // Mock useExtensionState implementation + afterAll(() => { + jest.useRealTimers() + }) + + beforeEach(() => { + jest.clearAllMocks() ;(useExtensionState as jest.Mock).mockReturnValue({ taskHistory: mockTaskHistory, }) }) - afterEach(() => { - jest.useRealTimers() - }) - it("renders history items correctly", () => { const onDone = jest.fn() render() @@ -67,7 +67,7 @@ describe("HistoryView", () => { expect(screen.getByText("Test task 2")).toBeInTheDocument() }) - it("handles search functionality", async () => { + it("handles search functionality", () => { const onDone = jest.fn() render() @@ -76,17 +76,23 @@ describe("HistoryView", () => { const radioGroup = screen.getByRole("radiogroup") // Type in search - await userEvent.type(searchInput, "task 1") + fireEvent.input(searchInput, { target: { value: "task 1" } }) + + // Advance timers to process search state update + jest.advanceTimersByTime(100) // Check if sort option automatically changes to "Most Relevant" const mostRelevantRadio = within(radioGroup).getByLabelText("Most Relevant") expect(mostRelevantRadio).not.toBeDisabled() - // Click and wait for radio update + // Click the radio button fireEvent.click(mostRelevantRadio) - // Wait for radio button to be checked - const updatedRadio = await within(radioGroup).findByRole("radio", { name: "Most Relevant", checked: true }) + // Advance timers to process radio button state update + jest.advanceTimersByTime(100) + + // Verify radio button is checked + const updatedRadio = within(radioGroup).getByRole("radio", { name: "Most Relevant", checked: true }) expect(updatedRadio).toBeInTheDocument() }) @@ -148,6 +154,7 @@ describe("HistoryView", () => { }) it("handles task copying", async () => { + // Setup clipboard mock that resolves immediately const mockClipboard = { writeText: jest.fn().mockResolvedValue(undefined), } @@ -161,20 +168,29 @@ describe("HistoryView", () => { fireEvent.mouseEnter(taskContainer) const copyButton = within(taskContainer).getByTitle("Copy Prompt") - await userEvent.click(copyButton) - // Verify clipboard API was called + // Click the copy button and wait for clipboard operation + await act(async () => { + fireEvent.click(copyButton) + // Let the clipboard Promise resolve + await Promise.resolve() + // Let React process the first state update + await Promise.resolve() + }) + + // Verify clipboard was called expect(navigator.clipboard.writeText).toHaveBeenCalledWith("Test task 1") - // Wait for copy modal to appear - const copyModal = await screen.findByText("Prompt Copied to Clipboard") - expect(copyModal).toBeInTheDocument() + // Verify modal appears immediately after clipboard operation + expect(screen.getByText("Prompt Copied to Clipboard")).toBeInTheDocument() - // Fast-forward timers and wait for modal to disappear - jest.advanceTimersByTime(2000) - await waitFor(() => { - expect(screen.queryByText("Prompt Copied to Clipboard")).not.toBeInTheDocument() + // Advance timer to trigger the setTimeout for modal disappearance + act(() => { + jest.advanceTimersByTime(2000) }) + + // Verify modal is gone + expect(screen.queryByText("Prompt Copied to Clipboard")).not.toBeInTheDocument() }) it("formats dates correctly", () => { diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 277a532c48..4f9c8e1b78 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -1,8 +1,9 @@ -import { Checkbox, Dropdown, Pane } from "vscrui" -import type { DropdownOption } from "vscrui" -import { VSCodeLink, VSCodeRadio, VSCodeRadioGroup, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" -import { Fragment, memo, useCallback, useEffect, useMemo, useState } from "react" +import { memo, useCallback, useEffect, useMemo, useState } from "react" import { useEvent, useInterval } from "react-use" +import { Checkbox, Dropdown, Pane, type DropdownOption } from "vscrui" +import { VSCodeLink, VSCodeRadio, VSCodeRadioGroup, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" +import * as vscodemodels from "vscode" + import { ApiConfiguration, ModelInfo, @@ -32,14 +33,12 @@ import { import { ExtensionMessage } from "../../../../src/shared/ExtensionMessage" import { useExtensionState } from "../../context/ExtensionStateContext" import { vscode } from "../../utils/vscode" -import * as vscodemodels from "vscode" import VSCodeButtonLink from "../common/VSCodeButtonLink" -import OpenRouterModelPicker, { - ModelDescriptionMarkdown, - OPENROUTER_MODEL_PICKER_Z_INDEX, -} from "./OpenRouterModelPicker" +import { OpenRouterModelPicker } from "./OpenRouterModelPicker" import OpenAiModelPicker from "./OpenAiModelPicker" -import GlamaModelPicker from "./GlamaModelPicker" +import { GlamaModelPicker } from "./GlamaModelPicker" +import { ModelInfoView } from "./ModelInfoView" +import { DROPDOWN_Z_INDEX } from "./styles" interface ApiOptionsProps { apiErrorMessage?: string @@ -137,7 +136,7 @@ const ApiOptions = ({ apiErrorMessage, modelIdErrorMessage }: ApiOptionsProps) = }, }) }} - style={{ minWidth: 130, position: "relative", zIndex: OPENROUTER_MODEL_PICKER_Z_INDEX + 1 }} + style={{ minWidth: 130, position: "relative", zIndex: DROPDOWN_Z_INDEX + 1 }} options={[ { value: "openrouter", label: "OpenRouter" }, { value: "anthropic", label: "Anthropic" }, @@ -1386,136 +1385,6 @@ export function getOpenRouterAuthUrl(uriScheme?: string) { return `https://openrouter.ai/auth?callback_url=${uriScheme || "vscode"}://rooveterinaryinc.roo-cline/openrouter` } -export const formatPrice = (price: number) => { - return new Intl.NumberFormat("en-US", { - style: "currency", - currency: "USD", - minimumFractionDigits: 2, - maximumFractionDigits: 2, - }).format(price) -} - -export const ModelInfoView = ({ - selectedModelId, - modelInfo, - isDescriptionExpanded, - setIsDescriptionExpanded, -}: { - selectedModelId: string - modelInfo: ModelInfo - isDescriptionExpanded: boolean - setIsDescriptionExpanded: (isExpanded: boolean) => void -}) => { - const isGemini = Object.keys(geminiModels).includes(selectedModelId) - - const infoItems = [ - modelInfo.description && ( - - ), - , - , - !isGemini && ( - - ), - modelInfo.maxTokens !== undefined && modelInfo.maxTokens > 0 && ( - - Max output: {modelInfo.maxTokens?.toLocaleString()} tokens - - ), - modelInfo.inputPrice !== undefined && modelInfo.inputPrice > 0 && ( - - Input price: {formatPrice(modelInfo.inputPrice)}/million tokens - - ), - modelInfo.supportsPromptCache && modelInfo.cacheWritesPrice && ( - - Cache writes price:{" "} - {formatPrice(modelInfo.cacheWritesPrice || 0)}/million tokens - - ), - modelInfo.supportsPromptCache && modelInfo.cacheReadsPrice && ( - - Cache reads price:{" "} - {formatPrice(modelInfo.cacheReadsPrice || 0)}/million tokens - - ), - modelInfo.outputPrice !== undefined && modelInfo.outputPrice > 0 && ( - - Output price: {formatPrice(modelInfo.outputPrice)}/million - tokens - - ), - isGemini && ( - - * Free up to {selectedModelId && selectedModelId.includes("flash") ? "15" : "2"} requests per minute. - After that, billing depends on prompt size.{" "} - - For more info, see pricing details. - - - ), - ].filter(Boolean) - - return ( -

- {infoItems.map((item, index) => ( - - {item} - {index < infoItems.length - 1 &&
} -
- ))} -

- ) -} - -const ModelInfoSupportsItem = ({ - isSupported, - supportsLabel, - doesNotSupportLabel, -}: { - isSupported: boolean - supportsLabel: string - doesNotSupportLabel: string -}) => ( - - - {isSupported ? supportsLabel : doesNotSupportLabel} - -) - export function normalizeApiConfiguration(apiConfiguration?: ApiConfiguration) { const provider = apiConfiguration?.apiProvider || "anthropic" const modelId = apiConfiguration?.apiModelId diff --git a/webview-ui/src/components/settings/GlamaModelPicker.tsx b/webview-ui/src/components/settings/GlamaModelPicker.tsx index 07d75bec79..cb813a0d05 100644 --- a/webview-ui/src/components/settings/GlamaModelPicker.tsx +++ b/webview-ui/src/components/settings/GlamaModelPicker.tsx @@ -1,415 +1,15 @@ -import { VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" -import debounce from "debounce" -import { Fzf } from "fzf" -import React, { KeyboardEvent, memo, useEffect, useMemo, useRef, useState } from "react" -import { useRemark } from "react-remark" -import { useMount } from "react-use" -import styled from "styled-components" +import { ModelPicker } from "./ModelPicker" import { glamaDefaultModelId } from "../../../../src/shared/api" -import { useExtensionState } from "../../context/ExtensionStateContext" -import { vscode } from "../../utils/vscode" -import { highlightFzfMatch } from "../../utils/highlight" -import { ModelInfoView, normalizeApiConfiguration } from "./ApiOptions" -const GlamaModelPicker: React.FC = () => { - const { apiConfiguration, setApiConfiguration, glamaModels, onUpdateApiConfig } = useExtensionState() - const [searchTerm, setSearchTerm] = useState(apiConfiguration?.glamaModelId || glamaDefaultModelId) - const [isDropdownVisible, setIsDropdownVisible] = useState(false) - const [selectedIndex, setSelectedIndex] = useState(-1) - const dropdownRef = useRef(null) - const itemRefs = useRef<(HTMLDivElement | null)[]>([]) - const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false) - const dropdownListRef = useRef(null) - - const handleModelChange = (newModelId: string) => { - // could be setting invalid model id/undefined info but validation will catch it - const apiConfig = { - ...apiConfiguration, - glamaModelId: newModelId, - glamaModelInfo: glamaModels[newModelId], - } - setApiConfiguration(apiConfig) - onUpdateApiConfig(apiConfig) - - setSearchTerm(newModelId) - } - - const { selectedModelId, selectedModelInfo } = useMemo(() => { - return normalizeApiConfiguration(apiConfiguration) - }, [apiConfiguration]) - - useEffect(() => { - if (apiConfiguration?.glamaModelId && apiConfiguration?.glamaModelId !== searchTerm) { - setSearchTerm(apiConfiguration?.glamaModelId) - } - }, [apiConfiguration, searchTerm]) - - const debouncedRefreshModels = useMemo( - () => - debounce(() => { - vscode.postMessage({ type: "refreshGlamaModels" }) - }, 50), - [], - ) - - useMount(() => { - debouncedRefreshModels() - - // Cleanup debounced function - return () => { - debouncedRefreshModels.clear() - } - }) - - useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { - if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { - setIsDropdownVisible(false) - } - } - - document.addEventListener("mousedown", handleClickOutside) - return () => { - document.removeEventListener("mousedown", handleClickOutside) - } - }, []) - - const modelIds = useMemo(() => { - return Object.keys(glamaModels).sort((a, b) => a.localeCompare(b)) - }, [glamaModels]) - - const searchableItems = useMemo(() => { - return modelIds.map((id) => ({ - id, - html: id, - })) - }, [modelIds]) - - const fzf = useMemo(() => { - return new Fzf(searchableItems, { - selector: (item) => item.html, - }) - }, [searchableItems]) - - const modelSearchResults = useMemo(() => { - if (!searchTerm) return searchableItems - - const searchResults = fzf.find(searchTerm) - return searchResults.map((result) => ({ - ...result.item, - html: highlightFzfMatch(result.item.html, Array.from(result.positions), "model-item-highlight"), - })) - }, [searchableItems, searchTerm, fzf]) - - const handleKeyDown = (event: KeyboardEvent) => { - if (!isDropdownVisible) return - - switch (event.key) { - case "ArrowDown": - event.preventDefault() - setSelectedIndex((prev) => (prev < modelSearchResults.length - 1 ? prev + 1 : prev)) - break - case "ArrowUp": - event.preventDefault() - setSelectedIndex((prev) => (prev > 0 ? prev - 1 : prev)) - break - case "Enter": - event.preventDefault() - if (selectedIndex >= 0 && selectedIndex < modelSearchResults.length) { - handleModelChange(modelSearchResults[selectedIndex].id) - setIsDropdownVisible(false) - } - break - case "Escape": - setIsDropdownVisible(false) - setSelectedIndex(-1) - break - } - } - - const hasInfo = useMemo(() => { - return modelIds.some((id) => id.toLowerCase() === searchTerm.toLowerCase()) - }, [modelIds, searchTerm]) - - useEffect(() => { - setSelectedIndex(-1) - if (dropdownListRef.current) { - dropdownListRef.current.scrollTop = 0 - } - }, [searchTerm]) - - useEffect(() => { - if (selectedIndex >= 0 && itemRefs.current[selectedIndex]) { - itemRefs.current[selectedIndex]?.scrollIntoView({ - block: "nearest", - behavior: "smooth", - }) - } - }, [selectedIndex]) - - return ( - <> - -
- - - { - handleModelChange((e.target as HTMLInputElement)?.value?.toLowerCase()) - setIsDropdownVisible(true) - }} - onFocus={() => setIsDropdownVisible(true)} - onKeyDown={handleKeyDown} - style={{ width: "100%", zIndex: GLAMA_MODEL_PICKER_Z_INDEX, position: "relative" }}> - {searchTerm && ( -
{ - handleModelChange("") - setIsDropdownVisible(true) - }} - slot="end" - style={{ - display: "flex", - justifyContent: "center", - alignItems: "center", - height: "100%", - }} - /> - )} - - {isDropdownVisible && ( - - {modelSearchResults.map((item, index) => ( - (itemRefs.current[index] = el)} - isSelected={index === selectedIndex} - onMouseEnter={() => setSelectedIndex(index)} - onClick={() => { - handleModelChange(item.id) - setIsDropdownVisible(false) - }} - dangerouslySetInnerHTML={{ - __html: item.html, - }} - /> - ))} - - )} - -
- - {hasInfo ? ( - - ) : ( -

- The extension automatically fetches the latest list of models available on{" "} - - Glama. - - If you're unsure which model to choose, Roo Code works best with{" "} - handleModelChange("anthropic/claude-3.5-sonnet")}> - anthropic/claude-3.5-sonnet. - - You can also try searching "free" for no-cost options currently available. -

- )} - - ) -} - -export default GlamaModelPicker - -// Dropdown - -const DropdownWrapper = styled.div` - position: relative; - width: 100%; -` - -export const GLAMA_MODEL_PICKER_Z_INDEX = 1_000 - -const DropdownList = styled.div` - position: absolute; - top: calc(100% - 3px); - left: 0; - width: calc(100% - 2px); - max-height: 200px; - overflow-y: auto; - background-color: var(--vscode-dropdown-background); - border: 1px solid var(--vscode-list-activeSelectionBackground); - z-index: ${GLAMA_MODEL_PICKER_Z_INDEX - 1}; - border-bottom-left-radius: 3px; - border-bottom-right-radius: 3px; -` - -const DropdownItem = styled.div<{ isSelected: boolean }>` - padding: 5px 10px; - cursor: pointer; - word-break: break-all; - white-space: normal; - - background-color: ${({ isSelected }) => (isSelected ? "var(--vscode-list-activeSelectionBackground)" : "inherit")}; - - &:hover { - background-color: var(--vscode-list-activeSelectionBackground); - } -` - -// Markdown - -const StyledMarkdown = styled.div` - font-family: - var(--vscode-font-family), - system-ui, - -apple-system, - BlinkMacSystemFont, - "Segoe UI", - Roboto, - Oxygen, - Ubuntu, - Cantarell, - "Open Sans", - "Helvetica Neue", - sans-serif; - font-size: 12px; - color: var(--vscode-descriptionForeground); - - p, - li, - ol, - ul { - line-height: 1.25; - margin: 0; - } - - ol, - ul { - padding-left: 1.5em; - margin-left: 0; - } - - p { - white-space: pre-wrap; - } - - a { - text-decoration: none; - } - a { - &:hover { - text-decoration: underline; - } - } -` - -export const ModelDescriptionMarkdown = memo( - ({ - markdown, - key, - isExpanded, - setIsExpanded, - }: { - markdown?: string - key: string - isExpanded: boolean - setIsExpanded: (isExpanded: boolean) => void - }) => { - const [reactContent, setMarkdown] = useRemark() - const [showSeeMore, setShowSeeMore] = useState(false) - const textContainerRef = useRef(null) - const textRef = useRef(null) - - useEffect(() => { - setMarkdown(markdown || "") - }, [markdown, setMarkdown]) - - useEffect(() => { - if (textRef.current && textContainerRef.current) { - const { scrollHeight } = textRef.current - const { clientHeight } = textContainerRef.current - const isOverflowing = scrollHeight > clientHeight - setShowSeeMore(isOverflowing) - } - }, [reactContent, setIsExpanded]) - - return ( - -
-
- {reactContent} -
- {!isExpanded && showSeeMore && ( -
-
- setIsExpanded(true)}> - See more - -
- )} -
- - ) - }, +export const GlamaModelPicker = () => ( + ) diff --git a/webview-ui/src/components/settings/ModelDescriptionMarkdown.tsx b/webview-ui/src/components/settings/ModelDescriptionMarkdown.tsx new file mode 100644 index 0000000000..351464f706 --- /dev/null +++ b/webview-ui/src/components/settings/ModelDescriptionMarkdown.tsx @@ -0,0 +1,90 @@ +import { VSCodeLink } from "@vscode/webview-ui-toolkit/react" +import { memo, useEffect, useRef, useState } from "react" +import { useRemark } from "react-remark" + +import { StyledMarkdown } from "./styles" + +export const ModelDescriptionMarkdown = memo( + ({ + markdown, + key, + isExpanded, + setIsExpanded, + }: { + markdown?: string + key: string + isExpanded: boolean + setIsExpanded: (isExpanded: boolean) => void + }) => { + const [reactContent, setMarkdown] = useRemark() + const [showSeeMore, setShowSeeMore] = useState(false) + const textContainerRef = useRef(null) + const textRef = useRef(null) + + useEffect(() => { + setMarkdown(markdown || "") + }, [markdown, setMarkdown]) + + useEffect(() => { + if (textRef.current && textContainerRef.current) { + const { scrollHeight } = textRef.current + const { clientHeight } = textContainerRef.current + const isOverflowing = scrollHeight > clientHeight + setShowSeeMore(isOverflowing) + } + }, [reactContent, setIsExpanded]) + + return ( + +
+
+ {reactContent} +
+ {!isExpanded && showSeeMore && ( +
+
+ setIsExpanded(true)}> + See more + +
+ )} +
+ + ) + }, +) diff --git a/webview-ui/src/components/settings/ModelInfoView.tsx b/webview-ui/src/components/settings/ModelInfoView.tsx new file mode 100644 index 0000000000..397d04e02f --- /dev/null +++ b/webview-ui/src/components/settings/ModelInfoView.tsx @@ -0,0 +1,124 @@ +import { VSCodeLink } from "@vscode/webview-ui-toolkit/react" +import { Fragment } from "react" + +import { ModelInfo, geminiModels } from "../../../../src/shared/api" +import { ModelDescriptionMarkdown } from "./ModelDescriptionMarkdown" +import { formatPrice } from "../../utils/formatPrice" + +export const ModelInfoView = ({ + selectedModelId, + modelInfo, + isDescriptionExpanded, + setIsDescriptionExpanded, +}: { + selectedModelId: string + modelInfo: ModelInfo + isDescriptionExpanded: boolean + setIsDescriptionExpanded: (isExpanded: boolean) => void +}) => { + const isGemini = Object.keys(geminiModels).includes(selectedModelId) + + const infoItems = [ + modelInfo.description && ( + + ), + , + , + !isGemini && ( + + ), + modelInfo.maxTokens !== undefined && modelInfo.maxTokens > 0 && ( + + Max output: {modelInfo.maxTokens?.toLocaleString()} tokens + + ), + modelInfo.inputPrice !== undefined && modelInfo.inputPrice > 0 && ( + + Input price: {formatPrice(modelInfo.inputPrice)}/million tokens + + ), + modelInfo.supportsPromptCache && modelInfo.cacheWritesPrice && ( + + Cache writes price:{" "} + {formatPrice(modelInfo.cacheWritesPrice || 0)}/million tokens + + ), + modelInfo.supportsPromptCache && modelInfo.cacheReadsPrice && ( + + Cache reads price:{" "} + {formatPrice(modelInfo.cacheReadsPrice || 0)}/million tokens + + ), + modelInfo.outputPrice !== undefined && modelInfo.outputPrice > 0 && ( + + Output price: {formatPrice(modelInfo.outputPrice)}/million + tokens + + ), + isGemini && ( + + * Free up to {selectedModelId && selectedModelId.includes("flash") ? "15" : "2"} requests per minute. + After that, billing depends on prompt size.{" "} + + For more info, see pricing details. + + + ), + ].filter(Boolean) + + return ( +
+ {infoItems.map((item, index) => ( + + {item} + {index < infoItems.length - 1 &&
} +
+ ))} +
+ ) +} + +const ModelInfoSupportsItem = ({ + isSupported, + supportsLabel, + doesNotSupportLabel, +}: { + isSupported: boolean + supportsLabel: string + doesNotSupportLabel: string +}) => ( + + + {isSupported ? supportsLabel : doesNotSupportLabel} + +) diff --git a/webview-ui/src/components/settings/ModelPicker.tsx b/webview-ui/src/components/settings/ModelPicker.tsx new file mode 100644 index 0000000000..db306ac7ce --- /dev/null +++ b/webview-ui/src/components/settings/ModelPicker.tsx @@ -0,0 +1,130 @@ +import { VSCodeLink } from "@vscode/webview-ui-toolkit/react" +import debounce from "debounce" +import { useMemo, useState, useCallback, useEffect } from "react" +import { useMount } from "react-use" +import { CaretSortIcon, CheckIcon } from "@radix-ui/react-icons" + +import { cn } from "@/lib/utils" +import { + Button, + Command, + CommandEmpty, + CommandGroup, + CommandInput, + CommandItem, + CommandList, + Popover, + PopoverContent, + PopoverTrigger, +} from "@/components/ui" + +import { useExtensionState } from "../../context/ExtensionStateContext" +import { vscode } from "../../utils/vscode" +import { normalizeApiConfiguration } from "./ApiOptions" +import { ModelInfoView } from "./ModelInfoView" + +interface ModelPickerProps { + defaultModelId: string + modelsKey: "glamaModels" | "openRouterModels" + configKey: "glamaModelId" | "openRouterModelId" + infoKey: "glamaModelInfo" | "openRouterModelInfo" + refreshMessageType: "refreshGlamaModels" | "refreshOpenRouterModels" + serviceName: string + serviceUrl: string + recommendedModel: string +} + +export const ModelPicker = ({ + defaultModelId, + modelsKey, + configKey, + infoKey, + refreshMessageType, + serviceName, + serviceUrl, + recommendedModel, +}: ModelPickerProps) => { + const [open, setOpen] = useState(false) + const [value, setValue] = useState(defaultModelId) + const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false) + + const { apiConfiguration, setApiConfiguration, [modelsKey]: models, onUpdateApiConfig } = useExtensionState() + const modelIds = useMemo(() => Object.keys(models).sort((a, b) => a.localeCompare(b)), [models]) + + const { selectedModelId, selectedModelInfo } = useMemo( + () => normalizeApiConfiguration(apiConfiguration), + [apiConfiguration], + ) + + const onSelect = useCallback( + (modelId: string) => { + const apiConfig = { ...apiConfiguration, [configKey]: modelId, [infoKey]: models[modelId] } + setApiConfiguration(apiConfig) + onUpdateApiConfig(apiConfig) + setValue(modelId) + setOpen(false) + }, + [apiConfiguration, configKey, infoKey, models, onUpdateApiConfig, setApiConfiguration], + ) + + const debouncedRefreshModels = useMemo( + () => debounce(() => vscode.postMessage({ type: refreshMessageType }), 50), + [refreshMessageType], + ) + + useMount(() => { + debouncedRefreshModels() + return () => debouncedRefreshModels.clear() + }) + + useEffect(() => setValue(selectedModelId), [selectedModelId]) + + return ( + <> +
Model
+ + + + + + + + + No model found. + + {modelIds.map((model) => ( + + {model} + + + ))} + + + + + + {selectedModelId && selectedModelInfo && ( + + )} +

+ The extension automatically fetches the latest list of models available on{" "} + + {serviceName}. + + If you're unsure which model to choose, Roo Code works best with{" "} + onSelect(recommendedModel)}>{recommendedModel}. + You can also try searching "free" for no-cost options currently available. +

+ + ) +} diff --git a/webview-ui/src/components/settings/OpenAiModelPicker.tsx b/webview-ui/src/components/settings/OpenAiModelPicker.tsx index 721c45d183..a8243547c6 100644 --- a/webview-ui/src/components/settings/OpenAiModelPicker.tsx +++ b/webview-ui/src/components/settings/OpenAiModelPicker.tsx @@ -1,12 +1,12 @@ -import { VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" -import { Fzf } from "fzf" -import React, { KeyboardEvent, memo, useEffect, useMemo, useRef, useState } from "react" +import { VSCodeTextField } from "@vscode/webview-ui-toolkit/react" import debounce from "debounce" -import { useRemark } from "react-remark" -import styled from "styled-components" +import { Fzf } from "fzf" +import React, { KeyboardEvent, useEffect, useMemo, useRef, useState } from "react" + import { useExtensionState } from "../../context/ExtensionStateContext" import { vscode } from "../../utils/vscode" import { highlightFzfMatch } from "../../utils/highlight" +import { DropdownWrapper, DropdownList, DropdownItem } from "./styles" const OpenAiModelPicker: React.FC = () => { const { apiConfiguration, setApiConfiguration, openAiModels, onUpdateApiConfig } = useExtensionState() @@ -23,6 +23,7 @@ const OpenAiModelPicker: React.FC = () => { ...apiConfiguration, openAiModelId: newModelId, } + setApiConfiguration(apiConfig) onUpdateApiConfig(apiConfig) setSearchTerm(newModelId) @@ -185,12 +186,12 @@ const OpenAiModelPicker: React.FC = () => { )} {isDropdownVisible && ( - + {modelSearchResults.map((item, index) => ( (itemRefs.current[index] = el)} - isSelected={index === selectedIndex} onMouseEnter={() => setSelectedIndex(index)} onClick={() => { handleModelChange(item.id) @@ -213,177 +214,4 @@ export default OpenAiModelPicker // Dropdown -const DropdownWrapper = styled.div` - position: relative; - width: 100%; -` - export const OPENAI_MODEL_PICKER_Z_INDEX = 1_000 - -const DropdownList = styled.div` - position: absolute; - top: calc(100% - 3px); - left: 0; - width: calc(100% - 2px); - max-height: 200px; - overflow-y: auto; - background-color: var(--vscode-dropdown-background); - border: 1px solid var(--vscode-list-activeSelectionBackground); - z-index: ${OPENAI_MODEL_PICKER_Z_INDEX - 1}; - border-bottom-left-radius: 3px; - border-bottom-right-radius: 3px; -` - -const DropdownItem = styled.div<{ isSelected: boolean }>` - padding: 5px 10px; - cursor: pointer; - word-break: break-all; - white-space: normal; - - background-color: ${({ isSelected }) => (isSelected ? "var(--vscode-list-activeSelectionBackground)" : "inherit")}; - - &:hover { - background-color: var(--vscode-list-activeSelectionBackground); - } -` - -// Markdown - -const StyledMarkdown = styled.div` - font-family: - var(--vscode-font-family), - system-ui, - -apple-system, - BlinkMacSystemFont, - "Segoe UI", - Roboto, - Oxygen, - Ubuntu, - Cantarell, - "Open Sans", - "Helvetica Neue", - sans-serif; - font-size: 12px; - color: var(--vscode-descriptionForeground); - - p, - li, - ol, - ul { - line-height: 1.25; - margin: 0; - } - - ol, - ul { - padding-left: 1.5em; - margin-left: 0; - } - - p { - white-space: pre-wrap; - } - - a { - text-decoration: none; - } - a { - &:hover { - text-decoration: underline; - } - } -` - -export const ModelDescriptionMarkdown = memo( - ({ - markdown, - key, - isExpanded, - setIsExpanded, - }: { - markdown?: string - key: string - isExpanded: boolean - setIsExpanded: (isExpanded: boolean) => void - }) => { - const [reactContent, setMarkdown] = useRemark() - // const [isExpanded, setIsExpanded] = useState(false) - const [showSeeMore, setShowSeeMore] = useState(false) - const textContainerRef = useRef(null) - const textRef = useRef(null) - - useEffect(() => { - setMarkdown(markdown || "") - }, [markdown, setMarkdown]) - - useEffect(() => { - if (textRef.current && textContainerRef.current) { - const { scrollHeight } = textRef.current - const { clientHeight } = textContainerRef.current - const isOverflowing = scrollHeight > clientHeight - setShowSeeMore(isOverflowing) - // if (!isOverflowing) { - // setIsExpanded(false) - // } - } - }, [reactContent, setIsExpanded]) - - return ( - -
-
- {reactContent} -
- {!isExpanded && showSeeMore && ( -
-
- setIsExpanded(true)}> - See more - -
- )} -
- - ) - }, -) diff --git a/webview-ui/src/components/settings/OpenRouterModelPicker.tsx b/webview-ui/src/components/settings/OpenRouterModelPicker.tsx index a1761cd618..9111407cd6 100644 --- a/webview-ui/src/components/settings/OpenRouterModelPicker.tsx +++ b/webview-ui/src/components/settings/OpenRouterModelPicker.tsx @@ -1,437 +1,15 @@ -import { VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" -import debounce from "debounce" -import { Fzf } from "fzf" -import React, { KeyboardEvent, memo, useEffect, useMemo, useRef, useState } from "react" -import { useRemark } from "react-remark" -import { useMount } from "react-use" -import styled from "styled-components" +import { ModelPicker } from "./ModelPicker" import { openRouterDefaultModelId } from "../../../../src/shared/api" -import { useExtensionState } from "../../context/ExtensionStateContext" -import { vscode } from "../../utils/vscode" -import { highlightFzfMatch } from "../../utils/highlight" -import { ModelInfoView, normalizeApiConfiguration } from "./ApiOptions" -const OpenRouterModelPicker: React.FC = () => { - const { apiConfiguration, setApiConfiguration, openRouterModels, onUpdateApiConfig } = useExtensionState() - const [searchTerm, setSearchTerm] = useState(apiConfiguration?.openRouterModelId || openRouterDefaultModelId) - const [isDropdownVisible, setIsDropdownVisible] = useState(false) - const [selectedIndex, setSelectedIndex] = useState(-1) - const dropdownRef = useRef(null) - const itemRefs = useRef<(HTMLDivElement | null)[]>([]) - const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false) - const dropdownListRef = useRef(null) - - const handleModelChange = (newModelId: string) => { - // could be setting invalid model id/undefined info but validation will catch it - const apiConfig = { - ...apiConfiguration, - openRouterModelId: newModelId, - openRouterModelInfo: openRouterModels[newModelId], - } - - setApiConfiguration(apiConfig) - onUpdateApiConfig(apiConfig) - setSearchTerm(newModelId) - } - - const { selectedModelId, selectedModelInfo } = useMemo(() => { - return normalizeApiConfiguration(apiConfiguration) - }, [apiConfiguration]) - - useEffect(() => { - if (apiConfiguration?.openRouterModelId && apiConfiguration?.openRouterModelId !== searchTerm) { - setSearchTerm(apiConfiguration?.openRouterModelId) - } - }, [apiConfiguration, searchTerm]) - - const debouncedRefreshModels = useMemo( - () => - debounce(() => { - vscode.postMessage({ type: "refreshOpenRouterModels" }) - }, 50), - [], - ) - - useMount(() => { - debouncedRefreshModels() - - // Cleanup debounced function - return () => { - debouncedRefreshModels.clear() - } - }) - - useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { - if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { - setIsDropdownVisible(false) - } - } - - document.addEventListener("mousedown", handleClickOutside) - return () => { - document.removeEventListener("mousedown", handleClickOutside) - } - }, []) - - const modelIds = useMemo(() => { - return Object.keys(openRouterModels).sort((a, b) => a.localeCompare(b)) - }, [openRouterModels]) - - const searchableItems = useMemo(() => { - return modelIds.map((id) => ({ - id, - html: id, - })) - }, [modelIds]) - - const fzf = useMemo(() => { - return new Fzf(searchableItems, { - selector: (item) => item.html, - }) - }, [searchableItems]) - - const modelSearchResults = useMemo(() => { - if (!searchTerm) return searchableItems - - const searchResults = fzf.find(searchTerm) - return searchResults.map((result) => ({ - ...result.item, - html: highlightFzfMatch(result.item.html, Array.from(result.positions), "model-item-highlight"), - })) - }, [searchableItems, searchTerm, fzf]) - - const handleKeyDown = (event: KeyboardEvent) => { - if (!isDropdownVisible) return - - switch (event.key) { - case "ArrowDown": - event.preventDefault() - setSelectedIndex((prev) => (prev < modelSearchResults.length - 1 ? prev + 1 : prev)) - break - case "ArrowUp": - event.preventDefault() - setSelectedIndex((prev) => (prev > 0 ? prev - 1 : prev)) - break - case "Enter": - event.preventDefault() - if (selectedIndex >= 0 && selectedIndex < modelSearchResults.length) { - handleModelChange(modelSearchResults[selectedIndex].id) - setIsDropdownVisible(false) - } - break - case "Escape": - setIsDropdownVisible(false) - setSelectedIndex(-1) - break - } - } - - const hasInfo = useMemo(() => { - return modelIds.some((id) => id.toLowerCase() === searchTerm.toLowerCase()) - }, [modelIds, searchTerm]) - - useEffect(() => { - setSelectedIndex(-1) - if (dropdownListRef.current) { - dropdownListRef.current.scrollTop = 0 - } - }, [searchTerm]) - - useEffect(() => { - if (selectedIndex >= 0 && itemRefs.current[selectedIndex]) { - itemRefs.current[selectedIndex]?.scrollIntoView({ - block: "nearest", - behavior: "smooth", - }) - } - }, [selectedIndex]) - - return ( - <> - -
- - - { - handleModelChange((e.target as HTMLInputElement)?.value?.toLowerCase()) - setIsDropdownVisible(true) - }} - onFocus={() => setIsDropdownVisible(true)} - onKeyDown={handleKeyDown} - style={{ width: "100%", zIndex: OPENROUTER_MODEL_PICKER_Z_INDEX, position: "relative" }}> - {searchTerm && ( -
{ - handleModelChange("") - setIsDropdownVisible(true) - }} - slot="end" - style={{ - display: "flex", - justifyContent: "center", - alignItems: "center", - height: "100%", - }} - /> - )} - - {isDropdownVisible && ( - - {modelSearchResults.map((item, index) => ( - (itemRefs.current[index] = el)} - isSelected={index === selectedIndex} - onMouseEnter={() => setSelectedIndex(index)} - onClick={() => { - handleModelChange(item.id) - setIsDropdownVisible(false) - }} - dangerouslySetInnerHTML={{ - __html: item.html, - }} - /> - ))} - - )} - -
- - {hasInfo ? ( - - ) : ( -

- The extension automatically fetches the latest list of models available on{" "} - - OpenRouter. - - If you're unsure which model to choose, Roo Code works best with{" "} - handleModelChange("anthropic/claude-3.5-sonnet:beta")}> - anthropic/claude-3.5-sonnet:beta. - - You can also try searching "free" for no-cost options currently available. -

- )} - - ) -} - -export default OpenRouterModelPicker - -// Dropdown - -const DropdownWrapper = styled.div` - position: relative; - width: 100%; -` - -export const OPENROUTER_MODEL_PICKER_Z_INDEX = 1_000 - -const DropdownList = styled.div` - position: absolute; - top: calc(100% - 3px); - left: 0; - width: calc(100% - 2px); - max-height: 200px; - overflow-y: auto; - background-color: var(--vscode-dropdown-background); - border: 1px solid var(--vscode-list-activeSelectionBackground); - z-index: ${OPENROUTER_MODEL_PICKER_Z_INDEX - 1}; - border-bottom-left-radius: 3px; - border-bottom-right-radius: 3px; -` - -const DropdownItem = styled.div<{ isSelected: boolean }>` - padding: 5px 10px; - cursor: pointer; - word-break: break-all; - white-space: normal; - - background-color: ${({ isSelected }) => (isSelected ? "var(--vscode-list-activeSelectionBackground)" : "inherit")}; - - &:hover { - background-color: var(--vscode-list-activeSelectionBackground); - } -` - -// Markdown - -const StyledMarkdown = styled.div` - font-family: - var(--vscode-font-family), - system-ui, - -apple-system, - BlinkMacSystemFont, - "Segoe UI", - Roboto, - Oxygen, - Ubuntu, - Cantarell, - "Open Sans", - "Helvetica Neue", - sans-serif; - font-size: 12px; - color: var(--vscode-descriptionForeground); - - p, - li, - ol, - ul { - line-height: 1.25; - margin: 0; - } - - ol, - ul { - padding-left: 1.5em; - margin-left: 0; - } - - p { - white-space: pre-wrap; - } - - a { - text-decoration: none; - } - a { - &:hover { - text-decoration: underline; - } - } -` - -export const ModelDescriptionMarkdown = memo( - ({ - markdown, - key, - isExpanded, - setIsExpanded, - }: { - markdown?: string - key: string - isExpanded: boolean - setIsExpanded: (isExpanded: boolean) => void - }) => { - const [reactContent, setMarkdown] = useRemark() - // const [isExpanded, setIsExpanded] = useState(false) - const [showSeeMore, setShowSeeMore] = useState(false) - const textContainerRef = useRef(null) - const textRef = useRef(null) - - useEffect(() => { - setMarkdown(markdown || "") - }, [markdown, setMarkdown]) - - useEffect(() => { - if (textRef.current && textContainerRef.current) { - const { scrollHeight } = textRef.current - const { clientHeight } = textContainerRef.current - const isOverflowing = scrollHeight > clientHeight - setShowSeeMore(isOverflowing) - // if (!isOverflowing) { - // setIsExpanded(false) - // } - } - }, [reactContent, setIsExpanded]) - - return ( - -
-
- {reactContent} -
- {!isExpanded && showSeeMore && ( -
-
- setIsExpanded(true)}> - See more - -
- )} -
- {/* {isExpanded && showSeeMore && ( -
setIsExpanded(false)}> - See less -
- )} */} - - ) - }, +export const OpenRouterModelPicker = () => ( + ) diff --git a/webview-ui/src/components/settings/__tests__/ModelPicker.test.tsx b/webview-ui/src/components/settings/__tests__/ModelPicker.test.tsx new file mode 100644 index 0000000000..4e7c67c187 --- /dev/null +++ b/webview-ui/src/components/settings/__tests__/ModelPicker.test.tsx @@ -0,0 +1,86 @@ +// cd webview-ui && npx jest src/components/settings/__tests__/ModelPicker.test.ts + +import { screen, fireEvent, render } from "@testing-library/react" +import { act } from "react" +import { ModelPicker } from "../ModelPicker" +import { useExtensionState } from "../../../context/ExtensionStateContext" + +jest.mock("../../../context/ExtensionStateContext", () => ({ + useExtensionState: jest.fn(), +})) + +class MockResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} + +global.ResizeObserver = MockResizeObserver + +Element.prototype.scrollIntoView = jest.fn() + +describe("ModelPicker", () => { + const mockOnUpdateApiConfig = jest.fn() + const mockSetApiConfiguration = jest.fn() + + const defaultProps = { + defaultModelId: "model1", + modelsKey: "glamaModels" as const, + configKey: "glamaModelId" as const, + infoKey: "glamaModelInfo" as const, + refreshMessageType: "refreshGlamaModels" as const, + serviceName: "Test Service", + serviceUrl: "https://test.service", + recommendedModel: "recommended-model", + } + + const mockModels = { + model1: { name: "Model 1", description: "Test model 1" }, + model2: { name: "Model 2", description: "Test model 2" }, + } + + beforeEach(() => { + jest.clearAllMocks() + ;(useExtensionState as jest.Mock).mockReturnValue({ + apiConfiguration: {}, + setApiConfiguration: mockSetApiConfiguration, + glamaModels: mockModels, + onUpdateApiConfig: mockOnUpdateApiConfig, + }) + }) + + it("calls onUpdateApiConfig when a model is selected", async () => { + await act(async () => { + render() + }) + + await act(async () => { + // Open the popover by clicking the button. + const button = screen.getByRole("combobox") + fireEvent.click(button) + }) + + // Wait for popover to open and animations to complete. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 100)) + }) + + await act(async () => { + // Find and click the model item by its value. + const modelItem = screen.getByRole("option", { name: "model2" }) + fireEvent.click(modelItem) + }) + + // Verify the API config was updated. + expect(mockSetApiConfiguration).toHaveBeenCalledWith({ + glamaModelId: "model2", + glamaModelInfo: mockModels["model2"], + }) + + // Verify onUpdateApiConfig was called with the new config. + expect(mockOnUpdateApiConfig).toHaveBeenCalledWith({ + glamaModelId: "model2", + glamaModelInfo: mockModels["model2"], + }) + }) +}) diff --git a/webview-ui/src/components/settings/styles.ts b/webview-ui/src/components/settings/styles.ts new file mode 100644 index 0000000000..85b50579fb --- /dev/null +++ b/webview-ui/src/components/settings/styles.ts @@ -0,0 +1,80 @@ +import styled from "styled-components" + +export const DROPDOWN_Z_INDEX = 1_000 + +export const DropdownWrapper = styled.div` + position: relative; + width: 100%; +` + +export const DropdownList = styled.div<{ $zIndex: number }>` + position: absolute; + top: calc(100% - 3px); + left: 0; + width: calc(100% - 2px); + max-height: 200px; + overflow-y: auto; + background-color: var(--vscode-dropdown-background); + border: 1px solid var(--vscode-list-activeSelectionBackground); + z-index: ${({ $zIndex }) => $zIndex}; + border-bottom-left-radius: 3px; + border-bottom-right-radius: 3px; +` + +export const DropdownItem = styled.div<{ $selected: boolean }>` + padding: 5px 10px; + cursor: pointer; + word-break: break-all; + white-space: normal; + + background-color: ${({ $selected }) => ($selected ? "var(--vscode-list-activeSelectionBackground)" : "inherit")}; + + &:hover { + background-color: var(--vscode-list-activeSelectionBackground); + } +` + +export const StyledMarkdown = styled.div` + font-family: + var(--vscode-font-family), + system-ui, + -apple-system, + BlinkMacSystemFont, + "Segoe UI", + Roboto, + Oxygen, + Ubuntu, + Cantarell, + "Open Sans", + "Helvetica Neue", + sans-serif; + font-size: 12px; + color: var(--vscode-descriptionForeground); + + p, + li, + ol, + ul { + line-height: 1.25; + margin: 0; + } + + ol, + ul { + padding-left: 1.5em; + margin-left: 0; + } + + p { + white-space: pre-wrap; + } + + a { + text-decoration: none; + } + a { + &:hover { + text-decoration: underline; + } + } +` diff --git a/webview-ui/src/components/ui/button.tsx b/webview-ui/src/components/ui/button.tsx index 370ff4a19f..e78a06b4fb 100644 --- a/webview-ui/src/components/ui/button.tsx +++ b/webview-ui/src/components/ui/button.tsx @@ -10,11 +10,14 @@ const buttonVariants = cva( variants: { variant: { default: "bg-primary text-primary-foreground shadow hover:bg-primary/90", - destructive: "bg-destructive text-destructive-foreground shadow-sm hover:bg-destructive/90", - outline: "border border-input bg-foreground shadow-sm hover:bg-foreground/80", secondary: "bg-secondary text-secondary-foreground shadow-sm hover:bg-secondary/80", + outline: + "border border-vscode-dropdown-border bg-vscode-background shadow-sm hover:border-vscode-dropdown-border/80", ghost: "hover:bg-accent hover:text-accent-foreground", link: "text-primary underline-offset-4 hover:underline", + destructive: "bg-destructive text-destructive-foreground shadow-sm hover:bg-destructive/90", + combobox: + "bg-vscode-dropdown-background text-vscode-dropdown-foreground border border-vscode-dropdown-border", }, size: { default: "h-7 px-3", diff --git a/webview-ui/src/components/ui/command.tsx b/webview-ui/src/components/ui/command.tsx index fb8011893d..9580351139 100644 --- a/webview-ui/src/components/ui/command.tsx +++ b/webview-ui/src/components/ui/command.tsx @@ -38,7 +38,7 @@ const CommandInput = React.forwardRef< React.ElementRef, React.ComponentPropsWithoutRef >(({ className, ...props }, ref) => ( -
+
, React.ComponentPropsWithoutRef >(({ className, ...props }, ref) => ( - + )) CommandSeparator.displayName = CommandPrimitive.Separator.displayName @@ -104,7 +108,7 @@ const CommandItem = React.forwardRef< { + return new Intl.NumberFormat("en-US", { + style: "currency", + currency: "USD", + minimumFractionDigits: 2, + maximumFractionDigits: 2, + }).format(price) +} From 85dacb03a3afa1b5967d6032a309f86bac554f6d Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 7 Feb 2025 15:57:35 -0500 Subject: [PATCH 07/15] Better UX for adding new API config profiles --- .changeset/dirty-coins-exist.md | 5 + .../components/settings/ApiConfigManager.tsx | 323 ++++++++++++++---- .../__tests__/ApiConfigManager.test.tsx | 154 ++++++++- 3 files changed, 399 insertions(+), 83 deletions(-) create mode 100644 .changeset/dirty-coins-exist.md diff --git a/.changeset/dirty-coins-exist.md b/.changeset/dirty-coins-exist.md new file mode 100644 index 0000000000..d01a3ba76e --- /dev/null +++ b/.changeset/dirty-coins-exist.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Improve the user experience for adding a new configuration profile diff --git a/webview-ui/src/components/settings/ApiConfigManager.tsx b/webview-ui/src/components/settings/ApiConfigManager.tsx index b10adf4a49..652803fe76 100644 --- a/webview-ui/src/components/settings/ApiConfigManager.tsx +++ b/webview-ui/src/components/settings/ApiConfigManager.tsx @@ -1,8 +1,9 @@ import { VSCodeButton, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" -import { memo, useEffect, useRef, useState } from "react" +import { memo, useEffect, useReducer, useRef } from "react" import { ApiConfigMeta } from "../../../../src/shared/ExtensionMessage" import { Dropdown } from "vscrui" import type { DropdownOption } from "vscrui" +import { Dialog, DialogContent } from "../ui/dialog" interface ApiConfigManagerProps { currentApiConfigName?: string @@ -13,6 +14,86 @@ interface ApiConfigManagerProps { onUpsertConfig: (configName: string) => void } +type State = { + isRenaming: boolean + isCreating: boolean + inputValue: string + newProfileName: string + error: string | null +} + +type Action = + | { type: "START_RENAME"; payload: string } + | { type: "CANCEL_EDIT" } + | { type: "SET_INPUT"; payload: string } + | { type: "SET_NEW_NAME"; payload: string } + | { type: "START_CREATE" } + | { type: "CANCEL_CREATE" } + | { type: "SET_ERROR"; payload: string | null } + | { type: "RESET_STATE" } + +const initialState: State = { + isRenaming: false, + isCreating: false, + inputValue: "", + newProfileName: "", + error: null, +} + +const reducer = (state: State, action: Action): State => { + switch (action.type) { + case "START_RENAME": + return { + ...state, + isRenaming: true, + inputValue: action.payload, + error: null, + } + case "CANCEL_EDIT": + return { + ...state, + isRenaming: false, + inputValue: "", + error: null, + } + case "SET_INPUT": + return { + ...state, + inputValue: action.payload, + error: null, + } + case "SET_NEW_NAME": + return { + ...state, + newProfileName: action.payload, + error: null, + } + case "START_CREATE": + return { + ...state, + isCreating: true, + newProfileName: "", + error: null, + } + case "CANCEL_CREATE": + return { + ...state, + isCreating: false, + newProfileName: "", + error: null, + } + case "SET_ERROR": + return { + ...state, + error: action.payload, + } + case "RESET_STATE": + return initialState + default: + return state + } +} + const ApiConfigManager = ({ currentApiConfigName = "", listApiConfigMeta = [], @@ -21,55 +102,93 @@ const ApiConfigManager = ({ onRenameConfig, onUpsertConfig, }: ApiConfigManagerProps) => { - const [editState, setEditState] = useState<"new" | "rename" | null>(null) - const [inputValue, setInputValue] = useState("") - const inputRef = useRef() + const [state, dispatch] = useReducer(reducer, initialState) + const inputRef = useRef(null) + const newProfileInputRef = useRef(null) - // Focus input when entering edit mode - useEffect(() => { - if (editState) { - setTimeout(() => inputRef.current?.focus(), 0) + const validateName = (name: string, isNewProfile: boolean): string | null => { + const trimmed = name.trim() + if (!trimmed) return "Name cannot be empty" + + const nameExists = listApiConfigMeta?.some((config) => config.name.toLowerCase() === trimmed.toLowerCase()) + + // For new profiles, any existing name is invalid + if (isNewProfile && nameExists) { + return "A profile with this name already exists" } - }, [editState]) - // Reset edit state when current profile changes + // For rename, only block if trying to rename to a different existing profile + if (!isNewProfile && nameExists && trimmed.toLowerCase() !== currentApiConfigName?.toLowerCase()) { + return "A profile with this name already exists" + } + + return null + } + + // Focus input when entering rename mode useEffect(() => { - setEditState(null) - setInputValue("") + if (state.isRenaming) { + const timeoutId = setTimeout(() => inputRef.current?.focus(), 0) + return () => clearTimeout(timeoutId) + } + }, [state.isRenaming]) + + // Focus input when opening new dialog + useEffect(() => { + if (state.isCreating) { + const timeoutId = setTimeout(() => newProfileInputRef.current?.focus(), 0) + return () => clearTimeout(timeoutId) + } + }, [state.isCreating]) + + // Reset state when current profile changes + useEffect(() => { + dispatch({ type: "RESET_STATE" }) }, [currentApiConfigName]) const handleAdd = () => { - const newConfigName = currentApiConfigName + " (copy)" - onUpsertConfig(newConfigName) + dispatch({ type: "START_CREATE" }) } const handleStartRename = () => { - setEditState("rename") - setInputValue(currentApiConfigName || "") + dispatch({ type: "START_RENAME", payload: currentApiConfigName || "" }) } const handleCancel = () => { - setEditState(null) - setInputValue("") + dispatch({ type: "CANCEL_EDIT" }) } const handleSave = () => { - const trimmedValue = inputValue.trim() - if (!trimmedValue) return + const trimmedValue = state.inputValue.trim() + const error = validateName(trimmedValue, false) - if (editState === "new") { - onUpsertConfig(trimmedValue) - } else if (editState === "rename" && currentApiConfigName) { + if (error) { + dispatch({ type: "SET_ERROR", payload: error }) + return + } + + if (state.isRenaming && currentApiConfigName) { if (currentApiConfigName === trimmedValue) { - setEditState(null) - setInputValue("") + dispatch({ type: "CANCEL_EDIT" }) return } onRenameConfig(currentApiConfigName, trimmedValue) } - setEditState(null) - setInputValue("") + dispatch({ type: "CANCEL_EDIT" }) + } + + const handleNewProfileSave = () => { + const trimmedValue = state.newProfileName.trim() + const error = validateName(trimmedValue, true) + + if (error) { + dispatch({ type: "SET_ERROR", payload: error }) + return + } + + onUpsertConfig(trimmedValue) + dispatch({ type: "CANCEL_CREATE" }) } const handleDelete = () => { @@ -93,49 +212,62 @@ const ApiConfigManager = ({ Configuration Profile - {editState ? ( -
- setInputValue(e.target.value)} - placeholder={editState === "new" ? "Enter profile name" : "Enter new name"} - style={{ flexGrow: 1 }} - onKeyDown={(e: any) => { - if (e.key === "Enter" && inputValue.trim()) { - handleSave() - } else if (e.key === "Escape") { - handleCancel() - } - }} - /> - - - - - - + {state.isRenaming ? ( +
+
+ { + const target = e as { target: { value: string } } + dispatch({ type: "SET_INPUT", payload: target.target.value }) + }} + placeholder="Enter new name" + style={{ flexGrow: 1 }} + onKeyDown={(e: unknown) => { + const event = e as { key: string } + if (event.key === "Enter" && state.inputValue.trim()) { + handleSave() + } else if (event.key === "Escape") { + handleCancel() + } + }} + /> + + + + + + +
+ {state.error && ( +

+ {state.error} +

+ )}
) : ( <> @@ -211,6 +343,57 @@ const ApiConfigManager = ({

)} + + dispatch({ type: open ? "START_CREATE" : "CANCEL_CREATE" })} + aria-labelledby="new-profile-title"> + +

+ New Configuration Profile +

+ + { + const target = e as { target: { value: string } } + dispatch({ type: "SET_NEW_NAME", payload: target.target.value }) + }} + placeholder="Enter profile name" + style={{ width: "100%" }} + onKeyDown={(e: unknown) => { + const event = e as { key: string } + if (event.key === "Enter" && state.newProfileName.trim()) { + handleNewProfileSave() + } else if (event.key === "Escape") { + dispatch({ type: "CANCEL_CREATE" }) + } + }} + /> + {state.error && ( +

+ {state.error} +

+ )} +
+ dispatch({ type: "CANCEL_CREATE" })}> + Cancel + + + Create Profile + +
+
+
) diff --git a/webview-ui/src/components/settings/__tests__/ApiConfigManager.test.tsx b/webview-ui/src/components/settings/__tests__/ApiConfigManager.test.tsx index ac6245d6d1..24e62215ec 100644 --- a/webview-ui/src/components/settings/__tests__/ApiConfigManager.test.tsx +++ b/webview-ui/src/components/settings/__tests__/ApiConfigManager.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent } from "@testing-library/react" +import { render, screen, fireEvent, within } from "@testing-library/react" import ApiConfigManager from "../ApiConfigManager" // Mock VSCode components @@ -8,11 +8,12 @@ jest.mock("@vscode/webview-ui-toolkit/react", () => ({ {children} ), - VSCodeTextField: ({ value, onInput, placeholder }: any) => ( + VSCodeTextField: ({ value, onInput, placeholder, onKeyDown }: any) => ( onInput(e)} placeholder={placeholder} + onKeyDown={onKeyDown} ref={undefined} // Explicitly set ref to undefined to avoid warning /> ), @@ -32,6 +33,16 @@ jest.mock("vscrui", () => ({ ), })) +// Mock Dialog component +jest.mock("@/components/ui/dialog", () => ({ + Dialog: ({ children, open, onOpenChange }: any) => ( +
+ {children} +
+ ), + DialogContent: ({ children }: any) =>
{children}
, +})) + describe("ApiConfigManager", () => { const mockOnSelectConfig = jest.fn() const mockOnDeleteConfig = jest.fn() @@ -54,34 +65,74 @@ describe("ApiConfigManager", () => { jest.clearAllMocks() }) - it("immediately creates a copy when clicking add button", () => { + const getRenameForm = () => screen.getByTestId("rename-form") + const getDialogContent = () => screen.getByTestId("dialog-content") + + it("opens new profile dialog when clicking add button", () => { render() - // Find and click the add button const addButton = screen.getByTitle("Add profile") fireEvent.click(addButton) - // Verify that onUpsertConfig was called with the correct name - expect(mockOnUpsertConfig).toHaveBeenCalledTimes(1) - expect(mockOnUpsertConfig).toHaveBeenCalledWith("Default Config (copy)") + expect(screen.getByTestId("dialog")).toBeVisible() + expect(screen.getByText("New Configuration Profile")).toBeInTheDocument() }) - it("creates copy with correct name when current config has spaces", () => { - render() + it("creates new profile with entered name", () => { + render() + // Open dialog const addButton = screen.getByTitle("Add profile") fireEvent.click(addButton) - expect(mockOnUpsertConfig).toHaveBeenCalledWith("My Test Config (copy)") + // Enter new profile name + const input = screen.getByPlaceholderText("Enter profile name") + fireEvent.input(input, { target: { value: "New Profile" } }) + + // Click create button + const createButton = screen.getByText("Create Profile") + fireEvent.click(createButton) + + expect(mockOnUpsertConfig).toHaveBeenCalledWith("New Profile") }) - it("handles empty current config name gracefully", () => { - render() + it("shows error when creating profile with existing name", () => { + render() + // Open dialog const addButton = screen.getByTitle("Add profile") fireEvent.click(addButton) - expect(mockOnUpsertConfig).toHaveBeenCalledWith(" (copy)") + // Enter existing profile name + const input = screen.getByPlaceholderText("Enter profile name") + fireEvent.input(input, { target: { value: "Default Config" } }) + + // Click create button to trigger validation + const createButton = screen.getByText("Create Profile") + fireEvent.click(createButton) + + // Verify error message + const dialogContent = getDialogContent() + const errorMessage = within(dialogContent).getByTestId("error-message") + expect(errorMessage).toHaveTextContent("A profile with this name already exists") + expect(mockOnUpsertConfig).not.toHaveBeenCalled() + }) + + it("prevents creating profile with empty name", () => { + render() + + // Open dialog + const addButton = screen.getByTitle("Add profile") + fireEvent.click(addButton) + + // Enter empty name + const input = screen.getByPlaceholderText("Enter profile name") + fireEvent.input(input, { target: { value: " " } }) + + // Verify create button is disabled + const createButton = screen.getByText("Create Profile") + expect(createButton).toBeDisabled() + expect(mockOnUpsertConfig).not.toHaveBeenCalled() }) it("allows renaming the current config", () => { @@ -102,6 +153,45 @@ describe("ApiConfigManager", () => { expect(mockOnRenameConfig).toHaveBeenCalledWith("Default Config", "New Name") }) + it("shows error when renaming to existing config name", () => { + render() + + // Start rename + const renameButton = screen.getByTitle("Rename profile") + fireEvent.click(renameButton) + + // Find input and enter existing name + const input = screen.getByDisplayValue("Default Config") + fireEvent.input(input, { target: { value: "Another Config" } }) + + // Save to trigger validation + const saveButton = screen.getByTitle("Save") + fireEvent.click(saveButton) + + // Verify error message + const renameForm = getRenameForm() + const errorMessage = within(renameForm).getByTestId("error-message") + expect(errorMessage).toHaveTextContent("A profile with this name already exists") + expect(mockOnRenameConfig).not.toHaveBeenCalled() + }) + + it("prevents renaming to empty name", () => { + render() + + // Start rename + const renameButton = screen.getByTitle("Rename profile") + fireEvent.click(renameButton) + + // Find input and enter empty name + const input = screen.getByDisplayValue("Default Config") + fireEvent.input(input, { target: { value: " " } }) + + // Verify save button is disabled + const saveButton = screen.getByTitle("Save") + expect(saveButton).toBeDisabled() + expect(mockOnRenameConfig).not.toHaveBeenCalled() + }) + it("allows selecting a different config", () => { render() @@ -149,4 +239,42 @@ describe("ApiConfigManager", () => { // Verify we're back to normal view expect(screen.queryByDisplayValue("New Name")).not.toBeInTheDocument() }) + + it("handles keyboard events in new profile dialog", () => { + render() + + // Open dialog + const addButton = screen.getByTitle("Add profile") + fireEvent.click(addButton) + + const input = screen.getByPlaceholderText("Enter profile name") + + // Test Enter key + fireEvent.input(input, { target: { value: "New Profile" } }) + fireEvent.keyDown(input, { key: "Enter" }) + expect(mockOnUpsertConfig).toHaveBeenCalledWith("New Profile") + + // Test Escape key + fireEvent.keyDown(input, { key: "Escape" }) + expect(screen.getByTestId("dialog")).not.toBeVisible() + }) + + it("handles keyboard events in rename mode", () => { + render() + + // Start rename + const renameButton = screen.getByTitle("Rename profile") + fireEvent.click(renameButton) + + const input = screen.getByDisplayValue("Default Config") + + // Test Enter key + fireEvent.input(input, { target: { value: "New Name" } }) + fireEvent.keyDown(input, { key: "Enter" }) + expect(mockOnRenameConfig).toHaveBeenCalledWith("Default Config", "New Name") + + // Test Escape key + fireEvent.keyDown(input, { key: "Escape" }) + expect(screen.queryByDisplayValue("New Name")).not.toBeInTheDocument() + }) }) From 30cf0d53198031e74755fef9964dcc98e36ec457 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Fri, 7 Feb 2025 15:50:28 -0700 Subject: [PATCH 08/15] Update HistoryPreview.tsx Added a copy button to the history preview view --- .../src/components/history/HistoryPreview.tsx | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/webview-ui/src/components/history/HistoryPreview.tsx b/webview-ui/src/components/history/HistoryPreview.tsx index 08aca2a44d..562c110097 100644 --- a/webview-ui/src/components/history/HistoryPreview.tsx +++ b/webview-ui/src/components/history/HistoryPreview.tsx @@ -1,7 +1,7 @@ import { VSCodeButton } from "@vscode/webview-ui-toolkit/react" import { useExtensionState } from "../../context/ExtensionStateContext" import { vscode } from "../../utils/vscode" -import { memo } from "react" +import { memo, useState } from "react" import { formatLargeNumber } from "../../utils/format" type HistoryPreviewProps = { @@ -10,6 +10,18 @@ type HistoryPreviewProps = { const HistoryPreview = ({ showHistoryView }: HistoryPreviewProps) => { const { taskHistory } = useExtensionState() + const [showCopyModal, setShowCopyModal] = useState(false) + + const handleCopyTask = async (e: React.MouseEvent, task: string) => { + e.stopPropagation() + try { + await navigator.clipboard.writeText(task) + setShowCopyModal(true) + setTimeout(() => setShowCopyModal(false), 2000) + } catch (error) { + console.error("Failed to copy to clipboard:", error) + } + } const handleHistorySelect = (id: string) => { vscode.postMessage({ type: "showTaskWithId", text: id }) } @@ -31,8 +43,30 @@ const HistoryPreview = ({ showHistoryView }: HistoryPreviewProps) => { return (
+ {showCopyModal &&
Prompt Copied to Clipboard
} - {showCopyModal &&
Prompt Copied to Clipboard
} + {showCopyFeedback &&
Prompt Copied to Clipboard
}
{ title="Copy Prompt" className="copy-button" data-appearance="icon" - onClick={(e) => handleCopyTask(e, item.task)}> + onClick={(e) => copyWithFeedback(item.task, e)}>