From ccbf9dc09353eda74597809e3b534eda46be018b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 13 Feb 2026 13:33:25 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B0=20fix:=20Convert=20`const`=20to=20?= =?UTF-8?q?`enum`=20in=20MCP=20Schemas=20for=20Gemini=20Compatibility=20(#?= =?UTF-8?q?11784)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Convert `const` to `enum` in MCP tool schemas for Gemini/Vertex AI compatibility Gemini/Vertex AI rejects the JSON Schema `const` keyword in function declarations with a 400 error. Previously, the Zod conversion layer accidentally stripped `const`, but after migrating to pass raw JSON schemas directly to providers, the unsupported keyword now reaches Gemini verbatim. Add `normalizeJsonSchema` to recursively convert `const: X` → `enum: [X]`, which is semantically equivalent per the JSON Schema spec and supported by all providers. * fix: Update secure cookie handling in AuthService to use dynamic secure flag Replaced the static `secure: isProduction` with a call to `shouldUseSecureCookie()` in the `setOpenIDAuthTokens` function. This change ensures that the secure cookie setting is evaluated at runtime, improving cookie handling in development environments while maintaining security in production. * refactor: Simplify MCP tool key formatting and remove unused mocks in tests - Updated MCP test suite to replace static tool key formatting with a dynamic delimiter from Constants, enhancing consistency and maintainability. - Removed unused mock implementations for `@langchain/core/tools` and `@librechat/agents`, streamlining the test setup. - Adjusted related test cases to reflect the new tool key format, ensuring all tests remain functional. * chore: import order --- api/server/services/AuthService.js | 2 +- api/server/services/MCP.js | 5 +- api/server/services/MCP.spec.js | 96 +++------- packages/api/src/mcp/__tests__/zod.spec.ts | 195 ++++++++++++++++++++- packages/api/src/mcp/zod.ts | 59 +++++++ packages/api/src/tools/definitions.ts | 9 +- 6 files changed, 287 insertions(+), 79 deletions(-) diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 1280f9f358..1b4b653c1a 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -488,7 +488,7 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) = res.cookie('openid_id_token', tokenset.id_token, { expires: expirationDate, httpOnly: true, - secure: isProduction, + secure: shouldUseSecureCookie(), sameSite: 'strict', }); } diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 8cb9932097..ad1f9f5cc3 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -11,8 +11,9 @@ const { MCPOAuthHandler, isMCPDomainAllowed, normalizeServerName, - resolveJsonSchemaRefs, + normalizeJsonSchema, GenerationJobManager, + resolveJsonSchemaRefs, } = require('@librechat/api'); const { Time, @@ -443,7 +444,7 @@ function createToolInstance({ const { description, parameters } = toolDefinition; const isGoogle = _provider === Providers.VERTEXAI || _provider === Providers.GOOGLE; - let schema = parameters ? resolveJsonSchemaRefs(parameters) : null; + let schema = parameters ? normalizeJsonSchema(resolveJsonSchemaRefs(parameters)) : null; if (!schema || (isGoogle && isEmptyObjectSchema(schema))) { schema = { diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index 84ec3013dd..b2caebc91e 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -9,30 +9,6 @@ jest.mock('@librechat/data-schemas', () => ({ }, })); -jest.mock('@langchain/core/tools', () => ({ - tool: jest.fn((fn, config) => { - const toolInstance = { _call: fn, ...config }; - return toolInstance; - }), -})); - -jest.mock('@librechat/agents', () => ({ - Providers: { - VERTEXAI: 'vertexai', - GOOGLE: 'google', - }, - StepTypes: { - TOOL_CALLS: 'tool_calls', - }, - GraphEvents: { - ON_RUN_STEP_DELTA: 'on_run_step_delta', - ON_RUN_STEP: 'on_run_step', - }, - Constants: { - CONTENT_AND_ARTIFACT: 'content_and_artifact', - }, -})); - // Create mock registry instance const mockRegistryInstance = { getOAuthServers: jest.fn(() => Promise.resolve(new Set())), @@ -46,26 +22,23 @@ const mockIsMCPDomainAllowed = jest.fn(() => Promise.resolve(true)); const mockGetAppConfig = jest.fn(() => Promise.resolve({})); jest.mock('@librechat/api', () => { - // Access mock via getter to avoid hoisting issues + const actual = jest.requireActual('@librechat/api'); return { - MCPOAuthHandler: { - generateFlowId: jest.fn(), - }, + ...actual, sendEvent: jest.fn(), - normalizeServerName: jest.fn((name) => name), - resolveJsonSchemaRefs: jest.fn((params) => params), get isMCPDomainAllowed() { return mockIsMCPDomainAllowed; }, - MCPServersRegistry: { - getInstance: () => mockRegistryInstance, + GenerationJobManager: { + emitChunk: jest.fn(), }, }; }); const { logger } = require('@librechat/data-schemas'); const { MCPOAuthHandler } = require('@librechat/api'); -const { CacheKeys } = require('librechat-data-provider'); +const { CacheKeys, Constants } = require('librechat-data-provider'); +const D = Constants.mcp_delimiter; const { createMCPTool, createMCPTools, @@ -74,24 +47,6 @@ const { getServerConnectionStatus, } = require('./MCP'); -jest.mock('librechat-data-provider', () => ({ - CacheKeys: { - FLOWS: 'flows', - }, - Constants: { - USE_PRELIM_RESPONSE_MESSAGE_ID: 'prelim_response_id', - mcp_delimiter: '::', - mcp_prefix: 'mcp_', - }, - ContentTypes: { - TEXT: 'text', - }, - isAssistantsEndpoint: jest.fn(() => false), - Time: { - TWO_MINUTES: 120000, - }, -})); - jest.mock('./Config', () => ({ loadCustomConfig: jest.fn(), get getAppConfig() { @@ -132,6 +87,7 @@ describe('tests for the new helper functions used by the MCP connection status e beforeEach(() => { jest.clearAllMocks(); + jest.spyOn(MCPOAuthHandler, 'generateFlowId'); mockGetMCPManager = require('~/config').getMCPManager; mockGetFlowStateManager = require('~/config').getFlowStateManager; @@ -735,7 +691,7 @@ describe('User parameter passing tests', () => { mockReinitMCPServer.mockResolvedValue({ tools: [{ name: 'test-tool' }], availableTools: { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Test tool', parameters: { type: 'object', properties: {} }, @@ -795,7 +751,7 @@ describe('User parameter passing tests', () => { mockReinitMCPServer.mockResolvedValue({ availableTools: { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Test tool', parameters: { type: 'object', properties: {} }, @@ -808,7 +764,7 @@ describe('User parameter passing tests', () => { await createMCPTool({ res: mockRes, user: mockUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', signal: mockSignal, userMCPAuthMap: {}, @@ -830,7 +786,7 @@ describe('User parameter passing tests', () => { const mockRes = { write: jest.fn(), flush: jest.fn() }; const availableTools = { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Cached tool', parameters: { type: 'object', properties: {} }, @@ -841,7 +797,7 @@ describe('User parameter passing tests', () => { await createMCPTool({ res: mockRes, user: mockUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', userMCPAuthMap: {}, availableTools: availableTools, @@ -864,8 +820,8 @@ describe('User parameter passing tests', () => { return Promise.resolve({ tools: [{ name: 'tool1' }, { name: 'tool2' }], availableTools: { - 'tool1::server1': { function: { description: 'Tool 1', parameters: {} } }, - 'tool2::server1': { function: { description: 'Tool 2', parameters: {} } }, + [`tool1${D}server1`]: { function: { description: 'Tool 1', parameters: {} } }, + [`tool2${D}server1`]: { function: { description: 'Tool 2', parameters: {} } }, }, }); }); @@ -896,7 +852,7 @@ describe('User parameter passing tests', () => { reinitCalls.push(params); return Promise.resolve({ availableTools: { - 'my-tool::my-server': { + [`my-tool${D}my-server`]: { function: { description: 'My Tool', parameters: {} }, }, }, @@ -906,7 +862,7 @@ describe('User parameter passing tests', () => { await createMCPTool({ res: mockRes, user: mockUser, - toolKey: 'my-tool::my-server', + toolKey: `my-tool${D}my-server`, provider: 'google', userMCPAuthMap: {}, availableTools: undefined, // Force reinit @@ -940,11 +896,11 @@ describe('User parameter passing tests', () => { const result = await createMCPTool({ res: mockRes, user: mockUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', userMCPAuthMap: {}, availableTools: { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Test tool', parameters: { type: 'object', properties: {} }, @@ -987,7 +943,7 @@ describe('User parameter passing tests', () => { mockIsMCPDomainAllowed.mockResolvedValueOnce(true); const availableTools = { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Test tool', parameters: { type: 'object', properties: {} }, @@ -998,7 +954,7 @@ describe('User parameter passing tests', () => { const result = await createMCPTool({ res: mockRes, user: mockUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', userMCPAuthMap: {}, availableTools, @@ -1027,7 +983,7 @@ describe('User parameter passing tests', () => { }); const availableTools = { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Test tool', parameters: { type: 'object', properties: {} }, @@ -1038,7 +994,7 @@ describe('User parameter passing tests', () => { const result = await createMCPTool({ res: mockRes, user: mockUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', userMCPAuthMap: {}, availableTools, @@ -1104,7 +1060,7 @@ describe('User parameter passing tests', () => { mockIsMCPDomainAllowed.mockResolvedValue(true); const availableTools = { - 'test-tool::test-server': { + [`test-tool${D}test-server`]: { function: { description: 'Test tool', parameters: { type: 'object', properties: {} }, @@ -1116,7 +1072,7 @@ describe('User parameter passing tests', () => { await createMCPTool({ res: mockRes, user: adminUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', userMCPAuthMap: {}, availableTools, @@ -1130,7 +1086,7 @@ describe('User parameter passing tests', () => { await createMCPTool({ res: mockRes, user: regularUser, - toolKey: 'test-tool::test-server', + toolKey: `test-tool${D}test-server`, provider: 'openai', userMCPAuthMap: {}, availableTools, @@ -1158,7 +1114,7 @@ describe('User parameter passing tests', () => { return Promise.resolve({ tools: [{ name: 'test' }], availableTools: { - 'test::server': { function: { description: 'Test', parameters: {} } }, + [`test${D}server`]: { function: { description: 'Test', parameters: {} } }, }, }); }); diff --git a/packages/api/src/mcp/__tests__/zod.spec.ts b/packages/api/src/mcp/__tests__/zod.spec.ts index 71713389bf..9566ba0def 100644 --- a/packages/api/src/mcp/__tests__/zod.spec.ts +++ b/packages/api/src/mcp/__tests__/zod.spec.ts @@ -2,7 +2,12 @@ // zod.spec.ts import { z } from 'zod'; import type { JsonSchemaType } from '@librechat/data-schemas'; -import { resolveJsonSchemaRefs, convertJsonSchemaToZod, convertWithResolvedRefs } from '../zod'; +import { + convertWithResolvedRefs, + convertJsonSchemaToZod, + resolveJsonSchemaRefs, + normalizeJsonSchema, +} from '../zod'; describe('convertJsonSchemaToZod', () => { describe('integer type handling', () => { @@ -206,7 +211,7 @@ describe('convertJsonSchemaToZod', () => { type: 'number' as const, enum: [1, 2, 3, 5, 8, 13], }; - const zodSchema = convertWithResolvedRefs(schema as JsonSchemaType); + const zodSchema = convertWithResolvedRefs(schema as unknown as JsonSchemaType); expect(zodSchema?.parse(1)).toBe(1); expect(zodSchema?.parse(13)).toBe(13); @@ -2002,3 +2007,189 @@ describe('convertJsonSchemaToZod', () => { }); }); }); + +describe('normalizeJsonSchema', () => { + it('should convert const to enum', () => { + const schema = { type: 'string', const: 'hello' } as any; + const result = normalizeJsonSchema(schema); + expect(result).toEqual({ type: 'string', enum: ['hello'] }); + expect(result).not.toHaveProperty('const'); + }); + + it('should preserve existing enum when const is also present', () => { + const schema = { type: 'string', const: 'hello', enum: ['hello', 'world'] } as any; + const result = normalizeJsonSchema(schema); + expect(result).toEqual({ type: 'string', enum: ['hello', 'world'] }); + expect(result).not.toHaveProperty('const'); + }); + + it('should handle non-string const values (number, boolean, null)', () => { + expect(normalizeJsonSchema({ type: 'number', const: 42 } as any)).toEqual({ + type: 'number', + enum: [42], + }); + expect(normalizeJsonSchema({ type: 'boolean', const: true } as any)).toEqual({ + type: 'boolean', + enum: [true], + }); + expect(normalizeJsonSchema({ type: 'string', const: null } as any)).toEqual({ + type: 'string', + enum: [null], + }); + }); + + it('should recursively normalize nested object properties', () => { + const schema = { + type: 'object', + properties: { + mode: { type: 'string', const: 'advanced' }, + count: { type: 'number', const: 5 }, + name: { type: 'string', description: 'A name' }, + }, + } as any; + + const result = normalizeJsonSchema(schema); + expect(result.properties.mode).toEqual({ type: 'string', enum: ['advanced'] }); + expect(result.properties.count).toEqual({ type: 'number', enum: [5] }); + expect(result.properties.name).toEqual({ type: 'string', description: 'A name' }); + }); + + it('should normalize inside oneOf/anyOf/allOf arrays', () => { + const schema = { + type: 'object', + oneOf: [ + { type: 'object', properties: { kind: { type: 'string', const: 'A' } } }, + { type: 'object', properties: { kind: { type: 'string', const: 'B' } } }, + ], + anyOf: [{ type: 'string', const: 'x' }], + allOf: [{ type: 'number', const: 1 }], + } as any; + + const result = normalizeJsonSchema(schema); + expect(result.oneOf[0].properties.kind).toEqual({ type: 'string', enum: ['A'] }); + expect(result.oneOf[1].properties.kind).toEqual({ type: 'string', enum: ['B'] }); + expect(result.anyOf[0]).toEqual({ type: 'string', enum: ['x'] }); + expect(result.allOf[0]).toEqual({ type: 'number', enum: [1] }); + }); + + it('should normalize array items with const', () => { + const schema = { + type: 'array', + items: { type: 'string', const: 'fixed' }, + } as any; + + const result = normalizeJsonSchema(schema); + expect(result.items).toEqual({ type: 'string', enum: ['fixed'] }); + }); + + it('should normalize additionalProperties with const', () => { + const schema = { + type: 'object', + additionalProperties: { type: 'string', const: 'val' }, + } as any; + + const result = normalizeJsonSchema(schema); + expect(result.additionalProperties).toEqual({ type: 'string', enum: ['val'] }); + }); + + it('should handle null, undefined, and primitive inputs safely', () => { + expect(normalizeJsonSchema(null as any)).toBeNull(); + expect(normalizeJsonSchema(undefined as any)).toBeUndefined(); + expect(normalizeJsonSchema('string' as any)).toBe('string'); + expect(normalizeJsonSchema(42 as any)).toBe(42); + expect(normalizeJsonSchema(true as any)).toBe(true); + }); + + it('should be a no-op when no const is present', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string', description: 'Name' }, + age: { type: 'number' }, + tags: { type: 'array', items: { type: 'string' } }, + }, + required: ['name'], + } as any; + + const result = normalizeJsonSchema(schema); + expect(result).toEqual(schema); + }); + + it('should handle a Tavily-like schema pattern with const', () => { + const schema = { + type: 'object', + properties: { + query: { + type: 'string', + description: 'The search query', + }, + search_depth: { + type: 'string', + const: 'advanced', + description: 'The depth of the search', + }, + topic: { + type: 'string', + enum: ['general', 'news'], + description: 'The search topic', + }, + include_answer: { + type: 'boolean', + const: true, + }, + max_results: { + type: 'number', + const: 5, + }, + }, + required: ['query'], + } as any; + + const result = normalizeJsonSchema(schema); + + // const fields should be converted to enum + expect(result.properties.search_depth).toEqual({ + type: 'string', + enum: ['advanced'], + description: 'The depth of the search', + }); + expect(result.properties.include_answer).toEqual({ + type: 'boolean', + enum: [true], + }); + expect(result.properties.max_results).toEqual({ + type: 'number', + enum: [5], + }); + + // Existing enum should be preserved + expect(result.properties.topic).toEqual({ + type: 'string', + enum: ['general', 'news'], + description: 'The search topic', + }); + + // Non-const fields should be unchanged + expect(result.properties.query).toEqual({ + type: 'string', + description: 'The search query', + }); + + // Top-level fields preserved + expect(result.required).toEqual(['query']); + expect(result.type).toBe('object'); + }); + + it('should handle arrays at the top level', () => { + const schemas = [ + { type: 'string', const: 'a' }, + { type: 'number', const: 1 }, + ] as any; + + const result = normalizeJsonSchema(schemas); + expect(result).toEqual([ + { type: 'string', enum: ['a'] }, + { type: 'number', enum: [1] }, + ]); + }); +}); diff --git a/packages/api/src/mcp/zod.ts b/packages/api/src/mcp/zod.ts index a218392755..4f6e955ce8 100644 --- a/packages/api/src/mcp/zod.ts +++ b/packages/api/src/mcp/zod.ts @@ -248,6 +248,65 @@ export function resolveJsonSchemaRefs>( return result as T; } +/** + * Recursively normalizes a JSON schema by converting `const` values to `enum` arrays. + * Gemini/Vertex AI does not support the `const` keyword in function declarations, + * but `const: X` is semantically equivalent to `enum: [X]` per the JSON Schema spec. + * + * @param schema - The JSON schema to normalize + * @returns The normalized schema with `const` converted to `enum` + */ +export function normalizeJsonSchema>(schema: T): T { + if (!schema || typeof schema !== 'object') { + return schema; + } + + if (Array.isArray(schema)) { + return schema.map((item) => + item && typeof item === 'object' ? normalizeJsonSchema(item) : item, + ) as unknown as T; + } + + const result: Record = {}; + + for (const [key, value] of Object.entries(schema)) { + if (key === 'const' && !('enum' in schema)) { + result['enum'] = [value]; + continue; + } + + if (key === 'const' && 'enum' in schema) { + // Skip `const` when `enum` already exists + continue; + } + + if (key === 'properties' && value && typeof value === 'object' && !Array.isArray(value)) { + const newProps: Record = {}; + for (const [propKey, propValue] of Object.entries(value as Record)) { + newProps[propKey] = + propValue && typeof propValue === 'object' + ? normalizeJsonSchema(propValue as Record) + : propValue; + } + result[key] = newProps; + } else if ( + (key === 'items' || key === 'additionalProperties') && + value && + typeof value === 'object' + ) { + result[key] = normalizeJsonSchema(value as Record); + } else if ((key === 'oneOf' || key === 'anyOf' || key === 'allOf') && Array.isArray(value)) { + result[key] = value.map((item) => + item && typeof item === 'object' ? normalizeJsonSchema(item) : item, + ); + } else { + result[key] = value; + } + } + + return result as T; +} + /** * Converts a JSON Schema to a Zod schema. * diff --git a/packages/api/src/tools/definitions.ts b/packages/api/src/tools/definitions.ts index 97312883f0..a5b35ac7d8 100644 --- a/packages/api/src/tools/definitions.ts +++ b/packages/api/src/tools/definitions.ts @@ -8,9 +8,10 @@ import { Constants, actionDelimiter } from 'librechat-data-provider'; import type { AgentToolOptions } from 'librechat-data-provider'; import type { LCToolRegistry, JsonSchemaType, LCTool, GenericTool } from '@librechat/agents'; -import { buildToolClassification, type ToolDefinition } from './classification'; +import type { ToolDefinition } from './classification'; +import { resolveJsonSchemaRefs, normalizeJsonSchema } from '~/mcp/zod'; +import { buildToolClassification } from './classification'; import { getToolDefinition } from './registry/definitions'; -import { resolveJsonSchemaRefs } from '~/mcp/zod'; export interface MCPServerTool { function?: { @@ -138,7 +139,7 @@ export async function loadToolDefinitions( name: actualToolName, description: toolDef.function.description, parameters: toolDef.function.parameters - ? resolveJsonSchemaRefs(toolDef.function.parameters) + ? normalizeJsonSchema(resolveJsonSchemaRefs(toolDef.function.parameters)) : undefined, serverName, }); @@ -153,7 +154,7 @@ export async function loadToolDefinitions( name: toolName, description: toolDef.function.description, parameters: toolDef.function.parameters - ? resolveJsonSchemaRefs(toolDef.function.parameters) + ? normalizeJsonSchema(resolveJsonSchemaRefs(toolDef.function.parameters)) : undefined, serverName, });