From 85f3a8c21090c6ea4f0b23915716fe52ad50427a Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Mon, 27 Oct 2025 16:46:35 -0700 Subject: [PATCH] Migrate to coreEvents/debugLogger (#12107) --- packages/core/src/code_assist/oauth2.test.ts | 100 +++----- packages/core/src/code_assist/oauth2.ts | 26 +- packages/core/src/tools/mcp-client-manager.ts | 5 +- packages/core/src/tools/mcp-client.test.ts | 15 +- packages/core/src/tools/mcp-client.ts | 237 ++++++++---------- packages/core/src/tools/tool-registry.ts | 8 +- 6 files changed, 173 insertions(+), 218 deletions(-) diff --git a/packages/core/src/code_assist/oauth2.test.ts b/packages/core/src/code_assist/oauth2.test.ts index 2210c695f9..b15a7aa89b 100644 --- a/packages/core/src/code_assist/oauth2.test.ts +++ b/packages/core/src/code_assist/oauth2.test.ts @@ -70,7 +70,7 @@ describe('oauth2', () => { tempHomeDir = fs.mkdtempSync( path.join(os.tmpdir(), 'gemini-cli-test-home-'), ); - (os.homedir as Mock).mockReturnValue(tempHomeDir); + vi.mocked(os.homedir).mockReturnValue(tempHomeDir); }); afterEach(() => { fs.rmSync(tempHomeDir, { recursive: true, force: true }); @@ -102,15 +102,15 @@ describe('oauth2', () => { credentials: mockTokens, on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); vi.spyOn(crypto, 'randomBytes').mockReturnValue(mockState as never); - (open as Mock).mockImplementation(async () => ({ on: vi.fn() }) as never); + vi.mocked(open).mockImplementation( + async () => ({ on: vi.fn() }) as never, + ); // Mock the UserInfo API response - (global.fetch as Mock).mockResolvedValue({ + vi.mocked(global.fetch).mockResolvedValue({ ok: true, json: vi .fn() @@ -232,9 +232,7 @@ describe('oauth2', () => { generateCodeVerifierAsync: mockGenerateCodeVerifierAsync, on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); const mockReadline = { question: vi.fn((_query, callback) => callback(mockCode)), @@ -307,7 +305,7 @@ describe('oauth2', () => { }; // To mock the new OAuth2Client() inside the function - (OAuth2Client as unknown as Mock).mockImplementation( + vi.mocked(OAuth2Client).mockImplementation( () => mockClient as unknown as OAuth2Client, ); @@ -387,7 +385,7 @@ describe('oauth2', () => { getTokenInfo: vi.fn().mockResolvedValue({}), on: vi.fn(), }; - (OAuth2Client as unknown as Mock).mockImplementation( + vi.mocked(OAuth2Client).mockImplementation( () => mockClient as unknown as OAuth2Client, ); @@ -411,7 +409,7 @@ describe('oauth2', () => { getTokenInfo: vi.fn().mockResolvedValue({}), on: vi.fn(), }; - (OAuth2Client as unknown as Mock).mockImplementation( + vi.mocked(OAuth2Client).mockImplementation( () => mockClient as unknown as OAuth2Client, ); @@ -483,9 +481,7 @@ describe('oauth2', () => { getAccessToken: mockGetAccessToken, on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); // Mock the UserInfo API response for fetchAndCacheUserInfo (global.fetch as Mock).mockResolvedValue({ @@ -543,9 +539,7 @@ describe('oauth2', () => { getTokenInfo: mockGetTokenInfo, on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); // Make it fall through to cached credentials path const cachedCreds = { refresh_token: 'cached-token' }; @@ -578,9 +572,7 @@ describe('oauth2', () => { getTokenInfo: mockGetTokenInfo, on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); // Make it fall through to cached credentials path const cachedCreds = { refresh_token: 'cached-token' }; @@ -609,9 +601,7 @@ describe('oauth2', () => { generateAuthUrl: vi.fn().mockReturnValue('https://example.com/auth'), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); await expect( getOauthClient(AuthType.LOGIN_WITH_GOOGLE, mockConfig), @@ -624,11 +614,9 @@ describe('oauth2', () => { generateAuthUrl: vi.fn().mockReturnValue(mockAuthUrl), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); - (open as Mock).mockImplementation( + vi.mocked(open).mockImplementation( async () => ({ on: vi.fn() }) as never, ); @@ -663,11 +651,9 @@ describe('oauth2', () => { generateAuthUrl: vi.fn().mockReturnValue(mockAuthUrl), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); - (open as Mock).mockImplementation( + vi.mocked(open).mockImplementation( async () => ({ on: vi.fn() }) as never, ); @@ -722,11 +708,9 @@ describe('oauth2', () => { generateAuthUrl: vi.fn().mockReturnValue(mockAuthUrl), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); - (open as Mock).mockImplementation( + vi.mocked(open).mockImplementation( async () => ({ on: vi.fn() }) as never, ); @@ -787,12 +771,10 @@ describe('oauth2', () => { .mockRejectedValue(new Error('Token exchange failed')), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); vi.spyOn(crypto, 'randomBytes').mockReturnValue(mockState as never); - (open as Mock).mockImplementation( + vi.mocked(open).mockImplementation( async () => ({ on: vi.fn() }) as never, ); @@ -858,24 +840,22 @@ describe('oauth2', () => { .mockResolvedValue({ token: 'test-access-token' }), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); vi.spyOn(crypto, 'randomBytes').mockReturnValue(mockState as never); - (open as Mock).mockImplementation( + vi.mocked(open).mockImplementation( async () => ({ on: vi.fn() }) as never, ); // Mock fetch to fail - (global.fetch as Mock).mockResolvedValue({ + vi.mocked(global.fetch).mockResolvedValue({ ok: false, status: 500, statusText: 'Internal Server Error', } as unknown as Response); - const consoleErrorSpy = vi - .spyOn(console, 'error') + const consoleLogSpy = vi + .spyOn(console, 'log') .mockImplementation(() => {}); let requestCallback!: http.RequestListener; @@ -894,10 +874,10 @@ describe('oauth2', () => { close: vi.fn(), on: vi.fn(), address: () => ({ port: 3000 }), - }; + } as unknown as http.Server; (http.createServer as Mock).mockImplementation((cb) => { requestCallback = cb; - return mockHttpServer as unknown as http.Server; + return mockHttpServer; }); const clientPromise = getOauthClient( @@ -919,13 +899,13 @@ describe('oauth2', () => { // Authentication should succeed even if fetchAndCacheUserInfo fails expect(client).toBe(mockOAuth2Client); - expect(consoleErrorSpy).toHaveBeenCalledWith( + expect(consoleLogSpy).toHaveBeenCalledWith( 'Failed to fetch user info:', 500, 'Internal Server Error', ); - consoleErrorSpy.mockRestore(); + consoleLogSpy.mockRestore(); }); it('should handle user code authentication failure with descriptive error', async () => { @@ -946,9 +926,7 @@ describe('oauth2', () => { .mockRejectedValue(new Error('Invalid authorization code')), on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); const mockReadline = { question: vi.fn((_query, callback) => callback('invalid-code')), @@ -1028,9 +1006,7 @@ describe('oauth2', () => { getTokenInfo: mockGetTokenInfo, on: vi.fn(), } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); // Pre-populate credentials to make getOauthClient resolve quickly const credsPath = path.join( @@ -1112,12 +1088,12 @@ describe('oauth2', () => { on: mockOn, credentials: mockTokens, } as unknown as OAuth2Client; - (OAuth2Client as unknown as Mock).mockImplementation( - () => mockOAuth2Client, - ); + vi.mocked(OAuth2Client).mockImplementation(() => mockOAuth2Client); vi.spyOn(crypto, 'randomBytes').mockReturnValue(mockState as never); - (open as Mock).mockImplementation(async () => ({ on: vi.fn() }) as never); + vi.mocked(open).mockImplementation( + async () => ({ on: vi.fn() }) as never, + ); (global.fetch as Mock).mockResolvedValue({ ok: true, @@ -1203,7 +1179,7 @@ describe('oauth2', () => { on: vi.fn(), }; - (OAuth2Client as unknown as Mock).mockImplementation( + vi.mocked(OAuth2Client).mockImplementation( () => mockClient as unknown as OAuth2Client, ); diff --git a/packages/core/src/code_assist/oauth2.ts b/packages/core/src/code_assist/oauth2.ts index ef0be547f0..46ff9fcb00 100644 --- a/packages/core/src/code_assist/oauth2.ts +++ b/packages/core/src/code_assist/oauth2.ts @@ -185,7 +185,7 @@ async function initOauthClient( for (let i = 0; !success && i < maxRetries; i++) { success = await authWithUserCode(client); if (!success) { - console.error( + debugLogger.error( '\nFailed to authenticate with user code.', i === maxRetries - 1 ? '' : 'Retrying...\n', ); @@ -215,17 +215,17 @@ async function initOauthClient( // in a minimal Docker container), it will emit an unhandled 'error' event, // causing the entire Node.js process to crash. childProcess.on('error', (error) => { - console.error( - 'Failed to open browser automatically. Please try running again with NO_BROWSER=true set.', + debugLogger.error( + `Failed to open browser with error:`, + getErrorMessage(error), + `\nPlease try running again with NO_BROWSER=true set.`, ); - console.error('Browser error details:', getErrorMessage(error)); }); } catch (err) { - console.error( - 'An unexpected error occurred while trying to open the browser:', + debugLogger.error( + `Failed to open browser with error:`, getErrorMessage(err), - '\nThis might be due to browser compatibility issues or system configuration.', - '\nPlease try running again with NO_BROWSER=true set for manual authentication.', + `\nPlease try running again with NO_BROWSER=true set.`, ); throw new FatalAuthenticationError( `Failed to open browser: ${getErrorMessage(err)}`, @@ -293,7 +293,7 @@ async function authWithUserCode(client: OAuth2Client): Promise { }); if (!code) { - console.error('Authorization code is required.'); + debugLogger.error('Authorization code is required.'); return false; } @@ -305,7 +305,7 @@ async function authWithUserCode(client: OAuth2Client): Promise { }); client.setCredentials(tokens); } catch (error) { - console.error( + debugLogger.error( 'Failed to authenticate with authorization code:', getErrorMessage(error), ); @@ -528,7 +528,7 @@ export async function clearCachedCredentialFile() { // Clear the in-memory OAuth client cache to force re-authentication clearOauthClientCache(); } catch (e) { - console.error('Failed to clear cached credentials:', e); + debugLogger.warn('Failed to clear cached credentials:', e); } } @@ -549,7 +549,7 @@ async function fetchAndCacheUserInfo(client: OAuth2Client): Promise { ); if (!response.ok) { - console.error( + debugLogger.log( 'Failed to fetch user info:', response.status, response.statusText, @@ -560,7 +560,7 @@ async function fetchAndCacheUserInfo(client: OAuth2Client): Promise { const userInfo = await response.json(); await userAccountManager.cacheGoogleAccount(userInfo.email); } catch (error) { - console.error('Error retrieving user info:', error); + debugLogger.log('Error retrieving user info:', error); } } diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index ec05563d3d..d482da3722 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -13,6 +13,7 @@ import { } from './mcp-client.js'; import { getErrorMessage } from '../utils/errors.js'; import type { EventEmitter } from 'node:events'; +import { coreEvents } from '../utils/events.js'; /** * Manages the lifecycle of multiple MCP clients, including local child processes. @@ -70,10 +71,12 @@ export class McpClientManager { } catch (error) { this.eventEmitter?.emit('mcp-client-update', this.clients); // Log the error but don't let a single failed server stop the others - console.error( + coreEvents.emitFeedback( + 'error', `Error during discovery for server '${name}': ${getErrorMessage( error, )}`, + error, ); } }); diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index fe755db7bc..23760e9914 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -29,6 +29,7 @@ import type { ToolRegistry } from './tool-registry.js'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { coreEvents } from '../utils/events.js'; vi.mock('@modelcontextprotocol/sdk/client/stdio.js'); vi.mock('@modelcontextprotocol/sdk/client/index.js'); @@ -37,6 +38,12 @@ vi.mock('../mcp/oauth-provider.js'); vi.mock('../mcp/oauth-token-storage.js'); vi.mock('../mcp/oauth-utils.js'); +vi.mock('../utils/events.js', () => ({ + coreEvents: { + emitFeedback: vi.fn(), + }, +})); + describe('mcp-client', () => { let workspaceContext: WorkspaceContext; let testWorkspace: string; @@ -164,9 +171,6 @@ describe('mcp-client', () => { }); it('should handle errors when discovering prompts', async () => { - const consoleErrorSpy = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); const mockedClient = { connect: vi.fn(), discover: vi.fn(), @@ -200,10 +204,11 @@ describe('mcp-client', () => { await expect(client.discover({} as Config)).rejects.toThrow( 'No prompts or tools found on the server.', ); - expect(consoleErrorSpy).toHaveBeenCalledWith( + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'error', `Error discovering prompts from test-server: Test error`, + expect.any(Error), ); - consoleErrorSpy.mockRestore(); }); it('should not discover tools if server does not support them', async () => { diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index b0e46900a7..a2afc04736 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -42,6 +42,7 @@ import type { } from '../utils/workspaceContext.js'; import type { ToolRegistry } from './tool-registry.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { coreEvents } from '../utils/events.js'; export const MCP_DEFAULT_TIMEOUT_MSEC = 10 * 60 * 1000; // default to 10 minutes @@ -119,7 +120,11 @@ export class McpClient { return; } if (originalOnError) originalOnError(error); - console.error(`MCP ERROR (${this.serverName}):`, error.toString()); + coreEvents.emitFeedback( + 'error', + `MCP ERROR (${this.serverName})`, + error, + ); this.updateStatus(MCPServerStatus.DISCONNECTED); }; this.updateStatus(MCPServerStatus.CONNECTED); @@ -340,8 +345,9 @@ async function handleAutomaticOAuth( } if (!oauthConfig) { - console.error( - `❌ Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`, + coreEvents.emitFeedback( + 'error', + `Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`, ); return false; } @@ -370,8 +376,10 @@ async function handleAutomaticOAuth( ); return true; } catch (error) { - console.error( + coreEvents.emitFeedback( + 'error', `Failed to handle automatic OAuth for server '${mcpServerName}': ${getErrorMessage(error)}`, + error, ); return false; } @@ -420,8 +428,10 @@ async function createTransportWithOAuth( return null; } catch (error) { - console.error( + coreEvents.emitFeedback( + 'error', `Failed to create OAuth transport for server '${mcpServerName}': ${getErrorMessage(error)}`, + error, ); return null; } @@ -520,7 +530,7 @@ export async function connectAndDiscover( ); mcpClient.onerror = (error) => { - console.error(`MCP ERROR (${mcpServerName}):`, error.toString()); + coreEvents.emitFeedback('error', `MCP ERROR (${mcpServerName}):`, error); updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); }; @@ -553,10 +563,12 @@ export async function connectAndDiscover( if (mcpClient) { mcpClient.close(); } - console.error( + coreEvents.emitFeedback( + 'error', `Error connecting to MCP server '${mcpServerName}': ${getErrorMessage( error, )}`, + error, ); updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); } @@ -614,10 +626,12 @@ export async function discoverTools( ), ); } catch (error) { - console.error( + coreEvents.emitFeedback( + 'error', `Error discovering tool: '${ funcDecl.name }' from MCP server '${mcpServerName}': ${(error as Error).message}`, + error, ); } } @@ -627,10 +641,12 @@ export async function discoverTools( error instanceof Error && !error.message?.includes('Method not found') ) { - console.error( + coreEvents.emitFeedback( + 'error', `Error discovering tools from ${mcpServerName}: ${getErrorMessage( error, )}`, + error, ); } return []; @@ -674,10 +690,12 @@ export async function discoverPrompts( error instanceof Error && !error.message?.includes('Method not found') ) { - console.error( + coreEvents.emitFeedback( + 'error', `Error discovering prompts from ${mcpServerName}: ${getErrorMessage( error, )}`, + error, ); } return []; @@ -717,10 +735,12 @@ export async function invokeMcpPrompt( error instanceof Error && !error.message?.includes('Method not found') ) { - console.error( + coreEvents.emitFeedback( + 'error', `Error invoking prompt '${promptName}' from ${mcpServerName} ${promptParams}: ${getErrorMessage( error, )}`, + error, ); } throw error; @@ -842,12 +862,14 @@ export async function connectToMcpServer( }, ); if (hasStoredTokens) { - debugLogger.log( + coreEvents.emitFeedback( + 'error', `Stored OAuth token for SSE server '${mcpServerName}' was rejected. ` + `Please re-authenticate using: /mcp auth ${mcpServerName}`, ); } else { - debugLogger.log( + coreEvents.emitFeedback( + 'error', `401 error received for SSE server '${mcpServerName}' without OAuth configuration. ` + `Please authenticate using: /mcp auth ${mcpServerName}`, ); @@ -935,49 +957,27 @@ export async function connectToMcpServer( accessToken, ); if (oauthTransport) { - try { - await mcpClient.connect(oauthTransport, { - timeout: - mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, - }); - // Connection successful with OAuth - return mcpClient; - } catch (retryError) { - console.error( - `Failed to connect with OAuth token: ${getErrorMessage( - retryError, - )}`, - ); - throw retryError; - } + await mcpClient.connect(oauthTransport, { + timeout: mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, + }); + // Connection successful with OAuth + return mcpClient; } else { - console.error( - `Failed to create OAuth transport for server '${mcpServerName}'`, - ); throw new Error( `Failed to create OAuth transport for server '${mcpServerName}'`, ); } } else { - console.error( - `Failed to get OAuth token for server '${mcpServerName}'`, - ); throw new Error( `Failed to get OAuth token for server '${mcpServerName}'`, ); } } else { - console.error( - `Failed to get credentials for server '${mcpServerName}' after successful OAuth authentication`, - ); throw new Error( `Failed to get credentials for server '${mcpServerName}' after successful OAuth authentication`, ); } } else { - console.error( - `Failed to handle automatic OAuth for server '${mcpServerName}'`, - ); throw new Error( `Failed to handle automatic OAuth for server '${mcpServerName}'`, ); @@ -1002,12 +1002,14 @@ export async function connectToMcpServer( }, ); if (hasStoredTokens) { - debugLogger.log( + coreEvents.emitFeedback( + 'error', `Stored OAuth token for SSE server '${mcpServerName}' was rejected. ` + `Please re-authenticate using: /mcp auth ${mcpServerName}`, ); } else { - debugLogger.log( + coreEvents.emitFeedback( + 'error', `401 error received for SSE server '${mcpServerName}' without OAuth configuration. ` + `Please authenticate using: /mcp auth ${mcpServerName}`, ); @@ -1030,116 +1032,85 @@ export async function connectToMcpServer( ); const baseUrl = `${serverUrl.protocol}//${serverUrl.host}`; - try { - // Try to discover OAuth configuration from the base URL - const oauthConfig = await OAuthUtils.discoverOAuthConfig(baseUrl); - if (oauthConfig) { - debugLogger.log( - `Discovered OAuth configuration from base URL for server '${mcpServerName}'`, - ); + // Try to discover OAuth configuration from the base URL + const oauthConfig = await OAuthUtils.discoverOAuthConfig(baseUrl); + if (oauthConfig) { + debugLogger.log( + `Discovered OAuth configuration from base URL for server '${mcpServerName}'`, + ); - // Create OAuth configuration for authentication - const oauthAuthConfig = { - enabled: true, - authorizationUrl: oauthConfig.authorizationUrl, - tokenUrl: oauthConfig.tokenUrl, - scopes: oauthConfig.scopes || [], - }; + // Create OAuth configuration for authentication + const oauthAuthConfig = { + enabled: true, + authorizationUrl: oauthConfig.authorizationUrl, + tokenUrl: oauthConfig.tokenUrl, + scopes: oauthConfig.scopes || [], + }; - // Perform OAuth authentication - // Pass the server URL for proper discovery - const authServerUrl = - mcpServerConfig.httpUrl || mcpServerConfig.url; - debugLogger.log( - `Starting OAuth authentication for server '${mcpServerName}'...`, - ); - const authProvider = new MCPOAuthProvider( - new MCPOAuthTokenStorage(), - ); - await authProvider.authenticate( + // Perform OAuth authentication + // Pass the server URL for proper discovery + const authServerUrl = + mcpServerConfig.httpUrl || mcpServerConfig.url; + debugLogger.log( + `Starting OAuth authentication for server '${mcpServerName}'...`, + ); + const authProvider = new MCPOAuthProvider( + new MCPOAuthTokenStorage(), + ); + await authProvider.authenticate( + mcpServerName, + oauthAuthConfig, + authServerUrl, + ); + + // Retry connection with OAuth token + const tokenStorage = new MCPOAuthTokenStorage(); + const credentials = + await tokenStorage.getCredentials(mcpServerName); + if (credentials) { + const authProvider = new MCPOAuthProvider(tokenStorage); + const accessToken = await authProvider.getValidToken( mcpServerName, - oauthAuthConfig, - authServerUrl, + { + // Pass client ID if available + clientId: credentials.clientId, + }, ); - - // Retry connection with OAuth token - const tokenStorage = new MCPOAuthTokenStorage(); - const credentials = - await tokenStorage.getCredentials(mcpServerName); - if (credentials) { - const authProvider = new MCPOAuthProvider(tokenStorage); - const accessToken = await authProvider.getValidToken( + if (accessToken) { + // Create transport with OAuth token + const oauthTransport = await createTransportWithOAuth( mcpServerName, - { - // Pass client ID if available - clientId: credentials.clientId, - }, + mcpServerConfig, + accessToken, ); - if (accessToken) { - // Create transport with OAuth token - const oauthTransport = await createTransportWithOAuth( - mcpServerName, - mcpServerConfig, - accessToken, - ); - if (oauthTransport) { - try { - await mcpClient.connect(oauthTransport, { - timeout: - mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, - }); - // Connection successful with OAuth - return mcpClient; - } catch (retryError) { - console.error( - `Failed to connect with OAuth token: ${getErrorMessage( - retryError, - )}`, - ); - throw retryError; - } - } else { - console.error( - `Failed to create OAuth transport for server '${mcpServerName}'`, - ); - throw new Error( - `Failed to create OAuth transport for server '${mcpServerName}'`, - ); - } + if (oauthTransport) { + await mcpClient.connect(oauthTransport, { + timeout: + mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, + }); + // Connection successful with OAuth + return mcpClient; } else { - console.error( - `Failed to get OAuth token for server '${mcpServerName}'`, - ); throw new Error( - `Failed to get OAuth token for server '${mcpServerName}'`, + `Failed to create OAuth transport for server '${mcpServerName}'`, ); } } else { - console.error( - `Failed to get stored credentials for server '${mcpServerName}'`, - ); throw new Error( - `Failed to get stored credentials for server '${mcpServerName}'`, + `Failed to get OAuth token for server '${mcpServerName}'`, ); } } else { - console.error( - `❌ Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`, - ); throw new Error( - `OAuth configuration failed for '${mcpServerName}'. Please authenticate manually with /mcp auth ${mcpServerName}`, + `Failed to get stored credentials for server '${mcpServerName}'`, ); } - } catch (discoveryError) { - console.error( - `❌ OAuth discovery failed for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`, + } else { + throw new Error( + `OAuth configuration failed for '${mcpServerName}'. Please authenticate manually with /mcp auth ${mcpServerName}`, ); - throw discoveryError; } } else { - console.error( - `❌ '${mcpServerName}' requires authentication but no OAuth configuration found`, - ); throw new Error( `MCP server '${mcpServerName}' requires authentication. Please configure OAuth or check server settings.`, ); @@ -1239,10 +1210,6 @@ export async function createTransport( ); if (!accessToken) { - console.error( - `MCP server '${mcpServerName}' requires OAuth authentication. ` + - `Please authenticate using the /mcp auth command.`, - ); throw new Error( `MCP server '${mcpServerName}' requires OAuth authentication. ` + `Please authenticate using the /mcp auth command.`, diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index f24365913e..4a4a13c5f9 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -23,6 +23,7 @@ import { safeJsonStringify } from '../utils/safeJsonStringify.js'; import type { EventEmitter } from 'node:events'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { coreEvents } from '../utils/events.js'; type ToolParams = Record; @@ -350,8 +351,11 @@ export class ToolRegistry { } if (code !== 0) { - console.error(`Command failed with code ${code}`); - console.error(stderr); + coreEvents.emitFeedback( + 'error', + `Tool discovery command failed with code ${code}.`, + stderr, + ); return reject( new Error(`Tool discovery command failed with exit code ${code}`), );