diff --git a/mcp/client.go b/mcp/client.go index d1d17502..fdf05417 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -226,7 +226,6 @@ func (c *Client) AddRoots(roots ...*Root) { // RemoveRoots removes the roots with the given URIs, // and notifies any connected servers if the list has changed. // It is not an error to remove a nonexistent root. -// TODO: notification func (c *Client) RemoveRoots(uris ...string) { changeAndNotify(c, notificationRootsListChanged, &RootsListChangedParams{}, func() bool { return c.roots.remove(uris...) }) diff --git a/mcp/features.go b/mcp/features.go index 43c58854..438370fe 100644 --- a/mcp/features.go +++ b/mcp/features.go @@ -17,7 +17,9 @@ import ( // A featureSet is a collection of features of type T. // Every feature has a unique ID, and the spec never mentions // an ordering for the List calls, so what it calls a "list" is actually a set. -// TODO: switch to an ordered map +// +// An alternative implementation would use an ordered map, but that's probably +// not necessary as adds and removes are rare, and usually batched. type featureSet[T any] struct { uniqueID func(T) string features map[string]T diff --git a/mcp/server.go b/mcp/server.go index c3fbd9e3..84e34c27 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -89,17 +89,18 @@ type ServerOptions struct { // The first argument must not be nil. // // If non-nil, the provided options are used to configure the server. -func NewServer(impl *Implementation, opts *ServerOptions) *Server { +func NewServer(impl *Implementation, options *ServerOptions) *Server { if impl == nil { panic("nil Implementation") } - if opts == nil { - opts = new(ServerOptions) + var opts ServerOptions + if options != nil { + opts = *options } + options = nil // prevent reuse if opts.PageSize < 0 { panic(fmt.Errorf("invalid page size %d", opts.PageSize)) } - // TODO(jba): don't modify opts, modify Server.opts. if opts.PageSize == 0 { opts.PageSize = DefaultPageSize } @@ -111,7 +112,7 @@ func NewServer(impl *Implementation, opts *ServerOptions) *Server { } return &Server{ impl: impl, - opts: *opts, + opts: opts, prompts: newFeatureSet(func(p *serverPrompt) string { return p.prompt.Name }), tools: newFeatureSet(func(t *serverTool) string { return t.tool.Name }), resources: newFeatureSet(func(r *serverResource) string { return r.resource.URI }), @@ -455,7 +456,7 @@ func fileResourceHandler(dir string) ResourceHandler { return func(ctx context.Context, req *ServerRequest[*ReadResourceParams]) (_ *ReadResourceResult, err error) { defer util.Wrapf(&err, "reading resource %s", req.Params.URI) - // TODO: use a memoizing API here. + // TODO(#25): use a memoizing API here. rootRes, err := req.Session.ListRoots(ctx, nil) if err != nil { return nil, fmt.Errorf("listing roots: %w", err) diff --git a/mcp/shared.go b/mcp/shared.go index 608e2aaf..6086c745 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -4,8 +4,10 @@ // This file contains code shared between client and server, including // method handler and middleware definitions. -// TODO: much of this is here so that we can factor out commonalities using -// generics. Perhaps it can be simplified with reflection. +// +// Much of this is here so that we can factor out commonalities using +// generics. If this becomes unwieldy, it can perhaps be simplified with +// reflection. package mcp diff --git a/mcp/streamable.go b/mcp/streamable.go index 526ee515..130ae07f 100644 --- a/mcp/streamable.go +++ b/mcp/streamable.go @@ -1053,7 +1053,6 @@ func (c *streamableClientConn) Write(ctx context.Context, msg jsonrpc.Message) e } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - // TODO: do a best effort read of the body here, and format it in the error. resp.Body.Close() return fmt.Errorf("broken session: %v", resp.Status) }