-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add support for runtime MCP servers via IPC API (#7518) #7521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add initializeRuntimeMcpServers method to McpHub for programmatic server initialization - Update API.startNewTask to detect and initialize MCP servers from configuration - Add "runtime" as a new server source type alongside "global" and "project" - Update type definitions to include mcpServers in GlobalSettings - Add comprehensive tests for the new functionality This allows MCP servers to be defined and initialized through the IPC socket API configuration rather than only through file-based configuration, enabling headless VSCode environments to use MCP servers programmatically. Fixes #7518
- Update method signatures to accept "runtime" as valid source parameter - Fix createPlaceholderConnection to handle runtime source - Update findConnection and fetch methods to support runtime servers
|
|
||
| // Initialize runtime servers with "runtime" source | ||
| try { | ||
| await this.updateServerConnections(this.runtimeServers, "runtime" as any, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initializeRuntimeMcpServers method correctly stores runtime configurations and calls updateServerConnections. Consider avoiding the 'as any' cast for the source parameter when calling updateServerConnections; using the proper union type ('runtime') can preserve type safety.
| await this.updateServerConnections(this.runtimeServers, "runtime" as any, false) | |
| await this.updateServerConnections(this.runtimeServers, "runtime", false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code because apparently I trust no one, not even myself.
| isConnecting: boolean = false | ||
| private refCount: number = 0 // Reference counter for active clients | ||
| private configChangeDebounceTimers: Map<string, NodeJS.Timeout> = new Map() | ||
| private runtimeServers: Record<string, any> = {} // Store runtime MCP servers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The runtimeServers property stores runtime servers indefinitely in memory. Could we consider clearing this when all runtime servers are disconnected to prevent potential memory leaks in long-running sessions? Maybe add a cleanup method or clear it when the last runtime server disconnects?
| const mcpHub = provider.getMcpHub() | ||
| if (mcpHub) { | ||
| // Initialize runtime MCP servers | ||
| await mcpHub.initializeRuntimeMcpServers(configuration.mcpServers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When initializeRuntimeMcpServers fails, should we log this error or notify the user? Currently, if the mcpHub exists but initialization fails, the error is silently swallowed in McpHub. Consider adding error handling here:
| await mcpHub.initializeRuntimeMcpServers(configuration.mcpServers) | |
| if (configuration.mcpServers && typeof configuration.mcpServers === "object") { | |
| const mcpHub = provider.getMcpHub() | |
| if (mcpHub) { | |
| try { | |
| // Initialize runtime MCP servers | |
| await mcpHub.initializeRuntimeMcpServers(configuration.mcpServers) | |
| } catch (error) { | |
| console.error('Failed to initialize runtime MCP servers:', error) | |
| // Optionally notify user or continue without MCP servers | |
| } | |
| } | |
| } |
|
|
||
| mcpEnabled: z.boolean().optional(), | ||
| enableMcpServerCreation: z.boolean().optional(), | ||
| mcpServers: z.record(z.string(), z.any()).optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mcpServers property uses z.any() which bypasses type safety. Could we define a proper Zod schema for MCP server configurations? This would provide better validation and type safety. Something like:
| mcpServers: z.record(z.string(), z.any()).optional(), | |
| mcpServers: z.record(z.string(), z.union([ | |
| z.object({ | |
| type: z.literal('stdio'), | |
| command: z.string(), | |
| args: z.array(z.string()).optional(), | |
| env: z.record(z.string()).optional(), | |
| disabled: z.boolean().optional(), | |
| alwaysAllow: z.array(z.string()).optional() | |
| }), | |
| z.object({ | |
| type: z.literal('sse'), | |
| url: z.string().url(), | |
| headers: z.record(z.string()).optional(), | |
| disabled: z.boolean().optional() | |
| }) | |
| ])).optional(), |
| * Initialize runtime MCP servers passed programmatically (e.g., through IPC API) | ||
| * @param servers Record of server configurations | ||
| */ | ||
| public async initializeRuntimeMcpServers(servers: Record<string, any>): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding JSDoc comments to document the purpose and usage patterns of this new method. This would help future developers understand when and how to use runtime MCP servers:
| public async initializeRuntimeMcpServers(servers: Record<string, any>): Promise<void> { | |
| /** | |
| * Initialize runtime MCP servers passed programmatically (e.g., through IPC API) | |
| * These servers have the highest priority and override project/global servers with the same name. | |
| * Runtime servers are not persisted to configuration files. | |
| * @param servers Record of server configurations keyed by server name | |
| * @returns Promise<void> | |
| */ | |
| public async initializeRuntimeMcpServers(servers: Record<string, any>): Promise<void> { |
| }) | ||
|
|
||
| describe("startNewTask", () => { | ||
| it("should initialize runtime MCP servers when mcpServers is provided in configuration", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good test coverage for the happy path! Consider adding tests for error scenarios:
- Invalid server configurations
- Initialization failures
- What happens when
initializeRuntimeMcpServersthrows an error
This would ensure the error handling is robust.
|
This doesn't look like a good implementation, maybe it makes more sense to allow setting them up via an env var. Closing as the issue needs better scoping. |
This PR adds support for defining and initializing MCP (Model Context Protocol) servers programmatically through the IPC socket API, addressing the issue where MCP servers could only be configured through file-based configuration.
Problem
Previously, MCP servers could only be defined through:
cline_mcp_settings.json).roo/mcp.json)This limitation prevented headless VSCode environments from using MCP servers when configuration was passed programmatically through the IPC API.
Solution
This PR introduces support for "runtime" MCP servers that can be:
Changes
initializeRuntimeMcpServers()method to handle programmatically defined serversstartNewTask()to detect and initialize MCP servers from configurationmcpServersas an optional property in GlobalSettings schemaTesting
Usage Example
Fixes #7518
Important
Adds runtime MCP server support via IPC API, enabling dynamic server configuration without file-based settings.
McpHub.ts: IntroducesinitializeRuntimeMcpServers()for handling runtime servers.api.ts: UpdatesstartNewTask()to initialize runtime MCP servers from configuration.global-settings.ts: AddsmcpServerstoGlobalSettingsschema.api.spec.ts: Adds tests for runtime MCP server initialization, including cases with complex configurations and empty server objects.McpServertype inmcp.tsto include "runtime" as a source.This description was created by
for 18305a9. You can customize this summary. It will automatically update as commits are pushed.