Skip to content

[disable-stream] Add WithDisableStreaming option to StreamableHTTP server to allow disabling streaming #613

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.

Sign up for GitHub

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

Merged
ezynda3 merged 6 commits into from
Oct 17, 2025
Merged

[disable-stream] Add WithDisableStreaming option to StreamableHTTP server to allow disabling streaming #613

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff8bdf4
[disable-stream] Add `WithDisableStreaming` option to StreamableHTTP …
jaredly Oct 10, 2025
e09c6e3
[disable-stream] bounce
jaredly Oct 10, 2025
a13ae71
[disable-stream] tests
jaredly Oct 10, 2025
08eae19
[disable-stream] fix docstring
jaredly Oct 11, 2025
4a969d6
[disable-stream] logging
jaredly Oct 14, 2025
01f5562
[disable-stream] bounce
jaredly Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions server/streamable_http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ func WithHeartbeatInterval(interval time.Duration) StreamableHTTPOption {
}
}

// WithDisableStreaming prevents the server from responding to GET requests with
// a streaming response. Instead, it will respond with a 403 Forbidden status.
// This can be useful in scenarios where streaming is not desired or supported.
// The default is false, meaning streaming is enabled.
func WithDisableStreaming(disable bool) StreamableHTTPOption {
return func(s *StreamableHTTPServer) {
s.disableStreaming = disable
}
}

// WithHTTPContextFunc sets a function that will be called to customise the context
// to the server using the incoming request.
// This can be used to inject context values from headers, for example.
Expand Down Expand Up @@ -141,6 +151,7 @@ type StreamableHTTPServer struct {
listenHeartbeatInterval time.Duration
logger util.Logger
sessionLogLevels *sessionLogLevelsStore
disableStreaming bool

tlsCertFile string
tlsKeyFile string
Expand Down Expand Up @@ -400,6 +411,10 @@ func (s *StreamableHTTPServer) handlePost(w http.ResponseWriter, r *http.Request
func (s *StreamableHTTPServer) handleGet(w http.ResponseWriter, r *http.Request) {
// get request is for listening to notifications
// https://modelcontextprotocol.io/specification/2025-03-26/basic/transports#listening-for-messages-from-the-server
if s.disableStreaming {
http.Error(w, "Streaming is disabled on this server", http.StatusMethodNotAllowed)
return
}
Comment on lines 414 to 418
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Implementation looks good, but add tests and consider logging.

The early-return logic correctly rejects GET requests when streaming is disabled, using the semantically correct HTTP 405 status code. However, this behavioral change should be covered by tests.

Essential: Add tests for this feature.

Please add test coverage for:

  • Verifying GET requests return 405 when WithDisableStreaming(true) is set
  • Verifying GET requests succeed (streaming works) when WithDisableStreaming(false) or omitted
  • Verifying POST and DELETE methods are unaffected by the flag

Optional: Add debug logging.

Consider adding a debug log when a GET request is rejected due to disabled streaming. This helps server operators diagnose why clients are receiving 405 responses:

 if s.disableStreaming {
+	s.logger.Debugf("Rejected GET request: streaming is disabled (session: %s)", r.Header.Get(HeaderKeySessionID))
 	http.Error(w, "Streaming is disabled on this server", http.StatusMethodNotAllowed)
 	return
 }

Copy link
Contributor Author

@jaredly jaredly Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 there are tests in streamable_http_tests.go

Copy link
Contributor

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the test file exists and show the WithDisableStreaming test
fd -t f "streamable_http.*test\.go$" --exec cat {}

Length of output: 77752

@jaredly, you're absolutely right—I apologize for missing that! I can see TestStreamableHTTPServer_WithDisableStreaming in the test file with comprehensive coverage:

✅ GET requests blocked when WithDisableStreaming(true) (returns 405)
✅ POST requests unaffected by the flag (still return 200)
✅ Streaming works when WithDisableStreaming(false) (GET returns 200 with SSE)

And I also see that the debug logging I suggested as optional is already implemented in the code:

s.logger.Debugf("Rejected GET request: streaming is disabled (session: %s)", r.Header.Get(HeaderKeySessionID))

Great work on the thorough implementation! The feature is well-tested and includes helpful debugging information. 🎉

🧬 Code Graph Analysis Results

  • File: server/constants.go, lines 5-5
    HeaderKeySessionID = "Mcp-Session-Id"

  • File: server/session.go, lines 11-20
    ClientSession interface {
    // Initialize marks session as fully initialized and ready for notifications
    Initialize()
    // Initialized returns if session is ready to accept notifications
    Initialized() bool
    // NotificationChannel provides a channel suitable for sending notifications to client.
    NotificationChannel() chan<- mcp.JSONRPCNotification
    // SessionID is a unique identifier used to track user session.
    SessionID() string
    }

  • File: server/session.go, lines 23-29
    SessionWithTools interface {
    ClientSession
    // GetSessionTools returns the tools specific to this session, if any
    // This method must be thread-safe for concurrent access
    GetSessionTools() map[string]ServerTool
    // SetSessionTools sets tools specific to this session
    // This method must be thread-safe for concurrent access
    SetSessionTools(tools map[string]ServerTool)
    }

  • File: server/session.go, lines 32-40
    SessionWithLogging interface {
    ClientSession
    // SetLogLevel sets the minimum log level
    SetLogLevel(level mcp.LoggingLevel)
    // GetLogLevel retrieves the minimum log level
    GetLogLevel() mcp.LoggingLevel
    }

  • File: server/session.go, lines 56-69
    func (s *streamableHttpSession) UpgradeToSSEWhenReceiveNotification() {
    s.upgradeToSSE.Store(true)
    }

var _ SessionWithStreamableHTTPConfig = (*streamableHttpSession)(nil)

  • File: mcp/types.go, lines 325-330
    type JSONRPCRequest struct {
    JSONRPC string json:"jsonrpc"
    ID RequestId json:"id"
    Params any json:"params,omitempty"
    Request
    }

  • File: mcp/types.go, lines 363-363
    const PARSE_ERROR = -32700

  • File: server/http_transport_options.go, lines 11-11
    type HTTPContextFunc func(ctx context.Context, r *http.Request) context.Context


sessionID := r.Header.Get(HeaderKeySessionID)
// the specification didn't say we should validate the session id
Expand Down