Skip to content

Python: Skip MCP tools and prompts whose normalized names collide - #14236

Merged
SergeyMenshykh merged 3 commits into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-fix-mcp-tool-name-collision
Jul 29, 2026
Merged

Python: Skip MCP tools and prompts whose normalized names collide#14236
SergeyMenshykh merged 3 commits into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-fix-mcp-tool-name-collision

Conversation

@SergeyMenshykh

@SergeyMenshykh SergeyMenshykh commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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

  • The code builds clean without any errors or warnings
  • The PR follows the SK Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible

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
Copilot AI review requested due to automatic review settings July 29, 2026 10:27
@SergeyMenshykh
SergeyMenshykh requested a review from a team as a code owner July 29, 2026 10:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Python MCP connector’s tool/prompt registration to avoid silent overwrites when different remote names normalize to the same local kernel function identifier, ensuring stable and correct dispatch to the intended remote MCP item.

Changes:

  • Track already-registered normalized MCP names across loads (tools + prompts) and detect collisions.
  • Skip colliding tools/prompts and emit a warning describing both the existing and conflicting remote items.
  • Add unit tests covering collisions within a single load, across reloads, and between prompts vs tools.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
python/semantic_kernel/connectors/mcp.py Adds persistent cross-load, cross-type collision tracking for normalized MCP names and logs warnings instead of overwriting registrations.
python/tests/unit/connectors/mcp/test_mcp.py Adds unit tests validating collision detection/skip behavior for tools, prompts, reload scenarios, and prompt-vs-tool collisions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 5 | Confidence: 91% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by SergeyMenshykh's agents

@SergeyMenshykh SergeyMenshykh self-assigned this Jul 29, 2026
@SergeyMenshykh SergeyMenshykh moved this to Sprint: In Review in Semantic Kernel Jul 29, 2026
SergeyMenshykh and others added 2 commits July 29, 2026 12:18
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
Resolve conflicts in python/pyproject.toml and python/uv.lock in favor of
upstream. PR microsoft#14222 already constrained mcp to the 1.x series, so the local
constraint added on this branch was redundant and is dropped in favor of the
upstream `<2.0` specifier.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cab25f4-d987-42b3-bb34-e8d8a7185c80
@github-actions

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
connectors
   mcp.py48220557%81, 86, 98, 108–117, 124–125, 128–129, 138–139, 146, 160–167, 175–179, 181–182, 184, 193, 302–308, 343–345, 350–352, 366–367, 370–371, 379–381, 425–426, 428, 430, 432–438, 442–444, 446–456, 460–461, 465–466, 470, 472, 476–477, 481, 489, 504, 519–527, 567–568, 586–587, 609–610, 613–614, 617–622, 626–627, 630–631, 634–640, 724, 726, 803, 805, 807, 809, 888, 890, 892, 894, 896, 964, 1007–1012, 1014–1017, 1022, 1075, 1077, 1080, 1094, 1114–1116, 1131–1135, 1138–1141, 1144–1146, 1150–1154, 1157–1158, 1167–1168, 1170–1172, 1186–1187, 1189–1190, 1192–1194, 1197, 1204–1207, 1215, 1220–1225, 1230, 1232, 1235–1237
TOTAL29020561980% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
4091 23 💤 0 ❌ 0 🔥 1m 46s ⏱️

@SergeyMenshykh
SergeyMenshykh enabled auto-merge July 29, 2026 12:52
@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue Jul 29, 2026
Merged via the queue into microsoft:main with commit b587704 Jul 29, 2026
32 checks passed
@SergeyMenshykh
SergeyMenshykh deleted the sergeymenshykh-fix-mcp-tool-name-collision branch July 29, 2026 16:43
@github-project-automation github-project-automation Bot moved this from Sprint: In Review to Sprint: Done in Semantic Kernel Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done
Status: Sprint: Done

Development

Successfully merging this pull request may close these issues.

4 participants