From 53462b990d95a34236cd42726e5b73ae6722d849 Mon Sep 17 00:00:00 2001 From: Harold Hunt Date: Sun, 15 Mar 2026 11:14:28 -0400 Subject: [PATCH] chore(gateway): ignore `.test.ts` changes in `gateway:watch` (#36211) --- scripts/watch-node.d.mts | 12 +++ scripts/watch-node.mjs | 143 ++++++++++++++++++++++++----------- src/infra/watch-node.test.ts | 117 ++++++++++++++++++++++++---- 3 files changed, 211 insertions(+), 61 deletions(-) diff --git a/scripts/watch-node.d.mts b/scripts/watch-node.d.mts index d0e9dd93751..362670826a6 100644 --- a/scripts/watch-node.d.mts +++ b/scripts/watch-node.d.mts @@ -4,8 +4,20 @@ export function runWatchMain(params?: { args: string[], options: unknown, ) => { + kill?: (signal?: NodeJS.Signals | number) => void; on: (event: "exit", cb: (code: number | null, signal: string | null) => void) => void; }; + createWatcher?: ( + paths: string[], + options: { + ignoreInitial: boolean; + ignored: (watchPath: string) => boolean; + }, + ) => { + on: (event: "add" | "change" | "unlink" | "error", cb: (arg?: unknown) => void) => void; + close?: () => Promise | void; + }; + watchPaths?: string[]; process?: NodeJS.Process; cwd?: string; args?: string[]; diff --git a/scripts/watch-node.mjs b/scripts/watch-node.mjs index e554796f03b..891e07439a1 100644 --- a/scripts/watch-node.mjs +++ b/scripts/watch-node.mjs @@ -2,16 +2,24 @@ import { spawn } from "node:child_process"; import process from "node:process"; import { pathToFileURL } from "node:url"; +import chokidar from "chokidar"; import { runNodeWatchedPaths } from "./run-node.mjs"; const WATCH_NODE_RUNNER = "scripts/run-node.mjs"; +const WATCH_RESTART_SIGNAL = "SIGTERM"; -const buildWatchArgs = (args) => [ - ...runNodeWatchedPaths.flatMap((watchPath) => ["--watch-path", watchPath]), - "--watch-preserve-output", - WATCH_NODE_RUNNER, - ...args, -]; +const buildRunnerArgs = (args) => [WATCH_NODE_RUNNER, ...args]; + +const normalizePath = (filePath) => String(filePath ?? "").replaceAll("\\", "/"); + +const isIgnoredWatchPath = (filePath) => { + const normalizedPath = normalizePath(filePath); + return ( + normalizedPath.endsWith(".test.ts") || + normalizedPath.endsWith(".test.tsx") || + normalizedPath.endsWith("test-helpers.ts") + ); +}; export async function runWatchMain(params = {}) { const deps = { @@ -21,6 +29,9 @@ export async function runWatchMain(params = {}) { args: params.args ?? process.argv.slice(2), env: params.env ? { ...params.env } : { ...process.env }, now: params.now ?? Date.now, + createWatcher: + params.createWatcher ?? ((watchPaths, options) => chokidar.watch(watchPaths, options)), + watchPaths: params.watchPaths ?? runNodeWatchedPaths, }; const childEnv = { ...deps.env }; @@ -31,54 +42,96 @@ export async function runWatchMain(params = {}) { childEnv.OPENCLAW_WATCH_COMMAND = deps.args.join(" "); } - const watchProcess = deps.spawn(deps.process.execPath, buildWatchArgs(deps.args), { - cwd: deps.cwd, - env: childEnv, - stdio: "inherit", - }); - - let settled = false; - let onSigInt; - let onSigTerm; - - const settle = (resolve, code) => { - if (settled) { - return; - } - settled = true; - if (onSigInt) { - deps.process.off("SIGINT", onSigInt); - } - if (onSigTerm) { - deps.process.off("SIGTERM", onSigTerm); - } - resolve(code); - }; - return await new Promise((resolve) => { - onSigInt = () => { - if (typeof watchProcess.kill === "function") { - watchProcess.kill("SIGTERM"); + let settled = false; + let shuttingDown = false; + let restartRequested = false; + let watchProcess = null; + let onSigInt; + let onSigTerm; + + const watcher = deps.createWatcher(deps.watchPaths, { + ignoreInitial: true, + ignored: (watchPath) => isIgnoredWatchPath(watchPath), + }); + + const settle = (code) => { + if (settled) { + return; } - settle(resolve, 130); + settled = true; + if (onSigInt) { + deps.process.off("SIGINT", onSigInt); + } + if (onSigTerm) { + deps.process.off("SIGTERM", onSigTerm); + } + watcher.close?.().catch?.(() => {}); + resolve(code); + }; + + const startRunner = () => { + watchProcess = deps.spawn(deps.process.execPath, buildRunnerArgs(deps.args), { + cwd: deps.cwd, + env: childEnv, + stdio: "inherit", + }); + watchProcess.on("exit", () => { + watchProcess = null; + if (shuttingDown) { + return; + } + if (restartRequested) { + restartRequested = false; + startRunner(); + } + }); + }; + + const requestRestart = (changedPath) => { + if (shuttingDown || isIgnoredWatchPath(changedPath)) { + return; + } + if (!watchProcess) { + startRunner(); + return; + } + restartRequested = true; + if (typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); + } + }; + + watcher.on("add", requestRestart); + watcher.on("change", requestRestart); + watcher.on("unlink", requestRestart); + watcher.on("error", () => { + shuttingDown = true; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); + } + settle(1); + }); + + startRunner(); + + onSigInt = () => { + shuttingDown = true; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); + } + settle(130); }; onSigTerm = () => { - if (typeof watchProcess.kill === "function") { - watchProcess.kill("SIGTERM"); + shuttingDown = true; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); } - settle(resolve, 143); + settle(143); }; deps.process.on("SIGINT", onSigInt); deps.process.on("SIGTERM", onSigTerm); - - watchProcess.on("exit", (code, signal) => { - if (signal) { - settle(resolve, 1); - return; - } - settle(resolve, code ?? 1); - }); }); } diff --git a/src/infra/watch-node.test.ts b/src/infra/watch-node.test.ts index 69adbab7fc4..89ec4b79ef2 100644 --- a/src/infra/watch-node.test.ts +++ b/src/infra/watch-node.test.ts @@ -11,40 +11,50 @@ const createFakeProcess = () => const createWatchHarness = () => { const child = Object.assign(new EventEmitter(), { - kill: vi.fn(), + kill: vi.fn(() => {}), }); const spawn = vi.fn(() => child); + const watcher = Object.assign(new EventEmitter(), { + close: vi.fn(async () => {}), + }); + const createWatcher = vi.fn(() => watcher); const fakeProcess = createFakeProcess(); - return { child, spawn, fakeProcess }; + return { child, spawn, watcher, createWatcher, fakeProcess }; }; describe("watch-node script", () => { - it("wires node watch to run-node with watched source/config paths", async () => { - const { child, spawn, fakeProcess } = createWatchHarness(); + it("wires chokidar watch to run-node with watched source/config paths", async () => { + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], cwd: "/tmp/openclaw", + createWatcher, env: { PATH: "/usr/bin" }, now: () => 1700000000000, process: fakeProcess, spawn, }); - queueMicrotask(() => child.emit("exit", 0, null)); - const exitCode = await runPromise; + expect(createWatcher).toHaveBeenCalledTimes(1); + const firstWatcherCall = createWatcher.mock.calls[0]; + expect(firstWatcherCall).toBeDefined(); + const [watchPaths, watchOptions] = firstWatcherCall as unknown as [ + string[], + { ignoreInitial: boolean; ignored: (watchPath: string) => boolean }, + ]; + expect(watchPaths).toEqual(runNodeWatchedPaths); + expect(watchOptions.ignoreInitial).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node.test.ts")).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node.test.tsx")).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node-test-helpers.ts")).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node.ts")).toBe(false); + expect(watchOptions.ignored("tsconfig.json")).toBe(false); - expect(exitCode).toBe(0); expect(spawn).toHaveBeenCalledTimes(1); expect(spawn).toHaveBeenCalledWith( "/usr/local/bin/node", - [ - ...runNodeWatchedPaths.flatMap((watchPath) => ["--watch-path", watchPath]), - "--watch-preserve-output", - "scripts/run-node.mjs", - "gateway", - "--force", - ], + ["scripts/run-node.mjs", "gateway", "--force"], expect.objectContaining({ cwd: "/tmp/openclaw", stdio: "inherit", @@ -56,13 +66,19 @@ describe("watch-node script", () => { }), }), ); + fakeProcess.emit("SIGINT"); + const exitCode = await runPromise; + expect(exitCode).toBe(130); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); }); it("terminates child on SIGINT and returns shell interrupt code", async () => { - const { child, spawn, fakeProcess } = createWatchHarness(); + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], + createWatcher, process: fakeProcess, spawn, }); @@ -72,15 +88,17 @@ describe("watch-node script", () => { expect(exitCode).toBe(130); expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); expect(fakeProcess.listenerCount("SIGINT")).toBe(0); expect(fakeProcess.listenerCount("SIGTERM")).toBe(0); }); it("terminates child on SIGTERM and returns shell terminate code", async () => { - const { child, spawn, fakeProcess } = createWatchHarness(); + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], + createWatcher, process: fakeProcess, spawn, }); @@ -90,7 +108,74 @@ describe("watch-node script", () => { expect(exitCode).toBe(143); expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); expect(fakeProcess.listenerCount("SIGINT")).toBe(0); expect(fakeProcess.listenerCount("SIGTERM")).toBe(0); }); + + it("ignores test-only changes and restarts on non-test source changes", async () => { + const childA = Object.assign(new EventEmitter(), { + kill: vi.fn(function () { + queueMicrotask(() => childA.emit("exit", 0, null)); + }), + }); + const childB = Object.assign(new EventEmitter(), { + kill: vi.fn(() => {}), + }); + const spawn = vi.fn().mockReturnValueOnce(childA).mockReturnValueOnce(childB); + const watcher = Object.assign(new EventEmitter(), { + close: vi.fn(async () => {}), + }); + const createWatcher = vi.fn(() => watcher); + const fakeProcess = createFakeProcess(); + + const runPromise = runWatchMain({ + args: ["gateway", "--force"], + createWatcher, + process: fakeProcess, + spawn, + }); + + watcher.emit("change", "src/infra/watch-node.test.ts"); + await new Promise((resolve) => setImmediate(resolve)); + expect(spawn).toHaveBeenCalledTimes(1); + expect(childA.kill).not.toHaveBeenCalled(); + + watcher.emit("change", "src/infra/watch-node.test.tsx"); + await new Promise((resolve) => setImmediate(resolve)); + expect(spawn).toHaveBeenCalledTimes(1); + expect(childA.kill).not.toHaveBeenCalled(); + + watcher.emit("change", "src/infra/watch-node-test-helpers.ts"); + await new Promise((resolve) => setImmediate(resolve)); + expect(spawn).toHaveBeenCalledTimes(1); + expect(childA.kill).not.toHaveBeenCalled(); + + watcher.emit("change", "src/infra/watch-node.ts"); + await new Promise((resolve) => setImmediate(resolve)); + expect(childA.kill).toHaveBeenCalledWith("SIGTERM"); + expect(spawn).toHaveBeenCalledTimes(2); + + fakeProcess.emit("SIGINT"); + const exitCode = await runPromise; + expect(exitCode).toBe(130); + }); + + it("kills child and exits when watcher emits an error", async () => { + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); + + const runPromise = runWatchMain({ + args: ["gateway", "--force"], + createWatcher, + process: fakeProcess, + spawn, + }); + + watcher.emit("error", new Error("watch failed")); + const exitCode = await runPromise; + + expect(exitCode).toBe(1); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); + }); });