Implement SEP-1577 - Sampling With Tools #1594
Conversation
Add support for tool calling during sampling requests, enabling MCP servers to execute agentic workflows using client LLM capabilities. Key changes: - Add ToolUseContent type for assistant tool invocation requests - Add ToolResultContent type for tool execution results - Add ToolChoice type to control tool usage behavior - Add UserMessage and AssistantMessage types for role-specific messages - Extend SamplingMessage to support tool content (backward compatible) - Add SamplingToolsCapability for capability negotiation - Update CreateMessageRequestParams with tools and toolChoice fields - Update CreateMessageResult to support tool use content - Update StopReason to include "toolUse" value - Add comprehensive unit tests for all new types The implementation maintains backward compatibility by keeping SamplingMessage as a flexible BaseModel while adding more specific UserMessage and AssistantMessage types for type-safe tool interactions. All new types follow existing patterns: - Use Pydantic V2 BaseModel - Allow extra fields with ConfigDict(extra="allow") - Include proper docstrings and field descriptions - Support optional fields where appropriate Github-Issue: #1577
Rename SamplingCapabilityNested to SamplingCapability and the old SamplingCapability to SamplingContextCapability for better clarity. - SamplingCapability is now the main structured type with context and tools fields - SamplingContextCapability represents the deprecated context capability - Updates all exports and tests to use the new names All tests pass and type checking passes.
ochafik
changed the title
- Replace UserMessage/AssistantMessage with unified SamplingMessage - Add SamplingMessageContentBlock type alias for content blocks - Update CreateMessageResult to use SamplingMessageContentBlock - Remove disable_parallel_tool_use from ToolChoice (not in spec) - Add ResourceLink to ToolResultContent.content for consistency This aligns with the draft spec where SamplingMessage uses a single content type union rather than separate UserMessage/AssistantMessage classes. The CreateMessageResult now extends SamplingMessage's pattern and supports both single content blocks and arrays.
The PR changed CreateMessageResult.content to be either a single content block or a list. Code accessing .type and .text directly now needs to handle both cases by checking isinstance(content, list) first. Also add explicit dict[str, Any] type annotations to test data containing nested empty dicts to satisfy pyright's type inference.
|
pushed a couple of minor CI fixes |
bhosmer-ant
left a comment
bhosmer-ant
left a comment
There was a problem hiding this comment.
Couple minor notes inline, also these less minor ones:
- runtime validations of the spec constraints are missing. some claude notes:
⏺ The validations would go in src/mcp/server/session.py:215-248 in the create_message method. This is where the server sends sampling requests
to the client.
Specifically:
- Capability check ("MUST throw when tools/toolChoice defined but sampling.tools missing"):
# Would be added before send_request:
if (tools or tool_choice) and not self._has_sampling_tools_capability():
raise McpError("Client does not support sampling tools capability")
- Content mixing validation ("tool result content blocks MUST NOT contain other content types"):
Could be validated either:
- Here when building the request messages
- Or in a Pydantic validator on SamplingMessage itself
- missing new props in the API. more claude notes:
Note that the create_message method is also missing the new tools and toolChoice parameters entirely — the PR only adds the types but
doesn't wire them into the API method. Let me verify:
⏺ Read(src/mcp/server/session.py)
⎿ Read 50 lines
⏺ This is a gap in the PR: The create_message() method at src/mcp/server/session.py:215 doesn't expose the new tools and toolChoice
parameters. Currently it only passes the old parameters to CreateMessageRequestParams.
To actually use the new feature, developers would have to bypass this helper and construct the request manually.
The PR should probably also update this method to:
async def create_message(
self,
messages: list[types.SamplingMessage],
*,
max_tokens: int,
# ... existing params ...
tools: list[types.Tool] | None = None, # NEW
tool_choice: types.ToolChoice | None = None, # NEW
) -> types.CreateMessageResult:
And add the capability validation there.
README.md
Outdated
| if result.content.type == "text": | ||
| return result.content.text | ||
| return str(result.content) | ||
| content = result.content[0] if isinstance(result.content, list) else result.content |
There was a problem hiding this comment.
I don't think we want this as the example of how to use the new multiple content blocks :)
(same comment on other places where this pattern shows up)
There was a problem hiding this comment.
Added a probably controversial content_as_list property + updated various usage places. WDYT? cc/ @felixweinberger
There was a problem hiding this comment.
Seems fine! The main issue was just dropping array items but a convenience method can't hurt.
src/mcp/types.py
Outdated
| toolUseId: str | ||
| """The unique identifier that corresponds to the tool call's id field.""" | ||
|
|
||
| content: list[Union[TextContent, ImageContent, AudioContent, "ResourceLink", "EmbeddedResource"]] = [] |
There was a problem hiding this comment.
I think we can just use ContentBlock here?
The test_create_message_tool_result_validation test was creating a ServerSession without using it as a context manager, which meant the internal streams were never properly cleaned up, causing intermittent ResourceWarning on CI. - Use `async with ServerSession(...) as session:` to ensure proper cleanup - Update test regex patterns to match actual error messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Thanks @bhosmer-ant !
Fixed
Fixed (+ tested)
Edit: David's suggestion was applied, adding the check |
- Add test cases for valid create_message calls that pass validation (covering branches 261->266, 266->272, 269->272 in session.py) - Add test for CreateMessageResult.content_as_list with single content (covering line 1245 else branch in types.py) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add test cases for valid create_message calls that pass validation (covering branches 261->266, 266->272, 269->272 in session.py) - Add test for CreateMessageResult.content_as_list with single content (covering line 1245 else branch in types.py) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…modelcontextprotocol/python-sdk into ochafik/sep-1577-sampling-tools
…modelcontextprotocol/python-sdk into ochafik/sep-1577-sampling-tools
This test covers lines 267-269 of session.py where McpError is raised when tools or tool_choice are provided but the client lacks the sampling.tools capability.
Implement types from modelcontextprotocol/modelcontextprotocol#1577 (accepted)
Spec PR: modelcontextprotocol/modelcontextprotocol#1796 (merged, and follow up modelcontextprotocol/modelcontextprotocol#1842 pending review)
cc/ @bhosmer-ant
Add support for tool calling during sampling requests, enabling MCP servers to execute agentic workflows using client LLM capabilities.
modelcontextprotocol/modelcontextprotocol#1577
Key changes:
The implementation maintains backward compatibility by keeping SamplingMessage as a flexible BaseModel while adding more specific UserMessage and AssistantMessage types for type-safe tool interactions.
All new types follow existing patterns: