Repository navigation
mcp draft support - #32
Conversation
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds draft MCP protocol support with stateless request handling, extension-aware capability negotiation, discovery and task update RPCs, HTTP/SSE routing changes, and updated OAuth token introspection and metadata handling. ChangesDraft stateless protocol support
OAuth and token introspection
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant streamable_http
participant Dispatcher
participant validate_stateless_http_metadata
participant StatlessJsonRpcMessageProcessor
participant handle_server_discover
Client->>streamable_http: POST /mcp with _meta and headers
streamable_http->>validate_stateless_http_metadata: validate stateless metadata
validate_stateless_http_metadata-->>streamable_http: ok or HttpValidationError
streamable_http->>Dispatcher: dispatch_stateless(data)
Dispatcher->>StatlessJsonRpcMessageProcessor: process(data)
StatlessJsonRpcMessageProcessor->>handle_server_discover: server/discover
handle_server_discover-->>StatlessJsonRpcMessageProcessor: supportedVersions, capabilities, serverInfo
StatlessJsonRpcMessageProcessor-->>Client: JSON-RPC response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Documentation BuildThe docs site built successfully for this pull request. Target GitHub Pages URL: https://Agent-Hellboy.github.io/py-mcp/ The published site updates after merge to |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #32 +/- ##
==========================================
+ Coverage 76.85% 77.47% +0.62%
==========================================
Files 78 86 +8
Lines 5383 6079 +696
==========================================
+ Hits 4137 4710 +573
- Misses 1246 1369 +123 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fedb7debc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if mode == "draft": | ||
| return (LATEST_DRAFT_PROTOCOL_VERSION,) | ||
| if mode == "dual": | ||
| return (LATEST_DRAFT_PROTOCOL_VERSION, *STABLE_PROTOCOL_VERSIONS) |
There was a problem hiding this comment.
When protocol_mode="draft" or "dual", this puts the stateless 2026-07-28 revision into the same ServerSettings.protocol_versions tuple that handle_initialize negotiates against. As a result, an initialize request with params.protocolVersion="2026-07-28" succeeds and creates a stateful session even though REVISION_2026_07_28 is marked stateless=True/supports_initialization=False, so draft-only servers can advertise and operate a protocol flow the revision explicitly forbids. Filter non-initializable revisions from the initialize negotiation or maintain separate enabled-vs-initializable version lists.
Useful? React with 👍 / 👎.
| ) | ||
| result: JSONObject = { | ||
| "supportedVersions": list(ctx.server_settings.protocol_versions), | ||
| "capabilities": negotiate_capabilities(client_capabilities, server_capabilities), |
There was a problem hiding this comment.
Draft server/discover replaces the initialize response for stateless clients, but this path returns the raw negotiated server capabilities without the authorizer.filter_capabilities(...) step used by handle_initialize. In deployments that configure capability hiding/filtering and allow discovery, draft clients will see capability fragments that stable clients have filtered out for the same principal; run the discovered capabilities through the authorizer before returning them.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tests/unit/test_draft_protocol.py (2)
46-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded extension identifier.
Line 69 hardcodes
"io.modelcontextprotocol/tasks"rather than importingMCP_TASKS(already imported constants pattern is used elsewhere in this file forMCP_APPS_UIetc.). Minor consistency nit, same as flagged intest_client_capabilities.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_draft_protocol.py` around lines 46 - 70, The test currently hardcodes the tasks extension string in test_draft_server_discover_is_stateless_and_cacheable, which is inconsistent with the imported constants pattern used elsewhere in this file. Update the assertion to use the existing MCP_TASKS constant instead of the literal value, matching the style already used for MCP_APPS_UI and similar symbols.
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport the draft version constant instead of duplicating the literal. Use
LATEST_DRAFT_PROTOCOL_VERSIONfrompymcp.protocol.revisionshere to avoid test/source drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_draft_protocol.py` at line 13, The DRAFT_VERSION constant in the test file is hardcoded as a literal string instead of importing the single source of truth. Import LATEST_DRAFT_PROTOCOL_VERSION from pymcp.protocol.revisions at the top of the test file and replace the hardcoded DRAFT_VERSION assignment with this imported constant to ensure the test stays in sync with the actual protocol version definition.tests/unit/test_client_capabilities.py (1)
125-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer importing the
MCP_TASKSconstant over the hardcoded literal.The test hardcodes
"io.modelcontextprotocol/tasks"instead of importing theMCP_TASKSconstant used internally byClientCapabilities.supports, which checkshas_extension(self.capabilities, MCP_TASKS)for the "tasks" feature. Using the literal string risks silent test drift if the constant value ever changes.♻️ Suggested fix
-from pymcp.protocol.extensions import ( +from pymcp.protocol.extensions import ( + MCP_TASKS, MCP_APPS_UI, MCP_ENTERPRISE_MANAGED_AUTHORIZATION, MCP_OAUTH_CLIENT_CREDENTIALS, ) ... def test_supports_tasks_when_declared_as_extension(self): - cc = ClientCapabilities({"extensions": {"io.modelcontextprotocol/tasks": {}}}) + cc = ClientCapabilities({"extensions": {MCP_TASKS: {}}}) assert cc.supports("tasks")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_client_capabilities.py` around lines 125 - 127, The test for ClientCapabilities.supports currently hardcodes the tasks extension string, which can drift from the internal MCP_TASKS constant used by supports. Update the test in test_supports_tasks_when_declared_as_extension to import and use MCP_TASKS instead of the literal so it stays aligned with ClientCapabilities.supports and has_extension.pymcp/protocol/revisions.py (1)
156-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSort
__all__per Ruff RUF022.Static analysis flags this list as unsorted.
🧹 Suggested fix
__all__ = [ "DEFAULT_PROTOCOL_MODE", + "Extension", "LATEST_DRAFT_PROTOCOL_VERSION", "LATEST_STABLE_PROTOCOL_VERSION", - "Extension", - "ProtocolMode", - "ProtocolRevision", "STABLE_PROTOCOL_VERSIONS", "enabled_protocol_revisions", "extension_for_feature", "get_protocol_revision", "is_draft_protocol", "is_stateless_protocol", "known_protocol_versions", + "ProtocolMode", + "ProtocolRevision", "protocol_versions_for_mode", ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pymcp/protocol/revisions.py` around lines 156 - 171, The __all__ export list in revisions.py is not sorted, triggering Ruff RUF022. Reorder the names in the __all__ assignment into alphabetical order while keeping the same exported symbols, and make sure the updated list remains the single source of public exports for this module.Source: Linters/SAST tools
pymcp/protocol/metadata.py (1)
36-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication between lenient and strict metadata extraction.
extract_request_metadataandrequire_request_metadataboth re-read the same_metakeys independently. A shared internal helper that parses raw fields, with the strict variant adding validation on top, would reduce the risk of the two falling out of sync if a_metakey is renamed or a new required field is added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pymcp/protocol/metadata.py` around lines 36 - 79, Refactor the duplicated `_meta` parsing in `extract_request_metadata` and `require_request_metadata` by introducing a shared internal helper in `metadata.py` that reads `extract_params`, `_meta`, `PROTOCOL_VERSION_META_KEY`, `CLIENT_INFO_META_KEY`, and `CLIENT_CAPABILITIES_META_KEY` once and returns the parsed values; keep `extract_request_metadata` lenient by mapping invalid fields to defaults/None, and have `require_request_metadata` build on the same helper while enforcing the existing validation rules and raising `RequestMetadataError` for missing or malformed fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pymcp/runtime/handlers/tasks.py`:
- Around line 174-212: The tasks/update handler in runtime/handlers/tasks.py
validates inputResponses but never uses it before returning success, so update
the task flow to apply that value to the pending task. In the update handler,
after task lookup and terminal-state checks, forward inputResponses into the
task manager or task record using the existing task_id, ctx.session_id, and
ctx.session.principal context so the waiting task receives the data and can
resume. Keep the current error handling paths intact, but make sure the success
response is only returned after the input has been persisted or dispatched.
In `@pymcp/transport/subscriptions.py`:
- Around line 44-49: The subscriptions/listen SSE loop currently only sends
heartbeat pings and waits on a new empty Queue every iteration, so it never
streams actual notifications. Update the listen flow in subscriptions.py to
consume from a real event source or shared queue used by the notification
producer, and remove the per-iteration Queue creation/task churn. If the
behavior is intentionally incomplete, clearly mark listen as a placeholder;
otherwise ensure the generator yields requested notifications instead of only
keepalive pings.
---
Nitpick comments:
In `@pymcp/protocol/metadata.py`:
- Around line 36-79: Refactor the duplicated `_meta` parsing in
`extract_request_metadata` and `require_request_metadata` by introducing a
shared internal helper in `metadata.py` that reads `extract_params`, `_meta`,
`PROTOCOL_VERSION_META_KEY`, `CLIENT_INFO_META_KEY`, and
`CLIENT_CAPABILITIES_META_KEY` once and returns the parsed values; keep
`extract_request_metadata` lenient by mapping invalid fields to defaults/None,
and have `require_request_metadata` build on the same helper while enforcing the
existing validation rules and raising `RequestMetadataError` for missing or
malformed fields.
In `@pymcp/protocol/revisions.py`:
- Around line 156-171: The __all__ export list in revisions.py is not sorted,
triggering Ruff RUF022. Reorder the names in the __all__ assignment into
alphabetical order while keeping the same exported symbols, and make sure the
updated list remains the single source of public exports for this module.
In `@tests/unit/test_client_capabilities.py`:
- Around line 125-127: The test for ClientCapabilities.supports currently
hardcodes the tasks extension string, which can drift from the internal
MCP_TASKS constant used by supports. Update the test in
test_supports_tasks_when_declared_as_extension to import and use MCP_TASKS
instead of the literal so it stays aligned with ClientCapabilities.supports and
has_extension.
In `@tests/unit/test_draft_protocol.py`:
- Around line 46-70: The test currently hardcodes the tasks extension string in
test_draft_server_discover_is_stateless_and_cacheable, which is inconsistent
with the imported constants pattern used elsewhere in this file. Update the
assertion to use the existing MCP_TASKS constant instead of the literal value,
matching the style already used for MCP_APPS_UI and similar symbols.
- Line 13: The DRAFT_VERSION constant in the test file is hardcoded as a literal
string instead of importing the single source of truth. Import
LATEST_DRAFT_PROTOCOL_VERSION from pymcp.protocol.revisions at the top of the
test file and replace the hardcoded DRAFT_VERSION assignment with this imported
constant to ensure the test stays in sync with the actual protocol version
definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76eed161-a900-4a9c-ba8d-d012a8126794
📒 Files selected for processing (25)
README.mddocs/runtime-surface.mdpymcp/capabilities/registry.pypymcp/protocol/__init__.pypymcp/protocol/errors.pypymcp/protocol/extensions.pypymcp/protocol/metadata.pypymcp/protocol/payload.pypymcp/protocol/revisions.pypymcp/runtime/dispatch.pypymcp/runtime/handlers/discovery.pypymcp/runtime/handlers/tasks.pypymcp/runtime/processors.pypymcp/runtime/protocol_context.pypymcp/runtime/server.pypymcp/runtime/types.pypymcp/settings.pypymcp/transport/http_validation.pypymcp/transport/stdio.pypymcp/transport/streamable_http.pypymcp/transport/subscriptions.pytests/unit/test_capability_direction.pytests/unit/test_client_capabilities.pytests/unit/test_draft_protocol.pytests/unit/test_tasks_runtime.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pymcp/security/configured.py`:
- Around line 186-196: The authenticate() flow in ConfiguredAuthentication
currently calls requests.post() synchronously inside an async path, which blocks
the event loop during token introspection. Update the introspection request in
configured.py to run off the loop by either wrapping the existing requests.post
call with asyncio.to_thread or switching the introspection logic to an async
HTTP client such as httpx, and keep the existing AuthenticationError handling in
place around the same authenticate()/introspection code path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 855d8682-6159-442e-8743-55d5bf6908b7
📒 Files selected for processing (17)
docs/security.mdpymcp/middleware.pypymcp/protocol/metadata.pypymcp/runtime/dispatch.pypymcp/runtime/handlers/tasks.pypymcp/runtime/processors.pypymcp/runtime/protocol_context.pypymcp/security/__init__.pypymcp/security/configured.pypymcp/security/oauth.pypymcp/tasks/engine.pypymcp/transport/streamable_http.pypymcp/transport/subscriptions.pytests/test_middleware.pytests/unit/test_client_capabilities.pytests/unit/test_draft_protocol.pytests/unit/test_tasks_runtime.py
✅ Files skipped from review due to trivial changes (2)
- docs/security.md
- pymcp/transport/streamable_http.py
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/unit/test_tasks_runtime.py
- tests/unit/test_client_capabilities.py
- pymcp/transport/subscriptions.py
- pymcp/runtime/processors.py
- tests/unit/test_draft_protocol.py
- pymcp/runtime/protocol_context.py
- pymcp/protocol/metadata.py
- pymcp/runtime/dispatch.py
📝 CodeRabbit Chat: Add ASCII flow to runtime surface docs
1 participant
Summary by CodeRabbit
server/discover, stateless request handling, and stateless streaming subscriptions.tasks/updatefor active tasks and task input-response persistence.resource_metadata_url.401error.