mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-05 17:30:50 +00:00
* fix(cli): keep a live preview's ownership record and stop past a bad one A missed liveness probe is not proof the preview is gone — a server blocked on a Puppeteer capture answers nothing for a second or two — but any miss retired the session record, and the record carries the only PID-reuse guard `--stop` has. Reproduced by SIGSTOPping a managed preview and running `--status`: the record was deleted and never came back, leaving every later stop to fall through to an unauthenticated port scan with no ownership proof at all. Only a wrapper process that is provably gone now retires a record. That record gains a process-birth token so a recycled PID reads as a different process, and it is written through a temp file and renamed — every reader deletes it when it fails to parse, so a torn read would otherwise destroy a live server's proof of ownership. Two failure-propagation bugs in the stop path: `--kill-all` collected the first unprovable record's exception and abandoned every server after it, so they were left running AND unreported; and a replacement refused to launch when the server it was replacing had already exited on its own, which is the goal state rather than a failure. `--list` now shows managed sessions ahead of whatever else answers the scan. * fix(cli): keep a record whose identity lookup gave no answer, not a different one Review blocker. The keep-alive path this PR adds could still retire a LIVE record — through a different door than the one it closed. `processIdentity` catches every failure into `null`, and on two of three platforms that failure is a subprocess timeout on a live process: the win32 `Win32_Process` CIM query and the POSIX `ps -o lstart=` both run on a 2 s budget, under exactly the load that made the HTTP probe miss in the first place. A `null` compared unequal to the saved token, so the record was deleted and `wrapperIdentity` — the only PID-reuse guard `--stop` has — was gone for good. Only Linux, reading /proc directly, was reliable. No answer is now distinguished from a different answer: the PID is checked with `kill(pid, 0)` first, which asks the kernel without signalling and treats EPERM as alive. A PID nothing can signal is gone and retires the record with no subprocess at all; a signalable PID whose token cannot be read keeps it. Only a token that comes back and differs retires it. That ordering also answers the `--list` note: the identity subprocess no longer runs for the stale records that made it slow, so the N x 2 s worst case is gone along with the timeouts that fed the bug. Verified by mutation: restoring the old "no answer means gone" behaviour reds the new case. Also clean up the temp file when a rename fails, rather than orphaning it in the session directory. * test(cli): assert only what the birth-token lookup actually guarantees `captures a stable birth token for the current process` made two assertions that a lookup allowed to fail cannot support. `processIdentity` returns null whenever the lookup cannot be completed — not only when the process is absent — and on Windows and macOS it shells out to PowerShell or `ps` on a 2 s budget that a cold CI runner routinely outruns. Both failed on windows-latest, in sequence: first `.toMatch()` received null, and once that was guarded, `expect(second).toBe(first)` compared a null from the cold first spawn against a token from the warm second one. Two lookups can disagree for exactly one reason — one of them failed — so stability is only assertable across two successful ones. The token itself cannot change between calls; it is a birth timestamp and the process did not restart. `processIdentity(-1)` stays unconditional: the guard rejects it before any subprocess runs. The strict shape assertion moves to a Linux-only case, where /proc is read directly with no subprocess and null is genuinely not allowed — keeping the guarantee on the one platform that can honour it rather than dropping it everywhere. Callers already depend on this contract: `wrapperProcessIsAlive` treats null as "no answer" rather than "gone" precisely because it is reachable.
128 lines
4.8 KiB
TypeScript
128 lines
4.8 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
|
import { spawn } from "node:child_process";
|
|
import {
|
|
isProcessDescendant,
|
|
killProcessTree,
|
|
killOrphanedProcesses,
|
|
processIdentity,
|
|
windowsProcessTreeKillArgs,
|
|
} from "./orphanCleanup.js";
|
|
|
|
const IS_UNIX = process.platform !== "win32";
|
|
|
|
describe("Windows process-tree cleanup", () => {
|
|
it("uses taskkill recursively and forcefully for the owned PID", () => {
|
|
expect(windowsProcessTreeKillArgs(4321)).toEqual(["/PID", "4321", "/T", "/F"]);
|
|
});
|
|
});
|
|
|
|
describe("process-tree ownership", () => {
|
|
it("captures a stable birth token for the current process", () => {
|
|
// `processIdentity` is documented to return null when the lookup cannot be
|
|
// completed, not only when the process is absent — and on Windows and macOS
|
|
// it shells out to PowerShell / `ps` on a 2 s budget, which a cold CI runner
|
|
// routinely outruns. Asserting an unconditional token therefore tested the
|
|
// runner's spawn latency rather than the function: it failed on
|
|
// windows-latest with `.toMatch()` receiving null.
|
|
//
|
|
// What is actually promised: a well-formed token OR null, the same answer
|
|
// twice in a row, and null for a pid that cannot exist. Callers are built
|
|
// on exactly that contract — `wrapperProcessIsAlive` treats null as "no
|
|
// answer" rather than "gone" precisely because it is reachable here.
|
|
const first = processIdentity(process.pid);
|
|
const second = processIdentity(process.pid);
|
|
|
|
// Stability is only assertable across two SUCCESSFUL lookups. Two calls can
|
|
// disagree here for one reason — one of them failed — and that is exactly
|
|
// what happens on a cold Windows runner: the first PowerShell spawn outruns
|
|
// the 2 s budget and returns null, the second is warm and returns a token.
|
|
// The token itself cannot change between them; it is a birth timestamp and
|
|
// the process did not restart.
|
|
if (first !== null && second !== null) {
|
|
expect(first).toMatch(/^(?:linux|posix|windows):/);
|
|
expect(second).toBe(first);
|
|
}
|
|
|
|
// This one holds everywhere: the guard rejects it before any subprocess.
|
|
expect(processIdentity(-1)).toBeNull();
|
|
});
|
|
|
|
it("reads a well-formed token where the lookup cannot fail", () => {
|
|
// Linux reads /proc directly with no subprocess, so there the token is not
|
|
// allowed to be null — this keeps the strict assertion on the one platform
|
|
// that can honour it, rather than dropping it everywhere.
|
|
if (process.platform !== "linux") return;
|
|
expect(processIdentity(process.pid)).toMatch(/^linux:\d+$/);
|
|
});
|
|
|
|
it("proves ancestry through every intermediate wrapper", () => {
|
|
const parents = new Map([
|
|
[400, 300],
|
|
[300, 200],
|
|
[200, 1],
|
|
]);
|
|
|
|
expect(isProcessDescendant(400, 200, (pid) => parents.get(pid) ?? null)).toBe(true);
|
|
expect(isProcessDescendant(400, 999, (pid) => parents.get(pid) ?? null)).toBe(false);
|
|
});
|
|
|
|
it("fails closed on missing or cyclic process metadata", () => {
|
|
expect(isProcessDescendant(400, 200, () => null)).toBe(false);
|
|
expect(isProcessDescendant(400, 200, (pid) => (pid === 400 ? 300 : 400))).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe.skipIf(!IS_UNIX)("killProcessTree", () => {
|
|
it("kills a process and all its children", async () => {
|
|
// Spawn a parent that spawns two sleeping children
|
|
const parent = spawn("bash", ["-c", "sleep 60 & sleep 60 & wait"], {
|
|
stdio: "ignore",
|
|
});
|
|
// Let children spawn
|
|
await new Promise((r) => setTimeout(r, 200));
|
|
|
|
const exitPromise = new Promise<void>((resolve) => parent.on("close", resolve));
|
|
killProcessTree(parent.pid!);
|
|
|
|
await exitPromise;
|
|
|
|
// Verify parent is dead
|
|
expect(() => process.kill(parent.pid!, 0)).toThrow();
|
|
}, 5000);
|
|
|
|
it("handles non-existent PID gracefully", () => {
|
|
// Should not throw for a PID that doesn't exist
|
|
killProcessTree(999999999);
|
|
});
|
|
|
|
it("escalates to SIGKILL after grace period", async () => {
|
|
// Spawn a process that traps SIGTERM
|
|
const proc = spawn("bash", ["-c", "trap '' TERM; sleep 60"], {
|
|
stdio: "ignore",
|
|
});
|
|
await new Promise((r) => setTimeout(r, 100));
|
|
|
|
const exitPromise = new Promise<void>((resolve) => proc.on("close", resolve));
|
|
killProcessTree(proc.pid!);
|
|
|
|
// Should die within 1s (500ms SIGKILL grace + buffer)
|
|
await exitPromise;
|
|
expect(() => process.kill(proc.pid!, 0)).toThrow();
|
|
}, 5000);
|
|
});
|
|
|
|
describe.skipIf(!IS_UNIX)("killOrphanedProcesses", () => {
|
|
it("returns 0 when no orphans exist", () => {
|
|
const killed = killOrphanedProcesses();
|
|
expect(killed).toBe(0);
|
|
});
|
|
|
|
it("does not kill non-orphaned Chrome processes", () => {
|
|
// Our current process is not an orphan (PPID !== 1), so any
|
|
// chrome-headless-shell processes we'd find with our PID as
|
|
// ancestor wouldn't be killed.
|
|
const killed = killOrphanedProcesses();
|
|
expect(killed).toBe(0);
|
|
});
|
|
});
|