fix: forward _meta to MCP tool calls and fix model_dump alias seriali…#1918
Merged
Unshure merged 2 commits intoApr 6, 2026
Merged
Conversation
7 tasks
Contributor
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! 🎉 |
Unshure
reviewed
Mar 19, 2026
Unshure
added a commit
to Unshure/sdk-python
that referenced
this pull request
Mar 20, 2026
- Add echo_meta tool to echo_server.py that returns _meta from request context, so integration tests can verify metadata reaches the server - Update integration tests to use echo_meta and assert the server received the metadata, removing the spy-based approach - Update unit test to verify the key name is '_meta' (alias) in the reconstructed params dict instead of checking inject positional args
f46caa8 to
132ea33
Compare
Unshure
added a commit
to mananpatel320/sdk-python
that referenced
this pull request
Mar 31, 2026
- Add echo_meta tool to echo_server.py that returns _meta from request context, so integration tests can verify metadata reaches the server - Update integration tests to use echo_meta and assert the server received the metadata, removing the spy-based approach - Update unit test to verify the key name is '_meta' (alias) in the reconstructed params dict instead of checking inject positional args
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Unshure
added a commit
to mananpatel320/sdk-python
that referenced
this pull request
Mar 31, 2026
- Add echo_meta tool to echo_server.py that returns _meta from request context, so integration tests can verify metadata reaches the server - Update integration tests to use echo_meta and assert the server received the metadata, removing the spy-based approach - Update unit test to verify the key name is '_meta' (alias) in the reconstructed params dict instead of checking inject positional args
132ea33 to
709a924
Compare
Unshure
added a commit
to mananpatel320/sdk-python
that referenced
this pull request
Apr 1, 2026
709a924 to
99f5d8b
Compare
99f5d8b to
3fb6359
Compare
Contributor
|
/strands review |
Contributor
Re-Review SummaryAssessment: Approve ✅ I've reviewed the new commit ( Changes in Latest CommitThe instrumentation code was improved to properly handle # Before (setdefault doesn't handle explicit None)
meta = params_dict.setdefault("_meta", {})
# After (properly handles None values)
meta = params_dict.get("_meta") if params_dict.get("_meta") is not None else {}
params_dict["_meta"] = metaThis is an improvement because Verification Summary
The PR is ready to merge. |
mkmeral
approved these changes
Apr 6, 2026
agent-of-mkmeral
added a commit
to agent-of-mkmeral/sdk-python
that referenced
this pull request
Apr 6, 2026
MCPClient's _create_call_tool_coroutine didn't forward the meta parameter to _call_tool_as_task_and_poll_async when using task-augmented execution. This meant that custom _meta per the MCP spec never reached the server for tools using the task execution path, even though it worked correctly for direct call_tool calls. Changes: - Pass meta from _create_call_tool_coroutine to _call_tool_as_task_and_poll_async - Add meta parameter to _call_tool_as_task_and_poll_async signature - Forward meta to session.experimental.call_tool_as_task() - Add tests for meta forwarding in sync, async, and None meta paths Follow-up from strands-agents#1918 per @mkmeral's review comment.
4 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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