diff --git a/CHANGELOG.md b/CHANGELOG.md index 89ba266e..f0e60b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Windows jscpd output bug (#270)** - Fixed `runDuplicateDetection` creating a mangled filename on Windows when `--output NUL` was passed to jscpd via `execFileSync` (no shell). Replaced platform-specific null device with a temp directory via `os.tmpdir()` that is cleaned up in a `finally` block. Added 5 regression tests for temp directory lifecycle. + - **task-discoverer**: Exclude issues that already have an open PR from discovery results (GitHub source only). Detection uses branch name suffix, PR body closing keywords (`closes/fixes/resolves #N`), and PR title `(#N)` convention. Fixes #236. - **`/debate` 240s timeout enforcement** — All tool invocations in the debate workflow now enforce a hard 240-second timeout. Round 1 proposer timeouts abort the debate; round 1 challenger timeouts proceed with an uncontested position; round 2+ timeouts synthesize from completed rounds. Added "all rounds timeout" error path (`[ERROR] Debate failed: all tool invocations timed out.`). Timeout handling is consistent across the Claude Code command, OpenCode adapter, Codex adapter, and the `debate-orchestrator` agent. Restored missing "Round 2+: Challenger Follow-up" template in the OpenCode adapter SKILL.md. Fixes issue #233. diff --git a/__tests__/cli-enhancers.test.js b/__tests__/cli-enhancers.test.js index a65c3129..1620b667 100644 --- a/__tests__/cli-enhancers.test.js +++ b/__tests__/cli-enhancers.test.js @@ -511,6 +511,160 @@ describe('cli-enhancers', () => { const result = runDuplicateDetection('/nonexistent/path', { minLines: 3 }); expect(result === null || Array.isArray(result)).toBe(true); }); + + describe('temp directory for jscpd output (Windows NUL fix)', () => { + // The module destructures execFileSync from child_process at require time, + // so we must use jest.isolateModules to re-require with mocked child_process. + function loadWithMocks(execFileSyncMock) { + let mod; + jest.isolateModules(() => { + jest.doMock('child_process', () => ({ + ...jest.requireActual('child_process'), + execFileSync: execFileSyncMock, + execSync: jest.requireActual('child_process').execSync + })); + mod = require('../lib/patterns/cli-enhancers'); + }); + return mod; + } + + it('should use temp directory for jscpd output instead of NUL', () => { + const mkdtempSpy = jest.spyOn(fs, 'mkdtempSync'); + const rmSpy = jest.spyOn(fs, 'rmSync').mockImplementation(() => {}); + + const mockExecFileSync = jest.fn((cmd, args) => { + if (cmd === 'jscpd' && args && args.includes('--version')) { + return 'jscpd v1.0.0'; + } + if (cmd === 'jscpd' && args && args.includes('--reporters')) { + return JSON.stringify({ duplicates: [] }); + } + return ''; + }); + + const mod = loadWithMocks(mockExecFileSync); + mod.runDuplicateDetection('/some/repo'); + + // Verify mkdtempSync was called with os.tmpdir() prefix + expect(mkdtempSpy).toHaveBeenCalled(); + const mkdtempArg = mkdtempSpy.mock.calls[0][0]; + expect(mkdtempArg.startsWith(os.tmpdir())).toBe(true); + + // Verify --output arg uses temp dir, not NUL or /dev/null + const jscpdCall = mockExecFileSync.mock.calls.find( + call => call[0] === 'jscpd' && call[1] && call[1].includes('--output') + ); + expect(jscpdCall).toBeDefined(); + const outputIndex = jscpdCall[1].indexOf('--output'); + const outputValue = jscpdCall[1][outputIndex + 1]; + expect(outputValue).not.toBe('NUL'); + expect(outputValue).not.toBe('/dev/null'); + + mkdtempSpy.mockRestore(); + rmSpy.mockRestore(); + }); + + it('should clean up temp directory after successful execution', () => { + const rmSpy = jest.spyOn(fs, 'rmSync').mockImplementation(() => {}); + + const mockExecFileSync = jest.fn((cmd, args) => { + if (cmd === 'jscpd' && args && args.includes('--version')) { + return 'jscpd v1.0.0'; + } + if (cmd === 'jscpd' && args && args.includes('--reporters')) { + return JSON.stringify({ duplicates: [] }); + } + return ''; + }); + + const mod = loadWithMocks(mockExecFileSync); + mod.runDuplicateDetection('/some/repo'); + + // Verify rmSync was called with recursive and force options + const rmCalls = rmSpy.mock.calls.filter( + call => call[1] && call[1].recursive && call[1].force + ); + expect(rmCalls.length).toBeGreaterThan(0); + // The path should be in the temp directory + expect(rmCalls[0][0].startsWith(os.tmpdir())).toBe(true); + + rmSpy.mockRestore(); + }); + + it('should clean up temp directory even on jscpd failure', () => { + const rmSpy = jest.spyOn(fs, 'rmSync').mockImplementation(() => {}); + + const mockExecFileSync = jest.fn((cmd, args) => { + if (cmd === 'jscpd' && args && args.includes('--version')) { + return 'jscpd v1.0.0'; + } + if (cmd === 'jscpd' && args && args.includes('--reporters')) { + throw new Error('jscpd execution failed'); + } + return ''; + }); + + const mod = loadWithMocks(mockExecFileSync); + const result = mod.runDuplicateDetection('/some/repo'); + + // Should return null on failure + expect(result).toBeNull(); + // Should still clean up temp dir via finally block + const rmCalls = rmSpy.mock.calls.filter( + call => call[1] && call[1].recursive && call[1].force + ); + expect(rmCalls.length).toBeGreaterThan(0); + + rmSpy.mockRestore(); + }); + + it('should return null if temp directory creation fails', () => { + const mkdtempSpy = jest.spyOn(fs, 'mkdtempSync').mockImplementation(() => { + throw new Error('ENOSPC: no space left on device'); + }); + + const mockExecFileSync = jest.fn((cmd, args) => { + if (cmd === 'jscpd' && args && args.includes('--version')) { + return 'jscpd v1.0.0'; + } + return ''; + }); + + const mod = loadWithMocks(mockExecFileSync); + const result = mod.runDuplicateDetection('/some/repo'); + + expect(result).toBeNull(); + mkdtempSpy.mockRestore(); + }); + + it('should not reference NUL or /dev/null in jscpd args', () => { + jest.spyOn(fs, 'rmSync').mockImplementation(() => {}); + + const mockExecFileSync = jest.fn((cmd, args) => { + if (cmd === 'jscpd' && args && args.includes('--version')) { + return 'jscpd v1.0.0'; + } + if (cmd === 'jscpd' && args && args.includes('--reporters')) { + return JSON.stringify({ duplicates: [] }); + } + return ''; + }); + + const mod = loadWithMocks(mockExecFileSync); + mod.runDuplicateDetection('/some/repo'); + + // Verify no args contain NUL or /dev/null as the output path + for (const call of mockExecFileSync.mock.calls) { + const args = call[1] || []; + const outputIdx = args.indexOf('--output'); + if (outputIdx !== -1) { + const outputVal = args[outputIdx + 1]; + expect(outputVal.toLowerCase()).not.toBe('nul'); + expect(outputVal).not.toBe('/dev/null'); + } + } + }); + }); }); describe('runDependencyAnalysis', () => { diff --git a/lib/patterns/cli-enhancers.js b/lib/patterns/cli-enhancers.js index 558b2490..5d5ea544 100644 --- a/lib/patterns/cli-enhancers.js +++ b/lib/patterns/cli-enhancers.js @@ -13,6 +13,7 @@ */ const { execSync, execFileSync } = require('child_process'); +const os = require('os'); const path = require('path'); const fs = require('fs'); // Note: escapeDoubleQuotes no longer needed - using execFileSync with arg arrays @@ -345,16 +346,20 @@ function runDuplicateDetection(repoPath, options = {}) { const minLines = options.minLines || 5; const minTokens = options.minTokens || 50; + // Use temp directory for jscpd output to avoid platform-specific null device issues. + // On Windows, NUL passed via execFileSync (no shell) is treated as a literal filename, + // creating a mangled path in the working directory. + let tempOutputDir; try { + tempOutputDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jscpd-')); // Run jscpd with JSON output // Use execFileSync with arg array to prevent command injection (no shell interpretation) - const outputPath = process.platform === 'win32' ? 'NUL' : '/dev/null'; const args = [ repoPath, '--min-lines', String(minLines), '--min-tokens', String(minTokens), '--reporters', 'json', - '--output', outputPath, + '--output', tempOutputDir, '--silent' ]; @@ -393,6 +398,10 @@ function runDuplicateDetection(repoPath, options = {}) { } catch { // Tool execution failed return null; + } finally { + if (tempOutputDir) { + try { fs.rmSync(tempOutputDir, { recursive: true, force: true }); } catch {} + } } }