fix: forward _meta to MCP tool calls and fix model_dump alias seriali…#1918
fix: forward _meta to MCP tool calls and fix model_dump alias seriali…#1918mananpatel320 wants to merge 1 commit intostrands-agents:mainfrom
Conversation
Review SummaryAssessment: Approve This PR correctly addresses both issues described in #1916: forwarding the Review Notes
Nice work on a thorough fix with good test coverage! 🎉 |
| patch_function(mock_wrapped, None, [mock_request], {}) | ||
|
|
||
| # inject should be called with the existing _meta dict (not a new empty one) | ||
| inject_call_args = mock_textmap_instance.inject.call_args[0][0] |
There was a problem hiding this comment.
instead of getting a positional argument here, can we instead confirm the argument name is _meta?
| """Test that OTel instrumentation correctly sets _meta on outgoing tool call requests.""" | ||
| captured_params = [] | ||
|
|
||
| def spy_send_request(wrapped, instance, args, kwargs): |
There was a problem hiding this comment.
We should not need to spy in our integ tests. The echo_server.py mcp server should be updated to return any sent metadata. Then we can assert that the actual running server is working as intended.
| assert result["content"] == [{"text": "SIMPLE_ECHO_TEST"}] | ||
|
|
||
|
|
||
| def test_call_tool_sync_with_meta(): |
There was a problem hiding this comment.
These tests dont actually assert that the mcp server received the metadata. The echo_server.py mcp server should be updated to return any sent metadata. Then we can assert that the actual running server is working as intended.
Description
MCPClient doesn't forward the _meta field to ClientSession.call_tool(), so custom metadata per the MCP
spec never reaches the
server. Separately, the OpenTelemetry instrumentation in mcp_instrumentation.py uses model_dump()
instead of model_dump(by_alias=True), which serializes the Pydantic field as "meta" (Python name)
rather than "_meta" (wire name). This causes setdefault("_meta", {}) to create a new empty dict instead
of reusing the existing one, corrupting the outgoing payload.
This PR adds a meta parameter to call_tool_sync, call_tool_async, and _create_call_tool_coroutine that
gets forwarded to ClientSession.call_tool(meta=...), and fixes the instrumentation to use
model_dump(by_alias=True).
Resolves: #1916
Public API Changes
call_tool_sync and call_tool_async now accept an optional meta keyword argument:
python
Before: no way to pass _meta
result = mcp_client.call_tool_sync(
tool_use_id="id", name="my_tool", arguments={"key": "value"}
)
After: meta forwarded to the MCP server
result = mcp_client.call_tool_sync(
tool_use_id="id", name="my_tool", arguments={"key": "value"},
meta={"com.example/request_id": "abc-123"}
)
The meta parameter defaults to None, so this is fully backward compatible.
Related Issues
#1916
Type of Change
Bug fix
Testing
Updated existing unit test assertions to match the new meta=None kwarg in call_tool calls
Updated mock model_dump signatures to accept by_alias kwarg
Added unit tests for meta forwarding in both sync and async paths
Added unit test verifying instrumentation preserves existing _meta values
Added integration tests for call_tool_sync/call_tool_async with meta
Added integration test verifying instrumentation correctly sets _meta on outgoing requests
I ran hatch run prepare
Checklist
are needed