fix(mcp): fix the config conflict between microagent MCP tools & MCP config (#8620)

Co-authored-by: openhands <openhands@all-hands.dev>

openhands/core/config/mcp_config.py

@@ -2,6 +2,7 @@ import os
 from urllib.parse import urlparse
 
 from pydantic import BaseModel, Field, ValidationError, model_validator
+
 from openhands.utils.import_utils import get_impl
 
 
@@ -32,6 +33,22 @@ class MCPStdioServerConfig(BaseModel):
     args: list[str] = Field(default_factory=list)
     env: dict[str, str] = Field(default_factory=dict)
 
+    def __eq__(self, other):
+        """Override equality operator to compare server configurations.
+
+        Two server configurations are considered equal if they have the same
+        name, command, args, and env values. The order of args is important,
+        but the order of env variables is not.
+        """
+        if not isinstance(other, MCPStdioServerConfig):
+            return False
+        return (
+            self.name == other.name
+            and self.command == other.command
+            and self.args == other.args
+            and set(self.env.items()) == set(other.env.items())
+        )
+
 
 class MCPConfig(BaseModel):
     """Configuration for MCP (Message Control Protocol) settings.
@@ -124,10 +141,11 @@ class MCPConfig(BaseModel):
         return mcp_mapping
 
 
-
 class OpenHandsMCPConfig:
     @staticmethod
-    def create_default_mcp_server_config(host: str, user_id: str | None = None) -> MCPSSEServerConfig | None:
+    def create_default_mcp_server_config(
+        host: str, user_id: str | None = None
+    ) -> MCPSSEServerConfig | None:
         """
         Create a default MCP server configuration.
 
@@ -141,12 +159,9 @@ class OpenHandsMCPConfig:
         return MCPSSEServerConfig(url=f'http://{host}/mcp/sse', api_key=None)
 
 
-
 openhands_mcp_config_cls = os.environ.get(
     'OPENHANDS_MCP_CONFIG_CLS',
     'openhands.core.config.mcp_config.OpenHandsMCPConfig',
 )
 
-OpenHandsMCPConfigImpl = get_impl(
-    OpenHandsMCPConfig, openhands_mcp_config_cls
-)
+OpenHandsMCPConfigImpl = get_impl(OpenHandsMCPConfig, openhands_mcp_config_cls)

openhands/runtime/impl/action_execution/action_execution_client.py

@@ -10,7 +10,11 @@ import httpx
 from tenacity import retry, retry_if_exception, stop_after_attempt, wait_exponential
 
 from openhands.core.config import AppConfig
-from openhands.core.config.mcp_config import MCPConfig, MCPStdioServerConfig, MCPSSEServerConfig
+from openhands.core.config.mcp_config import (
+    MCPConfig,
+    MCPSSEServerConfig,
+    MCPStdioServerConfig,
+)
 from openhands.core.exceptions import (
     AgentRuntimeTimeoutError,
 )
@@ -77,6 +81,7 @@ class ActionExecutionClient(Runtime):
         self._runtime_initialized: bool = False
         self._runtime_closed: bool = False
         self._vscode_token: str | None = None  # initial dummy value
+        self._last_updated_mcp_stdio_servers: list[MCPStdioServerConfig] = []
         super().__init__(
             config,
             event_stream,
@@ -357,43 +362,77 @@ class ActionExecutionClient(Runtime):
         # Add the runtime as another MCP server
         updated_mcp_config = self.config.mcp.model_copy()
 
-        # Send a request to the action execution server to updated MCP config
-        stdio_tools = [
-            server.model_dump(mode='json')
-            for server in updated_mcp_config.stdio_servers
-        ]
+        # Get current stdio servers
+        current_stdio_servers: list[MCPStdioServerConfig] = list(
+            updated_mcp_config.stdio_servers
+        )
         if extra_stdio_servers:
-            stdio_tools.extend(
-                [server.model_dump(mode='json') for server in extra_stdio_servers]
-            )
+            current_stdio_servers.extend(extra_stdio_servers)
 
-        if len(stdio_tools) > 0:
-            self.log('debug', f'Updating MCP server to: {stdio_tools}')
+        # Check if there are any new servers using the __eq__ operator
+        new_servers = [
+            server
+            for server in current_stdio_servers
+            if server not in self._last_updated_mcp_stdio_servers
+        ]
+
+        self.log(
+            'debug',
+            f'adding {len(new_servers)} new stdio servers to MCP config: {new_servers}',
+        )
+
+        # Only send update request if there are new servers
+        if new_servers:
+            # Use a union of current servers and last updated servers for the update
+            # This ensures we don't lose any servers that might be missing from either list
+            combined_servers = current_stdio_servers.copy()
+            for server in self._last_updated_mcp_stdio_servers:
+                if server not in combined_servers:
+                    combined_servers.append(server)
+
+            stdio_tools = [
+                server.model_dump(mode='json') for server in combined_servers
+            ]
+            stdio_tools.sort(key=lambda x: x.get('name', ''))  # Sort by server name
+
+            self.log(
+                'debug',
+                f'Updating MCP server with {len(new_servers)} new stdio servers (total: {len(combined_servers)})',
+            )
             response = self._send_action_server_request(
                 'POST',
                 f'{self.action_execution_server_url}/update_mcp_server',
                 json=stdio_tools,
                 timeout=10,
             )
+
             if response.status_code != 200:
                 self.log('warning', f'Failed to update MCP server: {response.text}')
-
-            # No API key by default. Child runtime can override this when appropriate
-            updated_mcp_config.sse_servers.append(
-                MCPSSEServerConfig(
-                    url=self.action_execution_server_url.rstrip('/') + '/sse',
-                    api_key=None,
+            else:
+                # Update our cached list with combined servers after successful update
+                self._last_updated_mcp_stdio_servers = combined_servers.copy()
+                self.log(
+                    'debug',
+                    f'Successfully updated MCP stdio servers, now tracking {len(combined_servers)} servers',
                 )
-            )
             self.log(
                 'info',
                 f'Updated MCP config: {updated_mcp_config.sse_servers}',
             )
         else:
-            self.log(
-                'debug',
-                'MCP servers inside runtime is not updated since no stdio servers are provided',
+            self.log('debug', 'No new stdio servers to update')
+
+
+        if len(self._last_updated_mcp_stdio_servers) > 0:
+            # We should always include the runtime as an MCP server whenever there's > 0 stdio servers
+            updated_mcp_config.sse_servers.append(
+                MCPSSEServerConfig(
+                    url=self.action_execution_server_url.rstrip('/') + '/sse',
+                    # No API key by default. Child runtime can override this when appropriate
+                    api_key=None,
+                )
             )
+
         return updated_mcp_config
 
     async def call_tool_mcp(self, action: MCPAction) -> Observation:

tests/runtime/test_mcp_action.py

@@ -264,3 +264,86 @@ async def test_both_stdio_and_sse_mcp(
         if runtime:
             runtime.close()
         # SSE Docker container cleanup is handled by the sse_mcp_docker_server fixture
+
+
+@pytest.mark.asyncio
+async def test_microagent_and_one_stdio_mcp_in_config(
+    temp_dir, runtime_cls, run_as_openhands
+):
+    runtime = None
+    try:
+        filesystem_config = MCPStdioServerConfig(
+            name='filesystem',
+            command='npx',
+            args=[
+                '@modelcontextprotocol/server-filesystem',
+                '/',
+            ],
+        )
+        override_mcp_config = MCPConfig(stdio_servers=[filesystem_config])
+        runtime, config = _load_runtime(
+            temp_dir,
+            runtime_cls,
+            run_as_openhands,
+            override_mcp_config=override_mcp_config,
+        )
+
+        # NOTE: this simulate the case where the microagent adds a new stdio server to the runtime
+        # but that stdio server is not in the initial config
+        # Actual invocation of the microagent involves `add_mcp_tools_to_agent`
+        # which will call `get_updated_mcp_config` with the stdio server from microagent's config
+        fetch_config = MCPStdioServerConfig(
+            name='fetch', command='uvx', args=['mcp-server-fetch']
+        )
+        updated_config = runtime.get_updated_mcp_config([fetch_config])
+        logger.info(f'updated_config: {updated_config}')
+
+        # ======= Test the stdio server in the config =======
+        mcp_action_sse = MCPAction(name='list_directory', arguments={'path': '/'})
+        obs_sse = await runtime.call_tool_mcp(mcp_action_sse)
+        logger.info(obs_sse, extra={'msg_type': 'OBSERVATION'})
+        assert isinstance(obs_sse, MCPObservation), (
+            'The observation should be a MCPObservation.'
+        )
+        assert '[FILE] .dockerenv' in obs_sse.content
+
+        # ======= Test the stdio server added by the microagent =======
+        # Test browser server
+        action_cmd_http = CmdRunAction(
+            command='python3 -m http.server 8000 > server.log 2>&1 &'
+        )
+        logger.info(action_cmd_http, extra={'msg_type': 'ACTION'})
+        obs_http = runtime.run_action(action_cmd_http)
+        logger.info(obs_http, extra={'msg_type': 'OBSERVATION'})
+
+        assert isinstance(obs_http, CmdOutputObservation)
+        assert obs_http.exit_code == 0
+        assert '[1]' in obs_http.content
+
+        action_cmd_cat = CmdRunAction(command='sleep 3 && cat server.log')
+        logger.info(action_cmd_cat, extra={'msg_type': 'ACTION'})
+        obs_cat = runtime.run_action(action_cmd_cat)
+        logger.info(obs_cat, extra={'msg_type': 'OBSERVATION'})
+        assert obs_cat.exit_code == 0
+
+        mcp_action_fetch = MCPAction(
+            name='fetch', arguments={'url': 'http://localhost:8000'}
+        )
+        obs_fetch = await runtime.call_tool_mcp(mcp_action_fetch)
+        logger.info(obs_fetch, extra={'msg_type': 'OBSERVATION'})
+        assert isinstance(obs_fetch, MCPObservation), (
+            'The observation should be a MCPObservation.'
+        )
+
+        result_json = json.loads(obs_fetch.content)
+        assert not result_json['isError']
+        assert len(result_json['content']) == 1
+        assert result_json['content'][0]['type'] == 'text'
+        assert (
+            result_json['content'][0]['text']
+            == 'Contents of http://localhost:8000/:\n---\n\n* <server.log>\n\n---'
+        )
+    finally:
+        if runtime:
+            runtime.close()
+        # SSE Docker container cleanup is handled by the sse_mcp_docker_server fixture

tests/unit/test_mcp_config.py

@@ -192,3 +192,57 @@ def test_mcp_config_extra_fields_forbidden():
 
     # Note: The nested models don't have 'extra': 'forbid' set, so they allow extra fields
     # We're only testing the main MCPConfig class here
+
+
+def test_stdio_server_equality_with_different_env_order():
+    """Test that MCPStdioServerConfig equality works with env in different order but respects arg order."""
+    # Test 1: Same args, different env order
+    server1 = MCPStdioServerConfig(
+        name='test-server',
+        command='python',
+        args=['--verbose', '--debug', '--port=8080'],
+        env={'DEBUG': 'true', 'PORT': '8080'},
+    )
+
+    server2 = MCPStdioServerConfig(
+        name='test-server',
+        command='python',
+        args=['--verbose', '--debug', '--port=8080'],  # Same order
+        env={'PORT': '8080', 'DEBUG': 'true'},  # Different order
+    )
+
+    # Should be equal because env is compared as a set
+    assert server1 == server2
+
+    # Test 2: Different args order
+    server3 = MCPStdioServerConfig(
+        name='test-server',
+        command='python',
+        args=['--debug', '--port=8080', '--verbose'],  # Different order
+        env={'DEBUG': 'true', 'PORT': '8080'},
+    )
+
+    # Should NOT be equal because args order matters
+    assert server1 != server3
+
+    # Test 3: Different arg value
+    server4 = MCPStdioServerConfig(
+        name='test-server',
+        command='python',
+        args=['--verbose', '--debug', '--port=9090'],  # Different port
+        env={'DEBUG': 'true', 'PORT': '8080'},
+    )
+
+    # Should not be equal
+    assert server1 != server4
+
+    # Test 4: Different env value
+    server5 = MCPStdioServerConfig(
+        name='test-server',
+        command='python',
+        args=['--verbose', '--debug', '--port=8080'],
+        env={'DEBUG': 'true', 'PORT': '9090'},  # Different port
+    )
+
+    # Should not be equal
+    assert server1 != server5