From b3ac1299e837f0e0a764393180f132ed85274e00 Mon Sep 17 00:00:00 2001 From: gemini-cli-robot Date: Tue, 17 Feb 2026 14:09:49 -0500 Subject: [PATCH] fix(patch): cherry-pick 9590a09 to release/v0.29.0-preview.4-pr-18771 to patch version v0.29.0-preview.4 and create version 0.29.0-preview.5 (#19274) Co-authored-by: Shreya Keshive --- packages/core/src/tools/mcp-client.test.ts | 16 ++--- packages/core/src/tools/mcp-client.ts | 68 ++++++---------------- 2 files changed, 25 insertions(+), 59 deletions(-) diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 05c68eaa55..39165bde45 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -901,9 +901,9 @@ describe('mcp-client', () => { vi.mocked(ClientLib.Client).mockReturnValue( mockedClient as unknown as ClientLib.Client, ); - vi.spyOn(SdkClientStdioLib, 'StdioClientTransport').mockReturnValue({ - close: vi.fn(), - } as unknown as SdkClientStdioLib.StdioClientTransport); + vi.spyOn(SdkClientStdioLib, 'StdioClientTransport').mockReturnValue( + {} as SdkClientStdioLib.StdioClientTransport, + ); const mockedToolRegistry = { registerTool: vi.fn(), unregisterTool: vi.fn(), @@ -1971,7 +1971,7 @@ describe('connectToMcpServer with OAuth', () => { EMPTY_CONFIG, ); - expect(client.client).toBe(mockedClient); + expect(client).toBe(mockedClient); expect(mockedClient.connect).toHaveBeenCalledTimes(2); expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce(); @@ -2017,7 +2017,7 @@ describe('connectToMcpServer with OAuth', () => { EMPTY_CONFIG, ); - expect(client.client).toBe(mockedClient); + expect(client).toBe(mockedClient); expect(mockedClient.connect).toHaveBeenCalledTimes(2); expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce(); expect(OAuthUtils.discoverOAuthConfig).toHaveBeenCalledWith(serverUrl); @@ -2112,7 +2112,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { EMPTY_CONFIG, ); - expect(client.client).toBe(mockedClient); + expect(client).toBe(mockedClient); // First HTTP attempt fails, second SSE attempt succeeds expect(mockedClient.connect).toHaveBeenCalledTimes(2); }); @@ -2153,7 +2153,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { EMPTY_CONFIG, ); - expect(client.client).toBe(mockedClient); + expect(client).toBe(mockedClient); expect(mockedClient.connect).toHaveBeenCalledTimes(2); }); }); @@ -2238,7 +2238,7 @@ describe('connectToMcpServer - OAuth with transport fallback', () => { EMPTY_CONFIG, ); - expect(client.client).toBe(mockedClient); + expect(client).toBe(mockedClient); expect(mockedClient.connect).toHaveBeenCalledTimes(3); expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce(); }); diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 3d969dd705..2588d54dba 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -142,7 +142,7 @@ export class McpClient { } this.updateStatus(MCPServerStatus.CONNECTING); try { - const { client, transport } = await connectToMcpServer( + this.client = await connectToMcpServer( this.clientVersion, this.serverName, this.serverConfig, @@ -150,13 +150,11 @@ export class McpClient { this.workspaceContext, this.cliConfig.sanitizationConfig, ); - this.client = client; - this.transport = transport; this.registerNotificationHandlers(); const originalOnError = this.client.onerror; - this.client.onerror = async (error) => { + this.client.onerror = (error) => { if (this.status !== MCPServerStatus.CONNECTED) { return; } @@ -167,14 +165,6 @@ export class McpClient { error, ); this.updateStatus(MCPServerStatus.DISCONNECTED); - // Close transport to prevent memory leaks - if (this.transport) { - try { - await this.transport.close(); - } catch { - // Ignore errors when closing transport on error - } - } }; this.updateStatus(MCPServerStatus.CONNECTED); } catch (error) { @@ -923,9 +913,8 @@ export async function connectAndDiscover( updateMCPServerStatus(mcpServerName, MCPServerStatus.CONNECTING); let mcpClient: Client | undefined; - let transport: Transport | undefined; try { - const result = await connectToMcpServer( + mcpClient = await connectToMcpServer( clientVersion, mcpServerName, mcpServerConfig, @@ -933,20 +922,10 @@ export async function connectAndDiscover( workspaceContext, cliConfig.sanitizationConfig, ); - mcpClient = result.client; - transport = result.transport; - mcpClient.onerror = async (error) => { + mcpClient.onerror = (error) => { coreEvents.emitFeedback('error', `MCP ERROR (${mcpServerName}):`, error); updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); - // Close transport to prevent memory leaks - if (transport) { - try { - await transport.close(); - } catch { - // Ignore errors when closing transport on error - } - } }; // Attempt to discover both prompts and tools @@ -1344,18 +1323,16 @@ function createSSETransportWithAuth( * @param client The MCP client to connect * @param config The MCP server configuration * @param accessToken Optional OAuth access token for authentication - * @returns The transport used for connection */ async function connectWithSSETransport( client: Client, config: MCPServerConfig, accessToken?: string | null, -): Promise { +): Promise { const transport = createSSETransportWithAuth(config, accessToken); await client.connect(transport, { timeout: config.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, }); - return transport; } /** @@ -1385,7 +1362,6 @@ async function showAuthRequiredMessage(serverName: string): Promise { * @param config The MCP server configuration * @param accessToken The OAuth access token to use * @param httpReturned404 Whether the HTTP transport returned 404 (indicating SSE-only server) - * @returns The transport used for connection */ async function retryWithOAuth( client: Client, @@ -1393,21 +1369,17 @@ async function retryWithOAuth( config: MCPServerConfig, accessToken: string, httpReturned404: boolean, -): Promise { +): Promise { if (httpReturned404) { // HTTP returned 404, only try SSE debugLogger.log( `Retrying SSE connection to '${serverName}' with OAuth token...`, ); - const transport = await connectWithSSETransport( - client, - config, - accessToken, - ); + await connectWithSSETransport(client, config, accessToken); debugLogger.log( `Successfully connected to '${serverName}' using SSE with OAuth.`, ); - return transport; + return; } // HTTP returned 401, try HTTP with OAuth first @@ -1431,7 +1403,6 @@ async function retryWithOAuth( debugLogger.log( `Successfully connected to '${serverName}' using HTTP with OAuth.`, ); - return httpTransport; } catch (httpError) { await httpTransport.close(); @@ -1443,15 +1414,10 @@ async function retryWithOAuth( !config.httpUrl ) { debugLogger.log(`HTTP with OAuth returned 404, trying SSE with OAuth...`); - const sseTransport = await connectWithSSETransport( - client, - config, - accessToken, - ); + await connectWithSSETransport(client, config, accessToken); debugLogger.log( `Successfully connected to '${serverName}' using SSE with OAuth.`, ); - return sseTransport; } else { throw httpError; } @@ -1465,7 +1431,7 @@ async function retryWithOAuth( * * @param mcpServerName The name of the MCP server, used for logging and identification. * @param mcpServerConfig The configuration specifying how to connect to the server. - * @returns A promise that resolves to a connected MCP `Client` instance and its transport. + * @returns A promise that resolves to a connected MCP `Client` instance. * @throws An error if the connection fails or the configuration is invalid. */ export async function connectToMcpServer( @@ -1475,7 +1441,7 @@ export async function connectToMcpServer( debugMode: boolean, workspaceContext: WorkspaceContext, sanitizationConfig: EnvironmentSanitizationConfig, -): Promise<{ client: Client; transport: Transport }> { +): Promise { const mcpClient = new Client( { name: 'gemini-cli-mcp-client', @@ -1547,7 +1513,7 @@ export async function connectToMcpServer( await mcpClient.connect(transport, { timeout: mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, }); - return { client: mcpClient, transport }; + return mcpClient; } catch (error) { await transport.close(); // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion @@ -1579,7 +1545,7 @@ export async function connectToMcpServer( try { // Try SSE with stored OAuth token if available // This ensures that SSE fallback works for authenticated servers - const sseTransport = await connectWithSSETransport( + await connectWithSSETransport( mcpClient, mcpServerConfig, await getStoredOAuthToken(mcpServerName), @@ -1588,7 +1554,7 @@ export async function connectToMcpServer( debugLogger.log( `MCP server '${mcpServerName}': Successfully connected using SSE transport.`, ); - return { client: mcpClient, transport: sseTransport }; + return mcpClient; } catch (sseFallbackError) { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion sseError = sseFallbackError as Error; @@ -1696,14 +1662,14 @@ export async function connectToMcpServer( ); } - const oauthTransport = await retryWithOAuth( + await retryWithOAuth( mcpClient, mcpServerName, mcpServerConfig, accessToken, httpReturned404, ); - return { client: mcpClient, transport: oauthTransport }; + return mcpClient; } else { throw new Error( `Failed to handle automatic OAuth for server '${mcpServerName}'`, @@ -1784,7 +1750,7 @@ export async function connectToMcpServer( timeout: mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, }); // Connection successful with OAuth - return { client: mcpClient, transport: oauthTransport }; + return mcpClient; } else { throw new Error( `OAuth configuration failed for '${mcpServerName}'. Please authenticate manually with /mcp auth ${mcpServerName}`,