Compare commits

..
1 Commits
Author SHA1 Message Date
ybc❤️zyrand猷诺 a652b90fd4 fix: resolve cross-product tool routing collision (issue #219) (#220)
When two MCP servers register tools with the same name (e.g. both drive
and doc have 'create_folder'), the tool-level endpoint map uses
last-writer-wins, causing invocations to route to the wrong server.

Fix: swap Priority 1 (product-level) and Priority 2 (tool-level) in
directRuntimeEndpoint so that when the caller already knows the
productID, the product endpoint is authoritative. Tool-level lookup
remains as a fallback for cases where productID is empty.

This fixes 'dws drive mkdir' and 'dws drive download' being silently
routed to the doc MCP server instead of the drive MCP server.

Closes #219

Co-authored-by: 猷诺 <bicheng.ybc@alibaba-inc.com>
2026-05-05 16:21:51 +08:00
2 changed files with 126 additions and 8 deletions
+15 -8
View File
@@ -227,14 +227,11 @@ func directRuntimeEndpoint(productID, toolName string) (string, bool) {
te := dynamicToolEndpoints
dynamicMu.RUnlock()
// Priority 1: tool-level endpoint (resolves multi-endpoint products).
if tool := strings.TrimSpace(toolName); tool != "" && te != nil {
if endpoint, ok := te[tool]; ok {
return endpoint, true
}
}
// Priority 2: product-level endpoint.
// Priority 1: product-level endpoint.
// When the caller already knows the productID (e.g. "drive"), the product
// endpoint is authoritative. This prevents cross-product tool name
// collisions (e.g. both "drive" and "doc" register "create_folder") from
// routing the request to the wrong MCP server. See issue #219.
for _, candidate := range []string{strings.TrimSpace(productID), normalized} {
if candidate == "" {
continue
@@ -246,6 +243,16 @@ func directRuntimeEndpoint(productID, toolName string) (string, bool) {
}
}
// Priority 2: tool-level endpoint (fallback for unknown productID).
// This path is used when the caller does not know the productID but has a
// tool name, e.g. in helper invocations or plugin routes where only the
// tool name is available.
if tool := strings.TrimSpace(toolName); tool != "" && te != nil {
if endpoint, ok := te[tool]; ok {
return endpoint, true
}
}
// Priority 3: built-in PAT fallback for cold-start paths that run before
// discovery/plugin registration has populated the dynamic registry.
for _, candidate := range []string{strings.TrimSpace(productID), normalized} {
@@ -181,3 +181,114 @@ func TestAppendDynamicServer_ServerOverrideDoesNotHijackToolEndpoint(t *testing.
})
}
}
// --- Issue #219 regression tests: cross-product tool name collision ---
//
// When two different products register tools with the same name (e.g. drive
// and doc both have "create_folder"), the product-level endpoint must win
// when the caller already knows the productID. Otherwise the tool-level map
// (last-writer-wins) routes the invocation to the wrong MCP server.
const (
testDriveEndpoint = "https://mcp-gw.dingtalk.com/server/drive-hash"
testDocEndpoint = "https://mcp-gw.dingtalk.com/server/doc-hash"
)
func driveDescriptor() market.ServerDescriptor {
return market.ServerDescriptor{
Endpoint: testDriveEndpoint,
CLI: market.CLIOverlay{
ID: "drive",
Command: "drive",
ToolOverrides: map[string]market.CLIToolOverride{
"create_folder": {CLIName: "mkdir"},
"list_files": {CLIName: "list"},
"download_file": {CLIName: "download"},
"get_upload_info": {CLIName: "upload-info"},
},
},
}
}
func docDescriptor() market.ServerDescriptor {
return market.ServerDescriptor{
Endpoint: testDocEndpoint,
CLI: market.CLIOverlay{
ID: "doc",
Command: "doc",
ToolOverrides: map[string]market.CLIToolOverride{
"create_folder": {CLIName: "create", Group: "folder"},
"download_file": {CLIName: "download"},
"search_documents": {CLIName: "search"},
"list_nodes": {CLIName: "list"},
},
},
}
}
// TestDirectRuntimeEndpoint_ProductLevelWinsOverConflictingToolLevel verifies
// that when productID is known and has a registered endpoint, the product-level
// endpoint is used even if the tool-level map points to a different server
// (due to same-name tool collision). This is the core fix for issue #219.
func TestDirectRuntimeEndpoint_ProductLevelWinsOverConflictingToolLevel(t *testing.T) {
tests := []struct {
name string
servers []market.ServerDescriptor
}{
{
name: "drive first, doc second",
servers: []market.ServerDescriptor{driveDescriptor(), docDescriptor()},
},
{
name: "doc first, drive second",
servers: []market.ServerDescriptor{docDescriptor(), driveDescriptor()},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
withCleanDynamicRegistry(t)
SetDynamicServers(tc.servers)
// Drive tools must always route to drive's endpoint regardless of
// registration order — productID "drive" is known.
assertEndpoint(t, "drive", "create_folder", testDriveEndpoint)
assertEndpoint(t, "drive", "download_file", testDriveEndpoint)
assertEndpoint(t, "drive", "list_files", testDriveEndpoint)
assertEndpoint(t, "drive", "get_upload_info", testDriveEndpoint)
// Doc tools must always route to doc's endpoint.
assertEndpoint(t, "doc", "create_folder", testDocEndpoint)
assertEndpoint(t, "doc", "download_file", testDocEndpoint)
assertEndpoint(t, "doc", "search_documents", testDocEndpoint)
assertEndpoint(t, "doc", "list_nodes", testDocEndpoint)
// Product-level fallback (no tool name) still works.
assertEndpoint(t, "drive", "", testDriveEndpoint)
assertEndpoint(t, "doc", "", testDocEndpoint)
})
}
}
// TestDirectRuntimeEndpoint_ToolLevelFallbackWhenProductUnknown verifies that
// tool-level routing still works as a fallback when productID is empty or has
// no registered endpoint (the original design intent for tool-level Priority 1).
func TestDirectRuntimeEndpoint_ToolLevelFallbackWhenProductUnknown(t *testing.T) {
withCleanDynamicRegistry(t)
SetDynamicServers([]market.ServerDescriptor{driveDescriptor(), docDescriptor()})
// When productID is empty, tool-level endpoint is the only option.
// The actual endpoint depends on registration order (last-writer-wins),
// but the lookup must succeed.
endpoint, ok := directRuntimeEndpoint("", "create_folder")
if !ok {
t.Fatal("directRuntimeEndpoint(\"\", \"create_folder\") returned ok=false, want ok=true")
}
if endpoint != testDriveEndpoint && endpoint != testDocEndpoint {
t.Fatalf("directRuntimeEndpoint(\"\", \"create_folder\") = %q, want one of drive/doc endpoints", endpoint)
}
// Unique tools (no collision) still resolve via tool-level.
assertEndpoint(t, "", "search_documents", testDocEndpoint)
assertEndpoint(t, "", "get_upload_info", testDriveEndpoint)
}