From 5a15ea1b5c6ad0568e0eb80644fdb00a851e1c46 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 17 Jun 2026 20:07:24 +0200 Subject: [PATCH] fix(scripts): wait for deadcode scan process groups --- scripts/check-deadcode-unused-files.mjs | 44 ++++++++- .../check-deadcode-unused-files.test.ts | 97 ++++++++++++++++++- 2 files changed, 138 insertions(+), 3 deletions(-) diff --git a/scripts/check-deadcode-unused-files.mjs b/scripts/check-deadcode-unused-files.mjs index 969b4f6709d..6dad061c481 100644 --- a/scripts/check-deadcode-unused-files.mjs +++ b/scripts/check-deadcode-unused-files.mjs @@ -17,6 +17,8 @@ export const KNIP_TIMEOUT_MS = 10 * 60 * 1000; * Grace period before force-killing a timed-out knip child process. */ export const KNIP_KILL_GRACE_MS = 5_000; +const KNIP_PROCESS_TREE_EXIT_POLL_MS = 25; +const KNIP_POST_FORCE_KILL_WAIT_MS = 1_000; /** * Heartbeat interval used while knip runs without output. */ @@ -154,6 +156,34 @@ function signalProcessTree(child, signal) { } } +function processTreeAlive(child) { + if (!child.pid) { + return false; + } + if (process.platform === "win32") { + return child.exitCode === null && child.signalCode === null; + } + try { + process.kill(-child.pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} + +async function waitForProcessTreeExit(child, timeoutMs) { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (!processTreeAlive(child)) { + return true; + } + await new Promise((resolvePoll) => { + setTimeout(resolvePoll, KNIP_PROCESS_TREE_EXIT_POLL_MS); + }); + } + return !processTreeAlive(child); +} + /** * Runs knip and returns parsed unused-file results. */ @@ -230,6 +260,16 @@ export async function runKnipUnusedFiles(params = {}) { output: output.join(""), }); }; + const finishAfterProcessTreeCleanup = async (result) => { + if (processTreeAlive(child)) { + await waitForProcessTreeExit(child, killGraceMs); + } + if (processTreeAlive(child)) { + signalProcessTree(child, "SIGKILL"); + await waitForProcessTreeExit(child, KNIP_POST_FORCE_KILL_WAIT_MS); + } + finish(result); + }; const appendOutput = (chunk) => { if (settled) { @@ -283,7 +323,7 @@ export async function runKnipUnusedFiles(params = {}) { exitSignal = exitSignal ?? signal; const elapsedSeconds = Math.round((Date.now() - startedAt) / 1000); if (timedOut) { - finish({ + void finishAfterProcessTreeCleanup({ errorCode: "ETIMEDOUT", errorMessage: `Knip unused-file scan timed out after ${elapsedSeconds}s`, signal: exitSignal, @@ -292,7 +332,7 @@ export async function runKnipUnusedFiles(params = {}) { return; } if (bufferExceeded) { - finish({ + void finishAfterProcessTreeCleanup({ errorCode: "ENOBUFS", errorMessage: `Knip unused-file scan exceeded ${maxBufferBytes} output bytes`, signal: exitSignal, diff --git a/test/scripts/check-deadcode-unused-files.test.ts b/test/scripts/check-deadcode-unused-files.test.ts index 8c22658e2f6..0c03fc4074a 100644 --- a/test/scripts/check-deadcode-unused-files.test.ts +++ b/test/scripts/check-deadcode-unused-files.test.ts @@ -1,6 +1,7 @@ // Check Deadcode Unused Files tests cover check deadcode unused files script behavior. +import { spawn } from "node:child_process"; import { EventEmitter } from "node:events"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; @@ -27,6 +28,43 @@ function finishFakeProcess( child.emit("close", status, signal); } +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function sleep(ms: number): Promise { + await new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +async function waitForFile(filePath: string, timeoutMs: number): Promise { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (existsSync(filePath)) { + return; + } + await sleep(25); + } + throw new Error(`timeout waiting for ${filePath}`); +} + +async function waitForDead(pid: number, timeoutMs: number): Promise { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (!isProcessAlive(pid)) { + return; + } + await sleep(25); + } + throw new Error(`process still alive: ${pid}`); +} + describe("check-deadcode-unused-files", () => { it("parses the compact Knip unused-file section", () => { expect( @@ -243,6 +281,9 @@ src/a.ts: src/a.ts const kills: Array = []; process.kill = ((pid: number, signal?: NodeJS.Signals | number) => { if (Math.abs(pid) === child.pid) { + if (signal === 0) { + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } kills.push(signal); finishFakeProcess(child, null, (signal as NodeJS.Signals | undefined) ?? "SIGTERM"); return true; @@ -274,6 +315,57 @@ src/a.ts: src/a.ts } }); + it.skipIf(process.platform === "win32")( + "waits for timed-out Knip process groups after the wrapper exits", + async () => { + const root = mkdtempSync(path.join(os.tmpdir(), "openclaw-knip-timeout-")); + const childPidPath = path.join(root, "child.pid"); + let childPid = 0; + + try { + const childScript = [ + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join(""); + const parentScript = [ + "const { spawn } = require('node:child_process');", + "const fs = require('node:fs');", + `const child = spawn(process.execPath, ['-e', ${JSON.stringify(childScript)}], { stdio: 'ignore' });`, + "fs.writeFileSync(process.env.OPENCLAW_TEST_CHILD_PID, String(child.pid));", + "process.on('SIGTERM', () => process.exit(0));", + "setInterval(() => {}, 1000);", + ].join(""); + + const resultPromise = runKnipUnusedFiles({ + env: { ...process.env, OPENCLAW_TEST_CHILD_PID: childPidPath }, + killGraceMs: 50, + spawnCommand(_command: string, _args: string[], options: unknown) { + return spawn(process.execPath, ["-e", parentScript], { + ...(options as Parameters[2]), + env: { ...process.env, OPENCLAW_TEST_CHILD_PID: childPidPath }, + }); + }, + timeoutMs: 100, + writeStatus: () => {}, + }); + + await waitForFile(childPidPath, 2_000); + childPid = Number.parseInt(readFileSync(childPidPath, "utf8"), 10); + expect(isProcessAlive(childPid)).toBe(true); + + await expect(resultPromise).resolves.toMatchObject({ + errorCode: "ETIMEDOUT", + }); + await waitForDead(childPid, 2_000); + } finally { + if (childPid && isProcessAlive(childPid)) { + process.kill(childPid, "SIGKILL"); + } + rmSync(root, { recursive: true, force: true }); + } + }, + ); + it("keeps output delivered after process exit but before stdio close", async () => { const child = new FakeKnipProcess(); const resultPromise = runKnipUnusedFiles({ @@ -300,6 +392,9 @@ src/a.ts: src/a.ts const originalKill = process.kill.bind(process); process.kill = ((pid: number, signal?: NodeJS.Signals | number) => { if (Math.abs(pid) === child.pid) { + if (signal === 0) { + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } finishFakeProcess(child, null, (signal as NodeJS.Signals | undefined) ?? "SIGTERM"); return true; }