fix: resolve node bins via package.json instead of spawning npx .cmd shims so the verify/smoke scripts run on Windows

On Windows npx/npm are .cmd shims a shell-free execFileSync/spawnSync
cannot start (ENOENT since the CVE-2024-27980 hardening), so
verify:typecheck-coverage died at validate's second step — doubly
silently, echoing "(no diagnostic captured)" per project and then
reporting all 918 tracked files as getting no tsc pass — and
verify:build-gate, the smoke test-server bootstraps, and pack:verify's
npm pack failed the same way.

Add scripts/lib/resolve-node-bin.mjs: resolves the JS entry behind a
package's bin from <pkg>/package.json (deep resolution is blocked by
Vite 8's exports map) and spawns it via process.execPath — the same
walk npx --no-install did, cross-platform and shell-free. Switch the
six npx call sites to it; the npm pack call gets the existing
shell-on-win32 idiom instead (npm has no in-tree package to resolve).
An unresolvable tsc is now a hard "cannot measure" error with
actionable stderr, pinned by a new main() regression test.

Closes #1939

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
olaservo 2026-08-12 09:04:31 -07:00
commit e3a026802b
9 changed files with 309 additions and 27 deletions

View file

@ -0,0 +1,41 @@
// Shared resolver for spawning a package's CLI cross-platform (#1939).
//
// On Windows, `npx`/`npm` are `.cmd` shims, not executables — a shell-free
// `execFileSync`/`spawnSync` cannot start one (Node refuses `.cmd`/`.bat`
// spawns without `shell: true` since the CVE-2024-27980 hardening) and throws
// `ENOENT`. GitHub CI runs Linux, so the gate stayed green there while being
// unrunnable for any Windows contributor. Instead of shelling through `npx`,
// resolve the JS entry behind the package's bin and run it with
// `process.execPath`: cross-platform, shell-free (no quoting hazards), faster
// (no npx resolution), and pinned to the locally installed package exactly as
// `npx --no-install` was.
import { readFileSync } from "node:fs";
import { createRequire } from "node:module";
import path from "node:path";
/**
* Absolute path of the JS entry behind a package's bin (e.g. typescript's
* `tsc`, vite's `vite`), resolved from `fromDir` up the node_modules tree —
* the same walk `npx --no-install` does, minus the `.cmd` shim a shell-free
* spawn can't start on Windows. Resolves `<pkg>/package.json` and reads its
* `bin` field (what npx itself does) rather than resolving the bin path
* directly, because an `exports` map blocks deep resolution — Vite 8 doesn't
* 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.
*/
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];
if (typeof rel !== "string")
throw new Error(`${pkg} declares no "${binName}" bin in its package.json`);
return path.join(path.dirname(pkgPath), rel);
}

View file

