fix: run the CLI when invoked through a symlink (npx / installed bin) - #54
Merged
Conversation
npm installs the bin as a symlink (node_modules/.bin/cdb-converter -> ../cdb-converter/dist/cli.mjs) and npx goes through that link. Node puts the symlink path in process.argv[1] but the real path in import.meta.url, and resolve() normalises without dereferencing, so the entry-point guard was always false: the module was imported, run() never called, exit 0 with no output and no file written. Dereference both sides with realpathSync before comparing. The guard is repaired, not removed: importing the module as a library still must not run the converter. Two edge cases are handled: realpathSync throws on paths that do not exist (fall back to the resolved path), and Windows drive-letter casing can differ (compare case-insensitively on win32). Add an end-to-end regression that spawns the built CLI through a symlink, as npm/npx do. Calling run() in-process cannot observe this bug, which is why it shipped. Against the previous code, 4 of its 5 tests fail; the library-import test passes in both, covering the guard it protects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes the CLI entrypoint guard so the converter actually runs when invoked via npx or an installed bin symlink, while still preventing accidental execution when the module is imported programmatically. It also adds an end-to-end regression test that exercises the “symlinked entrypoint” scenario by spawning the built CLI through a symlink.
Changes:
- Update
src/cli.tsentrypoint detection to dereference symlinks (viarealpathSync) and to compare Windows paths case-insensitively. - Add an end-to-end Vitest suite that builds the project, creates a
bin/symlink todist/cli.mjs, and validates conversions +--help/--version. - Add a “library import” check to ensure importing the CLI module doesn’t run conversion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/cli.ts |
Fixes the direct-run guard to work when executed via a symlinked entrypoint (npx/installed bin). |
test/cliEntrypoint.test.ts |
Adds an end-to-end regression test that spawns the built CLI through a symlink and validates expected behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
mpicciolli
force-pushed
the
fix/cli-entrypoint-symlink
branch
from
August 14, 2026 20:21
ea87587 to
845229d
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
The CLI did nothing when launched via
npxor the installed binary: exit code 0, no output, no file written — not even--help. Onlynode dist/cli.mjs in outworked.Cause
The entry-point guard at the end of
src/cli.ts:npm installs the bin as a symlink (
node_modules/.bin/cdb-converter→../cdb-converter/dist/cli.mjs), and npx goes through it. Node puts the symlink path inprocess.argv[1]but the real path inimport.meta.url.resolve()normalises without dereferencing, so the two URLs never matched and the guard was always false — the module was imported,run()was never called, exit 0.Verified independently with a probe script:
Fix
Dereference both sides with
realpathSyncbefore comparing. The guard is repaired, not removed — importing the module as a library (import { run } from "cdb-converter") still must not run the converter. Two edge cases handled:realpathSyncthrows on paths that do not exist → fall back to the resolved path.win32.isDirectRunis now a pure exported function takingargv1andmoduleUrl.Regression test
test/cliEntrypoint.test.tsspawns the built CLI through a symlink (bin/cdb-converter -> dist/cli.mjs, what npm creates), in a subprocess, on a real.cdbfixture, asserting the output file exists and the exit code is 0. A test callingrun()in-process cannot observe this bug — which is exactly why it shipped.Proof it catches it: with the previous code, 4 of its 5 tests fail. The fifth — the library import that must run nothing — passes in both, so the guard it protects stays covered.
It is a separate file because it is the only test in the repo that builds, writes to a temp dir and spawns subprocesses; putting its build
beforeAllintest/cli.test.tswould have made the pureparseArgsunit tests depend ondist/and pay the build on every run, including watch mode. Happy to merge the files if preferred.Verification
Full suite: 83 tests / 11 files, all passing.
End-to-end from a temp dir —
npm pack, install the tarball, run the installed binary (symlink confirmed):--help--version0.3.0, exit 0base.cdb base.sqliteTables : 136, 1,871,872 B written, exit 0base.sqlite roundtrip.cdbbase.cdb(default output)base.sqlitewrittenOut of scope
Noted while verifying, not changed here: the tarball ships whatever
dist/is in the working tree (noprepublishOnly), so a release depends on a manualnpm run buildfirst.