Skip to content

Commit 0811b7a

Browse files
committed
add warning to AddTools, testlogger formatted race condition to prevent log cluttering
1 parent 8f5b048 commit 0811b7a

File tree

4 files changed

+79
-2
lines changed

4 files changed

+79
-2
lines changed

server/server.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"sync"
1212

1313
"github.com/mark3labs/mcp-go/mcp"
14+
"github.com/mark3labs/mcp-go/util"
1415
)
1516

1617
// resourceEntry holds both a resource and its handler
@@ -160,6 +161,7 @@ type MCPServer struct {
160161
paginationLimit *int
161162
sessions sync.Map
162163
hooks *Hooks
164+
logger util.Logger
163165
}
164166

165167
// WithPaginationLimit sets the pagination limit for the server.
@@ -281,6 +283,12 @@ func WithLogging() ServerOption {
281283
}
282284
}
283285

286+
func WithMCPLogger(logger util.Logger) ServerOption {
287+
return func(s *MCPServer) {
288+
s.logger = logger
289+
}
290+
}
291+
284292
// WithInstructions sets the server instructions for the client returned in the initialize response
285293
func WithInstructions(instructions string) ServerOption {
286294
return func(s *MCPServer) {
@@ -308,6 +316,7 @@ func NewMCPServer(
308316
prompts: nil,
309317
logging: nil,
310318
},
319+
logger: util.DefaultLogger(),
311320
}
312321

313322
for _, opt := range opts {
@@ -474,6 +483,9 @@ func (s *MCPServer) AddTools(tools ...ServerTool) {
474483

475484
s.toolsMu.Lock()
476485
for _, entry := range tools {
486+
if _, exists := s.tools[entry.Tool.Name]; exists {
487+
s.logger.Infof("Tool with name %q already exists. Overwriting.", entry.Tool.Name)
488+
}
477489
s.tools[entry.Tool.Name] = entry
478490
}
479491
s.toolsMu.Unlock()

server/server_race_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package server
33
import (
44
"context"
55
"fmt"
6+
"math/rand"
67
"sync"
78
"testing"
89
"time"
@@ -56,7 +57,7 @@ func TestRaceConditions(t *testing.T) {
5657
})
5758

5859
runConcurrentOperation(&wg, testDuration, "add-tools", func() {
59-
name := fmt.Sprintf("tool-%d", time.Now().UnixNano())
60+
name := fmt.Sprintf("tool-%d-%d", time.Now().UnixNano(), rand.Int())
6061
srv.AddTool(mcp.Tool{
6162
Name: name,
6263
Description: "Test tool",

server/server_test.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package server
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/base64"
67
"encoding/json"
@@ -12,6 +13,7 @@ import (
1213
"time"
1314

1415
"github.com/mark3labs/mcp-go/mcp"
16+
"github.com/mark3labs/mcp-go/testutil"
1517
"github.com/stretchr/testify/assert"
1618
"github.com/stretchr/testify/require"
1719
)
@@ -342,11 +344,50 @@ func TestMCPServer_Tools(t *testing.T) {
342344
assert.Equal(t, "test-tool-2", tools[1].Name)
343345
},
344346
},
347+
{
348+
name: "AddTools overwrites tool with same name",
349+
action: func(t *testing.T, server *MCPServer, notificationChannel chan mcp.JSONRPCNotification) {
350+
err := server.RegisterSession(context.TODO(), &fakeSession{
351+
sessionID: "test",
352+
notificationChannel: notificationChannel,
353+
initialized: true,
354+
})
355+
require.NoError(t, err)
356+
server.AddTool(
357+
mcp.NewTool("test-tool-dup"),
358+
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
359+
return &mcp.CallToolResult{}, nil
360+
},
361+
)
362+
// Add same tool name with different handler or data to test overwrite
363+
server.AddTool(
364+
mcp.NewTool("test-tool-dup"),
365+
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
366+
return &mcp.CallToolResult{}, nil
367+
},
368+
)
369+
},
370+
expectedNotifications: 2, // one per AddTool with active session
371+
validate: func(t *testing.T, notifications []mcp.JSONRPCNotification, toolsList mcp.JSONRPCMessage) {
372+
// Both adds must have triggered notifications
373+
assert.Equal(t, mcp.MethodNotificationToolsListChanged, notifications[0].Method)
374+
assert.Equal(t, mcp.MethodNotificationToolsListChanged, notifications[1].Method)
375+
376+
tools := toolsList.(mcp.JSONRPCResponse).Result.(mcp.ListToolsResult).Tools
377+
assert.Len(t, tools, 1, "Expected only one tool after overwrite")
378+
assert.Equal(t, "test-tool-dup", tools[0].Name)
379+
},
380+
},
381+
345382
}
346383
for _, tt := range tests {
347384
t.Run(tt.name, func(t *testing.T) {
348385
ctx := context.Background()
349-
server := NewMCPServer("test-server", "1.0.0", WithToolCapabilities(true))
386+
387+
var buf bytes.Buffer
388+
logger := &testutil.TestLogger{Buf: &buf}
389+
390+
server := NewMCPServer("test-server", "1.0.0", WithToolCapabilities(true), WithMCPLogger(logger))
350391
_ = server.HandleMessage(ctx, []byte(`{
351392
"jsonrpc": "2.0",
352393
"id": 1,
@@ -373,6 +414,11 @@ func TestMCPServer_Tools(t *testing.T) {
373414
"method": "tools/list"
374415
}`))
375416
tt.validate(t, notifications, toolsList)
417+
418+
if tt.name == "AddTools overwrites tool with same name" {
419+
logOutput := buf.String()
420+
assert.Contains(t, logOutput, "already exists")
421+
}
376422
})
377423
}
378424
}

testutil/testlogger.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package testutil
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
)
7+
8+
type TestLogger struct {
9+
Buf *bytes.Buffer
10+
}
11+
12+
func (l *TestLogger) Infof(format string, v ...any) {
13+
fmt.Fprintf(l.Buf, "INFO: "+format, v...)
14+
}
15+
16+
func (l *TestLogger) Errorf(format string, v ...any) {
17+
fmt.Fprintf(l.Buf, "ERROR: "+format, v...)
18+
}

0 commit comments

Comments
 (0)