Skip to content

Commit 9ee690e

Browse files
committed
fix: prevent MCP servers from starting when MCP is globally disabled
- Added check for global mcpEnabled state in connectToServer method - Updated refreshAllConnections to respect global MCP setting - Updated restartConnection to check global MCP state - Added tests to verify servers don't start when MCP is disabled - Ensures disabled servers show correct status in UI
1 parent a2a8bfc commit 9ee690e

File tree

2 files changed

+178
-0
lines changed

2 files changed

+178
-0
lines changed

src/services/mcp/McpHub.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,34 @@ export class McpHub {
561561
// Remove existing connection if it exists with the same source
562562
await this.deleteConnection(name, source)
563563

564+
// Check if MCP is globally enabled
565+
const provider = this.providerRef.deref()
566+
if (provider) {
567+
const state = await provider.getState()
568+
const mcpEnabled = state.mcpEnabled ?? true
569+
570+
// Skip connecting if MCP is globally disabled
571+
if (!mcpEnabled) {
572+
// Still create a connection object to track the server, but don't actually connect
573+
const connection: McpConnection = {
574+
server: {
575+
name,
576+
config: JSON.stringify(config),
577+
status: "disconnected",
578+
disabled: config.disabled,
579+
source,
580+
projectPath:
581+
source === "project" ? vscode.workspace.workspaceFolders?.[0]?.uri.fsPath : undefined,
582+
errorHistory: [],
583+
},
584+
client: null as any, // We won't actually create a client when MCP is disabled
585+
transport: null as any, // We won't actually create a transport when MCP is disabled
586+
}
587+
this.connections.push(connection)
588+
return
589+
}
590+
}
591+
564592
// Skip connecting to disabled servers
565593
if (config.disabled) {
566594
// Still create a connection object to track the server, but don't actually connect
@@ -1100,6 +1128,16 @@ export class McpHub {
11001128
return
11011129
}
11021130

1131+
// Check if MCP is globally enabled
1132+
const state = await provider.getState()
1133+
const mcpEnabled = state.mcpEnabled ?? true
1134+
1135+
// Skip restarting if MCP is globally disabled
1136+
if (!mcpEnabled) {
1137+
this.isConnecting = false
1138+
return
1139+
}
1140+
11031141
// Get existing connection and update its status
11041142
const connection = this.findConnection(serverName, source)
11051143
const config = connection?.server.config
@@ -1138,6 +1176,29 @@ export class McpHub {
11381176
return
11391177
}
11401178

1179+
// Check if MCP is globally enabled
1180+
const provider = this.providerRef.deref()
1181+
if (provider) {
1182+
const state = await provider.getState()
1183+
const mcpEnabled = state.mcpEnabled ?? true
1184+
1185+
// Skip refreshing if MCP is globally disabled
1186+
if (!mcpEnabled) {
1187+
// Clear all existing connections
1188+
const existingConnections = [...this.connections]
1189+
for (const conn of existingConnections) {
1190+
await this.deleteConnection(conn.server.name, conn.server.source)
1191+
}
1192+
1193+
// Still initialize servers to track them, but they won't connect
1194+
await this.initializeMcpServers("global")
1195+
await this.initializeMcpServers("project")
1196+
1197+
await this.notifyWebviewOfServerChanges()
1198+
return
1199+
}
1200+
}
1201+
11411202
this.isConnecting = true
11421203
vscode.window.showInformationMessage(t("mcp:info.refreshing_all"))
11431204

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

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ describe("McpHub", () => {
108108
ensureSettingsDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"),
109109
ensureMcpServersDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"),
110110
postMessageToWebview: vi.fn(),
111+
getState: vi.fn().mockResolvedValue({ mcpEnabled: true }),
111112
context: {
112113
subscriptions: [],
113114
workspaceState: {} as any,
@@ -877,6 +878,122 @@ describe("McpHub", () => {
877878
})
878879
})
879880

881+
describe("MCP global enable/disable", () => {
882+
beforeEach(() => {
883+
// Clear all mocks before each test
884+
vi.clearAllMocks()
885+
})
886+
887+
it("should not connect to servers when MCP is globally disabled", async () => {
888+
// Mock provider with mcpEnabled: false
889+
const disabledMockProvider = {
890+
ensureSettingsDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"),
891+
ensureMcpServersDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"),
892+
postMessageToWebview: vi.fn(),
893+
getState: vi.fn().mockResolvedValue({ mcpEnabled: false }),
894+
context: mockProvider.context,
895+
}
896+
897+
// Mock the config file read with a different server name to avoid conflicts
898+
vi.mocked(fs.readFile).mockResolvedValue(
899+
JSON.stringify({
900+
mcpServers: {
901+
"disabled-test-server": {
902+
command: "node",
903+
args: ["test.js"],
904+
},
905+
},
906+
}),
907+
)
908+
909+
// Create a new McpHub instance with disabled MCP
910+
const mcpHub = new McpHub(disabledMockProvider as unknown as ClineProvider)
911+
912+
// Wait for initialization
913+
await new Promise((resolve) => setTimeout(resolve, 100))
914+
915+
// Find the disabled-test-server
916+
const disabledServer = mcpHub.connections.find((conn) => conn.server.name === "disabled-test-server")
917+
918+
// Verify that the server is tracked but not connected
919+
expect(disabledServer).toBeDefined()
920+
expect(disabledServer!.server.status).toBe("disconnected")
921+
expect(disabledServer!.client).toBeNull()
922+
expect(disabledServer!.transport).toBeNull()
923+
})
924+
925+
it("should connect to servers when MCP is globally enabled", async () => {
926+
// Clear all mocks
927+
vi.clearAllMocks()
928+
929+
// Mock StdioClientTransport
930+
const stdioModule = await import("@modelcontextprotocol/sdk/client/stdio.js")
931+
const StdioClientTransport = stdioModule.StdioClientTransport as ReturnType<typeof vi.fn>
932+
933+
const mockTransport = {
934+
start: vi.fn().mockResolvedValue(undefined),
935+
close: vi.fn().mockResolvedValue(undefined),
936+
stderr: {
937+
on: vi.fn(),
938+
},
939+
onerror: null,
940+
onclose: null,
941+
}
942+
943+
StdioClientTransport.mockImplementation(() => mockTransport)
944+
945+
// Mock Client
946+
const clientModule = await import("@modelcontextprotocol/sdk/client/index.js")
947+
const Client = clientModule.Client as ReturnType<typeof vi.fn>
948+
949+
Client.mockImplementation(() => ({
950+
connect: vi.fn().mockResolvedValue(undefined),
951+
close: vi.fn().mockResolvedValue(undefined),
952+
getInstructions: vi.fn().mockReturnValue("test instructions"),
953+
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
954+
}))
955+
956+
// Mock provider with mcpEnabled: true
957+
const enabledMockProvider = {
958+
ensureSettingsDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"),
959+
ensureMcpServersDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"),
960+
postMessageToWebview: vi.fn(),
961+
getState: vi.fn().mockResolvedValue({ mcpEnabled: true }),
962+
context: mockProvider.context,
963+
}
964+
965+
// Mock the config file read with a different server name
966+
vi.mocked(fs.readFile).mockResolvedValue(
967+
JSON.stringify({
968+
mcpServers: {
969+
"enabled-test-server": {
970+
command: "node",
971+
args: ["test.js"],
972+
},
973+
},
974+
}),
975+
)
976+
977+
// Create a new McpHub instance with enabled MCP
978+
const mcpHub = new McpHub(enabledMockProvider as unknown as ClineProvider)
979+
980+
// Wait for initialization
981+
await new Promise((resolve) => setTimeout(resolve, 100))
982+
983+
// Find the enabled-test-server
984+
const enabledServer = mcpHub.connections.find((conn) => conn.server.name === "enabled-test-server")
985+
986+
// Verify that the server is connected
987+
expect(enabledServer).toBeDefined()
988+
expect(enabledServer!.server.status).toBe("connected")
989+
expect(enabledServer!.client).toBeDefined()
990+
expect(enabledServer!.transport).toBeDefined()
991+
992+
// Verify StdioClientTransport was called
993+
expect(StdioClientTransport).toHaveBeenCalled()
994+
})
995+
})
996+
880997
describe("Windows command wrapping", () => {
881998
let StdioClientTransport: ReturnType<typeof vi.fn>
882999
let Client: ReturnType<typeof vi.fn>

0 commit comments

Comments
 (0)