-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add meta to call tool #2206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add meta to call tool #2206
Conversation
|
Thanks @aiorga-sherpas -- can you confirm whether this requires On the one hand I believe that the SDK generally ignores extra inputs in which case this does not require raising the pin to 1.19. On the other hand, users who provide this on I am comfortable raising the minimum in |
WalkthroughAdds an optional Changes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/fastmcp/client/client.py (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Comment |
|
If meta argument is used with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/client/client.py(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/client/client.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
src/fastmcp/client/client.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (3)
src/fastmcp/client/client.py (3)
6-6: LGTM!The
inspectimport is necessary for the runtime feature detection implemented below and is correctly positioned.
895-928: LGTM!The
metaparameter is correctly added tocall_tool, properly documented, and correctly forwarded tocall_tool_mcp. The type annotations follow the coding guidelines for Python ≥ 3.10.
861-885: Signature inspection approach verified and working correctly.The
inspect.signatureruntime detection successfully identifies whethermetais supported by the installed MCP version (tested with MCP 1.16.0, which lacks the parameter). The code correctly prevents TypeErrors by conditionally passingmetaonly when supported, and appropriately warns users when they attempt to use it with older MCP versions.Optional refinements for consideration:
Session lifecycle: If you add caching for the signature check (as suggested earlier), reset the cache when a new session is established—this ensures the check adapts if the session object changes.
Documentation: Add a note to the method docstring indicating that the
metaparameter requires MCP ≥ 1.19 and will be silently omitted on older versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aiorga-sherpas following #2283, MCP 1.19 is now the minimum version for FastMCP, so I think a lot of the version checking machinery can be removed. Do you mind simplifying the feature work here to strip it out, and adding a test to confirm the meta is received on the server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Description
Minor changes to add the meta argument to the call_tool method.
modelcontextprotocol/python-sdk#1231 has been merged and will be in the next mcp release. With that and this minor changes we fix #1315
Review Checklist