Coder/deepseek-harness
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tools): preserve canonical output boundaries

Browse Source
This commit is contained in:
Tianyi Cui
2026-07-21 18:03:01 +08:00
parent 8f3aca4128
commit e1633fbc3f
33 changed files with 269 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
packages/mcp/mcp-client/package.json
View File

@@ -28,14 +28,14 @@
},
"dependencies": {
"@modelcontextprotocol/sdk": "^1.12.0",
"schemastery": "^3.18.0"
"schemastery": "^3.18.0",
"zod": "^4.4.3"
},
"devDependencies": {
"@deepseek-ai/dsh-tools": "workspace:^",
"@deepseek-ai/dsh-llm": "workspace:^",
"@modelcontextprotocol/server-everything": "^2026.7.4",
"@modelcontextprotocol/server-filesystem": "^2026.7.4",
"cordis": "^4.0.0-rc.7",
"zod": "^4.4.3"
"cordis": "^4.0.0-rc.7"
}
}

69
packages/mcp/mcp-client/src/tools.ts
View File

@@ -14,6 +14,8 @@
import { createHash } from 'node:crypto'
import type { Client } from '@modelcontextprotocol/sdk/client/index.js'
import { ListToolsResultSchema } from '@modelcontextprotocol/sdk/types.js'
import { z } from 'zod'
import type { Context } from 'cordis'
import type { ToolDefinition, ToolExecution } from '@deepseek-ai/dsh-tools'
import { assertSupportedJsonSchema } from '@deepseek-ai/dsh-tools'
@@ -46,6 +48,35 @@ 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
/** Raw result record: the bridge owns JSON-value validation after transport. */
const RawCallToolResultSchema = z.record(z.string(), z.unknown())
/** List without mutating the SDK's per-page output-validator cache. */
function listToolsUncached(client: Client, cursor?: string) {
return client.request(
{ method: 'tools/list', ...cursor === undefined ? {} : { params: { cursor } } },
ListToolsResultSchema,
)
}
/** Call without the SDK pre-validating an output schema the bridge may not support. */
function callToolUncached(
client: Client,
rawName: string,
args: Record<string, unknown>,
exec: ToolExecution,
opts: ToolBridgeOptions,
) {
return client.request(
{ method: 'tools/call', params: { name: rawName, arguments: args } },
RawCallToolResultSchema,
{
...exec.signal ? { signal: exec.signal } : {},
timeout: opts.toolCallTimeoutMs,
},
)
}
/**
* Derive the model-facing public name for one MCP tool.
*
@@ -73,7 +104,7 @@ export function publicToolName(serverName: string, rawName: string): string {
*
* Two phases keep the swap safe:
*
* 1. Fetch: drain `client.listTools()` pagination and build the full next
* 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.
@@ -101,7 +132,7 @@ export async function syncTools(
const definitions = new Map<string, ToolDefinition>()
let cursor: string | undefined
do {
const response = await client.listTools(cursor ? { cursor } : undefined)
const response = await listToolsUncached(client, cursor)
for (const tool of response.tools) {
const publicName = publicToolName(opts.serverName, tool.name)
if (definitions.has(publicName)) {
@@ -114,7 +145,7 @@ export async function syncTools(
description: tool.description ?? '',
parameters: tool.inputSchema,
output: createOutput(tool.name, supportedOutputSchema(tool.outputSchema)),
execute: createExecutor(client, tool.name, opts),
execute: createExecutor(client, tool.name, tool.execution?.taskSupport === 'required', opts),
})
}
cursor = response.nextCursor
@@ -170,7 +201,7 @@ function createOutput(rawName: string, structuredSchema: JsonSchemaNode | undefi
content: { type: 'array', items: {} },
structuredContent: structuredSchema ?? {},
},
required: ['content'],
required: structuredSchema === undefined ? ['content'] : ['content', 'structuredContent'],
additionalProperties: false,
},
render(_args, value) {
@@ -182,9 +213,10 @@ function createOutput(rawName: string, structuredSchema: JsonSchemaNode | undefi
/**
* 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.
* raw MCP tool name and sends an uncached `tools/call` request with it (never
* the public name), with abort signal and timeout, then maps the result to
* harness ContentBlocks. Owning the raw request prevents the SDK's internal
* per-page schema cache from pre-validating a different contract.
*
* When the MCP server returns `isError: true`, the executor throws so that
* the ToolRegistry's catch path produces an `isError` result for the model.
@@ -192,33 +224,30 @@ function createOutput(rawName: string, structuredSchema: JsonSchemaNode | undefi
function createExecutor(
client: Client,
rawName: string,
taskRequired: boolean,
opts: ToolBridgeOptions,
): ToolDefinition['execute'] {
return async (args: unknown, exec: ToolExecution) => {
if (taskRequired) {
throw new Error(`Tool "${rawName}" requires task-based execution, which this bridge does not support`)
}
// The agent loop passes `JSON.parse(model_arguments)` which is usually an
// object, but can be any JSON value if the model misbehaves (outputs a bare
// string/number/null). Fallback to {} lets the MCP server produce a
// 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: rawName, arguments: argsObj },
undefined,
{
...exec.signal ? { signal: exec.signal } : {},
timeout: opts.toolCallTimeoutMs,
},
)
const result = await callToolUncached(client, rawName, argsObj, exec, opts)
// The SDK may return a legacy `toolResult` shape; normalize to content array.
if (!('content' in result) || !Array.isArray(result.content)) {
if (!Array.isArray(result.content)) {
const rendered: unknown = 'toolResult' in result
? JSON.stringify(result.toolResult)
: '(no output)'
const text = typeof rendered === 'string' ? rendered : '(no output)'
if ('isError' in result && result.isError === true) throw new Error(text)
if (result.isError === true) throw new Error(text)
return {
content: [{ type: 'text', text }],
...'structuredContent' in result && result.structuredContent !== undefined
...result.structuredContent !== undefined
? { structuredContent: result.structuredContent as JsonValue }
: {},
}
@@ -232,13 +261,13 @@ function createExecutor(
const text = extractText(content, rawName)
// MCP isError → throw so ToolRegistry produces an isError result for the model.
if ('isError' in result && result.isError === true) {
if (result.isError === true) {
throw new Error(text)
}
return {
content,
...'structuredContent' in result && result.structuredContent !== undefined
...result.structuredContent !== undefined
? { structuredContent: result.structuredContent as JsonValue }
: {},
}

113
packages/mcp/mcp-client/tests/mcp-client.spec.ts
View File

@@ -1,4 +1,6 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
import { Context } from 'cordis'
import { CallId } from '@deepseek-ai/dsh-llm'
import SystemPrompt from '@deepseek-ai/dsh-system-prompt'
@@ -14,6 +16,7 @@ interface MockTool {
description?: string
inputSchema: Record<string, unknown>
outputSchema?: Record<string, unknown>
execution?: { taskSupport?: 'optional' | 'required' | 'forbidden' }
}
interface MockCallResult {
@@ -23,9 +26,26 @@ interface MockCallResult {
}
function createMockClient(tools: MockTool[], callResult: MockCallResult = { content: [{ type: 'text', text: 'ok' }] }) {
const listTools = vi.fn(async (
_params?: Record<string, unknown>,
): Promise<{ tools: MockTool[]; nextCursor: string | undefined }> => ({ tools, nextCursor: undefined }))
const callTool = vi.fn(async (
_params?: Record<string, unknown>,
_compatibilitySchema?: unknown,
_options?: unknown,
): Promise<Record<string, unknown>> => ({ ...callResult }))
return {
listTools: vi.fn().mockResolvedValue({ tools, nextCursor: undefined }),
callTool: vi.fn().mockResolvedValue(callResult),
listTools,
callTool,
request: vi.fn(async (
request: { method: string; params?: Record<string, unknown> },
_schema: unknown,
options?: unknown,
): Promise<unknown> => {
if (request.method === 'tools/list') return listTools(request.params)
if (request.method === 'tools/call') return callTool(request.params, undefined, options)
throw new Error(`unexpected MCP request: ${request.method}`)
}),
setNotificationHandler: vi.fn(),
connect: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
@@ -205,6 +225,80 @@ describe('syncTools', () => {
expect(ctx.tools.get('mcp__srv__page1')).toBeDefined()
expect(ctx.tools.get('mcp__srv__page2')).toBeDefined()
})
it('owns output validation independently of the SDK per-page cache', async () => {
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
serverTransport.onmessage = (message) => {
if (!('id' in message) || !('method' in message)) return
const params = 'params' in message ? message.params : undefined
let result: Record<string, unknown>
if (message.method === 'initialize') {
const protocolVersion = params && 'protocolVersion' in params
? params.protocolVersion
: '2025-11-25'
result = {
protocolVersion,
capabilities: { tools: {} },
serverInfo: { name: 'raw-test', version: '1' },
}
} else if (message.method === 'tools/list') {
const cursor = params && 'cursor' in params ? params.cursor : undefined
result = cursor === undefined
? {
tools: [{
name: 'supported',
inputSchema: { type: 'object' },
outputSchema: {
type: 'object',
additionalProperties: false,
properties: { answer: { type: 'integer' } },
required: ['answer'],
},
}],
nextCursor: 'page-2',
}
: {
tools: [{
name: 'future-schema',
inputSchema: { type: 'object' },
outputSchema: { type: 'object', patternProperties: { '^x-': { type: 'string' } } },
}],
}
} else if (message.method === 'tools/call') {
const name = params && 'name' in params ? params.name : undefined
result = name === 'supported'
? { content: [{ type: 'text', text: 'missing structured content' }] }
: { content: [42, null], structuredContent: ['kept', { nested: true }] }
} else {
result = {}
}
void serverTransport.send({ jsonrpc: '2.0', id: message.id, result })
}
await serverTransport.start()
const client = new Client({ name: 'cache-independent-test', version: '1' })
await client.connect(clientTransport)
try {
await syncTools(client, ctx, defaultOpts, new Map())
const missing = await ctx.tools.execute({
callId: CallId('missing'), name: 'mcp__srv__supported', arguments: {},
})
expect(missing.error).toMatchObject({ info: { code: 'INVALID_TOOL_OUTPUT' } })
expect(missing.error?.message).toContain('structuredContent')
const fallback = await ctx.tools.execute({
callId: CallId('fallback'), name: 'mcp__srv__future-schema', arguments: {},
})
if (fallback.isError) throw new Error('unsupported schema must use the bridge fallback')
expect(fallback.value).toEqual({
content: [42, null],
structuredContent: ['kept', { nested: true }],
})
} finally {
await client.close()
}
})
})
describe('tool execution', () => {
@@ -360,6 +454,21 @@ describe('tool execution', () => {
expect('value' in result).toBe(false)
})
it('rejects tools that require task-based execution', async () => {
const client = createMockClient([
{ name: 'task-only', inputSchema: { type: 'object' }, execution: { taskSupport: 'required' } },
])
await syncTools(client as never, ctx, defaultOpts, new Map())
const result = await ctx.tools.execute({
callId: CallId('task-only'), name: 'mcp__srv__task-only', arguments: {},
})
expect(result.isError).toBe(true)
expect(result.error?.message).toContain('requires task-based execution')
expect(client.callTool).not.toHaveBeenCalled()
})
it('passes abort signal to callTool', async () => {
const controller = new AbortController()
const client = createMockClient(