Skip to content

Commit b587704

Browse files
Python: Skip MCP tools and prompts whose normalized names collide (#14236)
### Motivation and Context MCP tool and prompt name normalization is many-to-one, so two remote names can produce the same local name. The second registration silently replaced the first. ### Description Track normalized names already registered by the plugin and skip colliding tools or prompts with a warning instead of overwriting. Reloading the same remote item still refreshes it. Adds unit tests. ### Contribution Checklist - [x] The code builds clean without any errors or warnings - [x] The PR follows the SK Contribution Guidelines - [x] All unit tests pass, and I have added new tests where possible --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4cab25f4-d987-42b3-bb34-e8d8a7185c80
1 parent 7d885dd commit b587704

2 files changed

Lines changed: 121 additions & 0 deletions

File tree

‎python/semantic_kernel/connectors/mcp.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ def __init__(
278278
self.sampling_auto_approve = sampling_auto_approve
279279
self._sampling_auto_approved_warning_logged = False
280280
self._mcp_reserved_attribute_names: set[str] | None = None
281+
self._mcp_registered_names: dict[str, tuple[str, str]] = {}
281282
self._current_task: asyncio.Task | None = None
282283
self._stop_event: asyncio.Event | None = None
283284

@@ -538,6 +539,27 @@ def _has_mcp_function_name_conflict(self, item_type: str, remote_name: str, loca
538539
)
539540
return True
540541

542+
def _is_mcp_local_name_taken(self, item_type: str, remote_name: str, local_name: str) -> bool:
543+
"""Check whether a normalized name is already bound to a different MCP tool or prompt.
544+
545+
Normalization is not injective, so distinct remote names can collapse into the same local
546+
name. Tools and prompts share one attribute namespace, so registering a collision would
547+
silently rebind an already advertised name to a different remote item.
548+
"""
549+
owner = self._mcp_registered_names.get(local_name)
550+
if owner is None or owner == (item_type, remote_name):
551+
return False
552+
owner_type, owner_name = owner
553+
logger.warning(
554+
"Skipping MCP %s '%s' because normalized name '%s' is already registered by %s '%s'.",
555+
item_type,
556+
remote_name,
557+
local_name,
558+
owner_type,
559+
owner_name,
560+
)
561+
return True
562+
541563
async def load_prompts(self):
542564
"""Load prompts from the MCP server."""
543565
try:
@@ -546,8 +568,11 @@ async def load_prompts(self):
546568
prompt_list = None
547569
for prompt in prompt_list.prompts if prompt_list else []:
548570
local_name = _normalize_mcp_name(prompt.name)
571+
if self._is_mcp_local_name_taken("prompt", prompt.name, local_name):
572+
continue
549573
if self._has_mcp_function_name_conflict("prompt", prompt.name, local_name):
550574
continue
575+
self._mcp_registered_names[local_name] = ("prompt", prompt.name)
551576
func = kernel_function(name=local_name, description=prompt.description)(
552577
partial(self.get_prompt, prompt.name)
553578
)
@@ -563,8 +588,11 @@ async def load_tools(self):
563588
# Create methods with the kernel_function decorator for each tool
564589
for tool in tool_list.tools if tool_list else []:
565590
local_name = _normalize_mcp_name(tool.name)
591+
if self._is_mcp_local_name_taken("tool", tool.name, local_name):
592+
continue
566593
if self._has_mcp_function_name_conflict("tool", tool.name, local_name):
567594
continue
595+
self._mcp_registered_names[local_name] = ("tool", tool.name)
568596
func = kernel_function(name=local_name, description=tool.description)(partial(self.call_tool, tool.name))
569597
func.__kernel_function_parameters__ = _get_parameter_dicts_from_mcp_tool(tool)
570598
setattr(self, local_name, func)

‎python/tests/unit/connectors/mcp/test_mcp.py‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,99 @@ async def test_mcp_normalization_function(mock_session, list_tool_calls_with_sla
429429
assert _normalize_mcp_name("Name-With.Dots_And-Hyphens") == "Name-With.Dots_And-Hyphens"
430430

431431

432+
async def test_mcp_tool_name_collision_detected(caplog):
433+
"""Test that tools with names that normalize to the same identifier are detected and skipped."""
434+
plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse")
435+
session = AsyncMock(spec=ClientSession)
436+
session.list_tools.return_value = ListToolsResult(
437+
tools=[
438+
Tool(name="read-document", description="first tool", inputSchema={}),
439+
Tool(name="read document", description="second tool", inputSchema={}),
440+
]
441+
)
442+
plugin.session = session
443+
444+
with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"):
445+
await plugin.load_tools()
446+
447+
# Only the first tool should be registered
448+
assert hasattr(plugin, "read-document")
449+
func = getattr(plugin, "read-document")
450+
assert func.__kernel_function_description__ == "first tool"
451+
# Warning should be emitted for the collision
452+
assert "read document" in caplog.text
453+
assert "already registered" in caplog.text
454+
455+
456+
async def test_mcp_prompt_name_collision_detected(caplog):
457+
"""Test that prompts with names that normalize to the same identifier are detected and skipped."""
458+
plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse")
459+
session = AsyncMock(spec=ClientSession)
460+
session.list_tools.return_value = ListToolsResult(tools=[])
461+
session.list_prompts.return_value = types.ListPromptsResult(
462+
prompts=[
463+
types.Prompt(name="get-summary", description="first prompt", arguments=[]),
464+
types.Prompt(name="get summary", description="second prompt", arguments=[]),
465+
]
466+
)
467+
plugin.session = session
468+
469+
with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"):
470+
await plugin.load_prompts()
471+
472+
# Only the first prompt should be registered
473+
assert hasattr(plugin, "get-summary")
474+
func = getattr(plugin, "get-summary")
475+
assert func.__kernel_function_description__ == "first prompt"
476+
# Warning should be emitted for the collision
477+
assert "get summary" in caplog.text
478+
assert "already registered" in caplog.text
479+
480+
481+
async def test_mcp_tool_name_collision_detected_across_reload(caplog):
482+
"""Test that a later tool reload cannot overwrite a previously registered normalized name."""
483+
plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse")
484+
session = AsyncMock(spec=ClientSession)
485+
session.list_tools.side_effect = [
486+
ListToolsResult(tools=[Tool(name="read-document", description="first tool", inputSchema={})]),
487+
ListToolsResult(tools=[Tool(name="read document", description="second tool", inputSchema={})]),
488+
]
489+
plugin.session = session
490+
491+
await plugin.load_tools()
492+
493+
with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"):
494+
await plugin.load_tools()
495+
496+
func = getattr(plugin, "read-document")
497+
assert func.__kernel_function_description__ == "first tool"
498+
assert "read document" in caplog.text
499+
assert "already registered" in caplog.text
500+
501+
502+
async def test_mcp_prompt_does_not_replace_registered_tool_name(caplog):
503+
"""Test that a prompt does not rebind a normalized name already registered by a tool."""
504+
plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse")
505+
session = AsyncMock(spec=ClientSession)
506+
session.list_tools.return_value = ListToolsResult(
507+
tools=[Tool(name="read-document", description="first tool", inputSchema={})]
508+
)
509+
session.list_prompts.return_value = types.ListPromptsResult(
510+
prompts=[types.Prompt(name="read document", description="second item", arguments=[])]
511+
)
512+
plugin.session = session
513+
514+
await plugin.load_tools()
515+
516+
with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"):
517+
await plugin.load_prompts()
518+
519+
func = getattr(plugin, "read-document")
520+
assert func.__kernel_function_description__ == "first tool"
521+
assert "read document" in caplog.text
522+
assert "already registered" in caplog.text
523+
524+
432525
async def test_excluded_function_cannot_be_called(kernel: "Kernel"):
433526
"""Test that excluded functions are rejected at call time, not just hidden from listing."""
434527
from semantic_kernel.connectors.mcp import create_mcp_server_from_kernel

0 commit comments

Comments
 (0)