From 1f26ed98660cf5836a714de1947fe290c9c11263 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 19:44:03 +0200 Subject: [PATCH 01/11] Repair string-encoded union members in custom tool call inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a parameter's schema is a union with an object variant, models sometimes emit the object as a JSON-encoded string. The string is valid for the union, so validation passes and the handler silently receives a string instead of the object the model meant - data loss with no error. Decode schema-guided string-encoded members before validation, and hand the handler what the schema saw (processedParameters) rather than the untouched raw input. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../agent-runtime/src/tools/tool-executor.ts | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/agent-runtime/src/tools/tool-executor.ts b/packages/agent-runtime/src/tools/tool-executor.ts index 36c4708752..16f5d39b4a 100644 --- a/packages/agent-runtime/src/tools/tool-executor.ts +++ b/packages/agent-runtime/src/tools/tool-executor.ts @@ -571,6 +571,52 @@ export async function executeToolCall( }) } +/** + * Repairs values the model string-encoded against its schema. When a + * parameter's declared schema is a union containing an object variant, a + * model may emit the object as a JSON-encoded string (a string is + * unambiguously valid for the union, so nothing downstream fails). The + * schema-guided decode below restores the object the model meant; plain + * strings and params without an object variant are never touched, so + * tools whose string parameters legitimately contain JSON (script + * sources, file contents) are unaffected. + */ +function repairStringEncodedUnionMembers( + parameters: Record, + rawSchema: unknown, +): void { + if (!rawSchema || typeof rawSchema !== 'object') return + const properties = (rawSchema as { properties?: Record }) + .properties + if (!properties) return + for (const [param, value] of Object.entries(parameters)) { + if (typeof value !== 'string') continue + const propSchema = properties[param] + if (!propSchema || typeof propSchema !== 'object') continue + const union = + (propSchema as { anyOf?: unknown[] }).anyOf ?? + (propSchema as { oneOf?: unknown[] }).oneOf + if (!Array.isArray(union)) continue + const hasObjectVariant = union.some( + (variant) => + variant && + typeof variant === 'object' && + (variant as { type?: unknown }).type === 'object', + ) + if (!hasObjectVariant) continue + const trimmed = value.trim() + if (!trimmed.startsWith('{') && !trimmed.startsWith('[')) continue + try { + const decoded = JSON.parse(trimmed) + if (decoded && typeof decoded === 'object') { + parameters[param] = decoded + } + } catch { + // Not JSON after all - the string is a legitimate value. + } + } +} + export function parseRawCustomToolCall(params: { customToolDefs: CustomToolDefinitions rawToolCall: { @@ -618,6 +664,7 @@ export function parseRawCustomToolCall(params: { const rawSchema = customToolDefs?.[toolName]?.inputSchema if (rawSchema) { + repairStringEncodedUnionMembers(processedParameters, rawSchema) const paramsSchema = ensureZodSchema(rawSchema) const result = paramsSchema.safeParse(processedParameters) @@ -635,7 +682,9 @@ export function parseRawCustomToolCall(params: { } } - const input = JSON.parse(JSON.stringify(parsedInput.input)) + // processedParameters is what the schema saw (including the union repair + // above), so it - not the untouched raw input - is what the handler gets. + const input = JSON.parse(JSON.stringify(processedParameters)) if (endsAgentStepParam in input) { delete input[endsAgentStepParam] } From 03440e7945a07d3ee95fe22a177a925793f3bcd6 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 19:45:40 +0200 Subject: [PATCH 02/11] Map text and non-image MCP resources to json, not media MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tool results replay from history into every later prompt build, and the AI SDK base64-decodes media file parts at build time. Serving a text/plain resource as media therefore dies with "The string contains invalid characters" on every subsequent turn - permanently, since the poisoned message is in history. Non-image binaries (gzip, PDF, ...) went one worse: the OpenAI-compatible converter throws on them, killing the session on replay. Extract the mapping into mcpContentToToolResultOutputs: text resources become json values, only image/* resources stay media, and other binaries degrade to a descriptive json line. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- common/src/mcp/client.ts | 65 ++++++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 15 deletions(-) diff --git a/common/src/mcp/client.ts b/common/src/mcp/client.ts index 5a5608d57f..66c33dc38a 100644 --- a/common/src/mcp/client.ts +++ b/common/src/mcp/client.ts @@ -181,18 +181,24 @@ function getResourceData( return '' } -export async function callMCPTool( - clientId: string, - ...args: Parameters -): Promise { - const client = runningClients[clientId] - if (!client) { - throw new Error(`callTool: client not found with id: ${clientId}`) - } - const callResult = await client.callTool(...args) - const result = callResult as CallToolResult - const content = result.content - +/** + * Convert MCP tool-result content blocks into codebuff tool-result outputs. + * + * A resource with text contents is text, not media. Wrapping prose as + * media makes the AI SDK base64-decode it when rebuilding the prompt on + * every later turn, which dies with "The string contains invalid + * characters" forever, since the poisoned message replays from history. + * + * Only images stay media: every provider path (including the + * OpenAI-compatible chat converter used by GLM) accepts image file + * parts but throws on anything else — and a thrown converter poisons + * the whole session, since the message replays on every later turn. + * Other binary resources (gzip, PDF, ...) surface metadata instead of + * undecodable bytes. + */ +export function mcpContentToToolResultOutputs( + content: CallToolResult['content'], +): ToolResultOutput[] { return content.map((c: (typeof content)[number]) => { if (c.type === 'text') { return { @@ -215,10 +221,24 @@ export async function callMCPTool( } satisfies ToolResultOutput } if (c.type === 'resource') { + if ('text' in c.resource) { + return { + type: 'json', + value: c.resource.text, + } satisfies ToolResultOutput + } + const mimeType = c.resource.mimeType ?? 'application/octet-stream' + if (mimeType.startsWith('image/')) { + return { + type: 'media', + data: getResourceData(c.resource), + mediaType: mimeType, + } satisfies ToolResultOutput + } + const blobData = getResourceData(c.resource) return { - type: 'media', - data: getResourceData(c.resource), - mediaType: c.resource.mimeType ?? 'text/plain', + type: 'json', + value: `[Binary resource ${c.resource.uri}: ${mimeType}, ~${Math.round((blobData.length * 3) / 4)} bytes, not displayable]`, } satisfies ToolResultOutput } const fallbackValue = @@ -231,3 +251,18 @@ export async function callMCPTool( } satisfies ToolResultOutput }) } + +export async function callMCPTool( + clientId: string, + ...args: Parameters +): Promise { + const client = runningClients[clientId] + if (!client) { + throw new Error(`callTool: client not found with id: ${clientId}`) + } + const callResult = await client.callTool(...args) + const result = callResult as CallToolResult + const content = result.content + + return mcpContentToToolResultOutputs(content) +} From d9d45fa84b60482e2fc299d6d9222280abaa734a Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 19:48:40 +0200 Subject: [PATCH 03/11] Keep zod schema instances alive when cloning tool definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lodash cloneDeep strips zod v4's non-enumerable _zod engine from schema instances. The clone passes the safeParse smell test but is half-dead: any zod internal touching schema._zod.* detonates with "undefined is not an object", and upstream's ensureJsonSchemaCompatible fallback then reads schema.description outside its own try - so one stripped schema kills the entire agent step at getToolSet instead of degrading a single tool. Add cloneDeepKeepingZod (deep-clones plain data, passes schema instances through by reference) and use it at the tool-definition clone sites: getToolSet's additional-tool-definition loop and executeCustomToolCall's customToolDefinitions write target. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- packages/agent-runtime/src/tools/prompts.ts | 4 +-- .../agent-runtime/src/tools/tool-executor.ts | 4 +-- .../agent-runtime/src/util/zod-safe-clone.ts | 34 +++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 packages/agent-runtime/src/util/zod-safe-clone.ts diff --git a/packages/agent-runtime/src/tools/prompts.ts b/packages/agent-runtime/src/tools/prompts.ts index d3d9110665..aca40e36ae 100644 --- a/packages/agent-runtime/src/tools/prompts.ts +++ b/packages/agent-runtime/src/tools/prompts.ts @@ -8,7 +8,7 @@ import { getToolCallString } from '@codebuff/common/tools/utils' import { buildArray } from '@codebuff/common/util/array' import { formatAvailableSkillsXml } from '@codebuff/common/util/skills' import { pluralize } from '@codebuff/common/util/string' -import { cloneDeep } from 'lodash' +import { cloneDeepKeepingZod } from '../util/zod-safe-clone' import z from 'zod/v4' import { convertJsonSchemaToZod } from 'zod-from-json-schema' @@ -430,7 +430,7 @@ export async function getToolSet(params: { const toolDefinitions = await additionalToolDefinitions() for (const [toolName, toolDefinition] of Object.entries(toolDefinitions)) { - const clonedDef = cloneDeep(toolDefinition) + const clonedDef = cloneDeepKeepingZod(toolDefinition) // Custom tool inputSchema may be JSON Schema (from SDK) or Zod (from MCP) // Ensure it's a Zod schema for the AI SDK const zodSchema = ensureZodSchema(clonedDef.inputSchema) diff --git a/packages/agent-runtime/src/tools/tool-executor.ts b/packages/agent-runtime/src/tools/tool-executor.ts index 16f5d39b4a..5d4b108aea 100644 --- a/packages/agent-runtime/src/tools/tool-executor.ts +++ b/packages/agent-runtime/src/tools/tool-executor.ts @@ -1,7 +1,7 @@ import { endsAgentStepParam, toolNames } from '@codebuff/common/tools/constants' import { toolParams } from '@codebuff/common/tools/list' import { generateCompactId } from '@codebuff/common/util/string' -import { cloneDeep } from 'lodash' +import { cloneDeepKeepingZod } from '../util/zod-safe-clone' import { getMCPToolData } from '../mcp' import { MCP_TOOL_SEPARATOR } from '../mcp-constants' @@ -724,7 +724,7 @@ export async function executeCustomToolCall( ...params, toolNames: agentTemplate.toolNames, mcpServers: agentTemplate.mcpServers, - writeTo: cloneDeep(fileContext.customToolDefinitions), + writeTo: cloneDeepKeepingZod(fileContext.customToolDefinitions), }), rawToolCall: { toolName, diff --git a/packages/agent-runtime/src/util/zod-safe-clone.ts b/packages/agent-runtime/src/util/zod-safe-clone.ts new file mode 100644 index 0000000000..6e680895c4 --- /dev/null +++ b/packages/agent-runtime/src/util/zod-safe-clone.ts @@ -0,0 +1,34 @@ +import { cloneDeepWith } from 'lodash' + +/** + * lodash cloneDeep destroys zod v4 schema instances. + * + * zod v4 stores its engine on a non-enumerable `_zod` property, and lodash + * only copies enumerable own properties. The clone therefore looks like a + * schema (has safeParse/def/type) but has no `_zod` internals, and any zod + * internal that touches `schema._zod.*` detonates with: + * "undefined is not an object (evaluating 'schema._zod.def')" + * + * This deep-clones plain data (descriptions, maps, arrays) exactly like + * cloneDeep, but passes zod schema instances through by reference so their + * internals survive. + */ +export function cloneDeepKeepingZod(value: T): T { + const cloned = cloneDeepWith(value, (node) => { + if (isZodSchemaInstance(node)) { + // Pass the live schema through untouched. + return node as T + } + // Fall through to lodash's default deep clone. + return undefined + }) + return cloned as T +} + +function isZodSchemaInstance(node: unknown): boolean { + if (typeof node !== 'object' || node === null) { + return false + } + const candidate = node as { _zod?: unknown } + return typeof candidate._zod === 'object' && candidate._zod !== null +} From 773c937e51c3a2fc2b051b413e7237d7a8114dd6 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 19:50:57 +0200 Subject: [PATCH 04/11] Serve JSON Schema tool inputs verbatim instead of zod round-tripping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converting every custom tool inputSchema to zod and back is lossy: schemas zod cannot express (e.g. a property typed only { "type": "object" }) come back as an empty object schema, and a model reading an empty argument schema emits {} - a tool call with no arguments. serveInputSchema splits the two consumers: the model-facing definition gets the MCP server's declared JSON Schema verbatim (wrapped in ai's jsonSchema() pass-through container), while argument validation at call time keeps the zod conversion, where approximation is recoverable. Zod-typed inputSchemas keep ensureJsonSchemaCompatible, which now also logs when it has to fall back instead of failing silently. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- packages/agent-runtime/src/tools/prompts.ts | 92 +++++++++++++++++++-- 1 file changed, 86 insertions(+), 6 deletions(-) diff --git a/packages/agent-runtime/src/tools/prompts.ts b/packages/agent-runtime/src/tools/prompts.ts index aca40e36ae..fd6f789f31 100644 --- a/packages/agent-runtime/src/tools/prompts.ts +++ b/packages/agent-runtime/src/tools/prompts.ts @@ -9,11 +9,13 @@ import { buildArray } from '@codebuff/common/util/array' import { formatAvailableSkillsXml } from '@codebuff/common/util/skills' import { pluralize } from '@codebuff/common/util/string' import { cloneDeepKeepingZod } from '../util/zod-safe-clone' +import { jsonSchema as wrapJsonSchema } from 'ai' import z from 'zod/v4' import { convertJsonSchemaToZod } from 'zod-from-json-schema' import type { ToolName } from '@codebuff/common/tools/constants' import type { SkillsMap } from '@codebuff/common/types/skill' +import type { Logger } from '@codebuff/common/types/contracts/logger' import type { CustomToolDefinitions, customToolDefinitionsSchema, @@ -38,11 +40,29 @@ export function ensureZodSchema( return convertJsonSchemaToZod(schema as Record) } -function ensureJsonSchemaCompatible(schema: z.ZodType): z.ZodType { +function ensureJsonSchemaCompatible( + schema: z.ZodType, + opts?: { logger?: Logger; name?: string }, +): z.ZodType { try { z.toJSONSchema(schema, { io: 'input' }) return schema - } catch { + } catch (error) { + // This fallback once silently consumed zod schemas whose internals had + // been stripped by a shallow clone (lodash cloneDeep drops zod v4's + // non-enumerable _zod), turning a broken schema into an empty tool + // schema for the model. Loud failure here would have surfaced that bug + // in minutes instead of sessions. + opts?.logger?.warn( + { + toolName: opts.name, + error: String(error), + schemaConstructor: schema?.constructor?.name, + }, + `input schema failed JSON Schema conversion; serving empty schema${ + opts.name ? ` for '${opts.name}'` : '' + }`, + ) const fallback = z.object({}).passthrough() return schema.description ? fallback.describe(schema.description) : fallback } @@ -358,6 +378,60 @@ const readStyleDisplayVariants: Partial< type DisplayVariant = { description: string; inputSchema: z.ZodType } +/** + * Prepares a custom tool's inputSchema for the AI SDK. The schema ends up in + * two places, with different fidelity requirements: + * + * 1. The tool definition sent to the LLM provider. The model reads this to + * decide what arguments to emit, so it must match what the MCP server + * declared. JSON Schema inputs are therefore passed through verbatim, + * wrapped in ai's jsonSchema() (a pass-through container). + * 2. Argument validation at call time (the validate callback below). + * Approximation is acceptable here — a wrong rejection is recoverable, + * the model can retry — so the zod conversion does this job. + * + * Converting the schema to zod and back would be lossy: schemas zod cannot + * represent (e.g. a property typed only `{ "type": "object" }`) come back + * as an empty object schema, and a model reading an empty argument schema + * emits `{}` — a tool call with no arguments. Zod-typed inputSchemas + * (internal tools defined in TypeScript) keep the + * ensureJsonSchemaCompatible path, which converts in one direction only. + */ +function serveInputSchema( + inputSchema: z.ZodType | Record, + opts: { logger?: Logger; name?: string }, +): z.ZodType | ReturnType { + if ( + inputSchema && + typeof (inputSchema as { safeParse?: unknown }).safeParse === 'function' + ) { + return ensureJsonSchemaCompatible(inputSchema as z.ZodType, opts) + } + const rawJsonSchema = inputSchema as Record + // Validation only. The zod conversion handles checking arguments fine; + // its weakness is serializing back to JSON Schema, which we never do here. + const validationSchema = ensureZodSchema(rawJsonSchema) + const served = wrapJsonSchema( + rawJsonSchema as unknown as Parameters[0], + { + validate: (value: unknown) => { + const result = validationSchema.safeParse(value) + return result.success + ? { success: true as const, value: result.data } + : { success: false as const, error: result.error } + }, + }, + ) + if ( + typeof rawJsonSchema.description === 'string' && + rawJsonSchema.description.length > 0 + ) { + ;(served as { description?: string }).description ??= + rawJsonSchema.description + } + return served +} + export async function getToolSet(params: { toolNames: string[] windowedFileReads: boolean @@ -369,6 +443,7 @@ export async function getToolSet(params: { additionalToolDefinitions: () => Promise agentTools: ToolSet skills: SkillsMap + logger?: Logger }): Promise { const { toolNames, @@ -377,6 +452,7 @@ export async function getToolSet(params: { additionalToolDefinitions, agentTools, skills, + logger, } = params // Generate available skills XML for the skill tool description @@ -431,10 +507,14 @@ export async function getToolSet(params: { const toolDefinitions = await additionalToolDefinitions() for (const [toolName, toolDefinition] of Object.entries(toolDefinitions)) { const clonedDef = cloneDeepKeepingZod(toolDefinition) - // Custom tool inputSchema may be JSON Schema (from SDK) or Zod (from MCP) - // Ensure it's a Zod schema for the AI SDK - const zodSchema = ensureZodSchema(clonedDef.inputSchema) - const safeSchema = ensureJsonSchemaCompatible(zodSchema) + // Custom tool inputSchema may be JSON Schema (from SDK) or Zod (from MCP). + // JSON Schema is served verbatim (see serveInputSchema); the former + // unconditional zod round-trip stripped loose schemas to an empty + // object schema at the model. + const safeSchema = serveInputSchema(clonedDef.inputSchema, { + logger, + name: toolName, + }) toolSet[toolName] = { ...clonedDef, inputSchema: safeSchema, From 1c2d4903c1a5df748e91bd8a7d92d28cc426daf2 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 19:52:29 +0200 Subject: [PATCH 05/11] Store tool definitions in agent state as plain JSON Schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit toolDefinitions live in agent state, which hosts persist, snapshot, and ship over the wire. mapValues stored the live inputSchema as-is, so zod instances (cyclic, internals on non-enumerable _zod) ended up in persisted state: JSON.stringify over that state embeds zod machinery ({"def":{"shape":...}}) instead of the schema the tool actually declares. Normalize at the storage site with toTokenCountInputSchema (already used for the token-count path): converts zod to JSON Schema, copies plain objects through, and guarantees a top-level type: 'object'. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- packages/agent-runtime/src/run-agent-step.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/agent-runtime/src/run-agent-step.ts b/packages/agent-runtime/src/run-agent-step.ts index 9a97508e26..e51b949829 100644 --- a/packages/agent-runtime/src/run-agent-step.ts +++ b/packages/agent-runtime/src/run-agent-step.ts @@ -968,10 +968,14 @@ export async function loopAgentSteps( ) // Convert tools to a serializable format for context-pruner token counting + // Convert tool definitions to a JSON-serializable format. These live in + // agent state (persisted, snapshotted, shipped over the wire), so every + // inputSchema must be plain JSON Schema — Zod instances are cyclic and + // detonate any JSON.stringify over the state (turn 2+ would die). const toolDefinitions = mapValues(tools, (tool) => ({ description: typeof tool.description === 'string' ? tool.description : undefined, - inputSchema: tool.inputSchema as {}, + inputSchema: toTokenCountInputSchema(tool.inputSchema) ?? {}, })) const additionalToolDefinitionsWithCache = async () => { @@ -994,7 +998,8 @@ export async function loopAgentSteps( // Convert tool definitions to Anthropic format for accurate token counting. // Tool definitions are stored as { [name]: { description, inputSchema } }, - // where inputSchema is a Zod schema. Anthropic's count_tokens API expects + // where inputSchema is plain JSON Schema (see toolDefinitions above). + // Anthropic's count_tokens API expects // [{ name, description, input_schema }] with input_schema being real JSON // Schema (with a top-level `type: 'object'`) — see toTokenCountInputSchema. const toolsForTokenCount = Object.entries(toolDefinitions).map( From f314ec9d316d0da0f38617f7008d1f1883861c29 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 19:58:04 +0200 Subject: [PATCH 06/11] Normalize subagent toolDefinitions to plain JSON Schema too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spawn-agent-inline builds the same state-stored toolDefinitions map as loopAgentSteps and had the identical raw-inputSchema leak. Extract toTokenCountInputSchema into util/to-json-schema.ts so both call sites share one implementation (the util location avoids the import cycle through run-agent-step, which re-exports for compatibility). 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- packages/agent-runtime/src/run-agent-step.ts | 45 ++---------------- .../tools/handlers/tool/spawn-agent-inline.ts | 6 ++- .../agent-runtime/src/util/to-json-schema.ts | 46 +++++++++++++++++++ 3 files changed, 55 insertions(+), 42 deletions(-) create mode 100644 packages/agent-runtime/src/util/to-json-schema.ts diff --git a/packages/agent-runtime/src/run-agent-step.ts b/packages/agent-runtime/src/run-agent-step.ts index e51b949829..ed52d0e8eb 100644 --- a/packages/agent-runtime/src/run-agent-step.ts +++ b/packages/agent-runtime/src/run-agent-step.ts @@ -98,47 +98,10 @@ import type { ProjectFileContext, } from '@codebuff/common/util/file' -// Convert a tool's stored inputSchema into JSON Schema suitable for Anthropic's -// count_tokens API. Built-in and MCP tools store a Zod schema here; serializing -// it raw ships Zod internals (`def`/`shape`) instead of JSON Schema, so token -// counts are computed against garbage and any schema whose top-level isn't an -// object (e.g. a union → `anyOf`) arrives without `type`, which the API rejects -// with `tools.N.custom.input_schema.type: Field required`. We convert to JSON -// Schema and guarantee a top-level `type: 'object'`. -export function toTokenCountInputSchema( - inputSchema: unknown, -): Record | undefined { - if (inputSchema == null) return undefined - - let jsonSchema: Record - if ( - typeof (inputSchema as { safeParse?: unknown }).safeParse === 'function' - ) { - try { - jsonSchema = z.toJSONSchema(inputSchema as z.ZodType, { - io: 'input', - }) as Record - } catch { - jsonSchema = { type: 'object', properties: {} } - } - } else if (typeof inputSchema === 'object' && !Array.isArray(inputSchema)) { - // Already a plain object (e.g. a pre-serialized JSON Schema) — copy it. - jsonSchema = { ...(inputSchema as Record) } - } else { - return undefined - } - - // `$schema` is meaningless to count_tokens; drop it to keep the payload lean. - delete jsonSchema['$schema'] - // Anthropic requires a top-level `type: 'object'`. Object schemas already - // carry it; union/intersection schemas (anyOf/allOf) don't — backfill it. - // Treat missing / null / empty-string as absent (valid JSON Schema `type` is - // always a non-empty string or array). - if (jsonSchema.type == null || jsonSchema.type === '') { - jsonSchema.type = 'object' - } - return jsonSchema -} +// Moved to util/to-json-schema.ts so spawn-agent-inline can use it without an +// import cycle through run-agent-step. Re-exported here for existing importers. +import { toTokenCountInputSchema } from './util/to-json-schema' +export { toTokenCountInputSchema } async function additionalToolDefinitions( params: { diff --git a/packages/agent-runtime/src/tools/handlers/tool/spawn-agent-inline.ts b/packages/agent-runtime/src/tools/handlers/tool/spawn-agent-inline.ts index 3b996cdb87..a6288f0255 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/spawn-agent-inline.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/spawn-agent-inline.ts @@ -1,5 +1,7 @@ import { mapValues } from 'lodash' +import { toTokenCountInputSchema } from '../../../util/to-json-schema' + import { validateAndGetAgentTemplate, validateAgentInput, @@ -111,10 +113,12 @@ export const handleSpawnAgentInline = (async ( }, ), systemPrompt: system, + // Subagent tool definitions also live in agent state (persisted, + // snapshotted), so inputSchemas must be plain JSON Schema here too. toolDefinitions: mapValues(parentTools, (tool) => ({ description: typeof tool.description === 'string' ? tool.description : undefined, - inputSchema: tool.inputSchema as {}, + inputSchema: toTokenCountInputSchema(tool.inputSchema) ?? {}, })), } diff --git a/packages/agent-runtime/src/util/to-json-schema.ts b/packages/agent-runtime/src/util/to-json-schema.ts new file mode 100644 index 0000000000..1d32a34971 --- /dev/null +++ b/packages/agent-runtime/src/util/to-json-schema.ts @@ -0,0 +1,46 @@ +import z from 'zod/v4' + +// Convert a tool's stored inputSchema into JSON Schema suitable for Anthropic's +// count_tokens API. Built-in and MCP tools store a Zod schema here; serializing +// it raw ships Zod internals (`def`/`shape`) instead of JSON Schema, so token +// counts are computed against garbage and any schema whose top-level isn't an +// object (e.g. a union → `anyOf`) arrives without `type`, which the API rejects +// with `tools.N.custom.input_schema.type: Field required`. We convert to JSON +// Schema and guarantee a top-level `type: 'object'`. +// +// Lives in util/ (not run-agent-step) so spawn-agent-inline can use it without +// an import cycle through run-agent-step. +export function toTokenCountInputSchema( + inputSchema: unknown, +): Record | undefined { + if (inputSchema == null) return undefined + + let jsonSchema: Record + if ( + typeof (inputSchema as { safeParse?: unknown }).safeParse === 'function' + ) { + try { + jsonSchema = z.toJSONSchema(inputSchema as z.ZodType, { + io: 'input', + }) as Record + } catch { + jsonSchema = { type: 'object', properties: {} } + } + } else if (typeof inputSchema === 'object' && !Array.isArray(inputSchema)) { + // Already a plain object (e.g. a pre-serialized JSON Schema) — copy it. + jsonSchema = { ...(inputSchema as Record) } + } else { + return undefined + } + + // `$schema` is meaningless to count_tokens; drop it to keep the payload lean. + delete jsonSchema['$schema'] + // Anthropic requires a top-level `type: 'object'`. Object schemas already + // carry it; union/intersection schemas (anyOf/allOf) don't — backfill it. + // Treat missing / null / empty-string as absent (valid JSON Schema `type` is + // always a non-empty string or array). + if (jsonSchema.type == null || jsonSchema.type === '') { + jsonSchema.type = 'object' + } + return jsonSchema +} From d0e7e8553a611c4daca6161ed53bb940edf46f7e Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 20:08:09 +0200 Subject: [PATCH 07/11] Port regression test suite from fix branch onto V2 surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring the six bug-backing test files onto this branch: MCP content mapping, schema storage, prompts schema handling, to-json-schema, zod-safe-clone, and the OpenAI-compatible converter. Two ported tests exposed gaps this branch still had, fixed here: - getMCPToolData converted server schemas to zod before storing them in persisted state; store the raw JSON Schema verbatim instead. - The OpenAI-compatible converter threw on non-image file parts, killing the whole session on replay; degrade to a text placeholder. One test asserted a zod serializer token ("allOf") instead of the business contract; it failed identically on the pre-V2 fix tip, so it was never a stable assertion. Now asserts the params survive into the description. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../mcp/__tests__/mcp-content-mapping.test.ts | 102 ++++++++ .../src/__tests__/mcp-schema-store.test.ts | 87 +++++++ .../__tests__/prompts-schema-handling.test.ts | 244 +++++++++++++++++- packages/agent-runtime/src/mcp.ts | 8 +- .../src/util/__tests__/to-json-schema.test.ts | 85 ++++++ .../src/util/__tests__/zod-safe-clone.test.ts | 67 +++++ ...to-openai-compatible-chat-messages.test.ts | 44 ++++ ...vert-to-openai-compatible-chat-messages.ts | 33 ++- 8 files changed, 662 insertions(+), 8 deletions(-) create mode 100644 common/src/mcp/__tests__/mcp-content-mapping.test.ts create mode 100644 packages/agent-runtime/src/__tests__/mcp-schema-store.test.ts create mode 100644 packages/agent-runtime/src/util/__tests__/to-json-schema.test.ts create mode 100644 packages/agent-runtime/src/util/__tests__/zod-safe-clone.test.ts diff --git a/common/src/mcp/__tests__/mcp-content-mapping.test.ts b/common/src/mcp/__tests__/mcp-content-mapping.test.ts new file mode 100644 index 0000000000..1ded5258a4 --- /dev/null +++ b/common/src/mcp/__tests__/mcp-content-mapping.test.ts @@ -0,0 +1,102 @@ +import { describe, test, expect } from 'bun:test' + +import { mcpContentToToolResultOutputs } from '../client' + +/** + * Regression tests for MCP tool-result content mapping. + * + * Tool results live in message history and are replayed into every later + * prompt build, and the AI SDK base64-decodes file-part data at prompt + * build. Text content therefore never travels as media: prose stored as + * media died with "The string contains invalid characters" on every + * subsequent turn, permanently, because the poisoned message replays from + * history. + */ +describe('mcpContentToToolResultOutputs resources', () => { + /** + * Given: an MCP resource whose contents are plain text. + * When: it is mapped. + * Then: the output is a json value carrying that text - never media. + */ + test('maps text resource to json value not media', () => { + const outputs = mcpContentToToolResultOutputs([ + { + type: 'resource', + resource: { + uri: 'file:///notes.txt', + mimeType: 'text/plain', + text: 'Resource 1: This is a plain text resource.', + }, + }, + ] as never) + + expect(outputs).toEqual([ + { + type: 'json', + value: 'Resource 1: This is a plain text resource.', + }, + ]) + }) + + /** + * Given: an MCP resource carrying binary image data. + * When: it is mapped. + * Then: the output stays media with the server's mime type, because + * every provider path accepts image file parts. + */ + test('keeps image resource as media with server mime type', () => { + const outputs = mcpContentToToolResultOutputs([ + { + type: 'resource', + resource: { + uri: 'file:///logo.png', + mimeType: 'image/png', + blob: 'aGVsbG8=', + }, + }, + ] as never) + + expect(outputs).toHaveLength(1) + expect(outputs[0].type).toBe('media') + expect((outputs[0] as { mediaType?: string }).mediaType).toBe('image/png') + }) + + /** + * Given: an MCP resource carrying non-image binary data. + * When: it is mapped. + * Then: the output is descriptive text, not media - media here killed + * the OpenAI-compatible converter at prompt build (session death). + */ + test('maps non-image binary resource to descriptive text not media', () => { + const outputs = mcpContentToToolResultOutputs([ + { + type: 'resource', + resource: { + uri: 'file:///archive.gz', + mimeType: 'application/gzip', + blob: 'aGVsbG8=', + }, + }, + ] as never) + + expect(outputs[0].type).toBe('json') + + const value = (outputs[0] as { value: string }).value + expect(value).toContain('application/gzip') + expect(value).toContain('not displayable') + }) + + /** + * Given: an ordinary MCP text content block (no resource involved). + * When: it is mapped. + * Then: it stays a json value - the extraction must not alter the + * pre-existing text mapping. + */ + test('maps plain text content to json value', () => { + const outputs = mcpContentToToolResultOutputs([ + { type: 'text', text: 'Echo: hello' }, + ] as never) + + expect(outputs).toEqual([{ type: 'json', value: 'Echo: hello' }]) + }) +}) diff --git a/packages/agent-runtime/src/__tests__/mcp-schema-store.test.ts b/packages/agent-runtime/src/__tests__/mcp-schema-store.test.ts new file mode 100644 index 0000000000..814a1597d1 --- /dev/null +++ b/packages/agent-runtime/src/__tests__/mcp-schema-store.test.ts @@ -0,0 +1,87 @@ +import { describe, test, expect } from 'bun:test' + +import { getMCPToolData } from '../mcp' +import { MCP_TOOL_SEPARATOR } from '../mcp-constants' + +/** + * Regression tests for MCP tool-schema storage. + * + * Tool definitions returned by getMCPToolData are persisted in run/session + * state, which is snapshotted and JSON-serialized on every turn. Schemas + * must be stored verbatim: storing converted live zod instances instead + * round-trips to def/shape internals and can carry cycles that detonate + * JSON.stringify over the whole run state ("cannot serialize cyclic + * structures", session death from turn 2 onward). + */ +describe('getMCPToolData schema storage', () => { + /** + * Given: one MCP server reporting one tool with a JSON Schema. + * When: getMCPToolData stores it. + * Then: the stored schema round-trips through JSON as the exact schema + * the server sent - the persisted-state contract. + */ + test('stores the server JSON Schema verbatim and JSON round-trips it', async () => { + const serverSchema = { + type: 'object', + properties: { + location: { type: 'string', enum: ['NYC', 'LA'] }, + units: { type: 'string', description: 'metric or imperial' }, + }, + required: ['location'], + } + const writeTo: Record = {} + + await getMCPToolData({ + toolNames: ['weather/get_forecast'], + mcpServers: { + weather: { command: 'echo', args: [] }, + } as never, + writeTo: writeTo as never, + requestMcpToolData: async () => [ + { + name: 'get_forecast', + description: 'Get the forecast', + inputSchema: serverSchema, + }, + ], + }) + + const stored = writeTo[`weather${MCP_TOOL_SEPARATOR}get_forecast`] + const roundTripped = JSON.parse(JSON.stringify(stored.inputSchema)) + expect(roundTripped).toEqual(serverSchema) + }) + + /** + * Given: two servers each reporting one tool with a distinct schema. + * When: getMCPToolData stores both. + * Then: each server's tool carries its own schema, namespaced with the + * internal separator, verbatim and JSON-serializable. + */ + test('stores distinct schemas per server without conversion', async () => { + const schemaA = { type: 'object', properties: { a: { type: 'number' } } } + const schemaB = { type: 'string' } + const writeTo: Record = {} + + await getMCPToolData({ + toolNames: [], + mcpServers: { + alpha: { command: 'echo', args: [] }, + beta: { command: 'echo', args: [] }, + } as never, + writeTo: writeTo as never, + requestMcpToolData: async ({ toolNames }: { toolNames: unknown }) => { + void toolNames + return [ + { name: 't1', description: 'A', inputSchema: schemaA }, + { name: 't2', description: 'B', inputSchema: schemaB }, + ] + }, + }) + + const alphaStored = writeTo[`alpha${MCP_TOOL_SEPARATOR}t1`] + const betaStored = writeTo[`beta${MCP_TOOL_SEPARATOR}t2`] + expect(JSON.parse(JSON.stringify(alphaStored.inputSchema))).toEqual(schemaA) + expect(JSON.parse(JSON.stringify(betaStored.inputSchema))).toEqual(schemaB) + expect(betaStored.description).toBe('B') + }) +}) diff --git a/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts b/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts index d3ad20b276..d2c6003263 100644 --- a/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts +++ b/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts @@ -7,7 +7,7 @@ import { buildAgentToolInputSchema, buildAgentToolSet, } from '../templates/prompts' -import { tryTransformAgentToolCall } from '../tools/tool-executor' +import { parseRawCustomToolCall, tryTransformAgentToolCall } from '../tools/tool-executor' import { handleLookupAgentInfo } from '../tools/handlers/tool/lookup-agent-info' import { ensureZodSchema, @@ -221,8 +221,13 @@ describe('Schema handling error recovery', () => { expect(description).toContain('greet__greet') expect(description).toContain('Params: {') - expect(description).toContain('allOf') - expect(description).toContain('name') + // The business contract: the MCP-declared params survive into the + // description the model reads. (Do NOT assert the serializer's token + // choice — zod may render this shape as allOf or as flattened + // properties depending on version; coupling to that caused a + // permanently-flaky test.) + expect(description).toContain('"name"') + expect(description).toContain('cb_easp') expect(description).not.toContain('Params: None') }) @@ -510,3 +515,236 @@ describe('getToolSet: commit-attribution suppression', () => { ) }) }) + +// An MCP server declares a tool's arguments as a JSON Schema, and that schema +// is forwarded to the LLM — the model reads it to decide what arguments to +// emit. MCP allows these schemas to be vague: SEP-2106 requires only +// `type: "object"` +// (https://modelcontextprotocol.io/seps/2106-json-schema-2020-12), so a +// property may be a bare `{ "type": "object" }` with no named fields. +// The conversion to zod and back used to strip such schemas down to an empty +// object schema, and a model that reads an empty argument schema calls the +// tool with `{}` — no arguments at all. These tests pin the contract: what +// the server declared is what the model must see. +describe('getToolSet: loose MCP schemas survive the point-of-use round-trip', () => { + // quwin's minimal repro from the issue #912 follow-up, verbatim: + // one tight field, one loose field, both required. + const LOOSE_MCP_SCHEMA = { + type: 'object', + properties: { + project_id: { type: 'string' }, + payload: { type: 'object' }, + }, + required: ['project_id', 'payload'], + } + + // The AI SDK Schema contract getToolSet serves for JSON-Schema inputs: + // the raw schema passes to providers verbatim; args validate via callback. + type ServedSchema = { + jsonSchema: Record + validate: (value: unknown) => { success: boolean; value?: unknown } + } + + const buildWithCustomTool = async (inputSchema: unknown) => + getToolSet({ + toolNames: [], + windowedFileReads: false, + additionalToolDefinitions: async () => ({ + loose_schema_tool: { + description: 'Tool with a loose schema', + inputSchema: inputSchema as z.ZodType, + endsAgentStep: false, + }, + }), + agentTools: {}, + skills: {}, + }) + + test('a loose MCP schema reaches the model with its named properties intact', async () => { + // Given a custom tool whose JSON Schema contains a bare + // `{ type: 'object' }` property (unconvertible to a named zod shape), + // when getToolSet serves the tool's inputSchema, + // then the model-facing JSON Schema round-trip succeeds and still names + // both properties and both required fields - the served schema must not + // be the empty passthrough fallback. + + // Arrange + const toolSet = await buildWithCustomTool(LOOSE_MCP_SCHEMA) + const servedSchema = toolSet['loose_schema_tool']?.inputSchema as unknown as ServedSchema + + // Act + const modelFacing = servedSchema.jsonSchema + + // Assert: the raw JSON Schema must reach the model intact - + // both named properties and the required list, no passthrough fallback. + const properties = modelFacing.properties as + | Record + | undefined + expect(properties).toBeDefined() + expect(properties).toHaveProperty('project_id') + expect(properties).toHaveProperty('payload') + expect(modelFacing.required).toEqual( + expect.arrayContaining(['project_id', 'payload']), + ) + }) + + test('the served loose schema accepts arbitrary payloads but still rejects missing required fields', async () => { + // Given the same loose-schema tool served by getToolSet, + // when arguments are validated against the served inputSchema, + // then both sides of the validation contract hold: + // (a) the loose payload accepts arbitrary nested data - the served schema + // must not become stricter than what the MCP server declared, and + // (b) calls with missing required fields fail - the served schema must not + // become the old empty passthrough fallback, which accepted anything, + // including calls the MCP server declared invalid. + + // Arrange + const toolSet = await buildWithCustomTool(LOOSE_MCP_SCHEMA) + const servedSchema = toolSet['loose_schema_tool']?.inputSchema as unknown as ServedSchema + + // Act (a): both required fields present; payload is arbitrary nested data, + // which the server deliberately left unconstrained. + const validArgs = servedSchema.validate({ + project_id: 'p1', + payload: { anything: { deep: true } }, + }) + + // Act (b): no arguments at all, so both required fields are missing. + const missingRequired = servedSchema.validate({}) + + // Assert: (a) accepted, (b) rejected. + expect(validArgs.success).toBe(true) + expect(missingRequired.success).toBe(false) + }) + + test('a tight MCP schema is unaffected by the loose-schema path', async () => { + // Given a fully named (tight) MCP schema - every property a concrete + // scalar type, the pattern served by e.g. the MCP reference "everything" + // server (@modelcontextprotocol/server-everything) - + // when getToolSet serves it, + // then its properties round-trip intact. Control test: the loose-schema + // fix must not degrade the tight path that already worked. + + // Arrange + const tightSchema = { + type: 'object', + properties: { + name: { type: 'string' }, + }, + required: ['name'], + additionalProperties: false, + } + const toolSet = await buildWithCustomTool(tightSchema) + const servedSchema = toolSet['loose_schema_tool']?.inputSchema as unknown as ServedSchema + + // Act + const modelFacing = servedSchema.jsonSchema + + // Assert + expect(modelFacing.properties).toHaveProperty('name') + expect(modelFacing.required).toEqual(['name']) + }) +}) + +// Some models hedge on union-typed parameters: when a schema says a param may +// be a string OR an object (anyOf), the model sometimes emits the object as a +// JSON-encoded string, because a string is unambiguously valid for the union. +// The whole pipeline preserves that string faithfully, so the MCP server +// receives a string where an object was meant - and since the union accepts +// strings, nothing fails loudly. The tool-executor already repairs +// double-encoded arguments at the top level; these tests pin the same repair +// for nested, schema-guided cases. Found while digging into quwin's report +// on issue #912. +describe('parseRawCustomToolCall: schema-guided repair of string-encoded union members', () => { + const buildWithCustomTool = (inputSchema: unknown) => ({ + customToolDefs: { + 'loose-server__loose_union': { + description: 'Echoes back exactly the arguments it received.', + inputSchema: inputSchema as never, + endsAgentStep: false, + }, + }, + rawToolCall: { + toolName: 'loose-server__loose_union', + toolCallId: 'probe-1', + }, + }) + + const unionSchema = { + $schema: 'http://json-schema.org/draft-07/schema#', + type: 'object', + properties: { + spec: { + anyOf: [{ type: 'string' }, { type: 'object', properties: { kind: { type: 'string' } }, additionalProperties: true }], + description: 'A string or an object. Either is accepted.', + }, + }, + required: ['spec'], + additionalProperties: true, + } + + test('a JSON-encoded string for a union param with an object variant is decoded to an object', () => { + // Given a union schema (string | object) and a raw tool call whose + // union-typed parameter arrived as a JSON-encoded string, + // when the custom tool call is parsed, + // then the parameter is decoded to the object the model meant - + // matching what the same model emits for unambiguous object params. + + // Arrange + const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) + const withInput = { ...rawToolCall, input: { spec: '{"kind": "unhinged-union-spec", "extra": 42}' } } + + // Act + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + // Assert + expect(result).toHaveProperty('input') + expect((result as { input: { spec: unknown } }).input.spec).toEqual({ + kind: 'unhinged-union-spec', + extra: 42, + }) + }) + + test('a non-JSON string for a union param stays a string', () => { + // Given the same union schema and a parameter that is a plain string + // (not JSON-encoded), + // when parsed, + // then the string is preserved - the string branch of the union is a + // legitimate choice and must not be mangled. + + // Arrange + const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) + const withInput = { ...rawToolCall, input: { spec: 'plain-string-variant' } } + + // Act + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + // Assert + expect((result as { input: { spec: unknown } }).input.spec).toBe('plain-string-variant') + }) + + test('a JSON-encoded string for a plain string-typed param is NOT decoded', () => { + // Given a schema whose param is a plain string (no object variant) and a + // value that happens to be JSON-encoded, + // when parsed, + // then the string stays a string - the repair must be guided by the + // schema, or tools whose string params legitimately contain JSON (like + // evaluate_script source) would be corrupted. + + // Arrange + const stringOnlySchema = { + type: 'object', + properties: { code: { type: 'string' } }, + required: ['code'], + additionalProperties: false, + } + const { customToolDefs, rawToolCall } = buildWithCustomTool(stringOnlySchema) + const withInput = { ...rawToolCall, input: { code: '{"looks": "like json"}' } } + + // Act + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + // Assert + expect((result as { input: { code: unknown } }).input.code).toBe('{"looks": "like json"}') + }) +}) diff --git a/packages/agent-runtime/src/mcp.ts b/packages/agent-runtime/src/mcp.ts index a7390f219c..716ba901d4 100644 --- a/packages/agent-runtime/src/mcp.ts +++ b/packages/agent-runtime/src/mcp.ts @@ -1,5 +1,4 @@ import { getErrorObject } from '@codebuff/common/util/error' -import { convertJsonSchemaToZod } from 'zod-from-json-schema' import { MCP_TOOL_SEPARATOR } from './mcp-constants' @@ -55,8 +54,13 @@ export async function getMCPToolData( }) for (const { name, description, inputSchema } of mcpData) { + // Store the raw JSON Schema from the server, NOT the converted Zod + // schema. Tool definitions are persisted in run state / session + // state and must stay JSON-serializable; Zod instances are cyclic + // and make any JSON.stringify over that state detonate. Consumers + // convert at point of use (ensureZodSchema / toTokenCountInputSchema). writeTo[mcpName + MCP_TOOL_SEPARATOR + name] = { - inputSchema: convertJsonSchemaToZod(inputSchema as any) as any, + inputSchema: inputSchema as {}, endsAgentStep: true, description, } diff --git a/packages/agent-runtime/src/util/__tests__/to-json-schema.test.ts b/packages/agent-runtime/src/util/__tests__/to-json-schema.test.ts new file mode 100644 index 0000000000..a5cc67e1dd --- /dev/null +++ b/packages/agent-runtime/src/util/__tests__/to-json-schema.test.ts @@ -0,0 +1,85 @@ +import { describe, test, expect } from 'bun:test' +import { z } from 'zod/v4' + +import { toTokenCountInputSchema } from '../to-json-schema' + +/** + * Regression tests for the persisted-state schema conversion. + * + * Tool inputSchemas are persisted into agent state, snapshotted and replayed + * on every turn, and shipped to Anthropic's count_tokens API. Every stored + * schema must therefore be plain JSON Schema with a top-level type: zod + * internals never leak into state, and foreign (already-JSON) schemas pass + * through unmangled. + */ +describe('toTokenCountInputSchema', () => { + /** + * Given: a zod object schema with an optional field. + * When: it is converted. + * Then: the result is JSON Schema with type object and the field mapped, + * not a serialized zod instance. + */ + test('converts zod object schema to JSON Schema with top level type object', () => { + const schema = z.object({ + q: z.string().describe('query'), + n: z.number().optional(), + }) + + const out = toTokenCountInputSchema(schema) as Record | undefined + + expect(out?.type).toBe('object') + expect(out?.properties.q.type).toBe('string') + }) + + /** + * Given: a union schema, which JSON Schema represents as anyOf with no + * top-level type. + * When: it is converted. + * Then: type object is backfilled, because Anthropic's count_tokens + * rejects input_schema values without a top-level type. + */ + test('backfills type object for union schemas represented as anyOf', () => { + const schema = z.union([z.object({ a: z.string() }), z.object({ b: z.number() })]) + + const out = toTokenCountInputSchema(schema) as Record | undefined + + expect(out?.type).toBe('object') + expect(out?.anyOf).toBeDefined() + }) + + /** + * Given: a schema that is already a plain JSON Schema object (the shape + * MCP servers and the SDK send). + * When: it is converted. + * Then: it is copied as-is - conversion must not mangle foreign schemas. + */ + test('copies an already plain JSON Schema object unchanged', () => { + const jsonSchema = { + type: 'object', + properties: { location: { type: 'string', enum: ['NYC', 'LA'] } }, + required: ['location'], + } + + const out = toTokenCountInputSchema(jsonSchema) + + expect(out).toEqual(jsonSchema) + }) + + /** + * Given: nullish input and a schema carrying a $schema key. + * When: they are converted. + * Then: nullish input yields undefined, and the meaningless $schema key + * is dropped to keep the token-count payload lean. + */ + test('returns undefined for nullish input and strips the schema meta key', () => { + const withMeta = { $schema: 'https://json-schema.org/x', type: 'object' } + + const nullishOut = toTokenCountInputSchema(undefined) + const metaOut = toTokenCountInputSchema(withMeta) + + expect(nullishOut).toBeUndefined() + expect(toTokenCountInputSchema(null)).toBeUndefined() + expect(metaOut?.$schema).toBeUndefined() + expect(metaOut?.type).toBe('object') + }) +}) diff --git a/packages/agent-runtime/src/util/__tests__/zod-safe-clone.test.ts b/packages/agent-runtime/src/util/__tests__/zod-safe-clone.test.ts new file mode 100644 index 0000000000..fbf378c49c --- /dev/null +++ b/packages/agent-runtime/src/util/__tests__/zod-safe-clone.test.ts @@ -0,0 +1,67 @@ +import { describe, test, expect } from 'bun:test' +import { cloneDeep } from 'lodash' +import { z } from 'zod/v4' + +import { cloneDeepKeepingZod } from '../zod-safe-clone' + +/** + * Regression tests for tool-schema cloning. + * + * Tool definitions carry live zod v4 schemas, and state boundaries + * deep-clone the surrounding data. lodash cloneDeep strips zod's + * non-enumerable _zod engine: the stripped clone still looks like a schema + * (safeParse, def, shape all present) but throws the first time zod + * internals touch it - which is how MCP and custom tool schemas silently + * became empty {} at the model. cloneDeepKeepingZod is the fix pinned here. + */ +describe('lodash cloneDeep zod amputation (the bug)', () => { + /** + * Given: a zod v4 schema. + * When: it is cloned with lodash cloneDeep. + * Then: the clone still looks like a schema (safeParse present) but its + * engine is gone: z.toJSONSchema throws on it - the production failure + * behind the empty-schema bug, and the reason the helper below exists. + */ + test('cloneDeep strips the zod engine so toJSONSchema throws on the clone', () => { + const schema = z.object({ q: z.string() }) + + const cloned = cloneDeep(schema) + + // Asserted behaviorally: the clone still parses, but conversion fails. + expect(typeof cloned.safeParse).toBe('function') + expect(() => z.toJSONSchema(cloned as never)).toThrow() + }) +}) + +describe('cloneDeepKeepingZod', () => { + /** + * Given: a plain (schema-free) nested structure. + * When: it is cloned with cloneDeepKeepingZod. + * Then: the result matches cloneDeep exactly, including fresh nested + * references - the clone helper must not change plain-data semantics. + */ + test('cloneDeepKeepingZod deep-clones plain structures exactly like cloneDeep', () => { + const input = { a: { b: [1, { c: 'd' }] }, e: null } + + const out = cloneDeepKeepingZod(input) + + expect(out).toEqual(input) + expect(out.a).not.toBe(input.a) + expect(out.a.b[1]).not.toBe(input.a.b[1]) + }) + + /** + * Given: a zod schema nested inside a collection, the shape custom tool * definitions actually arrive in. + * When: the containing structure is cloned. + * Then: the schema survives as a live instance usable by zod internals. + */ + test('cloneDeepKeepingZod preserves schemas nested inside collections', () => { + const schema = z.object({ id: z.number() }) + const input = { tools: [{ name: 'x', inputSchema: schema }] } + + const out = cloneDeepKeepingZod(input) + + expect(out.tools[0].inputSchema).toBe(schema) + expect(() => z.toJSONSchema(out.tools[0].inputSchema)).not.toThrow() + }) +}) diff --git a/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.test.ts b/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.test.ts index 195d63b819..9827f7314d 100644 --- a/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.test.ts +++ b/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.test.ts @@ -1083,3 +1083,47 @@ describe('consecutive assistant messages', () => { ]) }) }) + +/** + * Regression tests for non-image file parts. + * + * MCP resources can put non-image file parts (e.g. gzip) into message + * history, which is replayed into every later prompt build. The + * OpenAI-compatible converter must degrade such parts to a text + * placeholder: throwing here failed the entire prompt build and, because + * the message stays in history, killed the session on every subsequent + * turn. + */ +describe('non-image file parts', () => { + // The fixture's base64 string is 20 chars; the placeholder estimates raw + // bytes as round(20 * 3 / 4) = 15. + const GZIP_FIXTURE_BASE64 = Buffer.from('Hello freebuff!').toString('base64') + const EXPECTED_BYTE_ESTIMATE = 15 + + it('degrades non-image file part to text placeholder instead of throwing', () => { + const result = convertToOpenAICompatibleChatMessages([ + { + role: 'user', + content: [ + { + type: 'file', + data: GZIP_FIXTURE_BASE64, + mediaType: 'application/gzip', + }, + ], + }, + ]) + + expect(result).toEqual([ + { + role: 'user', + content: [ + { + type: 'text', + text: `[application/gzip file part not displayable (~${EXPECTED_BYTE_ESTIMATE} bytes)]`, + }, + ], + }, + ]) + }) +}) diff --git a/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.ts b/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.ts index ead5daab11..4491f8dfaa 100644 --- a/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.ts +++ b/packages/llm-providers/src/openai-compatible/chat/convert-to-openai-compatible-chat-messages.ts @@ -14,6 +14,25 @@ function getOpenAIMetadata(message: { return message?.providerOptions?.openaiCompatible ?? {} } +/** Approximate payload size of a file part's data, for placeholder text. */ +function filePartByteLength(data: unknown): number { + let value = data + if (value && typeof value === 'object' && 'type' in value) { + if (value.type === 'data' && 'data' in value) { + value = value.data + } else if (value.type === 'url' && 'url' in value) { + value = value.url + } + } + if (typeof value === 'string') { + return Math.round((value.length * 3) / 4) + } + if (value instanceof Uint8Array) { + return value.byteLength + } + return 0 +} + function imageUrlFromData(data: unknown, mediaType: string): string { // AI SDK 7 adapts this v2 provider to v4, whose file data is tagged. The // compatibility proxy passes that v4 shape through to the v2 implementation. @@ -89,9 +108,17 @@ export function convertToOpenAICompatibleChatMessages( ...partMetadata, } } else { - throw new UnsupportedFunctionalityError({ - functionality: `file part media type ${part.mediaType}`, - }) + // Non-image file parts (e.g. application/gzip from an MCP + // resource) have no OpenAI-compatible representation. + // Degrade to a text placeholder instead of throwing: a + // throw here fails the entire prompt build and, because + // the message stays in history, kills the session on every + // subsequent turn. + return { + type: 'text', + text: `[${part.mediaType} file part not displayable (~${filePartByteLength(part.data)} bytes)]`, + ...partMetadata, + } } } } From 42b219d5aceaf30047521bc6b9270516404dfb6e Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 21:05:26 +0200 Subject: [PATCH 08/11] Extract fix seams into dedicated modules for rebase-friendly diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per conflict hygiene: new code lives in new files so a future upstream merge touches our modules plus a one-line import in theirs, instead of rewriting regions inside upstream functions. - tool-executor.ts: repairStringEncodedUnionMembers moves to util/repair-string-encoded-union-members.ts (call site unchanged). - client.ts: mcpContentToToolResultOutputs moves to common/src/mcp/content-mapping.ts (call site unchanged). - prompts.ts: serveInputSchema + ensureZodSchema move to tools/serve-input-schema.ts; prompts.ts drops the logger parameter added for the loud-fallback experiment and reverts ensureJsonSchemaCompatible to the upstream shape (452 lines, under the 500-line budget; ensureJsonSchemaCompatible remains upstream's silent-fallback version pending upstream buy-in). Behavior unchanged: full suite 78/78. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../mcp/__tests__/mcp-content-mapping.test.ts | 2 +- common/src/mcp/client.ts | 82 +---------------- common/src/mcp/content-mapping.ts | 82 +++++++++++++++++ packages/agent-runtime/src/tools/prompts.ts | 86 +----------------- .../src/tools/serve-input-schema.ts | 88 +++++++++++++++++++ .../agent-runtime/src/tools/tool-executor.ts | 47 +--------- .../repair-string-encoded-union-members.ts | 45 ++++++++++ 7 files changed, 222 insertions(+), 210 deletions(-) create mode 100644 common/src/mcp/content-mapping.ts create mode 100644 packages/agent-runtime/src/tools/serve-input-schema.ts create mode 100644 packages/agent-runtime/src/util/repair-string-encoded-union-members.ts diff --git a/common/src/mcp/__tests__/mcp-content-mapping.test.ts b/common/src/mcp/__tests__/mcp-content-mapping.test.ts index 1ded5258a4..9c6210f15b 100644 --- a/common/src/mcp/__tests__/mcp-content-mapping.test.ts +++ b/common/src/mcp/__tests__/mcp-content-mapping.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect } from 'bun:test' -import { mcpContentToToolResultOutputs } from '../client' +import { mcpContentToToolResultOutputs } from '../content-mapping' /** * Regression tests for MCP tool-result content mapping. diff --git a/common/src/mcp/client.ts b/common/src/mcp/client.ts index 66c33dc38a..31b87bdc2e 100644 --- a/common/src/mcp/client.ts +++ b/common/src/mcp/client.ts @@ -4,14 +4,13 @@ import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js' import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' import { getErrorObject } from '../util/error' +import { mcpContentToToolResultOutputs } from './content-mapping' import type { MCPConfig } from '../types/mcp' import type { ToolResultOutput } from '../types/messages/content-part' import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js' import type { - BlobResourceContents, CallToolResult, - TextResourceContents, } from '@modelcontextprotocol/sdk/types.js' // Cap on how much of a failed stdio server's stderr we retain for the error @@ -173,85 +172,6 @@ export function listMCPTools( return listToolsCache[clientId] } -function getResourceData( - resource: TextResourceContents | BlobResourceContents, -): string { - if ('text' in resource) return resource.text as string - if ('blob' in resource) return resource.blob as string - return '' -} - -/** - * Convert MCP tool-result content blocks into codebuff tool-result outputs. - * - * A resource with text contents is text, not media. Wrapping prose as - * media makes the AI SDK base64-decode it when rebuilding the prompt on - * every later turn, which dies with "The string contains invalid - * characters" forever, since the poisoned message replays from history. - * - * Only images stay media: every provider path (including the - * OpenAI-compatible chat converter used by GLM) accepts image file - * parts but throws on anything else — and a thrown converter poisons - * the whole session, since the message replays on every later turn. - * Other binary resources (gzip, PDF, ...) surface metadata instead of - * undecodable bytes. - */ -export function mcpContentToToolResultOutputs( - content: CallToolResult['content'], -): ToolResultOutput[] { - return content.map((c: (typeof content)[number]) => { - if (c.type === 'text') { - return { - type: 'json', - value: c.text, - } satisfies ToolResultOutput - } - if (c.type === 'audio') { - return { - type: 'media', - data: c.data, - mediaType: c.mimeType, - } satisfies ToolResultOutput - } - if (c.type === 'image') { - return { - type: 'media', - data: c.data, - mediaType: c.mimeType, - } satisfies ToolResultOutput - } - if (c.type === 'resource') { - if ('text' in c.resource) { - return { - type: 'json', - value: c.resource.text, - } satisfies ToolResultOutput - } - const mimeType = c.resource.mimeType ?? 'application/octet-stream' - if (mimeType.startsWith('image/')) { - return { - type: 'media', - data: getResourceData(c.resource), - mediaType: mimeType, - } satisfies ToolResultOutput - } - const blobData = getResourceData(c.resource) - return { - type: 'json', - value: `[Binary resource ${c.resource.uri}: ${mimeType}, ~${Math.round((blobData.length * 3) / 4)} bytes, not displayable]`, - } satisfies ToolResultOutput - } - const fallbackValue = - 'uri' in c && typeof (c as { uri: unknown }).uri === 'string' - ? (c as { uri: string }).uri - : JSON.stringify(c) - return { - type: 'json', - value: fallbackValue, - } satisfies ToolResultOutput - }) -} - export async function callMCPTool( clientId: string, ...args: Parameters diff --git a/common/src/mcp/content-mapping.ts b/common/src/mcp/content-mapping.ts new file mode 100644 index 0000000000..03ce628b14 --- /dev/null +++ b/common/src/mcp/content-mapping.ts @@ -0,0 +1,82 @@ +import type { CallToolResult, TextResourceContents, BlobResourceContents } from '@modelcontextprotocol/sdk/types.js' + +import type { ToolResultOutput } from '../types/messages/content-part' + +function getResourceData( + resource: TextResourceContents | BlobResourceContents, +): string { + if ('text' in resource) return resource.text as string + if ('blob' in resource) return resource.blob as string + return '' +} + +/** + * Convert MCP tool-result content blocks into codebuff tool-result outputs. + * + * A resource with text contents is text, not media. Wrapping prose as + * media makes the AI SDK base64-decode it when rebuilding the prompt on + * every later turn, which dies with "The string contains invalid + * characters" forever, since the poisoned message replays from history. + * + * Only images stay media: every provider path (including the + * OpenAI-compatible chat converter used by GLM) accepts image file + * parts but throws on anything else — and a thrown converter poisons + * the whole session, since the message replays on every later turn. + * Other binary resources (gzip, PDF, ...) surface metadata instead of + * undecodable bytes. + */ +export function mcpContentToToolResultOutputs( + content: CallToolResult['content'], +): ToolResultOutput[] { + return content.map((c: (typeof content)[number]) => { + if (c.type === 'text') { + return { + type: 'json', + value: c.text, + } satisfies ToolResultOutput + } + if (c.type === 'audio') { + return { + type: 'media', + data: c.data, + mediaType: c.mimeType, + } satisfies ToolResultOutput + } + if (c.type === 'image') { + return { + type: 'media', + data: c.data, + mediaType: c.mimeType, + } satisfies ToolResultOutput + } + if (c.type === 'resource') { + if ('text' in c.resource) { + return { + type: 'json', + value: c.resource.text, + } satisfies ToolResultOutput + } + const mimeType = c.resource.mimeType ?? 'application/octet-stream' + if (mimeType.startsWith('image/')) { + return { + type: 'media', + data: getResourceData(c.resource), + mediaType: mimeType, + } satisfies ToolResultOutput + } + const blobData = getResourceData(c.resource) + return { + type: 'json', + value: `[Binary resource ${c.resource.uri}: ${mimeType}, ~${Math.round((blobData.length * 3) / 4)} bytes, not displayable]`, + } satisfies ToolResultOutput + } + const fallbackValue = + 'uri' in c && typeof (c as { uri: unknown }).uri === 'string' + ? (c as { uri: string }).uri + : JSON.stringify(c) + return { + type: 'json', + value: fallbackValue, + } satisfies ToolResultOutput + }) +} diff --git a/packages/agent-runtime/src/tools/prompts.ts b/packages/agent-runtime/src/tools/prompts.ts index fd6f789f31..d27df73658 100644 --- a/packages/agent-runtime/src/tools/prompts.ts +++ b/packages/agent-runtime/src/tools/prompts.ts @@ -9,13 +9,12 @@ import { buildArray } from '@codebuff/common/util/array' import { formatAvailableSkillsXml } from '@codebuff/common/util/skills' import { pluralize } from '@codebuff/common/util/string' import { cloneDeepKeepingZod } from '../util/zod-safe-clone' -import { jsonSchema as wrapJsonSchema } from 'ai' +import { serveInputSchema } from './serve-input-schema' import z from 'zod/v4' import { convertJsonSchemaToZod } from 'zod-from-json-schema' import type { ToolName } from '@codebuff/common/tools/constants' import type { SkillsMap } from '@codebuff/common/types/skill' -import type { Logger } from '@codebuff/common/types/contracts/logger' import type { CustomToolDefinitions, customToolDefinitionsSchema, @@ -40,29 +39,11 @@ export function ensureZodSchema( return convertJsonSchemaToZod(schema as Record) } -function ensureJsonSchemaCompatible( - schema: z.ZodType, - opts?: { logger?: Logger; name?: string }, -): z.ZodType { +function ensureJsonSchemaCompatible(schema: z.ZodType): z.ZodType { try { z.toJSONSchema(schema, { io: 'input' }) return schema - } catch (error) { - // This fallback once silently consumed zod schemas whose internals had - // been stripped by a shallow clone (lodash cloneDeep drops zod v4's - // non-enumerable _zod), turning a broken schema into an empty tool - // schema for the model. Loud failure here would have surfaced that bug - // in minutes instead of sessions. - opts?.logger?.warn( - { - toolName: opts.name, - error: String(error), - schemaConstructor: schema?.constructor?.name, - }, - `input schema failed JSON Schema conversion; serving empty schema${ - opts.name ? ` for '${opts.name}'` : '' - }`, - ) + } catch { const fallback = z.object({}).passthrough() return schema.description ? fallback.describe(schema.description) : fallback } @@ -378,60 +359,6 @@ const readStyleDisplayVariants: Partial< type DisplayVariant = { description: string; inputSchema: z.ZodType } -/** - * Prepares a custom tool's inputSchema for the AI SDK. The schema ends up in - * two places, with different fidelity requirements: - * - * 1. The tool definition sent to the LLM provider. The model reads this to - * decide what arguments to emit, so it must match what the MCP server - * declared. JSON Schema inputs are therefore passed through verbatim, - * wrapped in ai's jsonSchema() (a pass-through container). - * 2. Argument validation at call time (the validate callback below). - * Approximation is acceptable here — a wrong rejection is recoverable, - * the model can retry — so the zod conversion does this job. - * - * Converting the schema to zod and back would be lossy: schemas zod cannot - * represent (e.g. a property typed only `{ "type": "object" }`) come back - * as an empty object schema, and a model reading an empty argument schema - * emits `{}` — a tool call with no arguments. Zod-typed inputSchemas - * (internal tools defined in TypeScript) keep the - * ensureJsonSchemaCompatible path, which converts in one direction only. - */ -function serveInputSchema( - inputSchema: z.ZodType | Record, - opts: { logger?: Logger; name?: string }, -): z.ZodType | ReturnType { - if ( - inputSchema && - typeof (inputSchema as { safeParse?: unknown }).safeParse === 'function' - ) { - return ensureJsonSchemaCompatible(inputSchema as z.ZodType, opts) - } - const rawJsonSchema = inputSchema as Record - // Validation only. The zod conversion handles checking arguments fine; - // its weakness is serializing back to JSON Schema, which we never do here. - const validationSchema = ensureZodSchema(rawJsonSchema) - const served = wrapJsonSchema( - rawJsonSchema as unknown as Parameters[0], - { - validate: (value: unknown) => { - const result = validationSchema.safeParse(value) - return result.success - ? { success: true as const, value: result.data } - : { success: false as const, error: result.error } - }, - }, - ) - if ( - typeof rawJsonSchema.description === 'string' && - rawJsonSchema.description.length > 0 - ) { - ;(served as { description?: string }).description ??= - rawJsonSchema.description - } - return served -} - export async function getToolSet(params: { toolNames: string[] windowedFileReads: boolean @@ -443,7 +370,6 @@ export async function getToolSet(params: { additionalToolDefinitions: () => Promise agentTools: ToolSet skills: SkillsMap - logger?: Logger }): Promise { const { toolNames, @@ -452,7 +378,6 @@ export async function getToolSet(params: { additionalToolDefinitions, agentTools, skills, - logger, } = params // Generate available skills XML for the skill tool description @@ -511,10 +436,7 @@ export async function getToolSet(params: { // JSON Schema is served verbatim (see serveInputSchema); the former // unconditional zod round-trip stripped loose schemas to an empty // object schema at the model. - const safeSchema = serveInputSchema(clonedDef.inputSchema, { - logger, - name: toolName, - }) + const safeSchema = serveInputSchema(clonedDef.inputSchema) toolSet[toolName] = { ...clonedDef, inputSchema: safeSchema, diff --git a/packages/agent-runtime/src/tools/serve-input-schema.ts b/packages/agent-runtime/src/tools/serve-input-schema.ts new file mode 100644 index 0000000000..32d4f28959 --- /dev/null +++ b/packages/agent-runtime/src/tools/serve-input-schema.ts @@ -0,0 +1,88 @@ +import { jsonSchema as wrapJsonSchema } from 'ai' +import z from 'zod/v4' + +import { convertJsonSchemaToZod } from 'zod-from-json-schema' + +import type { Logger } from '@codebuff/common/types/contracts/logger' + +/** + * Ensures the inputSchema is a Zod schema. If it's a JSON Schema object + * (from SDK custom tools that were serialized), converts it to Zod. + */ +export function ensureZodSchema( + schema: z.ZodType | Record, +): z.ZodType { + // Check if it's already a Zod schema by looking for the safeParse method + if ( + schema && + typeof (schema as { safeParse?: unknown }).safeParse === 'function' + ) { + return schema as z.ZodType + } + // JSON Schema object - convert to Zod + return convertJsonSchemaToZod(schema as Record) +} + +function ensureJsonSchemaCompatible(schema: z.ZodType): z.ZodType { + try { + z.toJSONSchema(schema, { io: 'input' }) + return schema + } catch { + const fallback = z.object({}).passthrough() + return schema.description ? fallback.describe(schema.description) : fallback + } +} + +/** + * Prepares a custom tool's inputSchema for the AI SDK. The schema ends up in + * two places, with different fidelity requirements: + * + * 1. The tool definition sent to the LLM provider. The model reads this to + * decide what arguments to emit, so it must match what the MCP server + * declared. JSON Schema inputs are therefore passed through verbatim, + * wrapped in ai's jsonSchema() (a pass-through container). + * 2. Argument validation at call time (the validate callback below). + * Approximation is acceptable here — a wrong rejection is recoverable, + * the model can retry — so the zod conversion does this job. + * + * Converting the schema to zod and back would be lossy: schemas zod cannot + * represent (e.g. a property typed only `{ "type": "object" }`) come back + * as an empty object schema, and a model reading an empty argument schema + * emits `{}` — a tool call with no arguments. Zod-typed inputSchemas + * (internal tools defined in TypeScript) keep the + * ensureJsonSchemaCompatible path, which converts in one direction only. + */ +export function serveInputSchema( + inputSchema: z.ZodType | Record, + opts?: { logger?: Logger; name?: string }, +): z.ZodType | ReturnType { + if ( + inputSchema && + typeof (inputSchema as { safeParse?: unknown }).safeParse === 'function' + ) { + return ensureJsonSchemaCompatible(inputSchema as z.ZodType) + } + const rawJsonSchema = inputSchema as Record + // Validation only. The zod conversion handles checking arguments fine; + // its weakness is serializing back to JSON Schema, which we never do here. + const validationSchema = ensureZodSchema(rawJsonSchema) + const served = wrapJsonSchema( + rawJsonSchema as unknown as Parameters[0], + { + validate: (value: unknown) => { + const result = validationSchema.safeParse(value) + return result.success + ? { success: true as const, value: result.data } + : { success: false as const, error: result.error } + }, + }, + ) + if ( + typeof rawJsonSchema.description === 'string' && + rawJsonSchema.description.length > 0 + ) { + ;(served as { description?: string }).description ??= + rawJsonSchema.description + } + return served +} diff --git a/packages/agent-runtime/src/tools/tool-executor.ts b/packages/agent-runtime/src/tools/tool-executor.ts index 5d4b108aea..361fadd749 100644 --- a/packages/agent-runtime/src/tools/tool-executor.ts +++ b/packages/agent-runtime/src/tools/tool-executor.ts @@ -10,6 +10,7 @@ import { formatValueForError } from '../util/format-value' import { codebuffToolHandlers } from './handlers/list' import { getMatchingSpawn } from './handlers/tool/spawn-agent-utils' import { getAgentTemplate } from '../templates/agent-registry' +import { repairStringEncodedUnionMembers } from '../util/repair-string-encoded-union-members' import { resolveGravityIndexLink } from './gravity-index-cta' import { ensureZodSchema } from './prompts' @@ -571,52 +572,6 @@ export async function executeToolCall( }) } -/** - * Repairs values the model string-encoded against its schema. When a - * parameter's declared schema is a union containing an object variant, a - * model may emit the object as a JSON-encoded string (a string is - * unambiguously valid for the union, so nothing downstream fails). The - * schema-guided decode below restores the object the model meant; plain - * strings and params without an object variant are never touched, so - * tools whose string parameters legitimately contain JSON (script - * sources, file contents) are unaffected. - */ -function repairStringEncodedUnionMembers( - parameters: Record, - rawSchema: unknown, -): void { - if (!rawSchema || typeof rawSchema !== 'object') return - const properties = (rawSchema as { properties?: Record }) - .properties - if (!properties) return - for (const [param, value] of Object.entries(parameters)) { - if (typeof value !== 'string') continue - const propSchema = properties[param] - if (!propSchema || typeof propSchema !== 'object') continue - const union = - (propSchema as { anyOf?: unknown[] }).anyOf ?? - (propSchema as { oneOf?: unknown[] }).oneOf - if (!Array.isArray(union)) continue - const hasObjectVariant = union.some( - (variant) => - variant && - typeof variant === 'object' && - (variant as { type?: unknown }).type === 'object', - ) - if (!hasObjectVariant) continue - const trimmed = value.trim() - if (!trimmed.startsWith('{') && !trimmed.startsWith('[')) continue - try { - const decoded = JSON.parse(trimmed) - if (decoded && typeof decoded === 'object') { - parameters[param] = decoded - } - } catch { - // Not JSON after all - the string is a legitimate value. - } - } -} - export function parseRawCustomToolCall(params: { customToolDefs: CustomToolDefinitions rawToolCall: { diff --git a/packages/agent-runtime/src/util/repair-string-encoded-union-members.ts b/packages/agent-runtime/src/util/repair-string-encoded-union-members.ts new file mode 100644 index 0000000000..8026db1832 --- /dev/null +++ b/packages/agent-runtime/src/util/repair-string-encoded-union-members.ts @@ -0,0 +1,45 @@ +/** + * Repairs values the model string-encoded against its schema. When a + * parameter's declared schema is a union containing an object variant, a + * model may emit the object as a JSON-encoded string (a string is + * unambiguously valid for the union, so nothing downstream fails). The + * schema-guided decode below restores the object the model meant; plain + * strings and params without an object variant are never touched, so + * tools whose string parameters legitimately contain JSON (script + * sources, file contents) are unaffected. + */ +export function repairStringEncodedUnionMembers( + parameters: Record, + rawSchema: unknown, +): void { + if (!rawSchema || typeof rawSchema !== 'object') return + const properties = (rawSchema as { properties?: Record }) + .properties + if (!properties) return + for (const [param, value] of Object.entries(parameters)) { + if (typeof value !== 'string') continue + const propSchema = properties[param] + if (!propSchema || typeof propSchema !== 'object') continue + const union = + (propSchema as { anyOf?: unknown[] }).anyOf ?? + (propSchema as { oneOf?: unknown[] }).oneOf + if (!Array.isArray(union)) continue + const hasObjectVariant = union.some( + (variant) => + variant && + typeof variant === 'object' && + (variant as { type?: unknown }).type === 'object', + ) + if (!hasObjectVariant) continue + const trimmed = value.trim() + if (!trimmed.startsWith('{') && !trimmed.startsWith('[')) continue + try { + const decoded = JSON.parse(trimmed) + if (decoded && typeof decoded === 'object') { + parameters[param] = decoded + } + } catch { + // Not JSON after all - the string is a legitimate value. + } + } +} From ff3f4b61141b52c89172acd0448a49964158195d Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 21:08:48 +0200 Subject: [PATCH 09/11] Promote audit suite to permanent checklist-shaped regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tmp/ audit tests proved the bugs and drove the fixes; this replaces them with committed tests at the modules they guard, rewritten to the test-review checklist: cyclomatic complexity 1, AAA with fresh fixtures built through small DSL helpers, contractual trigger-outcome names, single logical outcome per test, no narration comments. - repair-string-encoded-union-members.test.ts: 4 cases (decode, plain string passthrough, real object passthrough, JSON-text string param). - serve-input-schema.test.ts: zod survival + verbatim JSON Schema serving incl. the bare {type:object} amputation repro. - call-mcp-tool-resources.test.ts: real stdio MCP server, fresh client per test; text->json, gzip->descriptive text, png->media. - json-safe-state.test.ts: loopAgentSteps stores plain JSON Schema in agent state, no zod def/shape internals. 79 tests green across the 10 regression files. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../__tests__/call-mcp-tool-resources.test.ts | 113 +++++++++++++ .../mcp/__tests__/mapping-contract-server.ts | 40 +++++ .../__tests__/serve-input-schema.test.ts | 95 +++++++++++ .../util/__tests__/json-safe-state.test.ts | 148 ++++++++++++++++++ ...epair-string-encoded-union-members.test.ts | 103 ++++++++++++ 5 files changed, 499 insertions(+) create mode 100644 common/src/mcp/__tests__/call-mcp-tool-resources.test.ts create mode 100644 common/src/mcp/__tests__/mapping-contract-server.ts create mode 100644 packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts create mode 100644 packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts create mode 100644 packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts diff --git a/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts b/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts new file mode 100644 index 0000000000..18f1c6f4f0 --- /dev/null +++ b/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, test } from 'bun:test' +import { writeFileSync } from 'node:fs' +import { join, dirname } from 'node:path' + +import { callMCPTool, getMCPClient } from '../client' + +import type { MCPConfig } from '../../types/mcp' + +/** + * Application-tier regression tests: a real stdio MCP server is spawned + * with bun and called through the real callMCPTool. Tool results replay + * from history into every later prompt build, so the mapping contract + * (text resources as json values, only images as media, non-image + * binaries degraded to descriptive text) protects the session from + * permanent prompt-build poisoning. + */ + +const SERVER_SCRIPT = String.raw` +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' + +const server = new McpServer({ name: 'mapping-contract-server', version: '1.0.0' }) + +server.registerTool('get_text_resource', { inputSchema: {} }, async () => ({ + content: [{ + type: 'resource', + resource: { + uri: 'file:///notes.txt', + mimeType: 'text/plain', + text: 'Resource 1: This is a plain text resource.', + }, + }], +})) + +server.registerTool('get_gzip_resource', { inputSchema: {} }, async () => ({ + content: [{ + type: 'resource', + resource: { + uri: 'file:///archive.gz', + mimeType: 'application/gzip', + blob: 'aGVsbG8=', + }, + }], +})) + +server.registerTool('get_png_resource', { inputSchema: {} }, async () => ({ + content: [{ + type: 'resource', + resource: { + uri: 'file:///logo.png', + mimeType: 'image/png', + blob: 'aGVsbG8=', + }, + }], +})) + +await server.connect(new StdioServerTransport()) +` + +const EXPECTED_TEXT_RESOURCE = 'Resource 1: This is a plain text resource.' + +const startServer = async (): Promise => { + const scriptPath = join(dirname(import.meta.path), 'mapping-contract-server.ts') + writeFileSync(scriptPath, SERVER_SCRIPT) + const config: MCPConfig = { + type: 'stdio', + command: 'bun', + args: [scriptPath], + env: process.env as Record, + } + return getMCPClient(config) +} + +const callResourceTool = async ( + clientId: string, + toolName: string, +): Promise<{ type: string; value?: string; mediaType?: string }[]> => { + const outputs = await callMCPTool(clientId, { name: toolName, arguments: {} } as never) + return outputs as never +} + +describe('callMCPTool resource mapping contract', () => { + test('maps_text_resource_to_json_value', async () => { + const clientId = await startServer() + + const outputs = await callResourceTool(clientId, 'get_text_resource') + + expect(outputs).toHaveLength(1) + expect(outputs[0].type).toBe('json') + expect(outputs[0].value).toBe(EXPECTED_TEXT_RESOURCE) + }) + + test('degrades_non_image_binary_resource_to_descriptive_text', async () => { + const clientId = await startServer() + + const outputs = await callResourceTool(clientId, 'get_gzip_resource') + + expect(outputs).toHaveLength(1) + expect(outputs[0].type).toBe('json') + expect(outputs[0].value).toContain('application/gzip') + expect(outputs[0].value).toContain('not displayable') + }) + + test('keeps_image_resource_as_media_with_server_mime_type', async () => { + const clientId = await startServer() + + const outputs = await callResourceTool(clientId, 'get_png_resource') + + expect(outputs).toHaveLength(1) + expect(outputs[0].type).toBe('media') + expect(outputs[0].mediaType).toBe('image/png') + }) +}) diff --git a/common/src/mcp/__tests__/mapping-contract-server.ts b/common/src/mcp/__tests__/mapping-contract-server.ts new file mode 100644 index 0000000000..84d082996d --- /dev/null +++ b/common/src/mcp/__tests__/mapping-contract-server.ts @@ -0,0 +1,40 @@ + +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' + +const server = new McpServer({ name: 'mapping-contract-server', version: '1.0.0' }) + +server.registerTool('get_text_resource', { inputSchema: {} }, async () => ({ + content: [{ + type: 'resource', + resource: { + uri: 'file:///notes.txt', + mimeType: 'text/plain', + text: 'Resource 1: This is a plain text resource.', + }, + }], +})) + +server.registerTool('get_gzip_resource', { inputSchema: {} }, async () => ({ + content: [{ + type: 'resource', + resource: { + uri: 'file:///archive.gz', + mimeType: 'application/gzip', + blob: 'aGVsbG8=', + }, + }], +})) + +server.registerTool('get_png_resource', { inputSchema: {} }, async () => ({ + content: [{ + type: 'resource', + resource: { + uri: 'file:///logo.png', + mimeType: 'image/png', + blob: 'aGVsbG8=', + }, + }], +})) + +await server.connect(new StdioServerTransport()) diff --git a/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts b/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts new file mode 100644 index 0000000000..c91ec030d6 --- /dev/null +++ b/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, test } from 'bun:test' + +import { getToolSet } from '../prompts' + +import type { CustomToolDefinitions } from '@codebuff/common/util/file' + +const NO_TOOLS: string[] = [] + +const zodSchemaTool = (schema: unknown): CustomToolDefinitions => + ({ + shaped_tool: { + description: 'A tool defined with a live zod schema', + inputSchema: schema, + }, + }) as never + +const jsonSchemaTool = (schema: Record): CustomToolDefinitions => + ({ + shaped_tool: { + description: 'A tool declared with a JSON Schema', + inputSchema: schema, + }, + }) as never + +const makeToolSet = async (defs: CustomToolDefinitions) => + getToolSet({ + toolNames: NO_TOOLS, + windowedFileReads: false, + additionalToolDefinitions: async () => defs, + agentTools: {} as never, + skills: {} as never, + }) + +const effectiveJsonSchema = async ( + served: unknown, +): Promise> => { + const wrapper = served as { jsonSchema?: unknown; safeParse?: unknown } + const isVerbatimWrapper = + wrapper && + typeof wrapper === 'object' && + 'jsonSchema' in wrapper && + typeof wrapper.safeParse !== 'function' + if (isVerbatimWrapper) { + return wrapper.jsonSchema as Record + } + const { z } = await import('zod/v4') + return z.toJSONSchema(served as never, { io: 'input' }) as Record +} + +describe('getToolSet serves custom tool inputSchemas', () => { + test('keeps_live_zod_schema_functional_through_clone_and_serving', async () => { + const { z } = await import('zod/v4') + const liveSchema = z.object({ path: z.string() }) + + const toolSet = await makeToolSet(zodSchemaTool(liveSchema)) + + const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema + const parse = (served as { safeParse?: (v: unknown) => { success: boolean } }).safeParse + expect(typeof parse).toBe('function') + expect(parse!({ path: 'a.ts' }).success).toBe(true) + + const { z: zod } = await import('zod/v4') + expect(() => zod.toJSONSchema(served as never, { io: 'input' })).not.toThrow() + }) + + test('serves_json_schema_verbatim_preserving_loose_properties', async () => { + const looseSchema = { + type: 'object', + properties: { + verbose: { type: 'boolean', description: 'Enable verbose output' }, + }, + } + + const toolSet = await makeToolSet(jsonSchemaTool(looseSchema)) + + const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema + const modelSchema = await effectiveJsonSchema(served) + expect(modelSchema.properties?.['verbose']).toBeDefined() + }) + + test('serves_bare_object_typed_property_without_amputation', async () => { + const bareObjectSchema = { + type: 'object', + properties: { + payload: { type: 'object', description: 'Arbitrary payload' }, + }, + } + + const toolSet = await makeToolSet(jsonSchemaTool(bareObjectSchema)) + + const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema + const modelSchema = await effectiveJsonSchema(served) + expect((modelSchema.properties?.['payload'] as { type?: string })?.type).toBe('object') + }) +}) diff --git a/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts b/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts new file mode 100644 index 0000000000..e1f4a74c6f --- /dev/null +++ b/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts @@ -0,0 +1,148 @@ +import * as analytics from '@codebuff/common/analytics' +import { TEST_USER_ID } from '@codebuff/common/old-constants' +import { TEST_AGENT_RUNTIME_IMPL } from '@codebuff/common/testing/impl/agent-runtime' +import { + createMockDbOperations, + setupDbSpies, +} from '@codebuff/common/testing/mocks/database' +import { getInitialSessionState } from '@codebuff/common/types/session-state' +import { promptSuccess } from '@codebuff/common/util/error' +import { afterEach, describe, expect, spyOn, test } from 'bun:test' + +import { loopAgentSteps } from '../../run-agent-step' + +import type { AgentTemplate } from '../../templates/types' +import type { DbSpies } from '@codebuff/common/testing/mocks/database' +import type { ProjectFileContext } from '@codebuff/common/util/file' + +/** + * Application-tier regression test: toolDefinitions live in agent state + * (persisted, snapshotted, shipped over the wire), so every stored + * inputSchema must be plain JSON Schema. A live zod instance in state + * serializes as {"def":{...}} internals instead of the declared schema. + */ + +const CUSTOM_TOOL_NAME = 'declared_tool' + +const baseFileContext: ProjectFileContext = { + projectRoot: '/test', + cwd: '/test', + fileTree: [], + fileTokenScores: {}, + knowledgeFiles: {}, + gitChanges: { status: '', diff: '', diffCached: '', lastCommitMessages: '' }, + changesSinceLastChat: {}, + shellConfigFiles: {}, + systemInfo: { + platform: 'test', + shell: 'test', + nodeVersion: 'test', + arch: 'test', + homedir: '/home/test', + cpus: 1, + chromeAvailable: false, + }, + agentTemplates: {}, + customToolDefinitions: {}, +} + +const makeAgent = (): AgentTemplate => ({ + id: 'json-safe-state-agent', + displayName: 'JSON Safe State Agent', + spawnerPrompt: 'Regression: state toolDefinitions stay JSON-safe', + model: 'google/gemini-2.5-flash', + inputSchema: {}, + outputMode: 'last_message' as const, + includeMessageHistory: true, + inheritParentSystemPrompt: false, + mcpServers: {}, + toolNames: [CUSTOM_TOOL_NAME], + spawnableAgents: [], + systemPrompt: 'Test system prompt', + instructionsPrompt: '', + stepPrompt: '', +}) + +const makeFileContextWithDeclaredTool = (schema: unknown): ProjectFileContext => + ({ + ...baseFileContext, + customToolDefinitions: { + [CUSTOM_TOOL_NAME]: { + description: 'A tool declared with a JSON Schema', + inputSchema: schema, + }, + }, + }) as ProjectFileContext + +const runStepToPopulation = async (fileContext: ProjectFileContext) => { + const agent = makeAgent() + const sessionState = getInitialSessionState(baseFileContext) + const agentState = sessionState.mainAgentState + agentState.messageHistory = [] + + await loopAgentSteps({ + ...(TEST_AGENT_RUNTIME_IMPL as never), + sendAction: () => {}, + additionalToolDefinitions: () => Promise.resolve({}), + ancestorRunIds: [], + clientSessionId: 'json-safe-state-session', + fileContext, + fingerprintId: 'json-safe-state-fingerprint', + onResponseChunk: () => {}, + repoId: undefined, + repoUrl: undefined, + runId: 'json-safe-state-run', + signal: new AbortController().signal, + spawnParams: undefined, + system: 'Test system prompt', + tools: {}, + userId: TEST_USER_ID, + userInputId: 'json-safe-state-input', + promptAiSdkStream: async function* () { + yield { type: 'text' as const, text: 'response text' } + return promptSuccess('mock-message-id') + }, + agentType: agent.id, + localAgentTemplates: { [agent.id]: agent }, + agentTemplate: agent, + agentState, + prompt: 'hello', + } as never) + + return agentState +} + +describe('agent state toolDefinitions serialization', () => { + let dbSpies: DbSpies + let analyticsSpy: ReturnType + + afterEach(() => { + dbSpies.restore() + analyticsSpy.mockRestore() + }) + + test('stores_declared_json_schema_without_zod_internals', async () => { + dbSpies = setupDbSpies(createMockDbOperations()) + analyticsSpy = spyOn(analytics, 'trackEvent').mockImplementation(() => {}) + const declaredSchema = { + type: 'object', + properties: { path: { type: 'string', description: 'The file path' } }, + required: ['path'], + } + + const agentState = await runStepToPopulation( + makeFileContextWithDeclaredTool(declaredSchema), + ) + + const toolDefs = agentState.toolDefinitions as Record< + string, + { inputSchema?: unknown } + > + expect(Object.keys(toolDefs)).toContain(CUSTOM_TOOL_NAME) + + const serialized = JSON.stringify(toolDefs[CUSTOM_TOOL_NAME].inputSchema) + const roundTripped = JSON.parse(serialized) as { type?: string } + expect(roundTripped.type).toBe('object') + expect(serialized).not.toContain('"def"') + }) +}) diff --git a/packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts b/packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts new file mode 100644 index 0000000000..17397a4df9 --- /dev/null +++ b/packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, test } from 'bun:test' + +import { parseRawCustomToolCall } from '../../tools/tool-executor' +import { MCP_TOOL_SEPARATOR } from '../../mcp-constants' + +import type { CustomToolDefinitions } from '@codebuff/common/util/file' + +const TOOL_NAME = `demo${MCP_TOOL_SEPARATOR}record_target` + +const UNION_TARGET_SCHEMA = { + type: 'object', + properties: { + target: { + anyOf: [ + { type: 'string' }, + { type: 'object', properties: { path: { type: 'string' } }, required: ['path'] }, + ], + }, + }, + required: ['target'], +} + +const PLAIN_STRING_SCHEMA = { + type: 'object', + properties: { target: { type: 'string' } }, + required: ['target'], +} + +type RawInput = Record + +const makeUnionToolDefs = (): CustomToolDefinitions => + ({ + [TOOL_NAME]: { + description: 'Records a file target', + inputSchema: UNION_TARGET_SCHEMA, + }, + }) as never + +const makeStringToolDefs = (): CustomToolDefinitions => + ({ + [TOOL_NAME]: { + description: 'Records a file target', + inputSchema: PLAIN_STRING_SCHEMA, + }, + }) as never + +const parseTarget = ( + defs: CustomToolDefinitions, + rawInput: RawInput, +): { target?: unknown; error?: string } => { + const result = parseRawCustomToolCall({ + customToolDefs: defs, + rawToolCall: { + toolName: TOOL_NAME, + toolCallId: 'call-target-1', + input: JSON.stringify(rawInput), + }, + }) as { input?: RawInput; error?: string } + return { target: result.input?.target, error: result.error } +} + +describe('parseRawCustomToolCall string-encoded union members', () => { + test('decodes_json_encoded_object_string_for_union_param', () => { + const defs = makeUnionToolDefs() + + const { target, error } = parseTarget(defs, { + target: JSON.stringify({ path: 'src/index.ts' }), + }) + + expect(error).toBeUndefined() + expect(target).toEqual({ path: 'src/index.ts' }) + }) + + test('keeps_plain_string_value_for_union_param', () => { + const defs = makeUnionToolDefs() + + const { target, error } = parseTarget(defs, { target: 'src/plain.txt' }) + + expect(error).toBeUndefined() + expect(target).toBe('src/plain.txt') + }) + + test('keeps_real_object_value_for_union_param', () => { + const defs = makeUnionToolDefs() + + const { target, error } = parseTarget(defs, { + target: { path: 'src/obj.ts' }, + }) + + expect(error).toBeUndefined() + expect(target).toEqual({ path: 'src/obj.ts' }) + }) + + test('keeps_json_text_for_plain_string_param', () => { + const defs = makeStringToolDefs() + const jsonText = '{"path": "src/not-decoded.ts"}' + + const { target, error } = parseTarget(defs, { target: jsonText }) + + expect(error).toBeUndefined() + expect(target).toBe(jsonText) + }) +}) From 580f5bb3645d7a610ef67e60975f37125fa745a0 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 21:13:09 +0200 Subject: [PATCH 10/11] Fix type errors in promoted regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Index the model-facing schema through a typed propertyAt helper and spread the runtime-impl fixture as Record so the new test files typecheck clean alongside the suite. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../src/tools/__tests__/serve-input-schema.test.ts | 12 ++++++++++-- .../src/util/__tests__/json-safe-state.test.ts | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts b/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts index c91ec030d6..8207e62f7b 100644 --- a/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts +++ b/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts @@ -47,6 +47,14 @@ const effectiveJsonSchema = async ( return z.toJSONSchema(served as never, { io: 'input' }) as Record } +const propertyAt = ( + schema: Record, + name: string, +): Record | undefined => + schema.properties as Record> | undefined + ? (schema.properties as Record>)[name] + : undefined + describe('getToolSet serves custom tool inputSchemas', () => { test('keeps_live_zod_schema_functional_through_clone_and_serving', async () => { const { z } = await import('zod/v4') @@ -75,7 +83,7 @@ describe('getToolSet serves custom tool inputSchemas', () => { const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema const modelSchema = await effectiveJsonSchema(served) - expect(modelSchema.properties?.['verbose']).toBeDefined() + expect(propertyAt(modelSchema, 'verbose')).toBeDefined() }) test('serves_bare_object_typed_property_without_amputation', async () => { @@ -90,6 +98,6 @@ describe('getToolSet serves custom tool inputSchemas', () => { const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema const modelSchema = await effectiveJsonSchema(served) - expect((modelSchema.properties?.['payload'] as { type?: string })?.type).toBe('object') + expect((propertyAt(modelSchema, 'payload') as { type?: string })?.type).toBe('object') }) }) diff --git a/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts b/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts index e1f4a74c6f..8491827127 100644 --- a/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts +++ b/packages/agent-runtime/src/util/__tests__/json-safe-state.test.ts @@ -81,7 +81,7 @@ const runStepToPopulation = async (fileContext: ProjectFileContext) => { agentState.messageHistory = [] await loopAgentSteps({ - ...(TEST_AGENT_RUNTIME_IMPL as never), + ...(TEST_AGENT_RUNTIME_IMPL as unknown as Record), sendAction: () => {}, additionalToolDefinitions: () => Promise.resolve({}), ancestorRunIds: [], From 059792d3a6ecf6551156fd37821d481630d9c0b3 Mon Sep 17 00:00:00 2001 From: hsm207 Date: Tue, 8 Sep 2026 21:32:41 +0200 Subject: [PATCH 11/11] Prune redundant tests per handbook test-pruning guidelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same-entry-point duplicates removed: union-repair tests consolidated into parse-raw-custom-tool-call.test.ts (the only file with the real- object-passthrough case), loose-schema cases covered once by the quwin repro in prompts-schema-handling, and the three-transport e2e reduced to one wiring guard since the mapping itself is unit-tested next door. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../__tests__/call-mcp-tool-resources.test.ts | 85 +++------------ .../mcp/__tests__/mapping-contract-server.ts | 22 ---- .../__tests__/prompts-schema-handling.test.ts | 102 ----------------- .../parse-raw-custom-tool-call.test.ts | 82 ++++++++++++++ .../__tests__/serve-input-schema.test.ts | 92 ++-------------- ...epair-string-encoded-union-members.test.ts | 103 ------------------ 6 files changed, 109 insertions(+), 377 deletions(-) create mode 100644 packages/agent-runtime/src/tools/__tests__/parse-raw-custom-tool-call.test.ts delete mode 100644 packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts diff --git a/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts b/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts index 18f1c6f4f0..90aab414b2 100644 --- a/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts +++ b/common/src/mcp/__tests__/call-mcp-tool-resources.test.ts @@ -7,12 +7,12 @@ import { callMCPTool, getMCPClient } from '../client' import type { MCPConfig } from '../../types/mcp' /** - * Application-tier regression tests: a real stdio MCP server is spawned - * with bun and called through the real callMCPTool. Tool results replay - * from history into every later prompt build, so the mapping contract - * (text resources as json values, only images as media, non-image - * binaries degraded to descriptive text) protects the session from - * permanent prompt-build poisoning. + * Wiring guard: the resource-mapping fix lives in + * mcpContentToToolResultOutputs (unit-tested exhaustively next door in + * mcp-content-mapping.test.ts). This test pins only the unique confidence + * of the wiring — that the real stdio transport's tool results flow + * through that mapping and reach callMCPTool's caller — not the mapping + * itself. */ const SERVER_SCRIPT = String.raw` @@ -32,34 +32,12 @@ server.registerTool('get_text_resource', { inputSchema: {} }, async () => ({ }], })) -server.registerTool('get_gzip_resource', { inputSchema: {} }, async () => ({ - content: [{ - type: 'resource', - resource: { - uri: 'file:///archive.gz', - mimeType: 'application/gzip', - blob: 'aGVsbG8=', - }, - }], -})) - -server.registerTool('get_png_resource', { inputSchema: {} }, async () => ({ - content: [{ - type: 'resource', - resource: { - uri: 'file:///logo.png', - mimeType: 'image/png', - blob: 'aGVsbG8=', - }, - }], -})) - await server.connect(new StdioServerTransport()) ` -const EXPECTED_TEXT_RESOURCE = 'Resource 1: This is a plain text resource.' +const EXPECTED_TEXT = 'Resource 1: This is a plain text resource.' -const startServer = async (): Promise => { +test('callMCPTool wires real stdio tool results through the resource mapping', async () => { const scriptPath = join(dirname(import.meta.path), 'mapping-contract-server.ts') writeFileSync(scriptPath, SERVER_SCRIPT) const config: MCPConfig = { @@ -68,46 +46,15 @@ const startServer = async (): Promise => { args: [scriptPath], env: process.env as Record, } - return getMCPClient(config) -} - -const callResourceTool = async ( - clientId: string, - toolName: string, -): Promise<{ type: string; value?: string; mediaType?: string }[]> => { - const outputs = await callMCPTool(clientId, { name: toolName, arguments: {} } as never) - return outputs as never -} - -describe('callMCPTool resource mapping contract', () => { - test('maps_text_resource_to_json_value', async () => { - const clientId = await startServer() - - const outputs = await callResourceTool(clientId, 'get_text_resource') - - expect(outputs).toHaveLength(1) - expect(outputs[0].type).toBe('json') - expect(outputs[0].value).toBe(EXPECTED_TEXT_RESOURCE) - }) - - test('degrades_non_image_binary_resource_to_descriptive_text', async () => { - const clientId = await startServer() - - const outputs = await callResourceTool(clientId, 'get_gzip_resource') - - expect(outputs).toHaveLength(1) - expect(outputs[0].type).toBe('json') - expect(outputs[0].value).toContain('application/gzip') - expect(outputs[0].value).toContain('not displayable') - }) - test('keeps_image_resource_as_media_with_server_mime_type', async () => { - const clientId = await startServer() + const clientId = await getMCPClient(config) - const outputs = await callResourceTool(clientId, 'get_png_resource') + const outputs = (await callMCPTool(clientId, { + name: 'get_text_resource', + arguments: {}, + } as never)) as { type: string; value?: string }[] - expect(outputs).toHaveLength(1) - expect(outputs[0].type).toBe('media') - expect(outputs[0].mediaType).toBe('image/png') - }) + expect(outputs).toHaveLength(1) + expect(outputs[0].type).toBe('json') + expect(outputs[0].value).toBe(EXPECTED_TEXT) }) diff --git a/common/src/mcp/__tests__/mapping-contract-server.ts b/common/src/mcp/__tests__/mapping-contract-server.ts index 84d082996d..9fbff91371 100644 --- a/common/src/mcp/__tests__/mapping-contract-server.ts +++ b/common/src/mcp/__tests__/mapping-contract-server.ts @@ -15,26 +15,4 @@ server.registerTool('get_text_resource', { inputSchema: {} }, async () => ({ }], })) -server.registerTool('get_gzip_resource', { inputSchema: {} }, async () => ({ - content: [{ - type: 'resource', - resource: { - uri: 'file:///archive.gz', - mimeType: 'application/gzip', - blob: 'aGVsbG8=', - }, - }], -})) - -server.registerTool('get_png_resource', { inputSchema: {} }, async () => ({ - content: [{ - type: 'resource', - resource: { - uri: 'file:///logo.png', - mimeType: 'image/png', - blob: 'aGVsbG8=', - }, - }], -})) - await server.connect(new StdioServerTransport()) diff --git a/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts b/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts index d2c6003263..6bcc5deb0c 100644 --- a/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts +++ b/packages/agent-runtime/src/__tests__/prompts-schema-handling.test.ts @@ -646,105 +646,3 @@ describe('getToolSet: loose MCP schemas survive the point-of-use round-trip', () }) }) -// Some models hedge on union-typed parameters: when a schema says a param may -// be a string OR an object (anyOf), the model sometimes emits the object as a -// JSON-encoded string, because a string is unambiguously valid for the union. -// The whole pipeline preserves that string faithfully, so the MCP server -// receives a string where an object was meant - and since the union accepts -// strings, nothing fails loudly. The tool-executor already repairs -// double-encoded arguments at the top level; these tests pin the same repair -// for nested, schema-guided cases. Found while digging into quwin's report -// on issue #912. -describe('parseRawCustomToolCall: schema-guided repair of string-encoded union members', () => { - const buildWithCustomTool = (inputSchema: unknown) => ({ - customToolDefs: { - 'loose-server__loose_union': { - description: 'Echoes back exactly the arguments it received.', - inputSchema: inputSchema as never, - endsAgentStep: false, - }, - }, - rawToolCall: { - toolName: 'loose-server__loose_union', - toolCallId: 'probe-1', - }, - }) - - const unionSchema = { - $schema: 'http://json-schema.org/draft-07/schema#', - type: 'object', - properties: { - spec: { - anyOf: [{ type: 'string' }, { type: 'object', properties: { kind: { type: 'string' } }, additionalProperties: true }], - description: 'A string or an object. Either is accepted.', - }, - }, - required: ['spec'], - additionalProperties: true, - } - - test('a JSON-encoded string for a union param with an object variant is decoded to an object', () => { - // Given a union schema (string | object) and a raw tool call whose - // union-typed parameter arrived as a JSON-encoded string, - // when the custom tool call is parsed, - // then the parameter is decoded to the object the model meant - - // matching what the same model emits for unambiguous object params. - - // Arrange - const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) - const withInput = { ...rawToolCall, input: { spec: '{"kind": "unhinged-union-spec", "extra": 42}' } } - - // Act - const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) - - // Assert - expect(result).toHaveProperty('input') - expect((result as { input: { spec: unknown } }).input.spec).toEqual({ - kind: 'unhinged-union-spec', - extra: 42, - }) - }) - - test('a non-JSON string for a union param stays a string', () => { - // Given the same union schema and a parameter that is a plain string - // (not JSON-encoded), - // when parsed, - // then the string is preserved - the string branch of the union is a - // legitimate choice and must not be mangled. - - // Arrange - const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) - const withInput = { ...rawToolCall, input: { spec: 'plain-string-variant' } } - - // Act - const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) - - // Assert - expect((result as { input: { spec: unknown } }).input.spec).toBe('plain-string-variant') - }) - - test('a JSON-encoded string for a plain string-typed param is NOT decoded', () => { - // Given a schema whose param is a plain string (no object variant) and a - // value that happens to be JSON-encoded, - // when parsed, - // then the string stays a string - the repair must be guided by the - // schema, or tools whose string params legitimately contain JSON (like - // evaluate_script source) would be corrupted. - - // Arrange - const stringOnlySchema = { - type: 'object', - properties: { code: { type: 'string' } }, - required: ['code'], - additionalProperties: false, - } - const { customToolDefs, rawToolCall } = buildWithCustomTool(stringOnlySchema) - const withInput = { ...rawToolCall, input: { code: '{"looks": "like json"}' } } - - // Act - const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) - - // Assert - expect((result as { input: { code: unknown } }).input.code).toBe('{"looks": "like json"}') - }) -}) diff --git a/packages/agent-runtime/src/tools/__tests__/parse-raw-custom-tool-call.test.ts b/packages/agent-runtime/src/tools/__tests__/parse-raw-custom-tool-call.test.ts new file mode 100644 index 0000000000..b6a3addb5c --- /dev/null +++ b/packages/agent-runtime/src/tools/__tests__/parse-raw-custom-tool-call.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, test } from 'bun:test' + +import { parseRawCustomToolCall } from '../tool-executor' + +/** + * Regression tests for schema-guided repair of string-encoded union + * members in parseRawCustomToolCall. + */ + +const buildWithCustomTool = (inputSchema: unknown) => ({ + customToolDefs: { + 'loose-server__loose_union': { + description: 'Echoes back exactly the arguments it received.', + inputSchema: inputSchema as never, + endsAgentStep: false, + }, + }, + rawToolCall: { + toolName: 'loose-server__loose_union', + toolCallId: 'probe-1', + }, +}) + +const unionSchema = { + $schema: 'http://json-schema.org/draft-07/schema#', + type: 'object', + properties: { + spec: { + anyOf: [{ type: 'string' }, { type: 'object', properties: { kind: { type: 'string' } }, additionalProperties: true }], + description: 'A string or an object. Either is accepted.', + }, + }, + required: ['spec'], + additionalProperties: true, +} + +describe('parseRawCustomToolCall: schema-guided repair of string-encoded union members', () => { + test('decodes a JSON-encoded string for a union param with an object variant', () => { + const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) + const withInput = { ...rawToolCall, input: { spec: '{"kind": "unhinged-union-spec", "extra": 42}' } } + + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + expect((result as { input: { spec: unknown } }).input.spec).toEqual({ + kind: 'unhinged-union-spec', + extra: 42, + }) + }) + + test('keeps a real object value for a union param unchanged', () => { + const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) + const withInput = { ...rawToolCall, input: { spec: { kind: 'plain-object' } } } + + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + expect((result as { input: { spec: unknown } }).input.spec).toEqual({ kind: 'plain-object' }) + }) + + test('keeps a non-JSON string for a union param as a string', () => { + const { customToolDefs, rawToolCall } = buildWithCustomTool(unionSchema) + const withInput = { ...rawToolCall, input: { spec: 'plain-string-variant' } } + + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + expect((result as { input: { spec: unknown } }).input.spec).toBe('plain-string-variant') + }) + + test('does not decode a JSON-encoded string for a plain string-typed param', () => { + const stringOnlySchema = { + type: 'object', + properties: { code: { type: 'string' } }, + required: ['code'], + additionalProperties: false, + } + const { customToolDefs, rawToolCall } = buildWithCustomTool(stringOnlySchema) + const withInput = { ...rawToolCall, input: { code: '{"looks": "like json"}' } } + + const result = parseRawCustomToolCall({ customToolDefs, rawToolCall: withInput }) + + expect((result as { input: { code: unknown } }).input.code).toBe('{"looks": "like json"}') + }) +}) diff --git a/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts b/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts index 8207e62f7b..cac38c4108 100644 --- a/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts +++ b/packages/agent-runtime/src/tools/__tests__/serve-input-schema.test.ts @@ -2,102 +2,32 @@ import { describe, expect, test } from 'bun:test' import { getToolSet } from '../prompts' -import type { CustomToolDefinitions } from '@codebuff/common/util/file' - -const NO_TOOLS: string[] = [] - -const zodSchemaTool = (schema: unknown): CustomToolDefinitions => - ({ - shaped_tool: { - description: 'A tool defined with a live zod schema', - inputSchema: schema, - }, - }) as never - -const jsonSchemaTool = (schema: Record): CustomToolDefinitions => - ({ - shaped_tool: { - description: 'A tool declared with a JSON Schema', - inputSchema: schema, - }, - }) as never - -const makeToolSet = async (defs: CustomToolDefinitions) => +const makeToolSet = async (inputSchema: unknown) => getToolSet({ - toolNames: NO_TOOLS, + toolNames: [], windowedFileReads: false, - additionalToolDefinitions: async () => defs, + additionalToolDefinitions: async () => + ({ + shaped_tool: { + description: 'A tool defined with a live zod schema', + inputSchema, + }, + }) as never, agentTools: {} as never, skills: {} as never, }) -const effectiveJsonSchema = async ( - served: unknown, -): Promise> => { - const wrapper = served as { jsonSchema?: unknown; safeParse?: unknown } - const isVerbatimWrapper = - wrapper && - typeof wrapper === 'object' && - 'jsonSchema' in wrapper && - typeof wrapper.safeParse !== 'function' - if (isVerbatimWrapper) { - return wrapper.jsonSchema as Record - } - const { z } = await import('zod/v4') - return z.toJSONSchema(served as never, { io: 'input' }) as Record -} - -const propertyAt = ( - schema: Record, - name: string, -): Record | undefined => - schema.properties as Record> | undefined - ? (schema.properties as Record>)[name] - : undefined - describe('getToolSet serves custom tool inputSchemas', () => { test('keeps_live_zod_schema_functional_through_clone_and_serving', async () => { const { z } = await import('zod/v4') const liveSchema = z.object({ path: z.string() }) - const toolSet = await makeToolSet(zodSchemaTool(liveSchema)) + const toolSet = await makeToolSet(liveSchema) const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema const parse = (served as { safeParse?: (v: unknown) => { success: boolean } }).safeParse expect(typeof parse).toBe('function') expect(parse!({ path: 'a.ts' }).success).toBe(true) - - const { z: zod } = await import('zod/v4') - expect(() => zod.toJSONSchema(served as never, { io: 'input' })).not.toThrow() - }) - - test('serves_json_schema_verbatim_preserving_loose_properties', async () => { - const looseSchema = { - type: 'object', - properties: { - verbose: { type: 'boolean', description: 'Enable verbose output' }, - }, - } - - const toolSet = await makeToolSet(jsonSchemaTool(looseSchema)) - - const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema - const modelSchema = await effectiveJsonSchema(served) - expect(propertyAt(modelSchema, 'verbose')).toBeDefined() - }) - - test('serves_bare_object_typed_property_without_amputation', async () => { - const bareObjectSchema = { - type: 'object', - properties: { - payload: { type: 'object', description: 'Arbitrary payload' }, - }, - } - - const toolSet = await makeToolSet(jsonSchemaTool(bareObjectSchema)) - - const served = (toolSet['shaped_tool'] as { inputSchema: unknown }).inputSchema - const modelSchema = await effectiveJsonSchema(served) - expect((propertyAt(modelSchema, 'payload') as { type?: string })?.type).toBe('object') + expect(() => z.toJSONSchema(served as never, { io: 'input' })).not.toThrow() }) }) diff --git a/packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts b/packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts deleted file mode 100644 index 17397a4df9..0000000000 --- a/packages/agent-runtime/src/util/__tests__/repair-string-encoded-union-members.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { describe, expect, test } from 'bun:test' - -import { parseRawCustomToolCall } from '../../tools/tool-executor' -import { MCP_TOOL_SEPARATOR } from '../../mcp-constants' - -import type { CustomToolDefinitions } from '@codebuff/common/util/file' - -const TOOL_NAME = `demo${MCP_TOOL_SEPARATOR}record_target` - -const UNION_TARGET_SCHEMA = { - type: 'object', - properties: { - target: { - anyOf: [ - { type: 'string' }, - { type: 'object', properties: { path: { type: 'string' } }, required: ['path'] }, - ], - }, - }, - required: ['target'], -} - -const PLAIN_STRING_SCHEMA = { - type: 'object', - properties: { target: { type: 'string' } }, - required: ['target'], -} - -type RawInput = Record - -const makeUnionToolDefs = (): CustomToolDefinitions => - ({ - [TOOL_NAME]: { - description: 'Records a file target', - inputSchema: UNION_TARGET_SCHEMA, - }, - }) as never - -const makeStringToolDefs = (): CustomToolDefinitions => - ({ - [TOOL_NAME]: { - description: 'Records a file target', - inputSchema: PLAIN_STRING_SCHEMA, - }, - }) as never - -const parseTarget = ( - defs: CustomToolDefinitions, - rawInput: RawInput, -): { target?: unknown; error?: string } => { - const result = parseRawCustomToolCall({ - customToolDefs: defs, - rawToolCall: { - toolName: TOOL_NAME, - toolCallId: 'call-target-1', - input: JSON.stringify(rawInput), - }, - }) as { input?: RawInput; error?: string } - return { target: result.input?.target, error: result.error } -} - -describe('parseRawCustomToolCall string-encoded union members', () => { - test('decodes_json_encoded_object_string_for_union_param', () => { - const defs = makeUnionToolDefs() - - const { target, error } = parseTarget(defs, { - target: JSON.stringify({ path: 'src/index.ts' }), - }) - - expect(error).toBeUndefined() - expect(target).toEqual({ path: 'src/index.ts' }) - }) - - test('keeps_plain_string_value_for_union_param', () => { - const defs = makeUnionToolDefs() - - const { target, error } = parseTarget(defs, { target: 'src/plain.txt' }) - - expect(error).toBeUndefined() - expect(target).toBe('src/plain.txt') - }) - - test('keeps_real_object_value_for_union_param', () => { - const defs = makeUnionToolDefs() - - const { target, error } = parseTarget(defs, { - target: { path: 'src/obj.ts' }, - }) - - expect(error).toBeUndefined() - expect(target).toEqual({ path: 'src/obj.ts' }) - }) - - test('keeps_json_text_for_plain_string_param', () => { - const defs = makeStringToolDefs() - const jsonText = '{"path": "src/not-decoded.ts"}' - - const { target, error } = parseTarget(defs, { target: jsonText }) - - expect(error).toBeUndefined() - expect(target).toBe(jsonText) - }) -})