Skip to content

fix(cli): derive bundled file paths from file URLs - #156

Open
FurkanKra wants to merge 1 commit into
nodejs:mainfrom
FurkanKra:fix/cli-file-url-paths
Open

FurkanKra wants to merge 1 commit into
nodejs:mainfrom
FurkanKra:fix/cli-file-url-paths

Conversation

@FurkanKra

Copy link
Copy Markdown

--help and --version crash with an unhandled ENOENT on every Windows
install, and on any platform where the install path contains a space or a
non-ASCII character.

> core-validate-commit --version
Error: ENOENT: no such file or directory, open 'C:\C:\tmp\cvc\package.json'
    at async open (node:internal/fs/promises:1281:25)
    at async Object.readFile (node:internal/fs/promises:1929:14)

Cause

Both flags build their path with:

path.join(new URL(import.meta.url).pathname, '../usage.txt')

URL.pathname is a URL component, not a filesystem path:

  1. It keeps percent-encoding. A path containing a space comes back as
    /home/user/my%20project/..., which fs.readFile then takes literally.
  2. On Windows it keeps the leading / before the drive letter
    (file:///C:/x/C:/x). path.join treats that as relative and
    prefixes the cwd, producing C:\C:\x.

(2) fires on every Windows install regardless of the path, so both flags are
unconditionally broken there.

Fix

fs.readFile accepts a file: URL directly, so the conversion is unnecessary:

const usagePath = new URL('usage.txt', import.meta.url)

This is the pattern already used in test/cli-test.js for reading
package.json. node:path had no other use in bin/cmd.js, so its import is
dropped — otherwise standard fails pretest on no-unused-vars.

Why CI never caught this

There are already tests for --help and --version, but .github/workflows/node.js.yml
does not pass runs-on to pkgjs/action, whose default is ubuntu-latest.
The matrix is Linux-only.

Rather than add a Windows runner in this PR, the added regression test runs the
CLI from a directory whose name contains a space, which triggers the same
decoding bug on POSIX. It copies bin/, lib/ and package.json into a
mkdtemp directory created inside the repository — deliberately inside, so
that Node's upward node_modules lookup still resolves gitlint-parser-base
without a separate install. It is removed in a finally block.

Verification

  • Windows 11, Node.js 25.9: --help and --version crash before the change,
    succeed after. Reproduced from a plain ASCII path (C:\tmp\cvc) as well, to
    confirm it is not specific to my home directory.
  • npx standard passes.
  • Linux, via CI on my fork:
    • test only, without the fix → fails
      with ENOENT ... tmp%20cli%20gvOBJc/package.json
    • test plus the fix → passes

Not covered: I did not run the full suite on Windows. That is a separate
problem — the CLI tests use spawn('./bin/cmd.js', ...), and Windows cannot
execute a .js file directly, so the suite does not run there at all. I left
that out of this PR deliberately; happy to open a separate issue for it.

Happy to drop the test, or switch the fix to import.meta.dirname, if you'd
prefer either.

AI disclosure

Per the AI use policy:
I used Claude Code to investigate this bug class and draft the change. I
reviewed every line, reproduced the failure myself on Windows before and after,
and set up the two fork CI runs linked above to confirm the test is genuinely
red without the fix rather than trusting that claim. I can explain each part of
the change on request.

`--help` and `--version` built their file paths with
`path.join(new URL(import.meta.url).pathname, ...)`. `URL.pathname` is a
URL component, not a filesystem path: it keeps percent-encoding, and on
Windows it keeps the leading `/` in front of the drive letter. Both flags
therefore crashed with an unhandled `ENOENT` on every Windows install,
and on any platform when the install path contains a space or a
non-ASCII character.

Pass the `file:` URL to `fs.readFile` directly instead, matching the
pattern already used in `test/cli-test.js`. `node:path` is no longer used
in this file, so its import is dropped.

Add a regression test that runs the CLI from a directory whose name
contains a space, so the decoding is not a no-op on POSIX either. CI only
runs on `ubuntu-latest`, which is why the existing `--help` and
`--version` tests never caught this.

Signed-off-by: bawdy <[email protected]>
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.

1 participant