Open
Conversation
mcp-hub had stream-http support between itself and mcp-servers, but not between mcp-hub and clients. This commit adds this support and updates the SDK spec version. This is specially important for Codex, which DOOES NOT support SSE at all.
There was a problem hiding this comment.
Pull Request Overview
This PR adds Streamable HTTP transport support for client connections to mcp-hub, implementing the MCP 2025-03-26 specification while maintaining backward compatibility with existing SSE-based clients. This addresses the critical need for Codex client support, which doesn't support SSE.
- Added unified
/mcpendpoint supporting both POST and GET requests with automatic transport detection - Implemented session management with UUID-based session IDs and proper lifecycle callbacks
- Updated MCP SDK from v1.15.1 to v1.20.0 to support new transport features
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/server.js | Added unified MCP endpoint with transport detection logic for Streamable HTTP and legacy SSE |
| src/mcp/server.js | Implemented handleStreamableHTTP method with session management and transport lifecycle |
| package.json | Updated version to 5.0.2 and upgraded MCP SDK and dev dependencies |
| CHANGELOG.md | Added comprehensive changelog entries documenting the new features and fixes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
I need this as well. Please approve, author. Thank you in advance.. |
Author
|
I'm using this in my daily workflow both with SSE and Streamable HTTP clients without issues. I can create a release in my fork while the maintainer is unable to review this PR if anyone's interested. |
Definitely much appreciated to do so. I am using your version of the code. Thank you! |
|
Thank you @bcdonadio , using your fork now. |
bcdonadio
force-pushed
the
feat/stream-http
branch
from
ea12d1d to
9ef4efb
Compare
This commit addresses three critical issues and adds comprehensive test coverage: ## Fixes 1. **SSE Connection Routing** - Fixed 406 errors for SSE clients with non-standard Accept headers - Changed GET /mcp routing logic to default to SSE transport for any sessionless request - Previously rejected clients sending `Accept: */*` instead of `Accept: text/event-stream` - Now properly supports legacy clients like Kilo Code that don't send proper Accept headers - Resolves "SSE error: Non-200 status code (406)" connection failures 2. **Transport Cleanup Stack Overflow** - Fixed infinite recursion in cleanup handlers - Added cleanup guard flags to prevent circular cleanup calls - Issue occurred when `cleanup()` → `server.close()` → `transport.close()` → `onclose` → `cleanup()` created infinite loop - Affected both SSE and Streamable HTTP transports during graceful shutdown - Resolves "RangeError: Maximum call stack size exceeded" errors on client disconnect 3. **MCP Server Logging** - Improved stderr logging level for MCP server output - Changed MCP server stderr output from `warn` to `debug` level - Prevents normal informational messages from appearing as warnings - Improves log clarity by distinguishing between actual warnings and routine server output ## Tests Added comprehensive test coverage (20 passing tests): - **tests/server-routing.test.js** (13 tests) - Tests GET /mcp endpoint routing logic - Verifies SSE routing for sessionless requests regardless of Accept header - Tests backward compatibility with clients sending `Accept: */*` - Validates 406 error prevention for legacy clients - **tests/mcp-server.test.js** (7 tests) - Tests cleanup guard flags in both SSE and Streamable HTTP transports - Verifies prevention of infinite recursion during cleanup - Tests session management and cleanup behavior - **tests/MCPConnection.test.js** (updated) - Enhanced logger mock with debug method - Updated stderr test to verify debug logging instead of warn ## Changes - src/server.js: Updated GET /mcp routing to default to SSE for sessionless requests - src/mcp/server.js: Added cleanup guard flags to prevent recursion - src/MCPConnection.js: Changed stderr logging from warn to debug level - CHANGELOG.md: Added release notes for v5.0.3 - package.json: Bumped version to 5.0.3 - README.md: Updated transport documentation - tests/: Added comprehensive test coverage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
bcdonadio
force-pushed
the
feat/stream-http
branch
from
840b0a4 to
c38fdd2
Compare
|
Nice work! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mcp-hub had stream-http support between itself and mcp-servers, but not between mcp-hub and clients. This commit adds this support and updates the SDK spec version.
This is specially important for Codex, which DOES NOT support SSE at all.