Skip to content

Fix six bug families in MCP tool schema and result handling - #1259

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

Fix six bug families in MCP tool schema and result handling#1259
hsm207 wants to merge 11 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 six bug families 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, and verified every fix two ways: a two-sided regression suite (each test fails on pre-fix main for the exact intended reason) and a live end-to-end session.

The six bug families

  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. Fixed with cloneDeepKeepingZod.
  2. Live zod schemas stored in run state. Tool definitions are serialized every turn, and storing zod instances makes JSON.stringify throw, so sessions die from the second turn onward. Schemas are now normalized to plain JSON Schema before storage (including the subagent path).
  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 degrade to descriptive text, and the converter degrades other file parts to a placeholder.
  5. Loose JSON Schemas amputated by the zod round-trip. MCP permits bare { "type": "object" } properties (SEP-2106); converting to zod and back stripped them, so models called tools with no arguments. JSON Schemas are now served to the model verbatim via ai's jsonSchema(), with zod-backed validation at call time.
  6. String-encoded union members. Models sometimes emit union-typed params as JSON-encoded strings; the pipeline preserved them faithfully and the server received a string where an object was meant. The tool executor now repairs these, guided by the declared schema.

Branch shape

The series is rebuilt from a clean main base: each fix is one small semantic commit, and its new code lives in its own module (util/zod-safe-clone.ts, util/to-json-schema.ts, util/repair-string-encoded-union-members.ts, tools/serve-input-schema.ts, common/src/mcp/content-mapping.ts), leaving one-line import/call-site changes in upstream files. Future rebases against main stay small. Two additional seams the regression suite caught during the rebuild — verbatim schema storage in getMCPToolData, and the non-image file-part degrade in the OpenAI converter — are fixed in the same series.

Testing

Each fix has regression tests at its module. The suite was validated two-sided against pre-fix and post-fix trees, includes real-stdio-server tests for the resource mapping, and the whole set was confirmed in a live session exercising all 13 tools of server-everything with no anomalies across multi-turn replay.

Note: this series intentionally does not add a warning log when a schema falls back to empty — that would modify an upstream function body. Separately, ensureJsonSchemaCompatible's catch block reads schema.description on a clone-stripped schema and throws TypeError (schema._zod.parent), killing the agent step instead of degrading; I'd like to propose that small fix upstream separately, with repro evidence available.

@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?

@codebuff-team

Copy link
Copy Markdown
Contributor

Good work — this is exactly the kind of PR that's easy to evaluate because each fix ships with a regression test that fails on old code and passes on new code.

  • zod-safe-clone.ts: correctly diagnoses that lodash cloneDeep only copies enumerable own properties, and zod v4 keeps its engine on non-enumerable _zod. Passing the schema through by reference in cloneDeepKeepingZod is the right fix, not a deeper clone attempt. Test in zod-safe-clone.test.ts demonstrating the amputation is a nice touch.
  • mcp.ts: storing the raw JSON Schema instead of eagerly converting to zod is the correct layering — ensureZodSchema already existed downstream to convert at point of use, so this isn't adding new machinery, just moving the conversion later so state stays JSON-serializable.
  • client.ts: splitting resource handling into text / image / other-binary is a sensible reading of what the AI SDK's prompt rebuild actually does (base64-decoding file parts). The new mcpContentToToolResultOutputs export is a clean, behavior-preserving extraction that makes this testable.
  • convert-to-openai-compatible-chat-messages.ts: degrading unsupported file parts to a text placeholder instead of throwing is defensible given a thrown error here poisons the whole session via message history replay — though note this also silently swallows any future unsupported file-part case, not just MCP-sourced gzip/pdf; worth flagging in the PR description so reviewers know the tradeoff is deliberate.

Main concern: this bundles four independent fixes plus a to-json-schema.ts extraction into one 679-line, 15-file PR. Each fix stands on its own and would be far easier for a maintainer to port and bisect if it were split into four PRs (you note the commits are already separated, so this is mostly a packaging suggestion, not a rewrite ask).

Overall: correct root causes, in-scope, tested at the right layer. Recommend splitting for future submissions, but this is portable as-is.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 4, 2026
@hsm207

hsm207 commented Sep 5, 2026

Copy link
Copy Markdown
Author

Thanks for the catch. The example schema you gave exposed a gap I had not considered, and your pointer to PR #921 was key to building a better fix. Raw MCP schemas now go through the AI SDK's jsonSchema() wrapper while native zod schemas keep the existing path. A zod-backed validate callback hands back the original value, so loose fields survive instead of being stripped.

Your finding also surfaced a second gap: for anyOf: [string, object] params, the model sometimes emits the object as a JSON-encoded string, which the union accepts, so the server receives a string where an object was meant. Fixed and pinned in the regression tests, where your repro is credited.

Now, if a tool still fails, its schema is complicated, and let's just deal with it when it gets reported. No dependent PRs needed, but thanks for the offer!

