mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(repository-plugin): reject incomplete MCP publication
This commit is contained in:
@@ -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: c87255917a2def0aef938af9f2c910b65d5a96d5
|
||||
README.zh.md: 6fd6df39d7c5034795021d41de57f8500cbfd1c5
|
||||
README.md: 76d1271f6f7a3e9c959bdcf5e969906f25563c56
|
||||
README.zh.md: 49de996863ab16a46cd7ee82b13624523dbb853f
|
||||
|
||||
@@ -44,7 +44,7 @@ The model sees `mcp__github__create_issue`, `mcp__web__search`, … — the same
|
||||
| `url` | http | yes | MCP server URL |
|
||||
| `headers` | http | no | Extra headers (e.g. auth tokens) |
|
||||
| `toolCallTimeoutMs` | both | no | Timeout per `callTool` invocation (default 60000) |
|
||||
| `failOnStartupError` | both | no | Reject plugin activation when the initial connection or tool discovery fails (default `false`) |
|
||||
| `failOnStartupError` | both | no | Reject plugin activation when initial connection or tool synchronization fails (default `false`) |
|
||||
|
||||
## Tool naming
|
||||
|
||||
@@ -57,8 +57,8 @@ Every MCP tool has two names: the raw MCP name (sent on the wire in `tools/call`
|
||||
|
||||
## Behavior
|
||||
|
||||
- On connect: plugin activation awaits `listTools()` and registers each tool via `ctx.tools.register()` under its public name before the composition starts its first turn. Initial connection or discovery failure is always logged; it rejects activation when `failOnStartupError` is true and otherwise activates with no tools.
|
||||
- Listens for `notifications/tools/list_changed` → re-syncs; a failed re-sync keeps the previous generation registered.
|
||||
- On connect: plugin activation awaits `listTools()` and registers each tool via `ctx.tools.register()` under its public name before the composition starts its first turn. Initial connection, discovery, or registration failure is always logged; it rejects activation when `failOnStartupError` is true and otherwise activates with no tools.
|
||||
- Listens for `notifications/tools/list_changed` → re-syncs; a fetch-phase failure keeps the previous generation registered, while a registration conflict rolls back the attempted generation and leaves no tools from that server.
|
||||
- Tool execute: `client.callTool({ name: rawName, arguments }, { signal })` with timeout + abort support—the public name is never sent to the server.
|
||||
- Canonical success is `{ content: JsonValue[], structuredContent? }`; complete JSON MCP blocks survive for programmatic callers. A supported advertised `outputSchema` validates `structuredContent`; unsupported schema vocabulary falls back to unconstrained `JsonValue`.
|
||||
- Native/model rendering keeps the existing text projection: text blocks join with newlines while image, audio, resource, and unsupported blocks become placeholders.
|
||||
@@ -103,6 +103,7 @@ Append-only; newly visible content follows the reusable request prefix and does
|
||||
## Known Limitations and Deferred Work
|
||||
|
||||
- **Tools are the only bridged MCP capability** — Resources and Prompts have no harness consumption surface and are deferred.
|
||||
- **Startup timeout is inherited from the MCP SDK** — DSH does not yet expose a connection/discovery timeout. Each initialize or paginated `tools/list` request uses the SDK's 60-second default, so an unresponsive server or cursor chain can delay both activation and teardown while the initial synchronization settles.
|
||||
- **Crash recovery is manual** — transport closure does not auto-reconnect; registered tools can remain visible but fail against the closed transport until an HMR reload or Host restart.
|
||||
- **Native non-text rendering is lossy** — image, audio, and resource payloads become placeholders in model context even though the execution-local canonical value preserves their JSON blocks. Richer Native multimedia projection is deferred.
|
||||
- **Unsupported MCP output schemas are not enforced** — `structuredContent` falls back to `JsonValue` when the advertised schema uses vocabulary outside the harness subset.
|
||||
|
||||
@@ -44,7 +44,7 @@ MCP 客户端桥接插件:连接外部 [Model Context Protocol](https://modelc
|
||||
| `url` | http | 是 | MCP 服务器 URL |
|
||||
| `headers` | http | 否 | 额外标头(例如认证 token) |
|
||||
| `toolCallTimeoutMs` | 两者 | 否 | 每次 `callTool` 调用的超时(默认 60000) |
|
||||
| `failOnStartupError` | 两者 | 否 | 初始连接或工具发现失败时拒绝插件激活(默认 `false`) |
|
||||
| `failOnStartupError` | 两者 | 否 | 初始连接或工具同步失败时拒绝插件激活(默认 `false`) |
|
||||
|
||||
## 工具命名
|
||||
|
||||
@@ -57,8 +57,8 @@ MCP 客户端桥接插件:连接外部 [Model Context Protocol](https://modelc
|
||||
|
||||
## 行为
|
||||
|
||||
- 连接时:插件激活会等待 `listTools()`,并在组合开始首个轮次前通过 `ctx.tools.register()` 以公开名称注册每个工具。初始连接或发现失败始终会记录日志;`failOnStartupError` 为 true 时拒绝激活,否则插件仍会激活但不注册工具。
|
||||
- 监听 `notifications/tools/list_changed` → 重新同步;同步失败时保留上一世代的注册。
|
||||
- 连接时:插件激活会等待 `listTools()`,并在组合开始首个轮次前通过 `ctx.tools.register()` 以公开名称注册每个工具。初始连接、发现或注册失败始终会记录日志;`failOnStartupError` 为 true 时拒绝激活,否则插件仍会激活但不注册工具。
|
||||
- 监听 `notifications/tools/list_changed` → 重新同步;获取阶段失败时保留上一世代的注册,注册冲突则会回滚本次尝试的世代,并且不保留该服务器的任何工具。
|
||||
- 工具执行:`client.callTool({ name: rawName, arguments }, { signal })`,支持超时 + 中止;公开名称绝不会发给服务器。
|
||||
- 规范成功值是 `{ content: JsonValue[], structuredContent? }`;完整的 JSON MCP 块会保留给编程调用方。受支持且已声明的 `outputSchema` 会验证 `structuredContent`;不受支持的 schema 词汇会回退为不受约束的 `JsonValue`。
|
||||
- Native/模型渲染保留现有文本投影:文本块以换行连接,图片、音频、资源和不受支持的块会变成占位符。
|
||||
@@ -103,6 +103,7 @@ MCP 客户端桥接插件:连接外部 [Model Context Protocol](https://modelc
|
||||
## 已知限制与暂缓事项
|
||||
|
||||
- **只桥接 MCP 的工具能力**:资源和提示词没有 harness 消费接口,暂缓实现。
|
||||
- **启动超时继承自 MCP SDK**:DSH 尚未公开连接/发现超时。每次 initialize 请求或分页 `tools/list` 请求都使用 SDK 默认的 60 秒,因此在初始同步完成期间,无响应的 server 或 cursor chain 可能同时延迟激活与 teardown。
|
||||
- **崩溃恢复需要手动触发**:传输关闭后不会自动重新连接;已注册工具可能仍然可见,但会因传输已关闭而调用失败,直到 HMR 重载或重启 Host。
|
||||
- **Native 非文本渲染有损**:图片、音频与资源载荷在模型上下文中会变成占位符,即使执行局部的规范值保留了其 JSON 块。更丰富的 Native 多媒体投影暂缓实现。
|
||||
- **不强制执行不受支持的 MCP 输出 schema**:已声明 schema 使用 harness 子集之外的词汇时,`structuredContent` 会回退到 `JsonValue`。
|
||||
|
||||
@@ -72,7 +72,7 @@ export interface StdioConfig {
|
||||
cwd: string
|
||||
/** Per-tool-call timeout in milliseconds. */
|
||||
toolCallTimeoutMs: number
|
||||
/** Fail plugin activation when the initial connection or tool discovery fails. */
|
||||
/** Fail plugin activation when the initial connection or tool synchronization fails. */
|
||||
failOnStartupError: boolean
|
||||
}
|
||||
|
||||
@@ -92,7 +92,7 @@ export interface StreamableHttpConfig {
|
||||
headers: Record<string, string>
|
||||
/** Per-tool-call timeout in milliseconds. */
|
||||
toolCallTimeoutMs: number
|
||||
/** Fail plugin activation when the initial connection or tool discovery fails. */
|
||||
/** Fail plugin activation when the initial connection or tool synchronization fails. */
|
||||
failOnStartupError: boolean
|
||||
}
|
||||
|
||||
@@ -155,6 +155,7 @@ export async function apply(ctx: Context, config: Config): Promise<void> {
|
||||
)
|
||||
|
||||
const opts = {
|
||||
registrationFailure: 'contain' as const,
|
||||
serverName: config.serverName,
|
||||
toolCallTimeoutMs: config.toolCallTimeoutMs,
|
||||
}
|
||||
@@ -166,7 +167,10 @@ export async function apply(ctx: Context, config: Config): Promise<void> {
|
||||
const ready = (async () => {
|
||||
await client.connect(transport)
|
||||
|
||||
let disposers = await syncTools(client, ctx, opts, new Map())
|
||||
let disposers = await syncTools(client, ctx, {
|
||||
...opts,
|
||||
registrationFailure: config.failOnStartupError ? 'throw' : 'contain',
|
||||
}, new Map())
|
||||
|
||||
client.setNotificationHandler(
|
||||
ToolListChangedNotificationSchema,
|
||||
@@ -184,7 +188,7 @@ export async function apply(ctx: Context, config: Config): Promise<void> {
|
||||
|
||||
return { getDisposers: () => disposers }
|
||||
})().catch((error: unknown) => {
|
||||
ctx.logger.error(`mcp-client(${config.serverName}): failed to connect: ${String(error)}`)
|
||||
ctx.logger.error(`mcp-client(${config.serverName}): startup failed: ${String(error)}`)
|
||||
return { getDisposers: () => new Map<string, () => void>(), error }
|
||||
})
|
||||
|
||||
@@ -196,6 +200,6 @@ export async function apply(ctx: Context, config: Config): Promise<void> {
|
||||
|
||||
const outcome = await ready
|
||||
if ('error' in outcome && config.failOnStartupError) {
|
||||
throw new Error(`mcp-client(${config.serverName}): initial connection or tool discovery failed`, { cause: outcome.error })
|
||||
throw new Error(`mcp-client(${config.serverName}): initial connection or tool synchronization failed`, { cause: outcome.error })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,6 +23,8 @@ import type { JsonSchemaNode, JsonValue } from '@deepseek-ai/dsh-tools'
|
||||
|
||||
/** Resolved options relevant to tool bridging. */
|
||||
export interface ToolBridgeOptions {
|
||||
/** Whether a registry conflict is contained or rejects this synchronization. */
|
||||
registrationFailure: 'contain' | 'throw'
|
||||
serverName: string
|
||||
toolCallTimeoutMs: number
|
||||
}
|
||||
@@ -111,8 +113,9 @@ export function publicToolName(serverName: string, rawName: string): string {
|
||||
* 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.
|
||||
* rolled back (zero tools from this server) and logged. Initial strict
|
||||
* synchronization may propagate the conflict so its parent transaction
|
||||
* rejects; ordinary clients and later re-syncs return an empty map.
|
||||
*
|
||||
* @param client - Connected MCP Client instance used to list and call tools.
|
||||
* @param ctx - Cordis context providing the `tools` service for registration.
|
||||
@@ -164,6 +167,7 @@ export async function syncTools(
|
||||
// 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)}`)
|
||||
if (opts.registrationFailure === 'throw') throw error
|
||||
return new Map()
|
||||
}
|
||||
return disposers
|
||||
|
||||
@@ -229,7 +229,7 @@ describe('apply (plugin lifecycle)', () => {
|
||||
await expect(apply(ctx, {
|
||||
...stdioConfig,
|
||||
failOnStartupError: true,
|
||||
})).rejects.toThrow('initial connection or tool discovery failed')
|
||||
})).rejects.toThrow('initial connection or tool synchronization failed')
|
||||
|
||||
expect(mockListTools).not.toHaveBeenCalled()
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeUndefined()
|
||||
@@ -237,6 +237,28 @@ describe('apply (plugin lifecycle)', () => {
|
||||
expect(mockClose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects strict startup when the initial tool generation cannot be registered', async () => {
|
||||
ctx.tools.register({
|
||||
name: 'mcp__srv__remote',
|
||||
description: 'Foreign squatter',
|
||||
parameters: { type: 'object' },
|
||||
output: {
|
||||
schema: { type: 'string' },
|
||||
render: (_args, value) => [{ type: 'text', text: value as string }],
|
||||
},
|
||||
execute: async () => 'foreign',
|
||||
})
|
||||
|
||||
await expect(apply(ctx, {
|
||||
...stdioConfig,
|
||||
failOnStartupError: true,
|
||||
})).rejects.toThrow('initial connection or tool synchronization failed')
|
||||
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeDefined()
|
||||
await ctx.fiber.dispose()
|
||||
expect(mockClose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('re-syncs tools on ToolListChanged notification', async () => {
|
||||
await apply(ctx, stdioConfig)
|
||||
|
||||
|
||||
@@ -64,6 +64,7 @@ async function mountRegistry(): Promise<Context> {
|
||||
}
|
||||
|
||||
const defaultOpts: ToolBridgeOptions = {
|
||||
registrationFailure: 'contain',
|
||||
serverName: 'srv',
|
||||
toolCallTimeoutMs: 60_000,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user