Bug Report: Settings save incorrectly triggers MCP server refresh notifications

Description

When saving any settings in the Roo Code VS Code extension (via the sprocket icon), the system incorrectly displays multiple "All MCP servers have been refreshed" notifications, even when the changed settings are completely unrelated to MCP functionality. This creates a confusing user experience and triggers unnecessary server refresh operations.

Steps to Reproduce

1. Open Roo Code extension in VS Code
2. Click the sprocket icon to open settings
3. Change any non-MCP related setting (e.g., sound volume, terminal settings, or auto-approve options)
4. Click Save
5. Observe multiple "All MCP servers have been refreshed" notifications appear

Expected Behavior

• MCP server refresh should only occur when the mcpEnabled setting actually changes value
• Saving unrelated settings should not trigger MCP refresh notifications
• The system should compare the previous and new values before triggering refresh operations

Actual Behavior

• Every settings save triggers MCP server refresh, regardless of what was changed
• Multiple notifications appear even for simple setting changes
• Unnecessary refresh operations are performed, potentially disrupting MCP connections

Additional Context

• Version: Latest Roo Code extension
• Environment: VS Code with MCP servers configured
• Error logs: No errors, but unnecessary refresh notifications are displayed

Code Investigation

Through analysis of the codebase, I've identified the root cause:

Current flow:

1. webview-ui/src/components/settings/SettingsView.tsx:322 - Settings save always sends mcpEnabled message
2. src/core/webview/webviewMessageHandler.ts:901-910 - Handler unconditionally calls handleMcpEnabledChange
3. src/services/mcp/McpHub.ts:1752-1803 - handleMcpEnabledChange refreshes all connections without checking if value changed

🔍 Comprehensive Issue Scoping

Root Cause / Implementation Target

The issue occurs because the settings save mechanism sends ALL settings values to the backend on every save, and the mcpEnabled handler doesn't check if the value has actually changed before triggering a full MCP server refresh.

Affected Components

• Primary Files:

  • src/core/webview/webviewMessageHandler.ts (lines 901-910): Message handler that processes mcpEnabled messages
  • src/services/mcp/McpHub.ts (lines 1752-1803): handleMcpEnabledChange method that performs the refresh

• Secondary Impact:

  • webview-ui/src/components/settings/SettingsView.tsx (line 322): Sends the mcpEnabled message on every save
  • All MCP server connections get unnecessarily refreshed

Current Implementation Analysis

The handleSubmit function at line 278 in SettingsView.tsx sends every setting value to the backend, including mcpEnabled at line 322. The backend handler at line 901 in webviewMessageHandler.ts receives this message and unconditionally calls McpHub.handleMcpEnabledChange(), which then refreshes all MCP connections without checking if the value actually changed.

Proposed Implementation

Step 1: Add state comparison in webviewMessageHandler.ts

• File: src/core/webview/webviewMessageHandler.ts
• Changes: Before calling handleMcpEnabledChange, compare the new value with the current state
• Rationale: Only trigger refresh when the value actually changes

case "mcpEnabled":
    const mcpEnabled = message.bool ?? true
    const currentMcpEnabled = getGlobalState("mcpEnabled") ?? true
    
    // Only update and refresh if the value actually changed
    if (currentMcpEnabled !== mcpEnabled) {
        await updateGlobalState("mcpEnabled", mcpEnabled)
        
        // Delegate MCP enable/disable logic to McpHub
        const mcpHubInstance = provider.getMcpHub()
        if (mcpHubInstance) {
            await mcpHubInstance.handleMcpEnabledChange(mcpEnabled)
        }
    } else {
        // Just update the state without triggering refresh
        await updateGlobalState("mcpEnabled", mcpEnabled)
    }
    
    await provider.postStateToWebview()
    break

Code Architecture Considerations

• The fix follows the existing pattern of checking state before performing actions
• Maintains separation of concerns between UI, message handling, and MCP management
• No breaking changes to the API or message format

Testing Requirements

• Unit Tests:
  • Test case 1: Verify no refresh when mcpEnabled value doesn't change
  • Test case 2: Verify refresh occurs when mcpEnabled changes from true to false
  • Test case 3: Verify refresh occurs when mcpEnabled changes from false to true
• Edge Cases:
  • Edge case 1: Handle undefined/null values correctly
  • Edge case 2: Handle rapid successive saves

Performance Impact

• Expected performance change: Improvement - eliminates unnecessary refresh operations
• Benchmarking needed: No
• Optimization opportunities: Reduces unnecessary network/process operations

Security Considerations

• No security implications - this is a UI/UX bug fix
• No changes to authentication or data exposure

Migration Strategy

Not applicable - this is a bug fix with no data migration needed

Rollback Plan

If issues arise, revert the change to webviewMessageHandler.ts to restore original behavior

Dependencies and Breaking Changes

• External dependencies affected: None
• API contract changes: None
• Breaking changes for users: None - this is a backwards-compatible bug fix

Acceptance Criteria

Given the user has the settings panel open
When they change a non-MCP related setting and click Save
Then no MCP refresh notifications should appear
And MCP servers should not be refreshed
But the changed settings should still be saved correctly

Given the user has the settings panel open
When they toggle the "MCP Enabled" setting and click Save
Then MCP refresh notifications should appear
And MCP servers should be properly refreshed based on the new state

Given the user saves settings multiple times without changing mcpEnabled
When each save operation completes
Then no MCP refresh should occur on any of the saves
But all other settings changes should be persisted correctly