fix(scripts): validate the bin name and that its file exists

Copilot review on #1997:

- A string-form `bin` is npm's shorthand for ONE command, named after the
  package, so ignoring `binName` let a typo silently resolve the package's
  only executable while bypassing the documented "declares no such bin"
  failure. Match it against the manifest's unscoped name instead.
- A declared bin whose file is absent (partial install) was returned
  unchecked. `process.execPath <missing>` still spawns and exits 1 with
  empty stdout, which `rawProjectListing` records as "no diagnostic
  captured" — reproducing the bogus every-file-uncovered report this
  helper exists to eliminate. Throw here, where the remedy is actionable.

Also restores `rawProjectListing`'s docblock, which the new `tscEntry`
block had orphaned above itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01387r3MotGk52hrkfjzHFsN
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-16 23:43:24 -04:00
commit 4b143ce043
3 changed files with 121 additions and 20 deletions

View file

@ -10,7 +10,7 @@
// (no npx resolution), and pinned to the locally installed package exactly as
// `npx --no-install` was.
import { readFileSync } from "node:fs";
import { existsSync, readFileSync } from "node:fs";
import { createRequire } from "node:module";
import path from "node:path";
@ -24,18 +24,42 @@ import path from "node:path";
* export `./bin/vite.js`, so `require.resolve("vite/bin/vite.js")` throws
* `ERR_PACKAGE_PATH_NOT_EXPORTED` (`./package.json` is always exported).
*
* Throws if the package isn't installed from `fromDir` or declares no such
* bin — the caller decides whether that's a hard "cannot measure" error or a
* fallback.
* Throws if the package isn't installed from `fromDir`, declares no such bin,
* or declares one whose file is missing — the caller decides whether that's a
* hard "cannot measure" error or a fallback.
*/
export function resolveNodeBin(pkg, binName, fromDir) {
const pkgPath = createRequire(path.join(fromDir, "package.json")).resolve(
`${pkg}/package.json`,
);
const bin = JSON.parse(readFileSync(pkgPath, "utf8")).bin;
// A string-form `bin` names a single command (the package's own name).
const rel = typeof bin === "string" ? bin : bin?.[binName];
const manifest = JSON.parse(readFileSync(pkgPath, "utf8"));
const { bin } = manifest;
// A string-form `bin` is npm's shorthand for ONE command, named after the
// package (unscoped). It does not make every requested `binName` valid, so
// match it rather than accepting whatever was asked for — otherwise a typo
// silently resolves the package's only executable instead of failing.
const rel =
typeof bin === "string"
? unscopedName(manifest.name ?? pkg) === binName
? bin
: undefined
: bin?.[binName];
if (typeof rel !== "string")
throw new Error(`${pkg} declares no "${binName}" bin in its package.json`);
return path.join(path.dirname(pkgPath), rel);
const entry = path.join(path.dirname(pkgPath), rel);
// A declared bin whose file is absent is a partial install, and it fails
// *silently* downstream: `process.execPath <missing>` still spawns fine and
// exits 1 with nothing on stdout, which `rawProjectListing` records as "no
// diagnostic captured" and turns into the bogus every-file-uncovered report
// this helper exists to eliminate. Fail here, where the remedy is actionable.
if (!existsSync(entry))
throw new Error(
`${pkg}'s "${binName}" bin points at a missing file: ${entry}`,
);
return entry;
}
/** `@scope/name` → `name`; an unscoped name is returned unchanged. */
function unscopedName(name) {
return name.startsWith("@") ? name.slice(name.indexOf("/") + 1) : name;
}

View file

