Merge pull request #1361 from Runfusion/gsxdsm/vitest-killguard
fix(tui): stop vitest auto-kill firing on garbage memory metric and killing non-vitest processes
This commit is contained in:
5
.changeset/vitest-autokill-guard.md
Normal file
5
.changeset/vitest-autokill-guard.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix the vitest memory-pressure auto-kill firing on a garbage metric and killing innocent processes. The guard probed `os.availableMemory` (which does not exist) and silently fell back to `os.freemem()`, which on macOS reads ~99% used on an idle machine — so with the toggle on, every vitest process was SIGKILLed every 30 seconds regardless of real memory pressure. It now reads `process.availableMemory()` (Node 22+) and refuses to auto-kill when only the unreliable freemem fallback is available. Kill targeting is also fixed: `pgrep -f vitest` matches full command lines (wrapper shells, monitors, editors that merely mention vitest); the TUI auto-kill/manual kill and the dashboard `POST /api/kill-vitest` + system-stats count now filter matches to actual node processes via a shared `findVitestProcessIds` helper.
|
||||
@@ -0,0 +1,54 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import os from "node:os";
|
||||
import { getAvailableMemoryInfo } from "../controller.js";
|
||||
|
||||
type ProcessWithAvailableMemory = NodeJS.Process & { availableMemory?: () => number };
|
||||
|
||||
describe("getAvailableMemoryInfo", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("reports a reliable reading from process.availableMemory when present", () => {
|
||||
const proc = process as ProcessWithAvailableMemory;
|
||||
if (typeof proc.availableMemory !== "function") {
|
||||
// Older runtime without the API — covered by the fallback test below.
|
||||
return;
|
||||
}
|
||||
const spy = vi.spyOn(proc, "availableMemory").mockReturnValue(123_456_789);
|
||||
|
||||
expect(getAvailableMemoryInfo()).toEqual({ bytes: 123_456_789, reliable: true });
|
||||
expect(spy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to os.freemem and flags the reading unreliable when the API is missing", () => {
|
||||
const proc = process as ProcessWithAvailableMemory;
|
||||
const original = proc.availableMemory;
|
||||
// Simulate a runtime without process.availableMemory (Node < 22). The
|
||||
// freemem fallback must be flagged unreliable: on macOS freemem reads
|
||||
// ~99% used on an idle machine, and treating it as a pressure signal made
|
||||
// the vitest auto-kill fire every 30s (2026-06-03 incident).
|
||||
Reflect.deleteProperty(proc, "availableMemory");
|
||||
const freememSpy = vi.spyOn(os, "freemem").mockReturnValue(42);
|
||||
try {
|
||||
expect(getAvailableMemoryInfo()).toEqual({ bytes: 42, reliable: false });
|
||||
} finally {
|
||||
if (original) proc.availableMemory = original;
|
||||
freememSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back unreliable when process.availableMemory throws", () => {
|
||||
const proc = process as ProcessWithAvailableMemory;
|
||||
if (typeof proc.availableMemory !== "function") return;
|
||||
vi.spyOn(proc, "availableMemory").mockImplementation(() => {
|
||||
throw new Error("not supported");
|
||||
});
|
||||
const freememSpy = vi.spyOn(os, "freemem").mockReturnValue(7);
|
||||
try {
|
||||
expect(getAvailableMemoryInfo()).toEqual({ bytes: 7, reliable: false });
|
||||
} finally {
|
||||
freememSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,24 +1,39 @@
|
||||
import os from "node:os";
|
||||
import v8 from "node:v8";
|
||||
import { execFile } from "node:child_process";
|
||||
import { appendFileSync } from "node:fs";
|
||||
import { findVitestProcessIds } from "@fusion/core";
|
||||
|
||||
// `os.freemem()` on macOS only counts truly-free pages and excludes the large
|
||||
// "inactive"/cached pool that the OS will reclaim on demand — so total-free
|
||||
// reads ~95%+ used on an otherwise-idle machine. `os.availableMemory()` (Node
|
||||
// 22+) reports memory the OS considers available, matching Activity Monitor's
|
||||
// notion of "used". Fall back to freemem on older runtimes.
|
||||
function getAvailableMemory(): number {
|
||||
const fn = (os as unknown as { availableMemory?: () => number }).availableMemory;
|
||||
if (typeof fn === "function") {
|
||||
// reads ~95%+ used on an otherwise-idle machine. `process.availableMemory()`
|
||||
// (Node 22+ — NOT `os.availableMemory`, which does not exist and silently
|
||||
// fell through to the freemem trap this function was written to avoid)
|
||||
// reports memory the OS considers available, matching Activity Monitor's
|
||||
// notion of "used". The freemem fallback is flagged unreliable so pressure-
|
||||
// triggered actions can refuse to fire on a garbage ratio: with freemem, an
|
||||
// idle 256GB Mac reads ~99% used and the vitest auto-kill fired every 30s
|
||||
// regardless of real pressure (2026-06-03 incident).
|
||||
interface AvailableMemoryReading {
|
||||
bytes: number;
|
||||
/** False when only `os.freemem()` was available — unusable as a pressure signal. */
|
||||
reliable: boolean;
|
||||
}
|
||||
|
||||
export function getAvailableMemoryInfo(): AvailableMemoryReading {
|
||||
const processFn = (process as unknown as { availableMemory?: () => number }).availableMemory;
|
||||
if (typeof processFn === "function") {
|
||||
try {
|
||||
const v = fn.call(os);
|
||||
if (Number.isFinite(v) && v >= 0) return v;
|
||||
const v = processFn.call(process);
|
||||
if (Number.isFinite(v) && v >= 0) return { bytes: v, reliable: true };
|
||||
} catch {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
return os.freemem();
|
||||
return { bytes: os.freemem(), reliable: false };
|
||||
}
|
||||
|
||||
function getAvailableMemory(): number {
|
||||
return getAvailableMemoryInfo().bytes;
|
||||
}
|
||||
|
||||
const TUI_DEBUG_LOG = process.env.FUSION_TUI_DEBUG_LOG;
|
||||
@@ -299,8 +314,10 @@ export class DashboardTUI {
|
||||
|
||||
if (this.autoKillVitestOnPressure) {
|
||||
const total = os.totalmem();
|
||||
const free = getAvailableMemory();
|
||||
if (total > 0) {
|
||||
const { bytes: free, reliable } = getAvailableMemoryInfo();
|
||||
// Without a reliable availability reading the ratio is garbage (freemem
|
||||
// on macOS ≈ always >90% used) — never SIGKILL on a garbage signal.
|
||||
if (total > 0 && reliable) {
|
||||
const usedRatio = (total - free) / total;
|
||||
// 30s minimum gap between auto-kills — vitest restart and OS reclaim
|
||||
// both take a few seconds; firing every 2s would flap.
|
||||
@@ -325,24 +342,13 @@ export class DashboardTUI {
|
||||
* gone by the time we send the signal).
|
||||
*/
|
||||
async killVitestProcesses(): Promise<{ killed: number; pids: number[] }> {
|
||||
// pgrep is POSIX-only; Windows path is a no-op above.
|
||||
if (process.platform === "win32") {
|
||||
return { killed: 0, pids: [] };
|
||||
}
|
||||
const selfPid = process.pid;
|
||||
// execFile (not execSync) so the TUI render loop stays responsive while
|
||||
// pgrep walks the process table — that walk can take 100ms+ on a busy
|
||||
// machine and previously froze the UI on every memory-pressure check.
|
||||
const stdout: string = await new Promise((resolve) => {
|
||||
execFile("pgrep", ["-f", "vitest"], { encoding: "utf8" }, (err, out) => {
|
||||
// pgrep exits non-zero when no matches — treat as empty result.
|
||||
resolve(err ? "" : (typeof out === "string" ? out : ""));
|
||||
});
|
||||
});
|
||||
const pids = stdout
|
||||
.split("\n")
|
||||
.map((s) => Number.parseInt(s.trim(), 10))
|
||||
.filter((n) => Number.isFinite(n) && n > 0 && n !== selfPid);
|
||||
// findVitestProcessIds is pgrep-based (POSIX-only; no-op on Windows) and
|
||||
// uses async execFile so the TUI render loop stays responsive while the
|
||||
// process table is walked. Crucially it filters matches to actual node
|
||||
// processes: a bare `pgrep -f vitest` also matches wrapper shells whose
|
||||
// command line mentions vitest, monitors, and editors — SIGKILLing those
|
||||
// took out unrelated process trees (2026-06-03 incident).
|
||||
const pids = await findVitestProcessIds();
|
||||
|
||||
let killed = 0;
|
||||
for (const pid of pids) {
|
||||
|
||||
86
packages/core/src/__tests__/vitest-processes.test.ts
Normal file
86
packages/core/src/__tests__/vitest-processes.test.ts
Normal file
@@ -0,0 +1,86 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { execFile as nodeExecFile } from "node:child_process";
|
||||
import { findVitestProcessIds } from "../vitest-processes.js";
|
||||
|
||||
type ExecFileCallback = (err: Error | null, stdout: string, stderr: string) => void;
|
||||
|
||||
function makeExecFileMock(responses: { pgrep?: string; ps?: string; pgrepError?: boolean }) {
|
||||
const calls: Array<{ cmd: string; args: string[] }> = [];
|
||||
const impl = ((cmd: string, args: string[], _opts: unknown, cb: ExecFileCallback) => {
|
||||
calls.push({ cmd, args });
|
||||
if (cmd === "pgrep") {
|
||||
if (responses.pgrepError) {
|
||||
cb(new Error("pgrep: no matches"), "", "");
|
||||
} else {
|
||||
cb(null, responses.pgrep ?? "", "");
|
||||
}
|
||||
return {} as never;
|
||||
}
|
||||
if (cmd === "ps") {
|
||||
cb(null, responses.ps ?? "", "");
|
||||
return {} as never;
|
||||
}
|
||||
cb(new Error(`unexpected command ${cmd}`), "", "");
|
||||
return {} as never;
|
||||
}) as unknown as typeof nodeExecFile;
|
||||
return { impl, calls };
|
||||
}
|
||||
|
||||
describe("findVitestProcessIds", () => {
|
||||
it("returns only pids whose executable is node — wrapper shells and monitors are spared", async () => {
|
||||
const { impl, calls } = makeExecFileMock({
|
||||
// pgrep -f vitest matches the runner, two workers, a zsh wrapper whose
|
||||
// command line contains "npx vitest run", and a watch loop grepping for
|
||||
// "node (vitest".
|
||||
pgrep: "101\n102\n103\n104\n105\n",
|
||||
ps: [
|
||||
" 101 /opt/homebrew/bin/node",
|
||||
" 102 node",
|
||||
" 103 /usr/local/bin/node",
|
||||
" 104 zsh",
|
||||
" 105 /bin/zsh",
|
||||
].join("\n"),
|
||||
});
|
||||
|
||||
const pids = await findVitestProcessIds({ execFileImpl: impl });
|
||||
|
||||
expect(pids).toEqual([101, 102, 103]);
|
||||
expect(calls[0]).toEqual({ cmd: "pgrep", args: ["-f", "vitest"] });
|
||||
expect(calls[1]?.cmd).toBe("ps");
|
||||
expect(calls[1]?.args).toEqual(["-o", "pid=,comm=", "-p", "101,102,103,104,105"]);
|
||||
});
|
||||
|
||||
it("always excludes the calling process and any caller-supplied pids", async () => {
|
||||
const self = process.pid;
|
||||
const { impl } = makeExecFileMock({
|
||||
pgrep: `${self}\n201\n202\n`,
|
||||
ps: [` ${self} node`, " 201 node", " 202 node"].join("\n"),
|
||||
});
|
||||
|
||||
const pids = await findVitestProcessIds({ execFileImpl: impl, excludePids: [202] });
|
||||
|
||||
expect(pids).toEqual([201]);
|
||||
});
|
||||
|
||||
it("returns empty when pgrep finds nothing (non-zero exit)", async () => {
|
||||
const { impl, calls } = makeExecFileMock({ pgrepError: true });
|
||||
|
||||
const pids = await findVitestProcessIds({ execFileImpl: impl });
|
||||
|
||||
expect(pids).toEqual([]);
|
||||
// ps must not run with an empty pid list.
|
||||
expect(calls.map((c) => c.cmd)).toEqual(["pgrep"]);
|
||||
});
|
||||
|
||||
it("returns empty on win32 without spawning anything", async () => {
|
||||
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
try {
|
||||
const { impl, calls } = makeExecFileMock({ pgrep: "999\n", ps: " 999 node" });
|
||||
const pids = await findVitestProcessIds({ execFileImpl: impl });
|
||||
expect(pids).toEqual([]);
|
||||
expect(calls).toEqual([]);
|
||||
} finally {
|
||||
platformSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -326,6 +326,10 @@ export {
|
||||
type MergeTargetResolution,
|
||||
type MergeTargetResolverOptions,
|
||||
} from "./task-merge.js";
|
||||
export {
|
||||
findVitestProcessIds,
|
||||
type FindVitestProcessIdsOptions,
|
||||
} from "./vitest-processes.js";
|
||||
export {
|
||||
countRecentIdenticalStallEntries,
|
||||
getInReviewStallReason,
|
||||
|
||||
87
packages/core/src/vitest-processes.ts
Normal file
87
packages/core/src/vitest-processes.ts
Normal file
@@ -0,0 +1,87 @@
|
||||
import { execFile as nodeExecFile } from "node:child_process";
|
||||
|
||||
/**
|
||||
* Locate running vitest processes safely.
|
||||
*
|
||||
* `pgrep -f vitest` matches FULL command lines, so a bare pattern also matches
|
||||
* innocent bystanders whose argv merely mentions vitest:
|
||||
* - wrapper shells (`zsh -c '... npx vitest run ...'`) — killing these
|
||||
* strands the `$?` handler so failures look like silent truncation,
|
||||
* - monitoring/grep one-liners that mention vitest,
|
||||
* - editors or tools opened on `vitest.config.ts`.
|
||||
* Root cause of the 2026-06-03 incident where the memory-pressure auto-kill
|
||||
* SIGKILLed unrelated process trees every 30s.
|
||||
*
|
||||
* This helper filters pgrep candidates to processes whose executable (`comm`)
|
||||
* is actually node, so only the vitest runner and its workers are reported.
|
||||
*/
|
||||
|
||||
export interface FindVitestProcessIdsOptions {
|
||||
/** PIDs to exclude in addition to the calling process. */
|
||||
excludePids?: number[];
|
||||
/** Test seam — injected execFile. */
|
||||
execFileImpl?: typeof nodeExecFile;
|
||||
}
|
||||
|
||||
function execToStdout(
|
||||
execFileImpl: typeof nodeExecFile,
|
||||
cmd: string,
|
||||
args: string[],
|
||||
): Promise<string> {
|
||||
return new Promise((resolve) => {
|
||||
execFileImpl(cmd, args, { encoding: "utf8" }, (err, out) => {
|
||||
// pgrep/ps exit non-zero when nothing matches — treat as empty result.
|
||||
resolve(err ? "" : (typeof out === "string" ? out : ""));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function parsePids(stdout: string): number[] {
|
||||
return stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => Number.parseInt(line.trim(), 10))
|
||||
.filter((pid) => Number.isFinite(pid) && pid > 0);
|
||||
}
|
||||
|
||||
/** Keep only pids whose executable is node (vitest runner + pool workers). */
|
||||
async function filterToNodeProcesses(
|
||||
execFileImpl: typeof nodeExecFile,
|
||||
pids: number[],
|
||||
): Promise<number[]> {
|
||||
if (pids.length === 0) return [];
|
||||
const stdout = await execToStdout(execFileImpl, "ps", [
|
||||
"-o",
|
||||
"pid=,comm=",
|
||||
"-p",
|
||||
pids.join(","),
|
||||
]);
|
||||
const nodePids: number[] = [];
|
||||
for (const line of stdout.split(/\r?\n/)) {
|
||||
const trimmed = line.trim();
|
||||
if (!trimmed) continue;
|
||||
const spaceIdx = trimmed.indexOf(" ");
|
||||
if (spaceIdx <= 0) continue;
|
||||
const pid = Number.parseInt(trimmed.slice(0, spaceIdx), 10);
|
||||
if (!Number.isFinite(pid) || pid <= 0) continue;
|
||||
const comm = trimmed.slice(spaceIdx + 1).trim();
|
||||
const executable = comm.split("/").pop() ?? comm;
|
||||
if (executable === "node" || executable === "node.exe" || executable === "nodejs") {
|
||||
nodePids.push(pid);
|
||||
}
|
||||
}
|
||||
return nodePids;
|
||||
}
|
||||
|
||||
export async function findVitestProcessIds(
|
||||
options: FindVitestProcessIdsOptions = {},
|
||||
): Promise<number[]> {
|
||||
// pgrep/ps are POSIX-only; Windows callers treat this as a no-op.
|
||||
if (process.platform === "win32") return [];
|
||||
|
||||
const execFileImpl = options.execFileImpl ?? nodeExecFile;
|
||||
const excluded = new Set<number>([process.pid, ...(options.excludePids ?? [])]);
|
||||
|
||||
const candidates = parsePids(await execToStdout(execFileImpl, "pgrep", ["-f", "vitest"]));
|
||||
const nodePids = await filterToNodeProcesses(execFileImpl, candidates);
|
||||
return nodePids.filter((pid) => !excluded.has(pid));
|
||||
}
|
||||
@@ -406,7 +406,13 @@ describe("GET /api/system-stats", () => {
|
||||
});
|
||||
|
||||
mockExecFile.mockImplementation((...callArgs: unknown[]) => {
|
||||
const [file] = callArgs as [string];
|
||||
const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void;
|
||||
if (file === "ps") {
|
||||
// comm filter pass: all candidates are real node processes.
|
||||
cb(null, ` ${process.pid} node\n 111 /opt/homebrew/bin/node\n 222 node\n`, "");
|
||||
return;
|
||||
}
|
||||
cb(null, `${process.pid}\n111\n222\n`, "");
|
||||
});
|
||||
|
||||
@@ -584,10 +590,18 @@ describe("POST /api/kill-vitest", () => {
|
||||
|
||||
it("kills all matched vitest pids except the current dashboard process", async () => {
|
||||
const store = createMockStore();
|
||||
mockExecFile.mockImplementationOnce((...callArgs: unknown[]) => {
|
||||
const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void;
|
||||
cb(null, `${process.pid}\n1001\n1002\nnot-a-pid\n`, "");
|
||||
});
|
||||
mockExecFile
|
||||
.mockImplementationOnce((...callArgs: unknown[]) => {
|
||||
// pgrep -f vitest: matches the dashboard itself, two node processes,
|
||||
// a wrapper shell whose command line mentions vitest, and garbage.
|
||||
const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void;
|
||||
cb(null, `${process.pid}\n1001\n1002\n1003\nnot-a-pid\n`, "");
|
||||
})
|
||||
.mockImplementationOnce((...callArgs: unknown[]) => {
|
||||
// ps comm filter: 1003 is a zsh wrapper and must be spared.
|
||||
const cb = callArgs[callArgs.length - 1] as (err: unknown, stdout?: string, stderr?: string) => void;
|
||||
cb(null, ` ${process.pid} node\n 1001 /opt/homebrew/bin/node\n 1002 node\n 1003 zsh\n`, "");
|
||||
});
|
||||
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true);
|
||||
|
||||
const res = await REQUEST(buildApp(store), "POST", "/api/kill-vitest");
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
MemoryBackendError,
|
||||
RoutineStore,
|
||||
discoverPiExtensions,
|
||||
findVitestProcessIds,
|
||||
getFusionAgentDir,
|
||||
getLegacyPiAgentDir,
|
||||
isWebhookTrigger,
|
||||
@@ -1496,22 +1497,13 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
};
|
||||
|
||||
const getVitestProcessIds = async (): Promise<number[]> => {
|
||||
// execFile (not execSync) so the dashboard's event loop stays responsive
|
||||
// while pgrep walks the process table — that walk can take 100ms+ on a
|
||||
// busy machine and previously froze every concurrent request.
|
||||
const { execFile } = await import("node:child_process");
|
||||
|
||||
const stdout: string = await new Promise((resolve) => {
|
||||
execFile("pgrep", ["-f", "vitest"], { encoding: "utf8" }, (err, out) => {
|
||||
// pgrep exits non-zero when no matches — treat as empty result.
|
||||
resolve(err ? "" : (typeof out === "string" ? out : ""));
|
||||
});
|
||||
});
|
||||
|
||||
return stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => Number.parseInt(line.trim(), 10))
|
||||
.filter((pid) => Number.isInteger(pid) && pid > 0 && pid !== process.pid);
|
||||
// Async pgrep/ps via findVitestProcessIds so the dashboard's event loop
|
||||
// stays responsive while the process table is walked. The helper filters
|
||||
// matches to actual node processes — a bare `pgrep -f vitest` also matches
|
||||
// wrapper shells, monitors, and editors whose command line merely mentions
|
||||
// vitest, and SIGKILLing those took out unrelated process trees
|
||||
// (2026-06-03 incident).
|
||||
return findVitestProcessIds();
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user