Skip to content

feat: implement MCP-Protocol-Version header requirement for HTTP transport #898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 12, 2025

Conversation

felixweinberger
Copy link
Contributor

@felixweinberger felixweinberger commented Jun 7, 2025

Implement MCP-Protocol-Version header requirement for HTTP transport.

Motivation and Context

This PR implements the MCP-Protocol-Version header requirement as specified in:

Clients extract and store the negotiated protocol version from the initialization response, include the MCP-Protocol-Version header in all subsequent HTTP requests after initialization.

Servers validate the presence and correctness of this header for non-initialization requests, returning 400 Bad Request for unsupported or invalid protocol version headers.

How Has This Been Tested?

Updated all existing tests to include the MCP-Protocol-Version header. Added new tests for header validation.

Breaking Changes

This is a breaking change that requires both clients and servers to be updated. Clients that don't send the MCP-Protocol-Version header will receive 400 Bad Request errors from updated servers.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Approach:

Client-side (StreamableHTTP): Extract the protocolVersion from the initialization response's result field, store it in the transport instance, and include it as MCP-Protocol-Version header in all subsequent requests via _prepare_request_headers().

Server-side (StreamableHTTP): Add validation in _validate_protocol_version() that checks header presence and protocol version validity (returns 400 if missing or unsupported), apply validation to all non-initialization requests in POST, GET, and DELETE handlers, and consolidate header validation logic in _validate_request_headers() for extensibility.

@felixweinberger felixweinberger force-pushed the fweinberger/require-version branch from bdea62c to 16c94ae Compare June 7, 2025 03:46
Comment on lines 146 to 150
def _maybe_extract_protocol_version_from_message(
self,
message: JSONRPCMessage,
) -> None:
"""Extract protocol version from initialization response message."""
if isinstance(message.root, JSONRPCResponse) and message.root.result:
# Check if result has protocolVersion field
result = message.root.result
if "protocolVersion" in result:
self.protocol_version = result["protocolVersion"]
logger.info(f"Negotiated protocol version: {self.protocol_version}")

Copy link
Contributor Author

@felixweinberger felixweinberger Jun 7, 2025

Choose a reason for hiding this comment

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

inspecting the response in this way feels a bit wrong on the transport, but it looks like we're "peeking" into the response in various parts of the transport already

it felt a lot cleaner than doing something like creating a callback, passing that back to the session and have the session jam the version back into the transport, but keen for thoughts here

@felixweinberger felixweinberger force-pushed the fweinberger/require-version branch from 16c94ae to d28a7fc Compare June 9, 2025 15:29
@felixweinberger felixweinberger requested a review from dsp-ant June 9, 2025 17:40
@felixweinberger felixweinberger marked this pull request as ready for review June 9, 2025 18:37
@felixweinberger felixweinberger force-pushed the fweinberger/require-version branch from 9ee6458 to 14c3af8 Compare June 9, 2025 18:41
Copy link

@ochafik ochafik left a comment

Choose a reason for hiding this comment

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

Thanks Felix!

ihrpr
ihrpr previously approved these changes Jun 12, 2025
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

Thank you for working on this 🙏

Please can you address a few small things and good to go

felixweinberger and others added 9 commits June 12, 2025 15:37
Add integration tests to verify the new spec requirement that HTTP
clients must include the negotiated MCP-Protocol-Version header in
all requests after initialization.

Tests verify:
1. Client includes MCP-Protocol-Version header after initialization
2. Server validates header presence and returns 400 for missing/invalid
3. Server accepts requests with valid negotiated version

These tests currently fail as the feature is not yet implemented.

Related to spec change: modelcontextprotocol/modelcontextprotocol#548
The client now tracks the negotiated protocol version from the server's
response headers, enabling version-aware communication between client
and server.
- Add MCP_PROTOCOL_VERSION_HEADER constant
- Add _validate_protocol_version method to check header presence and validity
- Validate protocol version for all non-initialization requests (POST, GET, DELETE)
- Return 400 Bad Request for missing or invalid protocol versions
- Update tests to include MCP-Protocol-Version header in requests
- Fix test_streamablehttp_client_resumption to pass protocol version when resuming

This implements the server-side validation required by the spec change
that mandates clients include the negotiated protocol version in all
subsequent HTTP requests after initialization.

Github-Issue: #548
- Add extract_protocol_version_from_sse helper function to reduce code duplication
- Replace repeated protocol version extraction logic in 5 test functions
- Fix line length issues in docstrings to comply with 88 char limit

