Python: Skip MCP prompt loading when unsupported#5370
Conversation
|
@microsoft-github-policy-service agree |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR fixes MCP client connections to tool-only servers by honoring negotiated server capabilities from MCP initialization, avoiding prompt discovery calls when the server does not support prompts.
Changes:
- Cache MCP
initialize()capabilities onMCPTooland use them to detect prompt support. - Skip
prompts/listdiscovery when the server does not advertise thepromptscapability (preserving existing behavior when capabilities are unavailable). - Add a regression test ensuring prompt discovery is skipped for tool-only servers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_mcp.py | Store negotiated server capabilities and gate prompt loading on prompts capability support. |
| python/packages/core/tests/core/test_mcp.py | Add regression coverage for connecting to a server that supports tools but not prompts. |
b621758 to
629d5a4
Compare
giles17
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 89% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach
Automated review by giles17's agents
|
@karimbaidar looks good. One thing to flag: |
629d5a4 to
b44b49e
Compare
|
Thanks for the review @giles17. I just Removed the constant, added _supports_logging, made missing capabilities unsupported after init, and added reconnect handling for ping-less paginated loads. Could you please take another look? |
|
Hi @giles17 i fixed the pyright checks. If possible, can you please approve this PR ? Thank you! |
|
Hey @karimbaidar thanks! There's a little merge conflict to be resolved |
…360-mcp-prompts # Conflicts: # python/packages/core/agent_framework/_mcp.py
|
@giles17 thanks! just resolved the merge conflict but I think github requires your approval again :) |
|
@eavanvalkenburg can you please also check and approve so that this PR could be merged successfully. Because we need at least two approvers for this PR to be merged successfully. thank you! |
eavanvalkenburg
left a comment
There was a problem hiding this comment.
small nit, otherwise looks good
|
@giles17 @eavanvalkenburg all Set :), waiting for the final Approval and thanks a lot for your review. It helped a lot to improve this PR further. Truly appreciate that. |
Motivation and Context
Fixes #5360.
MCP servers can advertise tools without supporting prompts. The client was still calling
prompts/listwhenever prompt loading was enabled, so tool-only servers could fail during connect.Description
Stores the server capabilities returned by MCP initialization and skips prompt discovery when the server does not advertise the
promptscapability. If capabilities are unavailable, the existing behavior is preserved.Adds a focused core MCP regression test for a tool-only server that would otherwise raise
Method 'prompts/list' is not available.Contribution Checklist
Update
This now generalizes the negotiated MCP capability handling for tools, prompts, and logging while keeping the constructor load flags as caller intent. After initialization, missing or unadvertised capabilities are treated as unsupported, so unsupported tools/list, prompts/list, and set_logging_level calls are skipped.
Ping support is handled separately as session-scoped probed state, since it is not part of ServerCapabilities. Paginated tool/prompt loading also reconnects once on ClosedResourceError for servers where ping is unavailable.