From f453ef0595c5a92c32545b5a279d58b264a07c4e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 19:39:12 +0000 Subject: [PATCH 1/3] Initial plan From d9762d6c7694fb59bf880bbf3450c43b4f0a597d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 19:42:34 +0000 Subject: [PATCH 2/3] Add de-duplication logic to prevent duplicate MCP server registration Co-authored-by: JesuTerraz <96103167+JesuTerraz@users.noreply.github.com> --- .../services/mcp_tool_registration_service.py | 39 ++++-- .../test_mcp_tool_registration_service.py | 124 ++++++++++++++++++ 2 files changed, 153 insertions(+), 10 deletions(-) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py index b9522a9..e64fbe2 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py @@ -91,7 +91,16 @@ 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 + existing_server_urls = [] + 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.append(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}", @@ -99,6 +108,14 @@ async def add_tool_servers_to_agent( } 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.append(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.""" diff --git a/tests/tooling/extensions/googleadk/test_mcp_tool_registration_service.py b/tests/tooling/extensions/googleadk/test_mcp_tool_registration_service.py index b711d01..5f13c5c 100644 --- a/tests/tooling/extensions/googleadk/test_mcp_tool_registration_service.py +++ b/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.""" From 580d86966aa303df44580c941f5694bf9ce7d034 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 19:43:51 +0000 Subject: [PATCH 3/3] Optimize duplicate detection with set for O(1) lookup performance Co-authored-by: JesuTerraz <96103167+JesuTerraz@users.noreply.github.com> --- .../googleadk/services/mcp_tool_registration_service.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py index e64fbe2..97fe39d 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-googleadk/microsoft_agents_a365/tooling/extensions/googleadk/services/mcp_tool_registration_service.py @@ -91,12 +91,12 @@ async def add_tool_servers_to_agent( self._logger.info(f"Loaded {len(mcp_server_configs)} MCP server configurations") - # Collect existing server URLs to prevent duplicates - existing_server_urls = [] + # 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.append(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") @@ -126,7 +126,7 @@ async def add_tool_servers_to_agent( mcp_servers_info.append(server_info) self._connected_servers.append(server_info) - existing_server_urls.append(server_config.url) + 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}"