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

[olaservo]

Closes#1939

npx/npm are .cmd shims on Windows and Node refuses shell-less .cmd spawns, so validate/ci died at verify:typecheck-coverage — silently, as a bogus "918 files get no tsc pass" report. This adds a shared scripts/lib/resolve-node-bin.mjs (resolves a package's bin via its package.json, spawned with process.execPath) and patches all seven affected call sites across the verify/smoke/pack scripts; a resolution failure is now a hard "cannot resolve — run npm install" error instead of an empty file set. Verified on Windows: test:scripts 94/94 (+7 new tests incl. a regression pin), verify:typecheck-coverage and verify:build-gate green. Full root-cause in the issue comment.

🤖 Generated with Claude Code

[olaservo]

Merged v2/main: #1965 moved the tsc spawn into the shared scripts/lib/tsc-program.mjs, so the fix now lives there — which also fixes verify:dep-lockstep on Windows (first green run: 3 deps across 5 installs, derived from 10 tsc programs). With that in place, the full npm run ci passes end-to-end on Windows (exit 0): validate, coverage, build-gate, all smokes including smoke:tui (which Linux CI can't run — no TTY), and Storybook (478 tests).

[Copilot]

Pull request overview

Makes verification and smoke tooling Windows-compatible by resolving Node package binaries directly instead of spawning .cmd shims.

Changes:

• Adds a shared package-bin resolver and tests.
• Updates TypeScript, Vite, and smoke command execution.
• Enables Windows shell resolution for npm pack.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
scripts/lib/resolve-node-bin.mjs	Adds shared binary resolution.
scripts/lib/resolve-node-bin.test.mjs	Tests resolver behavior.
scripts/lib/tsc-program.mjs	Runs resolved TypeScript entry.
scripts/verify-typecheck-coverage.mjs	Uses the shared TypeScript resolver.
scripts/verify-typecheck-coverage.main.test.mjs	Tests hard failure when TypeScript is unavailable.
scripts/verify-dep-lockstep.main.test.mjs	Includes the resolver in fixtures.
scripts/verify-build-gate.mjs	Runs Vite through Node.
scripts/smoke-cli.mjs	Builds test servers cross-platform.
scripts/smoke-tui.mjs	Builds test servers cross-platform.
scripts/smoke-web-app.mjs	Builds app test servers cross-platform.
scripts/pack-and-verify.mjs	Makes npm pack spawnable on Windows.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Copilot review round 1 — both comments were correct and both are fixed in 4b143ce. Mirroring at PR level since inline replies get hidden once the threads go outdated.

1. String-form bin ignored binName (thread) — npm's string shorthand declares one command, named after the package, so accepting any requested name meant a typo silently resolved the package's only executable and bypassed the documented "declares no such bin" failure. Now matched against the manifest name, unscoped (@scope/tool accepts tool). Tests: a mismatch case (prettier / prettierd) and a scoped fixture asserting tool resolves while scope-tool throws.

2. No existence check on the resolved entry (thread) — the sharper of the two, because it relocates rather than removes the failure this PR targets: on a partial install (typescript/package.json present, bin/tsc gone) process.execPath <missing> still spawns and exits 1 with empty stdout, which rawProjectListing records as "no diagnostic captured" and turns back into the bogus every-file-uncovered report. resolveNodeBin now existsSync-checks the entry and throws, so callers take the actionable "cannot resolve — run npm install" path. Covered by a throwaway node_modules fixture (ghostpkg declaring a bin/ghost.js that was never written).

Also folded in one thing found independently of the review: the new tscEntry docblock had been inserted betweenrawProjectListing's docblock and rawProjectListing itself, leaving two stacked blocks where the first read as documenting tscEntry and the function it described was left undocumented. Moved back down.

Verification — full npm run ci green locally on a clean worktree (real npm install, not a symlink):

• verify:format-coverage OK (975 files), verify:typecheck-coverage OK (937 files), verify:dep-lockstep OK (3 install-crossing deps across 5 installs, 10 tsc programs) — the three guards this PR rewrites the spawn path of
• test:scripts 100/100 pass
• verify:build-gate OK — the Build gate: fail vite build on a browser-externalized Node built-in (SEP of #1615) #1769 gate still fires through the new process.execPath+Vite-entry spawn
• all six smokes OK: launcher, cli, tui, web, web:browser, web:app
• coverage gate + Storybook 113 files / 478 tests pass

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

scripts/lib/resolve-node-bin.mjs:25

• <pkg>/package.json is not always exported. A valid package can define exports without ./package.json; in that case this throws ERR_PACKAGE_PATH_NOT_EXPORTED even though its declared bin is installed, contradicting the shared resolver's contract. Locate the manifest through the node_modules search paths (or another export-independent package-root lookup) and add a fixture whose exports hides package.json.

 * 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).

scripts/pack-and-verify.mjs:142

• shell: true makes Node concatenate these arguments without escaping them. On Windows, tmpdir() commonly contains spaces (for example under a user profile), so --pack-destination is split into multiple arguments and pack:verify still fails; shell metacharacters in the path are also interpreted. Invoke npm's JavaScript CLI with process.execPath and keep arguments shell-free, applying the same mechanism to this file's other npm invocations that pass generated temp/tarball paths.

 // 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" },

scripts/lib/resolve-node-bin.mjs:1

• The documentation was not updated for this new shared tooling module. AGENTS.md:124-133 still describes scripts/lib around tsc-program.mjs and only pure-parser tests, while README.md:315 says test:scripts covers only the listed parsers. Add resolve-node-bin.mjs and its resolution tests to the relevant project-structure/test descriptions so the documented tooling architecture matches the repository.

// Shared resolver for spawning a package's CLI cross-platform (#1939).

[cliffhall]

Copilot review round 2 — the visible review said "no new comments", but the Suppressed comments (3) block held three real findings. All fixed in b399bc6.

1. require.resolve("<pkg>/package.json") is not export-independent. Correct, and my own docblock asserted the opposite ("./package.json is always exported") — Node keeps no special case for it. A package declaring an exports map that omits ./package.json threw ERR_PACKAGE_PATH_NOT_EXPORTED even though its bin was installed and perfectly spawnable: a false "not installed" sending someone to npm install for a package already on disk. The manifest is now found by walking require.resolve.paths(), which never consults exports, and the misleading comment is corrected. Fixture added as suggested (walledpkg, exports: { ".": "./index.js" }).

2. shell: true splits generated paths on Windows. Also correct. With a shell Node hands cmd.exe one space-joined string, so --pack-destination <tmpdir()> splits on the space that C:\Users\First Last\AppData\Local\Temp contains. Added a shellArgs() quoter and applied it at all four spawn sites in pack-and-verify.mjs, the command path included (bin also lives under the throwaway consumer's temp dir).

One scoping note: this hazard predates this PR — runInherit, runBin, and the --webspawn have carried shell: process.platform === "win32" all along, and this PR only added a fourth instance. So rather than quoting just the new line I fixed the class in that file. I did not take the "invoke npm's JS CLI with process.execPath" half of the suggestion: unlike typescript/vite, npm is not resolvable from node_modules, and locating npm-cli.js relative to process.execPath varies across nvm/volta/system installs — that trades a quoting bug for a resolution bug. Quoting is the smaller, more durable fix, and the whole point of resolveNodeBin is that the shell is avoided entirely wherever the package is locally resolvable.

3. Docs drift. Fair catch and required by this repo's own maintenance rules. Added resolve-node-bin.mjs to the scripts/ tree in AGENTS.md (what it resolves and why) and its tests to the test:scripts row in README.md.

Verification — full npm run ci green again on the clean worktree: verify:format-coverage 975 files, verify:typecheck-coverage 937 files, verify:dep-lockstep OK, verify:build-gate fired, all six smokes OK, test:scripts 101/101, coverage + Storybook 113 files / 478 tests. GitHub build passed on the previous commit (4b143ce) and is now running on b399bc6.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

scripts/pack-and-verify.mjs:114

• This quoting predicate misses valid Windows path characters that cmd.exe still interprets, notably parentheses, and quoting does not suppress %VAR% expansion. A temp/profile path such as C:\Temp(1) is therefore passed unquoted and can be parsed as shell syntax; a path containing %NAME% can be rewritten even when quoted. Since these path-bearing calls are the Windows fix this PR introduces, use a complete cmd.exe escaping strategy (or avoid the shell where possible) and cover spaces plus shell metacharacters in an automated test.

 return args.map((arg) =>
/[\s&|<>^"]/.test(arg) ? `"${arg.replace(/"/g, '""')}"` : arg,
);

AGENTS.md:136

• This documentation says the pack script uses resolve-node-bin.mjs and process.execPath, but pack-and-verify.mjs does not import the resolver and intentionally continues spawning npx/npm through a Windows shell. Please distinguish the verify/smoke direct-Node approach from the pack script's shell-quoting approach so the source-of-truth architecture description matches the implementation.

│ # through — #1965; resolve-node-bin.mjs resolves a
│ # package's bin through its own package.json so the
│ # verify/smoke/pack scripts spawn it with
│ # `process.execPath` instead of the `npx`/`npm`
│ # `.cmd` shims Node refuses to spawn shell-free on

[cliffhall]

Copilot review round 3 — 1 visible comment + 2 more in the Suppressed comments block. All three were real; all three fixed in c935521.

1. require.resolve.paths() is not limited to the ancestor chain (thread) — the sharpest catch of the three rounds, because it broke the guarantee this module exists to keep rather than an edge case. resolve.paths() appends Node's GLOBAL FOLDERS ($HOME/.node_modules, $HOME/.node_libraries, $PREFIX/lib/node) and NODE_PATH entries, so a missing repo install could be satisfied by a globally installed TypeScript — measuring the programs with the wrong compiler, which is a worse failure than the ENOENT this PR set out to fix, since it is green and wrong. Now filtered to node_modules dirs on fromDir's own ancestor chain (localSearchDirs, exported for its test), with the requested regression in two forms: a unit test that the global folders and an off-chain NODE_PATH entry are dropped while the chain is kept in order, and an end-to-end test that a package on disk but off-chain throws rather than resolving.

2. The quoting predicate was incomplete — correct on both counts. Parentheses group in cmd.exe, so C:\Temp(1) was passed unquoted, and quoting genuinely cannot save %VAR% because expansion happens before quote processing. Extracted to scripts/lib/win-shell-args.mjs: a deliberately broad predicate (parens, ^, !, ;, ,, =, quotes, backtick, brackets), and a hard throw on % — silently running against a different directory is exactly the class of failure this PR exists to remove, so it says so instead. platform is a parameter rather than a read of process.platform, so the Windows behaviour is exercised on the POSIX machines that actually run this suite; 9 tests, test:scripts now 110/110.

3. AGENTS.md overstated the fix — right, and worth correcting precisely because that file is the source of truth. It said the pack script spawns via process.execPath; it does not and cannot, since its children are npm itself and the installed .bin shim. Now states the split explicitly: verify/smoke go direct-Node through the resolver, pack-and-verify keeps a Windows shell and quotes via win-shell-args.mjs.

Verification — full npm run ci green again: all three verify guards, verify:build-gate, all six smokes, test:scripts 110/110, coverage + Storybook 113 files / 478 tests. GitHub build passed on both prior commits (4b143ce, b399bc6) and is now running on c935521.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.