feat(mcp): adopt mainstream server-qualified MCP tool naming
Research across 8 multi-server agent clients (Claude Code, Codex, Gemini
CLI, VS Code, Cline, Roo Code, Goose, OpenCode) showed all of them keep
the server namespace in model-facing MCP tool names; the RFC's premise
for raw names ("servers already prefix their tools") is false for the
official GitHub/filesystem/Sentry servers.
- Config: drop toolPrefix; require serverName ([A-Za-z0-9_-]{1,32}),
duplicate serverName fails the later instance at load (per-root
reservation, released on dispose)
- Names: always mcp__<serverName>__<rawName>; normalize to the DeepSeek
64-char [A-Za-z0-9_-] contract with a deterministic 12-hex identity
hash on lossy normalization; raw name is the only thing sent on the
wire (tools/call)
- Sync: two-phase fetch/swap — fetch failure keeps the previous
generation; a swap conflict rolls back the whole generation (never a
partial set); duplicate raw names reject the tool list
- RFC: moved to implemented/ (status + skeleton rewritten per the
format contract), naming design + tier-level test coverage recorded
- Tests: naming algorithm unit suite; keyless Streamable HTTP e2e
against an in-process StreamableHTTPServerTransport (namespace
discovery, execution, per-request auth headers); dotted-name
normalization e2e via a new fixture tool
This commit is contained in:
@@ -1,37 +1,87 @@
|
||||
/**
|
||||
* Tool bridge: discovers MCP tools, registers them on the harness ToolRegistry,
|
||||
* and handles re-sync when the server's tool list changes.
|
||||
* Tool bridge: discovers MCP tools, registers them on the harness ToolRegistry
|
||||
* under deterministic server-qualified public names, and handles re-sync when
|
||||
* the server's tool list changes.
|
||||
*
|
||||
* Naming contract (see the mcp-client RFC "Naming invariants"): every MCP tool
|
||||
* has the stable identity `(serverName, rawName)`; the model-facing public name
|
||||
* is `mcp__<serverName>__<rawName>`, normalized to the DeepSeek function-name
|
||||
* constraints. The raw name is only ever sent on the wire (`tools/call`); the
|
||||
* public name is never parsed to recover it.
|
||||
*
|
||||
* @module
|
||||
*/
|
||||
|
||||
import { createHash } from 'node:crypto'
|
||||
import type { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
||||
import type { Context } from 'cordis'
|
||||
import type { ToolDefinition, ToolExecution } from '@deepseek-ai/dsh-tools'
|
||||
|
||||
/** Resolved options relevant to tool bridging. */
|
||||
export interface ToolBridgeOptions {
|
||||
toolPrefix: string
|
||||
serverName: string
|
||||
toolCallTimeoutMs: number
|
||||
}
|
||||
|
||||
/** State for one sync generation: the current set of disposers keyed by tool name. */
|
||||
type ToolDisposers = Map<string, () => void>
|
||||
/** State for one sync generation: the current set of disposers keyed by public name. */
|
||||
export type ToolDisposers = Map<string, () => void>
|
||||
|
||||
/**
|
||||
* DeepSeek function-name contract: at most 64 characters. Wire-protocol
|
||||
* constant, not configuration.
|
||||
*/
|
||||
const MAX_PUBLIC_NAME_LENGTH = 64
|
||||
|
||||
/** DeepSeek function-name contract: only `[A-Za-z0-9_-]` is allowed. */
|
||||
const INVALID_NAME_CHARS = /[^A-Za-z0-9_-]/g
|
||||
|
||||
/** Hex chars of the SHA-256 identity hash appended on lossy normalization. */
|
||||
const HASH_LENGTH = 12
|
||||
|
||||
/**
|
||||
* Derive the model-facing public name for one MCP tool.
|
||||
*
|
||||
* Deterministic pure function of `(serverName, rawName)`: the clean case is
|
||||
* `mcp__<serverName>__<rawName>` verbatim. When character replacement or
|
||||
* truncation to the DeepSeek function-name contract (64 chars,
|
||||
* `[A-Za-z0-9_-]`) changes the name, a 12-hex-char SHA-256 hash of the
|
||||
* identity is appended so distinct MCP identities never collapse into the
|
||||
* same public name.
|
||||
*
|
||||
* @param serverName - Stable local namespace from plugin config.
|
||||
* @param rawName - The MCP server's own tool name.
|
||||
* @returns The globally unique, model-facing ToolRegistry name.
|
||||
*/
|
||||
export function publicToolName(serverName: string, rawName: string): string {
|
||||
const joined = `mcp__${serverName}__${rawName}`
|
||||
const normalized = joined.replace(INVALID_NAME_CHARS, '_')
|
||||
if (normalized === joined && normalized.length <= MAX_PUBLIC_NAME_LENGTH) return normalized
|
||||
const hash = createHash('sha256').update(`${serverName}\0${rawName}`).digest('hex').slice(0, HASH_LENGTH)
|
||||
return `${normalized.slice(0, MAX_PUBLIC_NAME_LENGTH - HASH_LENGTH - 1)}_${hash}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Sync the MCP server's tool list into the harness ToolRegistry.
|
||||
*
|
||||
* - Calls `client.listTools()` (paginated: drains all pages).
|
||||
* - Registers each tool as a raw `ToolDefinition`.
|
||||
* - On name conflict: logs a warning and skips that tool.
|
||||
* - Returns a disposer map; call each value to unregister.
|
||||
* Two phases keep the swap safe:
|
||||
*
|
||||
* 1. Fetch: drain `client.listTools()` pagination and build the full next
|
||||
* generation of `ToolDefinition`s under public names. Any failure here
|
||||
* (network error, duplicate raw name in the server's list) rejects and
|
||||
* leaves the previous generation registered untouched.
|
||||
* 2. Swap: dispose the previous generation, register the new one. A registry
|
||||
* conflict here can only mean a foreign registration squats on this
|
||||
* server's `mcp__<serverName>__` namespace — the partial generation is
|
||||
* rolled back (zero tools from this server), the error is logged, and an
|
||||
* empty map is returned.
|
||||
*
|
||||
* @param client - Connected MCP Client instance used to list and call tools.
|
||||
* @param ctx - Cordis context providing the `tools` service for registration.
|
||||
* @param opts - Bridge options: tool name prefix and per-call timeout.
|
||||
* @param previous - Disposer map from a prior sync generation; all entries are
|
||||
* disposed before re-registering.
|
||||
* @returns A map of registered tool names to their unregister disposers.
|
||||
* @param opts - Bridge options: server namespace and per-call timeout.
|
||||
* @param previous - Disposer map from the prior sync generation; disposed
|
||||
* during the swap phase (only after the fetch phase succeeded).
|
||||
* @returns A map of registered public tool names to their unregister
|
||||
* disposers — the exact set of live registrations owned by this server.
|
||||
*/
|
||||
export async function syncTools(
|
||||
client: Client,
|
||||
@@ -39,32 +89,43 @@ export async function syncTools(
|
||||
opts: ToolBridgeOptions,
|
||||
previous: ToolDisposers,
|
||||
): Promise<ToolDisposers> {
|
||||
for (const dispose of previous.values()) dispose()
|
||||
|
||||
const disposers: ToolDisposers = new Map()
|
||||
|
||||
// Phase 1: fetch and build the next generation without touching the registry.
|
||||
const definitions = new Map<string, ToolDefinition>()
|
||||
let cursor: string | undefined
|
||||
do {
|
||||
const response = await client.listTools(cursor ? { cursor } : undefined)
|
||||
for (const tool of response.tools) {
|
||||
const registeredName = opts.toolPrefix + tool.name
|
||||
const definition: ToolDefinition = {
|
||||
name: registeredName,
|
||||
const publicName = publicToolName(opts.serverName, tool.name)
|
||||
if (definitions.has(publicName)) {
|
||||
throw new Error(
|
||||
`mcp-client(${opts.serverName}): server listed tool "${tool.name}" more than once — invalid tool list`,
|
||||
)
|
||||
}
|
||||
definitions.set(publicName, {
|
||||
name: publicName,
|
||||
description: tool.description ?? '',
|
||||
parameters: tool.inputSchema,
|
||||
execute: createExecutor(client, tool.name, opts),
|
||||
}
|
||||
try {
|
||||
const dispose = ctx.tools.register(definition)
|
||||
disposers.set(registeredName, dispose)
|
||||
} catch {
|
||||
// Name conflict — another tool with this name is already registered.
|
||||
ctx.logger.warn(`mcp-client: skipping tool "${registeredName}" (name conflict)`)
|
||||
}
|
||||
})
|
||||
}
|
||||
cursor = response.nextCursor
|
||||
} while (cursor)
|
||||
|
||||
// Phase 2: swap generations.
|
||||
for (const dispose of previous.values()) dispose()
|
||||
const disposers: ToolDisposers = new Map()
|
||||
try {
|
||||
for (const [publicName, definition] of definitions) {
|
||||
disposers.set(publicName, ctx.tools.register(definition))
|
||||
}
|
||||
} catch (error) {
|
||||
// A conflict on an `mcp__<serverName>__`-qualified name means a foreign
|
||||
// registration occupies this server's namespace. Roll back so the model
|
||||
// sees either the full generation or none of it — never a partial set.
|
||||
for (const dispose of disposers.values()) dispose()
|
||||
ctx.logger.error(`mcp-client(${opts.serverName}): tool registration failed, no tools registered: ${String(error)}`)
|
||||
return new Map()
|
||||
}
|
||||
return disposers
|
||||
}
|
||||
|
||||
@@ -81,16 +142,17 @@ interface McpContentBlock {
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an execute function for one MCP tool. The executor calls
|
||||
* `client.callTool` with abort signal and timeout, then maps the result
|
||||
* to harness ContentBlocks.
|
||||
* Create an execute function for one MCP tool. The executor closes over the
|
||||
* raw MCP tool name and calls `client.callTool` with it (never the public
|
||||
* name), with abort signal and timeout, then maps the result to harness
|
||||
* ContentBlocks.
|
||||
*
|
||||
* When the MCP server returns `isError: true`, the executor throws so that
|
||||
* the ToolRegistry's catch path produces an `isError` result for the model.
|
||||
*/
|
||||
function createExecutor(
|
||||
client: Client,
|
||||
mcpToolName: string,
|
||||
rawName: string,
|
||||
opts: ToolBridgeOptions,
|
||||
): ToolDefinition['execute'] {
|
||||
return async (args: unknown, exec: ToolExecution) => {
|
||||
@@ -100,7 +162,7 @@ function createExecutor(
|
||||
// specific "missing required param" error the model can learn from.
|
||||
const argsObj = (typeof args === 'object' && args !== null ? args : {}) as Record<string, unknown>
|
||||
const result = await client.callTool(
|
||||
{ name: mcpToolName, arguments: argsObj },
|
||||
{ name: rawName, arguments: argsObj },
|
||||
undefined,
|
||||
{
|
||||
...exec.signal ? { signal: exec.signal } : {},
|
||||
@@ -122,7 +184,7 @@ function createExecutor(
|
||||
// with optional fallbacks).
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const content: McpContentBlock[] = result.content
|
||||
const text = extractText(content, mcpToolName)
|
||||
const text = extractText(content, rawName)
|
||||
|
||||
// MCP isError → throw so ToolRegistry produces an isError result for the model.
|
||||
if ('isError' in result && result.isError === true) {
|
||||
|
||||
Reference in New Issue
Block a user