Skip to content

Commit d8ef217

Browse files
committed
feat(MCPServer): avoid unnecessary notifications when Resource/Tool not exists
1 parent 5c19710 commit d8ef217

File tree

3 files changed

+45
-9
lines changed

3 files changed

+45
-9
lines changed

server/resource_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func TestMCPServer_RemoveResource(t *testing.T) {
9898
},
9999
},
100100
{
101-
name: "RemoveResource with non-existent resource does nothing",
101+
name: "RemoveResource with non-existent resource does nothing and not receives notifications from MCPServer",
102102
action: func(t *testing.T, server *MCPServer, notificationChannel chan mcp.JSONRPCNotification) {
103103
// Add a test resource
104104
server.AddResource(
@@ -130,10 +130,11 @@ func TestMCPServer_RemoveResource(t *testing.T) {
130130
// Remove a non-existent resource
131131
server.RemoveResource("test://nonexistent")
132132
},
133-
expectedNotifications: 1, // Still sends a notification
133+
expectedNotifications: 0, // No notifications expected
134134
validate: func(t *testing.T, notifications []mcp.JSONRPCNotification, resourcesList mcp.JSONRPCMessage) {
135-
// Check that we received a list_changed notification
136-
assert.Equal(t, mcp.MethodNotificationResourcesListChanged, notifications[0].Method)
135+
// verify that no notifications were sent
136+
assert.Empty(t, notifications)
137+
137138

138139
// The original resource should still be there
139140
resp, ok := resourcesList.(mcp.JSONRPCResponse)

server/server.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,14 @@ func (s *MCPServer) AddResource(
336336
// RemoveResource removes a resource from the server
337337
func (s *MCPServer) RemoveResource(uri string) {
338338
s.resourcesMu.Lock()
339-
delete(s.resources, uri)
339+
_, exists := s.resources[uri]
340+
if exists {
341+
delete(s.resources, uri)
342+
}
340343
s.resourcesMu.Unlock()
341344

342-
// Send notification to all initialized sessions if listChanged capability is enabled
343-
if s.capabilities.resources != nil && s.capabilities.resources.listChanged {
345+
// Send notification to all initialized sessions if listChanged capability is enabled and we actually remove a resource
346+
if exists && s.capabilities.resources != nil && s.capabilities.resources.listChanged {
344347
s.SendNotificationToAllClients(mcp.MethodNotificationResourcesListChanged, nil)
345348
}
346349
}
@@ -448,13 +451,17 @@ func (s *MCPServer) SetTools(tools ...ServerTool) {
448451
// DeleteTools removes a tool from the server
449452
func (s *MCPServer) DeleteTools(names ...string) {
450453
s.toolsMu.Lock()
454+
var exists bool
451455
for _, name := range names {
452-
delete(s.tools, name)
456+
if _, ok := s.tools[name]; ok {
457+
delete(s.tools, name)
458+
exists = true
459+
}
453460
}
454461
s.toolsMu.Unlock()
455462

456463
// When the list of available tools changes, servers that declared the listChanged capability SHOULD send a notification.
457-
if s.capabilities.tools.listChanged {
464+
if exists && s.capabilities.tools != nil && s.capabilities.tools.listChanged {
458465
// Send notification to all initialized sessions
459466
s.SendNotificationToAllClients(mcp.MethodNotificationToolsListChanged, nil)
460467
}

server/server_test.go

+28
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,34 @@ func TestMCPServer_Tools(t *testing.T) {
308308
assert.Empty(t, result.Tools, "Expected empty tools list")
309309
},
310310
},
311+
{
312+
name: "DeleteTools with non-existent tools does nothing and not receives notifications from MCPServer",
313+
action: func(t *testing.T, server *MCPServer, notificationChannel chan mcp.JSONRPCNotification) {
314+
err := server.RegisterSession(context.TODO(), &fakeSession{
315+
sessionID: "test",
316+
notificationChannel: notificationChannel,
317+
initialized: true,
318+
})
319+
require.NoError(t, err)
320+
server.SetTools(
321+
ServerTool{Tool: mcp.NewTool("test-tool-1")},
322+
ServerTool{Tool: mcp.NewTool("test-tool-2")})
323+
324+
// Remove non-existing tools
325+
server.DeleteTools("test-tool-3", "test-tool-4")
326+
},
327+
expectedNotifications: 1,
328+
validate: func(t *testing.T, notifications []mcp.JSONRPCNotification, toolsList mcp.JSONRPCMessage) {
329+
// Only one notification expected for SetTools
330+
assert.Equal(t, mcp.MethodNotificationToolsListChanged, notifications[0].Method)
331+
332+
// Confirm the tool list does not change
333+
tools := toolsList.(mcp.JSONRPCResponse).Result.(mcp.ListToolsResult).Tools
334+
assert.Len(t, tools, 2)
335+
assert.Equal(t, "test-tool-1", tools[0].Name)
336+
assert.Equal(t, "test-tool-2", tools[1].Name)
337+
},
338+
},
311339
}
312340
for _, tt := range tests {
313341
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)