mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-01-04 01:28:51 +01:00
🧩 refactor: Decouple MCP Config from Startup Config (#10689)
* Decouple mcp config from start up config * Chore: Work on AI Review and Copilot Comments - setRawConfig is not needed since the private raw config is not needed any more - !!serversLoading bug fixed - added unit tests for route /api/mcp/servers - copilot comments addressed * chore: remove comments * chore: rename data-provider dir for MCP * chore: reorganize mcp specific query hooks * fix: consolidate imports for MCP server manager * chore: add dev-staging branch to frontend review workflow triggers * feat: add GitHub Actions workflow for building and pushing Docker images to GitHub Container Registry and Docker Hub * fix: update label for tag input in BookmarkForm tests to improve clarity --------- Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com> Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
Atef Bellaaj
Danny Avila
parent
cbafe214ee
commit
1b0b27b30c
36 changed files with 548 additions and 301 deletions
|
|
@ -435,8 +435,7 @@ function createToolInstance({ res, toolName, serverName, toolDefinition, provide
|
|||
* @returns {Object} Object containing mcpConfig, appConnections, userConnections, and oauthServers
|
||||
*/
|
||||
async function getMCPSetupData(userId) {
|
||||
const config = await getAppConfig();
|
||||
const mcpConfig = config?.mcpConfig;
|
||||
const mcpConfig = await mcpServersRegistry.getAllServerConfigs(userId);
|
||||
|
||||
if (!mcpConfig) {
|
||||
throw new Error('MCP config not found');
|
||||
|
|
@ -451,7 +450,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();
|
||||
const oauthServers = await mcpServersRegistry.getOAuthServers(userId);
|
||||
|
||||
return {
|
||||
mcpConfig,
|
||||
|
|
@ -524,24 +523,26 @@ async function checkOAuthFlowStatus(userId, serverName) {
|
|||
* Get connection status for a specific MCP server
|
||||
* @param {string} userId - The user ID
|
||||
* @param {string} serverName - The server name
|
||||
* @param {Map} appConnections - App-level connections
|
||||
* @param {Map} userConnections - User-level connections
|
||||
* @param {import('@librechat/api').ParsedServerConfig} config - The server configuration
|
||||
* @param {Map<string, import('@librechat/api').MCPConnection>} appConnections - App-level connections
|
||||
* @param {Map<string, import('@librechat/api').MCPConnection>} userConnections - User-level connections
|
||||
* @param {Set} oauthServers - Set of OAuth servers
|
||||
* @returns {Object} Object containing requiresOAuth and connectionState
|
||||
*/
|
||||
async function getServerConnectionStatus(
|
||||
userId,
|
||||
serverName,
|
||||
config,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
) {
|
||||
const getConnectionState = () =>
|
||||
appConnections.get(serverName)?.connectionState ??
|
||||
userConnections.get(serverName)?.connectionState ??
|
||||
'disconnected';
|
||||
const connection = appConnections.get(serverName) || userConnections.get(serverName);
|
||||
const isStaleOrDoNotExist = connection ? connection?.isStale(config.lastUpdatedAt) : true;
|
||||
|
||||
const baseConnectionState = getConnectionState();
|
||||
const baseConnectionState = isStaleOrDoNotExist
|
||||
? 'disconnected'
|
||||
: connection?.connectionState || 'disconnected';
|
||||
let finalConnectionState = baseConnectionState;
|
||||
|
||||
// connection state overrides specific to OAuth servers
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ jest.mock('@librechat/api', () => ({
|
|||
convertWithResolvedRefs: jest.fn((params) => params),
|
||||
mcpServersRegistry: {
|
||||
getOAuthServers: jest.fn(() => Promise.resolve(new Set())),
|
||||
getAllServerConfigs: jest.fn(() => Promise.resolve({})),
|
||||
},
|
||||
}));
|
||||
|
||||
|
|
@ -118,31 +119,28 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
describe('getMCPSetupData', () => {
|
||||
const mockUserId = 'user-123';
|
||||
const mockConfig = {
|
||||
mcpServers: {
|
||||
server1: { type: 'stdio' },
|
||||
server2: { type: 'http' },
|
||||
},
|
||||
server1: { type: 'stdio' },
|
||||
server2: { type: 'http' },
|
||||
};
|
||||
let mockGetAppConfig;
|
||||
|
||||
beforeEach(() => {
|
||||
mockGetAppConfig = require('./Config').getAppConfig;
|
||||
mockGetMCPManager.mockReturnValue({
|
||||
appConnections: { getAll: jest.fn(() => new Map()) },
|
||||
getUserConnections: jest.fn(() => new Map()),
|
||||
});
|
||||
mockMcpServersRegistry.getOAuthServers.mockResolvedValue(new Set());
|
||||
mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockConfig);
|
||||
});
|
||||
|
||||
it('should successfully return MCP setup data', async () => {
|
||||
mockGetAppConfig.mockResolvedValue({ mcpConfig: mockConfig.mcpServers });
|
||||
mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockConfig);
|
||||
|
||||
const mockAppConnections = new Map([['server1', { status: 'connected' }]]);
|
||||
const mockUserConnections = new Map([['server2', { status: 'disconnected' }]]);
|
||||
const mockOAuthServers = new Set(['server2']);
|
||||
|
||||
const mockMCPManager = {
|
||||
appConnections: { getAll: jest.fn(() => mockAppConnections) },
|
||||
appConnections: { getAll: jest.fn(() => Promise.resolve(mockAppConnections)) },
|
||||
getUserConnections: jest.fn(() => mockUserConnections),
|
||||
};
|
||||
mockGetMCPManager.mockReturnValue(mockMCPManager);
|
||||
|
|
@ -150,14 +148,14 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
|
||||
const result = await getMCPSetupData(mockUserId);
|
||||
|
||||
expect(mockGetAppConfig).toHaveBeenCalled();
|
||||
expect(mockMcpServersRegistry.getAllServerConfigs).toHaveBeenCalledWith(mockUserId);
|
||||
expect(mockGetMCPManager).toHaveBeenCalledWith(mockUserId);
|
||||
expect(mockMCPManager.appConnections.getAll).toHaveBeenCalled();
|
||||
expect(mockMCPManager.getUserConnections).toHaveBeenCalledWith(mockUserId);
|
||||
expect(mockMcpServersRegistry.getOAuthServers).toHaveBeenCalled();
|
||||
expect(mockMcpServersRegistry.getOAuthServers).toHaveBeenCalledWith(mockUserId);
|
||||
|
||||
expect(result).toEqual({
|
||||
mcpConfig: mockConfig.mcpServers,
|
||||
mcpConfig: mockConfig,
|
||||
appConnections: mockAppConnections,
|
||||
userConnections: mockUserConnections,
|
||||
oauthServers: mockOAuthServers,
|
||||
|
|
@ -165,15 +163,15 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
});
|
||||
|
||||
it('should throw error when MCP config not found', async () => {
|
||||
mockGetAppConfig.mockResolvedValue({});
|
||||
mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(null);
|
||||
await expect(getMCPSetupData(mockUserId)).rejects.toThrow('MCP config not found');
|
||||
});
|
||||
|
||||
it('should handle null values from MCP manager gracefully', async () => {
|
||||
mockGetAppConfig.mockResolvedValue({ mcpConfig: mockConfig.mcpServers });
|
||||
mockMcpServersRegistry.getAllServerConfigs.mockResolvedValue(mockConfig);
|
||||
|
||||
const mockMCPManager = {
|
||||
appConnections: { getAll: jest.fn(() => null) },
|
||||
appConnections: { getAll: jest.fn(() => Promise.resolve(null)) },
|
||||
getUserConnections: jest.fn(() => null),
|
||||
};
|
||||
mockGetMCPManager.mockReturnValue(mockMCPManager);
|
||||
|
|
@ -182,7 +180,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getMCPSetupData(mockUserId);
|
||||
|
||||
expect(result).toEqual({
|
||||
mcpConfig: mockConfig.mcpServers,
|
||||
mcpConfig: mockConfig,
|
||||
appConnections: new Map(),
|
||||
userConnections: new Map(),
|
||||
oauthServers: new Set(),
|
||||
|
|
@ -329,15 +327,25 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
describe('getServerConnectionStatus', () => {
|
||||
const mockUserId = 'user-123';
|
||||
const mockServerName = 'test-server';
|
||||
const mockConfig = { lastUpdatedAt: Date.now() };
|
||||
|
||||
it('should return app connection state when available', async () => {
|
||||
const appConnections = new Map([[mockServerName, { connectionState: 'connected' }]]);
|
||||
const appConnections = new Map([
|
||||
[
|
||||
mockServerName,
|
||||
{
|
||||
connectionState: 'connected',
|
||||
isStale: jest.fn(() => false),
|
||||
},
|
||||
],
|
||||
]);
|
||||
const userConnections = new Map();
|
||||
const oauthServers = new Set();
|
||||
|
||||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -351,12 +359,21 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
|
||||
it('should fallback to user connection state when app connection not available', async () => {
|
||||
const appConnections = new Map();
|
||||
const userConnections = new Map([[mockServerName, { connectionState: 'connecting' }]]);
|
||||
const userConnections = new Map([
|
||||
[
|
||||
mockServerName,
|
||||
{
|
||||
connectionState: 'connecting',
|
||||
isStale: jest.fn(() => false),
|
||||
},
|
||||
],
|
||||
]);
|
||||
const oauthServers = new Set();
|
||||
|
||||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -376,6 +393,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -388,13 +406,30 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
});
|
||||
|
||||
it('should prioritize app connection over user connection', async () => {
|
||||
const appConnections = new Map([[mockServerName, { connectionState: 'connected' }]]);
|
||||
const userConnections = new Map([[mockServerName, { connectionState: 'disconnected' }]]);
|
||||
const appConnections = new Map([
|
||||
[
|
||||
mockServerName,
|
||||
{
|
||||
connectionState: 'connected',
|
||||
isStale: jest.fn(() => false),
|
||||
},
|
||||
],
|
||||
]);
|
||||
const userConnections = new Map([
|
||||
[
|
||||
mockServerName,
|
||||
{
|
||||
connectionState: 'disconnected',
|
||||
isStale: jest.fn(() => false),
|
||||
},
|
||||
],
|
||||
]);
|
||||
const oauthServers = new Set();
|
||||
|
||||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -420,6 +455,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -454,6 +490,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -491,6 +528,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -524,6 +562,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -549,6 +588,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -571,13 +611,22 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
mockGetFlowStateManager.mockReturnValue(mockFlowManager);
|
||||
mockGetLogStores.mockReturnValue({});
|
||||
|
||||
const appConnections = new Map([[mockServerName, { connectionState: 'connected' }]]);
|
||||
const appConnections = new Map([
|
||||
[
|
||||
mockServerName,
|
||||
{
|
||||
connectionState: 'connected',
|
||||
isStale: jest.fn(() => false),
|
||||
},
|
||||
],
|
||||
]);
|
||||
const userConnections = new Map();
|
||||
const oauthServers = new Set([mockServerName]);
|
||||
|
||||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
@ -606,6 +655,7 @@ describe('tests for the new helper functions used by the MCP connection status e
|
|||
const result = await getServerConnectionStatus(
|
||||
mockUserId,
|
||||
mockServerName,
|
||||
mockConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
oauthServers,
|
||||
|
|
|
|||
|
|
@ -427,6 +427,7 @@ async function loadAgentTools({ req, res, agent, signal, tool_resources, openAIA
|
|||
|
||||
/** @type {Record<string, Record<string, string>>} */
|
||||
let userMCPAuthMap;
|
||||
//TODO pass config from registry
|
||||
if (hasCustomUserVars(req.config)) {
|
||||
userMCPAuthMap = await getUserMCPAuthMap({
|
||||
tools: agent.tools,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue