mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 04:54:01 +00:00
fix: Windows CLI backends fail through npm shims (#101378)
* fix(process): resolve Windows npm CLI shims directly Co-authored-by: wendy-chsy <wan.wenyan@xydigit.com> * docs(changelog): note Windows CLI shim fix * docs(process): explain Windows shim boundary * chore: keep release changelog owner-only * test(process): isolate Windows shim resolution * fix(process): classify Windows forwarding shims safely --------- Co-authored-by: wendy-chsy <wan.wenyan@xydigit.com>
This commit is contained in:
committed by
GitHub
parent
72bd74e9dd
commit
7b366e16b0
@@ -79,4 +79,54 @@ describe("resolveWindowsSpawnProgram", () => {
|
||||
windowsHide: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves custom batch-wrapper behavior instead of bypassing its target", async () => {
|
||||
const dir = await createTempDir("openclaw-windows-spawn-test-");
|
||||
const targetPath = path.join(dir, "tool.exe");
|
||||
const wrapperPath = path.join(dir, "wrapper.cmd");
|
||||
await writeFile(targetPath, "", "utf8");
|
||||
await writeFile(
|
||||
wrapperPath,
|
||||
'@ECHO off\r\nSET WRAPPER_FLAG=1\r\n"%~dp0\\tool.exe" %*\r\n',
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const resolved = resolveWindowsSpawnProgram({
|
||||
command: wrapperPath,
|
||||
platform: "win32",
|
||||
env: { PATH: dir, PATHEXT: ".CMD;.EXE;.BAT" },
|
||||
execPath: "C:\\node\\node.exe",
|
||||
allowShellFallback: true,
|
||||
});
|
||||
|
||||
expect(resolved).toEqual({
|
||||
command: wrapperPath,
|
||||
leadingArgv: [],
|
||||
resolution: "shell-fallback",
|
||||
shell: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not reinterpret a forwarded batch wrapper as a Node script", async () => {
|
||||
const dir = await createTempDir("openclaw-windows-spawn-test-");
|
||||
const targetPath = path.join(dir, "inner.cmd");
|
||||
const wrapperPath = path.join(dir, "wrapper.cmd");
|
||||
await writeFile(targetPath, "@ECHO off\r\necho inner\r\n", "utf8");
|
||||
await writeFile(wrapperPath, '@ECHO off\r\n"%~dp0\\inner.cmd" %*\r\n', "utf8");
|
||||
|
||||
const resolved = resolveWindowsSpawnProgram({
|
||||
command: wrapperPath,
|
||||
platform: "win32",
|
||||
env: { PATH: dir, PATHEXT: ".CMD;.EXE;.BAT" },
|
||||
execPath: "C:\\node\\node.exe",
|
||||
allowShellFallback: true,
|
||||
});
|
||||
|
||||
expect(resolved).toEqual({
|
||||
command: wrapperPath,
|
||||
leadingArgv: [],
|
||||
resolution: "shell-fallback",
|
||||
shell: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -181,6 +181,25 @@ function resolveEntrypointFromCmdShim(wrapperPath: string): string | null {
|
||||
|
||||
try {
|
||||
const content = readFileSync(wrapperPath, "utf8");
|
||||
const normalizedContent = content.replaceAll("\r\n", "\n").toLowerCase();
|
||||
const significantLines = content
|
||||
.split(/\r?\n/u)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean);
|
||||
const isNpmCmdShim =
|
||||
normalizedContent.includes("\ngoto start\n") &&
|
||||
normalizedContent.includes("\n:find_dp0\n") &&
|
||||
normalizedContent.includes("set dp0=%~dp0") &&
|
||||
normalizedContent.includes("call :find_dp0");
|
||||
const isDirectForwarder =
|
||||
significantLines.length === 2 &&
|
||||
/^@echo off$/iu.test(significantLines[0] ?? "") &&
|
||||
/^"%~?dp0%?[\\/][^"\r\n]+"\s+%\*$/iu.test(significantLines[1] ?? "");
|
||||
// Only known direct-forwarder shapes are safe to bypass; arbitrary batch
|
||||
// wrappers can depend on setup commands before dispatching their target.
|
||||
if (!isNpmCmdShim && !isDirectForwarder) {
|
||||
return null;
|
||||
}
|
||||
const candidates: string[] = [];
|
||||
for (const match of content.matchAll(/"([^"\r\n]*)"/g)) {
|
||||
const token = match[1] ?? "";
|
||||
@@ -199,6 +218,12 @@ function resolveEntrypointFromCmdShim(wrapperPath: string): string | null {
|
||||
const base = normalizeLowercaseStringOrEmpty(path.basename(candidate));
|
||||
return base !== "node.exe" && base !== "node";
|
||||
});
|
||||
if (isDirectForwarder && nonNode) {
|
||||
const ext = normalizeLowercaseStringOrEmpty(path.extname(nonNode));
|
||||
if (ext !== ".exe" && ext !== ".js" && ext !== ".cjs" && ext !== ".mjs") {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
return nonNode ?? null;
|
||||
} catch {
|
||||
return null;
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
// Child adapter tests cover adapting child processes to supervisor runs.
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { PassThrough } from "node:stream";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../../../test/helpers/temp-dir.js";
|
||||
import {
|
||||
getWindowsInstallRoots,
|
||||
resetWindowsInstallRootsForTests,
|
||||
@@ -37,6 +39,7 @@ vi.mock("../../../infra/windows-encoding.js", () => ({
|
||||
}));
|
||||
|
||||
let createChildAdapter: typeof import("./child.js").createChildAdapter;
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
function createStubChild(pid = 1234) {
|
||||
const child = new EventEmitter() as ChildProcess;
|
||||
@@ -161,6 +164,25 @@ describe("createChildAdapter", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const createWindowsNpmShim = async (params: { command: string; packagePath: string[] }) => {
|
||||
const binDir = tempDirs.make("openclaw-child-shim-");
|
||||
const entrypoint = path.join(binDir, "node_modules", ...params.packagePath);
|
||||
await mkdir(path.dirname(entrypoint), { recursive: true });
|
||||
await writeFile(entrypoint, "", "utf8");
|
||||
const relativeEntrypoint = path.relative(binDir, entrypoint).replaceAll(path.sep, "\\");
|
||||
const shimHead =
|
||||
"@ECHO off\r\nGOTO start\r\n:find_dp0\r\nSET dp0=%~dp0\r\nEXIT /b\r\n:start\r\nSETLOCAL\r\nCALL :find_dp0\r\n";
|
||||
const shimCommand = entrypoint.endsWith(".exe")
|
||||
? `"%dp0%\\${relativeEntrypoint}" %*\r\n`
|
||||
: `IF EXIST "%dp0%\\node.exe" (\r\n SET "_prog=%dp0%\\node.exe"\r\n) ELSE (\r\n SET "_prog=node"\r\n)\r\nendLocal & goto #_undefined_# 2>NUL || title %COMSPEC% & "%_prog%" "%dp0%\\${relativeEntrypoint}" %*\r\n`;
|
||||
await writeFile(
|
||||
path.join(binDir, `${params.command}.cmd`),
|
||||
`${shimHead}${shimCommand}`,
|
||||
"utf8",
|
||||
);
|
||||
return { binDir, entrypoint };
|
||||
};
|
||||
|
||||
it("uses process-tree kill for default SIGKILL", async () => {
|
||||
const { adapter, killMock } = await createAdapterHarness({ pid: 4321 });
|
||||
|
||||
@@ -408,6 +430,7 @@ describe("createChildAdapter", () => {
|
||||
await createAdapterHarness({
|
||||
pid: 3335,
|
||||
argv: ["pnpm", "--version"],
|
||||
env: { PATH: "", PATHEXT: ".EXE;.CMD;.BAT" },
|
||||
});
|
||||
|
||||
const spawnArgs = firstSpawnWithFallbackParams();
|
||||
@@ -424,6 +447,48 @@ describe("createChildAdapter", () => {
|
||||
expect(spawnArgs.fallbacks).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("unwraps Gemini's npm shim and preserves prompt argv on Windows", async () => {
|
||||
setPlatform("win32");
|
||||
const { binDir, entrypoint } = await createWindowsNpmShim({
|
||||
command: "gemini",
|
||||
packagePath: ["@google", "gemini-cli", "bundle", "gemini.js"],
|
||||
});
|
||||
const nodePath = path.join(binDir, "node.exe");
|
||||
await writeFile(nodePath, "", "utf8");
|
||||
const prompt = "explain A&B | C > D and 100% coverage";
|
||||
|
||||
await createAdapterHarness({
|
||||
pid: 3336,
|
||||
argv: ["gemini", "--prompt", prompt],
|
||||
env: { PATH: binDir, PATHEXT: ".EXE;.CMD;.BAT" },
|
||||
});
|
||||
|
||||
const spawnArgs = firstSpawnWithFallbackParams();
|
||||
expect(spawnArgs.argv?.[0]?.toLowerCase()).toBe(nodePath.toLowerCase());
|
||||
expect(spawnArgs.argv?.slice(1)).toEqual([entrypoint, "--prompt", prompt]);
|
||||
expect(spawnArgs.options?.windowsVerbatimArguments).toBeUndefined();
|
||||
expect(spawnArgs.fallbacks).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("unwraps Claude's npm shim to its native executable on Windows", async () => {
|
||||
setPlatform("win32");
|
||||
const { binDir, entrypoint } = await createWindowsNpmShim({
|
||||
command: "claude",
|
||||
packagePath: ["@anthropic-ai", "claude-code", "bin", "claude.exe"],
|
||||
});
|
||||
|
||||
await createAdapterHarness({
|
||||
pid: 3337,
|
||||
argv: ["claude", "--version"],
|
||||
env: { PATH: binDir, PATHEXT: ".EXE;.CMD;.BAT" },
|
||||
});
|
||||
|
||||
const spawnArgs = firstSpawnWithFallbackParams();
|
||||
expect(spawnArgs.argv).toEqual([entrypoint, "--version"]);
|
||||
expect(spawnArgs.options?.windowsVerbatimArguments).toBeUndefined();
|
||||
expect(spawnArgs.fallbacks).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("wraps Linux child spawns and strips shell-init env", async () => {
|
||||
const originalBashEnv = process.env.BASH_ENV;
|
||||
const originalEnv = process.env.ENV;
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
import type { ChildProcessWithoutNullStreams, SpawnOptions } from "node:child_process";
|
||||
import { toErrorObject } from "../../../infra/errors.js";
|
||||
import { createWindowsOutputDecoder } from "../../../infra/windows-encoding.js";
|
||||
import {
|
||||
resolveWindowsExecutablePath,
|
||||
resolveWindowsSpawnProgramCandidate,
|
||||
} from "../../../plugin-sdk/windows-spawn.js";
|
||||
import { signalProcessTree } from "../../kill-tree.js";
|
||||
import { prepareOomScoreAdjustedSpawn } from "../../linux-oom-score.js";
|
||||
import { spawnWithFallback } from "../../spawn-utils.js";
|
||||
@@ -16,21 +20,37 @@ import { toStringEnv } from "./env.js";
|
||||
|
||||
const FORCE_KILL_WAIT_FALLBACK_MS = 4000;
|
||||
const WINDOWS_CLOSE_STATE_SETTLE_TIMEOUT_MS = 250;
|
||||
const WINDOWS_PACKAGE_MANAGER_SHIMS = ["npm", "pnpm", "yarn", "npx"] as const;
|
||||
|
||||
function resolveCommand(command: string): string {
|
||||
return resolveWindowsCommandShim({
|
||||
command,
|
||||
cmdCommands: ["npm", "pnpm", "yarn", "npx"],
|
||||
});
|
||||
}
|
||||
|
||||
function resolveChildInvocation(params: { argv: string[]; windowsVerbatimArguments?: boolean }): {
|
||||
function resolveChildInvocation(params: {
|
||||
argv: string[];
|
||||
env?: NodeJS.ProcessEnv;
|
||||
windowsVerbatimArguments?: boolean;
|
||||
}): {
|
||||
args: string[];
|
||||
command: string;
|
||||
windowsVerbatimArguments?: boolean;
|
||||
} {
|
||||
const resolvedCommand = resolveCommand(params.argv[0] ?? "");
|
||||
const args = params.argv.slice(1);
|
||||
const command = params.argv[0] ?? "";
|
||||
const candidate = resolveWindowsSpawnProgramCandidate({
|
||||
command,
|
||||
env: params.env,
|
||||
// npm shims invoke `node` from PATH; process.execPath may be a packaged OpenClaw executable.
|
||||
execPath:
|
||||
process.platform === "win32"
|
||||
? resolveWindowsExecutablePath("node", params.env ?? process.env)
|
||||
: undefined,
|
||||
});
|
||||
const args = [...candidate.leadingArgv, ...params.argv.slice(1)];
|
||||
// Keep the historical package-manager fallback when PATH probing cannot see
|
||||
// its shim; every resolved wrapper takes the direct Node/exe path above.
|
||||
const resolvedCommand =
|
||||
candidate.resolution === "direct" && candidate.command === command
|
||||
? resolveWindowsCommandShim({
|
||||
command,
|
||||
cmdCommands: WINDOWS_PACKAGE_MANAGER_SHIMS,
|
||||
})
|
||||
: candidate.command;
|
||||
if (!isWindowsBatchCommand(resolvedCommand)) {
|
||||
return {
|
||||
command: resolvedCommand,
|
||||
@@ -59,11 +79,12 @@ export async function createChildAdapter(params: {
|
||||
input?: string;
|
||||
stdinMode?: "inherit" | "pipe-open" | "pipe-closed";
|
||||
}): Promise<ChildAdapter> {
|
||||
const baseEnv = params.env ? toStringEnv(params.env) : undefined;
|
||||
const invocation = resolveChildInvocation({
|
||||
argv: params.argv,
|
||||
env: baseEnv,
|
||||
windowsVerbatimArguments: params.windowsVerbatimArguments,
|
||||
});
|
||||
const baseEnv = params.env ? toStringEnv(params.env) : undefined;
|
||||
const preparedSpawn = prepareOomScoreAdjustedSpawn(invocation.command, invocation.args, {
|
||||
env: baseEnv,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user