fix(cli): keep a live preview's ownership record and stop past a bad one (#3308)

* 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.
This commit is contained in:
Miguel Ángel
2026-08-18 19:50:02 -04:00
committed by GitHub
parent b31dde35b1
commit 74149e249a
5 changed files with 949 additions and 78 deletions
+72 -3
View File
@@ -1,8 +1,9 @@
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { studioLandingSearch } from "./preview.js";
import { join, resolve } from "node:path";
import * as clack from "@clack/prompts";
import { afterEach, describe, expect, it, vi } from "vitest";
import { handlePreviewKillAll, handlePreviewList, studioLandingSearch } from "./preview.js";
const tempDirs: string[] = [];
@@ -51,3 +52,71 @@ describe("studioLandingSearch", () => {
expect(studioLandingSearch(dir)).toBe("");
});
});
describe("preview --kill-all", () => {
const session = (port: number, projectDir: string) => ({
pid: 4321,
port,
projectDir,
logPath: `${projectDir}.log`,
});
it("keeps stopping after a record whose ownership cannot be proven", async () => {
// Propagating the first failure left every later preview running AND
// unreported — the one thing a stop pass must never do.
const log = vi.spyOn(console, "log").mockImplementation(() => {});
const warn = vi.spyOn(clack.log, "warn").mockImplementation(() => {});
await handlePreviewKillAll(3002, {
listManaged: async () => [session(41402, "/tmp/unprovable"), session(41403, "/tmp/healthy")],
stopManaged: async (projectDir) => {
if (projectDir === "/tmp/unprovable") throw new Error("ownership failed");
return true;
},
killScanned: async () => ({ killed: 0, unverified: [] }),
});
expect(log.mock.calls.flat().join("\n")).toContain("Killed 1 preview server");
expect(warn.mock.calls.flat().join("\n")).toContain("/tmp/unprovable: ownership failed");
log.mockRestore();
warn.mockRestore();
});
it("reports nothing to kill when no preview is running", async () => {
const log = vi.spyOn(console, "log").mockImplementation(() => {});
await handlePreviewKillAll(3002, {
listManaged: async () => [],
killScanned: async () => ({ killed: 0, unverified: [] }),
});
expect(log.mock.calls.flat().join("\n")).toContain("No active preview servers to kill");
log.mockRestore();
});
});
describe("preview --list", () => {
it("prefers the managed record over the same server's own self-report", async () => {
const log = vi.spyOn(console, "log").mockImplementation(() => {});
await handlePreviewList(3002, {
listManaged: async () => [
{ pid: 99, port: 3002, projectDir: resolve("/tmp/demo"), logPath: "/tmp/demo.log" },
],
scan: async () => [
{
port: 3002,
projectName: "demo",
projectDir: resolve("/tmp/demo"),
version: "test",
pid: "99",
},
],
});
const printed = log.mock.calls.flat().join("\n");
expect(printed).toContain("1 server running");
expect(printed).toContain("PID 99");
log.mockRestore();
});
});
+99 -29
View File
@@ -65,6 +65,7 @@ import { resolveProject } from "../utils/project.js";
import { resolveAutoProxy } from "../utils/projectConfig.js";
import { studioProxyEnv } from "../utils/studioProxyEnv.js";
import {
listBackgroundPreviewStatuses,
readBackgroundPreviewStatus,
startBackgroundPreview,
stopBackgroundPreview,
@@ -239,40 +240,13 @@ export default defineCommand({
// --list: scan and display active servers
if (args.list) {
const servers = await scanActiveServers(startPort);
if (servers.length === 0) {
console.log("\n No active preview servers found.\n");
return;
}
console.log(`\n ${c.bold("Active preview servers:")}\n`);
for (const s of servers) {
const pidStr = s.pid ? c.dim(` (PID ${s.pid})`) : "";
console.log(
` ${c.accent(`Port ${s.port}`)} ${s.projectName} ${c.dim(s.projectDir)}${pidStr}`,
);
}
console.log(`\n ${servers.length} server${servers.length === 1 ? "" : "s"} running.\n`);
await handlePreviewList(startPort);
return;
}
// --kill-all: kill all active servers
if (args["kill-all"]) {
const servers = await scanActiveServers(startPort);
if (servers.length === 0) {
console.log("\n No active preview servers to kill.\n");
return;
}
const { killed, unverified } = await killActiveServers(startPort);
console.log(`\n Killed ${killed} preview server${killed === 1 ? "" : "s"}.`);
if (unverified.length > 0) {
clack.log.warn(
`Left ${unverified.length} server${unverified.length === 1 ? "" : "s"} alone ` +
`(port${unverified.length === 1 ? "" : "s"} ${unverified.join(", ")}): the OS could ` +
`not confirm which process owns the socket, and the server's own claim is not proof. ` +
`Install lsof, or stop it with its own preview --stop.`,
);
}
console.log();
await handlePreviewKillAll(startPort);
return;
}
@@ -447,6 +421,102 @@ export default defineCommand({
},
});
interface PreviewActionDependencies {
scan?: typeof scanActiveServers;
listManaged?: typeof listBackgroundPreviewStatuses;
stopManaged?: typeof stopBackgroundPreview;
killScanned?: typeof killActiveServers;
}
/**
* Managed previews first, then anything else answering on the scanned range.
* A managed session is the authoritative entry for its project and port — the
* scan would otherwise list the same server again from its own self-report.
*/
export async function handlePreviewList(
startPort: number,
dependencies: PreviewActionDependencies = {},
): Promise<void> {
const [scannedServers, managedSessions] = await Promise.all([
(dependencies.scan ?? scanActiveServers)(startPort),
(dependencies.listManaged ?? listBackgroundPreviewStatuses)(),
]);
const managedKeys = new Set(
managedSessions.map((session) => `${resolve(session.projectDir)}\0${session.port}`),
);
const servers = [
...managedSessions.map((session) => ({
port: session.port,
projectName: basename(session.projectDir),
projectDir: session.projectDir,
pid: String(session.pid),
})),
...scannedServers.filter(
(server) => !managedKeys.has(`${resolve(server.projectDir)}\0${server.port}`),
),
];
if (servers.length === 0) {
console.log("\n No active preview servers found.\n");
return;
}
console.log(`\n ${c.bold("Active preview servers:")}\n`);
for (const server of servers) {
const pid = server.pid ? c.dim(` (PID ${server.pid})`) : "";
console.log(
` ${c.accent(`Port ${server.port}`)} ${server.projectName} ${c.dim(server.projectDir)}${pid}`,
);
}
console.log(`\n ${servers.length} server${servers.length === 1 ? "" : "s"} running.\n`);
}
/**
* Stop every managed preview through its ownership record, then sweep whatever
* else is still listening.
*
* Per-record failures are collected rather than propagated: one record whose
* ownership cannot be proven must not abandon the servers after it, which would
* leave them running AND unreported.
*/
export async function handlePreviewKillAll(
startPort: number,
dependencies: PreviewActionDependencies = {},
): Promise<void> {
const managedSessions = await (dependencies.listManaged ?? listBackgroundPreviewStatuses)();
let killed = 0;
const failures: string[] = [];
for (const session of managedSessions) {
try {
if (
await (dependencies.stopManaged ?? stopBackgroundPreview)(session.projectDir, session.port)
) {
killed++;
}
} catch (error) {
failures.push(`${session.projectDir}: ${error instanceof Error ? error.message : error}`);
}
}
const swept = await (dependencies.killScanned ?? killActiveServers)(startPort);
killed += swept.killed;
if (killed > 0) {
console.log(`\n Killed ${killed} preview server${killed === 1 ? "" : "s"}.`);
} else if (failures.length === 0 && swept.unverified.length === 0) {
console.log("\n No active preview servers to kill.");
}
for (const failure of failures) clack.log.warn(`Could not stop ${failure}`);
if (swept.unverified.length > 0) {
// Fail-closed, said out loud: a security control that degrades silently is
// one nobody knows to fix.
const ports = swept.unverified.join(", ");
const plural = swept.unverified.length === 1 ? "" : "s";
clack.log.warn(
`Left ${swept.unverified.length} server${plural} alone (port${plural} ${ports}): the OS ` +
`could not confirm which process owns the socket, and the server's own claim is not ` +
`proof. Install lsof, or stop it with its own preview --stop.`,
);
}
console.log();
}
// `host` is the loopback the server actually bound (Vite binds `[::1]`, embedded
// binds `127.0.0.1`); default to IPv4 for the embedded/legacy callers.
function previewBaseUrl(port: number, host = "127.0.0.1"): string {
@@ -1,10 +1,11 @@
import { existsSync, mkdtempSync } from "node:fs";
import { existsSync, mkdtempSync, readFileSync, readdirSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { describe, expect, it, vi } from "vitest";
import type { ActiveServer } from "../server/portUtils.js";
import {
buildBackgroundPreviewArgs,
listBackgroundPreviewStatuses,
previewSessionPath,
readBackgroundPreviewStatus,
startBackgroundPreview,
@@ -48,7 +49,7 @@ describe("background preview lifecycle", () => {
);
});
it("builds a detached child invocation without recursively preserving --background", () => {
it("does not let the detached child inherit launcher-only flags", () => {
expect(
buildBackgroundPreviewArgs([
"/opt/hyperframes/cli.js",
@@ -76,6 +77,77 @@ describe("background preview lifecycle", () => {
expect(spawn).not.toHaveBeenCalled();
});
it("reuses a saved managed preview on a custom port without repeating --port", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{ pid: 4321, port: 41402, projectDir, logPath: "/tmp/custom.log" },
stateHome,
);
const customServer = { ...server, port: 41402, browserGpuMode: "software" as const };
const scan = vi.fn(async (startPort?: number) => (startPort === 41402 ? [customServer] : []));
const spawn = vi.fn();
const result = await startBackgroundPreview(projectDir, 3002, {
scan,
spawn,
stateHome,
});
expect(result).toMatchObject({ type: "reused", port: 41402, pid: 4321 });
expect(scan).toHaveBeenCalledWith(41402);
expect(spawn).not.toHaveBeenCalled();
});
it("discovers managed previews outside the default port scan and removes stale records", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const otherProjectDir = resolve("/tmp/hyperframes-preview-managed-custom-port");
const staleProjectDir = resolve("/tmp/hyperframes-preview-managed-stale");
writePreviewSession(
{
pid: 8765,
port: 41402,
projectDir: otherProjectDir,
logPath: "/tmp/custom.log",
},
stateHome,
);
writePreviewSession(
{
pid: 9999,
port: 45000,
projectDir: staleProjectDir,
logPath: "/tmp/stale.log",
},
stateHome,
);
const statuses = await listBackgroundPreviewStatuses({
stateHome,
scan: async (startPort) =>
startPort === 41402
? [
{
port: 41402,
projectName: "managed-custom-port",
projectDir: otherProjectDir,
version: "test",
pid: "8765",
},
]
: [],
});
expect(statuses).toEqual([
{
pid: 8765,
port: 41402,
projectDir: otherProjectDir,
logPath: "/tmp/custom.log",
},
]);
expect(existsSync(previewSessionPath(staleProjectDir, stateHome))).toBe(false);
});
it("force-new waits for a different server instead of reusing the existing one", async () => {
const replacement = { ...server, port: 3211, pid: "5432" };
let scans = 0;
@@ -94,6 +166,143 @@ describe("background preview lifecycle", () => {
expect(spawn).toHaveBeenCalledOnce();
});
it.each([
["force-new", true],
["a GPU-policy change", false],
])(
"%s replaces a previously managed server instead of orphaning it",
async (_label, forceNew) => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const oldServer = { ...server, port: 41490, browserGpuMode: "hardware" as const };
writePreviewSession(
{ pid: 4321, port: 41490, projectDir, logPath: "/tmp/preview.log" },
stateHome,
);
const replacement = {
...server,
port: 41491,
pid: "5432",
browserGpuMode: "software" as const,
};
let oldRunning = true;
let replacementRunning = false;
const scan = vi.fn(async () =>
oldRunning ? [oldServer] : replacementRunning ? [replacement] : [],
);
const kill = vi.fn((pid: number) => {
if (pid === 4321) oldRunning = false;
});
const spawn = vi.fn(() => {
replacementRunning = true;
return { pid: 5432, unref: vi.fn() };
});
const result = await startBackgroundPreview(projectDir, 41491, {
browserGpuMode: "software",
forceNew,
kill,
scan,
sleep: async () => {},
spawn,
stateHome,
});
expect(scan).toHaveBeenNthCalledWith(1, 41490);
expect(kill).toHaveBeenCalledWith(4321);
expect(result).toMatchObject({ type: "started", port: 41491, pid: 5432 });
expect(readFileSync(previewSessionPath(projectDir, stateHome), "utf8")).toContain(
'"port": 41491',
);
},
);
it("replaces the owned preview instead of reusing an unmanaged policy-matching sibling", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const owned = { ...server, port: 41490, browserGpuMode: "hardware" as const };
const sibling = {
...server,
port: 41491,
pid: "8765",
browserGpuMode: "software" as const,
};
const replacement = {
...server,
port: 41492,
pid: "5432",
browserGpuMode: "software" as const,
};
writePreviewSession(
{ pid: 4321, port: owned.port, projectDir, logPath: "/tmp/preview.log" },
stateHome,
);
let ownedRunning = true;
let replacementRunning = false;
const scan = vi.fn(async () => [
...(ownedRunning ? [owned] : []),
sibling,
...(replacementRunning ? [replacement] : []),
]);
const kill = vi.fn((pid: number) => {
if (pid === 4321) ownedRunning = false;
});
const spawn = vi.fn(() => {
replacementRunning = true;
return { pid: 5432, unref: vi.fn() };
});
const result = await startBackgroundPreview(projectDir, replacement.port, {
browserGpuMode: "software",
kill,
scan,
sleep: async () => {},
spawn,
stateHome,
});
expect(kill).toHaveBeenCalledWith(4321);
expect(spawn).toHaveBeenCalledOnce();
expect(result).toMatchObject({ type: "started", port: replacement.port, pid: 5432 });
});
it("launches the replacement when the owned server died on its own", async () => {
// The owned preview crashes (or is Ctrl-C'd) between the outer scan and the
// one inside the stop. "Nothing left to stop" is the goal state for a
// replacement; treating it as fatal refused to start any preview at all
// until the session record was deleted by hand.
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const owned = { ...server, port: 41490 };
const replacement = { ...server, port: 41491, pid: "5432" };
writePreviewSession(
{ pid: 4321, port: owned.port, projectDir, logPath: "/tmp/preview.log" },
stateHome,
);
let scans = 0;
let replacementRunning = false;
const scan = vi.fn(async () => {
scans += 1;
// Alive for the first look, gone by the time the stop path scans.
const ownedNow = scans === 1 ? [owned] : [];
return [...ownedNow, ...(replacementRunning ? [replacement] : [])];
});
const kill = vi.fn();
const spawn = vi.fn(() => {
replacementRunning = true;
return { pid: 5432, unref: vi.fn() };
});
const result = await startBackgroundPreview(projectDir, replacement.port, {
forceNew: true,
kill,
scan,
sleep: async () => {},
spawn,
stateHome,
});
expect(kill).not.toHaveBeenCalled();
expect(result).toMatchObject({ type: "started", port: replacement.port, pid: 5432 });
});
it("starts a replacement when the existing server uses a different GPU policy", async () => {
const hardwareServer = { ...server, browserGpuMode: "hardware" as const };
const softwareServer = {
@@ -102,11 +311,15 @@ describe("background preview lifecycle", () => {
pid: "5432",
browserGpuMode: "software" as const,
};
let scans = 0;
const scan = vi.fn(async () =>
++scans < 2 ? [hardwareServer] : [hardwareServer, softwareServer],
);
const spawn = vi.fn(() => ({ pid: 5432, unref: vi.fn() }));
let replacementRunning = false;
const scan = vi.fn(async () => [
hardwareServer,
...(replacementRunning ? [softwareServer] : []),
]);
const spawn = vi.fn(() => {
replacementRunning = true;
return { pid: 5432, unref: vi.fn() };
});
const result = await startBackgroundPreview(projectDir, 3002, {
browserGpuMode: "software",
@@ -121,10 +334,13 @@ describe("background preview lifecycle", () => {
});
it("returns after a detached child becomes reachable and records its session", async () => {
let scans = 0;
const scan = vi.fn(async () => (++scans < 2 ? [] : [server]));
let spawned = false;
const scan = vi.fn(async () => (spawned ? [server] : []));
const unref = vi.fn();
const spawn = vi.fn(() => ({ pid: 4321, unref }));
const spawn = vi.fn(() => {
spawned = true;
return { pid: 4321, unref };
});
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const result = await startBackgroundPreview(projectDir, 3002, {
@@ -141,6 +357,46 @@ describe("background preview lifecycle", () => {
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(true);
});
it("reports the live server PID while retaining the wrapper PID for cleanup", async () => {
const liveServer = { ...server, pid: "9876" };
let spawned = false;
const scan = vi.fn(async () => (spawned ? [liveServer] : []));
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const result = await startBackgroundPreview(projectDir, 3002, {
scan,
spawn: () => {
spawned = true;
return { pid: 4321, unref: vi.fn() };
},
stateHome,
});
expect(result).toMatchObject({ type: "started", pid: 9876 });
expect(
JSON.parse(readFileSync(previewSessionPath(projectDir, stateHome), "utf8")),
).toMatchObject({ pid: 4321 });
});
it("reaps a detached child that never becomes reachable without recording ownership", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const kill = vi.fn();
await expect(
startBackgroundPreview(projectDir, 3002, {
scan: async () => [],
spawn: () => ({ pid: 4321, unref: vi.fn() }),
sleep: async () => {},
kill,
stateHome,
}),
).rejects.toThrow(/did not become ready/i);
expect(kill).toHaveBeenCalledOnce();
expect(kill).toHaveBeenCalledWith(4321);
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false);
});
it("removes a stale session when no matching server or process survives", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
@@ -158,12 +414,113 @@ describe("background preview lifecycle", () => {
await expectStaleSessionRemoved(stateHome);
});
it("keeps a live preview's record when a single probe misses it", async () => {
// A server whose event loop is briefly blocked (a Puppeteer thumbnail
// capture will do it) answers nothing for a second or two. Retiring the
// record on that destroys the wrapperIdentity that is the only PID-reuse
// guard `--stop` has, and it never comes back.
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{ pid: 4321, wrapperIdentity: "posix:birth", port: 3210, projectDir, logPath: "/tmp/p.log" },
stateHome,
);
const status = await readBackgroundPreviewStatus(projectDir, 3002, {
scan: async () => [],
identity: () => "posix:birth",
isSignalable: () => true,
stateHome,
});
expect(status).toBeNull();
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(true);
});
it("keeps a live preview's record when the identity lookup gives no answer", async () => {
// `processIdentity` catches every failure into `null`, and on Windows and
// macOS that failure is a subprocess timeout on a LIVE process — under the
// same load that made the HTTP probe miss. Treating no-answer as
// "recycled" destroyed the only PID-reuse guard `--stop` has.
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{ pid: 4321, wrapperIdentity: "posix:birth", port: 3210, projectDir, logPath: "/tmp/p.log" },
stateHome,
);
const status = await readBackgroundPreviewStatus(projectDir, 3002, {
scan: async () => [],
identity: () => null,
isSignalable: () => true,
stateHome,
});
expect(status).toBeNull();
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(true);
});
it("retires the record when the PID cannot be signalled at all", async () => {
// Gone is gone: no birth token needed, and no subprocess spawned for it.
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{ pid: 4321, wrapperIdentity: "posix:birth", port: 3210, projectDir, logPath: "/tmp/p.log" },
stateHome,
);
const identity = vi.fn(() => "posix:birth");
const status = await readBackgroundPreviewStatus(projectDir, 3002, {
scan: async () => [],
identity,
isSignalable: () => false,
stateHome,
});
expect(status).toBeNull();
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false);
expect(identity).not.toHaveBeenCalled();
});
it("retires the record once the wrapper PID has been recycled", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{ pid: 4321, wrapperIdentity: "posix:birth", port: 3210, projectDir, logPath: "/tmp/p.log" },
stateHome,
);
const status = await readBackgroundPreviewStatus(projectDir, 3002, {
scan: async () => [],
identity: () => "posix:someone-else",
isSignalable: () => true,
stateHome,
});
expect(status).toBeNull();
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false);
});
it("never leaves a partial session record for a concurrent reader", () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
savePreviewSession(stateHome);
const path = previewSessionPath(projectDir, stateHome);
writePreviewSession({ pid: 55, port: 3211, projectDir, logPath: "/tmp/two.log" }, stateHome);
// Written through a temp file and renamed, so a reader mid-write sees the
// whole previous record rather than truncated JSON it would then delete.
expect(JSON.parse(readFileSync(path, "utf8"))).toMatchObject({ pid: 55, port: 3211 });
expect(
readdirSync(join(stateHome, "hyperframes", "previews")).filter((n) => n.endsWith(".tmp")),
).toHaveLength(0);
});
it("uses the recorded custom port when status is called without repeating --port", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
savePreviewSession(stateHome);
const scan = vi.fn(async () => [server]);
const status = await readBackgroundPreviewStatus(projectDir, 3002, { scan, stateHome });
const status = await readBackgroundPreviewStatus(projectDir, 3002, {
scan,
stateHome,
});
expect(status?.port).toBe(3210);
expect(scan).toHaveBeenCalledWith(3210);
@@ -204,27 +561,87 @@ describe("background preview lifecycle", () => {
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false);
});
it("uses the saved child PID when a matching live server cannot report one", async () => {
it("refuses to stop when the live server cannot prove its own PID", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{ pid: 4321, port: 3210, projectDir, logPath: "/tmp/preview.log" },
stateHome,
);
const scan = vi.fn(async () => [{ ...server, pid: null }]);
const kill = vi.fn();
await expect(
stopBackgroundPreview(projectDir, 3002, {
scan,
kill,
sleep: async () => {},
stateHome,
}),
).rejects.toThrow(/ownership/i);
expect(kill).not.toHaveBeenCalled();
});
it("reaps the saved wrapper when the live server is proven to be its descendant", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{
pid: 4321,
wrapperIdentity: "wrapper-birth",
port: 3210,
projectDir,
logPath: "/tmp/preview.log",
},
stateHome,
);
let running = true;
const scan = vi.fn(async () => (running ? [{ ...server, pid: null }] : []));
const kill = vi.fn(() => {
running = false;
const scan = vi.fn(async () => (running ? [{ ...server, pid: "9876" }] : []));
const kill = vi.fn((pid: number) => {
if (pid === 4321) running = false;
});
const result = await stopBackgroundPreview(projectDir, 3002, {
scan,
kill,
isDescendant: (childPid, ancestorPid) => childPid === 9876 && ancestorPid === 4321,
identity: (pid) => (pid === 4321 ? "wrapper-birth" : null),
sleep: async () => {},
stateHome,
});
expect(result).toBe(true);
expect(kill).toHaveBeenCalledWith(4321);
expect(kill.mock.calls).toEqual([[4321]]);
});
it("kills only the live server when the saved wrapper birth identity has changed", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
writePreviewSession(
{
pid: 4321,
wrapperIdentity: "original-wrapper-birth",
port: 3210,
projectDir,
logPath: "/tmp/preview.log",
},
stateHome,
);
let running = true;
const scan = vi.fn(async () => (running ? [{ ...server, pid: "9876" }] : []));
const kill = vi.fn((pid: number) => {
if (pid === 9876) running = false;
});
const result = await stopBackgroundPreview(projectDir, 3002, {
scan,
kill,
isDescendant: () => true,
identity: () => "reused-pid-birth",
sleep: async () => {},
stateHome,
});
expect(result).toBe(true);
expect(kill.mock.calls).toEqual([[9876]]);
});
it("fails loudly when the server remains reachable after stop", async () => {
@@ -244,4 +661,30 @@ describe("background preview lifecycle", () => {
).rejects.toThrow(/did not stop/i);
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(true);
});
it("verifies the owned port stopped even when another server serves the same project", async () => {
const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-"));
const owned = { ...server, port: 41490 };
const sibling = { ...server, port: 41491, pid: "8765" };
writePreviewSession(
{ pid: 4321, port: owned.port, projectDir, logPath: "/tmp/preview.log" },
stateHome,
);
let ownedRunning = true;
const scan = vi.fn(async () => [...(ownedRunning ? [owned] : []), sibling]);
const kill = vi.fn((pid: number) => {
if (pid === 4321) ownedRunning = false;
});
const stopped = await stopBackgroundPreview(projectDir, owned.port, {
kill,
scan,
sleep: async () => {},
stateHome,
});
expect(stopped).toBe(true);
expect(kill).toHaveBeenCalledWith(4321);
expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false);
});
});
+286 -28
View File
@@ -6,6 +6,8 @@ import {
mkdirSync,
openSync,
readFileSync,
readdirSync,
renameSync,
rmSync,
writeFileSync,
} from "node:fs";
@@ -13,10 +15,11 @@ import { homedir } from "node:os";
import { dirname, join, resolve } from "node:path";
import { scanActiveServers, type ActiveServer } from "../server/portUtils.js";
import type { BrowserGpuMode } from "../browser/gpuPolicy.js";
import { killProcessTree } from "../utils/orphanCleanup.js";
import { isProcessDescendant, killProcessTree, processIdentity } from "../utils/orphanCleanup.js";
export interface PreviewSession {
pid: number;
wrapperIdentity?: string;
port: number;
projectDir: string;
logPath: string;
@@ -40,6 +43,9 @@ interface LifecycleDependencies {
spawn?: SpawnPreview;
sleep?: (ms: number) => Promise<void>;
kill?: (pid: number) => void;
isDescendant?: (childPid: number, ancestorPid: number) => boolean;
identity?: (pid: number) => string | null;
isSignalable?: (pid: number) => boolean;
stateHome?: string;
forceNew?: boolean;
browserGpuMode?: BrowserGpuMode;
@@ -70,7 +76,19 @@ function previewLogPath(projectDir: string, stateHome = defaultStateHome()): str
export function writePreviewSession(session: PreviewSession, stateHome = defaultStateHome()): void {
const path = previewSessionPath(session.projectDir, stateHome);
mkdirSync(dirname(path), { recursive: true });
writeFileSync(path, `${JSON.stringify(session, null, 2)}\n`, { mode: 0o600 });
// Written through a temp file and renamed: every reader deletes this record
// when it fails to parse, so a concurrent reader catching a half-written file
// would destroy a live server's only ownership proof. `rename` is atomic
// within a directory, so a reader sees either the old record or the new one.
const temporary = `${path}.${process.pid}.tmp`;
try {
writeFileSync(temporary, `${JSON.stringify(session, null, 2)}\n`, { mode: 0o600 });
renameSync(temporary, path);
} finally {
// A failed rename would otherwise orphan the temp file. The `.json` filter
// hides it from the lister, so it accumulates silently.
rmSync(temporary, { force: true });
}
}
function readPreviewSession(
@@ -95,6 +113,43 @@ function readPreviewSession(
}
}
function hasValidPreviewProcess(session: PreviewSession): boolean {
return Number.isInteger(session.pid) && session.pid > 0;
}
function hasValidPreviewEndpoint(session: PreviewSession): boolean {
return Number.isInteger(session.port) && session.port > 0 && session.port <= 65535;
}
function matchesPreviewSessionFile(
session: PreviewSession,
path: string,
stateHome: string,
): boolean {
return (
typeof session.projectDir === "string" &&
typeof session.logPath === "string" &&
previewSessionPath(session.projectDir, stateHome) === path
);
}
function readPreviewSessionFile(path: string, stateHome: string): PreviewSession | null {
try {
const parsed = JSON.parse(readFileSync(path, "utf8")) as PreviewSession;
if (
!hasValidPreviewProcess(parsed) ||
!hasValidPreviewEndpoint(parsed) ||
!matchesPreviewSessionFile(parsed, path, stateHome)
) {
throw new Error("invalid preview session");
}
return parsed;
} catch {
rmSync(path, { force: true });
return null;
}
}
function removePreviewSession(projectDir: string, stateHome = defaultStateHome()): void {
rmSync(previewSessionPath(projectDir, stateHome), { force: true });
}
@@ -113,6 +168,26 @@ function matchingServer(
);
}
function matchingServerAtPort(
servers: ActiveServer[],
projectDir: string,
port: number,
): ActiveServer | null {
return matchingServer(
servers.filter((server) => server.port === port),
projectDir,
);
}
function sameProjectPorts(servers: ActiveServer[], projectDir: string): Set<number> {
const project = normalized(projectDir);
return new Set(
servers
.filter((server) => normalized(server.projectDir) === project)
.map((server) => server.port),
);
}
function stopProcess(pid: number): void {
killProcessTree(pid);
if (process.platform === "win32") {
@@ -130,7 +205,7 @@ function spawnDetachedPreview(
projectDir: string,
stateHome: string,
dependencies: LifecycleDependencies,
): { pid: number; logPath: string } {
): { pid: number; wrapperIdentity: string | undefined; logPath: string } {
const logPath = previewLogPath(projectDir, stateHome);
mkdirSync(dirname(logPath), { recursive: true });
const logFd = openSync(logPath, "a", 0o600);
@@ -151,18 +226,20 @@ function spawnDetachedPreview(
}
if (!child.pid) throw new Error("background preview child did not report a PID");
child.unref();
return { pid: child.pid, logPath };
return {
pid: child.pid,
wrapperIdentity: (dependencies.identity ?? processIdentity)(child.pid) ?? undefined,
logPath,
};
}
function startedServer(
servers: ActiveServer[],
projectDir: string,
existing: ActiveServer | null,
forceNew: boolean,
preLaunchPorts: Set<number>,
browserGpuMode?: BrowserGpuMode,
): ActiveServer | null {
const candidates =
forceNew && existing ? servers.filter((server) => server.port !== existing.port) : servers;
const candidates = servers.filter((server) => !preLaunchPorts.has(server.port));
return matchingServer(candidates, projectDir, browserGpuMode);
}
@@ -198,10 +275,154 @@ export async function readBackgroundPreviewStatus(
}
}
// A missed probe is not proof the preview is gone: a server whose event loop
// is momentarily blocked (a Puppeteer thumbnail capture will do it) answers
// nothing for a second or two. Deleting the record on that would destroy the
// wrapperIdentity that is the only PID-reuse guard `--stop` has, and it never
// comes back. Only a wrapper process that is provably gone retires a record.
if (saved && wrapperProcessIsAlive(saved, dependencies)) return null;
removePreviewSession(projectDir, stateHome);
return null;
}
/**
* Whether the recorded wrapper process is still the process we launched.
*
* `processIdentity` returns a birth token, so a recycled PID reads as a
* different process and the record is correctly retired. A null token (no such
* process, or the platform lookup failed) is only treated as "gone" when the
* record has no token to compare against — failing closed there would pin dead
* records forever on platforms where the lookup is unavailable.
*/
/**
* Whether a PID exists at all. `kill(pid, 0)` sends no signal; it only asks the
* kernel. `EPERM` means the process is there but owned by someone else — still
* alive, which is the question being asked.
*/
function processIsSignalable(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch (error) {
return (error as NodeJS.ErrnoException | undefined)?.code === "EPERM";
}
}
function wrapperProcessIsAlive(
saved: PreviewSession,
dependencies: LifecycleDependencies,
): boolean {
if (!saved.wrapperIdentity) return false;
// Cheap and decisive first: a PID nothing can signal is gone, and no birth
// token is needed to say so. This also keeps the identity subprocess off the
// path for exactly the stale records that make `--list` slow.
const signalable = dependencies.isSignalable ?? processIsSignalable;
if (!signalable(saved.pid)) return false;
const identity = (dependencies.identity ?? processIdentity)(saved.pid);
// No answer is NOT the same as a different answer. `processIdentity` catches
// every failure into `null`, and on two of three platforms that failure is a
// subprocess timeout on a live process — `ps -o lstart=` and PowerShell's CIM
// query both run on a 2 s budget, under the very load that made the HTTP
// probe miss in the first place. Treating that as "recycled" would destroy
// the only PID-reuse guard `--stop` has, which is the loss this whole path
// exists to prevent. The PID is signalable, so keep the record.
if (identity === null) return true;
return identity === saved.wrapperIdentity;
}
export async function listBackgroundPreviewStatuses(
dependencies: LifecycleDependencies = {},
): Promise<PreviewSession[]> {
const stateHome = dependencies.stateHome ?? defaultStateHome();
const directory = sessionDirectory(stateHome);
let files: string[];
try {
files = readdirSync(directory)
.filter((name) => name.endsWith(".json"))
.map((name) => join(directory, name));
} catch {
return [];
}
const saved = files
.map((path) => readPreviewSessionFile(path, stateHome))
.filter((session): session is PreviewSession => session !== null);
const statuses = await Promise.all(
saved.map((session) =>
readBackgroundPreviewStatus(session.projectDir, session.port, {
...dependencies,
stateHome,
}),
),
);
return statuses.filter((status): status is PreviewSession => status !== null);
}
function readyPreviewSession(
server: ActiveServer,
pid: number,
wrapperIdentity: string | undefined,
projectDir: string,
logPath: string,
dependencies: LifecycleDependencies,
): { session: PreviewSession; publicPid: number } {
const identity = wrapperIdentity ?? (dependencies.identity ?? processIdentity)(pid) ?? undefined;
const liveServerPid = Number(server.pid);
return {
session: {
pid,
wrapperIdentity: identity,
port: server.port,
projectDir: resolve(projectDir),
logPath,
},
publicPid: Number.isInteger(liveServerPid) && liveServerPid > 0 ? liveServerPid : pid,
};
}
function ownedStopTargetPid(
saved: PreviewSession | null,
liveServerPid: number,
dependencies: LifecycleDependencies,
): number {
if (!saved?.wrapperIdentity) return liveServerPid;
const identity = dependencies.identity ?? processIdentity;
if (identity(saved.pid) !== saved.wrapperIdentity) return liveServerPid;
if (saved.pid === liveServerPid) return saved.pid;
const isDescendant = dependencies.isDescendant ?? isProcessDescendant;
return isDescendant(liveServerPid, saved.pid) ? saved.pid : liveServerPid;
}
async function stopOwnedPreviewBeforeReplacement(
owned: ActiveServer | null,
projectDir: string,
dependencies: LifecycleDependencies,
): Promise<void> {
if (!owned) return;
// `false` means "there was nothing left to stop" — the server went away
// between the outer scan and this one. That is the goal state for a
// replacement, not a failure; treating it as fatal refused to start any
// preview at all until the user deleted the session record by hand.
// A server that is still listening throws from inside stopBackgroundPreview.
await stopBackgroundPreview(projectDir, owned.port, dependencies);
}
function savedOwnedPreview(
servers: ActiveServer[],
saved: PreviewSession | null,
projectDir: string,
): ActiveServer | null {
if (!saved) return null;
// Ownership comes from the saved project+port, not the replacement's GPU
// policy. Filtering here would miss an owned hardware→software replacement
// and overwrite the only session record while leaving the old listener live.
const savedPortServers = servers.filter((server) => server.port === saved.port);
return matchingServer(savedPortServers, projectDir);
}
export async function startBackgroundPreview(
projectDir: string,
startPort: number,
@@ -211,37 +432,65 @@ export async function startBackgroundPreview(
| { type: "started"; port: number; pid: number; logPath: string }
> {
const scan = dependencies.scan ?? scanActiveServers;
const existing = matchingServer(await scan(startPort), projectDir, dependencies.browserGpuMode);
if (existing && !dependencies.forceNew) {
const stateHome = dependencies.stateHome ?? defaultStateHome();
const saved = readPreviewSession(projectDir, stateHome);
// Always inspect a saved custom port first. `--force-new --port <new>` must
// replace that owned server before recording the replacement, otherwise the
// single per-project ownership record would orphan the old listener.
const scanStart = saved?.port ?? startPort;
const scanned = await scan(scanStart);
const requestedExisting = matchingServer(scanned, projectDir, dependencies.browserGpuMode);
const ownedExisting = savedOwnedPreview(scanned, saved, projectDir);
// A saved managed preview is the authoritative same-project instance. An
// explicit GPU-policy change replaces it; it must not silently adopt an
// unmanaged sibling that happens to match the new policy.
const reusableOwned = ownedExisting
? matchingServer([ownedExisting], projectDir, dependencies.browserGpuMode)
: null;
const reusableExisting = reusableOwned ?? (ownedExisting ? null : requestedExisting);
if (reusableExisting && !dependencies.forceNew) {
return {
type: "reused",
port: existing.port,
pid: existing.pid ? Number(existing.pid) : null,
port: reusableExisting.port,
pid: reusableExisting.pid ? Number(reusableExisting.pid) : null,
logPath: null,
};
}
await stopOwnedPreviewBeforeReplacement(ownedExisting, projectDir, dependencies);
// Snapshot every same-project listener in the prospective launch range only
// after the owned listener is gone. Readiness must identify a newly appeared
// server, never a pre-existing unmanaged sibling.
const preLaunchPorts = sameProjectPorts(await scan(startPort), projectDir);
const stateHome = dependencies.stateHome ?? defaultStateHome();
const { pid, logPath } = spawnDetachedPreview(projectDir, stateHome, dependencies);
const { pid, wrapperIdentity, logPath } = spawnDetachedPreview(
projectDir,
stateHome,
dependencies,
);
const sleep = dependencies.sleep ?? delay;
for (let attempt = 0; attempt < 50; attempt++) {
const server = startedServer(
await scan(startPort),
projectDir,
existing,
dependencies.forceNew === true,
preLaunchPorts,
dependencies.browserGpuMode,
);
if (server) {
const session = {
const ready = readyPreviewSession(
server,
pid,
port: server.port,
projectDir: resolve(projectDir),
wrapperIdentity,
projectDir,
logPath,
dependencies,
);
writePreviewSession(ready.session, stateHome);
return {
type: "started",
...ready.session,
pid: ready.publicPid,
};
writePreviewSession(session, stateHome);
return { type: "started", ...session };
}
await sleep(200);
}
@@ -259,19 +508,28 @@ export async function stopBackgroundPreview(
const stateHome = dependencies.stateHome ?? defaultStateHome();
const saved = readPreviewSession(projectDir, stateHome);
const scanStart = saved?.port ?? startPort;
const server = matchingServer(await scan(scanStart), projectDir);
// A saved PID can be reused after a crashed preview, so only trust it while
// a currently reachable server proves this exact project is still running.
const pid = Number(server ? (server.pid ?? saved?.pid) : undefined);
if (!Number.isInteger(pid) || pid <= 0) {
const scanned = await scan(scanStart);
const server = saved
? matchingServerAtPort(scanned, projectDir, saved.port)
: matchingServer(scanned, projectDir);
if (!server) {
removePreviewSession(projectDir, stateHome);
return false;
}
// A saved PID can be reused after a crashed preview. The HTTP probe proves
// the project, but only the live server's own metadata proves which process
// owns it; never substitute the saved wrapper PID here.
const pid = Number(server.pid);
if (!Number.isInteger(pid) || pid <= 0) {
throw new Error(`preview ownership could not be proven for ${resolve(projectDir)}`);
}
const kill = dependencies.kill ?? stopProcess;
kill(ownedStopTargetPid(saved, pid, dependencies));
(dependencies.kill ?? stopProcess)(pid);
const sleep = dependencies.sleep ?? delay;
for (let attempt = 0; attempt < 25; attempt++) {
if (!matchingServer(await scan(scanStart), projectDir)) {
if (!matchingServerAtPort(await scan(scanStart), projectDir, server.port)) {
removePreviewSession(projectDir, stateHome);
return true;
}
+33 -2
View File
@@ -18,12 +18,43 @@ describe("Windows process-tree cleanup", () => {
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);
expect(first).toMatch(/^(?:linux|posix|windows):/);
expect(processIdentity(process.pid)).toBe(first);
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],