From 460ce5bc7a7e089dfce9bae2ac25a09415b435a7 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Mon, 9 Jun 2025 16:32:50 +0900 Subject: [PATCH] Normalize function names to allowed tool calling values. Add tests. --- python/semantic_kernel/connectors/mcp.py | 17 ++++-- python/tests/unit/connectors/mcp/test_mcp.py | 61 ++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/python/semantic_kernel/connectors/mcp.py b/python/semantic_kernel/connectors/mcp.py index 2575e487ad5f..1359119aa3f6 100644 --- a/python/semantic_kernel/connectors/mcp.py +++ b/python/semantic_kernel/connectors/mcp.py @@ -3,6 +3,7 @@ import asyncio import json import logging +import re import sys from abc import abstractmethod from collections.abc import Callable, Sequence @@ -177,6 +178,12 @@ def _get_parameter_dicts_from_mcp_tool(tool: types.Tool) -> list[dict[str, Any]] return params +@experimental +def _normalize_mcp_name(name: str) -> str: + """Normalize MCP tool/prompt names to allowed identifier pattern (A-Za-z0-9_.-).""" + return re.sub(r"[^A-Za-z0-9_.-]", "-", name) + + # region: MCP Plugin @@ -366,11 +373,12 @@ async def load_prompts(self): except Exception: prompt_list = None for prompt in prompt_list.prompts if prompt_list else []: - func = kernel_function(name=prompt.name, description=prompt.description)( + local_name = _normalize_mcp_name(prompt.name) + func = kernel_function(name=local_name, description=prompt.description)( partial(self.get_prompt, prompt.name) ) func.__kernel_function_parameters__ = _get_parameter_dict_from_mcp_prompt(prompt) - setattr(self, prompt.name, func) + setattr(self, local_name, func) async def load_tools(self): """Load tools from the MCP server.""" @@ -380,9 +388,10 @@ async def load_tools(self): tool_list = None # Create methods with the kernel_function decorator for each tool for tool in tool_list.tools if tool_list else []: - func = kernel_function(name=tool.name, description=tool.description)(partial(self.call_tool, tool.name)) + local_name = _normalize_mcp_name(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, tool.name, func) + setattr(self, local_name, func) async def close(self) -> None: """Disconnect from the MCP server.""" diff --git a/python/tests/unit/connectors/mcp/test_mcp.py b/python/tests/unit/connectors/mcp/test_mcp.py index 8e58acc5e9a0..55ca71313574 100644 --- a/python/tests/unit/connectors/mcp/test_mcp.py +++ b/python/tests/unit/connectors/mcp/test_mcp.py @@ -1,4 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. + +import re from typing import TYPE_CHECKING from unittest.mock import AsyncMock, MagicMock, patch @@ -12,6 +14,24 @@ from semantic_kernel import Kernel +@pytest.fixture +def list_tool_calls_with_slash() -> ListToolsResult: + return ListToolsResult( + tools=[ + Tool( + name="nasa/get-astronomy-picture", + description="func with slash", + inputSchema={"properties": {}, "required": []}, + ), + Tool( + name="weird\\name with spaces", + description="func with backslash and spaces", + inputSchema={"properties": {}, "required": []}, + ), + ] + ) + + @pytest.fixture def list_tool_calls() -> ListToolsResult: return ListToolsResult( @@ -230,3 +250,44 @@ async def test_kernel_as_mcp_server(kernel: "Kernel", decorated_native_function, assert types.ListToolsRequest in server.request_handlers assert types.CallToolRequest in server.request_handlers assert server.name == "Semantic Kernel MCP Server" + + +@patch("semantic_kernel.connectors.mcp.sse_client") +@patch("semantic_kernel.connectors.mcp.ClientSession") +async def test_mcp_tool_name_normalization(mock_session, mock_client, list_tool_calls_with_slash, kernel: "Kernel"): + """Test that MCP tool names with illegal characters are normalized.""" + mock_read = MagicMock() + mock_write = MagicMock() + mock_generator = MagicMock() + mock_generator.__aenter__.return_value = (mock_read, mock_write) + mock_generator.__aexit__.return_value = (mock_read, mock_write) + mock_client.return_value = mock_generator + mock_session.return_value.__aenter__.return_value.list_tools.return_value = list_tool_calls_with_slash + + async with MCPSsePlugin( + name="TestMCPPlugin", + description="Test MCP Plugin", + url="http://localhost:8080/sse", + ) as plugin: + loaded_plugin = kernel.add_plugin(plugin) + # The normalized names: + assert "nasa-get-astronomy-picture" in loaded_plugin.functions + assert "weird-name-with-spaces" in loaded_plugin.functions + # They should not exist with their original (invalid) names: + assert "nasa/get-astronomy-picture" not in loaded_plugin.functions + assert "weird\\name with spaces" not in loaded_plugin.functions + + normalized_names = list(loaded_plugin.functions.keys()) + for name in normalized_names: + assert re.match(r"^[A-Za-z0-9_.-]+$", name) + + +@patch("semantic_kernel.connectors.mcp.ClientSession") +async def test_mcp_normalization_function(mock_session, list_tool_calls_with_slash): + """Unit test for the normalize_mcp_name function (should exist in codebase).""" + from semantic_kernel.connectors.mcp import _normalize_mcp_name + + assert _normalize_mcp_name("nasa/get-astronomy-picture") == "nasa-get-astronomy-picture" + assert _normalize_mcp_name("weird\\name with spaces") == "weird-name-with-spaces" + assert _normalize_mcp_name("simple_name") == "simple_name" + assert _normalize_mcp_name("Name-With.Dots_And-Hyphens") == "Name-With.Dots_And-Hyphens"