Skip to content

Commit 3f50226

Browse files
committed
fix: prevent duplicate MCP connections during initialization
- Add isInitializing flag to prevent file watchers from creating duplicate connections - Refactor constructor to use async initialization method - Skip config change handling during initialization phase - Add test to verify no duplicate connections are created - Fixes issue where Docker (MCP) processes were duplicated on startup
1 parent da27b72 commit 3f50226

File tree

2 files changed

+85
-4
lines changed

2 files changed

+85
-4
lines changed

src/services/mcp/McpHub.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,27 @@ export class McpHub {
134134
isConnecting: boolean = false
135135
private configChangeDebounceTimers: Map<string, NodeJS.Timeout> = new Map()
136136
private toggleOperations: Map<string, Promise<void>> = new Map() // Track ongoing toggle operations
137+
private isInitializing: boolean = true // Flag to prevent duplicate connections during initialization
137138

138139
constructor(provider: ClineProvider) {
139140
this.providerRef = new WeakRef(provider)
140-
this.watchMcpSettingsFile()
141-
this.watchProjectMcpFile().catch(console.error)
141+
this.initializeHub().catch(console.error)
142+
}
143+
144+
private async initializeHub(): Promise<void> {
145+
// Set up watchers first but mark as initializing to prevent them from creating connections
146+
this.isInitializing = true
147+
148+
await this.watchMcpSettingsFile()
149+
await this.watchProjectMcpFile()
142150
this.setupWorkspaceFoldersWatcher()
143-
this.initializeGlobalMcpServers()
144-
this.initializeProjectMcpServers()
151+
152+
// Now initialize servers - watchers won't trigger duplicate connections
153+
await this.initializeGlobalMcpServers()
154+
await this.initializeProjectMcpServers()
155+
156+
// Clear the initialization flag after servers are set up
157+
this.isInitializing = false
145158
}
146159
/**
147160
* Registers a client (e.g., ClineProvider) using this hub.
@@ -255,6 +268,12 @@ export class McpHub {
255268
* Debounced wrapper for handling config file changes
256269
*/
257270
private debounceConfigChange(filePath: string, source: "global" | "project"): void {
271+
// Skip file change handling during initialization to prevent duplicate connections
272+
if (this.isInitializing) {
273+
console.log(`Skipping config change for ${source} during initialization`)
274+
return
275+
}
276+
258277
const key = `${source}-${filePath}`
259278

260279
// Clear existing timer if any

src/services/mcp/__tests__/McpHub.spec.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,68 @@ describe("McpHub", () => {
10171017
// Still should not connect
10181018
expect(StdioClientTransport).not.toHaveBeenCalled()
10191019
})
1020+
1021+
it("should not create duplicate connections during initialization", async () => {
1022+
// Mock the config file read
1023+
const mockConfig = {
1024+
mcpServers: {
1025+
"test-server": {
1026+
type: "stdio" as const,
1027+
command: "node",
1028+
args: ["test.js"],
1029+
disabled: false,
1030+
timeout: 60,
1031+
cwd: process.cwd(),
1032+
alwaysAllow: [],
1033+
disabledTools: [],
1034+
},
1035+
},
1036+
}
1037+
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockConfig))
1038+
1039+
// Mock StdioClientTransport
1040+
const mockTransport = {
1041+
start: vi.fn().mockResolvedValue(undefined),
1042+
close: vi.fn().mockResolvedValue(undefined),
1043+
stderr: {
1044+
on: vi.fn(),
1045+
},
1046+
onerror: null,
1047+
onclose: null,
1048+
}
1049+
1050+
const StdioClientTransport = (await import("@modelcontextprotocol/sdk/client/stdio.js"))
1051+
.StdioClientTransport as ReturnType<typeof vi.fn>
1052+
StdioClientTransport.mockClear()
1053+
StdioClientTransport.mockImplementation(() => mockTransport)
1054+
1055+
// Mock Client
1056+
const mockClient = {
1057+
connect: vi.fn().mockResolvedValue(undefined),
1058+
close: vi.fn().mockResolvedValue(undefined),
1059+
getInstructions: vi.fn().mockReturnValue("test instructions"),
1060+
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
1061+
}
1062+
const Client = (await import("@modelcontextprotocol/sdk/client/index.js")).Client as ReturnType<
1063+
typeof vi.fn
1064+
>
1065+
Client.mockClear()
1066+
Client.mockImplementation(() => mockClient)
1067+
1068+
// Create a new McpHub instance
1069+
const newMcpHub = new McpHub(mockProvider as ClineProvider)
1070+
1071+
// Wait for initialization to complete
1072+
await new Promise((resolve) => setTimeout(resolve, 200))
1073+
1074+
// Verify that only one connection was created (not duplicated)
1075+
expect(StdioClientTransport).toHaveBeenCalledTimes(1)
1076+
expect(mockClient.connect).toHaveBeenCalledTimes(1)
1077+
1078+
// Verify the hub has exactly one connection
1079+
expect(newMcpHub.connections.length).toBe(1)
1080+
expect(newMcpHub.connections[0].server.name).toBe("test-server")
1081+
})
10201082
})
10211083

10221084
describe("callTool", () => {

0 commit comments

Comments
 (0)