From 08ae3ff9b8c02340dedc610c06466d7eb1dc1ecc Mon Sep 17 00:00:00 2001 From: Mahima Shanware Date: Wed, 8 Apr 2026 22:35:39 +0000 Subject: [PATCH] fix(core): align logging and error handling with strict development rules Addressed reviewer feedback by replacing process.stderr.write with debugLogger.error in config.ts and using isNodeError in storage.ts to adhere to codebase standards. --- packages/core/src/config/config.test.ts | 8 ++++---- packages/core/src/config/config.ts | 13 +++++-------- packages/core/src/config/storage.ts | 10 ++-------- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 68278bc705..0d2ee5d258 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3423,8 +3423,8 @@ describe('Plans Directory Initialization', () => { it('should log a warning if the plan directory path is blocked by an existing file (EEXIST) in PLAN mode', async () => { const writeSpy = vi - .spyOn(process.stderr, 'write') - .mockImplementation(() => true); + .spyOn(debugLogger, 'error') + .mockImplementation(() => {}); vi.spyOn(fs, 'mkdirSync').mockImplementation(() => { const err = new Error('File exists') as NodeJS.ErrnoException; err.code = 'EEXIST'; @@ -3449,8 +3449,8 @@ describe('Plans Directory Initialization', () => { it('should log a warning if mkdirSync fails during getPlansDir (e.g. EACCES) in PLAN mode', async () => { const writeSpy = vi - .spyOn(process.stderr, 'write') - .mockImplementation(() => true); + .spyOn(debugLogger, 'error') + .mockImplementation(() => {}); vi.spyOn(fs, 'mkdirSync').mockImplementation(() => { const err = new Error('Permission denied') as NodeJS.ErrnoException; err.code = 'EACCES'; diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 9679a7ccc0..07060d65b0 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -2275,20 +2275,17 @@ export class Config implements McpContext, AgentLoopContext { let realPlansDir = plansDir; try { - const resolved = resolveToRealPath(plansDir); - if (resolved) { - realPlansDir = resolved; - } + realPlansDir = resolveToRealPath(plansDir); } catch { // Ignore failures in mock environments } this.workspaceContext.addDirectory(realPlansDir); - this.initializedPlanDirs.add(plansDir); } catch (e: unknown) { - const errorMessage = e instanceof Error ? e.message : String(e); - process.stderr.write( - `Failed to initialize active plan directory at '${plansDir}': ${errorMessage}\n`, + debugLogger.error( + `Failed to initialize active plan directory at '${plansDir}': ${getErrorMessage(e)}`, ); + } finally { + this.initializedPlanDirs.add(plansDir); } } else if (!this.planEnabled) { this.initializedPlanDirs.add(plansDir); diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 1766cc60d9..4c21b6d16f 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -18,6 +18,7 @@ import { } from '../utils/paths.js'; import { ProjectRegistry } from './projectRegistry.js'; import { StorageMigration } from './storageMigration.js'; +import { isNodeError } from '../utils/errors.js'; export const OAUTH_FILE = 'oauth_creds.json'; const TMP_DIR_NAME = 'tmp'; @@ -330,14 +331,7 @@ export class Storage { try { realResolvedPath = resolveToRealPath(resolvedPath); } catch (e: unknown) { - if ( - !( - e && - typeof e === 'object' && - 'code' in e && - (e.code === 'ENOENT' || e.code === 'EISDIR') - ) - ) { + if (!(isNodeError(e) && (e.code === 'ENOENT' || e.code === 'EISDIR'))) { throw e; } // Construct the fallback path safely against the real project root