Skip to content

Commit eb835b9

Browse files
fix: Gate notifications on capabilities (#290)
Servers may report their [tools.listChanged][] capability as false, in which case they indicate that they will not send notifications when available tools change. Honor the spec by not sending notifications/tools/list_changed notifications when capabilities.tools.listChanged is false. [tools.listChanged]: https://modelcontextprotocol.io/specification/2025-03-26/server/tools#capabilities
1 parent e7d2547 commit eb835b9

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

server/session.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,12 @@ func (s *MCPServer) AddSessionTools(sessionID string, tools ...ServerTool) error
247247

248248
// It only makes sense to send tool notifications to initialized sessions --
249249
// if we're not initialized yet the client can't possibly have sent their
250-
// initial tools/list message
251-
if session.Initialized() {
250+
// initial tools/list message.
251+
//
252+
// For initialized sessions, honor tools.listChanged, which is specifically
253+
// about whether notifications will be sent or not.
254+
// see <https://modelcontextprotocol.io/specification/2025-03-26/server/tools#capabilities>
255+
if session.Initialized() && s.capabilities.tools != nil && s.capabilities.tools.listChanged {
252256
// Send notification only to this session
253257
if err := s.SendNotificationToSpecificClient(sessionID, "notifications/tools/list_changed", nil); err != nil {
254258
// Log the error but don't fail the operation
@@ -305,8 +309,12 @@ func (s *MCPServer) DeleteSessionTools(sessionID string, names ...string) error
305309

306310
// It only makes sense to send tool notifications to initialized sessions --
307311
// if we're not initialized yet the client can't possibly have sent their
308-
// initial tools/list message
309-
if session.Initialized() {
312+
// initial tools/list message.
313+
//
314+
// For initialized sessions, honor tools.listChanged, which is specifically
315+
// about whether notifications will be sent or not.
316+
// see <https://modelcontextprotocol.io/specification/2025-03-26/server/tools#capabilities>
317+
if session.Initialized() && s.capabilities.tools != nil && s.capabilities.tools.listChanged {
310318
// Send notification only to this session
311319
if err := s.SendNotificationToSpecificClient(sessionID, "notifications/tools/list_changed", nil); err != nil {
312320
// Log the error but don't fail the operation

server/session_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,3 +858,62 @@ func TestMCPServer_SessionToolCapabilitiesBehavior(t *testing.T) {
858858
})
859859
}
860860
}
861+
862+
func TestMCPServer_ToolNotificationsDisabled(t *testing.T) {
863+
// This test verifies that when tool capabilities are disabled, we still
864+
// add/delete tools correctly but don't send notifications about it.
865+
//
866+
// This is important because:
867+
// 1. Tools should still work even if notifications are disabled
868+
// 2. We shouldn't waste resources sending notifications that won't be used
869+
// 3. The client might not be ready to handle tool notifications yet
870+
871+
// Create a server WITHOUT tool capabilities
872+
server := NewMCPServer("test-server", "1.0.0", WithToolCapabilities(false))
873+
ctx := context.Background()
874+
875+
// Create an initialized session
876+
sessionChan := make(chan mcp.JSONRPCNotification, 1)
877+
session := &sessionTestClientWithTools{
878+
sessionID: "session-1",
879+
notificationChannel: sessionChan,
880+
initialized: true,
881+
}
882+
883+
// Register the session
884+
err := server.RegisterSession(ctx, session)
885+
require.NoError(t, err)
886+
887+
// Add a tool
888+
err = server.AddSessionTools(session.SessionID(),
889+
ServerTool{Tool: mcp.NewTool("test-tool")},
890+
)
891+
require.NoError(t, err)
892+
893+
// Verify no notification was sent
894+
select {
895+
case <-sessionChan:
896+
t.Error("Expected no notification to be sent when capabilities.tools.listChanged is false")
897+
default:
898+
// This is the expected case - no notification should be sent
899+
}
900+
901+
// Verify tool was added to session
902+
assert.Len(t, session.GetSessionTools(), 1)
903+
assert.Contains(t, session.GetSessionTools(), "test-tool")
904+
905+
// Delete the tool
906+
err = server.DeleteSessionTools(session.SessionID(), "test-tool")
907+
require.NoError(t, err)
908+
909+
// Verify no notification was sent
910+
select {
911+
case <-sessionChan:
912+
t.Error("Expected no notification to be sent when capabilities.tools.listChanged is false")
913+
default:
914+
// This is the expected case - no notification should be sent
915+
}
916+
917+
// Verify tool was deleted from session
918+
assert.Len(t, session.GetSessionTools(), 0)
919+
}

0 commit comments

Comments
 (0)