ci: Remove MCP tests from PluginController test file and add comprehensive tests for MCP tools controller

This commit is contained in:
Danny Avila 2025-09-21 07:33:56 -04:00
parent 9db37d44aa
commit 15012d7a60
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
2 changed files with 579 additions and 118 deletions

View file

@ -1,4 +1,3 @@
const { Constants } = require('librechat-data-provider');
const { getCachedTools, getAppConfig } = require('~/server/services/Config');
const { getLogStores } = require('~/cache');
@ -17,18 +16,10 @@ jest.mock('~/server/services/Config', () => ({
includedTools: [],
}),
setCachedTools: jest.fn(),
mergeUserTools: jest.fn(),
}));
// loadAndFormatTools mock removed - no longer used in PluginController
jest.mock('~/config', () => ({
getMCPManager: jest.fn(() => ({
getAllToolFunctions: jest.fn().mockResolvedValue({}),
getRawConfig: jest.fn().mockReturnValue({}),
})),
getFlowStateManager: jest.fn(),
}));
// getMCPManager mock removed - no longer used in PluginController
jest.mock('~/app/clients/tools', () => ({
availableTools: [],
@ -183,9 +174,6 @@ describe('PluginController', () => {
paths: { structuredTools: '/mock/path' },
};
// Mock second call to return tool definitions
getCachedTools.mockResolvedValueOnce(mockUserTools);
await getAvailableTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
@ -208,14 +196,7 @@ describe('PluginController', () => {
require('~/app/clients/tools').availableTools.push(mockPlugin);
mockCache.get.mockResolvedValue(null);
// First call returns null for user tools
getCachedTools.mockResolvedValueOnce(null);
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
// Second call (with includeGlobal: true) returns the tool definitions
// getCachedTools returns the tool definitions
getCachedTools.mockResolvedValueOnce({
tool1: {
type: 'function',
@ -226,6 +207,10 @@ describe('PluginController', () => {
},
},
});
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
await getAvailableTools(mockReq, mockRes);
@ -256,14 +241,7 @@ describe('PluginController', () => {
});
mockCache.get.mockResolvedValue(null);
// First call returns null for user tools
getCachedTools.mockResolvedValueOnce(null);
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
// Second call (with includeGlobal: true) returns the tool definitions
// getCachedTools returns the tool definitions
getCachedTools.mockResolvedValueOnce({
toolkit1_function: {
type: 'function',
@ -274,6 +252,10 @@ describe('PluginController', () => {
},
},
});
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
await getAvailableTools(mockReq, mockRes);
@ -285,63 +267,6 @@ describe('PluginController', () => {
});
});
describe('plugin.icon behavior', () => {
const callGetAvailableToolsWithMCPServer = async (serverConfig) => {
mockCache.get.mockResolvedValue(null);
const functionTools = {
[`test-tool${Constants.mcp_delimiter}test-server`]: {
type: 'function',
function: {
name: `test-tool${Constants.mcp_delimiter}test-server`,
description: 'A test tool',
parameters: { type: 'object', properties: {} },
},
},
};
// Mock the MCP manager to return tools and server config
const mockMCPManager = {
getAllToolFunctions: jest.fn().mockResolvedValue(functionTools),
getRawConfig: jest.fn().mockReturnValue(serverConfig),
};
require('~/config').getMCPManager.mockReturnValue(mockMCPManager);
// First call returns empty user tools
getCachedTools.mockResolvedValueOnce({});
// Mock getAppConfig to return the mcpConfig
mockReq.config = {
mcpConfig: {
'test-server': serverConfig,
},
};
// Second call (with includeGlobal: true) returns the tool definitions
getCachedTools.mockResolvedValueOnce(functionTools);
await getAvailableTools(mockReq, mockRes);
const responseData = mockRes.json.mock.calls[0][0];
return responseData.find(
(tool) => tool.pluginKey === `test-tool${Constants.mcp_delimiter}test-server`,
);
};
it('should set plugin.icon when iconPath is defined', async () => {
const serverConfig = {
iconPath: '/path/to/icon.png',
};
const testTool = await callGetAvailableToolsWithMCPServer(serverConfig);
expect(testTool.icon).toBe('/path/to/icon.png');
});
it('should set plugin.icon to undefined when iconPath is not defined', async () => {
const serverConfig = {};
const testTool = await callGetAvailableToolsWithMCPServer(serverConfig);
expect(testTool.icon).toBeUndefined();
});
});
describe('helper function integration', () => {
it('should handle error cases gracefully', async () => {
mockCache.get.mockRejectedValue(new Error('Cache error'));
@ -364,23 +289,13 @@ describe('PluginController', () => {
it('should handle null cachedTools and cachedUserTools', async () => {
mockCache.get.mockResolvedValue(null);
// First call returns null for user tools
getCachedTools.mockResolvedValueOnce(null);
// getCachedTools returns empty object instead of null
getCachedTools.mockResolvedValueOnce({});
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
// Mock MCP manager to return no tools
const mockMCPManager = {
getAllToolFunctions: jest.fn().mockResolvedValue({}),
getRawConfig: jest.fn().mockReturnValue({}),
};
require('~/config').getMCPManager.mockReturnValue(mockMCPManager);
// Second call (with includeGlobal: true) returns empty object instead of null
getCachedTools.mockResolvedValueOnce({});
await getAvailableTools(mockReq, mockRes);
// Should handle null values gracefully
@ -395,9 +310,9 @@ describe('PluginController', () => {
paths: { structuredTools: '/mock/path' },
};
// Mock getCachedTools to return undefined for both calls
// Mock getCachedTools to return undefined
getCachedTools.mockReset();
getCachedTools.mockResolvedValueOnce(undefined).mockResolvedValueOnce(undefined);
getCachedTools.mockResolvedValueOnce(undefined);
await getAvailableTools(mockReq, mockRes);
@ -416,13 +331,6 @@ describe('PluginController', () => {
// Ensure no plugins are available
require('~/app/clients/tools').availableTools.length = 0;
// Reset MCP manager to default state
const mockMCPManager = {
getAllToolFunctions: jest.fn().mockResolvedValue({}),
getRawConfig: jest.fn().mockReturnValue({}),
};
require('~/config').getMCPManager.mockReturnValue(mockMCPManager);
await getAvailableTools(mockReq, mockRes);
// With empty tool definitions, no tools should be in the final output
@ -457,16 +365,13 @@ describe('PluginController', () => {
require('~/app/clients/tools').availableTools.push(mockToolkit);
mockCache.get.mockResolvedValue(null);
// First call returns empty object
// getCachedTools returns empty object to avoid null reference error
getCachedTools.mockResolvedValueOnce({});
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
// Second call (with includeGlobal: true) returns empty object to avoid null reference error
getCachedTools.mockResolvedValueOnce({});
await getAvailableTools(mockReq, mockRes);
// Should handle null toolDefinitions gracefully
@ -487,15 +392,12 @@ describe('PluginController', () => {
mockCache.get.mockResolvedValue(null);
// First call returns null for user tools
getCachedTools.mockResolvedValueOnce(null);
mockReq.config = {
mcpConfig: null,
paths: { structuredTools: '/mock/path' },
};
// CRITICAL: Second call (with includeGlobal: true) returns undefined
// CRITICAL: getCachedTools returns undefined
// This is what causes the bug when trying to access toolDefinitions[plugin.pluginKey]
getCachedTools.mockResolvedValueOnce(undefined);
@ -534,9 +436,8 @@ describe('PluginController', () => {
{ name: 'Tool 2', pluginKey: 'tool2', description: 'Tool 2' },
);
// First call: Simulate cache cleared state (returns null for both global and user tools)
// Simulate cache cleared state (returns null)
mockCache.get.mockResolvedValue(null);
getCachedTools.mockResolvedValueOnce(null); // User tools
getCachedTools.mockResolvedValueOnce(null); // Global tools (cache cleared)
mockReq.config = {
@ -574,7 +475,6 @@ describe('PluginController', () => {
// Cache returns null (cleared state)
mockCache.get.mockResolvedValue(null);
getCachedTools.mockResolvedValueOnce(null); // User tools
getCachedTools.mockResolvedValueOnce(null); // Global tools (cache cleared)
mockReq.config = {

View file

@ -0,0 +1,561 @@
const { getMCPTools } = require('./mcp');
const { getAppConfig, getMCPServerTools } = require('~/server/services/Config');
const { getMCPManager } = require('~/config');
const { convertMCPToolToPlugin } = require('@librechat/api');
jest.mock('@librechat/data-schemas', () => ({
logger: {
debug: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
},
}));
jest.mock('librechat-data-provider', () => ({
Constants: {
mcp_delimiter: '~~~',
},
}));
jest.mock('@librechat/api', () => ({
convertMCPToolToPlugin: jest.fn(),
}));
jest.mock('~/server/services/Config', () => ({
getAppConfig: jest.fn(),
getMCPServerTools: jest.fn(),
cacheMCPServerTools: jest.fn(),
}));
jest.mock('~/config', () => ({
getMCPManager: jest.fn(),
}));
describe('MCP Controller', () => {
let mockReq, mockRes, mockMCPManager;
beforeEach(() => {
jest.clearAllMocks();
mockReq = {
user: { id: 'test-user-id', role: 'user' },
config: null,
};
mockRes = {
status: jest.fn().mockReturnThis(),
json: jest.fn(),
};
mockMCPManager = {
getAllToolFunctions: jest.fn().mockResolvedValue({}),
};
getMCPManager.mockReturnValue(mockMCPManager);
getAppConfig.mockResolvedValue({
mcpConfig: {},
});
getMCPServerTools.mockResolvedValue(null);
});
describe('getMCPTools', () => {
it('should return 401 when user ID is not found', async () => {
mockReq.user = null;
await getMCPTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(401);
expect(mockRes.json).toHaveBeenCalledWith({ message: 'Unauthorized' });
const { logger } = require('@librechat/data-schemas');
expect(logger.warn).toHaveBeenCalledWith('[getMCPTools] User ID not found in request');
});
it('should return empty array when no mcpConfig exists', async () => {
getAppConfig.mockResolvedValue({
// No mcpConfig
});
await getMCPTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([]);
});
it('should use cached server tools when available', async () => {
const cachedTools = {
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
};
getMCPServerTools.mockResolvedValue(cachedTools);
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
convertMCPToolToPlugin.mockReturnValue(mockPlugin);
await getMCPTools(mockReq, mockRes);
expect(getMCPServerTools).toHaveBeenCalledWith('server1');
expect(mockMCPManager.getAllToolFunctions).not.toHaveBeenCalled();
expect(convertMCPToolToPlugin).toHaveBeenCalledWith({
toolKey: 'tool1~~~server1',
toolData: cachedTools['tool1~~~server1'],
mcpManager: mockMCPManager,
});
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [],
authenticated: true,
},
]);
});
it('should fetch from MCP manager when cache is empty', async () => {
getMCPServerTools.mockResolvedValue(null);
const allTools = {
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
'tool2~~~server2': {
type: 'function',
function: {
name: 'tool2',
description: 'Tool 2',
parameters: {},
},
},
};
mockMCPManager.getAllToolFunctions.mockResolvedValue(allTools);
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
convertMCPToolToPlugin.mockReturnValue(mockPlugin);
await getMCPTools(mockReq, mockRes);
expect(getMCPServerTools).toHaveBeenCalledWith('server1');
expect(mockMCPManager.getAllToolFunctions).toHaveBeenCalledWith('test-user-id');
// Should cache the server tools
const { cacheMCPServerTools } = require('~/server/services/Config');
expect(cacheMCPServerTools).toHaveBeenCalledWith({
serverName: 'server1',
serverTools: {
'tool1~~~server1': allTools['tool1~~~server1'],
},
});
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [],
authenticated: true,
},
]);
});
it('should handle custom user variables in server config', async () => {
getMCPServerTools.mockResolvedValue({
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
});
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {
customUserVars: {
API_KEY: {
title: 'API Key',
description: 'Your API key',
},
SECRET: {
title: 'Secret Token',
description: 'Your secret token',
},
},
},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
convertMCPToolToPlugin.mockReturnValue(mockPlugin);
await getMCPTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [
{
authField: 'API_KEY',
label: 'API Key',
description: 'Your API key',
},
{
authField: 'SECRET',
label: 'Secret Token',
description: 'Your secret token',
},
],
authenticated: false,
},
]);
});
it('should handle empty custom user variables', async () => {
getMCPServerTools.mockResolvedValue({
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
});
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {
customUserVars: {},
},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
convertMCPToolToPlugin.mockReturnValue(mockPlugin);
await getMCPTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [],
authenticated: true,
},
]);
});
it('should handle multiple servers', async () => {
getMCPServerTools.mockResolvedValue(null);
const allTools = {
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
'tool2~~~server2': {
type: 'function',
function: {
name: 'tool2',
description: 'Tool 2',
parameters: {},
},
},
};
mockMCPManager.getAllToolFunctions.mockResolvedValue(allTools);
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {},
server2: {},
},
});
const mockPlugin1 = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
const mockPlugin2 = {
name: 'Tool 2',
pluginKey: 'tool2~~~server2',
description: 'Tool 2',
};
convertMCPToolToPlugin.mockReturnValueOnce(mockPlugin1).mockReturnValueOnce(mockPlugin2);
await getMCPTools(mockReq, mockRes);
expect(getMCPServerTools).toHaveBeenCalledWith('server1');
expect(getMCPServerTools).toHaveBeenCalledWith('server2');
expect(mockMCPManager.getAllToolFunctions).toHaveBeenCalledTimes(2);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin1,
authConfig: [],
authenticated: true,
},
{
...mockPlugin2,
authConfig: [],
authenticated: true,
},
]);
});
it('should handle server-specific errors gracefully', async () => {
getMCPServerTools.mockResolvedValue(null);
mockMCPManager.getAllToolFunctions.mockRejectedValue(new Error('Server connection failed'));
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {},
server2: {},
},
});
await getMCPTools(mockReq, mockRes);
const { logger } = require('@librechat/data-schemas');
expect(logger.error).toHaveBeenCalledWith(
'[getMCPTools] Error loading tools for server server1:',
expect.any(Error),
);
expect(logger.error).toHaveBeenCalledWith(
'[getMCPTools] Error loading tools for server server2:',
expect.any(Error),
);
// Should still return 200 with empty array
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([]);
});
it('should skip tools when convertMCPToolToPlugin returns null', async () => {
getMCPServerTools.mockResolvedValue({
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
'tool2~~~server1': {
type: 'function',
function: {
name: 'tool2',
description: 'Tool 2',
parameters: {},
},
},
});
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
// First tool returns plugin, second returns null
convertMCPToolToPlugin.mockReturnValueOnce(mockPlugin).mockReturnValueOnce(null);
await getMCPTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [],
authenticated: true,
},
]);
});
it('should use req.config when available', async () => {
const reqConfig = {
mcpConfig: {
server1: {},
},
};
mockReq.config = reqConfig;
getMCPServerTools.mockResolvedValue({
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
convertMCPToolToPlugin.mockReturnValue(mockPlugin);
await getMCPTools(mockReq, mockRes);
// Should not call getAppConfig when req.config is available
expect(getAppConfig).not.toHaveBeenCalled();
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [],
authenticated: true,
},
]);
});
it('should handle general error in getMCPTools', async () => {
const error = new Error('Unexpected error');
getAppConfig.mockRejectedValue(error);
await getMCPTools(mockReq, mockRes);
const { logger } = require('@librechat/data-schemas');
expect(logger.error).toHaveBeenCalledWith('[getMCPTools]', error);
expect(mockRes.status).toHaveBeenCalledWith(500);
expect(mockRes.json).toHaveBeenCalledWith({ message: 'Unexpected error' });
});
it('should handle custom user variables without title or description', async () => {
getMCPServerTools.mockResolvedValue({
'tool1~~~server1': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
});
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {
customUserVars: {
MY_VAR: {
// No title or description
},
},
},
},
});
const mockPlugin = {
name: 'Tool 1',
pluginKey: 'tool1~~~server1',
description: 'Tool 1',
};
convertMCPToolToPlugin.mockReturnValue(mockPlugin);
await getMCPTools(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([
{
...mockPlugin,
authConfig: [
{
authField: 'MY_VAR',
label: 'MY_VAR', // Falls back to key
description: '', // Empty string
},
],
authenticated: false,
},
]);
});
it('should not cache when no tools are found for a server', async () => {
getMCPServerTools.mockResolvedValue(null);
const allTools = {
'tool1~~~otherserver': {
type: 'function',
function: {
name: 'tool1',
description: 'Tool 1',
parameters: {},
},
},
};
mockMCPManager.getAllToolFunctions.mockResolvedValue(allTools);
getAppConfig.mockResolvedValue({
mcpConfig: {
server1: {},
},
});
await getMCPTools(mockReq, mockRes);
const { cacheMCPServerTools } = require('~/server/services/Config');
expect(cacheMCPServerTools).not.toHaveBeenCalled();
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith([]);
});
});
});