fix(cli): wait for respawn child shutdown

This commit is contained in:
Vincent Koc
2026-05-27 22:19:42 +02:00
parent 7f7eca1ad2
commit 00004ca798
6 changed files with 60 additions and 23 deletions

View File

@@ -1,32 +1,29 @@
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { resolveNextcloudTalkRoomKind, testing } from "./room-info.js";
const fetchWithSsrFGuard = vi.hoisted(() => vi.fn());
const readFileSync = vi.hoisted(() => vi.fn());
const tempDirs: string[] = [];
vi.mock("../runtime-api.js", () => {
return vi
.importActual<typeof import("../runtime-api.js")>("../runtime-api.js")
.then((actual) => ({
...actual,
fetchWithSsrFGuard,
}));
});
vi.mock("node:fs", () => {
return vi.importActual<typeof import("node:fs")>("node:fs").then((actual) => ({
...actual,
readFileSync,
}));
return { fetchWithSsrFGuard };
});
afterEach(() => {
fetchWithSsrFGuard.mockReset();
readFileSync.mockReset();
testing.resetRoomCache();
for (const dir of tempDirs.splice(0)) {
rmSync(dir, { force: true, recursive: true });
}
});
function requireFirstFetchParams(): { auditContext?: string; url?: string } {
function requireFirstFetchParams(): {
auditContext?: string;
init?: { headers?: { Authorization?: string } };
url?: string;
} {
const [call] = fetchWithSsrFGuard.mock.calls;
if (!call) {
throw new Error("expected Nextcloud Talk room info fetch call");
@@ -81,7 +78,10 @@ describe("nextcloud talk room info", () => {
const log = vi.fn();
const error = vi.fn();
const exit = vi.fn();
readFileSync.mockReturnValue("file-secret\n");
const tempDir = mkdtempSync(path.join(tmpdir(), "nextcloud-talk-room-info-"));
tempDirs.push(tempDir);
const passwordFile = path.join(tempDir, "secret");
writeFileSync(passwordFile, "file-secret\n", "utf-8");
fetchWithSsrFGuard.mockResolvedValue({
response: {
ok: false,
@@ -97,7 +97,7 @@ describe("nextcloud talk room info", () => {
baseUrl: "https://nc.example.com",
config: {
apiUser: "bot",
apiPasswordFile: "/tmp/nextcloud-secret",
apiPasswordFile: passwordFile,
},
} as never,
roomToken: "room-group",
@@ -105,7 +105,9 @@ describe("nextcloud talk room info", () => {
});
expect(kind).toBeUndefined();
expect(readFileSync).toHaveBeenCalledWith("/tmp/nextcloud-secret", "utf-8");
expect(requireFirstFetchParams().init?.headers?.Authorization).toBe(
"Basic Ym90OmZpbGUtc2VjcmV0",
);
expect(log).toHaveBeenCalledWith("nextcloud-talk: room lookup failed (403) token=room-group");
expect(release).toHaveBeenCalledTimes(1);
});

View File

@@ -90,6 +90,7 @@ const respawnSignals =
: ["SIGTERM", "SIGINT", "SIGHUP", "SIGQUIT"];
const respawnSignalExitGraceMs = 1_000;
const respawnSignalForceKillGraceMs = 1_000;
const respawnSignalHardExitGraceMs = 1_000;
const runRespawnedChild = (command, args, env) => {
const child = spawn(command, args, {
@@ -103,6 +104,7 @@ const runRespawnedChild = (command, args, env) => {
// a child that ignores SIGTERM cannot keep the launcher alive indefinitely.
let signalExitTimer = null;
let signalForceKillTimer = null;
let signalHardExitTimer = null;
const detach = () => {
for (const [signal, listener] of listeners) {
process.off(signal, listener);
@@ -116,6 +118,10 @@ const runRespawnedChild = (command, args, env) => {
clearTimeout(signalForceKillTimer);
signalForceKillTimer = null;
}
if (signalHardExitTimer) {
clearTimeout(signalHardExitTimer);
signalHardExitTimer = null;
}
};
const forceKillChild = () => {
try {
@@ -132,7 +138,10 @@ const runRespawnedChild = (command, args, env) => {
}
signalForceKillTimer = setTimeout(() => {
forceKillChild();
process.exit(1);
signalHardExitTimer = setTimeout(() => {
process.exit(1);
}, respawnSignalHardExitGraceMs);
signalHardExitTimer.unref?.();
}, respawnSignalForceKillGraceMs);
signalForceKillTimer.unref?.();
};

View File

@@ -200,7 +200,7 @@ describe("entry compile cache", () => {
expect(exit).toHaveBeenCalledWith(1);
});
it("terminates before force-killing a signaled compile-cache respawn child", () => {
it("waits for a signaled compile-cache respawn child after force-killing it", () => {
vi.useFakeTimers();
const child = new EventEmitter() as ChildProcess;
const kill = vi.fn<(signal?: NodeJS.Signals) => boolean>(() => true);
@@ -236,6 +236,10 @@ describe("entry compile cache", () => {
vi.advanceTimersByTime(1_000);
expect(kill).toHaveBeenCalledWith(process.platform === "win32" ? "SIGTERM" : "SIGKILL");
expect(exit).not.toHaveBeenCalled();
child.emit("exit", null, "SIGKILL");
expect(exit).toHaveBeenCalledWith(1);
} finally {
vi.useRealTimers();

View File

@@ -7,6 +7,7 @@ import { attachChildProcessBridge } from "./process/child-process-bridge.js";
const COMPILE_CACHE_RESPAWN_SIGNAL_EXIT_GRACE_MS = 1_000;
const COMPILE_CACHE_RESPAWN_SIGNAL_FORCE_KILL_GRACE_MS = 1_000;
const COMPILE_CACHE_RESPAWN_SIGNAL_HARD_EXIT_GRACE_MS = 1_000;
export function resolveEntryInstallRoot(entryFile: string): string {
const entryDir = path.dirname(entryFile);
@@ -170,6 +171,7 @@ export function runOpenClawCompileCacheRespawnPlan(
// a child that ignores SIGTERM cannot keep the compile-cache wrapper alive indefinitely.
let signalExitTimer: NodeJS.Timeout | undefined;
let signalForceKillTimer: NodeJS.Timeout | undefined;
let signalHardExitTimer: NodeJS.Timeout | undefined;
const clearSignalExitTimer = (): void => {
if (signalExitTimer) {
clearTimeout(signalExitTimer);
@@ -179,6 +181,10 @@ export function runOpenClawCompileCacheRespawnPlan(
clearTimeout(signalForceKillTimer);
signalForceKillTimer = undefined;
}
if (signalHardExitTimer) {
clearTimeout(signalHardExitTimer);
signalHardExitTimer = undefined;
}
};
const forceKillChild = (): void => {
try {
@@ -195,7 +201,10 @@ export function runOpenClawCompileCacheRespawnPlan(
}
signalForceKillTimer = setTimeout(() => {
forceKillChild();
runtime.exit(1);
signalHardExitTimer = setTimeout(() => {
runtime.exit(1);
}, COMPILE_CACHE_RESPAWN_SIGNAL_HARD_EXIT_GRACE_MS);
signalHardExitTimer.unref?.();
}, COMPILE_CACHE_RESPAWN_SIGNAL_FORCE_KILL_GRACE_MS);
signalForceKillTimer.unref?.();
};

View File

@@ -309,6 +309,10 @@ describe("runCliRespawnPlan", () => {
vi.advanceTimersByTime(1_000);
expect(kill).toHaveBeenCalledWith(process.platform === "win32" ? "SIGTERM" : "SIGKILL");
expect(exit).not.toHaveBeenCalled();
child.emit("exit", null, "SIGKILL");
expect(exit).toHaveBeenCalledWith(1);
} finally {
vi.useRealTimers();

View File

@@ -15,6 +15,7 @@ export const OPENCLAW_NODE_EXTRA_CA_CERTS_READY = "OPENCLAW_NODE_EXTRA_CA_CERTS_
const WINDOWS_STACK_SIZE_FLAG = "--stack-size=8192";
const CLI_RESPAWN_SIGNAL_EXIT_GRACE_MS = 1_000;
const CLI_RESPAWN_SIGNAL_FORCE_KILL_GRACE_MS = 1_000;
const CLI_RESPAWN_SIGNAL_HARD_EXIT_GRACE_MS = 1_000;
type CliRespawnPlan = {
command: string;
@@ -169,6 +170,7 @@ export function runCliRespawnPlan(
});
let signalExitTimer: NodeJS.Timeout | undefined;
let signalForceKillTimer: NodeJS.Timeout | undefined;
let signalHardExitTimer: NodeJS.Timeout | undefined;
const clearSignalTimers = (): void => {
if (signalExitTimer) {
clearTimeout(signalExitTimer);
@@ -178,6 +180,10 @@ export function runCliRespawnPlan(
clearTimeout(signalForceKillTimer);
signalForceKillTimer = undefined;
}
if (signalHardExitTimer) {
clearTimeout(signalHardExitTimer);
signalHardExitTimer = undefined;
}
};
const forceKillChild = (): void => {
try {
@@ -194,7 +200,10 @@ export function runCliRespawnPlan(
}
signalForceKillTimer = setTimeout(() => {
forceKillChild();
runtime.exit(1);
signalHardExitTimer = setTimeout(() => {
runtime.exit(1);
}, CLI_RESPAWN_SIGNAL_HARD_EXIT_GRACE_MS);
signalHardExitTimer.unref?.();
}, CLI_RESPAWN_SIGNAL_FORCE_KILL_GRACE_MS);
signalForceKillTimer.unref?.();
};