From 7c38d6d1b042b149a003e9291d1b83900be05f8f Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Tue, 9 Dec 2025 07:01:37 -0800 Subject: [PATCH] fix: attempt reconnection on token expiry --- .../oauth/OAuthReconnectionManager.test.ts | 35 ++++++++++++++++--- .../src/mcp/oauth/OAuthReconnectionManager.ts | 6 ---- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts index f9a3c7ab73..f8f17bd54c 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts @@ -250,23 +250,50 @@ describe('OAuthReconnectionManager', () => { expect(mockMCPManager.disconnectUserConnection).toHaveBeenCalledWith(userId, 'server1'); }); - it('should not reconnect servers with expired tokens', async () => { + it('should attempt to reconnect servers with expired tokens (refresh will be attempted)', async () => { const userId = 'user-123'; const oauthServers = new Set(['server1']); (mcpServersRegistry.getOAuthServers as jest.Mock).mockResolvedValue(oauthServers); - // server1: has expired token + // server1: has expired token (but refresh token should still work) tokenMethods.findToken.mockResolvedValue({ userId, identifier: 'mcp:server1', expiresAt: new Date(Date.now() - 3600000), // 1 hour ago } as unknown as MCPOAuthTokens); + // Mock successful reconnection (token refresh happens internally) + const mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + }; + mockMCPManager.getUserConnection.mockResolvedValue( + mockConnection as unknown as MCPConnection, + ); + (mcpServersRegistry.getServerConfig as jest.Mock).mockResolvedValue( + {} as unknown as MCPOptions, + ); + await reconnectionManager.reconnectServers(userId); - // Verify no reconnection attempt was made + // Verify reconnection attempt was made (token refresh will happen during getUserConnection) + expect(reconnectionTracker.isActive(userId, 'server1')).toBe(true); + + // Wait for async tryReconnect to complete + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(mockMCPManager.getUserConnection).toHaveBeenCalledWith({ + serverName: 'server1', + user: { id: userId }, + flowManager, + tokenMethods, + forceNew: false, + connectionTimeout: 10000, // DEFAULT_CONNECTION_TIMEOUT_MS + returnOnOAuth: true, + }); + + // Verify successful reconnection cleared the states + expect(reconnectionTracker.isFailed(userId, 'server1')).toBe(false); expect(reconnectionTracker.isActive(userId, 'server1')).toBe(false); - expect(mockMCPManager.getUserConnection).not.toHaveBeenCalled(); }); it('should handle connection that returns but is not connected', async () => { diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts index db81a2ffae..3130e9cc43 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts @@ -174,12 +174,6 @@ export class OAuthReconnectionManager { return false; } - // if the token has expired, don't attempt to reconnect - const now = new Date(); - if (accessToken.expiresAt && accessToken.expiresAt < now) { - return false; - } - // …otherwise, we're good to go with the reconnect attempt return true; }