fix(coderd/x/chatd): sanitize workspace MCP tool names - #26928
Merged
Conversation
Workspace MCP tools take their model-facing name from the server key
joined with the tool name as serverName__toolName. That name reached
the model unsanitized, so a server or tool name with a character
outside ^[a-zA-Z0-9_-]{1,128}$ (for example "@") produced an invalid
tool name and Anthropic/Bedrock rejected the whole request with HTTP
400, failing the entire turn.
Sanitize and length-cap the model-facing name while keeping the
original serverName__toolName as the routing name the workspace agent
uses to reach the original server and tool. This matches the remote
MCP (mcpclient) and AI Gateway (aibridge/mcp) paths.
To avoid a fourth copy of the sanitize/truncate/dedupe logic, extract
two shared helpers into aibridge/mcp:
- SanitizeAndTruncateToolName: sanitize then cap at MaxToolNameLen.
- DisambiguateToolName: resolve post-sanitization collisions with a
numeric suffix so every tool stays addressable in the model's
name-keyed dispatch map.
The workspace path and mcpclient both use the helpers. mcpclient now
disambiguates colliding names instead of only logging that one tool
will be unreachable.
Alternative fix for #26853, which sanitizes only the
workspace path with path-local helpers.
Drop the shared aibridge/mcp helper and the mcpclient changes from the previous commit. Sanitizing/truncating and collision disambiguation now live entirely in coderd/x/chatd/chattool, so the fix touches neither the aibridge package nor the remote MCP client. The workspace path defines its own provider-safe sanitizer (mirroring the ^[a-zA-Z0-9_-] limit), carrying no new cross-package dependency.
kylecarbs
changed the title
kylecarbs
enabled auto-merge (squash)
July 1, 2026 18:29
ibetitsmike
approved these changes
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Workspace MCP tools (servers a workspace declares in
.mcp.json) take their model-facing name from the server key joined with the tool name asserverName__toolName. That name reached the model unsanitized, so a server or tool name containing a character outside^[a-zA-Z0-9_-]{1,128}$(for example@) produced an invalid tool name. Anthropic and Bedrock reject the whole request withHTTP 400:which fails the entire turn, not just the one tool. The remote MCP path (
mcpclient) and the AI Gateway path (aibridge/mcp) already sanitize; the workspace path did not.Alternative to #26853 (thanks @ibdafna for the report and repro).
Fix
Sanitize and length-cap the model-facing name, and keep the original
serverName__toolNameas aroutingNamethe workspace agent uses to reach the original server and tool.NewWorkspaceMCPToolsbuilds a whole set and disambiguates names that collide after sanitization (for example server keysfoo.barandfoo_barboth exposingecho) so every tool stays addressable in the model's name-keyed dispatch map. Names already within the allowed set are unchanged, so there is no behavior change for valid names.The sanitizer is local to
coderd/x/chatd/chattool; the fix does not touch theaibridgepackage or the remote MCP client.Changes
coderd/x/chatd/chattool/mcpworkspace.go: local provider-safe sanitizer + length cap,routingNamefor the agent proxy, andNewWorkspaceMCPToolsfor set-level collision disambiguation.coderd/x/chatd/chatd.go: build the pinned workspace tool set viaNewWorkspaceMCPTools.Why sanitize here (not at
.mcp.json/ agent parse)?The agent uses
serverName__toolNameto route to the real downstream server (it splits on__and calls the original tool name), so sanitizing at parse time would break routing or merely relocate the original->sanitized mapping. Sanitization is also a provider constraint the agent has no knowledge of, and coderd/agent version skew means coderd must sanitize at its own boundary regardless. The model-facing boundary in chatd is the right place.Test plan
@in a name is sanitized for the model while the original routes to the agent; a valid name is unchanged; an over-length name is truncated; colliding names in a set are disambiguated while each still routes to its own original name.go build,go vet,golangci-lint, andgo test ./coderd/x/chatd/chattool/...pass locally.Design notes / decision log
Constraint that drives the design. The tool name is both the identifier shown to the model (and the key the model layer dispatches tool calls by) and, for the workspace path, the string the agent splits on
__to route back to the original server and tool. Those roles conflict once sanitization changes the name, so the name is sanitized for the model while the unsanitized form is kept asroutingName.Options considered.
chattool. Smallest blast radius; no new cross-package dependency. This matches the shape of the other MCP paths (mcpclientkeepsoriginalName+configID) without sharing code..mcp.jsonparse time or in the agent. Rejected: breaks routing (the agent needs the original name), pushes a provider concern into the agent, and coderd must still defend its own boundary because the agent and coderd version independently. Tool names also come from the downstream server at list time, not from.mcp.json, so parsing cannot fully validate them.aibridge/mcpand adopt it inmcpclienttoo (so the remote path also gains collision disambiguation). This DRYs all paths, but it grows chatd's coupling to theaibridgesubsystem and expands scope/behavior/tests in the remote path for what is a workspace-path bug. Left out deliberately to keep this change minimal and self-contained; it can be a separate refactor.Notes.
[^a-zA-Z0-9_-]->_) and amaxModelToolNameLen = 64constant that mirrors the strictest provider limit (OpenAI 64, Bedrock 128), rather than importingaibridge/mcp, so it carries no new dependency.Opened by Coder Agents on behalf of @kylecarbs. Alternative to #26853.