@ -0,0 +1,68 @@
// Tests for `resolve-node-bin.mjs` (#1939) — the shared resolver that replaces
// shelling through `npx`, which is a `.cmd` shim on Windows that a shell-free
// `execFileSync`/`spawnSync` cannot start (ENOENT). Resolution is exercised
// against the packages the callers actually spawn (typescript, vite, prettier),
// installed by the repo's own `npm install`, so the contract is pinned against
// the real `bin`/`exports` shapes rather than fixtures that can drift.
import { test } from "node:test";
import assert from "node:assert/strict";
import { existsSync } from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { resolveNodeBin } from "./resolve-node-bin.mjs";
const repoRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
"..",
"..",
);
test("resolves typescript's tsc (object-form bin) to an existing JS entry", () => {
const entry = resolveNodeBin("typescript", "tsc", repoRoot);
assert.ok(path.isAbsolute(entry), entry);
assert.ok(existsSync(entry), `resolved entry does not exist: ${entry}`);
assert.match(entry.split(path.sep).join("/"), /\/typescript\/.*tsc/);
});
test("resolves from a client dir, walking node_modules up like `npx --no-install`", () => {
const entry = resolveNodeBin(
"typescript",
"tsc",
path.join(repoRoot, "clients", "cli"),
);
assert.ok(existsSync(entry), `resolved entry does not exist: ${entry}`);
});
test("resolves vite's bin despite Vite 8's exports map (no deep bin export)", () => {
// `require.resolve("vite/bin/vite.js")` throws ERR_PACKAGE_PATH_NOT_EXPORTED
// under Vite 8 — the reason the helper goes through `<pkg>/package.json`.
const entry = resolveNodeBin(
"vite",
"vite",
path.join(repoRoot, "clients", "web"),
);
assert.ok(existsSync(entry), `resolved entry does not exist: ${entry}`);
assert.match(entry.split(path.sep).join("/"), /\/vite\/.*vite\.js$/);
});
test("resolves a string-form bin (prettier), ignoring the binName", () => {
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("throws when the package is not installed from fromDir", () => {
assert.throws(
() => resolveNodeBin("definitely-not-installed-anywhere", "x", repoRoot),
/definitely-not-installed-anywhere/,
);
});
test("throws when the package declares no such bin", () => {
// typescript's bin map has `tsc`/`tsserver`, not `vite`.
assert.throws(
() => resolveNodeBin("typescript", "vite", repoRoot),
/typescript declares no "vite" bin/,
);
});

View file

@ -137,7 +137,9 @@ step("packing the publishable tarball (npm pack)...");
const pack = spawnSync(
"npm",
["pack", "--json", "--ignore-scripts", "--pack-destination", tmpdir()],
{ cwd: repoRoot, encoding: "utf8" },
// npm is npm.cmd on Windows, which needs a shell to resolve (#1939) — the
// same idiom as runInherit/runBin below.
{ cwd: repoRoot, encoding: "utf8", shell: process.platform === "win32" },
);
if (pack.status !== 0) {
fail(`\`npm pack\` failed:\n${pack.stderr || pack.stdout}`);

View file

@ -44,6 +44,7 @@ import { mkdtempSync, rmSync, existsSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { pathToFileURL } from "node:url";
import { resolveNodeBin } from "./lib/resolve-node-bin.mjs";
const repoRoot = resolve(import.meta.dirname, "..");
const launcher = join(repoRoot, "clients", "launcher", "build", "index.js");
@ -69,10 +70,18 @@ function fail(message) {
function ensureTestServer() {
if (existsSync(testServer) && existsSync(httpTestServerModule)) return;
console.log("smoke:cli — building test-servers (missing build output)...");
const r = spawnSync("npx", ["tsc", "-p", "test-servers", "--noCheck"], {
cwd: repoRoot,
stdio: "inherit",
});
// The root-installed tsc, run via this Node — `npx` is a `.cmd` shim on
// Windows that a shell-free spawnSync can't start (ENOENT — #1939).
const r = spawnSync(
process.execPath,
[
resolveNodeBin("typescript", "tsc", repoRoot),
"-p",
"test-servers",
"--noCheck",
],
{ cwd: repoRoot, stdio: "inherit" },
);
if (
r.status !== 0 ||
!existsSync(testServer) ||

View file

@ -24,6 +24,7 @@ import { mkdtempSync, existsSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { removeSafe } from "./lib/child-cleanup.mjs";
import { resolveNodeBin } from "./lib/resolve-node-bin.mjs";
const repoRoot = resolve(import.meta.dirname, "..");
const launcher = join(repoRoot, "clients", "launcher", "build", "index.js");
@ -44,10 +45,18 @@ function fail(message) {
function ensureTestServer() {
if (existsSync(testServer)) return;
console.log("smoke:tui — building test-servers (missing build output)...");
const r = spawnSync("npx", ["tsc", "-p", "test-servers", "--noCheck"], {
cwd: repoRoot,
stdio: "inherit",
});
// The root-installed tsc, run via this Node — `npx` is a `.cmd` shim on
// Windows that a shell-free spawnSync can't start (ENOENT — #1939).
const r = spawnSync(
process.execPath,
[
resolveNodeBin("typescript", "tsc", repoRoot),
"-p",
"test-servers",
"--noCheck",
],
{ cwd: repoRoot, stdio: "inherit" },
);
if (r.status !== 0 || !existsSync(testServer)) {
fail(
"could not build the stdio test server (test-servers/build/test-server-stdio.js). " +

View file

@ -53,6 +53,7 @@ import { setTimeout as delay } from "node:timers/promises";
import { join, resolve } from "node:path";
import { startProdWebServer } from "./lib/prod-web-server.mjs";
import { stopChild } from "./lib/child-cleanup.mjs";
import { resolveNodeBin } from "./lib/resolve-node-bin.mjs";
const repoRoot = resolve(import.meta.dirname, "..");
const requireFromWeb = createRequire(
@ -140,10 +141,18 @@ function ensureTestServer() {
console.log(
"smoke:web:app — building test-servers (missing build output)...",
);
const r = spawnSync("npx", ["tsc", "-p", "test-servers", "--noCheck"], {
cwd: repoRoot,
stdio: "inherit",
});
// The root-installed tsc, run via this Node — `npx` is a `.cmd` shim on
// Windows that a shell-free spawnSync can't start (ENOENT — #1939).
const r = spawnSync(
process.execPath,
[
resolveNodeBin("typescript", "tsc", repoRoot),
"-p",
"test-servers",
"--noCheck",
],
{ cwd: repoRoot, stdio: "inherit" },
);
if (r.status !== 0 || !existsSync(composableServer)) {
throw new Error(
"could not build the test servers (test-servers/build/server-composable.js). " +

View file

@ -32,6 +32,7 @@ import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { resolveNodeBin } from "./lib/resolve-node-bin.mjs";
const repoRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
@ -82,6 +83,20 @@ function escapeRegExp(literal) {
return literal.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}
// The repo-pinned Vite's JS entry, resolved from clients/web — the old `npx
// --no-install` guarantee (never a registry fetch) — and spawned via this Node,
// since `npx` is a `.cmd` shim on Windows that a shell-free spawnSync can't
// start (ENOENT — #1939). Resolved up front, BEFORE the probe is injected, so a
// missing install fails actionably without ever touching src/main.tsx.
let viteEntry;
try {
viteEntry = resolveNodeBin("vite", "vite", webDir);
} catch (err) {
fail(
`cannot resolve \`vite\` from clients/web (${err.message}) — run \`npm install\` at the repo root first`,
);
}
// Write the captured original to a backup and fail — the honest remedy when the
// in-place restore can't be trusted, since it preserves any uncommitted edits
// the developer had (unlike `git checkout --`). The backup goes in a fresh
@ -229,15 +244,14 @@ try {
console.log(
"verify:build-gate: running a real `vite build` with a node:fs probe (takes a minute)…",
);
// `--no-install` pins to the locally installed (repo-pinned) Vite: the whole
// point is proving the message-keyed gate fires against THIS Vite, so `npx`
// must never silently fetch a different version from the registry when
// clients/web/node_modules is missing/partial. A missing local bin then
// surfaces via the `result.error` check below. `timeout` bounds a hung build:
// spawnSync sets `result.error` (ETIMEDOUT) on timeout, so the same branch
// reports it — otherwise a hang would burn to the GitHub job's 360-min default
// with no output (this step captures rather than inherits stdio).
result = spawnSync("npx", ["--no-install", "vite", "build"], {
// `viteEntry` (resolved above) pins to the locally installed (repo-pinned)
// Vite: the whole point is proving the message-keyed gate fires against THIS
// Vite, so nothing may silently fetch a different version from the registry
// when clients/web/node_modules is missing/partial. `timeout` bounds a hung
// build: spawnSync sets `result.error` (ETIMEDOUT) on timeout, so that branch
// below reports it — otherwise a hang would burn to the GitHub job's 360-min
// default with no output (this step captures rather than inherits stdio).
result = spawnSync(process.execPath, [viteEntry, "build"], {
cwd: webDir,
encoding: "utf8",
timeout: 10 * 60_000,
@ -266,7 +280,7 @@ if (afterRestore !== original) {
);
}
// A spawn failure (e.g. `npx` missing) leaves `status` null with no output —
// A spawn failure (or the timeout above) leaves `status` null with no output —
// surface it as itself rather than falling through to the "not via the gate"
// diagnosis, which would send someone chasing a build regression that isn't real.
if (result.error) {

View file

@ -0,0 +1,105 @@
// Regression test for #1939's "doubly silent" failure mode: when the tsc entry
// cannot be resolved (on Windows the old `execFileSync("npx", …)` threw ENOENT;
// today, a missing install), the guard must hard-fail with an actionable
// "cannot measure" error — NOT swallow it, echo "(no diagnostic captured)" per
// project, and then report every tracked source file in the repo as getting no
// tsc pass, which is what shipped before and sent Windows contributors chasing
// a 900-file coverage regression that wasn't real.
//
// The fixture is a throwaway repo with one enrolled client whose `typecheck`
// names a project, but with NO node_modules anywhere up the temp tree — so the
// guard reaches the tsc-entry resolution and it fails. Mirrors the
// `verify-format-coverage.main.test.mjs` / `verify-dep-lockstep.main.test.mjs`
// pattern. Run via `npm run test:scripts`.
import { test } from "node:test";
import assert from "node:assert/strict";
import { execFileSync, spawnSync } from "node:child_process";
import {
cpSync,
mkdirSync,
mkdtempSync,
realpathSync,
rmSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
const scriptsDir = path.dirname(fileURLToPath(import.meta.url));
test("unresolvable tsc is a hard 'cannot measure' error, not an empty file set", () => {
// realpath'd because the script only executes when `import.meta.url` matches
// `process.argv[1]`, and macOS `tmpdir()` is a symlink — see the note in
// `verify-dep-lockstep.main.test.mjs`.
const dir = realpathSync(mkdtempSync(path.join(tmpdir(), "typecheck-cov-")));
try {
// Root manifest: the full guard cycle is wired so phase 1 gets as far as
// measuring the client (an unwired client is `continue`d past, and the
// resolution would never be reached).
writeFileSync(
path.join(dir, "package.json"),
JSON.stringify({
name: "fixture",
scripts: {
validate:
"npm run verify:format-coverage && npm run verify:typecheck-coverage && npm run verify:dep-lockstep && npm run test:scripts && npm --prefix clients/cli run validate",
"verify:format-coverage": "node scripts/verify-format-coverage.mjs",
"verify:typecheck-coverage":
"node scripts/verify-typecheck-coverage.mjs",
"verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs",
"test:scripts": 'node --test "scripts/**/*.test.mjs"',
},
}),
);
// One enrolled client with a `typecheck` reachable from `validate`, a
// project for it to name, and a tracked source file — so if the hard error
// ever regresses to the old behavior, the "get no `tsc` pass" report the
// second assertion forbids would actually have a file to list.
mkdirSync(path.join(dir, "clients", "cli", "src"), { recursive: true });
writeFileSync(
path.join(dir, "clients", "cli", "package.json"),
JSON.stringify({
name: "fixture-cli",
scripts: {
validate: "npm run typecheck",
typecheck: "tsc --noEmit -p tsconfig.json",
},
}),
);
writeFileSync(
path.join(dir, "clients", "cli", "tsconfig.json"),
JSON.stringify({ include: ["src"] }),
);
writeFileSync(
path.join(dir, "clients", "cli", "src", "index.ts"),
"export const x = 1;\n",
);
mkdirSync(path.join(dir, "scripts", "lib"), { recursive: true });
for (const rel of [
"verify-typecheck-coverage.mjs",
path.join("lib", "npm-scripts.mjs"),
path.join("lib", "resolve-node-bin.mjs"),
])
cpSync(path.join(scriptsDir, rel), path.join(dir, "scripts", rel));
execFileSync("git", ["init", "-q"], { cwd: dir });
execFileSync("git", ["add", "-A"], { cwd: dir });
const r = spawnSync(
process.execPath,
[path.join(dir, "scripts", "verify-typecheck-coverage.mjs")],
{ cwd: dir, encoding: "utf8" },
);
const out = `${r.stdout}${r.stderr}`;
assert.equal(r.status, 1, out);
assert.match(out, /cannot resolve `typescript` from clients\/cli/, out);
assert.match(out, /npm install/, out);
// The pre-#1939 failure shape: per-project "(no diagnostic captured)"
// warnings followed by every tracked file reported uncovered.
assert.doesNotMatch(out, /get no `tsc` pass/, out);
assert.doesNotMatch(out, /no diagnostic captured/, out);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

View file

@ -47,6 +47,7 @@ import {
rootRunsClientValidate,
tokenize,
} from "./lib/npm-scripts.mjs";
import { resolveNodeBin } from "./lib/resolve-node-bin.mjs";
const repoRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
@ -456,8 +457,8 @@ export function typecheckProjects(scripts) {
function projectDisablesChecking(clientDir, project) {
try {
const out = execFileSync(
"npx",
["--no-install", "tsc", "-p", project, "--showConfig"],
process.execPath,
[tscEntry(clientDir), "-p", project, "--showConfig"],
{ cwd: path.join(repoRoot, clientDir), encoding: "utf8" },
);
return JSON.parse(out)?.compilerOptions?.noCheck === true;
@ -473,6 +474,30 @@ function projectDisablesChecking(clientDir, project) {
* in the set but are harmless — the set is only ever queried with client-relative
* paths. Cached: `resolveLeafProjects` and `projectFiles` both list a project.
*/
// The tsc JS entry each client's projects are measured with, resolved from the
// client dir up the node_modules tree exactly as `npx --no-install tsc` walked
// — but spawnable shell-free on Windows, where `npx` is a `.cmd` shim that
// `execFileSync` can't start (ENOENT — #1939). A resolution failure is a hard
// "cannot measure" error rather than an empty file set: the old ENOENT was
// doubly silent, echoing "(no diagnostic captured)" per project and then
// reporting every tracked file in the repo as uncovered.
const tscEntryCache = new Map();
function tscEntry(clientDir) {
const cached = tscEntryCache.get(clientDir);
if (cached) return cached;
let entry;
try {
entry = resolveNodeBin("typescript", "tsc", path.join(repoRoot, clientDir));
} catch (err) {
console.error(
`verify:typecheck-coverage — cannot resolve \`typescript\` from ${clientDir} (${err.message}): this guard cannot measure anything. Run \`npm install\` at the repo root first.`,
);
process.exit(1);
}
tscEntryCache.set(clientDir, entry);
return entry;
}
const rawFilesCache = new Map();
function rawProjectFiles(clientDir, project) {
const key = `${clientDir}|${project}`;
@ -482,8 +507,8 @@ function rawProjectFiles(clientDir, project) {
let stdout;
try {
stdout = execFileSync(
"npx",
["--no-install", "tsc", "-p", project, "--listFilesOnly"],
process.execPath,
[tscEntry(clientDir), "-p", project, "--listFilesOnly"],
{ cwd: absClient, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] },
);
} catch (err) {