-
Notifications
You must be signed in to change notification settings - Fork 57
[Server] Rework transport architecture and add StreamableHttpTransport #49
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
[Server] Rework transport architecture and add StreamableHttpTransport #49
Conversation
Following up on this, I introduced a formal session management system. Why a Dedicated Session System?The MCP spec details a stateful lifecycle that goes beyond a simple request/response. To properly support it, the server needs to track client state across multiple interactions (regardless of transport). A dedicated session system is crucial for handling:
What's New?I've introduced a set of flexible, decoupled components:
Next Steps & DiscussionThe next logical step is to integrate this session logic into the
IN the future, we could implement a stateless mode ("stateless session" sounds like an oxymoron 😄). It refers to creating a temporary, per-request session object that isn't persisted (or deleted at the end of the request) which can allow us to bypass the I wanted to introduce these classes as a discrete step to get feedback on the session architecture itself before weaving it into the request handling logic. (Or should it be an entire different PR? INcluded it here since it's integral to getting the Http Transport up and running) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm more in favor of what you had before for the examples in php-mcp - one main composer setup and shared dependencies. makes it easier to follow the example code I'd say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. I went this route because this HTTP example needs specific PSR-7 implementations (Nyholm PSR-7, PSR-7 Server, and Laminas HTTP Handler Runner) to demonstrate the HTTP transport properly.
Adding these as dev dependencies to the main composer.json
felt like a stretch since they're only relevant for this one HTTP example (unlike the STDIO examples which work with the base dependencies, or the former HTTP examples where the server was inbuilt)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the example has its own composer.json and installs dependencies locally so we wouldn't want to include that (unless we agree to move the dependencies to the base)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that's clear - sorry, should have been more explicit.
If the vendor path is really needed, let's add it in the root level one - but I'd be fine with adding the deps to root dev dependencies - we'll need them in other examples as well potentially
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Got it!
Run with the MCP Inspector for testing: | ||
|
||
```bash | ||
npx @modelcontextprotocol/inspector http://localhost:8000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently doesn't work for me - am i missing something? it was running for you already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Make sure you’ve pulled the latest commit and run composer update (I added a new dependency: psr/clock). Also, check that the inspector is set to Streamable HTTP instead of STDIO cos it sometimes defaults to STDIO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, did all of that ... will try again later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting 🤔. Waiting for feedback then
Thanks @CodeWithKyrian - great to see this tackled! :) Just dropping my thoughts here, while wrapping my head around it: other than that, i'd love to have the transport extension point that abstract in the long run, that we could have various kind of infrastructure adapters. http-foundation, reactphp, swool, mercure ... donno - not sure if realistic, but you pointed out the challenges on that layer already 👍 |
@Nyholm was just thinking if there is some synergy or conceptual similarity with that php-runtime idea ... just for MCP transports tho |
Thanks for the feedback
Glad to see we’re aligned on this 👍🏽 |
40ebb80
to
260f59e
Compare
…nsport - `Server::connect()` no longer contains a processing loop. - `TransportInterface` is updated with `setMessageHandler()` and `listen()`. - `StdioTransport` is updated to implement the new interface. - A new, minimal `StreamableHttpTransport` is added for stateless HTTP.
- Updated `TransportInterface` to use `onMessage` for handling incoming messages with session IDs. - Refactored `Server`, `Handler`, and transport classes to accommodate session management using `Uuid`. - Introduced methods for creating sessions with auto-generated and specific UUIDs in `SessionFactory` and `SessionFactoryInterface`.
- Added session support to the `Server` and `Handler` classes, allowing for session data to be managed during message processing. - Updated `TransportInterface` to include session context in the `send` method. - Refactored various request handlers to utilize session information, ensuring proper session handling for incoming requests. - Introduced a file-based session store for persistent session data management
9946beb
to
25bec57
Compare
Quick updates on this:
This PR touches quite a few files, so the sooner we can wrap up reviews and merge, the smoother things will be for everyone as we move forward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes agree, let's get this merged to move on 👍
Thanks @CodeWithKyrian - the pipeline still needs attention tho
On it. Quick one: It's safe to nuke the |
I'd say yes - it's deprecated and we have the new one in place than |
b2a067f
to
ec99e1f
Compare
ec99e1f
to
5605f07
Compare
Alright. All set @chr-hertel |
Hey team,
I've been working on the Streamable HTTP transport and wanted to share a proof-of-concept to get everyone's thoughts. While building it, I realized that to properly support different transport lifecycles (like blocking STDIO vs. single-request HTTP), we might need a more flexible architecture.
This PR introduces that new architecture along with a foundational implementation of the
StreamableHttpTransport
.The Core Idea: Decoupling the Server from the Transport Lifecycle
The main change here is a shift in responsibility. The previous
Server::connect()
method owned the message processing loop, which worked well for a simple, generator-based transport.However, this model creates a challenge for different transport types:
Forcing the
Server
class to manage these different lifecycles feels complex. This PR proposes a cleaner pattern:The
Server
is only responsible for processing messages. TheTransport
is responsible for its own execution lifecycle.Now, the flow is:
$server->connect($transport)
simply registers the server's message handler with the transport.$transport->listen()
is called to start the transport's specific lifecycle.This decouples the two components, making the whole system much more flexible and easier to extend.
What Changed?
To achieve this, I've made a few key changes:
TransportInterface
Rework:receive()
andisConnected()
.setMessageHandler(callable $handler)
for the server to register its callback.listen()
method, which serves as the main entry point for any transport's execution.Server
Refactor:connect()
method is now very lightweight. It just wires up the message handler.StdioTransport
Update:listen()
method now contains the blockingwhile
loop that continuously reads fromSTDIN
.New
StreamableHttpTransport
:ServerRequest
and PSR factories.listen()
method is non-blocking...it basically processes the single request and immediately returns a PSR-7Response
.I've included a basic example (see
10-simple-http-transport
) showing how to wire this up in a vanilla PHP setup usingnyholm/psr7
showing the clean separation between connecting the server and listening with the transport.Points for Discussion
I intentionally kept the
StreamableHttpTransport
very minimal to start a conversation on the next steps.1. Naming the Execution Method
I've introduced a new method on the
TransportInterface
calledlisten()
. This name felt descriptive for a transport, but I'm open to other suggestions if the team thinks another name likerun()
orstart()
would be clearer. What are your thoughts?2. Event Emitter vs. Callback?
I've used a simple
setMessageHandler
callback to connect the server and transport. An alternative could be to use an event emitter pattern (e.g., withevenement/evenement
), where the transport emits amessage
event. (which I was using in the PHP MCP project). I find the event pattern a bit cleaner, as the transport doesn't need to know if a handler is attached. Would there be any appetite for bringing in a lightweight event emitter for this, or is the direct callback approach preferred for its simplicity and lack of dependencies?3. Next Steps for
StreamableHttpTransport
This implementation is deliberately bare-bones. It doesn't handle:
SessionManager
?/mcp
).My goal with this PR is to get a "vibe check" on this architectural direction before diving deeper into features like session management. I believe this new structure gives us a solid and highly flexible foundation for supporting all kinds of PHP environments. What do you think?