diff --git a/src/cli/hooks-cli.process.test.ts b/src/cli/hooks-cli.process.test.ts index 736c7590de77..ce88d3038825 100644 --- a/src/cli/hooks-cli.process.test.ts +++ b/src/cli/hooks-cli.process.test.ts @@ -1,23 +1,33 @@ // Hooks CLI process tests cover plugin-owned handles that outlive command output. -import { spawn } from "node:child_process"; +import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import { once } from "node:events"; import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; +import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; -const tempDirs: string[] = []; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); +const activeChildren = new Set(); afterEach(async () => { - await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { force: true, recursive: true }))); + await Promise.all(Array.from(activeChildren, terminateChild)); }); +async function terminateChild(child: ChildProcessWithoutNullStreams): Promise { + if (child.exitCode !== null || child.signalCode !== null) { + return; + } + child.kill("SIGKILL"); + await once(child, "close"); +} + async function createLingeringPluginFixture(): Promise<{ configPath: string; markerPath: string; stateDir: string; }> { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-hooks-cli-")); - tempDirs.push(root); + const root = tempDirs.make("openclaw-hooks-cli-"); const stateDir = path.join(root, "state"); const pluginDir = path.join(root, "linger-plugin"); const markerPath = path.join(root, "registered"); @@ -68,24 +78,45 @@ async function createLingeringPluginFixture(): Promise<{ return { configPath, markerPath, stateDir }; } -async function runHooksList(fixture: Awaited>) { - const child = spawn( - process.execPath, - ["--import", "tsx", "src/entry.ts", "hooks", "list", "--json"], - { - cwd: path.resolve("."), - env: { - ...process.env, - LINGER_MARKER: fixture.markerPath, - OPENCLAW_CONFIG_PATH: fixture.configPath, - OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1", - OPENCLAW_STATE_DIR: fixture.stateDir, - NODE_ENV: undefined, - VITEST: undefined, - }, - stdio: ["ignore", "pipe", "pipe"], - }, +async function createLingeringPreloadFixture(): Promise<{ + markerPath: string; + preloadPath: string; + stateDir: string; +}> { + const root = tempDirs.make("openclaw-hooks-relay-"); + const markerPath = path.join(root, "loaded"); + const preloadPath = path.join(root, "linger.mjs"); + const stateDir = path.join(root, "state"); + await fs.mkdir(stateDir, { recursive: true }); + await fs.writeFile( + preloadPath, + [ + 'import fs from "node:fs";', + 'fs.writeFileSync(process.env.LINGER_MARKER, "loaded\\n");', + "setInterval(() => {}, 60_000);", + "", + ].join("\n"), ); + return { markerPath, preloadPath, stateDir }; +} + +async function runHooksCli(params: { + args: string[]; + env?: NodeJS.ProcessEnv; + stdin?: string; + timeoutMessage: string; +}) { + const child = spawn(process.execPath, ["--import", "tsx", "src/entry.ts", ...params.args], { + cwd: path.resolve("."), + env: { + ...process.env, + NODE_ENV: undefined, + VITEST: undefined, + ...params.env, + }, + stdio: ["pipe", "pipe", "pipe"], + }); + activeChildren.add(child); let stdout = ""; let stderr = ""; child.stdout.setEncoding("utf8"); @@ -96,6 +127,7 @@ async function runHooksList(fixture: Awaited { stderr += chunk; }); + child.stdin.end(params.stdin ?? ""); return await new Promise<{ code: number | null; @@ -103,30 +135,102 @@ async function runHooksList(fixture: Awaited((resolve, reject) => { + let timedOut = false; const timer = setTimeout(() => { + timedOut = true; child.kill("SIGKILL"); - reject(new Error("hooks list did not exit after emitting output")); }, 15_000); child.once("error", (error) => { clearTimeout(timer); + activeChildren.delete(child); reject(error); }); child.once("close", (code, signal) => { clearTimeout(timer); + activeChildren.delete(child); + if (timedOut) { + reject(new Error(`${params.timeoutMessage}\nstdout:\n${stdout}\nstderr:\n${stderr}`)); + return; + } resolve({ code, signal, stderr, stdout }); }); }); } +async function runHooksRelay(params: { event: "post_tool_use" | "pre_tool_use"; stdin: string }) { + const fixture = await createLingeringPreloadFixture(); + const result = await runHooksCli({ + args: [ + "hooks", + "relay", + "--provider", + "codex", + "--relay-id", + "missing-relay", + "--event", + params.event, + "--timeout", + "50", + ], + env: { + LINGER_MARKER: fixture.markerPath, + NODE_OPTIONS: `--import=${pathToFileURL(fixture.preloadPath).href}`, + OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1", + OPENCLAW_NO_RESPAWN: "1", + OPENCLAW_STATE_DIR: fixture.stateDir, + }, + stdin: params.stdin, + timeoutMessage: `hooks relay ${params.event} did not exit after emitting output`, + }); + await expect(fs.readFile(fixture.markerPath, "utf8")).resolves.toBe("loaded\n"); + return result; +} + describe("hooks CLI process lifecycle", () => { it("exits after JSON output when plugin registration leaves a ref'd handle", async () => { const fixture = await createLingeringPluginFixture(); - const result = await runHooksList(fixture); + const result = await runHooksCli({ + args: ["hooks", "list", "--json"], + env: { + LINGER_MARKER: fixture.markerPath, + OPENCLAW_CONFIG_PATH: fixture.configPath, + OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1", + OPENCLAW_STATE_DIR: fixture.stateDir, + }, + timeoutMessage: "hooks list did not exit after emitting output", + }); expect(result, result.stderr).toMatchObject({ code: 0, signal: null }); expect(result.stderr).not.toContain("Error:"); expect(JSON.parse(result.stdout)).toMatchObject({ hooks: expect.any(Array) }); await expect(fs.readFile(fixture.markerPath, "utf8")).resolves.toBe("registered\n"); }, 20_000); + + it("exits successfully after an observational relay result with a ref'd handle", async () => { + const result = await runHooksRelay({ event: "post_tool_use", stdin: "{}" }); + + expect(result, result.stderr).toMatchObject({ code: 0, signal: null, stdout: "" }); + expect(result.stderr).toMatch(/native hook relay (timed out|unavailable)/); + }, 20_000); + + it("flushes fail-closed PreToolUse JSON before exiting with a ref'd handle", async () => { + const result = await runHooksRelay({ event: "pre_tool_use", stdin: "{}" }); + + expect(result, result.stderr).toMatchObject({ code: 0, signal: null }); + expect(JSON.parse(result.stdout)).toMatchObject({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: expect.any(String), + }, + }); + }, 20_000); + + it("preserves a malformed-input exit code with a ref'd handle", async () => { + const result = await runHooksRelay({ event: "post_tool_use", stdin: "{nope" }); + + expect(result).toMatchObject({ code: 1, signal: null, stdout: "" }); + expect(result.stderr).toContain("failed to read native hook input"); + }, 20_000); }); diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts index 2dedb263200c..87378333cef9 100644 --- a/src/cli/hooks-cli.ts +++ b/src/cli/hooks-cli.ts @@ -161,19 +161,20 @@ function writeHooksOutput(value: string, json: boolean | undefined): void { defaultRuntime.log(value); } -async function runHooksCliAction(action: () => Promise | void): Promise { +async function runHooksCliAction(action: () => Promise | T): Promise { try { - await action(); + return await action(); } catch (err) { - exitHooksCliWithError(err); + return exitHooksCliWithError(err); } } -async function runOneShotHooksCliAction(action: () => Promise | void): Promise { - await runHooksCliAction(action); - // Plugin registration can leave ref'd handles behind. Defer exit until runCli - // finishes shared teardown and drains both output streams. - requestExitAfterOneShotOutput(); +async function runOneShotHooksCliAction(action: () => Promise): Promise { + const result = await runHooksCliAction(action); + const exitCode = typeof result === "number" ? result : 0; + // CLI setup and handlers can leave ref'd handles behind. Defer exit until + // runCli finishes shared teardown and drains both output streams. + requestExitAfterOneShotOutput(defaultRuntime, exitCode); } /** @@ -554,9 +555,7 @@ export function registerHooksCli(program: Command): void { ) .option("--timeout ", "Gateway timeout in ms", "5000") .action(async (opts: NativeHookRelayCliOptions) => - runHooksCliAction(async () => { - process.exitCode = await runNativeHookRelayCli(opts); - }), + runOneShotHooksCliAction(() => runNativeHookRelayCli(opts)), ); hooks