fix(mcp): migrate to MCP SDK 2.x so 2026-07-28 clients can connect - #2363
Conversation
markitdown-mcp pins `mcp~=1.8.0`, which predates the 2026-07-28 protocol revision. Clients implementing that revision open a connection with `server/discover` instead of the `initialize` handshake, and a 1.8.x server cannot answer it: the method is absent from the SDK's `ClientRequest` union, so validation raises inside the session receive loop, the exception unwinds the anyio task group, and the server process exits without replying. The client is left waiting on a peer that is already gone. Moving to `mcp>=2.1.1,<3.0.0` fixes this. The 2.x server detects the era from the client's first frame and serves either one on the same build, so hosts still sending `initialize` are unaffected. - `FastMCP` is `MCPServer` in 2.x. - The hand-rolled Starlette app is replaced by the SDK's `sse_app()` and `streamable_http_app()`, whose routes are merged so /sse, /messages/ and /mcp keep their current paths and behavior. Both sub-app lifespans are chained, which is what starts the Streamable HTTP session manager. - Adds stdio tests covering both handshake eras and asserting that an unknown method no longer takes the server process down. Verified against Claude Code, Codex and the Antigravity CLI: the first two open with `initialize`, the third with `server/discover`, and a `convert_to_markdown` call now succeeds on all three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The migration disables plugins, breaks remote HTTP binds, removes a security warning, and leaves the new tests outside CI.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Migrates the MCP server to SDK 2.x for legacy and 2026-07-28 protocol compatibility.
Changes:
- Replaces
FastMCPwithMCPServer. - Adopts SDK-provided HTTP/SSE applications.
- Adds end-to-end stdio protocol tests.
File summaries
| File | Description |
|---|---|
pyproject.toml |
Updates the MCP SDK dependency. |
__main__.py |
Migrates server and transport setup. |
test_stdio_protocols.py |
Tests both protocol eras and unknown methods. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
HTTP coverage, Python-version execution, and response-order assumptions need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
|
Çağdaş Yürekli (@cagdasyurekli) thanks for your PR. The original version omitted important features (enabling plugins), as well as the warning when binding to a non-localhost interface. The latter was added vert intentionally after many people were using the MCP insecurely. I've fixed both issues, and integrated the tests with the CI. Please test this in your environment, and let me know if it's working for you. If everything checks out, then I will merge it. |
|
Additional native Windows ARM64 datapoint from a Surface Pro. Environment:
The current So this is another real-world case supporting the SDK 2.x migration in this PR, this time on a native Windows ARM64 Codex host. I have not yet tested this PR branch itself on the Surface, so this is a reproduction of the current-package failure rather than a verification of the PR head. There is also a separate Windows ARM64 packaging issue which this PR does not address: I'll keep that separate from this MCP protocol migration rather than expanding this PR's scope. |
Conflicts:
- .github/workflows/tests.yml: upstream added an `ocr-tests` job and
switched `tests` to `hatch test -py=<matrix>`; this branch added an
`mcp-tests` job. Kept both, and re-pinned mcp-tests to the same
checkout/setup-python SHAs upstream now uses.
- packages/markitdown-mcp/src/markitdown_mcp/__main__.py: upstream added
`Settings.model_rebuild()` to silence a pydantic forward-reference
warning from `mcp.server.fastmcp.server.Settings`. That module no
longer exists in MCP SDK 2.x (FastMCP was renamed MCPServer), and
MCPServer("markitdown") constructs warning-free, so the workaround is
dropped as obsolete.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The substantially rewritten HTTP transports and lifespan remain untested in CI.
Review details
Suppressed comments (1)
packages/markitdown-mcp/src/markitdown_mcp/main.py:56
- This HTTP migration is not exercised by the new suite: every protocol test launches the default stdio transport. Please add an ASGI integration test that enters this lifespan and verifies the preserved
/mcp,/sse, and/messages/routes (especially that/mcpcan initialize while the session-manager lifespan is running); otherwise the most substantial transport change can regress while CI remains green.
async with sse_app.router.lifespan_context(app):
async with http_app.router.lifespan_context(app):
yield
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Unexpected converter failures can still expose internal exception details through FileConversionException.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The stdio fixture constructs invalid local file URIs on Windows.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Problem
markitdown-mcppinsmcp~=1.8.0. That SDK predates the 2026-07-28 protocol revision, in which a client opens a connection withserver/discoverrather than theinitializehandshake.A 1.8.x server cannot answer
server/discover. The method is not in the SDK'sClientRequestunion, somodel_validateraises insideBaseSession._receive_loop, which at that version has no guard around the request branch. TheValidationErrorunwinds the anyio task group and the server process exits without sending a reply:Because nothing is written back, the client is left waiting on a peer that has already died. Observed with the Antigravity CLI, which sits at "still connecting" indefinitely:
Related client-side reports: google-antigravity/antigravity-cli#657 (which identifies
server/discover+ the Python SDKValidationErroras the trigger) and google-antigravity/antigravity-cli#701. The unknown-method handling was fixed upstream in the SDK's 2.x line and closed for 1.x as maintenance-only (modelcontextprotocol/python-sdk#3193), so the fix has to happen here.Change
Move to
mcp>=2.1.1,<3.0.0.The 2.x server decides a connection's protocol era from the client's first frame — an
initializeopens a legacy connection, an enveloped request opens a 2026-07-28 one — so a single build serves both. Hosts still sendinginitializeare unaffected.FastMCP→MCPServer(renamed in 2.x).sse_app()andstreamable_http_app(). Their routes are merged into one app so/sse,/messages/and/mcpkeep their current paths,json_response=Trueandstateless_http=Trueare preserved, and both sub-app lifespans are chained (this is what starts the Streamable HTTP session manager).requires-pythonis unchanged:mcp2.1.1 is also>=3.10.Tests
Adds
tests/test_stdio_protocols.py, covering both handshake eras end to end plus a regression test that an unknown method no longer takes the process down.black(23.7.0, the pinned pre-commit version) reports no changes.Verified against real hosts
convert_to_markdowninitializeinitializeagy1.1.23)server/discoverStreamable HTTP and SSE were re-checked under
--http:POST /mcpreturns the initialize result andGET /ssereturns200 text/event-stream.One note for reviewers: on the modern path, closing stdin immediately after writing a request can tear the connection down before an in-flight tool call is answered. Real hosts hold the pipe open, and the test does the same, but it may be worth a look upstream.