Python: Support structuredContent in MCP tool results and fix sampling options type (#4763)

* Support MCP sampling tools capability (#4625)

Forward systemPrompt, tools, and toolChoice from MCP sampling requests
to the chat client's get_response() call. Also advertise the
sampling.tools capability to MCP servers when a client is configured.

- Pass SamplingCapability with tools support to ClientSession
- Convert systemPrompt to instructions in options
- Convert MCP Tool objects to FunctionTool instances for options
- Map MCP ToolChoice.mode to tool_choice in options
- Add tests for all new behaviors and update existing sampling tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix #4625: Support MCP sampling tool with proper typing and structured content

- Fix mypy error by typing sampling callback options as ChatOptions[None]
  instead of dict[str, Any], and importing ChatOptions from _types
- Handle structuredContent from CallToolResult in _parse_tool_result_from_mcp,
  serializing it as JSON text Content when present
- Add tests for structuredContent parsing (with and without regular content)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix lint: add author to TODO comment

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4625: remove default=str, add edge-case tests

- Remove default=str from json.dumps for structuredContent to fail fast
  on non-JSON-serializable values instead of silently converting
- Add test for non-JSON-serializable structuredContent (TypeError)
- Add tests for empty systemPrompt ('') and empty tools list ([]) edge
  cases in sampling callback
- Expand TODO comment noting list[Content] return type constraint for
  future result_type support

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Sanitize sampling callback error to avoid leaking internals (#4625)

Log exception details at DEBUG level instead of including them in the
ErrorData message returned to the MCP server, which may be untrusted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4625: move params to options, restore error info

- Remove stale TODO comment about response_format (ChatOptions already has it)
- Restore {ex} in sampling callback error message for useful debugging info
- Set structuredContent as additional_property on Content for structured access
- Move temperature, max_tokens, stop into options dict (not top-level kwargs)
- Only set temperature when provided (not all models support it)
- Add tests for generation params in options and temperature omission

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix MCP sampling callback and structured content error handling (#4625)

- Guard max_tokens like temperature: only set when not None, so options
  can properly evaluate to None when all params are absent
- Wrap json.dumps of structuredContent in try/except to fall back to
  str() for non-serializable values instead of propagating TypeError
- Extract test_connect_sampling_capabilities_with_client into its own
  test function so pytest can discover it independently
- Add test for max_tokens=None omission from options
- Update structured content non-serializable test to expect fallback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #4625: review comment fixes

* Fix MCP and Azure validation regressions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Eduard van Valkenburg
2026-03-26 08:33:19 +01:00
committed by GitHub
Unverified
parent dd3d085539
commit efb14cedb1
5 changed files with 409 additions and 7 deletions
+37 -4
View File
@@ -18,7 +18,11 @@ from typing import TYPE_CHECKING, Any, Literal, TypedDict, cast
from opentelemetry import propagate
from ._tools import FunctionTool
from ._types import Content, Message
from ._types import (
ChatOptions,
Content,
Message,
)
from .exceptions import ToolException, ToolExecutionException
if sys.version_info >= (3, 11):
@@ -640,6 +644,7 @@ class MCPTool:
raise ToolException(error_msg, inner_exception=ex) from ex
try:
try:
from mcp import types
from mcp.client.session import ClientSession as runtime_client_session
except ModuleNotFoundError as ex:
await self._safe_close_exit_stack()
@@ -647,6 +652,12 @@ class MCPTool:
"MCP support requires `mcp`. Please install `mcp`.",
inner_exception=ex,
) from ex
sampling_capabilities = None
if self.client is not None:
sampling_capabilities = types.SamplingCapability(
tools=types.SamplingToolsCapability(),
)
session = await self._exit_stack.enter_async_context(
runtime_client_session(
read_stream=transport[0],
@@ -657,6 +668,7 @@ class MCPTool:
message_handler=self.message_handler,
logging_callback=self.logging_callback,
sampling_callback=self.sampling_callback,
sampling_capabilities=sampling_capabilities,
)
)
except Exception as ex:
@@ -733,14 +745,35 @@ class MCPTool:
messages: list[Message] = []
for msg in params.messages:
messages.append(self._parse_message_from_mcp(msg))
options: ChatOptions[None] = {}
if params.systemPrompt is not None:
options["instructions"] = params.systemPrompt
if params.tools is not None:
options["tools"] = [
FunctionTool(
name=tool.name,
description=tool.description or "",
input_model=tool.inputSchema,
)
for tool in params.tools
]
if params.toolChoice is not None and params.toolChoice.mode is not None:
options["tool_choice"] = params.toolChoice.mode
if params.temperature is not None:
options["temperature"] = params.temperature
options["max_tokens"] = params.maxTokens
if params.stopSequences is not None:
options["stop"] = params.stopSequences
try:
response = await self.client.get_response(
messages,
temperature=params.temperature,
max_tokens=params.maxTokens,
stop=params.stopSequences,
options=options or None,
)
except Exception as ex:
logger.debug("Sampling callback error: %s", ex, exc_info=True)
return types.ErrorData(
code=types.INTERNAL_ERROR,
message=f"Failed to get chat message content: {ex}",
+365 -1
View File
@@ -1696,12 +1696,15 @@ async def test_mcp_tool_sampling_callback_chat_client_exception():
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = None
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.ErrorData)
assert result.code == types.INTERNAL_ERROR
assert "Failed to get chat message content: Chat client error" in result.message
assert "Failed to get chat message content" in result.message
async def test_mcp_tool_sampling_callback_no_valid_content():
@@ -1739,6 +1742,9 @@ async def test_mcp_tool_sampling_callback_no_valid_content():
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = None
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
@@ -1757,6 +1763,9 @@ async def test_mcp_tool_sampling_callback_no_response_and_successful_message_cre
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = None
params.tools = None
params.toolChoice = None
tool.client.get_response.return_value = None
no_response = await tool.sampling_callback(Mock(), params)
@@ -1787,6 +1796,361 @@ async def test_mcp_tool_logging_callback_logs_at_requested_level() -> None:
mock_log.assert_called_once_with(logging.WARNING, "be careful")
async def test_mcp_tool_sampling_callback_forwards_system_prompt():
"""Test sampling callback passes systemPrompt as instructions in options."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = "You are a helpful assistant"
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert options.get("instructions") == "You are a helpful assistant"
async def test_mcp_tool_sampling_callback_forwards_tools():
"""Test sampling callback converts MCP tools to FunctionTools and passes them in options."""
from agent_framework import FunctionTool, Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
mcp_tool = types.Tool(
name="get_weather",
description="Get weather",
inputSchema={"type": "object", "properties": {"city": {"type": "string"}}},
)
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = None
params.tools = [mcp_tool]
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
tools = options.get("tools")
assert tools is not None
assert len(tools) == 1
assert isinstance(tools[0], FunctionTool)
assert tools[0].name == "get_weather"
assert tools[0].description == "Get weather"
async def test_mcp_tool_sampling_callback_forwards_tool_choice():
"""Test sampling callback passes toolChoice mode in options."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = None
params.tools = None
params.toolChoice = types.ToolChoice(mode="required")
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert options.get("tool_choice") == "required"
async def test_mcp_tool_sampling_callback_forwards_empty_system_prompt():
"""Test sampling callback forwards empty string systemPrompt as instructions."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = ""
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert options.get("instructions") == ""
async def test_mcp_tool_sampling_callback_forwards_empty_tools_list():
"""Test sampling callback forwards empty tools list in options."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = None
params.stopSequences = None
params.systemPrompt = None
params.tools = []
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert options.get("tools") == []
async def test_mcp_tool_sampling_callback_forwards_generation_params_in_options():
"""Test sampling callback passes temperature, max_tokens, and stop in options."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = 0.7
params.maxTokens = 256
params.stopSequences = ["STOP"]
params.systemPrompt = None
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert options.get("temperature") == 0.7
assert options.get("max_tokens") == 256
assert options.get("stop") == ["STOP"]
# These should not be passed as top-level kwargs
assert "temperature" not in call_kwargs.kwargs
assert "max_tokens" not in call_kwargs.kwargs
assert "stop" not in call_kwargs.kwargs
async def test_mcp_tool_sampling_callback_omits_temperature_when_none():
"""Test sampling callback does not set temperature in options when it is None."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = 100
params.stopSequences = None
params.systemPrompt = None
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert "temperature" not in options
assert options.get("max_tokens") == 100
assert "stop" not in options
async def test_mcp_tool_sampling_callback_always_passes_max_tokens():
"""Test sampling callback always sets max_tokens in options since maxTokens is a required int field."""
from agent_framework import Message
tool = MCPStdioTool(name="test_tool", command="python")
mock_chat_client = AsyncMock()
mock_response = Mock()
mock_response.messages = [Message(role="assistant", contents=[Content.from_text("response")])]
mock_response.model_id = "test-model"
mock_chat_client.get_response.return_value = mock_response
tool.client = mock_chat_client
params = Mock()
mock_message = Mock()
mock_message.role = "user"
mock_message.content = Mock()
mock_message.content.text = "Test question"
params.messages = [mock_message]
params.temperature = None
params.maxTokens = 200
params.stopSequences = None
params.systemPrompt = None
params.tools = None
params.toolChoice = None
result = await tool.sampling_callback(Mock(), params)
assert isinstance(result, types.CreateMessageResult)
call_kwargs = mock_chat_client.get_response.call_args
options = call_kwargs.kwargs.get("options") or {}
assert options["max_tokens"] == 200
async def test_connect_sampling_capabilities_with_client():
"""Test connect() passes sampling_capabilities to ClientSession when client is set."""
tool = MCPStdioTool(name="test", command="test-command", load_tools=False, load_prompts=False)
tool.client = Mock()
mock_transport = (Mock(), Mock())
mock_context_manager = Mock()
mock_context_manager.__aenter__ = AsyncMock(return_value=mock_transport)
mock_context_manager.__aexit__ = AsyncMock(return_value=None)
tool.get_mcp_client = Mock(return_value=mock_context_manager)
with patch("mcp.client.session.ClientSession") as mock_session_class:
mock_session = AsyncMock()
mock_session._request_id = 1
session_cm = AsyncMock()
session_cm.__aenter__ = AsyncMock(return_value=mock_session)
session_cm.__aexit__ = AsyncMock(return_value=None)
mock_session_class.return_value = session_cm
await tool.connect()
call_kwargs = mock_session_class.call_args.kwargs
sampling_caps = call_kwargs.get("sampling_capabilities")
assert sampling_caps is not None
assert isinstance(sampling_caps, types.SamplingCapability)
assert sampling_caps.tools is not None
assert isinstance(sampling_caps.tools, types.SamplingToolsCapability)
async def test_connect_no_sampling_capabilities_without_client():
"""Test connect() does not pass sampling_capabilities when no client is set."""
tool = MCPStdioTool(name="test", command="test-command", load_tools=False, load_prompts=False)
# No client set
mock_transport = (Mock(), Mock())
mock_context_manager = Mock()
mock_context_manager.__aenter__ = AsyncMock(return_value=mock_transport)
mock_context_manager.__aexit__ = AsyncMock(return_value=None)
tool.get_mcp_client = Mock(return_value=mock_context_manager)
with patch("mcp.client.session.ClientSession") as mock_session_class:
mock_session = AsyncMock()
mock_session._request_id = 1
session_cm = AsyncMock()
session_cm.__aenter__ = AsyncMock(return_value=mock_session)
session_cm.__aexit__ = AsyncMock(return_value=None)
mock_session_class.return_value = session_cm
await tool.connect()
call_kwargs = mock_session_class.call_args.kwargs
assert call_kwargs.get("sampling_capabilities") is None
# Test error handling in connect() method
@@ -216,7 +216,9 @@ def load_openai_service_settings(
openai_settings["model"] = resolved_model
break
if not openai_settings.get("api_version"):
if api_version is not None:
openai_settings["api_version"] = api_version
else:
resolved_api_version = _get_setting_from_alias(
"AZURE_OPENAI_API_VERSION",
dotenv_values_by_name=dotenv_values_by_name,
@@ -48,6 +48,7 @@ def openai_unit_test_env(monkeypatch, exclude_list, override_env_param_dict): #
"OPENAI_AUDIO_TO_TEXT_MODEL_ID",
"OPENAI_TEXT_TO_AUDIO_MODEL_ID",
"OPENAI_REALTIME_MODEL_ID",
"OPENAI_API_VERSION",
"OPENAI_BASE_URL",
"AZURE_OPENAI_ENDPOINT",
"AZURE_OPENAI_BASE_URL",
@@ -101,6 +102,7 @@ def azure_openai_unit_test_env(monkeypatch, exclude_list, override_env_param_dic
"OPENAI_AUDIO_TO_TEXT_MODEL_ID",
"OPENAI_TEXT_TO_AUDIO_MODEL_ID",
"OPENAI_REALTIME_MODEL_ID",
"OPENAI_API_VERSION",
"OPENAI_BASE_URL",
"AZURE_OPENAI_ENDPOINT",
"AZURE_OPENAI_BASE_URL",
@@ -79,7 +79,8 @@ def test_init_auto_detects_azure_env(azure_openai_unit_test_env: dict[str, str])
@pytest.mark.parametrize("exclude_list", [["AZURE_OPENAI_API_VERSION"]], indirect=True)
def test_init_uses_default_azure_api_version(azure_openai_unit_test_env: dict[str, str]) -> None:
def test_init_uses_default_azure_api_version(monkeypatch, azure_openai_unit_test_env: dict[str, str]) -> None:
monkeypatch.setenv("OPENAI_API_VERSION", "preview")
client = _create_azure_chat_completion_client()
assert client.model == azure_openai_unit_test_env["AZURE_OPENAI_DEPLOYMENT_NAME"]