From 154bdebdc45a065832693a19638988217fbbebc1 Mon Sep 17 00:00:00 2001 From: Avi Fenesh Date: Wed, 25 Feb 2026 12:28:18 +0200 Subject: [PATCH 1/3] fix: use temp directory for jscpd output to fix Windows NUL bug (#270) On Windows, passing NUL to execFileSync (no shell) treats it as a literal filename, creating a mangled "nul" directory. Replace with fs.mkdtempSync temp directory that is cleaned up in a finally block. --- __tests__/cli-enhancers.test.js | 135 ++++++++++++++++++++++++++++++++ lib/patterns/cli-enhancers.js | 10 ++- 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/__tests__/cli-enhancers.test.js b/__tests__/cli-enhancers.test.js index a65c3129..c694ff37 100644 --- a/__tests__/cli-enhancers.test.js +++ b/__tests__/cli-enhancers.test.js @@ -511,6 +511,141 @@ 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 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..9f1b68db 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,19 @@ 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 "nul" directory in the working directory. + const tempOutputDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jscpd-')); try { // 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 +397,8 @@ function runDuplicateDetection(repoPath, options = {}) { } catch { // Tool execution failed return null; + } finally { + try { fs.rmSync(tempOutputDir, { recursive: true, force: true }); } catch {} } } From 2c1e51f59205d08da9508110e739b8ee6ae4b861 Mon Sep 17 00:00:00 2001 From: Avi Fenesh Date: Wed, 25 Feb 2026 12:36:25 +0200 Subject: [PATCH 2/3] fix: guard mkdtempSync inside try-catch for graceful degradation Move temp directory creation inside try block so mkdtempSync failures (e.g., ENOSPC) return null instead of throwing. Add conditional guard in finally block. Add test for mkdtempSync failure scenario. --- __tests__/cli-enhancers.test.js | 19 +++++++++++++++++++ lib/patterns/cli-enhancers.js | 9 ++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/__tests__/cli-enhancers.test.js b/__tests__/cli-enhancers.test.js index c694ff37..1620b667 100644 --- a/__tests__/cli-enhancers.test.js +++ b/__tests__/cli-enhancers.test.js @@ -618,6 +618,25 @@ describe('cli-enhancers', () => { 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(() => {}); diff --git a/lib/patterns/cli-enhancers.js b/lib/patterns/cli-enhancers.js index 9f1b68db..5d5ea544 100644 --- a/lib/patterns/cli-enhancers.js +++ b/lib/patterns/cli-enhancers.js @@ -348,9 +348,10 @@ function runDuplicateDetection(repoPath, options = {}) { // 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 "nul" directory in the working directory. - const tempOutputDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jscpd-')); + // 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 args = [ @@ -398,7 +399,9 @@ function runDuplicateDetection(repoPath, options = {}) { // Tool execution failed return null; } finally { - try { fs.rmSync(tempOutputDir, { recursive: true, force: true }); } catch {} + if (tempOutputDir) { + try { fs.rmSync(tempOutputDir, { recursive: true, force: true }); } catch {} + } } } From 9dcad7b8616e3d8fd9dca83eca5f081552d1e9ee Mon Sep 17 00:00:00 2001 From: Avi Fenesh Date: Wed, 25 Feb 2026 12:43:02 +0200 Subject: [PATCH 3/3] docs: add CHANGELOG entry for Windows jscpd output bug fix (#270) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) 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.