Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: drop unresolved union members in zod schemas #150

Conversation

disintegrator
Copy link
Contributor

@disintegrator disintegrator commented Feb 16, 2025

Motivation and Context

This change enables the strictUnions option in zod-to-json-schema so that any union members that cannot be resolved are excluded from the resulting JSON Schema anyOf array.

Previously, a schema like this:

Before

import { zodToJsonSchema } from "zod-to-json-schema";
import { z } from "zod";

console.dir(
  zodToJsonSchema(
    z.union([
      z
        .string()
        .base64()
        .transform((v) => Uint8Array.from(atob(v), (v) => v.charCodeAt(0))),
      z.instanceof(ReadableStream<Uint8Array>),
      z.instanceof(Blob),
      z.instanceof(ArrayBuffer),
      z.instanceof(Uint8Array),
    ])
  ),
  { depth: null }
);

Prints:

{
  anyOf: [
    { type: "string", contentEncoding: "base64" }, {}, {}, {}, {}
  ],
  $schema: "http://json-schema.org/draft-07/schema#",
}

After

import { zodToJsonSchema } from "zod-to-json-schema";
import { z } from "zod";

console.dir(
  zodToJsonSchema(
    z.union([
      z
        .string()
        .base64()
        .transform((v) => Uint8Array.from(atob(v), (v) => v.charCodeAt(0))),
      z.instanceof(ReadableStream<Uint8Array>),
      z.instanceof(Blob),
      z.instanceof(ArrayBuffer),
      z.instanceof(Uint8Array),
    ]),
    { strictUnions: true }, // 👈
  ),
  { depth: null }
);

Prints:

{
  anyOf: [
    { type: "string", contentEncoding: "base64" }
  ],
  $schema: "http://json-schema.org/draft-07/schema#",
}

I picked this example in particular because I have modelled file uploads using a permissive schema that accepts streams, byte arrays / blobs and base64-encoded strings. In the context of MCP, base64 would be most applicable so I wanted to drop the other union members that couldn't be represented in JSON Schema.

How Has This Been Tested?

Not tested in a real world application yet. I was mostly observing how zod-to-json-schema converted various schemas to JSON Schema and noticed the odd behavior described above.

Breaking Changes

I'm not sure if this is a breaking change. It's possible that some schemas that were serializing to {} were interpreted in some interesting way by a language model but I could not assert if that was a meaningful interpretation.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

This change enables the strictUnions option in zod-to-json-schema so
that any union members that cannot be resolved are excluded from the
resulting JSON Schema anyOf array.

Previously, a schema like this:

**Before**

```ts
import { zodToJsonSchema } from "zod-to-json-schema";
import { z } from "zod";

console.dir(
  zodToJsonSchema(
    z.union([
      z
        .string()
        .base64()
        .transform((v) => Uint8Array.from(atob(v), (v) => v.charCodeAt(0))),
      z.instanceof(ReadableStream<Uint8Array>),
      z.instanceof(Blob),
      z.instanceof(ArrayBuffer),
      z.instanceof(Uint8Array),
    ])
  ),
  { depth: null }
);
```

Prints:

```js
{
  anyOf: [
    { type: "string", contentEncoding: "base64" }, {}, {}, {}, {}
  ],
  $schema: "http://json-schema.org/draft-07/schema#",
}
```

**After**

```ts
import { zodToJsonSchema } from "zod-to-json-schema";
import { z } from "zod";

console.dir(
  zodToJsonSchema(
    z.union([
      z
        .string()
        .base64()
        .transform((v) => Uint8Array.from(atob(v), (v) => v.charCodeAt(0))),
      z.instanceof(ReadableStream<Uint8Array>),
      z.instanceof(Blob),
      z.instanceof(ArrayBuffer),
      z.instanceof(Uint8Array),
    ]),
    { strictUnions: true }, // 👈
  ),
  { depth: null }
);
```

Prints:

```js
{
  anyOf: [
    { type: "string", contentEncoding: "base64" }
  ],
  $schema: "http://json-schema.org/draft-07/schema#",
}
```

---

I picked this example in particular because I have modelled file uploads
using a permissive schema that accepts streams, byte arrays / blobs and
base64-encoded strings. In the context of MCP, base64 would be most
applicable so I wanted to drop the other union members that couldn't be
represented in JSON Schema.
@disintegrator
Copy link
Contributor Author

disintegrator commented Feb 16, 2025

An alternative approach to solving this would be to allow McpServer::tool(...) to take a prepared JSON Schema object. This would allow developers to pull in zod-to-json-schema and configure it as they see fit. Contrived example:

import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { JsonSchema } from "@modelcontextprotocol/sdk/server/types.js";
import { zodToJsonSchema } from "zod-to-json-schema";
import * as z from "zod";

const server = new McpServer();

const fileSchema = z.object({
  file: z.union([
    z.string().base64(),
    z.instanceof(ReadableStream<Uint8Array>),
    z.instanceof(Blob),
    z.instanceof(ArrayBuffer),
    z.instanceof(Uint8Array),
  ]),
});

server.tool(
  "test",
  "description",
  new JsonSchema(
    zodToJsonSchema(fileSchema, { strictUnions: true }),
  ),
  async (args, ctx) => {
    return { content: [] };
  }
);

JsonSchema is a simple data class that can power an instanceof check inside the SDK and go down a different code path than using zodToJsonSchema internally:

this.server.setRequestHandler(
  ListToolsRequestSchema,
  (): ListToolsResult => ({
    tools: Object.entries(this._registeredTools).map(
      ([name, tool]): Tool => {
+       let inputSchema: Tool["inputSchema"];
+       if (tool.inputSchema instanceof JsonSchema) {
+         schema = tool.inputSchema.jsonSchema;
+       } else if (tool.inputSchema) {
+         schema = zodToJsonSchema(tool.inputSchema) as Tool["inputSchema"];
+       } else {
+         schema = EMPTY_OBJECT_JSON_SCHEMA;
+       }

        return {
          name,
          description: tool.description,
+         inputSchema,
-         inputSchema: tool.inputSchema
-           ? zodToJsonSchema(tool.inputSchema) as Tool["inputSchema"])
-           : EMPTY_OBJECT_JSON_SCHEMA,
        };
      },
    ),
  }),
);

If that's preferred then I can close this PR and submit a new one.

Copy link
Member

@jspahrsummers jspahrsummers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fix makes sense—thanks!

@jspahrsummers jspahrsummers merged commit 506ae06 into modelcontextprotocol:main Feb 19, 2025
2 checks passed
@disintegrator disintegrator deleted the zod-to-json-schema-strict-unions branch February 20, 2025 09:13
MediaInfluences pushed a commit to MediaInfluences/typescript-sdk that referenced this pull request Apr 3, 2025
…-json-schema-strict-unions

fix: drop unresolved union members in zod schemas
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