mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-02 04:12:36 +00:00
* 🗄️ fix: Gate Request-Scoped MCP Servers Out of Persistent Tool Cache PR #13626 established that request-scoped MCP servers (runtime OPENID/GRAPH/BODY placeholders) must not use the persistent 12h tool cache, but only gated three of five touchpoints. The panel endpoint still back-filled the cache and the OAuth callback still wrote to it, while agent loading read those entries ungated — pinning ephemeral model-spec/agent toolsets to stale definitions for up to 12h. Centralize the invariant in createMCPToolCacheService: a getServerConfig resolver dep gates both writers and a new service-owned getMCPServerTools read, so every current and future caller is covered. Callers that already hold the parsed config pass it to skip resolution; the per-call skipCache flag and duplicated call-site gates are removed in favor of the single config-based mechanism. Resolution failures fail open to preserve prior behavior. * 🩹 fix: Address Codex Review on Cache Gating - Repair getCachedTools.spec.js, which destructured the relocated getMCPServerTools directly from the module; its coverage now lives in the service-level tools.spec.ts. - Resolve the merged (Config-tier-aware) server config in the OAuth callback before writing tool definitions, so the cache gate detects request-scoped servers supplied via admin Config overlays that the base registry lookup cannot see. - Discover tools actively for request-scoped servers in the panel endpoint via ephemeral reinitialization: such servers have no stored app/user connections, so the previous getServerToolFunctions fallback returned an empty toolset once the cache read was gated. * 🧵 fix: Address Second Codex Review on Cache Gating - Resolve the merged server config before the OAuth callback reconnects, so the connection itself uses Config-tier overlays rather than only the subsequent cache write. - Pass Config-tier candidates into the panel's request-scoped discovery, matching the reinitialize route: reinitMCPServer forwards configServers (not the provided serverConfig) to its OAuth discovery fallback. - Document the accepted read-path trade-off: the gate resolver sees base configs only, all writers pass merged configs, so a pre-gating or overlay-divergent entry survives at most one cache TTL. * 🚏 chore: Rework Cache Gating for BODY-Only Request Scoping After #13673 narrowed requiresEphemeralUserConnection to BODY placeholders, the central gate follows the predicate unchanged, but the panel's active discovery no longer serves a purpose: the only remaining request-scoped class cannot connect outside a chat turn, so the reinitialization attempt would always fail at the missing-body check. Remove that path; OpenID/Graph servers are persistent user-scoped again and flow through the stored-connection and cache lookups as before. Flip test fixtures that used OPENID placeholders to denote request-scoped configs over to BODY placeholders. * 🪟 fix: Check Config Overlays in Agent-Loading Cache Reads The cache service's registry resolver sees only base YAML/DB configs, so a BODY placeholder introduced by a request-tier Config overlay was invisible to the gate on the agent-loading read path: model-spec and ephemeral-agent expansion could read a leftover persistent entry and pin stale concrete tool names instead of the mcp_all fresh-discovery path. Check the raw overlay candidate inline in loadEphemeralAgent and loadAddedAgent — a pure placeholder scan with no extra IO — and skip the cache read when the overlay makes the server request-scoped. Widen UserScopedConnectionConfig so raw (pre-inspection) configs qualify for the scoping predicates, which only check key presence. * 🧪 test: Guard Run-Scoped MCP Definition Handoff Boundaries The original ClickHouse breaker storm regressed precisely at field pass-through boundaries that unit tests of each end could not see: initializeAgent dropping mcpAvailableTools from its destructure, and the agent tool context losing it on the way into ON_TOOL_EXECUTE. Add direct guards on both hops: the loadTools result must surface on the initialized agent, and the captured toolExecuteOptions closure must forward it to loadToolsForExecution.
454 lines
15 KiB
JavaScript
454 lines
15 KiB
JavaScript
const mongoose = require('mongoose');
|
|
const { MongoMemoryServer } = require('mongodb-memory-server');
|
|
|
|
const mockPluginService = {
|
|
updateUserPluginAuth: jest.fn(),
|
|
deleteUserPluginAuth: jest.fn(),
|
|
getUserPluginAuthValue: jest.fn(),
|
|
};
|
|
const mockGetMCPServerTools = jest.fn();
|
|
const mockCreateMCPTool = jest.fn();
|
|
const mockCreateMCPTools = jest.fn();
|
|
const mockGetServerConfig = jest.fn();
|
|
|
|
jest.mock('~/server/services/PluginService', () => mockPluginService);
|
|
|
|
jest.mock('~/server/services/Config', () => ({
|
|
getAppConfig: jest.fn().mockResolvedValue({
|
|
// Default app config for tool tests
|
|
paths: { uploads: '/tmp' },
|
|
fileStrategy: 'local',
|
|
filteredTools: [],
|
|
includedTools: [],
|
|
}),
|
|
getCachedTools: jest.fn().mockResolvedValue({
|
|
// Default cached tools for tests
|
|
dalle: {
|
|
type: 'function',
|
|
function: {
|
|
name: 'dalle',
|
|
description: 'DALL-E image generation',
|
|
parameters: {},
|
|
},
|
|
},
|
|
}),
|
|
getMCPServerTools: (...args) => mockGetMCPServerTools(...args),
|
|
}));
|
|
|
|
jest.mock('~/server/services/MCP', () => ({
|
|
createMCPTool: (...args) => mockCreateMCPTool(...args),
|
|
createMCPTools: (...args) => mockCreateMCPTools(...args),
|
|
createMCPPermissionContext: jest.fn(() => ({
|
|
canUseServers: jest.fn().mockResolvedValue(true),
|
|
})),
|
|
resolveConfigServers: jest.fn().mockResolvedValue({}),
|
|
}));
|
|
|
|
jest.mock('~/config', () => ({
|
|
getMCPServersRegistry: jest.fn(() => ({
|
|
getServerConfig: (...args) => mockGetServerConfig(...args),
|
|
})),
|
|
}));
|
|
|
|
const { Calculator } = require('@librechat/agents');
|
|
const { Constants } = require('librechat-data-provider');
|
|
|
|
const { User } = require('~/db/models');
|
|
const PluginService = require('~/server/services/PluginService');
|
|
const { validateTools, loadTools, loadToolWithAuth } = require('./handleTools');
|
|
const { StructuredSD, availableTools, DALLE3 } = require('../');
|
|
|
|
describe('Tool Handlers', () => {
|
|
let mongoServer;
|
|
let fakeUser;
|
|
const pluginKey = 'dalle';
|
|
const pluginKey2 = 'wolfram';
|
|
const ToolClass = DALLE3;
|
|
const initialTools = [pluginKey, pluginKey2];
|
|
const mockCredential = 'mock-credential';
|
|
const mainPlugin = availableTools.find((tool) => tool.pluginKey === pluginKey);
|
|
const authConfigs = mainPlugin.authConfig;
|
|
|
|
beforeAll(async () => {
|
|
mongoServer = await MongoMemoryServer.create();
|
|
const mongoUri = mongoServer.getUri();
|
|
await mongoose.connect(mongoUri);
|
|
|
|
const userAuthValues = {};
|
|
mockPluginService.getUserPluginAuthValue.mockImplementation((userId, authField) => {
|
|
return userAuthValues[`${userId}-${authField}`];
|
|
});
|
|
mockPluginService.updateUserPluginAuth.mockImplementation(
|
|
(userId, authField, _pluginKey, credential) => {
|
|
const fields = authField.split('||');
|
|
fields.forEach((field) => {
|
|
userAuthValues[`${userId}-${field}`] = credential;
|
|
});
|
|
},
|
|
);
|
|
|
|
fakeUser = new User({
|
|
name: 'Fake User',
|
|
username: 'fakeuser',
|
|
email: 'fakeuser@example.com',
|
|
emailVerified: false,
|
|
// file deepcode ignore NoHardcodedPasswords/test: fake value
|
|
password: 'fakepassword123',
|
|
avatar: '',
|
|
provider: 'local',
|
|
role: 'USER',
|
|
googleId: null,
|
|
plugins: [],
|
|
refreshToken: [],
|
|
});
|
|
await fakeUser.save();
|
|
for (const authConfig of authConfigs) {
|
|
await PluginService.updateUserPluginAuth(
|
|
fakeUser._id,
|
|
authConfig.authField,
|
|
pluginKey,
|
|
mockCredential,
|
|
);
|
|
}
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await mongoose.disconnect();
|
|
await mongoServer.stop();
|
|
});
|
|
|
|
beforeEach(async () => {
|
|
// Clear mocks but not the database since we need the user to persist
|
|
jest.clearAllMocks();
|
|
|
|
// Reset the mock implementations
|
|
const userAuthValues = {};
|
|
mockPluginService.getUserPluginAuthValue.mockImplementation((userId, authField) => {
|
|
return userAuthValues[`${userId}-${authField}`];
|
|
});
|
|
mockPluginService.updateUserPluginAuth.mockImplementation(
|
|
(userId, authField, _pluginKey, credential) => {
|
|
const fields = authField.split('||');
|
|
fields.forEach((field) => {
|
|
userAuthValues[`${userId}-${field}`] = credential;
|
|
});
|
|
},
|
|
);
|
|
|
|
// Re-add the auth configs for the user
|
|
for (const authConfig of authConfigs) {
|
|
await PluginService.updateUserPluginAuth(
|
|
fakeUser._id,
|
|
authConfig.authField,
|
|
pluginKey,
|
|
mockCredential,
|
|
);
|
|
}
|
|
});
|
|
|
|
describe('validateTools', () => {
|
|
it('returns valid tools given input tools and user authentication', async () => {
|
|
const validTools = await validateTools(fakeUser._id, initialTools);
|
|
expect(validTools).toBeDefined();
|
|
expect(validTools.some((tool) => tool === pluginKey)).toBeTruthy();
|
|
expect(validTools.length).toBeGreaterThan(0);
|
|
});
|
|
|
|
it('removes tools without valid credentials from the validTools array', async () => {
|
|
const validTools = await validateTools(fakeUser._id, initialTools);
|
|
expect(validTools.some((tool) => tool.pluginKey === pluginKey2)).toBeFalsy();
|
|
});
|
|
|
|
it('returns an empty array when no authenticated tools are provided', async () => {
|
|
const validTools = await validateTools(fakeUser._id, []);
|
|
expect(validTools).toEqual([]);
|
|
});
|
|
|
|
it('should validate a tool from an Environment Variable', async () => {
|
|
const plugin = availableTools.find((tool) => tool.pluginKey === pluginKey2);
|
|
const authConfigs = plugin.authConfig;
|
|
for (const authConfig of authConfigs) {
|
|
process.env[authConfig.authField] = mockCredential;
|
|
}
|
|
const validTools = await validateTools(fakeUser._id, [pluginKey2]);
|
|
expect(validTools.length).toEqual(1);
|
|
for (const authConfig of authConfigs) {
|
|
delete process.env[authConfig.authField];
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('loadTools', () => {
|
|
let toolFunctions;
|
|
let loadTool1;
|
|
let loadTool2;
|
|
let loadTool3;
|
|
const sampleTools = [...initialTools, 'calculator'];
|
|
let ToolClass2 = Calculator;
|
|
let remainingTools = availableTools.filter(
|
|
(tool) => sampleTools.indexOf(tool.pluginKey) === -1,
|
|
);
|
|
|
|
beforeAll(async () => {
|
|
const toolMap = await loadTools({
|
|
user: fakeUser._id,
|
|
tools: sampleTools,
|
|
returnMap: true,
|
|
useSpecs: true,
|
|
});
|
|
toolFunctions = toolMap;
|
|
loadTool1 = toolFunctions[sampleTools[0]];
|
|
loadTool2 = toolFunctions[sampleTools[1]];
|
|
loadTool3 = toolFunctions[sampleTools[2]];
|
|
});
|
|
|
|
let originalEnv;
|
|
|
|
beforeEach(() => {
|
|
originalEnv = process.env;
|
|
process.env = { ...originalEnv };
|
|
});
|
|
|
|
afterEach(() => {
|
|
process.env = originalEnv;
|
|
});
|
|
|
|
it('returns the expected load functions for requested tools', async () => {
|
|
expect(loadTool1).toBeDefined();
|
|
expect(loadTool2).toBeDefined();
|
|
expect(loadTool3).toBeDefined();
|
|
|
|
for (const tool of remainingTools) {
|
|
expect(toolFunctions[tool.pluginKey]).toBeUndefined();
|
|
}
|
|
});
|
|
|
|
it('should initialize an authenticated tool or one without authentication', async () => {
|
|
const authTool = await loadTool1();
|
|
const tool = await loadTool3();
|
|
expect(authTool).toBeInstanceOf(ToolClass);
|
|
expect(tool).toBeInstanceOf(ToolClass2);
|
|
});
|
|
|
|
it('should initialize an authenticated tool with primary auth field', async () => {
|
|
process.env.DALLE3_API_KEY = 'mocked_api_key';
|
|
const initToolFunction = loadToolWithAuth(
|
|
'userId',
|
|
['DALLE3_API_KEY||DALLE_API_KEY'],
|
|
ToolClass,
|
|
);
|
|
const authTool = await initToolFunction();
|
|
|
|
expect(authTool).toBeInstanceOf(ToolClass);
|
|
expect(mockPluginService.getUserPluginAuthValue).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('should initialize an authenticated tool with alternate auth field when primary is missing', async () => {
|
|
delete process.env.DALLE3_API_KEY; // Ensure the primary key is not set
|
|
process.env.DALLE_API_KEY = 'mocked_alternate_api_key';
|
|
const initToolFunction = loadToolWithAuth(
|
|
'userId',
|
|
['DALLE3_API_KEY||DALLE_API_KEY'],
|
|
ToolClass,
|
|
);
|
|
const authTool = await initToolFunction();
|
|
|
|
expect(authTool).toBeInstanceOf(ToolClass);
|
|
expect(mockPluginService.getUserPluginAuthValue).toHaveBeenCalledTimes(1);
|
|
expect(mockPluginService.getUserPluginAuthValue).toHaveBeenCalledWith(
|
|
'userId',
|
|
'DALLE3_API_KEY',
|
|
true,
|
|
);
|
|
});
|
|
|
|
it('should fallback to getUserPluginAuthValue when env vars are missing', async () => {
|
|
mockPluginService.updateUserPluginAuth('userId', 'DALLE_API_KEY', 'dalle', 'mocked_api_key');
|
|
const initToolFunction = loadToolWithAuth(
|
|
'userId',
|
|
['DALLE3_API_KEY||DALLE_API_KEY'],
|
|
ToolClass,
|
|
);
|
|
const authTool = await initToolFunction();
|
|
|
|
expect(authTool).toBeInstanceOf(ToolClass);
|
|
expect(mockPluginService.getUserPluginAuthValue).toHaveBeenCalledTimes(2);
|
|
});
|
|
|
|
it('should throw an error for an unauthenticated tool', async () => {
|
|
try {
|
|
await loadTool2();
|
|
} catch (error) {
|
|
expect(error).toBeDefined();
|
|
}
|
|
});
|
|
it('returns an empty object when no tools are requested', async () => {
|
|
toolFunctions = await loadTools({
|
|
user: fakeUser._id,
|
|
returnMap: true,
|
|
useSpecs: true,
|
|
});
|
|
expect(toolFunctions).toEqual({});
|
|
});
|
|
it('should return the StructuredTool version when using functions', async () => {
|
|
process.env.SD_WEBUI_URL = mockCredential;
|
|
toolFunctions = await loadTools({
|
|
user: fakeUser._id,
|
|
tools: ['stable-diffusion'],
|
|
functions: true,
|
|
returnMap: true,
|
|
useSpecs: true,
|
|
});
|
|
const structuredTool = await toolFunctions['stable-diffusion']();
|
|
expect(structuredTool).toBeInstanceOf(StructuredSD);
|
|
delete process.env.SD_WEBUI_URL;
|
|
});
|
|
|
|
it('passes request body to chat MCP tool creation and skips stale cache for BODY-scoped servers', async () => {
|
|
const serverName = 'body-scoped';
|
|
const toolKey = `search${Constants.mcp_delimiter}${serverName}`;
|
|
const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' };
|
|
const serverConfig = {
|
|
type: 'streamable-http',
|
|
url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
|
source: 'yaml',
|
|
};
|
|
|
|
mockGetServerConfig.mockResolvedValue(serverConfig);
|
|
mockCreateMCPTool.mockResolvedValue({ name: 'loaded-mcp-tool' });
|
|
|
|
const result = await loadTools({
|
|
user: fakeUser._id.toString(),
|
|
tools: [toolKey],
|
|
options: {
|
|
req: {
|
|
user: { id: fakeUser._id.toString(), role: 'USER' },
|
|
body: requestBody,
|
|
},
|
|
},
|
|
});
|
|
|
|
expect(result.loadedTools).toEqual([{ name: 'loaded-mcp-tool' }]);
|
|
expect(mockGetMCPServerTools).toHaveBeenCalledWith(
|
|
fakeUser._id.toString(),
|
|
serverName,
|
|
serverConfig,
|
|
);
|
|
expect(mockCreateMCPTool).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
requestBody,
|
|
toolKey,
|
|
config: serverConfig,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('uses run-scoped MCP tool definitions before cache lookup', async () => {
|
|
const serverName = 'body-scoped';
|
|
const toolKey = `search${Constants.mcp_delimiter}${serverName}`;
|
|
const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' };
|
|
const serverConfig = {
|
|
type: 'streamable-http',
|
|
url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
|
source: 'yaml',
|
|
};
|
|
const runScopedTools = {
|
|
[toolKey]: {
|
|
function: {
|
|
name: toolKey,
|
|
description: 'Run-scoped search',
|
|
parameters: { type: 'object', properties: {} },
|
|
},
|
|
},
|
|
};
|
|
|
|
mockGetServerConfig.mockResolvedValue(serverConfig);
|
|
mockCreateMCPTool.mockResolvedValue({ name: 'loaded-mcp-tool' });
|
|
|
|
const result = await loadTools({
|
|
user: fakeUser._id.toString(),
|
|
tools: [toolKey],
|
|
options: {
|
|
mcpAvailableTools: {
|
|
[serverName]: runScopedTools,
|
|
},
|
|
req: {
|
|
user: { id: fakeUser._id.toString(), role: 'USER' },
|
|
body: requestBody,
|
|
},
|
|
},
|
|
});
|
|
|
|
expect(result.loadedTools).toEqual([{ name: 'loaded-mcp-tool' }]);
|
|
expect(mockGetMCPServerTools).not.toHaveBeenCalled();
|
|
expect(mockCreateMCPTool).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
availableTools: runScopedTools,
|
|
requestBody,
|
|
toolKey,
|
|
config: serverConfig,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('reuses discovered request-scoped MCP tool definitions within a server loop', async () => {
|
|
const serverName = 'body-scoped';
|
|
const firstToolKey = `search${Constants.mcp_delimiter}${serverName}`;
|
|
const secondToolKey = `lookup${Constants.mcp_delimiter}${serverName}`;
|
|
const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' };
|
|
const serverConfig = {
|
|
type: 'streamable-http',
|
|
url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
|
source: 'yaml',
|
|
};
|
|
const discoveredTools = {
|
|
[firstToolKey]: {
|
|
function: {
|
|
description: 'Search',
|
|
parameters: { type: 'object', properties: {} },
|
|
},
|
|
},
|
|
[secondToolKey]: {
|
|
function: {
|
|
description: 'Lookup',
|
|
parameters: { type: 'object', properties: {} },
|
|
},
|
|
},
|
|
};
|
|
|
|
mockGetServerConfig.mockResolvedValue(serverConfig);
|
|
mockCreateMCPTool
|
|
.mockImplementationOnce(async ({ onAvailableTools }) => {
|
|
onAvailableTools(discoveredTools);
|
|
return { name: 'search-tool' };
|
|
})
|
|
.mockImplementationOnce(async ({ availableTools }) => {
|
|
expect(availableTools).toBe(discoveredTools);
|
|
return { name: 'lookup-tool' };
|
|
});
|
|
|
|
const result = await loadTools({
|
|
user: fakeUser._id.toString(),
|
|
tools: [firstToolKey, secondToolKey],
|
|
options: {
|
|
req: {
|
|
user: { id: fakeUser._id.toString(), role: 'USER' },
|
|
body: requestBody,
|
|
},
|
|
},
|
|
});
|
|
|
|
expect(result.loadedTools).toEqual([{ name: 'search-tool' }, { name: 'lookup-tool' }]);
|
|
expect(mockGetMCPServerTools).toHaveBeenCalledTimes(1);
|
|
expect(mockCreateMCPTool).toHaveBeenCalledTimes(2);
|
|
expect(mockCreateMCPTool).toHaveBeenNthCalledWith(
|
|
2,
|
|
expect.objectContaining({
|
|
availableTools: discoveredTools,
|
|
requestBody,
|
|
toolKey: secondToolKey,
|
|
}),
|
|
);
|
|
});
|
|
});
|
|
});
|