fix: improve MCP tool name collision safety and registry overwrite warning
- MCPTool.Name(): append FNV-32a hash of original (unsanitized) server+tool names whenever sanitization is lossy or total length exceeds 64 chars, ensuring names that differ only in disallowed characters remain distinct - ToolRegistry.Register(): emit warn log when a tool registration overwrites an existing tool with the same name, making collisions observable - scripts/test-docker-mcp.sh: switch shebang from #/bin/bash /Users/yuchou/Work/klook-calendar/klook-google-cal-sync/src/googlecalconversrv/bin/start.sh to # for portability on minimal distros and Nix environments
This commit is contained in:
parent
0eec640c37
commit
a2591e03a9
3 changed files with 35 additions and 4 deletions
|
|
@ -4,6 +4,7 @@ import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"hash/fnv"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/modelcontextprotocol/go-sdk/mcp"
|
"github.com/modelcontextprotocol/go-sdk/mcp"
|
||||||
|
|
@ -90,12 +91,37 @@ func sanitizeIdentifierComponent(s string) string {
|
||||||
return result
|
return result
|
||||||
}
|
}
|
||||||
|
|
||||||
// Name returns the tool name, prefixed with the server name
|
// Name returns the tool name, prefixed with the server name.
|
||||||
|
// The total length is capped at 64 characters (OpenAI-compatible API limit).
|
||||||
|
// A short hash of the original (unsanitized) server and tool names is appended
|
||||||
|
// whenever sanitization is lossy or the name is truncated, ensuring that two
|
||||||
|
// names which differ only in disallowed characters remain distinct after sanitization.
|
||||||
func (t *MCPTool) Name() string {
|
func (t *MCPTool) Name() string {
|
||||||
// Prefix with server name to avoid conflicts, and sanitize components
|
// Prefix with server name to avoid conflicts, and sanitize components
|
||||||
sanitizedServer := sanitizeIdentifierComponent(t.serverName)
|
sanitizedServer := sanitizeIdentifierComponent(t.serverName)
|
||||||
sanitizedTool := sanitizeIdentifierComponent(t.tool.Name)
|
sanitizedTool := sanitizeIdentifierComponent(t.tool.Name)
|
||||||
return fmt.Sprintf("mcp_%s_%s", sanitizedServer, sanitizedTool)
|
full := fmt.Sprintf("mcp_%s_%s", sanitizedServer, sanitizedTool)
|
||||||
|
|
||||||
|
// Check if sanitization was lossless (only lowercasing, no char replacement/truncation)
|
||||||
|
lossless := strings.ToLower(t.serverName) == sanitizedServer &&
|
||||||
|
strings.ToLower(t.tool.Name) == sanitizedTool
|
||||||
|
|
||||||
|
const maxTotal = 64
|
||||||
|
if lossless && len(full) <= maxTotal {
|
||||||
|
return full
|
||||||
|
}
|
||||||
|
|
||||||
|
// Sanitization was lossy or name too long: append hash of the ORIGINAL names
|
||||||
|
// (not the sanitized names) so different originals always yield different hashes.
|
||||||
|
h := fnv.New32a()
|
||||||
|
_, _ = h.Write([]byte(t.serverName + "\x00" + t.tool.Name))
|
||||||
|
suffix := fmt.Sprintf("%08x", h.Sum32()) // 8 chars
|
||||||
|
|
||||||
|
base := full
|
||||||
|
if len(base) > maxTotal-9 {
|
||||||
|
base = strings.TrimRight(full[:maxTotal-9], "_")
|
||||||
|
}
|
||||||
|
return base + "_" + suffix
|
||||||
}
|
}
|
||||||
|
|
||||||
// Description returns the tool description
|
// Description returns the tool description
|
||||||
|
|
|
||||||
|
|
@ -25,7 +25,12 @@ func NewToolRegistry() *ToolRegistry {
|
||||||
func (r *ToolRegistry) Register(tool Tool) {
|
func (r *ToolRegistry) Register(tool Tool) {
|
||||||
r.mu.Lock()
|
r.mu.Lock()
|
||||||
defer r.mu.Unlock()
|
defer r.mu.Unlock()
|
||||||
r.tools[tool.Name()] = tool
|
name := tool.Name()
|
||||||
|
if _, exists := r.tools[name]; exists {
|
||||||
|
logger.WarnCF("tools", "Tool registration overwrites existing tool",
|
||||||
|
map[string]any{"name": name})
|
||||||
|
}
|
||||||
|
r.tools[name] = tool
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *ToolRegistry) Get(name string) (Tool, bool) {
|
func (r *ToolRegistry) Get(name string) (Tool, bool) {
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
#!/bin/bash
|
#!/bin/sh
|
||||||
# Test script for MCP tools in Docker (full-featured image)
|
# Test script for MCP tools in Docker (full-featured image)
|
||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue