mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-01-22 10:16:13 +01:00
🧬 refactor: Wire Database Methods into MCP Package via Registry Pattern (#10715)
* Refactor: MCPServersRegistry Singleton Pattern with Dependency Injection for DB methods consumption * refactor: error handling in MCP initialization and improve logging for MCPServersRegistry instance creation. - Added checks for mongoose instance in ServerConfigsDB constructor and refined error messages for clarity. - Reorder and use type imports --------- Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com> Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
6fc762b73b
commit
d9fbe55c2b
24 changed files with 328 additions and 150 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue