fix: prevent disabled MCP servers from starting processes and show correct status #6084
+1,396
−250
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
dosubot
bot
added
size:L
…rrect status - Backend: Skip connecting to disabled servers in connectToServer() - Backend: Handle enable/disable state changes properly in toggleServerDisabled() - Backend: Only setup file watchers for enabled servers - Frontend: Show grey status indicator for disabled servers - Frontend: Hide error messages and retry buttons for disabled servers - Frontend: Prevent expansion of disabled server rows Fixes #6036
roomote
bot
force-pushed
the
fix/disabled-mcp-servers-status
branch
from
c6b819d to
a2a8bfc
Compare
hannesrudolph
added
the
Issue/PR - Triage
- 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
dosubot
bot
added
size:XL
- Modified webviewMessageHandler to actively disconnect all running MCP servers when the 'Enable MCP Servers' checkbox is unchecked - Added logic to reconnect servers when MCP is re-enabled - This ensures servers are shut down immediately without requiring a plugin reload
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
- Extract duplicate placeholder connection creation logic into createPlaceholderConnection helper - Fix type safety by allowing null client/transport in McpConnection type - Add proper null checks throughout the codebase - Add error handling to server disconnection loop in webviewMessageHandler - Centralize MCP enabled state checking with isMcpEnabled helper - Extract repeated UI condition into isExpandable computed property - Add comprehensive test coverage for edge cases including: - Toggling global MCP enabled state while servers are active - Handling refreshAllConnections when MCP is disabled - Skipping connection restart when MCP is disabled
hannesrudolph
removed
the
Issue/PR - Triage
…ance connection handling
dosubot
bot
added
size:XXL
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Needs Review]
in Roo Code Roadmap
…ection management
| * @param enabled Whether MCP should be enabled or disabled | ||
| * @returns Promise<void> | ||
| */ | ||
| async handleMcpEnabledChange(enabled: boolean): Promise<void> { |
There was a problem hiding this comment.
The handleMcpEnabledChange method cleanly disconnects all servers when MCP is globally disabled and then refreshes connections. However, inline fallback strings (using the || operator) in translation calls should be removed to adhere to i18n best practices.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the issue where disabled MCP servers were still starting processes and showing incorrect status indicators in the UI. It also ensures that when MCP is globally disabled, all servers are immediately disconnected and prevented from starting.
Problem
Solution
When "Enable MCP Servers" is unchecked globally:
All MCP servers are now immediately disconnected and prevented from starting.
When individual MCP server toggle is set to off:
The individual MCP server is properly disabled and shows correct status.
Changes Made
Backend Changes
McpHub.tsisMcpEnabled()helper to centralize MCP enabled state checkingcreatePlaceholderConnection()helper to eliminate duplicate codeconnectToServer()to check both global MCP state and individual server disabled staterefreshAllConnections()to respect global MCP settingtoggleServerDisabled()to properly disconnect/reconnect serverswebviewMessageHandler.tsFrontend Changes (
McpView.tsx)getStatusColor()to always show grey (--vscode-descriptionForeground) for disabled serversisExpandablecomputed property to centralize expansion logicTest Coverage (
McpHub.spec.ts)refreshAllConnections()when MCP is disabledResult
Fixes #6036
Co-authored-by: Hannes Rudolph hrudolph@gmail.com
Important
Fixes MCP server management to prevent disabled servers from starting and updates UI to reflect correct status.
McpHub.ts: AddedisMcpEnabled()andcreatePlaceholderConnection()helpers.connectToServer()andrefreshAllConnections()to respect global MCP state.toggleServerDisabled()to handle server state changes.McpView.tsx: UpdatedgetStatusColor()to show grey for disabled servers.isExpandableproperty for server expansion logic.McpHub.spec.ts.This description was created by
for 7485066. You can customize this summary. It will automatically update as commits are pushed.