From 8a94e825cd5fe2babe1308d722710e06950fc448 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 23 May 2026 17:24:11 +0200 Subject: [PATCH] fix(scripts): run Windows check commands through shims --- CHANGELOG.md | 1 + scripts/changed-lanes.mjs | 4 +- scripts/check-changed.mjs | 4 +- scripts/ci-changed-scope.mjs | 4 +- scripts/e2e/parallels/host-command.ts | 106 ++++++++++++++++++- scripts/generate-npm-shrinkwrap.mjs | 24 +++-- scripts/lib/direct-run.mjs | 18 ++++ test/scripts/changed-lanes.test.ts | 22 +++- test/scripts/generate-npm-shrinkwrap.test.ts | 33 +++++- test/scripts/parallels-smoke-model.test.ts | 52 ++++++++- 10 files changed, 246 insertions(+), 22 deletions(-) create mode 100644 scripts/lib/direct-run.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1a8d00ad0..9a901d990a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Docs: https://docs.openclaw.ai - Directive tags: preserve message and content-part object identity when display stripping makes no directive-tag changes. (#85682) Thanks @willamhou. - Telegram: send local `path`/`filePath` and structured attachment media from `sendMessage` actions instead of dropping them or sending text-only messages. (#85219) Thanks @keshavbotagent. - Gateway/config: pin relative `OPENCLAW_STATE_DIR` overrides to an absolute path at startup so later working-directory changes cannot retarget gateway state. (#52264) Thanks @PerfectPan. +- Checks/Parallels: make changed-lane scripts, shrinkwrap generation, and Parallels package smoke host commands run through native Windows-safe paths and `npm`/`pnpm` shims. - Release/package: run npm release, prepublish, and postpublish verification through Windows-safe npm command shims so native Windows checks can execute `npm.cmd` instead of treating it as a binary. - Agents/harness: pass CLI runtime aliases through harness selection so provider-owned CLI aliases no longer get rejected before reaching the right runtime. (#85631) Thanks @potterdigital. - Secrets: show the irreversible apply warning after interactive `secrets configure` confirmation so confirmed migrations still get the final safety prompt. (#85638) Thanks @alkor2000. diff --git a/scripts/changed-lanes.mjs b/scripts/changed-lanes.mjs index 81a3c66d67f..be7c9498c7b 100644 --- a/scripts/changed-lanes.mjs +++ b/scripts/changed-lanes.mjs @@ -1,6 +1,7 @@ import { execFileSync } from "node:child_process"; import { appendFileSync, existsSync, readFileSync } from "node:fs"; import { booleanFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; +import { isDirectRunUrl } from "./lib/direct-run.mjs"; const GIT_OUTPUT_MAX_BUFFER = 64 * 1024 * 1024; @@ -445,8 +446,7 @@ function parseArgs(argv) { } function isDirectRun() { - const direct = process.argv[1]; - return Boolean(direct && import.meta.url.endsWith(direct)); + return isDirectRunUrl(process.argv[1], import.meta.url); } function printHuman(result) { diff --git a/scripts/check-changed.mjs b/scripts/check-changed.mjs index 0b560ec962b..2be9a237564 100644 --- a/scripts/check-changed.mjs +++ b/scripts/check-changed.mjs @@ -11,6 +11,7 @@ import { import { shrinkwrapPackageDirsForChangedPaths } from "./generate-npm-shrinkwrap.mjs"; import { booleanFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; import { printTimingSummary } from "./lib/check-timing-summary.mjs"; +import { isDirectRunUrl } from "./lib/direct-run.mjs"; import { acquireLocalHeavyCheckLockSync, resolveLocalHeavyCheckEnv, @@ -472,8 +473,7 @@ function parseArgs(argv) { } function isDirectRun() { - const direct = process.argv[1]; - return Boolean(direct && import.meta.url.endsWith(direct)); + return isDirectRunUrl(process.argv[1], import.meta.url); } if (isDirectRun()) { diff --git a/scripts/ci-changed-scope.mjs b/scripts/ci-changed-scope.mjs index 8d9bdd8386b..63aca1b20cc 100644 --- a/scripts/ci-changed-scope.mjs +++ b/scripts/ci-changed-scope.mjs @@ -1,5 +1,6 @@ import { execFileSync } from "node:child_process"; import { appendFileSync } from "node:fs"; +import { isDirectRunUrl } from "./lib/direct-run.mjs"; /** @typedef {{ runNode: boolean; runMacos: boolean; runAndroid: boolean; runWindows: boolean; runSkillsPython: boolean; runChangedSmoke: boolean; runControlUiI18n: boolean }} ChangedScope */ /** @typedef {{ runFastOnly: boolean; runPluginContracts: boolean; runCiRouting: boolean }} NodeFastScope */ @@ -286,8 +287,7 @@ export function writeGitHubOutput( } function isDirectRun() { - const direct = process.argv[1]; - return Boolean(direct && import.meta.url.endsWith(direct)); + return isDirectRunUrl(process.argv[1], import.meta.url); } /** @param {string[]} argv */ diff --git a/scripts/e2e/parallels/host-command.ts b/scripts/e2e/parallels/host-command.ts index 3d0fbe18609..4b69db2dc03 100644 --- a/scripts/e2e/parallels/host-command.ts +++ b/scripts/e2e/parallels/host-command.ts @@ -2,10 +2,38 @@ import { spawn, spawnSync, type SpawnOptions } from "node:child_process"; import { writeFile } from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { resolveNpmRunner } from "../../npm-runner.mjs"; +import { resolvePnpmRunner } from "../../pnpm-runner.mjs"; +import { buildCmdExeCommandLine } from "../../windows-cmd-helpers.mjs"; import type { CommandResult, RunOptions } from "./types.ts"; export const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../.."); +type HostCommandInvocation = { + args: string[]; + command: string; + env?: NodeJS.ProcessEnv; + shell?: boolean; + windowsVerbatimArguments?: boolean; +}; + +type ResolveHostCommandOptions = { + comSpec?: string; + env?: NodeJS.ProcessEnv; + execPath?: string; + existsSync?: (path: string) => boolean; + platform?: NodeJS.Platform; +}; + +function hostInvocationFromRunner(runner: HostCommandInvocation): HostCommandInvocation { + if (runner.env === undefined) { + const invocation = { ...runner }; + delete invocation.env; + return invocation; + } + return runner; +} + export function say(message: string): void { process.stdout.write(`==> ${message}\n`); } @@ -23,15 +51,81 @@ export function shellQuote(value: string): string { return `'${value.replaceAll("'", `'"'"'`)}'`; } +function portableBasename(value: string): string { + return value.split(/[/\\]/u).at(-1) ?? value; +} + +function portableExtension(value: string): string { + return path.posix.extname(portableBasename(value)).toLowerCase(); +} + +function isBareCommand(command: string, name: "npm" | "pnpm"): boolean { + return portableBasename(command) === command && command.toLowerCase() === name; +} + +function resolveEnvValue(env: NodeJS.ProcessEnv, name: string): string | undefined { + const key = Object.keys(env).find((candidate) => candidate.toLowerCase() === name.toLowerCase()); + return key === undefined ? undefined : env[key]; +} + +export function resolveHostCommandInvocation( + command: string, + args: string[], + options: ResolveHostCommandOptions = {}, +): HostCommandInvocation { + const env = options.env ?? process.env; + const platform = options.platform ?? process.platform; + const comSpec = options.comSpec ?? resolveEnvValue(env, "ComSpec") ?? "cmd.exe"; + + if (isBareCommand(command, "pnpm")) { + const runner = resolvePnpmRunner({ + comSpec, + npmExecPath: env.npm_execpath, + nodeExecPath: options.execPath ?? process.execPath, + platform, + pnpmArgs: args, + }); + return hostInvocationFromRunner(runner); + } + + if (isBareCommand(command, "npm")) { + const runner = resolveNpmRunner({ + comSpec, + env, + execPath: options.execPath ?? process.execPath, + existsSync: options.existsSync, + npmArgs: args, + platform, + }); + return hostInvocationFromRunner(runner); + } + + const extension = portableExtension(command); + if (platform === "win32" && (extension === ".cmd" || extension === ".bat")) { + return { + args: ["/d", "/s", "/c", buildCmdExeCommandLine(command, args)], + command: comSpec, + shell: false, + windowsVerbatimArguments: true, + }; + } + + return { args, command, shell: false }; +} + export function run(command: string, args: string[], options: RunOptions = {}): CommandResult { - const result = spawnSync(command, args, { + const env = { ...process.env, ...options.env }; + const invocation = resolveHostCommandInvocation(command, args, { env }); + const result = spawnSync(invocation.command, invocation.args, { cwd: options.cwd ?? repoRoot, encoding: "utf8", - env: { ...process.env, ...options.env }, + env: invocation.env ?? env, input: options.input, maxBuffer: 50 * 1024 * 1024, stdio: options.quiet ? ["pipe", "pipe", "pipe"] : ["pipe", "pipe", "pipe"], + shell: invocation.shell, timeout: options.timeoutMs, + windowsVerbatimArguments: invocation.windowsVerbatimArguments, }); const timedOut = (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT"; @@ -67,10 +161,14 @@ export async function runStreaming( options: RunOptions & { logPath?: string } = {}, ): Promise { return await new Promise((resolve, reject) => { - const child = spawn(command, args, { + const env = { ...process.env, ...options.env }; + const invocation = resolveHostCommandInvocation(command, args, { env }); + const child = spawn(invocation.command, invocation.args, { cwd: options.cwd ?? repoRoot, - env: { ...process.env, ...options.env }, + env: invocation.env ?? env, + shell: invocation.shell, stdio: ["pipe", "pipe", "pipe"], + windowsVerbatimArguments: invocation.windowsVerbatimArguments, } satisfies SpawnOptions); let log = ""; diff --git a/scripts/generate-npm-shrinkwrap.mjs b/scripts/generate-npm-shrinkwrap.mjs index dcb323b3550..906ff4f7840 100644 --- a/scripts/generate-npm-shrinkwrap.mjs +++ b/scripts/generate-npm-shrinkwrap.mjs @@ -6,6 +6,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { parse as parseYaml } from "yaml"; import { listChangedPathsFromGit, listStagedChangedPaths } from "./changed-lanes.mjs"; +import { resolveNpmRunner } from "./npm-runner.mjs"; const ROOT_DIR = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const EXACT_VERSION_PATTERN = /^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/u; @@ -17,10 +18,6 @@ function usage() { ].join("\n"); } -function npmCommand() { - return process.platform === "win32" ? "npm.cmd" : "npm"; -} - function normalizeOverrideValue(value) { if (value === null || value === undefined) { return value; @@ -144,10 +141,25 @@ function packageJsonForShrinkwrap(packageJson, shrinkwrapOverrides) { return normalized; } +export function createNpmShrinkwrapCommand(args, options = {}) { + return resolveNpmRunner({ + comSpec: options.comSpec, + env: options.env, + execPath: options.execPath, + existsSync: options.existsSync, + npmArgs: args, + platform: options.platform, + }); +} + function runNpm(args, cwd) { - execFileSync(npmCommand(), args, { + const npm = createNpmShrinkwrapCommand(args); + execFileSync(npm.command, npm.args, { cwd, + env: npm.env ?? process.env, + shell: npm.shell, stdio: ["ignore", "pipe", "pipe"], + windowsVerbatimArguments: npm.windowsVerbatimArguments, }); } @@ -393,7 +405,7 @@ function listPublishablePluginPackageDirs() { const extensionsDir = path.join(ROOT_DIR, "extensions"); return readdirSync(extensionsDir, { withFileTypes: true }) .filter((entry) => entry.isDirectory()) - .map((entry) => path.join("extensions", entry.name)) + .map((entry) => path.posix.join("extensions", entry.name)) .filter((packageDir) => { const packageJsonPath = path.join(ROOT_DIR, packageDir, "package.json"); if (!existsSync(packageJsonPath)) { diff --git a/scripts/lib/direct-run.mjs b/scripts/lib/direct-run.mjs new file mode 100644 index 00000000000..f2b573b039e --- /dev/null +++ b/scripts/lib/direct-run.mjs @@ -0,0 +1,18 @@ +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +export function isDirectRunPath(directPath, modulePath, platform = process.platform) { + if (!directPath || !modulePath) { + return false; + } + const pathImpl = platform === "win32" ? path.win32 : path; + const normalize = + platform === "win32" + ? (value) => pathImpl.resolve(value).toLowerCase() + : (value) => pathImpl.resolve(value); + return normalize(directPath) === normalize(modulePath); +} + +export function isDirectRunUrl(directPath, moduleUrl, platform = process.platform) { + return isDirectRunPath(directPath, fileURLToPath(moduleUrl), platform); +} diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index 431674480d9..5f58b358c61 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -17,6 +17,7 @@ import { shouldRunShrinkwrapGuard, createShrinkwrapGuardCommand, } from "../../scripts/check-changed.mjs"; +import { isDirectRunPath } from "../../scripts/lib/direct-run.mjs"; import { cleanupTempDirs, makeTempRepoRoot } from "../helpers/temp-repo.js"; const tempDirs: string[] = []; @@ -72,6 +73,23 @@ afterEach(() => { }); describe("scripts/changed-lanes", () => { + it("detects direct script execution from Windows argv paths", () => { + expect( + isDirectRunPath( + "C:\\repo\\scripts\\check-changed.mjs", + "c:\\repo\\scripts\\check-changed.mjs", + "win32", + ), + ).toBe(true); + expect( + isDirectRunPath( + "C:\\repo\\scripts\\changed-lanes.mjs", + "C:\\repo\\scripts\\check-changed.mjs", + "win32", + ), + ).toBe(false); + }); + it("includes untracked worktree files in the default local diff", () => { const dir = makeTempRepoRoot(tempDirs, "openclaw-changed-lanes-"); git(dir, ["init", "-q", "--initial-branch=main"]); @@ -864,7 +882,9 @@ describe("scripts/changed-lanes", () => { const plan = createChangedCheckPlan(result); const shrinkwrapGuard = createShrinkwrapGuardCommand(["extensions/slack/package.json"]); - expect(shrinkwrapGuard?.args.some((arg) => arg.endsWith("extensions/slack"))).toBe(true); + expect( + shrinkwrapGuard?.args.some((arg) => arg.replaceAll("\\", "/").endsWith("extensions/slack")), + ).toBe(true); expect(plan.commands.map((command) => command.name)).toContain("npm shrinkwrap guard"); expect(plan.commands.map((command) => command.args[0])).not.toContain("deps:shrinkwrap:check"); }); diff --git a/test/scripts/generate-npm-shrinkwrap.test.ts b/test/scripts/generate-npm-shrinkwrap.test.ts index daa07dc4568..07f27c20702 100644 --- a/test/scripts/generate-npm-shrinkwrap.test.ts +++ b/test/scripts/generate-npm-shrinkwrap.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { collectOverrideViolations, collectPnpmLockViolations, + createNpmShrinkwrapCommand, disableShrinkwrappedOverrideConflictSources, exactOverrideRulesFromOverrides, exactVersionFromOverrideSpec, @@ -13,6 +14,30 @@ import { } from "../../scripts/generate-npm-shrinkwrap.mjs"; describe("generate-npm-shrinkwrap", () => { + function repoRelativePath(value: string): string { + return path.relative(process.cwd(), value).replaceAll("\\", "/"); + } + + it("runs npm shrinkwrap through cmd.exe for Windows npm shims", () => { + const execPath = "C:\\nodejs\\node.exe"; + const npmCmdPath = path.win32.resolve(path.win32.dirname(execPath), "npm.cmd"); + + expect( + createNpmShrinkwrapCommand(["shrinkwrap", "--ignore-scripts"], { + comSpec: "C:\\Windows\\System32\\cmd.exe", + env: {}, + execPath, + existsSync: (candidate: string) => candidate === npmCmdPath, + platform: "win32", + }), + ).toEqual({ + args: ["/d", "/s", "/c", `${npmCmdPath} shrinkwrap --ignore-scripts`], + command: "C:\\Windows\\System32\\cmd.exe", + shell: false, + windowsVerbatimArguments: true, + }); + }); + it("extracts exact versions from npm override specs", () => { expect(exactVersionFromOverrideSpec("8.4.0")).toBe("8.4.0"); expect(exactVersionFromOverrideSpec("npm:@nolyfill/domexception@1.0.28")).toBe("1.0.28"); @@ -152,13 +177,13 @@ describe("generate-npm-shrinkwrap", () => { shrinkwrapPackageDirsForChangedPaths([ "extensions/acpx/package.json", "extensions/acpx/npm-shrinkwrap.json", - ]).map((packageDir) => path.relative(process.cwd(), packageDir)), + ]).map(repoRelativePath), ).toEqual(["extensions/acpx"]); }); it("falls back to every shrinkwrap when lockfile ownership is ambiguous", () => { - const packageDirs = shrinkwrapPackageDirsForChangedPaths(["pnpm-lock.yaml"]).map((packageDir) => - path.relative(process.cwd(), packageDir), + const packageDirs = shrinkwrapPackageDirsForChangedPaths(["pnpm-lock.yaml"]).map( + repoRelativePath, ); expect(packageDirs).toContain(""); @@ -169,7 +194,7 @@ describe("generate-npm-shrinkwrap", () => { const packageDirs = shrinkwrapPackageDirsForChangedPaths([ "extensions/acpx/package.json", "pnpm-lock.yaml", - ]).map((packageDir) => path.relative(process.cwd(), packageDir)); + ]).map(repoRelativePath); expect(packageDirs).toContain(""); expect(packageDirs).toContain("extensions/acpx"); diff --git a/test/scripts/parallels-smoke-model.test.ts b/test/scripts/parallels-smoke-model.test.ts index bd7e5593654..58228d7ce6c 100644 --- a/test/scripts/parallels-smoke-model.test.ts +++ b/test/scripts/parallels-smoke-model.test.ts @@ -7,9 +7,10 @@ import { writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; -import { delimiter, join } from "node:path"; +import { delimiter, join, win32 } from "node:path"; import { pathToFileURL } from "node:url"; import { describe, expect, it } from "vitest"; +import { resolveHostCommandInvocation } from "../../scripts/e2e/parallels/host-command.ts"; import { execNodeEvalSync, spawnNodeEvalSync } from "../../src/test-utils/node-process.js"; const WRAPPERS = { @@ -568,6 +569,55 @@ console.log(JSON.stringify(result)); expect(result.stdout).toBeTypeOf("string"); }); + it("routes Windows host pnpm and npm shims through safe runners", () => { + const comSpec = "C:\\Windows\\System32\\cmd.exe"; + + expect( + resolveHostCommandInvocation("pnpm", ["build"], { + env: { + ComSpec: comSpec, + npm_execpath: "C:\\Tools\\pnpm.cmd", + }, + platform: "win32", + }), + ).toEqual({ + args: ["/d", "/s", "/c", "C:\\Tools\\pnpm.cmd build"], + command: comSpec, + shell: false, + windowsVerbatimArguments: true, + }); + + const execPath = "C:\\nodejs\\node.exe"; + const npmCmdPath = win32.resolve(win32.dirname(execPath), "npm.cmd"); + expect( + resolveHostCommandInvocation("npm", ["view", "openclaw", "version"], { + env: { ComSpec: comSpec }, + execPath, + existsSync: (candidate) => candidate === npmCmdPath, + platform: "win32", + }), + ).toEqual({ + args: ["/d", "/s", "/c", `${npmCmdPath} view openclaw version`], + command: comSpec, + shell: false, + windowsVerbatimArguments: true, + }); + }); + + it("wraps explicit Windows batch host commands without shell mode", () => { + expect( + resolveHostCommandInvocation("C:\\Tools\\helper.cmd", ["@scope/pkg@^1.0.0"], { + comSpec: "cmd.exe", + platform: "win32", + }), + ).toEqual({ + args: ["/d", "/s", "/c", "C:\\Tools\\helper.cmd @scope/pkg@^^1.0.0"], + command: "cmd.exe", + shell: false, + windowsVerbatimArguments: true, + }); + }); + it("runs the Windows agent turn through the detached done-file runner", () => { const script = readFileSync(TS_PATHS.windows, "utf8");