Align with the original mcp functions

Author: aheizi

src/core/prompts/instructions/create-mcp-server.ts

@@ -11,7 +11,7 @@ export async function createMCPServerInstructions(
 
 When creating MCP servers, it's important to understand that they operate in a non-interactive environment. The server cannot initiate OAuth flows, open browser windows, or prompt for user input during runtime. All credentials and authentication tokens must be provided upfront through environment variables in the MCP settings configuration. For example, Spotify's API uses OAuth to get a refresh token for the user, but the MCP server cannot initiate this flow. While you can walk the user through obtaining an application client ID and secret, you may have to create a separate one-time setup script (like get-refresh-token.js) that captures and logs the final piece of the puzzle: the user's refresh token (i.e. you might run the script using execute_command which would open a browser for authentication, and then log the refresh token so that you can see it in the command output for you to use in the MCP settings configuration).
 
-Unless the user specifies otherwise, new local MCP servers should be created in: /mock/settings/path
+Unless the user specifies otherwise, new local MCP servers should be created in: ${await mcpHub.getMcpServersPath()}
 
 ### MCP Server Types and Configuration
 
@@ -60,7 +60,7 @@ The following example demonstrates how to build a local MCP server that provides
 1. Use the \`create-typescript-server\` tool to bootstrap a new project in the default MCP servers directory:
 
 \`\`\`bash
-cd /mock/settings/path
+cd ${await mcpHub.getMcpServersPath()}
 npx @modelcontextprotocol/create-server weather-server
 cd weather-server
 # Install dependencies
@@ -360,7 +360,7 @@ npm run build
 
 4. Whenever you need an environment variable such as an API key to configure the MCP server, walk the user through the process of getting the key. For example, they may need to create an account and go to a developer dashboard to generate the key. Provide step-by-step instructions and URLs to make it easy for the user to retrieve the necessary information. Then use the ask_followup_question tool to ask the user for the key, in this case the OpenWeather API key.
 
-5. Install the MCP Server by adding the MCP server configuration to the settings file located at '/mock/settings/path/cline_mcp_settings.json'. The settings file may have other MCP servers already configured, so you would read it first and then add your new server to the existing \`mcpServers\` object.
+5. Install the MCP Server by adding the MCP server configuration to the settings file located at '${await mcpHub.getGlobalMcpSettingsFilePath()}'. The settings file may have other MCP servers already configured, so you would read it first and then add your new server to the existing \`mcpServers\` object.
 
 IMPORTANT: Regardless of what else you see in the MCP settings file, you must default any new MCP servers you create to disabled=false and alwaysAllow=[].
 

src/core/webview/ClineProvider.ts

@@ -110,6 +110,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 		McpServerManager.getInstance(this.context, this)
 			.then((hub) => {
 				this.mcpHub = hub
+				this.mcpHub.registerClient()
 			})
 			.catch((error) => {
 				this.log(`Failed to initialize MCP Hub: ${error}`)
@@ -216,7 +217,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 
 		this._workspaceTracker?.dispose()
 		this._workspaceTracker = undefined
-		await this.mcpHub?.dispose()
+		await this.mcpHub?.unregisterClient()
 		this.mcpHub = undefined
 		this.customModesManager?.dispose()
 		this.log("Disposed all disposables")

src/services/mcp/McpHub.ts

@@ -1,16 +1,18 @@
 import * as vscode from "vscode"
 import { ClineProvider } from "../../core/webview/ClineProvider"
-import { ConfigManager } from "./config"
 import type { ConfigChangeEvent } from "./config"
-import { ConnectionFactory, ConnectionManager, StdioHandler, SseHandler } from "./connection"
-import { McpServer, McpToolCallResponse, McpResourceResponse, ServerConfig, ConfigSource, McpConnection } from "./types"
+import { ConfigManager } from "./config"
+import { ConnectionFactory, ConnectionManager, SseHandler, StdioHandler } from "./connection"
+import { ConfigSource, McpConnection, McpResourceResponse, McpServer, McpToolCallResponse, ServerConfig } from "./types"
 
 export class McpHub {
 	private configManager: ConfigManager
 	private connectionManager: ConnectionManager
 	private disposables: vscode.Disposable[] = []
 	private providerRef: WeakRef<ClineProvider>
 	private isConnectingFlag = false
+	private refCount: number = 0 // Reference counter for active clients
+	private isDisposed = false // Flag to prevent multiple disposals
 
 	constructor(private provider: ClineProvider) {
 		this.providerRef = new WeakRef(provider)
@@ -47,6 +49,41 @@ export class McpHub {
 		void this.configManager.watchConfigFiles(provider)
 	}
 
+	/**
+	 * Registers a client (e.g., ClineProvider) using this hub.
+	 * Increments the reference count.
+	 */
+	public registerClient(): void {
+		this.refCount++
+		console.log(`McpHub: Client registered. Ref count: ${this.refCount}`)
+	}
+
+	/**
+	 * Unregisters a client. Decrements the reference count.
+	 * If the count reaches zero, disposes the hub.
+	 */
+	public async unregisterClient(): Promise<void> {
+		this.refCount--
+		console.log(`McpHub: Client unregistered. Ref count: ${this.refCount}`)
+		if (this.refCount <= 0) {
+			console.log("McpHub: Last client unregistered. Disposing hub.")
+			await this.dispose()
+		}
+	}
+
+	/**
+	 * Get the path where MCP servers should be stored
+	 * @returns Path to MCP servers directory
+	 */
+	async getMcpServersPath(): Promise<string> {
+		const provider = this.providerRef.deref()
+		if (!provider) {
+			throw new Error("Provider not available")
+		}
+		const mcpServersPath = await provider.ensureMcpServersDirectoryExists()
+		return mcpServersPath
+	}
+
 	/**
 	 * Execute a server action with common checks.
 	 */
@@ -73,7 +110,9 @@ export class McpHub {
 				? await this.configManager.getGlobalConfigPath(this.provider)
 				: await this.configManager.getProjectConfigPath()
 		if (!configPath) throw new Error(`Cannot get config path for source: ${source}`)
-		return configPath
+		// Normalize path for cross-platform compatibility
+		// Use a consistent path format for both reading and writing
+		return process.platform === "win32" ? configPath.replace(/\\/g, "/") : configPath
 	}
 
 	/**
@@ -111,6 +150,14 @@ export class McpHub {
 		return this.configManager.getGlobalConfigPath(provider)
 	}
 
+	async getGlobalMcpSettingsFilePath(): Promise<string> {
+		const provider = this.providerRef.deref()
+		if (!provider) {
+			throw new Error("Provider not available")
+		}
+		return this.getGlobalConfigPath(provider)
+	}
+
 	async callTool(
 		serverName: string,
 		toolName: string,
@@ -292,7 +339,35 @@ export class McpHub {
 	}
 
 	async dispose(): Promise<void> {
-		await this.connectionManager.dispose()
-		this.disposables.forEach((d) => d.dispose())
+		// Prevent multiple disposals
+		if (this.isDisposed) {
+			console.log("McpHub: Already disposed.")
+			return
+		}
+
+		// Check for active clients
+		if (this.refCount > 0) {
+			console.log(`McpHub: Cannot dispose, still has ${this.refCount} active clients`)
+			return
+		}
+
+		console.log("McpHub: Disposing...")
+		this.isDisposed = true
+
+		try {
+			// Dispose connection manager (includes file watchers and connections)
+			await this.connectionManager.dispose()
+		} catch (error) {
+			console.error("Failed to dispose connection manager:", error)
+		}
+
+		// Dispose all other disposables
+		for (const disposable of this.disposables) {
+			try {
+				disposable.dispose()
+			} catch (error) {
+				console.error("Failed to dispose disposable:", error)
+			}
+		}
 	}
 }

src/services/mcp/config/ConfigManager.ts

@@ -9,6 +9,7 @@ import { GlobalFileNames } from "../../../shared/globalFileNames"
 import { fileExistsAtPath } from "../../../utils/fs"
 import { ServerConfig, McpServer, ConfigSource } from "../types"
 import { ConfigChangeEvent, ConfigChangeListener } from "./types"
+import { safeParseSeverConfig } from "./validation"
 
 /**
  * Configuration Manager
@@ -27,26 +28,7 @@ export class ConfigManager {
 	/** Configuration file path cache */
 	private configPaths: Partial<Record<ConfigSource, string>> = {}
 
-	// Validation schemas
-	private readonly ServerConfigSchema = z.object({
-		type: z.enum(["stdio", "sse"]),
-		command: z.string().optional(),
-		args: z.array(z.string()).optional(),
-		env: z.record(z.string()).optional(),
-		url: z.string().optional(),
-		headers: z.record(z.string()).optional(),
-		disabled: z.boolean().optional(),
-		timeout: z
-			.number()
-			.optional()
-			.refine(
-				(val) => val === undefined || (val >= 0 && val <= 3600),
-				(val) => ({ message: `Timeout must be between 0 and 3600 seconds, got ${val}` }),
-			),
-		alwaysAllow: z.array(z.string()).optional(),
-		watchPaths: z.array(z.string()).optional(),
-	})
-
+	// Validation schema for MCP settings
 	private readonly McpSettingsSchema = z.object({
 		mcpServers: z.record(z.any()),
 	})
@@ -131,12 +113,6 @@ export class ConfigManager {
 		}
 	}
 
-	private inferServerType(config: Record<string, unknown>): "stdio" | "sse" | undefined {
-		if (config.command) return "stdio"
-		if (config.url) return "sse"
-		return undefined
-	}
-
 	/**
 	 * Validate server configuration
 	 * @param config Configuration object to validate
@@ -146,23 +122,7 @@ export class ConfigManager {
 	public validateServerConfig(config: unknown, serverName?: string): ServerConfig {
 		try {
 			const configCopy = { ...(config as Record<string, unknown>) }
-
-			if (!configCopy.type) {
-				configCopy.type = this.inferServerType(configCopy)
-			}
-
-			const hasStdioFields = configCopy.command !== undefined
-			const hasSseFields = configCopy.url !== undefined
-
-			if (hasStdioFields && hasSseFields) {
-				throw new Error(t("common:errors.invalid_mcp_config"))
-			}
-
-			if (!hasStdioFields && !hasSseFields) {
-				throw new Error(t("common:errors.invalid_mcp_config"))
-			}
-
-			const result = this.ServerConfigSchema.safeParse(configCopy)
+			const result = safeParseSeverConfig(configCopy)
 			if (!result.success) {
 				const errors = result.error.errors.map((err) => `${err.path.join(".")}: ${err.message}`).join(", ")
 				throw new Error(t("common:errors.invalid_mcp_settings_validation", { errorMessages: errors }))
@@ -190,14 +150,6 @@ export class ConfigManager {
 				throw new Error(t("common:errors.invalid_mcp_settings_syntax"))
 			}
 
-			if (config.mcpServers && typeof config.mcpServers === "object") {
-				Object.values(config.mcpServers).forEach((server: any) => {
-					if (!server.type) {
-						server.type = this.inferServerType(server)
-					}
-				})
-			}
-
 			const result = this.McpSettingsSchema.safeParse(config)
 			if (!result.success) {
 				const errors = result.error.errors.map((err) => `${err.path.join(".")}: ${err.message}`).join("\n")

src/services/mcp/config/validation.ts

@@ -0,0 +1,69 @@
+import { z } from "zod"
+import { ServerConfig } from "../types"
+import * as vscode from "vscode"
+
+const typeErrorMessage = "Server type must match the provided configuration"
+
+const BaseConfigSchema = z.object({
+	disabled: z.boolean().optional(),
+	timeout: z.number().optional(),
+	alwaysAllow: z.array(z.string()).optional(),
+	watchPaths: z.array(z.string()).optional(),
+})
+
+const createServerConfigSchema = () => {
+	return z.union([
+		// Stdio config (has command field)
+		BaseConfigSchema.extend({
+			type: z.enum(["stdio"]).optional(),
+			command: z.string().min(1, "Command cannot be empty"),
+			args: z.array(z.string()).optional(),
+			cwd: z.string().default(() => vscode.workspace.workspaceFolders?.at(0)?.uri.fsPath ?? process.cwd()),
+			env: z.record(z.string()).optional(),
+			// Ensure no SSE fields are present
+			url: z.undefined().optional(),
+			headers: z.undefined().optional(),
+		})
+			.transform((data) => ({
+				...data,
+				type: "stdio" as const,
+			}))
+			.refine((data) => data.type === undefined || data.type === "stdio", { message: typeErrorMessage }),
+
+		// SSE config (has url field)
+		BaseConfigSchema.extend({
+			type: z.enum(["sse"]).optional(),
+			url: z.string().url("URL must be a valid URL format"),
+			headers: z.record(z.string()).optional(),
+			// Ensure no stdio fields are present
+			command: z.undefined().optional(),
+			args: z.undefined().optional(),
+			cwd: z.undefined().optional(),
+			env: z.undefined().optional(),
+		})
+			.transform((data) => ({
+				...data,
+				type: "sse" as const,
+			}))
+			.refine((data) => data.type === undefined || data.type === "sse", { message: typeErrorMessage }),
+	])
+}
+
+/**
+ * Validates a server configuration object.
+ * @param config The configuration object to validate
+ * @returns The validated server configuration
+ * @throws {ZodError} If validation fails
+ */
+export const validateServerConfig = (config: unknown): ServerConfig => {
+	return createServerConfigSchema().parse(config)
+}
+
+/**
+ * Safely validates a server configuration object.
+ * @param config The configuration object to validate
+ * @returns The validation result
+ */
+export const safeParseSeverConfig = (config: unknown): z.SafeParseReturnType<unknown, ServerConfig> => {
+	return createServerConfigSchema().safeParse(config)
+}