Support MCP initialize handshake#848
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds MCP JSON-RPC initialize handling: server protocol/version constants, an _initialize_response builder that negotiates protocolVersion and reports capabilities/serverInfo, routes initialize requests in handle_mcp_request, and adds tests validating responses and defaulting unsupported protocol inputs. ChangesMCP Initialize Support
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dff5881f-55bf-415f-8923-8bf7f5b86936
📒 Files selected for processing (2)
app/mcp.pytests/test_api_mcp.py
SYZBZ
force-pushed
the
syzbz-mcp-initialize-799
branch
from
1e42916 to
adf40d8
Compare
jakerated-r
left a comment
jakerated-r
left a comment
There was a problem hiding this comment.
Current-head review for 1e4291689771308390baccc8f665dafb5fb82505.
Validation I ran locally on this head:
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_initialize_returns_server_capabilities -q-> 1 passeduv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_tools_list_and_call tests/test_api_mcp.py::test_mcp_rejects_malformed_requests_without_500 -q-> 2 passeduv run --extra dev python -m pytest -q-> 791 passeduv run --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> passeduv run --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> passeduv run --extra dev mypy app-> successpython3 scripts/docs_smoke.py-> okgit diff --check origin/main...HEAD-> cleangit merge-tree --write-tree origin/main HEAD-> clean
Blocker: _initialize_response() currently accepts any string in params.protocolVersion and returns it as the negotiated result.protocolVersion. That lets the server claim compatibility with unsupported or empty protocol versions instead of returning a server-supported version.
The MCP 2025-06-18 lifecycle docs say that if the server supports the requested protocol version, it must respond with the same version; otherwise it must respond with another version it supports. Because this server only declares MCP_PROTOCOL_VERSION = "2025-06-18", echoing arbitrary strings makes the initialize handshake advertise versions the server does not actually support.
I confirmed this on the current head with direct /mcp probes:
- Supported request
protocolVersion: "2025-06-18"-> returns"2025-06-18" - Missing
params-> returns"2025-06-18" - Missing
protocolVersion-> returns"2025-06-18" - Non-string
protocolVersion: 123-> returns"2025-06-18" - Unsupported string
protocolVersion: "not-a-version"-> returns"not-a-version" - Empty string
protocolVersion: ""-> returns""
That last pair is the compatibility failure: a client can believe it successfully negotiated an invalid or unsupported protocol, then proceed to tools/list / tools/call under a version contract the server never implemented.
Please validate the requested version against the supported protocol set before building the initialize result. A safe repair would be either:
- return
"2025-06-18"whenever the client sends an unsupported protocol version, matching MCP version negotiation, or - reject malformed/empty protocol versions with a JSON-RPC invalid-params error if you want stricter request validation.
Please also add regression coverage for unsupported strings and empty strings. The current happy-path test does not catch this negotiation edge.
jakerated-r
left a comment
jakerated-r
left a comment
There was a problem hiding this comment.
Re-reviewed current head adf40d89436337d1e3d442d9c8e0389e9bda0bbb after the PR updated while my previous review was posting.
The protocol negotiation blocker I flagged on the earlier snapshot is fixed on this head. _initialize_response() now returns the supported server protocol version unless the client asks for the exact supported MCP_PROTOCOL_VERSION, so unsupported strings no longer get echoed back as negotiated versions.
Current-head probe results:
- Supported request
protocolVersion: "2025-06-18"-> returns"2025-06-18" - Missing
params-> returns"2025-06-18" - Missing
protocolVersion-> returns"2025-06-18" - Non-string
protocolVersion: 123-> returns"2025-06-18" - Unsupported string
protocolVersion: "not-a-version"-> returns"2025-06-18" - Empty string
protocolVersion: ""-> returns"2025-06-18"
Current-head validation:
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_initialize_returns_server_capabilities -q-> 1 passeduv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_tools_list_and_call tests/test_api_mcp.py::test_mcp_rejects_malformed_requests_without_500 -q-> 2 passeduv run --extra dev python -m pytest -q-> 794 passeduv run --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> passeduv run --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> passeduv run --extra dev mypy app-> successpython3 scripts/docs_smoke.py-> okgit diff --check origin/main...HEAD-> cleangit merge-tree --write-tree origin/main HEAD-> clean
The current patch also adds regression coverage for unsupported protocol-version payloads. No remaining blocker from me on this head.
|
Addressed the current-head review in Changes made:
Fresh verification on current head:
|
|
Current-head verification for This is the exact head I validated after the PR updated during my earlier reviews. No remaining blocker from me on this head. What changed since the prior validated head
Direct
Validation on this exact head:
The implementation now returns the supported server protocol for unsupported, malformed, missing, and empty client protocol-version payloads, and the regression tests cover those initialize edge cases. |
xiefuzheng713-alt
left a comment
xiefuzheng713-alt
left a comment
There was a problem hiding this comment.
Reviewed current head fc443ebe0f5cad31f27df1ffd4ac5b36795ba92a as a non-author.
Scope inspected:
app/mcp.py: verifiedinitializeis handled beforetools/list/tools/call, returns the server-supported MCP protocol version, advertisestools.listChanged: false, and includes stableserverInfowithout invoking any tool handler, DB path, ledger path, or mutation behavior.tests/test_api_mcp.py: verified coverage for a normal initialize request and missing, non-string, unsupported, and emptyprotocolVersioninputs defaulting to the server-supported version.- Live repro/source check: production
POST https://mcp.mrwk.online/mcpwithmethod=initializestill returns JSON-RPC-32601 unknown method, whiletools/listsucceeds, so this is a focused compatibility fix for the existing public MCP endpoint.
Validation run on this head:
uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_initialize_returns_server_capabilities tests/test_api_mcp.py::test_mcp_initialize_defaults_unsupported_protocol_versions -q-> 5 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py -q-> 110 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev python -m pytest -q-> 795 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> passed.uv run --python 3.12 --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> 2 files already formatted.uv run --python 3.12 --extra dev mypy app-> success.uv run --python 3.12 --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree0a2e03b5bccf5a5e954ab4970b461821ded3de8c.
Currentness and duplicate check: this PR is mergeable at the reviewed head and hosted Quality/readiness/docs/image plus CodeRabbit checks are success. Before posting, I found one current-head human approval and one #654 claim for PR #848, so this is a second current-head non-author review rather than a duplicate pile-on.
No blocker found. The change is narrow MCP handshake compatibility work and does not touch admin-token APIs, bounty lifecycle, treasury/proposal execution, payout, proof creation, ledger mutation, wallet material, exchange, bridge, cash-out, price behavior, private data, or secrets.
aglichandrap
left a comment
aglichandrap
left a comment
There was a problem hiding this comment.
Review evidence (head SHA: a4208100fa):
- ✅ Scope: Implements MCP
initializehandshake withprotocolVersion,capabilities, andserverInfo. - ✅ Constants:
MCP_PROTOCOL_VERSION = "2025-06-18"andMCP_SERVER_INFOproperly defined. - ✅ Version negotiation: Falls back to server version if client sends mismatched
protocolVersion. - ✅ Files: 2 files, +88/-0 — clean addition.
- ✅ Mergeable: state=clean.
No blockers. Approve.
4 participants
Summary
initializesupport on the public MCP endpoint.serverInfo.unknown methodbeforetools/list/tools/call.Bounty #799
Source report: #798 (comment)
Why
The current production MCP endpoint exposes
tools/list, but a standard MCP lifecycleinitializerequest returns JSON-RPC-32601 unknown method. That can break MCP clients that perform the normal initialize handshake before listing or calling tools.Production repro before this fix:
The official MCP schema describes
initializeas returningprotocolVersion, servercapabilities, andserverInfo: https://modelcontextprotocol.io/specification/2025-06-18/schema#initializeReview Follow-up
CodeRabbit and jakerated-r asked for protocol-version handling and initialize edge-case coverage. Current head returns the server-supported protocol version for missing, non-string, unsupported, or empty requested versions, with regression tests for those cases.
Test Evidence
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_initialize_returns_server_capabilities tests/test_api_mcp.py::test_mcp_initialize_defaults_unsupported_protocol_versions -q-> 5 passeduv run --extra dev python -m pytest -q-> 795 passed, 1 existing Starlette/httpx warninguv run --extra dev ruff format --check .-> 111 files already formatteduv run --extra dev ruff check .-> All checks passeduv run --extra dev mypy app-> Success: no issues found in 42 source filesgit diff --check origin/main...HEAD-> cleanScope and Safety
This is a focused MCP handshake compatibility fix only. It does not change tool dispatch, bounty lifecycle, ledger, treasury, wallet, payout, proof, balance, exchange, bridge, off-ramp, cash-out, price behavior, private data, secrets, or production mutation behavior.
Payout
payout address will be provided by the contributor on request.
Summary by CodeRabbit
New Features
Tests