This improves test maintainability by centralizing the SSE response parsing logic.
- Add _validate_request_headers method that combines session and protocol validation
- Replace repeated calls to _validate_session and _validate_protocol_version
- Improves code maintainability and extensibility for future header validations
- No functional changes, all tests passing

This refactoring makes it easier to add new header validations in the future
by having a single entry point for all non-initialization request validations.
This better reflects that the method prepares headers for outgoing HTTP
requests, not just updating them with context. The method adds both
session ID and protocol version headers as needed.
…ader

When the MCP-Protocol-Version header is not present in requests, the server
now assumes protocol version "2025-03-26" instead of returning an error.
This maintains backwards compatibility with older clients that don't send
the version header.

The server still validates and returns 400 Bad Request for invalid or
unsupported protocol versions when the header is explicitly provided.

This change addresses the backwards compatibility requirement from
modelcontextprotocol/modelcontextprotocol#668

- Modified _validate_protocol_version to assume "2025-03-26" when header is missing
- Updated tests to verify backwards compatibility behavior
- Added new test specifically for backwards compatibility scenario
- update test comment
- log warning if no protocol version is set
- nits
@felixweinberger felixweinberger force-pushed the fweinberger/require-version branch from 622763a to 172db96 Compare June 12, 2025 14:37
- Use InitializeResult type for parsing server init response
- Add explanatory comment for DEFAULT_NEGOTIATED_VERSION constant
- Include supported protocol versions in error response when version mismatch occurs
@felixweinberger felixweinberger force-pushed the fweinberger/require-version branch from 172db96 to 5a5e7a9 Compare June 12, 2025 14:38
@felixweinberger felixweinberger requested a review from ihrpr June 12, 2025 14:43
try:
# Parse the result as InitializeResult for type safety
init_result = InitializeResult.model_validate(message.root.result)
self.protocol_version = str(init_result.protocolVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

something unsettling about this 😅 technically it will work, but can we handle None?

Copy link
Contributor

Choose a reason for hiding this comment

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

wait, actually it will not work, as now we'll have "None" and will not set it to the default

Copy link
Contributor Author

@felixweinberger felixweinberger Jun 12, 2025

Choose a reason for hiding this comment

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

protocolVersion can only be str | int:

class InitializeResult(Result):
    """After receiving an initialize request from the client, the server sends this."""

    protocolVersion: str | int
    """The version of the Model Context Protocol that the server wants to use."""
    capabilities: ServerCapabilities
    serverInfo: Implementation
    instructions: str | None = None
    """Instructions describing how to use the server and its features."""

Do you mean handling when message.root.result === None implying a broken server? if that happens the try-catch ensures we don't crash out, self.protocol_version just stays unset (this is on the client).

Copy link
Contributor Author

@felixweinberger felixweinberger Jun 12, 2025

Choose a reason for hiding this comment

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

We could also do something like this, where we set the self.protocol_version = DEFAULT_NEGOTIATED_VERSION but I would argue against this because we're on the client here and that might mask server issues:

try:
    # Parse the result as InitializeResult for type safety
    init_result = InitializeResult.model_validate(message.root.result)
    self.protocol_version = str(init_result.protocolVersion)
    logger.info(f"Negotiated protocol version: {self.protocol_version}")
except Exception as exc:
    # Assume the default version if parsing fails for any reason
    self.protocol_version = DEFAULT_NEGOTIATED_VERSION
    logger.warning(f"Failed to parse initialization response as InitializeResult: {exc}")
    logger.warning(f"Raw result: {message.root.result}")

If the server is returning incorrect responses I feel like client shouldn't be "hiding" that?

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I mean just remove str(init_result.protocolVersion) and instead do

self.protocol_version = str(init_result.protocolVersion) if init_result.protocolVersion is not None else None

Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

please can you double check handling None?

@felixweinberger
Copy link
Contributor Author

please can you double check handling None?

Left some additional comments in the thread - not quite sure what the concern about None is, whether it's handling a broken server response or incorrect parsing?

The protocolVersion field on InitializeResult can't be None according to the types.

@felixweinberger felixweinberger requested a review from ihrpr June 12, 2025 15:40
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

The protocolVersion field on InitializeResult can't be None according to the types.

ah, then all good!

@ihrpr ihrpr merged commit df15e95 into main Jun 12, 2025
20 of 21 checks passed
@ihrpr ihrpr deleted the fweinberger/require-version branch June 12, 2025 17:01
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.

require negotiated version when using HTTP
4 participants