@ -7,7 +7,14 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { existsSync } from "node:fs";
import {
existsSync,
mkdirSync,
mkdtempSync,
rmSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { resolveNodeBin } from "./resolve-node-bin.mjs";
@ -46,12 +53,21 @@ test("resolves vite's bin despite Vite 8's exports map (no deep bin export)", ()
assert.match(entry.split(path.sep).join("/"), /\/vite\/.*vite\.js$/);
});
test("resolves a string-form bin (prettier), ignoring the binName", () => {
test("resolves a string-form bin (prettier) under the package's own name", () => {
const entry = resolveNodeBin("prettier", "prettier", repoRoot);
assert.ok(existsSync(entry), `resolved entry does not exist: ${entry}`);
assert.match(entry.split(path.sep).join("/"), /\/prettier\//);
});
test("rejects a mismatched binName against a string-form bin", () => {
// npm's string shorthand declares ONE command, named after the package — so
// a typo must fail rather than silently resolving prettier's executable.
assert.throws(
() => resolveNodeBin("prettier", "prettierd", repoRoot),
/prettier declares no "prettierd" bin/,
);
});
test("throws when the package is not installed from fromDir", () => {
assert.throws(
() => resolveNodeBin("definitely-not-installed-anywhere", "x", repoRoot),
@ -66,3 +82,64 @@ test("throws when the package declares no such bin", () => {
/typescript declares no "vite" bin/,
);
});
// The remaining cases need a manifest shape the real installs don't have, so
// they run against a throwaway node_modules tree rather than a real package.
function fixtureDir(manifest, { createBinFile = false } = {}) {
const dir = mkdtempSync(path.join(tmpdir(), "resolve-node-bin-"));
const pkgDir = path.join(dir, "node_modules", manifest.name);
mkdirSync(pkgDir, { recursive: true });
writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify(manifest),
"utf8",
);
if (createBinFile) {
const rel =
typeof manifest.bin === "string"
? manifest.bin
: Object.values(manifest.bin)[0];
const target = path.join(pkgDir, rel);
mkdirSync(path.dirname(target), { recursive: true });
writeFileSync(target, "", "utf8");
}
// The consumer `package.json` `createRequire` is based at.
writeFileSync(
path.join(dir, "package.json"),
JSON.stringify({ name: "consumer" }),
"utf8",
);
return dir;
}
test("throws when a declared bin's file is missing (partial install)", () => {
// The silent case this helper exists to kill: `process.execPath <missing>`
// spawns fine and exits 1 with empty stdout, which downstream reads as "no
// diagnostic captured" and reports every tracked file as uncovered.
const dir = fixtureDir({ name: "ghostpkg", bin: { ghost: "bin/ghost.js" } });
try {
assert.throws(
() => resolveNodeBin("ghostpkg", "ghost", dir),
/ghostpkg's "ghost" bin points at a missing file/,
);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
test("accepts a scoped package's string-form bin under its unscoped name", () => {
const dir = fixtureDir(
{ name: "@scope/tool", bin: "bin/tool.js" },
{ createBinFile: true },
);
try {
const entry = resolveNodeBin("@scope/tool", "tool", dir);
assert.ok(existsSync(entry), `resolved entry does not exist: ${entry}`);
assert.throws(
() => resolveNodeBin("@scope/tool", "scope-tool", dir),
/declares no "scope-tool" bin/,
);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

View file

@ -191,16 +191,6 @@ export function resolveLeafProjects(clientDir, project, seen = new Set()) {
);
}
/**
* Every file ONE project's program resolves, as absolute POSIX paths and with no
* filtering at all — first-party sources, `node_modules` declarations, and the
* out-of-repo `lib.*.d.ts` — plus the config diagnostic if `tsc` exited
* non-zero. The two consumers want different slices of the files, so the slicing
* happens in {@link projectSourceFiles} / {@link projectPackageFiles} rather than
* here; they differ on the `error` too, which is why it is reported rather than
* only warned about. Memoized per (client, project): the same project is listed
* by `resolveLeafProjects` and again by whichever slice the caller asks for.
*/
/**
* The tsc JS entry a client's programs are measured with, resolved from the
* client dir up the node_modules tree exactly as `npx --no-install tsc` walked
@ -229,6 +219,16 @@ export function tscEntry(clientDir) {
return entry;
}
/**
* Every file ONE project's program resolves, as absolute POSIX paths and with no
* filtering at all — first-party sources, `node_modules` declarations, and the
* out-of-repo `lib.*.d.ts` — plus the config diagnostic if `tsc` exited
* non-zero. The two consumers want different slices of the files, so the slicing
* happens in {@link projectSourceFiles} / {@link projectPackageFiles} rather than
* here; they differ on the `error` too, which is why it is reported rather than
* only warned about. Memoized per (client, project): the same project is listed
* by `resolveLeafProjects` and again by whichever slice the caller asks for.
*/
const listingCache = new Map();
export function rawProjectListing(clientDir, project) {
const key = `${clientDir}|${project}`;