diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index 550e362701..15ccd38129 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -10,8 +10,8 @@ const { createSafeUser, mcpToolPattern, loadWebSearchAuth, - mcpServersRegistry, } = require('@librechat/api'); +const { getMCPServersRegistry } = require('~/config'); const { Tools, Constants, @@ -348,7 +348,10 @@ Anchor pattern: \\ue202turn{N}{type}{index} where N=turn number, type=search|new /** Placeholder used for UI purposes */ continue; } - if (serverName && (await mcpServersRegistry.getServerConfig(serverName, user)) == undefined) { + if ( + serverName && + (await getMCPServersRegistry().getServerConfig(serverName, user)) == undefined + ) { logger.warn( `MCP server "${serverName}" for "${toolName}" tool is not configured${agent?.id != null && agent.id ? ` but attached to "${agent.id}"` : ''}`, ); diff --git a/api/config/index.js b/api/config/index.js index 0ddbee1661..3b6d869332 100644 --- a/api/config/index.js +++ b/api/config/index.js @@ -1,6 +1,11 @@ const { EventSource } = require('eventsource'); const { Time } = require('librechat-data-provider'); -const { MCPManager, FlowStateManager, OAuthReconnectionManager } = require('@librechat/api'); +const { + MCPManager, + FlowStateManager, + MCPServersRegistry, + OAuthReconnectionManager, +} = require('@librechat/api'); const logger = require('./winston'); global.EventSource = EventSource; @@ -23,6 +28,8 @@ function getFlowStateManager(flowsCache) { module.exports = { logger, + createMCPServersRegistry: MCPServersRegistry.createInstance, + getMCPServersRegistry: MCPServersRegistry.getInstance, createMCPManager: MCPManager.createInstance, getMCPManager: MCPManager.getInstance, getFlowStateManager, diff --git a/api/server/controllers/UserController.js b/api/server/controllers/UserController.js index 9bdf6c5e28..75f3ab2ce3 100644 --- a/api/server/controllers/UserController.js +++ b/api/server/controllers/UserController.js @@ -3,7 +3,6 @@ const { Tools, CacheKeys, Constants, FileSources } = require('librechat-data-pro const { MCPOAuthHandler, MCPTokenStorage, - mcpServersRegistry, normalizeHttpError, extractWebSearchEnvVars, } = require('@librechat/api'); @@ -34,9 +33,9 @@ const { const { updateUserPluginAuth, deleteUserPluginAuth } = require('~/server/services/PluginService'); const { updateUserPluginsService, deleteUserKey } = require('~/server/services/UserService'); const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService'); +const { getMCPManager, getFlowStateManager, getMCPServersRegistry } = require('~/config'); const { needsRefresh, getNewS3URL } = require('~/server/services/Files/S3/crud'); const { processDeleteRequest } = require('~/server/services/Files/process'); -const { getMCPManager, getFlowStateManager } = require('~/config'); const { getAppConfig } = require('~/server/services/Config'); const { deleteToolCalls } = require('~/models/ToolCall'); const { deleteUserPrompts } = require('~/models/Prompt'); @@ -321,9 +320,9 @@ const maybeUninstallOAuthMCP = async (userId, pluginKey, appConfig) => { const serverName = pluginKey.replace(Constants.mcp_prefix, ''); const serverConfig = - (await mcpServersRegistry.getServerConfig(serverName, userId)) ?? + (await getMCPServersRegistry().getServerConfig(serverName, userId)) ?? appConfig?.mcpServers?.[serverName]; - const oauthServers = await mcpServersRegistry.getOAuthServers(); + const oauthServers = await getMCPServersRegistry().getOAuthServers(); if (!oauthServers.has(serverName)) { // this server does not use OAuth, so nothing to do here as well return; diff --git a/api/server/controllers/mcp.js b/api/server/controllers/mcp.js index 2a557e5a8b..591816cd14 100644 --- a/api/server/controllers/mcp.js +++ b/api/server/controllers/mcp.js @@ -5,8 +5,7 @@ const { logger } = require('@librechat/data-schemas'); const { Constants } = require('librechat-data-provider'); const { cacheMCPServerTools, getMCPServerTools } = require('~/server/services/Config'); -const { getMCPManager } = require('~/config'); -const { mcpServersRegistry } = require('@librechat/api'); +const { getMCPManager, getMCPServersRegistry } = require('~/config'); /** * Get all MCP tools available to the user @@ -19,7 +18,7 @@ const getMCPTools = async (req, res) => { return res.status(401).json({ message: 'Unauthorized' }); } - const mcpConfig = await mcpServersRegistry.getAllServerConfigs(userId); + const mcpConfig = await getMCPServersRegistry().getAllServerConfigs(userId); const configuredServers = mcpConfig ? Object.keys(mcpConfig) : []; if (!mcpConfig || Object.keys(mcpConfig).length == 0) { @@ -69,7 +68,7 @@ const getMCPTools = async (req, res) => { // Get server config once const serverConfig = mcpConfig[serverName]; - const rawServerConfig = await mcpServersRegistry.getServerConfig(serverName, userId); + const rawServerConfig = await getMCPServersRegistry().getServerConfig(serverName, userId); // Initialize server object with all server-level data const server = { @@ -137,7 +136,7 @@ const getMCPServersList = async (req, res) => { // TODO - Ensure DB servers loaded into registry (configs only) // 2. Get all server configs from registry (YAML + DB) - const serverConfigs = await mcpServersRegistry.getAllServerConfigs(userId); + const serverConfigs = await getMCPServersRegistry().getAllServerConfigs(userId); return res.json(serverConfigs); } catch (error) { diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 5fb8b86695..4a53581d4c 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -3,6 +3,12 @@ const request = require('supertest'); const mongoose = require('mongoose'); const { MongoMemoryServer } = require('mongodb-memory-server'); +const mockRegistryInstance = { + getServerConfig: jest.fn(), + getOAuthServers: jest.fn(), + getAllServerConfigs: jest.fn(), +}; + jest.mock('@librechat/api', () => ({ ...jest.requireActual('@librechat/api'), MCPOAuthHandler: { @@ -13,12 +19,13 @@ jest.mock('@librechat/api', () => ({ }, MCPTokenStorage: { storeTokens: jest.fn(), + getClientInfoAndMetadata: jest.fn(), + getTokens: jest.fn(), + deleteUserTokens: jest.fn(), }, getUserMCPAuthMap: jest.fn(), - mcpServersRegistry: { - getServerConfig: jest.fn(), - getOAuthServers: jest.fn(), - getAllServerConfigs: jest.fn(), + MCPServersRegistry: { + getInstance: () => mockRegistryInstance, }, })); @@ -39,6 +46,9 @@ jest.mock('@librechat/data-schemas', () => ({ findById: jest.fn(), }, })), + createMethods: jest.fn(() => ({ + findUser: jest.fn(), + })), })); jest.mock('~/models', () => ({ @@ -72,6 +82,8 @@ jest.mock('~/server/services/PluginService', () => ({ jest.mock('~/config', () => ({ getMCPManager: jest.fn(), getFlowStateManager: jest.fn(), + getOAuthReconnectionManager: jest.fn(), + getMCPServersRegistry: jest.fn(() => mockRegistryInstance), })); jest.mock('~/cache', () => ({ @@ -120,7 +132,7 @@ describe('MCP Routes', () => { }); describe('GET /:serverName/oauth/initiate', () => { - const { MCPOAuthHandler, mcpServersRegistry } = require('@librechat/api'); + const { MCPOAuthHandler } = require('@librechat/api'); const { getLogStores } = require('~/cache'); it('should initiate OAuth flow successfully', async () => { @@ -135,7 +147,7 @@ describe('MCP Routes', () => { getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); MCPOAuthHandler.initiateOAuthFlow.mockResolvedValue({ authorizationUrl: 'https://oauth.example.com/auth', @@ -289,7 +301,7 @@ describe('MCP Routes', () => { }); it('should handle OAuth callback successfully', async () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file const mockFlowManager = { getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING' }), completeFlow: jest.fn().mockResolvedValue(), @@ -310,7 +322,7 @@ describe('MCP Routes', () => { MCPOAuthHandler.getFlowState.mockResolvedValue(mockFlowState); MCPOAuthHandler.completeOAuthFlow.mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockResolvedValue(); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); @@ -382,7 +394,7 @@ describe('MCP Routes', () => { }); it('should handle system-level OAuth completion', async () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file const mockFlowManager = { getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING' }), completeFlow: jest.fn().mockResolvedValue(), @@ -403,7 +415,7 @@ describe('MCP Routes', () => { MCPOAuthHandler.getFlowState.mockResolvedValue(mockFlowState); MCPOAuthHandler.completeOAuthFlow.mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockResolvedValue(); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); @@ -418,7 +430,7 @@ describe('MCP Routes', () => { }); it('should handle reconnection failure after OAuth', async () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file const mockFlowManager = { getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING' }), completeFlow: jest.fn().mockResolvedValue(), @@ -439,7 +451,7 @@ describe('MCP Routes', () => { MCPOAuthHandler.getFlowState.mockResolvedValue(mockFlowState); MCPOAuthHandler.completeOAuthFlow.mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockResolvedValue(); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); @@ -464,7 +476,7 @@ describe('MCP Routes', () => { }); it('should redirect to error page if token storage fails', async () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file const mockFlowManager = { completeFlow: jest.fn().mockResolvedValue(), deleteFlow: jest.fn().mockResolvedValue(true), @@ -484,7 +496,7 @@ describe('MCP Routes', () => { MCPOAuthHandler.getFlowState.mockResolvedValue(mockFlowState); MCPOAuthHandler.completeOAuthFlow.mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockRejectedValue(new Error('store failed')); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); @@ -504,7 +516,7 @@ describe('MCP Routes', () => { }); it('should use original flow state credentials when storing tokens', async () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file const mockFlowManager = { getFlowState: jest.fn(), completeFlow: jest.fn().mockResolvedValue(), @@ -536,7 +548,7 @@ describe('MCP Routes', () => { MCPOAuthHandler.getFlowState.mockResolvedValue(flowState); MCPOAuthHandler.completeOAuthFlow.mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockResolvedValue(); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); @@ -837,14 +849,14 @@ describe('MCP Routes', () => { }); describe('POST /:serverName/reinitialize', () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file it('should return 404 when server is not found in configuration', async () => { const mockMcpManager = { disconnectUserConnection: jest.fn().mockResolvedValue(), }; - mcpServersRegistry.getServerConfig.mockResolvedValue(null); + mockRegistryInstance.getServerConfig.mockResolvedValue(null); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); require('~/config').getFlowStateManager.mockReturnValue({}); require('~/cache').getLogStores.mockReturnValue({}); @@ -869,7 +881,7 @@ describe('MCP Routes', () => { }), }; - mcpServersRegistry.getServerConfig.mockResolvedValue({ + mockRegistryInstance.getServerConfig.mockResolvedValue({ customUserVars: {}, }); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); @@ -902,7 +914,7 @@ describe('MCP Routes', () => { getUserConnection: jest.fn().mockRejectedValue(new Error('Connection failed')), }; - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); require('~/config').getFlowStateManager.mockReturnValue({}); require('~/cache').getLogStores.mockReturnValue({}); @@ -921,7 +933,7 @@ describe('MCP Routes', () => { disconnectUserConnection: jest.fn(), }; - mcpServersRegistry.getServerConfig.mockImplementation(() => { + mockRegistryInstance.getServerConfig.mockImplementation(() => { throw new Error('Config loading failed'); }); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); @@ -960,7 +972,9 @@ describe('MCP Routes', () => { getUserConnection: jest.fn().mockResolvedValue(mockUserConnection), }; - mcpServersRegistry.getServerConfig.mockResolvedValue({ endpoint: 'http://test-server.com' }); + mockRegistryInstance.getServerConfig.mockResolvedValue({ + endpoint: 'http://test-server.com', + }); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); require('~/config').getFlowStateManager.mockReturnValue({}); require('~/cache').getLogStores.mockReturnValue({}); @@ -1005,7 +1019,7 @@ describe('MCP Routes', () => { getUserConnection: jest.fn().mockResolvedValue(mockUserConnection), }; - mcpServersRegistry.getServerConfig.mockResolvedValue({ + mockRegistryInstance.getServerConfig.mockResolvedValue({ endpoint: 'http://test-server.com', customUserVars: { API_KEY: 'some-env-var', @@ -1215,12 +1229,12 @@ describe('MCP Routes', () => { describe('GET /:serverName/auth-values', () => { const { getUserPluginAuthValue } = require('~/server/services/PluginService'); - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file it('should return auth value flags for server', async () => { const mockMcpManager = {}; - mcpServersRegistry.getServerConfig.mockResolvedValue({ + mockRegistryInstance.getServerConfig.mockResolvedValue({ customUserVars: { API_KEY: 'some-env-var', SECRET_TOKEN: 'another-env-var', @@ -1247,7 +1261,7 @@ describe('MCP Routes', () => { it('should return 404 when server is not found in configuration', async () => { const mockMcpManager = {}; - mcpServersRegistry.getServerConfig.mockResolvedValue(null); + mockRegistryInstance.getServerConfig.mockResolvedValue(null); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); const response = await request(app).get('/api/mcp/non-existent-server/auth-values'); @@ -1261,7 +1275,7 @@ describe('MCP Routes', () => { it('should handle errors when checking auth values', async () => { const mockMcpManager = {}; - mcpServersRegistry.getServerConfig.mockResolvedValue({ + mockRegistryInstance.getServerConfig.mockResolvedValue({ customUserVars: { API_KEY: 'some-env-var', }, @@ -1284,7 +1298,7 @@ describe('MCP Routes', () => { it('should return 500 when auth values check throws unexpected error', async () => { const mockMcpManager = {}; - mcpServersRegistry.getServerConfig.mockImplementation(() => { + mockRegistryInstance.getServerConfig.mockImplementation(() => { throw new Error('Config loading failed'); }); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); @@ -1298,7 +1312,7 @@ describe('MCP Routes', () => { it('should handle customUserVars that is not an object', async () => { const mockMcpManager = {}; - mcpServersRegistry.getServerConfig.mockResolvedValue({ + mockRegistryInstance.getServerConfig.mockResolvedValue({ customUserVars: 'not-an-object', }); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); @@ -1327,7 +1341,7 @@ describe('MCP Routes', () => { describe('GET /:serverName/oauth/callback - Edge Cases', () => { it('should handle OAuth callback without toolFlowId (falsy toolFlowId)', async () => { - const { MCPOAuthHandler, MCPTokenStorage, mcpServersRegistry } = require('@librechat/api'); + const { MCPOAuthHandler, MCPTokenStorage } = require('@librechat/api'); const mockTokens = { access_token: 'edge-access-token', refresh_token: 'edge-refresh-token', @@ -1345,7 +1359,7 @@ describe('MCP Routes', () => { }); MCPOAuthHandler.completeOAuthFlow = jest.fn().mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockResolvedValue(); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); const mockFlowManager = { getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING' }), @@ -1372,7 +1386,7 @@ describe('MCP Routes', () => { it('should handle null cached tools in OAuth callback (triggers || {} fallback)', async () => { const { getCachedTools } = require('~/server/services/Config'); getCachedTools.mockResolvedValue(null); - const { MCPOAuthHandler, MCPTokenStorage, mcpServersRegistry } = require('@librechat/api'); + const { MCPOAuthHandler, MCPTokenStorage } = require('@librechat/api'); const mockTokens = { access_token: 'edge-access-token', refresh_token: 'edge-refresh-token', @@ -1398,7 +1412,7 @@ describe('MCP Routes', () => { }); MCPOAuthHandler.completeOAuthFlow.mockResolvedValue(mockTokens); MCPTokenStorage.storeTokens.mockResolvedValue(); - mcpServersRegistry.getServerConfig.mockResolvedValue({}); + mockRegistryInstance.getServerConfig.mockResolvedValue({}); const mockMcpManager = { getUserConnection: jest.fn().mockResolvedValue({ @@ -1418,7 +1432,7 @@ describe('MCP Routes', () => { }); describe('GET /servers', () => { - const { mcpServersRegistry } = require('@librechat/api'); + // mockRegistryInstance is defined at the top of the file it('should return all server configs for authenticated user', async () => { const mockServerConfigs = { @@ -1432,17 +1446,17 @@ describe('MCP Routes', () => { }, }; - mcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockServerConfigs); + mockRegistryInstance.getAllServerConfigs.mockResolvedValue(mockServerConfigs); const response = await request(app).get('/api/mcp/servers'); expect(response.status).toBe(200); expect(response.body).toEqual(mockServerConfigs); - expect(mcpServersRegistry.getAllServerConfigs).toHaveBeenCalledWith('test-user-id'); + expect(mockRegistryInstance.getAllServerConfigs).toHaveBeenCalledWith('test-user-id'); }); it('should return empty object when no servers are configured', async () => { - mcpServersRegistry.getAllServerConfigs.mockResolvedValue({}); + mockRegistryInstance.getAllServerConfigs.mockResolvedValue({}); const response = await request(app).get('/api/mcp/servers'); @@ -1466,7 +1480,7 @@ describe('MCP Routes', () => { }); it('should return 500 when server config retrieval fails', async () => { - mcpServersRegistry.getAllServerConfigs.mockRejectedValue(new Error('Database error')); + mockRegistryInstance.getAllServerConfigs.mockRejectedValue(new Error('Database error')); const response = await request(app).get('/api/mcp/servers'); diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index 851255b59d..62d735a797 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -6,9 +6,13 @@ const { MCPOAuthHandler, MCPTokenStorage, getUserMCPAuthMap, - mcpServersRegistry, } = require('@librechat/api'); -const { getMCPManager, getFlowStateManager, getOAuthReconnectionManager } = require('~/config'); +const { + getMCPManager, + getFlowStateManager, + getOAuthReconnectionManager, + getMCPServersRegistry, +} = require('~/config'); const { getMCPSetupData, getServerConnectionStatus } = require('~/server/services/MCP'); const { findToken, updateToken, createToken, deleteTokens } = require('~/models'); const { getUserPluginAuthValue } = require('~/server/services/PluginService'); @@ -365,7 +369,7 @@ router.post('/:serverName/reinitialize', requireJwtAuth, async (req, res) => { logger.info(`[MCP Reinitialize] Reinitializing server: ${serverName}`); const mcpManager = getMCPManager(); - const serverConfig = await mcpServersRegistry.getServerConfig(serverName, user.id); + const serverConfig = await getMCPServersRegistry().getServerConfig(serverName, user.id); if (!serverConfig) { return res.status(404).json({ error: `MCP server '${serverName}' not found in configuration`, @@ -526,7 +530,7 @@ router.get('/:serverName/auth-values', requireJwtAuth, async (req, res) => { return res.status(401).json({ error: 'User not authenticated' }); } - const serverConfig = await mcpServersRegistry.getServerConfig(serverName, user.id); + const serverConfig = await getMCPServersRegistry().getServerConfig(serverName, user.id); if (!serverConfig) { return res.status(404).json({ error: `MCP server '${serverName}' not found in configuration`, @@ -566,7 +570,7 @@ router.get('/:serverName/auth-values', requireJwtAuth, async (req, res) => { }); async function getOAuthHeaders(serverName, userId) { - const serverConfig = await mcpServersRegistry.getServerConfig(serverName, userId); + const serverConfig = await getMCPServersRegistry().getServerConfig(serverName, userId); return serverConfig?.oauth_headers ?? {}; } /** diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 1e261bdacf..d0568fd189 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -20,12 +20,15 @@ const { ContentTypes, isAssistantsEndpoint, } = require('librechat-data-provider'); -const { getMCPManager, getFlowStateManager, getOAuthReconnectionManager } = require('~/config'); +const { + getMCPManager, + getFlowStateManager, + getOAuthReconnectionManager, + getMCPServersRegistry, +} = require('~/config'); const { findToken, createToken, updateToken } = require('~/models'); const { reinitMCPServer } = require('./Tools/mcp'); -const { getAppConfig } = require('./Config'); const { getLogStores } = require('~/cache'); -const { mcpServersRegistry } = require('@librechat/api'); /** * @param {object} params @@ -435,7 +438,7 @@ function createToolInstance({ res, toolName, serverName, toolDefinition, provide * @returns {Object} Object containing mcpConfig, appConnections, userConnections, and oauthServers */ async function getMCPSetupData(userId) { - const mcpConfig = await mcpServersRegistry.getAllServerConfigs(userId); + const mcpConfig = await getMCPServersRegistry().getAllServerConfigs(userId); if (!mcpConfig) { throw new Error('MCP config not found'); @@ -450,7 +453,7 @@ async function getMCPSetupData(userId) { logger.error(`[MCP][User: ${userId}] Error getting app connections:`, error); } const userConnections = mcpManager.getUserConnections(userId) || new Map(); - const oauthServers = await mcpServersRegistry.getOAuthServers(userId); + const oauthServers = await getMCPServersRegistry().getOAuthServers(userId); return { mcpConfig, diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index 8f7c467743..2f6d1603a9 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -43,6 +43,11 @@ jest.mock('@librechat/agents', () => ({ }, })); +const mockRegistryInstance = { + getOAuthServers: jest.fn(() => Promise.resolve(new Set())), + getAllServerConfigs: jest.fn(() => Promise.resolve({})), +}; + jest.mock('@librechat/api', () => ({ MCPOAuthHandler: { generateFlowId: jest.fn(), @@ -50,9 +55,8 @@ jest.mock('@librechat/api', () => ({ sendEvent: jest.fn(), normalizeServerName: jest.fn((name) => name), convertWithResolvedRefs: jest.fn((params) => params), - mcpServersRegistry: { - getOAuthServers: jest.fn(() => Promise.resolve(new Set())), - getAllServerConfigs: jest.fn(() => Promise.resolve({})), + MCPServersRegistry: { + getInstance: () => mockRegistryInstance, }, })); @@ -83,6 +87,7 @@ jest.mock('~/config', () => ({ getMCPManager: jest.fn(), getFlowStateManager: jest.fn(), getOAuthReconnectionManager: jest.fn(), + getMCPServersRegistry: jest.fn(() => mockRegistryInstance), })); jest.mock('~/cache', () => ({ @@ -104,7 +109,6 @@ describe('tests for the new helper functions used by the MCP connection status e let mockGetFlowStateManager; let mockGetLogStores; let mockGetOAuthReconnectionManager; - let mockMcpServersRegistry; beforeEach(() => { jest.clearAllMocks(); @@ -113,7 +117,6 @@ describe('tests for the new helper functions used by the MCP connection status e mockGetFlowStateManager = require('~/config').getFlowStateManager; mockGetLogStores = require('~/cache').getLogStores; mockGetOAuthReconnectionManager = require('~/config').getOAuthReconnectionManager; - mockMcpServersRegistry = require('@librechat/api').mcpServersRegistry; }); describe('getMCPSetupData', () => { @@ -128,12 +131,12 @@ describe('tests for the new helper functions used by the MCP connection status e appConnections: { getAll: jest.fn(() => new Map()) }, getUserConnections: jest.fn(() => new Map()), }); - mockMcpServersRegistry.getOAuthServers.mockResolvedValue(new Set()); - mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockConfig); + mockRegistryInstance.getOAuthServers.mockResolvedValue(new Set()); + mockRegistryInstance.getAllServerConfigs.mockResolvedValue(mockConfig); }); it('should successfully return MCP setup data', async () => { - mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockConfig); + mockRegistryInstance.getAllServerConfigs.mockResolvedValue(mockConfig); const mockAppConnections = new Map([['server1', { status: 'connected' }]]); const mockUserConnections = new Map([['server2', { status: 'disconnected' }]]); @@ -144,15 +147,15 @@ describe('tests for the new helper functions used by the MCP connection status e getUserConnections: jest.fn(() => mockUserConnections), }; mockGetMCPManager.mockReturnValue(mockMCPManager); - mockMcpServersRegistry.getOAuthServers.mockResolvedValue(mockOAuthServers); + mockRegistryInstance.getOAuthServers.mockResolvedValue(mockOAuthServers); const result = await getMCPSetupData(mockUserId); - expect(mockMcpServersRegistry.getAllServerConfigs).toHaveBeenCalledWith(mockUserId); + expect(mockRegistryInstance.getAllServerConfigs).toHaveBeenCalledWith(mockUserId); expect(mockGetMCPManager).toHaveBeenCalledWith(mockUserId); expect(mockMCPManager.appConnections.getAll).toHaveBeenCalled(); expect(mockMCPManager.getUserConnections).toHaveBeenCalledWith(mockUserId); - expect(mockMcpServersRegistry.getOAuthServers).toHaveBeenCalledWith(mockUserId); + expect(mockRegistryInstance.getOAuthServers).toHaveBeenCalledWith(mockUserId); expect(result).toEqual({ mcpConfig: mockConfig, @@ -163,19 +166,19 @@ describe('tests for the new helper functions used by the MCP connection status e }); it('should throw error when MCP config not found', async () => { - mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(null); + mockRegistryInstance.getAllServerConfigs.mockResolvedValue(null); await expect(getMCPSetupData(mockUserId)).rejects.toThrow('MCP config not found'); }); it('should handle null values from MCP manager gracefully', async () => { - mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockConfig); + mockRegistryInstance.getAllServerConfigs.mockResolvedValue(mockConfig); const mockMCPManager = { appConnections: { getAll: jest.fn(() => Promise.resolve(null)) }, getUserConnections: jest.fn(() => null), }; mockGetMCPManager.mockReturnValue(mockMCPManager); - mockMcpServersRegistry.getOAuthServers.mockResolvedValue(new Set()); + mockRegistryInstance.getOAuthServers.mockResolvedValue(new Set()); const result = await getMCPSetupData(mockUserId); diff --git a/api/server/services/initializeMCPs.js b/api/server/services/initializeMCPs.js index 7fdb128683..e4306245bb 100644 --- a/api/server/services/initializeMCPs.js +++ b/api/server/services/initializeMCPs.js @@ -1,6 +1,7 @@ +const mongoose = require('mongoose'); const { logger } = require('@librechat/data-schemas'); const { mergeAppTools, getAppConfig } = require('./Config'); -const { createMCPManager } = require('~/config'); +const { createMCPServersRegistry, createMCPManager } = require('~/config'); /** * Initialize MCP servers @@ -12,6 +13,14 @@ async function initializeMCPs() { return; } + // Initialize MCPServersRegistry first (required for MCPManager) + try { + createMCPServersRegistry(mongoose); + } catch (error) { + logger.error('[MCP] Failed to initialize MCPServersRegistry:', error); + throw error; + } + const mcpManager = await createMCPManager(mcpServers); try { diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts index d5f6eba6c3..4e2ad5bc9d 100644 --- a/packages/api/src/mcp/ConnectionsRepository.ts +++ b/packages/api/src/mcp/ConnectionsRepository.ts @@ -1,7 +1,7 @@ import { logger } from '@librechat/data-schemas'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { MCPConnection } from './connection'; -import { mcpServersRegistry as registry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import type * as t from './types'; /** @@ -25,7 +25,7 @@ export class ConnectionsRepository { /** Checks whether this repository can connect to a specific server */ async has(serverName: string): Promise { - const config = await registry.getServerConfig(serverName, this.ownerId); + const config = await MCPServersRegistry.getInstance().getServerConfig(serverName, this.ownerId); const canConnect = !!config && this.isAllowedToConnectToServer(config); if (!canConnect) { //if connection is no longer possible we attempt to disconnect any leftover connections @@ -36,7 +36,10 @@ export class ConnectionsRepository { /** Gets or creates a connection for the specified server with lazy loading */ async get(serverName: string): Promise { - const serverConfig = await registry.getServerConfig(serverName, this.ownerId); + const serverConfig = await MCPServersRegistry.getInstance().getServerConfig( + serverName, + this.ownerId, + ); const existingConnection = this.connections.get(serverName); if (!serverConfig || !this.isAllowedToConnectToServer(serverConfig)) { @@ -94,7 +97,7 @@ export class ConnectionsRepository { async getAll(): Promise> { //TODO in the future we should use a scoped config getter (APPLevel, UserLevel, Private) //for now the absent config will not throw error - const allConfigs = await registry.getAllServerConfigs(this.ownerId); + const allConfigs = await MCPServersRegistry.getInstance().getAllServerConfigs(this.ownerId); return this.getMany(Object.keys(allConfigs)); } diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index 3ed4737dfd..fbd5bd050d 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -11,7 +11,7 @@ import { UserConnectionManager } from './UserConnectionManager'; import { ConnectionsRepository } from './ConnectionsRepository'; import { MCPServerInspector } from './registry/MCPServerInspector'; import { MCPServersInitializer } from './registry/MCPServersInitializer'; -import { mcpServersRegistry as registry } from './registry/MCPServersRegistry'; +import { MCPServersRegistry } from './registry/MCPServersRegistry'; import { formatToolContent } from './parsers'; import { MCPConnection } from './connection'; import { processMCPEnv } from '~/utils/env'; @@ -69,7 +69,7 @@ export class MCPManager extends UserConnectionManager { /** Returns all available tool functions from app-level connections */ public async getAppToolFunctions(): Promise { const toolFunctions: t.LCAvailableTools = {}; - const configs = await registry.getAllServerConfigs(); + const configs = await MCPServersRegistry.getInstance().getAllServerConfigs(); for (const config of Object.values(configs)) { if (config.toolFunctions != null) { Object.assign(toolFunctions, config.toolFunctions); @@ -115,7 +115,7 @@ export class MCPManager extends UserConnectionManager { */ private async getInstructions(serverNames?: string[]): Promise> { const instructions: Record = {}; - const configs = await registry.getAllServerConfigs(); + const configs = await MCPServersRegistry.getInstance().getAllServerConfigs(); for (const [serverName, config] of Object.entries(configs)) { if (config.serverInstructions != null) { instructions[serverName] = config.serverInstructions as string; @@ -216,7 +216,10 @@ Please follow these instructions when using tools from the respective MCP server ); } - const rawConfig = (await registry.getServerConfig(serverName, userId)) as t.MCPOptions; + const rawConfig = (await MCPServersRegistry.getInstance().getServerConfig( + serverName, + userId, + )) as t.MCPOptions; const currentOptions = processMCPEnv({ user, options: rawConfig, diff --git a/packages/api/src/mcp/UserConnectionManager.ts b/packages/api/src/mcp/UserConnectionManager.ts index 0ce97b0e48..a775b4d78d 100644 --- a/packages/api/src/mcp/UserConnectionManager.ts +++ b/packages/api/src/mcp/UserConnectionManager.ts @@ -1,7 +1,7 @@ import { logger } from '@librechat/data-schemas'; import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; -import { mcpServersRegistry as serversRegistry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { MCPConnection } from './connection'; import type * as t from './types'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; @@ -61,7 +61,7 @@ export abstract class UserConnectionManager { ); } - const config = await serversRegistry.getServerConfig(serverName, userId); + const config = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId); const userServerMap = this.userConnections.get(userId); let connection = forceNew ? undefined : userServerMap?.get(serverName); diff --git a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts index 3248f13daf..bc382ebe39 100644 --- a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts +++ b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts @@ -21,18 +21,21 @@ jest.mock('../MCPConnectionFactory', () => ({ jest.mock('../connection'); // Mock the registry +const mockRegistryInstance = { + getServerConfig: jest.fn(), + getAllServerConfigs: jest.fn(), +}; + jest.mock('../registry/MCPServersRegistry', () => ({ - mcpServersRegistry: { - getServerConfig: jest.fn(), - getAllServerConfigs: jest.fn(), + MCPServersRegistry: { + getInstance: () => mockRegistryInstance, }, })); const mockLogger = logger as jest.Mocked; -// Import mocked registry -import { mcpServersRegistry as registry } from '../registry/MCPServersRegistry'; -const mockRegistry = registry as jest.Mocked; +// Use mocked registry instance +const mockRegistry = mockRegistryInstance as jest.Mocked; describe('ConnectionsRepository', () => { let repository: ConnectionsRepository; diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index f1452c2938..e75cd09b33 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -1,7 +1,6 @@ import { logger } from '@librechat/data-schemas'; import type * as t from '~/mcp/types'; import { MCPManager } from '~/mcp/MCPManager'; -import { mcpServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { MCPServersInitializer } from '~/mcp/registry/MCPServersInitializer'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; @@ -17,11 +16,15 @@ jest.mock('@librechat/data-schemas', () => ({ }, })); +const mockRegistryInstance = { + getServerConfig: jest.fn(), + getAllServerConfigs: jest.fn(), + getOAuthServers: jest.fn(), +}; + jest.mock('~/mcp/registry/MCPServersRegistry', () => ({ - mcpServersRegistry: { - getServerConfig: jest.fn(), - getAllServerConfigs: jest.fn(), - getOAuthServers: jest.fn(), + MCPServersRegistry: { + getInstance: () => mockRegistryInstance, }, })); @@ -47,7 +50,7 @@ describe('MCPManager', () => { // Set up default mock implementations (MCPServersInitializer.initialize as jest.Mock).mockResolvedValue(undefined); - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({}); + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({}); }); function mockAppConnections( @@ -75,7 +78,7 @@ describe('MCPManager', () => { describe('getAppToolFunctions', () => { it('should return empty object when no servers have tool functions', async () => { - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ server1: { type: 'stdio', command: 'test', args: [] }, server2: { type: 'stdio', command: 'test2', args: [] }, }); @@ -109,7 +112,7 @@ describe('MCPManager', () => { }, }; - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ server1: { type: 'stdio', command: 'test', @@ -145,7 +148,7 @@ describe('MCPManager', () => { }, }; - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ server1: { type: 'stdio', command: 'test', @@ -174,7 +177,7 @@ describe('MCPManager', () => { describe('formatInstructionsForContext', () => { it('should return empty string when no servers have instructions', async () => { - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ server1: { type: 'stdio', command: 'test', args: [] }, server2: { type: 'stdio', command: 'test2', args: [] }, }); @@ -186,7 +189,7 @@ describe('MCPManager', () => { }); it('should format instructions from multiple servers', async () => { - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ github: { type: 'sse', url: 'https://api.github.com', @@ -211,7 +214,7 @@ describe('MCPManager', () => { }); it('should filter instructions by server names when provided', async () => { - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ github: { type: 'sse', url: 'https://api.github.com', @@ -243,7 +246,7 @@ describe('MCPManager', () => { }); it('should handle servers with null or undefined instructions', async () => { - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ github: { type: 'sse', url: 'https://api.github.com', @@ -272,7 +275,7 @@ describe('MCPManager', () => { }); it('should return empty string when filtered servers have no instructions', async () => { - (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({ github: { type: 'sse', url: 'https://api.github.com', diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts index f9a3c7ab73..4b2e82a05f 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts @@ -1,7 +1,6 @@ import { TokenMethods } from '@librechat/data-schemas'; import { FlowStateManager, MCPConnection, MCPOAuthTokens, MCPOptions } from '../..'; import { MCPManager } from '../MCPManager'; -import { mcpServersRegistry } from '../../mcp/registry/MCPServersRegistry'; import { OAuthReconnectionManager } from './OAuthReconnectionManager'; import { OAuthReconnectionTracker } from './OAuthReconnectionTracker'; @@ -14,11 +13,15 @@ jest.mock('@librechat/data-schemas', () => ({ }, })); +const mockRegistryInstance = { + getServerConfig: jest.fn(), + getOAuthServers: jest.fn(), +}; + jest.mock('../MCPManager'); jest.mock('../../mcp/registry/MCPServersRegistry', () => ({ - mcpServersRegistry: { - getServerConfig: jest.fn(), - getOAuthServers: jest.fn(), + MCPServersRegistry: { + getInstance: () => mockRegistryInstance, }, })); @@ -61,7 +64,7 @@ describe('OAuthReconnectionManager', () => { } as unknown as jest.Mocked; (MCPManager.getInstance as jest.Mock).mockReturnValue(mockMCPManager); - (mcpServersRegistry.getServerConfig as jest.Mock).mockResolvedValue({}); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({}); }); afterEach(() => { @@ -159,7 +162,7 @@ describe('OAuthReconnectionManager', () => { it('should reconnect eligible servers', async () => { const userId = 'user-123'; const oauthServers = new Set(['server1', 'server2', 'server3']); - (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); + (mockRegistryInstance.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); // server1: has failed reconnection reconnectionTracker.setFailed(userId, 'server1'); @@ -193,7 +196,7 @@ describe('OAuthReconnectionManager', () => { mockMCPManager.getUserConnection.mockResolvedValue( mockNewConnection as unknown as MCPConnection, ); - (mcpServersRegistry.getServerConfig as jest.Mock).mockResolvedValue({ + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ initTimeout: 5000, } as unknown as MCPOptions); @@ -224,7 +227,7 @@ describe('OAuthReconnectionManager', () => { it('should handle failed reconnection attempts', async () => { const userId = 'user-123'; const oauthServers = new Set(['server1']); - (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); + (mockRegistryInstance.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); // server1: has valid token tokenMethods.findToken.mockResolvedValue({ @@ -235,7 +238,7 @@ describe('OAuthReconnectionManager', () => { // Mock failed connection mockMCPManager.getUserConnection.mockRejectedValue(new Error('Connection failed')); - (mcpServersRegistry.getServerConfig as jest.Mock).mockResolvedValue( + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue( {} as unknown as MCPOptions, ); @@ -253,7 +256,7 @@ describe('OAuthReconnectionManager', () => { it('should not reconnect servers with expired tokens', async () => { const userId = 'user-123'; const oauthServers = new Set(['server1']); - (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); + (mockRegistryInstance.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); // server1: has expired token tokenMethods.findToken.mockResolvedValue({ @@ -272,7 +275,7 @@ describe('OAuthReconnectionManager', () => { it('should handle connection that returns but is not connected', async () => { const userId = 'user-123'; const oauthServers = new Set(['server1']); - (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); + (mockRegistryInstance.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); tokenMethods.findToken.mockResolvedValue({ userId, @@ -288,7 +291,7 @@ describe('OAuthReconnectionManager', () => { mockMCPManager.getUserConnection.mockResolvedValue( mockConnection as unknown as MCPConnection, ); - (mcpServersRegistry.getServerConfig as jest.Mock).mockResolvedValue( + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue( {} as unknown as MCPOptions, ); @@ -372,7 +375,7 @@ describe('OAuthReconnectionManager', () => { it('should not attempt to reconnect servers that have timed out during reconnection', async () => { const userId = 'user-123'; const oauthServers = new Set(['server1', 'server2']); - (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); + (mockRegistryInstance.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); const now = Date.now(); jest.setSystemTime(now); @@ -427,7 +430,7 @@ describe('OAuthReconnectionManager', () => { const userId = 'user-123'; const serverName = 'server1'; const oauthServers = new Set([serverName]); - (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); + (mockRegistryInstance.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); const now = Date.now(); jest.setSystemTime(now); @@ -441,7 +444,7 @@ describe('OAuthReconnectionManager', () => { // First reconnect attempt - will fail mockMCPManager.getUserConnection.mockRejectedValueOnce(new Error('Connection failed')); - (mcpServersRegistry.getServerConfig as jest.Mock).mockResolvedValue( + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue( {} as unknown as MCPOptions, ); diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts index db81a2ffae..186f3652e3 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts @@ -4,7 +4,7 @@ import type { MCPOAuthTokens } from './types'; import { OAuthReconnectionTracker } from './OAuthReconnectionTracker'; import { FlowStateManager } from '~/flow/manager'; import { MCPManager } from '~/mcp/MCPManager'; -import { mcpServersRegistry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; const DEFAULT_CONNECTION_TIMEOUT_MS = 10_000; // ms @@ -72,7 +72,7 @@ export class OAuthReconnectionManager { // 1. derive the servers to reconnect const serversToReconnect = []; - for (const serverName of await mcpServersRegistry.getOAuthServers()) { + for (const serverName of await MCPServersRegistry.getInstance().getOAuthServers()) { const canReconnect = await this.canReconnect(userId, serverName); if (canReconnect) { serversToReconnect.push(serverName); @@ -104,7 +104,7 @@ export class OAuthReconnectionManager { logger.info(`${logPrefix} Attempting reconnection`); - const config = await mcpServersRegistry.getServerConfig(serverName, userId); + const config = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId); const cleanupOnFailedReconnect = () => { this.reconnectionsTracker.setFailed(userId, serverName); diff --git a/packages/api/src/mcp/registry/MCPServerInspector.ts b/packages/api/src/mcp/registry/MCPServerInspector.ts index 3ae51d7b36..69aef464c4 100644 --- a/packages/api/src/mcp/registry/MCPServerInspector.ts +++ b/packages/api/src/mcp/registry/MCPServerInspector.ts @@ -2,9 +2,9 @@ import { Constants } from 'librechat-data-provider'; import type { JsonSchemaType } from '@librechat/data-schemas'; import type { MCPConnection } from '~/mcp/connection'; import type * as t from '~/mcp/types'; +import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { detectOAuthRequirement } from '~/mcp/oauth'; import { isEnabled } from '~/utils'; -import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; /** * Inspects MCP servers to discover their metadata, capabilities, and tools. diff --git a/packages/api/src/mcp/registry/MCPServersInitializer.ts b/packages/api/src/mcp/registry/MCPServersInitializer.ts index 5265a11d70..15f6a42f4e 100644 --- a/packages/api/src/mcp/registry/MCPServersInitializer.ts +++ b/packages/api/src/mcp/registry/MCPServersInitializer.ts @@ -5,7 +5,7 @@ import { logger } from '@librechat/data-schemas'; import { ParsedServerConfig } from '~/mcp/types'; import { sanitizeUrlForLogging } from '~/mcp/utils'; import type * as t from '~/mcp/types'; -import { mcpServersRegistry as registry } from './MCPServersRegistry'; +import { MCPServersRegistry } from './MCPServersRegistry'; const MCP_INIT_TIMEOUT_MS = process.env.MCP_INIT_TIMEOUT_MS != null ? parseInt(process.env.MCP_INIT_TIMEOUT_MS) : 30_000; @@ -36,7 +36,7 @@ export class MCPServersInitializer { if (await isLeader()) { // Leader performs initialization await statusCache.reset(); - await registry.reset(); + await MCPServersRegistry.getInstance().reset(); const serverNames = Object.keys(rawConfigs); await Promise.allSettled( serverNames.map((serverName) => @@ -59,7 +59,11 @@ export class MCPServersInitializer { /** Initializes a single server with all its metadata and adds it to appropriate collections */ public static async initializeServer(serverName: string, rawConfig: t.MCPOptions): Promise { try { - const config = await registry.addServer(serverName, rawConfig, 'CACHE'); + const config = await MCPServersRegistry.getInstance().addServer( + serverName, + rawConfig, + 'CACHE', + ); MCPServersInitializer.logParsedConfig(serverName, config); } catch (error) { logger.error(`${MCPServersInitializer.prefix(serverName)} Failed to initialize:`, error); diff --git a/packages/api/src/mcp/registry/MCPServersRegistry.ts b/packages/api/src/mcp/registry/MCPServersRegistry.ts index b99d4423c2..b12afdeeb5 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -1,8 +1,9 @@ +import { logger } from '@librechat/data-schemas'; +import type { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInterface'; import type * as t from '~/mcp/types'; import { ServerConfigsCacheFactory } from './cache/ServerConfigsCacheFactory'; import { MCPServerInspector } from './MCPServerInspector'; import { ServerConfigsDB } from './db/ServerConfigsDB'; -import { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInterface'; /** * Central registry for managing MCP server configurations. @@ -14,15 +15,42 @@ import { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInte * * Query priority: Cache configs are checked first, then DB configs. */ -class MCPServersRegistry { +export class MCPServersRegistry { + private static instance: MCPServersRegistry; + private readonly dbConfigsRepo: IServerConfigsRepositoryInterface; private readonly cacheConfigsRepo: IServerConfigsRepositoryInterface; - constructor() { - this.dbConfigsRepo = new ServerConfigsDB(); + constructor(mongoose: typeof import('mongoose')) { + this.dbConfigsRepo = new ServerConfigsDB(mongoose); this.cacheConfigsRepo = ServerConfigsCacheFactory.create('App', false); } + /** Creates and initializes the singleton MCPServersRegistry instance */ + public static createInstance(mongoose: typeof import('mongoose')): MCPServersRegistry { + if (!mongoose) { + throw new Error( + 'MCPServersRegistry creation failed: mongoose instance is required for database operations. ' + + 'Ensure mongoose is initialized before creating the registry.', + ); + } + if (MCPServersRegistry.instance) { + logger.debug('[MCPServersRegistry] Returning existing instance'); + return MCPServersRegistry.instance; + } + logger.info('[MCPServersRegistry] Creating new instance'); + MCPServersRegistry.instance = new MCPServersRegistry(mongoose); + return MCPServersRegistry.instance; + } + + /** Returns the singleton MCPServersRegistry instance */ + public static getInstance(): MCPServersRegistry { + if (!MCPServersRegistry.instance) { + throw new Error('MCPServersRegistry has not been initialized.'); + } + return MCPServersRegistry.instance; + } + public async getServerConfig( serverName: string, userId?: string, @@ -100,5 +128,3 @@ class MCPServersRegistry { } } } - -export const mcpServersRegistry = new MCPServersRegistry(); diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts index 22a1542671..4119b17ea0 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts @@ -1,5 +1,6 @@ import type * as t from '~/mcp/types'; import type { MCPConnection } from '~/mcp/connection'; +import type { MCPServersRegistry as MCPServersRegistryType } from '../MCPServersRegistry'; // Mock isLeader to always return true to avoid lock contention during parallel operations jest.mock('~/cluster', () => ({ @@ -9,7 +10,8 @@ jest.mock('~/cluster', () => ({ describe('MCPServersInitializer Redis Integration Tests', () => { let MCPServersInitializer: typeof import('../MCPServersInitializer').MCPServersInitializer; - let registry: typeof import('../MCPServersRegistry').mcpServersRegistry; + let MCPServersRegistry: typeof import('../MCPServersRegistry').MCPServersRegistry; + let registry: MCPServersRegistryType; let registryStatusCache: typeof import('../cache/RegistryStatusCache').registryStatusCache; let MCPServerInspector: typeof import('../MCPServerInspector').MCPServerInspector; let MCPConnectionFactory: typeof import('~/mcp/MCPConnectionFactory').MCPConnectionFactory; @@ -124,15 +126,21 @@ describe('MCPServersInitializer Redis Integration Tests', () => { const connectionFactoryModule = await import('~/mcp/MCPConnectionFactory'); const redisClients = await import('~/cache/redisClients'); const leaderElectionModule = await import('~/cluster/LeaderElection'); + const mongoose = await import('mongoose'); MCPServersInitializer = initializerModule.MCPServersInitializer; - registry = registryModule.mcpServersRegistry; + MCPServersRegistry = registryModule.MCPServersRegistry; registryStatusCache = statusCacheModule.registryStatusCache; MCPServerInspector = inspectorModule.MCPServerInspector; MCPConnectionFactory = connectionFactoryModule.MCPConnectionFactory; keyvRedisClient = redisClients.keyvRedisClient; LeaderElection = leaderElectionModule.LeaderElection; + // Reset singleton and create new instance with mongoose + (MCPServersRegistry as unknown as { instance: undefined }).instance = undefined; + MCPServersRegistry.createInstance(mongoose.default); + registry = MCPServersRegistry.getInstance(); + // Ensure Redis is connected if (!keyvRedisClient) throw new Error('Redis client is not initialized'); diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts index 1c259d8da8..22b505df5b 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts @@ -5,11 +5,15 @@ import { MCPServersInitializer } from '~/mcp/registry/MCPServersInitializer'; import { MCPConnection } from '~/mcp/connection'; import { registryStatusCache } from '~/mcp/registry/cache/RegistryStatusCache'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; -import { mcpServersRegistry as registry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; + const FIXED_TIME = 1699564800000; const originalDateNow = Date.now; Date.now = jest.fn(() => FIXED_TIME); +// Mock mongoose for registry initialization +const mockMongoose = {} as typeof import('mongoose'); + // Mock external dependencies jest.mock('../../MCPConnectionFactory'); jest.mock('../../connection'); @@ -26,6 +30,18 @@ jest.mock('@librechat/data-schemas', () => ({ }, })); +// Mock ServerConfigsDB to avoid mongoose dependency +jest.mock('~/mcp/registry/db/ServerConfigsDB', () => ({ + ServerConfigsDB: jest.fn().mockImplementation(() => ({ + get: jest.fn().mockResolvedValue(undefined), + getAll: jest.fn().mockResolvedValue({}), + add: jest.fn().mockResolvedValue(undefined), + update: jest.fn().mockResolvedValue(undefined), + remove: jest.fn().mockResolvedValue(undefined), + reset: jest.fn().mockResolvedValue(undefined), + })), +})); + const mockLogger = logger as jest.Mocked; const mockInspect = MCPServerInspector.inspect as jest.MockedFunction< typeof MCPServerInspector.inspect @@ -33,6 +49,7 @@ const mockInspect = MCPServerInspector.inspect as jest.MockedFunction< describe('MCPServersInitializer', () => { let mockConnection: jest.Mocked; + let registry: MCPServersRegistry; afterAll(() => { Date.now = originalDateNow; @@ -134,6 +151,13 @@ describe('MCPServersInitializer', () => { }; beforeEach(async () => { + // Reset the singleton instance before each test + (MCPServersRegistry as unknown as { instance: undefined }).instance = undefined; + + // Create a new instance for testing + MCPServersRegistry.createInstance(mockMongoose); + registry = MCPServersRegistry.getInstance(); + // Setup MCPConnection mock mockConnection = { disconnect: jest.fn().mockResolvedValue(undefined), diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts index ca5e9f8063..7e0accf687 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts @@ -1,5 +1,7 @@ import { expect } from '@playwright/test'; +import { MongoMemoryServer } from 'mongodb-memory-server'; import type * as t from '~/mcp/types'; +import type { MCPServersRegistry as MCPServersRegistryType } from '../MCPServersRegistry'; /** * Integration tests for MCPServersRegistry using Redis-backed cache. @@ -10,11 +12,13 @@ import type * as t from '~/mcp/types'; * The registry now uses a unified cache repository for YAML configs only. */ describe('MCPServersRegistry Redis Integration Tests', () => { - let registry: typeof import('../MCPServersRegistry').mcpServersRegistry; + let MCPServersRegistry: typeof import('../MCPServersRegistry').MCPServersRegistry; + let registry: MCPServersRegistryType; let keyvRedisClient: Awaited['keyvRedisClient']; let LeaderElection: typeof import('~/cluster/LeaderElection').LeaderElection; let leaderInstance: InstanceType; let MCPServerInspector: typeof import('../MCPServerInspector').MCPServerInspector; + let mongoServer: MongoMemoryServer; const testParsedConfig: t.ParsedServerConfig = { type: 'stdio', @@ -55,12 +59,27 @@ describe('MCPServersRegistry Redis Integration Tests', () => { const redisClients = await import('~/cache/redisClients'); const leaderElectionModule = await import('~/cluster/LeaderElection'); const inspectorModule = await import('../MCPServerInspector'); + const mongoose = await import('mongoose'); + const { userSchema } = await import('@librechat/data-schemas'); - registry = registryModule.mcpServersRegistry; + MCPServersRegistry = registryModule.MCPServersRegistry; keyvRedisClient = redisClients.keyvRedisClient; LeaderElection = leaderElectionModule.LeaderElection; MCPServerInspector = inspectorModule.MCPServerInspector; + // Set up MongoDB with MongoMemoryServer for db methods + mongoServer = await MongoMemoryServer.create(); + const mongoUri = mongoServer.getUri(); + if (!mongoose.default.models.User) { + mongoose.default.model('User', userSchema); + } + await mongoose.default.connect(mongoUri); + + // Reset singleton and create new instance with mongoose + (MCPServersRegistry as unknown as { instance: undefined }).instance = undefined; + MCPServersRegistry.createInstance(mongoose.default); + registry = MCPServersRegistry.getInstance(); + // Ensure Redis is connected if (!keyvRedisClient) throw new Error('Redis client is not initialized'); @@ -76,13 +95,15 @@ describe('MCPServersRegistry Redis Integration Tests', () => { beforeEach(() => { // Mock MCPServerInspector.inspect to avoid actual server connections // Use mockImplementation to return the config that's actually passed in - jest.spyOn(MCPServerInspector, 'inspect').mockImplementation(async (_serverName: string, rawConfig: t.MCPOptions) => { - return { - ...testParsedConfig, - ...rawConfig, - requiresOAuth: false, - } as unknown as t.ParsedServerConfig; - }); + jest + .spyOn(MCPServerInspector, 'inspect') + .mockImplementation(async (_serverName: string, rawConfig: t.MCPOptions) => { + return { + ...testParsedConfig, + ...rawConfig, + requiresOAuth: false, + } as unknown as t.ParsedServerConfig; + }); }); afterEach(async () => { @@ -114,6 +135,11 @@ describe('MCPServersRegistry Redis Integration Tests', () => { // Close Redis connection if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + + // Close MongoDB connection and stop memory server + const mongoose = await import('mongoose'); + await mongoose.default.disconnect(); + if (mongoServer) await mongoServer.stop(); }); // Tests for the old privateServersCache API have been removed diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index 30b9e8984c..54956ec340 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -1,18 +1,36 @@ import * as t from '~/mcp/types'; -import { mcpServersRegistry as registry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; // Mock MCPServerInspector to avoid actual server connections jest.mock('~/mcp/registry/MCPServerInspector'); +// Mock ServerConfigsDB to avoid mongoose dependency +jest.mock('~/mcp/registry/db/ServerConfigsDB', () => ({ + ServerConfigsDB: jest.fn().mockImplementation(() => ({ + get: jest.fn().mockResolvedValue(undefined), + getAll: jest.fn().mockResolvedValue({}), + add: jest.fn().mockResolvedValue(undefined), + update: jest.fn().mockResolvedValue(undefined), + remove: jest.fn().mockResolvedValue(undefined), + reset: jest.fn().mockResolvedValue(undefined), + })), +})); + const FIXED_TIME = 1699564800000; const originalDateNow = Date.now; Date.now = jest.fn(() => FIXED_TIME); + +// Mock mongoose for registry initialization +const mockMongoose = {} as typeof import('mongoose'); + /** * Unit tests for MCPServersRegistry using in-memory cache. * For integration tests using Redis-backed cache, see MCPServersRegistry.cache_integration.spec.ts */ describe('MCPServersRegistry', () => { + let registry: MCPServersRegistry; + const testParsedConfig: t.ParsedServerConfig = { type: 'stdio', command: 'node', @@ -41,6 +59,13 @@ describe('MCPServersRegistry', () => { Date.now = originalDateNow; }); beforeEach(async () => { + // Reset the singleton instance before each test + (MCPServersRegistry as unknown as { instance: undefined }).instance = undefined; + + // Create a new instance for testing + MCPServersRegistry.createInstance(mockMongoose); + registry = MCPServersRegistry.getInstance(); + // Mock MCPServerInspector.inspect to return the config that's passed in jest .spyOn(MCPServerInspector, 'inspect') diff --git a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts index 302a086193..81540e6e87 100644 --- a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts +++ b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ -import { ParsedServerConfig } from '~/mcp/types'; -import { IServerConfigsRepositoryInterface } from '../ServerConfigsRepositoryInterface'; -import { logger } from '@librechat/data-schemas'; +import { AllMethods, createMethods, logger } from '@librechat/data-schemas'; +import type { IServerConfigsRepositoryInterface } from '~/mcp/registry/ServerConfigsRepositoryInterface'; +import type { ParsedServerConfig } from '~/mcp/types'; /** * DB backed config storage @@ -9,6 +9,14 @@ import { logger } from '@librechat/data-schemas'; * Will handle Permission ACL */ export class ServerConfigsDB implements IServerConfigsRepositoryInterface { + private _dbMethods: AllMethods; + constructor(mongoose: typeof import('mongoose')) { + if (!mongoose) { + throw new Error('ServerConfigsDB requires mongoose instance'); + } + this._dbMethods = createMethods(mongoose); + } + public async add(serverName: string, config: ParsedServerConfig, userId?: string): Promise { logger.debug('ServerConfigsDB add not yet implemented'); return; @@ -39,7 +47,8 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface { * @returns record of parsed configs */ public async getAll(userId?: string): Promise> { - logger.debug('ServerConfigsDB getAll not yet implemented'); + // TODO: Implement DB-backed config retrieval + logger.debug('[ServerConfigsDB] getAll not yet implemented', { userId }); return {}; }