Feat: Impl Server-Side Streamable HTTP transport #273
Conversation
WalkthroughThis change updates the MCP protocol version constant, removes a local constant in the StreamableHTTP client transport in favor of using the protocol package's constant, and introduces a new StreamableHTTPServer implementation with comprehensive tests. It also adds a minimal logging utility and removes the previously introduced StreamableHTTPOption interface and stub server from HTTP transport options. Additionally, it refactors the SSE server configuration to use functional options and updates example programs to replace SSE transport with the new StreamableHTTP transport. Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
util/logger.go (1)
16-20: Consider adding a constructor parameter for custom log.Logger.The DefaultLogger function creates a logger using log.Default(), which is good for simple cases. For more flexibility, consider allowing users to provide their own log.Logger instance.
-func DefaultLogger() Logger { +func DefaultLogger(opts ...LoggerOption) Logger { return &stdLogger{ logger: log.Default(), } } + +type LoggerOption func(*stdLogger) + +// WithCustomLogger allows using a custom log.Logger instance +func WithCustomLogger(logger *log.Logger) LoggerOption { + return func(l *stdLogger) { + l.logger = logger + } +}server/streamable_http_test.go (2)
265-321: Consider more deterministic testing for SSE responses.The test relies on sleep intervals to coordinate notification timing, which could make the test flaky in certain environments.
Consider using channels or other synchronization mechanisms to make the test more deterministic:
func addSSETool(mcpServer *MCPServer) { + var wg sync.WaitGroup mcpServer.AddTool(mcp.Tool{ Name: "sseTool", }, func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Send notification to client server := ServerFromContext(ctx) + wg.Add(10) for i := 0; i < 10; i++ { server.SendNotificationToClient(ctx, "test/notification", map[string]any{ "value": i, }) - time.Sleep(10 * time.Millisecond) + wg.Done() } + wg.Wait() // send final response return mcp.NewToolResultText("done"), nil }) }
458-502: Add verification for cancellation handling.The test for GET requests with SSE verifies basic functionality, but it would be beneficial to also test what happens when the context is canceled.
Add a test case that verifies proper cleanup when a client disconnects:
t.Run("Client disconnect", func(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) req, _ := http.NewRequestWithContext(ctx, "GET", server.URL, nil) req.Header.Set("Content-Type", "text/event-stream") // Start the request but cancel it shortly after go func() { time.Sleep(50 * time.Millisecond) cancel() }() resp, err := server.Client().Do(req) if err == nil { defer resp.Body.Close() // Read a bit of data to ensure the connection was established buf := make([]byte, 10) _, _ = resp.Body.Read(buf) } // Verify the server handled the disconnection gracefully // This might require exposing some metrics from the server // or checking logs to ensure proper cleanup occurred })server/streamable_http.go (2)
183-186: Content-Type check is too strict – rejects valid clientsThe spec allows
Content-Type: application/json; charset=utf-8.
Using a plain equality check will cause legitimate requests to fail.-contentType := r.Header.Get("Content-Type") -if contentType != "application/json" { +contentType := r.Header.Get("Content-Type") +if !strings.HasPrefix(contentType, "application/json") {This small change improves interoperability without relaxing security.
458-467: Potential concurrent‐map writes when tools are added afterget
getreturns the underlying map by reference.
If one goroutine iterates over the map while another writes viaSetSessionTools, the program can panic with “concurrent map read and map write”.Consider returning a copy of the map:
func (s *sessionToolsStore) get(sessionID string) map[string]ServerTool { s.mu.RLock() defer s.mu.RUnlock() - return s.tools[sessionID] + orig := s.tools[sessionID] + if orig == nil { + return nil + } + clone := make(map[string]ServerTool, len(orig)) + for k, v := range orig { + clone[k] = v + } + return clone }This small safeguard prevents rare but hard-to-diagnose race conditions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/transport/streamable_http.go(1 hunks)mcp/types.go(1 hunks)server/streamable_http.go(1 hunks)server/streamable_http_test.go(1 hunks)util/logger.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
client/transport/streamable_http.go (1)
mcp/types.go (1)
MethodInitialize(16-16)
server/streamable_http.go (5)
server/http_transport_options.go (1)
HTTPContextFunc(14-14)util/logger.go (2)
Logger(8-11)DefaultLogger(16-20)mcp/types.go (5)
Request(103-116)PARSE_ERROR(236-236)MCPMethod(11-11)MethodInitialize(16-16)JSONRPCNotification(206-209)server/server.go (1)
ServerTool(50-53)server/session.go (2)
ClientSession(11-20)SessionWithTools(23-31)
🪛 GitHub Check: test
server/streamable_http.go
[failure] 20-20:
StreamableHTTPOption redeclared in this block
[failure] 25-25:
cannot use func(s *StreamableHTTPServer) {…} (value of type func(s *StreamableHTTPServer)) as StreamableHTTPOption value in return statement: func(s *StreamableHTTPServer) does not implement StreamableHTTPOption (missing method applyToStreamableHTTP)
[failure] 28-28:
s.endpointPath undefined (type *StreamableHTTPServer has no field or method endpointPath)
[failure] 37-37:
cannot use func(s *StreamableHTTPServer) {…} (value of type func(s *StreamableHTTPServer)) as StreamableHTTPOption value in return statement: func(s *StreamableHTTPServer) does not implement StreamableHTTPOption (missing method applyToStreamableHTTP)
[failure] 38-38:
s.sessionIdManager undefined (type *StreamableHTTPServer has no field or method sessionIdManager)
[failure] 47-47:
cannot use func(s *StreamableHTTPServer) {…} (value of type func(s *StreamableHTTPServer)) as StreamableHTTPOption value in return statement: func(s *StreamableHTTPServer) does not implement StreamableHTTPOption (missing method applyToStreamableHTTP)
[failure] 48-48:
s.sessionIdManager undefined (type *StreamableHTTPServer has no field or method sessionIdManager)
[failure] 58-58:
cannot use func(s *StreamableHTTPServer) {…} (value of type func(s *StreamableHTTPServer)) as StreamableHTTPOption value in return statement: func(s *StreamableHTTPServer) does not implement StreamableHTTPOption (missing method applyToStreamableHTTP)
[failure] 59-59:
s.listenHeartbeatInterval undefined (type *StreamableHTTPServer has no field or method listenHeartbeatInterval)
🪛 GitHub Actions: go
server/streamable_http.go
[error] 20-20: StreamableHTTPOption redeclared in this block
🔇 Additional comments (10)
util/logger.go (2)
7-11: Well-designed minimal logging interface.The Logger interface is clean and provides just what's needed for basic logging functionality with appropriate method signatures.
27-33: LGTM!The implementation of the logging methods is straightforward and correct. It properly prepends the log level to the message.
mcp/types.go (1)
92-92: Version update reflects protocol evolution.Updating LATEST_PROTOCOL_VERSION to "2025-03-26" aligns with the PR objective of implementing the server-side streamable HTTP transport based on the latest protocol version.
client/transport/streamable_http.go (1)
200-200: Good improvement using centralized constant.Replacing the hardcoded string with the protocol package's constant improves maintainability and ensures consistency across the codebase.
server/streamable_http_test.go (6)
38-53: Well-structured test helper for SSE notifications.The
addSSEToolfunction provides a clean way to test server-sent events by sending multiple notifications followed by a final response.
55-75: Basic server instantiation tests are comprehensive.These tests verify that the server instantiates correctly with the expected configuration options.
77-122: Good error handling tests for invalid inputs.The tests properly verify error handling for invalid content types and malformed JSON.
130-161: Initialization and session management tests look good.The test properly verifies that initialization works and generates a session ID in the response header.
238-263: Security validation for session ID.Good test for invalid session ID handling, which is an important security check to prevent session impersonation.
504-551: Good integration test with HTTP multiplexer.This test verifies that the StreamableHTTPServer works correctly when integrated with a custom HTTP mux, which is important for real-world usage.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
server/streamable_http_test.go (2)
124-131: Apply the sameserver.Close()hygiene here
TestStreamableHTTP_POST_SendAndReceivealso leaves thehttptest.Serverrunning for the remainder of the test suite.
Uset.Cleanup(server.Close)right after construction.
323-327: Stateless variant leaks the test server as wellAdd
defer server.Close()(ort.Cleanup) for the stateless test block.
🧹 Nitpick comments (2)
server/streamable_http_test.go (2)
291-293: Relax the strict header comparisonSome HTTP stacks append a
charsetparameter (text/event-stream; charset=utf-8).
Comparing the whole value can lead to flaky failures.- if resp.Header.Get("content-type") != "text/event-stream" { + if !strings.HasPrefix(resp.Header.Get("content-type"), "text/event-stream") { t.Errorf("Expected content-type text/event-stream, got %s", resp.Header.Get("content-type")) }
151-154: Avoid hard-coding the protocol version in testsThe constant
"2025-03-26"is duplicated throughout the tests.
Import the canonical value from themcppackage (e.g.mcp.ProtocolVersion) so that updates in the library don’t break the test suite.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/streamable_http_test.go(1 hunks)
🔇 Additional comments (1)
server/streamable_http_test.go (1)
295-303: Reading the entire SSE stream can hang if the connection stays open
io.ReadAll(resp.Body)blocks until the server closes the connection.
If the implementation starts keeping the stream open for heart-beats or resumability, this test will dead-lock.Consider switching to a buffered reader and breaking once the terminating “done” event is observed or after a timeout:
reader := bufio.NewReader(resp.Body) for { line, err := reader.ReadString('\n') if err != nil { ... } if strings.Contains(line, `"done"`) { break } }
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
server/http_transport_options.go (1)
70-78: 💡 Verification agent❓ Verification inconclusive
No-op stub setters silently drop options
All setters on
StreamableHTTPServerare currently empty.
Calling helpers such asWithStaticBasePathorWithKeepAliveIntervalagainst a streamable server will therefore succeed compilation-wise but have zero runtime effect – a classic “silent mis-configuration” trap.If these options are genuinely irrelevant, consider:
- Documenting that they are ignored for
StreamableHTTPServer; or- Removing the
httpTransportConfigurableconformance altogether so that accidental use is caught at compile-time; or- Implementing meaningful behaviour.
Fail-fast beats fail-silent.
Stub setter methods silently drop configuration
All
set*methods onStreamableHTTPServerin server/http_transport_options.go (lines 70–78) are empty, so calls likeWithStaticBasePathorWithKeepAliveIntervalcompile but have no runtime effect. To avoid silent mis-configuration, please choose one of the following:
- Document that these options are intentionally ignored for
StreamableHTTPServer.- Remove the
httpTransportConfigurableconformance so invalid calls fail at compile time.- Implement the actual behavior for each setter.
🧹 Nitpick comments (3)
server/http_transport_options.go (3)
43-46: Remove stale reference to the deprecated interface
CommonHTTPServerOptionstill carries a commented out reference toStreamableHTTPOption.
Keeping obsolete crumbs around invites confusion for future maintainers and makesgodocoutput noisy. Simply delete the line.- // StreamableHTTPOption
55-59:streamableHTTPOptionlooks orphaned – verify necessity or eliminateThe wrapper type used to satisfy the (now removed)
StreamableHTTPOptioninterface is still present, but nothing in this file – or any referenced interface – requires theapplyToStreamableHTTPmethod any longer.If the new implementation in
streamable_http.gore-introduces an exportedStreamableHTTPOption(note the capital “S”), consider re-using that instead of keeping this unexported duplicate; otherwise delete this block to avoid dead code.-type streamableHTTPOption func(*StreamableHTTPServer) - -func (o streamableHTTPOption) isHTTPServerOption() {} -func (o streamableHTTPOption) applyToStreamableHTTP(s *StreamableHTTPServer) { o(s) }🧰 Tools
🪛 golangci-lint (1.64.8)
55-55: type
streamableHTTPOptionis unused(unused)
57-57: func
streamableHTTPOption.isHTTPServerOptionis unused(unused)
58-58: func
streamableHTTPOption.applyToStreamableHTTPis unused(unused)
147-170:WithBaseURLfails silently on invalid input – surface the errorThe current validation short-circuits on an invalid URL but does not inform the caller.
A mis-typed scheme or host will therefore quietly leave the server with an empty base URL.Returning an error (or at least logging one) greatly improves debuggability:
-func WithBaseURL(baseURL string) CommonHTTPServerOption { - return commonOption{ - apply: func(c httpTransportConfigurable) { +func WithBaseURL(baseURL string) CommonHTTPServerOption { + return commonOption{ + apply: func(c httpTransportConfigurable) { if baseURL != "" { u, err := url.Parse(baseURL) if err != nil { - return + // TODO: replace with your logger of choice + logger.Errorf("invalid baseURL %q: %v", baseURL, err) + return } if u.Scheme != "http" && u.Scheme != "https" { - return + logger.Errorf("baseURL %q must be http or https", baseURL) + return } if u.Host == "" || strings.HasPrefix(u.Host, ":") { - return + logger.Errorf("baseURL %q must contain a host", baseURL) + return } if len(u.Query()) > 0 { - return + logger.Errorf("baseURL %q must not contain query parameters", baseURL) + return } } c.setBaseURL(strings.TrimSuffix(baseURL, "/")) }, } }(Replace
loggerwith the minimal logger introduced inutil/logger.go.)
|
Is the idea that this replaces #228? |
Yes. Implemented similar things weeks ago. Compared to 228, it reflects a different interpretation of the spec (around SSE upgrading and notification routing), and offers a more customizable API (with reference to the JS version). So I believe it’s worth submitting as a PR. |
|
@robert-jackson-glean Regarding #253 http_transport_options.go, I think there are actually very few options that are shared with SSE. The only common ones are:
Since Streamable HTTP uses a single path, most other options related to HTTP routing and server setup aren’t necessary. The StreamableHTTPServer can simply be used as an http.Handler, and more advanced use cases can plug it into their own HTTP server or mux — with any framework they prefer. Introducing a unified interface may make things a little bit unnecessarily complex, and might be harder for end users to understand. Additionally, since SSE is now marked as deprecated in the official spec, combining them could increase the cost of introducing new options. So, I feel a separation of the options types would be simpler and more convenient. If that sounds good to you, I’ll simplify http_transport_options.go in a follow-up commit. |
Since SSE is deprecated in the current spec I would say simplifying |
|
@leavez I think it would make sense to also update the README and any SSE examples to use the new streamable http if possible as well. |
FWIW, these were the main ones that I was focusing on as being shared in that PR (also |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
examples/everything/main.go (1)
481-486: Transport implementation correctly switched to StreamableHTTPThe code now uses the new StreamableHTTPServer implementation instead of SSEServer. The changes are appropriate and the endpoint path ("/mcp") is correctly reflected in the log message.
One consideration: the new implementation doesn't pass any options to StreamableHTTPServer, relying on defaults. While this works, you might want to consider adding explicit configuration options like session token management or heartbeat in a future update to take full advantage of the new transport's capabilities.
server/streamable_http_test.go (3)
63-68: Avoid potential nil-pointer dereference in the basic instantiation testStaticcheck points out that
httpServer.serveris dereferenced even whenhttpServercould benil.
Combining the two checks (or usingrequire.NotNil) removes the false-positive and makes the intent clearer:- if httpServer == nil { - t.Error("SSEServer should not be nil") - } - if httpServer.server == nil { - t.Error("MCPServer should not be nil") - } + if assert := func() bool { return httpServer != nil && httpServer.server != nil }; !assert() { + t.Fatal("StreamableHTTPServer (or its inner MCP server) is nil") + }🧰 Tools
🪛 golangci-lint (1.64.8)
66-66: SA5011: possible nil pointer dereference
(staticcheck)
63-63: SA5011(related information): this check suggests that the pointer can be nil
(staticcheck)
185-187:Content-Typecomparison should ignore charset suffixGo’s
httppackage frequently appends a; charset=utf-8suffix, e.g.
application/json; charset=utf-8.Strict equality will therefore fail sporadically.
A safer assertion is to usestrings.HasPrefix:- if resp.Header.Get("content-type") != "application/json" { + if !strings.HasPrefix(resp.Header.Get("Content-Type"), "application/json") {(Do the analogous change for the
text/event-streamchecks.)Also applies to: 480-482
256-258: Use the semantichttp.StatusBadRequestconstantFor consistency with the rest of the file and to improve readability:
- if resp.StatusCode != 400 { + if resp.StatusCode != http.StatusBadRequest {Also applies to: 443-445
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)examples/custom_context/main.go(3 hunks)examples/everything/main.go(1 hunks)examples/simple_client/main.go(2 hunks)mcp/types.go(1 hunks)server/http_transport_options.go(0 hunks)server/sse.go(6 hunks)server/streamable_http.go(1 hunks)server/streamable_http_test.go(1 hunks)
💤 Files with no reviewable changes (1)
- server/http_transport_options.go
✅ Files skipped from review due to trivial changes (2)
- README.md
- mcp/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
- server/streamable_http.go
🧰 Additional context used
🧬 Code Graph Analysis (2)
examples/everything/main.go (3)
examples/custom_context/main.go (1)
NewMCPServer(104-123)server/server.go (1)
NewMCPServer(275-301)server/streamable_http.go (1)
NewStreamableHTTPServer(122-136)
examples/custom_context/main.go (2)
server/server.go (1)
MCPServer(125-150)server/streamable_http.go (3)
StreamableHTTPServer(107-119)NewStreamableHTTPServer(122-136)WithHTTPContextFunc(70-74)
🪛 golangci-lint (1.64.8)
server/streamable_http_test.go
46-46: Error return value of server.SendNotificationToClient is not checked
(errcheck)
66-66: SA5011: possible nil pointer dereference
(staticcheck)
63-63: SA5011(related information): this check suggests that the pointer can be nil
(staticcheck)
🔇 Additional comments (16)
examples/everything/main.go (1)
475-476: Updated flag descriptions correctly for new transport typeThe flag descriptions have been updated consistently to reflect the new transport option "http" instead of "sse", aligning with the introduction of the StreamableHTTP transport.
examples/simple_client/main.go (4)
20-20: Updated flag from SSE to HTTP appropriatelyThe command line flag has been correctly updated from "sse" to "http" to align with the new StreamableHTTP transport option.
24-26: Updated validation message for flag consistencyThe validation logic has been correctly updated to match the new flag name.
76-85: Client creation updated to use StreamableHTTP transportThe client creation logic has been properly updated to use the new StreamableHTTP transport instead of the previous SSE transport.
87-90: Added explicit client start callThe new code adds an explicit call to
c.Start(ctx)after client creation, which is important for the new transport architecture. This is a good change as it standardizes the client startup process regardless of transport type.examples/custom_context/main.go (4)
125-129: Replaced SSE with StreamableHTTP while preserving context functionalityThe implementation correctly replaces the SSE transport with the new StreamableHTTP transport while maintaining the important authorization context functionality via
WithHTTPContextFunc. This ensures that authentication extraction from HTTP headers continues to work with the new transport.
137-143: Updated flag descriptions for consistencyThe flag descriptions have been updated to reflect the new transport option "http" instead of "sse".
153-158: Updated transport handling in switch statementThe switch case has been correctly updated from "sse" to "http", and now creates and starts a StreamableHTTP server instead. The error handling is maintained, ensuring robustness.
160-163: Updated error message for invalid transport typeThe error message has been updated to reflect the valid transport options.
server/sse.go (7)
39-45: Added dynamic base path supportThe new
DynamicBasePathFunctype is a good addition that allows for runtime determination of base paths. This enhances flexibility for SSE server configuration, particularly in complex routing scenarios or multi-tenant applications where paths might include dynamic segments.
103-104: Renamed contextFunc for clarityRenaming the field from
HTTPContextFunctoSSEContextFuncimproves clarity and prevents confusion with the similar function in the new StreamableHTTP transport.
112-136: Implemented functional options pattern for SSE server configurationThe changes adopt the functional options pattern for configuring the SSE server, which is a modern Go best practice. This makes the API more flexible, extensible, and maintainable. The implementation of
WithBaseURLincludes proper validation of the URL structure.
138-143: Added WithStaticBasePath option functionThe new option function for setting a static base path enhances configuration clarity, especially in conjunction with the deprecated method notice for the older function.
154-167: Added support for dynamic base pathsThe
WithDynamicBasePathfunction provides an elegant way to configure dynamic base path generation, which is particularly useful in advanced routing scenarios. The implementation properly normalizes the path for consistency.
203-229: Migrated remaining options to functional patternThe remaining configuration options (HTTP server, keep-alive settings, context functions) have been properly migrated to the functional options pattern, maintaining a consistent API design.
242-245: Updated option application in constructorThe NewSSEServer constructor now applies options directly using the functional pattern, which simplifies the code and makes it more maintainable.
Have we decided to move forward with this one. If this is the case, I won't waste time on #228. Happy to collaborate on this feature and contribute in anyway. This feature is very much needed. Would like to push it forward as soon as possible. |
pottekkat
added
type: enhancement
If everyone is happy to move ahead with this version, I vote we do that as it seems to be the most complete and it's a highly requested feature. |
|
The streamable HTTP transport specification is somewhat ambiguous. The stdio transport operates over a full-duplex, bi-directional long-lived connection — a straightforward model where both request and response can flow freely. This is the baseline on which the spec is built. The old SSE transport is very straightforward compared the local communication. It omit the client-to-server response channel, which is rarely used, and other things keep the same. However, the streamable HTTP transport introduces complexity. When the client sets up an optional long-lived GET connection for listening, the behavior is logically the same to SSE. (For the real transportation, response will transmit through the request http connection. But logically it’s the same). But when that long-lived GET listener is not enabled, things getting wired. According to the spec, the request http connection could be upgraded to SSE, and it should be terminated when the corresponding response returned. So the SSE connection persists only for the duration of the response handling. When there's no on-going request, i.e. no real connections, the server notifications (e.g., tool-list-changed) cannot even be sent. Attempting to queue such notifications and send them over a future request connection seems impractical for the client business. While the spec mentions resumability and redelivery, that section is intended for broken or interrupted connections — I don't think it's our scenario. Furthermore, the idea of an “upgradable stream” for a response — which by definition must eventually terminate — is confusing. If the stream is treated as a general-purpose notification pipe, it burdens the server with streams avaiable for unpredictable durations. If not, then the server can only use it for request-scoped updates (e.g., progress updates related to that specific request). In my implementation, I treat this pragmatically:
I think the TypeScript version implemented it this way too, though I’m not very familiar with TypeScript — I might have misunderstood. I’d appreciate it very much if the reviewers could help verify my understanding of the spec. Even though the protocol allows server-to-client requests, I’ve yet to see this used in the wild. I tested Cursor and Claude Desktop in March, neither even supported notifications very well. Although HTTP spec may be confusing, I think, in practice most user only needs the simplest one-shot request-response interactions. Anyway, HTTP transport is a highly requested feature, and I’d love to see it supported in the project, in any form, ASAP. |
|
For the reasons mentioned above, certain mcp-go features, such as the OnRegisterSession hook, may not work as originally designed. Currently, the hook is only triggered when a long-lived GET connection is established. I believe it’s possible to support this by introducing a logical session concept, but doing so would significantly increase the complexity of this PR and make it harder to merge. This PR is not full-featured for the spec, but I want make it more solid for what added, and keeping it small to facilitate review and merging. I’d love to see others join in and shape this feature together. |
|
I will have a look at this tomorrow. |
|
nice work |
Streamable HTTP transport for the server
Beside the spec, this implementation supports:
The current implementation does not support the following features from the specification:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores