Conversation
`--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 <fndkra16@gmail.com>
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.
--helpand--versioncrash with an unhandledENOENTon every Windowsinstall, and on any platform where the install path contains a space or a
non-ASCII character.
Cause
Both flags build their path with:
URL.pathnameis a URL component, not a filesystem path:/home/user/my%20project/..., whichfs.readFilethen takes literally./before the drive letter(
file:///C:/x→/C:/x).path.jointreats that as relative andprefixes 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.readFileaccepts afile:URL directly, so the conversion is unnecessary:This is the pattern already used in
test/cli-test.jsfor readingpackage.json.node:pathhad no other use inbin/cmd.js, so its import isdropped — otherwise
standardfailspretestonno-unused-vars.Why CI never caught this
There are already tests for
--helpand--version, but.github/workflows/node.js.ymldoes not pass
runs-ontopkgjs/action, whose default isubuntu-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/andpackage.jsoninto amkdtempdirectory created inside the repository — deliberately inside, sothat Node's upward
node_moduleslookup still resolvesgitlint-parser-basewithout a separate install. It is removed in a
finallyblock.Verification
--helpand--versioncrash before the change,succeed after. Reproduced from a plain ASCII path (
C:\tmp\cvc) as well, toconfirm it is not specific to my home directory.
npx standardpasses.with
ENOENT ... tmp%20cli%20gvOBJc/package.jsonNot 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 cannotexecute a
.jsfile directly, so the suite does not run there at all. I leftthat 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'dprefer 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.