hsm207 added a commit to hsm207/codebuff that referenced this pull request Sep 5, 2026
Cold-boot live testing against an echo MCP server showed that when a
tool schema declares a param as a union with an object variant
(anyOf/oneOf), the model may emit the object as a JSON-encoded string -
unambiguously valid for the union, so nothing downstream fails, and the
server receives a string where the model meant an object. The repair is
schema-guided: only params whose declared union includes an object
variant, and whose value parses as JSON, are decoded; plain strings and
string-typed params containing JSON (script sources, file contents) are
untouched. The parse result now returns the validated parameters rather
than the raw input, so repairs reach the handler (this also stops a
latent crash when input is absent).

Tests use quwin's loose-schema shape from PR CodebuffAI#1259 follow-up discussion.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@hsm207
hsm207 force-pushed the fix/mcp-schema-and-media branch from a7c54d7 to bc5d8e9 Compare September 5, 2026 16:39
hsm207 added a commit to hsm207/codebuff that referenced this pull request Sep 5, 2026
Cold-boot live testing against an echo MCP server showed that when a
tool schema declares a param as a union with an object variant
(anyOf/oneOf), the model may emit the object as a JSON-encoded string -
unambiguously valid for the union, so nothing downstream fails, and the
server receives a string where the model meant an object. The repair is
schema-guided: only params whose declared union includes an object
variant, and whose value parses as JSON, are decoded; plain strings and
string-typed params containing JSON (script sources, file contents) are
untouched. The parse result now returns the validated parameters rather
than the raw input, so repairs reach the handler (this also stops a
latent crash when input is absent).

Tests use quwin's loose-schema shape from PR CodebuffAI#1259 follow-up discussion.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
hsm207 and others added 11 commits September 8, 2026 19:44
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
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 <noreply@codebuff.com>
Index the model-facing schema through a typed propertyAt helper and
spread the runtime-impl fixture as Record<string, unknown> so the new
test files typecheck clean alongside the suite.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
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 <noreply@codebuff.com>
@hsm207
hsm207 force-pushed the fix/mcp-schema-and-media branch from bc5d8e9 to 059792d Compare September 8, 2026 20:44
@hsm207 hsm207 changed the title Fix four bugs in MCP tool schema and result handling Fix six bug families in MCP tool schema and result handling Sep 8, 2026
@hsm207

hsm207 commented Sep 8, 2026

Copy link
Copy Markdown
Author

I've force-pushed a rebuilt version of this series. Since there were no reviews yet, nothing is lost — the discussion stays intact.

What changed and why: the original branch was a rebase that resurrected old files on top of upstream's rewrite, which made the branch diverge structurally from main and would have made every future rebase painful. I rebuilt the series from a clean main base: same fixes (now organized as six bug families, including two extra seams the regression suite caught during the rebuild), but each fix is one small semantic commit with its new code in its own module — so future rebases stay small.

Verification is stronger than before:

  • Every regression test was validated two-sided: it fails on pre-fix main for the exact intended reason, and passes after the fix.
  • The resource-mapping tests run against a real stdio MCP server.
  • The whole set was confirmed in a live end-to-end session exercising all 13 tools of server-everything with multi-turn replay and no anomalies.

@dvelm

dvelm commented Sep 9, 2026

Copy link
Copy Markdown

Independent reproduction confirming the impact of bug families 1, 5, and 6 — filed as #1306 with full logs.

Setup: freebuff CLI 0.0.93, Windows, models glm-5.3-flash and (in a separate session) gpt-5.6-luna.

Repro: with mainstream MCP servers configured (tavily, brave-search, context7, chrome-devtools), every parameterized tool call reaches the server as {} — zod errors expected string, received undefined at paths like ["query"], ["pageId"], ["url"]. Repro rate ~100%; zero successful parameterized MCP calls across the entire session.

Controls (same session): zero-param MCP tools (chrome-devtools__list_pages) succeed, and built-in tools with equally complex nested schemas succeed — isolating the failure to the model-facing MCP schema, i.e. exactly the "no named properties → constrained decoding emits {} → MCP server receives no arguments" path @quwin traced above.

Happy to re-run the same 4-server × 2-model matrix against this branch if it helps the merge decision. Side note for anyone debugging similar reports: knowing this root cause gives a recognizable signature — an MCP tool that "works" only when called with zero arguments is almost certainly hitting the empty-schema fallback, and this fix is what makes such tools fully usable again.

@dvelm

dvelm commented Sep 9, 2026

Copy link
Copy Markdown

Cross-control confirmation (2026-09-09): ran the same 4-server x 1-model matrix with deepseek-v4-flash on the same CLI build - all parameterized MCP calls succeed (tavily, brave, chrome-devtools navigate, context7). Combined with #1306's glm-5.3-flash failures and the reported gpt-5.6-luna failures, this is consistent with your families 1/5 diagnosis: the empty model-facing schema only manifests on models that decode strictly against it (emitting {}), while models that fill args from descriptions work around it invisibly. i.e., the fix is model-agnostic, but the repro signature is model-dependent - matching the "no named properties -> constrained decoding emits {}" chain quwin traced.

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

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants