Skip to content

Fix four bugs in MCP tool schema and result handling - #1259

Open
hsm207 wants to merge 12 commits into
CodebuffAI:mainfrom
hsm207:fix/mcp-schema-and-media
Open

Fix four bugs in MCP tool schema and result handling#1259
hsm207 wants to merge 12 commits into
CodebuffAI:mainfrom
hsm207:fix/mcp-schema-and-media

Conversation

@hsm207

@hsm207 hsm207 commented Sep 3, 2026

Copy link
Copy Markdown

This PR fixes four bugs in the MCP server integration, including the one reported in #912 (tool inputSchema.properties stripped 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

  1. Tool schemas arrive at the model as {}. lodash cloneDeep drops zod v4's non-enumerable _zod engine, and a silent fallback then serves an empty schema. This is MCP Tool inputSchema.properties stripped when registering tools from external MCP servers #912's properties: {} and its expected string, received undefined errors.
  2. Live zod schemas stored in run state. Tool definitions are serialized every turn, and storing zod instances makes JSON.stringify throw, so the session errors from the second turn onward. Schemas are now stored as plain JSON Schema.
  3. Text resources stored as media. The prompt rebuild base64-decodes file-part data, so a text resource stored as media made every later turn fail with The string contains invalid characters. Text resources now stay text.
  4. Non-image binary resources become media. The OpenAI-compatible chat converter accepts only image file parts and threw at prompt build. Non-image resources now become descriptive text, and the converter degrades other file parts to a placeholder.

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.

hsm207 and others added 12 commits September 3, 2026 18:31
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>
@quwin

quwin commented Sep 3, 2026

Copy link
Copy Markdown

I tested this PR at commit 780db6f against a local NodeSpec Community Edition MCP server using both ordinary and loose/complex JSON Schemas.

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:

Error: Custom types cannot be represented in JSON Schema

The remaining failure path is:

  1. The MCP JSON Schema is correctly preserved in run state.
  2. getToolSet() converts it using zod-from-json-schema.
  3. For the affected schemas, loose objects or union constructs become Zod custom types.
  4. z.toJSONSchema() throws because those custom types cannot be represented.
  5. ensureJsonSchemaCompatible() catches the error and substitutes z.object({}).passthrough().
  6. The model-facing schema consequently has no named properties, so constrained decoding emits {} and the MCP server receives no arguments.

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 getToolSet() but became stale.

My follow-up wraps raw MCP schemas with AI SDK's jsonSchema() helper, while retaining the existing Zod path for native Zod schemas. The wrapper also supplies a Zod-backed validate callback, so AI SDK input validation is preserved without converting the schema back to JSON Schema. On successful validation it returns the original value, preventing permitted loose fields from being stripped.

The focused regression test verifies that:

  • the model-facing JSON Schema remains identical to the MCP schema;
  • permitted arbitrary nested fields survive validation; and
  • invalid arguments are rejected.

Would it be helpful if I opened a small dependent PR against this PR's branch with the implementation and regression test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants