diff --git a/CHANGELOG.md b/CHANGELOG.md index 55ca8043eb0..b7853021e3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Checks/Windows: route full `pnpm check` stage commands through the managed child runner so Windows avoids Node shell-argv deprecation warnings there too. - Checks/Windows: run managed child commands through explicit `cmd.exe` wrapping instead of Node shell mode with argv, avoiding Node 24 subprocess deprecation warnings during changed checks. - Models: prune retired Groq, GitHub Copilot, OpenAI, xAI, and old Claude catalog entries, with doctor migration to upgrade existing configs to current provider refs. - Doctor/update: recognize junction-backed source checkouts as git installs by comparing canonical paths before showing package-manager update guidance. Fixes #82215. Thanks @igormf. diff --git a/scripts/check.mjs b/scripts/check.mjs index 252c52c0b64..9de8f4edf0c 100644 --- a/scripts/check.mjs +++ b/scripts/check.mjs @@ -1,6 +1,6 @@ -import { spawn } from "node:child_process"; import { performance } from "node:perf_hooks"; import { printTimingSummary } from "./lib/check-timing-summary.mjs"; +import { runManagedCommand } from "./lib/managed-child-process.mjs"; export async function main(argv = process.argv.slice(2)) { const timed = argv.includes("--timed"); @@ -111,30 +111,22 @@ async function runSerial(commands) { return results; } -async function runCommand(command) { +export async function runCommand(command, runManagedCommandImpl = runManagedCommand) { const startedAt = performance.now(); - const child = spawn("pnpm", command.args, { - stdio: "inherit", - shell: process.platform === "win32", - }); - - return await new Promise((resolve) => { - child.once("error", (error) => { - console.error(error); - resolve({ - name: command.name, - durationMs: performance.now() - startedAt, - status: 1, - }); + let status = 1; + try { + status = await runManagedCommandImpl({ + args: command.args, + bin: "pnpm", }); - child.once("close", (status) => { - resolve({ - name: command.name, - durationMs: performance.now() - startedAt, - status: status ?? 1, - }); - }); - }); + } catch (error) { + console.error(error); + } + return { + name: command.name, + durationMs: performance.now() - startedAt, + status, + }; } function printSummary(timings) { diff --git a/scripts/lib/managed-child-process.mjs b/scripts/lib/managed-child-process.mjs index 0b078573719..9efc998cee0 100644 --- a/scripts/lib/managed-child-process.mjs +++ b/scripts/lib/managed-child-process.mjs @@ -3,6 +3,8 @@ import { buildCmdExeCommandLine } from "../windows-cmd-helpers.mjs"; const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGHUP"]; const FORCE_KILL_DELAY_MS = 5_000; +const managedChildren = new Set(); +const signalHandlers = new Map(); /** * @param {NodeJS.Signals} signal @@ -80,37 +82,83 @@ export async function runManagedCommand({ comSpec, }); const child = spawn(spawnSpec.command, spawnSpec.args, spawnSpec.options); - - let receivedSignal = null; - let forceKillTimer = null; - - const forwardSignal = (signal) => { - receivedSignal ??= signal; - terminateManagedChild(child, signal); - forceKillTimer ??= setTimeout(() => { - terminateManagedChild(child, "SIGKILL"); - }, FORCE_KILL_DELAY_MS); + const managedChild = { + child, + forceKillTimer: null, + receivedSignal: null, }; - - for (const signal of FORWARDED_SIGNALS) { - process.once(signal, forwardSignal); - } + addManagedChild(managedChild); onReady?.(child); try { return await new Promise((resolve, reject) => { child.once("error", reject); child.once("close", (status) => { - if (forceKillTimer) { - clearTimeout(forceKillTimer); + if (managedChild.forceKillTimer) { + clearTimeout(managedChild.forceKillTimer); } - resolve(receivedSignal ? signalExitCode(receivedSignal) : (status ?? 1)); + resolve(managedChild.receivedSignal ? signalExitCode(managedChild.receivedSignal) : (status ?? 1)); }); }); } finally { - for (const signal of FORWARDED_SIGNALS) { - process.off(signal, forwardSignal); + removeManagedChild(managedChild); + } +} + +/** + * @param {{ + * child: import("node:child_process").ChildProcess; + * forceKillTimer: NodeJS.Timeout | null; + * receivedSignal: NodeJS.Signals | null; + * }} managedChild + */ +function addManagedChild(managedChild) { + managedChildren.add(managedChild); + installSignalHandlers(); +} + +/** + * @param {{ + * child: import("node:child_process").ChildProcess; + * forceKillTimer: NodeJS.Timeout | null; + * receivedSignal: NodeJS.Signals | null; + * }} managedChild + */ +function removeManagedChild(managedChild) { + managedChildren.delete(managedChild); + if (managedChildren.size === 0) { + removeSignalHandlers(); + } +} + +function installSignalHandlers() { + for (const signal of FORWARDED_SIGNALS) { + if (signalHandlers.has(signal)) { + continue; } + const handler = () => forwardSignalToManagedChildren(signal); + signalHandlers.set(signal, handler); + process.on(signal, handler); + } +} + +function removeSignalHandlers() { + for (const [signal, handler] of signalHandlers) { + process.off(signal, handler); + } + signalHandlers.clear(); +} + +/** + * @param {NodeJS.Signals} signal + */ +function forwardSignalToManagedChildren(signal) { + for (const managedChild of managedChildren) { + managedChild.receivedSignal ??= signal; + terminateManagedChild(managedChild.child, signal); + managedChild.forceKillTimer ??= setTimeout(() => { + terminateManagedChild(managedChild.child, "SIGKILL"); + }, FORCE_KILL_DELAY_MS); } } diff --git a/test/scripts/check.test.ts b/test/scripts/check.test.ts new file mode 100644 index 00000000000..ccb5f8561db --- /dev/null +++ b/test/scripts/check.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { runCommand } from "../../scripts/check.mjs"; + +describe("scripts/check", () => { + it("runs pnpm commands through the managed child runner", async () => { + const calls: Array<{ args: string[]; bin: string }> = []; + const result = await runCommand( + { args: ["lint"], name: "lint" }, + async (options: { args: string[]; bin: string }) => { + calls.push(options); + return 0; + }, + ); + + expect(calls).toEqual([{ args: ["lint"], bin: "pnpm" }]); + expect(result).toMatchObject({ name: "lint", status: 0 }); + expect(result.durationMs).toBeGreaterThanOrEqual(0); + }); +}); diff --git a/test/scripts/managed-child-process.test.ts b/test/scripts/managed-child-process.test.ts index 8294d7dc310..7458984738a 100644 --- a/test/scripts/managed-child-process.test.ts +++ b/test/scripts/managed-child-process.test.ts @@ -6,6 +6,7 @@ import { pathToFileURL } from "node:url"; import { describe, expect, it } from "vitest"; import { createManagedCommandSpawnSpec, + runManagedCommand, signalExitCode, } from "../../scripts/lib/managed-child-process.mjs"; import { createScriptTestHarness } from "./test-helpers.js"; @@ -84,6 +85,36 @@ describe("managed-child-process", () => { ).toThrow("unsafe Windows cmd.exe argument detected"); }); + it("shares process signal listeners across parallel managed commands", async () => { + const signals = ["SIGHUP", "SIGINT", "SIGTERM"] as const; + const baseline = new Map(signals.map((signal) => [signal, process.listenerCount(signal)])); + let readyCount = 0; + const commands = Array.from({ length: 12 }, () => + runManagedCommand({ + args: ["-e", "setTimeout(() => {}, 500)"], + bin: process.execPath, + shell: false, + stdio: "ignore", + onReady: () => { + readyCount += 1; + }, + }), + ); + + try { + await waitFor(() => readyCount === commands.length); + for (const signal of signals) { + expect(process.listenerCount(signal)).toBe((baseline.get(signal) ?? 0) + 1); + } + } finally { + await Promise.all(commands); + } + + for (const signal of signals) { + expect(process.listenerCount(signal)).toBe(baseline.get(signal) ?? 0); + } + }); + posixIt("kills the managed child process group when the runner is terminated", async () => { const dir = createTempDir("openclaw-managed-child-"); const childPath = path.join(dir, "child.mjs");