Skip to content

Commit ec4b453

Browse files
committed
fix: prevent disabled MCP servers from starting (#2797)
- Add disabled checks before calling connectToServer() in updateServerConnections() - Prevent new disabled servers from connecting (line 977) - Disconnect servers when they become disabled (line 985-993) - Add test cases for disabled server connection prevention - Follows existing pattern used in readResource() and callTool() methods
1 parent 8c34976 commit ec4b453

File tree

2 files changed

+112
-11
lines changed

2 files changed

+112
-11
lines changed

src/services/mcp/McpHub.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -974,20 +974,31 @@ export class McpHub {
974974

975975
if (!currentConnection) {
976976
// New server
977-
try {
978-
this.setupFileWatcher(name, validatedConfig, source)
979-
await this.connectToServer(name, validatedConfig, source)
980-
} catch (error) {
981-
this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error)
977+
if (!validatedConfig.disabled) {
978+
try {
979+
this.setupFileWatcher(name, validatedConfig, source)
980+
await this.connectToServer(name, validatedConfig, source)
981+
} catch (error) {
982+
this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error)
983+
}
982984
}
983985
} else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) {
984986
// Existing server with changed config
985-
try {
986-
this.setupFileWatcher(name, validatedConfig, source)
987-
await this.deleteConnection(name, source)
988-
await this.connectToServer(name, validatedConfig, source)
989-
} catch (error) {
990-
this.showErrorMessage(`Failed to reconnect MCP server ${name}`, error)
987+
if (!validatedConfig.disabled) {
988+
try {
989+
this.setupFileWatcher(name, validatedConfig, source)
990+
await this.deleteConnection(name, source)
991+
await this.connectToServer(name, validatedConfig, source)
992+
} catch (error) {
993+
this.showErrorMessage(`Failed to reconnect MCP server ${name}`, error)
994+
}
995+
} else {
996+
// Server is now disabled, disconnect it
997+
try {
998+
await this.deleteConnection(name, source)
999+
} catch (error) {
1000+
this.showErrorMessage(`Failed to disconnect disabled MCP server ${name}`, error)
1001+
}
9911002
}
9921003
}
9931004
// If server exists with same config, do nothing

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,96 @@ describe("McpHub", () => {
570570
'Server "disabled-server" is disabled',
571571
)
572572
})
573+
574+
it("should not connect to disabled servers during updateServerConnections", async () => {
575+
const mockConfig = {
576+
mcpServers: {
577+
"disabled-server": {
578+
type: "stdio",
579+
command: "node",
580+
args: ["test.js"],
581+
disabled: true,
582+
},
583+
},
584+
}
585+
586+
// Mock reading initial config
587+
vi.mocked(fs.readFile).mockResolvedValueOnce(JSON.stringify(mockConfig))
588+
589+
// Ensure no connections exist initially
590+
mcpHub.connections = []
591+
592+
// Mock the connectToServer method to track if it's called
593+
const connectToServerSpy = vi.spyOn(mcpHub as any, "connectToServer")
594+
595+
// Update server connections with disabled server
596+
await mcpHub.updateServerConnections(mockConfig.mcpServers, "global", false)
597+
598+
// Verify that connectToServer was never called for the disabled server
599+
expect(connectToServerSpy).not.toHaveBeenCalled()
600+
601+
// Verify no connections were created
602+
expect(mcpHub.connections.length).toBe(0)
603+
})
604+
605+
it("should disconnect server when it becomes disabled", async () => {
606+
// First, set up an enabled server
607+
const enabledConfig = {
608+
mcpServers: {
609+
"test-server": {
610+
type: "stdio",
611+
command: "node",
612+
args: ["test.js"],
613+
disabled: false,
614+
},
615+
},
616+
}
617+
618+
// Mock reading initial config
619+
vi.mocked(fs.readFile).mockResolvedValueOnce(JSON.stringify(enabledConfig))
620+
621+
// Set up existing connection
622+
const mockConnection: McpConnection = {
623+
server: {
624+
name: "test-server",
625+
config: JSON.stringify(enabledConfig.mcpServers["test-server"]),
626+
status: "connected",
627+
disabled: false,
628+
source: "global",
629+
},
630+
client: {
631+
close: vi.fn().mockResolvedValue(undefined),
632+
} as any,
633+
transport: {
634+
close: vi.fn().mockResolvedValue(undefined),
635+
} as any,
636+
}
637+
mcpHub.connections = [mockConnection]
638+
639+
// Now update with disabled config
640+
const disabledConfig = {
641+
mcpServers: {
642+
"test-server": {
643+
type: "stdio",
644+
command: "node",
645+
args: ["test.js"],
646+
disabled: true,
647+
},
648+
},
649+
}
650+
651+
// Mock reading disabled config
652+
vi.mocked(fs.readFile).mockResolvedValueOnce(JSON.stringify(disabledConfig))
653+
654+
// Mock the deleteConnection method to track if it's called
655+
const deleteConnectionSpy = vi.spyOn(mcpHub as any, "deleteConnection")
656+
657+
// Update server connections with disabled server
658+
await mcpHub.updateServerConnections(disabledConfig.mcpServers, "global", false)
659+
660+
// Verify that deleteConnection was called to disconnect the server
661+
expect(deleteConnectionSpy).toHaveBeenCalledWith("test-server", "global")
662+
})
573663
})
574664

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

0 commit comments

Comments
 (0)