diff --git a/.changeset/vitest-autokill-guard.md b/.changeset/vitest-autokill-guard.md new file mode 100644 index 0000000000..841ae063cd --- /dev/null +++ b/.changeset/vitest-autokill-guard.md @@ -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. diff --git a/packages/cli/src/commands/dashboard-tui/__tests__/available-memory.test.ts b/packages/cli/src/commands/dashboard-tui/__tests__/available-memory.test.ts new file mode 100644 index 0000000000..1dd934f926 --- /dev/null +++ b/packages/cli/src/commands/dashboard-tui/__tests__/available-memory.test.ts @@ -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(); + } + }); +}); diff --git a/packages/cli/src/commands/dashboard-tui/controller.ts b/packages/cli/src/commands/dashboard-tui/controller.ts index acd812f4a6..f4862db202 100644 --- a/packages/cli/src/commands/dashboard-tui/controller.ts +++ b/packages/cli/src/commands/dashboard-tui/controller.ts @@ -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) { diff --git a/packages/core/src/__tests__/vitest-processes.test.ts b/packages/core/src/__tests__/vitest-processes.test.ts new file mode 100644 index 0000000000..f3e92f59ca --- /dev/null +++ b/packages/core/src/__tests__/vitest-processes.test.ts @@ -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(); + } + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a362011a06..3c708e318e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -326,6 +326,10 @@ export { type MergeTargetResolution, type MergeTargetResolverOptions, } from "./task-merge.js"; +export { + findVitestProcessIds, + type FindVitestProcessIdsOptions, +} from "./vitest-processes.js"; export { countRecentIdenticalStallEntries, getInReviewStallReason, diff --git a/packages/core/src/vitest-processes.ts b/packages/core/src/vitest-processes.ts new file mode 100644 index 0000000000..47e998b672 --- /dev/null +++ b/packages/core/src/vitest-processes.ts @@ -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 { + 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 { + 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") { + nodePids.push(pid); + } + } + return nodePids; +} + +export async function findVitestProcessIds( + options: FindVitestProcessIdsOptions = {}, +): Promise { + // 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([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)); +} diff --git a/packages/dashboard/src/__tests__/routes-system.test.ts b/packages/dashboard/src/__tests__/routes-system.test.ts index 7babdd990b..512181ad76 100644 --- a/packages/dashboard/src/__tests__/routes-system.test.ts +++ b/packages/dashboard/src/__tests__/routes-system.test.ts @@ -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"); diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts index e1bcde6aaa..8289fb18db 100644 --- a/packages/dashboard/src/routes.ts +++ b/packages/dashboard/src/routes.ts @@ -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 => { - // 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(); }; /**