mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix: prevent completed Codex hook relays from lingering (#106899)
This commit is contained in:
@@ -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<ChildProcessWithoutNullStreams>();
|
||||
|
||||
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<void> {
|
||||
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<ReturnType<typeof createLingeringPluginFixture>>) {
|
||||
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<ReturnType<typeof createLingeringPl
|
||||
child.stderr.on("data", (chunk: string) => {
|
||||
stderr += chunk;
|
||||
});
|
||||
child.stdin.end(params.stdin ?? "");
|
||||
|
||||
return await new Promise<{
|
||||
code: number | null;
|
||||
@@ -103,30 +135,102 @@ async function runHooksList(fixture: Awaited<ReturnType<typeof createLingeringPl
|
||||
stderr: string;
|
||||
stdout: string;
|
||||
}>((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);
|
||||
});
|
||||
|
||||
+10
-11
@@ -161,19 +161,20 @@ function writeHooksOutput(value: string, json: boolean | undefined): void {
|
||||
defaultRuntime.log(value);
|
||||
}
|
||||
|
||||
async function runHooksCliAction(action: () => Promise<void> | void): Promise<void> {
|
||||
async function runHooksCliAction<T>(action: () => Promise<T> | T): Promise<T> {
|
||||
try {
|
||||
await action();
|
||||
return await action();
|
||||
} catch (err) {
|
||||
exitHooksCliWithError(err);
|
||||
return exitHooksCliWithError(err);
|
||||
}
|
||||
}
|
||||
|
||||
async function runOneShotHooksCliAction(action: () => Promise<void> | void): Promise<void> {
|
||||
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<number | void>): Promise<void> {
|
||||
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 <ms>", "Gateway timeout in ms", "5000")
|
||||
.action(async (opts: NativeHookRelayCliOptions) =>
|
||||
runHooksCliAction(async () => {
|
||||
process.exitCode = await runNativeHookRelayCli(opts);
|
||||
}),
|
||||
runOneShotHooksCliAction(() => runNativeHookRelayCli(opts)),
|
||||
);
|
||||
|
||||
hooks
|
||||
|
||||
Reference in New Issue
Block a user