fix: use temp directory for jscpd output on Windows (#270)#271
fix: use temp directory for jscpd output on Windows (#270)#271
Conversation
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.
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.
There was a problem hiding this comment.
Pull request overview
Fixes a Windows-specific jscpd invocation bug in the slop-detection CLI enhancers by avoiding null-device output paths and using a temporary output directory instead.
Changes:
- Switch
runDuplicateDetection()to writejscpdoutput into a temp directory (os.tmpdir()+mkdtempSync) and remove it in afinallyblock. - Add regression tests covering temp directory creation/cleanup and ensuring no
NUL//dev/nulloutput path usage. - Document the fix in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/patterns/cli-enhancers.js | Replaces --output NUL//dev/null with a temp output directory and cleans it up reliably. |
| tests/cli-enhancers.test.js | Adds regression tests validating temp dir lifecycle and absence of null-device output paths. |
| CHANGELOG.md | Adds an Unreleased “Fixed” entry describing the Windows jscpd output fix and tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // On Windows, NUL passed via execFileSync (no shell) is treated as a literal filename, | ||
| // creating a mangled path in the working directory. |
There was a problem hiding this comment.
The explanatory comment is a bit inaccurate: execFileSync doesn’t treat NUL as a filename—jscpd interprets the --output value and ends up creating the mangled path on Windows. Consider rewording to attribute the behavior to jscpd and clarify that NUL//dev/null aren’t valid output paths for jscpd in this mode.
| // On Windows, NUL passed via execFileSync (no shell) is treated as a literal filename, | |
| // creating a mangled path in the working directory. | |
| // jscpd interprets the value passed to --output itself, so using NUL or /dev/null here | |
| // results in jscpd creating a mangled path on Windows rather than discarding the output. |
|
|
||
| it('should use temp directory for jscpd output instead of NUL', () => { | ||
| const mkdtempSpy = jest.spyOn(fs, 'mkdtempSync'); | ||
| const rmSpy = jest.spyOn(fs, 'rmSync').mockImplementation(() => {}); |
There was a problem hiding this comment.
These tests spy on fs.rmSync but also mock it to a no-op. Since runDuplicateDetection() uses a real fs.mkdtempSync(), this can leave real temp directories behind on disk (cleanup runs but is stubbed). Prefer spying without overriding implementation (call-through), or ensure the created temp dir is actually removed in test teardown.
| const rmSpy = jest.spyOn(fs, 'rmSync').mockImplementation(() => {}); | |
| const rmSpy = jest.spyOn(fs, 'rmSync'); |
Summary
runDuplicateDetectioninlib/patterns/cli-enhancers.jscreating a mangled filename on Windows when--output NULwas passed to jscpd viaexecFileSync(no shell interpretation)NUL//dev/null) with a temp directory viaos.tmpdir()that is cleaned up in afinallyblockmkdtempSyncinside try-catch to preserve graceful degradation when tmpdir creation failsTest Plan
npm run validate)/deslopon Windows, confirm nonulor mangled-path files in project rootNotes
2>/dev/null) is in the deslop repo, not agentsys - tracked as follow-uplib/patterns/fix will propagate to deslop and other repos via agent-core syncCloses #270