-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add HTTP transport (Streamable HTTP) #20
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
base: main
Are you sure you want to change the base?
feat: add HTTP transport (Streamable HTTP) #20
Conversation
Add HTTP server mode using Server-Sent Events (SSE) transport, following conventions from mcp-atlassian. Changes: - Add express and cors dependencies - Create src/http-server.ts with Express + SSEServerTransport - Update CLI to support --transport and --port flags - Update main.ts to support both stdio and sse transports - Update Dockerfile with EXPOSE and new env var documentation - Update README with HTTP/SSE usage instructions Usage: # Stdio (default) node bin/icepanel-mcp-server.js # HTTP/SSE node bin/icepanel-mcp-server.js --transport sse --port 9846 # Docker with SSE docker run -d -p 9846:9846 ... icepanel-mcp-server --transport sse --port 9846
Replace deprecated SSE transport with the new Streamable HTTP transport per MCP specification 2025-03-26. Changes: - Rewrite http-server.ts to use StreamableHTTPServerTransport - Single /mcp endpoint replaces dual /sse + /messages endpoints - Update CLI: --transport http (with legacy 'sse' mapping) - Update Dockerfile and README documentation - Bump version to 0.2.0 The new transport provides: - Single endpoint architecture (simpler, easier to load balance) - Dynamic connection upgrades (JSON for quick ops, SSE for streaming) - Session management with resumability support - Better HTTP/2 and HTTP/3 compatibility Breaking change: MCP clients should update URL from /sse to /mcp
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.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| "license": "MIT", | ||
| "dependencies": { | ||
| "@modelcontextprotocol/sdk": "1.24.0", | ||
| "@modelcontextprotocol/sdk": "1.9.0", |
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.
SDK downgrade breaks StreamableHTTPServerTransport import
High Severity
The @modelcontextprotocol/sdk dependency is being downgraded from version 1.24.0 to 1.9.0, but the code imports StreamableHTTPServerTransport from @modelcontextprotocol/sdk/server/streamableHttp.js. The Streamable HTTP transport was added to the MCP spec in 2025-03-26 and is not available in the older 1.9.0 SDK version. This will cause a runtime module resolution failure when HTTP transport is used.
Additional Locations (1)
| }); | ||
|
|
||
| // Connect the MCP server to this transport | ||
| await server.connect(transport); |
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.
Single McpServer incorrectly shared across multiple transports
High Severity
A single McpServer instance is created in main.ts and passed to startHttpServer, where it is connected to a new StreamableHTTPServerTransport for each HTTP session via server.connect(transport). The MCP SDK expects one transport per server instance. Connecting multiple transports to one server can cause message routing issues and unexpected behavior when multiple clients connect simultaneously.
Additional Locations (1)
| { | ||
| "name": "@icepanel/mcp-server", | ||
| "version": "0.2.0", | ||
| "version": "0.1.1", |
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.
Package and server version mismatch after changes
Medium Severity
The diff changes package.json version from 0.2.0 to 0.1.1 while simultaneously changing src/main.ts version from 0.1.1 to 0.2.0. The new http-server.ts also hardcodes "0.2.0" in the health endpoint. After this PR, the package version is 0.1.1 but the server reports 0.2.0 in both the MCP handshake and health endpoint, creating an inconsistency that complicates debugging and version tracking.
Additional Locations (2)
| }); | ||
|
|
||
| // Connect the MCP server to this transport | ||
| await server.connect(transport); |
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.
Missing cleanup if server.connect fails causes resource leak
Medium Severity
The StreamableHTTPServerTransport is created at line 59-61, then server.connect(transport) is called at line 64 outside the try-catch block. If server.connect() throws (especially likely given the multiple-transport-per-server issue), the created transport is never closed. The transport isn't in the sessions map, so it won't be cleaned up on shutdown either. This causes a resource leak as the transport may hold internal event listeners and state.
| import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; | ||
|
|
||
| // Store active transports by session ID for stateful mode | ||
| const sessions = new Map<string, StreamableHTTPServerTransport>(); |
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.
Module-level sessions with per-call signal handlers is fragile
Low Severity
The sessions Map is declared at module level but modified inside startHttpServer, while SIGINT/SIGTERM handlers are registered inside the function on each call. If the function were called multiple times (e.g., during testing), sessions would persist across calls and multiple shutdown handlers would accumulate. Moving sessions inside the function or documenting single-call-only semantics would make the design clearer.
I wanted a first-class HTTP transport so MCP clients that can’t spawn a local process still have a clean option.
What I changed
/mcpand/healthssealias that maps tohttpWhy I did it
Test plan
Recommended merge order