fix: sanitize MCP tool names to satisfy LLM provider constraints (#26539)

Fixes #26325

coderd/x/chatd/mcpclient/mcpclient.go

@@ -24,6 +24,7 @@ import (
 	"golang.org/x/xerrors"
 
 	"cdr.dev/slog/v3"
+	aidmcp "github.com/coder/coder/v2/aibridge/mcp"
 	"github.com/coder/coder/v2/buildinfo"
 	"github.com/coder/coder/v2/coderd/database"
 )
@@ -39,6 +40,15 @@ import (
 // directly when calling the remote server.
 const toolNameSep = "__"
 
+// truncateToolName caps the assembled tool name at MaxToolNameLen so
+// it fits within provider limits (e.g. OpenAI 64, Bedrock 128).
+func truncateToolName(name string) string {
+	if len(name) > aidmcp.MaxToolNameLen {
+		return name[:aidmcp.MaxToolNameLen]
+	}
+	return name
+}
+
 // connectTimeout bounds how long we wait for a single MCP server
 // to start its transport and complete initialization. Servers that
 // take longer are skipped so one slow server cannot block the
@@ -163,6 +173,31 @@ func ConnectAll(
 		)
 	})
 
+	// Warn about name collisions that may result from sanitization
+	// and truncation. When two tools resolve to the same name, the
+	// LLM tool-call dispatch map keeps only one, so the other
+	// becomes silently unreachable.
+	for i := 1; i < len(tools); i++ {
+		if tools[i-1].Info().Name == tools[i].Info().Name {
+			prevTool, ok := tools[i-1].(MCPToolIdentifier)
+			if !ok {
+				continue
+			}
+			currTool, ok := tools[i].(MCPToolIdentifier)
+			if !ok {
+				continue
+			}
+			if prevTool.MCPServerConfigID() != currTool.MCPServerConfigID() {
+				logger.Warn(ctx,
+					"duplicate tool name after sanitization; one tool will be unreachable",
+					slog.F("tool_name", tools[i].Info().Name),
+					slog.F("prev_config_id", prevTool.MCPServerConfigID()),
+					slog.F("curr_config_id", currTool.MCPServerConfigID()),
+				)
+			}
+		}
+	}
+
 	return tools, cleanup
 }
 
@@ -520,7 +555,7 @@ func newMCPTool(
 ) *mcpToolWrapper {
 	return &mcpToolWrapper{
 		configID:     configID,
-		prefixedName: serverSlug + toolNameSep + tool.Name,
+		prefixedName: truncateToolName(aidmcp.SanitizeToolName(serverSlug) + toolNameSep + aidmcp.SanitizeToolName(tool.Name)),
 		originalName: tool.Name,
 		description:  tool.Description,
 		parameters:   tool.InputSchema.Properties,

coderd/x/chatd/mcpclient/mcpclient_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"net/http/httptest"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -113,6 +114,69 @@ func TestConnectAll_DiscoverTools(t *testing.T) {
 	assert.Equal(t, "Echoes the input", echoInfo.Description)
 }
 
+func TestConnectAll_SanitizesDottedSlug(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	ts := newTestMCPServer(t, echoTool())
+
+	// Use a dotted slug like awslabs.* MCP servers ship with.
+	// Dots violate Bedrock's tool name pattern ^[a-zA-Z0-9_-]{1,128}$.
+	cfg := makeConfig("awslabs.aws-documentation-mcp-server", ts.URL)
+	tools, cleanup := mcpclient.ConnectAll(ctx, logger, []database.MCPServerConfig{cfg}, nil, uuid.Nil, nil, nil)
+	t.Cleanup(cleanup)
+
+	require.Len(t, tools, 1)
+
+	// Dots in the slug must be replaced with underscores.
+	names := toolNames(tools)
+	assert.Equal(t, []string{"awslabs_aws-documentation-mcp-server__echo"}, names)
+
+	// The tool should still be callable; the original name is
+	// used when contacting the remote MCP server.
+	resp, err := tools[0].Run(ctx, fantasy.ToolCall{
+		ID:    "call-1",
+		Name:  "awslabs_aws-documentation-mcp-server__echo",
+		Input: `{"input":"hello"}`,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "echo: hello", resp.Content)
+}
+
+func TestConnectAll_TruncationCollisionWarning(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	// Two servers whose slugs differ only in a trailing suffix.
+	// After sanitization + truncation to 64 chars, both produce
+	// the same prefixed tool name, triggering a collision warning.
+	// slug (65) + "__" (2) + "echo" (4) = 71 chars; truncated to
+	// 64 chops the suffix and tool name entirely.
+	base := strings.Repeat("a", 64)
+	slug1 := base + "x"
+	slug2 := base + "y"
+
+	ts := newTestMCPServer(t, echoTool())
+
+	cfg1 := makeConfig(slug1, ts.URL)
+	cfg2 := makeConfig(slug2, ts.URL)
+
+	tools, cleanup := mcpclient.ConnectAll(
+		ctx, logger,
+		[]database.MCPServerConfig{cfg1, cfg2},
+		nil, uuid.Nil, nil, nil,
+	)
+	t.Cleanup(cleanup)
+
+	// Both tools should be present (the caller decides policy),
+	// but their names collide after truncation.
+	require.Len(t, tools, 2)
+	assert.Equal(t, tools[0].Info().Name, tools[1].Info().Name,
+		"truncated names should collide")
+}
+
 func TestConnectAll_CallTool(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()