fix(cli): run when reached through a symlinked entry path - #22
Merged
Conversation
The CLI guarded its entry point by comparing `import.meta.url` against `pathToFileURL(process.argv[1])`. Node canonicalises the former through symlinks but leaves the latter exactly as the caller wrote it, so the comparison fails whenever the CLI is reached through a symlink — the norm under pnpm, which links `node_modules/<pkg>` into `node_modules/.pnpm/…`. Whether it worked depended on which path the generated bin shim happened to use. `putio-roku` got a shim pointing at the real `.pnpm` path and works; a fresh `pnpm add -D @putdotio/vref` in `putio-ios` got one pointing at the symlink, where `vref describe`, `vref build`, and `vref validate` all exited 0 having done nothing. Same pnpm, same Node, same package checksum. Silent success is the dangerous part: a CI step running `vref build --check` would pass while validating nothing at all. Canonicalise both sides before comparing, via an exported `isDirectInvocation` so the behaviour is testable without spawning a process. A missing `argv[1]` falls through to the plain comparison rather than throwing during startup. Closes #21
There was a problem hiding this comment.
Pull request overview
This PR fixes a long-standing CLI entry-point guard bug where vref could silently no-op when invoked through a symlinked path (commonly produced by pnpm’s node_modules/@scope/pkg symlink layout). It introduces a small, testable helper that canonicalizes the entry path before comparing it to import.meta.url, preventing false negatives and ensuring vref build/validate/describe actually run in CI.
Changes:
- Added
isDirectInvocation(moduleUrl, entryPath)to canonicalize the entry path viarealpathSync()before comparing URLs. - Updated the CLI entry guard to use
isDirectInvocation(import.meta.url, process.argv[1]). - Added regression tests covering symlinked entry paths, unrelated paths,
undefined, and non-existent paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/cli.ts |
Adds isDirectInvocation and updates the CLI entry-point guard to handle symlinked invocation paths reliably. |
test/vref.test.ts |
Adds tests validating direct-invocation detection for symlinked paths and error-tolerant behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…main Canonicalising the entry path unconditionally broke a supported mode: with `node --preserve-symlinks-main`, Node deliberately keeps `import.meta.url` on the symlink, so resolving only `argv[1]` made the two disagree and skipped `main` silently — the same class of failure this change set out to remove. Compare the raw paths first, then fall back to the canonical comparison, so both the pnpm symlinked-shim case and the preserve-symlinks case run. Refs #21
The comment named only a non-existent argv[1], but the catch also covers unreadable paths and symlink loops. Returning false in every case is deliberate: an entry path that cannot be resolved is not proven to be this module, so fail closed instead of throwing during startup. Addresses a review comment on #22.
Contributor
|
🎉 This PR is included in version 1.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Closes #21
Summary
vref's CLI guarded its entry point with:Node canonicalises
import.meta.urlthrough symlinks but leavesprocess.argv[1]exactly as the caller wrote it. So the comparison fails whenever the CLI is reached via a symlink — which is the normal case under pnpm, wherenode_modules/<pkg>links intonode_modules/.pnpm/….Whether the CLI worked came down to which path the generated bin shim used:
…/.bin/../.pnpm/@putdotio+vref@1.1.0/…/dist/cli.mjs…/.bin/../@putdotio/vref/dist/cli.mjsBoth occur in practice.
putio-rokugot the first. A freshpnpm add -D @putdotio/vrefinputio-iosgot the second, wherevref describe,vref buildandvref validateall silently no-op — same pnpm 11.2.2, same Node 24.18.0, same package checksum.The silence is the dangerous part: a CI step running
vref build --checkpasses while validating nothing.Changed
isDirectInvocation(moduleUrl, entryPath)canonicalises both sides before comparing, so the behaviour is testable without spawning a processargv[1]falls through to the plain comparison instead of throwing during startupReview aids
Built
dist/cli.mjs, symlinked it, and invoked through the link — the case that produced no output before:Verification
pnpm run verifygreen — format, lint, typecheck, 22 tests (3 new),npm pack --dry-run.New tests cover a symlinked entry path (direct invocation), the real path (still direct), an unrelated path and
undefined(both not direct), and a non-existent path (returns false rather than throwing).Complexity
Low. One guard, one exported helper.