mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-02-14 14:38:11 +01:00
🧰 fix: Convert const to enum in MCP Schemas for Gemini Compatibility (#11784)
* 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
This commit is contained in:
parent
276ac8d011
commit
ccbf9dc093
6 changed files with 287 additions and 79 deletions
|
|
@ -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',
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 = {
|
||||
|
|
|
|||
|
|
@ -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: {} } },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue