Skip to content

Commit 6104077

Browse files
steipetesteipete
committed
fix: stop qa lab children cleanly
1 parent c643e3c commit 6104077

9 files changed

Lines changed: 313 additions & 26 deletions

‎extensions/qa-lab/src/bus-server.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { Agent, createServer, request } from "node:http";
2+
import { describe, expect, it } from "vitest";
3+
import { closeQaHttpServer } from "./bus-server.js";
4+
5+
async function listenOnLoopback(server: ReturnType<typeof createServer>): Promise<number> {
6+
await new Promise<void>((resolve, reject) => {
7+
server.once("error", reject);
8+
server.listen(0, "127.0.0.1", () => resolve());
9+
});
10+
const address = server.address();
11+
if (!address || typeof address === "string") {
12+
throw new Error("expected server to bind a TCP port");
13+
}
14+
return address.port;
15+
}
16+
17+
async function requestOnce(params: { port: number; agent: Agent }): Promise<void> {
18+
await new Promise<void>((resolve, reject) => {
19+
const req = request(
20+
{
21+
host: "127.0.0.1",
22+
port: params.port,
23+
path: "/",
24+
agent: params.agent,
25+
},
26+
(res) => {
27+
res.resume();
28+
res.on("end", resolve);
29+
res.on("error", reject);
30+
},
31+
);
32+
req.on("error", reject);
33+
req.end();
34+
});
35+
}
36+
37+
describe("closeQaHttpServer", () => {
38+
it("closes idle keep-alive sockets so suite processes can exit", async () => {
39+
const server = createServer((_req, res) => {
40+
res.writeHead(200, {
41+
"content-type": "text/plain",
42+
connection: "keep-alive",
43+
});
44+
res.end("ok");
45+
});
46+
const agent = new Agent({ keepAlive: true });
47+
const port = await listenOnLoopback(server);
48+
49+
try {
50+
await requestOnce({ port, agent });
51+
const startedAt = Date.now();
52+
await closeQaHttpServer(server);
53+
expect(Date.now() - startedAt).toBeLessThan(1_000);
54+
} finally {
55+
agent.destroy();
56+
server.closeAllConnections?.();
57+
}
58+
});
59+
});

‎extensions/qa-lab/src/bus-server.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,24 @@ export function writeError(res: ServerResponse, statusCode: number, error: unkno
3838
});
3939
}
4040

41+
export async function closeQaHttpServer(server: Server): Promise<void> {
42+
let forceCloseTimer: NodeJS.Timeout | undefined;
43+
try {
44+
await new Promise<void>((resolve, reject) => {
45+
server.close((error) => (error ? reject(error) : resolve()));
46+
server.closeIdleConnections?.();
47+
forceCloseTimer = setTimeout(() => {
48+
server.closeAllConnections?.();
49+
}, 250);
50+
forceCloseTimer.unref();
51+
});
52+
} finally {
53+
if (forceCloseTimer) {
54+
clearTimeout(forceCloseTimer);
55+
}
56+
}
57+
}
58+
4159
export async function handleQaBusRequest(params: {
4260
req: IncomingMessage;
4361
res: ServerResponse;
@@ -172,9 +190,7 @@ export async function startQaBusServer(params: { state: QaBusState; port?: numbe
172190
port: address.port,
173191
baseUrl: `http://127.0.0.1:${address.port}`,
174192
async stop() {
175-
await new Promise<void>((resolve, reject) =>
176-
server.close((error) => (error ? reject(error) : resolve())),
177-
);
193+
await closeQaHttpServer(server);
178194
},
179195
};
180196
}

‎extensions/qa-lab/src/gateway-child.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { spawn } from "node:child_process";
12
import { lstat, mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises";
23
import os from "node:os";
34
import path from "node:path";
@@ -302,6 +303,49 @@ describe("buildQaRuntimeEnv", () => {
302303
);
303304
expect(release).toHaveBeenCalledTimes(1);
304305
});
306+
307+
it("force-stops gateway children that ignore the graceful signal", async () => {
308+
const child = spawn(
309+
process.execPath,
310+
[
311+
"-e",
312+
[
313+
"process.on('SIGTERM', () => {});",
314+
"process.stdout.write('ready\\n');",
315+
"setInterval(() => {}, 1000);",
316+
].join(""),
317+
],
318+
{
319+
detached: process.platform !== "win32",
320+
stdio: ["ignore", "pipe", "ignore"],
321+
},
322+
);
323+
cleanups.push(async () => {
324+
if (child.exitCode === null && child.signalCode === null) {
325+
try {
326+
if (process.platform === "win32") {
327+
child.kill("SIGKILL");
328+
} else if (child.pid) {
329+
process.kill(-child.pid, "SIGKILL");
330+
}
331+
} catch {
332+
// The child already exited.
333+
}
334+
}
335+
});
336+
337+
await new Promise<void>((resolve, reject) => {
338+
child.once("error", reject);
339+
child.stdout?.once("data", () => resolve());
340+
});
341+
342+
await __testing.stopQaGatewayChildProcessTree(child, {
343+
gracefulTimeoutMs: 50,
344+
forceTimeoutMs: 1_000,
345+
});
346+
347+
expect(child.exitCode !== null || child.signalCode !== null).toBe(true);
348+
});
305349
});
306350

307351
describe("resolveQaControlUiRoot", () => {

‎extensions/qa-lab/src/gateway-child.ts‎

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { spawn } from "node:child_process";
1+
import { spawn, type ChildProcess } from "node:child_process";
22
import { randomUUID } from "node:crypto";
33
import { createWriteStream, existsSync } from "node:fs";
44
import fs from "node:fs/promises";
@@ -339,8 +339,57 @@ export const __testing = {
339339
resolveQaBundledPluginsSourceRoot,
340340
resolveQaRuntimeHostVersion,
341341
createQaBundledPluginsDir,
342+
stopQaGatewayChildProcessTree,
342343
};
343344

345+
function hasChildExited(child: ChildProcess) {
346+
return child.exitCode !== null || child.signalCode !== null;
347+
}
348+
349+
function signalQaGatewayChildProcessTree(child: ChildProcess, signal: NodeJS.Signals) {
350+
if (!child.pid) {
351+
return;
352+
}
353+
try {
354+
if (process.platform === "win32") {
355+
child.kill(signal);
356+
return;
357+
}
358+
process.kill(-child.pid, signal);
359+
} catch {
360+
try {
361+
child.kill(signal);
362+
} catch {
363+
// The child already exited.
364+
}
365+
}
366+
}
367+
368+
async function waitForQaGatewayChildExit(child: ChildProcess, timeoutMs: number) {
369+
if (hasChildExited(child)) {
370+
return true;
371+
}
372+
return await Promise.race([
373+
new Promise<boolean>((resolve) => child.once("exit", () => resolve(true))),
374+
sleep(timeoutMs).then(() => false),
375+
]);
376+
}
377+
378+
async function stopQaGatewayChildProcessTree(
379+
child: ChildProcess,
380+
opts?: { gracefulTimeoutMs?: number; forceTimeoutMs?: number },
381+
) {
382+
if (hasChildExited(child)) {
383+
return;
384+
}
385+
signalQaGatewayChildProcessTree(child, "SIGTERM");
386+
if (await waitForQaGatewayChildExit(child, opts?.gracefulTimeoutMs ?? 5_000)) {
387+
return;
388+
}
389+
signalQaGatewayChildProcessTree(child, "SIGKILL");
390+
await waitForQaGatewayChildExit(child, opts?.forceTimeoutMs ?? 2_000);
391+
}
392+
344393
function resolveQaBundledPluginsSourceRoot(repoRoot: string) {
345394
const candidates = [
346395
path.join(repoRoot, "dist", "extensions"),
@@ -811,6 +860,7 @@ export async function startQaGatewayChild(params: {
811860
{
812861
cwd: runtimeCwd,
813862
env,
863+
detached: process.platform !== "win32",
814864
stdio: ["ignore", "pipe", "pipe"],
815865
},
816866
);
@@ -868,7 +918,7 @@ export async function startQaGatewayChild(params: {
868918
} catch (error) {
869919
stdoutLog.end();
870920
stderrLog.end();
871-
child.kill("SIGTERM");
921+
await stopQaGatewayChildProcessTree(child, { gracefulTimeoutMs: 1_000 }).catch(() => {});
872922
if (!keepTemp && stagedBundledPluginsRoot) {
873923
await fs.rm(stagedBundledPluginsRoot, { recursive: true, force: true }).catch(() => {});
874924
}
@@ -925,17 +975,7 @@ export async function startQaGatewayChild(params: {
925975
await rpcClient.stop().catch(() => {});
926976
stdoutLog.end();
927977
stderrLog.end();
928-
if (!child.killed) {
929-
child.kill("SIGTERM");
930-
await Promise.race([
931-
new Promise<void>((resolve) => child.once("exit", () => resolve())),
932-
sleep(5_000).then(() => {
933-
if (!child.killed) {
934-
child.kill("SIGKILL");
935-
}
936-
}),
937-
]);
938-
}
978+
await stopQaGatewayChildProcessTree(child);
939979
if (!(opts?.keepTemp ?? keepTemp)) {
940980
await fs.rm(tempRoot, { recursive: true, force: true });
941981
if (stagedBundledPluginsRoot) {

‎extensions/qa-lab/src/lab-server.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ async function waitForRunnerCatalog(baseUrl: string, timeoutMs = 5_000) {
6464
throw new Error("runner catalog stayed loading");
6565
}
6666

67+
async function waitForFile(filePath: string, timeoutMs = 5_000) {
68+
const startedAt = Date.now();
69+
while (Date.now() - startedAt < timeoutMs) {
70+
try {
71+
return await readFile(filePath, "utf8");
72+
} catch (error) {
73+
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
74+
throw error;
75+
}
76+
await sleep(50);
77+
}
78+
}
79+
throw new Error(`file did not appear: ${filePath}`);
80+
}
81+
6782
describe("qa-lab server", () => {
6883
it("serves bootstrap state and writes a self-check report", async () => {
6984
const tempDir = await mkdtemp(path.join(os.tmpdir(), "qa-lab-test-"));
@@ -405,6 +420,56 @@ describe("qa-lab server", () => {
405420
expect(await readFile(markerPath, "utf8")).toContain("models list --all --json");
406421
});
407422

423+
it("aborts an in-flight runner model catalog when the lab stops", async () => {
424+
const repoRoot = await mkdtemp(path.join(os.tmpdir(), "qa-lab-abort-catalog-"));
425+
cleanups.push(async () => {
426+
await rm(repoRoot, { recursive: true, force: true });
427+
});
428+
const markerPath = path.join(repoRoot, "runner-catalog-started.txt");
429+
const stoppedPath = path.join(repoRoot, "runner-catalog-stopped.txt");
430+
431+
await mkdir(path.join(repoRoot, "dist"), { recursive: true });
432+
await mkdir(path.join(repoRoot, "extensions/qa-lab/web/dist"), { recursive: true });
433+
await writeFile(
434+
path.join(repoRoot, "dist/index.js"),
435+
[
436+
'const fs = require("node:fs");',
437+
`fs.writeFileSync(${JSON.stringify(markerPath)}, process.env.OPENCLAW_CODEX_DISCOVERY_LIVE || "", "utf8");`,
438+
"process.on('SIGTERM', () => {",
439+
` fs.writeFileSync(${JSON.stringify(stoppedPath)}, "terminated", "utf8");`,
440+
" process.exit(0);",
441+
"});",
442+
"setInterval(() => {}, 1000);",
443+
].join("\n"),
444+
"utf8",
445+
);
446+
await writeFile(
447+
path.join(repoRoot, "extensions/qa-lab/web/dist/index.html"),
448+
"<!doctype html><html><body>abort catalog</body></html>",
449+
"utf8",
450+
);
451+
452+
const lab = await startQaLabServer({
453+
host: "127.0.0.1",
454+
port: 0,
455+
repoRoot,
456+
});
457+
let stopped = false;
458+
cleanups.push(async () => {
459+
if (!stopped) {
460+
await lab.stop();
461+
}
462+
});
463+
464+
const bootstrapResponse = await fetchWithRetry(`${lab.baseUrl}/api/bootstrap`);
465+
expect(bootstrapResponse.status).toBe(200);
466+
expect(await waitForFile(markerPath)).toBe("0");
467+
468+
await lab.stop();
469+
stopped = true;
470+
expect(await waitForFile(stoppedPath)).toBe("terminated");
471+
});
472+
408473
it("can disable the embedded echo gateway for real-suite runs", async () => {
409474
const lab = await startQaLabServer({
410475
host: "127.0.0.1",

‎extensions/qa-lab/src/lab-server.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import tls from "node:tls";
1414
import { fileURLToPath } from "node:url";
1515
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
1616
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime";
17-
import { handleQaBusRequest, writeError, writeJson } from "./bus-server.js";
17+
import { closeQaHttpServer, handleQaBusRequest, writeError, writeJson } from "./bus-server.js";
1818
import { createQaBusState, type QaBusState } from "./bus-state.js";
1919
import { createQaRunnerRuntime } from "./harness-runtime.js";
2020
import type {
@@ -465,22 +465,27 @@ export async function startQaLabServer(
465465

466466
let publicBaseUrl = "";
467467
let runnerModelCatalogPromise: Promise<void> | null = null;
468+
let runnerModelCatalogAbort: AbortController | null = null;
468469
const ensureRunnerModelCatalog = () => {
469470
if (runnerModelCatalogPromise) {
470471
return runnerModelCatalogPromise;
471472
}
473+
runnerModelCatalogAbort = new AbortController();
472474
runnerModelCatalogPromise = (async () => {
473475
try {
474476
const { loadQaRunnerModelOptions } = await import("./model-catalog.runtime.js");
475477
runnerModelOptions = await loadQaRunnerModelOptions({
476478
repoRoot,
479+
signal: runnerModelCatalogAbort?.signal,
477480
});
478481
runnerModelCatalogStatus = "ready";
479482
} catch {
480483
runnerModelOptions = [];
481484
runnerModelCatalogStatus = "failed";
482485
}
483-
})();
486+
})().finally(() => {
487+
runnerModelCatalogAbort = null;
488+
});
484489
return runnerModelCatalogPromise;
485490
};
486491

@@ -802,10 +807,10 @@ export async function startQaLabServer(
802807
},
803808
runSelfCheck,
804809
async stop() {
810+
runnerModelCatalogAbort?.abort();
811+
await runnerModelCatalogPromise?.catch(() => undefined);
805812
await gateway?.stop();
806-
await new Promise<void>((resolve, reject) =>
807-
server.close((error) => (error ? reject(error) : resolve())),
808-
);
813+
await closeQaHttpServer(server);
809814
},
810815
};
811816
labHandle = lab;

‎extensions/qa-lab/src/mock-openai-server.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createServer, type IncomingMessage, type ServerResponse } from "node:http";
22
import { setTimeout as sleep } from "node:timers/promises";
3+
import { closeQaHttpServer } from "./bus-server.js";
34

45
type ResponsesInputItem = Record<string, unknown>;
56

@@ -805,9 +806,7 @@ export async function startQaMockOpenAiServer(params?: { host?: string; port?: n
805806
return {
806807
baseUrl: `http://${host}:${address.port}`,
807808
async stop() {
808-
await new Promise<void>((resolve, reject) =>
809-
server.close((error) => (error ? reject(error) : resolve())),
810-
);
809+
await closeQaHttpServer(server);
811810
},
812811
};
813812
}

0 commit comments

Comments
 (0)