diff --git a/api/server/controllers/mcp.js b/api/server/controllers/mcp.js index 19ea8d90b5..1d1f6eed7c 100644 --- a/api/server/controllers/mcp.js +++ b/api/server/controllers/mcp.js @@ -6,10 +6,11 @@ * @import { MCPServerDocument } from 'librechat-data-provider' */ const { randomUUID } = require('crypto'); -const { logger, SystemCapabilities } = require('@librechat/data-schemas'); +const { logger, getTenantId, SystemCapabilities } = require('@librechat/data-schemas'); const { checkAccess, isUserSourced, + createAuthIdentityContext, MCPConnection, MCPErrorCodes, splitMCPToolKey, @@ -35,6 +36,8 @@ const { resolveMcpConfigNames, resolveAllMcpConfigs, } = require('~/server/services/MCP'); +const { loadMCPServerCatalogs } = require('~/server/services/Tools/mcp'); +const { createOpenIDSessionTokenProvider } = require('~/server/services/OpenIDSessionRefresh'); const { cacheMCPServerTools, getMCPServerTools, @@ -193,65 +196,26 @@ const getMCPTools = async (req, res) => { return res.status(200).json({ servers: {} }); } - const mcpManager = getMCPManager(); const mcpServers = {}; - - const serverToolsMap = new Map(); - const serversWithoutTools = []; - const cacheResults = await Promise.all( - configuredServers.map(async (serverName) => { - try { - return { - serverName, - tools: await getMCPServerTools(userId, serverName, mcpConfig[serverName]), - }; - } catch (error) { - logger.error(`[getMCPTools] Error fetching cached tools for ${serverName}:`, error); - return { serverName, tools: null }; - } - }), - ); - for (const { serverName, tools } of cacheResults) { - if (tools) { - serverToolsMap.set(serverName, tools); - continue; - } - - let serverTools; - let publicationGeneration; - let publicationRevision; - try { - ({ - tools: serverTools, - publicationGeneration, - publicationRevision, - } = await mcpManager.getServerToolFunctionsSnapshot( - userId, - serverName, - mcpConfig[serverName], - )); - } catch (error) { - logger.error(`[getMCPTools] Error fetching tools for server ${serverName}:`, error); - continue; - } - if (!serverTools) { - serversWithoutTools.push(serverName); - continue; - } - serverToolsMap.set(serverName, serverTools); - - // Empty is an authoritative catalog too; re-cache it after TTL expiry to avoid polling. - cacheMCPServerTools({ - userId, + const oboIdentityContext = createAuthIdentityContext({ + user: req.user, + tenantId: getTenantId(), + }); + const { serverTools: serverToolsMap, serversWithoutTools } = await loadMCPServerCatalogs({ + user: req.user, + servers: configuredServers.map((serverName) => ({ serverName, - serverTools, serverConfig: mcpConfig[serverName], - publicationGeneration, - publicationRevision, - }).catch((err) => - logger.error(`[getMCPTools] Failed to cache tools for ${serverName}:`, err), - ); - } + })), + upstreamTokenProvider: createOpenIDSessionTokenProvider({ + req, + res, + user: req.user, + identityContext: oboIdentityContext, + tokenPreference: 'access_token', + }), + oboIdentityContext, + }); if (serversWithoutTools.length > 0) { logger.debug( `[getMCPTools] No tools (${serversWithoutTools.length}): ${serversWithoutTools.join(', ')}`, diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 17b6d4e678..fe5ca60df5 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -33,6 +33,10 @@ const mockRegistryInstance = { }), }; let mockMCPUseAllowed = true; +const mockLoadMCPServerCatalogs = jest.fn().mockResolvedValue({ + serverTools: new Map(), + serversWithoutTools: [], +}); jest.mock('@librechat/api', () => { const actual = jest.requireActual('@librechat/api'); @@ -174,6 +178,7 @@ jest.mock('~/server/middleware', () => ({ jest.mock('~/server/services/Tools/mcp', () => ({ reinitMCPServer: jest.fn(), + loadMCPServerCatalogs: (...args) => mockLoadMCPServerCatalogs(...args), })); const mockOAuthCompletion = (tokens) => { @@ -225,6 +230,10 @@ describe('MCP Routes', () => { beforeEach(() => { jest.clearAllMocks(); + mockLoadMCPServerCatalogs.mockResolvedValue({ + serverTools: new Map(), + serversWithoutTools: [], + }); currentUser = undefined; mockResolveAllMcpConfigs.mockResolvedValue({}); mockResolveMcpConfigNames.mockResolvedValue([]); @@ -3284,9 +3293,8 @@ describe('MCP Routes', () => { expect(mockResolveAllMcpConfigs).not.toHaveBeenCalled(); }); - it('caches a live user snapshot with its connection-bound publication generation', async () => { + it('delegates catalog loading and renders the returned tools', async () => { const { Constants } = require('librechat-data-provider'); - const { cacheMCPServerTools, getMCPServerTools } = require('~/server/services/Config'); const pluginKey = `search${Constants.mcp_delimiter}user-server`; const serverTools = { [pluginKey]: { @@ -3300,58 +3308,80 @@ describe('MCP Routes', () => { }; const serverConfig = { type: 'sse', url: 'https://user.example.com/sse' }; mockResolveAllMcpConfigs.mockResolvedValueOnce({ 'user-server': serverConfig }); - getMCPServerTools.mockResolvedValueOnce(null); - cacheMCPServerTools.mockResolvedValueOnce(); - require('~/config').getMCPManager.mockReturnValue({ - getServerToolFunctionsSnapshot: jest.fn().mockResolvedValue({ - tools: serverTools, - publicationGeneration: 'connection-generation', - }), + mockLoadMCPServerCatalogs.mockResolvedValueOnce({ + serverTools: new Map([['user-server', serverTools]]), + serversWithoutTools: [], }); const response = await request(app).get('/api/mcp/tools'); expect(response.status).toBe(200); - expect(cacheMCPServerTools).toHaveBeenCalledWith({ - userId: 'test-user-id', - serverName: 'user-server', - serverTools, - serverConfig, - publicationGeneration: 'connection-generation', + expect(response.body.servers['user-server'].tools).toEqual([ + { name: 'search', pluginKey, description: 'Search' }, + ]); + expect(mockLoadMCPServerCatalogs).toHaveBeenCalledWith({ + user: { id: 'test-user-id' }, + servers: [{ serverName: 'user-server', serverConfig }], + upstreamTokenProvider: expect.any(Function), + oboIdentityContext: expect.any(Object), }); }); - it('re-caches an authoritative empty live snapshot after a cache miss', async () => { - const { cacheMCPServerTools, getMCPServerTools } = require('~/server/services/Config'); + it('renders an authoritative empty catalog as a configured server', async () => { const serverConfig = { type: 'sse', url: 'https://empty.example.com/sse' }; mockResolveAllMcpConfigs.mockResolvedValueOnce({ 'empty-server': serverConfig }); - getMCPServerTools.mockResolvedValueOnce(null); - cacheMCPServerTools.mockResolvedValueOnce(); - const getServerToolFunctionsSnapshot = jest.fn().mockResolvedValue({ - tools: {}, - publicationGeneration: undefined, + mockLoadMCPServerCatalogs.mockResolvedValueOnce({ + serverTools: new Map([['empty-server', {}]]), + serversWithoutTools: [], }); - require('~/config').getMCPManager.mockReturnValue({ getServerToolFunctionsSnapshot }); const response = await request(app).get('/api/mcp/tools'); expect(response.status).toBe(200); expect(response.body.servers['empty-server'].tools).toEqual([]); - expect(cacheMCPServerTools).toHaveBeenCalledWith({ - userId: 'test-user-id', - serverName: 'empty-server', - serverTools: {}, - serverConfig, - publicationGeneration: undefined, + }); + + it('renders a passively recovered catalog returned by the loader', async () => { + const { Constants } = require('librechat-data-provider'); + const pluginKey = `search${Constants.mcp_delimiter}connected-server`; + const serverTools = { + [pluginKey]: { + type: 'function', + function: { + name: pluginKey, + description: 'Search', + parameters: { type: 'object' }, + }, + }, + }; + const serverConfig = { type: 'sse', url: 'https://connected.example.com/sse' }; + mockResolveAllMcpConfigs.mockResolvedValueOnce({ 'connected-server': serverConfig }); + mockLoadMCPServerCatalogs.mockResolvedValueOnce({ + serverTools: new Map([['connected-server', serverTools]]), + serversWithoutTools: [], + }); + + const response = await request(app).get('/api/mcp/tools'); + + expect(response.status).toBe(200); + expect(response.body.servers['connected-server'].tools).toEqual([ + { + name: 'search', + pluginKey, + description: 'Search', + }, + ]); + expect(mockLoadMCPServerCatalogs).toHaveBeenCalledWith({ + user: { id: 'test-user-id' }, + servers: [{ serverName: 'connected-server', serverConfig }], + upstreamTokenProvider: expect.any(Function), + oboIdentityContext: expect.any(Object), }); }); - it('should continue returning MCP tools when one server cache lookup fails', async () => { + it('continues rendering available tools when another server has no catalog', async () => { const { Constants } = require('librechat-data-provider'); - const { logger } = require('@librechat/data-schemas'); - const { getMCPServerTools } = require('~/server/services/Config'); - - mockResolveAllMcpConfigs.mockResolvedValueOnce({ + const configs = { 'bad-server': { type: 'sse', url: 'https://bad.example.com/sse', @@ -3361,45 +3391,30 @@ describe('MCP Routes', () => { url: 'https://good.example.com/sse', iconPath: '/icons/good.svg', }, - }); - - // Mock order matches Object.keys() order from the config above. - getMCPServerTools - .mockRejectedValueOnce(new Error('cache unavailable')) - .mockResolvedValueOnce({ - [`search${Constants.mcp_delimiter}good-server`]: { - type: 'function', - function: { - name: `search${Constants.mcp_delimiter}good-server`, - description: 'Search good server', - parameters: { type: 'object' }, + }; + mockResolveAllMcpConfigs.mockResolvedValueOnce(configs); + mockLoadMCPServerCatalogs.mockResolvedValueOnce({ + serverTools: new Map([ + [ + 'good-server', + { + [`search${Constants.mcp_delimiter}good-server`]: { + type: 'function', + function: { + name: `search${Constants.mcp_delimiter}good-server`, + description: 'Search good server', + parameters: { type: 'object' }, + }, + }, }, - }, - }); - - const mockGetServerToolFunctionsSnapshot = jest.fn().mockResolvedValue({ - tools: null, - publicationGeneration: 'test-generation', - }); - require('~/config').getMCPManager.mockReturnValue({ - getServerToolFunctionsSnapshot: mockGetServerToolFunctionsSnapshot, + ], + ]), + serversWithoutTools: ['bad-server'], }); const response = await request(app).get('/api/mcp/tools'); expect(response.status).toBe(200); - expect(logger.error).toHaveBeenCalledWith( - '[getMCPTools] Error fetching cached tools for bad-server:', - expect.any(Error), - ); - expect(mockGetServerToolFunctionsSnapshot).toHaveBeenCalledWith( - 'test-user-id', - 'bad-server', - { - type: 'sse', - url: 'https://bad.example.com/sse', - }, - ); expect(response.body.servers['good-server']).toMatchObject({ name: 'good-server', icon: '/icons/good.svg', @@ -3419,9 +3434,7 @@ describe('MCP Routes', () => { it('should return configured servers when all cache lookups fail', async () => { const { logger } = require('@librechat/data-schemas'); - const { getMCPServerTools } = require('~/server/services/Config'); - - mockResolveAllMcpConfigs.mockResolvedValueOnce({ + const configs = { 'first-server': { type: 'sse', url: 'https://first.example.com/sse', @@ -3430,16 +3443,11 @@ describe('MCP Routes', () => { type: 'sse', url: 'https://second.example.com/sse', }, - }); - - getMCPServerTools.mockRejectedValue(new Error('cache unavailable')); - - const mockGetServerToolFunctionsSnapshot = jest.fn().mockResolvedValue({ - tools: null, - publicationGeneration: 'test-generation', - }); - require('~/config').getMCPManager.mockReturnValue({ - getServerToolFunctionsSnapshot: mockGetServerToolFunctionsSnapshot, + }; + mockResolveAllMcpConfigs.mockResolvedValueOnce(configs); + mockLoadMCPServerCatalogs.mockResolvedValueOnce({ + serverTools: new Map(), + serversWithoutTools: ['first-server', 'second-server'], }); const response = await request(app).get('/api/mcp/tools'); @@ -3457,8 +3465,16 @@ describe('MCP Routes', () => { expect(logger.debug).toHaveBeenCalledWith( '[getMCPTools] No tools (2): first-server, second-server', ); - expect(logger.error).toHaveBeenCalledTimes(2); - expect(mockGetServerToolFunctionsSnapshot).toHaveBeenCalledTimes(2); + expect(mockLoadMCPServerCatalogs).toHaveBeenCalledTimes(1); + expect(mockLoadMCPServerCatalogs).toHaveBeenCalledWith({ + user: { id: 'test-user-id' }, + servers: Object.entries(configs).map(([serverName, serverConfig]) => ({ + serverName, + serverConfig, + })), + upstreamTokenProvider: expect.any(Function), + oboIdentityContext: expect.any(Object), + }); }); }); diff --git a/api/server/services/Tools/mcp.js b/api/server/services/Tools/mcp.js index c74b80ce10..b1eb82f303 100644 --- a/api/server/services/Tools/mcp.js +++ b/api/server/services/Tools/mcp.js @@ -1,16 +1,30 @@ const { logger } = require('@librechat/data-schemas'); const { + formatMCPServerTools, + getUserMCPAuthMap, getMissingCustomUserVars, + loadMCPServerCatalogs: loadCatalogs, requiresEphemeralUserConnection, getMissingRuntimeBodyPlaceholderFields, } = require('@librechat/api'); const { CacheKeys, Constants } = require('librechat-data-provider'); const { getMCPManager, getMCPServersRegistry, getFlowStateManager } = require('~/config'); -const { findToken, createToken, updateToken, deleteTokens } = require('~/models'); +const { + findToken, + createToken, + updateToken, + deleteTokens, + findPluginAuthsByKeys, +} = require('~/models'); const { getGraphApiToken } = require('~/server/services/GraphTokenService'); const { exchangeOboToken } = require('~/server/services/OboTokenService'); const { createOboTrustChecker } = require('~/server/services/OboPolicyService'); -const { getMCPToolsCacheGeneration, updateMCPServerTools } = require('~/server/services/Config'); +const { + getMCPServerTools, + cacheMCPServerTools, + getMCPToolsCacheGeneration, + updateMCPServerTools, +} = require('~/server/services/Config'); const { getLogStores } = require('~/cache'); const MCP_REINITIALIZE_FAILURE_REASONS = { @@ -20,6 +34,46 @@ const MCP_REINITIALIZE_FAILURE_REASONS = { INITIALIZATION_FAILED: 'initialization_failed', }; +/** Wires application dependencies into the passive, request-local catalog recovery service. + * @param {Object} params + * @param {IUser} params.user + * @param {Array<{ serverName: string, serverConfig: object }>} params.servers + * @param {import('@librechat/api').UpstreamTokenProvider} [params.upstreamTokenProvider] - Live upstream-token closure for OBO discovery, built at the request boundary so this layer never receives the raw Express request. + * @param {import('@librechat/api').AuthIdentityContext} [params.oboIdentityContext] - Non-template-visible OBO identity context built from the real request user. + */ +async function loadMCPServerCatalogs({ user, servers, upstreamTokenProvider, oboIdentityContext }) { + const flowManager = getFlowStateManager(getLogStores(CacheKeys.FLOWS)); + const tokenMethods = { findToken, updateToken, createToken, deleteTokens }; + const mcpManager = getMCPManager(); + return loadCatalogs( + { user, servers }, + { + loadUserMCPAuthMap: (userId, serverNames) => + getUserMCPAuthMap({ + userId, + servers: serverNames, + findPluginAuthsByKeys, + }), + discoverServerTools: (options) => + mcpManager.discoverServerTools({ + ...options, + flowManager, + tokenMethods, + graphTokenResolver: getGraphApiToken, + oboTokenResolver: exchangeOboToken, + oboTrustChecker: createOboTrustChecker(), + upstreamTokenProvider, + oboIdentityContext, + }), + formatServerTools: formatMCPServerTools, + getCachedServerTools: getMCPServerTools, + getServerToolFunctionsSnapshot: (userId, serverName, serverConfig) => + mcpManager.getServerToolFunctionsSnapshot(userId, serverName, serverConfig), + cacheServerTools: cacheMCPServerTools, + }, + ); +} + /** * Reinitializes an MCP server connection and discovers available tools. * When OAuth is required, uses discovery mode to list tools without full authentication @@ -383,4 +437,5 @@ async function reinitMCPServer({ module.exports = { reinitMCPServer, + loadMCPServerCatalogs, }; diff --git a/api/server/services/Tools/mcp.spec.js b/api/server/services/Tools/mcp.spec.js index d5bdeff5f9..8eb8bac327 100644 --- a/api/server/services/Tools/mcp.spec.js +++ b/api/server/services/Tools/mcp.spec.js @@ -7,11 +7,25 @@ const mockGetGraphApiToken = jest.fn(); const mockUpdateMCPServerTools = jest.fn(); const mockGetMCPToolsCacheGeneration = jest.fn().mockResolvedValue('generation-current'); const mockGetToolPublicationGeneration = jest.fn().mockReturnValue('generation-current'); +const mockLoadCatalogs = jest.fn(); +const mockGetUserMCPAuthMap = jest.fn(); +const mockFormatMCPServerTools = jest.fn(); +const mockGetMCPServerTools = jest.fn(); +const mockCacheMCPServerTools = jest.fn(); +const mockGetServerToolFunctionsSnapshot = jest.fn(); + +jest.mock('@librechat/api', () => ({ + ...jest.requireActual('@librechat/api'), + loadMCPServerCatalogs: (...args) => mockLoadCatalogs(...args), + getUserMCPAuthMap: (...args) => mockGetUserMCPAuthMap(...args), + formatMCPServerTools: (...args) => mockFormatMCPServerTools(...args), +})); jest.mock('~/config', () => ({ getMCPManager: jest.fn(() => ({ getConnection: mockGetConnection, discoverServerTools: mockDiscoverServerTools, + getServerToolFunctionsSnapshot: mockGetServerToolFunctionsSnapshot, getToolPublicationGeneration: mockGetToolPublicationGeneration, })), getMCPServersRegistry: jest.fn(() => ({ getServerConfig: jest.fn() })), @@ -22,10 +36,13 @@ jest.mock('~/models', () => ({ createToken: jest.fn(), updateToken: jest.fn(), deleteTokens: jest.fn(), + findPluginAuthsByKeys: jest.fn(), })); jest.mock('~/server/services/Config', () => ({ updateMCPServerTools: mockUpdateMCPServerTools, getMCPToolsCacheGeneration: mockGetMCPToolsCacheGeneration, + getMCPServerTools: mockGetMCPServerTools, + cacheMCPServerTools: mockCacheMCPServerTools, })); jest.mock('~/server/services/GraphTokenService', () => ({ getGraphApiToken: mockGetGraphApiToken, @@ -34,7 +51,89 @@ jest.mock('~/cache', () => ({ getLogStores: jest.fn(() => ({})), })); -const { reinitMCPServer } = require('./mcp'); +const { reinitMCPServer, loadMCPServerCatalogs } = require('./mcp'); + +describe('loadMCPServerCatalogs', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('wires batched auth and passive discovery without opening a managed connection', async () => { + const user = { id: 'user-123' }; + const servers = [ + { + serverName: 'config-only', + serverConfig: { type: 'sse', url: 'https://config.example.com/sse' }, + }, + { + serverName: 'user-server', + serverConfig: { type: 'sse', url: 'https://user.example.com/sse' }, + }, + ]; + mockGetUserMCPAuthMap.mockResolvedValue({}); + mockDiscoverServerTools.mockResolvedValue({ tools: [] }); + mockFormatMCPServerTools.mockReturnValue({}); + mockLoadCatalogs.mockImplementation(async (params, deps) => { + await deps.loadUserMCPAuthMap( + user.id, + servers.map(({ serverName }) => serverName), + ); + await deps.discoverServerTools({ + user, + serverName: 'config-only', + configServers: { 'config-only': servers[0].serverConfig }, + }); + deps.formatServerTools('config-only', []); + await deps.getCachedServerTools(user.id, 'config-only', servers[0].serverConfig); + await deps.getServerToolFunctionsSnapshot(user.id, 'config-only', servers[0].serverConfig); + await deps.cacheServerTools({ serverName: 'config-only' }); + return { serverTools: new Map([['config-only', {}]]), serversWithoutTools: [] }; + }); + + const upstreamTokenProvider = jest.fn(); + const oboIdentityContext = { appUserId: 'user-123' }; + const result = await loadMCPServerCatalogs({ + user, + servers, + upstreamTokenProvider, + oboIdentityContext, + }); + + expect(mockGetUserMCPAuthMap).toHaveBeenCalledTimes(1); + expect(mockGetUserMCPAuthMap).toHaveBeenCalledWith({ + userId: user.id, + servers: ['config-only', 'user-server'], + findPluginAuthsByKeys: require('~/models').findPluginAuthsByKeys, + }); + expect(mockDiscoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ + user, + serverName: 'config-only', + configServers: { 'config-only': servers[0].serverConfig }, + flowManager: expect.any(Object), + tokenMethods: expect.any(Object), + upstreamTokenProvider, + oboIdentityContext, + }), + ); + expect(mockGetConnection).not.toHaveBeenCalled(); + expect(mockGetMCPServerTools).toHaveBeenCalledWith( + user.id, + 'config-only', + servers[0].serverConfig, + ); + expect(mockGetServerToolFunctionsSnapshot).toHaveBeenCalledWith( + user.id, + 'config-only', + servers[0].serverConfig, + ); + expect(mockCacheMCPServerTools).toHaveBeenCalledWith({ serverName: 'config-only' }); + expect(result).toEqual({ + serverTools: new Map([['config-only', {}]]), + serversWithoutTools: [], + }); + }); +}); describe('reinitMCPServer — customUserVars gating (issue #10969)', () => { const user = { id: 'user-123' }; diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index d6beb61e67..ddd8cd516c 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -23,6 +23,7 @@ export * from './mcp/errors'; export * from './mcp/cache'; export * from './mcp/tools'; export * from './mcp/catalog/store'; +export * from './mcp/catalog/recovery'; export * from './mcp/assistants'; export * from './mcp/request'; /* Utilities */ diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index 5a465d44be..8d658bdecc 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -264,9 +264,21 @@ export class MCPManager extends UserConnectionManager { */ public async discoverServerTools(args: t.ToolDiscoveryOptions): Promise { const { serverName, user } = args; + const registry = MCPServersRegistry.getInstance(); + const serverConfig = await registry.getServerConfig(serverName, user?.id, args.configServers); + + if (!serverConfig) { + logger.warn('[MCP][Discovery] Server configuration not found'); + return { tools: null, oauthRequired: false, oauthUrl: null }; + } try { - const existingAppConnection = await this.appConnections?.get(serverName); + const useAppConnection = + canUseAppConnection(serverConfig) && + (await registry.isAppServerConfig(serverName, serverConfig)); + const existingAppConnection = useAppConnection + ? await this.appConnections?.get(serverName) + : null; if (existingAppConnection && (await existingAppConnection.isConnected())) { const snapshot = await existingAppConnection.fetchOrderedToolsSnapshot(); return { @@ -279,17 +291,6 @@ export class MCPManager extends UserConnectionManager { logger.debug('[MCP][Discovery] App connection unavailable; trying discovery mode'); } - const serverConfig = await MCPServersRegistry.getInstance().getServerConfig( - serverName, - user?.id, - args.configServers, - ); - - if (!serverConfig) { - logger.warn('[MCP][Discovery] Server configuration not found'); - return { tools: null, oauthRequired: false, oauthUrl: null }; - } - const missingBodyFields = getMissingRuntimeBodyPlaceholderFields( serverConfig, args.requestBody, @@ -301,7 +302,6 @@ export class MCPManager extends UserConnectionManager { return { tools: null, oauthRequired: false, oauthUrl: null }; } - const registry = MCPServersRegistry.getInstance(); const { allowedDomains, allowedAddresses, useSSRFProtection } = await registry.resolveAllowlists({ userId: user?.id, role: user?.role }); await this.assertResolvedRuntimeConfigAllowed({ diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index 64e1dc7131..2caa1ab518 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -2957,6 +2957,41 @@ describe('MCPManager', () => { expect(MCPConnectionFactory.discoverTools).not.toHaveBeenCalled(); }); + it('does not reuse an app connection for a tenant-scoped config override', async () => { + const configOverride = { + type: 'streamable-http' as const, + url: 'https://tenant.example.com/mcp', + source: 'config' as const, + }; + const appConnections = { get: jest.fn().mockResolvedValue(mockConnection) }; + mockAppConnections(appConnections); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(configOverride); + (mockRegistryInstance.isAppServerConfig as jest.Mock).mockResolvedValue(false); + (MCPConnectionFactory.discoverTools as jest.Mock).mockResolvedValue({ + tools: mockTools, + connection: null, + oauthRequired: false, + oauthUrl: null, + }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const result = await manager.discoverServerTools({ + serverName, + user: { id: 'tenant-user' } as IUser, + configServers: { [serverName]: configOverride }, + }); + + expect(result.tools).toEqual(mockTools); + expect(mockRegistryInstance.getServerConfig).toHaveBeenCalledWith(serverName, 'tenant-user', { + [serverName]: configOverride, + }); + expect(appConnections.get).not.toHaveBeenCalled(); + expect(MCPConnectionFactory.discoverTools).toHaveBeenCalledWith( + expect.objectContaining({ serverConfig: configOverride }), + expect.objectContaining({ user: expect.objectContaining({ id: 'tenant-user' }) }), + ); + }); + it('should use MCPConnectionFactory.discoverTools when no app connection available', async () => { const discoveryConnection = { disconnect: jest.fn().mockResolvedValue(undefined), diff --git a/packages/api/src/mcp/catalog/recovery.spec.ts b/packages/api/src/mcp/catalog/recovery.spec.ts new file mode 100644 index 0000000000..255b606e51 --- /dev/null +++ b/packages/api/src/mcp/catalog/recovery.spec.ts @@ -0,0 +1,484 @@ +import { Constants } from 'librechat-data-provider'; +import type { IUser } from '@librechat/data-schemas'; +import type { LCAvailableTools, ParsedServerConfig, ToolDiscoveryOptions } from '../types'; +import { loadMCPServerCatalogs, recoverMCPServerCatalogs } from './recovery'; + +const user = { id: 'user-1' } as IUser; +const serverConfig = (name: string): ParsedServerConfig => + ({ type: 'streamable-http', url: `https://${name}.example.com/mcp` }) as ParsedServerConfig; +const withUserVars = (config: ParsedServerConfig): ParsedServerConfig => + ({ + ...config, + customUserVars: { API_KEY: { title: 'API key', description: 'Server API key' } }, + }) as ParsedServerConfig; +const availableTools = (name: string): LCAvailableTools => ({ + [name]: { + type: 'function', + function: { + name, + description: '', + parameters: { type: 'object', properties: {} }, + }, + }, +}); + +describe('recoverMCPServerCatalogs', () => { + it('loads user auth once and preserves config-only lookup context for each server', async () => { + const servers = [ + { serverName: 'alpha', serverConfig: withUserVars(serverConfig('alpha')) }, + { serverName: 'beta', serverConfig: withUserVars(serverConfig('beta')) }, + ]; + const loadUserMCPAuthMap = jest.fn().mockResolvedValue({ + [`${Constants.mcp_prefix}alpha`]: { API_KEY: 'alpha-secret' }, + [`${Constants.mcp_prefix}beta`]: { API_KEY: 'beta-secret' }, + }); + const discoverServerTools = jest.fn(async ({ serverName }: ToolDiscoveryOptions) => ({ + tools: [{ name: `${serverName}-tool`, inputSchema: { type: 'object' as const } }], + })); + const formatServerTools = jest.fn((serverName: string) => + availableTools(`tool${Constants.mcp_delimiter}${serverName}`), + ); + + const result = await recoverMCPServerCatalogs( + { user, servers }, + { loadUserMCPAuthMap, discoverServerTools, formatServerTools }, + ); + + expect(loadUserMCPAuthMap).toHaveBeenCalledTimes(1); + expect(loadUserMCPAuthMap).toHaveBeenCalledWith('user-1', ['alpha', 'beta']); + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ + user, + serverName: 'alpha', + configServers: { alpha: servers[0].serverConfig }, + customUserVars: { API_KEY: 'alpha-secret' }, + }), + ); + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ + user, + serverName: 'beta', + configServers: { beta: servers[1].serverConfig }, + customUserVars: { API_KEY: 'beta-secret' }, + }), + ); + expect(result.size).toBe(2); + }); + + it('limits passive discovery to three concurrent servers', async () => { + const servers = Array.from({ length: 7 }, (_, index) => ({ + serverName: `server-${index}`, + serverConfig: serverConfig(`server-${index}`), + })); + let active = 0; + let maxActive = 0; + const discoverServerTools = jest.fn(async () => { + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise((resolve) => setTimeout(resolve, 5)); + active -= 1; + return { tools: [] }; + }); + + const result = await recoverMCPServerCatalogs( + { user, servers }, + { + loadUserMCPAuthMap: jest.fn().mockResolvedValue({}), + discoverServerTools, + formatServerTools: jest.fn().mockReturnValue({}), + }, + ); + + expect(discoverServerTools).toHaveBeenCalledTimes(7); + expect(maxActive).toBe(3); + expect(result.size).toBe(7); + }); + + it('keeps successful catalogs when another server fails or has no authoritative tools', async () => { + const servers = ['good', 'failed', 'missing'].map((serverName) => ({ + serverName, + serverConfig: serverConfig(serverName), + })); + + const result = await recoverMCPServerCatalogs( + { user, servers }, + { + loadUserMCPAuthMap: jest.fn().mockResolvedValue({}), + discoverServerTools: jest.fn(async ({ serverName }: ToolDiscoveryOptions) => { + if (serverName === 'failed') { + throw new Error('offline'); + } + return { tools: serverName === 'missing' ? null : [] }; + }), + formatServerTools: jest.fn().mockReturnValue({}), + }, + ); + + expect([...result.keys()]).toEqual(['good']); + }); +}); + +describe('loadMCPServerCatalogs', () => { + it('loads cache hits and connected snapshots in parallel, then caches only the snapshot', async () => { + const cachedTools = availableTools('cached'); + const snapshotTools = availableTools('live'); + const servers = [ + { serverName: 'cached-server', serverConfig: serverConfig('cached') }, + { serverName: 'live-server', serverConfig: serverConfig('live') }, + ]; + const getCachedServerTools = jest.fn( + async ( + _userId: string, + serverName: string, + _serverConfig: ParsedServerConfig, + ): Promise => (serverName === 'cached-server' ? cachedTools : null), + ); + const getServerToolFunctionsSnapshot = jest.fn().mockResolvedValue({ + tools: snapshotTools, + publicationGeneration: 'generation-1', + }); + const cacheServerTools = jest.fn().mockResolvedValue(undefined); + const loadUserMCPAuthMap = jest.fn(); + + const result = await loadMCPServerCatalogs( + { user, servers }, + { + getCachedServerTools, + getServerToolFunctionsSnapshot, + cacheServerTools, + loadUserMCPAuthMap, + discoverServerTools: jest.fn(), + formatServerTools: jest.fn(), + }, + ); + + expect(getCachedServerTools).toHaveBeenCalledTimes(2); + expect(getServerToolFunctionsSnapshot).toHaveBeenCalledTimes(1); + expect(loadUserMCPAuthMap).not.toHaveBeenCalled(); + expect(cacheServerTools).toHaveBeenCalledWith({ + userId: user.id, + serverName: 'live-server', + serverTools: snapshotTools, + serverConfig: servers[1].serverConfig, + publicationGeneration: 'generation-1', + publicationRevision: undefined, + }); + expect(result.serverTools).toEqual( + new Map([ + ['cached-server', cachedTools], + ['live-server', snapshotTools], + ]), + ); + expect(result.serversWithoutTools).toEqual([]); + }); + + it('serves passive recovery only to the request and does not cache it without a fence', async () => { + const servers = [{ serverName: 'cold-server', serverConfig: serverConfig('cold') }]; + const recoveredTools = availableTools('recovered'); + const cacheServerTools = jest.fn(); + + const result = await loadMCPServerCatalogs( + { user, servers }, + { + getCachedServerTools: jest.fn().mockResolvedValue(null), + getServerToolFunctionsSnapshot: jest.fn().mockResolvedValue({ tools: null }), + cacheServerTools, + loadUserMCPAuthMap: jest.fn().mockResolvedValue({}), + discoverServerTools: jest.fn().mockResolvedValue({ + tools: [{ name: 'recovered', inputSchema: { type: 'object' as const } }], + }), + formatServerTools: jest.fn().mockReturnValue(recoveredTools), + }, + ); + + expect(result.serverTools).toEqual(new Map([['cold-server', recoveredTools]])); + expect(result.serversWithoutTools).toEqual([]); + expect(cacheServerTools).not.toHaveBeenCalled(); + }); + + it('isolates cache and snapshot failures and reports only unresolved servers', async () => { + const servers = [ + { serverName: 'recovered', serverConfig: serverConfig('recovered') }, + { serverName: 'missing', serverConfig: serverConfig('missing') }, + ]; + + const result = await loadMCPServerCatalogs( + { user, servers }, + { + getCachedServerTools: jest.fn().mockRejectedValue(new Error('cache unavailable')), + getServerToolFunctionsSnapshot: jest + .fn() + .mockRejectedValueOnce(new Error('connection unavailable')) + .mockResolvedValueOnce({ tools: null }), + cacheServerTools: jest.fn(), + loadUserMCPAuthMap: jest.fn().mockResolvedValue({}), + discoverServerTools: jest.fn(async ({ serverName }: ToolDiscoveryOptions) => ({ + tools: serverName === 'recovered' ? [] : null, + })), + formatServerTools: jest.fn().mockReturnValue({}), + }, + ); + + expect(result.serverTools).toEqual(new Map([['recovered', {}]])); + expect(result.serversWithoutTools).toEqual(['missing']); + }); +}); + +describe('recoverMCPServerCatalogs — bounded, skippable discovery', () => { + const recoveryDeps = ( + discoverServerTools: jest.Mock, + userMCPAuthMap: Record> = {}, + ) => ({ + loadUserMCPAuthMap: jest.fn().mockResolvedValue(userMCPAuthMap), + discoverServerTools, + formatServerTools: jest.fn().mockReturnValue({}), + }); + + it('bounds each connection attempt the factory makes', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + + await recoverMCPServerCatalogs( + { user, servers: [{ serverName: 'slow', serverConfig: serverConfig('slow') }] }, + recoveryDeps(discoverServerTools), + ); + + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ serverName: 'slow', connectionTimeout: 1500 }), + ); + }); + + it('keeps a shorter configured initTimeout instead of raising it to the cap', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const impatient = { ...serverConfig('impatient'), initTimeout: 900 } as ParsedServerConfig; + + await recoverMCPServerCatalogs( + { user, servers: [{ serverName: 'impatient', serverConfig: impatient }] }, + recoveryDeps(discoverServerTools), + ); + + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ connectionTimeout: 900 }), + ); + }); + + it('leaves a server the config tier marked unreachable to that tier’s retry window', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const deps = recoveryDeps(discoverServerTools); + const failed = { + ...serverConfig('failed'), + inspectionFailed: true, + source: 'config', + } as ParsedServerConfig; + + const result = await recoverMCPServerCatalogs( + { + user, + servers: [ + { serverName: 'failed', serverConfig: failed }, + { serverName: 'healthy', serverConfig: serverConfig('healthy') }, + ], + }, + deps, + ); + + expect(discoverServerTools).toHaveBeenCalledTimes(1); + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ serverName: 'healthy' }), + ); + expect([...result.keys()]).toEqual(['healthy']); + }); + + it('skips a server whose user-provided variables are unset and recovers its siblings', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const needsVars = withUserVars(serverConfig('needs-vars')); + + const result = await recoverMCPServerCatalogs( + { + user, + servers: [ + { serverName: 'needs-vars', serverConfig: needsVars }, + { serverName: 'open', serverConfig: serverConfig('open') }, + ], + }, + recoveryDeps(discoverServerTools), + ); + + expect(discoverServerTools).toHaveBeenCalledTimes(1); + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ serverName: 'open' }), + ); + expect([...result.keys()]).toEqual(['open']); + }); + + it('discovers a server whose user-provided variables are satisfied', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const needsVars = withUserVars(serverConfig('needs-vars')); + + await recoverMCPServerCatalogs( + { user, servers: [{ serverName: 'needs-vars', serverConfig: needsVars }] }, + recoveryDeps(discoverServerTools, { + [`${Constants.mcp_prefix}needs-vars`]: { API_KEY: 'set' }, + }), + ); + + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ + serverName: 'needs-vars', + customUserVars: { API_KEY: 'set' }, + }), + ); + }); + + it('skips the auth lookup entirely when every cold server is ineligible', async () => { + const discoverServerTools = jest.fn(); + const deps = recoveryDeps(discoverServerTools); + const failed = { + ...serverConfig('failed'), + inspectionFailed: true, + source: 'config', + } as ParsedServerConfig; + + const result = await recoverMCPServerCatalogs( + { user, servers: [{ serverName: 'failed', serverConfig: failed }] }, + deps, + ); + + expect(deps.loadUserMCPAuthMap).not.toHaveBeenCalled(); + expect(discoverServerTools).not.toHaveBeenCalled(); + expect(result.size).toBe(0); + }); + + it('still attempts a yaml stub, which has no retry timer of its own', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const stub = { + ...serverConfig('yaml-stub'), + inspectionFailed: true, + source: 'yaml', + } as ParsedServerConfig; + + await recoverMCPServerCatalogs( + { user, servers: [{ serverName: 'yaml-stub', serverConfig: stub }] }, + recoveryDeps(discoverServerTools), + ); + + expect(discoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ serverName: 'yaml-stub' }), + ); + }); + + it('reads plugin auth only when a cold server actually declares user variables', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const deps = recoveryDeps(discoverServerTools); + + await recoverMCPServerCatalogs( + { + user, + servers: [ + { serverName: 'open-a', serverConfig: serverConfig('open-a') }, + { serverName: 'open-b', serverConfig: serverConfig('open-b') }, + ], + }, + deps, + ); + + expect(deps.loadUserMCPAuthMap).not.toHaveBeenCalled(); + expect(discoverServerTools).toHaveBeenCalledTimes(2); + }); + + it('asks plugin auth only for the credential-bearing servers in a mixed list', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const deps = recoveryDeps(discoverServerTools, { + [`${Constants.mcp_prefix}guarded`]: { API_KEY: 'set' }, + }); + + await recoverMCPServerCatalogs( + { + user, + servers: [ + { serverName: 'open', serverConfig: serverConfig('open') }, + { serverName: 'guarded', serverConfig: withUserVars(serverConfig('guarded')) }, + ], + }, + deps, + ); + + expect(deps.loadUserMCPAuthMap).toHaveBeenCalledWith('user-1', ['guarded']); + expect(discoverServerTools).toHaveBeenCalledTimes(2); + }); + + it('holds a limiter slot until its discovery settles, so concurrency stays honest', async () => { + /** A slot released while its network operation is still running would let a fourth + * discovery start; awaiting the work rather than racing it keeps the limit real. */ + let active = 0; + let maxActive = 0; + const discoverServerTools = jest.fn(async () => { + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise((resolve) => setTimeout(resolve, 20)); + active -= 1; + return { tools: null }; + }); + const servers = Array.from({ length: 9 }, (_, index) => ({ + serverName: `slow-${index}`, + serverConfig: serverConfig(`slow-${index}`), + })); + + const result = await recoverMCPServerCatalogs( + { user, servers }, + { + loadUserMCPAuthMap: jest.fn().mockResolvedValue({}), + discoverServerTools, + formatServerTools: jest.fn().mockReturnValue({}), + }, + ); + + expect(discoverServerTools).toHaveBeenCalledTimes(9); + expect(maxActive).toBe(3); + expect(result.size).toBe(0); + }); + + it('attempts every cold server, so none is starved by those ahead of it', async () => { + const discoverServerTools = jest.fn().mockResolvedValue({ tools: [] }); + const servers = Array.from({ length: 12 }, (_, index) => ({ + serverName: `server-${index}`, + serverConfig: serverConfig(`server-${index}`), + })); + + await recoverMCPServerCatalogs({ user, servers }, recoveryDeps(discoverServerTools)); + await recoverMCPServerCatalogs({ user, servers }, recoveryDeps(discoverServerTools)); + + expect(discoverServerTools).toHaveBeenCalledTimes(24); + for (const { serverName } of servers) { + expect(discoverServerTools).toHaveBeenCalledWith(expect.objectContaining({ serverName })); + } + }); + + it('bounds snapshot refreshes, which each issue a real tools/list', async () => { + let active = 0; + let maxActive = 0; + const servers = Array.from({ length: 9 }, (_, index) => ({ + serverName: `server-${index}`, + serverConfig: serverConfig(`server-${index}`), + })); + + await loadMCPServerCatalogs( + { user, servers }, + { + getCachedServerTools: jest.fn().mockResolvedValue(null), + getServerToolFunctionsSnapshot: jest.fn(async () => { + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise((resolve) => setTimeout(resolve, 20)); + active -= 1; + return { tools: null }; + }), + cacheServerTools: jest.fn(), + loadUserMCPAuthMap: jest.fn().mockResolvedValue({}), + discoverServerTools: jest.fn().mockResolvedValue({ tools: null }), + formatServerTools: jest.fn().mockReturnValue({}), + }, + ); + + expect(maxActive).toBe(3); + }); +}); diff --git a/packages/api/src/mcp/catalog/recovery.ts b/packages/api/src/mcp/catalog/recovery.ts new file mode 100644 index 0000000000..06d61b8b0f --- /dev/null +++ b/packages/api/src/mcp/catalog/recovery.ts @@ -0,0 +1,262 @@ +import { logger } from '@librechat/data-schemas'; +import type { Tool } from '@modelcontextprotocol/sdk/types.js'; +import type { IUser } from '@librechat/data-schemas'; +import type { LCAvailableTools, ParsedServerConfig, ToolDiscoveryOptions } from '../types'; +import { hasCustomUserVars, getMissingCustomUserVars } from '../utils'; +import { createConcurrencyLimiter } from '~/utils/promise'; +import { getServerCustomUserVars } from '../auth'; + +/** Bounds outbound MCP fan-out from one catalog request: snapshot refreshes issue a real + * `tools/list`, so they burst as readily as passive discovery does. */ +const CATALOG_FANOUT_CONCURRENCY = 3; +/** + * Bounds `connection.connect()` only — it is the sole segment of discovery a caller can bound + * today. It does NOT bound the whole operation: `discoverToolsInternal` spends this value once + * per connection attempt (authenticated, then unauthenticated), and `fetchToolsSnapshot` then + * applies its own `TOOLS_LIST_TIMEOUT_MS` (30s) to `tools/list`. A server that connects quickly + * and stalls while listing therefore still holds its slot for that longer window. + * + * Bounding discovery end to end needs a deadline threaded through `MCPConnectionFactory` into + * both `connect()` and `fetchToolsSnapshot()`; until that exists, this keeps the common + * unreachable case cheap, because recovery targets a server that is reachable and authorized but + * whose catalog cache expired, and such a server connects well inside this window. + */ +const RECOVERY_ATTEMPT_TIMEOUT_MS = 1500; + +export interface MCPServerCatalogRecoveryInput { + serverName: string; + serverConfig: ParsedServerConfig; +} + +export interface MCPServerCatalogRecoveryDeps { + loadUserMCPAuthMap: ( + userId: string, + serverNames: readonly string[], + ) => Promise>>; + discoverServerTools: (options: ToolDiscoveryOptions) => Promise<{ tools: Tool[] | null }>; + formatServerTools: (serverName: string, tools: Tool[]) => LCAvailableTools; +} + +export interface MCPServerCatalogSnapshot { + tools: LCAvailableTools | null; + publicationGeneration?: string; + publicationRevision?: string; +} + +export interface MCPServerCatalogLoaderDeps extends MCPServerCatalogRecoveryDeps { + getCachedServerTools: ( + userId: string, + serverName: string, + serverConfig: ParsedServerConfig, + ) => Promise; + getServerToolFunctionsSnapshot: ( + userId: string, + serverName: string, + serverConfig: ParsedServerConfig, + ) => Promise; + cacheServerTools: (params: { + userId: string; + serverName: string; + serverTools: LCAvailableTools; + serverConfig: ParsedServerConfig; + publicationGeneration?: string; + publicationRevision?: string; + }) => Promise; +} + +export interface MCPServerCatalogLoaderResult { + serverTools: Map; + serversWithoutTools: string[]; +} + +interface RecoveryCandidate extends MCPServerCatalogRecoveryInput { + customUserVars?: Record; +} + +/** Bounds one connection attempt, honouring a shorter operator `initTimeout`. */ +function resolveAttemptTimeout(serverConfig: ParsedServerConfig): number { + const { initTimeout } = serverConfig; + if (typeof initTimeout === 'number') { + return Math.min(initTimeout, RECOVERY_ATTEMPT_TIMEOUT_MS); + } + return RECOVERY_ATTEMPT_TIMEOUT_MS; +} + +async function discoverCandidate( + user: IUser, + { serverName, serverConfig, customUserVars }: RecoveryCandidate, + deps: MCPServerCatalogRecoveryDeps, +): Promise<[string, LCAvailableTools | null]> { + try { + const result = await deps.discoverServerTools({ + user, + serverName, + configServers: { [serverName]: serverConfig }, + customUserVars, + connectionTimeout: resolveAttemptTimeout(serverConfig), + }); + return [ + serverName, + result.tools == null ? null : deps.formatServerTools(serverName, result.tools), + ]; + } catch (error) { + logger.error(`[MCP catalog recovery] Failed to discover tools for ${serverName}:`, error); + return [serverName, null]; + } +} + +/** + * Passively discovers cold MCP catalogs for one request. + * + * A recovered catalog cannot be retained: a discovery connection owns no publication generation + * and is disposed, and the tool cache refuses unfenced writes, so the result is served only to + * the requesting user. Recovery therefore stays stateless and individually cheap rather than + * scheduling around a result it is not allowed to keep — it skips only what configuration alone + * proves pointless, and fails fast on everything else. + */ +export async function recoverMCPServerCatalogs( + params: { user: IUser; servers: readonly MCPServerCatalogRecoveryInput[] }, + deps: MCPServerCatalogRecoveryDeps, +): Promise> { + const { user, servers } = params; + /** Only the config tier retries a failed stub on its own clock. A `yaml`- or `user`-sourced + * stub has no such timer, so skipping it unconditionally would hide the server for good — + * exactly the state this recovery exists to escape. */ + const recoverable = servers.filter(({ serverName, serverConfig }) => { + if (!serverConfig.inspectionFailed || serverConfig.source !== 'config') { + return true; + } + logger.debug(`[MCP catalog recovery] Skipping ${serverName}: awaiting config-tier retry`); + return false; + }); + if (recoverable.length === 0) { + return new Map(); + } + + /** Only credential-bearing servers can consume the auth map, so a list without any avoids + * the plugin-auth round trip entirely. */ + const credentialServers = recoverable.filter(({ serverConfig }) => + hasCustomUserVars(serverConfig), + ); + const userMCPAuthMap = credentialServers.length + ? await deps.loadUserMCPAuthMap( + user.id, + credentialServers.map(({ serverName }) => serverName), + ) + : {}; + + /** A server missing its user-provided credentials fails auth on connect (see issue #10969), + * so discovering it would spend a doomed connection on every request. */ + const authorized: RecoveryCandidate[] = []; + for (const candidate of recoverable) { + const customUserVars = getServerCustomUserVars(userMCPAuthMap, candidate.serverName); + const missingUserVars = getMissingCustomUserVars(candidate.serverConfig, customUserVars); + if (missingUserVars.length > 0) { + logger.debug( + `[MCP catalog recovery] Skipping ${candidate.serverName}: ${missingUserVars.length} user-provided variable(s) unset`, + ); + continue; + } + authorized.push({ ...candidate, customUserVars }); + } + if (authorized.length === 0) { + return new Map(); + } + + const recover = createConcurrencyLimiter(CATALOG_FANOUT_CONCURRENCY); + const results = await Promise.all( + authorized.map((candidate) => recover(() => discoverCandidate(user, candidate, deps))), + ); + + return new Map(results.filter((entry): entry is [string, LCAvailableTools] => entry[1] != null)); +} + +/** Loads cached, connected, then passive MCP catalogs for a marketplace-style list request. */ +export async function loadMCPServerCatalogs( + params: { user: IUser; servers: readonly MCPServerCatalogRecoveryInput[] }, + deps: MCPServerCatalogLoaderDeps, +): Promise { + const { user, servers } = params; + const cached = await Promise.all( + servers.map(async ({ serverName, serverConfig }) => { + try { + const tools = await deps.getCachedServerTools(user.id, serverName, serverConfig); + return { serverName, serverConfig, tools, source: 'cache' as const }; + } catch (error) { + logger.error(`[MCP catalog loader] Failed to read cached tools for ${serverName}:`, error); + return { serverName, serverConfig, tools: null, source: 'cache' as const }; + } + }), + ); + + /** A snapshot is not a local read — both connection paths issue a fresh `tools/list` — so a + * cache reset across many servers would otherwise burst unbounded outbound requests. */ + const refresh = createConcurrencyLimiter(CATALOG_FANOUT_CONCURRENCY); + const snapshots = await Promise.all( + cached.map((entry) => { + if (entry.tools != null) { + return entry; + } + return refresh(async () => { + try { + const snapshot = await deps.getServerToolFunctionsSnapshot( + user.id, + entry.serverName, + entry.serverConfig, + ); + return { ...entry, ...snapshot, source: 'snapshot' as const }; + } catch (error) { + logger.error( + `[MCP catalog loader] Failed to read connected tools for ${entry.serverName}:`, + error, + ); + return { ...entry, tools: null, source: 'snapshot' as const }; + } + }); + }), + ); + + const coldServers = snapshots + .filter(({ tools }) => tools == null) + .map(({ serverName, serverConfig }) => ({ serverName, serverConfig })); + let recovered = new Map(); + if (coldServers.length > 0) { + try { + recovered = await recoverMCPServerCatalogs({ user, servers: coldServers }, deps); + } catch (error) { + logger.error('[MCP catalog loader] Failed to recover cold server catalogs:', error); + } + } + + const serverTools = new Map(); + const serversWithoutTools: string[] = []; + for (const snapshot of snapshots) { + const tools = snapshot.tools ?? recovered.get(snapshot.serverName); + if (tools == null) { + serversWithoutTools.push(snapshot.serverName); + continue; + } + serverTools.set(snapshot.serverName, tools); + + if (snapshot.source !== 'snapshot' || snapshot.tools == null) { + continue; + } + void deps + .cacheServerTools({ + userId: user.id, + serverName: snapshot.serverName, + serverTools: snapshot.tools, + serverConfig: snapshot.serverConfig, + publicationGeneration: snapshot.publicationGeneration, + publicationRevision: snapshot.publicationRevision, + }) + .catch((error) => + logger.error( + `[MCP catalog loader] Failed to cache tools for ${snapshot.serverName}:`, + error, + ), + ); + } + + return { serverTools, serversWithoutTools }; +} diff --git a/packages/api/src/mcp/tools.ts b/packages/api/src/mcp/tools.ts index 5f58d4bc28..cc68e53ba8 100644 --- a/packages/api/src/mcp/tools.ts +++ b/packages/api/src/mcp/tools.ts @@ -83,6 +83,35 @@ export interface MCPToolCacheService { ) => Promise; } +/** Converts an MCP tools/list response into LibreChat's server-qualified catalog format. */ +export function formatMCPServerTools(serverName: string, tools: MCPToolInput[]): LCAvailableTools { + const serverTools: LCAvailableTools = {}; + const keyServerName = normalizeServerName(serverName); + const keyToolNames = stripServerNamePrefixes( + tools.map((tool) => tool.name), + keyServerName, + ); + for (const tool of tools) { + const keyToolName = keyToolNames.get(tool.name) ?? tool.name; + const name = `${keyToolName}${Constants.mcp_delimiter}${keyServerName}`; + const entry: LCFunctionTool = { + type: 'function', + ['function']: { + name, + description: tool.description ?? '', + parameters: tool.inputSchema + ? (normalizeJsonSchema(resolveJsonSchemaRefs(tool.inputSchema)) as JsonSchemaType) + : ({ type: 'object', properties: {} } as JsonSchemaType), + }, + }; + if (keyToolName !== tool.name) { + entry.serverToolName = tool.name; + } + serverTools[name] = entry; + } + return serverTools; +} + interface AppServerBoundary { serverName: string; suffix: string; @@ -222,42 +251,12 @@ export function createMCPToolCacheService(deps: MCPToolCacheDeps): MCPToolCacheS const { userId, serverName, tools, serverConfig, publicationGeneration, publicationRevision } = params; try { - const serverTools: LCAvailableTools = {}; - const mcpDelimiter = Constants.mcp_delimiter; - if (tools == null) { logger.debug('[MCP Cache] No tools to update'); - return serverTools; - } - - /** Cache keys are MODEL-FACING: they become builder tool ids, agent.tools - * entries, tool_options keys, and definition names, and must equal the - * runtime instance name (`createToolInstance` in MCP.js), which embeds - * `normalizeServerName(serverName)`. The cache STORE itself stays keyed - * by the raw config name. */ - const keyServerName = normalizeServerName(serverName); - const keyToolNames = stripServerNamePrefixes( - tools.map((tool) => tool.name), - keyServerName, - ); - for (const tool of tools) { - const keyToolName = keyToolNames.get(tool.name) ?? tool.name; - const name = `${keyToolName}${mcpDelimiter}${keyServerName}`; - const entry: LCFunctionTool = { - type: 'function', - ['function']: { - name, - description: tool.description ?? '', - parameters: tool.inputSchema - ? (normalizeJsonSchema(resolveJsonSchemaRefs(tool.inputSchema)) as JsonSchemaType) - : ({ type: 'object', properties: {} } as JsonSchemaType), - }, - }; - if (keyToolName !== tool.name) { - entry.serverToolName = tool.name; - } - serverTools[name] = entry; + return {}; } + /** Cache keys are model-facing and must match runtime tool instance names. */ + const serverTools = formatMCPServerTools(serverName, tools); const resolvedConfig = await resolveCacheConfig(userId, serverName, serverConfig); const configGeneration = resolvedConfig