forked from modelcontextprotocol/go-sdk
-
Notifications
You must be signed in to change notification settings - Fork 0
Streamable #4
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
Draft
BC-ACherednichenko
wants to merge
38
commits into
main
Choose a base branch
from
streamable
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Streamable #4
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
Update inaccurate doc comment for ClientSession.
…#236) - Correct MethodHandler type parameter to use fully qualified mcp.ServerSession - Update parameter types to use proper mcp.Params and mcp.Result instead of any - Ensure example code matches the actual API
This CL makes the following changes to our Github workflows, motivated by modelcontextprotocol#227: - Add -race tests. - Add go1.25rc2 to the build matrix. - Check out code before setting up Go, so that the setup step can use dependency caching.
As reported in modelcontextprotocol#227, there was a race where streams could be terminated before the final reply was written. Fix this by checking the number of outstanding requests in the same critical section that acquires outgoing messages to write. Fixes modelcontextprotocol#227
Test that explicit nulls don't cause panics.
Add ForType, which is like For but takes a reflect.Type. Fixes modelcontextprotocol#233.
Use a read loop and incoming channel so that we can select on incoming messages and unblock calls of ioConn.Read as soon as Close is called. This avoids the scenario of modelcontextprotocol#224, where a close of the StdioTransport does not gracefully shut down the JSON-RPC connection. Fixes modelcontextprotocol#224
…#179) (modelcontextprotocol#192) You can make MCP server log error, send response and then abruptly exit, with a json input malformed at the end. While using json.RawMessage, once a valid first json is decoded, trailing bad input is silently ignored. Without graciously handling this input, mcp server is currently sending response as well as encountering error. It should just report error without further processing of request. Fixes modelcontextprotocol#179.
Marshal the empty schema to true, and its negation to false. This is technically a breaking behavior change, but a properly written consumer of JSON Schema will not notice. For modelcontextprotocol#244. Fixes modelcontextprotocol#230.
Add a client example that lists all features on a stdio server. Since this is our first client example, organize examples by client / server -- otherwise it is too difficult to find client examples. For modelcontextprotocol#33
…rotocol#251) This CL effectively removes any dependencies on mcp from jsonschema. This will enable us to move the repo. This CL also deletes unused marshalStructWithMap and unmarshalStructWithMap which had a dependency on FieldJSONInfo. Fixes: modelcontextprotocol#250
Make the simplifications described in modelcontextprotocol#222. I don't know why we originally had such fine granularity of locking: perhaps we were going to execute 'oninitialized' callbacks. Also update the TODO with a bit more information about jsonrpc2 behavior. Fixes modelcontextprotocol#222
Update ServerSession so that the session is not marked as initialized until 'notifications/initialized' is actually received from the client. Include a new test of this lifecycle strictness. Fixes modelcontextprotocol#225
…modelcontextprotocol#262) This CL prunes the jsonschema directory and updates the go.mod. For modelcontextprotocol#28
Add unexported methods to the Params and Result interface, so that they're harder to implement outside the mcp package. It looks like these are the only two interfaces we need to lock down: others are either intentionally open (Transport, Connection), or already closed (Session). Fixes modelcontextprotocol#263
…tocol#261) Add auth.RequireBearerToken and associated types. This piece of middleware authenticates clients using OAuth 2.0, as specified in the MCP spec. For modelcontextprotocol#237. Usage: ``` st := mcp.NewStreamableServerTransport(...) http.Handle(path, auth.RequireBearerToken(verifier, nil)(st.ServeHTTP)) ```
This CL introduces a fundamental change to the jsonrpc2 library: connection writes, which were previously serialized by the jsonrpc2 library itself, are now allowed to be concurrent. The change in semantics of the jsonrpc2 library should hopefully be easy to review, since moving the synchronization to the Writer implementation is equivalent to the previous logic. However, this change is critical for the streamable client transport, because it allows for concurrent http requests to the server. Consider that a write is a POST to the server, and we don't know that write succeeded until we get the response header. Previously, we had the following problem: if the client POSTs a request, and the server blocks its response on a request made through the hanging GET, the client was unable to respond because the initial POST is still blocked. We could update our streamable server transport to force a flush of the response headers, but we can't guarantee that other servers behave the same way. Fundamentally, writes in the spec are asynchronous, and we need to support that.
Make several cleanups of the streamable client transport, encountered during work on JSON support for the streamable server: - The 'Close' condition is differentiated from asynchronous failures. A failure should unblock Read with an error, at which point the JSON-RPC connection will be broken and closed. - Fields are reordered in streamableClientConn to make guards more apparent. - The handling of sessionID is simplified: we simply set the session ID whenever we receive response headers. No need to have special handling for the first request, as the serializeation of session initialization is implemented in Client.Connect. - Since the above bullet makes Write a trivial wrapper around postMessage, the two methods are merged. - A bug is fixed where JSON responses were handled synchronously in Write. This lead to deadlock when a hanging client->server request is waiting on a server->client request. Now JSON is handled symmetrically to SSE: the Write returns once response headers are received. asynchronous to Write. - The httpConnection interface is renamed to clientConnection, and receive the entire InitializeResult. - streamableClientConn receivers are renamed to be consistently 'c'.
Add a new (currently unexported) jsonResponse option to StreamableServerTransportOptions, and use it to control the response content type, serving application/json if set. Additionally: - A transportOptions field is added to the StreamableHTTPOptions. - A messages iterator is added to encapsulate the iteration of stream messages, since the handling of JSON and SSE responses are otherwise very different. - The serving flow is refactored to avoid returning (statusCode, message), primarily because this seemed liable to lead to redundant calls to WriteHeader, because only local logic knows whether or not any data has been written to the response. - The serving flow is refactored to delegate to responseJSON and responseSSE, according to the currently unexported jsonResponse option. - A bug is fixed where all GET streams were considered persistent: the terminal condition req.Method == http.MethodPost && nOutstanding == 0 was not right: GET requests may implement stream resumption. Updates modelcontextprotocol#211
Now that rc3 is out, update our 1.25 builds to use it.
…ocol#270) For some reason, when making these marker methods, the standard naming convention escaped me.
Make it clear that it's not needed for an HTTP server.
In jsonprc2_test.go, binder.Bind starts a goroutine. That goroutine begins to run during jsonrpc2.bindConnection, and can race with the setting of conn.write in bindConnection. This PR adds a sleep, which is a poor way to deal with the race, but the least invasive change. Better ones include running the test function after Dial returns, or adding a Connection.Ready method to detect when initialization is complete.
Add `.gitignore` for some common files.
…tprotocol#248) I inadvertently merged modelcontextprotocol#234 without pushing my latest patches. This CL adds the missing changes.
Support stateless streamable sessions by adding a GetSessionID function to StreamableHTTPOptions. If GetSessionID returns "", the session is stateless, and no validation is performed. This is implemented by providing the session a trivial initialization state. To implement this, some parts of modelcontextprotocol#232 (distributed sessions) are copied over, since they add an API for creating an already-initialized session. In total, the following new API is added: - StreamableHTTPOptions.GetSessionID - ServerSessionOptions (a new parameter to Server.Connect) - ServerSessionState - ClientSessionOptions (a new parameter to Client.Connect, for symmetry) For modelcontextprotocol#10
pull_request_template: update template to reflect correct form
As described in modelcontextprotocol#272, there is really no reason for transports to be closed structs with constructors, since their state must be established by a call to Connect. Making them open structs simplifies their APIs, and means that all transports can be extended in the future: we don't have to create empty Options structs just for the purpose of future compatibility. For now, the related constructors and options structs are simply deprecated (with go:fix directives where possible). A future CL will remove them prior to the v1.0.0 release. For modelcontextprotocol#272
Handlers and client/server methods take a single request argument combining the session and params. This slightly simplifies the signature for handlers, since there is no longer an often-ignored session argument. But more importantly, it opens the door to adding more information in requests, such as auth info and HTTP request headers. For modelcontextprotocol#243.
Add a test that attempts (and fails) to reproduce the bug reported in issue modelcontextprotocol#285. For modelcontextprotocol#285
Allow other header values for the 'Accept' header that imply application/json and text/event-stream for requests to the Streamable HTTP Transport. Fixes modelcontextprotocol#290 Unit tests here seemed like unnecessary complexity and wouldn't fit cleanly with the style of the `streamable_test.go` tests. Happy to add those if desired though.
In modelcontextprotocol#202, I added the checkRequest helper to validate incoming requests, and invoked it in the stremable transports to preemptively reject invalid HTTP requests, so that a jsonrpc error could be translated to an HTTP error. However, this introduced a bug: since cancellation was handled in the jsonrpc2 layer, we never had to validate it in the mcp layer, and therefore never added methodInfo. As a result, it was reported as an invalid request in the http layer. Add a test, and a fix. The simplest fix was to create stubs that are placeholders for cancellation. This was discovered in the course of investigating modelcontextprotocol#285.
This CL causes the dup to fail.
Introduces a new test file content_nil_test.go which verifies that UnmarshalJSON methods for various Content types do not panic when unmarshaling onto nil pointers. Adds a nil check in contentFromWire function to guard against a nil wire.Content parameter. Tests cover different scenarios, including valid and invalid content types, as well as cases with empty or missing content fields. For modelcontextprotocol#205
Add a test following up on the fix for modelcontextprotocol#290.
The next CL will test stateless and distributable server transport configurations, using the HTTP testing strategy of TestStreamableServerTransport.
WIP: needs self review and more tests. Several improvements for the stateless streamable mode, plus support for a 'distributed' (or rather, distributable) version of the stateless server. - Add a 'Stateless' option to StreamableHTTPOptions and StreamableServerTransport, which controls stateless behavior. GetSessionID may still return a non-empty session ID. - Audit validation of stateless mode to allow requests with a session id. Propagate this session ID to the temporary connection. - Peek at requests to allow 'initialize' requests to go through to the session, so that version negotiation can occur (FIXME: add tests). Fixes modelcontextprotocol#284 For modelcontextprotocol#148
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.
PR Tips
Typically, PRs should consist of a single commit, and so should generally follow
the rules for Go commit messages, with the following
changes and additions:
Markdown is allowed.
For a pervasive change, use "all" in the title instead of a package name.
The PR description should provide context (why this change?) and describe the changes
at a high level. Changes that are obvious from the diffs don't need to be mentioned.