feat: implement MCP-Protocol-Version header requirement for HTTP transport #614
Conversation
… it doesn't match negotiated version (it SHOULD match)
src/client/sse.ts
Outdated
| // Note: this._requestInit?.headers already set in _commonHeaders | ||
| // const headers = new Headers({ ...commonHeaders, ...this._requestInit?.headers }); |
There was a problem hiding this comment.
nit: personally don't think this comment adds much value and just adds potential stuff that becomes outdated over time, so would propose just removing the comments
that this._requestInit?.headers is already set in _commonHeaders is imo sufficiently clear from its implementation
| private validateProtocolVersion(req: IncomingMessage, res: ServerResponse): boolean { | ||
| let protocolVersion = req.headers["mcp-protocol-version"]; | ||
| if (Array.isArray(protocolVersion)) { | ||
| protocolVersion = protocolVersion[protocolVersion.length - 1]; |
There was a problem hiding this comment.
should we make sure protocolVersion.length > 0? not sure if req.headers would ever return an empty array
There was a problem hiding this comment.
I was implicitly relying on the fact that [][-1] === undefined tbh, but yes in practice if a header isn't set we get undefined, if it's set once we get a string, and if set mode than once we get an array of the values.
There was a problem hiding this comment.
ah TIL that returns undefined and not an error!
| if (Array.isArray(protocolVersion)) { | ||
| protocolVersion = protocolVersion[protocolVersion.length - 1]; | ||
| } |
There was a problem hiding this comment.
Type-wise it's a possiblity (happens if the client sets the header twice, which is legal; I'd rather not explode when that's the case)
src/client/index.ts
Outdated
| @@ -165,6 +165,8 @@ export class Client< | |||
|
|
|||
| this._serverCapabilities = result.capabilities; | |||
| this._serverVersion = result.serverInfo; | |||
| // HTTP transports must set the protocol version in each header after initialization. | |||
| transport.protocolVersion = result.protocolVersion; | |||
There was a problem hiding this comment.
I would prefer not to set the property directly, instead, expose setProtocolVersion
There was a problem hiding this comment.
Added setProtocolVersion (optional to avoid having to implement it in all transports - incl. test transports)
…ntextprotocol/typescript-sdk into ochafik/protocol-version
Implement MCP-Protocol-Version header requirement for HTTP transport (closes #604)
Motivation and Context
This PR implements the MCP-Protocol-Version header requirement as specified in:
(see similar update in Python SDK: modelcontextprotocol/python-sdk#898)
Clients extract and store the negotiated protocol version from the initialization response, include the
MCP-Protocol-Versionheader 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 missing 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 be stuck at the protocol version 2025-03-26.
Types of changes
Checklist
Additional context
Implementation Details
The implementation adds protocol version tracking to all HTTP-bas 8000 ed transports:
Client-side changes:
protocolVersion?: stringproperty to the Transport interfaceMcp-Protocol-Versionheader in all requests after initializationServer-side changes:
transport.protocolVersionafter successful initializationMcp-Protocol-Versionheader on all non-initialization requestsKey behaviors: