From 9b6a6fb39708c0d320a95907742f239546804110 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Wed, 29 Jul 2026 11:26:50 +0100 Subject: [PATCH 1/2] Python: Skip MCP tools and prompts whose normalized names collide MCP tool and prompt names are normalized to the allowed kernel function identifier pattern. The normalization is many-to-one, so two distinct remote names can produce the same local name. Previously the second registration replaced the first via setattr with no check, leaving a single kernel function whose displayed name no longer matched the remote item it invokes. Track the normalized names already registered by the plugin and skip any tool or prompt that would rebind a name owned by a different remote item, logging a warning instead. Reloading the same remote item still refreshes its function, and tools and prompts share one registry because both bind attributes on the same plugin instance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4cab25f4-d987-42b3-bb34-e8d8a7185c80 --- python/semantic_kernel/connectors/mcp.py | 28 ++++++ python/tests/unit/connectors/mcp/test_mcp.py | 93 ++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/python/semantic_kernel/connectors/mcp.py b/python/semantic_kernel/connectors/mcp.py index 0dd0935188dc..789461b2c805 100644 --- a/python/semantic_kernel/connectors/mcp.py +++ b/python/semantic_kernel/connectors/mcp.py @@ -278,6 +278,7 @@ def __init__( self.sampling_auto_approve = sampling_auto_approve self._sampling_auto_approved_warning_logged = False self._mcp_reserved_attribute_names: set[str] | None = None + self._mcp_registered_names: dict[str, tuple[str, str]] = {} self._current_task: asyncio.Task | None = None self._stop_event: asyncio.Event | None = None @@ -538,6 +539,27 @@ def _has_mcp_function_name_conflict(self, item_type: str, remote_name: str, loca ) return True + def _is_mcp_local_name_taken(self, item_type: str, remote_name: str, local_name: str) -> bool: + """Check whether a normalized name is already bound to a different MCP tool or prompt. + + Normalization is not injective, so distinct remote names can collapse into the same local + name. Tools and prompts share one attribute namespace, so registering a collision would + silently rebind an already advertised name to a different remote item. + """ + owner = self._mcp_registered_names.get(local_name) + if owner is None or owner == (item_type, remote_name): + return False + owner_type, owner_name = owner + logger.warning( + "Skipping MCP %s '%s' because normalized name '%s' is already registered by %s '%s'.", + item_type, + remote_name, + local_name, + owner_type, + owner_name, + ) + return True + async def load_prompts(self): """Load prompts from the MCP server.""" try: @@ -546,8 +568,11 @@ async def load_prompts(self): prompt_list = None for prompt in prompt_list.prompts if prompt_list else []: local_name = _normalize_mcp_name(prompt.name) + if self._is_mcp_local_name_taken("prompt", prompt.name, local_name): + continue if self._has_mcp_function_name_conflict("prompt", prompt.name, local_name): continue + self._mcp_registered_names[local_name] = ("prompt", prompt.name) func = kernel_function(name=local_name, description=prompt.description)( partial(self.get_prompt, prompt.name) ) @@ -563,8 +588,11 @@ async def load_tools(self): # Create methods with the kernel_function decorator for each tool for tool in tool_list.tools if tool_list else []: local_name = _normalize_mcp_name(tool.name) + if self._is_mcp_local_name_taken("tool", tool.name, local_name): + continue if self._has_mcp_function_name_conflict("tool", tool.name, local_name): continue + self._mcp_registered_names[local_name] = ("tool", tool.name) func = kernel_function(name=local_name, description=tool.description)(partial(self.call_tool, tool.name)) func.__kernel_function_parameters__ = _get_parameter_dicts_from_mcp_tool(tool) setattr(self, local_name, func) diff --git a/python/tests/unit/connectors/mcp/test_mcp.py b/python/tests/unit/connectors/mcp/test_mcp.py index 2e2eee7d8403..b4623489de12 100644 --- a/python/tests/unit/connectors/mcp/test_mcp.py +++ b/python/tests/unit/connectors/mcp/test_mcp.py @@ -429,6 +429,99 @@ async def test_mcp_normalization_function(mock_session, list_tool_calls_with_sla assert _normalize_mcp_name("Name-With.Dots_And-Hyphens") == "Name-With.Dots_And-Hyphens" +async def test_mcp_tool_name_collision_detected(caplog): + """Test that tools with names that normalize to the same identifier are detected and skipped.""" + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") + session = AsyncMock(spec=ClientSession) + session.list_tools.return_value = ListToolsResult( + tools=[ + Tool(name="read-document", description="first tool", inputSchema={}), + Tool(name="read document", description="second tool", inputSchema={}), + ] + ) + plugin.session = session + + with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"): + await plugin.load_tools() + + # Only the first tool should be registered + assert hasattr(plugin, "read-document") + func = getattr(plugin, "read-document") + assert func.__kernel_function_description__ == "first tool" + # Warning should be emitted for the collision + assert "read document" in caplog.text + assert "already registered" in caplog.text + + +async def test_mcp_prompt_name_collision_detected(caplog): + """Test that prompts with names that normalize to the same identifier are detected and skipped.""" + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") + session = AsyncMock(spec=ClientSession) + session.list_tools.return_value = ListToolsResult(tools=[]) + session.list_prompts.return_value = types.ListPromptsResult( + prompts=[ + types.Prompt(name="get-summary", description="first prompt", arguments=[]), + types.Prompt(name="get summary", description="second prompt", arguments=[]), + ] + ) + plugin.session = session + + with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"): + await plugin.load_prompts() + + # Only the first prompt should be registered + assert hasattr(plugin, "get-summary") + func = getattr(plugin, "get-summary") + assert func.__kernel_function_description__ == "first prompt" + # Warning should be emitted for the collision + assert "get summary" in caplog.text + assert "already registered" in caplog.text + + +async def test_mcp_tool_name_collision_detected_across_reload(caplog): + """Test that a later tool reload cannot overwrite a previously registered normalized name.""" + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") + session = AsyncMock(spec=ClientSession) + session.list_tools.side_effect = [ + ListToolsResult(tools=[Tool(name="read-document", description="first tool", inputSchema={})]), + ListToolsResult(tools=[Tool(name="read document", description="second tool", inputSchema={})]), + ] + plugin.session = session + + await plugin.load_tools() + + with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"): + await plugin.load_tools() + + func = getattr(plugin, "read-document") + assert func.__kernel_function_description__ == "first tool" + assert "read document" in caplog.text + assert "already registered" in caplog.text + + +async def test_mcp_prompt_does_not_replace_registered_tool_name(caplog): + """Test that a prompt does not rebind a normalized name already registered by a tool.""" + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") + session = AsyncMock(spec=ClientSession) + session.list_tools.return_value = ListToolsResult( + tools=[Tool(name="read-document", description="first tool", inputSchema={})] + ) + session.list_prompts.return_value = types.ListPromptsResult( + prompts=[types.Prompt(name="read document", description="second item", arguments=[])] + ) + plugin.session = session + + await plugin.load_tools() + + with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"): + await plugin.load_prompts() + + func = getattr(plugin, "read-document") + assert func.__kernel_function_description__ == "first tool" + assert "read document" in caplog.text + assert "already registered" in caplog.text + + async def test_excluded_function_cannot_be_called(kernel: "Kernel"): """Test that excluded functions are rejected at call time, not just hidden from listing.""" from semantic_kernel.connectors.mcp import create_mcp_server_from_kernel From cb520a31d3b812ffdabf2fd9d0b96b3fd76d9cc1 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Wed, 29 Jul 2026 12:18:30 +0100 Subject: [PATCH 2/2] Python: Constrain mcp dependency to the 1.x series The mcp requirement had no upper bound, and the Python CI workflows install with `uv sync -U`, which resolves to the newest release rather than the locked one. mcp 2.0.0 reorganizes the package, so `semantic_kernel/connectors/mcp.py` fails to import and every unit test job errors during collection. mcp 2.0 removes `mcp.client.websocket` and `mcp.shared.session`, relocates `McpError` and `RequestContext`, and renames `streamablehttp_client` to `streamable_http_client`. Supporting it requires a separate migration, so pin the supported range to `<2` in the base dependency and the mcp extra to keep CI resolving a compatible 1.x release. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4cab25f4-d987-42b3-bb34-e8d8a7185c80 --- python/pyproject.toml | 4 ++-- python/uv.lock | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/pyproject.toml b/python/pyproject.toml index 5513be4cb45a..48950434ca3f 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -56,7 +56,7 @@ dependencies = [ # Protobuf # explicit typing extensions "typing-extensions>=4.13", - "mcp>=1.26.0", + "mcp>=1.26.0,<2", ] ### Optional dependencies @@ -96,7 +96,7 @@ hugging_face = [ "torch==2.13.0" ] mcp = [ - "mcp>=1.8", + "mcp>=1.8,<2", ] milvus = [ "pymilvus >= 2.3,< 2.7", diff --git a/python/uv.lock b/python/uv.lock index f3649edd472a..27863f4b48d2 100644 --- a/python/uv.lock +++ b/python/uv.lock @@ -6800,8 +6800,8 @@ requires-dist = [ { name = "google-genai", marker = "extra == 'google'", specifier = ">=1.51,<1.75" }, { name = "ipykernel", marker = "extra == 'notebooks'", specifier = ">=6.29,<8.0" }, { name = "jinja2", specifier = "~=3.1" }, - { name = "mcp", specifier = ">=1.26.0" }, - { name = "mcp", marker = "extra == 'mcp'", specifier = ">=1.8" }, + { name = "mcp", specifier = ">=1.26.0,<2" }, + { name = "mcp", marker = "extra == 'mcp'", specifier = ">=1.8,<2" }, { name = "microsoft-agents-activity", marker = "extra == 'copilotstudio'", specifier = ">=0.3.1" }, { name = "microsoft-agents-copilotstudio-client", marker = "extra == 'copilotstudio'", specifier = ">=0.3.1" }, { name = "milvus", marker = "sys_platform != 'win32' and extra == 'milvus'", specifier = ">=2.3,<2.3.8" },