From da473bf43a4fe5c161b4dca6a95df4e172b8d45e Mon Sep 17 00:00:00 2001 From: Atef Bellaaj Date: Fri, 28 Nov 2025 16:07:09 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=83=EF=B8=8F=20refactor:=20Simplify=20?= =?UTF-8?q?MCP=20Server=20Config=20to=20Two-Repository=20Pattern=20(#10705?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(mcp): simplify registry to two-repository architecture with explicit storage * Chore: address AI Review comments * Simplify MCP config cache architecture and remove legacy code: Follow-up cleanup to commit d2bfdd033 which refactored MCP registry to two-repository architecture. This removes leftover legacy abstractions that were no longer used. What changed: - Simplified ServerConfigsCacheFactory.create() from 3 params to 2 (namespace, leaderOnly) - Removed unused scope: 'Shared' | 'Private' parameter (only 'Shared' was ever used) - Removed dead set() and getNamespace() methods from cache classes - Updated JSDoc to reflect two-repository architecture (Cache + DB) instead of old three-tier system - Fixed stale mocks and comments referencing removed sharedAppServers, sharedUserServers, privateServersCache Files changed: - ServerConfigsCacheFactory.ts - Simplified factory signature - ServerConfigsCacheRedis.ts - Removed scope, renamed owner→namespace - ServerConfigsCacheInMemory.ts - Removed unused methods - MCPServersRegistry.ts - Updated JSDoc, simplified factory call - RegistryStatusCache.ts - Removed stale JSDoc reference - MCPManager.test.ts - Fixed legacy mock - ServerConfigsCacheFactory.test.ts - Updated test assertions * fix: Update error message in MCPServersRegistry for clarity --------- Co-authored-by: Atef Bellaaj Co-authored-by: Danny Avila --- packages/api/package.json | 2 +- packages/api/src/index.ts | 1 - packages/api/src/mcp/ConnectionsRepository.ts | 23 +- .../__tests__/ConnectionsRepository.test.ts | 148 ++++ .../api/src/mcp/__tests__/MCPManager.test.ts | 4 - .../mcp/registry/MCPPrivateServerLoader.ts | 276 ------- .../src/mcp/registry/MCPServersInitializer.ts | 5 +- .../src/mcp/registry/MCPServersRegistry.ts | 167 ++--- .../ServerConfigsRepositoryInterface.ts | 22 + ...vateServerLoader.cache_integration.spec.ts | 593 --------------- .../__tests__/MCPPrivateServerLoader.test.ts | 702 ------------------ ...rversInitializer.cache_integration.spec.ts | 74 +- .../__tests__/MCPServersInitializer.test.ts | 77 +- ...PServersRegistry.cache_integration.spec.ts | 277 +++---- .../__tests__/MCPServersRegistry.test.ts | 301 +++----- .../PrivateServerConfigsCacheBase.ts | 115 --- .../PrivateServerConfigsCacheFactory.ts | 32 - .../PrivateServerConfigsCacheInMemory.ts | 105 --- .../PrivateServerConfigsCacheRedis.ts | 284 ------- .../cache/PrivateServersLoadStatusCache.ts | 166 ----- .../mcp/registry/cache/RegistryStatusCache.ts | 3 - .../cache/ServerConfigsCacheFactory.ts | 12 +- .../cache/ServerConfigsCacheInMemory.ts | 16 - .../registry/cache/ServerConfigsCacheRedis.ts | 47 +- .../PrivateServerConfigsCacheFactory.test.ts | 71 -- .../PrivateServerConfigsCacheInMemory.test.ts | 346 --------- ...onfigsCacheRedis.cache_integration.spec.ts | 606 --------------- ...sLoadStatusCache.cache_integration.spec.ts | 397 ---------- .../PrivateServersLoadStatusCache.test.ts | 329 -------- .../ServerConfigsCacheFactory.test.ts | 14 +- .../src/mcp/registry/db/ServerConfigsDB.ts | 50 ++ 31 files changed, 551 insertions(+), 4714 deletions(-) delete mode 100644 packages/api/src/mcp/registry/MCPPrivateServerLoader.ts create mode 100644 packages/api/src/mcp/registry/ServerConfigsRepositoryInterface.ts delete mode 100644 packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.cache_integration.spec.ts delete mode 100644 packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.test.ts delete mode 100644 packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheBase.ts delete mode 100644 packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheFactory.ts delete mode 100644 packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheInMemory.ts delete mode 100644 packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheRedis.ts delete mode 100644 packages/api/src/mcp/registry/cache/PrivateServersLoadStatusCache.ts delete mode 100644 packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheFactory.test.ts delete mode 100644 packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheInMemory.test.ts delete mode 100644 packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheRedis.cache_integration.spec.ts delete mode 100644 packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.cache_integration.spec.ts delete mode 100644 packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.test.ts create mode 100644 packages/api/src/mcp/registry/db/ServerConfigsDB.ts diff --git a/packages/api/package.json b/packages/api/package.json index 41e6c663b3..771d16685f 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -22,7 +22,7 @@ "test:ci": "jest --coverage --ci --testPathIgnorePatterns=\"\\.*integration\\.|\\.*helper\\.\"", "test:cache-integration:core": "jest --testPathPatterns=\"src/cache/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false", "test:cache-integration:cluster": "jest --testPathPatterns=\"src/cluster/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false --runInBand", - "test:cache-integration:mcp": "jest --testPathPatterns=\"src/mcp/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false --runInBand --forceExit", + "test:cache-integration:mcp": "jest --testPathPatterns=\"src/mcp/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false", "test:cache-integration": "npm run test:cache-integration:core && npm run test:cache-integration:cluster && npm run test:cache-integration:mcp", "verify": "npm run test:ci", "b:clean": "bun run rimraf dist", diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index 8acaa84ba8..6350247a69 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -4,7 +4,6 @@ export * from './cdn'; export * from './auth'; /* MCP */ export * from './mcp/registry/MCPServersRegistry'; -export * from './mcp/registry/MCPPrivateServerLoader'; export * from './mcp/MCPManager'; export * from './mcp/connection'; export * from './mcp/oauth'; diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts index b5f8f3609c..d5f6eba6c3 100644 --- a/packages/api/src/mcp/ConnectionsRepository.ts +++ b/packages/api/src/mcp/ConnectionsRepository.ts @@ -26,18 +26,20 @@ 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); - if (!config) { - //if the config does not exist, clean up any potential orphaned connections (caused by server tier migration) + const canConnect = !!config && this.isAllowedToConnectToServer(config); + if (!canConnect) { + //if connection is no longer possible we attempt to disconnect any leftover connections await this.disconnect(serverName); } - return !!config; + return canConnect; } /** 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 existingConnection = this.connections.get(serverName); - if (!serverConfig) { + if (!serverConfig || !this.isAllowedToConnectToServer(serverConfig)) { if (existingConnection) { await existingConnection.disconnect(); } @@ -64,7 +66,6 @@ export class ConnectionsRepository { await this.disconnect(serverName); } } - const connection = await MCPConnectionFactory.create( { serverName, @@ -92,13 +93,13 @@ export class ConnectionsRepository { /** Gets or creates connections for all configured servers in this repository's scope */ async getAll(): Promise> { //TODO in the future we should use a scoped config getter (APPLevel, UserLevel, Private) - //for now the unexisting config will not throw error + //for now the absent config will not throw error const allConfigs = await registry.getAllServerConfigs(this.ownerId); return this.getMany(Object.keys(allConfigs)); } /** Disconnects and removes a specific server connection from the pool */ - disconnect(serverName: string): Promise { + async disconnect(serverName: string): Promise { const connection = this.connections.get(serverName); if (!connection) return Promise.resolve(); this.connections.delete(serverName); @@ -117,4 +118,12 @@ export class ConnectionsRepository { protected prefix(serverName: string): string { return `[MCP][${serverName}]`; } + + private isAllowedToConnectToServer(config: t.ParsedServerConfig) { + //the repository is not allowed to be connected in case the Connection repository is shared (ownerId is undefined/null) and the server requires Auth or startup false. + if (this.ownerId === undefined && (config.startup === false || config.requiresOAuth)) { + return false; + } + return true; + } } diff --git a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts index 7dae35a392..3248f13daf 100644 --- a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts +++ b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts @@ -319,4 +319,152 @@ describe('ConnectionsRepository', () => { expect(Array.isArray(promises)).toBe(true); }); }); + + describe('Connection policy (isAllowedToConnectToServer)', () => { + describe('App-level repository (ownerId = undefined)', () => { + beforeEach(() => { + repository = new ConnectionsRepository(undefined); + }); + + it('should allow connection to regular servers (startup !== false, !requiresOAuth)', async () => { + mockServerConfigs.regularServer = { + type: 'stdio', + command: 'test', + args: [], + startup: true, + requiresOAuth: false, + }; + + expect(await repository.has('regularServer')).toBe(true); + }); + + it('should allow connection when startup is undefined and requiresOAuth is false', async () => { + mockServerConfigs.defaultServer = { + type: 'stdio', + command: 'test', + args: [], + requiresOAuth: false, + }; + + expect(await repository.has('defaultServer')).toBe(true); + }); + + it('should NOT allow connection to OAuth servers', async () => { + mockServerConfigs.oauthServer = { + type: 'streamable-http', + url: 'http://example.com', + requiresOAuth: true, + }; + + expect(await repository.has('oauthServer')).toBe(false); + }); + + it('should NOT allow connection to disabled servers (startup: false)', async () => { + mockServerConfigs.disabledServer = { + type: 'stdio', + command: 'test', + args: [], + startup: false, + requiresOAuth: false, + }; + + expect(await repository.has('disabledServer')).toBe(false); + }); + + it('should NOT allow connection to OAuth servers that are also disabled', async () => { + mockServerConfigs.oauthDisabledServer = { + type: 'streamable-http', + url: 'http://example.com', + startup: false, + requiresOAuth: true, + }; + + expect(await repository.has('oauthDisabledServer')).toBe(false); + }); + + it('should disconnect existing connection when server becomes not allowed', async () => { + // Initially setup as regular server + mockServerConfigs.changingServer = { + type: 'stdio', + command: 'test', + args: [], + requiresOAuth: false, + }; + + // Create connection + const connection = await repository.get('changingServer'); + expect(connection).toBe(mockConnection); + expect(repository['connections'].has('changingServer')).toBe(true); + + // Change config to require OAuth (app-level can't connect) + mockServerConfigs.changingServer = { + type: 'streamable-http', + url: 'http://example.com', + requiresOAuth: true, + }; + + // Check if connection is allowed + const allowed = await repository.has('changingServer'); + + expect(allowed).toBe(false); + expect(mockConnection.disconnect).toHaveBeenCalled(); + }); + }); + + describe('User-level repository (ownerId = userId)', () => { + beforeEach(() => { + repository = new ConnectionsRepository('user123'); + }); + + it('should allow connection to regular servers', async () => { + mockServerConfigs.regularServer = { + type: 'stdio', + command: 'test', + args: [], + startup: true, + requiresOAuth: false, + }; + + expect(await repository.has('regularServer')).toBe(true); + }); + + it('should allow connection to OAuth servers', async () => { + mockServerConfigs.oauthServer = { + type: 'streamable-http', + url: 'http://example.com', + requiresOAuth: true, + }; + + expect(await repository.has('oauthServer')).toBe(true); + }); + + it('should allow connection to disabled servers (startup: false)', async () => { + mockServerConfigs.disabledServer = { + type: 'stdio', + command: 'test', + args: [], + startup: false, + requiresOAuth: false, + }; + + expect(await repository.has('disabledServer')).toBe(true); + }); + + it('should allow connection to OAuth servers that are also disabled', async () => { + mockServerConfigs.oauthDisabledServer = { + type: 'streamable-http', + url: 'http://example.com', + startup: false, + requiresOAuth: true, + }; + + expect(await repository.has('oauthDisabledServer')).toBe(true); + }); + + it('should return null from get() when server config does not exist', async () => { + const connection = await repository.get('nonexistent'); + expect(connection).toBeNull(); + }); + }); + }); }); diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index 20aa0d1bd9..f1452c2938 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -19,9 +19,6 @@ jest.mock('@librechat/data-schemas', () => ({ jest.mock('~/mcp/registry/MCPServersRegistry', () => ({ mcpServersRegistry: { - sharedAppServers: { - getAll: jest.fn(), - }, getServerConfig: jest.fn(), getAllServerConfigs: jest.fn(), getOAuthServers: jest.fn(), @@ -50,7 +47,6 @@ describe('MCPManager', () => { // Set up default mock implementations (MCPServersInitializer.initialize as jest.Mock).mockResolvedValue(undefined); - (mcpServersRegistry.sharedAppServers.getAll as jest.Mock).mockResolvedValue({}); (mcpServersRegistry.getAllServerConfigs as jest.Mock).mockResolvedValue({}); }); diff --git a/packages/api/src/mcp/registry/MCPPrivateServerLoader.ts b/packages/api/src/mcp/registry/MCPPrivateServerLoader.ts deleted file mode 100644 index 61cd634ed4..0000000000 --- a/packages/api/src/mcp/registry/MCPPrivateServerLoader.ts +++ /dev/null @@ -1,276 +0,0 @@ -import { mcpServersRegistry as registry } from './MCPServersRegistry'; -import { privateServersLoadStatusCache as loadStatusCache } from './cache/PrivateServersLoadStatusCache'; -import type * as t from '~/mcp/types'; -import { logger } from '@librechat/data-schemas'; -import { MCPServerDB } from 'librechat-data-provider'; - -/** - * Handles loading and updating private MCP servers for users. - * Static methods work directly with the registry's privateServersCache. - * Similar pattern to MCPServersInitializer but for runtime private server management. - */ -export class MCPPrivateServerLoader { - /** - * Load private servers for a specific user with TTL synchronization and distributed locking. - * Use case: User logs in, lazy-load their private servers from DB - * - * Handles three critical issues: - * 1. Partial Load Prevention: Loaded flag only set after ALL servers load successfully - * 2. TTL Synchronization: Loaded flag expires with cache entries (prevents desync) - * 3. Race Condition Prevention: Distributed locking prevents concurrent loads - * - * Edge cases handled: - * - Process crashes mid-load: Flag not set, will retry on next attempt - * - Cache eviction: TTL ensures flag expires with cache entries - * - Concurrent loads: Lock ensures only one process loads, others wait - * - Users with 0 servers: Correctly handled (no cache verification needed) - * - * @param userId - User ID - * @param configsLoader - a callback that fetches db servers available for a user - * @param cacheTTL - Cache TTL in milliseconds (default: 3600000 = 1 hour) - * @throws {Error} If userId is invalid or loading fails - */ - public static async loadPrivateServers( - userId: string, - configsLoader: (userId: string) => Promise, - cacheTTL: number = 3600000, // 1 hour default - ): Promise { - // Input validation - if (!userId?.trim()) { - throw new Error('[MCP][PrivateServerLoader] userId is required and cannot be empty'); - } - if (typeof configsLoader !== 'function') { - throw new Error('[MCP][PrivateServerLoader] configsLoader must be a function'); - } - - const alreadyLoaded = await loadStatusCache.isLoaded(userId); - if (alreadyLoaded) { - logger.debug(`[MCP][PrivateServerLoader] User ${userId} private servers already loaded`); - return; - } - - const lockAcquired = await loadStatusCache.acquireLoadLock(userId); - - if (!lockAcquired) { - logger.debug( - `[MCP][PrivateServerLoader] Another process is loading user ${userId}, waiting...`, - ); - const completed = await loadStatusCache.waitForLoad(userId); - - if (completed) { - logger.debug(`[MCP][PrivateServerLoader] User ${userId} loaded by another process`); - return; - } else { - // Timeout - try to acquire lock again (maybe the other process crashed) - logger.warn( - `[MCP][PrivateServerLoader] Timeout waiting for user ${userId}, retrying lock acquisition`, - ); - const retryLock = await loadStatusCache.acquireLoadLock(userId); - if (!retryLock) { - throw new Error( - `[MCP][PrivateServerLoader] Failed to acquire load lock for user ${userId}`, - ); - } - } - } - - // We have the lock, proceed with loading - try { - logger.info(`[MCP][PrivateServerLoader] Loading private servers for user ${userId}`); - const servers = await configsLoader(userId); - //reset cache for the user - await registry.privateServersCache.reset(userId); - - for (const server of servers) { - const serverName = server.mcp_id; - const existing = await registry.privateServersCache.get(userId, serverName); - if (!existing) { - // Add new server config - await registry.privateServersCache.add(userId, serverName, { - ...server.config, - dbId: server._id, - }); - logger.debug(`${this.prefix(serverName)} Added private server for user ${userId}`); - } else { - logger.debug( - `${this.prefix(serverName)} Private server already exists for user ${userId}`, - ); - } - } - - // Mark as fully loaded with TTL (synchronized with cache entries) - await loadStatusCache.setLoaded(userId, cacheTTL); - logger.debug( - `[MCP][PrivateServerLoader] User ${userId} private servers fully loaded (${servers.length} servers, TTL: ${cacheTTL}ms)`, - ); - } catch (error) { - logger.error( - `[MCP][PrivateServerLoader] Loading private servers for user ${userId} failed.`, - error, - ); - throw error; - } finally { - // Always release the lock, even on error - await loadStatusCache.releaseLoadLock(userId); - } - } - - /** - * Propagate metadata changes to all users who have this server or update shared cache if the server is shared with PUBLIC. - * Use case: Admin updates server url, auth etc.. - * Efficient: Uses pattern-based scan, updates only affected users - * - * @param serverName - Server name - * @param config - Updated server configuration - */ - public static async updatePrivateServer( - serverName: string, - config: t.ParsedServerConfig, - ): Promise { - //check if the private server is promoted to a app level or user shared level server - const sharedServer = await registry.getServerConfig(serverName); - if (sharedServer) { - logger.info(`${this.prefix(serverName)} Promoted private server update`); - // server must be removed to simplify moving from App -> Shared and Shared -> App based on the config. - // Otherwise we need to figure out if it is an APP or a User shared and whether to migrate or not. - - await registry.removeServer(serverName); - await registry.addSharedServer(serverName, config); - return; - } - logger.info(`${this.prefix(serverName)} Propagating metadata update to all users`); - await registry.privateServersCache.updateServerConfigIfExists(serverName, config); - } - - /** - * Add a private server - * Use case: Admin / user creates an mcp server from the UI - * - * @param userId - userId - * @param serverName - Server name - * @param config - Updated server configuration - */ - public static async addPrivateServer( - userId: string, - serverName: string, - config: t.ParsedServerConfig, - ): Promise { - logger.info(`${this.prefix(serverName)} add private server to user with Id ${userId}`); - await registry.privateServersCache.add(userId, serverName, config); - } - - /** - * Handle permission changes - grant/revoke access. - * Use case: Admin shares/unshares server with users - * - * @param serverName - Server name - * @param allowedUserIds - Array of user IDs who should have access - * @param config - Server configuration - */ - public static async updatePrivateServerAccess( - serverName: string, - allowedUserIds: string[], - config: t.ParsedServerConfig, - ): Promise { - if (allowedUserIds.length === 0) { - // Revoke from everyone - logger.info(`${this.prefix(serverName)} Revoking access from all users`); - const allUsers = await registry.privateServersCache.findUsersWithServer(serverName); - await registry.privateServersCache.removeServerConfigIfCacheExists(allUsers, serverName); - return; - } - - logger.info(`${this.prefix(serverName)} Updating access for ${allowedUserIds.length} users`); - - // Find current state - const currentUsers = await registry.privateServersCache.findUsersWithServer(serverName); - const allowedSet = new Set(allowedUserIds); - - // Revoke from users no longer allowed - const toRevoke = currentUsers.filter((id) => !allowedSet.has(id)); - if (toRevoke.length > 0) { - logger.debug(`${this.prefix(serverName)} Revoking access from ${toRevoke.length} users`); - await registry.privateServersCache.removeServerConfigIfCacheExists(toRevoke, serverName); - } - - // Grant to allowed users (only initialized caches) - logger.debug(`${this.prefix(serverName)} Granting access to ${allowedUserIds.length} users`); - await registry.privateServersCache.addServerConfigIfCacheExists( - allowedUserIds, - serverName, - config, - ); - } - - /** - * Promote a private server to shared (public) registry. - * Use case: Admin shares a private server with PUBLIC - * - * Migrates server from private user caches to shared registry (app or user tier). - * Removes from all private caches to avoid duplication. - * - * @param serverName - Server name - * @param config - Server configuration - */ - public static async promoteToSharedServer( - serverName: string, - config: t.ParsedServerConfig, - ): Promise { - logger.info(`${this.prefix(serverName)} Promoting to shared server`); - - // 1. Add to shared registry (app or user tier based on config) - await registry.addSharedServer(serverName, config); - - // 2. Remove from all private user caches - const affectedUsers = await registry.privateServersCache.findUsersWithServer(serverName); - if (affectedUsers.length > 0) { - logger.debug( - `${this.prefix(serverName)} Removing from ${affectedUsers.length} private user caches`, - ); - await registry.privateServersCache.removeServerConfigIfCacheExists(affectedUsers, serverName); - } - - logger.info(`${this.prefix(serverName)} Successfully promoted to shared server`); - } - - /** - * Demote a shared server to private registry. - * Use case: Admin un-shares a server from PUBLIC - * - * Removes server from shared registry and adds to specified users' private caches. - * Only adds to users with initialized caches. - * - * @param serverName - Server name - * @param allowedUserIds - Array of user IDs who should have private access - * @param config - Server configuration - */ - public static async demoteToPrivateServer( - serverName: string, - allowedUserIds: string[], - config: t.ParsedServerConfig, - ): Promise { - logger.info(`${this.prefix(serverName)} Demoting to private server`); - - // 1. Remove from shared registries - await registry.removeServer(serverName); - - // 2. Add to private caches for allowed users (only if caches exist) - if (allowedUserIds.length > 0) { - logger.debug( - `${this.prefix(serverName)} Adding to ${allowedUserIds.length} users' private caches`, - ); - await registry.privateServersCache.addServerConfigIfCacheExists( - allowedUserIds, - serverName, - config, - ); - } - - logger.info(`${this.prefix(serverName)} Successfully demoted to private server`); - } - - // Returns formatted log prefix for server messages - private static prefix(serverName: string): string { - return `[MCP][PrivateServer][${serverName}]`; - } -} diff --git a/packages/api/src/mcp/registry/MCPServersInitializer.ts b/packages/api/src/mcp/registry/MCPServersInitializer.ts index e23cfd1ea9..5265a11d70 100644 --- a/packages/api/src/mcp/registry/MCPServersInitializer.ts +++ b/packages/api/src/mcp/registry/MCPServersInitializer.ts @@ -2,7 +2,6 @@ import { registryStatusCache as statusCache } from './cache/RegistryStatusCache' import { isLeader } from '~/cluster'; import { withTimeout } from '~/utils'; import { logger } from '@librechat/data-schemas'; -import { MCPServerInspector } from './MCPServerInspector'; import { ParsedServerConfig } from '~/mcp/types'; import { sanitizeUrlForLogging } from '~/mcp/utils'; import type * as t from '~/mcp/types'; @@ -60,9 +59,7 @@ 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 MCPServerInspector.inspect(serverName, rawConfig); - - await registry.addSharedServer(serverName, config); + const config = await registry.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 a9b3681c66..b99d4423c2 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -1,81 +1,69 @@ import type * as t from '~/mcp/types'; -import { - ServerConfigsCacheFactory, - type ServerConfigsCache, -} from './cache/ServerConfigsCacheFactory'; -import { - PrivateServerConfigsCache, - PrivateServerConfigsCacheFactory, -} from './cache/PrivateServerConfigs/PrivateServerConfigsCacheFactory'; +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 across different scopes and users. + * Central registry for managing MCP server configurations. * Authoritative source of truth for all MCP servers provided by LibreChat. * - * Maintains three-tier cache structure: - * - Shared App Servers: Auto-started servers available to all users (initialized at startup) - * - Shared User Servers: User-scope servers that require OAuth or on-demand startup - * - Private Servers: Per-user configurations dynamically added during runtime + * Uses a two-repository architecture: + * - Cache Repository: Stores YAML-defined configs loaded at startup (in-memory or Redis-backed) + * - DB Repository: Stores dynamic configs created at runtime (not yet implemented) * - * Provides a unified query interface with proper fallback hierarchy: - * checks shared app servers first, then shared user servers, then private user servers. + * Query priority: Cache configs are checked first, then DB configs. */ class MCPServersRegistry { - public readonly sharedAppServers: ServerConfigsCache = ServerConfigsCacheFactory.create( - 'App', - 'Shared', - false, - ); + private readonly dbConfigsRepo: IServerConfigsRepositoryInterface; + private readonly cacheConfigsRepo: IServerConfigsRepositoryInterface; - public readonly sharedUserServers: ServerConfigsCache = ServerConfigsCacheFactory.create( - 'User', - 'Shared', - false, - ); - - public readonly privateServersCache: PrivateServerConfigsCache = - PrivateServerConfigsCacheFactory.create(); - - private rawConfigs: t.MCPServers = {}; + constructor() { + this.dbConfigsRepo = new ServerConfigsDB(); + this.cacheConfigsRepo = ServerConfigsCacheFactory.create('App', false); + } public async getServerConfig( serverName: string, userId?: string, ): Promise { - const sharedAppServer = await this.sharedAppServers.get(serverName); - if (sharedAppServer) return sharedAppServer; - - if (userId) { - //we require user id to also access sharedServers to ensure that getServerConfig(serverName, undefined) returns only app level configs. - const sharedUserServer = await this.sharedUserServers.get(serverName); - if (sharedUserServer) return sharedUserServer; - - const privateUserServer = await this.privateServersCache.get(userId, serverName); - if (privateUserServer) return privateUserServer; - } - + // First we check if any config exist with the cache + // Yaml config are pre loaded to the cache + const configFromCache = await this.cacheConfigsRepo.get(serverName); + if (configFromCache) return configFromCache; + const configFromDB = await this.dbConfigsRepo.get(serverName, userId); + if (configFromDB) return configFromDB; return undefined; } public async getAllServerConfigs(userId?: string): Promise> { - const privateConfigs = userId ? await this.privateServersCache.getAll(userId) : {}; - const registryConfigs = { - ...(await this.sharedAppServers.getAll()), - ...(await this.sharedUserServers.getAll()), - ...privateConfigs, + return { + ...(await this.cacheConfigsRepo.getAll()), + ...(await this.dbConfigsRepo.getAll(userId)), }; - /** Include all raw configs, but registry configs take precedence (they have inspection data) */ - const allConfigs: Record = {}; - for (const serverName in this.rawConfigs) { - allConfigs[serverName] = this.rawConfigs[serverName] as t.ParsedServerConfig; - } + } - /** Override with registry configs where available (they have richer data) */ - for (const serverName in registryConfigs) { - allConfigs[serverName] = registryConfigs[serverName]; - } + public async addServer( + serverName: string, + config: t.MCPOptions, + storageLocation: 'CACHE' | 'DB', + userId?: string, + ): Promise { + const configRepo = this.getConfigRepository(storageLocation); + const parsedConfig = await MCPServerInspector.inspect(serverName, config); + await configRepo.add(serverName, parsedConfig, userId); + return parsedConfig; + } - return allConfigs; + public async updateServer( + serverName: string, + config: t.MCPOptions, + storageLocation: 'CACHE' | 'DB', + userId?: string, + ): Promise { + const configRepo = this.getConfigRepository(storageLocation); + const parsedConfig = await MCPServerInspector.inspect(serverName, config); + await configRepo.update(serverName, parsedConfig, userId); } // TODO: This is currently used to determine if a server requires OAuth. However, this info can @@ -86,57 +74,30 @@ class MCPServersRegistry { return new Set(oauthServers.map(([name]) => name)); } - /** - * Add a shared server configuration. - * Automatically routes to appropriate cache (app vs user) based on config properties. - * - Servers requiring OAuth or with startup=false → sharedUserServers - * - All other servers → sharedAppServers - * - * @param serverName - Name of the MCP server - * @param config - Parsed server configuration - */ - public async addSharedServer(serverName: string, config: t.ParsedServerConfig): Promise { - if (config.requiresOAuth || config.startup === false) { - await this.sharedUserServers.add(serverName, config); - } else { - await this.sharedAppServers.add(serverName, config); - } - } - public async reset(): Promise { - await this.sharedAppServers.reset(); - await this.sharedUserServers.reset(); - await this.privateServersCache.resetAll(); + await this.cacheConfigsRepo.reset(); } - public async removeServer(serverName: string, userId?: string): Promise { - const appServer = await this.sharedAppServers.get(serverName); - if (appServer) { - await this.sharedAppServers.remove(serverName); - return; - } + public async removeServer( + serverName: string, + storageLocation: 'CACHE' | 'DB', + userId?: string, + ): Promise { + const configRepo = this.getConfigRepository(storageLocation); + await configRepo.remove(serverName, userId); + } - const userServer = await this.sharedUserServers.get(serverName); - if (userServer) { - await this.sharedUserServers.remove(serverName); - return; + private getConfigRepository(storageLocation: 'CACHE' | 'DB'): IServerConfigsRepositoryInterface { + switch (storageLocation) { + case 'CACHE': + return this.cacheConfigsRepo; + case 'DB': + return this.dbConfigsRepo; + default: + throw new Error( + `MCPServersRegistry: The provided storage location "${storageLocation}" is not supported`, + ); } - - if (userId) { - const privateServer = await this.privateServersCache.get(userId, serverName); - if (privateServer) { - await this.privateServersCache.remove(userId, serverName); - return; - } - } else { - const affectedUsers = await this.privateServersCache.findUsersWithServer(serverName); - if (affectedUsers.length > 0) { - await this.privateServersCache.removeServerConfigIfCacheExists(affectedUsers, serverName); - return; - } - } - - throw new Error(`Server ${serverName} not found`); } } diff --git a/packages/api/src/mcp/registry/ServerConfigsRepositoryInterface.ts b/packages/api/src/mcp/registry/ServerConfigsRepositoryInterface.ts new file mode 100644 index 0000000000..9f8b2e25fe --- /dev/null +++ b/packages/api/src/mcp/registry/ServerConfigsRepositoryInterface.ts @@ -0,0 +1,22 @@ +import { ParsedServerConfig } from '~/mcp/types'; + +/** + * Interface for future DB implementation + */ +export interface IServerConfigsRepositoryInterface { + add(serverName: string, config: ParsedServerConfig, userId?: string): Promise; + + //ACL Entry check if update is possible + update(serverName: string, config: ParsedServerConfig, userId?: string): Promise; + + //ACL Entry check if remove is possible + remove(serverName: string, userId?: string): Promise; + + //ACL Entry check if read is possible + get(serverName: string, userId?: string): Promise; + + //ACL Entry get all accessible mcp config definitions + any mcp configured with agents + getAll(userId?: string): Promise>; + + reset(): Promise; +} diff --git a/packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.cache_integration.spec.ts b/packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.cache_integration.spec.ts deleted file mode 100644 index c50b613e9f..0000000000 --- a/packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.cache_integration.spec.ts +++ /dev/null @@ -1,593 +0,0 @@ -import type * as t from '~/mcp/types'; -import type { MCPServerDB } from 'librechat-data-provider'; - -describe('MCPPrivateServerLoader Cache Integration Tests', () => { - let keyvRedisClient: Awaited['keyvRedisClient']; - let registry: typeof import('../MCPServersRegistry').mcpServersRegistry; - let MCPPrivateServerLoader: typeof import('../MCPPrivateServerLoader').MCPPrivateServerLoader; - let loadStatusCache: typeof import('../cache/PrivateServersLoadStatusCache').privateServersLoadStatusCache; - const mockConfig1: t.ParsedServerConfig = { - command: 'node', - args: ['server1.js'], - env: { TEST: 'value1' }, - }; - - const mockConfig2: t.ParsedServerConfig = { - command: 'python', - args: ['server2.py'], - env: { TEST: 'value2' }, - }; - - const mockConfig3: t.ParsedServerConfig = { - url: 'http://localhost:3000', - requiresOAuth: true, - }; - - const mockUpdatedConfig: t.ParsedServerConfig = { - command: 'node', - args: ['server1-updated.js'], - env: { TEST: 'updated', NEW_VAR: 'added' }, - }; - beforeAll(async () => { - jest.resetModules(); - // Set up environment variables for Redis (only if not already set) - process.env.USE_REDIS = process.env.USE_REDIS ?? 'true'; - process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER ?? 'true'; - process.env.REDIS_URI = process.env.REDIS_URI ?? 'redis://127.0.0.1:6379'; - process.env.REDIS_KEY_PREFIX = - process.env.REDIS_KEY_PREFIX ?? `MCPPrivateServerLoader-integration-test`; - - // Import modules after setting env vars - const registryModule = await import('../MCPServersRegistry'); - const redisClients = await import('~/cache/redisClients'); - const MCPPrivateServerLoaderModule = await import('../MCPPrivateServerLoader'); - const loadStatusCacheLoaderModule = await import('../cache/PrivateServersLoadStatusCache'); - - loadStatusCache = loadStatusCacheLoaderModule.privateServersLoadStatusCache; - MCPPrivateServerLoader = MCPPrivateServerLoaderModule.MCPPrivateServerLoader; - registry = registryModule.mcpServersRegistry; - keyvRedisClient = redisClients.keyvRedisClient; - - if (!keyvRedisClient) throw new Error('Redis client is not initialized'); - - await redisClients.keyvRedisClientReady; - }); - - beforeEach(async () => { - // Clear load status for commonly used test users - // This ensures each test starts with a clean state - const testUsers = ['user1', 'user2', 'user3', 'user4', 'user5', 'user99']; - for (const userId of testUsers) { - try { - await loadStatusCache.clearLoaded(userId); - } catch { - // Ignore errors (user may not have load status set) - } - } - }); - - afterEach(async () => { - // Restore all mocks after each test - if (keyvRedisClient && 'scanIterator' in keyvRedisClient) { - const pattern = '*MCPPrivateServerLoader-integration-test*'; - const keysToDelete: string[] = []; - - // Collect all keys first - for await (const key of keyvRedisClient.scanIterator({ MATCH: pattern })) { - keysToDelete.push(key); - } - - // Delete in parallel for cluster mode efficiency - if (keysToDelete.length > 0) { - await Promise.all(keysToDelete.map((key) => keyvRedisClient!.del(key))); - } - } - jest.restoreAllMocks(); - }); - - afterAll(async () => { - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); - }); - - describe('loadPrivateServers() integration', () => { - it('should load private servers from configsLoader and cache them', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - // Verify servers were cached - const server1 = await registry.privateServersCache.get('user1', 'server1'); - const server2 = await registry.privateServersCache.get('user1', 'server2'); - - expect(server1).toMatchObject(mockConfig1); - expect(server2).toMatchObject(mockConfig2); - expect(configsLoader).toHaveBeenCalledWith('user1'); - }); - - it('should not reload servers if already cached for user', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - - // First load - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - expect(configsLoader).toHaveBeenCalledTimes(1); - - // Second load should skip - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - expect(configsLoader).toHaveBeenCalledTimes(1); // Still 1, not called again - }); - - it('should isolate servers between different users', async () => { - const user1Configs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - - const user2Configs: MCPServerDB[] = [ - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - const user1Loader = jest.fn().mockResolvedValue(user1Configs); - const user2Loader = jest.fn().mockResolvedValue(user2Configs); - - await MCPPrivateServerLoader.loadPrivateServers('user1', user1Loader); - await MCPPrivateServerLoader.loadPrivateServers('user2', user2Loader); - - // Verify isolation - const user1Server1 = await registry.privateServersCache.get('user1', 'server1'); - const user1Server2 = await registry.privateServersCache.get('user1', 'server2'); - const user2Server1 = await registry.privateServersCache.get('user2', 'server1'); - const user2Server2 = await registry.privateServersCache.get('user2', 'server2'); - - expect(user1Server1).toMatchObject(mockConfig1); - expect(user1Server2).toBeUndefined(); - expect(user2Server1).toBeUndefined(); - expect(user2Server2).toMatchObject(mockConfig2); - }); - - it('should handle partial failures gracefully', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - // Mock to fail on second server - let callCount = 0; - jest - .spyOn(registry.privateServersCache, 'add') - .mockImplementation(async (userId, serverName, config) => { - callCount++; - if (callCount === 2) { - throw new Error('Cache write failed for server2'); - } - // Call the real implementation for other calls - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const cache = (registry.privateServersCache as any).getOrCreatePrivateUserCache(userId); - return cache.add(serverName, config); - }); - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - - await expect( - MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader), - ).rejects.toThrow('Cache write failed for server2'); - - // Verify first server was added before failure - const server1 = await registry.privateServersCache.get('user1', 'server1'); - expect(server1).toMatchObject(mockConfig1); - - jest.restoreAllMocks(); - }); - - it('should reset cache before loading, removing old servers no longer in configs', async () => { - // Initial load with server1 and server2 - const initialConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - const initialLoader = jest.fn().mockResolvedValue(initialConfigs); - await MCPPrivateServerLoader.loadPrivateServers('user1', initialLoader); - - // Verify both servers are cached - expect(await registry.privateServersCache.get('user1', 'server1')).toBeDefined(); - expect(await registry.privateServersCache.get('user1', 'server2')).toBeDefined(); - - // Clear load status to force reload - await loadStatusCache.clearLoaded('user1'); - - // Second load with only server1 and a new server3 - const updatedConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-3', - mcp_id: 'server3', - config: mockConfig3, - }, - ]; - - const updatedLoader = jest.fn().mockResolvedValue(updatedConfigs); - await MCPPrivateServerLoader.loadPrivateServers('user1', updatedLoader); - - // Verify server2 is removed (cache was reset) - expect(await registry.privateServersCache.get('user1', 'server1')).toBeDefined(); - expect(await registry.privateServersCache.get('user1', 'server2')).toBeUndefined(); - expect(await registry.privateServersCache.get('user1', 'server3')).toBeDefined(); - }); - }); - - describe('updatePrivateServer() integration', () => { - beforeEach(async () => { - // Setup: Load same server for multiple users - const configs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - const loader = jest.fn().mockResolvedValue(configs); - - await MCPPrivateServerLoader.loadPrivateServers('user1', loader); - await MCPPrivateServerLoader.loadPrivateServers('user2', loader); - await MCPPrivateServerLoader.loadPrivateServers('user3', loader); - }); - - it('should update server config for all users who have it', async () => { - await MCPPrivateServerLoader.updatePrivateServer('server1', mockUpdatedConfig); - - // Verify all users got the update - const user1Server = await registry.privateServersCache.get('user1', 'server1'); - const user2Server = await registry.privateServersCache.get('user2', 'server1'); - const user3Server = await registry.privateServersCache.get('user3', 'server1'); - - expect(user1Server).toMatchObject({ - command: 'node', - args: ['server1-updated.js'], - env: { TEST: 'updated', NEW_VAR: 'added' }, - }); - expect(user2Server).toMatchObject(user1Server!); - expect(user3Server).toMatchObject(user1Server!); - }); - - it('should not affect other servers', async () => { - // Add another server to user1 - await registry.privateServersCache.add('user1', 'server2', mockConfig2); - - await MCPPrivateServerLoader.updatePrivateServer('server1', mockUpdatedConfig); - - // Verify server2 unchanged - const server2 = await registry.privateServersCache.get('user1', 'server2'); - expect(server2).toMatchObject(mockConfig2); - }); - - it('should handle updating non-existent server gracefully', async () => { - await expect( - MCPPrivateServerLoader.updatePrivateServer('non-existent-server', mockUpdatedConfig), - ).resolves.not.toThrow(); - }); - - it('should handle promoted server by updating shared registry instead of private caches', async () => { - // First add server to shared registry (simulating promotion) - await registry.addSharedServer('promoted-server', mockConfig1); - - // Verify it's in shared registry - const sharedConfig = await registry.getServerConfig('promoted-server'); - expect(sharedConfig).toBeDefined(); - - // Now update it - should go through the promoted server path - await MCPPrivateServerLoader.updatePrivateServer('promoted-server', mockUpdatedConfig); - - // Verify the shared registry was updated with new config - const updatedSharedConfig = await registry.getServerConfig('promoted-server'); - expect(updatedSharedConfig).toMatchObject( - expect.objectContaining({ - command: 'node', - args: ['server1-updated.js'], - env: { TEST: 'updated', NEW_VAR: 'added' }, - }), - ); - - // Verify users' private caches were NOT updated (no users had it privately) - expect(await registry.privateServersCache.get('user1', 'promoted-server')).toBeUndefined(); - expect(await registry.privateServersCache.get('user2', 'promoted-server')).toBeUndefined(); - expect(await registry.privateServersCache.get('user3', 'promoted-server')).toBeUndefined(); - }); - - it('should handle promoted server update when users had it privately before promotion', async () => { - // Setup: Users have server1 in private caches - const configs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - const loader = jest.fn().mockResolvedValue(configs); - - await MCPPrivateServerLoader.loadPrivateServers('user1', loader); - await MCPPrivateServerLoader.loadPrivateServers('user2', loader); - - // Verify they have it privately - expect(await registry.privateServersCache.get('user1', 'server1')).toBeDefined(); - expect(await registry.privateServersCache.get('user2', 'server1')).toBeDefined(); - - await MCPPrivateServerLoader.promoteToSharedServer('server1', mockConfig1); - // Verify it's now in shared registry and removed from private caches - expect(await registry.getServerConfig('server1')).toBeDefined(); - expect(await registry.privateServersCache.get('user1', 'server1')).toBeUndefined(); - expect(await registry.privateServersCache.get('user2', 'server1')).toBeUndefined(); - - // Now update it - should update shared registry - await MCPPrivateServerLoader.updatePrivateServer('server1', mockUpdatedConfig); - - // Verify shared registry was updated - const updatedSharedConfig = await registry.getServerConfig('server1'); - expect(updatedSharedConfig).toMatchObject( - expect.objectContaining({ - command: 'node', - args: ['server1-updated.js'], - }), - ); - }); - }); - - describe('updatePrivateServerAccess() integration', () => { - beforeEach(async () => { - // Setup: Load server for user1, user2, user3 - const configs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - const loader = jest.fn().mockResolvedValue(configs); - - await MCPPrivateServerLoader.loadPrivateServers('user1', loader); - await MCPPrivateServerLoader.loadPrivateServers('user2', loader); - await MCPPrivateServerLoader.loadPrivateServers('user3', loader); - - // Also initialize cache for user4 and user5 but without server1 - await registry.privateServersCache.add('user4', 'other-server', mockConfig2); - await registry.privateServersCache.add('user5', 'other-server', mockConfig2); - }); - - it('should revoke access from all users when allowedUserIds is empty', async () => { - await MCPPrivateServerLoader.updatePrivateServerAccess('server1', [], mockConfig1); - - // Verify all users lost access - expect(await registry.privateServersCache.get('user1', 'server1')).toBeUndefined(); - expect(await registry.privateServersCache.get('user2', 'server1')).toBeUndefined(); - expect(await registry.privateServersCache.get('user3', 'server1')).toBeUndefined(); - }); - - it('should grant access to new users with initialized caches', async () => { - const allowedUserIds = ['user1', 'user4', 'user5']; - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // user1 should still have it - expect(await registry.privateServersCache.get('user1', 'server1')).toBeDefined(); - - // user4 and user5 should now have it (they had initialized caches) - expect(await registry.privateServersCache.get('user4', 'server1')).toBeDefined(); - expect(await registry.privateServersCache.get('user5', 'server1')).toBeDefined(); - - // user2 and user3 should have lost access - expect(await registry.privateServersCache.get('user2', 'server1')).toBeUndefined(); - expect(await registry.privateServersCache.get('user3', 'server1')).toBeUndefined(); - }); - - it('should not grant access to users without initialized caches', async () => { - const allowedUserIds = ['user1', 'user99']; // user99 has no cache - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // user1 should still have it - expect(await registry.privateServersCache.get('user1', 'server1')).toBeDefined(); - - // user99 should not have it (cache not initialized) - expect(await registry.privateServersCache.get('user99', 'server1')).toBeUndefined(); - }); - - it('should handle complex permission changes', async () => { - // Start: user1, user2, user3 have server1 - // End: user2, user4, user5 should have server1 - - const allowedUserIds = ['user2', 'user4', 'user5']; - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // Revoked: user1, user3 - expect(await registry.privateServersCache.get('user1', 'server1')).toBeUndefined(); - expect(await registry.privateServersCache.get('user3', 'server1')).toBeUndefined(); - - // Kept: user2 - expect(await registry.privateServersCache.get('user2', 'server1')).toBeDefined(); - - // Granted: user4, user5 - expect(await registry.privateServersCache.get('user4', 'server1')).toBeDefined(); - expect(await registry.privateServersCache.get('user5', 'server1')).toBeDefined(); - }); - - it('should be idempotent when called with same permissions', async () => { - const allowedUserIds = ['user1', 'user2']; - - // First call - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { lastUpdatedAt: _1, ...user1ServerAfterFirst } = - (await registry.privateServersCache.get('user1', 'server1')) || {}; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { lastUpdatedAt: _2, ...user2ServerAfterFirst } = - (await registry.privateServersCache.get('user2', 'server1')) || {}; - // Second call with same permissions - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { lastUpdatedAt: _3, ...user1ServerAfterSecond } = - (await registry.privateServersCache.get('user1', 'server1')) || {}; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { lastUpdatedAt: _4, ...user2ServerAfterSecond } = - (await registry.privateServersCache.get('user2', 'server1')) || {}; - - // Should be unchanged - expect(user1ServerAfterSecond).toMatchObject(user1ServerAfterFirst!); - expect(user2ServerAfterSecond).toMatchObject(user2ServerAfterFirst!); - }); - }); - - describe('Combined operations integration', () => { - it('should handle load, update metadata, and update access in sequence', async () => { - // 1. Load servers for user1 and user2 - const configs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - const loader = jest.fn().mockResolvedValue(configs); - - await MCPPrivateServerLoader.loadPrivateServers('user1', loader); - await MCPPrivateServerLoader.loadPrivateServers('user2', loader); - - // Verify initial state - let user1Server = await registry.privateServersCache.get('user1', 'server1'); - let user2Server = await registry.privateServersCache.get('user2', 'server1'); - expect((user1Server as typeof mockConfig1)?.args).toEqual(['server1.js']); - expect((user2Server as typeof mockConfig1)?.args).toEqual(['server1.js']); - - // 2. Update metadata for all users - await MCPPrivateServerLoader.updatePrivateServer('server1', mockUpdatedConfig); - - user1Server = await registry.privateServersCache.get('user1', 'server1'); - user2Server = await registry.privateServersCache.get('user2', 'server1'); - - expect((user1Server as typeof mockUpdatedConfig)?.args).toEqual(['server1-updated.js']); - expect((user2Server as typeof mockUpdatedConfig)?.args).toEqual(['server1-updated.js']); - - // 3. Update access - revoke from user1 - await MCPPrivateServerLoader.updatePrivateServerAccess('server1', ['user2'], mockConfig3); - - user1Server = await registry.privateServersCache.get('user1', 'server1'); - user2Server = await registry.privateServersCache.get('user2', 'server1'); - - expect(user1Server).toBeUndefined(); - expect(user2Server).toBeDefined(); - }); - - it('should handle concurrent user logins correctly', async () => { - const user1Configs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - const user2Configs: MCPServerDB[] = [ - { - _id: 'mock-id-2', - mcp_id: 'server1', - config: mockConfig2, - }, - ]; - const user3Configs: MCPServerDB[] = [ - { - _id: 'mock-id-3', - mcp_id: 'server2', - config: mockConfig3, - }, - ]; - - const user1Loader = jest.fn().mockResolvedValue(user1Configs); - const user2Loader = jest.fn().mockResolvedValue(user2Configs); - const user3Loader = jest.fn().mockResolvedValue(user3Configs); - - // Simulate concurrent loads - await Promise.all([ - MCPPrivateServerLoader.loadPrivateServers('user1', user1Loader), - MCPPrivateServerLoader.loadPrivateServers('user2', user2Loader), - MCPPrivateServerLoader.loadPrivateServers('user3', user3Loader), - ]); - - // Verify all users got their configs - const user1Server1 = await registry.privateServersCache.get('user1', 'server1'); - const user2Server1 = await registry.privateServersCache.get('user2', 'server1'); - const user3Server2 = await registry.privateServersCache.get('user3', 'server2'); - - expect(user1Server1).toMatchObject(mockConfig1); - expect(user2Server1).toMatchObject(mockConfig2); - expect(user3Server2).toMatchObject(mockConfig3); - }); - }); -}); diff --git a/packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.test.ts b/packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.test.ts deleted file mode 100644 index 59435daf07..0000000000 --- a/packages/api/src/mcp/registry/__tests__/MCPPrivateServerLoader.test.ts +++ /dev/null @@ -1,702 +0,0 @@ -import { mcpServersRegistry as registry } from '../MCPServersRegistry'; -import { privateServersLoadStatusCache as loadStatusCache } from '../cache/PrivateServersLoadStatusCache'; -import { MCPPrivateServerLoader } from '../MCPPrivateServerLoader'; -import { logger } from '@librechat/data-schemas'; -import type * as t from '~/mcp/types'; -import type { MCPServerDB } from 'librechat-data-provider'; - -// Mock dependencies -jest.mock('../MCPServersRegistry', () => ({ - mcpServersRegistry: { - privateServersCache: { - get: jest.fn(), - add: jest.fn(), - reset: jest.fn(), - updateServerConfigIfExists: jest.fn(), - findUsersWithServer: jest.fn(), - removeServerConfigIfCacheExists: jest.fn(), - addServerConfigIfCacheExists: jest.fn(), - reset: jest.fn(), - }, - getServerConfig: jest.fn(), - addSharedServer: jest.fn(), - removeServer: jest.fn(), - getServerConfig: jest.fn(), - }, -})); - -jest.mock('../cache/PrivateServersLoadStatusCache', () => ({ - privateServersLoadStatusCache: { - isLoaded: jest.fn(), - setLoaded: jest.fn(), - acquireLoadLock: jest.fn(), - releaseLoadLock: jest.fn(), - waitForLoad: jest.fn(), - }, -})); - -jest.mock('@librechat/data-schemas', () => ({ - logger: { - debug: jest.fn(), - info: jest.fn(), - warn: jest.fn(), - error: jest.fn(), - }, -})); - -describe('MCPPrivateServerLoader', () => { - const mockConfig1: t.ParsedServerConfig = { - command: 'node', - args: ['server1.js'], - env: { TEST: 'value1' }, - lastUpdatedAt: Date.now(), - }; - - const mockConfig2: t.ParsedServerConfig = { - command: 'python', - args: ['server2.py'], - env: { TEST: 'value2' }, - lastUpdatedAt: Date.now(), - }; - - beforeEach(() => { - jest.clearAllMocks(); - }); - - describe('loadPrivateServers()', () => { - it('should validate userId and throw error if empty', async () => { - const configsLoader = jest.fn(); - - await expect(MCPPrivateServerLoader.loadPrivateServers('', configsLoader)).rejects.toThrow( - 'userId is required and cannot be empty', - ); - - await expect(MCPPrivateServerLoader.loadPrivateServers(' ', configsLoader)).rejects.toThrow( - 'userId is required and cannot be empty', - ); - }); - - it('should validate configsLoader and throw error if not a function', async () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await expect(MCPPrivateServerLoader.loadPrivateServers('user1', null as any)).rejects.toThrow( - 'configsLoader must be a function', - ); - - await expect( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - MCPPrivateServerLoader.loadPrivateServers('user1', 'not-a-function' as any), - ).rejects.toThrow('configsLoader must be a function'); - }); - - it('should skip loading if user servers are already loaded', async () => { - const configsLoader = jest.fn(); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(true); - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - expect(loadStatusCache.isLoaded).toHaveBeenCalledWith('user1'); - expect(configsLoader).not.toHaveBeenCalled(); - expect(loadStatusCache.acquireLoadLock).not.toHaveBeenCalled(); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][PrivateServerLoader] User user1 private servers already loaded', - ); - }); - - it('should load private servers for a user successfully', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - (registry.privateServersCache.get as jest.Mock).mockResolvedValue(undefined); - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - expect(loadStatusCache.acquireLoadLock).toHaveBeenCalledWith('user1'); - expect(configsLoader).toHaveBeenCalledWith('user1'); - expect(registry.privateServersCache.add).toHaveBeenCalledWith('user1', 'server1', { - ...mockConfig1, - dbId: 'mock-id-1', - }); - expect(registry.privateServersCache.add).toHaveBeenCalledWith('user1', 'server2', { - ...mockConfig2, - dbId: 'mock-id-2', - }); - expect(loadStatusCache.setLoaded).toHaveBeenCalledWith('user1', 3600_000); - expect(loadStatusCache.releaseLoadLock).toHaveBeenCalledWith('user1'); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServerLoader] Loading private servers for user user1', - ); - }); - - it('should skip servers that already exist in cache', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - (registry.privateServersCache.get as jest.Mock) - .mockResolvedValueOnce(mockConfig1) // server1 exists - .mockResolvedValueOnce(undefined); // server2 doesn't exist - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - expect(registry.privateServersCache.add).toHaveBeenCalledTimes(1); - expect(registry.privateServersCache.add).toHaveBeenCalledWith('user1', 'server2', { - ...mockConfig2, - dbId: 'mock-id-2', - }); - expect(loadStatusCache.setLoaded).toHaveBeenCalledWith('user1', 3600_000); - expect(loadStatusCache.releaseLoadLock).toHaveBeenCalledWith('user1'); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Private server already exists for user user1', - ); - }); - - it('should throw error if configsLoader fails', async () => { - const configsLoader = jest.fn().mockRejectedValue(new Error('DB connection failed')); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - - await expect( - MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader), - ).rejects.toThrow('DB connection failed'); - - expect(logger.error).toHaveBeenCalledWith( - '[MCP][PrivateServerLoader] Loading private servers for user user1 failed.', - expect.any(Error), - ); - expect(loadStatusCache.setLoaded).not.toHaveBeenCalled(); - expect(loadStatusCache.releaseLoadLock).toHaveBeenCalledWith('user1'); - }); - - it('should throw error if cache.add fails', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - (registry.privateServersCache.get as jest.Mock).mockResolvedValue(undefined); - (registry.privateServersCache.add as jest.Mock).mockRejectedValue( - new Error('Cache write failed'), - ); - - await expect( - MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader), - ).rejects.toThrow('Cache write failed'); - expect(loadStatusCache.setLoaded).not.toHaveBeenCalled(); - expect(loadStatusCache.releaseLoadLock).toHaveBeenCalledWith('user1'); - }); - - it('should handle empty configs gracefully', async () => { - const mockConfigs: MCPServerDB[] = []; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - expect(registry.privateServersCache.add).not.toHaveBeenCalled(); - expect(loadStatusCache.setLoaded).toHaveBeenCalledWith('user1', 3600_000); - expect(loadStatusCache.releaseLoadLock).toHaveBeenCalledWith('user1'); - }); - - it('should prevent partial loads after crash - loaded flag not set on failure', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - { - _id: 'mock-id-2', - mcp_id: 'server2', - config: mockConfig2, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - (registry.privateServersCache.get as jest.Mock).mockResolvedValue(undefined); - - // Simulate crash after loading first server - (registry.privateServersCache.add as jest.Mock) - .mockResolvedValueOnce(undefined) // server1 succeeds - .mockRejectedValueOnce(new Error('Process crashed')); // server2 fails - - await expect( - MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader), - ).rejects.toThrow('Process crashed'); - - // Loaded flag should NOT be set - expect(loadStatusCache.setLoaded).not.toHaveBeenCalled(); - // Lock should be released even on error - expect(loadStatusCache.releaseLoadLock).toHaveBeenCalledWith('user1'); - - // On next call, should retry full load - jest.clearAllMocks(); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(true); - (registry.privateServersCache.add as jest.Mock).mockResolvedValue(undefined); - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - // Now flag should be set - expect(loadStatusCache.setLoaded).toHaveBeenCalledWith('user1', 3600_000); - }); - - it('should wait for another process when lock is already held', async () => { - const configsLoader = jest.fn(); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(false); // Lock held - (loadStatusCache.waitForLoad as jest.Mock).mockResolvedValue(true); // Wait completes - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - expect(loadStatusCache.acquireLoadLock).toHaveBeenCalledWith('user1'); - expect(loadStatusCache.waitForLoad).toHaveBeenCalledWith('user1'); - expect(configsLoader).not.toHaveBeenCalled(); // Didn't load, waited instead - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][PrivateServerLoader] Another process is loading user user1, waiting...', - ); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][PrivateServerLoader] User user1 loaded by another process', - ); - }); - - it('should retry lock acquisition after wait timeout', async () => { - const mockConfigs: MCPServerDB[] = [ - { - _id: 'mock-id-1', - mcp_id: 'server1', - config: mockConfig1, - }, - ]; - - const configsLoader = jest.fn().mockResolvedValue(mockConfigs); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock) - .mockResolvedValueOnce(false) // First attempt: lock held - .mockResolvedValueOnce(true); // Retry after timeout: success - (loadStatusCache.waitForLoad as jest.Mock).mockResolvedValue(false); // Wait times out - (registry.privateServersCache.get as jest.Mock).mockResolvedValue(undefined); - - await MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader); - - expect(loadStatusCache.acquireLoadLock).toHaveBeenCalledTimes(2); - expect(loadStatusCache.waitForLoad).toHaveBeenCalledWith('user1'); - expect(configsLoader).toHaveBeenCalled(); // Loaded after retry - expect(logger.warn).toHaveBeenCalledWith( - '[MCP][PrivateServerLoader] Timeout waiting for user user1, retrying lock acquisition', - ); - }); - - it('should throw error if retry lock acquisition fails', async () => { - const configsLoader = jest.fn(); - (loadStatusCache.isLoaded as jest.Mock).mockResolvedValue(false); - (loadStatusCache.acquireLoadLock as jest.Mock).mockResolvedValue(false); // Both attempts fail - (loadStatusCache.waitForLoad as jest.Mock).mockResolvedValue(false); // Wait times out - - await expect( - MCPPrivateServerLoader.loadPrivateServers('user1', configsLoader), - ).rejects.toThrow('Failed to acquire load lock for user user1'); - - expect(loadStatusCache.acquireLoadLock).toHaveBeenCalledTimes(2); - expect(configsLoader).not.toHaveBeenCalled(); - }); - }); - - describe('updatePrivateServer()', () => { - it('should propagate metadata update to all users when server is not promoted', async () => { - (registry.getServerConfig as jest.Mock).mockResolvedValue(undefined); - - await MCPPrivateServerLoader.updatePrivateServer('server1', mockConfig2); - - expect(registry.getServerConfig).toHaveBeenCalledWith('server1'); - expect(registry.privateServersCache.updateServerConfigIfExists).toHaveBeenCalledWith( - 'server1', - mockConfig2, - ); - expect(registry.removeServer).not.toHaveBeenCalled(); - expect(registry.addSharedServer).not.toHaveBeenCalled(); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Propagating metadata update to all users', - ); - }); - - it('should handle promoted server by updating shared registry', async () => { - const sharedConfig: t.ParsedServerConfig = { - command: 'node', - args: ['shared.js'], - env: { SHARED: 'true' }, - lastUpdatedAt: Date.now(), - }; - - (registry.getServerConfig as jest.Mock).mockResolvedValue(sharedConfig); - - await MCPPrivateServerLoader.updatePrivateServer('server1', mockConfig2); - - expect(registry.getServerConfig).toHaveBeenCalledWith('server1'); - expect(registry.removeServer).toHaveBeenCalledWith('server1'); - expect(registry.addSharedServer).toHaveBeenCalledWith('server1', mockConfig2); - expect(registry.privateServersCache.updateServerConfigIfExists).not.toHaveBeenCalled(); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Promoted private server update', - ); - }); - - it('should throw error if updateServerConfigIfExists fails', async () => { - (registry.getServerConfig as jest.Mock).mockResolvedValue(undefined); - (registry.privateServersCache.updateServerConfigIfExists as jest.Mock).mockRejectedValue( - new Error('Redis update failed'), - ); - - await expect( - MCPPrivateServerLoader.updatePrivateServer('server1', mockConfig2), - ).rejects.toThrow('Redis update failed'); - }); - - it('should throw error if removeServer fails for promoted server', async () => { - (registry.getServerConfig as jest.Mock).mockResolvedValue(mockConfig1); - (registry.removeServer as jest.Mock).mockRejectedValue(new Error('Remove server failed')); - - await expect( - MCPPrivateServerLoader.updatePrivateServer('server1', mockConfig2), - ).rejects.toThrow('Remove server failed'); - }); - - it('should throw error if addSharedServer fails for promoted server', async () => { - (registry.getServerConfig as jest.Mock).mockResolvedValue(mockConfig1); - (registry.removeServer as jest.Mock).mockResolvedValue(undefined); // Ensure removeServer succeeds - (registry.addSharedServer as jest.Mock).mockRejectedValue( - new Error('Add shared server failed'), - ); - - await expect( - MCPPrivateServerLoader.updatePrivateServer('server1', mockConfig2), - ).rejects.toThrow('Add shared server failed'); - }); - }); - - describe('updatePrivateServerAccess()', () => { - it('should revoke access from all users when allowedUserIds is empty', async () => { - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([ - 'user1', - 'user2', - 'user3', - ]); - - await MCPPrivateServerLoader.updatePrivateServerAccess('server1', [], mockConfig1); - - expect(registry.privateServersCache.findUsersWithServer).toHaveBeenCalledWith('server1'); - expect(registry.privateServersCache.removeServerConfigIfCacheExists).toHaveBeenCalledWith( - ['user1', 'user2', 'user3'], - 'server1', - ); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Revoking access from all users', - ); - }); - - it('should grant access to new users and revoke from removed users', async () => { - const allowedUserIds = ['user1', 'user2', 'user4']; - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([ - 'user1', - 'user2', - 'user3', - ]); - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // Should revoke from user3 (no longer in allowed list) - expect(registry.privateServersCache.removeServerConfigIfCacheExists).toHaveBeenCalledWith( - ['user3'], - 'server1', - ); - - // Should grant to all allowed users (includes existing and new) - expect(registry.privateServersCache.addServerConfigIfCacheExists).toHaveBeenCalledWith( - allowedUserIds, - 'server1', - mockConfig1, - ); - - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Updating access for 3 users', - ); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Revoking access from 1 users', - ); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Granting access to 3 users', - ); - }); - - it('should only grant access when no users currently have the server', async () => { - const allowedUserIds = ['user1', 'user2']; - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([]); - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - expect(registry.privateServersCache.removeServerConfigIfCacheExists).not.toHaveBeenCalled(); - expect(registry.privateServersCache.addServerConfigIfCacheExists).toHaveBeenCalledWith( - allowedUserIds, - 'server1', - mockConfig1, - ); - }); - - it('should only revoke access when granting to users who already have it', async () => { - const allowedUserIds = ['user1', 'user2']; - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([ - 'user1', - 'user2', - ]); - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // No one to revoke - expect(registry.privateServersCache.removeServerConfigIfCacheExists).not.toHaveBeenCalled(); - - // Still grant (idempotent) - expect(registry.privateServersCache.addServerConfigIfCacheExists).toHaveBeenCalledWith( - allowedUserIds, - 'server1', - mockConfig1, - ); - }); - - it('should handle completely new access list', async () => { - const allowedUserIds = ['user4', 'user5', 'user6']; - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([ - 'user1', - 'user2', - 'user3', - ]); - - await MCPPrivateServerLoader.updatePrivateServerAccess( - 'server1', - allowedUserIds, - mockConfig1, - ); - - // Revoke from all current users - expect(registry.privateServersCache.removeServerConfigIfCacheExists).toHaveBeenCalledWith( - ['user1', 'user2', 'user3'], - 'server1', - ); - - // Grant to all new users - expect(registry.privateServersCache.addServerConfigIfCacheExists).toHaveBeenCalledWith( - allowedUserIds, - 'server1', - mockConfig1, - ); - }); - - it('should throw error if findUsersWithServer fails', async () => { - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockRejectedValue( - new Error('Redis scan failed'), - ); - - await expect( - MCPPrivateServerLoader.updatePrivateServerAccess('server1', [], mockConfig1), - ).rejects.toThrow('Redis scan failed'); - }); - }); - - describe('promoteToSharedServer()', () => { - it('should promote private server to shared registry and remove from private caches', async () => { - (registry.addSharedServer as jest.Mock).mockResolvedValue(undefined); - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([ - 'user1', - 'user2', - 'user3', - ]); - - await MCPPrivateServerLoader.promoteToSharedServer('server1', mockConfig1); - - // Should add to shared registry - expect(registry.addSharedServer).toHaveBeenCalledWith('server1', mockConfig1); - - // Should remove from all private user caches - expect(registry.privateServersCache.removeServerConfigIfCacheExists).toHaveBeenCalledWith( - ['user1', 'user2', 'user3'], - 'server1', - ); - - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Promoting to shared server', - ); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Successfully promoted to shared server', - ); - }); - - it('should handle promoting when no users have the server privately', async () => { - (registry.addSharedServer as jest.Mock).mockResolvedValue(undefined); - (registry.privateServersCache.findUsersWithServer as jest.Mock).mockResolvedValue([]); - - await MCPPrivateServerLoader.promoteToSharedServer('server1', mockConfig1); - - expect(registry.addSharedServer).toHaveBeenCalledWith('server1', mockConfig1); - expect(registry.privateServersCache.removeServerConfigIfCacheExists).not.toHaveBeenCalled(); - }); - - it('should throw error if addSharedServer fails', async () => { - (registry.addSharedServer as jest.Mock).mockRejectedValue( - new Error('Failed to add to shared registry'), - ); - - await expect( - MCPPrivateServerLoader.promoteToSharedServer('server1', mockConfig1), - ).rejects.toThrow('Failed to add to shared registry'); - }); - }); - - describe('demoteToPrivateServer()', () => { - it('should demote shared server to private caches for specified users', async () => { - const allowedUserIds = ['user1', 'user2', 'user3']; - (registry.removeServer as jest.Mock).mockResolvedValue(undefined); - - await MCPPrivateServerLoader.demoteToPrivateServer('server1', allowedUserIds, mockConfig1); - - // Should remove from shared registry - expect(registry.removeServer).toHaveBeenCalledWith('server1'); - - // Should add to private caches for allowed users - expect(registry.privateServersCache.addServerConfigIfCacheExists).toHaveBeenCalledWith( - allowedUserIds, - 'server1', - mockConfig1, - ); - - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Demoting to private server', - ); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] Successfully demoted to private server', - ); - }); - - it('should handle demoting with empty user list', async () => { - (registry.removeServer as jest.Mock).mockResolvedValue(undefined); - - await MCPPrivateServerLoader.demoteToPrivateServer('server1', [], mockConfig1); - - expect(registry.removeServer).toHaveBeenCalledWith('server1'); - expect(registry.privateServersCache.addServerConfigIfCacheExists).not.toHaveBeenCalled(); - }); - - it('should throw error if removeServer fails', async () => { - (registry.removeServer as jest.Mock).mockRejectedValue( - new Error('Server not found in shared registry'), - ); - - await expect( - MCPPrivateServerLoader.demoteToPrivateServer('server1', ['user1'], mockConfig1), - ).rejects.toThrow('Server not found in shared registry'); - }); - }); - - describe('addPrivateServer()', () => { - it('should add private server for a user', async () => { - await MCPPrivateServerLoader.addPrivateServer('user1', 'server1', mockConfig1); - - expect(registry.privateServersCache.add).toHaveBeenCalledWith( - 'user1', - 'server1', - mockConfig1, - ); - expect(logger.info).toHaveBeenCalledWith( - '[MCP][PrivateServer][server1] add private server to user with Id user1', - ); - }); - - it('should add private server with different configs for different users', async () => { - await MCPPrivateServerLoader.addPrivateServer('user1', 'server1', mockConfig1); - await MCPPrivateServerLoader.addPrivateServer('user2', 'server2', mockConfig2); - - expect(registry.privateServersCache.add).toHaveBeenCalledWith( - 'user1', - 'server1', - mockConfig1, - ); - expect(registry.privateServersCache.add).toHaveBeenCalledWith( - 'user2', - 'server2', - mockConfig2, - ); - }); - - it('should throw error if cache add fails', async () => { - (registry.privateServersCache.add as jest.Mock).mockRejectedValue( - new Error('Cache write failed'), - ); - - await expect( - MCPPrivateServerLoader.addPrivateServer('user1', 'server1', mockConfig1), - ).rejects.toThrow('Cache write failed'); - }); - - it('should handle adding server with complex config', async () => { - const complexConfig: t.ParsedServerConfig = { - url: 'https://api.example.com', - requiresOAuth: true, - lastUpdatedAt: Date.now(), - }; - - // Reset the mock to ensure it succeeds for this test - (registry.privateServersCache.add as jest.Mock).mockResolvedValue(undefined); - - await MCPPrivateServerLoader.addPrivateServer('user1', 'complex-server', complexConfig); - - expect(registry.privateServersCache.add).toHaveBeenCalledWith( - 'user1', - 'complex-server', - complexConfig, - ); - }); - }); -}); 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 703758c959..22a1542671 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 @@ -213,20 +213,20 @@ describe('MCPServersInitializer Redis Integration Tests', () => { describe('initialize()', () => { it('should reset registry and status cache before initialization', async () => { - // Pre-populate registry with some old servers - await registry.sharedAppServers.add('old_app_server', testParsedConfigs.file_tools_server); - await registry.sharedUserServers.add('old_user_server', testParsedConfigs.oauth_server); + // Pre-populate registry with some old servers using public API + await registry.addServer('old_app_server', testConfigs.file_tools_server, 'CACHE'); + await registry.addServer('old_user_server', testConfigs.oauth_server, 'CACHE'); // Initialize with new configs (this should reset first) await MCPServersInitializer.initialize(testConfigs); // Verify old servers are gone - expect(await registry.sharedAppServers.get('old_app_server')).toBeUndefined(); - expect(await registry.sharedUserServers.get('old_user_server')).toBeUndefined(); + expect(await registry.getServerConfig('old_app_server')).toBeUndefined(); + expect(await registry.getServerConfig('old_user_server')).toBeUndefined(); // Verify new servers are present - expect(await registry.sharedAppServers.get('file_tools_server')).toBeDefined(); - expect(await registry.sharedUserServers.get('oauth_server')).toBeDefined(); + expect(await registry.getServerConfig('file_tools_server')).toBeDefined(); + expect(await registry.getServerConfig('oauth_server')).toBeDefined(); expect(await registryStatusCache.isInitialized()).toBe(true); }); @@ -244,54 +244,14 @@ describe('MCPServersInitializer Redis Integration Tests', () => { expect(MCPServerInspector.inspect).not.toHaveBeenCalled(); }); - it('should add disabled servers to sharedUserServers', async () => { + it('should initialize all servers to cache repository', async () => { await MCPServersInitializer.initialize(testConfigs); - const disabledServer = await registry.sharedUserServers.get('disabled_server'); - expect(disabledServer).toBeDefined(); - expect(disabledServer).toMatchObject({ - ...testParsedConfigs.disabled_server, - _processedByInspector: true, - }); - }); - - it('should add OAuth servers to sharedUserServers', async () => { - await MCPServersInitializer.initialize(testConfigs); - - const oauthServer = await registry.sharedUserServers.get('oauth_server'); - expect(oauthServer).toBeDefined(); - expect(oauthServer).toMatchObject({ - ...testParsedConfigs.oauth_server, - _processedByInspector: true, - }); - }); - - it('should add enabled non-OAuth servers to sharedAppServers', async () => { - await MCPServersInitializer.initialize(testConfigs); - - const fileToolsServer = await registry.sharedAppServers.get('file_tools_server'); - expect(fileToolsServer).toBeDefined(); - expect(fileToolsServer).toMatchObject({ - ...testParsedConfigs.file_tools_server, - _processedByInspector: true, - }); - - const searchToolsServer = await registry.sharedAppServers.get('search_tools_server'); - expect(searchToolsServer).toBeDefined(); - expect(searchToolsServer).toMatchObject({ - ...testParsedConfigs.search_tools_server, - _processedByInspector: true, - }); - }); - - it('should successfully initialize all servers', async () => { - await MCPServersInitializer.initialize(testConfigs); - - // Verify all servers were added to appropriate registries - expect(await registry.sharedUserServers.get('disabled_server')).toBeDefined(); - expect(await registry.sharedUserServers.get('oauth_server')).toBeDefined(); - expect(await registry.sharedAppServers.get('file_tools_server')).toBeDefined(); - expect(await registry.sharedAppServers.get('search_tools_server')).toBeDefined(); + // Verify all server types (disabled, OAuth, and regular) were added to cache + expect(await registry.getServerConfig('disabled_server')).toBeDefined(); + expect(await registry.getServerConfig('oauth_server')).toBeDefined(); + expect(await registry.getServerConfig('file_tools_server')).toBeDefined(); + expect(await registry.getServerConfig('search_tools_server')).toBeDefined(); }); it('should handle inspection failures gracefully', async () => { @@ -309,17 +269,17 @@ describe('MCPServersInitializer Redis Integration Tests', () => { await MCPServersInitializer.initialize(testConfigs); // Verify other servers were still processed - const disabledServer = await registry.sharedUserServers.get('disabled_server'); + const disabledServer = await registry.getServerConfig('disabled_server'); expect(disabledServer).toBeDefined(); - const oauthServer = await registry.sharedUserServers.get('oauth_server'); + const oauthServer = await registry.getServerConfig('oauth_server'); expect(oauthServer).toBeDefined(); - const searchToolsServer = await registry.sharedAppServers.get('search_tools_server'); + const searchToolsServer = await registry.getServerConfig('search_tools_server'); expect(searchToolsServer).toBeDefined(); // Verify file_tools_server was not added (due to inspection failure) - const fileToolsServer = await registry.sharedAppServers.get('file_tools_server'); + const fileToolsServer = await registry.getServerConfig('file_tools_server'); expect(fileToolsServer).toBeUndefined(); }); diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts index a52d9c21b8..1c259d8da8 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts @@ -157,8 +157,7 @@ describe('MCPServersInitializer', () => { // Reset caches before each test await registryStatusCache.reset(); - await registry.sharedAppServers.reset(); - await registry.sharedUserServers.reset(); + await registry.reset(); jest.clearAllMocks(); }); @@ -168,20 +167,20 @@ describe('MCPServersInitializer', () => { describe('initialize()', () => { it('should reset registry and status cache before initialization', async () => { - // Pre-populate registry with some old servers - await registry.sharedAppServers.add('old_app_server', testParsedConfigs.file_tools_server); - await registry.sharedUserServers.add('old_user_server', testParsedConfigs.oauth_server); + // Pre-populate registry with some old servers using the public API + await registry.addServer('old_app_server', testConfigs.file_tools_server, 'CACHE'); + await registry.addServer('old_user_server', testConfigs.oauth_server, 'CACHE'); // Initialize with new configs (this should reset first) await MCPServersInitializer.initialize(testConfigs); // Verify old servers are gone - expect(await registry.sharedAppServers.get('old_app_server')).toBeUndefined(); - expect(await registry.sharedUserServers.get('old_user_server')).toBeUndefined(); + expect(await registry.getServerConfig('old_app_server')).toBeUndefined(); + expect(await registry.getServerConfig('old_user_server')).toBeUndefined(); // Verify new servers are present - expect(await registry.sharedAppServers.get('file_tools_server')).toBeDefined(); - expect(await registry.sharedUserServers.get('oauth_server')).toBeDefined(); + expect(await registry.getServerConfig('file_tools_server')).toBeDefined(); + expect(await registry.getServerConfig('oauth_server')).toBeDefined(); expect(await registryStatusCache.isInitialized()).toBe(true); }); @@ -215,54 +214,14 @@ describe('MCPServersInitializer', () => { ); }); - it('should add disabled servers to sharedUserServers', async () => { + it('should initialize all servers to cache repository', async () => { await MCPServersInitializer.initialize(testConfigs); - const disabledServer = await registry.sharedUserServers.get('disabled_server'); - expect(disabledServer).toBeDefined(); - expect(disabledServer).toMatchObject({ - ...testParsedConfigs.disabled_server, - _processedByInspector: true, - }); - }); - - it('should add OAuth servers to sharedUserServers', async () => { - await MCPServersInitializer.initialize(testConfigs); - - const oauthServer = await registry.sharedUserServers.get('oauth_server'); - expect(oauthServer).toBeDefined(); - expect(oauthServer).toMatchObject({ - ...testParsedConfigs.oauth_server, - _processedByInspector: true, - }); - }); - - it('should add enabled non-OAuth servers to sharedAppServers', async () => { - await MCPServersInitializer.initialize(testConfigs); - - const fileToolsServer = await registry.sharedAppServers.get('file_tools_server'); - expect(fileToolsServer).toBeDefined(); - expect(fileToolsServer).toMatchObject({ - ...testParsedConfigs.file_tools_server, - _processedByInspector: true, - }); - - const searchToolsServer = await registry.sharedAppServers.get('search_tools_server'); - expect(searchToolsServer).toBeDefined(); - expect(searchToolsServer).toMatchObject({ - ...testParsedConfigs.search_tools_server, - _processedByInspector: true, - }); - }); - - it('should successfully initialize all servers', async () => { - await MCPServersInitializer.initialize(testConfigs); - - // Verify all servers were added to appropriate registries - expect(await registry.sharedUserServers.get('disabled_server')).toBeDefined(); - expect(await registry.sharedUserServers.get('oauth_server')).toBeDefined(); - expect(await registry.sharedAppServers.get('file_tools_server')).toBeDefined(); - expect(await registry.sharedAppServers.get('search_tools_server')).toBeDefined(); + // Verify all server types (disabled, OAuth, and regular) were added to cache + expect(await registry.getServerConfig('disabled_server')).toBeDefined(); + expect(await registry.getServerConfig('oauth_server')).toBeDefined(); + expect(await registry.getServerConfig('file_tools_server')).toBeDefined(); + expect(await registry.getServerConfig('search_tools_server')).toBeDefined(); }); it('should handle inspection failures gracefully', async () => { @@ -283,17 +242,17 @@ describe('MCPServersInitializer', () => { await MCPServersInitializer.initialize(testConfigs); // Verify other servers were still processed - const disabledServer = await registry.sharedUserServers.get('disabled_server'); + const disabledServer = await registry.getServerConfig('disabled_server'); expect(disabledServer).toBeDefined(); - const oauthServer = await registry.sharedUserServers.get('oauth_server'); + const oauthServer = await registry.getServerConfig('oauth_server'); expect(oauthServer).toBeDefined(); - const searchToolsServer = await registry.sharedAppServers.get('search_tools_server'); + const searchToolsServer = await registry.getServerConfig('search_tools_server'); expect(searchToolsServer).toBeDefined(); // Verify file_tools_server was not added (due to inspection failure) - const fileToolsServer = await registry.sharedAppServers.get('file_tools_server'); + const fileToolsServer = await registry.getServerConfig('file_tools_server'); expect(fileToolsServer).toBeUndefined(); }); 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 46b049d875..ca5e9f8063 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 @@ -4,12 +4,17 @@ import type * as t from '~/mcp/types'; /** * Integration tests for MCPServersRegistry using Redis-backed cache. * For unit tests using in-memory cache, see MCPServersRegistry.test.ts + * + * NOTE: After refactoring, these tests have been simplified. + * Private server functionality has been moved to DB (not yet implemented). + * The registry now uses a unified cache repository for YAML configs only. */ describe('MCPServersRegistry Redis Integration Tests', () => { let registry: typeof import('../MCPServersRegistry').mcpServersRegistry; let keyvRedisClient: Awaited['keyvRedisClient']; let LeaderElection: typeof import('~/cluster/LeaderElection').LeaderElection; let leaderInstance: InstanceType; + let MCPServerInspector: typeof import('../MCPServerInspector').MCPServerInspector; const testParsedConfig: t.ParsedServerConfig = { type: 'stdio', @@ -31,6 +36,12 @@ describe('MCPServersRegistry Redis Integration Tests', () => { }, }; + const testRawConfig: t.MCPOptions = { + type: 'stdio', + command: 'node', + args: ['tools.js'], + }; + beforeAll(async () => { // Set up environment variables for Redis (only if not already set) process.env.USE_REDIS = process.env.USE_REDIS ?? 'true'; @@ -43,10 +54,12 @@ describe('MCPServersRegistry Redis Integration Tests', () => { const registryModule = await import('../MCPServersRegistry'); const redisClients = await import('~/cache/redisClients'); const leaderElectionModule = await import('~/cluster/LeaderElection'); + const inspectorModule = await import('../MCPServerInspector'); registry = registryModule.mcpServersRegistry; keyvRedisClient = redisClients.keyvRedisClient; LeaderElection = leaderElectionModule.LeaderElection; + MCPServerInspector = inspectorModule.MCPServerInspector; // Ensure Redis is connected if (!keyvRedisClient) throw new Error('Redis client is not initialized'); @@ -60,6 +73,18 @@ describe('MCPServersRegistry Redis Integration Tests', () => { expect(isLeader).toBe(true); }); + 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; + }); + }); + afterEach(async () => { // Clean up: reset registry to clear all test data await registry.reset(); @@ -79,6 +104,8 @@ describe('MCPServersRegistry Redis Integration Tests', () => { await Promise.all(keysToDelete.map((key) => keyvRedisClient!.del(key))); } } + + jest.restoreAllMocks(); }); afterAll(async () => { @@ -89,230 +116,100 @@ describe('MCPServersRegistry Redis Integration Tests', () => { if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); }); - describe('private user servers', () => { - it('should add and remove private user server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; + // Tests for the old privateServersCache API have been removed + // The refactoring simplified the architecture to use unified repositories (cache + DB) + // Private server functionality is now handled by the DB repository (not yet implemented) - // Add private user server - await registry.privateServersCache.add(userId, serverName, testParsedConfig); + describe('cache repository functionality', () => { + it('should add and retrieve server config from cache', async () => { + const serverName = 'test_server'; + + // Add server using public API + await registry.addServer(serverName, testRawConfig, 'CACHE'); // Verify server was added - const retrievedConfig = await registry.getServerConfig(serverName, userId); - expect(retrievedConfig).toMatchObject(testParsedConfig); - - // Remove private user server - await registry.privateServersCache.remove(userId, serverName); - - // Verify server was removed - const configAfterRemoval = await registry.getServerConfig(serverName, userId); - expect(configAfterRemoval).toBeUndefined(); + const retrievedConfig = await registry.getServerConfig(serverName); + expect(retrievedConfig).toBeDefined(); + expect(retrievedConfig?.type).toBe('stdio'); + if (retrievedConfig && 'command' in retrievedConfig) { + expect(retrievedConfig.command).toBe('node'); + expect(retrievedConfig.args).toEqual(['tools.js']); + } }); - it('should throw error when adding duplicate private user server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - - await registry.privateServersCache.add(userId, serverName, testParsedConfig); - await expect( - registry.privateServersCache.add(userId, serverName, testParsedConfig), - ).rejects.toThrow( - 'Server "private_server" already exists in cache. Use update() to modify existing configs.', - ); - }); - - it('should update an existing private user server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - const updatedConfig: t.ParsedServerConfig = { + it('should update existing server config in cache', async () => { + const serverName = 'test_server'; + const updatedConfig: t.MCPOptions = { type: 'stdio', command: 'python', args: ['updated.py'], - requiresOAuth: true, }; - // Add private user server - await registry.privateServersCache.add(userId, serverName, testParsedConfig); + // Add server + await registry.addServer(serverName, testRawConfig, 'CACHE'); - // Update the server config - await registry.privateServersCache.update(userId, serverName, updatedConfig); + // Update server + await registry.updateServer(serverName, updatedConfig, 'CACHE'); // Verify server was updated - const retrievedConfig = await registry.getServerConfig(serverName, userId); - expect(retrievedConfig).toMatchObject(updatedConfig); - }); - - it('should throw error when updating non-existent server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - - // Add a user cache first - await registry.privateServersCache.add(userId, 'other_server', testParsedConfig); - - await expect( - registry.privateServersCache.update(userId, serverName, testParsedConfig), - ).rejects.toThrow( - 'Server "private_server" does not exist in cache. Use add() to create new configs.', - ); - }); - - it('should throw error when updating non-existent server (lazy-loads cache)', async () => { - const userId = 'nonexistent_user'; - const serverName = 'private_server'; - - // With lazy-loading, cache is created but server doesn't exist in it - await expect( - registry.privateServersCache.update(userId, serverName, testParsedConfig), - ).rejects.toThrow( - 'Server "private_server" does not exist in cache. Use add() to create new configs.', - ); - }); - }); - - describe('getPrivateServerConfig', () => { - it('should retrieve private server config for a specific user', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - - await registry.privateServersCache.add(userId, serverName, testParsedConfig); - - const retrievedConfig = await registry.privateServersCache.get(userId, serverName); - expect(retrievedConfig).toMatchObject(testParsedConfig); - }); - - it('should return undefined if server does not exist in user private cache', async () => { - const userId = 'user123'; - - // Create a cache for this user with a different server - await registry.privateServersCache.add(userId, 'other_server', testParsedConfig); - - // Try to get a server that doesn't exist - const retrievedConfig = await registry.privateServersCache.get(userId, 'nonexistent_server'); - expect(retrievedConfig).toBeUndefined(); - }); - - it('should return undefined when user has no private servers (lazy-loads cache)', async () => { - const userId = 'user_with_no_cache'; - - // With lazy-loading, cache is created but is empty - const config = await registry.privateServersCache.get(userId, 'server_name'); - expect(config).toBeUndefined(); - }); - - it('should isolate private servers between different users', async () => { - const user1 = 'user1'; - const user2 = 'user2'; - const serverName = 'shared_name_server'; - - const config1: t.ParsedServerConfig = { - ...testParsedConfig, - args: ['user1.js'], - }; - const config2: t.ParsedServerConfig = { - ...testParsedConfig, - args: ['user2.js'], - }; - - await registry.privateServersCache.add(user1, serverName, config1); - await registry.privateServersCache.add(user2, serverName, config2); - - const user1Config = await registry.privateServersCache.get(user1, serverName); - const user2Config = await registry.privateServersCache.get(user2, serverName); - - // Verify each user gets their own config - expect(user1Config).toBeDefined(); - expect(user2Config).toBeDefined(); - if (user1Config && 'args' in user1Config) { - expect(user1Config.args).toEqual(['user1.js']); - } - if (user2Config && 'args' in user2Config) { - expect(user2Config.args).toEqual(['user2.js']); + const retrievedConfig = await registry.getServerConfig(serverName); + expect(retrievedConfig).toBeDefined(); + expect(retrievedConfig?.type).toBe('stdio'); + if (retrievedConfig && 'command' in retrievedConfig) { + expect(retrievedConfig.command).toBe('python'); + expect(retrievedConfig.args).toEqual(['updated.py']); } }); - it('should not retrieve shared servers through privateServersCache.get', async () => { - const userId = 'user123'; + it('should remove server config from cache', async () => { + const serverName = 'test_server'; - // Add servers to shared caches - await registry.sharedAppServers.add('app_server', testParsedConfig); - await registry.sharedUserServers.add('user_server', testParsedConfig); + // Add server + await registry.addServer(serverName, testRawConfig, 'CACHE'); - // Create a private cache for the user (but don't add these servers to it) - await registry.privateServersCache.add(userId, 'private_server', testParsedConfig); + // Verify server exists + const configBefore = await registry.getServerConfig(serverName); + expect(configBefore).toBeDefined(); - // Try to get shared servers using privateServersCache.get - should return undefined - // because privateServersCache.get only looks at private cache, not shared caches - const appServerConfig = await registry.privateServersCache.get(userId, 'app_server'); - const userServerConfig = await registry.privateServersCache.get(userId, 'user_server'); + // Remove server + await registry.removeServer(serverName, 'CACHE'); - expect(appServerConfig).toBeUndefined(); - expect(userServerConfig).toBeUndefined(); + // Verify server was removed + const configAfter = await registry.getServerConfig(serverName); + expect(configAfter).toBeUndefined(); }); }); describe('getAllServerConfigs', () => { - it('should return correct servers based on userId', async () => { - // Add servers to all three caches - await registry.sharedAppServers.add('app_server', testParsedConfig); - await registry.sharedUserServers.add('user_server', testParsedConfig); - await registry.privateServersCache.add('abc', 'abc_private_server', testParsedConfig); - await registry.privateServersCache.add('xyz', 'xyz_private_server', testParsedConfig); + it('should return servers from cache repository', async () => { + // Add servers using public API + await registry.addServer('app_server', testRawConfig, 'CACHE'); + await registry.addServer('user_server', testRawConfig, 'CACHE'); - // Without userId: should return only shared app + shared user servers - const configsNoUser = await registry.getAllServerConfigs(); - expect(Object.keys(configsNoUser)).toHaveLength(2); - expect(configsNoUser).toHaveProperty('app_server'); - expect(configsNoUser).toHaveProperty('user_server'); - - // With userId 'abc': should return shared app + shared user + abc's private servers - const configsAbc = await registry.getAllServerConfigs('abc'); - expect(Object.keys(configsAbc)).toHaveLength(3); - expect(configsAbc).toHaveProperty('app_server'); - expect(configsAbc).toHaveProperty('user_server'); - expect(configsAbc).toHaveProperty('abc_private_server'); - - // With userId 'xyz': should return shared app + shared user + xyz's private servers - const configsXyz = await registry.getAllServerConfigs('xyz'); - expect(Object.keys(configsXyz)).toHaveLength(3); - expect(configsXyz).toHaveProperty('app_server'); - expect(configsXyz).toHaveProperty('user_server'); - expect(configsXyz).toHaveProperty('xyz_private_server'); + // Get all configs + const configs = await registry.getAllServerConfigs(); + expect(Object.keys(configs)).toHaveLength(2); + expect(configs).toHaveProperty('app_server'); + expect(configs).toHaveProperty('user_server'); }); }); describe('reset', () => { - it('should clear all servers from all caches (shared app, shared user, and private user)', async () => { - const userId = 'user123'; + it('should clear all servers from cache repository', async () => { + // Add servers + await registry.addServer('app_server', testRawConfig, 'CACHE'); + await registry.addServer('user_server', testRawConfig, 'CACHE'); - // Add servers to all three caches - await registry.sharedAppServers.add('app_server', testParsedConfig); - await registry.sharedUserServers.add('user_server', testParsedConfig); - await registry.privateServersCache.add(userId, 'private_server', testParsedConfig); - - // Verify all servers are accessible before reset - const appConfigBefore = await registry.getServerConfig('app_server'); - const userConfigBefore = await registry.getServerConfig('user_server', userId); - const privateConfigBefore = await registry.getServerConfig('private_server', userId); - const allConfigsBefore = await registry.getAllServerConfigs(userId); - - expect(appConfigBefore).toMatchObject(testParsedConfig); - expect(userConfigBefore).toMatchObject(testParsedConfig); - expect(privateConfigBefore).toMatchObject(testParsedConfig); - expect(Object.keys(allConfigsBefore)).toHaveLength(3); + // Verify servers exist + const configsBefore = await registry.getAllServerConfigs(); + expect(Object.keys(configsBefore)).toHaveLength(2); // Reset everything await registry.reset(); - // Verify all servers are cleared after reset - const appConfigAfter = await registry.getServerConfig('app_server'); - const userConfigAfter = await registry.getServerConfig('user_server'); - const privateConfigAfter = await registry.getServerConfig('private_server', userId); - const allConfigsAfter = await registry.getAllServerConfigs(userId); - - expect(appConfigAfter).toBeUndefined(); - expect(userConfigAfter).toBeUndefined(); - expect(privateConfigAfter).toBeUndefined(); - expect(Object.keys(allConfigsAfter)).toHaveLength(0); + // Verify all servers are cleared + const configsAfter = await registry.getAllServerConfigs(); + expect(Object.keys(configsAfter)).toHaveLength(0); }); }); }); diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index b7f67bbff4..30b9e8984c 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -1,5 +1,10 @@ import * as t from '~/mcp/types'; import { mcpServersRegistry as registry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; + +// Mock MCPServerInspector to avoid actual server connections +jest.mock('~/mcp/registry/MCPServerInspector'); + const FIXED_TIME = 1699564800000; const originalDateNow = Date.now; Date.now = jest.fn(() => FIXED_TIME); @@ -36,220 +41,55 @@ describe('MCPServersRegistry', () => { Date.now = originalDateNow; }); beforeEach(async () => { + // Mock MCPServerInspector.inspect to return the config that's passed in + jest + .spyOn(MCPServerInspector, 'inspect') + .mockImplementation(async (_serverName: string, rawConfig: t.MCPOptions) => { + return { + ...testParsedConfig, + ...rawConfig, + requiresOAuth: false, + } as unknown as t.ParsedServerConfig; + }); await registry.reset(); }); - describe('private user servers', () => { - it('should add and remove private user server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; + // Tests for the old privateServersCache API have been removed + // The refactoring simplified the architecture to use unified repositories (cache + DB) + // instead of the three-tier system (sharedAppServers, sharedUserServers, privateServersCache) - // Add private user server - await registry.privateServersCache.add(userId, serverName, testParsedConfig); - - // Verify server was added - const retrievedConfig = await registry.getServerConfig(serverName, userId); - expect(retrievedConfig).toEqual(testParsedConfig); - - // Remove private user server - await registry.privateServersCache.remove(userId, serverName); - - // Verify server was removed - const configAfterRemoval = await registry.getServerConfig(serverName, userId); - expect(configAfterRemoval).toBeUndefined(); - }); - - it('should throw error when adding duplicate private user server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - - await registry.privateServersCache.add(userId, serverName, testParsedConfig); - await expect( - registry.privateServersCache.add(userId, serverName, testParsedConfig), - ).rejects.toThrow( - 'Server "private_server" already exists in cache. Use update() to modify existing configs.', - ); - }); - - it('should update an existing private user server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - const updatedConfig: t.ParsedServerConfig = { - type: 'stdio', - command: 'python', - args: ['updated.py'], - requiresOAuth: true, - lastUpdatedAt: FIXED_TIME, - }; - - // Add private user server - await registry.privateServersCache.add(userId, serverName, testParsedConfig); - - // Update the server config - await registry.privateServersCache.update(userId, serverName, updatedConfig); - - // Verify server was updated - const retrievedConfig = await registry.getServerConfig(serverName, userId); - expect(retrievedConfig).toEqual(updatedConfig); - }); - - it('should throw error when updating non-existent server', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - - // Add a user cache first - await registry.privateServersCache.add(userId, 'other_server', testParsedConfig); - - await expect( - registry.privateServersCache.update(userId, serverName, testParsedConfig), - ).rejects.toThrow( - 'Server "private_server" does not exist in cache. Use add() to create new configs.', - ); - }); - - it('should throw error when updating non-existent server (lazy-loads cache)', async () => { - const userId = 'nonexistent_user'; - const serverName = 'private_server'; - - // With lazy-loading, cache is created but server doesn't exist in it - await expect( - registry.privateServersCache.update(userId, serverName, testParsedConfig), - ).rejects.toThrow( - 'Server "private_server" does not exist in cache. Use add() to create new configs.', - ); - }); - }); - - describe('getPrivateServerConfig', () => { - it('should retrieve private server config for a specific user', async () => { - const userId = 'user123'; - const serverName = 'private_server'; - - await registry.privateServersCache.add(userId, serverName, testParsedConfig); - - const retrievedConfig = await registry.privateServersCache.get(userId, serverName); - expect(retrievedConfig).toEqual(testParsedConfig); - }); - - it('should return undefined if server does not exist in user private cache', async () => { - const userId = 'user123'; - - // Create a cache for this user with a different server - await registry.privateServersCache.add(userId, 'other_server', testParsedConfig); - - // Try to get a server that doesn't exist - const retrievedConfig = await registry.privateServersCache.get(userId, 'nonexistent_server'); - expect(retrievedConfig).toBeUndefined(); - }); - - it('should return undefined when user has no private servers (lazy-loads cache)', async () => { - const userId = 'user_with_no_cache'; - - // With lazy-loading, cache is created but is empty - const config = await registry.privateServersCache.get(userId, 'server_name'); - expect(config).toBeUndefined(); - }); - - it('should isolate private servers between different users', async () => { - const user1 = 'user1'; - const user2 = 'user2'; - const serverName = 'shared_name_server'; - - const config1: t.ParsedServerConfig = { - ...testParsedConfig, - args: ['user1.js'], - }; - const config2: t.ParsedServerConfig = { - ...testParsedConfig, - args: ['user2.js'], - }; - - await registry.privateServersCache.add(user1, serverName, config1); - await registry.privateServersCache.add(user2, serverName, config2); - - const user1Config = await registry.privateServersCache.get(user1, serverName); - const user2Config = await registry.privateServersCache.get(user2, serverName); - - // Verify each user gets their own config - expect(user1Config).toBeDefined(); - expect(user2Config).toBeDefined(); - if (user1Config && 'args' in user1Config) { - expect(user1Config.args).toEqual(['user1.js']); - } - if (user2Config && 'args' in user2Config) { - expect(user2Config.args).toEqual(['user2.js']); - } - }); - - it('should not retrieve shared servers through privateServersCache.get', async () => { - const userId = 'user123'; - - // Add servers to shared caches - await registry.sharedAppServers.add('app_server', testParsedConfig); - await registry.sharedUserServers.add('user_server', testParsedConfig); - - // Create a private cache for the user (but don't add these servers to it) - await registry.privateServersCache.add(userId, 'private_server', testParsedConfig); - - // Try to get shared servers using privateServersCache.get - should return undefined - // because privateServersCache.get only looks at private cache, not shared caches - const appServerConfig = await registry.privateServersCache.get(userId, 'app_server'); - const userServerConfig = await registry.privateServersCache.get(userId, 'user_server'); - - expect(appServerConfig).toBeUndefined(); - expect(userServerConfig).toBeUndefined(); - }); - }); + // Tests for getPrivateServerConfig have been removed + // The new architecture uses getServerConfig() which checks cache first, then DB + // Private server functionality is now handled by the DB repository (not yet implemented) describe('getAllServerConfigs', () => { - it('should return correct servers based on userId', async () => { - // Add servers to all three caches - await registry.sharedAppServers.add('app_server', testParsedConfig); - await registry.sharedUserServers.add('user_server', testParsedConfig); - await registry.privateServersCache.add('abc', 'abc_private_server', testParsedConfig); - await registry.privateServersCache.add('xyz', 'xyz_private_server', testParsedConfig); + it('should return servers from cache repository', async () => { + // Add servers to cache using the new API + await registry['cacheConfigsRepo'].add('app_server', testParsedConfig); + await registry['cacheConfigsRepo'].add('user_server', testParsedConfig); - // Without userId: should return only shared app + shared user servers - const configsNoUser = await registry.getAllServerConfigs(); - expect(Object.keys(configsNoUser)).toHaveLength(2); - expect(configsNoUser).toHaveProperty('app_server'); - expect(configsNoUser).toHaveProperty('user_server'); - - // With userId 'abc': should return shared app + shared user + abc's private servers - const configsAbc = await registry.getAllServerConfigs('abc'); - expect(Object.keys(configsAbc)).toHaveLength(3); - expect(configsAbc).toHaveProperty('app_server'); - expect(configsAbc).toHaveProperty('user_server'); - expect(configsAbc).toHaveProperty('abc_private_server'); - - // With userId 'xyz': should return shared app + shared user + xyz's private servers - const configsXyz = await registry.getAllServerConfigs('xyz'); - expect(Object.keys(configsXyz)).toHaveLength(3); - expect(configsXyz).toHaveProperty('app_server'); - expect(configsXyz).toHaveProperty('user_server'); - expect(configsXyz).toHaveProperty('xyz_private_server'); + // getAllServerConfigs should return configs from cache (DB is not implemented yet) + const configs = await registry.getAllServerConfigs(); + expect(Object.keys(configs)).toHaveLength(2); + expect(configs).toHaveProperty('app_server'); + expect(configs).toHaveProperty('user_server'); }); }); describe('reset', () => { - it('should clear all servers from all caches (shared app, shared user, and private user)', async () => { - const userId = 'user123'; + it('should clear all servers from cache repository', async () => { + // Add servers to cache using the new API + await registry.addServer('app_server', testParsedConfig, 'CACHE'); + await registry.addServer('user_server', testParsedConfig, 'CACHE'); - // Add servers to all three caches - await registry.sharedAppServers.add('app_server', testParsedConfig); - await registry.sharedUserServers.add('user_server', testParsedConfig); - await registry.privateServersCache.add(userId, 'private_server', testParsedConfig); - - // Verify all servers are accessible before reset + // Verify servers are accessible before reset const appConfigBefore = await registry.getServerConfig('app_server'); - const userConfigBefore = await registry.getServerConfig('user_server', userId); - const privateConfigBefore = await registry.getServerConfig('private_server', userId); - const allConfigsBefore = await registry.getAllServerConfigs(userId); + const userConfigBefore = await registry.getServerConfig('user_server'); + const allConfigsBefore = await registry.getAllServerConfigs(); expect(appConfigBefore).toEqual(testParsedConfig); expect(userConfigBefore).toEqual(testParsedConfig); - expect(privateConfigBefore).toEqual(testParsedConfig); - expect(Object.keys(allConfigsBefore)).toHaveLength(3); + expect(Object.keys(allConfigsBefore)).toHaveLength(2); // Reset everything await registry.reset(); @@ -257,13 +97,72 @@ describe('MCPServersRegistry', () => { // Verify all servers are cleared after reset const appConfigAfter = await registry.getServerConfig('app_server'); const userConfigAfter = await registry.getServerConfig('user_server'); - const privateConfigAfter = await registry.getServerConfig('private_server', userId); - const allConfigsAfter = await registry.getAllServerConfigs(userId); + const allConfigsAfter = await registry.getAllServerConfigs(); expect(appConfigAfter).toBeUndefined(); expect(userConfigAfter).toBeUndefined(); - expect(privateConfigAfter).toBeUndefined(); expect(Object.keys(allConfigsAfter)).toHaveLength(0); }); }); + + describe('Storage location routing (getConfigRepository)', () => { + describe('CACHE storage location', () => { + it('should route addServer to cache repository', async () => { + await registry.addServer('cache_server', testParsedConfig, 'CACHE'); + + const config = await registry.getServerConfig('cache_server'); + expect(config).toBeDefined(); + expect(config?.type).toBe('stdio'); + if (config && 'command' in config) { + expect(config.command).toBe('node'); + } + }); + + it('should route updateServer to cache repository', async () => { + await registry.addServer('cache_server', testParsedConfig, 'CACHE'); + + const updatedConfig = { ...testParsedConfig, command: 'python' } as t.ParsedServerConfig; + await registry.updateServer('cache_server', updatedConfig, 'CACHE'); + + const config = await registry.getServerConfig('cache_server'); + expect(config).toBeDefined(); + if (config && 'command' in config) { + expect(config.command).toBe('python'); + } + }); + + it('should route removeServer to cache repository', async () => { + await registry.addServer('cache_server', testParsedConfig, 'CACHE'); + expect(await registry.getServerConfig('cache_server')).toBeDefined(); + + await registry.removeServer('cache_server', 'CACHE'); + + const config = await registry.getServerConfig('cache_server'); + expect(config).toBeUndefined(); + }); + }); + + describe('Invalid storage location', () => { + it('should throw error for unsupported storage location in addServer', async () => { + await expect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + registry.addServer('test_server', testParsedConfig, 'INVALID' as any), + ).rejects.toThrow('The provided storage location "INVALID" is not supported'); + }); + + it('should throw error for unsupported storage location in updateServer', async () => { + await expect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + registry.updateServer('test_server', testParsedConfig, 'REDIS' as any), + ).rejects.toThrow('The provided storage location "REDIS" is not supported'); + }); + + it('should throw error for unsupported storage location in removeServer', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + await expect(registry.removeServer('test_server', 'S3' as any)).rejects.toThrow( + 'The provided storage location "S3" is not supported', + ); + }); + }); + }); }); diff --git a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheBase.ts b/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheBase.ts deleted file mode 100644 index 791971c7ec..0000000000 --- a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheBase.ts +++ /dev/null @@ -1,115 +0,0 @@ -import type * as t from '~/mcp/types'; -import { ServerConfigsCache, ServerConfigsCacheFactory } from '../ServerConfigsCacheFactory'; -import { logger } from '@librechat/data-schemas'; - -export abstract class PrivateServerConfigsCacheBase { - protected readonly PREFIX = 'MCP::ServersRegistry::Servers::Private'; - protected caches: Map = new Map(); - - public async add( - userId: string, - serverName: string, - config: t.ParsedServerConfig, - ): Promise { - const userCache = this.getOrCreatePrivateUserCache(userId); - await userCache.add(serverName, config); - } - - public async update( - userId: string, - serverName: string, - config: t.ParsedServerConfig, - ): Promise { - const userCache = this.getOrCreatePrivateUserCache(userId); - await userCache.update(serverName, config); - } - - /** - * Get a specific server config from a user's cache. - */ - public async get(userId: string, serverName: string): Promise { - const cache = this.getOrCreatePrivateUserCache(userId); - return await cache.get(serverName); - } - - /** - * Get all server configs for a user. - */ - public async getAll(userId: string): Promise> { - const cache = this.getOrCreatePrivateUserCache(userId); - return await cache.getAll(); - } - - /** - * Check if a user has a cache instance loaded. - */ - public abstract has(userId: string): Promise; - - public async remove(userId: string, serverName: string): Promise { - const userCache = this.getOrCreatePrivateUserCache(userId); - await userCache.remove(serverName); - } - - public async reset(userId: string): Promise { - const cache = this.getOrCreatePrivateUserCache(userId); - return cache.reset(); - } - - // ============= BATCH OPERATION PRIMITIVES ============= - // Simple primitives for MCPPrivateServerLoader orchestration - no business logic - - /** - * Update server config in ALL user caches that already have it. - * Efficient: Uses pattern-based scan, skips users who don't have it. - * Use case: Metadata changed (command, args, env) - */ - public abstract updateServerConfigIfExists( - serverName: string, - config: t.ParsedServerConfig, - ): Promise; - - /** - * Add server config ONLY to users whose caches are already initialized. - * Skips users without initialized caches (doesn't create new caches). - * Use case: Granting access to existing users - */ - public abstract addServerConfigIfCacheExists( - userIds: string[], - serverName: string, - config: t.ParsedServerConfig, - ): Promise; - - /** - * Remove server config ONLY from users whose caches exist. - * Ignores users without initialized caches. - * Use case: Revoking access from users - */ - public abstract removeServerConfigIfCacheExists( - userIds: string[], - serverName: string, - ): Promise; - - /** - * Find all users who have this server in their cache. - * Primitive for determining affected users. - */ - public abstract findUsersWithServer(serverName: string): Promise; - - /** - * Clear all private server configs for all users (nuclear option). - * Use sparingly - typically only for testing or full reset. - */ - public abstract resetAll(): Promise; - - protected getOrCreatePrivateUserCache(userId: string): ServerConfigsCache { - if (!userId) { - logger.error('userId is required to get or create private user cache'); - throw new Error('userId is required to get or create private user cache'); - } - if (!this.caches.has(userId)) { - const cache = ServerConfigsCacheFactory.create(userId, 'Private', false); - this.caches.set(userId, cache); - } - return this.caches.get(userId)!; - } -} diff --git a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheFactory.ts b/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheFactory.ts deleted file mode 100644 index 6a91909b52..0000000000 --- a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheFactory.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { cacheConfig } from '~/cache'; - -import { PrivateServerConfigsCacheInMemory } from './PrivateServerConfigsCacheInMemory'; -import { PrivateServerConfigsCacheRedis } from './PrivateServerConfigsCacheRedis'; - -export type PrivateServerConfigsCache = - | PrivateServerConfigsCacheInMemory - | PrivateServerConfigsCacheRedis; - -/** - * Factory for creating the appropriate PrivateServerConfigsCache implementation based on deployment mode. - * Automatically selects between in-memory and Redis-backed storage depending on USE_REDIS config. - * In single-instance mode (USE_REDIS=false), returns lightweight in-memory cache. - * In cluster mode (USE_REDIS=true), returns Redis-backed cache with distributed coordination. - * Provides a unified interface regardless of the underlying storage mechanism. - */ -export class PrivateServerConfigsCacheFactory { - /** - * Create a ServerConfigsCache instance. - * Returns Redis implementation if Redis is configured, otherwise in-memory implementation. - * - * @returns PrivateServerConfigsCache instance - */ - static create(): PrivateServerConfigsCache { - if (cacheConfig.USE_REDIS) { - return new PrivateServerConfigsCacheRedis(); - } - - // In-memory mode uses a simple Map - doesn't need owner/namespace - return new PrivateServerConfigsCacheInMemory(); - } -} diff --git a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheInMemory.ts b/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheInMemory.ts deleted file mode 100644 index 7b237c536e..0000000000 --- a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheInMemory.ts +++ /dev/null @@ -1,105 +0,0 @@ -import { ParsedServerConfig } from '~/mcp/types'; -import { PrivateServerConfigsCacheBase } from './PrivateServerConfigsCacheBase'; -import { logger } from '@librechat/data-schemas'; -import { ServerConfigsCacheInMemory } from '../ServerConfigsCacheInMemory'; - -export class PrivateServerConfigsCacheInMemory extends PrivateServerConfigsCacheBase { - public async has(userId: string): Promise { - return this.caches.has(userId); - } - - public async updateServerConfigIfExists( - serverName: string, - config: ParsedServerConfig, - ): Promise { - let updatedCount = 0; - - for (const [userId, userCache] of this.caches.entries()) { - const existing = await userCache.get(serverName); - if (existing) { - const inMemoryCache = userCache as ServerConfigsCacheInMemory; - await inMemoryCache.set(serverName, config); - updatedCount++; - logger.debug(`[MCP][PrivateServers][InMemory] Updated "${serverName}" for user ${userId}`); - } - } - - logger.info( - `[MCP][PrivateServers][InMemory] Propagated config update for "${serverName}" to ${updatedCount} users`, - ); - } - - public async addServerConfigIfCacheExists( - userIds: string[], - serverName: string, - config: ParsedServerConfig, - ): Promise { - let addedCount = 0; - - for (const userId of userIds) { - if (this.caches.has(userId)) { - // Only if cache initialized - const userCache = this.getOrCreatePrivateUserCache(userId); - const inMemoryCache = userCache as ServerConfigsCacheInMemory; - await inMemoryCache.set(serverName, config); - addedCount++; - logger.debug(`[MCP][PrivateServers][InMemory] Added "${serverName}" to user ${userId}`); - } - } - - logger.info( - `[MCP][PrivateServers][InMemory] Granted access to "${serverName}" for ${addedCount}/${userIds.length} initialized users`, - ); - } - - public async removeServerConfigIfCacheExists( - userIds: string[], - serverName: string, - ): Promise { - let removedCount = 0; - - for (const userId of userIds) { - if (this.caches.has(userId)) { - try { - const userCache = this.getOrCreatePrivateUserCache(userId); - await userCache.remove(serverName); - removedCount++; - logger.debug( - `[MCP][PrivateServers][InMemory] Removed "${serverName}" from user ${userId}`, - ); - } catch (error) { - // Ignore - server might not exist for this user - logger.debug( - `[MCP][PrivateServers][InMemory] Server "${serverName}" not found for user ${userId}`, - error, - ); - } - } - } - - logger.info( - `[MCP][PrivateServers][InMemory] Revoked access to "${serverName}" from ${removedCount}/${userIds.length} users`, - ); - } - - public async findUsersWithServer(serverName: string): Promise { - const userIds: string[] = []; - - for (const [userId, userCache] of this.caches.entries()) { - const config = await userCache.get(serverName); - if (config) { - userIds.push(userId); - } - } - - return userIds; - } - - /** - * Clear ALL servers from ALL user caches (nuclear option). - */ - public async resetAll(): Promise { - this.caches.clear(); - logger.info(`[MCP][PrivateServers][InMemory] Cleared ALL user caches`); - } -} diff --git a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheRedis.ts b/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheRedis.ts deleted file mode 100644 index fdd88b1f4d..0000000000 --- a/packages/api/src/mcp/registry/cache/PrivateServerConfigs/PrivateServerConfigsCacheRedis.ts +++ /dev/null @@ -1,284 +0,0 @@ -import { ParsedServerConfig } from '~/mcp/types'; -import { keyvRedisClient } from '~/cache'; -import { PrivateServerConfigsCacheBase } from './PrivateServerConfigsCacheBase'; -import { logger } from '@librechat/data-schemas'; -import { cacheConfig } from '~/cache/cacheConfig'; -import { batchDeleteKeys, scanKeys } from '~/cache/redisUtils'; - -export class PrivateServerConfigsCacheRedis extends PrivateServerConfigsCacheBase { - /** - * Detect if Redis is running in cluster mode. - * In cluster mode, we need to avoid CROSSSLOT errors by using pipelines instead of multi() transactions. - */ - private isClusterMode(): boolean { - return cacheConfig.USE_REDIS_CLUSTER; - } - - public async has(userId: string): Promise { - if (!userId || !keyvRedisClient || !('scanIterator' in keyvRedisClient)) { - return false; - } - - const pattern = `*${this.PREFIX}::${userId}:*`; - - for await (const _key of keyvRedisClient.scanIterator({ - MATCH: pattern, - COUNT: 1, - })) { - return true; - } - - return false; // No keys found - cache not initialized - } - - public async updateServerConfigIfExists( - serverName: string, - config: ParsedServerConfig, - ): Promise { - if (!keyvRedisClient || !('scanIterator' in keyvRedisClient)) { - logger.warn('[MCP][PrivateServers][Redis] Redis SCAN not available'); - return; - } - - const pattern = this.generateScanKeyPattern(serverName); - - try { - // Efficient: Pattern-based scan for specific serverName - // All cache keys that have the serverName - const keysToUpdate = await scanKeys(keyvRedisClient, pattern); - - if (keysToUpdate.length > 0) { - const updatedConfig = { ...config, lastUpdatedAt: Date.now() }; - const keyvFormat = { value: updatedConfig, expires: null }; - const serializedConfig = JSON.stringify(keyvFormat); - - const chunkSize = cacheConfig.REDIS_UPDATE_CHUNK_SIZE; - - if (this.isClusterMode()) { - // Cluster mode: Use individual commands in parallel (no atomicity, but works across slots) - for (let i = 0; i < keysToUpdate.length; i += chunkSize) { - const chunk = keysToUpdate.slice(i, i + chunkSize); - await Promise.all( - chunk.map((key) => keyvRedisClient!.set(key, serializedConfig, { XX: true })), - ); - } - } else { - // Single-node mode: Use multi() for atomic transactions - for (let i = 0; i < keysToUpdate.length; i += chunkSize) { - const chunk = keysToUpdate.slice(i, i + chunkSize); - const multi = keyvRedisClient.multi(); - for (const key of chunk) { - multi.set(key, serializedConfig, { XX: true }); - } - await multi.exec(); - } - } - - logger.info( - `[MCP][PrivateServers][Redis] Propagated config update for "${serverName}" to ${keysToUpdate.length} users`, - ); - } else { - logger.debug(`[MCP][PrivateServers][Redis] No users have "${serverName}"`); - } - } catch (error) { - logger.error(`[MCP][PrivateServers][Redis] Error updating "${serverName}"`, error); - throw error; - } - } - - public async addServerConfigIfCacheExists( - userIds: string[], - serverName: string, - config: ParsedServerConfig, - ): Promise { - if (!keyvRedisClient) return; - - // Optimized: Single SCAN to get all users with initialized caches - const allUsersWithCaches = await this.getAllUserIds(); - - // Filter to only users with initialized caches - const eligibleUserIds = userIds.filter((id) => allUsersWithCaches.has(id)); - - if (eligibleUserIds.length === 0) { - logger.info( - `[MCP][PrivateServers][Redis] No initialized users to grant access to "${serverName}"`, - ); - return; - } - - // Batch add using pipeline with NX (only set if key doesn't exist) - const updatedConfig = { ...config, lastUpdatedAt: Date.now() }; - const keyvFormat = { value: updatedConfig, expires: null }; - const serializedConfig = JSON.stringify(keyvFormat); - - const globalPrefix = cacheConfig.REDIS_KEY_PREFIX; - const separator = cacheConfig.GLOBAL_PREFIX_SEPARATOR; - - const chunkSize = cacheConfig.REDIS_UPDATE_CHUNK_SIZE; - - if (this.isClusterMode()) { - // Cluster mode: Use individual commands in parallel (no atomicity, but works across slots) - for (let i = 0; i < eligibleUserIds.length; i += chunkSize) { - const chunk = eligibleUserIds.slice(i, i + chunkSize); - await Promise.all( - chunk.map((userId) => { - const namespace = `${this.PREFIX}::${userId}`; - const fullKey = globalPrefix - ? `${globalPrefix}${separator}${namespace}:${serverName}` - : `${namespace}:${serverName}`; - return keyvRedisClient!.set(fullKey, serializedConfig, { NX: true }); - }), - ); - } - } else { - // Single-node mode: Use multi() for atomic transactions - for (let i = 0; i < eligibleUserIds.length; i += chunkSize) { - const chunk = eligibleUserIds.slice(i, i + chunkSize); - const multi = keyvRedisClient.multi(); - for (const userId of chunk) { - const namespace = `${this.PREFIX}::${userId}`; - const fullKey = globalPrefix - ? `${globalPrefix}${separator}${namespace}:${serverName}` - : `${namespace}:${serverName}`; - multi.set(fullKey, serializedConfig, { NX: true }); - } - await multi.exec(); - } - } - - logger.info( - `[MCP][PrivateServers][Redis] Granted access to "${serverName}" for ${eligibleUserIds.length}/${userIds.length} initialized users`, - ); - } - - public async removeServerConfigIfCacheExists( - userIds: string[], - serverName: string, - ): Promise { - if (!keyvRedisClient) return; - - // Optimized: Direct key construction - no SCAN needed! - // Build full Redis keys directly since we know userId and serverName - const globalPrefix = cacheConfig.REDIS_KEY_PREFIX; - const separator = cacheConfig.GLOBAL_PREFIX_SEPARATOR; - const keysToDelete: string[] = []; - - for (const userId of userIds) { - // Construct the full Redis key - const namespace = `${this.PREFIX}::${userId}`; - const fullKey = globalPrefix - ? `${globalPrefix}${separator}${namespace}:${serverName}` - : `${namespace}:${serverName}`; - keysToDelete.push(fullKey); - } - - if (keysToDelete.length > 0) { - // Use utility function for efficient parallel deletion - const removedCount = await batchDeleteKeys(keyvRedisClient, keysToDelete); - - logger.info( - `[MCP][PrivateServers][Redis] Revoked access to "${serverName}" from ${removedCount}/${userIds.length} users`, - ); - } - } - - public async findUsersWithServer(serverName: string): Promise { - if (!keyvRedisClient || !('scanIterator' in keyvRedisClient)) { - return []; - } - - const pattern = this.generateScanKeyPattern(serverName); - - try { - const keys = await scanKeys(keyvRedisClient, pattern); - const userIds: string[] = []; - - for (const key of keys) { - const userId = this.extractUserIdFromKey(key); - if (userId) { - userIds.push(userId); - } - } - - return userIds; - } catch (error) { - logger.error(`[MCP][PrivateServers][Redis] Error finding users with "${serverName}"`, error); - return []; - } - } - - /** - * Scans Redis to find all unique userIds that have private server configs. - * This method is used for efficient batch operations (add/update/delete) across all users. - * - * Performance note: This scans all private server config keys in Redis. - * Use sparingly as it can be expensive with many users. - */ - private async getAllUserIds(): Promise> { - if (!keyvRedisClient || !('scanIterator' in keyvRedisClient)) { - logger.warn('[MCP][PrivateServerConfigs][Redis] Redis SCAN not available'); - return new Set(); - } - - const userIds = new Set(); - // Pattern to match all private server configs: MCP::ServersRegistry::Servers::*:* - const pattern = `*${this.PREFIX}::*:*`; - - try { - const keys = await scanKeys(keyvRedisClient, pattern); - - for (const key of keys) { - const userId = this.extractUserIdFromKey(key); - if (userId) { - userIds.add(userId); - } - } - } catch (error) { - logger.error('[MCP][PrivateServerConfigs][Redis] Error scanning for userIds', error); - throw error; - } - - return userIds; - } - - /** - * Extract userId from a Redis key. - * Key format: MCP::ServersRegistry::Servers::userId:serverName - */ - private extractUserIdFromKey(key: string): string | null { - // Remove any global prefix, then extract userId - const keyWithoutGlobalPrefix = key.includes(this.PREFIX) - ? key.substring(key.indexOf(this.PREFIX)) - : key; - - const withoutPrefix = keyWithoutGlobalPrefix.replace(`${this.PREFIX}::`, ''); - const lastColonIndex = withoutPrefix.lastIndexOf(':'); - if (lastColonIndex === -1) return null; - - return withoutPrefix.substring(0, lastColonIndex); - } - - /** - * Clear ALL servers from ALL user caches (nuclear option). - */ - public async resetAll(): Promise { - if (!keyvRedisClient || !('scanIterator' in keyvRedisClient)) return; - - // Pattern to match all private user server configs - // Format: MCP::ServersRegistry::Servers::userId:serverName - const pattern = `*${this.PREFIX}::*:*`; - - // Use utility functions for efficient scan and parallel deletion - const keysToDelete = await scanKeys(keyvRedisClient, pattern); - - if (keysToDelete.length > 0) { - await batchDeleteKeys(keyvRedisClient, keysToDelete); - } - - logger.info(`[MCP][Cache][Redis] Cleared all user caches: ${keysToDelete.length} entries`); - } - - private generateScanKeyPattern(serverName: string): string { - return `*${this.PREFIX}::*:${serverName}`; - } -} diff --git a/packages/api/src/mcp/registry/cache/PrivateServersLoadStatusCache.ts b/packages/api/src/mcp/registry/cache/PrivateServersLoadStatusCache.ts deleted file mode 100644 index 2aafcf70d9..0000000000 --- a/packages/api/src/mcp/registry/cache/PrivateServersLoadStatusCache.ts +++ /dev/null @@ -1,166 +0,0 @@ -import { standardCache, keyvRedisClient } from '~/cache'; -import { cacheConfig } from '~/cache/cacheConfig'; -import { BaseRegistryCache } from './BaseRegistryCache'; -import { logger } from '@librechat/data-schemas'; - -const LOADED_KEY_PREFIX = 'USER_PRIVATE_SERVERS_LOADED'; -const LOCK_KEY_PREFIX = 'USER_PRIVATE_SERVERS_LOAD_LOCK'; - -// Default TTL values (in milliseconds) -const DEFAULT_LOADED_TTL = 3600 * 1000; // 1 hour - should match cache entry TTL -const DEFAULT_LOCK_TTL = 30 * 1000; // 30 seconds - lock timeout -const DEFAULT_WAIT_INTERVAL = 100; // 100ms between checks - -/** - * Dedicated cache for managing private server loading status with TTL synchronization. - * Solves three critical issues: - * 1. TTL Synchronization: Loaded flags expire in sync with cache entries - * 2. Cache Eviction Detection: When cache expires, flag expires too - * 3. Race Condition Prevention: Distributed locking prevents concurrent loads - * - * Design: - * - Loaded flags have same TTL as cache entries (prevents desync) - * - Distributed locks prevent multiple processes loading same user - * - Wait mechanism allows processes to wait for ongoing loads - * - Works correctly for users with 0 servers (trusts TTL, no cache verification) - */ -class PrivateServersLoadStatusCache extends BaseRegistryCache { - protected readonly cache = standardCache(`${this.PREFIX}::PrivateServersLoadStatus`); - - /** - * Check if user's private servers are fully loaded. - * If false, servers need to be loaded from DB. - * - * @param userId - User ID - * @returns true if user's private servers are fully loaded - */ - public async isLoaded(userId: string): Promise { - const key = `${LOADED_KEY_PREFIX}::${userId}`; - return (await this.cache.get(key)) === true; - } - - /** - * Mark user's private servers as fully loaded with TTL. - * TTL MUST match the cache entry TTL to prevent desync. - * - * @param userId - User ID - * @param ttl - Time to live in milliseconds (default: 1 hour) - */ - public async setLoaded(userId: string, ttl: number = DEFAULT_LOADED_TTL): Promise { - const key = `${LOADED_KEY_PREFIX}::${userId}`; - const success = await this.cache.set(key, true, ttl); - this.successCheck(`set loaded status for user ${userId}`, success); - logger.debug(`[MCP][LoadStatusCache] Marked user ${userId} as loaded (TTL: ${ttl}ms)`); - } - - /** - * Acquire a distributed lock for loading a user's private servers. - * Prevents concurrent processes from loading the same user's servers. - * - * Uses atomic Redis SET NX PX for race-free locking. - * Returns true immediately if Redis is not available (no locking needed in single-process mode). - * - * @param userId - User ID - * @param ttl - Lock timeout in milliseconds (default: 30s) - * @returns true if lock acquired, false if already locked - */ - public async acquireLoadLock(userId: string, ttl: number = DEFAULT_LOCK_TTL): Promise { - const key = `${LOCK_KEY_PREFIX}::${userId}`; - - // Distributed locking only needed when Redis is available (multi-instance mode) - if (!keyvRedisClient || !('set' in keyvRedisClient)) { - logger.debug(`[MCP][LoadStatusCache] Redis not available, skipping lock for user ${userId}`); - return true; - } - - try { - // Build the full Redis key (accounting for namespace and global prefix) - const namespace = `${this.PREFIX}::PrivateServersLoadStatus`; - const globalPrefix = cacheConfig.REDIS_KEY_PREFIX; - const separator = cacheConfig.GLOBAL_PREFIX_SEPARATOR; - const fullKey = globalPrefix - ? `${globalPrefix}${separator}${namespace}:${key}` - : `${namespace}:${key}`; - - // Redis SET with NX (only if not exists) and PX (millisecond expiry) - ATOMIC! - const result = await keyvRedisClient.set(fullKey, Date.now().toString(), { - NX: true, // Only set if key doesn't exist - PX: ttl, // Expiry in milliseconds - }); - - const acquired = result === 'OK'; - - if (acquired) { - logger.debug( - `[MCP][LoadStatusCache] Acquired load lock for user ${userId} (TTL: ${ttl}ms)`, - ); - } else { - logger.debug(`[MCP][LoadStatusCache] Load lock already held for user ${userId}`); - } - - return acquired; - } catch (error) { - logger.error(`[MCP][LoadStatusCache] Error acquiring lock for user ${userId}:`, error); - return false; - } - } - - /** - * Release the distributed lock for a user's private server loading. - * Should be called in a finally block to ensure lock is always released. - * - * @param userId - User ID - */ - public async releaseLoadLock(userId: string): Promise { - const key = `${LOCK_KEY_PREFIX}::${userId}`; - await this.cache.delete(key); - logger.debug(`[MCP][LoadStatusCache] Released load lock for user ${userId}`); - } - - /** - * Wait for another process to finish loading a user's private servers. - * Used when a lock is already held by another process. - * - * @param userId - User ID - * @param maxWaitTime - Maximum time to wait in milliseconds (default: 5s) - * @param checkInterval - Interval between checks in milliseconds (default: 100ms) - * @returns true if loading completed within maxWaitTime, false if timeout - */ - public async waitForLoad( - userId: string, - maxWaitTime: number = 5000, - checkInterval: number = DEFAULT_WAIT_INTERVAL, - ): Promise { - const startTime = Date.now(); - - while (Date.now() - startTime < maxWaitTime) { - const loaded = await this.isLoaded(userId); - if (loaded) { - logger.debug(`[MCP][LoadStatusCache] User ${userId} loading completed by another process`); - return true; - } - - // Wait before checking again - await new Promise((resolve) => setTimeout(resolve, checkInterval)); - } - - logger.warn( - `[MCP][LoadStatusCache] Timeout waiting for user ${userId} loading (waited ${maxWaitTime}ms)`, - ); - return false; - } - - /** - * Clear loaded status for a user. - * Used for testing or manual cache invalidation. - * - * @param userId - User ID - */ - public async clearLoaded(userId: string): Promise { - const key = `${LOADED_KEY_PREFIX}::${userId}`; - await this.cache.delete(key); - logger.debug(`[MCP][LoadStatusCache] Cleared loaded status for user ${userId}`); - } -} - -export const privateServersLoadStatusCache = new PrivateServersLoadStatusCache(); diff --git a/packages/api/src/mcp/registry/cache/RegistryStatusCache.ts b/packages/api/src/mcp/registry/cache/RegistryStatusCache.ts index 6555eb291a..5fbd18ffdf 100644 --- a/packages/api/src/mcp/registry/cache/RegistryStatusCache.ts +++ b/packages/api/src/mcp/registry/cache/RegistryStatusCache.ts @@ -9,9 +9,6 @@ const INITIALIZED = 'INITIALIZED'; * Uses Redis-backed storage to coordinate state between leader and follower nodes. * Tracks global initialization status for the registry. * - * Note: Per-user private server loading status is tracked separately in PrivateServersLoadStatusCache - * to enable TTL synchronization and distributed locking. - * * Designed to be extended with additional global registry metadata as needed * (e.g., last update timestamps, version info, health status). * This cache is only meant to be used internally by registry management components. diff --git a/packages/api/src/mcp/registry/cache/ServerConfigsCacheFactory.ts b/packages/api/src/mcp/registry/cache/ServerConfigsCacheFactory.ts index a707edfeba..ba0cec90ea 100644 --- a/packages/api/src/mcp/registry/cache/ServerConfigsCacheFactory.ts +++ b/packages/api/src/mcp/registry/cache/ServerConfigsCacheFactory.ts @@ -16,20 +16,16 @@ export class ServerConfigsCacheFactory { * Create a ServerConfigsCache instance. * Returns Redis implementation if Redis is configured, otherwise in-memory implementation. * - * @param owner - The owner of the cache (e.g., 'user', 'global') - only used for Redis namespacing + * @param namespace - The namespace for the cache (e.g., 'App') - only used for Redis namespacing * @param leaderOnly - Whether operations should only be performed by the leader (only applies to Redis) * @returns ServerConfigsCache instance */ - static create( - owner: string, - scope: 'Shared' | 'Private', - leaderOnly: boolean, - ): ServerConfigsCache { + static create(namespace: string, leaderOnly: boolean): ServerConfigsCache { if (cacheConfig.USE_REDIS) { - return new ServerConfigsCacheRedis(owner, scope, leaderOnly); + return new ServerConfigsCacheRedis(namespace, leaderOnly); } - // In-memory mode uses a simple Map - doesn't need owner/namespace + // In-memory mode uses a simple Map - doesn't need namespace return new ServerConfigsCacheInMemory(); } } diff --git a/packages/api/src/mcp/registry/cache/ServerConfigsCacheInMemory.ts b/packages/api/src/mcp/registry/cache/ServerConfigsCacheInMemory.ts index 4261dd3b85..0e03bf8dff 100644 --- a/packages/api/src/mcp/registry/cache/ServerConfigsCacheInMemory.ts +++ b/packages/api/src/mcp/registry/cache/ServerConfigsCacheInMemory.ts @@ -26,14 +26,6 @@ export class ServerConfigsCacheInMemory { this.cache.set(serverName, { ...config, lastUpdatedAt: Date.now() }); } - /** - * Sets a server config without checking if it exists (upsert operation). - * Use this for bulk operations where you want to add or update without error handling. - */ - public async set(serverName: string, config: ParsedServerConfig): Promise { - this.cache.set(serverName, { ...config, lastUpdatedAt: Date.now() }); - } - public async remove(serverName: string): Promise { if (!this.cache.delete(serverName)) { throw new Error(`Failed to remove server "${serverName}" in cache.`); @@ -51,12 +43,4 @@ export class ServerConfigsCacheInMemory { public async reset(): Promise { this.cache.clear(); } - - /** - * Returns a placeholder namespace for consistency with Redis implementation. - * In-memory cache doesn't use namespaces, so this always returns empty string. - */ - public getNamespace(): string { - return ''; - } } diff --git a/packages/api/src/mcp/registry/cache/ServerConfigsCacheRedis.ts b/packages/api/src/mcp/registry/cache/ServerConfigsCacheRedis.ts index 99e64cc52b..f5126a640f 100644 --- a/packages/api/src/mcp/registry/cache/ServerConfigsCacheRedis.ts +++ b/packages/api/src/mcp/registry/cache/ServerConfigsCacheRedis.ts @@ -3,61 +3,54 @@ import { fromPairs } from 'lodash'; import { standardCache, keyvRedisClient } from '~/cache'; import { ParsedServerConfig } from '~/mcp/types'; import { BaseRegistryCache } from './BaseRegistryCache'; +import { IServerConfigsRepositoryInterface } from '../ServerConfigsRepositoryInterface'; /** * Redis-backed implementation of MCP server configurations cache for distributed deployments. - * Stores server configs in Redis with namespace isolation by owner (App, User, or specific user ID). + * Stores server configs in Redis with namespace isolation. * Enables data sharing across multiple server instances in a cluster environment. * Supports optional leader-only write operations to prevent race conditions during initialization. * Data persists across server restarts and is accessible from any instance in the cluster. */ -export class ServerConfigsCacheRedis extends BaseRegistryCache { +export class ServerConfigsCacheRedis + extends BaseRegistryCache + implements IServerConfigsRepositoryInterface +{ protected readonly cache: Keyv; - private readonly owner: string; + private readonly namespace: string; - constructor(owner: string, scope: 'Shared' | 'Private', leaderOnly: boolean) { + constructor(namespace: string, leaderOnly: boolean) { super(leaderOnly); - this.owner = owner; - this.cache = standardCache(`${this.PREFIX}::Servers::${scope}::${owner}`); + this.namespace = namespace; + this.cache = standardCache(`${this.PREFIX}::Servers::${namespace}`); } public async add(serverName: string, config: ParsedServerConfig): Promise { - if (this.leaderOnly) await this.leaderCheck(`add ${this.owner} MCP servers`); + if (this.leaderOnly) await this.leaderCheck(`add ${this.namespace} MCP servers`); const exists = await this.cache.has(serverName); if (exists) throw new Error( `Server "${serverName}" already exists in cache. Use update() to modify existing configs.`, ); const success = await this.cache.set(serverName, { ...config, lastUpdatedAt: Date.now() }); - this.successCheck(`add ${this.owner} server "${serverName}"`, success); + this.successCheck(`add ${this.namespace} server "${serverName}"`, success); } public async update(serverName: string, config: ParsedServerConfig): Promise { - if (this.leaderOnly) await this.leaderCheck(`update ${this.owner} MCP servers`); + if (this.leaderOnly) await this.leaderCheck(`update ${this.namespace} MCP servers`); const exists = await this.cache.has(serverName); if (!exists) throw new Error( `Server "${serverName}" does not exist in cache. Use add() to create new configs.`, ); const success = await this.cache.set(serverName, { ...config, lastUpdatedAt: Date.now() }); - this.successCheck(`update ${this.owner} server "${serverName}"`, success); - } - - /** - * Sets a server config without checking if it exists (upsert operation). - * Use this for bulk operations where you want to add or update without error handling. - * Note: Respects leaderOnly flag if set. - */ - public async set(serverName: string, config: ParsedServerConfig): Promise { - if (this.leaderOnly) await this.leaderCheck(`set ${this.owner} MCP servers`); - const success = await this.cache.set(serverName, { ...config, lastUpdatedAt: Date.now() }); - this.successCheck(`set ${this.owner} server "${serverName}"`, success); + this.successCheck(`update ${this.namespace} server "${serverName}"`, success); } public async remove(serverName: string): Promise { - if (this.leaderOnly) await this.leaderCheck(`remove ${this.owner} MCP servers`); + if (this.leaderOnly) await this.leaderCheck(`remove ${this.namespace} MCP servers`); const success = await this.cache.delete(serverName); - this.successCheck(`remove ${this.owner} server "${serverName}"`, success); + this.successCheck(`remove ${this.namespace} server "${serverName}"`, success); } public async get(serverName: string): Promise { @@ -88,12 +81,4 @@ export class ServerConfigsCacheRedis extends BaseRegistryCache { return fromPairs(entries); } - - /** - * Returns the Redis namespace for this cache instance. - * Used for constructing full Redis keys when needed for batch operations. - */ - public getNamespace(): string { - return this.cache.namespace ?? ''; - } } diff --git a/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheFactory.test.ts b/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheFactory.test.ts deleted file mode 100644 index 018f2db8b5..0000000000 --- a/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheFactory.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { PrivateServerConfigsCacheFactory } from '../PrivateServerConfigs/PrivateServerConfigsCacheFactory'; -import { PrivateServerConfigsCacheInMemory } from '../PrivateServerConfigs/PrivateServerConfigsCacheInMemory'; -import { PrivateServerConfigsCacheRedis } from '../PrivateServerConfigs/PrivateServerConfigsCacheRedis'; -import { cacheConfig } from '~/cache'; - -// Mock the cache implementations -jest.mock('../PrivateServerConfigs/PrivateServerConfigsCacheInMemory'); -jest.mock('../PrivateServerConfigs/PrivateServerConfigsCacheRedis'); - -// Mock the cache config module -jest.mock('~/cache', () => ({ - cacheConfig: { - USE_REDIS: false, - }, -})); - -describe('PrivateServerConfigsCacheFactory', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - describe('create()', () => { - it('should return PrivateServerConfigsCacheRedis when USE_REDIS is true', () => { - // Arrange - cacheConfig.USE_REDIS = true; - - // Act - const cache = PrivateServerConfigsCacheFactory.create(); - - // Assert - expect(cache).toBeInstanceOf(PrivateServerConfigsCacheRedis); - expect(PrivateServerConfigsCacheRedis).toHaveBeenCalled(); - }); - - it('should return PrivateServerConfigsCacheInMemory when USE_REDIS is false', () => { - // Arrange - cacheConfig.USE_REDIS = false; - - // Act - const cache = PrivateServerConfigsCacheFactory.create(); - - // Assert - expect(cache).toBeInstanceOf(PrivateServerConfigsCacheInMemory); - expect(PrivateServerConfigsCacheInMemory).toHaveBeenCalled(); - }); - - it('should create PrivateServerConfigsCacheInMemory without parameters when USE_REDIS is false', () => { - // Arrange - cacheConfig.USE_REDIS = false; - - // Act - PrivateServerConfigsCacheFactory.create(); - - // Assert - // Private cache doesn't use any parameters - expect(PrivateServerConfigsCacheInMemory).toHaveBeenCalledWith(); - }); - - it('should create PrivateServerConfigsCacheRedis without parameters when USE_REDIS is true', () => { - // Arrange - cacheConfig.USE_REDIS = true; - - // Act - PrivateServerConfigsCacheFactory.create(); - - // Assert - // Private cache doesn't use any parameters - expect(PrivateServerConfigsCacheRedis).toHaveBeenCalledWith(); - }); - }); -}); diff --git a/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheInMemory.test.ts b/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheInMemory.test.ts deleted file mode 100644 index 64a7feca2a..0000000000 --- a/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheInMemory.test.ts +++ /dev/null @@ -1,346 +0,0 @@ -import { expect } from '@playwright/test'; -import { ParsedServerConfig } from '~/mcp/types'; -const FIXED_TIME = 1699564800000; -const originalDateNow = Date.now; -Date.now = jest.fn(() => FIXED_TIME); - -describe('PrivateServerConfigsCacheInMemory Tests', () => { - let PrivateServerConfigsCacheInMemory: typeof import('../PrivateServerConfigs/PrivateServerConfigsCacheInMemory').PrivateServerConfigsCacheInMemory; - let cache: InstanceType< - typeof import('../PrivateServerConfigs/PrivateServerConfigsCacheInMemory').PrivateServerConfigsCacheInMemory - >; - - // Test data - const mockConfig1: ParsedServerConfig = { - command: 'node', - args: ['server1.js'], - env: { TEST: 'value1' }, - lastUpdatedAt: FIXED_TIME, - }; - - const mockConfig2: ParsedServerConfig = { - command: 'python', - args: ['server2.py'], - env: { TEST: 'value2' }, - lastUpdatedAt: FIXED_TIME, - }; - - const mockConfig3: ParsedServerConfig = { - command: 'node', - args: ['server3.js'], - url: 'http://localhost:3000', - requiresOAuth: true, - lastUpdatedAt: FIXED_TIME, - }; - - beforeAll(async () => { - // Import modules - const cacheModule = await import('../PrivateServerConfigs/PrivateServerConfigsCacheInMemory'); - PrivateServerConfigsCacheInMemory = cacheModule.PrivateServerConfigsCacheInMemory; - }); - - afterAll(() => { - Date.now = originalDateNow; - }); - - beforeEach(() => { - // Create a fresh instance for each test - cache = new PrivateServerConfigsCacheInMemory(); - }); - - describe('add and get operations', () => { - it('should add and retrieve a server config for a user', async () => { - await cache.add('user1', 'server1', mockConfig1); - const result = await cache.get('user1', 'server1'); - expect(result).toEqual(mockConfig1); - }); - - it('should return undefined for non-existent server', async () => { - const result = await cache.get('user1', 'non-existent'); - expect(result).toBeUndefined(); - }); - - it('should throw error when adding duplicate server for same user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await expect(cache.add('user1', 'server1', mockConfig2)).rejects.toThrow( - 'Server "server1" already exists in cache. Use update() to modify existing configs.', - ); - }); - - it('should handle multiple server configs for a single user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user1', 'server2', mockConfig2); - await cache.add('user1', 'server3', mockConfig3); - - const result1 = await cache.get('user1', 'server1'); - const result2 = await cache.get('user1', 'server2'); - const result3 = await cache.get('user1', 'server3'); - - expect(result1).toEqual(mockConfig1); - expect(result2).toEqual(mockConfig2); - expect(result3).toEqual(mockConfig3); - }); - - it('should isolate server configs between different users', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user2', 'server1', mockConfig2); - - const user1Result = await cache.get('user1', 'server1'); - const user2Result = await cache.get('user2', 'server1'); - - expect(user1Result).toEqual(mockConfig1); - expect(user2Result).toEqual(mockConfig2); - }); - }); - - describe('getAll operation', () => { - it('should return empty object when user has no servers', async () => { - const result = await cache.getAll('user1'); - expect(result).toEqual({}); - }); - - it('should return all server configs for a user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user1', 'server2', mockConfig2); - await cache.add('user1', 'server3', mockConfig3); - - const result = await cache.getAll('user1'); - expect(result).toEqual({ - server1: mockConfig1, - server2: mockConfig2, - server3: mockConfig3, - }); - }); - - it('should only return configs for specific user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user1', 'server2', mockConfig2); - await cache.add('user2', 'server3', mockConfig3); - - const user1Result = await cache.getAll('user1'); - const user2Result = await cache.getAll('user2'); - - expect(Object.keys(user1Result).length).toBe(2); - expect(Object.keys(user2Result).length).toBe(1); - expect(user1Result.server3).toBeUndefined(); - expect(user2Result.server1).toBeUndefined(); - }); - }); - - describe('update operation', () => { - it('should update an existing server config', async () => { - await cache.add('user1', 'server1', mockConfig1); - expect(await cache.get('user1', 'server1')).toEqual(mockConfig1); - - await cache.update('user1', 'server1', mockConfig2); - const result = await cache.get('user1', 'server1'); - expect(result).toEqual(mockConfig2); - }); - - it('should throw error when updating non-existent server', async () => { - await expect(cache.update('user1', 'non-existent', mockConfig1)).rejects.toThrow( - 'Server "non-existent" does not exist in cache. Use add() to create new configs.', - ); - }); - - it('should only update for specific user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user2', 'server1', mockConfig2); - - await cache.update('user1', 'server1', mockConfig3); - - expect(await cache.get('user1', 'server1')).toEqual(mockConfig3); - expect(await cache.get('user2', 'server1')).toEqual(mockConfig2); - }); - }); - - describe('remove operation', () => { - it('should remove an existing server config', async () => { - await cache.add('user1', 'server1', mockConfig1); - expect(await cache.get('user1', 'server1')).toEqual(mockConfig1); - - await cache.remove('user1', 'server1'); - expect(await cache.get('user1', 'server1')).toBeUndefined(); - }); - - it('should throw error when removing non-existent server', async () => { - await expect(cache.remove('user1', 'non-existent')).rejects.toThrow( - 'Failed to remove server "non-existent" in cache.', - ); - }); - - it('should only remove from specific user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user2', 'server1', mockConfig2); - - await cache.remove('user1', 'server1'); - - expect(await cache.get('user1', 'server1')).toBeUndefined(); - expect(await cache.get('user2', 'server1')).toEqual(mockConfig2); - }); - - it('should allow re-adding a removed server', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.remove('user1', 'server1'); - await cache.add('user1', 'server1', mockConfig3); - - const result = await cache.get('user1', 'server1'); - expect(result).toEqual(mockConfig3); - }); - }); - - describe('reset operation', () => { - it('should clear all servers for a specific user', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user1', 'server2', mockConfig2); - await cache.add('user2', 'server3', mockConfig3); - - await cache.reset('user1'); - - const user1Result = await cache.getAll('user1'); - const user2Result = await cache.getAll('user2'); - - expect(Object.keys(user1Result).length).toBe(0); - expect(Object.keys(user2Result).length).toBe(1); - }); - }); - - describe('has operation', () => { - it('should return true for users with loaded cache', async () => { - await cache.add('user1', 'server1', mockConfig1); - expect(await cache.has('user1')).toBe(true); - }); - - it('should return false for users without loaded cache', async () => { - expect(await cache.has('user1')).toBe(false); - }); - }); - - describe('updateServerConfigIfExists operation', () => { - it('should update server config for all users who have it', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user2', 'server1', mockConfig1); - await cache.add('user3', 'server2', mockConfig2); - - await cache.updateServerConfigIfExists('server1', mockConfig3); - - expect(await cache.get('user1', 'server1')).toEqual(mockConfig3); - expect(await cache.get('user2', 'server1')).toEqual(mockConfig3); - expect(await cache.get('user3', 'server1')).toBeUndefined(); - expect(await cache.get('user3', 'server2')).toEqual(mockConfig2); - }); - - it('should handle case when no users have the server', async () => { - await cache.add('user1', 'server2', mockConfig2); - await cache.add('user2', 'server3', mockConfig3); - - await expect(cache.updateServerConfigIfExists('server1', mockConfig1)).resolves.not.toThrow(); - - expect(await cache.get('user1', 'server2')).toEqual(mockConfig2); - expect(await cache.get('user2', 'server3')).toEqual(mockConfig3); - }); - - it('should handle case with no loaded user caches', async () => { - await expect(cache.updateServerConfigIfExists('server1', mockConfig1)).resolves.not.toThrow(); - }); - }); - - describe('addServerConfigIfCacheExists operation', () => { - it('should add server to specified users with initialized caches', async () => { - await cache.add('user1', 'other', mockConfig1); - await cache.add('user2', 'other', mockConfig1); - - await cache.addServerConfigIfCacheExists(['user1', 'user2', 'user3'], 'server1', mockConfig2); - - expect(await cache.get('user1', 'server1')).toEqual(mockConfig2); - expect(await cache.get('user2', 'server1')).toEqual(mockConfig2); - expect(await cache.get('user3', 'server1')).toBeUndefined(); - }); - - it('should not add to users without initialized caches', async () => { - await cache.addServerConfigIfCacheExists(['user1', 'user2'], 'server1', mockConfig1); - - expect(await cache.get('user1', 'server1')).toBeUndefined(); - expect(await cache.get('user2', 'server1')).toBeUndefined(); - }); - - it('should handle empty userIds array', async () => { - await expect( - cache.addServerConfigIfCacheExists([], 'server1', mockConfig1), - ).resolves.not.toThrow(); - }); - }); - - describe('removeServerConfigIfCacheExists operation', () => { - it('should remove server from specified users', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user2', 'server1', mockConfig1); - await cache.add('user3', 'server1', mockConfig1); - - await cache.removeServerConfigIfCacheExists(['user1', 'user3'], 'server1'); - - expect(await cache.get('user1', 'server1')).toBeUndefined(); - expect(await cache.get('user2', 'server1')).toEqual(mockConfig1); - expect(await cache.get('user3', 'server1')).toBeUndefined(); - }); - - it('should handle users who do not have the server', async () => { - await cache.add('user1', 'server1', mockConfig1); - - await expect( - cache.removeServerConfigIfCacheExists(['user1', 'user2'], 'server1'), - ).resolves.not.toThrow(); - - expect(await cache.get('user1', 'server1')).toBeUndefined(); - }); - - it('should handle empty userIds array', async () => { - await expect(cache.removeServerConfigIfCacheExists([], 'server1')).resolves.not.toThrow(); - }); - }); - - describe('findUsersWithServer operation', () => { - it('should return all users who have the server', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user2', 'server1', mockConfig1); - await cache.add('user3', 'other', mockConfig2); - - const users = await cache.findUsersWithServer('server1'); - - expect(users.sort()).toEqual(['user1', 'user2'].sort()); - }); - - it('should return empty array if no users have the server', async () => { - await cache.add('user1', 'other', mockConfig1); - - const users = await cache.findUsersWithServer('server1'); - - expect(users).toEqual([]); - }); - - it('should return empty array with no loaded user caches', async () => { - const users = await cache.findUsersWithServer('server1'); - - expect(users).toEqual([]); - }); - }); - - describe('resetAll operation', () => { - it('should clear all servers for all users', async () => { - await cache.add('user1', 'server1', mockConfig1); - await cache.add('user1', 'server2', mockConfig2); - await cache.add('user2', 'server1', mockConfig1); - await cache.add('user2', 'server3', mockConfig3); - - await cache.resetAll(); - - expect(await cache.has('user1')).toBe(false); - expect(await cache.has('user2')).toBe(false); - }); - - it('should handle case with no loaded user caches', async () => { - // Should not throw - await expect(cache.resetAll()).resolves.not.toThrow(); - }); - }); -}); diff --git a/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheRedis.cache_integration.spec.ts b/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheRedis.cache_integration.spec.ts deleted file mode 100644 index 3c4978169d..0000000000 --- a/packages/api/src/mcp/registry/cache/__tests__/PrivateServerConfigsCacheRedis.cache_integration.spec.ts +++ /dev/null @@ -1,606 +0,0 @@ -import { expect } from '@playwright/test'; -import { ParsedServerConfig } from '~/mcp/types'; - -describe('PrivateServerConfigsCacheRedis Integration Tests', () => { - let PrivateServerConfigsCacheRedis: typeof import('../PrivateServerConfigs/PrivateServerConfigsCacheRedis').PrivateServerConfigsCacheRedis; - let keyvRedisClient: Awaited['keyvRedisClient']; - let cache: InstanceType< - typeof import('../PrivateServerConfigs/PrivateServerConfigsCacheRedis').PrivateServerConfigsCacheRedis - >; - - // Test data - const mockConfig1: ParsedServerConfig = { - command: 'node', - args: ['server1.js'], - env: { TEST: 'value1' }, - }; - - const mockConfig2: ParsedServerConfig = { - command: 'python', - args: ['server2.py'], - env: { TEST: 'value2' }, - }; - - const mockConfig3: ParsedServerConfig = { - command: 'node', - args: ['server3.js'], - url: 'http://localhost:3000', - requiresOAuth: true, - }; - - beforeAll(async () => { - // Set up environment variables for Redis (only if not already set) - process.env.USE_REDIS = process.env.USE_REDIS ?? 'true'; - process.env.REDIS_URI = process.env.REDIS_URI ?? 'redis://127.0.0.1:6379'; - process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER ?? 'false'; - console.log('USING CLUSETER....', process.env.USE_REDIS_CLUSTER); - - process.env.REDIS_KEY_PREFIX = - process.env.REDIS_KEY_PREFIX ?? 'PrivateServerConfigsCacheRedis-IntegrationTest'; - - // Import modules after setting env vars - const cacheModule = await import('../PrivateServerConfigs/PrivateServerConfigsCacheRedis'); - const redisClients = await import('~/cache/redisClients'); - - PrivateServerConfigsCacheRedis = cacheModule.PrivateServerConfigsCacheRedis; - keyvRedisClient = redisClients.keyvRedisClient; - - // Ensure Redis is connected - if (!keyvRedisClient) throw new Error('Redis client is not initialized'); - - // Wait for connection and topology discovery to complete - await redisClients.keyvRedisClientReady; - }); - - beforeEach(() => { - // Create a fresh instance for each test - cache = new PrivateServerConfigsCacheRedis(); - }); - - afterEach(async () => { - // Clean up: clear all test keys from Redis - if (keyvRedisClient && 'scanIterator' in keyvRedisClient) { - const pattern = '*PrivateServerConfigsCacheRedis-IntegrationTest*'; - const keysToDelete: string[] = []; - - // Collect all keys first - for await (const key of keyvRedisClient.scanIterator({ MATCH: pattern })) { - keysToDelete.push(key); - } - - // Delete in parallel for cluster mode efficiency - if (keysToDelete.length > 0) { - await Promise.all(keysToDelete.map((key) => keyvRedisClient!.del(key))); - } - } - }); - - afterAll(async () => { - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); - }); - - describe('add and get operations', () => { - it('should add and retrieve a server config for a user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - const result = await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - expect(result).toMatchObject(mockConfig1); - }); - - it('should return undefined for non-existent server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const result = await cache.get(`user1-${randonPrefix}`, 'non-existent'); - expect(result).toBeUndefined(); - }); - - it('should throw error when adding duplicate server for same user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await expect( - cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig2), - ).rejects.toThrow( - `Server "server1-${randonPrefix}" already exists in cache. Use update() to modify existing configs.`, - ); - }); - - it('should handle multiple server configs for a single user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user1-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - await cache.add(`user1-${randonPrefix}`, `server3-${randonPrefix}`, mockConfig3); - - const result1 = await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - const result2 = await cache.get(`user1-${randonPrefix}`, `server2-${randonPrefix}`); - const result3 = await cache.get(`user1-${randonPrefix}`, `server3-${randonPrefix}`); - - expect(result1).toMatchObject(mockConfig1); - expect(result2).toMatchObject(mockConfig2); - expect(result3).toMatchObject(mockConfig3); - }); - - it('should isolate server configs between different users', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig2); - - const user1Result = await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - const user2Result = await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`); - - expect(user1Result).toMatchObject(mockConfig1); - expect(user2Result).toMatchObject(mockConfig2); - }); - }); - - describe('getAll operation', () => { - it('should return empty object when user has no servers', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const result = await cache.getAll(`user1-${randonPrefix}`); - expect(result).toMatchObject({}); - }); - - it('should return all server configs for a user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user1-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - await cache.add(`user1-${randonPrefix}`, `server3-${randonPrefix}`, mockConfig3); - - const result = await cache.getAll(`user1-${randonPrefix}`); - expect(result).toMatchObject({ - [`server1-${randonPrefix}`]: mockConfig1, - [`server2-${randonPrefix}`]: mockConfig2, - [`server3-${randonPrefix}`]: mockConfig3, - }); - }); - - it('should only return configs for specific user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user1-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - await cache.add(`user2-${randonPrefix}`, `server3-${randonPrefix}`, mockConfig3); - - const user1Result = await cache.getAll(`user1-${randonPrefix}`); - const user2Result = await cache.getAll(`user2-${randonPrefix}`); - - expect(Object.keys(user1Result).length).toBe(2); - expect(Object.keys(user2Result).length).toBe(1); - expect(user1Result.server3).toBeUndefined(); - expect(user2Result.server1).toBeUndefined(); - }); - }); - - describe('update operation', () => { - it('should update an existing server config', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig1, - ); - - await cache.update(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig2); - const result = await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - expect(result).toMatchObject(mockConfig2); - }); - - it('should throw error when updating non-existent server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await expect( - cache.update(`user1-${randonPrefix}`, 'non-existent', mockConfig1), - ).rejects.toThrow( - 'Server "non-existent" does not exist in cache. Use add() to create new configs.', - ); - }); - - it('should only update for specific user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig2); - - await cache.update(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig3); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig3, - ); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - }); - }); - - describe('remove operation', () => { - it('should remove an existing server config', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig1, - ); - - await cache.remove(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - - it('should throw error when removing non-existent server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await expect(cache.remove(`user1-${randonPrefix}`, 'non-existent')).rejects.toThrow( - `Failed to remove user1-${randonPrefix} server "non-existent" in cache.`, - ); - }); - - it('should only remove from specific user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig2); - - await cache.remove(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - }); - - it('should allow re-adding a removed server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.remove(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig3); - - const result = await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`); - expect(result).toMatchObject(mockConfig3); - }); - }); - - describe('reset operation', () => { - it('should clear all servers for a specific user', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user1-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - await cache.add(`user2-${randonPrefix}`, `server3-${randonPrefix}`, mockConfig3); - - await cache.reset(`user1-${randonPrefix}`); - - const user1Result = await cache.getAll(`user1-${randonPrefix}`); - const user2Result = await cache.getAll(`user2-${randonPrefix}`); - - expect(Object.keys(user1Result).length).toBe(0); - expect(Object.keys(user2Result).length).toBe(1); - }); - }); - - describe('has operation', () => { - it('should return true for users with loaded cache', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - console.log('check'); - expect(await cache.has(`user1-${randonPrefix}`)).toBe(true); - }); - - it('should return false for users without loaded cache', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - expect(await cache.has(`user1-${randonPrefix}`)).toBe(false); - }); - }); - - describe('updateServerConfigIfExists operation', () => { - it('should update server config for all users who have it', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user3-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - - await cache.updateServerConfigIfExists(`server1-${randonPrefix}`, mockConfig3); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig3, - ); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig3, - ); - expect(await cache.get(`user3-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user3-${randonPrefix}`, `server2-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - }); - - it('should update lastUpdatedAt timestamp', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, 'server1-share', mockConfig1); - await cache.add(`user2-${randonPrefix}`, 'server1-share', mockConfig1); - - const timeBeforeUpdate = Date.now(); - await new Promise((r) => setTimeout(() => r(true), 100)); - await cache.updateServerConfigIfExists('server1-share', mockConfig2); - - const user1Result = await cache.get(`user1-${randonPrefix}`, 'server1-share'); - const user2Result = await cache.get(`user2-${randonPrefix}`, 'server1-share'); - expect(user1Result).toBeDefined(); - expect(user1Result!.lastUpdatedAt! - timeBeforeUpdate).toBeGreaterThan(0); - expect(user2Result!.lastUpdatedAt! - timeBeforeUpdate).toBeGreaterThan(0); - }); - - it('should handle case when no users have the server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - await cache.add(`user2-${randonPrefix}`, `server3-${randonPrefix}`, mockConfig3); - - await expect( - cache.updateServerConfigIfExists(`server1-${randonPrefix}`, mockConfig1), - ).resolves.not.toThrow(); - - expect(await cache.get(`user1-${randonPrefix}`, `server2-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - expect(await cache.get(`user2-${randonPrefix}`, `server3-${randonPrefix}`)).toMatchObject( - mockConfig3, - ); - }); - - it('should handle case with no user caches', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await expect( - cache.updateServerConfigIfExists(`server1-${randonPrefix}`, mockConfig1), - ).resolves.not.toThrow(); - }); - - it('should work across multiple cache instances (distributed scenario)', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const cache1 = new PrivateServerConfigsCacheRedis(); - const cache2 = new PrivateServerConfigsCacheRedis(); - - await cache1.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache1.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - - await cache2.updateServerConfigIfExists(`server1-${randonPrefix}`, mockConfig3); - - expect(await cache1.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig3, - ); - expect(await cache1.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig3, - ); - }); - }); - - describe('addServerConfigIfCacheExists operation', () => { - it('should add server to specified users with initialized caches', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, 'other', mockConfig1); - await cache.add(`user2-${randonPrefix}`, 'other', mockConfig1); - - await cache.addServerConfigIfCacheExists( - [`user1-${randonPrefix}`, `user2-${randonPrefix}`, `user3-${randonPrefix}`], - `server1-${randonPrefix}`, - mockConfig2, - ); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - expect(await cache.get(`user3-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - - it('should not add to users without initialized caches', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.addServerConfigIfCacheExists( - [`user1-${randonPrefix}`, `user2-${randonPrefix}`], - `server1-${randonPrefix}`, - mockConfig1, - ); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - - it('should handle empty userIds array', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await expect( - cache.addServerConfigIfCacheExists([], `server1-${randonPrefix}`, mockConfig1), - ).resolves.not.toThrow(); - }); - - it('should work across multiple cache instances (distributed scenario)', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const cache1 = new PrivateServerConfigsCacheRedis(); - const cache2 = new PrivateServerConfigsCacheRedis(); - - await cache1.add(`user1-${randonPrefix}`, 'other', mockConfig1); - await cache1.add(`user2-${randonPrefix}`, 'other', mockConfig1); - - await cache2.addServerConfigIfCacheExists( - [`user1-${randonPrefix}`, `user2-${randonPrefix}`, `user3-${randonPrefix}`], - `server1-${randonPrefix}`, - mockConfig2, - ); - - expect(await cache1.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - expect(await cache1.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig2, - ); - expect(await cache1.get(`user3-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - }); - - describe('removeServerConfigIfCacheExists operation', () => { - it('should remove server from specified users', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user3-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - - await cache.removeServerConfigIfCacheExists( - [`user1-${randonPrefix}`, `user3-${randonPrefix}`], - `server1-${randonPrefix}`, - ); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toMatchObject( - mockConfig1, - ); - expect(await cache.get(`user3-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - - it('should handle users who do not have the server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - - await expect( - cache.removeServerConfigIfCacheExists( - [`user1-${randonPrefix}`, `user2-${randonPrefix}`], - `server1-${randonPrefix}`, - ), - ).resolves.not.toThrow(); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - - it('should handle empty userIds array', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await expect( - cache.removeServerConfigIfCacheExists([], `server1-${randonPrefix}`), - ).resolves.not.toThrow(); - }); - - it('should work across multiple cache instances (distributed scenario)', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const cache1 = new PrivateServerConfigsCacheRedis(); - const cache2 = new PrivateServerConfigsCacheRedis(); - - await cache1.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache1.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - - await cache2.removeServerConfigIfCacheExists( - [`user1-${randonPrefix}`, `user2-${randonPrefix}`], - `server1-${randonPrefix}`, - ); - - expect(await cache1.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache1.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - }); - }); - - describe('findUsersWithServer operation', () => { - it('should return all users who have the server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user3-${randonPrefix}`, 'other', mockConfig2); - - const users = await cache.findUsersWithServer(`server1-${randonPrefix}`); - - expect(users.sort()).toEqual([`user1-${randonPrefix}`, `user2-${randonPrefix}`].sort()); - }); - - it('should return empty array if no users have the server', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, 'other', mockConfig1); - - const users = await cache.findUsersWithServer(`server1-${randonPrefix}`); - - expect(users).toEqual([]); - }); - - it('should return empty array with no user caches', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const users = await cache.findUsersWithServer(`server1-${randonPrefix}`); - - expect(users).toEqual([]); - }); - - it('should work across multiple cache instances (distributed scenario)', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const cache1 = new PrivateServerConfigsCacheRedis(); - const cache2 = new PrivateServerConfigsCacheRedis(); - - await cache1.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache1.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache1.add(`user3-${randonPrefix}`, 'other', mockConfig2); - - const users = await cache2.findUsersWithServer(`server1-${randonPrefix}`); - - expect(users.sort()).toEqual([`user1-${randonPrefix}`, `user2-${randonPrefix}`].sort()); - }); - }); - - describe('resetAll operation', () => { - it('should clear all servers for all users in Redis', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - await cache.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user1-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - await cache.add(`user2-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache.add(`user2-${randonPrefix}`, `server3-${randonPrefix}`, mockConfig3); - - await cache.resetAll(); - - expect(await cache.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user1-${randonPrefix}`, `server2-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user2-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache.get(`user2-${randonPrefix}`, `server3-${randonPrefix}`)).toBeUndefined(); - }); - - it.skip('should handle case with no user caches', async () => { - // const randonPrefix = Math.random().toString(36).substring(2, 8); - - // Should not throw - await expect(cache.resetAll()).resolves.not.toThrow(); - }); - - it('should work across multiple cache instances (distributed scenario)', async () => { - const randonPrefix = Math.random().toString(36).substring(2, 8); - - const cache1 = new PrivateServerConfigsCacheRedis(); - const cache2 = new PrivateServerConfigsCacheRedis(); - - // Add servers using cache1 - await cache1.add(`user1-${randonPrefix}`, `server1-${randonPrefix}`, mockConfig1); - await cache1.add(`user2-${randonPrefix}`, `server2-${randonPrefix}`, mockConfig2); - - // Reset using cache2 - await cache2.resetAll(); - - // Verify using cache1 - expect(await cache1.get(`user1-${randonPrefix}`, `server1-${randonPrefix}`)).toBeUndefined(); - expect(await cache1.get(`user2-${randonPrefix}`, `server2-${randonPrefix}`)).toBeUndefined(); - }); - }); -}); diff --git a/packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.cache_integration.spec.ts b/packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.cache_integration.spec.ts deleted file mode 100644 index 5a98562dfa..0000000000 --- a/packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.cache_integration.spec.ts +++ /dev/null @@ -1,397 +0,0 @@ -import { expect } from '@playwright/test'; - -describe('PrivateServersLoadStatusCache Integration Tests', () => { - let loadStatusCache: typeof import('../PrivateServersLoadStatusCache').privateServersLoadStatusCache; - let keyvRedisClient: Awaited['keyvRedisClient']; - let testCounter = 0; - - beforeAll(async () => { - // Set up environment variables for Redis (only if not already set) - process.env.USE_REDIS = process.env.USE_REDIS ?? 'true'; - process.env.REDIS_URI = process.env.REDIS_URI ?? 'redis://127.0.0.1:6379'; - process.env.REDIS_KEY_PREFIX = 'PrivateServersLoadStatusCache-IntegrationTest'; - - // Import modules after setting env vars - const loadStatusCacheModule = await import('../PrivateServersLoadStatusCache'); - const redisClients = await import('~/cache/redisClients'); - - loadStatusCache = loadStatusCacheModule.privateServersLoadStatusCache; - keyvRedisClient = redisClients.keyvRedisClient; - - // Ensure Redis is connected - if (!keyvRedisClient) throw new Error('Redis client is not initialized'); - - // Wait for Redis connection and topology discovery to complete - await redisClients.keyvRedisClientReady; - - process.setMaxListeners(200); - }); - - beforeEach(() => { - jest.resetModules(); - testCounter++; - }); - - afterEach(async () => { - // Clean up: clear all test keys from Redis - if (keyvRedisClient && 'scanIterator' in keyvRedisClient) { - const pattern = '*PrivateServersLoadStatusCache-IntegrationTest*'; - const keysToDelete: string[] = []; - - // Collect all keys first - for await (const key of keyvRedisClient.scanIterator({ MATCH: pattern })) { - keysToDelete.push(key); - } - - // Delete in parallel for cluster mode efficiency - if (keysToDelete.length > 0) { - await Promise.all(keysToDelete.map((key) => keyvRedisClient!.del(key))); - } - } - }); - - afterAll(async () => { - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); - }); - - describe('isLoaded() and setLoaded() integration', () => { - it('should persist loaded status in cache', async () => { - const userId = `user-persist-${testCounter}`; - - expect(await loadStatusCache.isLoaded(userId)).toBe(false); - - await loadStatusCache.setLoaded(userId, 60000); - - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - }); - - it('should handle multiple users independently', async () => { - const user1 = `user-multi-1-${testCounter}`; - const user2 = `user-multi-2-${testCounter}`; - const user3 = `user-multi-3-${testCounter}`; - - await loadStatusCache.setLoaded(user1, 60000); - await loadStatusCache.setLoaded(user2, 60000); - - expect(await loadStatusCache.isLoaded(user1)).toBe(true); - expect(await loadStatusCache.isLoaded(user2)).toBe(true); - expect(await loadStatusCache.isLoaded(user3)).toBe(false); - }); - - it('should respect TTL expiration (short TTL for testing)', async () => { - const userId = `user-ttl-expire-${testCounter}`; - - // Set with 1 second TTL - await loadStatusCache.setLoaded(userId, 1000); - - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - - // Wait for TTL to expire - await new Promise((resolve) => setTimeout(resolve, 1100)); - - expect(await loadStatusCache.isLoaded(userId)).toBe(false); - }, 10000); - - it('should allow re-setting loaded status', async () => { - const userId = `user-reset-${testCounter}`; - - await loadStatusCache.setLoaded(userId, 60000); - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - - await loadStatusCache.clearLoaded(userId); - expect(await loadStatusCache.isLoaded(userId)).toBe(false); - - await loadStatusCache.setLoaded(userId, 60000); - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - }); - }); - - describe('acquireLoadLock() and releaseLoadLock() integration', () => { - it('should acquire lock successfully when available', async () => { - const userId = `user-lock-acquire-${testCounter}`; - - const acquired = await loadStatusCache.acquireLoadLock(userId, 10000); - - expect(acquired).toBe(true); - - // Clean up - await loadStatusCache.releaseLoadLock(userId); - }); - - it('should prevent concurrent lock acquisition', async () => { - const userId = `user-lock-concurrent-${testCounter}`; - - const lock1 = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(lock1).toBe(true); - - const lock2 = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(lock2).toBe(false); - - // Release lock - await loadStatusCache.releaseLoadLock(userId); - - // Should be able to acquire again - const lock3 = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(lock3).toBe(true); - - await loadStatusCache.releaseLoadLock(userId); - }); - - it('should auto-release lock after TTL expires', async () => { - const userId = `user-lock-ttl-${testCounter}`; - - const acquired = await loadStatusCache.acquireLoadLock(userId, 1000); // 1 second TTL - expect(acquired).toBe(true); - - // Lock should prevent acquisition - const blocked = await loadStatusCache.acquireLoadLock(userId, 1000); - expect(blocked).toBe(false); - - // Wait for TTL to expire - await new Promise((resolve) => setTimeout(resolve, 1100)); - - // Should be able to acquire now - const reacquired = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(reacquired).toBe(true); - - await loadStatusCache.releaseLoadLock(userId); - }, 10000); - - it('should handle locks for multiple users independently', async () => { - const user1 = `user-lock-multi-1-${testCounter}`; - const user2 = `user-lock-multi-2-${testCounter}`; - const user3 = `user-lock-multi-3-${testCounter}`; - - const lock1 = await loadStatusCache.acquireLoadLock(user1, 10000); - const lock2 = await loadStatusCache.acquireLoadLock(user2, 10000); - const lock3 = await loadStatusCache.acquireLoadLock(user3, 10000); - - expect(lock1).toBe(true); - expect(lock2).toBe(true); - expect(lock3).toBe(true); - - await loadStatusCache.releaseLoadLock(user1); - await loadStatusCache.releaseLoadLock(user2); - await loadStatusCache.releaseLoadLock(user3); - }); - - it('should allow release of non-existent lock without error', async () => { - const userId = `user-lock-nonexist-${testCounter}`; - await expect(loadStatusCache.releaseLoadLock(userId)).resolves.not.toThrow(); - }); - }); - - describe('waitForLoad() integration', () => { - it('should wait and detect when loaded flag is set', async () => { - const userId = `user-wait-detect-${testCounter}`; - - // Start waiting in background - const waitPromise = loadStatusCache.waitForLoad(userId, 2000, 100); - - // Simulate another process setting the loaded flag after 300ms - const setLoadedPromise = new Promise((resolve) => { - setTimeout(async () => { - await loadStatusCache.setLoaded(userId, 60000); - // Add small delay to ensure Redis write completes - await new Promise((r) => setTimeout(r, 50)); - resolve(); - }, 300); - }); - - // Await both in parallel - waitPromise should complete first - const [result] = await Promise.all([waitPromise, setLoadedPromise]); - - expect(result).toBe(true); - }, 5000); - - it('should timeout if loaded flag is never set', async () => { - const userId = `user-timeout-${testCounter}`; - - const result = await loadStatusCache.waitForLoad(userId, 300, 50); - - expect(result).toBe(false); - }, 1000); - - it('should return immediately if already loaded', async () => { - const userId = `user-immediate-${testCounter}`; - - await loadStatusCache.setLoaded(userId, 60000); - // Small delay to ensure Redis write completes - await new Promise((resolve) => setTimeout(resolve, 50)); - - const startTime = Date.now(); - const result = await loadStatusCache.waitForLoad(userId, 5000, 100); - const elapsed = Date.now() - startTime; - - expect(result).toBe(true); - expect(elapsed).toBeLessThan(300); // Increased tolerance for CI environments - }); - }); - - describe('Complete load workflow integration', () => { - it('should simulate distributed load coordination', async () => { - const userId = `user-distributed-${testCounter}`; - - // Process 1: Acquires lock and loads - const lock1 = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(lock1).toBe(true); - - // Process 2: Tries to acquire lock (should fail) and waits - const lock2 = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(lock2).toBe(false); - - const waitPromise = loadStatusCache.waitForLoad(userId, 3000, 100); - - // Process 1: Completes loading after 300ms - const process1Promise = new Promise((resolve) => { - setTimeout(async () => { - await loadStatusCache.setLoaded(userId, 60000); - await new Promise((r) => setTimeout(r, 50)); // Redis write delay - await loadStatusCache.releaseLoadLock(userId); - resolve(); - }, 300); - }); - - // Process 2: Should detect completion - const completed = await waitPromise; - expect(completed).toBe(true); - - // Both processes should now see it as loaded - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - - // Wait for process 1 to complete cleanup - await process1Promise; - }, 10000); - - it('should handle process crash scenario (lock timeout)', async () => { - const userId = `user-crash-${testCounter}`; - - // Process 1: Acquires lock but crashes (doesn't release) - const lock1 = await loadStatusCache.acquireLoadLock(userId, 1000); // 1 second TTL - expect(lock1).toBe(true); - // (simulate crash - no releaseLoadLock call) - - // Process 2: Waits for timeout - const waitResult = await loadStatusCache.waitForLoad(userId, 1500, 200); - expect(waitResult).toBe(false); // Timeout (process 1 never completed) - - // After lock TTL expires, process 2 can retry - await new Promise((resolve) => setTimeout(resolve, 200)); - - const retryLock = await loadStatusCache.acquireLoadLock(userId, 10000); - expect(retryLock).toBe(true); - - // Process 2 completes successfully - await loadStatusCache.setLoaded(userId, 60000); - await loadStatusCache.releaseLoadLock(userId); - - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - }, 10000); - - it('should handle concurrent user loads independently', async () => { - const user1 = `user-concurrent-1-${testCounter}`; - const user2 = `user-concurrent-2-${testCounter}`; - const user3 = `user-concurrent-3-${testCounter}`; - - // Simulate 3 users loading concurrently - const user1Lock = await loadStatusCache.acquireLoadLock(user1, 10000); - const user2Lock = await loadStatusCache.acquireLoadLock(user2, 10000); - const user3Lock = await loadStatusCache.acquireLoadLock(user3, 10000); - - expect(user1Lock).toBe(true); - expect(user2Lock).toBe(true); - expect(user3Lock).toBe(true); - - // All complete independently - await Promise.all([ - (async () => { - await loadStatusCache.setLoaded(user1, 60000); - await loadStatusCache.releaseLoadLock(user1); - })(), - (async () => { - await loadStatusCache.setLoaded(user2, 60000); - await loadStatusCache.releaseLoadLock(user2); - })(), - (async () => { - await loadStatusCache.setLoaded(user3, 60000); - await loadStatusCache.releaseLoadLock(user3); - })(), - ]); - - // Small delay for Redis propagation - await new Promise((resolve) => setTimeout(resolve, 100)); - - expect(await loadStatusCache.isLoaded(user1)).toBe(true); - expect(await loadStatusCache.isLoaded(user2)).toBe(true); - expect(await loadStatusCache.isLoaded(user3)).toBe(true); - }); - }); - - describe('TTL synchronization', () => { - it('should keep loaded flag and cache entry in sync via TTL', async () => { - const userId = `user-ttl-sync-${testCounter}`; - - // Set loaded flag with 1 second TTL - await loadStatusCache.setLoaded(userId, 1000); - - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - - // After TTL expires, both should be gone - await new Promise((resolve) => setTimeout(resolve, 1100)); - - expect(await loadStatusCache.isLoaded(userId)).toBe(false); - - // This simulates cache entry and loaded flag being in sync - // In real usage, if cache entries expire, loaded flag should also expire - }, 10000); - - it('should allow different TTLs for different users', async () => { - const user1 = `user-ttl-diff-1-${testCounter}`; - const user2 = `user-ttl-diff-2-${testCounter}`; - - await loadStatusCache.setLoaded(user1, 1000); // 1 second - await loadStatusCache.setLoaded(user2, 3000); // 3 seconds - - expect(await loadStatusCache.isLoaded(user1)).toBe(true); - expect(await loadStatusCache.isLoaded(user2)).toBe(true); - - // Wait for user1 to expire - await new Promise((resolve) => setTimeout(resolve, 1100)); - - expect(await loadStatusCache.isLoaded(user1)).toBe(false); - expect(await loadStatusCache.isLoaded(user2)).toBe(true); // Still valid - - // Wait for user2 to expire - await new Promise((resolve) => setTimeout(resolve, 2000)); - - expect(await loadStatusCache.isLoaded(user2)).toBe(false); - }, 10000); - }); - - describe('clearLoaded() integration', () => { - it('should clear loaded status immediately', async () => { - const userId = `user-clear-${testCounter}`; - - await loadStatusCache.setLoaded(userId, 60000); - expect(await loadStatusCache.isLoaded(userId)).toBe(true); - - await loadStatusCache.clearLoaded(userId); - expect(await loadStatusCache.isLoaded(userId)).toBe(false); - }); - - it('should allow clearing multiple users', async () => { - const user1 = `user-clear-multi-1-${testCounter}`; - const user2 = `user-clear-multi-2-${testCounter}`; - - await loadStatusCache.setLoaded(user1, 60000); - await loadStatusCache.setLoaded(user2, 60000); - - await loadStatusCache.clearLoaded(user1); - await loadStatusCache.clearLoaded(user2); - - expect(await loadStatusCache.isLoaded(user1)).toBe(false); - expect(await loadStatusCache.isLoaded(user2)).toBe(false); - }); - }); -}); diff --git a/packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.test.ts b/packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.test.ts deleted file mode 100644 index 881f82f2e5..0000000000 --- a/packages/api/src/mcp/registry/cache/__tests__/PrivateServersLoadStatusCache.test.ts +++ /dev/null @@ -1,329 +0,0 @@ -// Mock dependencies BEFORE imports to avoid hoisting issues -const mockGet = jest.fn(); -const mockSet = jest.fn(); -const mockDelete = jest.fn(); -const mockRedisSet = jest.fn(); -const mockRedisDel = jest.fn(); - -jest.mock('~/cache', () => ({ - standardCache: jest.fn(() => ({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - get: (...args: any[]) => mockGet(...args), - // eslint-disable-next-line @typescript-eslint/no-explicit-any - set: (...args: any[]) => mockSet(...args), - // eslint-disable-next-line @typescript-eslint/no-explicit-any - delete: (...args: any[]) => mockDelete(...args), - })), - keyvRedisClient: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - set: (...args: any[]) => mockRedisSet(...args), - // eslint-disable-next-line @typescript-eslint/no-explicit-any - del: (...args: any[]) => mockRedisDel(...args), - }, -})); - -jest.mock('~/cache/cacheConfig', () => ({ - cacheConfig: { - REDIS_KEY_PREFIX: '', - GLOBAL_PREFIX_SEPARATOR: '::', - }, -})); - -jest.mock('@librechat/data-schemas', () => ({ - logger: { - debug: jest.fn(), - info: jest.fn(), - warn: jest.fn(), - error: jest.fn(), - }, -})); - -jest.mock('~/cluster', () => ({ - isLeader: jest.fn().mockResolvedValue(true), -})); - -import { privateServersLoadStatusCache as loadStatusCache } from '../PrivateServersLoadStatusCache'; -import { logger } from '@librechat/data-schemas'; - -describe('PrivateServersLoadStatusCache', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - describe('isLoaded()', () => { - it('should return true when user servers are loaded', async () => { - mockGet.mockResolvedValue(true); - - const result = await loadStatusCache.isLoaded('user1'); - - expect(result).toBe(true); - expect(mockGet).toHaveBeenCalledWith('USER_PRIVATE_SERVERS_LOADED::user1'); - }); - - it('should return false when user servers are not loaded', async () => { - mockGet.mockResolvedValue(undefined); - - const result = await loadStatusCache.isLoaded('user1'); - - expect(result).toBe(false); - expect(mockGet).toHaveBeenCalledWith('USER_PRIVATE_SERVERS_LOADED::user1'); - }); - - it('should return false when loaded flag is explicitly false', async () => { - mockGet.mockResolvedValue(false); - - const result = await loadStatusCache.isLoaded('user1'); - - expect(result).toBe(false); - }); - }); - - describe('setLoaded()', () => { - it('should set loaded flag with default TTL', async () => { - mockSet.mockResolvedValue(true); - - await loadStatusCache.setLoaded('user1'); - - expect(mockSet).toHaveBeenCalledWith('USER_PRIVATE_SERVERS_LOADED::user1', true, 3600_000); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Marked user user1 as loaded (TTL: 3600000ms)', - ); - }); - - it('should set loaded flag with custom TTL', async () => { - mockSet.mockResolvedValue(true); - - await loadStatusCache.setLoaded('user1', 7200000); - - expect(mockSet).toHaveBeenCalledWith('USER_PRIVATE_SERVERS_LOADED::user1', true, 7200_000); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Marked user user1 as loaded (TTL: 7200000ms)', - ); - }); - - it('should throw error if cache.set fails', async () => { - mockSet.mockResolvedValue(false); - - await expect(loadStatusCache.setLoaded('user1')).rejects.toThrow(); - }); - }); - - describe('acquireLoadLock()', () => { - it('should acquire lock successfully when no lock exists (using Redis SET NX)', async () => { - mockRedisSet.mockResolvedValue('OK'); // Redis SET NX returns 'OK' on success - - const result = await loadStatusCache.acquireLoadLock('user1'); - - expect(result).toBe(true); - expect(mockRedisSet).toHaveBeenCalledWith( - 'MCP::ServersRegistry::PrivateServersLoadStatus:USER_PRIVATE_SERVERS_LOAD_LOCK::user1', - expect.any(String), // Timestamp as string - { NX: true, PX: 30000 }, - ); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Acquired load lock for user user1 (TTL: 30000ms)', - ); - }); - - it('should fail to acquire lock when lock already exists (Redis returns null)', async () => { - mockRedisSet.mockResolvedValue(null); // Redis SET NX returns null if key exists - - const result = await loadStatusCache.acquireLoadLock('user1'); - - expect(result).toBe(false); - expect(mockRedisSet).toHaveBeenCalledWith( - 'MCP::ServersRegistry::PrivateServersLoadStatus:USER_PRIVATE_SERVERS_LOAD_LOCK::user1', - expect.any(String), - { NX: true, PX: 30000 }, - ); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Load lock already held for user user1', - ); - }); - - it('should acquire lock with custom TTL', async () => { - mockRedisSet.mockResolvedValue('OK'); - - const result = await loadStatusCache.acquireLoadLock('user1', 60_000); - - expect(result).toBe(true); - expect(mockRedisSet).toHaveBeenCalledWith( - 'MCP::ServersRegistry::PrivateServersLoadStatus:USER_PRIVATE_SERVERS_LOAD_LOCK::user1', - expect.any(String), - { NX: true, PX: 60_000 }, - ); - }); - - it('should return false if Redis SET fails with error', async () => { - mockRedisSet.mockRejectedValue(new Error('Redis error')); - - const result = await loadStatusCache.acquireLoadLock('user1'); - - expect(result).toBe(false); - expect(logger.error).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Error acquiring lock for user user1:', - expect.any(Error), - ); - }); - }); - - describe('releaseLoadLock()', () => { - it('should release lock successfully', async () => { - await loadStatusCache.releaseLoadLock('user1'); - - expect(mockDelete).toHaveBeenCalledWith('USER_PRIVATE_SERVERS_LOAD_LOCK::user1'); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Released load lock for user user1', - ); - }); - - it('should not throw error if lock does not exist', async () => { - mockDelete.mockResolvedValue(undefined); - - await expect(loadStatusCache.releaseLoadLock('user1')).resolves.not.toThrow(); - }); - }); - - describe('waitForLoad()', () => { - let mockDateNow: jest.SpyInstance; - let currentTime: number; - - beforeEach(() => { - jest.useFakeTimers(); - currentTime = 1000000; // Starting time - mockDateNow = jest.spyOn(Date, 'now').mockImplementation(() => currentTime); - }); - - afterEach(() => { - jest.useRealTimers(); - mockDateNow.mockRestore(); - }); - - it('should return true when loading completes within timeout', async () => { - let checkCount = 0; - mockGet.mockImplementation(async () => { - checkCount++; - return checkCount >= 3; // Return true on third check - }); - - const waitPromise = loadStatusCache.waitForLoad('user1', 500, 100); - - // Simulate time passing - for (let i = 0; i < 3; i++) { - currentTime += 100; - await jest.advanceTimersByTimeAsync(100); - } - - const result = await waitPromise; - - expect(result).toBe(true); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] User user1 loading completed by another process', - ); - }); - - it('should return false when timeout is reached', async () => { - mockGet.mockResolvedValue(false); // Never becomes true - - const waitPromise = loadStatusCache.waitForLoad('user1', 300, 100); - - // Advance time past the timeout - currentTime += 400; - await jest.advanceTimersByTimeAsync(400); - - const result = await waitPromise; - - expect(result).toBe(false); - expect(logger.warn).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Timeout waiting for user user1 loading (waited 300ms)', - ); - }); - - it('should use default timeout and check interval', async () => { - mockGet.mockResolvedValue(true); - - const waitPromise = loadStatusCache.waitForLoad('user1'); - - currentTime += 100; - await jest.advanceTimersByTimeAsync(100); - - const result = await waitPromise; - - expect(result).toBe(true); - }); - - it('should poll at specified intervals', async () => { - let checkCount = 0; - mockGet.mockImplementation(async () => { - checkCount++; - return checkCount >= 4; // Return true on fourth check - }); - - const waitPromise = loadStatusCache.waitForLoad('user1', 1000, 200); - - // Advance time for each poll - for (let i = 0; i < 4; i++) { - currentTime += 200; - await jest.advanceTimersByTimeAsync(200); - } - - const result = await waitPromise; - - expect(result).toBe(true); - expect(checkCount).toBe(4); - }); - }); - - describe('clearLoaded()', () => { - it('should clear loaded status for a user', async () => { - await loadStatusCache.clearLoaded('user1'); - - expect(mockDelete).toHaveBeenCalledWith('USER_PRIVATE_SERVERS_LOADED::user1'); - expect(logger.debug).toHaveBeenCalledWith( - '[MCP][LoadStatusCache] Cleared loaded status for user user1', - ); - }); - - it('should not throw error if loaded status does not exist', async () => { - mockDelete.mockResolvedValue(undefined); - - await expect(loadStatusCache.clearLoaded('user1')).resolves.not.toThrow(); - }); - }); - - describe('Edge cases', () => { - it('should handle multiple users independently', async () => { - mockRedisSet.mockResolvedValue('OK'); - - const lock1 = await loadStatusCache.acquireLoadLock('user1'); - const lock2 = await loadStatusCache.acquireLoadLock('user2'); - - expect(lock1).toBe(true); - expect(lock2).toBe(true); - expect(mockRedisSet).toHaveBeenCalledWith( - 'MCP::ServersRegistry::PrivateServersLoadStatus:USER_PRIVATE_SERVERS_LOAD_LOCK::user1', - expect.any(String), - { NX: true, PX: 30000 }, - ); - expect(mockRedisSet).toHaveBeenCalledWith( - 'MCP::ServersRegistry::PrivateServersLoadStatus:USER_PRIVATE_SERVERS_LOAD_LOCK::user2', - expect.any(String), - { NX: true, PX: 30000 }, - ); - }); - - it('should handle concurrent operations on same user', async () => { - mockRedisSet - .mockResolvedValueOnce('OK') // First lock attempt succeeds - .mockResolvedValueOnce(null); // Second lock attempt fails (key exists) - - const [lock1, lock2] = await Promise.all([ - loadStatusCache.acquireLoadLock('user1'), - loadStatusCache.acquireLoadLock('user1'), - ]); - - // One should succeed, one should fail (order not guaranteed) - expect([lock1, lock2].sort()).toEqual([false, true]); - }); - }); -}); diff --git a/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheFactory.test.ts b/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheFactory.test.ts index 1be5bf290a..7499ae127e 100644 --- a/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheFactory.test.ts +++ b/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheFactory.test.ts @@ -25,11 +25,11 @@ describe('ServerConfigsCacheFactory', () => { cacheConfig.USE_REDIS = true; // Act - const cache = ServerConfigsCacheFactory.create('TestOwner', 'Private', true); + const cache = ServerConfigsCacheFactory.create('App', true); // Assert expect(cache).toBeInstanceOf(ServerConfigsCacheRedis); - expect(ServerConfigsCacheRedis).toHaveBeenCalledWith('TestOwner', 'Private', true); + expect(ServerConfigsCacheRedis).toHaveBeenCalledWith('App', true); }); it('should return ServerConfigsCacheInMemory when USE_REDIS is false', () => { @@ -37,7 +37,7 @@ describe('ServerConfigsCacheFactory', () => { cacheConfig.USE_REDIS = false; // Act - const cache = ServerConfigsCacheFactory.create('TestOwner', 'Private', false); + const cache = ServerConfigsCacheFactory.create('App', false); // Assert expect(cache).toBeInstanceOf(ServerConfigsCacheInMemory); @@ -49,10 +49,10 @@ describe('ServerConfigsCacheFactory', () => { cacheConfig.USE_REDIS = true; // Act - ServerConfigsCacheFactory.create('App', 'Shared', true); + ServerConfigsCacheFactory.create('CustomNamespace', true); // Assert - expect(ServerConfigsCacheRedis).toHaveBeenCalledWith('App', 'Shared', true); + expect(ServerConfigsCacheRedis).toHaveBeenCalledWith('CustomNamespace', true); }); it('should create ServerConfigsCacheInMemory without parameters when USE_REDIS is false', () => { @@ -60,10 +60,10 @@ describe('ServerConfigsCacheFactory', () => { cacheConfig.USE_REDIS = false; // Act - ServerConfigsCacheFactory.create('User', 'Shared', false); + ServerConfigsCacheFactory.create('App', false); // Assert - // In-memory cache doesn't use owner/leaderOnly parameters + // In-memory cache doesn't use namespace/leaderOnly parameters expect(ServerConfigsCacheInMemory).toHaveBeenCalledWith(); }); }); diff --git a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts new file mode 100644 index 0000000000..302a086193 --- /dev/null +++ b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts @@ -0,0 +1,50 @@ +/* eslint-disable @typescript-eslint/no-unused-vars */ +import { ParsedServerConfig } from '~/mcp/types'; +import { IServerConfigsRepositoryInterface } from '../ServerConfigsRepositoryInterface'; +import { logger } from '@librechat/data-schemas'; + +/** + * DB backed config storage + * Handles CRUD Methods of dynamic mcp servers + * Will handle Permission ACL + */ +export class ServerConfigsDB implements IServerConfigsRepositoryInterface { + public async add(serverName: string, config: ParsedServerConfig, userId?: string): Promise { + logger.debug('ServerConfigsDB add not yet implemented'); + return; + } + + public async update( + serverName: string, + config: ParsedServerConfig, + userId?: string, + ): Promise { + logger.debug('ServerConfigsDB update not yet implemented'); + return; + } + + public async remove(serverName: string, userId?: string): Promise { + logger.debug('ServerConfigsDB remove not yet implemented'); + return; + } + + public async get(serverName: string, userId?: string): Promise { + logger.debug('ServerConfigsDB get not yet implemented'); + return; + } + + /** + * Return all DB stored configs (scoped by user Id if provided) + * @param userId optional user id. if not provided only publicly shared mcp configs will be returned + * @returns record of parsed configs + */ + public async getAll(userId?: string): Promise> { + logger.debug('ServerConfigsDB getAll not yet implemented'); + return {}; + } + + public async reset(): Promise { + logger.warn('Attempt to reset the DB config storage'); + return; + } +}