From 4bbdc4c402c387fc7474abb5e2f8fe0c82ccd9ed Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 10 Jul 2025 18:02:34 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=A9=20fix:=20`additionalProperties`=20?= =?UTF-8?q?Handling=20and=20Ref=20Resolution=20in=20Zod=20Schemas=20(#8381?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: false flagging object as empty object when it has `additionalProperties` field * 🔧 fix: Implement $ref resolution in JSON Schema handling * 🔧 fix: Resolve JSON Schema references before conversion to Zod * chore: move zod logic packages/api --- api/server/services/MCP.js | 16 +- api/typedefs.js | 2 +- packages/api/src/index.ts | 1 + packages/api/src/mcp/manager.ts | 9 +- packages/api/src/mcp/types/index.ts | 3 +- .../src => api/src/mcp}/zod.spec.ts | 403 +++++++++++++++++- .../{data-provider/src => api/src/mcp}/zod.ts | 100 ++++- packages/api/src/types/index.ts | 1 + packages/api/src/types/zod.ts | 15 + packages/data-provider/src/index.ts | 1 - 10 files changed, 518 insertions(+), 33 deletions(-) rename packages/{data-provider/src => api/src/mcp}/zod.spec.ts (74%) rename packages/{data-provider/src => api/src/mcp}/zod.ts (82%) create mode 100644 packages/api/src/types/zod.ts diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 527fe2d51..a058117fb 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -2,16 +2,17 @@ const { z } = require('zod'); const { tool } = require('@langchain/core/tools'); const { logger } = require('@librechat/data-schemas'); const { Time, CacheKeys, StepTypes } = require('librechat-data-provider'); -const { sendEvent, normalizeServerName, MCPOAuthHandler } = require('@librechat/api'); const { Constants: AgentConstants, Providers, GraphEvents } = require('@librechat/agents'); +const { Constants, ContentTypes, isAssistantsEndpoint } = require('librechat-data-provider'); const { - Constants, - ContentTypes, - isAssistantsEndpoint, + sendEvent, + MCPOAuthHandler, + normalizeServerName, + resolveJsonSchemaRefs, convertJsonSchemaToZod, -} = require('librechat-data-provider'); -const { getMCPManager, getFlowStateManager } = require('~/config'); +} = require('@librechat/api'); const { findToken, createToken, updateToken } = require('~/models'); +const { getMCPManager, getFlowStateManager } = require('~/config'); const { getCachedTools } = require('./Config'); const { getLogStores } = require('~/cache'); @@ -113,7 +114,8 @@ async function createMCPTool({ req, res, toolKey, provider: _provider }) { /** @type {LCTool} */ const { description, parameters } = toolDefinition; const isGoogle = _provider === Providers.VERTEXAI || _provider === Providers.GOOGLE; - let schema = convertJsonSchemaToZod(parameters, { + const resolvedJsonSchema = resolveJsonSchemaRefs(parameters); + let schema = convertJsonSchemaToZod(resolvedJsonSchema, { allowEmptyObject: !isGoogle, transformOneOfAnyOf: true, }); diff --git a/api/typedefs.js b/api/typedefs.js index c0e0dd5f4..3ad736624 100644 --- a/api/typedefs.js +++ b/api/typedefs.js @@ -1074,7 +1074,7 @@ /** * @exports JsonSchemaType - * @typedef {import('librechat-data-provider').JsonSchemaType} JsonSchemaType + * @typedef {import('@librechat/api').JsonSchemaType} JsonSchemaType * @memberof typedefs */ diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index 0b0029324..afdd2141c 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -2,6 +2,7 @@ export * from './mcp/manager'; export * from './mcp/oauth'; export * from './mcp/auth'; +export * from './mcp/zod'; /* Utilities */ export * from './mcp/utils'; export * from './utils'; diff --git a/packages/api/src/mcp/manager.ts b/packages/api/src/mcp/manager.ts index 8b51b3cf8..b4a6a71a1 100644 --- a/packages/api/src/mcp/manager.ts +++ b/packages/api/src/mcp/manager.ts @@ -1,11 +1,12 @@ import { logger } from '@librechat/data-schemas'; import { CallToolResultSchema, ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; -import type { RequestOptions } from '@modelcontextprotocol/sdk/shared/protocol.js'; import type { OAuthClientInformation } from '@modelcontextprotocol/sdk/shared/auth.js'; -import type { JsonSchemaType, TUser } from 'librechat-data-provider'; +import type { RequestOptions } from '@modelcontextprotocol/sdk/shared/protocol.js'; import type { TokenMethods } from '@librechat/data-schemas'; -import type { FlowStateManager } from '~/flow/manager'; +import type { TUser } from 'librechat-data-provider'; import type { MCPOAuthTokens, MCPOAuthFlowMetadata } from './oauth/types'; +import type { FlowStateManager } from '~/flow/manager'; +import type { JsonSchemaType } from '~/types/zod'; import type { FlowMetadata } from '~/flow/types'; import type * as t from './types'; import { CONSTANTS, isSystemUserId } from './enum'; @@ -892,7 +893,7 @@ export class MCPManager { this.updateUserLastActivity(userId); } this.checkIdleConnections(); - return formatToolContent(result, provider); + return formatToolContent(result as t.MCPToolCallResponse, provider); } catch (error) { // Log with context and re-throw or handle as needed logger.error(`${logPrefix}[${toolName}] Tool call failed`, error); diff --git a/packages/api/src/mcp/types/index.ts b/packages/api/src/mcp/types/index.ts index d95251eec..7fc72e18c 100644 --- a/packages/api/src/mcp/types/index.ts +++ b/packages/api/src/mcp/types/index.ts @@ -7,8 +7,9 @@ import { WebSocketOptionsSchema, StreamableHTTPOptionsSchema, } from 'librechat-data-provider'; -import type { JsonSchemaType, TPlugin } from 'librechat-data-provider'; import type * as t from '@modelcontextprotocol/sdk/types.js'; +import type { TPlugin } from 'librechat-data-provider'; +import type { JsonSchemaType } from '~/types/zod'; export type StdioOptions = z.infer; export type WebSocketOptions = z.infer; diff --git a/packages/data-provider/src/zod.spec.ts b/packages/api/src/mcp/zod.spec.ts similarity index 74% rename from packages/data-provider/src/zod.spec.ts rename to packages/api/src/mcp/zod.spec.ts index 6da3a4f24..28199646c 100644 --- a/packages/data-provider/src/zod.spec.ts +++ b/packages/api/src/mcp/zod.spec.ts @@ -1,8 +1,8 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ // zod.spec.ts import { z } from 'zod'; -import { convertJsonSchemaToZod } from './zod'; -import type { JsonSchemaType } from './zod'; +import type { JsonSchemaType } from '~/types'; +import { resolveJsonSchemaRefs, convertJsonSchemaToZod } from './zod'; describe('convertJsonSchemaToZod', () => { describe('primitive types', () => { @@ -1083,4 +1083,403 @@ describe('convertJsonSchemaToZod', () => { expect(() => zodSchema?.parse(true)).toThrow(); }); }); + + describe('additionalProperties with anyOf/oneOf and allowEmptyObject', () => { + it('should handle anyOf with object containing only additionalProperties when allowEmptyObject is false', () => { + const schema: JsonSchemaType & { anyOf?: any } = { + type: 'object', + properties: { + filter: { + description: 'Filter field', + anyOf: [ + { + type: 'object', + additionalProperties: { + type: 'object', + properties: { + _icontains: { type: 'string' }, + }, + }, + }, + { + type: 'null', + }, + ], + } as JsonSchemaType & { anyOf?: any }, + }, + }; + + const zodSchema = convertJsonSchemaToZod(schema, { + allowEmptyObject: false, + transformOneOfAnyOf: true, + }); + + expect(zodSchema).toBeDefined(); + + const testData = { + filter: { + title: { + _icontains: 'Pirate', + }, + }, + }; + + const result = zodSchema?.parse(testData); + expect(result).toEqual(testData); + expect(result?.filter).toBeDefined(); + expect(result?.filter?.title?._icontains).toBe('Pirate'); + }); + + it('should not treat objects with additionalProperties as empty', () => { + const schema: JsonSchemaType = { + type: 'object', + additionalProperties: { + type: 'string', + }, + }; + + const zodSchemaWithoutAllow = convertJsonSchemaToZod(schema, { + allowEmptyObject: false, + }); + + // Should not return undefined because it has additionalProperties + expect(zodSchemaWithoutAllow).toBeDefined(); + + const testData = { + customField: 'value', + }; + + expect(zodSchemaWithoutAllow?.parse(testData)).toEqual(testData); + }); + + it('should handle oneOf with object containing only additionalProperties', () => { + const schema: JsonSchemaType & { oneOf?: any } = { + type: 'object', + properties: {}, + oneOf: [ + { + type: 'object', + additionalProperties: true, + }, + { + type: 'object', + properties: { + specificField: { type: 'string' }, + }, + }, + ], + }; + + const zodSchema = convertJsonSchemaToZod(schema, { + allowEmptyObject: false, + transformOneOfAnyOf: true, + }); + + expect(zodSchema).toBeDefined(); + + // Test with additional properties + const testData1 = { + randomField: 'value', + anotherField: 123, + }; + + expect(zodSchema?.parse(testData1)).toEqual(testData1); + + // Test with specific field + const testData2 = { + specificField: 'test', + }; + + expect(zodSchema?.parse(testData2)).toEqual(testData2); + }); + + it('should handle complex nested schema with $ref-like structure', () => { + const schema: JsonSchemaType & { anyOf?: any } = { + type: 'object', + properties: { + query: { + type: 'object', + properties: { + filter: { + description: 'Filter conditions', + anyOf: [ + { + // This simulates a resolved $ref + anyOf: [ + { + type: 'object', + properties: { + _or: { + type: 'array', + items: { type: 'object' }, + }, + }, + required: ['_or'], + }, + { + type: 'object', + additionalProperties: { + anyOf: [ + { + type: 'object', + properties: { + _icontains: { type: 'string' }, + _eq: { type: 'string' }, + }, + }, + ], + }, + }, + ], + }, + { + type: 'null', + }, + ], + } as JsonSchemaType & { anyOf?: any }, + }, + }, + }, + }; + + const zodSchema = convertJsonSchemaToZod(schema, { + allowEmptyObject: false, + transformOneOfAnyOf: true, + }); + + expect(zodSchema).toBeDefined(); + + const testData = { + query: { + filter: { + title: { + _icontains: 'Pirate', + }, + }, + }, + }; + + const result = zodSchema?.parse(testData); + expect(result).toEqual(testData); + expect(result?.query?.filter?.title?._icontains).toBe('Pirate'); + }); + }); + + describe('$ref resolution with resolveJsonSchemaRefs', () => { + it('should handle schemas with $ref references when resolved', () => { + const schemaWithRefs = { + type: 'object' as const, + properties: { + collection: { + type: 'string' as const, + }, + query: { + type: 'object' as const, + properties: { + filter: { + anyOf: [{ $ref: '#/$defs/__schema0' }, { type: 'null' as const }], + }, + }, + }, + }, + required: ['collection', 'query'], + $defs: { + __schema0: { + anyOf: [ + { + type: 'object' as const, + properties: { + _or: { + type: 'array' as const, + items: { $ref: '#/$defs/__schema0' }, + }, + }, + required: ['_or'], + }, + { + type: 'object' as const, + additionalProperties: { + anyOf: [ + { + type: 'object' as const, + properties: { + _eq: { + anyOf: [ + { type: 'string' as const }, + { type: 'number' as const }, + { type: 'null' as const }, + ], + }, + }, + }, + ], + }, + }, + ], + }, + }, + }; + + // First test without resolving refs - should not work properly + const zodSchemaUnresolved = convertJsonSchemaToZod(schemaWithRefs as any, { + allowEmptyObject: true, + transformOneOfAnyOf: true, + }); + + const testData = { + collection: 'posts', + query: { + filter: { + status: { + _eq: 'draft', + }, + }, + }, + }; + + // Without resolving refs, the filter field won't work correctly + const resultUnresolved = zodSchemaUnresolved?.parse(testData); + expect(resultUnresolved?.query?.filter).toEqual({}); + + // Now resolve refs first + const resolvedSchema = resolveJsonSchemaRefs(schemaWithRefs); + + // Verify refs were resolved + expect(resolvedSchema.properties?.query?.properties?.filter?.anyOf?.[0]).not.toHaveProperty( + '$ref', + ); + expect(resolvedSchema.properties?.query?.properties?.filter?.anyOf?.[0]).toHaveProperty( + 'anyOf', + ); + + const zodSchemaResolved = convertJsonSchemaToZod(resolvedSchema as any, { + allowEmptyObject: true, + transformOneOfAnyOf: true, + }); + + // With resolved refs, it should work correctly + const resultResolved = zodSchemaResolved?.parse(testData); + expect(resultResolved).toEqual(testData); + expect(resultResolved?.query?.filter?.status?._eq).toBe('draft'); + }); + + it('should handle circular $ref references without infinite loops', () => { + const schemaWithCircularRefs = { + type: 'object' as const, + properties: { + node: { $ref: '#/$defs/TreeNode' }, + }, + $defs: { + TreeNode: { + type: 'object' as const, + properties: { + value: { type: 'string' as const }, + children: { + type: 'array' as const, + items: { $ref: '#/$defs/TreeNode' }, + }, + }, + }, + }, + }; + + // Should not throw or hang + const resolved = resolveJsonSchemaRefs(schemaWithCircularRefs); + expect(resolved).toBeDefined(); + + // The circular reference should be broken with a simple object schema + const zodSchema = convertJsonSchemaToZod(resolved as any, { + allowEmptyObject: true, + transformOneOfAnyOf: true, + }); + + expect(zodSchema).toBeDefined(); + + const testData = { + node: { + value: 'root', + children: [ + { + value: 'child1', + children: [], + }, + ], + }, + }; + + expect(() => zodSchema?.parse(testData)).not.toThrow(); + }); + + it('should handle various edge cases safely', () => { + // Test with null/undefined + expect(resolveJsonSchemaRefs(null as any)).toBeNull(); + expect(resolveJsonSchemaRefs(undefined as any)).toBeUndefined(); + + // Test with non-object primitives + expect(resolveJsonSchemaRefs('string' as any)).toBe('string'); + expect(resolveJsonSchemaRefs(42 as any)).toBe(42); + expect(resolveJsonSchemaRefs(true as any)).toBe(true); + + // Test with arrays + const arrayInput = [{ type: 'string' }, { $ref: '#/def' }]; + const arrayResult = resolveJsonSchemaRefs(arrayInput as any); + expect(Array.isArray(arrayResult)).toBe(true); + expect(arrayResult).toHaveLength(2); + + // Test with schema that has no refs + const noRefSchema = { + type: 'object' as const, + properties: { + name: { type: 'string' as const }, + nested: { + type: 'object' as const, + properties: { + value: { type: 'number' as const }, + }, + }, + }, + }; + + const resolvedNoRef = resolveJsonSchemaRefs(noRefSchema); + expect(resolvedNoRef).toEqual(noRefSchema); + + // Test with invalid ref (non-existent) + const invalidRefSchema = { + type: 'object' as const, + properties: { + item: { $ref: '#/$defs/nonExistent' }, + }, + $defs: { + other: { type: 'string' as const }, + }, + }; + + const resolvedInvalid = resolveJsonSchemaRefs(invalidRefSchema); + // Invalid refs should be preserved as-is + expect(resolvedInvalid.properties?.item?.$ref).toBe('#/$defs/nonExistent'); + + // Test with empty object + expect(resolveJsonSchemaRefs({})).toEqual({}); + + // Test with schema containing special JSON Schema keywords + const schemaWithKeywords = { + type: 'object' as const, + properties: { + value: { + type: 'string' as const, + minLength: 5, + maxLength: 10, + pattern: '^[A-Z]', + }, + }, + additionalProperties: false, + minProperties: 1, + }; + + const resolvedKeywords = resolveJsonSchemaRefs(schemaWithKeywords); + expect(resolvedKeywords).toEqual(schemaWithKeywords); + expect(resolvedKeywords.properties?.value?.minLength).toBe(5); + expect(resolvedKeywords.additionalProperties).toBe(false); + }); + }); }); diff --git a/packages/data-provider/src/zod.ts b/packages/api/src/mcp/zod.ts similarity index 82% rename from packages/data-provider/src/zod.ts rename to packages/api/src/mcp/zod.ts index bffc693b4..6a3e3194b 100644 --- a/packages/data-provider/src/zod.ts +++ b/packages/api/src/mcp/zod.ts @@ -1,30 +1,16 @@ import { z } from 'zod'; - -export type JsonSchemaType = { - type: 'string' | 'number' | 'boolean' | 'array' | 'object'; - enum?: string[]; - items?: JsonSchemaType; - properties?: Record; - required?: string[]; - description?: string; - additionalProperties?: boolean | JsonSchemaType; -}; +import type { JsonSchemaType, ConvertJsonSchemaToZodOptions } from '~/types'; function isEmptyObjectSchema(jsonSchema?: JsonSchemaType): boolean { return ( jsonSchema != null && typeof jsonSchema === 'object' && jsonSchema.type === 'object' && - (jsonSchema.properties == null || Object.keys(jsonSchema.properties).length === 0) + (jsonSchema.properties == null || Object.keys(jsonSchema.properties).length === 0) && + !jsonSchema.additionalProperties // Don't treat objects with additionalProperties as empty ); } -type ConvertJsonSchemaToZodOptions = { - allowEmptyObject?: boolean; - dropFields?: string[]; - transformOneOfAnyOf?: boolean; -}; - function dropSchemaFields( schema: JsonSchemaType | undefined, fields: string[], @@ -98,6 +84,10 @@ function convertToZodUnion( return convertJsonSchemaToZod(objSchema, options); } + return convertJsonSchemaToZod(objSchema, options); + } else if (!subSchema.type && subSchema.additionalProperties) { + // It's likely an object schema with additionalProperties + const objSchema = { ...subSchema, type: 'object' } as JsonSchemaType; return convertJsonSchemaToZod(objSchema, options); } else if (!subSchema.type && subSchema.items) { // It's likely an array schema @@ -182,6 +172,82 @@ function convertToZodUnion( return zodSchemas[0]; } +/** + * Helper function to resolve $ref references + * @param schema - The schema to resolve + * @param definitions - The definitions to use + * @param visited - The set of visited references + * @returns The resolved schema + */ +export function resolveJsonSchemaRefs>( + schema: T, + definitions?: Record, + visited = new Set(), +): T { + // Handle null, undefined, or non-object values first + if (!schema || typeof schema !== 'object') { + return schema; + } + + // If no definitions provided, try to extract from schema.$defs or schema.definitions + if (!definitions) { + definitions = (schema.$defs || schema.definitions) as Record; + } + + // Handle arrays + if (Array.isArray(schema)) { + return schema.map((item) => resolveJsonSchemaRefs(item, definitions, visited)) as unknown as T; + } + + // Handle objects + const result: Record = {}; + + for (const [key, value] of Object.entries(schema)) { + // Skip $defs/definitions at root level to avoid infinite recursion + if ((key === '$defs' || key === 'definitions') && !visited.size) { + result[key] = value; + continue; + } + + // Handle $ref + if (key === '$ref' && typeof value === 'string') { + // Prevent circular references + if (visited.has(value)) { + // Return a simple schema to break the cycle + return { type: 'object' } as unknown as T; + } + + // Extract the reference path + const refPath = value.replace(/^#\/(\$defs|definitions)\//, ''); + const resolved = definitions?.[refPath]; + + if (resolved) { + visited.add(value); + const resolvedSchema = resolveJsonSchemaRefs( + resolved as Record, + definitions, + visited, + ); + visited.delete(value); + + // Merge the resolved schema into the result + Object.assign(result, resolvedSchema); + } else { + // If we can't resolve the reference, keep it as is + result[key] = value; + } + } else if (value && typeof value === 'object') { + // Recursively resolve nested objects/arrays + result[key] = resolveJsonSchemaRefs(value as Record, definitions, visited); + } else { + // Copy primitive values as is + result[key] = value; + } + } + + return result as T; +} + export function convertJsonSchemaToZod( schema: JsonSchemaType & Record, options: ConvertJsonSchemaToZodOptions = {}, diff --git a/packages/api/src/types/index.ts b/packages/api/src/types/index.ts index 6db727529..75ee614ff 100644 --- a/packages/api/src/types/index.ts +++ b/packages/api/src/types/index.ts @@ -4,3 +4,4 @@ export * from './google'; export * from './mistral'; export * from './openai'; export * from './run'; +export * from './zod'; diff --git a/packages/api/src/types/zod.ts b/packages/api/src/types/zod.ts new file mode 100644 index 000000000..e8b3c9327 --- /dev/null +++ b/packages/api/src/types/zod.ts @@ -0,0 +1,15 @@ +export type JsonSchemaType = { + type: 'string' | 'number' | 'boolean' | 'array' | 'object'; + enum?: string[]; + items?: JsonSchemaType; + properties?: Record; + required?: string[]; + description?: string; + additionalProperties?: boolean | JsonSchemaType; +}; + +export type ConvertJsonSchemaToZodOptions = { + allowEmptyObject?: boolean; + dropFields?: string[]; + transformOneOfAnyOf?: boolean; +}; diff --git a/packages/data-provider/src/index.ts b/packages/data-provider/src/index.ts index 9869029ad..d80861acf 100644 --- a/packages/data-provider/src/index.ts +++ b/packages/data-provider/src/index.ts @@ -8,7 +8,6 @@ export * from './artifacts'; /* schema helpers */ export * from './parsers'; export * from './ocr'; -export * from './zod'; /* custom/dynamic configurations */ export * from './generate'; export * from './models';