Skip to content

feat(mcp): add Streamable HTTP with SSE fallback - #1552

Merged
lyj715824 merged 3 commits into
iflytek:mainfrom
onatozmenn:feature/mcp-streamable-http
Jul 28, 2026
Merged

feat(mcp): add Streamable HTTP with SSE fallback#1552
lyj715824 merged 3 commits into
iflytek:mainfrom
onatozmenn:feature/mcp-streamable-http

Conversation

@onatozmenn

@onatozmenn onatozmenn commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Streamable HTTP support for MCP clients while keeping legacy SSE servers working. Existing callers do not need to change anything: the current endpoints default to auto.

In auto, the client tries Streamable HTTP first and falls back to SSE only when the connection or transport negotiation fails. It does not retry authentication, TLS, or post-initialization errors, so a tool call cannot be sent twice through different transports.

The change also updates the Python MCP SDK from 1.6.0 to 1.28.1 with a <2 upper bound.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring

Related Issue

Closes #1548

Testing

  • Full link suite: 200 passed on Python 3.11 with the locked dependencies.
  • MCP transport/server tests: 20 passed, including a real FastMCP Streamable HTTP initialize -> list_tools -> call_tool roundtrip.
  • TLS certificate and handshake failures, auth failures, explicit transports, timeouts, and post-initialization errors are covered.
  • Manual testing completed
  • CI-equivalent Black, isort, flake8, strict mypy, pylint (8.99/10), uv lock --check, and git diff --check pass.

Screenshots (if applicable)

Not applicable; this only changes backend transport behavior.

Checklist

  • Code follows project coding standards
  • Self-review completed
  • Documentation updated (the request models expose the selector through OpenAPI)
  • Breaking changes documented (none)

Responsibility

GitHub Copilot assisted with implementation and testing. I reviewed the changes and take responsibility for this submission.

Signed-off-by: onatozmenn <onatozmen44@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces transport-aware connection handling and selection for Model Context Protocol (MCP) servers, adding support for both Streamable HTTP and SSE transports. It implements an automatic fallback mechanism from Streamable HTTP to SSE under certain network or protocol negotiation failures when using the AUTO transport setting. The changes include updating API schemas, upgrading dependencies (including updating the mcp library), refactoring connection logic, and adding comprehensive unit tests. No review comments were provided, so I have no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@FenjuFu FenjuFu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through the whole change — this is a careful, well-tested implementation. LGTM.

What I checked:

  • Fallback safety (the crux). initialized_mcp_session only falls back while transport is AUTO and before initialization completes; the initialized flag makes any post-init exception re-raise instead of retrying through SSE, so a tools/call can never be sent twice over two transports. _fallback_reason correctly refuses to fall back on SSLCertVerificationError and on 401/403, and the capture_response hook means a timeout after an HTTP exchange was accepted is treated as a real error, not transport discovery. That's exactly the right set of guards.
  • Backwards compatibility. MCPTransport defaults to AUTO at every new call site and the schema default is AUTO, so existing callers are untouched. Transport-setup failures are mapped back onto the existing MCP_SERVER_* error codes, so the public error contract doesn't change.
  • Tests. 16 focused tests covering the branches that matter — no-fallback-on-auth, no-fallback-on-cert, no-retry-after-init, no-fallback-after-accepted-timeout, explicit-transport-never-falls-back, plus real-SDK sessions. Good coverage of the failure matrix.
  • CI green, DCO and CLA signed.

One thing to flag for the record, not a blocker: the SDK bump is a wide jump (mcp==1.6.0>=1.28.1,<2). You note the existing 198-test suite passes on 3.13, which is the reassurance I'd want; the <2 upper bound is the right call to avoid an unpinned major. If anything elsewhere in link imports from mcp internals beyond the client transports, worth a quick grep, but the test pass makes me comfortable.

Nice work — approving. Merge is the maintainers' call.

Copy link
Copy Markdown
Contributor

Thanks for the implementation. The overall transport/session abstraction looks reasonable, especially the guard that prevents fallback after initialization. However, I found a few issues that should be addressed before merging:

  1. The new async tests do not collect in the repository CI environment. Both new test files use @pytest.mark.asyncio, but pytest-asyncio is not declared in pyproject.toml/uv.lock, and pytest.ini enables --strict-markers without registering asyncio. The current Test core-link job stops during collection with:

    'asyncio' not found in `markers` configuration option
    collected 180 items / 2 errors
    

    Please either add and lock pytest-asyncio (plus the appropriate configuration), or convert these tests to the project's supported AnyIO setup.

  2. The current Check core-link job also fails mypy. tests/unit/test_mcp_transport.py has five helpers without return type annotations (lines 48, 56, 69, 76, and 326), which fail the repository's --disallow-untyped-defs check. These annotations need to be added before CI can pass.

  3. TLS fallback protection is narrower than the PR description claims. _fallback_reason() rejects only ssl.SSLCertVerificationError. Other TLS failures such as handshake/protocol/alert errors are ssl.SSLError and are commonly wrapped in httpx.ConnectError; the later httpx.TransportError branch then classifies them as network_error and falls back to SSE. That contradicts the stated behavior that TLS failures are not retried. Please reject nested ssl.SSLError before the generic transport/OSError checks and add a handshake-failure test.

There is also a coverage gap around the main success path: the list/call tests mock initialized_mcp_session, while the tests using the real SDK only exercise 405/401 failures. A real successful Streamable HTTP initialize -> list_tools -> call_tool test would validate the feature's primary acceptance path and the SDK upgrade.

For reference, the latest required checks currently show Check core-link, Test core-link, and the summary job as failed.

Signed-off-by: onatozmenn <onatozmen44@gmail.com>
…ble-http

Signed-off-by: onatozmenn <onatozmen44@gmail.com>
@onatozmenn

Copy link
Copy Markdown
Contributor Author

Thanks, these were good catches. I pushed the fixes in 06a39ef and updated the branch to current main.

pytest-asyncio is now in the link dependencies and lockfile, the helpers are typed, and all nested ssl.SSLError cases skip SSE fallback. I added both the TLS handshake regression and a real FastMCP Streamable HTTP initialize -> list_tools -> call_tool roundtrip.

The full link suite is 200/200 on Python 3.11, and the CI-equivalent quality checks pass.

@onatozmenn

Copy link
Copy Markdown
Contributor Author

Just a quick ping on this one. All three points from the review are fixed in 06a39ef (pytest-asyncio added and locked, helpers typed, nested ssl.SSLError no longer falls back to SSE), plus the TLS handshake regression test and a real FastMCP Streamable HTTP roundtrip.

CI is fully green now, Check core-link and Test core-link included. Anything else you'd like changed before this can go in?

@lyj715824

Copy link
Copy Markdown
Contributor

Thanks for the PR! The following three items can be addressed in follow-up work — this PR is good to merge as-is:
Add protocol-level overall timeouts for initialization, listing tools, and tool invocation, aligned with the upstream budgets of 40/90/300 seconds.
Support safe fallback for protocol negotiation-related 4xx responses (400/406/415), while continuing to exclude auth and rate-limiting statuses.
Prevent a 200 received during initialization from being overwritten by a 404/405 on DELETE during the exit phase, which would incorrectly trigger SSE fallback.

@lyj715824
lyj715824 merged commit 81f58f5 into iflytek:main Jul 28, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Support MCP Streamable HTTP transport with legacy SSE fallback

3 participants