From 6ae59f3d19e85ebe16357ece4a384072fe7d3b31 Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:12:29 +0800 Subject: [PATCH] fix(verifiers): launch npm through Node on Windows (#4153) * fix(verifiers): launch npm through Node on Windows * test(verifiers): cover Windows npm fallback --- .github/workflows/ci.yml | 1 + scripts/ci/scopes.mjs | 1 + scripts/lib/verify-package-core.mjs | 20 +++++ scripts/verify-package-install.mjs | 10 +-- scripts/verify-package-pack.mjs | 12 +-- scripts/verify-packaged-ghidra-windows.mjs | 12 +-- scripts/verify-packaged-wasm-artifact.mjs | 5 +- scripts/verify-repository-skill.mjs | 5 +- scripts/verify-windows-package.mjs | 30 +++---- .../process/packageNpmInvocation.test.ts | 90 +++++++++++++++++++ .../process-global/curatedWindowsLane.test.ts | 6 ++ 11 files changed, 143 insertions(+), 49 deletions(-) create mode 100644 tests/boundary/process/packageNpmInvocation.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c10ef0d9c1..3873fb4a9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -741,6 +741,7 @@ jobs: tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts tests/boundary/filesystem/webScriptManifestPaths.test.ts tests/boundary/process/ProcessEvidence.host.test.ts + tests/boundary/process/packageNpmInvocation.test.ts tests/boundary/process/windowsOutputCapture.test.ts tests/acceptance/webModuleCli.test.ts tests/acceptance/webSourceLocationCli.test.ts diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index 6da5769a96..f1feacc95e 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -138,6 +138,7 @@ export const SCOPES = { "src/domain/javascript/javascriptRuntimeStaticCandidates.ts", "tests/boundary/filesystem/webScriptManifestPaths.test.ts", "tests/boundary/process/ProcessEvidence.host.test.ts", + "tests/boundary/process/packageNpmInvocation.test.ts", "tests/boundary/process/windowsOutputCapture.test.ts", "tests/acceptance/webModuleCli.test.ts", "tests/acceptance/webSourceLocationCli.test.ts", diff --git a/scripts/lib/verify-package-core.mjs b/scripts/lib/verify-package-core.mjs index cd543ead86..71fbe71f53 100644 --- a/scripts/lib/verify-package-core.mjs +++ b/scripts/lib/verify-package-core.mjs @@ -1,5 +1,6 @@ import { execFile } from "node:child_process"; import { lstat } from "node:fs/promises"; +import { dirname, join } from "node:path"; import { promisify } from "node:util"; import { Ajv2020 } from "ajv/dist/2020.js"; @@ -17,6 +18,25 @@ const loadMcpVerifierResults = () => import("./mcp-verifier-results.mjs"); /** Promisified child_process execution helper. */ export const exec = promisify(execFile); +/** Run npm through its JavaScript entry point so Windows command scripts are not required. */ +export const execNpm = (arguments_, options = {}) => { + const environment = options.env ?? process.env; + const selectedEntry = environment.npm_execpath; + if ( + selectedEntry !== undefined && + (typeof selectedEntry !== "string" || selectedEntry.length === 0) + ) + throw new Error("npm_execpath must name the selected npm CLI entry point"); + const entry = + selectedEntry ?? + (process.platform === "win32" + ? join(dirname(process.execPath), "node_modules/npm/bin/npm-cli.js") + : undefined); + return entry === undefined + ? exec("npm", arguments_, options) + : exec(process.execPath, [entry, ...arguments_], options); +}; + /** Run a command and return its stdout. */ export const run = async (command, args, env) => (await exec(command, args, { env })).stdout; diff --git a/scripts/verify-package-install.mjs b/scripts/verify-package-install.mjs index 722549e5d0..48778c77a7 100644 --- a/scripts/verify-package-install.mjs +++ b/scripts/verify-package-install.mjs @@ -2,7 +2,7 @@ import { rm } from "node:fs/promises"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; -import { exec, json, run } from "./lib/verify-package-core.mjs"; +import { exec, execNpm, json, run } from "./lib/verify-package-core.mjs"; /** Install the packed CLI globally with and without optional dependencies and verify PTY degradation. */ export async function verifyPackageInstall({ @@ -11,8 +11,7 @@ export async function verifyPackageInstall({ workspace, environment, }) { - await exec( - "npm", + await execNpm( [ "install", "--global", @@ -53,8 +52,7 @@ export async function verifyPackageInstall({ `packaged PTY backend failed without lifecycle scripts: ${JSON.stringify(processCaptureCapability)}`, ); const noOptionalPrefix = join(workspace, "prefix-no-optional"); - await exec( - "npm", + await execNpm( [ "install", "--global", @@ -108,7 +106,7 @@ export async function verifyPackageInstall({ const verifyInstalledPackageCanRepack = async (prefix, environment) => { const installedPackageRoot = join(prefix, "lib/node_modules/rea-agents"); const repackedTarball = ( - await exec("npm", ["pack", "--silent"], { + await execNpm(["pack", "--silent"], { cwd: installedPackageRoot, env: environment, }) diff --git a/scripts/verify-package-pack.mjs b/scripts/verify-package-pack.mjs index fc9676745b..2ec1b94ee4 100644 --- a/scripts/verify-package-pack.mjs +++ b/scripts/verify-package-pack.mjs @@ -2,7 +2,7 @@ import { mkdir } from "node:fs/promises"; import { join } from "node:path"; import { CATALOG_IDENTITY } from "../dist/catalogIdentity.js"; -import { exec } from "./lib/verify-package-core.mjs"; +import { exec, execNpm } from "./lib/verify-package-core.mjs"; import { verifyPackedBridge } from "./verify-packed-bridge.mjs"; /** Create a tarball and assert production packaging constraints. */ @@ -12,13 +12,9 @@ export async function verifyPackagePack({ root, workspace }) { const tarball = join( packDirectory, ( - await exec( - "npm", - ["pack", "--silent", "--pack-destination", packDirectory], - { - cwd: root, - }, - ) + await execNpm(["pack", "--silent", "--pack-destination", packDirectory], { + cwd: root, + }) ).stdout.trim(), ); const packedFiles = (await exec("tar", ["-tf", tarball])).stdout.split("\n"); diff --git a/scripts/verify-packaged-ghidra-windows.mjs b/scripts/verify-packaged-ghidra-windows.mjs index 1de36c76d7..ed4ce5fe48 100644 --- a/scripts/verify-packaged-ghidra-windows.mjs +++ b/scripts/verify-packaged-ghidra-windows.mjs @@ -28,6 +28,7 @@ import { fileURLToPath, pathToFileURL } from "node:url"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { execNpm } from "./lib/verify-package-core.mjs"; if (process.platform !== "win32" || process.arch !== "x64") throw new Error( @@ -82,15 +83,10 @@ let packageRoot = resolve(packageRootArgument ?? "."); // package roots support an already-installed artifact without a second install. if (packageRootArgument === undefined) { try { - const npmEntry = - process.env.npm_execpath ?? - join(dirname(process.execPath), "node_modules/npm/bin/npm-cli.js"); const packed = JSON.parse( ( - await exec( - process.execPath, + await execNpm( [ - npmEntry, "pack", "--json", "--ignore-scripts", @@ -107,10 +103,8 @@ if (packageRootArgument === undefined) { "The package lane must produce one artifact.", ); const prefix = join(workspace, "installed"); - await exec( - process.execPath, + await execNpm( [ - npmEntry, "install", "--prefix", prefix, diff --git a/scripts/verify-packaged-wasm-artifact.mjs b/scripts/verify-packaged-wasm-artifact.mjs index 95bdc138ac..98cd9ccb76 100644 --- a/scripts/verify-packaged-wasm-artifact.mjs +++ b/scripts/verify-packaged-wasm-artifact.mjs @@ -2,7 +2,7 @@ import { join } from "node:path"; import { mkdir } from "node:fs/promises"; import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js"; -import { exec } from "./lib/verify-package-core.mjs"; +import { exec, execNpm } from "./lib/verify-package-core.mjs"; import { WABT_WORKFLOW_TIMEOUT_MS } from "./lib/wabt-verifier-timeouts.mjs"; import { verifyPackagePack } from "./verify-package-pack.mjs"; @@ -24,8 +24,7 @@ try { XDG_CONFIG_HOME: join(home, ".config"), XDG_CACHE_HOME: join(home, ".cache"), }; - await exec( - "npm", + await execNpm( [ "install", "--global", diff --git a/scripts/verify-repository-skill.mjs b/scripts/verify-repository-skill.mjs index 8340bce7b6..c18d7c62dd 100644 --- a/scripts/verify-repository-skill.mjs +++ b/scripts/verify-repository-skill.mjs @@ -1,7 +1,7 @@ import { cp, mkdir, readFile, readdir } from "node:fs/promises"; import { dirname, join, relative } from "node:path"; -import { exec, pathExists } from "./lib/verify-package-core.mjs"; +import { exec, execNpm, pathExists } from "./lib/verify-package-core.mjs"; /** Exercise Skills' default root discovery without ignored build outputs. */ export async function verifyRepositorySkill({ root, workspace }) { @@ -34,8 +34,7 @@ export async function verifyRepositorySkill({ root, workspace }) { await mkdir(consumer, { recursive: true }); await mkdir(home, { recursive: true }); const skillsVersion = "1.7.2"; - await exec( - "npm", + await execNpm( [ "exec", "--yes", diff --git a/scripts/verify-windows-package.mjs b/scripts/verify-windows-package.mjs index 425b5cbd3c..fb7a20986c 100644 --- a/scripts/verify-windows-package.mjs +++ b/scripts/verify-windows-package.mjs @@ -18,7 +18,10 @@ import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; -import { verifyCompleteToolCatalog } from "./lib/verify-package-core.mjs"; +import { + execNpm, + verifyCompleteToolCatalog, +} from "./lib/verify-package-core.mjs"; import { verifyPackageUpdate } from "./verify-package-update.mjs"; import { completeVerifierRun, createVerifierRun } from "./lib/verifier-run.mjs"; @@ -32,9 +35,9 @@ let report; try { const packed = JSON.parse( ( - await npm( + await execNpm( ["pack", "--json", "--silent", "--pack-destination", workspace], - root, + { cwd: root, windowsHide: true }, ) ).stdout, ); @@ -63,9 +66,9 @@ try { const tarball = join(workspace, packageResult.filename); await access(tarball); - await npm( + await execNpm( ["install", "--no-package-lock", "--no-save", "--prefix", prefix, tarball], - workspace, + { cwd: workspace, windowsHide: true }, ); const entry = join( prefix, @@ -209,9 +212,9 @@ try { throw new Error(`Packaged uninstall did not remove ${id}`); if (process.platform !== "win32") - await npm( + await execNpm( ["install", "--global", "--ignore-scripts", "--prefix", prefix, tarball], - workspace, + { cwd: workspace, windowsHide: true }, ); const update = await verifyPackageUpdate({ prefix, @@ -241,16 +244,3 @@ try { process.stdout.write( `${JSON.stringify({ verifier_run: await completeVerifierRun(verifierRun), ...report })}\n`, ); - -function npm(arguments_, cwd) { - const npmExecPath = process.env.npm_execpath; - return npmExecPath === undefined - ? exec(process.platform === "win32" ? "npm.cmd" : "npm", arguments_, { - cwd, - windowsHide: true, - }) - : exec(process.execPath, [npmExecPath, ...arguments_], { - cwd, - windowsHide: true, - }); -} diff --git a/tests/boundary/process/packageNpmInvocation.test.ts b/tests/boundary/process/packageNpmInvocation.test.ts new file mode 100644 index 0000000000..ccc5c461a1 --- /dev/null +++ b/tests/boundary/process/packageNpmInvocation.test.ts @@ -0,0 +1,90 @@ +import { execFile } from "node:child_process"; +import { copyFile, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; +import { promisify } from "node:util"; +import { expect, it } from "vitest"; + +const execute = promisify(execFile); + +it("runs the caller-selected npm entry through Node without rewriting its inputs", async () => { + const root = await mkdtemp(join(tmpdir(), "rea-npm-invocation-")); + try { + const npmEntry = join(root, "selected npm entry.mjs"); + await writeFile( + npmEntry, + `process.stdout.write(JSON.stringify({ arguments: process.argv.slice(2), cwd: process.cwd(), marker: process.env.REA_NPM_TEST_MARKER }));\n`, + ); + const helper = pathToFileURL( + resolve("scripts/lib/verify-package-core.mjs"), + ).href; + const program = ` + const { execNpm } = await import(${JSON.stringify(helper)}); + const result = await execNpm(["pack", "value with spaces"], { + cwd: ${JSON.stringify(root)}, + env: { ...process.env, npm_execpath: ${JSON.stringify(npmEntry)}, REA_NPM_TEST_MARKER: "caller-selected" }, + }); + process.stdout.write(result.stdout); + `; + const result = await execute( + process.execPath, + ["--input-type=module", "--eval", program], + { windowsHide: true }, + ); + expect(JSON.parse(result.stdout)).toEqual({ + arguments: ["pack", "value with spaces"], + cwd: root, + marker: "caller-selected", + }); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +const windowsIt = process.platform === "win32" ? it : it.skip; + +windowsIt( + "uses the npm entry adjacent to Node when none is selected", + async () => { + const root = await mkdtemp(join(tmpdir(), "rea-adjacent-npm-")); + try { + const node = join(root, "node.exe"); + const npmEntry = join(root, "node_modules/npm/bin/npm-cli.js"); + await mkdir(dirname(npmEntry), { recursive: true }); + await Promise.all([ + copyFile(process.execPath, node), + writeFile( + npmEntry, + `process.stdout.write(JSON.stringify({ arguments: process.argv.slice(2), cwd: process.cwd() }));\n`, + ), + ]); + const helper = pathToFileURL( + resolve("scripts/lib/verify-package-core.mjs"), + ).href; + const program = ` + const { execNpm } = await import(${JSON.stringify(helper)}); + const environment = { ...process.env }; + delete environment.npm_execpath; + const result = await execNpm(["pack", "value with spaces"], { + cwd: ${JSON.stringify(root)}, + env: environment, + }); + process.stdout.write(result.stdout); + `; + const environment = { ...process.env }; + delete environment.npm_execpath; + const result = await execute( + node, + ["--input-type=module", "--eval", program], + { env: environment, windowsHide: true }, + ); + expect(JSON.parse(result.stdout)).toEqual({ + arguments: ["pack", "value with spaces"], + cwd: root, + }); + } finally { + await rm(root, { recursive: true, force: true }); + } + }, +); diff --git a/tests/process-global/curatedWindowsLane.test.ts b/tests/process-global/curatedWindowsLane.test.ts index e126d6c82a..8cd9c902cc 100644 --- a/tests/process-global/curatedWindowsLane.test.ts +++ b/tests/process-global/curatedWindowsLane.test.ts @@ -69,6 +69,12 @@ describe("curated Windows test lane", () => { "tests/boundary/process/windowsOutputCapture.test.ts", ); }); + + it("keeps the adjacent npm fallback regression in the lane", async () => { + expect(await explicitPaths("windows")).toContain( + "tests/boundary/process/packageNpmInvocation.test.ts", + ); + }); }); it("keeps every explicit focused CI test path executable after file moves", async () => {