Fix parsing middleware to handle SSE streams with any path #1624
+501
−12
Conversation
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
JAORMX
force-pushed
the
fix-issue-1187-sse-parsing
branch
from
8d71e3d to
f87d542
Compare
JAORMX
force-pushed
the
fix-issue-1187-sse-parsing
branch
from
f87d542 to
7c5a881
Compare
blkt
previously approved these changes
Aug 29, 2025
This fixes issue #1187 where the parsing middleware was failing on SSE streams when the path didn't contain /messages, /message, or /mcp. The problem was that the MCP spec allows for other endpoints to be used: - Streamable HTTP transport: single endpoint - SSE transport: two distinct endpoints (one for SSE stream, one for messages) Changes: - Remove path restriction in shouldParseMCPRequest function - Parse all POST requests with JSON content type (except SSE establishment) - Add comprehensive integration tests using real MCP clients and servers - Test with various endpoint paths including custom and tenant-specific paths - Update existing tests to reflect the new behavior - Refactor integration tests to use actual client-server interactions The integration tests now: - Use real MCP clients (SSE and Streamable HTTP) from mcp-go library - Test actual client-server communication through the middleware - Verify that requests are properly parsed regardless of endpoint path - Cover standard paths, custom paths, and tenant-specific paths This ensures the parser works correctly with MCP servers like gofetch that use different endpoint paths (e.g., /sse for both SSE stream and messages). Fixes #1187
Address code review feedback by replacing string-based test logic with explicit struct fields. Instead of using tc.name to determine the endpoint for streamable HTTP tests, add an 'endpoint' field to the test case struct. This makes the test logic more explicit and less fragile, following better testing practices by avoiding logic based on test names. Changes: - Add 'endpoint' field to test case struct for streamable-http transport - Remove tc.name == 'Streamable HTTP with custom path' logic - Use tc.endpoint directly for streamable HTTP server setup - Maintain same test coverage with cleaner, more maintainable code
JAORMX
force-pushed
the
fix-issue-1187-sse-parsing
branch
from
bd7f5d0 to
4ea7589
Compare
blkt
approved these changes
Aug 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR fixes issue #1187 where the parsing middleware was failing on SSE streams when the path didn't contain `/messages`, `/message`, or `/mcp`.
Problem
The MCP spec allows for various endpoints to be used:
However, our parsing middleware was only processing requests to specific paths, causing failures with MCP servers like gofetch that use different endpoint paths (e.g., `/sse` for both SSE stream and messages).
Solution
Changes
Testing
Related Issues
Fixes #1187
Checklist