Skip to content

Conversation

sdelamo
Copy link
Contributor

@sdelamo sdelamo commented Sep 24, 2025

Motivation and Context

This change adds methods to McpTransportContext: protocolVersion, lastEventId, sessionId, and principal. It also provides an implementation of Context extract for Servlet requests, which uses the HTTP Headers defined in the specification.

It aims to provide some structure to the McpTransportContext API.

How Has This Been Tested?

Yes, the change includes tests.

Breaking Changes

No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] 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

This change adds methods to McpTransportContext: `protocolVersion`, `lastEventId`, `sessionId`, and `principal`. It also provides an implementation of Context extract for Servlet requests, which uses the HTTP Headers defined in the specification.
* @return Extracts Map for MCP Transport Context
*/
protected Map<String, Object> metadata(HttpServletRequest request) {
Map<String, Object> metadata = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

might be worth passing a size to this hash map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added f02ec6f

…ocol/server/transport/ServerRequestMcpTransportContextExtractor.java
Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Are these methods of any use for the client-side case (e.g. McpAsyncHttpClientRequestCustomizer)? The McpTransportContext is used both in the server and in the client scenarios. Also, coupling McpTransportContext with java.security.Principal feels undesired for a general-purpose map-like container.
I am not against the utility of providing a unified context extractor that addresses common server-side concerns, but I don't think this is the right implementation.
Having in mind the client-side and the overfitting aspect of the basic interface, do you think providing a helper proxy for working with the McpTransportContext as an argument would be useful instead?

Copy link
Contributor

@Kehrlann Kehrlann left a comment

Choose a reason for hiding this comment

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

I see how that is useful on the server side.
However, initially, the McpTransportContext was introduced as a very thin, immutable data object (see its javadoc):

/**
 * Context associated with the transport layer. It allows to add transport-level metadata
 * for use further down the line. Specifically, it can be beneficial to extract HTTP
 * request metadata for use in MCP feature implementations.
 *
 * @author Dariusz Jędrzejczyk
 */

As such, it is used both in the server transports, to capture HTTP request information; as well as the client side to capture "context" information (notably thread-locals), and make that available inside the reactor context. See McpSyncClient and then subsequent usages in transports (e.g. HttpClientStreamableHttpTransport).

On the client side, all these new fields would be null. On the server side, when using stateless transport, I think the sessionId and lastEventId are also always null. Maybe the core McpTransportContext interface is not good candidate to add those fields, at least not in the current shape.

On the Spring side, we indeed need the principal when authorization is in use, but that's the only element we would be using ; we have extra needs (servlet request & response objects on the client side).

We have not provided McpTransportContextExtractor in the SDK for now, apart from your previous request #492, there seems to be no demand for this so far.

Should we postpone this until we have a clearer view of the community needs?

sdelamo and others added 4 commits September 24, 2025 15:09
…package-info.java

Co-authored-by: Daniel Garnier-Moiroux <[email protected]>
…HttpServletRequestMcpTransportContextExtractor.java

Co-authored-by: Daniel Garnier-Moiroux <[email protected]>
@graemerocher
Copy link
Contributor

perhaps what can be done is a sub-interface of McpTransportContext called McpServerTransportContext for the server use cases?

@sdelamo
Copy link
Contributor Author

sdelamo commented Sep 24, 2025

Sorry, but I don't understand the pushback.

The methods I added are transport concepts - protocol version , session id, Last event Id. Should not transport concepts be represented in Transport Context (McpTransportContext).

The same with Principal. Should the context of the transport not contain the authenticated user, if any? I don't see any coupling, as using Principal does not require any extra dependency.

The API return types are optional, and the API methods are default methods with empty options.

As an implementer of the context API you don't have to do anything. If you use the context in the client and it doesn't apply to you, don't use these methods. They don't require extra work on the client side.

as @Kehrlann pointed out in the javadocs of McpTransportContext:

Specifically, it can be beneficial to extract HTTP request metadata for use in MCP feature implementations.

That it is exactly what I am doing in HttpServletRequestMcpTransportContextExtractor.

If we don't add methods to the McpTransportContext API, then a user will have to guess the keys of the map. How will a user check whether an authenticated user can access a resource template by checking the context? Debugging the context to see where the security is saved?

@sdelamo
Copy link
Contributor Author

sdelamo commented Sep 24, 2025

I add a commit which hopefully illustrates the benefit of this PR

24098f2

Now, whether you use a jakarta.servlet.http.HttpServletRequest or a springs: org.springframework.web.servlet.function.ServerRequest or org.springframework.web.reactive.function.server.ServerRequest. You create an McpTransportContext parsing the request the same way.

@chemicL
Copy link
Member

chemicL commented Sep 25, 2025

Thinking about it a bit more, there are further concerns.
McpTransportContext is a wrapper around something coming from "a transport", but it's not tied to a particular one.

You can implement, say, a tool. And in your implementation you can get the McpAsyncServerExchange for instance, which holds the McpTransportContext (or simply the context in the case of stateless servers). The thing is that this tool does not know and should not know whether it is executed in the context of an HTTP request or as a local process via STDIO pipes. The transport concern is a layer away.

Even in the HTTP context, you are suggesting we add Streamable HTTP concepts to the generic transport context, but e.g. Last-Event-Id does not belong to the legacy MCP SSE spec (it is not specified in the context of MCP despite being part of the generic SSE specification).

Not to mention these are probably of no use on the client side.

What I'm trying to say is that in order to work with a particular transport, you can create the abstraction about what the tool might need, e.g. an interface called ServerRequestContext. That ServerRequestContext can expose logical notions such as isCallerAuthorizedToXYZ(McpTransportContext) and depending on the transport it can decide. Then the tool is decoupled from the particular transport, but the implementation of the tool can be coupled to this particular ServerRequestContext implementation.

On the transport provider side, a companion StreamableHttpServerRequestContext interface can expose the McpTransportContext fill(HttpRequest) method knowing what exactly to extract.

As the instantiator of the particular transport implementation, e.g. HttpServletStreamableServerTransportProvider, you can both inject the relevant extractor (using StreamableHttpServerRequestContext) that puts the relevant data under well known keys, but can also inject the reader of that data from the McpTransportContext into the tool implementation so that the implementation does not need to know the actual keys but relies on the logical thing that abstracts the transport away.

Let's discuss.


Regarding Principal: by coupling I do not mean needing a 3rd party dependency library, but just logical coupling of unrelated concepts (https://en.wikipedia.org/wiki/Interface_segregation_principle).

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.

4 participants