mcp/tool: duplicate tools should not error (#295)

This CL causes the dup to fail.

Author: KamdynS

mcp/mcp_test.go

@@ -937,4 +937,39 @@ func TestKeepAliveFailure(t *testing.T) {
 	t.Errorf("expected connection to be closed by keepalive, but it wasn't. Last error: %v", err)
 }
 
+func TestAddTool_DuplicateNoPanicAndNoDuplicate(t *testing.T) {
+	// Adding the same tool pointer twice should not panic and should not
+	// produce duplicates in the server's tool list.
+	_, cs := basicConnection(t, func(s *Server) {
+		// Use two distinct Tool instances with the same name but different
+		// descriptions to ensure the second replaces the first
+		// This case was written specifically to reproduce a bug where duplicate tools where causing jsonschema errors
+		t1 := &Tool{Name: "dup", Description: "first", InputSchema: &jsonschema.Schema{}}
+		t2 := &Tool{Name: "dup", Description: "second", InputSchema: &jsonschema.Schema{}}
+		s.AddTool(t1, nopHandler)
+		s.AddTool(t2, nopHandler)
+	})
+	defer cs.Close()
+
+	ctx := context.Background()
+	res, err := cs.ListTools(ctx, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var count int
+	var gotDesc string
+	for _, tt := range res.Tools {
+		if tt.Name == "dup" {
+			count++
+			gotDesc = tt.Description
+		}
+	}
+	if count != 1 {
+		t.Fatalf("expected exactly one 'dup' tool, got %d", count)
+	}
+	if gotDesc != "second" {
+		t.Fatalf("expected replaced tool to have description %q, got %q", "second", gotDesc)
+	}
+}
+
 var testImpl = &Implementation{Name: "test", Version: "v1.0.0"}

mcp/server.go

@@ -171,6 +171,15 @@ func AddTool[In, Out any](s *Server, t *Tool, h ToolHandlerFor[In, Out]) {
 
 func addToolErr[In, Out any](s *Server, t *Tool, h ToolHandlerFor[In, Out]) (err error) {
 	defer util.Wrapf(&err, "adding tool %q", t.Name)
+	// If the exact same Tool pointer has already been registered under this name,
+	// avoid rebuilding schemas and re-registering. This prevents duplicate
+	// registration from causing errors (and unnecessary work).
+	s.mu.Lock()
+	if existing, ok := s.tools.get(t.Name); ok && existing.tool == t {
+		s.mu.Unlock()
+		return nil
+	}
+	s.mu.Unlock()
 	st, err := newServerTool(t, h)
 	if err != nil {
 		return err