Server: Added Streamable HTTP & Bump MCP SDK version #166
Conversation
…m guards; bump SDK
|
thanks for the PR! we need to make this optional. can you share your usecase? |
|
The Use case is that there are some MCP clients that only support the new Http Streamable method. |
Co-authored-by: AhmedElhadidii <89388836+AhmedElhadidii@users.noreply.github.com>
Co-authored-by: AhmedElhadidii <89388836+AhmedElhadidii@users.noreply.github.com>
…-36a8041e48ba Support both Streamable HTTP and SSE transports for backward compatibility
AhmedElhadidii
changed the title
|
@AhmedElhadidii this PR is lit 🔥 🔥 ! I'll check it thoroughly with n8n and I'll merge. |
|
btw, why was the types.d file needed? these types are exported already in the mcp package itself. thank you so much for this PR!!!! 🙏 |
Yes You're absolutely Right!👌 |
Removed checks for iOS Simulator availability on non-macOS platforms. Device won't be returned in list_available_devices
WalkthroughIntroduces a dual-transport HTTP server supporting StreamableHTTPServerTransport and SSEServerTransport with session-based management. Replaces SSE-only setup with unified /mcp endpoint, adds backward-compatible SSE routes, and refactors server initialization with improved error handling and updated startup messaging. Changes
Possibly related issues
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (2)
src/server.ts (2)
createMcpServer(34-612)getAgentVersion(29-32)src/logger.ts (1)
error(19-21)
🔇 Additional comments (4)
src/index.ts (4)
22-97: Streamable HTTP transport implementation looks solid.The session management logic correctly validates transport types, creates new sessions only for initialization requests, and properly cleans up on close. The
app.all()approach appropriately delegates HTTP method handling (including DELETE for session termination) to the SDK'stransport.handleRequest().
122-156: SSE message handling is well-structured.Good validation chain for session ID and transport type, with appropriate error responses. The type guard on line 136 ensures type safety before calling
handlePostMessage.
158-163: Startup logging is clear and helpful.Good documentation of both available transport endpoints. The logging is consistent with existing patterns in the codebase.
181-195: CLI interface properly updated.Help text accurately describes the dual-transport capability, and the function invocation correctly uses the renamed
startHttpServer.
| app.get("/sse", async (req, res) => { | ||
| console.log("Received GET request to /sse (SSE transport)"); | ||
| try { | ||
| const transport = new SSEServerTransport("/messages", res); | ||
| transports[transport.sessionId] = transport; | ||
|
|
||
| res.on("close", () => { | ||
| delete transports[transport.sessionId]; | ||
| }); | ||
|
|
||
| const server = createMcpServer(); | ||
| await server.connect(transport); | ||
| } catch (error: any) { | ||
| console.error("Error setting up SSE transport:", error); | ||
| if (!res.headersSent) { | ||
| res.status(500).send("Failed to establish SSE connection"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential memory leak if server.connect() fails.
The transport is added to the transports map on line 106 before server.connect() is called on line 113. If connect() throws, the entry remains in the map since res.on("close") may not fire for an incompletely established SSE connection.
🔎 Proposed fix: store transport after successful connection
app.get("/sse", async (req, res) => {
console.log("Received GET request to /sse (SSE transport)");
try {
const transport = new SSEServerTransport("/messages", res);
- transports[transport.sessionId] = transport;
res.on("close", () => {
delete transports[transport.sessionId];
});
const server = createMcpServer();
await server.connect(transport);
+ transports[transport.sessionId] = transport;
} catch (error: any) {
console.error("Error setting up SSE transport:", error);
if (!res.headersSent) {
res.status(500).send("Failed to establish SSE connection");
}
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| app.get("/sse", async (req, res) => { | |
| console.log("Received GET request to /sse (SSE transport)"); | |
| try { | |
| const transport = new SSEServerTransport("/messages", res); | |
| transports[transport.sessionId] = transport; | |
| res.on("close", () => { | |
| delete transports[transport.sessionId]; | |
| }); | |
| const server = createMcpServer(); | |
| await server.connect(transport); | |
| } catch (error: any) { | |
| console.error("Error setting up SSE transport:", error); | |
| if (!res.headersSent) { | |
| res.status(500).send("Failed to establish SSE connection"); | |
| } | |
| } | |
| app.get("/sse", async (req, res) => { | |
| console.log("Received GET request to /sse (SSE transport)"); | |
| try { | |
| const transport = new SSEServerTransport("/messages", res); | |
| res.on("close", () => { | |
| delete transports[transport.sessionId]; | |
| }); | |
| const server = createMcpServer(); | |
| await server.connect(transport); | |
| transports[transport.sessionId] = transport; | |
| } catch (error: any) { | |
| console.error("Error setting up SSE transport:", error); | |
| if (!res.headersSent) { | |
| res.status(500).send("Failed to establish SSE connection"); | |
| } | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/index.ts around lines 102 to 119, the SSE transport is inserted into the
transports map before server.connect(), so if connect() throws the transport
remains registered and leaks; instead instantiate the transport but only add it
to transports and attach the res.on("close") cleanup after await
server.connect(transport) succeeds; on connect failure ensure you clean up by
calling any transport.close() / dispose method (or otherwise free resources) and
do not leave the transport in the map, then send the 500 response if headers not
sent.
3 participants
Summary;
-Migrate server transport from SSE to Streamable HTTP.
-Consolidate all MCP traffic to a single /mcp endpoint with StreamableHTTPServerTransport.handleRequest.
-Keep stdio mode unchanged.
-Add Linux-safe guards to avoid xcrun calls on non-macOS.
-Bump MCP SDK to ^1.13.2.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.