From 8a88f452308e0dba8688f90ab6d0bac66d060f53 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Tue, 16 Sep 2025 23:49:38 +0900 Subject: [PATCH 01/15] feat(cli): wrap entire defaultAction in child_process while optimizing other workers with worker_threads Implemented a comprehensive worker process architecture that balances performance and isolation: - Created defaultActionWorker.ts that runs entire CLI processing pipeline in child_process for better resource isolation - Migrated all other worker implementations (fileProcess, security, metrics, etc.) from child_process to worker_threads for improved performance - Enhanced logger.ts to support both worker_threads (via workerData) and child_process (via environment variables) - Added color support for child_process workers by passing FORCE_COLOR and TERM environment variables - Moved Spinner from main process to defaultActionWorker for direct stdout control This hybrid approach provides the best of both worlds: the main CLI operation runs in isolated child_process for stability, while intensive parallel tasks use faster worker_threads for optimal performance. --- src/cli/actions/defaultAction.ts | 129 +-- .../actions/workers/defaultActionWorker.ts | 103 +++ src/core/file/fileProcess.ts | 2 +- src/core/file/globbyExecute.ts | 2 +- src/core/metrics/calculateGitDiffMetrics.ts | 2 +- src/core/metrics/calculateGitLogMetrics.ts | 2 +- src/core/metrics/calculateOutputMetrics.ts | 2 +- .../metrics/calculateSelectiveFileMetrics.ts | 2 +- src/core/security/securityCheck.ts | 2 +- src/shared/logger.ts | 17 + src/shared/processConcurrency.ts | 9 + tests/cli/actions/defaultAction.test.ts | 804 +++--------------- 12 files changed, 269 insertions(+), 807 deletions(-) create mode 100644 src/cli/actions/workers/defaultActionWorker.ts diff --git a/src/cli/actions/defaultAction.ts b/src/cli/actions/defaultAction.ts index d75ff90a0..28f5c1109 100644 --- a/src/cli/actions/defaultAction.ts +++ b/src/cli/actions/defaultAction.ts @@ -1,22 +1,18 @@ -import path from 'node:path'; -import { loadFileConfig, mergeConfigs } from '../../config/configLoad.js'; import { type RepomixConfigCli, - type RepomixConfigFile, type RepomixConfigMerged, type RepomixOutputStyle, repomixConfigCliSchema, } from '../../config/configSchema.js'; -import { readFilePathsFromStdin } from '../../core/file/fileStdin.js'; -import { type PackResult, pack } from '../../core/packager.js'; -import { RepomixError } from '../../shared/errorHandle.js'; +import type { PackResult } from '../../core/packager.js'; import { rethrowValidationErrorIfZodError } from '../../shared/errorHandle.js'; import { logger } from '../../shared/logger.js'; import { splitPatterns } from '../../shared/patternUtils.js'; +import { initTaskRunner } from '../../shared/processConcurrency.js'; import { reportResults } from '../cliReport.js'; -import { Spinner } from '../cliSpinner.js'; import type { CliOptions } from '../types.js'; import { runMigrationAction } from './migrationAction.js'; +import type { DefaultActionTask, DefaultActionWorkerResult } from './workers/defaultActionWorker.js'; export interface DefaultActionRunnerResult { packResult: PackResult; @@ -33,107 +29,36 @@ export const runDefaultAction = async ( // Run migration before loading config await runMigrationAction(cwd); - // Load the config file - const fileConfig: RepomixConfigFile = await loadFileConfig(cwd, cliOptions.config ?? null); - logger.trace('Loaded file config:', fileConfig); - - // Parse the CLI options into a config - const cliConfig: RepomixConfigCli = buildCliConfig(cliOptions); - logger.trace('CLI config:', cliConfig); - - // Merge default, file, and CLI configs - const config: RepomixConfigMerged = mergeConfigs(cwd, fileConfig, cliConfig); - logger.trace('Merged config:', config); - - // Initialize spinner that can be shared across operations - const spinner = new Spinner('Initializing...', cliOptions); - spinner.start(); - - const result = cliOptions.stdin - ? await handleStdinProcessing(directories, cwd, config, spinner) - : await handleDirectoryProcessing(directories, cwd, config, spinner); - - spinner.succeed('Packing completed successfully!'); - - const packResult = result.packResult; - - reportResults(cwd, packResult, config); - - return { - packResult, - config, - }; -}; - -/** - * Handles stdin processing workflow for file paths input. - */ -export const handleStdinProcessing = async ( - directories: string[], - cwd: string, - config: RepomixConfigMerged, - spinner: Spinner, -): Promise => { - // Validate directory arguments for stdin mode - const firstDir = directories[0] ?? '.'; - if (directories.length > 1 || firstDir !== '.') { - throw new RepomixError( - 'When using --stdin, do not specify directory arguments. File paths will be read from stdin.', - ); - } - - let packResult: PackResult; + // Create worker task runner + const taskRunner = initTaskRunner({ + numOfTasks: 1, + workerPath: new URL('./workers/defaultActionWorker.js', import.meta.url).href, + runtime: 'child_process', + }); try { - const stdinResult = await readFilePathsFromStdin(cwd); - - // Use pack with predefined files from stdin - packResult = await pack( - [cwd], - config, - (message) => { - spinner.update(message); - }, - {}, - stdinResult.filePaths, - ); - } catch (error) { - spinner.fail('Error reading from stdin or during packing'); - throw error; - } + // Create task for worker + const task: DefaultActionTask = { + directories, + cwd, + cliOptions, + isStdin: !!cliOptions.stdin, + }; - return { - packResult, - config, - }; -}; + // Run the task in worker (spinner is handled inside worker) + const result = await taskRunner.run(task); -/** - * Handles normal directory processing workflow. - */ -export const handleDirectoryProcessing = async ( - directories: string[], - cwd: string, - config: RepomixConfigMerged, - spinner: Spinner, -): Promise => { - const targetPaths = directories.map((directory) => path.resolve(cwd, directory)); + // Report results in main process + reportResults(cwd, result.packResult, result.config); - let packResult: PackResult; - - try { - packResult = await pack(targetPaths, config, (message) => { - spinner.update(message); - }); - } catch (error) { - spinner.fail('Error during packing'); - throw error; + return { + packResult: result.packResult, + config: result.config, + }; + } finally { + // Always cleanup worker pool + await taskRunner.cleanup(); } - - return { - packResult, - config, - }; }; /** diff --git a/src/cli/actions/workers/defaultActionWorker.ts b/src/cli/actions/workers/defaultActionWorker.ts new file mode 100644 index 000000000..556122d51 --- /dev/null +++ b/src/cli/actions/workers/defaultActionWorker.ts @@ -0,0 +1,103 @@ +import path from 'node:path'; +import { loadFileConfig, mergeConfigs } from '../../../config/configLoad.js'; +import type { RepomixConfigCli, RepomixConfigFile, RepomixConfigMerged } from '../../../config/configSchema.js'; +import { readFilePathsFromStdin } from '../../../core/file/fileStdin.js'; +import { type PackResult, pack } from '../../../core/packager.js'; +import { RepomixError } from '../../../shared/errorHandle.js'; +import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; +import { Spinner } from '../../cliSpinner.js'; +import type { CliOptions } from '../../types.js'; +import { buildCliConfig } from '../defaultAction.js'; + +// Initialize logger configuration from workerData at module load time +// This must be called before any logging operations in the worker +setLogLevelByWorkerData(); + +export interface DefaultActionTask { + directories: string[]; + cwd: string; + cliOptions: CliOptions; + isStdin: boolean; +} + +export interface DefaultActionWorkerResult { + packResult: PackResult; + config: RepomixConfigMerged; +} + +export default async ({ + directories, + cwd, + cliOptions, + isStdin, +}: DefaultActionTask): Promise => { + logger.trace('Worker: Loaded CLI options:', cliOptions); + + // Load the config file + const fileConfig: RepomixConfigFile = await loadFileConfig(cwd, cliOptions.config ?? null); + logger.trace('Worker: Loaded file config:', fileConfig); + + // Parse the CLI options into a config + const cliConfig: RepomixConfigCli = buildCliConfig(cliOptions); + logger.trace('Worker: CLI config:', cliConfig); + + // Merge default, file, and CLI configs + const config: RepomixConfigMerged = mergeConfigs(cwd, fileConfig, cliConfig); + logger.trace('Worker: Merged config:', config); + + // Initialize spinner in worker + const spinner = new Spinner('Initializing...', cliOptions); + spinner.start(); + + let packResult: PackResult; + + try { + + if (isStdin) { + // Handle stdin processing + // Validate directory arguments for stdin mode + const firstDir = directories[0] ?? '.'; + if (directories.length > 1 || firstDir !== '.') { + throw new RepomixError( + 'When using --stdin, do not specify directory arguments. File paths will be read from stdin.', + ); + } + + const stdinResult = await readFilePathsFromStdin(cwd); + + // Use pack with predefined files from stdin + packResult = await pack( + [cwd], + config, + (message) => { + spinner.update(message); + }, + {}, + stdinResult.filePaths, + ); + } else { + // Handle directory processing + const targetPaths = directories.map((directory) => path.resolve(cwd, directory)); + + packResult = await pack(targetPaths, config, (message) => { + spinner.update(message); + }); + } + + spinner.succeed('Packing completed successfully!'); + + return { + packResult, + config, + }; + } catch (error) { + spinner.fail('Error during packing'); + throw error; + } +}; + +// Export cleanup function for Tinypool teardown +export const onWorkerTermination = async () => { + // Any cleanup needed when worker terminates + // Currently no specific cleanup required for defaultAction worker +}; diff --git a/src/core/file/fileProcess.ts b/src/core/file/fileProcess.ts index 5a1483412..85e7016d7 100644 --- a/src/core/file/fileProcess.ts +++ b/src/core/file/fileProcess.ts @@ -25,7 +25,7 @@ export const processFiles = async ( numOfTasks: rawFiles.length, workerPath: new URL('./workers/fileProcessWorker.js', import.meta.url).href, // High memory usage and leak risk - runtime: 'child_process', + runtime: 'worker_threads', }); const tasks = rawFiles.map( (rawFile, _index) => diff --git a/src/core/file/globbyExecute.ts b/src/core/file/globbyExecute.ts index 1f7200078..6b7d9a53d 100644 --- a/src/core/file/globbyExecute.ts +++ b/src/core/file/globbyExecute.ts @@ -16,7 +16,7 @@ export const executeGlobbyInWorker = async ( const taskRunner = deps.initTaskRunner({ numOfTasks: 1, workerPath: new URL('./workers/globbyWorker.js', import.meta.url).href, - runtime: 'child_process', + runtime: 'worker_threads', }); try { diff --git a/src/core/metrics/calculateGitDiffMetrics.ts b/src/core/metrics/calculateGitDiffMetrics.ts index 7c2cadac5..64e461b93 100644 --- a/src/core/metrics/calculateGitDiffMetrics.ts +++ b/src/core/metrics/calculateGitDiffMetrics.ts @@ -26,7 +26,7 @@ export const calculateGitDiffMetrics = async ( const taskRunner = deps.initTaskRunner({ numOfTasks: 1, // Single task for git diff calculation workerPath: new URL('./workers/gitDiffMetricsWorker.js', import.meta.url).href, - runtime: 'child_process', + runtime: 'worker_threads', }); try { diff --git a/src/core/metrics/calculateGitLogMetrics.ts b/src/core/metrics/calculateGitLogMetrics.ts index f9ec1d6c7..5949d312d 100644 --- a/src/core/metrics/calculateGitLogMetrics.ts +++ b/src/core/metrics/calculateGitLogMetrics.ts @@ -31,7 +31,7 @@ export const calculateGitLogMetrics = async ( const taskRunner = deps.initTaskRunner({ numOfTasks: 1, // Single task for git log calculation workerPath: new URL('./workers/gitLogMetricsWorker.js', import.meta.url).href, - runtime: 'child_process', + runtime: 'worker_threads', }); try { diff --git a/src/core/metrics/calculateOutputMetrics.ts b/src/core/metrics/calculateOutputMetrics.ts index 44d8200f0..db3d27e66 100644 --- a/src/core/metrics/calculateOutputMetrics.ts +++ b/src/core/metrics/calculateOutputMetrics.ts @@ -19,7 +19,7 @@ export const calculateOutputMetrics = async ( const taskRunner = deps.initTaskRunner({ numOfTasks, workerPath: new URL('./workers/outputMetricsWorker.js', import.meta.url).href, - runtime: 'child_process', + runtime: 'worker_threads', }); try { diff --git a/src/core/metrics/calculateSelectiveFileMetrics.ts b/src/core/metrics/calculateSelectiveFileMetrics.ts index 611520433..e89f87fe7 100644 --- a/src/core/metrics/calculateSelectiveFileMetrics.ts +++ b/src/core/metrics/calculateSelectiveFileMetrics.ts @@ -26,7 +26,7 @@ export const calculateSelectiveFileMetrics = async ( const taskRunner = deps.initTaskRunner({ numOfTasks: filesToProcess.length, workerPath: new URL('./workers/fileMetricsWorker.js', import.meta.url).href, - runtime: 'child_process', + runtime: 'worker_threads', }); const tasks = filesToProcess.map( (file, index) => diff --git a/src/core/security/securityCheck.ts b/src/core/security/securityCheck.ts index 4dd1aa43b..d20cf692a 100644 --- a/src/core/security/securityCheck.ts +++ b/src/core/security/securityCheck.ts @@ -58,7 +58,7 @@ export const runSecurityCheck = async ( const taskRunner = deps.initTaskRunner({ numOfTasks: rawFiles.length + gitDiffTasks.length + gitLogTasks.length, workerPath: new URL('./workers/securityCheckWorker.js', import.meta.url).href, - runtime: 'child_process', + runtime: 'worker_threads', }); const fileTasks = rawFiles.map( (file) => diff --git a/src/shared/logger.ts b/src/shared/logger.ts index 68c929a36..ba544b978 100644 --- a/src/shared/logger.ts +++ b/src/shared/logger.ts @@ -97,6 +97,23 @@ export const setLogLevel = (level: RepomixLogLevel) => { * This is used in worker threads where configuration is passed via workerData. */ export const setLogLevelByWorkerData = () => { + // Try to get log level from environment variable first (for child_process workers) + const envLogLevel = process.env.REPOMIX_LOG_LEVEL; + if (envLogLevel !== undefined) { + const logLevel = Number(envLogLevel); + if ( + logLevel === repomixLogLevels.SILENT || + logLevel === repomixLogLevels.ERROR || + logLevel === repomixLogLevels.WARN || + logLevel === repomixLogLevels.INFO || + logLevel === repomixLogLevels.DEBUG + ) { + setLogLevel(logLevel); + return; + } + } + + // Fallback to workerData for worker_threads if (Array.isArray(workerData) && workerData.length > 1 && workerData[1]?.logLevel !== undefined) { const logLevel = workerData[1].logLevel; if ( diff --git a/src/shared/processConcurrency.ts b/src/shared/processConcurrency.ts index fde833a9e..f6bef0a9d 100644 --- a/src/shared/processConcurrency.ts +++ b/src/shared/processConcurrency.ts @@ -51,6 +51,15 @@ export const createWorkerPool = (options: WorkerOptions): Tinypool => { workerData: { logLevel: logger.getLogLevel(), }, + env: { + ...process.env, + // Pass log level as environment variable for child_process workers + REPOMIX_LOG_LEVEL: logger.getLogLevel().toString(), + // Ensure color support in child_process workers + FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0'), + // Pass terminal capabilities + TERM: process.env.TERM || 'xterm-256color', + }, }); const endTime = process.hrtime.bigint(); diff --git a/tests/cli/actions/defaultAction.test.ts b/tests/cli/actions/defaultAction.test.ts index 5401a05fd..7ca88db3f 100644 --- a/tests/cli/actions/defaultAction.test.ts +++ b/tests/cli/actions/defaultAction.test.ts @@ -1,28 +1,21 @@ -import path from 'node:path'; import process from 'node:process'; -import { globby } from 'globby'; import { type MockedFunction, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { - buildCliConfig, - handleDirectoryProcessing, - handleStdinProcessing, - runDefaultAction, -} from '../../../src/cli/actions/defaultAction.js'; +import { buildCliConfig, runDefaultAction } from '../../../src/cli/actions/defaultAction.js'; import { Spinner } from '../../../src/cli/cliSpinner.js'; import type { CliOptions } from '../../../src/cli/types.js'; import * as configLoader from '../../../src/config/configLoad.js'; -import * as fileStdin from '../../../src/core/file/fileStdin.js'; import * as packageJsonParser from '../../../src/core/file/packageJsonParse.js'; import * as packager from '../../../src/core/packager.js'; -import type { PackResult } from '../../../src/core/packager.js'; + +import * as processConcurrency from '../../../src/shared/processConcurrency.js'; import { createMockConfig } from '../../testing/testUtils.js'; -vi.mock('globby'); vi.mock('../../../src/core/packager'); vi.mock('../../../src/config/configLoad'); vi.mock('../../../src/core/file/packageJsonParse'); -vi.mock('../../../src/core/file/fileStdin'); vi.mock('../../../src/shared/logger'); +vi.mock('../../../src/shared/processConcurrency'); + const mockSpinner = { start: vi.fn() as MockedFunction<() => void>, update: vi.fn() as MockedFunction<(message: string) => void>, @@ -52,8 +45,6 @@ describe('defaultAction', () => { vi.mocked(packageJsonParser.getVersion).mockResolvedValue('1.0.0'); vi.mocked(configLoader.loadFileConfig).mockResolvedValue({}); - // Default globby mock - vi.mocked(globby).mockResolvedValue([]); vi.mocked(configLoader.mergeConfigs).mockReturnValue( createMockConfig({ cwd: process.cwd(), @@ -109,6 +100,33 @@ describe('defaultAction', () => { gitLogTokenCount: 0, skippedFiles: [], }); + + // Mock initTaskRunner to return a simple task runner + const mockTaskRunner = { + run: vi.fn().mockResolvedValue({ + packResult: { + totalFiles: 10, + totalCharacters: 1000, + totalTokens: 200, + fileCharCounts: {}, + fileTokenCounts: {}, + suspiciousFilesResults: [], + suspiciousGitDiffResults: [], + suspiciousGitLogResults: [], + processedFiles: [], + safeFilePaths: [], + gitDiffTokenCount: 0, + gitLogTokenCount: 0, + skippedFiles: [], + }, + config: createMockConfig({ + cwd: process.cwd(), + }), + }), + cleanup: vi.fn().mockResolvedValue(undefined), + }; + + vi.mocked(processConcurrency.initTaskRunner).mockReturnValue(mockTaskRunner); }); afterEach(() => { @@ -123,9 +141,16 @@ describe('defaultAction', () => { await runDefaultAction(['.'], process.cwd(), options); - expect(configLoader.loadFileConfig).toHaveBeenCalled(); - expect(configLoader.mergeConfigs).toHaveBeenCalled(); - expect(packager.pack).toHaveBeenCalled(); + expect(processConcurrency.initTaskRunner).toHaveBeenCalledWith({ + numOfTasks: 1, + workerPath: expect.stringContaining('defaultActionWorker.js'), + runtime: 'child_process', + }); + + const taskRunner = vi.mocked(processConcurrency.initTaskRunner).mock.results[0].value; + expect(taskRunner.run).toHaveBeenCalled(); + expect(taskRunner.cleanup).toHaveBeenCalled(); + }); }); it('should handle custom include patterns', async () => { @@ -135,736 +160,119 @@ describe('defaultAction', () => { await runDefaultAction(['.'], process.cwd(), options); - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - include: ['*.js', '*.ts'], - }), - ); - }); - - it('should handle custom ignore patterns', async () => { - const options: CliOptions = { - ignore: 'node_modules,*.log', - }; - - await runDefaultAction(['.'], process.cwd(), options); + const taskRunner = vi.mocked(processConcurrency.initTaskRunner).mock.results[0].value; + const task = taskRunner.run.mock.calls[0][0]; - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - ignore: { - customPatterns: ['node_modules', '*.log'], - }, + expect(task).toMatchObject({ + directories: ['.'], + cwd: process.cwd(), + cliOptions: expect.objectContaining({ + include: '*.js,*.ts', }), - ); + isStdin: false, + }); }); - it('should handle custom output style', async () => { + it('should handle stdin mode', async () => { const options: CliOptions = { - style: 'xml', + stdin: true, }; await runDefaultAction(['.'], process.cwd(), options); - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: expect.objectContaining({ - style: 'xml', - }), + const taskRunner = vi.mocked(processConcurrency.initTaskRunner).mock.results[0].value; + const task = taskRunner.run.mock.calls[0][0]; + + expect(task).toMatchObject({ + directories: ['.'], + cwd: process.cwd(), + cliOptions: expect.objectContaining({ + stdin: true, }), - ); + isStdin: true, + }); }); it('should handle errors gracefully', async () => { - vi.mocked(packager.pack).mockRejectedValue(new Error('Test error')); + // Create a fresh mock task runner that will fail + const failingTaskRunner = { + run: vi.fn().mockRejectedValue(new Error('Test error')), + cleanup: vi.fn().mockResolvedValue(undefined), + }; + + vi.mocked(processConcurrency.initTaskRunner).mockReturnValue(failingTaskRunner); const options: CliOptions = {}; await expect(runDefaultAction(['.'], process.cwd(), options)).rejects.toThrow('Test error'); + expect(mockSpinner.fail).toHaveBeenCalledWith('Error during packing'); + expect(failingTaskRunner.cleanup).toHaveBeenCalled(); }); - describe('parsableStyle flag', () => { - it('should handle --parsable-style flag', async () => { - const options: CliOptions = { - parsableStyle: true, + describe('buildCliConfig', () => { + it('should handle custom include patterns', () => { + const options = { + include: '*.js,*.ts', }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - parsableStyle: true, - }, - }), - ); + expect(config.include).toEqual(['*.js', '*.ts']); }); - it('should handle explicit --no-parsable-style flag', async () => { - const options: CliOptions = { - parsableStyle: false, + it('should handle custom ignore patterns', () => { + const options = { + ignore: 'node_modules,*.log', }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - parsableStyle: false, - }, - }), - ); + expect(config.ignore?.customPatterns).toEqual(['node_modules', '*.log']); }); - }); - describe('stdout flag', () => { - it('should set stdout to true when --stdout flag is set', async () => { + it('should handle custom output style', () => { const options: CliOptions = { - stdout: true, + style: 'xml' as const, }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: expect.objectContaining({ - stdout: true, - }), - }), - ); + expect(config.output?.style).toBe('xml'); }); - it('should handle both --stdout and custom style', async () => { - const options: CliOptions = { - stdout: true, - style: 'markdown', + it('should properly trim whitespace from comma-separated patterns', () => { + const options = { + include: 'src/**/*, tests/**/*, examples/**/*', + ignore: 'node_modules/**, dist/**, coverage/**', }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: expect.objectContaining({ - stdout: true, - style: 'markdown', - }), - }), - ); + expect(config.include).toEqual(['src/**/*', 'tests/**/*', 'examples/**/*']); + expect(config.ignore?.customPatterns).toEqual(['node_modules/**', 'dist/**', 'coverage/**']); }); - }); - describe('security check flag', () => { - it('should handle --no-security-check flag', async () => { - const options: CliOptions = { + it('should handle --no-security-check flag', () => { + const options = { securityCheck: false, }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - security: { - enableSecurityCheck: false, - }, - }), - ); - }); - - it('should handle explicit --security-check flag', async () => { - const options: CliOptions = { - securityCheck: true, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({}), - ); + expect(config.security?.enableSecurityCheck).toBe(false); }); - }); - describe('gitignore flag', () => { - it('should handle explicit --no-gitignore flag', async () => { - const options: CliOptions = { - gitignore: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - ignore: { - useGitignore: false, - }, - }), - ); - }); - - it('should handle explicit --no-gitignore flag', async () => { - const options: CliOptions = { - gitignore: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({}), - ); - }); - }); - - describe('defaultPatterns flag', () => { - it('should handle explicit --no-default-patterns flag', async () => { - const options: CliOptions = { - defaultPatterns: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - ignore: { - useDefaultPatterns: false, - }, - }), - ); - }); - - it('should handle explicit --no-default-patterns flag', async () => { - const options: CliOptions = { - defaultPatterns: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({}), - ); - }); - }); - - describe('fileSummary flag', () => { - it('should handle --no-file-summary flag', async () => { - const options: CliOptions = { + it('should handle --no-file-summary flag', () => { + const options = { fileSummary: false, }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - fileSummary: false, - }, - }), - ); + expect(config.output?.fileSummary).toBe(false); }); - it('should handle explicit --file-summary flag', async () => { - const options: CliOptions = { - fileSummary: true, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({}), - ); - }); - }); - - describe('directoryStructure flag', () => { - it('should handle --no-directory-structure flag', async () => { - const options: CliOptions = { - directoryStructure: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - directoryStructure: false, - }, - }), - ); - }); - - it('should handle explicit --directory-structure flag', async () => { - const options: CliOptions = { - directoryStructure: true, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({}), - ); - }); - }); - - describe('removeComments flag', () => { - it('should handle --remove-comments flag', async () => { - const options: CliOptions = { + it('should handle --remove-comments flag', () => { + const options = { removeComments: true, }; + const config = buildCliConfig(options); - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - removeComments: true, - }, - }), - ); - }); - - it('should handle explicit --no-remove-comments flag', async () => { - const options: CliOptions = { - removeComments: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - removeComments: false, - }, - }), - ); - }); - }); - - describe('removeEmptyLines flag', () => { - it('should handle --remove-empty-lines flag', async () => { - const options: CliOptions = { - removeEmptyLines: true, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - removeEmptyLines: true, - }, - }), - ); - }); - - it('should handle explicit --no-remove-empty-lines flag', async () => { - const options: CliOptions = { - removeEmptyLines: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - removeEmptyLines: false, - }, - }), - ); - }); - }); - - describe('headerText flag', () => { - it('should handle --header-text flag', async () => { - const options: CliOptions = { - headerText: 'Another header text', - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - headerText: 'Another header text', - }, - }), - ); - }); - }); - - describe('instructionFilePath flag', () => { - it('should handle --instruction-file-path flag', async () => { - const options: CliOptions = { - instructionFilePath: 'path/to/instruction.txt', - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - instructionFilePath: 'path/to/instruction.txt', - }, - }), - ); - }); - }); - - describe('includeEmptyDirectories flag', () => { - it('should handle --include-empty-directories flag', async () => { - const options: CliOptions = { - includeEmptyDirectories: true, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - includeEmptyDirectories: true, - }, - }), - ); - }); - }); - - it('should properly trim whitespace from comma-separated patterns', () => { - const options = { - include: 'src/**/*, tests/**/*, examples/**/*', - ignore: 'node_modules/**, dist/**, coverage/**', - }; - const config = buildCliConfig(options); - - expect(config.include).toEqual(['src/**/*', 'tests/**/*', 'examples/**/*']); - expect(config.ignore?.customPatterns).toEqual(['node_modules/**', 'dist/**', 'coverage/**']); - }); - - describe('files flag', () => { - it('should handle --no-files flag', async () => { - const options: CliOptions = { - files: false, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({ - output: { - files: false, - }, - }), - ); - }); - - it('should handle explicit --files flag', async () => { - const options: CliOptions = { - files: true, - }; - - await runDefaultAction(['.'], process.cwd(), options); - - expect(configLoader.mergeConfigs).toHaveBeenCalledWith( - process.cwd(), - expect.anything(), - expect.objectContaining({}), - ); - }); - }); - - describe('handleStdinProcessing', () => { - const testCwd = path.resolve('/test/cwd'); - const mockConfig = createMockConfig({ - cwd: testCwd, - input: { maxFileSize: 50 * 1024 * 1024 }, - output: { - filePath: 'output.txt', - style: 'plain' as const, - parsableStyle: false, - fileSummary: true, - directoryStructure: true, - topFilesLength: 5, - showLineNumbers: false, - removeComments: false, - removeEmptyLines: false, - compress: false, - copyToClipboard: false, - stdout: false, - git: { sortByChanges: true, sortByChangesMaxCommits: 100, includeDiffs: false }, - files: true, - }, - ignore: { useGitignore: true, useDefaultPatterns: true, customPatterns: [] }, - include: [], - security: { enableSecurityCheck: true }, - tokenCount: { encoding: 'cl100k_base' as const }, - }); - - beforeEach(() => { - vi.mocked(packager.pack).mockResolvedValue({ - totalTokens: 1000, - totalFiles: 3, - totalChars: 2500, - totalCharacters: 2500, - gitDiffTokenCount: 0, - gitLogTokenCount: 0, - processedFiles: [], - safeFilePaths: [], - suspiciousFilesResults: [], - suspiciousGitDiffResults: [], - suspiciousGitLogResults: [], - fileCharCounts: {}, - fileTokenCounts: {}, - outputFilePath: 'output.txt', - skippedFiles: [], - } as PackResult); - }); - - it('should validate directory arguments and throw error for multiple directories', async () => { - await expect(handleStdinProcessing(['dir1', 'dir2'], testCwd, mockConfig, mockSpinner)).rejects.toThrow( - 'When using --stdin, do not specify directory arguments', - ); - }); - - it('should validate directory arguments and throw error for non-default directory', async () => { - await expect(handleStdinProcessing(['src'], testCwd, mockConfig, mockSpinner)).rejects.toThrow( - 'When using --stdin, do not specify directory arguments', - ); - }); - - it('should accept default directory argument', async () => { - vi.mocked(fileStdin.readFilePathsFromStdin).mockResolvedValue({ - filePaths: [path.resolve(testCwd, 'file1.txt')], - emptyDirPaths: [], - }); - - const result = await handleStdinProcessing(['.'], testCwd, mockConfig, mockSpinner); - - expect(result).toEqual({ - packResult: expect.any(Object), - config: mockConfig, - }); - expect(fileStdin.readFilePathsFromStdin).toHaveBeenCalledWith(testCwd); - }); - - it('should handle empty directories array', async () => { - vi.mocked(fileStdin.readFilePathsFromStdin).mockResolvedValue({ - filePaths: [path.resolve(testCwd, 'file1.txt')], - emptyDirPaths: [], - }); - - const result = await handleStdinProcessing([], testCwd, mockConfig, mockSpinner); - - expect(result).toEqual({ - packResult: expect.any(Object), - config: mockConfig, - }); - }); - - it('should call pack with correct arguments from stdin result', async () => { - const stdinResult = { - filePaths: [path.resolve(testCwd, 'file1.txt'), path.resolve(testCwd, 'subdir/file2.txt')], - emptyDirPaths: [path.resolve(testCwd, 'emptydir')], - }; - - vi.mocked(fileStdin.readFilePathsFromStdin).mockResolvedValue(stdinResult); - - // Mock globby to return the expected filtered files (sorted by sortPaths) - vi.mocked(globby).mockResolvedValue([path.join('subdir', 'file2.txt'), 'file1.txt']); - - await handleStdinProcessing(['.'], testCwd, mockConfig, mockSpinner); - - expect(packager.pack).toHaveBeenCalledWith([testCwd], mockConfig, expect.any(Function), {}, [ - path.join(testCwd, 'file1.txt'), - path.join(testCwd, 'subdir', 'file2.txt'), - ]); - }); - - it('should propagate errors from readFilePathsFromStdin', async () => { - const error = new Error('stdin read error'); - vi.mocked(fileStdin.readFilePathsFromStdin).mockRejectedValue(error); - - await expect(handleStdinProcessing(['.'], testCwd, mockConfig, mockSpinner)).rejects.toThrow('stdin read error'); - }); - - it('should propagate errors from pack operation', async () => { - vi.mocked(fileStdin.readFilePathsFromStdin).mockResolvedValue({ - filePaths: [path.resolve(testCwd, 'file1.txt')], - emptyDirPaths: [], - }); - - const error = new Error('pack error'); - vi.mocked(packager.pack).mockRejectedValue(error); - - await expect(handleStdinProcessing(['.'], testCwd, mockConfig, mockSpinner)).rejects.toThrow('pack error'); - }); - }); - - describe('handleDirectoryProcessing', () => { - const testCwd = path.resolve('/test/cwd'); - const mockConfig = createMockConfig({ - cwd: testCwd, - input: { maxFileSize: 50 * 1024 * 1024 }, - output: { - filePath: 'output.txt', - style: 'plain' as const, - parsableStyle: false, - fileSummary: true, - directoryStructure: true, - topFilesLength: 5, - showLineNumbers: false, - removeComments: false, - removeEmptyLines: false, - compress: false, - copyToClipboard: false, - stdout: false, - git: { sortByChanges: true, sortByChangesMaxCommits: 100, includeDiffs: false }, - files: true, - }, - ignore: { useGitignore: true, useDefaultPatterns: true, customPatterns: [] }, - include: [], - security: { enableSecurityCheck: true }, - tokenCount: { encoding: 'cl100k_base' as const }, - }); - - beforeEach(() => { - vi.mocked(packager.pack).mockResolvedValue({ - totalTokens: 1000, - totalFiles: 3, - totalChars: 2500, - totalCharacters: 2500, - gitDiffTokenCount: 0, - gitLogTokenCount: 0, - processedFiles: [], - safeFilePaths: [], - suspiciousFilesResults: [], - suspiciousGitDiffResults: [], - suspiciousGitLogResults: [], - fileCharCounts: {}, - fileTokenCounts: {}, - outputFilePath: 'output.txt', - skippedFiles: [], - } as PackResult); - }); - - it('should resolve directory paths and call pack with absolute paths', async () => { - const directories = ['src', 'lib', './docs']; - - const result = await handleDirectoryProcessing(directories, testCwd, mockConfig, mockSpinner); - - expect(packager.pack).toHaveBeenCalledWith( - [path.resolve(testCwd, 'src'), path.resolve(testCwd, 'lib'), path.resolve(testCwd, 'docs')], - mockConfig, - expect.any(Function), - ); - - expect(result).toEqual({ - packResult: expect.any(Object), - config: mockConfig, - }); - }); - - it('should handle single directory', async () => { - const directories = ['.']; - - await handleDirectoryProcessing(directories, testCwd, mockConfig, mockSpinner); - - expect(packager.pack).toHaveBeenCalledWith([testCwd], mockConfig, expect.any(Function)); - }); - - it('should handle absolute directory paths', async () => { - const absolutePath1 = path.resolve('/absolute/path'); - const absolutePath2 = path.resolve('/another/absolute'); - const directories = [absolutePath1, absolutePath2]; - - await handleDirectoryProcessing(directories, testCwd, mockConfig, mockSpinner); - - expect(packager.pack).toHaveBeenCalledWith([absolutePath1, absolutePath2], mockConfig, expect.any(Function)); - }); - - it('should propagate errors from pack operation', async () => { - const error = new Error('pack error'); - vi.mocked(packager.pack).mockRejectedValue(error); - - await expect(handleDirectoryProcessing(['.'], testCwd, mockConfig, mockSpinner)).rejects.toThrow('pack error'); - }); - - it('should call progress callback during packing', async () => { - let progressCallback: ((message: string) => void) | undefined; - - vi.mocked(packager.pack).mockImplementation(async (paths, config, callback) => { - progressCallback = callback; - // Simulate progress callback - if (callback) { - callback('Processing files...'); - } - return { - totalTokens: 1000, - totalFiles: 3, - totalChars: 2500, - totalCharacters: 2500, - gitDiffTokenCount: 0, - gitLogTokenCount: 0, - processedFiles: [], - safeFilePaths: [], - suspiciousFilesResults: [], - suspiciousGitDiffResults: [], - suspiciousGitLogResults: [], - fileCharCounts: {}, - fileTokenCounts: {}, - outputFilePath: 'output.txt', - skippedFiles: [], - } as PackResult; - }); - - await handleDirectoryProcessing(['.'], testCwd, mockConfig, mockSpinner); - - expect(progressCallback).toBeDefined(); - expect(packager.pack).toHaveBeenCalledWith(expect.any(Array), expect.any(Object), expect.any(Function)); + expect(config.output?.removeComments).toBe(true); }); }); }); From bd88340da3a1cf2b68e677df126f6a812f5e8b78 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Tue, 16 Sep 2025 23:59:15 +0900 Subject: [PATCH 02/15] refactor(cli): optimize defaultAction by moving config loading to main process Moved configuration loading (loadFileConfig, buildCliConfig, mergeConfigs) from worker process to main process for better performance and cleaner separation of concerns. Changes: - Config loading now happens in main process before worker creation - Worker receives pre-loaded config via task interface, eliminating redundant config operations - Reduced worker startup overhead and simplified worker responsibilities - Worker now focuses solely on heavy processing tasks (pack + spinner) This optimization maintains the same functionality while improving startup performance and creating clearer architectural boundaries between main and worker processes. --- src/cli/actions/defaultAction.ts | 17 +++++++++++++++- .../actions/workers/defaultActionWorker.ts | 20 ++++--------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/cli/actions/defaultAction.ts b/src/cli/actions/defaultAction.ts index 28f5c1109..8109ba45c 100644 --- a/src/cli/actions/defaultAction.ts +++ b/src/cli/actions/defaultAction.ts @@ -1,5 +1,7 @@ +import { loadFileConfig, mergeConfigs } from '../../config/configLoad.js'; import { type RepomixConfigCli, + type RepomixConfigFile, type RepomixConfigMerged, type RepomixOutputStyle, repomixConfigCliSchema, @@ -29,6 +31,18 @@ export const runDefaultAction = async ( // Run migration before loading config await runMigrationAction(cwd); + // Load the config file in main process + const fileConfig: RepomixConfigFile = await loadFileConfig(cwd, cliOptions.config ?? null); + logger.trace('Loaded file config:', fileConfig); + + // Parse the CLI options into a config + const cliConfig: RepomixConfigCli = buildCliConfig(cliOptions); + logger.trace('CLI config:', cliConfig); + + // Merge default, file, and CLI configs + const config: RepomixConfigMerged = mergeConfigs(cwd, fileConfig, cliConfig); + logger.trace('Merged config:', config); + // Create worker task runner const taskRunner = initTaskRunner({ numOfTasks: 1, @@ -37,10 +51,11 @@ export const runDefaultAction = async ( }); try { - // Create task for worker + // Create task for worker (now with pre-loaded config) const task: DefaultActionTask = { directories, cwd, + config, cliOptions, isStdin: !!cliOptions.stdin, }; diff --git a/src/cli/actions/workers/defaultActionWorker.ts b/src/cli/actions/workers/defaultActionWorker.ts index 556122d51..2caea1d23 100644 --- a/src/cli/actions/workers/defaultActionWorker.ts +++ b/src/cli/actions/workers/defaultActionWorker.ts @@ -1,13 +1,11 @@ import path from 'node:path'; -import { loadFileConfig, mergeConfigs } from '../../../config/configLoad.js'; -import type { RepomixConfigCli, RepomixConfigFile, RepomixConfigMerged } from '../../../config/configSchema.js'; +import type { RepomixConfigMerged } from '../../../config/configSchema.js'; import { readFilePathsFromStdin } from '../../../core/file/fileStdin.js'; import { type PackResult, pack } from '../../../core/packager.js'; import { RepomixError } from '../../../shared/errorHandle.js'; import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; import { Spinner } from '../../cliSpinner.js'; import type { CliOptions } from '../../types.js'; -import { buildCliConfig } from '../defaultAction.js'; // Initialize logger configuration from workerData at module load time // This must be called before any logging operations in the worker @@ -16,6 +14,7 @@ setLogLevelByWorkerData(); export interface DefaultActionTask { directories: string[]; cwd: string; + config: RepomixConfigMerged; cliOptions: CliOptions; isStdin: boolean; } @@ -28,22 +27,11 @@ export interface DefaultActionWorkerResult { export default async ({ directories, cwd, + config, cliOptions, isStdin, }: DefaultActionTask): Promise => { - logger.trace('Worker: Loaded CLI options:', cliOptions); - - // Load the config file - const fileConfig: RepomixConfigFile = await loadFileConfig(cwd, cliOptions.config ?? null); - logger.trace('Worker: Loaded file config:', fileConfig); - - // Parse the CLI options into a config - const cliConfig: RepomixConfigCli = buildCliConfig(cliOptions); - logger.trace('Worker: CLI config:', cliConfig); - - // Merge default, file, and CLI configs - const config: RepomixConfigMerged = mergeConfigs(cwd, fileConfig, cliConfig); - logger.trace('Worker: Merged config:', config); + logger.trace('Worker: Using pre-loaded config:', config); // Initialize spinner in worker const spinner = new Spinner('Initializing...', cliOptions); From 681e377361767e3a69ec922ae254fcc1ac069128 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Wed, 17 Sep 2025 00:59:58 +0900 Subject: [PATCH 03/15] refactor(shared): improve error handling and cleanup code - Use class names for RepomixError type checking instead of hardcoded strings - Remove unused RepomixError import from fileProcess.ts - Simplify comments in errorHandle.ts and fileProcess.ts - Clean up constructor-based error checking logic --- package.json | 2 +- .../actions/workers/defaultActionWorker.ts | 53 +++++++++---------- src/shared/errorHandle.ts | 39 +++++++++++++- src/shared/processConcurrency.ts | 21 ++++---- tests/cli/actions/defaultAction.test.ts | 4 +- .../defaultAction.tokenCountTree.test.ts | 26 +++++++++ tests/shared/processConcurrency.test.ts | 5 ++ 7 files changed, 108 insertions(+), 42 deletions(-) diff --git a/package.json b/package.json index 59cc7ccc2..463e7ca3a 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "lint-secretlint": "secretlint \"**/*\" --secretlintignore .gitignore", "test": "vitest", "test-coverage": "vitest run --coverage", - "repomix": "node --run build && node --trace-warnings bin/repomix.cjs", + "repomix": "node --run build && node --enable-source-maps --trace-warnings bin/repomix.cjs", "repomix-src": "node --run repomix -- --include 'src,tests'", "repomix-website": "node --run repomix -- --include 'website'", "memory-check": "node --run repomix -- --verbose | grep Memory", diff --git a/src/cli/actions/workers/defaultActionWorker.ts b/src/cli/actions/workers/defaultActionWorker.ts index 2caea1d23..e6bfbf1b0 100644 --- a/src/cli/actions/workers/defaultActionWorker.ts +++ b/src/cli/actions/workers/defaultActionWorker.ts @@ -40,37 +40,36 @@ export default async ({ let packResult: PackResult; try { + if (isStdin) { + // Handle stdin processing + // Validate directory arguments for stdin mode + const firstDir = directories[0] ?? '.'; + if (directories.length > 1 || firstDir !== '.') { + throw new RepomixError( + 'When using --stdin, do not specify directory arguments. File paths will be read from stdin.', + ); + } - if (isStdin) { - // Handle stdin processing - // Validate directory arguments for stdin mode - const firstDir = directories[0] ?? '.'; - if (directories.length > 1 || firstDir !== '.') { - throw new RepomixError( - 'When using --stdin, do not specify directory arguments. File paths will be read from stdin.', - ); - } + const stdinResult = await readFilePathsFromStdin(cwd); - const stdinResult = await readFilePathsFromStdin(cwd); + // Use pack with predefined files from stdin + packResult = await pack( + [cwd], + config, + (message) => { + spinner.update(message); + }, + {}, + stdinResult.filePaths, + ); + } else { + // Handle directory processing + const targetPaths = directories.map((directory) => path.resolve(cwd, directory)); - // Use pack with predefined files from stdin - packResult = await pack( - [cwd], - config, - (message) => { + packResult = await pack(targetPaths, config, (message) => { spinner.update(message); - }, - {}, - stdinResult.filePaths, - ); - } else { - // Handle directory processing - const targetPaths = directories.map((directory) => path.resolve(cwd, directory)); - - packResult = await pack(targetPaths, config, (message) => { - spinner.update(message); - }); - } + }); + } spinner.succeed('Packing completed successfully!'); diff --git a/src/shared/errorHandle.ts b/src/shared/errorHandle.ts index 7ed3d0f11..8bdd4a566 100644 --- a/src/shared/errorHandle.ts +++ b/src/shared/errorHandle.ts @@ -19,7 +19,7 @@ export class RepomixConfigValidationError extends RepomixError { export const handleError = (error: unknown): void => { logger.log(''); - if (error instanceof RepomixError) { + if (isRepomixError(error)) { logger.error(`✖ ${error.message}`); if (logger.getLogLevel() < repomixLogLevels.DEBUG) { logger.log(''); @@ -31,7 +31,7 @@ export const handleError = (error: unknown): void => { if (error.cause) { logger.debug('Caused by:', error.cause); } - } else if (error instanceof Error) { + } else if (isError(error)) { logger.error(`✖ Unexpected error: ${error.message}`); // If unexpected error, show stack trace by default logger.note('Stack trace:', error.stack); @@ -43,6 +43,7 @@ export const handleError = (error: unknown): void => { } else { // Unknown errors logger.error('✖ An unknown error occurred'); + logger.note('Stack trace:', error); if (logger.getLogLevel() < repomixLogLevels.DEBUG) { logger.log(''); @@ -57,6 +58,40 @@ export const handleError = (error: unknown): void => { logger.info(`• Join our Discord community: ${REPOMIX_DISCORD_URL}`); }; +/** + * Checks if an unknown value is an Error-like object. + * Uses duck typing for errors serialized across worker process boundaries. + */ +const isError = (error: unknown): error is Error => { + if (error instanceof Error) return true; + + if (typeof error !== 'object' || error === null) return false; + + const obj = error as Record; + return ( + typeof obj.message === 'string' && + typeof obj.stack === 'string' && + ('name' in obj ? typeof obj.name === 'string' : true) + ); +}; + +/** + * Checks if an unknown value is a RepomixError-like object. + * Uses error name property for serialized RepomixError across worker boundaries. + */ +const isRepomixError = (error: unknown): error is RepomixError => { + if (error instanceof RepomixError) return true; + + if (typeof error !== 'object' || error === null) return false; + + const obj = error as Record; + return ( + typeof obj.message === 'string' && + 'name' in obj && + (obj.name === RepomixError.name || obj.name === RepomixConfigValidationError.name) + ); +}; + export const rethrowValidationErrorIfZodError = (error: unknown, message: string): void => { if (error instanceof z.ZodError) { const zodErrorText = error.errors.map((err) => `[${err.path.join('.')}] ${err.message}`).join('\n '); diff --git a/src/shared/processConcurrency.ts b/src/shared/processConcurrency.ts index f6bef0a9d..68691af81 100644 --- a/src/shared/processConcurrency.ts +++ b/src/shared/processConcurrency.ts @@ -51,15 +51,18 @@ export const createWorkerPool = (options: WorkerOptions): Tinypool => { workerData: { logLevel: logger.getLogLevel(), }, - env: { - ...process.env, - // Pass log level as environment variable for child_process workers - REPOMIX_LOG_LEVEL: logger.getLogLevel().toString(), - // Ensure color support in child_process workers - FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0'), - // Pass terminal capabilities - TERM: process.env.TERM || 'xterm-256color', - }, + // Only add env for child_process workers + ...(runtime === 'child_process' && { + env: { + ...process.env, + // Pass log level as environment variable for child_process workers + REPOMIX_LOG_LEVEL: logger.getLogLevel().toString(), + // Ensure color support in child_process workers + FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0'), + // Pass terminal capabilities + TERM: process.env.TERM || 'xterm-256color', + }, + }), }); const endTime = process.hrtime.bigint(); diff --git a/tests/cli/actions/defaultAction.test.ts b/tests/cli/actions/defaultAction.test.ts index 7ca88db3f..24c8054fd 100644 --- a/tests/cli/actions/defaultAction.test.ts +++ b/tests/cli/actions/defaultAction.test.ts @@ -151,7 +151,6 @@ describe('defaultAction', () => { expect(taskRunner.run).toHaveBeenCalled(); expect(taskRunner.cleanup).toHaveBeenCalled(); }); - }); it('should handle custom include patterns', async () => { const options: CliOptions = { @@ -199,13 +198,12 @@ describe('defaultAction', () => { run: vi.fn().mockRejectedValue(new Error('Test error')), cleanup: vi.fn().mockResolvedValue(undefined), }; - + vi.mocked(processConcurrency.initTaskRunner).mockReturnValue(failingTaskRunner); const options: CliOptions = {}; await expect(runDefaultAction(['.'], process.cwd(), options)).rejects.toThrow('Test error'); - expect(mockSpinner.fail).toHaveBeenCalledWith('Error during packing'); expect(failingTaskRunner.cleanup).toHaveBeenCalled(); }); diff --git a/tests/cli/actions/defaultAction.tokenCountTree.test.ts b/tests/cli/actions/defaultAction.tokenCountTree.test.ts index bd98104a1..786015c9d 100644 --- a/tests/cli/actions/defaultAction.tokenCountTree.test.ts +++ b/tests/cli/actions/defaultAction.tokenCountTree.test.ts @@ -4,10 +4,12 @@ import * as cliReport from '../../../src/cli/cliReport.js'; import type { CliOptions } from '../../../src/cli/types.js'; import * as configLoad from '../../../src/config/configLoad.js'; import * as packager from '../../../src/core/packager.js'; +import * as processConcurrency from '../../../src/shared/processConcurrency.js'; vi.mock('../../../src/config/configLoad.js'); vi.mock('../../../src/core/packager.js'); vi.mock('../../../src/cli/cliReport.js'); +vi.mock('../../../src/shared/processConcurrency.js'); vi.mock('../../../src/cli/actions/migrationAction.js', () => ({ runMigrationAction: vi.fn(), })); @@ -17,6 +19,7 @@ describe('defaultAction with tokenCountTree', () => { const mockMergeConfigs = configLoad.mergeConfigs as Mock; const mockPack = packager.pack as Mock; const mockReportResults = cliReport.reportResults as Mock; + const mockInitTaskRunner = processConcurrency.initTaskRunner as Mock; beforeEach(() => { vi.clearAllMocks(); @@ -48,6 +51,29 @@ describe('defaultAction with tokenCountTree', () => { ], safeFilePaths: ['/test/file1.js', '/test/file2.js'], }); + + // Mock initTaskRunner to return the config from mockMergeConfigs + mockInitTaskRunner.mockImplementation(() => ({ + run: vi.fn().mockImplementation(async () => ({ + packResult: { + totalFiles: 3, + totalCharacters: 1000, + totalTokens: 200, + fileCharCounts: {}, + fileTokenCounts: {}, + gitDiffTokenCount: 0, + suspiciousFilesResults: [], + suspiciousGitDiffResults: [], + processedFiles: [ + { path: '/test/file1.js', content: 'content1' }, + { path: '/test/file2.js', content: 'content2' }, + ], + safeFilePaths: ['/test/file1.js', '/test/file2.js'], + }, + config: mockMergeConfigs.mock.results[mockMergeConfigs.mock.results.length - 1]?.value || {}, + })), + cleanup: vi.fn().mockResolvedValue(undefined), + })); }); test('should display token count tree when --token-count-tree option is provided', async () => { diff --git a/tests/shared/processConcurrency.test.ts b/tests/shared/processConcurrency.test.ts index 2192d8d02..983da939d 100644 --- a/tests/shared/processConcurrency.test.ts +++ b/tests/shared/processConcurrency.test.ts @@ -85,6 +85,11 @@ describe('processConcurrency', () => { workerData: { logLevel: 2, }, + env: expect.objectContaining({ + REPOMIX_LOG_LEVEL: '2', + FORCE_COLOR: expect.any(String), + TERM: expect.any(String), + }), }); expect(tinypool).toBeDefined(); }); From 78b25b86e72d6c2fa33509d1cbf276e9a491c2ef Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Thu, 18 Sep 2025 23:51:23 +0900 Subject: [PATCH 04/15] feat(core): use direct globby import instead of worker isolation Replace executeGlobbyInWorker with direct globby calls since worker isolation is no longer necessary for globby execution. - Remove src/core/file/globbyExecute.ts wrapper - Remove src/core/file/workers/globbyWorker.ts - Update fileSearch.ts to import and use globby directly - Update tests to mock globby instead of executeGlobbyInWorker - Simplify integration tests by removing worker mocks --- src/core/file/fileSearch.ts | 7 ++-- src/core/file/globbyExecute.ts | 43 ------------------------ src/core/file/workers/globbyWorker.ts | 15 --------- tests/core/file/fileSearch.test.ts | 39 +++++++++++---------- tests/integration-tests/packager.test.ts | 12 ++----- 5 files changed, 25 insertions(+), 91 deletions(-) delete mode 100644 src/core/file/globbyExecute.ts delete mode 100644 src/core/file/workers/globbyWorker.ts diff --git a/src/core/file/fileSearch.ts b/src/core/file/fileSearch.ts index 563208043..af5faeae6 100644 --- a/src/core/file/fileSearch.ts +++ b/src/core/file/fileSearch.ts @@ -1,13 +1,14 @@ import type { Stats } from 'node:fs'; import fs from 'node:fs/promises'; import path from 'node:path'; +import { globby } from 'globby'; import { minimatch } from 'minimatch'; import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { defaultIgnoreList } from '../../config/defaultIgnore.js'; import { RepomixError } from '../../shared/errorHandle.js'; import { logger } from '../../shared/logger.js'; import { sortPaths } from './filePathSort.js'; -import { executeGlobbyInWorker } from './globbyExecute.js'; + import { PermissionError, checkDirectoryPermissions } from './permissionCheck.js'; export interface FileSearchResult { @@ -191,7 +192,7 @@ export const searchFiles = async ( logger.trace('Include patterns with explicit files:', includePatterns); - const filePaths = await executeGlobbyInWorker(includePatterns, { + const filePaths = await globby(includePatterns, { cwd: rootDir, ignore: [...adjustedIgnorePatterns], ignoreFiles: [...ignoreFilePatterns], @@ -212,7 +213,7 @@ export const searchFiles = async ( let emptyDirPaths: string[] = []; if (config.output.includeEmptyDirectories) { - const directories = await executeGlobbyInWorker(includePatterns, { + const directories = await globby(includePatterns, { cwd: rootDir, ignore: [...adjustedIgnorePatterns], ignoreFiles: [...ignoreFilePatterns], diff --git a/src/core/file/globbyExecute.ts b/src/core/file/globbyExecute.ts deleted file mode 100644 index 6b7d9a53d..000000000 --- a/src/core/file/globbyExecute.ts +++ /dev/null @@ -1,43 +0,0 @@ -import type { Options } from 'globby'; -import { logger } from '../../shared/logger.js'; -import { initTaskRunner } from '../../shared/processConcurrency.js'; -import type { GlobbyTask } from './workers/globbyWorker.js'; - -/** - * Execute globby in worker to isolate memory usage - */ -export const executeGlobbyInWorker = async ( - patterns: string[], - options: Options, - deps = { - initTaskRunner, - }, -): Promise => { - const taskRunner = deps.initTaskRunner({ - numOfTasks: 1, - workerPath: new URL('./workers/globbyWorker.js', import.meta.url).href, - runtime: 'worker_threads', - }); - - try { - logger.trace('Starting globby in worker for memory isolation'); - const startTime = process.hrtime.bigint(); - - const result = await taskRunner.run({ - patterns, - options, - }); - - const endTime = process.hrtime.bigint(); - const duration = Number(endTime - startTime) / 1e6; - logger.trace(`Globby completed in worker in ${duration.toFixed(2)}ms`); - - return result; - } catch (error) { - logger.error('Error during globby execution:', error); - throw error; - } finally { - // Always cleanup worker pool - await taskRunner.cleanup(); - } -}; diff --git a/src/core/file/workers/globbyWorker.ts b/src/core/file/workers/globbyWorker.ts deleted file mode 100644 index cbcb6b685..000000000 --- a/src/core/file/workers/globbyWorker.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { type Options, globby } from 'globby'; - -export interface GlobbyTask { - patterns: string[]; - options: Options; -} - -export default async ({ patterns, options }: GlobbyTask): Promise => { - return globby(patterns, options); -}; - -// Export cleanup function for Tinypool teardown (no cleanup needed for this worker) -export const onWorkerTermination = () => { - // No cleanup needed for globby worker -}; diff --git a/tests/core/file/fileSearch.test.ts b/tests/core/file/fileSearch.test.ts index 128faadea..df6328e8d 100644 --- a/tests/core/file/fileSearch.test.ts +++ b/tests/core/file/fileSearch.test.ts @@ -2,6 +2,7 @@ import type { Stats } from 'node:fs'; import * as fs from 'node:fs/promises'; import path from 'node:path'; import process from 'node:process'; +import { globby } from 'globby'; import { minimatch } from 'minimatch'; import { beforeEach, describe, expect, test, vi } from 'vitest'; import { @@ -16,13 +17,11 @@ import { PermissionError } from '../../../src/core/file/permissionCheck.js'; import { RepomixError } from '../../../src/shared/errorHandle.js'; import { createMockConfig, isWindows } from '../../testing/testUtils.js'; -import { executeGlobbyInWorker } from '../../../src/core/file/globbyExecute.js'; import { checkDirectoryPermissions } from '../../../src/core/file/permissionCheck.js'; vi.mock('fs/promises'); -vi.mock('globby'); -vi.mock('../../../src/core/file/globbyExecute.js', () => ({ - executeGlobbyInWorker: vi.fn(), +vi.mock('globby', () => ({ + globby: vi.fn(), })); vi.mock('../../../src/core/file/permissionCheck.js', () => ({ checkDirectoryPermissions: vi.fn(), @@ -51,8 +50,8 @@ describe('fileSearch', () => { hasAllPermission: true, details: { read: true, write: true, execute: true }, }); - // Default mock for executeGlobbyInWorker - vi.mocked(executeGlobbyInWorker).mockResolvedValue([]); + // Default mock for globby + vi.mocked(globby).mockResolvedValue([]); }); describe('getIgnoreFilePaths', () => { @@ -92,7 +91,7 @@ describe('fileSearch', () => { const mockFilePaths = ['src/file1.js', 'src/file2.js']; const mockEmptyDirs = ['src/empty', 'empty-root']; - vi.mocked(executeGlobbyInWorker).mockImplementation(async (_, options) => { + vi.mocked(globby).mockImplementation(async (_, options) => { if (options?.onlyDirectories) { return mockEmptyDirs; } @@ -116,7 +115,7 @@ describe('fileSearch', () => { const mockFilePaths = ['src/file1.js', 'src/file2.js']; - vi.mocked(executeGlobbyInWorker).mockImplementation(async (_, options) => { + vi.mocked(globby).mockImplementation(async (_, options) => { if (options?.onlyDirectories) { throw new Error('Should not search for directories when disabled'); } @@ -127,7 +126,7 @@ describe('fileSearch', () => { expect(result.filePaths).toEqual(mockFilePaths); expect(result.emptyDirPaths).toEqual([]); - expect(executeGlobbyInWorker).toHaveBeenCalledTimes(1); + expect(globby).toHaveBeenCalledTimes(1); }); }); @@ -257,12 +256,12 @@ node_modules }, }); - vi.mocked(executeGlobbyInWorker).mockResolvedValue(['file1.js', 'file2.js']); + vi.mocked(globby).mockResolvedValue(['file1.js', 'file2.js']); vi.mocked(fs.access).mockResolvedValue(undefined); await searchFiles('/mock/root', mockConfig); - expect(executeGlobbyInWorker).toHaveBeenCalledWith( + expect(globby).toHaveBeenCalledWith( ['**/*.js'], expect.objectContaining({ cwd: '/mock/root', @@ -298,7 +297,7 @@ node_modules '/mock/root/subdir/.gitignore': 'ignored.js', }; - vi.mocked(executeGlobbyInWorker).mockImplementation(async () => { + vi.mocked(globby).mockImplementation(async () => { // Simulate filtering files based on .gitignore return mockFileStructure.filter((file) => { const relativePath = file.replace('root/', ''); @@ -339,7 +338,7 @@ node_modules 'root/subdir/ignored.js', ]; - vi.mocked(executeGlobbyInWorker).mockResolvedValue(mockFileStructure); + vi.mocked(globby).mockResolvedValue(mockFileStructure); const result = await searchFiles('/mock/root', mockConfig); @@ -371,7 +370,7 @@ node_modules }); // Mock globby to return some test files - vi.mocked(executeGlobbyInWorker).mockResolvedValue(['file1.js', 'file2.js']); + vi.mocked(globby).mockResolvedValue(['file1.js', 'file2.js']); const mockConfig = createMockConfig({ ignore: { @@ -384,7 +383,7 @@ node_modules const result = await searchFiles('/test/dir', mockConfig); // Check that globby was called with correct ignore patterns - const executeGlobbyCall = vi.mocked(executeGlobbyInWorker).mock.calls[0]; + const executeGlobbyCall = vi.mocked(globby).mock.calls[0]; const ignorePatterns = executeGlobbyCall[1]?.ignore as string[]; // Verify .git file (not directory) is in ignore patterns @@ -415,7 +414,7 @@ node_modules }); // Mock globby to return some test files - vi.mocked(executeGlobbyInWorker).mockResolvedValue(['file1.js', 'file2.js']); + vi.mocked(globby).mockResolvedValue(['file1.js', 'file2.js']); const mockConfig = createMockConfig({ ignore: { @@ -428,7 +427,7 @@ node_modules const result = await searchFiles('/test/dir', mockConfig); // Check that globby was called with correct ignore patterns - const executeGlobbyCall = vi.mocked(executeGlobbyInWorker).mock.calls[0]; + const executeGlobbyCall = vi.mocked(globby).mock.calls[0]; const ignorePatterns = executeGlobbyCall[1]?.ignore as string[]; // Verify .git/** is in ignore patterns for regular git repos @@ -558,7 +557,7 @@ node_modules }); test('should succeed when target path is a valid directory', async () => { - vi.mocked(executeGlobbyInWorker).mockResolvedValue(['test.js']); + vi.mocked(globby).mockResolvedValue(['test.js']); const mockConfig = createMockConfig(); @@ -586,7 +585,7 @@ node_modules ]; // Mock globby to return the expected filtered files - vi.mocked(executeGlobbyInWorker).mockResolvedValue(['src/file1.ts', 'src/file3.ts']); + vi.mocked(globby).mockResolvedValue(['src/file1.ts', 'src/file3.ts']); const result = await searchFiles('/test', mockConfig, explicitFiles); @@ -607,7 +606,7 @@ node_modules const explicitFiles = ['/test/src/main.ts', '/test/tests/unit.test.ts', '/test/lib/utils.ts']; // Mock globby to return the expected filtered files - vi.mocked(executeGlobbyInWorker).mockResolvedValue(['src/main.ts', 'lib/utils.ts']); + vi.mocked(globby).mockResolvedValue(['src/main.ts', 'lib/utils.ts']); const result = await searchFiles('/test', mockConfig, explicitFiles); diff --git a/tests/integration-tests/packager.test.ts b/tests/integration-tests/packager.test.ts index f08e5b120..ad4fbb490 100644 --- a/tests/integration-tests/packager.test.ts +++ b/tests/integration-tests/packager.test.ts @@ -5,15 +5,13 @@ import process from 'node:process'; import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; // Mock globby worker for integration tests to avoid worker file loading issues -vi.mock('../../src/core/file/globbyExecute.js', () => ({ - executeGlobbyInWorker: vi.fn(), -})); + import { loadFileConfig, mergeConfigs } from '../../src/config/configLoad.js'; import type { RepomixConfigFile, RepomixConfigMerged, RepomixOutputStyle } from '../../src/config/configSchema.js'; import { collectFiles } from '../../src/core/file/fileCollect.js'; import { searchFiles } from '../../src/core/file/fileSearch.js'; import type { ProcessedFile } from '../../src/core/file/fileTypes.js'; -import { executeGlobbyInWorker } from '../../src/core/file/globbyExecute.js'; + import type { FileCollectTask } from '../../src/core/file/workers/fileCollectWorker.js'; import fileCollectWorker from '../../src/core/file/workers/fileCollectWorker.js'; import fileProcessWorker from '../../src/core/file/workers/fileProcessWorker.js'; @@ -83,12 +81,6 @@ describe.runIf(!isWindows)('packager integration', () => { beforeEach(async () => { // Create a temporary directory for each test tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'repomix-test-')); - - // Mock executeGlobbyInWorker to return the actual files in the test directory - vi.mocked(executeGlobbyInWorker).mockImplementation(async (patterns, options) => { - const { globby } = await import('globby'); - return globby(patterns, options); - }); }); afterEach(async () => { From 76dcbceaf79fce571ef2dca7063ce8ec86adf108 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Fri, 19 Sep 2025 00:26:39 +0900 Subject: [PATCH 05/15] refactor(metrics): unify worker system with single TaskRunner lifecycle Previously, each metrics calculation function (calculateSelectiveFileMetrics, calculateOutputMetrics, calculateGitDiffMetrics, calculateGitLogMetrics) was creating its own TaskRunner instance, leading to redundant worker pool initialization overhead. This change consolidates all metrics workers into a single unified worker (unifiedMetricsWorker) that handles all metric types through a discriminated union pattern. The calculateMetrics function now creates one TaskRunner instance and passes it to all calculation functions, with cleanup performed once at the end. Benefits: - Reduces worker pool initialization overhead - Improves resource efficiency by reusing single worker pool - Maintains consistent worker infrastructure across all metrics - Simplifies lifecycle management with centralized cleanup --- src/core/metrics/calculateGitDiffMetrics.ts | 23 ++-- src/core/metrics/calculateGitLogMetrics.ts | 22 ++-- src/core/metrics/calculateMetrics.ts | 106 +++++++++------- src/core/metrics/calculateOutputMetrics.ts | 37 +++--- .../metrics/calculateSelectiveFileMetrics.ts | 36 +++--- src/core/metrics/workers/fileMetricsWorker.ts | 43 ------- .../metrics/workers/gitDiffMetricsWorker.ts | 42 ------- .../metrics/workers/gitLogMetricsWorker.ts | 39 ------ .../metrics/workers/outputMetricsWorker.ts | 31 ----- .../metrics/workers/unifiedMetricsWorker.ts | 116 ++++++++++++++++++ tests/core/metrics/calculateMetrics.test.ts | 3 + .../metrics/calculateOutputMetrics.test.ts | 24 ++-- .../calculateSelectiveFileMetrics.test.ts | 11 +- 13 files changed, 244 insertions(+), 289 deletions(-) delete mode 100644 src/core/metrics/workers/fileMetricsWorker.ts delete mode 100644 src/core/metrics/workers/gitDiffMetricsWorker.ts delete mode 100644 src/core/metrics/workers/gitLogMetricsWorker.ts delete mode 100644 src/core/metrics/workers/outputMetricsWorker.ts create mode 100644 src/core/metrics/workers/unifiedMetricsWorker.ts diff --git a/src/core/metrics/calculateGitDiffMetrics.ts b/src/core/metrics/calculateGitDiffMetrics.ts index 64e461b93..5cd5058c8 100644 --- a/src/core/metrics/calculateGitDiffMetrics.ts +++ b/src/core/metrics/calculateGitDiffMetrics.ts @@ -1,8 +1,9 @@ import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { logger } from '../../shared/logger.js'; -import { initTaskRunner } from '../../shared/processConcurrency.js'; +import type { TaskRunner } from '../../shared/processConcurrency.js'; import type { GitDiffResult } from '../git/gitDiffHandle.js'; -import type { GitDiffMetricsTask } from './workers/gitDiffMetricsWorker.js'; +import type { FileMetrics } from './workers/types.js'; +import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; /** * Calculate token count for git diffs if included @@ -10,9 +11,7 @@ import type { GitDiffMetricsTask } from './workers/gitDiffMetricsWorker.js'; export const calculateGitDiffMetrics = async ( config: RepomixConfigMerged, gitDiffResult: GitDiffResult | undefined, - deps = { - initTaskRunner, - }, + deps: { taskRunner: TaskRunner }, ): Promise => { if (!config.output.git?.includeDiffs || !gitDiffResult) { return 0; @@ -23,21 +22,16 @@ export const calculateGitDiffMetrics = async ( return 0; } - const taskRunner = deps.initTaskRunner({ - numOfTasks: 1, // Single task for git diff calculation - workerPath: new URL('./workers/gitDiffMetricsWorker.js', import.meta.url).href, - runtime: 'worker_threads', - }); - try { const startTime = process.hrtime.bigint(); logger.trace('Starting git diff token calculation using worker'); - const result = await taskRunner.run({ + const result = (await deps.taskRunner.run({ + type: 'gitDiff', workTreeDiffContent: gitDiffResult.workTreeDiffContent, stagedDiffContent: gitDiffResult.stagedDiffContent, encoding: config.tokenCount.encoding, - }); + })) as number; const endTime = process.hrtime.bigint(); const duration = Number(endTime - startTime) / 1e6; @@ -47,8 +41,5 @@ export const calculateGitDiffMetrics = async ( } catch (error) { logger.error('Error during git diff token calculation:', error); throw error; - } finally { - // Always cleanup worker pool - await taskRunner.cleanup(); } }; diff --git a/src/core/metrics/calculateGitLogMetrics.ts b/src/core/metrics/calculateGitLogMetrics.ts index 5949d312d..8eab8b968 100644 --- a/src/core/metrics/calculateGitLogMetrics.ts +++ b/src/core/metrics/calculateGitLogMetrics.ts @@ -1,8 +1,9 @@ import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { logger } from '../../shared/logger.js'; -import { initTaskRunner } from '../../shared/processConcurrency.js'; +import type { TaskRunner } from '../../shared/processConcurrency.js'; import type { GitLogResult } from '../git/gitLogHandle.js'; -import type { GitLogMetricsTask } from './workers/gitLogMetricsWorker.js'; +import type { FileMetrics } from './workers/types.js'; +import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; /** * Calculate token count for git logs if included @@ -10,9 +11,7 @@ import type { GitLogMetricsTask } from './workers/gitLogMetricsWorker.js'; export const calculateGitLogMetrics = async ( config: RepomixConfigMerged, gitLogResult: GitLogResult | undefined, - deps = { - initTaskRunner, - }, + deps: { taskRunner: TaskRunner }, ): Promise<{ gitLogTokenCount: number }> => { // Return zero token count if git logs are disabled or no result if (!config.output.git?.includeLogs || !gitLogResult) { @@ -28,20 +27,15 @@ export const calculateGitLogMetrics = async ( }; } - const taskRunner = deps.initTaskRunner({ - numOfTasks: 1, // Single task for git log calculation - workerPath: new URL('./workers/gitLogMetricsWorker.js', import.meta.url).href, - runtime: 'worker_threads', - }); - try { const startTime = process.hrtime.bigint(); logger.trace('Starting git log token calculation using worker'); - const result = await taskRunner.run({ + const result = (await deps.taskRunner.run({ + type: 'gitLog', content: gitLogResult.logContent, encoding: config.tokenCount.encoding, - }); + })) as number; const endTime = process.hrtime.bigint(); const duration = Number(endTime - startTime) / 1e6; @@ -55,7 +49,5 @@ export const calculateGitLogMetrics = async ( return { gitLogTokenCount: 0, }; - } finally { - await taskRunner.cleanup(); } }; diff --git a/src/core/metrics/calculateMetrics.ts b/src/core/metrics/calculateMetrics.ts index 362289035..576a48b18 100644 --- a/src/core/metrics/calculateMetrics.ts +++ b/src/core/metrics/calculateMetrics.ts @@ -1,4 +1,5 @@ import type { RepomixConfigMerged } from '../../config/configSchema.js'; +import { initTaskRunner } from '../../shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../shared/types.js'; import type { ProcessedFile } from '../file/fileTypes.js'; import type { GitDiffResult } from '../git/gitDiffHandle.js'; @@ -7,6 +8,8 @@ import { calculateGitDiffMetrics } from './calculateGitDiffMetrics.js'; import { calculateGitLogMetrics } from './calculateGitLogMetrics.js'; import { calculateOutputMetrics } from './calculateOutputMetrics.js'; import { calculateSelectiveFileMetrics } from './calculateSelectiveFileMetrics.js'; +import type { FileMetrics } from './workers/types.js'; +import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; export interface CalculateMetricsResult { totalFiles: number; @@ -34,55 +37,68 @@ export const calculateMetrics = async ( ): Promise => { progressCallback('Calculating metrics...'); - // For top files display optimization: calculate token counts only for top files by character count - // However, if tokenCountTree is enabled, calculate for all files to avoid double calculation - const topFilesLength = config.output.topFilesLength; - const shouldCalculateAllFiles = !!config.output.tokenCountTree; + // Initialize a single task runner for all metrics calculations + const taskRunner = initTaskRunner({ + numOfTasks: processedFiles.length, + workerPath: new URL('../../../lib/core/metrics/workers/unifiedMetricsWorker.js', import.meta.url).href, + runtime: 'worker_threads', + }); - // Determine which files to calculate token counts for: - // - If tokenCountTree is enabled: calculate for all files to avoid double calculation - // - Otherwise: calculate only for top files by character count for optimization - const metricsTargetPaths = shouldCalculateAllFiles - ? processedFiles.map((file) => file.path) - : [...processedFiles] - .sort((a, b) => b.content.length - a.content.length) - .slice(0, Math.min(processedFiles.length, Math.max(topFilesLength * 10, topFilesLength))) - .map((file) => file.path); + try { + // For top files display optimization: calculate token counts only for top files by character count + // However, if tokenCountTree is enabled, calculate for all files to avoid double calculation + const topFilesLength = config.output.topFilesLength; + const shouldCalculateAllFiles = !!config.output.tokenCountTree; - const [selectiveFileMetrics, totalTokens, gitDiffTokenCount, gitLogTokenCount] = await Promise.all([ - deps.calculateSelectiveFileMetrics( - processedFiles, - metricsTargetPaths, - config.tokenCount.encoding, - progressCallback, - ), - deps.calculateOutputMetrics(output, config.tokenCount.encoding, config.output.filePath), - deps.calculateGitDiffMetrics(config, gitDiffResult), - deps.calculateGitLogMetrics(config, gitLogResult), - ]); + // Determine which files to calculate token counts for: + // - If tokenCountTree is enabled: calculate for all files to avoid double calculation + // - Otherwise: calculate only for top files by character count for optimization + const metricsTargetPaths = shouldCalculateAllFiles + ? processedFiles.map((file) => file.path) + : [...processedFiles] + .sort((a, b) => b.content.length - a.content.length) + .slice(0, Math.min(processedFiles.length, Math.max(topFilesLength * 10, topFilesLength))) + .map((file) => file.path); - const totalFiles = processedFiles.length; - const totalCharacters = output.length; + const [selectiveFileMetrics, totalTokens, gitDiffTokenCount, gitLogTokenCount] = await Promise.all([ + deps.calculateSelectiveFileMetrics( + processedFiles, + metricsTargetPaths, + config.tokenCount.encoding, + progressCallback, + { taskRunner }, + ), + deps.calculateOutputMetrics(output, config.tokenCount.encoding, config.output.filePath, { taskRunner }), + deps.calculateGitDiffMetrics(config, gitDiffResult, { taskRunner }), + deps.calculateGitLogMetrics(config, gitLogResult, { taskRunner }), + ]); - // Build character counts for all files - const fileCharCounts: Record = {}; - for (const file of processedFiles) { - fileCharCounts[file.path] = file.content.length; - } + const totalFiles = processedFiles.length; + const totalCharacters = output.length; - // Build token counts only for top files - const fileTokenCounts: Record = {}; - for (const file of selectiveFileMetrics) { - fileTokenCounts[file.path] = file.tokenCount; - } + // Build character counts for all files + const fileCharCounts: Record = {}; + for (const file of processedFiles) { + fileCharCounts[file.path] = file.content.length; + } - return { - totalFiles, - totalCharacters, - totalTokens, - fileCharCounts, - fileTokenCounts, - gitDiffTokenCount: gitDiffTokenCount, - gitLogTokenCount: gitLogTokenCount.gitLogTokenCount, - }; + // Build token counts only for top files + const fileTokenCounts: Record = {}; + for (const file of selectiveFileMetrics) { + fileTokenCounts[file.path] = file.tokenCount; + } + + return { + totalFiles, + totalCharacters, + totalTokens, + fileCharCounts, + fileTokenCounts, + gitDiffTokenCount: gitDiffTokenCount, + gitLogTokenCount: gitLogTokenCount.gitLogTokenCount, + }; + } finally { + // Cleanup the task runner after all calculations are complete + await taskRunner.cleanup(); + } }; diff --git a/src/core/metrics/calculateOutputMetrics.ts b/src/core/metrics/calculateOutputMetrics.ts index db3d27e66..5e148a37f 100644 --- a/src/core/metrics/calculateOutputMetrics.ts +++ b/src/core/metrics/calculateOutputMetrics.ts @@ -1,7 +1,8 @@ import type { TiktokenEncoding } from 'tiktoken'; import { logger } from '../../shared/logger.js'; -import { initTaskRunner } from '../../shared/processConcurrency.js'; -import type { OutputMetricsTask } from './workers/outputMetricsWorker.js'; +import type { TaskRunner } from '../../shared/processConcurrency.js'; +import type { FileMetrics } from './workers/types.js'; +import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; const CHUNK_SIZE = 1000; const MIN_CONTENT_LENGTH_FOR_PARALLEL = 1_000_000; // 1000KB @@ -9,18 +10,10 @@ const MIN_CONTENT_LENGTH_FOR_PARALLEL = 1_000_000; // 1000KB export const calculateOutputMetrics = async ( content: string, encoding: TiktokenEncoding, - path?: string, - deps = { - initTaskRunner, - }, + path: string | undefined, + deps: { taskRunner: TaskRunner }, ): Promise => { const shouldRunInParallel = content.length > MIN_CONTENT_LENGTH_FOR_PARALLEL; - const numOfTasks = shouldRunInParallel ? CHUNK_SIZE : 1; - const taskRunner = deps.initTaskRunner({ - numOfTasks, - workerPath: new URL('./workers/outputMetricsWorker.js', import.meta.url).href, - runtime: 'worker_threads', - }); try { logger.trace(`Starting output token count for ${path || 'output'}`); @@ -39,20 +32,27 @@ export const calculateOutputMetrics = async ( // Process chunks in parallel const chunkResults = await Promise.all( - chunks.map((chunk, index) => - taskRunner.run({ + chunks.map(async (chunk, index) => { + const result = await deps.taskRunner.run({ + type: 'output', content: chunk, encoding, path: path ? `${path}-chunk-${index}` : undefined, - }), - ), + }); + return result as number; // Output tasks always return numbers + }), ); // Sum up the results result = chunkResults.reduce((sum, count) => sum + count, 0); } else { // Process small content directly - result = await taskRunner.run({ content, encoding, path }); + result = (await deps.taskRunner.run({ + type: 'output', + content, + encoding, + path, + })) as number; } const endTime = process.hrtime.bigint(); @@ -63,8 +63,5 @@ export const calculateOutputMetrics = async ( } catch (error) { logger.error('Error during token count:', error); throw error; - } finally { - // Always cleanup worker pool - await taskRunner.cleanup(); } }; diff --git a/src/core/metrics/calculateSelectiveFileMetrics.ts b/src/core/metrics/calculateSelectiveFileMetrics.ts index e89f87fe7..4511541ab 100644 --- a/src/core/metrics/calculateSelectiveFileMetrics.ts +++ b/src/core/metrics/calculateSelectiveFileMetrics.ts @@ -1,20 +1,18 @@ import pc from 'picocolors'; import type { TiktokenEncoding } from 'tiktoken'; import { logger } from '../../shared/logger.js'; -import { initTaskRunner } from '../../shared/processConcurrency.js'; +import type { TaskRunner } from '../../shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../shared/types.js'; import type { ProcessedFile } from '../file/fileTypes.js'; -import type { FileMetricsTask } from './workers/fileMetricsWorker.js'; import type { FileMetrics } from './workers/types.js'; +import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; export const calculateSelectiveFileMetrics = async ( processedFiles: ProcessedFile[], targetFilePaths: string[], tokenCounterEncoding: TiktokenEncoding, progressCallback: RepomixProgressCallback, - deps = { - initTaskRunner, - }, + deps: { taskRunner: TaskRunner }, ): Promise => { const targetFileSet = new Set(targetFilePaths); const filesToProcess = processedFiles.filter((file) => targetFileSet.has(file.path)); @@ -23,19 +21,15 @@ export const calculateSelectiveFileMetrics = async ( return []; } - const taskRunner = deps.initTaskRunner({ - numOfTasks: filesToProcess.length, - workerPath: new URL('./workers/fileMetricsWorker.js', import.meta.url).href, - runtime: 'worker_threads', - }); const tasks = filesToProcess.map( (file, index) => ({ + type: 'file', file, index, totalFiles: filesToProcess.length, encoding: tokenCounterEncoding, - }) satisfies FileMetricsTask, + }) satisfies UnifiedMetricsTask, ); try { @@ -44,14 +38,15 @@ export const calculateSelectiveFileMetrics = async ( let completedTasks = 0; const results = await Promise.all( - tasks.map((task) => - taskRunner.run(task).then((result) => { - completedTasks++; - progressCallback(`Calculating metrics... (${completedTasks}/${task.totalFiles}) ${pc.dim(task.file.path)}`); - logger.trace(`Calculating metrics... (${completedTasks}/${task.totalFiles}) ${task.file.path}`); - return result; - }), - ), + tasks.map(async (task) => { + const result = (await deps.taskRunner.run(task)) as FileMetrics; + completedTasks++; + progressCallback( + `Calculating metrics... (${completedTasks}/${filesToProcess.length}) ${pc.dim(task.file.path)}`, + ); + logger.trace(`Calculating metrics... (${completedTasks}/${filesToProcess.length}) ${task.file.path}`); + return result; + }), ); const endTime = process.hrtime.bigint(); @@ -62,8 +57,5 @@ export const calculateSelectiveFileMetrics = async ( } catch (error) { logger.error('Error during selective metrics calculation:', error); throw error; - } finally { - // Always cleanup worker pool - await taskRunner.cleanup(); } }; diff --git a/src/core/metrics/workers/fileMetricsWorker.ts b/src/core/metrics/workers/fileMetricsWorker.ts deleted file mode 100644 index 5f5bb86e5..000000000 --- a/src/core/metrics/workers/fileMetricsWorker.ts +++ /dev/null @@ -1,43 +0,0 @@ -import type { TiktokenEncoding } from 'tiktoken'; -import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; -import type { ProcessedFile } from '../../file/fileTypes.js'; -import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; -import type { FileMetrics } from './types.js'; - -// Initialize logger configuration from workerData at module load time -// This must be called before any logging operations in the worker -setLogLevelByWorkerData(); - -export interface FileMetricsTask { - file: ProcessedFile; - index: number; - totalFiles: number; - encoding: TiktokenEncoding; -} - -export default async ({ file, encoding }: FileMetricsTask): Promise => { - const processStartAt = process.hrtime.bigint(); - const metrics = await calculateIndividualFileMetrics(file, encoding); - const processEndAt = process.hrtime.bigint(); - logger.trace( - `Calculated metrics for ${file.path}. Took: ${(Number(processEndAt - processStartAt) / 1e6).toFixed(2)}ms`, - ); - - return metrics; -}; - -export const calculateIndividualFileMetrics = async ( - file: ProcessedFile, - encoding: TiktokenEncoding, -): Promise => { - const charCount = file.content.length; - const tokenCounter = getTokenCounter(encoding); - const tokenCount = tokenCounter.countTokens(file.content, file.path); - - return { path: file.path, charCount, tokenCount }; -}; - -// Export cleanup function for Tinypool teardown -export const onWorkerTermination = () => { - freeTokenCounters(); -}; diff --git a/src/core/metrics/workers/gitDiffMetricsWorker.ts b/src/core/metrics/workers/gitDiffMetricsWorker.ts deleted file mode 100644 index 2aa97be30..000000000 --- a/src/core/metrics/workers/gitDiffMetricsWorker.ts +++ /dev/null @@ -1,42 +0,0 @@ -import type { TiktokenEncoding } from 'tiktoken'; -import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; -import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; - -// Initialize logger configuration from workerData at module load time -// This must be called before any logging operations in the worker -setLogLevelByWorkerData(); - -export interface GitDiffMetricsTask { - workTreeDiffContent?: string; - stagedDiffContent?: string; - encoding: TiktokenEncoding; -} - -export default async ({ workTreeDiffContent, stagedDiffContent, encoding }: GitDiffMetricsTask): Promise => { - const processStartAt = process.hrtime.bigint(); - - const tokenCounter = getTokenCounter(encoding); - - const countPromises = []; - if (workTreeDiffContent) { - countPromises.push(Promise.resolve().then(() => tokenCounter.countTokens(workTreeDiffContent))); - } - if (stagedDiffContent) { - countPromises.push(Promise.resolve().then(() => tokenCounter.countTokens(stagedDiffContent))); - } - - const results = await Promise.all(countPromises); - const totalTokens = results.reduce((sum, count) => sum + count, 0); - - const processEndAt = process.hrtime.bigint(); - logger.trace( - `Calculated git diff metrics. Tokens: ${totalTokens}. Took: ${(Number(processEndAt - processStartAt) / 1e6).toFixed(2)}ms`, - ); - - return totalTokens; -}; - -// Export cleanup function for Tinypool teardown -export const onWorkerTermination = () => { - freeTokenCounters(); -}; diff --git a/src/core/metrics/workers/gitLogMetricsWorker.ts b/src/core/metrics/workers/gitLogMetricsWorker.ts deleted file mode 100644 index aaa1f6f1e..000000000 --- a/src/core/metrics/workers/gitLogMetricsWorker.ts +++ /dev/null @@ -1,39 +0,0 @@ -import type { TiktokenEncoding } from 'tiktoken'; -import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; -import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; - -// Initialize logger configuration from workerData at module load time -// This must be called before any logging operations in the worker -setLogLevelByWorkerData(); - -export interface GitLogMetricsTask { - content: string; - encoding: TiktokenEncoding; -} - -export default async ({ content, encoding }: GitLogMetricsTask): Promise => { - const processStartAt = process.hrtime.bigint(); - - try { - if (!content) { - return 0; - } - - const tokenCounter = getTokenCounter(encoding); - const tokenCount = tokenCounter.countTokens(content); - - const processEndAt = process.hrtime.bigint(); - const processDuration = Number(processEndAt - processStartAt) / 1e6; - logger.trace(`Git log token count calculated in ${processDuration.toFixed(2)}ms`); - - return tokenCount; - } catch (error) { - logger.error('Error calculating git log token count:', error); - return 0; - } -}; - -// Export cleanup function for Tinypool teardown -export const onWorkerTermination = () => { - freeTokenCounters(); -}; diff --git a/src/core/metrics/workers/outputMetricsWorker.ts b/src/core/metrics/workers/outputMetricsWorker.ts deleted file mode 100644 index 62ea93a56..000000000 --- a/src/core/metrics/workers/outputMetricsWorker.ts +++ /dev/null @@ -1,31 +0,0 @@ -import type { TiktokenEncoding } from 'tiktoken'; -import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; -import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; - -// Initialize logger configuration from workerData at module load time -// This must be called before any logging operations in the worker -setLogLevelByWorkerData(); - -export interface OutputMetricsTask { - content: string; - encoding: TiktokenEncoding; - path?: string; -} - -export default async ({ content, encoding, path }: OutputMetricsTask): Promise => { - const processStartAt = process.hrtime.bigint(); - const counter = getTokenCounter(encoding); - const tokenCount = counter.countTokens(content, path); - - const processEndAt = process.hrtime.bigint(); - logger.trace( - `Counted output tokens. Count: ${tokenCount}. Took: ${(Number(processEndAt - processStartAt) / 1e6).toFixed(2)}ms`, - ); - - return tokenCount; -}; - -// Export cleanup function for Tinypool teardown -export const onWorkerTermination = () => { - freeTokenCounters(); -}; diff --git a/src/core/metrics/workers/unifiedMetricsWorker.ts b/src/core/metrics/workers/unifiedMetricsWorker.ts new file mode 100644 index 000000000..1d673ba92 --- /dev/null +++ b/src/core/metrics/workers/unifiedMetricsWorker.ts @@ -0,0 +1,116 @@ +import type { TiktokenEncoding } from 'tiktoken'; +import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; +import type { ProcessedFile } from '../../file/fileTypes.js'; +import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; +import type { FileMetrics } from './types.js'; + +// Initialize logger configuration from workerData at module load time +// This must be called before any logging operations in the worker +setLogLevelByWorkerData(); + +export type MetricsTaskType = 'output' | 'file' | 'gitDiff' | 'gitLog'; + +export interface OutputMetricsTask { + type: 'output'; + content: string; + encoding: TiktokenEncoding; + path?: string; +} + +export interface FileMetricsTask { + type: 'file'; + file: ProcessedFile; + index: number; + totalFiles: number; + encoding: TiktokenEncoding; +} + +export interface GitDiffMetricsTask { + type: 'gitDiff'; + workTreeDiffContent?: string; + stagedDiffContent?: string; + encoding: TiktokenEncoding; +} + +export interface GitLogMetricsTask { + type: 'gitLog'; + content: string; + encoding: TiktokenEncoding; +} + +export type UnifiedMetricsTask = OutputMetricsTask | FileMetricsTask | GitDiffMetricsTask | GitLogMetricsTask; + +export default async (task: UnifiedMetricsTask): Promise => { + const processStartAt = process.hrtime.bigint(); + + try { + switch (task.type) { + case 'output': { + const counter = getTokenCounter(task.encoding); + const tokenCount = counter.countTokens(task.content, task.path); + logger.trace(`Counted output tokens. Count: ${tokenCount}. Took: ${getProcessDuration(processStartAt)}ms`); + return tokenCount; + } + + case 'file': { + const charCount = task.file.content.length; + const tokenCounter = getTokenCounter(task.encoding); + const tokenCount = tokenCounter.countTokens(task.file.content, task.file.path); + const result: FileMetrics = { path: task.file.path, charCount, tokenCount }; + logger.trace(`Calculated metrics for ${task.file.path}. Took: ${getProcessDuration(processStartAt)}ms`); + return result; + } + + case 'gitDiff': { + const tokenCounter = getTokenCounter(task.encoding); + const countPromises = []; + + if (task.workTreeDiffContent) { + const content = task.workTreeDiffContent; + countPromises.push(Promise.resolve().then(() => tokenCounter.countTokens(content))); + } + if (task.stagedDiffContent) { + const content = task.stagedDiffContent; + countPromises.push(Promise.resolve().then(() => tokenCounter.countTokens(content))); + } + + const results = await Promise.all(countPromises); + const totalTokens = results.reduce((sum, count) => sum + count, 0); + logger.trace( + `Calculated git diff metrics. Tokens: ${totalTokens}. Took: ${getProcessDuration(processStartAt)}ms`, + ); + return totalTokens; + } + + case 'gitLog': { + if (!task.content) { + return 0; + } + + const tokenCounter = getTokenCounter(task.encoding); + const tokenCount = tokenCounter.countTokens(task.content); + logger.trace(`Git log token count calculated in ${getProcessDuration(processStartAt)}ms`); + return tokenCount; + } + + default: + throw new Error(`Unknown task type: ${(task as { type?: string }).type || 'unknown'}`); + } + } catch (error) { + logger.error(`Error in unified metrics worker for task type ${task.type}:`, error); + if (task.type === 'gitLog') { + return 0; // gitLog worker returns 0 on error + } + throw error; + } +}; + +const getProcessDuration = (startTime: bigint): string => { + const endTime = process.hrtime.bigint(); + return (Number(endTime - startTime) / 1e6).toFixed(2); +}; + +// Export cleanup function for Tinypool teardown +export const onWorkerTermination = () => { + freeTokenCounters(); +}; diff --git a/tests/core/metrics/calculateMetrics.test.ts b/tests/core/metrics/calculateMetrics.test.ts index 63cfc7518..743445777 100644 --- a/tests/core/metrics/calculateMetrics.test.ts +++ b/tests/core/metrics/calculateMetrics.test.ts @@ -67,6 +67,9 @@ describe('calculateMetrics', () => { ['file2.txt', 'file1.txt'], // sorted by character count desc 'o200k_base', progressCallback, + expect.objectContaining({ + taskRunner: expect.any(Object), + }), ); expect(result).toEqual(aggregatedResult); }); diff --git a/tests/core/metrics/calculateOutputMetrics.test.ts b/tests/core/metrics/calculateOutputMetrics.test.ts index aec40554d..d24014860 100644 --- a/tests/core/metrics/calculateOutputMetrics.test.ts +++ b/tests/core/metrics/calculateOutputMetrics.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it, vi } from 'vitest'; import { calculateOutputMetrics } from '../../../src/core/metrics/calculateOutputMetrics.js'; -import type { OutputMetricsTask } from '../../../src/core/metrics/workers/outputMetricsWorker.js'; -import outputMetricsWorker from '../../../src/core/metrics/workers/outputMetricsWorker.js'; +import unifiedMetricsWorker, { + type OutputMetricsTask, + type UnifiedMetricsTask, +} from '../../../src/core/metrics/workers/unifiedMetricsWorker.js'; import { logger } from '../../../src/shared/logger.js'; import type { WorkerOptions } from '../../../src/shared/processConcurrency.js'; @@ -10,7 +12,7 @@ vi.mock('../../../src/shared/logger'); const mockInitTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - return (await outputMetricsWorker(task as OutputMetricsTask)) as R; + return (await unifiedMetricsWorker(task as UnifiedMetricsTask)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests @@ -25,7 +27,7 @@ describe('calculateOutputMetrics', () => { const path = 'test.txt'; const result = await calculateOutputMetrics(content, encoding, path, { - initTaskRunner: mockInitTaskRunner, + taskRunner: mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }); expect(result).toBe(2); // 'test content' should be counted as 2 tokens @@ -36,7 +38,7 @@ describe('calculateOutputMetrics', () => { const encoding = 'o200k_base'; const result = await calculateOutputMetrics(content, encoding, undefined, { - initTaskRunner: mockInitTaskRunner, + taskRunner: mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }); expect(result).toBe(2); @@ -60,7 +62,7 @@ describe('calculateOutputMetrics', () => { await expect( calculateOutputMetrics(content, encoding, undefined, { - initTaskRunner: mockErrorTaskRunner, + taskRunner: mockErrorTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }), ).rejects.toThrow('Worker error'); @@ -72,7 +74,7 @@ describe('calculateOutputMetrics', () => { const encoding = 'o200k_base'; const result = await calculateOutputMetrics(content, encoding, undefined, { - initTaskRunner: mockInitTaskRunner, + taskRunner: mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }); expect(result).toBe(0); @@ -83,7 +85,7 @@ describe('calculateOutputMetrics', () => { const encoding = 'o200k_base'; const result = await calculateOutputMetrics(content, encoding, undefined, { - initTaskRunner: mockInitTaskRunner, + taskRunner: mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }); expect(result).toBeGreaterThan(0); @@ -111,7 +113,7 @@ describe('calculateOutputMetrics', () => { }; const result = await calculateOutputMetrics(content, encoding, path, { - initTaskRunner: mockParallelTaskRunner, + taskRunner: mockParallelTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }); expect(chunksProcessed).toBeGreaterThan(1); // Should have processed multiple chunks @@ -136,7 +138,7 @@ describe('calculateOutputMetrics', () => { await expect( calculateOutputMetrics(content, encoding, undefined, { - initTaskRunner: mockErrorTaskRunner, + taskRunner: mockErrorTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }), ).rejects.toThrow('Parallel processing error'); @@ -162,7 +164,7 @@ describe('calculateOutputMetrics', () => { }; await calculateOutputMetrics(content, encoding, undefined, { - initTaskRunner: mockChunkTrackingTaskRunner, + taskRunner: mockChunkTrackingTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }); // Check that chunks are roughly equal in size diff --git a/tests/core/metrics/calculateSelectiveFileMetrics.test.ts b/tests/core/metrics/calculateSelectiveFileMetrics.test.ts index 2e8b06e22..89e00051e 100644 --- a/tests/core/metrics/calculateSelectiveFileMetrics.test.ts +++ b/tests/core/metrics/calculateSelectiveFileMetrics.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it, vi } from 'vitest'; import type { ProcessedFile } from '../../../src/core/file/fileTypes.js'; import { calculateSelectiveFileMetrics } from '../../../src/core/metrics/calculateSelectiveFileMetrics.js'; -import type { FileMetricsTask } from '../../../src/core/metrics/workers/fileMetricsWorker.js'; -import fileMetricsWorker from '../../../src/core/metrics/workers/fileMetricsWorker.js'; +import unifiedMetricsWorker, { + type UnifiedMetricsTask, +} from '../../../src/core/metrics/workers/unifiedMetricsWorker.js'; import type { WorkerOptions } from '../../../src/shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../../src/shared/types.js'; @@ -13,7 +14,7 @@ vi.mock('../../shared/processConcurrency', () => ({ const mockInitTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - return (await fileMetricsWorker(task as FileMetricsTask)) as R; + return (await unifiedMetricsWorker(task as UnifiedMetricsTask)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests @@ -37,7 +38,7 @@ describe('calculateSelectiveFileMetrics', () => { 'o200k_base', progressCallback, { - initTaskRunner: mockInitTaskRunner, + taskRunner: mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }, ); @@ -58,7 +59,7 @@ describe('calculateSelectiveFileMetrics', () => { 'o200k_base', progressCallback, { - initTaskRunner: mockInitTaskRunner, + taskRunner: mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }), }, ); From 8efeaf98c72ab3ab951c97381b18bc67a41fc607 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 14:34:25 +0900 Subject: [PATCH 06/15] feat(cli): improve type safety and Bun compatibility in defaultActionWorker Added function overloads for better type inference when passing PingTask vs DefaultActionTask. Implemented separate PingTask/PingResult types for Bun worker initialization workaround. Added waitForWorkerReady method with retry logic to handle ES module timing issues in Bun. This resolves the "Cannot access 'default' before initialization" error that started occurring after commit 8a88f45 when using child_process workers with Bun runtime. --- src/cli/actions/defaultAction.ts | 56 ++++++++++++++++++- .../actions/workers/defaultActionWorker.ts | 37 +++++++++--- 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/src/cli/actions/defaultAction.ts b/src/cli/actions/defaultAction.ts index 8109ba45c..88d6ca56c 100644 --- a/src/cli/actions/defaultAction.ts +++ b/src/cli/actions/defaultAction.ts @@ -14,7 +14,12 @@ import { initTaskRunner } from '../../shared/processConcurrency.js'; import { reportResults } from '../cliReport.js'; import type { CliOptions } from '../types.js'; import { runMigrationAction } from './migrationAction.js'; -import type { DefaultActionTask, DefaultActionWorkerResult } from './workers/defaultActionWorker.js'; +import type { + DefaultActionTask, + DefaultActionWorkerResult, + PingResult, + PingTask, +} from './workers/defaultActionWorker.js'; export interface DefaultActionRunnerResult { packResult: PackResult; @@ -44,13 +49,16 @@ export const runDefaultAction = async ( logger.trace('Merged config:', config); // Create worker task runner - const taskRunner = initTaskRunner({ + const taskRunner = initTaskRunner({ numOfTasks: 1, workerPath: new URL('./workers/defaultActionWorker.js', import.meta.url).href, runtime: 'child_process', }); try { + // Wait for worker to be ready (Bun compatibility) + await waitForWorkerReady(taskRunner, { cwd, config }); + // Create task for worker (now with pre-loaded config) const task: DefaultActionTask = { directories, @@ -61,7 +69,7 @@ export const runDefaultAction = async ( }; // Run the task in worker (spinner is handled inside worker) - const result = await taskRunner.run(task); + const result = (await taskRunner.run(task)) as DefaultActionWorkerResult; // Report results in main process reportResults(cwd, result.packResult, result.config); @@ -257,3 +265,45 @@ export const buildCliConfig = (options: CliOptions): RepomixConfigCli => { throw error; } }; + +/** + * Wait for worker to be ready by sending a ping request. + * This is specifically needed for Bun compatibility due to ES module initialization timing issues. + */ +const waitForWorkerReady = async ( + taskRunner: { run: (task: DefaultActionTask | PingTask) => Promise }, + context: { cwd: string; config: RepomixConfigMerged }, +): Promise => { + const isBun = process.versions?.bun; + if (!isBun) { + // No need to wait for Node.js + return; + } + + const maxRetries = 3; + const retryDelay = 50; // ms + let pingSuccessful = false; + + for (let attempt = 1; attempt <= maxRetries; attempt++) { + try { + await taskRunner.run({ + ping: true, + cwd: context.cwd, + config: context.config, + }); + logger.debug(`Worker initialization ping successful on attempt ${attempt}`); + pingSuccessful = true; + break; + } catch (error) { + logger.debug(`Worker ping failed on attempt ${attempt}/${maxRetries}:`, error); + if (attempt < maxRetries) { + logger.debug(`Waiting ${retryDelay}ms before retry...`); + await new Promise((resolve) => setTimeout(resolve, retryDelay)); + } + } + } + + if (!pingSuccessful) { + logger.debug('All Worker ping attempts failed, proceeding anyway...'); + } +}; diff --git a/src/cli/actions/workers/defaultActionWorker.ts b/src/cli/actions/workers/defaultActionWorker.ts index e6bfbf1b0..9e43868f3 100644 --- a/src/cli/actions/workers/defaultActionWorker.ts +++ b/src/cli/actions/workers/defaultActionWorker.ts @@ -19,18 +19,37 @@ export interface DefaultActionTask { isStdin: boolean; } +export interface PingTask { + ping: true; + cwd: string; + config: RepomixConfigMerged; +} + export interface DefaultActionWorkerResult { packResult: PackResult; config: RepomixConfigMerged; } -export default async ({ - directories, - cwd, - config, - cliOptions, - isStdin, -}: DefaultActionTask): Promise => { +export interface PingResult { + ping: true; + config: RepomixConfigMerged; +} + +// Function overloads for better type inference +async function defaultActionWorker( + task: DefaultActionTask | PingTask, +): Promise { + // Handle ping requests for Bun compatibility check + if ('ping' in task) { + return { + ping: true, + config: task.config, + }; + } + + // At this point, task is guaranteed to be DefaultActionTask + const { directories, cwd, config, cliOptions, isStdin } = task as DefaultActionTask; + logger.trace('Worker: Using pre-loaded config:', config); // Initialize spinner in worker @@ -81,7 +100,9 @@ export default async ({ spinner.fail('Error during packing'); throw error; } -}; +} + +export default defaultActionWorker; // Export cleanup function for Tinypool teardown export const onWorkerTermination = async () => { From cd6591299b56eb8387e9d37194cb8e5e41179f89 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 19:36:56 +0900 Subject: [PATCH 07/15] fix(ci): add build step before tests to ensure worker files exist CI was failing because test jobs were trying to import compiled worker files from lib/ directory, but the build step was missing. Added npm run build step before running tests in all test jobs to ensure TypeScript files are compiled to JavaScript before tests run. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4daadb0a5..93aa72a23 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,6 +140,7 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: npm ci + - run: npm run build # Run tests directly instead of using npm scripts with node --run which is not supported in older Node versions - run: ./node_modules/.bin/vitest --reporter=verbose env: @@ -159,6 +160,7 @@ jobs: with: bun-version: ${{ matrix.bun-version }} - run: bun install + - run: bun run build - run: bun run test env: CI_OS: ${{ runner.os }} @@ -173,6 +175,7 @@ jobs: node-version-file: .tool-versions cache: npm - run: npm ci + - run: npm run build - run: npm run test-coverage -- --reporter=verbose env: CI_OS: ${{ runner.os }} From a85dd6b2a075fb02aadf75562899eaa04c90e705 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 19:51:26 +0900 Subject: [PATCH 08/15] refactor(test): eliminate build dependency by mocking worker logic directly Replaced worker file imports with direct function calls in tests to improve coverage and remove CI build requirements. Key changes: - Extracted calculateUnifiedMetrics function from unifiedMetricsWorker for direct testing - Added taskRunner dependency injection to calculateMetrics function - Updated all metric tests to use mocked taskRunner instead of actual worker files - Removed build steps from CI test jobs (test, test-bun, test-coverage) This approach provides better test coverage since worker logic runs directly in the test process rather than being executed in separate worker threads. Tests now run faster without requiring TypeScript compilation. --- .github/workflows/ci.yml | 3 --- src/core/metrics/calculateMetrics.ts | 21 ++++++++++++------- .../metrics/workers/unifiedMetricsWorker.ts | 6 +++++- tests/core/metrics/calculateMetrics.test.ts | 6 ++++++ .../metrics/calculateOutputMetrics.test.ts | 5 +++-- .../calculateSelectiveFileMetrics.test.ts | 5 +++-- tests/core/metrics/diffTokenCount.test.ts | 15 +++++++++++++ 7 files changed, 45 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93aa72a23..4daadb0a5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,7 +140,6 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: npm ci - - run: npm run build # Run tests directly instead of using npm scripts with node --run which is not supported in older Node versions - run: ./node_modules/.bin/vitest --reporter=verbose env: @@ -160,7 +159,6 @@ jobs: with: bun-version: ${{ matrix.bun-version }} - run: bun install - - run: bun run build - run: bun run test env: CI_OS: ${{ runner.os }} @@ -175,7 +173,6 @@ jobs: node-version-file: .tool-versions cache: npm - run: npm ci - - run: npm run build - run: npm run test-coverage -- --reporter=verbose env: CI_OS: ${{ runner.os }} diff --git a/src/core/metrics/calculateMetrics.ts b/src/core/metrics/calculateMetrics.ts index 576a48b18..f091c4da4 100644 --- a/src/core/metrics/calculateMetrics.ts +++ b/src/core/metrics/calculateMetrics.ts @@ -1,5 +1,5 @@ import type { RepomixConfigMerged } from '../../config/configSchema.js'; -import { initTaskRunner } from '../../shared/processConcurrency.js'; +import { type TaskRunner, initTaskRunner } from '../../shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../shared/types.js'; import type { ProcessedFile } from '../file/fileTypes.js'; import type { GitDiffResult } from '../git/gitDiffHandle.js'; @@ -33,16 +33,19 @@ export const calculateMetrics = async ( calculateOutputMetrics, calculateGitDiffMetrics, calculateGitLogMetrics, + taskRunner: undefined as TaskRunner | undefined, }, ): Promise => { progressCallback('Calculating metrics...'); // Initialize a single task runner for all metrics calculations - const taskRunner = initTaskRunner({ - numOfTasks: processedFiles.length, - workerPath: new URL('../../../lib/core/metrics/workers/unifiedMetricsWorker.js', import.meta.url).href, - runtime: 'worker_threads', - }); + const taskRunner = + deps.taskRunner ?? + initTaskRunner({ + numOfTasks: processedFiles.length, + workerPath: new URL('../../../lib/core/metrics/workers/unifiedMetricsWorker.js', import.meta.url).href, + runtime: 'worker_threads', + }); try { // For top files display optimization: calculate token counts only for top files by character count @@ -98,7 +101,9 @@ export const calculateMetrics = async ( gitLogTokenCount: gitLogTokenCount.gitLogTokenCount, }; } finally { - // Cleanup the task runner after all calculations are complete - await taskRunner.cleanup(); + // Cleanup the task runner after all calculations are complete (only if we created it) + if (!deps.taskRunner) { + await taskRunner.cleanup(); + } } }; diff --git a/src/core/metrics/workers/unifiedMetricsWorker.ts b/src/core/metrics/workers/unifiedMetricsWorker.ts index 1d673ba92..2922ed78e 100644 --- a/src/core/metrics/workers/unifiedMetricsWorker.ts +++ b/src/core/metrics/workers/unifiedMetricsWorker.ts @@ -40,7 +40,7 @@ export interface GitLogMetricsTask { export type UnifiedMetricsTask = OutputMetricsTask | FileMetricsTask | GitDiffMetricsTask | GitLogMetricsTask; -export default async (task: UnifiedMetricsTask): Promise => { +export const calculateUnifiedMetrics = async (task: UnifiedMetricsTask): Promise => { const processStartAt = process.hrtime.bigint(); try { @@ -110,6 +110,10 @@ const getProcessDuration = (startTime: bigint): string => { return (Number(endTime - startTime) / 1e6).toFixed(2); }; +export default async (task: UnifiedMetricsTask): Promise => { + return calculateUnifiedMetrics(task); +}; + // Export cleanup function for Tinypool teardown export const onWorkerTermination = () => { freeTokenCounters(); diff --git a/tests/core/metrics/calculateMetrics.test.ts b/tests/core/metrics/calculateMetrics.test.ts index 743445777..b672131e1 100644 --- a/tests/core/metrics/calculateMetrics.test.ts +++ b/tests/core/metrics/calculateMetrics.test.ts @@ -54,11 +54,17 @@ describe('calculateMetrics', () => { const gitDiffResult: GitDiffResult | undefined = undefined; + const mockTaskRunner = { + run: vi.fn(), + cleanup: vi.fn(), + }; + const result = await calculateMetrics(processedFiles, output, progressCallback, config, gitDiffResult, undefined, { calculateSelectiveFileMetrics, calculateOutputMetrics: () => Promise.resolve(30), calculateGitDiffMetrics: () => Promise.resolve(0), calculateGitLogMetrics: () => Promise.resolve({ gitLogTokenCount: 0 }), + taskRunner: mockTaskRunner, }); expect(progressCallback).toHaveBeenCalledWith('Calculating metrics...'); diff --git a/tests/core/metrics/calculateOutputMetrics.test.ts b/tests/core/metrics/calculateOutputMetrics.test.ts index d24014860..34101e834 100644 --- a/tests/core/metrics/calculateOutputMetrics.test.ts +++ b/tests/core/metrics/calculateOutputMetrics.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it, vi } from 'vitest'; import { calculateOutputMetrics } from '../../../src/core/metrics/calculateOutputMetrics.js'; -import unifiedMetricsWorker, { +import { type OutputMetricsTask, type UnifiedMetricsTask, + calculateUnifiedMetrics, } from '../../../src/core/metrics/workers/unifiedMetricsWorker.js'; import { logger } from '../../../src/shared/logger.js'; import type { WorkerOptions } from '../../../src/shared/processConcurrency.js'; @@ -12,7 +13,7 @@ vi.mock('../../../src/shared/logger'); const mockInitTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - return (await unifiedMetricsWorker(task as UnifiedMetricsTask)) as R; + return (await calculateUnifiedMetrics(task as UnifiedMetricsTask)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests diff --git a/tests/core/metrics/calculateSelectiveFileMetrics.test.ts b/tests/core/metrics/calculateSelectiveFileMetrics.test.ts index 89e00051e..dd315dd2e 100644 --- a/tests/core/metrics/calculateSelectiveFileMetrics.test.ts +++ b/tests/core/metrics/calculateSelectiveFileMetrics.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it, vi } from 'vitest'; import type { ProcessedFile } from '../../../src/core/file/fileTypes.js'; import { calculateSelectiveFileMetrics } from '../../../src/core/metrics/calculateSelectiveFileMetrics.js'; -import unifiedMetricsWorker, { +import { type UnifiedMetricsTask, + calculateUnifiedMetrics, } from '../../../src/core/metrics/workers/unifiedMetricsWorker.js'; import type { WorkerOptions } from '../../../src/shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../../src/shared/types.js'; @@ -14,7 +15,7 @@ vi.mock('../../shared/processConcurrency', () => ({ const mockInitTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - return (await unifiedMetricsWorker(task as UnifiedMetricsTask)) as R; + return (await calculateUnifiedMetrics(task as UnifiedMetricsTask)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests diff --git a/tests/core/metrics/diffTokenCount.test.ts b/tests/core/metrics/diffTokenCount.test.ts index 84ad318d6..6afdee8ca 100644 --- a/tests/core/metrics/diffTokenCount.test.ts +++ b/tests/core/metrics/diffTokenCount.test.ts @@ -88,6 +88,10 @@ index 123..456 100644 }); // Mock dependency functions + const mockTaskRunner = { + run: vi.fn(), + cleanup: vi.fn(), + }; const mockCalculateOutputMetrics = vi.fn().mockResolvedValue(15); @@ -106,6 +110,7 @@ index 123..456 100644 calculateOutputMetrics: mockCalculateOutputMetrics, calculateGitDiffMetrics: vi.fn().mockResolvedValue(25), calculateGitLogMetrics: vi.fn().mockResolvedValue({ gitLogTokenCount: 0 }), + taskRunner: mockTaskRunner, }, ); @@ -166,6 +171,10 @@ index 123..456 100644 }); // Mock dependency functions + const mockTaskRunner = { + run: vi.fn(), + cleanup: vi.fn(), + }; const mockCalculateOutputMetrics = vi.fn().mockResolvedValue(15); @@ -181,6 +190,7 @@ index 123..456 100644 calculateOutputMetrics: mockCalculateOutputMetrics, calculateGitDiffMetrics: vi.fn().mockResolvedValue(0), calculateGitLogMetrics: vi.fn().mockResolvedValue({ gitLogTokenCount: 0 }), + taskRunner: mockTaskRunner, }, ); @@ -239,6 +249,10 @@ index 123..456 100644 }); // Mock dependency functions + const mockTaskRunner = { + run: vi.fn(), + cleanup: vi.fn(), + }; const mockCalculateOutputMetrics = vi.fn().mockResolvedValue(15); @@ -254,6 +268,7 @@ index 123..456 100644 calculateOutputMetrics: mockCalculateOutputMetrics, calculateGitDiffMetrics: vi.fn().mockResolvedValue(0), calculateGitLogMetrics: vi.fn().mockResolvedValue({ gitLogTokenCount: 0 }), + taskRunner: mockTaskRunner, }, ); From e1be3df28057b5ec47f3a6ac14d5e44ce24b53a2 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 20:20:01 +0900 Subject: [PATCH 09/15] refactor(metrics): simplify worker to token counter and move logic to callers - Rename unifiedMetricsWorker to calculateMetricsWorker for clarity - Simplify worker from complex switch-case to pure token counting - Move business logic (FileMetrics building, git diff combining) to calling functions - Update all metric calculation functions to use new simple worker interface - Improve test coverage by enabling direct worker function mocking - Maintain separation of concerns with focused worker responsibility This change eliminates historical naming confusion and creates a cleaner architecture where workers handle only token counting while callers handle metric aggregation and business logic. --- src/core/metrics/calculateGitDiffMetrics.ts | 34 +++-- src/core/metrics/calculateGitLogMetrics.ts | 10 +- src/core/metrics/calculateMetrics.ts | 9 +- src/core/metrics/calculateOutputMetrics.ts | 14 +- .../metrics/calculateSelectiveFileMetrics.ts | 36 +++--- .../metrics/workers/calculateMetricsWorker.ts | 50 ++++++++ .../metrics/workers/unifiedMetricsWorker.ts | 120 ------------------ .../metrics/calculateOutputMetrics.test.ts | 10 +- .../calculateSelectiveFileMetrics.test.ts | 7 +- 9 files changed, 109 insertions(+), 181 deletions(-) create mode 100644 src/core/metrics/workers/calculateMetricsWorker.ts delete mode 100644 src/core/metrics/workers/unifiedMetricsWorker.ts diff --git a/src/core/metrics/calculateGitDiffMetrics.ts b/src/core/metrics/calculateGitDiffMetrics.ts index 5cd5058c8..cbe3ec5ae 100644 --- a/src/core/metrics/calculateGitDiffMetrics.ts +++ b/src/core/metrics/calculateGitDiffMetrics.ts @@ -2,8 +2,7 @@ import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { logger } from '../../shared/logger.js'; import type { TaskRunner } from '../../shared/processConcurrency.js'; import type { GitDiffResult } from '../git/gitDiffHandle.js'; -import type { FileMetrics } from './workers/types.js'; -import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; +import type { TokenCountTask } from './workers/calculateMetricsWorker.js'; /** * Calculate token count for git diffs if included @@ -11,7 +10,7 @@ import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; export const calculateGitDiffMetrics = async ( config: RepomixConfigMerged, gitDiffResult: GitDiffResult | undefined, - deps: { taskRunner: TaskRunner }, + deps: { taskRunner: TaskRunner }, ): Promise => { if (!config.output.git?.includeDiffs || !gitDiffResult) { return 0; @@ -26,18 +25,33 @@ export const calculateGitDiffMetrics = async ( const startTime = process.hrtime.bigint(); logger.trace('Starting git diff token calculation using worker'); - const result = (await deps.taskRunner.run({ - type: 'gitDiff', - workTreeDiffContent: gitDiffResult.workTreeDiffContent, - stagedDiffContent: gitDiffResult.stagedDiffContent, - encoding: config.tokenCount.encoding, - })) as number; + const countPromises: Promise[] = []; + + if (gitDiffResult.workTreeDiffContent) { + countPromises.push( + deps.taskRunner.run({ + content: gitDiffResult.workTreeDiffContent, + encoding: config.tokenCount.encoding, + }), + ); + } + if (gitDiffResult.stagedDiffContent) { + countPromises.push( + deps.taskRunner.run({ + content: gitDiffResult.stagedDiffContent, + encoding: config.tokenCount.encoding, + }), + ); + } + + const results = await Promise.all(countPromises); + const totalTokens = results.reduce((sum, count) => sum + count, 0); const endTime = process.hrtime.bigint(); const duration = Number(endTime - startTime) / 1e6; logger.trace(`Git diff token calculation completed in ${duration.toFixed(2)}ms`); - return result; + return totalTokens; } catch (error) { logger.error('Error during git diff token calculation:', error); throw error; diff --git a/src/core/metrics/calculateGitLogMetrics.ts b/src/core/metrics/calculateGitLogMetrics.ts index 8eab8b968..97e94ae95 100644 --- a/src/core/metrics/calculateGitLogMetrics.ts +++ b/src/core/metrics/calculateGitLogMetrics.ts @@ -2,8 +2,7 @@ import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { logger } from '../../shared/logger.js'; import type { TaskRunner } from '../../shared/processConcurrency.js'; import type { GitLogResult } from '../git/gitLogHandle.js'; -import type { FileMetrics } from './workers/types.js'; -import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; +import type { TokenCountTask } from './workers/calculateMetricsWorker.js'; /** * Calculate token count for git logs if included @@ -11,7 +10,7 @@ import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; export const calculateGitLogMetrics = async ( config: RepomixConfigMerged, gitLogResult: GitLogResult | undefined, - deps: { taskRunner: TaskRunner }, + deps: { taskRunner: TaskRunner }, ): Promise<{ gitLogTokenCount: number }> => { // Return zero token count if git logs are disabled or no result if (!config.output.git?.includeLogs || !gitLogResult) { @@ -31,11 +30,10 @@ export const calculateGitLogMetrics = async ( const startTime = process.hrtime.bigint(); logger.trace('Starting git log token calculation using worker'); - const result = (await deps.taskRunner.run({ - type: 'gitLog', + const result = await deps.taskRunner.run({ content: gitLogResult.logContent, encoding: config.tokenCount.encoding, - })) as number; + }); const endTime = process.hrtime.bigint(); const duration = Number(endTime - startTime) / 1e6; diff --git a/src/core/metrics/calculateMetrics.ts b/src/core/metrics/calculateMetrics.ts index f091c4da4..8402f803b 100644 --- a/src/core/metrics/calculateMetrics.ts +++ b/src/core/metrics/calculateMetrics.ts @@ -8,8 +8,7 @@ import { calculateGitDiffMetrics } from './calculateGitDiffMetrics.js'; import { calculateGitLogMetrics } from './calculateGitLogMetrics.js'; import { calculateOutputMetrics } from './calculateOutputMetrics.js'; import { calculateSelectiveFileMetrics } from './calculateSelectiveFileMetrics.js'; -import type { FileMetrics } from './workers/types.js'; -import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; +import type { TokenCountTask } from './workers/calculateMetricsWorker.js'; export interface CalculateMetricsResult { totalFiles: number; @@ -33,7 +32,7 @@ export const calculateMetrics = async ( calculateOutputMetrics, calculateGitDiffMetrics, calculateGitLogMetrics, - taskRunner: undefined as TaskRunner | undefined, + taskRunner: undefined as TaskRunner | undefined, }, ): Promise => { progressCallback('Calculating metrics...'); @@ -41,9 +40,9 @@ export const calculateMetrics = async ( // Initialize a single task runner for all metrics calculations const taskRunner = deps.taskRunner ?? - initTaskRunner({ + initTaskRunner({ numOfTasks: processedFiles.length, - workerPath: new URL('../../../lib/core/metrics/workers/unifiedMetricsWorker.js', import.meta.url).href, + workerPath: new URL('../../../lib/core/metrics/workers/calculateMetricsWorker.js', import.meta.url).href, runtime: 'worker_threads', }); diff --git a/src/core/metrics/calculateOutputMetrics.ts b/src/core/metrics/calculateOutputMetrics.ts index 5e148a37f..ad41ae918 100644 --- a/src/core/metrics/calculateOutputMetrics.ts +++ b/src/core/metrics/calculateOutputMetrics.ts @@ -1,8 +1,7 @@ import type { TiktokenEncoding } from 'tiktoken'; import { logger } from '../../shared/logger.js'; import type { TaskRunner } from '../../shared/processConcurrency.js'; -import type { FileMetrics } from './workers/types.js'; -import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; +import type { TokenCountTask } from './workers/calculateMetricsWorker.js'; const CHUNK_SIZE = 1000; const MIN_CONTENT_LENGTH_FOR_PARALLEL = 1_000_000; // 1000KB @@ -11,7 +10,7 @@ export const calculateOutputMetrics = async ( content: string, encoding: TiktokenEncoding, path: string | undefined, - deps: { taskRunner: TaskRunner }, + deps: { taskRunner: TaskRunner }, ): Promise => { const shouldRunInParallel = content.length > MIN_CONTENT_LENGTH_FOR_PARALLEL; @@ -33,13 +32,11 @@ export const calculateOutputMetrics = async ( // Process chunks in parallel const chunkResults = await Promise.all( chunks.map(async (chunk, index) => { - const result = await deps.taskRunner.run({ - type: 'output', + return deps.taskRunner.run({ content: chunk, encoding, path: path ? `${path}-chunk-${index}` : undefined, }); - return result as number; // Output tasks always return numbers }), ); @@ -47,12 +44,11 @@ export const calculateOutputMetrics = async ( result = chunkResults.reduce((sum, count) => sum + count, 0); } else { // Process small content directly - result = (await deps.taskRunner.run({ - type: 'output', + result = await deps.taskRunner.run({ content, encoding, path, - })) as number; + }); } const endTime = process.hrtime.bigint(); diff --git a/src/core/metrics/calculateSelectiveFileMetrics.ts b/src/core/metrics/calculateSelectiveFileMetrics.ts index 4511541ab..02f52726a 100644 --- a/src/core/metrics/calculateSelectiveFileMetrics.ts +++ b/src/core/metrics/calculateSelectiveFileMetrics.ts @@ -4,15 +4,15 @@ import { logger } from '../../shared/logger.js'; import type { TaskRunner } from '../../shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../shared/types.js'; import type { ProcessedFile } from '../file/fileTypes.js'; +import type { TokenCountTask } from './workers/calculateMetricsWorker.js'; import type { FileMetrics } from './workers/types.js'; -import type { UnifiedMetricsTask } from './workers/unifiedMetricsWorker.js'; export const calculateSelectiveFileMetrics = async ( processedFiles: ProcessedFile[], targetFilePaths: string[], tokenCounterEncoding: TiktokenEncoding, progressCallback: RepomixProgressCallback, - deps: { taskRunner: TaskRunner }, + deps: { taskRunner: TaskRunner }, ): Promise => { const targetFileSet = new Set(targetFilePaths); const filesToProcess = processedFiles.filter((file) => targetFileSet.has(file.path)); @@ -21,30 +21,28 @@ export const calculateSelectiveFileMetrics = async ( return []; } - const tasks = filesToProcess.map( - (file, index) => - ({ - type: 'file', - file, - index, - totalFiles: filesToProcess.length, - encoding: tokenCounterEncoding, - }) satisfies UnifiedMetricsTask, - ); - try { const startTime = process.hrtime.bigint(); logger.trace(`Starting selective metrics calculation for ${filesToProcess.length} files using worker pool`); let completedTasks = 0; const results = await Promise.all( - tasks.map(async (task) => { - const result = (await deps.taskRunner.run(task)) as FileMetrics; + filesToProcess.map(async (file) => { + const tokenCount = await deps.taskRunner.run({ + content: file.content, + encoding: tokenCounterEncoding, + path: file.path, + }); + + const result: FileMetrics = { + path: file.path, + charCount: file.content.length, + tokenCount, + }; + completedTasks++; - progressCallback( - `Calculating metrics... (${completedTasks}/${filesToProcess.length}) ${pc.dim(task.file.path)}`, - ); - logger.trace(`Calculating metrics... (${completedTasks}/${filesToProcess.length}) ${task.file.path}`); + progressCallback(`Calculating metrics... (${completedTasks}/${filesToProcess.length}) ${pc.dim(file.path)}`); + logger.trace(`Calculating metrics... (${completedTasks}/${filesToProcess.length}) ${file.path}`); return result; }), ); diff --git a/src/core/metrics/workers/calculateMetricsWorker.ts b/src/core/metrics/workers/calculateMetricsWorker.ts new file mode 100644 index 000000000..34fc0ee48 --- /dev/null +++ b/src/core/metrics/workers/calculateMetricsWorker.ts @@ -0,0 +1,50 @@ +import type { TiktokenEncoding } from 'tiktoken'; +import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; +import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; + +/** + * Simple token counting worker for metrics calculation. + * + * This worker provides a focused interface for counting tokens from text content, + * using the Tiktoken encoding. All complex metric calculation logic is handled + * by the calling side to maintain separation of concerns. + */ + +// Initialize logger configuration from workerData at module load time +// This must be called before any logging operations in the worker +setLogLevelByWorkerData(); + +export interface TokenCountTask { + content: string; + encoding: TiktokenEncoding; + path?: string; +} + +export const countTokens = async (task: TokenCountTask): Promise => { + const processStartAt = process.hrtime.bigint(); + + try { + const counter = getTokenCounter(task.encoding); + const tokenCount = counter.countTokens(task.content, task.path); + + logger.trace(`Counted tokens. Count: ${tokenCount}. Took: ${getProcessDuration(processStartAt)}ms`); + return tokenCount; + } catch (error) { + logger.error('Error in token counting worker:', error); + throw error; + } +}; + +const getProcessDuration = (startTime: bigint): string => { + const endTime = process.hrtime.bigint(); + return (Number(endTime - startTime) / 1e6).toFixed(2); +}; + +export default async (task: TokenCountTask): Promise => { + return countTokens(task); +}; + +// Export cleanup function for Tinypool teardown +export const onWorkerTermination = () => { + freeTokenCounters(); +}; diff --git a/src/core/metrics/workers/unifiedMetricsWorker.ts b/src/core/metrics/workers/unifiedMetricsWorker.ts deleted file mode 100644 index 2922ed78e..000000000 --- a/src/core/metrics/workers/unifiedMetricsWorker.ts +++ /dev/null @@ -1,120 +0,0 @@ -import type { TiktokenEncoding } from 'tiktoken'; -import { logger, setLogLevelByWorkerData } from '../../../shared/logger.js'; -import type { ProcessedFile } from '../../file/fileTypes.js'; -import { freeTokenCounters, getTokenCounter } from '../tokenCounterFactory.js'; -import type { FileMetrics } from './types.js'; - -// Initialize logger configuration from workerData at module load time -// This must be called before any logging operations in the worker -setLogLevelByWorkerData(); - -export type MetricsTaskType = 'output' | 'file' | 'gitDiff' | 'gitLog'; - -export interface OutputMetricsTask { - type: 'output'; - content: string; - encoding: TiktokenEncoding; - path?: string; -} - -export interface FileMetricsTask { - type: 'file'; - file: ProcessedFile; - index: number; - totalFiles: number; - encoding: TiktokenEncoding; -} - -export interface GitDiffMetricsTask { - type: 'gitDiff'; - workTreeDiffContent?: string; - stagedDiffContent?: string; - encoding: TiktokenEncoding; -} - -export interface GitLogMetricsTask { - type: 'gitLog'; - content: string; - encoding: TiktokenEncoding; -} - -export type UnifiedMetricsTask = OutputMetricsTask | FileMetricsTask | GitDiffMetricsTask | GitLogMetricsTask; - -export const calculateUnifiedMetrics = async (task: UnifiedMetricsTask): Promise => { - const processStartAt = process.hrtime.bigint(); - - try { - switch (task.type) { - case 'output': { - const counter = getTokenCounter(task.encoding); - const tokenCount = counter.countTokens(task.content, task.path); - logger.trace(`Counted output tokens. Count: ${tokenCount}. Took: ${getProcessDuration(processStartAt)}ms`); - return tokenCount; - } - - case 'file': { - const charCount = task.file.content.length; - const tokenCounter = getTokenCounter(task.encoding); - const tokenCount = tokenCounter.countTokens(task.file.content, task.file.path); - const result: FileMetrics = { path: task.file.path, charCount, tokenCount }; - logger.trace(`Calculated metrics for ${task.file.path}. Took: ${getProcessDuration(processStartAt)}ms`); - return result; - } - - case 'gitDiff': { - const tokenCounter = getTokenCounter(task.encoding); - const countPromises = []; - - if (task.workTreeDiffContent) { - const content = task.workTreeDiffContent; - countPromises.push(Promise.resolve().then(() => tokenCounter.countTokens(content))); - } - if (task.stagedDiffContent) { - const content = task.stagedDiffContent; - countPromises.push(Promise.resolve().then(() => tokenCounter.countTokens(content))); - } - - const results = await Promise.all(countPromises); - const totalTokens = results.reduce((sum, count) => sum + count, 0); - logger.trace( - `Calculated git diff metrics. Tokens: ${totalTokens}. Took: ${getProcessDuration(processStartAt)}ms`, - ); - return totalTokens; - } - - case 'gitLog': { - if (!task.content) { - return 0; - } - - const tokenCounter = getTokenCounter(task.encoding); - const tokenCount = tokenCounter.countTokens(task.content); - logger.trace(`Git log token count calculated in ${getProcessDuration(processStartAt)}ms`); - return tokenCount; - } - - default: - throw new Error(`Unknown task type: ${(task as { type?: string }).type || 'unknown'}`); - } - } catch (error) { - logger.error(`Error in unified metrics worker for task type ${task.type}:`, error); - if (task.type === 'gitLog') { - return 0; // gitLog worker returns 0 on error - } - throw error; - } -}; - -const getProcessDuration = (startTime: bigint): string => { - const endTime = process.hrtime.bigint(); - return (Number(endTime - startTime) / 1e6).toFixed(2); -}; - -export default async (task: UnifiedMetricsTask): Promise => { - return calculateUnifiedMetrics(task); -}; - -// Export cleanup function for Tinypool teardown -export const onWorkerTermination = () => { - freeTokenCounters(); -}; diff --git a/tests/core/metrics/calculateOutputMetrics.test.ts b/tests/core/metrics/calculateOutputMetrics.test.ts index 34101e834..f90b7f16a 100644 --- a/tests/core/metrics/calculateOutputMetrics.test.ts +++ b/tests/core/metrics/calculateOutputMetrics.test.ts @@ -1,10 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { calculateOutputMetrics } from '../../../src/core/metrics/calculateOutputMetrics.js'; -import { - type OutputMetricsTask, - type UnifiedMetricsTask, - calculateUnifiedMetrics, -} from '../../../src/core/metrics/workers/unifiedMetricsWorker.js'; +import { type TokenCountTask, countTokens } from '../../../src/core/metrics/workers/calculateMetricsWorker.js'; import { logger } from '../../../src/shared/logger.js'; import type { WorkerOptions } from '../../../src/shared/processConcurrency.js'; @@ -13,7 +9,7 @@ vi.mock('../../../src/shared/logger'); const mockInitTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - return (await calculateUnifiedMetrics(task as UnifiedMetricsTask)) as R; + return (await countTokens(task as TokenCountTask)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests @@ -154,7 +150,7 @@ describe('calculateOutputMetrics', () => { const mockChunkTrackingTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - const outputTask = task as OutputMetricsTask; + const outputTask = task as TokenCountTask; processedChunks.push(outputTask.content); return outputTask.content.length as R; }, diff --git a/tests/core/metrics/calculateSelectiveFileMetrics.test.ts b/tests/core/metrics/calculateSelectiveFileMetrics.test.ts index dd315dd2e..8577b6441 100644 --- a/tests/core/metrics/calculateSelectiveFileMetrics.test.ts +++ b/tests/core/metrics/calculateSelectiveFileMetrics.test.ts @@ -1,10 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; import type { ProcessedFile } from '../../../src/core/file/fileTypes.js'; import { calculateSelectiveFileMetrics } from '../../../src/core/metrics/calculateSelectiveFileMetrics.js'; -import { - type UnifiedMetricsTask, - calculateUnifiedMetrics, -} from '../../../src/core/metrics/workers/unifiedMetricsWorker.js'; +import { type TokenCountTask, countTokens } from '../../../src/core/metrics/workers/calculateMetricsWorker.js'; import type { WorkerOptions } from '../../../src/shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../../../src/shared/types.js'; @@ -15,7 +12,7 @@ vi.mock('../../shared/processConcurrency', () => ({ const mockInitTaskRunner = (_options: WorkerOptions) => { return { run: async (task: T) => { - return (await calculateUnifiedMetrics(task as UnifiedMetricsTask)) as R; + return (await countTokens(task as TokenCountTask)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests From 1a8bca2292e8d76c451750d79d3de600f33799cc Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 20:33:54 +0900 Subject: [PATCH 10/15] test(metrics): add comprehensive tests for worker functions - Add tests for defaultActionWorker.ts covering ping, directory processing, stdin handling, error cases, and path resolution - Add tests for calculateGitDiffMetrics.ts covering config validation, diff processing, error handling, and encoding - Add tests for calculateGitLogMetrics.ts covering log processing, error scenarios, and edge cases - All test files follow project conventions with proper mocking and type safety - Total of 46 new tests ensuring robust coverage of worker functionality These tests improve code coverage and provide safety net for future changes to the metrics calculation and worker system components. --- .../workers/defaultActionWorker.test.ts | 432 ++++++++++++++++++ .../metrics/calculateGitDiffMetrics.test.ts | 350 ++++++++++++++ .../metrics/calculateGitLogMetrics.test.ts | 419 +++++++++++++++++ 3 files changed, 1201 insertions(+) create mode 100644 tests/cli/actions/workers/defaultActionWorker.test.ts create mode 100644 tests/core/metrics/calculateGitDiffMetrics.test.ts create mode 100644 tests/core/metrics/calculateGitLogMetrics.test.ts diff --git a/tests/cli/actions/workers/defaultActionWorker.test.ts b/tests/cli/actions/workers/defaultActionWorker.test.ts new file mode 100644 index 000000000..f7d2c81ad --- /dev/null +++ b/tests/cli/actions/workers/defaultActionWorker.test.ts @@ -0,0 +1,432 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import defaultActionWorker, { + type DefaultActionTask, + type DefaultActionWorkerResult, + type PingTask, + type PingResult, +} from '../../../../src/cli/actions/workers/defaultActionWorker.js'; +import type { CliOptions } from '../../../../src/cli/types.js'; +import type { RepomixConfigMerged } from '../../../../src/config/configSchema.js'; +import { readFilePathsFromStdin } from '../../../../src/core/file/fileStdin.js'; +import { pack } from '../../../../src/core/packager.js'; +import { RepomixError } from '../../../../src/shared/errorHandle.js'; + +// Mock dependencies +vi.mock('../../../../src/core/file/fileStdin.js'); +vi.mock('../../../../src/core/packager.js'); +vi.mock('../../../../src/shared/logger.js', () => ({ + logger: { + trace: vi.fn(), + }, + setLogLevelByWorkerData: vi.fn(), +})); +vi.mock('../../../../src/cli/cliSpinner.js', () => ({ + Spinner: vi.fn().mockImplementation(() => ({ + start: vi.fn(), + update: vi.fn(), + succeed: vi.fn(), + fail: vi.fn(), + })), +})); + +const mockReadFilePathsFromStdin = vi.mocked(readFilePathsFromStdin); +const mockPack = vi.mocked(pack); + +describe('defaultActionWorker', () => { + const mockConfig: RepomixConfigMerged = { + input: { + maxFileSize: 50 * 1024 * 1024, + }, + output: { + filePath: 'test-output.txt', + style: 'xml', + parsableStyle: false, + headerText: '', + instructionFilePath: '', + fileSummary: true, + directoryStructure: true, + files: true, + removeComments: false, + removeEmptyLines: false, + compress: false, + topFilesLength: 10, + showLineNumbers: false, + truncateBase64: false, + copyToClipboard: false, + includeEmptyDirectories: false, + tokenCountTree: false, + git: { + sortByChanges: true, + sortByChangesMaxCommits: 100, + includeDiffs: false, + includeLogs: false, + includeLogsCount: 50, + }, + }, + include: ['**/*'], + ignore: { + useGitignore: true, + useDefaultPatterns: true, + customPatterns: [], + }, + security: { + enableSecurityCheck: true, + }, + tokenCount: { + encoding: 'o200k_base' as const, + }, + cwd: '/test/project', + }; + + const mockCliOptions: CliOptions = { + verbose: false, + output: 'test-output.txt', + include: undefined, + ignore: undefined, + 'ignore-file': undefined, + config: undefined, + style: 'xml', + 'output-show-line-numbers': false, + 'remove-comments': false, + 'remove-empty-lines': false, + 'copy-to-clipboard': false, + 'include-empty-directories': false, + 'git-log-output': false, + 'git-log-author': undefined, + 'git-log-after': undefined, + 'git-log-before': undefined, + 'git-log-max-count': undefined, + 'git-diff': false, + stdin: false, + 'top-files-length': 10, + version: false, + init: false, + remote: undefined, + 'process-concurrency': 8, + 'token-count-tree': false, + 'no-progress': false, + }; + + const mockPackResult = { + totalFiles: 1, + totalCharacters: 12, + totalTokens: 3, + fileCharCounts: { 'test.txt': 12 }, + fileTokenCounts: { 'test.txt': 3 }, + gitDiffTokenCount: 0, + gitLogTokenCount: 0, + suspiciousFilesResults: [], + suspiciousGitDiffResults: [], + suspiciousGitLogResults: [], + processedFiles: [{ path: 'test.txt', content: 'test content' }], + safeFilePaths: ['test.txt'], + skippedFiles: [], + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('ping functionality', () => { + it('should handle ping requests correctly', async () => { + const pingTask: PingTask = { + ping: true, + cwd: '/test/dir', + config: mockConfig, + }; + + const result = (await defaultActionWorker(pingTask)) as PingResult; + + expect(result).toEqual({ + ping: true, + config: mockConfig, + }); + }); + }); + + describe('directory processing', () => { + it('should process directories successfully', async () => { + const task: DefaultActionTask = { + directories: ['src', 'tests'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockResolvedValueOnce(mockPackResult); + + const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; + + expect(mockPack).toHaveBeenCalledWith( + ['/test/project/src', '/test/project/tests'], + mockConfig, + expect.any(Function), + ); + expect(result).toEqual({ + packResult: mockPackResult, + config: mockConfig, + }); + }); + + it('should handle single directory', async () => { + const task: DefaultActionTask = { + directories: ['.'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockResolvedValueOnce(mockPackResult); + + const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; + + expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function)); + expect(result).toEqual({ + packResult: mockPackResult, + config: mockConfig, + }); + }); + + it('should handle empty directories array', async () => { + const task: DefaultActionTask = { + directories: [], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockResolvedValueOnce(mockPackResult); + + await defaultActionWorker(task); + + expect(mockPack).toHaveBeenCalledWith([], mockConfig, expect.any(Function)); + }); + }); + + describe('stdin processing', () => { + it('should process stdin successfully with current directory', async () => { + const task: DefaultActionTask = { + directories: ['.'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: true, + }; + + const stdinResult = { + filePaths: ['file1.txt', 'file2.txt'], + emptyDirPaths: [], + }; + mockReadFilePathsFromStdin.mockResolvedValueOnce(stdinResult); + mockPack.mockResolvedValueOnce(mockPackResult); + + const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; + + expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); + expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function), {}, [ + 'file1.txt', + 'file2.txt', + ]); + expect(result).toEqual({ + packResult: mockPackResult, + config: mockConfig, + }); + }); + + it('should handle empty directories for stdin', async () => { + const task: DefaultActionTask = { + directories: [], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: true, + }; + + const stdinResult = { + filePaths: ['file1.txt'], + emptyDirPaths: [], + }; + mockReadFilePathsFromStdin.mockResolvedValueOnce(stdinResult); + mockPack.mockResolvedValueOnce(mockPackResult); + + await defaultActionWorker(task); + + expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); + expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function), {}, ['file1.txt']); + }); + + it('should throw error when multiple directories are specified with stdin', async () => { + const task: DefaultActionTask = { + directories: ['src', 'tests'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: true, + }; + + await expect(defaultActionWorker(task)).rejects.toThrow(RepomixError); + await expect(defaultActionWorker(task)).rejects.toThrow( + 'When using --stdin, do not specify directory arguments. File paths will be read from stdin.', + ); + }); + + it('should throw error when non-current directory is specified with stdin', async () => { + const task: DefaultActionTask = { + directories: ['src'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: true, + }; + + await expect(defaultActionWorker(task)).rejects.toThrow(RepomixError); + await expect(defaultActionWorker(task)).rejects.toThrow( + 'When using --stdin, do not specify directory arguments. File paths will be read from stdin.', + ); + }); + }); + + describe('error handling', () => { + it('should handle pack errors for directory processing', async () => { + const task: DefaultActionTask = { + directories: ['src'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + const packError = new Error('Pack failed'); + mockPack.mockRejectedValueOnce(packError); + + await expect(defaultActionWorker(task)).rejects.toThrow('Pack failed'); + }); + + it('should handle stdin read errors', async () => { + const task: DefaultActionTask = { + directories: ['.'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: true, + }; + + const stdinError = new Error('Stdin read failed'); + mockReadFilePathsFromStdin.mockRejectedValueOnce(stdinError); + + await expect(defaultActionWorker(task)).rejects.toThrow('Stdin read failed'); + }); + + it('should handle pack errors during stdin processing', async () => { + const task: DefaultActionTask = { + directories: ['.'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: true, + }; + + const stdinResult = { + filePaths: ['file1.txt'], + emptyDirPaths: [], + }; + mockReadFilePathsFromStdin.mockResolvedValueOnce(stdinResult); + + const packError = new Error('Pack failed during stdin'); + mockPack.mockRejectedValueOnce(packError); + + await expect(defaultActionWorker(task)).rejects.toThrow('Pack failed during stdin'); + }); + }); + + describe('spinner integration', () => { + it('should update spinner with progress messages', async () => { + const task: DefaultActionTask = { + directories: ['src'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockImplementationOnce(async (_paths, _config, progressCallback) => { + if (progressCallback) { + progressCallback('Processing files...'); + progressCallback('Calculating metrics...'); + } + return mockPackResult; + }); + + await defaultActionWorker(task); + + // The spinner mock should be imported and we can verify the calls + const { Spinner } = await import('../../../../src/cli/cliSpinner.js'); + const mockSpinner = vi.mocked(Spinner).mock.results[0]?.value; + + expect(mockSpinner.start).toHaveBeenCalled(); + expect(mockSpinner.update).toHaveBeenCalledWith('Processing files...'); + expect(mockSpinner.update).toHaveBeenCalledWith('Calculating metrics...'); + expect(mockSpinner.succeed).toHaveBeenCalledWith('Packing completed successfully!'); + }); + + it('should fail spinner on error', async () => { + const task: DefaultActionTask = { + directories: ['src'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockRejectedValueOnce(new Error('Pack failed')); + + await expect(defaultActionWorker(task)).rejects.toThrow('Pack failed'); + + const { Spinner } = await import('../../../../src/cli/cliSpinner.js'); + const mockSpinner = vi.mocked(Spinner).mock.results[0]?.value; + + expect(mockSpinner.fail).toHaveBeenCalledWith('Error during packing'); + }); + }); + + describe('path resolution', () => { + it('should resolve relative paths correctly', async () => { + const task: DefaultActionTask = { + directories: ['../parent', './current', 'child'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockResolvedValueOnce(mockPackResult); + + await defaultActionWorker(task); + + expect(mockPack).toHaveBeenCalledWith( + ['/test/parent', '/test/project/current', '/test/project/child'], + mockConfig, + expect.any(Function), + ); + }); + + it('should handle absolute paths', async () => { + const task: DefaultActionTask = { + directories: ['/absolute/path1', '/absolute/path2'], + cwd: '/test/project', + config: mockConfig, + cliOptions: mockCliOptions, + isStdin: false, + }; + + mockPack.mockResolvedValueOnce(mockPackResult); + + await defaultActionWorker(task); + + expect(mockPack).toHaveBeenCalledWith(['/absolute/path1', '/absolute/path2'], mockConfig, expect.any(Function)); + }); + }); +}); diff --git a/tests/core/metrics/calculateGitDiffMetrics.test.ts b/tests/core/metrics/calculateGitDiffMetrics.test.ts new file mode 100644 index 000000000..729ac8fc8 --- /dev/null +++ b/tests/core/metrics/calculateGitDiffMetrics.test.ts @@ -0,0 +1,350 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { RepomixConfigMerged } from '../../../src/config/configSchema.js'; +import type { GitDiffResult } from '../../../src/core/git/gitDiffHandle.js'; +import { calculateGitDiffMetrics } from '../../../src/core/metrics/calculateGitDiffMetrics.js'; +import { type TokenCountTask, countTokens } from '../../../src/core/metrics/workers/calculateMetricsWorker.js'; +import { logger } from '../../../src/shared/logger.js'; +import type { TaskRunner, WorkerOptions } from '../../../src/shared/processConcurrency.js'; + +vi.mock('../../../src/shared/logger'); + +const mockInitTaskRunner = (_options: WorkerOptions): TaskRunner => { + return { + run: async (task: TokenCountTask) => { + return await countTokens(task); + }, + cleanup: async () => { + // Mock cleanup - no-op for tests + }, + }; +}; + +describe('calculateGitDiffMetrics', () => { + const mockConfig: RepomixConfigMerged = { + input: { + maxFileSize: 50 * 1024 * 1024, + }, + output: { + filePath: 'test-output.txt', + style: 'xml', + parsableStyle: false, + headerText: '', + instructionFilePath: '', + fileSummary: true, + directoryStructure: true, + files: true, + removeComments: false, + removeEmptyLines: false, + compress: false, + topFilesLength: 10, + showLineNumbers: false, + truncateBase64: false, + copyToClipboard: false, + includeEmptyDirectories: false, + tokenCountTree: false, + git: { + sortByChanges: true, + sortByChangesMaxCommits: 100, + includeDiffs: true, + includeLogs: false, + includeLogsCount: 50, + }, + }, + include: ['**/*'], + ignore: { + useGitignore: true, + useDefaultPatterns: true, + customPatterns: [], + }, + security: { + enableSecurityCheck: true, + }, + tokenCount: { + encoding: 'o200k_base' as const, + }, + cwd: '/test/project', + }; + + const mockTaskRunner = mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('when git diffs are disabled', () => { + it('should return 0 when includeDiffs is false', async () => { + const configWithDisabledDiffs = { + ...mockConfig, + output: { + ...mockConfig.output, + git: { + ...mockConfig.output.git, + includeDiffs: false, + }, + }, + }; + + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'some diff content', + stagedDiffContent: 'some staged content', + }; + + const result = await calculateGitDiffMetrics(configWithDisabledDiffs, gitDiffResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toBe(0); + }); + + it('should return 0 when git config is undefined', async () => { + const configWithoutGit = { + ...mockConfig, + output: { + ...mockConfig.output, + git: undefined, + }, + } as RepomixConfigMerged; + + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'some diff content', + stagedDiffContent: 'some staged content', + }; + + const result = await calculateGitDiffMetrics(configWithoutGit, gitDiffResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toBe(0); + }); + }); + + describe('when git diff result is unavailable', () => { + it('should return 0 when gitDiffResult is undefined', async () => { + const result = await calculateGitDiffMetrics(mockConfig, undefined, { + taskRunner: mockTaskRunner, + }); + + expect(result).toBe(0); + }); + + it('should return 0 when both diff contents are empty', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: '', + stagedDiffContent: '', + }; + + const result = await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toBe(0); + }); + + it('should return 0 when both diff contents are undefined', async () => { + const gitDiffResult = { + workTreeDiffContent: undefined as unknown as string, + stagedDiffContent: undefined as unknown as string, + }; + + const result = await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toBe(0); + }); + }); + + describe('when processing git diffs', () => { + it('should calculate tokens for both workTree and staged diffs', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'work tree changes', + stagedDiffContent: 'staged changes', + }; + + const mockTaskRunnerSpy = vi + .fn() + .mockResolvedValueOnce(5) // workTree tokens + .mockResolvedValueOnce(3); // staged tokens + + const customTaskRunner: TaskRunner = { + run: mockTaskRunnerSpy, + cleanup: async () => {}, + }; + + const result = await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: customTaskRunner, + }); + + expect(mockTaskRunnerSpy).toHaveBeenCalledTimes(2); + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'work tree changes', + encoding: 'o200k_base', + }); + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'staged changes', + encoding: 'o200k_base', + }); + expect(result).toBe(8); // 5 + 3 + }); + + it('should calculate tokens for workTree diff only', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'work tree changes only', + stagedDiffContent: '', + }; + + const mockTaskRunnerSpy = vi.fn().mockResolvedValueOnce(7); + + const customTaskRunner: TaskRunner = { + run: mockTaskRunnerSpy, + cleanup: async () => {}, + }; + + const result = await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: customTaskRunner, + }); + + expect(mockTaskRunnerSpy).toHaveBeenCalledTimes(1); + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'work tree changes only', + encoding: 'o200k_base', + }); + expect(result).toBe(7); + }); + + it('should calculate tokens for staged diff only', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: '', + stagedDiffContent: 'staged changes only', + }; + + const mockTaskRunnerSpy = vi.fn().mockResolvedValueOnce(4); + + const customTaskRunner: TaskRunner = { + run: mockTaskRunnerSpy, + cleanup: async () => {}, + }; + + const result = await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: customTaskRunner, + }); + + expect(mockTaskRunnerSpy).toHaveBeenCalledTimes(1); + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'staged changes only', + encoding: 'o200k_base', + }); + expect(result).toBe(4); + }); + + it('should handle large diff content correctly', async () => { + const largeDiffContent = 'a'.repeat(10000); + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: largeDiffContent, + stagedDiffContent: largeDiffContent, + }; + + const result = await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toBeGreaterThan(0); + expect(typeof result).toBe('number'); + }); + }); + + describe('error handling', () => { + it('should throw error when task runner fails', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'some content', + stagedDiffContent: 'some staged content', + }; + + const errorTaskRunner: TaskRunner = { + run: vi.fn().mockRejectedValue(new Error('Task runner failed')), + cleanup: async () => {}, + }; + + await expect( + calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: errorTaskRunner, + }), + ).rejects.toThrow('Task runner failed'); + + expect(logger.error).toHaveBeenCalledWith('Error during git diff token calculation:', expect.any(Error)); + }); + + it('should handle partial task runner failures', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'work tree content', + stagedDiffContent: 'staged content', + }; + + const errorTaskRunner: TaskRunner = { + run: vi + .fn() + .mockResolvedValueOnce(5) // First call succeeds + .mockRejectedValueOnce(new Error('Second call fails')), // Second call fails + cleanup: async () => {}, + }; + + await expect( + calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: errorTaskRunner, + }), + ).rejects.toThrow('Second call fails'); + + expect(logger.error).toHaveBeenCalledWith('Error during git diff token calculation:', expect.any(Error)); + }); + }); + + describe('logging', () => { + it('should log trace messages for successful calculation', async () => { + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'test content', + stagedDiffContent: 'staged content', + }; + + await calculateGitDiffMetrics(mockConfig, gitDiffResult, { + taskRunner: mockTaskRunner, + }); + + expect(logger.trace).toHaveBeenCalledWith('Starting git diff token calculation using worker'); + expect(logger.trace).toHaveBeenCalledWith( + expect.stringMatching(/Git diff token calculation completed in \d+\.\d+ms/), + ); + }); + }); + + describe('encoding configuration', () => { + it('should use correct encoding from config', async () => { + const configWithDifferentEncoding = { + ...mockConfig, + tokenCount: { + encoding: 'cl100k_base' as const, + }, + }; + + const gitDiffResult: GitDiffResult = { + workTreeDiffContent: 'test content', + stagedDiffContent: '', + }; + + const mockTaskRunnerSpy = vi.fn().mockResolvedValueOnce(10); + + const customTaskRunner: TaskRunner = { + run: mockTaskRunnerSpy, + cleanup: async () => {}, + }; + + await calculateGitDiffMetrics(configWithDifferentEncoding, gitDiffResult, { + taskRunner: customTaskRunner, + }); + + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'test content', + encoding: 'cl100k_base', + }); + }); + }); +}); diff --git a/tests/core/metrics/calculateGitLogMetrics.test.ts b/tests/core/metrics/calculateGitLogMetrics.test.ts new file mode 100644 index 000000000..a0c9cd176 --- /dev/null +++ b/tests/core/metrics/calculateGitLogMetrics.test.ts @@ -0,0 +1,419 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { RepomixConfigMerged } from '../../../src/config/configSchema.js'; +import type { GitLogResult } from '../../../src/core/git/gitLogHandle.js'; +import { calculateGitLogMetrics } from '../../../src/core/metrics/calculateGitLogMetrics.js'; +import { type TokenCountTask, countTokens } from '../../../src/core/metrics/workers/calculateMetricsWorker.js'; +import { logger } from '../../../src/shared/logger.js'; +import type { TaskRunner, WorkerOptions } from '../../../src/shared/processConcurrency.js'; + +vi.mock('../../../src/shared/logger'); + +const mockInitTaskRunner = (_options: WorkerOptions): TaskRunner => { + return { + run: async (task: TokenCountTask) => { + return await countTokens(task); + }, + cleanup: async () => { + // Mock cleanup - no-op for tests + }, + }; +}; + +describe('calculateGitLogMetrics', () => { + const mockConfig: RepomixConfigMerged = { + input: { + maxFileSize: 50 * 1024 * 1024, + }, + output: { + filePath: 'test-output.txt', + style: 'xml', + parsableStyle: false, + headerText: '', + instructionFilePath: '', + fileSummary: true, + directoryStructure: true, + files: true, + removeComments: false, + removeEmptyLines: false, + compress: false, + topFilesLength: 10, + showLineNumbers: false, + truncateBase64: false, + copyToClipboard: false, + includeEmptyDirectories: false, + tokenCountTree: false, + git: { + sortByChanges: true, + sortByChangesMaxCommits: 100, + includeDiffs: false, + includeLogs: true, + includeLogsCount: 50, + }, + }, + include: ['**/*'], + ignore: { + useGitignore: true, + useDefaultPatterns: true, + customPatterns: [], + }, + security: { + enableSecurityCheck: true, + }, + tokenCount: { + encoding: 'o200k_base' as const, + }, + cwd: '/test/project', + }; + + const mockTaskRunner = mockInitTaskRunner({ numOfTasks: 1, workerPath: '', runtime: 'worker_threads' }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('when git logs are disabled', () => { + it('should return 0 when includeLogs is false', async () => { + const configWithDisabledLogs = { + ...mockConfig, + output: { + ...mockConfig.output, + git: { + ...mockConfig.output.git, + includeLogs: false, + }, + }, + }; + + const gitLogResult: GitLogResult = { + logContent: 'some log content', + commits: [], + }; + + const result = await calculateGitLogMetrics(configWithDisabledLogs, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + }); + + it('should return 0 when git config is undefined', async () => { + const configWithoutGit = { + ...mockConfig, + output: { + ...mockConfig.output, + git: undefined, + }, + } as RepomixConfigMerged; + + const gitLogResult: GitLogResult = { + logContent: 'some log content', + commits: [], + }; + + const result = await calculateGitLogMetrics(configWithoutGit, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + }); + }); + + describe('when git log result is unavailable', () => { + it('should return 0 when gitLogResult is undefined', async () => { + const result = await calculateGitLogMetrics(mockConfig, undefined, { + taskRunner: mockTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + }); + + it('should return 0 when logContent is empty', async () => { + const gitLogResult: GitLogResult = { + logContent: '', + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + }); + + it('should return 0 when logContent is undefined', async () => { + const gitLogResult = { + logContent: undefined as unknown as string, + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + }); + }); + + describe('when processing git logs', () => { + it('should calculate tokens for git log content', async () => { + const gitLogResult: GitLogResult = { + logContent: 'commit abc123\nAuthor: Test User\nDate: 2023-01-01\n\nTest commit message', + commits: [], + }; + + const mockTaskRunnerSpy = vi.fn().mockResolvedValueOnce(15); + + const customTaskRunner: TaskRunner = { + run: mockTaskRunnerSpy, + cleanup: async () => {}, + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: customTaskRunner, + }); + + expect(mockTaskRunnerSpy).toHaveBeenCalledTimes(1); + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'commit abc123\nAuthor: Test User\nDate: 2023-01-01\n\nTest commit message', + encoding: 'o200k_base', + }); + expect(result).toEqual({ gitLogTokenCount: 15 }); + }); + + it('should handle large log content correctly', async () => { + const largeLogContent = `${'commit '.repeat(1000)}large commit log`; + const gitLogResult: GitLogResult = { + logContent: largeLogContent, + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result.gitLogTokenCount).toBeGreaterThan(0); + expect(typeof result.gitLogTokenCount).toBe('number'); + }); + + it('should handle complex git log with multiple commits', async () => { + const complexLogContent = `commit abc123def456 +Author: John Doe +Date: Mon Jan 1 12:00:00 2023 +0000 + + Add new feature for user authentication + + - Implemented OAuth2 integration + - Added user session management + - Updated security middleware + +commit def456ghi789 +Author: Jane Smith +Date: Sun Dec 31 18:30:00 2022 +0000 + + Fix critical bug in payment processing + + - Resolved transaction timeout issue + - Added proper error handling + - Improved logging for debugging`; + + const gitLogResult: GitLogResult = { + logContent: complexLogContent, + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result.gitLogTokenCount).toBeGreaterThan(0); + expect(typeof result.gitLogTokenCount).toBe('number'); + }); + }); + + describe('error handling', () => { + it('should return 0 when task runner fails', async () => { + const gitLogResult: GitLogResult = { + logContent: 'some log content', + commits: [], + }; + + const errorTaskRunner: TaskRunner = { + run: vi.fn().mockRejectedValue(new Error('Task runner failed')), + cleanup: async () => {}, + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: errorTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + expect(logger.error).toHaveBeenCalledWith('Failed to calculate git log metrics:', expect.any(Error)); + }); + + it('should handle network timeout errors gracefully', async () => { + const gitLogResult: GitLogResult = { + logContent: 'test log content', + commits: [], + }; + + const timeoutError = new Error('Request timeout'); + const errorTaskRunner = { + run: vi.fn().mockRejectedValue(timeoutError), + cleanup: async () => {}, + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: errorTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + expect(logger.error).toHaveBeenCalledWith('Failed to calculate git log metrics:', timeoutError); + }); + }); + + describe('logging', () => { + it('should log trace messages for successful calculation', async () => { + const gitLogResult: GitLogResult = { + logContent: 'test log content', + commits: [], + }; + + await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(logger.trace).toHaveBeenCalledWith('Starting git log token calculation using worker'); + expect(logger.trace).toHaveBeenCalledWith( + expect.stringMatching(/Git log token calculation completed in \d+\.\d+ms/), + ); + }); + + it('should not log completion message on error', async () => { + const gitLogResult: GitLogResult = { + logContent: 'test content', + commits: [], + }; + + const errorTaskRunner = { + run: vi.fn().mockRejectedValue(new Error('Test error')), + cleanup: async () => {}, + }; + + await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: errorTaskRunner, + }); + + expect(logger.trace).toHaveBeenCalledWith('Starting git log token calculation using worker'); + expect(logger.trace).not.toHaveBeenCalledWith(expect.stringMatching(/Git log token calculation completed/)); + }); + }); + + describe('encoding configuration', () => { + it('should use correct encoding from config', async () => { + const configWithDifferentEncoding = { + ...mockConfig, + tokenCount: { + encoding: 'cl100k_base' as const, + }, + }; + + const gitLogResult: GitLogResult = { + logContent: 'test log content', + commits: [], + }; + + const mockTaskRunnerSpy = vi.fn().mockResolvedValueOnce(10); + + const customTaskRunner: TaskRunner = { + run: mockTaskRunnerSpy, + cleanup: async () => {}, + }; + + await calculateGitLogMetrics(configWithDifferentEncoding, gitLogResult, { + taskRunner: customTaskRunner, + }); + + expect(mockTaskRunnerSpy).toHaveBeenCalledWith({ + content: 'test log content', + encoding: 'cl100k_base', + }); + }); + }); + + describe('return value structure', () => { + it('should always return an object with gitLogTokenCount property', async () => { + const gitLogResult: GitLogResult = { + logContent: 'test content', + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result).toHaveProperty('gitLogTokenCount'); + expect(typeof result.gitLogTokenCount).toBe('number'); + }); + + it('should return consistent structure on error', async () => { + const gitLogResult: GitLogResult = { + logContent: 'test content', + commits: [], + }; + + const errorTaskRunner = { + run: vi.fn().mockRejectedValue(new Error('Test error')), + cleanup: async () => {}, + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: errorTaskRunner, + }); + + expect(result).toEqual({ gitLogTokenCount: 0 }); + expect(Object.keys(result)).toEqual(['gitLogTokenCount']); + }); + }); + + describe('edge cases', () => { + it('should handle very short log content', async () => { + const gitLogResult: GitLogResult = { + logContent: 'a', + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result.gitLogTokenCount).toBeGreaterThanOrEqual(0); + }); + + it('should handle log content with special characters', async () => { + const gitLogResult: GitLogResult = { + logContent: 'commit 🚀 emoji test\n\n日本語のコミットメッセヌゞ\n\nSpecial chars: ñáéíóú', + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result.gitLogTokenCount).toBeGreaterThan(0); + expect(typeof result.gitLogTokenCount).toBe('number'); + }); + + it('should handle log content with only whitespace', async () => { + const gitLogResult: GitLogResult = { + logContent: ' \n\t \r\n ', + commits: [], + }; + + const result = await calculateGitLogMetrics(mockConfig, gitLogResult, { + taskRunner: mockTaskRunner, + }); + + expect(result.gitLogTokenCount).toBeGreaterThanOrEqual(0); + }); + }); +}); From 3266ad1a155860eb8b1c5223c1e363f043cfa3d0 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 20:37:50 +0900 Subject: [PATCH 11/15] fix(test): make defaultActionWorker tests cross-platform compatible - Import path module and use path.resolve() for all path assertions - Fix failing tests on Windows CI by handling platform-specific path separators - Ensure tests work correctly on both Unix and Windows systems - Maintain test functionality while improving cross-platform compatibility This resolves CI failures on Windows builds while preserving test coverage. --- .../workers/defaultActionWorker.test.ts | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/tests/cli/actions/workers/defaultActionWorker.test.ts b/tests/cli/actions/workers/defaultActionWorker.test.ts index f7d2c81ad..c1d62bac8 100644 --- a/tests/cli/actions/workers/defaultActionWorker.test.ts +++ b/tests/cli/actions/workers/defaultActionWorker.test.ts @@ -1,3 +1,4 @@ +import path from 'node:path'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import defaultActionWorker, { type DefaultActionTask, @@ -159,7 +160,7 @@ describe('defaultActionWorker', () => { const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; expect(mockPack).toHaveBeenCalledWith( - ['/test/project/src', '/test/project/tests'], + [path.resolve('/test/project', 'src'), path.resolve('/test/project', 'tests')], mockConfig, expect.any(Function), ); @@ -182,7 +183,7 @@ describe('defaultActionWorker', () => { const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; - expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function)); + expect(mockPack).toHaveBeenCalledWith([path.resolve('/test/project', '.')], mockConfig, expect.any(Function)); expect(result).toEqual({ packResult: mockPackResult, config: mockConfig, @@ -226,10 +227,13 @@ describe('defaultActionWorker', () => { const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); - expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function), {}, [ - 'file1.txt', - 'file2.txt', - ]); + expect(mockPack).toHaveBeenCalledWith( + [path.resolve('/test/project', '.')], + mockConfig, + expect.any(Function), + {}, + ['file1.txt', 'file2.txt'], + ); expect(result).toEqual({ packResult: mockPackResult, config: mockConfig, @@ -255,7 +259,13 @@ describe('defaultActionWorker', () => { await defaultActionWorker(task); expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); - expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function), {}, ['file1.txt']); + expect(mockPack).toHaveBeenCalledWith( + [path.resolve('/test/project', '.')], + mockConfig, + expect.any(Function), + {}, + ['file1.txt'], + ); }); it('should throw error when multiple directories are specified with stdin', async () => { @@ -407,7 +417,11 @@ describe('defaultActionWorker', () => { await defaultActionWorker(task); expect(mockPack).toHaveBeenCalledWith( - ['/test/parent', '/test/project/current', '/test/project/child'], + [ + path.resolve('/test/project', '../parent'), + path.resolve('/test/project', './current'), + path.resolve('/test/project', 'child'), + ], mockConfig, expect.any(Function), ); @@ -426,7 +440,11 @@ describe('defaultActionWorker', () => { await defaultActionWorker(task); - expect(mockPack).toHaveBeenCalledWith(['/absolute/path1', '/absolute/path2'], mockConfig, expect.any(Function)); + expect(mockPack).toHaveBeenCalledWith( + [path.resolve('/test/project', '/absolute/path1'), path.resolve('/test/project', '/absolute/path2')], + mockConfig, + expect.any(Function), + ); }); }); }); From 7153526cc9b789d4dc281f2d571343a48bd4cd88 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 21:44:04 +0900 Subject: [PATCH 12/15] fix(test): correct stdin processing path expectations in defaultActionWorker tests - Fix stdin processing tests to expect raw cwd path instead of resolved path - Aligns test expectations with actual implementation where stdin uses [cwd] directly - Resolves Windows CI failures where path.resolve() was incorrectly applied to stdin cases - Maintains correct expectations for directory processing vs stdin processing paths This fixes the remaining Windows CI test failures while preserving test accuracy. --- tests/cli/actions/workers/defaultActionWorker.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli/actions/workers/defaultActionWorker.test.ts b/tests/cli/actions/workers/defaultActionWorker.test.ts index c1d62bac8..31f7848df 100644 --- a/tests/cli/actions/workers/defaultActionWorker.test.ts +++ b/tests/cli/actions/workers/defaultActionWorker.test.ts @@ -228,7 +228,7 @@ describe('defaultActionWorker', () => { expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); expect(mockPack).toHaveBeenCalledWith( - [path.resolve('/test/project', '.')], + ['/test/project'], mockConfig, expect.any(Function), {}, @@ -260,7 +260,7 @@ describe('defaultActionWorker', () => { expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); expect(mockPack).toHaveBeenCalledWith( - [path.resolve('/test/project', '.')], + ['/test/project'], mockConfig, expect.any(Function), {}, From c106ca8acbb54330220fcfdd5f9f398d5993dd5b Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 20 Sep 2025 12:44:54 +0000 Subject: [PATCH 13/15] [autofix.ci] apply automated fixes --- .../workers/defaultActionWorker.test.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/tests/cli/actions/workers/defaultActionWorker.test.ts b/tests/cli/actions/workers/defaultActionWorker.test.ts index 31f7848df..39d4e02c8 100644 --- a/tests/cli/actions/workers/defaultActionWorker.test.ts +++ b/tests/cli/actions/workers/defaultActionWorker.test.ts @@ -227,13 +227,10 @@ describe('defaultActionWorker', () => { const result = (await defaultActionWorker(task)) as DefaultActionWorkerResult; expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); - expect(mockPack).toHaveBeenCalledWith( - ['/test/project'], - mockConfig, - expect.any(Function), - {}, - ['file1.txt', 'file2.txt'], - ); + expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function), {}, [ + 'file1.txt', + 'file2.txt', + ]); expect(result).toEqual({ packResult: mockPackResult, config: mockConfig, @@ -259,13 +256,7 @@ describe('defaultActionWorker', () => { await defaultActionWorker(task); expect(mockReadFilePathsFromStdin).toHaveBeenCalledWith('/test/project'); - expect(mockPack).toHaveBeenCalledWith( - ['/test/project'], - mockConfig, - expect.any(Function), - {}, - ['file1.txt'], - ); + expect(mockPack).toHaveBeenCalledWith(['/test/project'], mockConfig, expect.any(Function), {}, ['file1.txt']); }); it('should throw error when multiple directories are specified with stdin', async () => { From 5898d6397c58d2b3f9531f408845284ad410b641 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 22:11:44 +0900 Subject: [PATCH 14/15] refactor(workers): improve code quality and type safety Address PR review feedback: - Fix worker path to use relative path instead of lib directory - Add proper function overloads for defaultActionWorker - Remove unsafe type assertions in worker code - Improve error handling with optional stack property - Extract log level validation logic to reduce duplication - Add NaN check for environment variable parsing All tests pass and linting issues resolved. --- .../actions/workers/defaultActionWorker.ts | 4 ++- src/core/file/fileSearch.ts | 5 ++-- src/core/metrics/calculateMetrics.ts | 2 +- src/shared/errorHandle.ts | 5 ++-- src/shared/logger.ts | 26 +++++++++---------- tests/core/file/fileSearch.test.ts | 8 +++--- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/cli/actions/workers/defaultActionWorker.ts b/src/cli/actions/workers/defaultActionWorker.ts index 9e43868f3..be3c1fa4e 100644 --- a/src/cli/actions/workers/defaultActionWorker.ts +++ b/src/cli/actions/workers/defaultActionWorker.ts @@ -36,6 +36,8 @@ export interface PingResult { } // Function overloads for better type inference +function defaultActionWorker(task: DefaultActionTask): Promise; +function defaultActionWorker(task: PingTask): Promise; async function defaultActionWorker( task: DefaultActionTask | PingTask, ): Promise { @@ -48,7 +50,7 @@ async function defaultActionWorker( } // At this point, task is guaranteed to be DefaultActionTask - const { directories, cwd, config, cliOptions, isStdin } = task as DefaultActionTask; + const { directories, cwd, config, cliOptions, isStdin } = task; logger.trace('Worker: Using pre-loaded config:', config); diff --git a/src/core/file/fileSearch.ts b/src/core/file/fileSearch.ts index af5faeae6..6377e53bc 100644 --- a/src/core/file/fileSearch.ts +++ b/src/core/file/fileSearch.ts @@ -200,9 +200,10 @@ export const searchFiles = async ( absolute: false, dot: true, followSymbolicLinks: false, - }).catch((error) => { + }).catch((error: unknown) => { // Handle EPERM errors specifically - if (error.code === 'EPERM' || error.code === 'EACCES') { + const code = (error as NodeJS.ErrnoException | { code?: string })?.code; + if (code === 'EPERM' || code === 'EACCES') { throw new PermissionError( `Permission denied while scanning directory. Please check folder access permissions for your terminal app. path: ${rootDir}`, rootDir, diff --git a/src/core/metrics/calculateMetrics.ts b/src/core/metrics/calculateMetrics.ts index 8402f803b..64fa8fa3c 100644 --- a/src/core/metrics/calculateMetrics.ts +++ b/src/core/metrics/calculateMetrics.ts @@ -42,7 +42,7 @@ export const calculateMetrics = async ( deps.taskRunner ?? initTaskRunner({ numOfTasks: processedFiles.length, - workerPath: new URL('../../../lib/core/metrics/workers/calculateMetricsWorker.js', import.meta.url).href, + workerPath: new URL('./workers/calculateMetricsWorker.js', import.meta.url).href, runtime: 'worker_threads', }); diff --git a/src/shared/errorHandle.ts b/src/shared/errorHandle.ts index 8bdd4a566..e41fcec92 100644 --- a/src/shared/errorHandle.ts +++ b/src/shared/errorHandle.ts @@ -70,8 +70,9 @@ const isError = (error: unknown): error is Error => { const obj = error as Record; return ( typeof obj.message === 'string' && - typeof obj.stack === 'string' && - ('name' in obj ? typeof obj.name === 'string' : true) + // stack is optional across boundaries + (!('stack' in obj) || typeof obj.stack === 'string') && + (!('name' in obj) || typeof obj.name === 'string') ); }; diff --git a/src/shared/logger.ts b/src/shared/logger.ts index ba544b978..6be976a6a 100644 --- a/src/shared/logger.ts +++ b/src/shared/logger.ts @@ -96,18 +96,22 @@ export const setLogLevel = (level: RepomixLogLevel) => { * Set logger log level from workerData if valid. * This is used in worker threads where configuration is passed via workerData. */ +const isValidLogLevel = (level: number): level is RepomixLogLevel => { + return ( + level === repomixLogLevels.SILENT || + level === repomixLogLevels.ERROR || + level === repomixLogLevels.WARN || + level === repomixLogLevels.INFO || + level === repomixLogLevels.DEBUG + ); +}; + export const setLogLevelByWorkerData = () => { // Try to get log level from environment variable first (for child_process workers) const envLogLevel = process.env.REPOMIX_LOG_LEVEL; if (envLogLevel !== undefined) { const logLevel = Number(envLogLevel); - if ( - logLevel === repomixLogLevels.SILENT || - logLevel === repomixLogLevels.ERROR || - logLevel === repomixLogLevels.WARN || - logLevel === repomixLogLevels.INFO || - logLevel === repomixLogLevels.DEBUG - ) { + if (!Number.isNaN(logLevel) && isValidLogLevel(logLevel)) { setLogLevel(logLevel); return; } @@ -116,13 +120,7 @@ export const setLogLevelByWorkerData = () => { // Fallback to workerData for worker_threads if (Array.isArray(workerData) && workerData.length > 1 && workerData[1]?.logLevel !== undefined) { const logLevel = workerData[1].logLevel; - if ( - logLevel === repomixLogLevels.SILENT || - logLevel === repomixLogLevels.ERROR || - logLevel === repomixLogLevels.WARN || - logLevel === repomixLogLevels.INFO || - logLevel === repomixLogLevels.DEBUG - ) { + if (isValidLogLevel(logLevel)) { setLogLevel(logLevel); } } diff --git a/tests/core/file/fileSearch.test.ts b/tests/core/file/fileSearch.test.ts index df6328e8d..8f4c6a0e9 100644 --- a/tests/core/file/fileSearch.test.ts +++ b/tests/core/file/fileSearch.test.ts @@ -91,8 +91,8 @@ describe('fileSearch', () => { const mockFilePaths = ['src/file1.js', 'src/file2.js']; const mockEmptyDirs = ['src/empty', 'empty-root']; - vi.mocked(globby).mockImplementation(async (_, options) => { - if (options?.onlyDirectories) { + vi.mocked(globby).mockImplementation(async (_: unknown, options: unknown) => { + if ((options as Record)?.onlyDirectories) { return mockEmptyDirs; } return mockFilePaths; @@ -115,8 +115,8 @@ describe('fileSearch', () => { const mockFilePaths = ['src/file1.js', 'src/file2.js']; - vi.mocked(globby).mockImplementation(async (_, options) => { - if (options?.onlyDirectories) { + vi.mocked(globby).mockImplementation(async (_: unknown, options: unknown) => { + if ((options as Record)?.onlyDirectories) { throw new Error('Should not search for directories when disabled'); } return mockFilePaths; From 1badf53c3cfddc9eaf6518c1aea09a311f856a5a Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sat, 20 Sep 2025 22:58:31 +0900 Subject: [PATCH 15/15] refactor(cli): optimize ping task and improve error handling - Remove unnecessary config parameter from PingTask interface for better performance - Improve error serialization using util.inspect with safety limits - Update function signatures to match simplified ping interface - Add comprehensive error details formatting with depth and length limits These changes address PR review comments and improve worker efficiency. --- package.json | 2 ++ src/cli/actions/defaultAction.ts | 11 ++++------- src/cli/actions/workers/defaultActionWorker.ts | 4 ---- src/shared/errorHandle.ts | 17 ++++++++++++++++- .../actions/workers/defaultActionWorker.test.ts | 3 --- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 463e7ca3a..b21619523 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,8 @@ "repomix": "node --run build && node --enable-source-maps --trace-warnings bin/repomix.cjs", "repomix-src": "node --run repomix -- --include 'src,tests'", "repomix-website": "node --run repomix -- --include 'website'", + "time-node": "node --run build && time node bin/repomix.cjs", + "time-bun": "bun run build && time bun bin/repomix.cjs", "memory-check": "node --run repomix -- --verbose | grep Memory", "memory-check-one-file": "node --run repomix -- --verbose --include 'package.json' | grep Memory", "website": "docker compose -f website/compose.yml up --build", diff --git a/src/cli/actions/defaultAction.ts b/src/cli/actions/defaultAction.ts index 88d6ca56c..fa07959da 100644 --- a/src/cli/actions/defaultAction.ts +++ b/src/cli/actions/defaultAction.ts @@ -57,7 +57,7 @@ export const runDefaultAction = async ( try { // Wait for worker to be ready (Bun compatibility) - await waitForWorkerReady(taskRunner, { cwd, config }); + await waitForWorkerReady(taskRunner); // Create task for worker (now with pre-loaded config) const task: DefaultActionTask = { @@ -270,10 +270,9 @@ export const buildCliConfig = (options: CliOptions): RepomixConfigCli => { * Wait for worker to be ready by sending a ping request. * This is specifically needed for Bun compatibility due to ES module initialization timing issues. */ -const waitForWorkerReady = async ( - taskRunner: { run: (task: DefaultActionTask | PingTask) => Promise }, - context: { cwd: string; config: RepomixConfigMerged }, -): Promise => { +const waitForWorkerReady = async (taskRunner: { + run: (task: DefaultActionTask | PingTask) => Promise; +}): Promise => { const isBun = process.versions?.bun; if (!isBun) { // No need to wait for Node.js @@ -288,8 +287,6 @@ const waitForWorkerReady = async ( try { await taskRunner.run({ ping: true, - cwd: context.cwd, - config: context.config, }); logger.debug(`Worker initialization ping successful on attempt ${attempt}`); pingSuccessful = true; diff --git a/src/cli/actions/workers/defaultActionWorker.ts b/src/cli/actions/workers/defaultActionWorker.ts index be3c1fa4e..c69af69a3 100644 --- a/src/cli/actions/workers/defaultActionWorker.ts +++ b/src/cli/actions/workers/defaultActionWorker.ts @@ -21,8 +21,6 @@ export interface DefaultActionTask { export interface PingTask { ping: true; - cwd: string; - config: RepomixConfigMerged; } export interface DefaultActionWorkerResult { @@ -32,7 +30,6 @@ export interface DefaultActionWorkerResult { export interface PingResult { ping: true; - config: RepomixConfigMerged; } // Function overloads for better type inference @@ -45,7 +42,6 @@ async function defaultActionWorker( if ('ping' in task) { return { ping: true, - config: task.config, }; } diff --git a/src/shared/errorHandle.ts b/src/shared/errorHandle.ts index e41fcec92..938c59bf0 100644 --- a/src/shared/errorHandle.ts +++ b/src/shared/errorHandle.ts @@ -1,3 +1,4 @@ +import { inspect } from 'node:util'; import { z } from 'zod'; import { REPOMIX_DISCORD_URL, REPOMIX_ISSUES_URL } from './constants.js'; import { logger, repomixLogLevels } from './logger.js'; @@ -43,7 +44,21 @@ export const handleError = (error: unknown): void => { } else { // Unknown errors logger.error('✖ An unknown error occurred'); - logger.note('Stack trace:', error); + // Safely serialize unknown error objects + try { + logger.note( + 'Error details:', + inspect(error, { + depth: 3, + colors: false, + maxArrayLength: 10, + maxStringLength: 200, + breakLength: Number.POSITIVE_INFINITY, + }), + ); + } catch { + logger.note('Error details: [Error object could not be serialized]'); + } if (logger.getLogLevel() < repomixLogLevels.DEBUG) { logger.log(''); diff --git a/tests/cli/actions/workers/defaultActionWorker.test.ts b/tests/cli/actions/workers/defaultActionWorker.test.ts index 39d4e02c8..588d678a9 100644 --- a/tests/cli/actions/workers/defaultActionWorker.test.ts +++ b/tests/cli/actions/workers/defaultActionWorker.test.ts @@ -132,15 +132,12 @@ describe('defaultActionWorker', () => { it('should handle ping requests correctly', async () => { const pingTask: PingTask = { ping: true, - cwd: '/test/dir', - config: mockConfig, }; const result = (await defaultActionWorker(pingTask)) as PingResult; expect(result).toEqual({ ping: true, - config: mockConfig, }); }); });