Prevent duplicate MCP server registration in Google ADK tooling extension

libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py

@@ -91,14 +91,31 @@ async def add_tool_servers_to_agent(
 
         self._logger.info(f"Loaded {len(mcp_server_configs)} MCP server configurations")
 
-        # Convert MCP server configs to McpToolset objects
+        # Collect existing server URLs to prevent duplicates (use set for O(1) lookup)
+        existing_server_urls = set()
+        for tool in agent.tools:
+            # Check if the tool is an McpToolset and has a connection_params.url
+            if hasattr(tool, "connection_params") and hasattr(tool.connection_params, "url"):
+                existing_server_urls.add(tool.connection_params.url)
+
+        self._logger.debug(f"Found {len(existing_server_urls)} existing MCP servers in agent")
+
+        # Convert MCP server configs to McpToolset objects (only new ones)
         mcp_servers_info = []
         mcp_server_headers = {
             Constants.Headers.AUTHORIZATION: f"{Constants.Headers.BEARER_PREFIX} {auth_token}",
             Constants.Headers.USER_AGENT: Utility.get_user_agent_header(self._orchestrator_name),
         }
 
         for server_config in mcp_server_configs:
+            # Skip if server URL already exists
+            if server_config.url in existing_server_urls:
+                self._logger.debug(
+                    f"Skipping MCP server '{server_config.mcp_server_name}' "
+                    f"at {server_config.url} - already exists in agent"
+                )
+                continue
+
             try:
                 server_info = McpToolset(
                     connection_params=StreamableHTTPConnectionParams(
@@ -109,6 +126,7 @@ async def add_tool_servers_to_agent(
 
                 mcp_servers_info.append(server_info)
                 self._connected_servers.append(server_info)
+                existing_server_urls.add(server_config.url)
                 self._logger.info(
                     f"Created MCP toolset for '{server_config.mcp_server_name}' "
                     f"at {server_config.url}"
@@ -119,15 +137,16 @@ async def add_tool_servers_to_agent(
                 self._logger.warning(f"Failed to create MCP toolset for {server_name}: {tool_ex}")
                 continue
 
-        # Combine existing tools with new MCP servers
-        all_tools = list(agent.tools) + mcp_servers_info
-
-        self._logger.info(
-            f"Successfully configured agent with {len(mcp_servers_info)} MCP tool servers "
-            f"(total tools: {len(all_tools)})"
-        )
-
-        agent.tools = all_tools
+        # Only modify agent.tools if we have new servers to add
+        if mcp_servers_info:
+            all_tools = list(agent.tools) + mcp_servers_info
+            agent.tools = all_tools
+            self._logger.info(
+                f"Successfully configured agent with {len(mcp_servers_info)} new MCP tool servers "
+                f"(total tools: {len(all_tools)})"
+            )
+        else:
+            self._logger.info("No new MCP servers to add to agent")
 
     async def cleanup(self):
         """Clean up any resources used by the service."""

tests/tooling/extensions/googleadk/test_mcp_tool_registration_service.py

@@ -345,6 +345,130 @@ async def test_add_tool_servers_handles_toolset_creation_error(
             # Existing tools should still be present
             assert existing_tool in mock_agent.tools
 
+    @pytest.mark.asyncio
+    @pytest.mark.unit
+    async def test_add_tool_servers_skips_duplicate_server_urls(
+        self, mock_agent, mock_authorization, mock_turn_context
+    ):
+        """Test that duplicate server URLs are not added multiple times."""
+        with (
+            patch(
+                "microsoft_agents_a365.tooling.extensions.googleadk.services.mcp_tool_registration_service.McpToolServerConfigurationService"
+            ) as mock_config_service_class,
+            patch(
+                "microsoft_agents_a365.tooling.extensions.googleadk.services.mcp_tool_registration_service.Utility"
+            ) as mock_utility,
+            patch(
+                "microsoft_agents_a365.tooling.extensions.googleadk.services.mcp_tool_registration_service.McpToolset"
+            ) as mock_toolset_class,
+        ):
+            # Setup mocks
+            mock_utility.resolve_agent_identity.return_value = "agent-123"
+            mock_utility.get_user_agent_header.return_value = "Agent365SDK/1.0"
+
+            # Create two server configs with the same URL
+            mock_server_config1 = MagicMock()
+            mock_server_config1.mcp_server_name = "server-1"
+            mock_server_config1.url = "https://test-server.example.com/mcp"
+
+            mock_server_config2 = MagicMock()
+            mock_server_config2.mcp_server_name = "server-2"
+            mock_server_config2.url = "https://test-server.example.com/mcp"  # Same URL
+
+            mock_config_service = AsyncMock()
+            mock_config_service.list_tool_servers = AsyncMock(
+                return_value=[mock_server_config1, mock_server_config2]
+            )
+            mock_config_service_class.return_value = mock_config_service
+
+            # Create mock toolsets
+            mock_toolset1 = MagicMock()
+            mock_toolset1.connection_params = MagicMock()
+            mock_toolset1.connection_params.url = "https://test-server.example.com/mcp"
+
+            mock_toolset_class.return_value = mock_toolset1
+
+            from microsoft_agents_a365.tooling.extensions.googleadk import (
+                McpToolRegistrationService,
+            )
+
+            service = McpToolRegistrationService()
+
+            # Set up agent with no existing tools
+            mock_agent.tools = []
+
+            # Act
+            await service.add_tool_servers_to_agent(
+                agent=mock_agent,
+                auth=mock_authorization,
+                auth_handler_name="graph",
+                context=mock_turn_context,
+                auth_token="test-token",
+            )
+
+            # Assert - toolset should be created only once despite two configs
+            assert mock_toolset_class.call_count == 1
+            assert len(service._connected_servers) == 1
+            assert len(mock_agent.tools) == 1
+
+    @pytest.mark.asyncio
+    @pytest.mark.unit
+    async def test_add_tool_servers_skips_existing_servers_in_agent(
+        self, mock_agent, mock_authorization, mock_turn_context
+    ):
+        """Test that servers already in the agent are not added again."""
+        with (
+            patch(
+                "microsoft_agents_a365.tooling.extensions.googleadk.services.mcp_tool_registration_service.McpToolServerConfigurationService"
+            ) as mock_config_service_class,
+            patch(
+                "microsoft_agents_a365.tooling.extensions.googleadk.services.mcp_tool_registration_service.Utility"
+            ) as mock_utility,
+            patch(
+                "microsoft_agents_a365.tooling.extensions.googleadk.services.mcp_tool_registration_service.McpToolset"
+            ) as mock_toolset_class,
+        ):
+            # Setup mocks
+            mock_utility.resolve_agent_identity.return_value = "agent-123"
+            mock_utility.get_user_agent_header.return_value = "Agent365SDK/1.0"
+
+            # Create server config
+            mock_server_config = MagicMock()
+            mock_server_config.mcp_server_name = "existing-server"
+            mock_server_config.url = "https://existing-server.example.com/mcp"
+
+            mock_config_service = AsyncMock()
+            mock_config_service.list_tool_servers = AsyncMock(return_value=[mock_server_config])
+            mock_config_service_class.return_value = mock_config_service
+
+            from microsoft_agents_a365.tooling.extensions.googleadk import (
+                McpToolRegistrationService,
+            )
+
+            service = McpToolRegistrationService()
+
+            # Set up agent with existing toolset that has the same URL
+            existing_toolset = MagicMock()
+            existing_toolset.connection_params = MagicMock()
+            existing_toolset.connection_params.url = "https://existing-server.example.com/mcp"
+            mock_agent.tools = [existing_toolset]
+
+            # Act
+            await service.add_tool_servers_to_agent(
+                agent=mock_agent,
+                auth=mock_authorization,
+                auth_handler_name="graph",
+                context=mock_turn_context,
+                auth_token="test-token",
+            )
+
+            # Assert - no new toolset should be created
+            mock_toolset_class.assert_not_called()
+            assert len(service._connected_servers) == 0
+            # Agent should still have the one existing tool
+            assert len(mock_agent.tools) == 1
+            assert mock_agent.tools[0] == existing_toolset
+
 
 class TestCleanup:
     """Tests for cleanup method."""