From ab013fb7e9504e840bb1588c11f555293d8a21a0 Mon Sep 17 00:00:00 2001 From: Sehoon Shon Date: Fri, 31 Oct 2025 14:17:51 -0400 Subject: [PATCH] migrating console.error to debugger for installationManager, oauth-provider, modifiable-tool (#12279) --- packages/core/src/mcp/oauth-provider.test.ts | 12 ++++++++++-- packages/core/src/mcp/oauth-provider.ts | 11 ++++++++--- packages/core/src/tools/modifiable-tool.ts | 5 +++-- packages/core/src/utils/installationManager.test.ts | 6 +++--- packages/core/src/utils/installationManager.ts | 3 ++- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/core/src/mcp/oauth-provider.test.ts b/packages/core/src/mcp/oauth-provider.test.ts index 8a156b28f0..6ff5ea0683 100644 --- a/packages/core/src/mcp/oauth-provider.test.ts +++ b/packages/core/src/mcp/oauth-provider.test.ts @@ -27,6 +27,11 @@ vi.mock('./oauth-token-storage.js', () => { })), }; }); +vi.mock('../utils/events.js', () => ({ + coreEvents: { + emitFeedback: vi.fn(), + }, +})); import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as http from 'node:http'; @@ -43,6 +48,7 @@ import type { OAuthAuthorizationServerMetadata, OAuthProtectedResourceMetadata, } from './oauth-utils.js'; +import { coreEvents } from '../utils/events.js'; // Mock fetch globally const mockFetch = vi.fn(); @@ -938,8 +944,10 @@ describe('MCPOAuthProvider', () => { expect(tokenStorage.deleteCredentials).toHaveBeenCalledWith( 'test-server', ); - expect(console.error).toHaveBeenCalledWith( - expect.stringContaining('Failed to refresh token'), + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'error', + expect.stringContaining('Failed to refresh auth token'), + expect.any(Error), ); }); diff --git a/packages/core/src/mcp/oauth-provider.ts b/packages/core/src/mcp/oauth-provider.ts index 3b67882c09..c366ea9f52 100644 --- a/packages/core/src/mcp/oauth-provider.ts +++ b/packages/core/src/mcp/oauth-provider.ts @@ -13,6 +13,7 @@ import type { OAuthToken } from './token-storage/types.js'; import { MCPOAuthTokenStorage } from './oauth-token-storage.js'; import { getErrorMessage } from '../utils/errors.js'; import { OAuthUtils } from './oauth-utils.js'; +import { coreEvents } from '../utils/events.js'; import { debugLogger } from '../utils/debugLogger.js'; export const OAUTH_DISPLAY_MESSAGE_EVENT = 'oauth-display-message' as const; @@ -811,12 +812,12 @@ ${authUrl} `✓ Token verification successful (fingerprint: ${tokenFingerprint})`, ); } else { - console.error( + debugLogger.warn( 'Token verification failed: token not found or invalid after save', ); } } catch (saveError) { - console.error(`Failed to save token: ${getErrorMessage(saveError)}`); + debugLogger.error('Failed to save auth token.', saveError); throw saveError; } @@ -889,7 +890,11 @@ ${authUrl} return newToken.accessToken; } catch (error) { - console.error(`Failed to refresh token: ${getErrorMessage(error)}`); + coreEvents.emitFeedback( + 'error', + 'Failed to refresh auth token.', + error, + ); // Remove invalid token await this.tokenStorage.deleteCredentials(serverName); } diff --git a/packages/core/src/tools/modifiable-tool.ts b/packages/core/src/tools/modifiable-tool.ts index 3dcf640890..0a2409e1c4 100644 --- a/packages/core/src/tools/modifiable-tool.ts +++ b/packages/core/src/tools/modifiable-tool.ts @@ -17,6 +17,7 @@ import type { DeclarativeTool, ToolResult, } from './tools.js'; +import { debugLogger } from '../utils/debugLogger.js'; /** * A declarative tool that supports a modify operation. @@ -128,13 +129,13 @@ function deleteTempFiles(oldPath: string, newPath: string): void { try { fs.unlinkSync(oldPath); } catch { - console.error(`Error deleting temp diff file: ${oldPath}`); + debugLogger.error(`Error deleting temp diff file: ${oldPath}`); } try { fs.unlinkSync(newPath); } catch { - console.error(`Error deleting temp diff file: ${newPath}`); + debugLogger.error(`Error deleting temp diff file: ${newPath}`); } } diff --git a/packages/core/src/utils/installationManager.test.ts b/packages/core/src/utils/installationManager.test.ts index f3a9c4c4e3..132c73e985 100644 --- a/packages/core/src/utils/installationManager.test.ts +++ b/packages/core/src/utils/installationManager.test.ts @@ -91,14 +91,14 @@ describe('InstallationManager', () => { readSpy.mockImplementationOnce(() => { throw new Error('Read error'); }); - const consoleErrorSpy = vi - .spyOn(console, 'error') + const consoleWarnSpy = vi + .spyOn(console, 'warn') .mockImplementation(() => {}); const id = installationManager.getInstallationId(); expect(id).toBe('123456789'); - expect(consoleErrorSpy).toHaveBeenCalled(); + expect(consoleWarnSpy).toHaveBeenCalled(); }); }); }); diff --git a/packages/core/src/utils/installationManager.ts b/packages/core/src/utils/installationManager.ts index 154e3b7597..3fa05c0301 100644 --- a/packages/core/src/utils/installationManager.ts +++ b/packages/core/src/utils/installationManager.ts @@ -8,6 +8,7 @@ import * as fs from 'node:fs'; import { randomUUID } from 'node:crypto'; import * as path from 'node:path'; import { Storage } from '../config/storage.js'; +import { debugLogger } from './debugLogger.js'; export class InstallationManager { private getInstallationIdPath(): string { @@ -48,7 +49,7 @@ export class InstallationManager { return installationId; } catch (error) { - console.error( + debugLogger.warn( 'Error accessing installation ID file, generating ephemeral ID:', error, );