From 81a8ece2ddcaf8b897b00abe5a51035aebc57853 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Tue, 13 May 2025 12:23:19 -0400 Subject: [PATCH] internal/mcp: resource tweaks Get the name and signature of the handler right. Change-Id: I743aaaee45cc07a741ab8ad16e90e1b1d5519b64 Reviewed-on: https://go-review.googlesource.com/c/tools/+/672336 LUCI-TryBot-Result: Go LUCI Reviewed-by: Sam Thanawalla Reviewed-by: Robert Findley --- design/design.md | 71 +++++++++++++++++++++++++++--------------------- mcp/mcp_test.go | 6 ++-- mcp/server.go | 12 ++++---- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/design/design.md b/design/design.md index d4e7c9f..1ecd2c2 100644 --- a/design/design.md +++ b/design/design.md @@ -875,52 +875,61 @@ handler to a Go function using reflection to derive its arguments. We provide ### Resources and resource templates -To add a resource or resource template to a server, users call the `AddResource` and -`AddResourceTemplate` methods, passing the resource or template and a function for reading it: - +In our design, each resource and resource template is associated with a function that reads it, +with this signature: ```go -type ReadResourceHandler func(context.Context, *ServerSession, *Resource, *ReadResourceParams) (*ReadResourceResult, error) +type ResourceHandler func(context.Context, *ServerSession, *ReadResourceParams) (*ReadResourceResult, error) +``` +The arguments include the `ServerSession` so the handler can observe the client's roots. +The handler should return the resource contents in a `ReadResourceResult`, calling either `NewTextResourceContents` +or `NewBlobResourceContents`. If the handler omits the URI or MIME type, the server will populate them from the +resource. -func (*Server) AddResource(*Resource, ReadResourceHandler) -func (*Server) AddResourceTemplate(*ResourceTemplate, ReadResourceHandler) +The `ServerResource` and `ServerResourceTemplate` types hold the association between the resource and its handler: +```go +type ServerResource struct { + Resource Resource + Handler ResourceHandler +} + +type ServerResourceTemplate struct { + Template ResourceTemplate + Handler ResourceHandler +} ``` -The `Resource` is passed to the reader function even though it is redundant (the function could have closed over it) -so a single handler can support multiple resources. -If the incoming resource matches a template, a `Resource` argument is constructed -from the fields in the `ResourceTemplate`. -The `ServerSession` argument is there so the reader can observe the client's roots. - -To read files from the local filesystem, we recommend using `FileReadResourceHandler` to construct a handler: +To add a resource or resource template to a server, users call the `AddResources` and +`AddResourceTemplates` methods with one or more `ServerResource`s or `ServerResourceTemplate`s: ```go -// FileReadResourceHandler returns a ReadResourceHandler that reads paths using dir as a root directory. +func (*Server) AddResources(...*ServerResource) +func (*Server) AddResourceTemplates(...*ServerResourceTemplate) + +func (s *Server) RemoveResources(uris ...string) +func (s *Server) RemoveResourceTemplates(uriTemplates ...string) +``` + +The `ReadResource` method finds a resource or resource template matching the argument URI and calls +its assocated handler. +If the argument URI matches a template, the `Resource` argument to the handler is constructed +from the fields in the `ResourceTemplate`. + +To read files from the local filesystem, we recommend using `FileResourceHandler` to construct a handler: +```go +// FileResourceHandler returns a ResourceHandler that reads paths using dir as a root directory. // It protects against path traversal attacks. // It will not read any file that is not in the root set of the client requesting the resource. -func (*Server) FileReadResourceHandler(dir string) ReadResourceHandler +func (*Server) FileResourceHandler(dir string) ResourceHandler ``` - -It guards against [path traversal attacks](https://go.dev/blog/osroot) -and observes the client's roots. Here is an example: ```go // Safely read "/public/puppies.txt". -s.AddResource( - &mcp.Resource{URI: "file:///puppies.txt"}, - s.FileReadResourceHandler("/public")) +s.AddResources(&mcp.ServerResource{ + Resource: mcp.Resource{URI: "file:///puppies.txt"}, + Handler: s.FileReadResourceHandler("/public")}) ``` -There are also server methods to remove resources and resource templates. - -```go -func (*Server) RemoveResources(uris ...string) -func (*Server) RemoveResourceTemplates(names ...string) -``` - -Resource templates don't have unique identifiers, so removing a name will remove all -resource templates with that name. - Servers support all of the resource-related spec methods: - `ListResources` and `ListResourceTemplates` for listings. diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index b487483..c15d2f1 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -198,15 +198,15 @@ func TestEndToEnd(t *testing.T) { URI: "file:///nonexistent.txt", } - readHandler := func(_ context.Context, r *Resource, _ *ReadResourceParams) (*ReadResourceResult, error) { - if r.URI == "file:///file1.txt" { + readHandler := func(_ context.Context, _ *ServerConnection, p *ReadResourceParams) (*ReadResourceResult, error) { + if p.URI == "file:///file1.txt" { return &ReadResourceResult{ Contents: &ResourceContents{ Text: "file contents", }, }, nil } - return nil, ResourceNotFoundError(r.URI) + return nil, ResourceNotFoundError(p.URI) } s.AddResources( &ServerResource{resource1, readHandler}, diff --git a/mcp/server.go b/mcp/server.go index 18d4d09..dd29495 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -119,18 +119,18 @@ func ResourceNotFoundError(uri string) error { // The immediate problem is that jsonprc2 defines -32002 as "server closing". const codeResourceNotFound = -31002 -// A ReadResourceHandler is a function that reads a resource. +// A ResourceHandler is a function that reads a resource. // If it cannot find the resource, it should return the result of calling [ResourceNotFoundError]. -type ReadResourceHandler func(context.Context, *Resource, *ReadResourceParams) (*ReadResourceResult, error) +type ResourceHandler func(context.Context, *ServerConnection, *ReadResourceParams) (*ReadResourceResult, error) // A ServerResource associates a Resource with its handler. type ServerResource struct { Resource *Resource - Handler ReadResourceHandler + Handler ResourceHandler } // AddResource adds the given resource to the server and associates it with -// a [ReadResourceHandler], which will be called when the client calls [ClientSession.ReadResource]. +// a [ResourceHandler], which will be called when the client calls [ClientSession.ReadResource]. // If a resource with the same URI already exists, this one replaces it. // AddResource panics if a resource URI is invalid or not absolute (has an empty scheme). func (s *Server) AddResources(resources ...*ServerResource) { @@ -217,7 +217,7 @@ func (s *Server) listResources(_ context.Context, _ *ServerConnection, params *L return res, nil } -func (s *Server) readResource(ctx context.Context, _ *ServerConnection, params *ReadResourceParams) (*ReadResourceResult, error) { +func (s *Server) readResource(ctx context.Context, ss *ServerConnection, params *ReadResourceParams) (*ReadResourceResult, error) { uri := params.URI // Look up the resource URI in the list we have. // This is a security check as well as an information lookup. @@ -229,7 +229,7 @@ func (s *Server) readResource(ctx context.Context, _ *ServerConnection, params * // Treat an unregistered resource the same as a registered one that couldn't be found. return nil, ResourceNotFoundError(uri) } - res, err := resource.Handler(ctx, resource.Resource, params) + res, err := resource.Handler(ctx, ss, params) if err != nil { return nil, err }