Skip to content

Commit 02bb864

Browse files
committed
feat: respect mcpEnabled setting to prevent automatic MCP server initialization
- Add conditional initialization in ClineProvider based on mcpEnabled setting - Update McpHub constructor to accept autoInitialize parameter - Add enableServers() and disableServers() methods for dynamic control - Handle mcpEnabled toggle in webview message handler - Update tests to cover new behavior Fixes #6014
1 parent 9fce90b commit 02bb864

File tree

5 files changed

+234
-14
lines changed

5 files changed

+234
-14
lines changed

src/core/webview/ClineProvider.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -148,19 +148,34 @@ export class ClineProvider
148148
await this.postStateToWebview()
149149
})
150150

151-
// Initialize MCP Hub through the singleton manager
152-
McpServerManager.getInstance(this.context, this)
153-
.then((hub) => {
154-
this.mcpHub = hub
155-
this.mcpHub.registerClient()
156-
})
157-
.catch((error) => {
158-
this.log(`Failed to initialize MCP Hub: ${error}`)
159-
})
151+
// Initialize MCP Hub through the singleton manager only if mcpEnabled
152+
this.initializeMcpIfEnabled()
160153

161154
this.marketplaceManager = new MarketplaceManager(this.context)
162155
}
163156

157+
private async initializeMcpIfEnabled() {
158+
try {
159+
// Get the mcpEnabled setting from global state
160+
const mcpEnabled = this.contextProxy.getValue("mcpEnabled") ?? true
161+
162+
if (mcpEnabled) {
163+
// Initialize MCP Hub through the singleton manager
164+
const hub = await McpServerManager.getInstance(this.context, this, true)
165+
this.mcpHub = hub
166+
this.mcpHub.registerClient()
167+
} else {
168+
this.log("MCP is disabled, skipping MCP Hub initialization")
169+
// Still create the hub instance but don't initialize servers
170+
const hub = await McpServerManager.getInstance(this.context, this, false)
171+
this.mcpHub = hub
172+
this.mcpHub.registerClient()
173+
}
174+
} catch (error) {
175+
this.log(`Failed to initialize MCP Hub: ${error}`)
176+
}
177+
}
178+
164179
// Adds a new Cline instance to clineStack, marking the start of a new task.
165180
// The instance is pushed to the top of the stack (LIFO order).
166181
// When the task is completed, the top instance is removed, reactivating the previous task.

