-
Notifications
You must be signed in to change notification settings - Fork 93
Add MCP Streamable HTTP specification support for the client #210
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?
Add MCP Streamable HTTP specification support for the client #210
Conversation
| if response.code == "200" | ||
| logger.info("SSE stream connected successfully") | ||
|
|
||
| response.read_body do |chunk| |
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.
Let's use https://rubygems.org/gems/event_stream_parser for parsing. SSE appears easy to parse but it has weird edge cases.
Thanks for kicking off the effort! I've been planning on porting this standalone client I had written for a project, but never got around to doing it: https://gist.github.com/atesgoral/75172912b5951d9be33497b80aba4397
You can see how event_stream_parser can be used for parsing chunks as they come in.
|
@atesgoral Thanks for reviewing this! I've addressed it. |
|
@atesgoral Any chance you could take a look at this PR? |
| Net::HTTP.start(uri.host, uri.port) do |http| | ||
| request = Net::HTTP::Get.new(uri) | ||
| request["Mcp-Session-Id"] = session_id | ||
| request["MCP-Session-Id"] = session_id |
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.
This change doesn't appear to be intentional.
| logger = create_logger | ||
|
|
||
| puts <<~MESSAGE | ||
| MCP Streamable HTTP Client (SDK + SSE) |
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.
The mention of the "SDK" seems redundant.
| puts "Session ID: #{client.session_id}" | ||
| puts "Protocol Version: #{client.protocol_version}" | ||
| puts "Server Info: #{init_response.dig("result", "serverInfo")}" |
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.
| puts "Session ID: #{client.session_id}" | |
| puts "Protocol Version: #{client.protocol_version}" | |
| puts "Server Info: #{init_response.dig("result", "serverInfo")}" | |
| puts <<~MESSAGE | |
| Session ID: #{client.session_id} | |
| Protocol Version: #{client.protocol_version} | |
| Server Info: #{init_response.dig("result", "serverInfo")} | |
| MESSAGE |
| Choose an action:#{" "} | ||
| MENU | ||
|
|
||
| choice = gets&.chomp |
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.
The safe navigation operator doesn't seem necessary. Is there any case where this could be nil?
| choice = gets&.chomp | |
| choice = gets.chomp |
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.
Can you look over the other similar parts as well?
| logger.error(e.backtrace.first(5).join("\n")) | ||
| ensure | ||
| # Clean up SSE thread | ||
| sse_thread&.kill if sse_thread&.alive? |
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.
At least if sse_thread is alive, the receiver of kill should exist. Is safe navigation necessary in each case?
| class Client | ||
| # https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#streamable-http | ||
| LATEST_PROTOCOL_VERSION = "2025-11-25" | ||
| SESSION_ID_HEADER = "MCP-Session-Id" |
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.
It may be better to make the notation consistent.
| SESSION_ID_HEADER = "MCP-Session-Id" | |
| SESSION_ID_HEADER = "Mcp-Session-Id" |
| module MCP | ||
| class Server | ||
| module Transports | ||
| # TODO: https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#resumability-and-redelivery |
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.
It would be helpful to describe what is expected to be done in this TODO.
| REQUIRED_GET_ACCEPT_TYPES = ["text/event-stream"].freeze | ||
|
|
||
| def handle_request(request) | ||
| # TODO: https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#security-warning |
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.
Ditto.
Motivation and Context
Implements the MCP Streamable HTTP specification for the Ruby SDK client.
Already supported:
Accept: application/json, text/event-streamheaderapplication/jsonresponseThis PR adds:
text/event-stream(SSE) responseMCP-Session-Idheader)MCP-Protocol-Version)How Has This Been Tested?
Terminal 1 - Server
Terminal 2 - Client
Breaking Changes
None. The public
send_request(request:)method signature is preserved for backward compatibility.Types of changes
Checklist
Additional context
API Design
MCP::Client::HTTP#send_request(request:)- Returns body only (backward compatible)MCP::Client::HTTP#post(body:, headers:)- ReturnsResponsestruct with body + headers (for session management)MCP::Client::HTTP#delete(headers:)- For session terminationMCP::Client#connect(client_info:, protocol_version:, capabilities:)- Initialization handshakeMCP::Client#close- Session terminationPending TODOs
Last-Event-IDheader