fix: use fileURLToPath for Windows path resolution#101
Merged
pbakaus merged 2 commits intopbakaus:mainfrom Apr 27, 2026
Merged
Conversation
On Windows, `new URL(import.meta.url).pathname` returns `/C:/...` (with a leading slash). Passing that to `path.resolve()` or `path.join()` causes Node to prepend the drive letter again, producing doubled paths like `C:\C:\Users\...\detect-antipatterns-browser.js`. Replace both occurrences (puppeteer scan at ~L2690 and live detect at ~L3506) with `fileURLToPath(import.meta.url)` from `node:url`, which correctly strips the leading slash on Windows while remaining a no-op on POSIX. Add regression tests verifying the source no longer uses the raw `.pathname` accessor for local path construction and that `fileURLToPath` handles both Windows and POSIX file URLs correctly. Closes pbakaus#95
- Added tests/windows-path-fix.test.js to package.json's test script so the regression suite actually runs in CI; without this the file lived on disk but no command picked it up. Verified with bun run test: 170 bun tests / 3 files, all green. - Rebased onto current main. The PR's second hunk (live-mode browser script load) no longer applies because that code path was extracted into source/skills/impeccable/scripts/live-*.mjs during the live-mode rewrite. The remaining puppeteer site at line 2700 still had the bug and now uses fileURLToPath, matching the PR's intent. - The added test file's mix of ESM imports + require/__dirname runs cleanly under Bun's test runner; left as-is to preserve the PR's authorship. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2ca565f to
6682638
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
npx impeccable detect <url>fails on Windows with:Note the doubled
C:\C:\.Fixes #95.
Root Cause
Two locations in
src/detect-antipatterns.mjsconstruct the path todetect-antipatterns-browser.jsusing:On Windows,
new URL("file:///C:/foo/bar").pathnamereturns/C:/foo/bar(with a leading slash). When passed topath.resolve()orpath.join(), Node prepends the drive letter again, producingC:\C:\....Fix
Replace both occurrences (~L2690 puppeteer scan and ~L3506 live detect) with
fileURLToPath(import.meta.url)fromnode:url, which correctly strips the leading slash on Windows while remaining a no-op on POSIX.The import is added inside the existing
!IS_BROWSERguard alongsidefsandpath, so the browser bundle is unaffected.Testing
Added
tests/windows-path-fix.test.jswith 4 regression tests:fileURLToPath+path.resolvenever producesC:\C:\patternsfileURLToPathreturns correct paths on Linux/macOSfile://URLpath.(resolve|join|dirname)(new URL(import.meta.url).pathnamepatterns in the sourceAll existing tests pass (157 pass, 1 pre-existing unrelated failure in jsdom stylesheet test).
Note
Low Risk
Low risk: a small Node-only path-resolution change plus a regression test; main risk is if the resolved path differs in unusual runtime/bundling environments.
Overview
Fixes Windows URL-scanning failures caused by
import.meta.urlpathname handling by switching local path construction tofileURLToPath(import.meta.url)when locatingdetect-antipatterns-browser.js.Adds a Bun regression test (
tests/windows-path-fix.test.js) and wires it into thenpm testscript to prevent drive-letter doubling from reappearing.Reviewed by Cursor Bugbot for commit 6682638. Bugbot is set up for automated code reviews on this repo. Configure here.