[tools] internal/mcp: add notifications
11 views
Skip to first unread message
Jonathan Amsterdam (Gerrit)
May 17, 2025, 4:04:34 PMMay 17
to Robert Findley, Sam Thanawalla, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam has uploaded the change for review![]( "Open in Gerrit")
Jonathan Amsterdam would like Robert Findley and Sam Thanawalla to review this change.
Commit message
internal/mcp: add notifications
Add notification logic for intialization and list changes.
Change-Id: I1d392bb49d5995f2046c400da955a033ab2715ce
Change diff
diff --git a/internal/mcp/client.go b/internal/mcp/client.go
index 06772b9..494a983 100644
--- a/internal/mcp/client.go
+++ b/internal/mcp/client.go
@@ -47,9 +47,14 @@
// ClientOptions configures the behavior of the client.
type ClientOptions struct {
+ //
// Handler for sampling.
// Called when a server calls CreateMessage.
CreateMessageHandler func(context.Context, *ClientSession, *CreateMessageParams) (*CreateMessageResult, error)
+ // Handlers for notifications from the server.
+ ToolListChangedHandler func(context.Context, *ToolListChangedParams)
+ PromptListChangedHandler func(context.Context, *PromptListChangedParams)
+ ResourceListChangedHandler func(context.Context, *ResourceListChangedParams)
}
// bind implements the binder[*ClientSession] interface, so that Clients can
@@ -134,11 +139,9 @@
// AddRoots adds the given roots to the client,
// replacing any with the same URIs,
// and notifies any connected servers.
-// TODO: notification
func (c *Client) AddRoots(roots ...*Root) {
- c.mu.Lock()
- defer c.mu.Unlock()
- c.roots.add(roots...)
+ c.changeFeature(notificationRootsListChanged, &RootsListChangedParams{},
+ func() bool { c.roots.add(roots...); return true })
}
// RemoveRoots removes the roots with the given URIs,
@@ -146,9 +149,22 @@
// It is not an error to remove a nonexistent root.
// TODO: notification
func (c *Client) RemoveRoots(uris ...string) {
+ c.changeFeature(notificationRootsListChanged, &RootsListChangedParams{},
+ func() bool { return c.roots.remove(uris...) })
+}
+
+// changeFeature is called when a feature is added or removed.
+// It calls change, which should do the work and report whether a change actually occurred.
+// If there was a change, it notifies a snapshot of the sessions.
+func (c *Client) changeFeature(notification string, params any, change func() bool) {
+ var sessions []*ClientSession
+ // Lock for the change, but not for the notification.
c.mu.Lock()
- defer c.mu.Unlock()
- c.roots.remove(uris...)
+ if change() {
+ sessions = slices.Clone(c.sessions)
+ }
+ c.mu.Unlock()
+ notifySessions(sessions, notification, params)
}
func (c *Client) listRoots(_ context.Context, _ *ClientSession, _ *ListRootsParams) (*ListRootsResult, error) {
@@ -184,10 +200,12 @@
// clientMethodInfos maps from the RPC method name to serverMethodInfos.
var clientMethodInfos = map[string]methodInfo[ClientSession]{
- methodPing: newMethodInfo(sessionMethod((*ClientSession).ping)),
- methodListRoots: newMethodInfo(clientMethod((*Client).listRoots)),
- methodCreateMessage: newMethodInfo(clientMethod((*Client).createMessage)),
- // TODO: notifications
+ methodPing: newMethodInfo(sessionMethod((*ClientSession).ping)),
+ methodListRoots: newMethodInfo(clientMethod((*Client).listRoots)),
+ methodCreateMessage: newMethodInfo(clientMethod((*Client).createMessage)),
+ notificationToolListChanged: newMethodInfo(clientMethod((*Client).callToolChangedHandler)),
+ notificationPromptListChanged: newMethodInfo(clientMethod((*Client).callPromptChangedHandler)),
+ notificationResourceListChanged: newMethodInfo(clientMethod((*Client).callResourceChangedHandler)),
}
var _ session[ClientSession] = (*ClientSession)(nil)
@@ -206,6 +224,8 @@
return cs.client.methodHandler
}
+func (cs *ClientSession) getConn() *jsonrpc2.Connection { return cs.conn }
+
func (c *ClientSession) ping(ct context.Context, params *PingParams) (struct{}, error) {
return struct{}{}, nil
}
@@ -267,6 +287,18 @@
return standardCall[ReadResourceResult](ctx, c.conn, methodReadResource, params)
}
+func (c *Client) callToolChangedHandler(ctx context.Context, _ *ClientSession, params *ToolListChangedParams) (any, error) {
+ return callNotificationHandler(ctx, c.opts.ToolListChangedHandler, params)
+}
+
+func (c *Client) callPromptChangedHandler(ctx context.Context, _ *ClientSession, params *PromptListChangedParams) (any, error) {
+ return callNotificationHandler(ctx, c.opts.PromptListChangedHandler, params)
+}
+
+func (c *Client) callResourceChangedHandler(ctx context.Context, _ *ClientSession, params *ResourceListChangedParams) (any, error) {
+ return callNotificationHandler(ctx, c.opts.ResourceListChangedHandler, params)
+}
+
func standardCall[TRes, TParams any](ctx context.Context, conn *jsonrpc2.Connection, method string, params TParams) (*TRes, error) {
var result TRes
if err := call(ctx, conn, method, params, &result); err != nil {
diff --git a/internal/mcp/generate.go b/internal/mcp/generate.go
index 4aee09d..608c3c8 100644
--- a/internal/mcp/generate.go
+++ b/internal/mcp/generate.go
@@ -120,8 +120,12 @@
"Prompt": {},
"PromptMessage": {},
"PromptArgument": {},
- "ProgressToken": {Name: "-", Substitute: "any"}, // null|number|string
- "RequestId": {Name: "-", Substitute: "any"}, // null|number|string
+ "PromptListChangedNotification": {
+ Name: "-",
+ Fields: config{"Params": {Name: "PromptListChangedParams"}},
+ },
+ "ProgressToken": {Name: "-", Substitute: "any"}, // null|number|string
+ "RequestId": {Name: "-", Substitute: "any"}, // null|number|string
"ReadResourceRequest": {
Name: "-",
Fields: config{"Params": {Name: "ReadResourceParams"}},
@@ -130,8 +134,16 @@
Fields: config{"Contents": {Substitute: "*ResourceContents"}},
},
"Resource": {},
- "Role": {},
- "Root": {},
+ "ResourceListChangedNotification": {
+ Name: "-",
+ Fields: config{"Params": {Name: "ResourceListChangedParams"}},
+ },
+ "Role": {},
+ "Root": {},
+ "RootsListChangedNotification": {
+ Name: "-",
+ Fields: config{"Params": {Name: "RootsListChangedParams"}},
+ },
"SamplingCapabilities": {Substitute: "struct{}"},
"SamplingMessage": {},
@@ -147,6 +159,10 @@
Fields: config{"InputSchema": {Substitute: "*jsonschema.Schema"}},
},
"ToolAnnotations": {},
+ "ToolListChangedNotification": {
+ Name: "-",
+ Fields: config{"Params": {Name: "ToolListChangedParams"}},
+ },
}
func main() {
diff --git a/internal/mcp/mcp_test.go b/internal/mcp/mcp_test.go
index 98ced31..e1a013a 100644
--- a/internal/mcp/mcp_test.go
+++ b/internal/mcp/mcp_test.go
@@ -39,7 +39,26 @@
ctx := context.Background()
ct, st := NewInMemoryTransports()
- s := NewServer("testServer", "v1.0.0", nil)
+ // Channels to check if notification callbacks happened.
+ notificationChans := map[string]chan int{}
+ for _, name := range []string{"initialized", "roots", "tools", "prompts", "resources"} {
+ notificationChans[name] = make(chan int, 1)
+ }
+ waitForNotification := func(t *testing.T, name string) {
+ t.Helper()
+ select {
+ case <-notificationChans[name]:
+ case <-time.After(time.Second):
+ t.Fatalf("%s handler never called", name)
+ }
+ }
+
+ sopts := &ServerOptions{
+ InitializedHandler: func(context.Context, *InitializedParams) { notificationChans["initialized"] <- 0 },
+ RootsListChangedHandler: func(context.Context, *RootsListChangedParams) { notificationChans["roots"] <- 0 },
+ }
+
+ s := NewServer("testServer", "v1.0.0", sopts)
// The 'greet' tool says hi.
s.AddTools(NewTool("greet", "say hi", sayHi))
@@ -89,6 +108,9 @@
CreateMessageHandler: func(context.Context, *ClientSession, *CreateMessageParams) (*CreateMessageResult, error) {
return &CreateMessageResult{Model: "aModel"}, nil
},
+ ToolListChangedHandler: func(context.Context, *ToolListChangedParams) { notificationChans["tools"] <- 0 },
+ PromptListChangedHandler: func(context.Context, *PromptListChangedParams) { notificationChans["prompts"] <- 0 },
+ ResourceListChangedHandler: func(context.Context, *ResourceListChangedParams) { notificationChans["resources"] <- 0 },
}
c := NewClient("testClient", "v1.0.0", opts)
rootAbs, err := filepath.Abs(filepath.FromSlash("testdata/files"))
@@ -103,6 +125,7 @@
t.Fatal(err)
}
+ waitForNotification(t, "initialized")
if err := cs.Ping(ctx, nil); err != nil {
t.Fatalf("ping failed: %v", err)
}
@@ -141,6 +164,11 @@
if _, err := cs.GetPrompt(ctx, &GetPromptParams{Name: "fail"}); err == nil || !strings.Contains(err.Error(), failure.Error()) {
t.Errorf("fail returned unexpected error: got %v, want containing %v", err, failure)
}
+
+ s.AddPrompts(&ServerPrompt{Prompt: &Prompt{Name: "T"}})
+ waitForNotification(t, "prompts")
+ s.RemovePrompts("T")
+ waitForNotification(t, "prompts")
})
t.Run("tools", func(t *testing.T) {
@@ -198,6 +226,11 @@
if diff := cmp.Diff(wantFail, gotFail); diff != "" {
t.Errorf("tools/call 'fail' mismatch (-want +got):\n%s", diff)
}
+
+ s.AddTools(&ServerTool{Tool: &Tool{Name: "T"}})
+ waitForNotification(t, "tools")
+ s.RemoveTools("T")
+ waitForNotification(t, "tools")
})
t.Run("resources", func(t *testing.T) {
@@ -254,6 +287,11 @@
}
}
}
+
+ s.AddResources(&ServerResource{Resource: &Resource{URI: "http://U"}})
+ waitForNotification(t, "resources")
+ s.RemoveResources("http://U")
+ waitForNotification(t, "resources")
})
t.Run("roots", func(t *testing.T) {
rootRes, err := ss.ListRoots(ctx, &ListRootsParams{})
@@ -265,6 +303,11 @@
if diff := cmp.Diff(wantRoots, gotRoots); diff != "" {
t.Errorf("roots/list mismatch (-want +got):\n%s", diff)
}
+
+ c.AddRoots(&Root{URI: "U"})
+ waitForNotification(t, "roots")
+ c.RemoveRoots("U")
+ waitForNotification(t, "roots")
})
t.Run("sampling", func(t *testing.T) {
// TODO: test that a client that doesn't have the handler returns CodeUnsupportedMethod.
@@ -462,6 +505,10 @@
2 >initialize
2 <initialize
1 <initialize
+1 >notifications/initialized
+2 >notifications/initialized
+2 <notifications/initialized
+1 <notifications/initialized
1 >tools/list
2 >tools/list
2 <tools/list
diff --git a/internal/mcp/protocol.go b/internal/mcp/protocol.go
index a3c0d18..2b1758b 100644
--- a/internal/mcp/protocol.go
+++ b/internal/mcp/protocol.go
@@ -330,6 +330,13 @@
Required bool `json:"required,omitempty"`
}
+type PromptListChangedParams struct {
+ // This parameter name is reserved by MCP to allow clients and servers to attach
+ // additional metadata to their notifications.
+ Meta struct {
+ } `json:"_meta,omitempty"`
+}
+
// Describes a message returned as part of a prompt.
//
// This is similar to `SamplingMessage`, but also supports the embedding of
@@ -379,6 +386,13 @@
URI string `json:"uri"`
}
+type ResourceListChangedParams struct {
+ // This parameter name is reserved by MCP to allow clients and servers to attach
+ // additional metadata to their notifications.
+ Meta struct {
+ } `json:"_meta,omitempty"`
+}
+
// The sender or recipient of messages and data in a conversation.
type Role string
@@ -394,6 +408,13 @@
URI string `json:"uri"`
}
+type RootsListChangedParams struct {
+ // This parameter name is reserved by MCP to allow clients and servers to attach
+ // additional metadata to their notifications.
+ Meta struct {
+ } `json:"_meta,omitempty"`
+}
+
// Present if the client supports sampling from an LLM.
type SamplingCapabilities struct{}
@@ -455,6 +476,13 @@
Title string `json:"title,omitempty"`
}
+type ToolListChangedParams struct {
+ // This parameter name is reserved by MCP to allow clients and servers to attach
+ // additional metadata to their notifications.
+ Meta struct {
+ } `json:"_meta,omitempty"`
+}
+
// Describes the name and version of an MCP implementation.
type implementation struct {
Name string `json:"name"`
@@ -500,28 +528,28 @@
}
const (
- notificationCancelled = "notifications/cancelled"
- methodInitialize = "initialize"
- notificationProgress = "notifications/progress"
- methodSetLevel = "logging/setLevel"
methodCreateMessage = "sampling/createMessage"
- notificationResourceListChanged = "notifications/resources/list_changed"
- notificationInitialized = "notifications/initialized"
- methodUnsubscribe = "resources/unsubscribe"
- notificationLoggingMessage = "notifications/message"
- methodSubscribe = "resources/subscribe"
- methodComplete = "completion/complete"
- methodCallTool = "tools/call"
- notificationPromptListChanged = "notifications/prompts/list_changed"
- methodReadResource = "resources/read"
- methodListResourceTemplates = "resources/templates/list"
- methodListRoots = "roots/list"
notificationToolListChanged = "notifications/tools/list_changed"
- methodGetPrompt = "prompts/get"
+ notificationResourceListChanged = "notifications/resources/list_changed"
methodListPrompts = "prompts/list"
- methodPing = "ping"
- notificationRootsListChanged = "notifications/roots/list_changed"
- methodListTools = "tools/list"
- methodListResources = "resources/list"
+ notificationPromptListChanged = "notifications/prompts/list_changed"
notificationResourceUpdated = "notifications/resources/updated"
+ notificationCancelled = "notifications/cancelled"
+ methodSetLevel = "logging/setLevel"
+ methodInitialize = "initialize"
+ methodListRoots = "roots/list"
+ notificationProgress = "notifications/progress"
+ methodGetPrompt = "prompts/get"
+ methodListResourceTemplates = "resources/templates/list"
+ methodPing = "ping"
+ methodComplete = "completion/complete"
+ methodListResources = "resources/list"
+ notificationInitialized = "notifications/initialized"
+ methodCallTool = "tools/call"
+ notificationLoggingMessage = "notifications/message"
+ methodReadResource = "resources/read"
+ methodListTools = "tools/list"
+ notificationRootsListChanged = "notifications/roots/list_changed"
+ methodSubscribe = "resources/subscribe"
+ methodUnsubscribe = "resources/unsubscribe"
)
diff --git a/internal/mcp/server.go b/internal/mcp/server.go
index 1a8eaf6..b74f4bf 100644
--- a/internal/mcp/server.go
+++ b/internal/mcp/server.go
@@ -36,11 +36,16 @@
// ServerOptions is used to configure behavior of the server.
type ServerOptions struct {
+ // TODO: document
Instructions string
+ // If non-nil, called when "notifications/intialized" is received.
+ InitializedHandler func(ctx context.Context, params *InitializedParams)
+ // If non-nil, called when "notifications/roots/list_changed" is received.
+ RootsListChangedHandler func(ctx context.Context, params *RootsListChangedParams)
}
// NewServer creates a new MCP server. The resulting server has no features:
-// add features using [Server.AddTools]. (TODO: support more features).
+// add features using [Server.AddTools], [Server.AddPrompts] and [Server.AddResources].
//
// The server can be connected to one or more MCP clients using [Server.Start]
// or [Server.Run].
@@ -64,43 +69,35 @@
// AddPrompts adds the given prompts to the server,
// replacing any with the same names.
func (s *Server) AddPrompts(prompts ...*ServerPrompt) {
- s.mu.Lock()
- defer s.mu.Unlock()
- s.prompts.add(prompts...)
- // Assume there was a change, since add replaces existing prompts.
- // (It's possible a prompt was replaced with an identical one, but not worth checking.)
- // TODO(rfindley): notify connected clients
+ // Assume there was a change, since add replaces existing roots.
+ // (It's possible a root was replaced with an identical one, but not worth checking.)
+ s.changeFeature(
+ notificationPromptListChanged,
+ &PromptListChangedParams{},
+ func() bool { s.prompts.add(prompts...); return true })
}
// RemovePrompts removes the prompts with the given names.
// It is not an error to remove a nonexistent prompt.
func (s *Server) RemovePrompts(names ...string) {
- s.mu.Lock()
- defer s.mu.Unlock()
- if s.prompts.remove(names...) {
- // TODO: notify
- }
+ s.changeFeature(notificationPromptListChanged, &PromptListChangedParams{},
+ func() bool { return s.prompts.remove(names...) })
}
// AddTools adds the given tools to the server,
// replacing any with the same names.
func (s *Server) AddTools(tools ...*ServerTool) {
- s.mu.Lock()
- defer s.mu.Unlock()
- s.tools.add(tools...)
// Assume there was a change, since add replaces existing tools.
// (It's possible a tool was replaced with an identical one, but not worth checking.)
- // TODO(rfindley): notify connected clients
+ s.changeFeature(notificationToolListChanged, &ToolListChangedParams{},
+ func() bool { s.tools.add(tools...); return true })
}
// RemoveTools removes the tools with the given names.
// It is not an error to remove a nonexistent tool.
func (s *Server) RemoveTools(names ...string) {
- s.mu.Lock()
- defer s.mu.Unlock()
- if s.tools.remove(names...) {
- // TODO: notify
- }
+ s.changeFeature(notificationToolListChanged, &ToolListChangedParams{},
+ func() bool { return s.tools.remove(names...) })
}
// AddResource adds the given resource to the server and associates it with
@@ -108,27 +105,41 @@
// 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) {
- s.mu.Lock()
- defer s.mu.Unlock()
- for _, r := range resources {
- u, err := url.Parse(r.Resource.URI)
- if err != nil {
- panic(err) // url.Parse includes the URI in the error
- }
- if !u.IsAbs() {
- panic(fmt.Errorf("URI %s needs a scheme", r.Resource.URI))
- }
- s.resources.add(r)
- }
- // TODO: notify
+ s.changeFeature(notificationResourceListChanged, &ResourceListChangedParams{},
+ func() bool {
+ for _, r := range resources {
+ u, err := url.Parse(r.Resource.URI)
+ if err != nil {
+ panic(err) // url.Parse includes the URI in the error
+ }
+ if !u.IsAbs() {
+ panic(fmt.Errorf("URI %s needs a scheme", r.Resource.URI))
+ }
+ s.resources.add(r)
+ }
+ return true
+ })
}
// RemoveResources removes the resources with the given URIs.
// It is not an error to remove a nonexistent resource.
func (s *Server) RemoveResources(uris ...string) {
+ s.changeFeature(notificationResourceListChanged, &ResourceListChangedParams{},
+ func() bool { return s.resources.remove(uris...) })
+}
+
+// changeFeature is called when a feature is added or removed.
+// It calls change, which should do the work and report whether a change actually occurred.
+// If there was a change, it notifies a snapshot of the sessions.
+func (s *Server) changeFeature(notification string, params any, change func() bool) {
+ var sessions []*ServerSession
+ // Lock for the change, but not for the notification.
s.mu.Lock()
- defer s.mu.Unlock()
- s.resources.remove(uris...)
+ if change() {
+ sessions = slices.Clone(s.sessions)
+ }
+ s.mu.Unlock()
+ notifySessions(sessions, notification, params)
}
// Sessions returns an iterator that yields the current set of server sessions.
@@ -302,6 +313,14 @@
return connect(ctx, t, s)
}
+func (s *Server) callInitializedHandler(ctx context.Context, _ *ServerSession, params *InitializedParams) (any, error) {
+ return callNotificationHandler(ctx, s.opts.InitializedHandler, params)
+}
+
+func (s *Server) callRootsListChangedHandler(ctx context.Context, _ *ServerSession, params *RootsListChangedParams) (any, error) {
+ return callNotificationHandler(ctx, s.opts.RootsListChangedHandler, params)
+}
+
// A ServerSession is a logical connection from a single MCP client. Its
// methods can be used to send requests or notifications to the client. Create
// a session by calling [Server.Connect].
@@ -346,15 +365,16 @@
// serverMethodInfos maps from the RPC method name to serverMethodInfos.
var serverMethodInfos = map[string]methodInfo[ServerSession]{
- methodInitialize: newMethodInfo(sessionMethod((*ServerSession).initialize)),
- methodPing: newMethodInfo(sessionMethod((*ServerSession).ping)),
- methodListPrompts: newMethodInfo(serverMethod((*Server).listPrompts)),
- methodGetPrompt: newMethodInfo(serverMethod((*Server).getPrompt)),
- methodListTools: newMethodInfo(serverMethod((*Server).listTools)),
- methodCallTool: newMethodInfo(serverMethod((*Server).callTool)),
- methodListResources: newMethodInfo(serverMethod((*Server).listResources)),
- methodReadResource: newMethodInfo(serverMethod((*Server).readResource)),
- // TODO: notifications
+ methodInitialize: newMethodInfo(sessionMethod((*ServerSession).initialize)),
+ methodPing: newMethodInfo(sessionMethod((*ServerSession).ping)),
+ methodListPrompts: newMethodInfo(serverMethod((*Server).listPrompts)),
+ methodGetPrompt: newMethodInfo(serverMethod((*Server).getPrompt)),
+ methodListTools: newMethodInfo(serverMethod((*Server).listTools)),
+ methodCallTool: newMethodInfo(serverMethod((*Server).callTool)),
+ methodListResources: newMethodInfo(serverMethod((*Server).listResources)),
+ methodReadResource: newMethodInfo(serverMethod((*Server).readResource)),
+ notificationInitialized: newMethodInfo(serverMethod((*Server).callInitializedHandler)),
+ notificationRootsListChanged: newMethodInfo(serverMethod((*Server).callRootsListChangedHandler)),
}
// *ServerSession implements the session interface.
@@ -371,6 +391,8 @@
return ss.server.methodHandler
}
+func (ss *ServerSession) getConn() *jsonrpc2.Connection { return ss.conn }
+
// handle invokes the method described by the given JSON RPC request.
func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc2.Request) (any, error) {
ss.mu.Lock()
diff --git a/internal/mcp/shared.go b/internal/mcp/shared.go
index 1e8983a..22afd34 100644
--- a/internal/mcp/shared.go
+++ b/internal/mcp/shared.go
@@ -13,7 +13,9 @@
"context"
"encoding/json"
"fmt"
+ "log"
"slices"
+ "time"
jsonrpc2 "golang.org/x/tools/internal/jsonrpc2_v2"
)
@@ -40,6 +42,7 @@
type session[S ClientSession | ServerSession] interface {
getMethodHandler() MethodHandler[S]
methodInfos() map[string]methodInfo[S]
+ getConn() *jsonrpc2.Connection
}
// toSession[S] converts its argument to a session[S].
@@ -134,3 +137,27 @@
return f(sess, ctx, p)
}
}
+
+func callNotificationHandler[P any](ctx context.Context, h func(context.Context, *P), params *P) (any, error) {
+ if h != nil {
+ h(ctx, params)
+ }
+ return nil, nil
+}
+
+// notifySessions calls Notify on all the sessions.
+// Should be called on a copy of the peer sessions.
+func notifySessions[S ClientSession | ServerSession](sessions []*S, method string, params any) {
+ if sessions == nil {
+ return
+ }
+ // TODO(jba): make this timeout configurable.
+ ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ defer cancel()
+ for _, ss := range sessions {
+ if err := toSession(ss).getConn().Notify(ctx, method, params); err != nil {
+ // TODO(jba): surface this error better
+ log.Printf("calling %s: %v", method, err)
+ }
+ }
+}
Change information
Files:
- M internal/mcp/client.go
- M internal/mcp/generate.go
- M internal/mcp/mcp_test.go
- M internal/mcp/protocol.go
- M internal/mcp/server.go
- M internal/mcp/shared.go
Change size: L
Delta: 6 files changed, 252 insertions(+), 80 deletions(-)
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 18, 2025, 7:57:37 AMMay 18
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam uploaded new patchset![]( "Open in Gerrit")
Jonathan Amsterdam uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Findley (Gerrit)
May 19, 2025, 1:24:23 PMMay 19
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam and Sam Thanawalla
Robert Findley added 5 comments![]( "Open in Gerrit")
File internal/mcp/client.go
Line 55, Patchset 2 (Latest):
ToolListChangedHandler func(context.Context, *ToolListChangedParams)
PromptListChangedHandler func(context.Context, *PromptListChangedParams)
ResourceListChangedHandler func(context.Context, *ResourceListChangedParams)Robert Findley . unresolved
Why don't these functions accept a ClientSession?
Ditto for Server handlers.
Or should they be options on Connect?
Line 170, Patchset 2 (Latest):
notifySessions(sessions, notification, params)Robert Findley . unresolved
Should this be async? Should each notification be async?
Should we return a function to await the notifications being sent?
I can see an argument for any of those...
File internal/mcp/server.go
Line 41, Patchset 2 (Latest):
// If non-nil, called when "notifications/intialized" is received.
InitializedHandler func(ctx context.Context, params *InitializedParams)Robert Findley . unresolved
Why do we need an InitializedHandler? That seems like an internal detail, and isn't part of the design. There's no particular need to handle the initialized notification.
Maybe we should have Server.Connect await the initialized notification?
I think all of these could also be implemented as middleware, so we don't have to provide absolutely every one.
Line 72, Patchset 2 (Latest):
// Assume there was a change, since add replaces existing roots.Robert Findley . unresolved
Super nit: `if len(prompts) == 0 { return }`
And similar for others. Just Hyrum's law: I don't want it to be possible to generate a notification if there was no actual change.
Line 438, Patchset 2 (Latest):
ListChanged: true, // not yet supportedRobert Findley . unresolved
Remove these comments.
Related details
Attention is currently required from:
- Jonathan Amsterdam
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 19, 2025, 3:10:28 PMMay 19
to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam added 5 comments![]( "Open in Gerrit")
File internal/mcp/client.go
Line 55, Patchset 2 (Latest):
ToolListChangedHandler func(context.Context, *ToolListChangedParams)
PromptListChangedHandler func(context.Context, *PromptListChangedParams)
ResourceListChangedHandler func(context.Context, *ResourceListChangedParams)Robert Findley . unresolved
Why don't these functions accept a ClientSession?
Ditto for Server handlers.
Or should they be options on Connect?
Jonathan Amsterdam
OK, what I have here is definitely wrong: every session is notified, and they all call this function, which doesn't know who called it.
We could add a ClientSession arg, which fixes that.
But maybe we should call this exactly once, with no ClientSession. The change happened to the Server, which broadcasted notifications to all sessions just so it didn't miss any Clients. But each client logically needs to know about the change only once.
Line 170, Patchset 2 (Latest):
notifySessions(sessions, notification, params)Robert Findley . unresolved
Should this be async? Should each notification be async?
Should we return a function to await the notifications being sent?
I can see an argument for any of those...
Jonathan Amsterdam
I think the timeout in notifySessions is OK for now.
Doing these async seems like it could get tricky. The client could be notified after the server closes, and so on.
File internal/mcp/server.go
Line 41, Patchset 2 (Latest):
// If non-nil, called when "notifications/intialized" is received.
InitializedHandler func(ctx context.Context, params *InitializedParams)Robert Findley . unresolved
Why do we need an InitializedHandler? That seems like an internal detail, and isn't part of the design. There's no particular need to handle the initialized notification.
Maybe we should have Server.Connect await the initialized notification?
I think all of these could also be implemented as middleware, so we don't have to provide absolutely every one.
Jonathan Amsterdam
I assumed there was a reason the spec says that "client MUST send" that notification. Presumably it's so the server could deal with it in some way. Don't we want the user to have that ability?
Line 72, Patchset 2 (Latest):
// Assume there was a change, since add replaces existing roots.Robert Findley . resolved
Super nit: `if len(prompts) == 0 { return }`
And similar for others. Just Hyrum's law: I don't want it to be possible to generate a notification if there was no actual change.
Jonathan Amsterdam
Done
Line 438, Patchset 2 (Latest):
ListChanged: true, // not yet supportedAttention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 19, 2025, 3:28:55 PMMay 19
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam uploaded new patchset![]( "Open in Gerrit")
Jonathan Amsterdam uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Thanawalla (Gerrit)
May 20, 2025, 1:02:21 PMMay 20
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam and Robert Findley
Sam Thanawalla added 1 comment![]( "Open in Gerrit")
File internal/mcp/server.go
Line 146, Patchset 3 (Latest):
func (s *Server) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . unresolved
notifyListChange or notifyChange?
Related details
Attention is currently required from:
- Jonathan Amsterdam
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 20, 2025, 2:59:59 PMMay 20
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam uploaded new patchset![]( "Open in Gerrit")
Jonathan Amsterdam uploaded patch set #4 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 20, 2025, 3:00:00 PMMay 20
to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam added 1 comment![]( "Open in Gerrit")
File internal/mcp/server.go
Line 146, Patchset 3:
func (s *Server) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . unresolved
notifyListChange or notifyChange?
Jonathan Amsterdam
It also does the change.
Renamed to changeAndNotify.
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Thanawalla (Gerrit)
May 21, 2025, 12:27:37 PMMay 21
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam and Robert Findley
Sam Thanawalla added 1 comment![]( "Open in Gerrit")
File internal/mcp/server.go
Line 146, Patchset 3:
func (s *Server) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . resolved
Jonathan AmsterdamnotifyListChange or notifyChange?
It also does the change.
Renamed to changeAndNotify.
Attention is currently required from:
- Jonathan Amsterdam
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Findley (Gerrit)
May 21, 2025, 12:35:02 PMMay 21
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam
Robert Findley added 4 comments![]( "Open in Gerrit")
File internal/mcp/client.go
Line 55, Patchset 2:
ToolListChangedHandler func(context.Context, *ToolListChangedParams)
PromptListChangedHandler func(context.Context, *PromptListChangedParams)
ResourceListChangedHandler func(context.Context, *ResourceListChangedParams)Robert Findley . unresolved
Jonathan AmsterdamWhy don't these functions accept a ClientSession?
Ditto for Server handlers.
Or should they be options on Connect?
OK, what I have here is definitely wrong: every session is notified, and they all call this function, which doesn't know who called it.
We could add a ClientSession arg, which fixes that.
But maybe we should call this exactly once, with no ClientSession. The change happened to the Server, which broadcasted notifications to all sessions just so it didn't miss any Clients. But each client logically needs to know about the change only once.
Robert Findley
I don't understand this comment. The client could be connected to 17 different servers with different capabilities. The handler needs to be associated with a session, not a client.
That's why I'm wondering if this needs to be an option to Connect...
Jonathan AmsterdamShould this be async? Should each notification be async?
Should we return a function to await the notifications being sent?
I can see an argument for any of those...
I think the timeout in notifySessions is OK for now.
Doing these async seems like it could get tricky. The client could be notified after the server closes, and so on.
Robert Findley
Can you add a TODO as a reminder to revisit this behavior?
I don't think I fully understand the design space.
func (cs *ClientSession) getConn() *jsonrpc2.Connection { return cs.conn }Robert Findley . unresolved
// ... implements the XXX interface.
(else I the reader will continue to be confused as to why this exists)
File internal/mcp/shared.go
Line 167, Patchset 4:
// TODO(jba): make this timeout configurable.Robert Findley . unresolved
, or async.
Related details
Attention is currently required from:
- Jonathan Amsterdam
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Thanawalla (Gerrit)
May 21, 2025, 1:28:51 PMMay 21
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam
Sam Thanawalla added 1 comment![]( "Open in Gerrit")
File internal/mcp/client.go
Line 165, Patchset 5 (Latest):
func (c *Client) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . unresolved
changeAndNotify here as well.
Can this be a shared function?
Related details
Attention is currently required from:
- Jonathan Amsterdam
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 2:31:29 PMMay 21
to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam added 5 comments![]( "Open in Gerrit")
File internal/mcp/client.go
Line 55, Patchset 2:
ToolListChangedHandler func(context.Context, *ToolListChangedParams)
PromptListChangedHandler func(context.Context, *PromptListChangedParams)
ResourceListChangedHandler func(context.Context, *ResourceListChangedParams)Robert Findley . resolved
Jonathan AmsterdamWhy don't these functions accept a ClientSession?
Ditto for Server handlers.
Or should they be options on Connect?
Robert FindleyOK, what I have here is definitely wrong: every session is notified, and they all call this function, which doesn't know who called it.
We could add a ClientSession arg, which fixes that.
But maybe we should call this exactly once, with no ClientSession. The change happened to the Server, which broadcasted notifications to all sessions just so it didn't miss any Clients. But each client logically needs to know about the change only once.
I don't understand this comment. The client could be connected to 17 different servers with different capabilities. The handler needs to be associated with a session, not a client.
That's why I'm wondering if this needs to be an option to Connect...
Jonathan Amsterdam
Ah yes, good point. I'll add the ClientSession.
Before we complexify things with ConnectOptions, let's think through the likely use cases.
I would guess there is one predominant use case: when you're told that the tools list changed, you call ListTools (either immediately or lazily) to get the new list. Since ListTools is a method on ClientSession, and you'll have the ClientSession, you'd just call it here (or set a bit to do so later). If the handler for every server behaves identically, then a single Client option suffices.
Line 165, Patchset 5:
func (c *Client) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . resolved
changeAndNotify here as well.
Can this be a shared function?
Jonathan Amsterdam
I tried to do that, but couldn't think of a nice way. You'd either need interface methods on Client and Server that returned mu and sessions, or you'd need a generic interface function like
ChangeAndSessions[S](change func() bool) []*S
whose implementation would be the middle three lines. It seems like more complexity than it's worth.
If you can think of a nicer way that's simpler than copying, let me know or send a CL.
Line 170, Patchset 2:
notifySessions(sessions, notification, params)Robert Findley . resolved
Jonathan AmsterdamShould this be async? Should each notification be async?
Should we return a function to await the notifications being sent?
I can see an argument for any of those...
Robert FindleyI think the timeout in notifySessions is OK for now.
Doing these async seems like it could get tricky. The client could be notified after the server closes, and so on.
Can you add a TODO as a reminder to revisit this behavior?
I don't think I fully understand the design space.
Jonathan Amsterdam
Done, on the notifySessions function.
Line 230, Patchset 4:
func (cs *ClientSession) getConn() *jsonrpc2.Connection { return cs.conn }Robert Findley . resolved
// ... implements the XXX interface.
(else I the reader will continue to be confused as to why this exists)
Jonathan Amsterdam
Done
File internal/mcp/shared.go
Line 167, Patchset 4:
// TODO(jba): make this timeout configurable.Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 2:31:31 PMMay 21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam uploaded new patchset![]( "Open in Gerrit")
Jonathan Amsterdam uploaded patch set #6 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Thanawalla (Gerrit)
May 21, 2025, 2:34:47 PMMay 21
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam and Robert Findley
Sam Thanawalla added 1 comment![]( "Open in Gerrit")
File internal/mcp/client.go
Line 165, Patchset 5:
func (c *Client) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . unresolved
Jonathan AmsterdamchangeAndNotify here as well.
Can this be a shared function?
I tried to do that, but couldn't think of a nice way. You'd either need interface methods on Client and Server that returned mu and sessions, or you'd need a generic interface function like
ChangeAndSessions[S](change func() bool) []*S
whose implementation would be the middle three lines. It seems like more complexity than it's worth.If you can think of a nicer way that's simpler than copying, let me know or send a CL.
Sam Thanawalla
Got it. What about changing the function name to changeAndNotify to match server?
Related details
Attention is currently required from:
- Jonathan Amsterdam
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 3:00:26 PMMay 21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam uploaded new patchset![]( "Open in Gerrit")
Jonathan Amsterdam uploaded patch set #7 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 3:00:27 PMMay 21
to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Robert Findley and Sam Thanawalla
Jonathan Amsterdam added 1 comment![]( "Open in Gerrit")
File internal/mcp/client.go
Line 165, Patchset 5:
func (c *Client) changeFeature(notification string, params any, change func() bool) {Sam Thanawalla . resolved
Jonathan AmsterdamchangeAndNotify here as well.
Can this be a shared function?
Sam ThanawallaI tried to do that, but couldn't think of a nice way. You'd either need interface methods on Client and Server that returned mu and sessions, or you'd need a generic interface function like
ChangeAndSessions[S](change func() bool) []*S
whose implementation would be the middle three lines. It seems like more complexity than it's worth.If you can think of a nicer way that's simpler than copying, let me know or send a CL.
Got it. What about changing the function name to changeAndNotify to match server?
Jonathan Amsterdam
Oops. Done.
Related details
Attention is currently required from:
- Robert Findley
- Sam Thanawalla
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Thanawalla (Gerrit)
May 21, 2025, 3:04:58 PMMay 21
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam and Robert Findley
Sam Thanawalla voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Jonathan Amsterdam
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Findley (Gerrit)
May 21, 2025, 4:58:24 PMMay 21
to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Go LUCI, Sam Thanawalla, golang-co...@googlegroups.com
Attention needed from Jonathan Amsterdam
Robert Findley voted and added 3 comments![]( "Open in Gerrit")
Votes added by Robert Findley
| Code-Review | +2 |
3 comments
File internal/mcp/client.go
File internal/mcp/server.go
Line 39, Patchset 7 (Latest):
// TODO: documentRobert Findley . unresolved
// Optional instructions for connected clients.
Line 41, Patchset 2:
// If non-nil, called when "notifications/intialized" is received.
InitializedHandler func(ctx context.Context, params *InitializedParams)Robert Findley . resolved
Why do we need an InitializedHandler? That seems like an internal detail, and isn't part of the design. There's no particular need to handle the initialized notification.
Maybe we should have Server.Connect await the initialized notification?
I think all of these could also be implemented as middleware, so we don't have to provide absolutely every one.
Jonathan Amsterdam
I assumed there was a reason the spec says that "client MUST send" that notification. Presumably it's so the server could deal with it in some way. Don't we want the user to have that ability?
Robert Findley
I think this needs more design -- let's merge it as-is but discuss tomorrow.
Related details
Attention is currently required from:
- Jonathan Amsterdam
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 5:00:21 PMMay 21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Jonathan Amsterdam uploaded new patchset![]( "Open in Gerrit")
Jonathan Amsterdam uploaded patch set #8 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 5:00:21 PMMay 21
to goph...@pubsubhelper.golang.org, Robert Findley, Go LUCI, Sam Thanawalla, golang-co...@googlegroups.com
Jonathan Amsterdam added 2 comments![]( "Open in Gerrit")
File internal/mcp/client.go
File internal/mcp/server.go
// Optional instructions for connected clients.
Jonathan Amsterdam
Done
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Amsterdam (Gerrit)
May 21, 2025, 7:01:56 PMMay 21
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Robert Findley, Sam Thanawalla, golang-co...@googlegroups.com
Jonathan Amsterdam submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: internal/mcp/server.go
Insertions: 1, Deletions: 1.
@@ -36,7 +36,7 @@
// ServerOptions is used to configure behavior of the server.
type ServerOptions struct {
- // TODO: document
+ // Optional instructions for connected clients.
Instructions string
// If non-nil, called when "notifications/intialized" is received.
InitializedHandler func(context.Context, *ServerSession, *InitializedParams)
```
```
The name of the file: internal/mcp/client.go
Insertions: 0, Deletions: 1.
@@ -46,7 +46,6 @@
// ClientOptions configures the behavior of the client.
type ClientOptions struct {
- //
// Handler for sampling.
// Called when a server calls CreateMessage.
CreateMessageHandler func(context.Context, *ClientSession, *CreateMessageParams) (*CreateMessageResult, error)
```
Change information
Commit message:
internal/mcp: add notifications
Add notification logic for intialization and list changes.
Change-Id: I1d392bb49d5995f2046c400da955a033ab2715ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/673775
Reviewed-by: Sam Thanawalla <samtha...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfin...@google.com>
Files:
- M internal/mcp/client.go
- M internal/mcp/generate.go
- M internal/mcp/mcp_test.go
- M internal/mcp/protocol.go
- M internal/mcp/server.go
- M internal/mcp/shared.go
Change size: L
Delta: 6 files changed, 276 insertions(+), 81 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Robert Findley, +1 by Sam Thanawalla
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages