fix: prevent disabled MCP servers from starting processes and show correct status (#6084)

Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: hannesrudolph <hrudolph@gmail.com>
Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
This commit is contained in:
roomote[bot] 2025-08-05 16:55:30 -07:00 committed by GitHub
parent 263e317ebd
commit 7a865e26c4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 1378 additions and 232 deletions

View file

@ -901,6 +901,13 @@ export const webviewMessageHandler = async (
case "mcpEnabled":
const mcpEnabled = message.bool ?? true
await updateGlobalState("mcpEnabled", mcpEnabled)
// Delegate MCP enable/disable logic to McpHub
const mcpHubInstance = provider.getMcpHub()
if (mcpHubInstance) {
await mcpHubInstance.handleMcpEnabledChange(mcpEnabled)
}
await provider.postStateToWebview()
break
case "enableMcpServerCreation":

View file

@ -33,12 +33,29 @@ import { fileExistsAtPath } from "../../utils/fs"
import { arePathsEqual } from "../../utils/path"
import { injectVariables } from "../../utils/config"
export type McpConnection = {
// Discriminated union for connection states
export type ConnectedMcpConnection = {
type: "connected"
server: McpServer
client: Client
transport: StdioClientTransport | SSEClientTransport | StreamableHTTPClientTransport
}
export type DisconnectedMcpConnection = {
type: "disconnected"
server: McpServer
client: null
transport: null
}
export type McpConnection = ConnectedMcpConnection | DisconnectedMcpConnection
// Enum for disable reasons
export enum DisableReason {
MCP_DISABLED = "mcpDisabled",
SERVER_DISABLED = "serverDisabled",
}
// Base configuration schema for common settings
const BaseConfigSchema = z.object({
disabled: z.boolean().optional(),
@ -497,6 +514,7 @@ export class McpHub {
const result = McpSettingsSchema.safeParse(config)
if (result.success) {
// Pass all servers including disabled ones - they'll be handled in updateServerConnections
await this.updateServerConnections(result.data.mcpServers || {}, source, false)
} else {
const errorMessages = result.error.errors
@ -552,6 +570,49 @@ export class McpHub {
await this.initializeMcpServers("project")
}
/**
* Creates a placeholder connection for disabled servers or when MCP is globally disabled
* @param name The server name
* @param config The server configuration
* @param source The source of the server (global or project)
* @param reason The reason for creating a placeholder (mcpDisabled or serverDisabled)
* @returns A placeholder DisconnectedMcpConnection object
*/
private createPlaceholderConnection(
name: string,
config: z.infer<typeof ServerConfigSchema>,
source: "global" | "project",
reason: DisableReason,
): DisconnectedMcpConnection {
return {
type: "disconnected",
server: {
name,
config: JSON.stringify(config),
status: "disconnected",
disabled: reason === DisableReason.SERVER_DISABLED ? true : config.disabled,
source,
projectPath: source === "project" ? vscode.workspace.workspaceFolders?.[0]?.uri.fsPath : undefined,
errorHistory: [],
},
client: null,
transport: null,
}
}
/**
* Checks if MCP is globally enabled
* @returns Promise<boolean> indicating if MCP is enabled
*/
private async isMcpEnabled(): Promise<boolean> {
const provider = this.providerRef.deref()
if (!provider) {
return true // Default to enabled if provider is not available
}
const state = await provider.getState()
return state.mcpEnabled ?? true
}
private async connectToServer(
name: string,
config: z.infer<typeof ServerConfigSchema>,
@ -560,6 +621,26 @@ export class McpHub {
// Remove existing connection if it exists with the same source
await this.deleteConnection(name, source)
// Check if MCP is globally enabled
const mcpEnabled = await this.isMcpEnabled()
if (!mcpEnabled) {
// Still create a connection object to track the server, but don't actually connect
const connection = this.createPlaceholderConnection(name, config, source, DisableReason.MCP_DISABLED)
this.connections.push(connection)
return
}
// Skip connecting to disabled servers
if (config.disabled) {
// Still create a connection object to track the server, but don't actually connect
const connection = this.createPlaceholderConnection(name, config, source, DisableReason.SERVER_DISABLED)
this.connections.push(connection)
return
}
// Set up file watchers for enabled servers
this.setupFileWatcher(name, config, source)
try {
const client = new Client(
{
@ -733,7 +814,9 @@ export class McpHub {
transport.start = async () => {}
}
const connection: McpConnection = {
// Create a connected connection
const connection: ConnectedMcpConnection = {
type: "connected",
server: {
name,
config: JSON.stringify(configInjected),
@ -826,8 +909,8 @@ export class McpHub {
// Use the helper method to find the connection
const connection = this.findConnection(serverName, source)
if (!connection) {
throw new Error(`Server ${serverName} not found`)
if (!connection || connection.type !== "connected") {
return []
}
const response = await connection.client.request({ method: "tools/list" }, ListToolsResultSchema)
@ -881,7 +964,7 @@ export class McpHub {
private async fetchResourcesList(serverName: string, source?: "global" | "project"): Promise<McpResource[]> {
try {
const connection = this.findConnection(serverName, source)
if (!connection) {
if (!connection || connection.type !== "connected") {
return []
}
const response = await connection.client.request({ method: "resources/list" }, ListResourcesResultSchema)
@ -898,7 +981,7 @@ export class McpHub {
): Promise<McpResourceTemplate[]> {
try {
const connection = this.findConnection(serverName, source)
if (!connection) {
if (!connection || connection.type !== "connected") {
return []
}
const response = await connection.client.request(
@ -913,6 +996,9 @@ export class McpHub {
}
async deleteConnection(name: string, source?: "global" | "project"): Promise<void> {
// Clean up file watchers for this server
this.removeFileWatchersForServer(name)
// If source is provided, only delete connections from that source
const connections = source
? this.connections.filter((conn) => conn.server.name === name && conn.server.source === source)
@ -920,8 +1006,10 @@ export class McpHub {
for (const connection of connections) {
try {
await connection.transport.close()
await connection.client.close()
if (connection.type === "connected") {
await connection.transport.close()
await connection.client.close()
}
} catch (error) {
console.error(`Failed to close transport for ${name}:`, error)
}
@ -975,7 +1063,10 @@ export class McpHub {
if (!currentConnection) {
// New server
try {
this.setupFileWatcher(name, validatedConfig, source)
// Only setup file watcher for enabled servers
if (!validatedConfig.disabled) {
this.setupFileWatcher(name, validatedConfig, source)
}
await this.connectToServer(name, validatedConfig, source)
} catch (error) {
this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error)
@ -983,7 +1074,10 @@ export class McpHub {
} else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) {
// Existing server with changed config
try {
this.setupFileWatcher(name, validatedConfig, source)
// Only setup file watcher for enabled servers
if (!validatedConfig.disabled) {
this.setupFileWatcher(name, validatedConfig, source)
}
await this.deleteConnection(name, source)
await this.connectToServer(name, validatedConfig, source)
} catch (error) {
@ -1066,10 +1160,21 @@ export class McpHub {
this.fileWatchers.clear()
}
private removeFileWatchersForServer(serverName: string) {
const watchers = this.fileWatchers.get(serverName)
if (watchers) {
watchers.forEach((watcher) => watcher.close())
this.fileWatchers.delete(serverName)
}
}
async restartConnection(serverName: string, source?: "global" | "project"): Promise<void> {
this.isConnecting = true
const provider = this.providerRef.deref()
if (!provider) {
// Check if MCP is globally enabled
const mcpEnabled = await this.isMcpEnabled()
if (!mcpEnabled) {
this.isConnecting = false
return
}
@ -1111,6 +1216,23 @@ export class McpHub {
return
}
// Check if MCP is globally enabled
const mcpEnabled = await this.isMcpEnabled()
if (!mcpEnabled) {
// Clear all existing connections
const existingConnections = [...this.connections]
for (const conn of existingConnections) {
await this.deleteConnection(conn.server.name, conn.server.source)
}
// Still initialize servers to track them, but they won't connect
await this.initializeMcpServers("global")
await this.initializeMcpServers("project")
await this.notifyWebviewOfServerChanges()
return
}
this.isConnecting = true
vscode.window.showInformationMessage(t("mcp:info.refreshing_all"))
@ -1257,8 +1379,21 @@ export class McpHub {
try {
connection.server.disabled = disabled
// Only refresh capabilities if connected
if (connection.server.status === "connected") {
// If disabling a connected server, disconnect it
if (disabled && connection.server.status === "connected") {
// Clean up file watchers when disabling
this.removeFileWatchersForServer(serverName)
await this.deleteConnection(serverName, serverSource)
// Re-add as a disabled connection
await this.connectToServer(serverName, JSON.parse(connection.server.config), serverSource)
} else if (!disabled && connection.server.status === "disconnected") {
// If enabling a disabled server, connect it
const config = JSON.parse(connection.server.config)
await this.deleteConnection(serverName, serverSource)
// When re-enabling, file watchers will be set up in connectToServer
await this.connectToServer(serverName, config, serverSource)
} else if (connection.server.status === "connected") {
// Only refresh capabilities if connected
connection.server.tools = await this.fetchToolsList(serverName, serverSource)
connection.server.resources = await this.fetchResourcesList(serverName, serverSource)
connection.server.resourceTemplates = await this.fetchResourceTemplatesList(
@ -1439,7 +1574,7 @@ export class McpHub {
async readResource(serverName: string, uri: string, source?: "global" | "project"): Promise<McpResourceResponse> {
const connection = this.findConnection(serverName, source)
if (!connection) {
if (!connection || connection.type !== "connected") {
throw new Error(`No connection found for server: ${serverName}${source ? ` with source ${source}` : ""}`)
}
if (connection.server.disabled) {
@ -1463,7 +1598,7 @@ export class McpHub {
source?: "global" | "project",
): Promise<McpToolCallResponse> {
const connection = this.findConnection(serverName, source)
if (!connection) {
if (!connection || connection.type !== "connected") {
throw new Error(
`No connection found for server: ${serverName}${source ? ` with source ${source}` : ""}. Please make sure to use MCP servers available under 'Connected MCP Servers'.`,
)
@ -1609,6 +1744,64 @@ export class McpHub {
}
}
/**
* Handles enabling/disabling MCP globally
* @param enabled Whether MCP should be enabled or disabled
* @returns Promise<void>
*/
async handleMcpEnabledChange(enabled: boolean): Promise<void> {
if (!enabled) {
// If MCP is being disabled, disconnect all servers with error handling
const existingConnections = [...this.connections]
const disconnectionErrors: Array<{ serverName: string; error: string }> = []
for (const conn of existingConnections) {
try {
await this.deleteConnection(conn.server.name, conn.server.source)
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
disconnectionErrors.push({
serverName: conn.server.name,
error: errorMessage,
})
console.error(`Failed to disconnect MCP server ${conn.server.name}: ${errorMessage}`)
}
}
// If there were errors, notify the user
if (disconnectionErrors.length > 0) {
const errorSummary = disconnectionErrors.map((e) => `${e.serverName}: ${e.error}`).join("\n")
vscode.window.showWarningMessage(
t("mcp:errors.disconnect_servers_partial", {
count: disconnectionErrors.length,
errors: errorSummary,
}) ||
`Failed to disconnect ${disconnectionErrors.length} MCP server(s). Check the output for details.`,
)
}
// Re-initialize servers to track them in disconnected state
try {
await this.refreshAllConnections()
} catch (error) {
console.error(`Failed to refresh MCP connections after disabling: ${error}`)
vscode.window.showErrorMessage(
t("mcp:errors.refresh_after_disable") || "Failed to refresh MCP connections after disabling",
)
}
} else {
// If MCP is being enabled, reconnect all servers
try {
await this.refreshAllConnections()
} catch (error) {
console.error(`Failed to refresh MCP connections after enabling: ${error}`)
vscode.window.showErrorMessage(
t("mcp:errors.refresh_after_enable") || "Failed to refresh MCP connections after enabling",
)
}
}
}
async dispose(): Promise<void> {
// Prevent multiple disposals
if (this.isDisposed) {

File diff suppressed because it is too large Load diff

View file

@ -206,6 +206,9 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
return configTimeout ?? 60 // Default 1 minute (60 seconds)
})
// Computed property to check if server is expandable
const isExpandable = server.status === "connected" && !server.disabled
const timeoutOptions = [
{ value: 15, label: t("mcp:networkTimeout.options.15seconds") },
{ value: 30, label: t("mcp:networkTimeout.options.30seconds") },
@ -218,6 +221,11 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
]
const getStatusColor = () => {
// Disabled servers should always show grey regardless of connection status
if (server.disabled) {
return "var(--vscode-descriptionForeground)"
}
switch (server.status) {
case "connected":
return "var(--vscode-testing-iconPassed)"
@ -229,7 +237,8 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
}
const handleRowClick = () => {
if (server.status === "connected") {
// Only allow expansion for connected and enabled servers
if (isExpandable) {
setIsExpanded(!isExpanded)
}
}
@ -270,12 +279,12 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
alignItems: "center",
padding: "8px",
background: "var(--vscode-textCodeBlock-background)",
cursor: server.status === "connected" ? "pointer" : "default",
borderRadius: isExpanded || server.status === "connected" ? "4px" : "4px 4px 0 0",
cursor: isExpandable ? "pointer" : "default",
borderRadius: isExpanded || isExpandable ? "4px" : "4px 4px 0 0",
opacity: server.disabled ? 0.6 : 1,
}}
onClick={handleRowClick}>
{server.status === "connected" && (
{isExpandable && (
<span
className={`codicon codicon-chevron-${isExpanded ? "down" : "right"}`}
style={{ marginRight: "8px" }}
@ -342,176 +351,195 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
</div>
</div>
{server.status === "connected" ? (
isExpanded && (
<div
style={{
background: "var(--vscode-textCodeBlock-background)",
padding: "0 10px 10px 10px",
fontSize: "13px",
borderRadius: "0 0 4px 4px",
}}>
<VSCodePanels style={{ marginBottom: "10px" }}>
<VSCodePanelTab id="tools">
{t("mcp:tabs.tools")} ({server.tools?.length || 0})
</VSCodePanelTab>
<VSCodePanelTab id="resources">
{t("mcp:tabs.resources")} (
{[...(server.resourceTemplates || []), ...(server.resources || [])].length || 0})
</VSCodePanelTab>
{server.instructions && (
<VSCodePanelTab id="instructions">{t("mcp:instructions")}</VSCodePanelTab>
)}
<VSCodePanelTab id="errors">
{t("mcp:tabs.errors")} ({server.errorHistory?.length || 0})
</VSCodePanelTab>
<VSCodePanelView id="tools-view">
{server.tools && server.tools.length > 0 ? (
<div
style={{ display: "flex", flexDirection: "column", gap: "8px", width: "100%" }}>
{server.tools.map((tool) => (
<McpToolRow
key={`${tool.name}-${server.name}-${server.source || "global"}`}
tool={tool}
serverName={server.name}
serverSource={server.source || "global"}
alwaysAllowMcp={alwaysAllowMcp}
/>
))}
</div>
) : (
<div style={{ padding: "10px 0", color: "var(--vscode-descriptionForeground)" }}>
{t("mcp:emptyState.noTools")}
</div>
{isExpandable
? isExpanded && (
<div
style={{
background: "var(--vscode-textCodeBlock-background)",
padding: "0 10px 10px 10px",
fontSize: "13px",
borderRadius: "0 0 4px 4px",
}}>
<VSCodePanels style={{ marginBottom: "10px" }}>
<VSCodePanelTab id="tools">
{t("mcp:tabs.tools")} ({server.tools?.length || 0})
</VSCodePanelTab>
<VSCodePanelTab id="resources">
{t("mcp:tabs.resources")} (
{[...(server.resourceTemplates || []), ...(server.resources || [])].length || 0})
</VSCodePanelTab>
{server.instructions && (
<VSCodePanelTab id="instructions">{t("mcp:instructions")}</VSCodePanelTab>
)}
</VSCodePanelView>
<VSCodePanelTab id="errors">
{t("mcp:tabs.errors")} ({server.errorHistory?.length || 0})
</VSCodePanelTab>
<VSCodePanelView id="resources-view">
{(server.resources && server.resources.length > 0) ||
(server.resourceTemplates && server.resourceTemplates.length > 0) ? (
<div
style={{ display: "flex", flexDirection: "column", gap: "8px", width: "100%" }}>
{[...(server.resourceTemplates || []), ...(server.resources || [])].map(
(item) => (
<McpResourceRow
key={"uriTemplate" in item ? item.uriTemplate : item.uri}
item={item}
<VSCodePanelView id="tools-view">
{server.tools && server.tools.length > 0 ? (
<div
style={{
display: "flex",
flexDirection: "column",
gap: "8px",
width: "100%",
}}>
{server.tools.map((tool) => (
<McpToolRow
key={`${tool.name}-${server.name}-${server.source || "global"}`}
tool={tool}
serverName={server.name}
serverSource={server.source || "global"}
alwaysAllowMcp={alwaysAllowMcp}
/>
),
)}
</div>
) : (
<div style={{ padding: "10px 0", color: "var(--vscode-descriptionForeground)" }}>
{t("mcp:emptyState.noResources")}
</div>
)}
</VSCodePanelView>
{server.instructions && (
<VSCodePanelView id="instructions-view">
<div style={{ padding: "10px 0", fontSize: "12px" }}>
<div className="opacity-80 whitespace-pre-wrap break-words">
{server.instructions}
</div>
</div>
</VSCodePanelView>
)}
<VSCodePanelView id="errors-view">
{server.errorHistory && server.errorHistory.length > 0 ? (
<div
style={{ display: "flex", flexDirection: "column", gap: "8px", width: "100%" }}>
{[...server.errorHistory]
.sort((a, b) => b.timestamp - a.timestamp)
.map((error, index) => (
<McpErrorRow key={`${error.timestamp}-${index}`} error={error} />
))}
</div>
) : (
<div style={{ padding: "10px 0", color: "var(--vscode-descriptionForeground)" }}>
{t("mcp:emptyState.noErrors")}
</div>
)}
</VSCodePanelView>
</VSCodePanels>
</div>
) : (
<div
style={{ padding: "10px 0", color: "var(--vscode-descriptionForeground)" }}>
{t("mcp:emptyState.noTools")}
</div>
)}
</VSCodePanelView>
{/* Network Timeout */}
<div style={{ padding: "10px 7px" }}>
<VSCodePanelView id="resources-view">
{(server.resources && server.resources.length > 0) ||
(server.resourceTemplates && server.resourceTemplates.length > 0) ? (
<div
style={{
display: "flex",
flexDirection: "column",
gap: "8px",
width: "100%",
}}>
{[...(server.resourceTemplates || []), ...(server.resources || [])].map(
(item) => (
<McpResourceRow
key={"uriTemplate" in item ? item.uriTemplate : item.uri}
item={item}
/>
),
)}
</div>
) : (
<div
style={{ padding: "10px 0", color: "var(--vscode-descriptionForeground)" }}>
{t("mcp:emptyState.noResources")}
</div>
)}
</VSCodePanelView>
{server.instructions && (
<VSCodePanelView id="instructions-view">
<div style={{ padding: "10px 0", fontSize: "12px" }}>
<div className="opacity-80 whitespace-pre-wrap break-words">
{server.instructions}
</div>
</div>
</VSCodePanelView>
)}
<VSCodePanelView id="errors-view">
{server.errorHistory && server.errorHistory.length > 0 ? (
<div
style={{
display: "flex",
flexDirection: "column",
gap: "8px",
width: "100%",
}}>
{[...server.errorHistory]
.sort((a, b) => b.timestamp - a.timestamp)
.map((error, index) => (
<McpErrorRow key={`${error.timestamp}-${index}`} error={error} />
))}
</div>
) : (
<div
style={{ padding: "10px 0", color: "var(--vscode-descriptionForeground)" }}>
{t("mcp:emptyState.noErrors")}
</div>
)}
</VSCodePanelView>
</VSCodePanels>
{/* Network Timeout */}
<div style={{ padding: "10px 7px" }}>
<div
style={{
display: "flex",
alignItems: "center",
gap: "10px",
marginBottom: "8px",
}}>
<span>{t("mcp:networkTimeout.label")}</span>
<select
value={timeoutValue}
onChange={handleTimeoutChange}
style={{
flex: 1,
padding: "4px",
background: "var(--vscode-dropdown-background)",
color: "var(--vscode-dropdown-foreground)",
border: "1px solid var(--vscode-dropdown-border)",
borderRadius: "2px",
outline: "none",
cursor: "pointer",
}}>
{timeoutOptions.map((option) => (
<option key={option.value} value={option.value}>
{option.label}
</option>
))}
</select>
</div>
<span
style={{
fontSize: "12px",
color: "var(--vscode-descriptionForeground)",
display: "block",
}}>
{t("mcp:networkTimeout.description")}
</span>
</div>
</div>
)
: // Only show error UI for non-disabled servers
!server.disabled && (
<div
style={{
fontSize: "13px",
background: "var(--vscode-textCodeBlock-background)",
borderRadius: "0 0 4px 4px",
width: "100%",
}}>
<div
style={{
display: "flex",
alignItems: "center",
gap: "10px",
color: "var(--vscode-testing-iconFailed)",
marginBottom: "8px",
padding: "0 10px",
overflowWrap: "break-word",
wordBreak: "break-word",
}}>
<span>{t("mcp:networkTimeout.label")}</span>
<select
value={timeoutValue}
onChange={handleTimeoutChange}
style={{
flex: 1,
padding: "4px",
background: "var(--vscode-dropdown-background)",
color: "var(--vscode-dropdown-foreground)",
border: "1px solid var(--vscode-dropdown-border)",
borderRadius: "2px",
outline: "none",
cursor: "pointer",
}}>
{timeoutOptions.map((option) => (
<option key={option.value} value={option.value}>
{option.label}
</option>
{server.error &&
server.error.split("\n").map((item, index) => (
<React.Fragment key={index}>
{index > 0 && <br />}
{item}
</React.Fragment>
))}
</select>
</div>
<span
style={{
fontSize: "12px",
color: "var(--vscode-descriptionForeground)",
display: "block",
}}>
{t("mcp:networkTimeout.description")}
</span>
<VSCodeButton
appearance="secondary"
onClick={handleRestart}
disabled={server.status === "connecting"}
style={{ width: "calc(100% - 20px)", margin: "0 10px 10px 10px" }}>
{server.status === "connecting"
? t("mcp:serverStatus.retrying")
: t("mcp:serverStatus.retryConnection")}
</VSCodeButton>
</div>
</div>
)
) : (
<div
style={{
fontSize: "13px",
background: "var(--vscode-textCodeBlock-background)",
borderRadius: "0 0 4px 4px",
width: "100%",
}}>
<div
style={{
color: "var(--vscode-testing-iconFailed)",
marginBottom: "8px",
padding: "0 10px",
overflowWrap: "break-word",
wordBreak: "break-word",
}}>
{server.error &&
server.error.split("\n").map((item, index) => (
<React.Fragment key={index}>
{index > 0 && <br />}
{item}
</React.Fragment>
))}
</div>
<VSCodeButton
appearance="secondary"
onClick={handleRestart}
disabled={server.status === "connecting"}
style={{ width: "calc(100% - 20px)", margin: "0 10px 10px 10px" }}>
{server.status === "connecting"
? t("mcp:serverStatus.retrying")
: t("mcp:serverStatus.retryConnection")}
</VSCodeButton>
</div>
)}
)}
{/* Delete Confirmation Dialog */}
<Dialog open={showDeleteConfirm} onOpenChange={setShowDeleteConfirm}>