From 9d2aba5df582d68a8b8529895dd0da265d21a17f Mon Sep 17 00:00:00 2001 From: Federico Ruggi Date: Sat, 20 Sep 2025 17:06:23 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20fix:=20Handle=20Null=20?= =?UTF-8?q?`MCPManager`=20In=20`OAuthReconnectionManager`=20(#9740)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../oauth/OAuthReconnectionManager.test.ts | 28 ++++++++++++++++ .../src/mcp/oauth/OAuthReconnectionManager.ts | 33 ++++++++++++++----- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts index 78fedb9c3..8f18df2f5 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts @@ -290,5 +290,33 @@ describe('OAuthReconnectionManager', () => { expect(reconnectionTracker.isActive(userId, 'server1')).toBe(false); expect(mockMCPManager.disconnectUserConnection).toHaveBeenCalledWith(userId, 'server1'); }); + + it('should handle MCPManager not available gracefully', async () => { + const userId = 'user-123'; + + // Reset singleton first + (OAuthReconnectionManager as unknown as { instance: null }).instance = null; + + // Mock MCPManager.getInstance to throw (simulating no MCP manager available) + (MCPManager.getInstance as jest.Mock).mockImplementation(() => { + throw new Error('MCPManager has not been initialized.'); + }); + + // Create a reconnection manager without MCPManager available + const reconnectionTracker = new OAuthReconnectionTracker(); + const reconnectionManagerWithoutMCP = await OAuthReconnectionManager.createInstance( + flowManager, + tokenMethods, + reconnectionTracker, + ); + + // Verify that the method does not throw and completes successfully + await expect(reconnectionManagerWithoutMCP.reconnectServers(userId)).resolves.toBeUndefined(); + + // Verify that the method returns early without attempting any reconnections + expect(tokenMethods.findToken).not.toHaveBeenCalled(); + expect(mockMCPManager.getUserConnection).not.toHaveBeenCalled(); + expect(mockMCPManager.disconnectUserConnection).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts index 48b751dfa..b819403a6 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts @@ -13,6 +13,7 @@ export class OAuthReconnectionManager { protected readonly flowManager: FlowStateManager; protected readonly tokenMethods: TokenMethods; + private readonly mcpManager: MCPManager | null; private readonly reconnectionsTracker: OAuthReconnectionTracker; @@ -46,6 +47,12 @@ export class OAuthReconnectionManager { this.flowManager = flowManager; this.tokenMethods = tokenMethods; this.reconnectionsTracker = reconnections ?? new OAuthReconnectionTracker(); + + try { + this.mcpManager = MCPManager.getInstance(); + } catch { + this.mcpManager = null; + } } public isReconnecting(userId: string, serverName: string): boolean { @@ -53,11 +60,17 @@ export class OAuthReconnectionManager { } public async reconnectServers(userId: string) { - const mcpManager = MCPManager.getInstance(); + // Check if MCPManager is available + if (this.mcpManager == null) { + logger.warn( + '[OAuthReconnectionManager] MCPManager not available, skipping OAuth MCP server reconnection', + ); + return; + } // 1. derive the servers to reconnect const serversToReconnect = []; - for (const serverName of mcpManager.getOAuthServers() ?? []) { + for (const serverName of this.mcpManager.getOAuthServers() ?? []) { const canReconnect = await this.canReconnect(userId, serverName); if (canReconnect) { serversToReconnect.push(serverName); @@ -81,23 +94,25 @@ export class OAuthReconnectionManager { } private async tryReconnect(userId: string, serverName: string) { - const mcpManager = MCPManager.getInstance(); + if (this.mcpManager == null) { + return; + } const logPrefix = `[tryReconnectOAuthMCPServer][User: ${userId}][${serverName}]`; logger.info(`${logPrefix} Attempting reconnection`); - const config = mcpManager.getRawConfig(serverName); + const config = this.mcpManager.getRawConfig(serverName); const cleanupOnFailedReconnect = () => { this.reconnectionsTracker.setFailed(userId, serverName); this.reconnectionsTracker.removeActive(userId, serverName); - mcpManager.disconnectUserConnection(userId, serverName); + this.mcpManager?.disconnectUserConnection(userId, serverName); }; try { // attempt to get connection (this will use existing tokens and refresh if needed) - const connection = await mcpManager.getUserConnection({ + const connection = await this.mcpManager.getUserConnection({ serverName, user: { id: userId } as TUser, flowManager: this.flowManager, @@ -125,7 +140,9 @@ export class OAuthReconnectionManager { } private async canReconnect(userId: string, serverName: string) { - const mcpManager = MCPManager.getInstance(); + if (this.mcpManager == null) { + return false; + } // if the server has failed reconnection, don't attempt to reconnect if (this.reconnectionsTracker.isFailed(userId, serverName)) { @@ -133,7 +150,7 @@ export class OAuthReconnectionManager { } // if the server is already connected, don't attempt to reconnect - const existingConnections = mcpManager.getUserConnections(userId); + const existingConnections = this.mcpManager.getUserConnections(userId); if (existingConnections?.has(serverName)) { const isConnected = await existingConnections.get(serverName)?.isConnected(); if (isConnected) {