🔄 refactor: MCP Server Init and Stale Cache Handling (#10984)

* 🔧 refactor: Update MCP connection handling to improve performance and testing

* refactor: Replace getAll() with getLoaded() in MCP.js to prevent unnecessary connection creation for user-context servers.
* test: Adjust MCP.spec.js to mock getLoaded() instead of getAll() for consistency with the new implementation.
* feat: Enhance MCPServersInitializer to reset initialization flag for better handling of process restarts and stale data.
* test: Add integration tests to verify re-initialization behavior and ensure stale data is cleared when necessary.

* 🔧 refactor: Enhance cached endpoints config handling for GPT plugins

* refactor: Update MCPServersInitializer tests to use new server management methods

* refactor: Replace direct Redis server manipulation with registry.addServer and registry.getServerConfig for better abstraction and consistency.
* test: Adjust integration tests to verify server initialization and stale data handling using the updated methods.

* 🔧 refactor: Increase retry limits and delay for MCP server creation

* Updated MAX_CREATE_RETRIES from 3 to 5 to allow for more attempts during server creation.
* Increased RETRY_BASE_DELAY_MS from 10 to 25 milliseconds to provide a longer wait time between retries, improving stability in server initialization.

* refactor: Update MCPServersInitializer tests to utilize new registry methods

* refactor: Replace direct access to sharedAppServers with registry.getServerConfig for improved abstraction.
* test: Adjust tests to verify server initialization and stale data handling using the updated registry methods, ensuring consistency and clarity in the test structure.
This commit is contained in:
Danny Avila 2025-12-15 16:46:56 -05:00 committed by GitHub
parent 02fc4647e1
commit dcd9273700
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 198 additions and 12 deletions

View file

@ -19,7 +19,11 @@ async function getEndpointsConfig(req) {
const cache = getLogStores(CacheKeys.CONFIG_STORE);
const cachedEndpointsConfig = await cache.get(CacheKeys.ENDPOINT_CONFIG);
if (cachedEndpointsConfig) {
return cachedEndpointsConfig;
if (cachedEndpointsConfig.gptPlugins) {
await cache.delete(CacheKeys.ENDPOINT_CONFIG);
} else {
return cachedEndpointsConfig;
}
}
const appConfig = req.config ?? (await getAppConfig({ role: req.user?.role }));

View file

@ -448,7 +448,10 @@ async function getMCPSetupData(userId) {
/** @type {Map<string, import('@librechat/api').MCPConnection>} */
let appConnections = new Map();
try {
appConnections = (await mcpManager.appConnections?.getAll()) || new Map();
// Use getLoaded() instead of getAll() to avoid forcing connection creation
// getAll() creates connections for all servers, which is problematic for servers
// that require user context (e.g., those with {{LIBRECHAT_USER_ID}} placeholders)
appConnections = (await mcpManager.appConnections?.getLoaded()) || new Map();
} catch (error) {
logger.error(`[MCP][User: ${userId}] Error getting app connections:`, error);
}

View file

@ -128,7 +128,7 @@ describe('tests for the new helper functions used by the MCP connection status e
beforeEach(() => {
mockGetMCPManager.mockReturnValue({
appConnections: { getAll: jest.fn(() => new Map()) },
appConnections: { getLoaded: jest.fn(() => new Map()) },
getUserConnections: jest.fn(() => new Map()),
});
mockRegistryInstance.getOAuthServers.mockResolvedValue(new Set());
@ -143,7 +143,7 @@ describe('tests for the new helper functions used by the MCP connection status e
const mockOAuthServers = new Set(['server2']);
const mockMCPManager = {
appConnections: { getAll: jest.fn(() => Promise.resolve(mockAppConnections)) },
appConnections: { getLoaded: jest.fn(() => Promise.resolve(mockAppConnections)) },
getUserConnections: jest.fn(() => mockUserConnections),
};
mockGetMCPManager.mockReturnValue(mockMCPManager);
@ -153,7 +153,7 @@ describe('tests for the new helper functions used by the MCP connection status e
expect(mockRegistryInstance.getAllServerConfigs).toHaveBeenCalledWith(mockUserId);
expect(mockGetMCPManager).toHaveBeenCalledWith(mockUserId);
expect(mockMCPManager.appConnections.getAll).toHaveBeenCalled();
expect(mockMCPManager.appConnections.getLoaded).toHaveBeenCalled();
expect(mockMCPManager.getUserConnections).toHaveBeenCalledWith(mockUserId);
expect(mockRegistryInstance.getOAuthServers).toHaveBeenCalledWith(mockUserId);
@ -174,7 +174,7 @@ describe('tests for the new helper functions used by the MCP connection status e
mockRegistryInstance.getAllServerConfigs.mockResolvedValue(mockConfig);
const mockMCPManager = {
appConnections: { getAll: jest.fn(() => Promise.resolve(null)) },
appConnections: { getLoaded: jest.fn(() => Promise.resolve(null)) },
getUserConnections: jest.fn(() => null),
};
mockGetMCPManager.mockReturnValue(mockMCPManager);