fix(mcp): reject repeated tool discovery cursors
Refs: https://github.com/deepseek-ai/deepseek-harness/discussions/3660
This commit is contained in:
1 parent
96a66ba478
commit
594305ce19
13 files changed
+180
-8
No files matched your search
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-07-mcp-client-plugin.md
|
||||
2026-07-07-mcp-client-plugin.md: 1c451e14ea2a7c494454956ce9bb09d97d4e1e1d
|
||||
2026-07-07-mcp-client-plugin.zh.md: 78a086d6c218a254e850777fe4024973e8848e17
|
||||
2026-07-07-mcp-client-plugin.md: 91c1bf428973eae7935e9c2a14dda7028a734313
|
||||
2026-07-07-mcp-client-plugin.zh.md: c0c2dbae5b854734240f8035edfc8d0b5a33408f
|
||||
@@ -96,12 +96,14 @@ Every MCP tool has two names:
|
||||
|
||||
This server-qualified shape is the de-facto standard among multi-server agent clients — every surveyed end-user product qualifies MCP tools by server ([Claude Code](https://code.claude.com/docs/en/agent-sdk/mcp#tool-naming-convention) `mcp__github__list_issues`, [Codex](https://openai.com/index/unrolling-the-codex-agent-loop/) `mcp__weather__get-forecast`, [Gemini CLI](https://geminicli.com/docs/tools/mcp-server/#3-tool-naming-and-namespaces), [VS Code](https://github.com/microsoft/vscode/blob/ab9ec62c6a61e429a9abd612ff220c3f4834c9ea/src/vs/workbench/contrib/mcp/common/mcpServer.ts#L217-L260), [Cline](https://github.com/cline/cline/blob/52fdbb1d72f7324a28142a7ba7678d4b53c902f4/sdk/packages/core/src/extensions/mcp/name-transform.ts#L20-L35), [Roo Code](https://github.com/RooCodeInc/Roo-Code/blob/b867ec9145750d0ae1ff7f02d35406e9bf2a0b16/src/utils/mcp-name.ts#L117-L140), [Goose](https://github.com/block/goose/blob/b3a012cbdde854b0fe14f95b1c48543bf6517c0a/crates/goose/src/agents/extension_manager.rs#L1391-L1441), [OpenCode](https://github.com/anomalyco/opencode/blob/d199b1bff90282a4f9cd6251b5fc7b16875a52f6/packages/opencode/src/mcp/catalog.ts#L117-L120)); the exact `mcp__<server>__<tool>` spelling follows Claude Code and Codex. The `mcp__` marker keeps MCP registrations out of the native tools' namespace and gives permission/telemetry rules a stable shape (`mcp__*`, `mcp__github__*`).
|
||||
|
||||
1. On connect: drain `client.listTools()` pagination, derive every tool's `publicName`, then register each as a raw `ToolDefinition` via `ctx.tools.register()`. The MCP JSON Schema and description pass through unchanged (no `defineTool` DSL conversion); only the model-facing `name` is replaced.
|
||||
1. On connect: drain uncached `tools/list` pagination, derive every tool's `publicName`, then register each as a raw `ToolDefinition` via `ctx.tools.register()`. The MCP JSON Schema and description pass through unchanged (no `defineTool` DSL conversion); only the model-facing `name` is replaced.
|
||||
2. Listen for `notifications/tools/list_changed` → re-run the same sync (dispose previous generation, register new). Deterministic names mean unchanged tools keep their names across re-syncs.
|
||||
3. The executor closes over `rawName`; the public name is never sent to the server and never parsed to recover the raw name.
|
||||
4. No `presentCall`/`presentResult` — UI consumers use the provider-neutral generic-card fallback.
|
||||
5. Tools are transparent in the system prompt — no "[via MCP]" annotation beyond the name itself.
|
||||
|
||||
Each synchronization rejects a repeated non-empty continuation cursor before requesting another page, retaining the previous tool generation. Empty pages cannot establish progress through tool-name uniqueness, so cursor history also detects cycles spanning several pages ([reported failure](https://github.com/deepseek-ai/deepseek-harness/discussions/3660)). Cursor history belongs to one synchronization: a later update may reuse the same cursors. Focused bridge and lifecycle tests cover cycle rejection, retained callable tools, strict startup failure, and notification recovery. This detects repeated cursors; it does not bound a server that continually returns distinct cursors.
|
||||
|
||||
### Public name normalization
|
||||
|
||||
MCP allows tool names up to 128 characters including `.`; the DeepSeek function-name contract allows `[A-Za-z0-9_-]` and at most 64. Public names are normalized deterministically: invalid characters become `_`, and when replacement or truncation changed the name, a 12-hex-char SHA-256 hash of the `(serverName, rawName)` identity is appended so distinct MCP identities can never collapse into the same public name:
|
||||
|
||||
@@ -96,12 +96,14 @@ type Config = StdioConfig | StreamableHttpConfig
|
||||
|
||||
这种按服务器限定的形式是多服务器 agent 客户端事实上的标准——所有被调研的终端用户产品都按服务器限定 MCP 工具名([Claude Code](https://code.claude.com/docs/en/agent-sdk/mcp#tool-naming-convention) `mcp__github__list_issues`、[Codex](https://openai.com/index/unrolling-the-codex-agent-loop/) `mcp__weather__get-forecast`、[Gemini CLI](https://geminicli.com/docs/tools/mcp-server/#3-tool-naming-and-namespaces)、[VS Code](https://github.com/microsoft/vscode/blob/ab9ec62c6a61e429a9abd612ff220c3f4834c9ea/src/vs/workbench/contrib/mcp/common/mcpServer.ts#L217-L260)、[Cline](https://github.com/cline/cline/blob/52fdbb1d72f7324a28142a7ba7678d4b53c902f4/sdk/packages/core/src/extensions/mcp/name-transform.ts#L20-L35)、[Roo Code](https://github.com/RooCodeInc/Roo-Code/blob/b867ec9145750d0ae1ff7f02d35406e9bf2a0b16/src/utils/mcp-name.ts#L117-L140)、[Goose](https://github.com/block/goose/blob/b3a012cbdde854b0fe14f95b1c48543bf6517c0a/crates/goose/src/agents/extension_manager.rs#L1391-L1441)、[OpenCode](https://github.com/anomalyco/opencode/blob/d199b1bff90282a4f9cd6251b5fc7b16875a52f6/packages/opencode/src/mcp/catalog.ts#L117-L120));`mcp__<server>__<tool>` 的拼写方式与 Claude Code 和 Codex 一致。`mcp__` 前缀将 MCP 注册与原生工具的命名空间隔离,并为权限/遥测规则提供稳定的匹配模式(`mcp__*`、`mcp__github__*`)。
|
||||
|
||||
1. 连接时:遍历 `client.listTools()` 的分页结果,推导每个工具的 `publicName`,然后通过 `ctx.tools.register()` 将其注册为原始 `ToolDefinition`。MCP 的 JSON Schema 和描述原样透传(不做 `defineTool` DSL 转换);仅替换模型可见的 `name`。
|
||||
1. 连接时:遍历未缓存的 `tools/list` 分页结果,推导每个工具的 `publicName`,然后通过 `ctx.tools.register()` 将其注册为原始 `ToolDefinition`。MCP 的 JSON Schema 和描述原样透传(不做 `defineTool` DSL 转换);仅替换模型可见的 `name`。
|
||||
2. 监听 `notifications/tools/list_changed` → 重新执行同步(dispose 上一代、注册新一代)。确定性命名意味着未变化的工具在重新同步后保持原名。
|
||||
3. 执行器闭包持有 `rawName`;公开名称永远不发送给服务器,也永远不被解析以还原原始名称。
|
||||
4. 无 `presentCall`/`presentResult`——UI 消费方使用提供方无关的通用卡片兜底。
|
||||
5. 工具在系统提示词中是透明的——除名称本身外不附加「[via MCP]」标注。
|
||||
|
||||
每次同步在请求下一页前拒绝重复的非空续传游标,并保留上一代工具。空页无法通过工具名唯一性证明分页在前进,因此游标记录还会检测跨越多页的循环([问题报告](https://github.com/deepseek-ai/deepseek-harness/discussions/3660))。游标记录只属于一次同步:后续更新可以复用相同游标。定向桥接与生命周期测试覆盖循环拒绝、保留可调用工具、严格启动失败及通知恢复。此机制检测重复游标;它不限制持续返回不同游标的服务器。
|
||||
|
||||
### 公开名称规范化
|
||||
|
||||
MCP 允许工具名最长 128 字符且可包含 `.`;DeepSeek 的函数名约定允许 `[A-Za-z0-9_-]` 且最多 64 字符。公开名称按确定性规则规范化:非法字符替换为 `_`,当替换或截断改变了名称时,追加 `(serverName, rawName)` 标识的 12 位十六进制 SHA-256 hash,确保不同的 MCP 标识永远不会坍缩为同一个公开名称:
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
Error: mcp-client(pagination-cycle): server repeated a tools/list continuation cursor — invalid tool list
|
||||
@@ -0,0 +1,30 @@
|
||||
/** Startup diagnostics through the shipped headless profile and real MCP stdio transport. */
|
||||
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { expect, it } from 'vitest'
|
||||
import { LOADER_SMOKE_TEST_TIMEOUT_MS, runLoaderSmoke } from '@deepseek-ai/dsh-loader-smoke'
|
||||
|
||||
const fixtureRoot = new URL('../../../../../../packages/mcp/mcp-client/tests/fixtures/', import.meta.url)
|
||||
const configPath = fileURLToPath(new URL('repeated-cursor.patch.yml', fixtureRoot))
|
||||
const expectedPath = fileURLToPath(new URL('./expected/mcp-pagination/stderr-cause.txt', import.meta.url))
|
||||
|
||||
it('reports a repeated MCP discovery cursor and exits before starting a turn', async () => {
|
||||
const { stdout, stderr } = await runLoaderSmoke({
|
||||
label: 'MCP discovery pagination cycle',
|
||||
tempDirPrefix: 'dsh-mcp-pagination-',
|
||||
binScript: fileURLToPath(new URL('../../../../src/bin.ts', import.meta.url)),
|
||||
libBinScript: fileURLToPath(new URL('../../../../lib/bin.js', import.meta.url)),
|
||||
configPath,
|
||||
binArgs: ['--profile', 'headless', '--patch', configPath, 'unreachable task'],
|
||||
tsconfigPath: fileURLToPath(new URL('../../../../../../tsconfig.json', import.meta.url)),
|
||||
expectedExitCode: 1,
|
||||
env: {
|
||||
DSH_MCP_PAGINATION_FIXTURE: fileURLToPath(new URL('repeated-cursor-server.ts', fixtureRoot)),
|
||||
DSH_TELEMETRY_DISABLED: '1',
|
||||
},
|
||||
})
|
||||
expect(stdout).toBe('')
|
||||
expect(stderr).toContain('initial connection or tool synchronization failed')
|
||||
const cause = stderr.split('\n').find(line => line.startsWith('Error: mcp-client(pagination-cycle):'))
|
||||
await expect(`${cause}\n`).toMatchFileSnapshot(expectedPath)
|
||||
}, LOADER_SMOKE_TEST_TIMEOUT_MS)
|
||||
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/mcp/mcp-client/README.md
|
||||
README.md: 112f477556d1b8ee2788c9b96251b6969c617f62
|
||||
README.zh.md: e642c7e130510cd717342a2e879c26396c77d55d
|
||||
README.md: de646256888f1c271ad94a4c863338dfb02551e6
|
||||
README.zh.md: 2c71060ec309e823ff3456207881e27ab9aca553
|
||||
@@ -76,6 +76,7 @@ The model sees each tool under a stable server-qualified name: `mcp__<serverName
|
||||
- Two servers publishing the same tool name (for example `search`) coexist under their own namespaces.
|
||||
- Two entries using the same server name: the later one fails to load with a clear error.
|
||||
- A server that lists the same tool twice gets its tool list rejected as invalid, and the previous tool set stays active.
|
||||
- A repeated non-empty `tools/list` continuation cursor rejects that update immediately, including cycles through empty pages; the previous tool set stays active and later updates can still succeed.
|
||||
- An update that conflicts with an already-registered tool name is rejected entirely — you never get a partial tool set from that server.
|
||||
|
||||
### Calling tools and reading results
|
||||
|
||||
@@ -76,6 +76,7 @@ kind: "package-reference"
|
||||
- 发布相同工具名称(例如 `search`)的两个服务器会在各自的 namespace 下共存。
|
||||
- 两条配置项使用相同的服务器名称时,后加载的一条会在加载时以明确错误失败。
|
||||
- 服务器在工具列表中两次列出同一工具时,其工具列表会被作为无效列表拒绝,上一组工具保持可用。
|
||||
- `tools/list` 返回重复的非空续传游标时会立即拒绝本次更新,包括经过空页的循环;上一组工具保持可用,后续更新仍可成功。
|
||||
- 工具更新与已有工具名称冲突时,该更新会被整体拒绝——绝不会得到该服务器的部分工具集。
|
||||
|
||||
### 调用工具与读取结果
|
||||
|
||||
@@ -124,8 +124,8 @@ export function publicToolName(serverName: string, rawName: string): string {
|
||||
*
|
||||
* 1. Fetch: drain uncached `tools/list` 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.
|
||||
* (network error, duplicate raw name, repeated continuation cursor) 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
|
||||
@@ -149,6 +149,7 @@ export async function syncTools(
|
||||
): Promise<ToolDisposers> {
|
||||
// Phase 1: fetch and build the next generation without touching the registry.
|
||||
const definitions = new Map<string, ToolDefinition>()
|
||||
const seenCursors = new Set<string>()
|
||||
let cursor: string | undefined
|
||||
do {
|
||||
const response = await listToolsUncached(client, cursor)
|
||||
@@ -172,6 +173,14 @@ export async function syncTools(
|
||||
))
|
||||
}
|
||||
cursor = response.nextCursor
|
||||
if (cursor) {
|
||||
if (seenCursors.has(cursor)) {
|
||||
throw new Error(
|
||||
`mcp-client(${opts.serverName}): server repeated a tools/list continuation cursor — invalid tool list`,
|
||||
)
|
||||
}
|
||||
seenCursors.add(cursor)
|
||||
}
|
||||
} while (cursor)
|
||||
|
||||
// Phase 2: swap generations.
|
||||
|
||||
@@ -303,6 +303,28 @@ describe('apply (plugin lifecycle)', () => {
|
||||
expect(mockClose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects strict startup on a repeated discovery cursor and closes the client', async () => {
|
||||
mockListTools
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockRejectedValue(new Error('pagination continued after the repeated cursor'))
|
||||
try {
|
||||
await expect(apply(ctx, {
|
||||
...stdioConfig,
|
||||
failOnStartupError: true,
|
||||
reconnect: { enabled: false },
|
||||
})).rejects.toMatchObject({
|
||||
message: 'mcp-client(srv): initial connection or tool synchronization failed',
|
||||
cause: new Error('mcp-client(srv): server repeated a tools/list continuation cursor — invalid tool list'),
|
||||
})
|
||||
expect(mockListTools).toHaveBeenCalledTimes(2)
|
||||
expect(mockClose).toHaveBeenCalledTimes(1)
|
||||
expect(ctx.tools.schemas()).toEqual([])
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
it('preserves strict startup registration when list_changed arrives before connect resolves', async () => {
|
||||
ctx.tools.register({
|
||||
name: 'mcp__srv__remote',
|
||||
@@ -358,6 +380,31 @@ describe('apply (plugin lifecycle)', () => {
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeDefined()
|
||||
})
|
||||
|
||||
it('continues notification synchronization after rejecting a pagination cycle', async () => {
|
||||
try {
|
||||
await apply(ctx, stdioConfig)
|
||||
const handler = mockSetNotificationHandler.mock.calls[0]![1] as () => Promise<void>
|
||||
mockListTools
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockRejectedValue(new Error('pagination continued after the repeated cursor'))
|
||||
|
||||
await handler()
|
||||
expect(mockListTools).toHaveBeenCalledTimes(3)
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeDefined()
|
||||
|
||||
mockListTools
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockResolvedValueOnce({ tools: [{ name: 'updated', inputSchema: { type: 'object' } }], nextCursor: undefined })
|
||||
await handler()
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeUndefined()
|
||||
expect(ctx.tools.get('mcp__srv__updated')).toBeDefined()
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
expect(mockClose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('effect disposer unregisters the CURRENT generation and closes client', async () => {
|
||||
// Load through ctx.plugin so ONLY the plugin's fiber is disposed — the
|
||||
// registry must survive to observe the unregistration.
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
/** MCP wire fixture returning a repeated discovery cursor over empty pages. */
|
||||
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
|
||||
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
|
||||
import { ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js'
|
||||
|
||||
const server = new McpServer(
|
||||
{ name: 'repeated-cursor', version: '1.0.0' },
|
||||
{ capabilities: { tools: {} } },
|
||||
)
|
||||
let requests = 0
|
||||
server.server.setRequestHandler(ListToolsRequestSchema, () => {
|
||||
// Bound the broken-client path without relying on a test timeout to kill it.
|
||||
if (++requests > 2) throw new Error('pagination continued after the repeated cursor')
|
||||
return { tools: [], nextCursor: 'same-cursor' }
|
||||
})
|
||||
|
||||
await server.connect(new StdioServerTransport())
|
||||
@@ -0,0 +1,17 @@
|
||||
- id: headless-startup
|
||||
disabled: true
|
||||
|
||||
- id: headless-runner
|
||||
disabled: true
|
||||
|
||||
- insert:
|
||||
- id: mcp-pagination-cycle
|
||||
name: '@deepseek-ai/dsh-mcp-client'
|
||||
config:
|
||||
transport: stdio
|
||||
serverName: pagination-cycle
|
||||
command: !!js process.execPath
|
||||
args: !!js '[process.env.DSH_MCP_PAGINATION_FIXTURE]'
|
||||
failOnStartupError: true
|
||||
reconnect:
|
||||
enabled: false
|
||||
@@ -309,6 +309,50 @@ describe('syncTools', () => {
|
||||
expect(ctx.tools.get('mcp__srv__page2')).toBeDefined()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['immediate', ['cursor1', 'cursor1']],
|
||||
['multi-page', ['cursor1', 'cursor2', 'cursor1']],
|
||||
])('rejects a pagination cycle through empty pages (%s)', async (_kind, cursors) => {
|
||||
const client = createMockClient([])
|
||||
client.listTools.mockRejectedValue(new Error('pagination continued after the repeated cursor'))
|
||||
for (const nextCursor of cursors) {
|
||||
client.listTools.mockResolvedValueOnce({ tools: [], nextCursor })
|
||||
}
|
||||
|
||||
await expect(syncTools(client as never, ctx, defaultOpts, new Map()))
|
||||
.rejects.toThrow('mcp-client(srv): server repeated a tools/list continuation cursor — invalid tool list')
|
||||
expect(client.listTools).toHaveBeenCalledTimes(cursors.length)
|
||||
expect(ctx.tools.schemas()).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps callable tools after a pagination cycle and accepts a later complete list', async () => {
|
||||
const client = createMockClient([{ name: 'stable', inputSchema: { type: 'object' } }])
|
||||
const previous = await syncTools(client as never, ctx, defaultOpts, new Map())
|
||||
const stable = ctx.tools.get('mcp__srv__stable')
|
||||
client.listTools
|
||||
.mockResolvedValueOnce({ tools: [{ name: 'partial', inputSchema: { type: 'object' } }], nextCursor: 'cursor1' })
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockRejectedValue(new Error('pagination continued after the repeated cursor'))
|
||||
|
||||
await expect(syncTools(client as never, ctx, defaultOpts, previous)).rejects.toThrow(/repeated.*cursor/)
|
||||
expect(client.listTools).toHaveBeenCalledTimes(3)
|
||||
expect(ctx.tools.get('mcp__srv__stable')).toBe(stable)
|
||||
expect(ctx.tools.get('mcp__srv__partial')).toBeUndefined()
|
||||
const result = await ctx.tools.execute({
|
||||
signal: testToolSignal, callId: ToolCallId('pagination-retained'), name: 'mcp__srv__stable', arguments: {},
|
||||
})
|
||||
expect(result.isError).toBe(false)
|
||||
expect(result.content).toEqual([{ type: 'text', text: 'ok' }])
|
||||
|
||||
client.listTools
|
||||
.mockResolvedValueOnce({ tools: [], nextCursor: 'cursor1' })
|
||||
.mockResolvedValueOnce({ tools: [{ name: 'recovered', inputSchema: { type: 'object' } }], nextCursor: undefined })
|
||||
const recovered = await syncTools(client as never, ctx, defaultOpts, previous)
|
||||
expect([...recovered.keys()]).toEqual(['mcp__srv__recovered'])
|
||||
expect(ctx.tools.get('mcp__srv__stable')).toBeUndefined()
|
||||
expect(client.listTools).toHaveBeenLastCalledWith({ cursor: 'cursor1' })
|
||||
})
|
||||
|
||||
it('owns output validation independently of the SDK per-page cache', async () => {
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
|
||||
serverTransport.onmessage = (message) => {
|
||||
|
||||
Reference in new issue
Block a user