From bc038213db4c205c69e8cfe9dccbaa88616be694 Mon Sep 17 00:00:00 2001
From: Danny Avila <danny@librechat.ai>
Date: Wed, 1 Apr 2026 20:14:07 -0400
Subject: [PATCH] test: add useOAuth derivation and getUserConnection coverage

- Fix fallback handler test to use connection.on (not once), remove
  redundant assertion, and verify removeListener cleanup
- Add MCPManager test for non-OAuth discoverServerTools path
- Add getUserConnection tests verifying useOAuth is derived from
  config.requiresOAuth, not hardcoded
- Add test that OAuth server without flowManager throws early
---
 .../__tests__/MCPConnectionFactory.test.ts    |  15 ++-
 .../api/src/mcp/__tests__/MCPManager.test.ts  | 123 ++++++++++++++++++
 2 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts
index 16b7e96fd8..ba2ac78d87 100644
--- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts
+++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts
@@ -103,23 +103,24 @@ describe('MCPConnectionFactory', () => {
 
       await MCPConnectionFactory.create(basicOptions);
 
-      expect(mockConnectionInstance.once).toHaveBeenCalledWith(
-        'oauthRequired',
-        expect.any(Function),
-      );
+      expect(mockConnectionInstance.on).toHaveBeenCalledWith('oauthRequired', expect.any(Function));
 
-      const onceCall = (mockConnectionInstance.once as jest.Mock).mock.calls.find(
+      const onCall = (mockConnectionInstance.on as jest.Mock).mock.calls.find(
         ([event]: [string]) => event === 'oauthRequired',
       );
-      expect(onceCall).toBeDefined();
 
-      const handler = onceCall![1] as () => void;
+      const handler = onCall![1] as () => void;
       handler();
 
       expect(mockConnectionInstance.emit).toHaveBeenCalledWith(
         'oauthFailed',
         expect.objectContaining({ message: 'Server does not use OAuth' }),
       );
+
+      expect(mockConnectionInstance.removeListener).toHaveBeenCalledWith(
+        'oauthRequired',
+        expect.any(Function),
+      );
     });
 
     it('should create a connection with OAuth', async () => {
diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts
index ba5b0b3b8e..094d03215e 100644
--- a/packages/api/src/mcp/__tests__/MCPManager.test.ts
+++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts
@@ -925,6 +925,44 @@ describe('MCPManager', () => {
       );
     });
 
+    it('should use isOAuthServer in discoverServerTools', async () => {
+      const mockUser = { id: 'user123', email: 'test@example.com' } as unknown as IUser;
+      const mockFlowManager = {
+        createFlow: jest.fn(),
+        getFlowState: jest.fn(),
+        deleteFlow: jest.fn(),
+      };
+
+      mockAppConnections({
+        get: jest.fn().mockResolvedValue(null),
+      });
+
+      (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
+        type: 'streamable-http',
+        url: 'http://private-mcp.svc:5446/mcp',
+        requiresOAuth: false,
+      });
+
+      (MCPConnectionFactory.discoverTools as jest.Mock).mockResolvedValue({
+        tools: mockTools,
+        connection: null,
+        oauthRequired: false,
+        oauthUrl: null,
+      });
+
+      const manager = await MCPManager.createInstance(newMCPServersConfig());
+      await manager.discoverServerTools({
+        serverName,
+        user: mockUser,
+        flowManager: mockFlowManager as unknown as t.ToolDiscoveryOptions['flowManager'],
+      });
+
+      expect(MCPConnectionFactory.discoverTools).toHaveBeenCalledWith(
+        expect.objectContaining({ serverName }),
+        expect.not.objectContaining({ useOAuth: true }),
+      );
+    });
+
     it('should discover tools with OAuth when user and flowManager provided', async () => {
       const mockUser = { id: 'user123', email: 'test@example.com' } as unknown as IUser;
       const mockFlowManager = {
@@ -966,4 +1004,89 @@ describe('MCPManager', () => {
       );
     });
   });
+
+  describe('getUserConnection - useOAuth derivation', () => {
+    const mockUser = { id: userId, email: 'test@example.com' } as unknown as IUser;
+    const mockFlowManager = {
+      createFlow: jest.fn(),
+      getFlowState: jest.fn(),
+      deleteFlow: jest.fn(),
+    };
+    const mockConnection = {
+      isConnected: jest.fn().mockResolvedValue(true),
+      isStale: jest.fn().mockReturnValue(false),
+      disconnect: jest.fn(),
+    } as unknown as MCPConnection;
+
+    it('should pass useOAuth: true for servers with requiresOAuth', async () => {
+      mockAppConnections({
+        has: jest.fn().mockResolvedValue(false),
+      });
+
+      (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
+        type: 'sse',
+        url: 'https://oauth-mcp.example.com',
+        requiresOAuth: true,
+      });
+
+      (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection);
+
+      const manager = await MCPManager.createInstance(newMCPServersConfig());
+      await manager.getUserConnection({
+        serverName,
+        user: mockUser,
+        flowManager: mockFlowManager as unknown as t.UserMCPConnectionOptions['flowManager'],
+      });
+
+      expect(MCPConnectionFactory.create).toHaveBeenCalledWith(
+        expect.objectContaining({ serverName }),
+        expect.objectContaining({ useOAuth: true }),
+      );
+    });
+
+    it('should not pass useOAuth for servers with requiresOAuth: false', async () => {
+      mockAppConnections({
+        has: jest.fn().mockResolvedValue(false),
+      });
+
+      (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
+        type: 'streamable-http',
+        url: 'http://private-mcp.svc:5446/mcp',
+        requiresOAuth: false,
+      });
+
+      (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection);
+
+      const manager = await MCPManager.createInstance(newMCPServersConfig());
+      await manager.getUserConnection({
+        serverName,
+        user: mockUser,
+      });
+
+      expect(MCPConnectionFactory.create).toHaveBeenCalledWith(
+        expect.objectContaining({ serverName }),
+        expect.not.objectContaining({ useOAuth: true }),
+      );
+    });
+
+    it('should throw when OAuth server lacks flowManager', async () => {
+      mockAppConnections({
+        has: jest.fn().mockResolvedValue(false),
+      });
+
+      (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
+        type: 'sse',
+        url: 'https://oauth-mcp.example.com',
+        requiresOAuth: true,
+      });
+
+      const manager = await MCPManager.createInstance(newMCPServersConfig());
+      await expect(
+        manager.getUserConnection({
+          serverName,
+          user: mockUser,
+        }),
+      ).rejects.toThrow('requires a flowManager');
+    });
+  });
 });