src/core/webview/webviewMessageHandler.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,17 @@ export const webviewMessageHandler = async (
880880
case "mcpEnabled":
881881
const mcpEnabled = message.bool ?? true
882882
await updateGlobalState("mcpEnabled", mcpEnabled)
883+
884+
// Enable or disable MCP servers based on the setting
885+
const mcpHubInstance = provider.getMcpHub()
886+
if (mcpHubInstance) {
887+
if (mcpEnabled) {
888+
await mcpHubInstance.enableServers()
889+
} else {
890+
await mcpHubInstance.disableServers()
891+
}
892+
}
893+
883894
await provider.postStateToWebview()
884895
break
885896
case "enableMcpServerCreation":

src/services/mcp/McpHub.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,16 @@ export class McpHub {
135135
private refCount: number = 0 // Reference counter for active clients
136136
private configChangeDebounceTimers: Map<string, NodeJS.Timeout> = new Map()
137137

138-
constructor(provider: ClineProvider) {
138+
constructor(provider: ClineProvider, autoInitialize: boolean = true) {
139139
this.providerRef = new WeakRef(provider)
140140
this.watchMcpSettingsFile()
141141
this.watchProjectMcpFile().catch(console.error)
142142
this.setupWorkspaceFoldersWatcher()
143-
this.initializeGlobalMcpServers()
144-
this.initializeProjectMcpServers()
143+
144+
if (autoInitialize) {
145+
this.initializeGlobalMcpServers()
146+
this.initializeProjectMcpServers()
147+
}
145148
}
146149
/**
147150
* Registers a client (e.g., ClineProvider) using this hub.
@@ -1643,4 +1646,38 @@ export class McpHub {
16431646
}
16441647
this.disposables.forEach((d) => d.dispose())
16451648
}
1649+
1650+
/**
1651+
* Enable MCP servers by initializing them
1652+
*/
1653+
async enableServers(): Promise<void> {
1654+
if (this.connections.length > 0) {
1655+
console.log("MCP servers are already initialized")
1656+
return
1657+
}
1658+
1659+
console.log("Enabling MCP servers...")
1660+
await this.initializeGlobalMcpServers()
1661+
await this.initializeProjectMcpServers()
1662+
await this.notifyWebviewOfServerChanges()
1663+
}
1664+
1665+
/**
1666+
* Disable MCP servers by disconnecting all connections
1667+
*/
1668+
async disableServers(): Promise<void> {
1669+
console.log("Disabling MCP servers...")
1670+
1671+
// Disconnect all servers
1672+
const allConnections = [...this.connections]
1673+
for (const conn of allConnections) {
1674+
await this.deleteConnection(conn.server.name, conn.server.source)
1675+
}
1676+
1677+
// Clear connections array
1678+
this.connections = []
1679+
1680+
// Notify webview of changes
1681+
await this.notifyWebviewOfServerChanges()
1682+
}
16461683
}

src/services/mcp/McpServerManager.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ export class McpServerManager {
1717
* Creates a new instance if one doesn't exist.
1818
* Thread-safe implementation using a promise-based lock.
1919
*/
20-
static async getInstance(context: vscode.ExtensionContext, provider: ClineProvider): Promise<McpHub> {
20+
static async getInstance(
21+
context: vscode.ExtensionContext,
22+
provider: ClineProvider,
23+
autoInitialize: boolean = true,
24+
): Promise<McpHub> {
2125
// Register the provider
2226
this.providers.add(provider)
2327

@@ -36,7 +40,7 @@ export class McpServerManager {
3640
try {
3741
// Double-check instance in case it was created while we were waiting
3842
if (!this.instance) {
39-
this.instance = new McpHub(provider)
43+
this.instance = new McpHub(provider, autoInitialize)
4044
// Store a unique identifier in global state to track the primary instance
4145
await context.globalState.update(this.GLOBAL_STATE_KEY, Date.now().toString())
4246
}

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

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,19 @@ vi.mock("vscode", () => ({
6565
Disposable: {
6666
from: vi.fn(),
6767
},
68+
Uri: {
69+
file: vi.fn((path) => ({
70+
scheme: "file",
71+
authority: "",
72+
path,
73+
query: "",
74+
fragment: "",
75+
fsPath: path,
76+
with: vi.fn(),
77+
toJSON: vi.fn(),
78+
})),
79+
},
80+
RelativePattern: vi.fn((base, pattern) => ({ base, pattern })),
6881
}))
6982
vi.mock("fs/promises")
7083
vi.mock("../../../core/webview/ClineProvider")
@@ -1226,4 +1239,144 @@ describe("McpHub", () => {
12261239
)
12271240
})
12281241
})
1242+
1243+
describe("Conditional initialization", () => {
1244+
it("should not initialize servers when autoInitialize is false", async () => {
1245+
// Clear existing mocks
1246+
vi.clearAllMocks()
1247+
1248+
// Create McpHub with autoInitialize = false
1249+
const mcpHubNoInit = new McpHub(mockProvider as ClineProvider, false)
1250+
1251+
// Verify no connections were created
1252+
expect(mcpHubNoInit.connections.length).toBe(0)
1253+
1254+
// Verify fs.readFile was not called for server initialization
1255+
expect(fs.readFile).not.toHaveBeenCalled()
1256+
})
1257+
1258+
it("should initialize servers when autoInitialize is true", async () => {
1259+
// Clear existing mocks
1260+
vi.clearAllMocks()
1261+
1262+
// Mock the config file read
1263+
vi.mocked(fs.readFile).mockResolvedValue(
1264+
JSON.stringify({
1265+
mcpServers: {
1266+
"test-server": {
1267+
type: "stdio",
1268+
command: "node",
1269+
args: ["test.js"],
1270+
},
1271+
},
1272+
}),
1273+
)
1274+
1275+
// Create McpHub with autoInitialize = true
1276+
const mcpHubInit = new McpHub(mockProvider as ClineProvider, true)
1277+
1278+
// Wait a bit for async initialization
1279+
await new Promise((resolve) => setTimeout(resolve, 100))
1280+
1281+
// Verify fs.readFile was called for initialization
1282+
expect(fs.readFile).toHaveBeenCalled()
1283+
})
1284+
1285+
it("should enable servers after creation when enableServers is called", async () => {
1286+
// Clear existing mocks
1287+
vi.clearAllMocks()
1288+
1289+
// Mock the config file read
1290+
vi.mocked(fs.readFile).mockResolvedValue(
1291+
JSON.stringify({
1292+
mcpServers: {
1293+
"test-server": {
1294+
type: "stdio",
1295+
command: "node",
1296+
args: ["test.js"],
1297+
},
1298+
},
1299+
}),
1300+
)
1301+
1302+
// Mock StdioClientTransport
1303+
const mockTransport = {
1304+
start: vi.fn().mockResolvedValue(undefined),
1305+
close: vi.fn().mockResolvedValue(undefined),
1306+
stderr: {
1307+
on: vi.fn(),
1308+
},
1309+
onerror: null,
1310+
onclose: null,
1311+
}
1312+
1313+
const stdioModule = await import("@modelcontextprotocol/sdk/client/stdio.js")
1314+
const StdioClientTransport = stdioModule.StdioClientTransport as ReturnType<typeof vi.fn>
1315+
StdioClientTransport.mockImplementation(() => mockTransport)
1316+
1317+
// Mock Client
1318+
const clientModule = await import("@modelcontextprotocol/sdk/client/index.js")
1319+
const Client = clientModule.Client as ReturnType<typeof vi.fn>
1320+
Client.mockImplementation(() => ({
1321+
connect: vi.fn().mockResolvedValue(undefined),
1322+
close: vi.fn().mockResolvedValue(undefined),
1323+
getInstructions: vi.fn().mockReturnValue("test instructions"),
1324+
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
1325+
}))
1326+
1327+
// Create McpHub without auto-initialization
1328+
const mcpHubNoInit = new McpHub(mockProvider as ClineProvider, false)
1329+
1330+
// Verify no connections initially
1331+
expect(mcpHubNoInit.connections.length).toBe(0)
1332+
1333+
// Enable servers
1334+
await mcpHubNoInit.enableServers()
1335+
1336+
// Verify connections were created
1337+
expect(mcpHubNoInit.connections.length).toBeGreaterThan(0)
1338+
})
1339+
1340+
it("should disable all servers when disableServers is called", async () => {
1341+
// Create McpHub with some connections
1342+
mcpHub.connections = [
1343+
{
1344+
server: {
1345+
name: "test-server-1",
1346+
source: "global",
1347+
} as any,
1348+
client: {
1349+
close: vi.fn().mockResolvedValue(undefined),
1350+
} as any,
1351+
transport: {
1352+
close: vi.fn().mockResolvedValue(undefined),
1353+
} as any,
1354+
},
1355+
{
1356+
server: {
1357+
name: "test-server-2",
1358+
source: "project",
1359+
} as any,
1360+
client: {
1361+
close: vi.fn().mockResolvedValue(undefined),
1362+
} as any,
1363+
transport: {
1364+
close: vi.fn().mockResolvedValue(undefined),
1365+
} as any,
1366+
},
1367+
]
1368+
1369+
// Disable servers
1370+
await mcpHub.disableServers()
1371+
1372+
// Verify all connections were closed
1373+
expect(mcpHub.connections.length).toBe(0)
1374+
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith(
1375+
expect.objectContaining({
1376+
type: "mcpServers",
1377+
mcpServers: [],
1378+
}),
1379+
)
1380+
})
1381+
})
12291382
})

0 commit comments

Comments
 (0)