Fix four bugs in MCP tool schema and result handling - #1259
Conversation
Verbatim move of the content.map body of callMCPTool into an exported mcpContentToToolResultOutputs function. callMCPTool now delegates to it. No behavior change; this makes the conversion rules unit-testable without a live MCP client, in preparation for regression tests around resource handling. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
lodash cloneDeep drops zod v4's non-enumerable _zod engine, leaving
clones that look like schemas (safeParse, def, shape all present) but
detonate on first internal use. The visible symptom was MCP/custom tool
input schemas randomly arriving at the model as empty {} - the
ensureJsonSchemaCompatible fallback silently consumed the amputated
schema, and whether a boot got healthy schemas depended on which call
sites happened to trip the clone.
cloneDeepKeepingZod passes zod instances through by reference while
cloning surrounding plain data. Applied at the three clone sites that
touch tool definitions: run-agent-step (custom tool defs), tools/prompts
getToolSet, and tool-executor writeTo.
Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
Characterizes the bug (lodash cloneDeep strips zod v4's non-enumerable _zod engine; the amputated clone then throws in z.toJSONSchema) and pins cloneDeepKeepingZod behavior: schemas pass through by reference with a working engine, plain data still deep-clones, nested schemas survive. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
Tool definitions land in agent state, which is snapshotted and persisted every turn. Storing the live zod schema (as getMCPToolData did for MCP tools, and as mapValues shipped into toolDefinitions) made any JSON.stringify over that state throw "cannot serialize cyclic structures" - visible as a hard session death from turn 2 onward. The fix moves toTokenCountInputSchema into util/to-json-schema.ts (also removes an import cycle) and stores plain JSON Schema at the two state boundaries: loopAgentSteps' toolDefinitions and spawn-agent-inline's parent tool definitions. MCP schemas are now stored exactly as the server sent them; zod conversion happens at point of use via ensureZodSchema / toTokenCountInputSchema. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
Pins the state contract: getMCPToolData must store the server's JSON Schema verbatim (JSON round-trip equality), because tool definitions are persisted and replayed every turn. Includes a characterization of the failure mode (a zod instance round-trips to def/shape internals, not the server schema) and covers toTokenCountInputSchema: zod conversion, the Anthropic type:object backfill for unions, plain passthrough, and $schema stripping. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
A resource whose contents are text was wrapped as a media part with the prose in the data field. When the AI SDK rebuilds the prompt on any later turn, a file part's data that is not a URL gets base64-decoded - and English prose is not base64, so it died with "The string contains invalid characters". Because the poisoned message stays in message history, the session never recovers. Text contents now flow as a plain json tool result. Binary (blob) resources keep the existing media path. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
Pins the resource mapping contract at the extracted pure function: a text resource must reach the model as a text value, never as media (media triggers base64-decoding of the prose on every later prompt build); an image resource stays media. Characterizes the pre-fix behavior that permanently poisoned session history. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
…essions Two layers for the same failure mode: the OpenAI-compatible chat converter (GLM and other OpenAI-compatible providers) accepts only image file parts and threw on anything else. An application/gzip MCP resource converted to media, the converter threw during the next prompt build, and - because the message replays from history every turn - the session was dead permanently. Ingestion (mcp/client.ts): only image/* resources stay media; other binary resources become a descriptive text result the model can read. Defense (convert-to-openai-compatible-chat-messages.ts): non-image file parts degrade to a text placeholder with an approximate byte size instead of throwing. Image data URIs are unchanged. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
Pins both halves of the degrade contract: a non-image binary MCP resource maps to descriptive text (never media) at ingestion, and the OpenAI-compatible converter degrades any non-image file part to a text placeholder with an approximate byte size instead of throwing during prompt build - the throw was what permanently killed sessions. Image file parts must still convert to image_url data URIs. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
… silently The ensureJsonSchemaCompatible fallback (duplicated in tools/prompts.ts and templates/prompts.ts) converts any schema that fails JSON Schema conversion into an empty permissive schema without a trace. During the zod-clone incident this fallback was the masking layer that turned a broken schema into a silent empty tool schema at the model; loud logging here would have surfaced it in minutes. Both copies now accept an optional logger and warn on fallback with the tool name and error. getToolSet and buildAgentToolSet thread an optional logger through; loopAgentSteps passes the one it already holds. getMCPToolData gains a debug receipt per server (tool count) so missing tools are attributable to a specific server load. Logger writes go to the CLI's file sink only, never the TUI. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
…ames No assertion changes. Every test now carries a docstring stating the Given/When/Then contract, names follow trigger-outcome form, arrange/ act/assert stages are visually demarcated, narration comments moved into the docstrings, and the converter test's byte estimate is a named constant carrying its derivation instead of a bare literal. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
The Given/When/Then format is a scenario shape and belongs in per-test docstrings only; module headers describe the shared contract the family of tests protects, so they now carry that context without the labels. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
|
I tested this PR at commit The clone and persistence fixes work for ordinary named schemas, but several tools with loose object fields or unions still fall back to an empty model-facing schema. The Freebuff log reports:
The remaining failure path is:
A minimal reproduction is: {
"type": "object",
"properties": {
"project_id": { "type": "string" },
"payload": { "type": "object" }
},
"required": ["project_id", "payload"]
}The approach I tested builds on the idea from PR #921, which previously proposed preserving the raw schema in My follow-up wraps raw MCP schemas with AI SDK's The focused regression test verifies that:
Would it be helpful if I opened a small dependent PR against this PR's branch with the implementation and regression test? |
This PR fixes four bugs in the MCP server integration, including the one reported in #912 (tool
inputSchema.propertiesstripped at registration). I found them by exercising all 13 tools of@modelcontextprotocol/server-everything, the official example MCP server, end-to-end against a source build.The four bugs
{}. lodashcloneDeepdrops zod v4's non-enumerable_zodengine, and a silent fallback then serves an empty schema. This is MCP Tool inputSchema.properties stripped when registering tools from external MCP servers #912'sproperties: {}and itsexpected string, received undefinederrors.JSON.stringifythrow, so the session errors from the second turn onward. Schemas are now stored as plain JSON Schema.The string contains invalid characters. Text resources now stay text.Testing
Each bug has its own regression tests, in a commit immediately after its fix. The tests fail against the pre-fix code and pass after. The first commit is a behavior-preserving extraction that makes the MCP content mapper testable without a live server. A final commit adds a warning when a schema falls back to empty, so this failure class shows up in logs.