ReviewOS

also looking at this

pantry-pm/pantry

fix(discovery): a package's own bin programs outrank its nested npm copies

#271
Merged glennmichael123 wants to merge fix/269-npm-precedence into main
1 file +65 -9
6 hours ago

Follow-up to 31f8448 (Refs #269). That commit made discoverNpmBinaries work; this fixes the precedence sitting behind it, which was latent only while the nested scan always came back empty.

The regression

discoverBinaries consults the npm layout first and returns it exclusively:

// First, try to find npm binaries (these take precedence)
const npm_bins = try discoverNpmBinaries(allocator, package_dir);
if (npm_bins.len > 0) return npm_bins;

packages/ts-pantry/src/recipes/npmjs.com.ts installs to /lib/node_modules/npm and keeps its own /bin, where it deliberately replaces npx:

our shim fixes a bug where npx doesn't work if ~/.local/lib doesn't exist

Upstream npm's nested bin/ holds seven executables (from the published tarball):

-rwxr-xr-x npm          -rwxr-xr-x npm-cli.js
-rwxr-xr-x npx          -rwxr-xr-x npm-prefix.js
-rwxr-xr-x npm.cmd      -rwxr-xr-x npx-cli.js
-rwxr-xr-x npx.cmd

So on current main, installing npmjs.com activates commands literally named npm-cli.js, npm-prefix.js and npx.cmd, resolves npx to upstream's instead of the shim, and never scans bin/ at all.

This is not a reading of the code. The regression test added here fails on stock main at eafe53d:

install.symlink.test.discoverBinaries keeps a package's own bin programs over its nested npm copies
       expected 2, found 3

— the three being the nested npm, npx and npm-cli.js, in place of the package's own two.

The fix

The npm layout becomes a fallback, consulted only when the package ships no bin/ or sbin/ programs of its own. A package that has both keeps exactly the behaviour it has today. A package carrying only the nested layout is unaffected — that is the case 31f8448's own relocatable_tree_test.zig regression covers, and it still passes.

Additive discovery was the other option and is deliberately not taken: it would still activate npm-cli.js and npm-prefix.js as global commands, since those have no bin/ counterpart to be shadowed by. Deciding which nested filenames are commands is policy this change has no basis to invent.

Verification

  • zig build test: 657 pass, 6 skip, 2 fail — the two are the pre-existing shell.install_route bash/zsh cases, which fail identically on main on this machine.
  • zig build test:relocatable passes, so the nested-only discovery path #269 fixed is intact.
  • No allocator leaks.
  • The new test is a filesystem regression mirroring the npmjs.com layout — own bin/npm and bin/npx alongside a nested lib/node_modules/npm/bin containing npm, npx and npm-cli.js — asserting the resolved commands come from the package's own bin/ and that no returned path contains node_modules.

Refs, not Closes: #269's own acceptance is satisfied by 31f8448, and it stays open for the independent review and release verification it asks for.

🤖 Generated with Claude Code

1 changed file on the files tab, with 0 review threads.

Sign in to comment on this pull request.