diff --git a/src/commands/cleanup-command.ts b/src/commands/cleanup-command.ts index 1db86c7b..017ef4c4 100644 --- a/src/commands/cleanup-command.ts +++ b/src/commands/cleanup-command.ts @@ -36,20 +36,52 @@ function formatBytes(bytes: number): string { return `${(bytes / Math.pow(1024, i)).toFixed(2)} ${units[i]}`; } -/** Return entries for a real directory, rejecting symlinked directory targets. */ -function readRealDirectory(dirPath: string): string[] { +interface DirectorySummary { + fileCount: number; + size: number; +} + +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function isMissingPathError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error as NodeJS.ErrnoException).code === 'ENOENT' + ); +} + +function pathExistsForCleanup(dirPath: string): boolean { try { - const stats = fs.lstatSync(dirPath); - if (!stats.isDirectory() || stats.isSymbolicLink()) return []; - return fs.readdirSync(dirPath); - } catch { - return []; + fs.lstatSync(dirPath); + return true; + } catch (error) { + if (isMissingPathError(error)) return false; + throw error; } } -/** Calculate total size of regular top-level files in a directory */ -function getDirSize(dirPath: string): number { - let totalSize = 0; +/** Return entries for a real directory, rejecting symlinked directory targets. */ +function readRealDirectory(dirPath: string): string[] { + let stats: fs.Stats; + + try { + stats = fs.lstatSync(dirPath); + } catch (error) { + if (isMissingPathError(error)) return []; + throw error; + } + + if (!stats.isDirectory() || stats.isSymbolicLink()) return []; + return fs.readdirSync(dirPath); +} + +/** Summarize regular top-level files in a real directory */ +function summarizeDirectory(dirPath: string): DirectorySummary { + const summary = { fileCount: 0, size: 0 }; const entries = readRealDirectory(dirPath); for (const entry of entries) { @@ -57,33 +89,15 @@ function getDirSize(dirPath: string): number { try { const stats = fs.lstatSync(filePath); if (stats.isFile() && !stats.isSymbolicLink()) { - totalSize += stats.size; + summary.fileCount++; + summary.size += stats.size; } } catch { // File may have been deleted between readdir and stat - skip } } - return totalSize; -} - -/** Count files in a directory */ -function countFiles(dirPath: string): number { - let count = 0; - const entries = readRealDirectory(dirPath); - - for (const entry of entries) { - const filePath = path.join(dirPath, entry); - try { - const stats = fs.lstatSync(filePath); - if (stats.isFile() && !stats.isSymbolicLink()) { - count++; - } - } catch { - // File may have been deleted - skip - } - } - return count; + return summary; } /** Delete all regular files in a real directory (skips symlinks for safety) */ @@ -154,10 +168,9 @@ function getErrorLogFiles(logsDir: string): ErrorLogInfo[] { /** Delete error logs older than specified days */ function cleanErrorLogs( - logsDir: string, + files: ErrorLogInfo[], maxAgeDays: number ): { deleted: number; freedBytes: number; kept: number } { - const files = getErrorLogFiles(logsDir); let deleted = 0; let freedBytes = 0; let kept = 0; @@ -255,14 +268,23 @@ async function handleErrorLogCleanup( dryRun: boolean, force: boolean ): Promise { - // Check if logs directory exists - if (!fs.existsSync(logsDir)) { - console.log(info('No CLIProxy logs directory found.')); + try { + if (!pathExistsForCleanup(logsDir)) { + console.log(info('No CLIProxy logs directory found.')); + return; + } + } catch (error) { + console.log(warn(`Could not inspect CLIProxy logs: ${getErrorMessage(error)}`)); return; } - // Get error log files - const errorLogs = getErrorLogFiles(logsDir); + let errorLogs: ErrorLogInfo[]; + try { + errorLogs = getErrorLogFiles(logsDir); + } catch (error) { + console.log(warn(`Could not read CLIProxy logs: ${getErrorMessage(error)}`)); + return; + } if (errorLogs.length === 0) { console.log(info('No error logs found.')); return; @@ -327,7 +349,14 @@ async function handleErrorLogCleanup( } // Perform cleanup - const { deleted, freedBytes, kept } = cleanErrorLogs(logsDir, maxAgeDays); + let result: { deleted: number; freedBytes: number; kept: number }; + try { + result = cleanErrorLogs(errorLogs, maxAgeDays); + } catch (error) { + console.log(warn(`Could not clean CLIProxy logs: ${getErrorMessage(error)}`)); + return; + } + const { deleted, freedBytes, kept } = result; console.log(ok(`Deleted ${deleted} error logs, freed ${formatBytes(freedBytes)}`)); if (kept > 0) { console.log(info(`Kept ${kept} recent error logs (less than ${maxAgeDays} days old)`)); @@ -344,15 +373,30 @@ async function handleMainLogCleanup(options: { dryRun: boolean; force: boolean; }): Promise { - const targets = [ + const targets: Array<{ label: string; dir: string } & DirectorySummary> = []; + const unreadableTargets: Array<{ label: string; dir: string; error: unknown }> = []; + for (const target of [ { label: 'CCS Logs', dir: options.ccsLogsDir }, { label: 'CCS Log Archives', dir: options.ccsArchiveDir }, { label: 'CLIProxy Logs', dir: options.cliproxyLogsDir }, - ].map((target) => ({ - ...target, - fileCount: countFiles(target.dir), - size: getDirSize(target.dir), - })); + ]) { + try { + targets.push({ + ...target, + ...summarizeDirectory(target.dir), + }); + } catch (error) { + unreadableTargets.push({ ...target, error }); + } + } + + if (unreadableTargets.length > 0) { + for (const target of unreadableTargets) { + console.log(warn(`Could not read ${target.label}: ${getErrorMessage(target.error)}`)); + console.log(` ${target.dir}`); + } + return; + } const activeTargets = targets.filter((target) => target.fileCount > 0); if (activeTargets.length === 0) { @@ -400,9 +444,13 @@ async function handleMainLogCleanup(options: { let deleted = 0; let freedBytes = 0; for (const target of activeTargets) { - const result = cleanDirectory(target.dir); - deleted += result.deleted; - freedBytes += result.freedBytes; + try { + const result = cleanDirectory(target.dir); + deleted += result.deleted; + freedBytes += result.freedBytes; + } catch (error) { + console.log(warn(`Could not clean ${target.label}: ${getErrorMessage(error)}`)); + } } console.log(ok(`Deleted ${deleted} files, freed ${formatBytes(freedBytes)}`)); diff --git a/tests/unit/commands/cleanup-command.test.ts b/tests/unit/commands/cleanup-command.test.ts index 1e09e680..9c1ae433 100644 --- a/tests/unit/commands/cleanup-command.test.ts +++ b/tests/unit/commands/cleanup-command.test.ts @@ -81,4 +81,94 @@ describe('cleanup command', () => { fs.rmSync(victimDir, { recursive: true, force: true }); } }); + + it('warns instead of reporting no logs when a cleanup directory cannot be read', async () => { + const archiveDir = getLogArchiveDir(); + fs.mkdirSync(archiveDir, { recursive: true }); + + const originalReaddirSync = fs.readdirSync; + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((dirPath, options) => { + if (String(dirPath) === archiveDir) { + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + } + + return originalReaddirSync(dirPath, options as never); + }); + const logSpy = spyOn(console, 'log').mockImplementation(() => {}); + + try { + await handleCleanupCommand(['--dry-run']); + + const output = logSpy.mock.calls + .flatMap((call) => call.map((value) => String(value))) + .join('\n'); + + expect(output).toContain('Could not read CCS Log Archives'); + expect(output).toContain('permission denied'); + expect(output).not.toContain('No CCS or CLIProxy logs found.'); + } finally { + readdirSpy.mockRestore(); + logSpy.mockRestore(); + } + }); + + it('warns instead of reporting no error logs when CLIProxy logs cannot be read', async () => { + const cliproxyLogsDir = path.join(getCliproxyDir(), 'logs'); + fs.mkdirSync(cliproxyLogsDir, { recursive: true }); + + const originalReaddirSync = fs.readdirSync; + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((dirPath, options) => { + if (String(dirPath) === cliproxyLogsDir) { + throw Object.assign(new Error('disk I/O failed'), { code: 'EIO' }); + } + + return originalReaddirSync(dirPath, options as never); + }); + const logSpy = spyOn(console, 'log').mockImplementation(() => {}); + + try { + await handleCleanupCommand(['--errors']); + + const output = logSpy.mock.calls + .flatMap((call) => call.map((value) => String(value))) + .join('\n'); + + expect(output).toContain('Could not read CLIProxy logs'); + expect(output).toContain('disk I/O failed'); + expect(output).not.toContain('No error logs found.'); + } finally { + readdirSpy.mockRestore(); + logSpy.mockRestore(); + } + }); + + it('warns instead of treating CLIProxy log inspection errors as missing directories', async () => { + const cliproxyLogsDir = path.join(getCliproxyDir(), 'logs'); + fs.mkdirSync(cliproxyLogsDir, { recursive: true }); + + const originalLstatSync = fs.lstatSync; + const lstatSpy = spyOn(fs, 'lstatSync').mockImplementation((targetPath, options) => { + if (String(targetPath) === cliproxyLogsDir) { + throw Object.assign(new Error('stat failed'), { code: 'EIO' }); + } + + return originalLstatSync(targetPath, options as never); + }); + const logSpy = spyOn(console, 'log').mockImplementation(() => {}); + + try { + await handleCleanupCommand(['--errors']); + + const output = logSpy.mock.calls + .flatMap((call) => call.map((value) => String(value))) + .join('\n'); + + expect(output).toContain('Could not inspect CLIProxy logs'); + expect(output).toContain('stat failed'); + expect(output).not.toContain('No CLIProxy logs directory found.'); + } finally { + lstatSpy.mockRestore(); + logSpy.mockRestore(); + } + }); });