Skip to content

fix: use fileURLToPath for Windows path resolution#101

Merged
pbakaus merged 2 commits intopbakaus:mainfrom
voidborne-d:fix/windows-detect-path-drive-letter
Apr 27, 2026
Merged

fix: use fileURLToPath for Windows path resolution#101
pbakaus merged 2 commits intopbakaus:mainfrom
voidborne-d:fix/windows-detect-path-drive-letter

Conversation

@voidborne-d
Copy link
Copy Markdown
Contributor

@voidborne-d voidborne-d commented Apr 16, 2026

Problem

npx impeccable detect <url> fails on Windows with:

Error: Browser script not found at C:\C:\Users\...\detect-antipatterns-browser.js

Note the doubled C:\C:\.

Fixes #95.

Root Cause

Two locations in src/detect-antipatterns.mjs construct the path to detect-antipatterns-browser.js using:

path.dirname(new URL(import.meta.url).pathname)

On Windows, new URL("file:///C:/foo/bar").pathname returns /C:/foo/bar (with a leading slash). When passed to path.resolve() or path.join(), Node prepends the drive letter again, producing C:\C:\....

Fix

Replace both occurrences (~L2690 puppeteer scan and ~L3506 live detect) with fileURLToPath(import.meta.url) from node:url, which correctly strips the leading slash on Windows while remaining a no-op on POSIX.

The import is added inside the existing !IS_BROWSER guard alongside fs and path, so the browser bundle is unaffected.

Testing

Added tests/windows-path-fix.test.js with 4 regression tests:

  1. Drive-letter doubling guard — verifies fileURLToPath + path.resolve never produces C:\C:\ patterns
  2. POSIX file URL handling — verifies fileURLToPath returns correct paths on Linux/macOS
  3. import.meta.url contract — verifies it produces a valid file:// URL
  4. Source audit — asserts no remaining path.(resolve|join|dirname)(new URL(import.meta.url).pathname patterns in the source

All 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.url pathname handling by switching local path construction to fileURLToPath(import.meta.url) when locating detect-antipatterns-browser.js.

Adds a Bun regression test (tests/windows-path-fix.test.js) and wires it into the npm test script 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.

@voidborne-d voidborne-d requested a review from pbakaus as a code owner April 16, 2026 14:55
voidborne-d and others added 2 commits April 27, 2026 14:44
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>
@pbakaus pbakaus force-pushed the fix/windows-detect-path-drive-letter branch from 2ca565f to 6682638 Compare April 27, 2026 21:47
Copy link
Copy Markdown
Owner

@pbakaus pbakaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good contribution, thank you!

@pbakaus pbakaus merged commit e3d488e into pbakaus:main Apr 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: detect command fails with doubled drive letter in path

2 participants