From 112703c5d93dda0c060617264e779625c30b1bb9 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 May 2026 15:23:54 +0900 Subject: [PATCH 1/3] Improvements for MCP --- python/semantic_kernel/connectors/mcp.py | 48 ++++++++++++++- python/tests/unit/connectors/mcp/test_mcp.py | 65 ++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/python/semantic_kernel/connectors/mcp.py b/python/semantic_kernel/connectors/mcp.py index 5f31886b28fa..9f462090b181 100644 --- a/python/semantic_kernel/connectors/mcp.py +++ b/python/semantic_kernel/connectors/mcp.py @@ -6,7 +6,7 @@ import re import sys from abc import abstractmethod -from collections.abc import Callable, Sequence +from collections.abc import Awaitable, Callable, Sequence from contextlib import AbstractAsyncContextManager, AsyncExitStack, _AsyncGeneratorContextManager from datetime import timedelta from functools import partial @@ -59,6 +59,8 @@ # region: Helpers +SamplingConsentCallback = Callable[[str, types.CreateMessageRequestParams], Awaitable[bool]] + LOG_LEVEL_MAPPING: dict[types.LoggingLevel, int] = { "debug": logging.DEBUG, "info": logging.INFO, @@ -243,6 +245,7 @@ def __init__( session: ClientSession | None = None, kernel: Kernel | None = None, request_timeout: int | None = None, + sampling_consent_callback: SamplingConsentCallback | None = None, ) -> None: """Initialize the MCP Plugin Base.""" self.name = name @@ -253,6 +256,8 @@ def __init__( self.session = session self.kernel = kernel or None self.request_timeout = request_timeout + self.sampling_consent_callback = sampling_consent_callback + self._sampling_auto_approved_warning_logged = False self._current_task: asyncio.Task | None = None self._stop_event: asyncio.Event | None = None @@ -364,6 +369,20 @@ async def sampling_callback( This is a simple version of this function, it can be overridden to allow more complex sampling. It get's added to the session at initialization time, so overriding it is the best way to do this. """ + if self.sampling_consent_callback is None: + if not self._sampling_auto_approved_warning_logged: + logger.warning( + "MCP sampling request for plugin '%s' was auto-approved because no sampling consent callback " + "was configured.", + self.name, + ) + self._sampling_auto_approved_warning_logged = True + elif not await self.sampling_consent_callback(self.name, params): + return types.ErrorData( + code=types.INTERNAL_ERROR, + message="Sampling denied by policy.", + ) + if not self.kernel or not self.kernel.services: return types.ErrorData( code=types.INTERNAL_ERROR, @@ -464,6 +483,17 @@ async def message_handler( case "notifications/prompts/list_changed": await self.load_prompts() + def _has_mcp_function_name_conflict(self, item_type: str, remote_name: str, local_name: str) -> bool: + if not hasattr(self, local_name): + return False + logger.warning( + "Skipping MCP %s '%s' because normalized name '%s' conflicts with an existing plugin attribute.", + item_type, + remote_name, + local_name, + ) + return True + async def load_prompts(self): """Load prompts from the MCP server.""" try: @@ -472,6 +502,8 @@ 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._has_mcp_function_name_conflict("prompt", prompt.name, local_name): + continue func = kernel_function(name=local_name, description=prompt.description)( partial(self.get_prompt, prompt.name) ) @@ -487,6 +519,8 @@ 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._has_mcp_function_name_conflict("tool", tool.name, local_name): + continue 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) @@ -558,6 +592,7 @@ def __init__( env: dict[str, str] | None = None, encoding: str | None = None, kernel: Kernel | None = None, + sampling_consent_callback: SamplingConsentCallback | None = None, **kwargs: Any, ) -> None: """Initialize the MCP stdio plugin. @@ -579,6 +614,7 @@ def __init__( env: The environment variables to set for the command. encoding: The encoding to use for the command output. kernel: The kernel instance with one or more Chat Completion clients. + sampling_consent_callback: Callback for approving MCP sampling requests. kwargs: Any extra arguments to pass to the stdio client. """ @@ -590,6 +626,7 @@ def __init__( load_tools=load_tools, load_prompts=load_prompts, request_timeout=request_timeout, + sampling_consent_callback=sampling_consent_callback, ) self.command = command self.args = args or [] @@ -628,6 +665,7 @@ def __init__( timeout: float | None = None, sse_read_timeout: float | None = None, kernel: Kernel | None = None, + sampling_consent_callback: SamplingConsentCallback | None = None, **kwargs: Any, ) -> None: """Initialize the MCP sse plugin. @@ -650,6 +688,7 @@ def __init__( timeout: The timeout for the request. sse_read_timeout: The timeout for reading from the SSE stream. kernel: The kernel instance with one or more Chat Completion clients. + sampling_consent_callback: Callback for approving MCP sampling requests. kwargs: Any extra arguments to pass to the sse client. """ @@ -661,6 +700,7 @@ def __init__( load_tools=load_tools, load_prompts=load_prompts, request_timeout=request_timeout, + sampling_consent_callback=sampling_consent_callback, ) self.url = url self.headers = headers or {} @@ -702,6 +742,7 @@ def __init__( sse_read_timeout: float | None = None, terminate_on_close: bool | None = None, kernel: Kernel | None = None, + sampling_consent_callback: SamplingConsentCallback | None = None, **kwargs: Any, ) -> None: """Initialize the MCP streamable http plugin. @@ -725,6 +766,7 @@ def __init__( sse_read_timeout: The timeout for reading from the SSE stream. terminate_on_close: Close the transport when the MCP client is terminated. kernel: The kernel instance with one or more Chat Completion clients. + sampling_consent_callback: Callback for approving MCP sampling requests. kwargs: Any extra arguments to pass to the sse client. """ super().__init__( @@ -735,6 +777,7 @@ def __init__( load_tools=load_tools, load_prompts=load_prompts, request_timeout=request_timeout, + sampling_consent_callback=sampling_consent_callback, ) self.url = url self.headers = headers or {} @@ -775,6 +818,7 @@ def __init__( session: ClientSession | None = None, description: str | None = None, kernel: Kernel | None = None, + sampling_consent_callback: SamplingConsentCallback | None = None, **kwargs: Any, ) -> None: """Initialize the MCP websocket plugin. @@ -794,6 +838,7 @@ def __init__( session: The session to use for the MCP connection. description: The description of the plugin. kernel: The kernel instance with one or more Chat Completion clients. + sampling_consent_callback: Callback for approving MCP sampling requests. kwargs: Any extra arguments to pass to the websocket client. """ @@ -805,6 +850,7 @@ def __init__( load_tools=load_tools, load_prompts=load_prompts, request_timeout=request_timeout, + sampling_consent_callback=sampling_consent_callback, ) self.url = url self._client_kwargs = kwargs diff --git a/python/tests/unit/connectors/mcp/test_mcp.py b/python/tests/unit/connectors/mcp/test_mcp.py index 55ca71313574..db55a47372a8 100644 --- a/python/tests/unit/connectors/mcp/test_mcp.py +++ b/python/tests/unit/connectors/mcp/test_mcp.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import logging import re from typing import TYPE_CHECKING from unittest.mock import AsyncMock, MagicMock, patch @@ -89,6 +90,70 @@ async def test_mcp_plugin_session_initialized(plugin_class, plugin_args): assert not mock_session.initialize.called +async def test_mcp_sampling_denied_by_consent_callback(): + sampling_consent_callback = AsyncMock(return_value=False) + plugin = MCPSsePlugin( + name="TestMCPPlugin", + url="http://localhost:8080/sse", + sampling_consent_callback=sampling_consent_callback, + ) + params = types.CreateMessageRequestParams( + messages=[types.SamplingMessage(role="user", content=types.TextContent(type="text", text="hello"))], + systemPrompt="server instructions", + maxTokens=100, + ) + + result = await plugin.sampling_callback(MagicMock(), params) + + sampling_consent_callback.assert_awaited_once_with("TestMCPPlugin", params) + assert isinstance(result, types.ErrorData) + assert result.message == "Sampling denied by policy." + + +async def test_mcp_sampling_without_consent_callback_logs_auto_approve_warning(caplog): + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") + params = types.CreateMessageRequestParams( + messages=[types.SamplingMessage(role="user", content=types.TextContent(type="text", text="hello"))], + systemPrompt="server instructions", + maxTokens=100, + ) + + with caplog.at_level(logging.WARNING, logger="semantic_kernel.connectors.mcp"): + result = await plugin.sampling_callback(MagicMock(), params) + + assert isinstance(result, types.ErrorData) + assert "auto-approved because no sampling consent callback was configured" in caplog.text + + +async def test_mcp_tool_and_prompt_names_do_not_shadow_plugin_attributes(): + kernel = MagicMock() + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse", kernel=kernel) + session = AsyncMock(spec=ClientSession) + session.list_tools.return_value = ListToolsResult( + tools=[ + Tool(name="kernel", description="reserved", inputSchema={}), + Tool(name="safe_tool", description="safe", inputSchema={}), + ] + ) + session.list_prompts.return_value = types.ListPromptsResult( + prompts=[ + types.Prompt(name="session", description="reserved", arguments=[]), + types.Prompt(name="safe_prompt", description="safe", arguments=[]), + ] + ) + plugin.session = session + + await plugin.load_tools() + + assert plugin.kernel is kernel + assert hasattr(plugin, "safe_tool") + + await plugin.load_prompts() + + assert plugin.session is session + assert hasattr(plugin, "safe_prompt") + + async def test_mcp_plugin_failed_get_session(): with ( patch("semantic_kernel.connectors.mcp.stdio_client") as mock_stdio_client, From 792a0b809330347a3a3ef2237952e0fa78d539af Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 May 2026 15:28:24 +0900 Subject: [PATCH 2/3] Document MCP sampling consent callback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/semantic_kernel/connectors/mcp.py | 41 ++++++++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/python/semantic_kernel/connectors/mcp.py b/python/semantic_kernel/connectors/mcp.py index 9f462090b181..8455b26f56b3 100644 --- a/python/semantic_kernel/connectors/mcp.py +++ b/python/semantic_kernel/connectors/mcp.py @@ -247,7 +247,21 @@ def __init__( request_timeout: int | None = None, sampling_consent_callback: SamplingConsentCallback | None = None, ) -> None: - """Initialize the MCP Plugin Base.""" + """Initialize the MCP Plugin Base. + + Args: + name: The name of the plugin. + description: The description of the plugin. + load_tools: Whether to load tools from the MCP server. + load_prompts: Whether to load prompts from the MCP server. + session: The session to use for the MCP connection. + kernel: The kernel instance with one or more Chat Completion clients. + request_timeout: The default timeout used for all requests. + sampling_consent_callback: Optional callback for approving MCP sampling requests. + Receives the plugin name and MCP sampling request params. Return + False to deny the request. When omitted, sampling requests are + auto-approved and a warning is logged. + """ self.name = name self.description = description self.load_tools_flag = load_tools @@ -366,8 +380,9 @@ async def sampling_callback( This function is called when the MCP server needs to get a message completed. - This is a simple version of this function, it can be overridden to allow more complex sampling. - It get's added to the session at initialization time, so overriding it is the best way to do this. + If a sampling consent callback is configured, it is called before forwarding the request to the configured + chat completion service. Returning False denies the request. If no callback is configured, requests are + auto-approved and a warning is logged. """ if self.sampling_consent_callback is None: if not self._sampling_auto_approved_warning_logged: @@ -614,7 +629,10 @@ def __init__( env: The environment variables to set for the command. encoding: The encoding to use for the command output. kernel: The kernel instance with one or more Chat Completion clients. - sampling_consent_callback: Callback for approving MCP sampling requests. + sampling_consent_callback: Optional callback for approving MCP sampling requests. + Receives the plugin name and MCP sampling request params. Return + False to deny the request. When omitted, sampling requests are + auto-approved and a warning is logged. kwargs: Any extra arguments to pass to the stdio client. """ @@ -688,7 +706,10 @@ def __init__( timeout: The timeout for the request. sse_read_timeout: The timeout for reading from the SSE stream. kernel: The kernel instance with one or more Chat Completion clients. - sampling_consent_callback: Callback for approving MCP sampling requests. + sampling_consent_callback: Optional callback for approving MCP sampling requests. + Receives the plugin name and MCP sampling request params. Return + False to deny the request. When omitted, sampling requests are + auto-approved and a warning is logged. kwargs: Any extra arguments to pass to the sse client. """ @@ -766,7 +787,10 @@ def __init__( sse_read_timeout: The timeout for reading from the SSE stream. terminate_on_close: Close the transport when the MCP client is terminated. kernel: The kernel instance with one or more Chat Completion clients. - sampling_consent_callback: Callback for approving MCP sampling requests. + sampling_consent_callback: Optional callback for approving MCP sampling requests. + Receives the plugin name and MCP sampling request params. Return + False to deny the request. When omitted, sampling requests are + auto-approved and a warning is logged. kwargs: Any extra arguments to pass to the sse client. """ super().__init__( @@ -838,7 +862,10 @@ def __init__( session: The session to use for the MCP connection. description: The description of the plugin. kernel: The kernel instance with one or more Chat Completion clients. - sampling_consent_callback: Callback for approving MCP sampling requests. + sampling_consent_callback: Optional callback for approving MCP sampling requests. + Receives the plugin name and MCP sampling request params. Return + False to deny the request. When omitted, sampling requests are + auto-approved and a warning is logged. kwargs: Any extra arguments to pass to the websocket client. """ From 448b887c6a6cd6762ae329129021ff51e7392f07 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 13 May 2026 15:41:47 +0900 Subject: [PATCH 3/3] Address MCP review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/semantic_kernel/connectors/mcp.py | 18 +++++-- python/tests/unit/connectors/mcp/test_mcp.py | 50 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/python/semantic_kernel/connectors/mcp.py b/python/semantic_kernel/connectors/mcp.py index 8455b26f56b3..6d7f8d2e182d 100644 --- a/python/semantic_kernel/connectors/mcp.py +++ b/python/semantic_kernel/connectors/mcp.py @@ -272,6 +272,7 @@ def __init__( self.request_timeout = request_timeout self.sampling_consent_callback = sampling_consent_callback self._sampling_auto_approved_warning_logged = False + self._mcp_reserved_attribute_names: set[str] | None = None self._current_task: asyncio.Task | None = None self._stop_event: asyncio.Event | None = None @@ -392,7 +393,7 @@ async def sampling_callback( self.name, ) self._sampling_auto_approved_warning_logged = True - elif not await self.sampling_consent_callback(self.name, params): + elif not await self._is_sampling_approved(params): return types.ErrorData( code=types.INTERNAL_ERROR, message="Sampling denied by policy.", @@ -465,6 +466,15 @@ async def sampling_callback( model=service.ai_model_id, ) + async def _is_sampling_approved(self, params: types.CreateMessageRequestParams) -> bool: + if self.sampling_consent_callback is None: + return True + try: + return await self.sampling_consent_callback(self.name, params) + except Exception: + logger.exception("MCP sampling consent callback failed for plugin '%s'.", self.name) + return False + async def logging_callback(self, params: types.LoggingMessageNotificationParams) -> None: """Callback function for logging. @@ -499,7 +509,9 @@ async def message_handler( await self.load_prompts() def _has_mcp_function_name_conflict(self, item_type: str, remote_name: str, local_name: str) -> bool: - if not hasattr(self, local_name): + if self._mcp_reserved_attribute_names is None: + self._mcp_reserved_attribute_names = set(dir(self)) + if local_name not in self._mcp_reserved_attribute_names: return False logger.warning( "Skipping MCP %s '%s' because normalized name '%s' conflicts with an existing plugin attribute.", @@ -531,7 +543,7 @@ async def load_tools(self): tool_list = await self.session.list_tools() except Exception: tool_list = None - # Create methods with the kernel_function decorator for each tool + # 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._has_mcp_function_name_conflict("tool", tool.name, local_name): diff --git a/python/tests/unit/connectors/mcp/test_mcp.py b/python/tests/unit/connectors/mcp/test_mcp.py index db55a47372a8..dc8ea38330d3 100644 --- a/python/tests/unit/connectors/mcp/test_mcp.py +++ b/python/tests/unit/connectors/mcp/test_mcp.py @@ -110,6 +110,28 @@ async def test_mcp_sampling_denied_by_consent_callback(): assert result.message == "Sampling denied by policy." +async def test_mcp_sampling_consent_callback_error_denies_request(caplog): + sampling_consent_callback = AsyncMock(side_effect=RuntimeError("policy failure")) + plugin = MCPSsePlugin( + name="TestMCPPlugin", + url="http://localhost:8080/sse", + sampling_consent_callback=sampling_consent_callback, + ) + params = types.CreateMessageRequestParams( + messages=[types.SamplingMessage(role="user", content=types.TextContent(type="text", text="hello"))], + systemPrompt="server instructions", + maxTokens=100, + ) + + with caplog.at_level(logging.ERROR, logger="semantic_kernel.connectors.mcp"): + result = await plugin.sampling_callback(MagicMock(), params) + + sampling_consent_callback.assert_awaited_once_with("TestMCPPlugin", params) + assert isinstance(result, types.ErrorData) + assert result.message == "Sampling denied by policy." + assert "MCP sampling consent callback failed" in caplog.text + + async def test_mcp_sampling_without_consent_callback_logs_auto_approve_warning(caplog): plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") params = types.CreateMessageRequestParams( @@ -154,6 +176,34 @@ async def test_mcp_tool_and_prompt_names_do_not_shadow_plugin_attributes(): assert hasattr(plugin, "safe_prompt") +async def test_mcp_tool_and_prompt_names_can_reload_existing_mcp_functions(): + plugin = MCPSsePlugin(name="TestMCPPlugin", url="http://localhost:8080/sse") + session = AsyncMock(spec=ClientSession) + session.list_tools.side_effect = [ + ListToolsResult(tools=[Tool(name="safe_tool", description="first tool", inputSchema={})]), + ListToolsResult(tools=[Tool(name="safe_tool", description="second tool", inputSchema={})]), + ] + session.list_prompts.side_effect = [ + types.ListPromptsResult(prompts=[types.Prompt(name="safe_prompt", description="first prompt", arguments=[])]), + types.ListPromptsResult(prompts=[types.Prompt(name="safe_prompt", description="second prompt", arguments=[])]), + ] + plugin.session = session + + await plugin.load_tools() + first_tool = plugin.safe_tool + await plugin.load_tools() + + assert plugin.safe_tool is not first_tool + assert plugin.safe_tool.__kernel_function_description__ == "second tool" + + await plugin.load_prompts() + first_prompt = plugin.safe_prompt + await plugin.load_prompts() + + assert plugin.safe_prompt is not first_prompt + assert plugin.safe_prompt.__kernel_function_description__ == "second prompt" + + async def test_mcp_plugin_failed_get_session(): with ( patch("semantic_kernel.connectors.mcp.stdio_client") as mock_stdio_client,