From 5f426778b25deede1f71cadfdffb8d043796635a Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 17 Aug 2026 14:27:49 -0400 Subject: [PATCH 1/2] fix(cli): signal only processes the OS says own the port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/__hyperframes_config` is unauthenticated and the PID it reports is what `--stop` and `--kill-all` send signals to, so any local process answering on a scanned port could name an arbitrary PID and have the CLI kill it. Reproduced with a twenty-line HTTP server on a scanned port self-reporting an unrelated PID: before this, `--kill-all` killed that process; after it, the process survives and only the real listener is stopped. The listening PID now comes from the OS — `lsof`, and `netstat` on Windows, where the lookup was previously unavailable and the self-reported value was taken on trust. The response's own PID is used only where the OS lookup fails, which is also the only case where it is unfalsifiable. Orphan cleanup moves to the last step before a launch. It reaches outside the process and kills other people's PIDs, so it must not run for an invocation that turns out to be a validation error and never starts anything. --- packages/cli/src/commands/preview.ts | 19 +-- packages/cli/src/server/portUtils.test.ts | 22 ++++ packages/cli/src/server/portUtils.ts | 79 +++++++----- packages/cli/src/utils/orphanCleanup.test.ts | 47 ++++++- packages/cli/src/utils/orphanCleanup.ts | 121 ++++++++++++++++++- 5 files changed, 244 insertions(+), 44 deletions(-) diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 1c297427f4..85f602a51f 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -287,14 +287,6 @@ export default defineCommand({ ); } - // Kill orphaned chrome-headless-shell processes from previous crashed sessions. - const orphansKilled = killOrphanedProcesses(); - if (orphansKilled > 0) { - console.log( - ` ${c.dim(`Cleaned up ${orphansKilled} orphaned process${orphansKilled === 1 ? "" : "es"} from a previous session.`)}`, - ); - } - const rawArg = args.dir; const isImplicitCwd = !rawArg || rawArg === "." || rawArg === "./"; const project = resolveProject(rawArg); @@ -352,6 +344,17 @@ export default defineCommand({ // modes all receive identical --proxy/--no-proxy + config semantics. const autoProxy = resolveAutoProxy(dir, args.proxy as boolean | undefined); + // Kill orphaned chrome-headless-shell processes from previous crashed + // sessions. Deliberately last: this reaches outside the process and kills + // other people's PIDs, so it must not run for an invocation that turns out + // to be a validation error and never starts anything. + const orphansKilled = killOrphanedProcesses(); + if (orphansKilled > 0) { + console.log( + ` ${c.dim(`Cleaned up ${orphansKilled} orphaned process${orphansKilled === 1 ? "" : "es"} from a previous session.`)}`, + ); + } + if (isDevMode()) { if (args.background) { clack.log.error("--background currently supports the embedded preview server only"); diff --git a/packages/cli/src/server/portUtils.test.ts b/packages/cli/src/server/portUtils.test.ts index 4c6ee962fd..bad74030d2 100644 --- a/packages/cli/src/server/portUtils.test.ts +++ b/packages/cli/src/server/portUtils.test.ts @@ -4,6 +4,7 @@ import { resolve } from "node:path"; import { createServer as createHttpServer, type Server as HttpServer } from "node:http"; import { PORT_PROBE_HOSTS, + activeServerOnPort, detectHyperframesServer, findPortAndServe, testPortOnAllHosts, @@ -161,6 +162,27 @@ describe("findPortAndServe — bind host (security: F-001)", () => { }); }); +describe("activeServerOnPort — PID provenance (security)", () => { + it("reports the PID that owns the socket, not the one the response claims", async () => { + // `/__hyperframes_config` is unauthenticated and `--stop` / `--kill-all` + // send signals to this field. Trusting the response let any local process + // on a scanned port name an arbitrary PID and have the CLI kill it. + if (process.platform === "win32") return; + const port = await startConfigProbeServer({ + isHyperframes: true, + projectName: "demo-project", + projectDir: "/tmp/demo-project", + serverBuildSignature: null, + version: "0.6.42", + pid: 999_999, + }); + + const server = await activeServerOnPort(port); + + expect(server?.pid).toBe(String(process.pid)); + }); +}); + describe("detectHyperframesServer", () => { it("treats same-project servers with a different server build signature as mismatch", async () => { const projectDir = "/tmp/demo-project"; diff --git a/packages/cli/src/server/portUtils.ts b/packages/cli/src/server/portUtils.ts index ee9cb6f4a9..42d4f0cae0 100644 --- a/packages/cli/src/server/portUtils.ts +++ b/packages/cli/src/server/portUtils.ts @@ -15,7 +15,6 @@ import http from "node:http"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { resolve } from "node:path"; -import { c } from "../ui/colors.js"; import type { BrowserGpuMode } from "../browser/gpuPolicy.js"; const execFileAsync = promisify(execFile); @@ -198,8 +197,13 @@ export function detectHyperframesServer( * Get the PID of the process listening on a port (macOS/Linux only). * Returns null on Windows or if detection fails. */ +/** + * The PID the OS says is listening on `port`, or null when it cannot be + * determined. This is the only trustworthy answer: a config response is + * whatever the process on the other end chose to say. + */ async function getProcessOnPort(port: number): Promise { - if (process.platform === "win32") return null; + if (process.platform === "win32") return windowsListenerPid(port); try { const { stdout } = await execFileAsync("lsof", [`-ti:${port}`, "-sTCP:LISTEN"], { timeout: 2000, @@ -211,6 +215,23 @@ async function getProcessOnPort(port: number): Promise { } } +async function windowsListenerPid(port: number): Promise { + try { + const { stdout } = await execFileAsync("netstat", ["-ano", "-p", "tcp"], { timeout: 4000 }); + for (const line of stdout.split(/\r?\n/)) { + const columns = line.trim().split(/\s+/); + if (columns.length < 5 || columns[3] !== "LISTENING") continue; + const local = columns[1] ?? ""; + if (local.slice(local.lastIndexOf(":") + 1) !== String(port)) continue; + const pid = columns[4] ?? ""; + return /^\d+$/.test(pid) && pid !== "0" ? pid : null; + } + return null; + } catch { + return null; + } +} + // ── Server discovery ─────────────────────────────────────────────────────── export interface ActiveServer { @@ -285,24 +306,7 @@ export async function scanActiveServers(startPort = 3002): Promise batchStart + i); - const results = await Promise.all( - ports.map(async (port) => { - const config = await probePort(port); - if (!config) return null; - const pid = - Number.isInteger(config.pid) && Number(config.pid) > 0 - ? String(config.pid) - : await getProcessOnPort(port); - return { - port, - projectName: config.projectName, - projectDir: config.projectDir, - version: config.version, - pid, - browserGpuMode: config.browserGpuMode, - }; - }), - ); + const results = await Promise.all(ports.map((port) => activeServerOnPort(port))); for (const r of results) { if (r) servers.push(r); @@ -312,6 +316,33 @@ export async function scanActiveServers(startPort = 3002): Promise { + const config = await probePort(port); + if (!config) return null; + const listenerPid = await getProcessOnPort(port); + const pid = + listenerPid ?? + (Number.isInteger(config.pid) && Number(config.pid) > 0 ? String(config.pid) : null); + return { + port, + projectName: config.projectName, + projectDir: config.projectDir, + version: config.version, + pid, + browserGpuMode: config.browserGpuMode, + }; +} + /** * Kill all active HyperFrames preview servers by sending SIGTERM to their PIDs. * Returns the number of servers killed. @@ -416,17 +447,9 @@ export async function findPortAndServe( return { type: "already-running", port }; } if (detection.type === "mismatch") { - console.log( - ` ${c.dim(`Port ${port} in use by HyperFrames project "${detection.projectName}" — skipping`)}`, - ); continue; } } - - const pid = await getProcessOnPort(port); - if (pid) { - console.log(` ${c.dim(`Port ${port} in use by PID ${pid} — skipping`)}`); - } } throw new Error( diff --git a/packages/cli/src/utils/orphanCleanup.test.ts b/packages/cli/src/utils/orphanCleanup.test.ts index b3dc2afdf2..aebe4d0b6c 100644 --- a/packages/cli/src/utils/orphanCleanup.test.ts +++ b/packages/cli/src/utils/orphanCleanup.test.ts @@ -1,13 +1,52 @@ import { describe, it, expect } from "vitest"; import { spawn } from "node:child_process"; -import { killProcessTree, killOrphanedProcesses } from "./orphanCleanup.js"; +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", () => { + const first = processIdentity(process.pid); + expect(first).toMatch(/^(?:linux|posix|windows):/); + expect(processIdentity(process.pid)).toBe(first); + expect(processIdentity(-1)).toBeNull(); + }); + + 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" }); + const parent = spawn("bash", ["-c", "sleep 60 & sleep 60 & wait"], { + stdio: "ignore", + }); // Let children spawn await new Promise((r) => setTimeout(r, 200)); @@ -27,7 +66,9 @@ describe.skipIf(!IS_UNIX)("killProcessTree", () => { 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" }); + const proc = spawn("bash", ["-c", "trap '' TERM; sleep 60"], { + stdio: "ignore", + }); await new Promise((r) => setTimeout(r, 100)); const exitPromise = new Promise((resolve) => proc.on("close", resolve)); diff --git a/packages/cli/src/utils/orphanCleanup.ts b/packages/cli/src/utils/orphanCleanup.ts index a34f01775c..ae630c0151 100644 --- a/packages/cli/src/utils/orphanCleanup.ts +++ b/packages/cli/src/utils/orphanCleanup.ts @@ -1,4 +1,5 @@ -import { execSync } from "node:child_process"; +import { execFileSync, execSync } from "node:child_process"; +import { readFileSync } from "node:fs"; /** * Find and kill orphaned Chrome processes from previous crashed sessions. @@ -34,11 +35,20 @@ export function killOrphanedProcesses(): number { * depth-first so children are killed before parents, preventing * re-adoption races. * - * No-op on Windows — process groups are managed differently and - * the pgrep/ps utilities are not available. + * Windows uses taskkill's tree mode because pgrep/ps are unavailable there. */ export function killProcessTree(pid: number, signal: NodeJS.Signals = "SIGTERM"): void { - if (process.platform === "win32") return; + if (process.platform === "win32") { + try { + execFileSync("taskkill", windowsProcessTreeKillArgs(pid), { + stdio: "ignore", + timeout: 5000, + }); + } catch { + // Process already exited or taskkill could not inspect it. + } + return; + } const descendants = getDescendants(pid); const allPids = [...descendants.reverse(), pid]; @@ -65,10 +75,111 @@ export function killProcessTree(pid: number, signal: NodeJS.Signals = "SIGTERM") } } +export function windowsProcessTreeKillArgs(pid: number): string[] { + return ["/PID", String(pid), "/T", "/F"]; +} + +/** + * Return a process birth token suitable for detecting PID reuse. The token is + * diagnostic state only: callers must still prove the live server is a + * descendant before treating a saved wrapper as the owned process-tree root. + */ +export function processIdentity(pid: number): string | null { + if (!Number.isInteger(pid) || pid <= 0) return null; + try { + if (process.platform === "win32") { + const created = execFileSync( + "powershell.exe", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + `(Get-CimInstance Win32_Process -Filter 'ProcessId = ${pid}').CreationDate.ToFileTimeUtc()`, + ], + { encoding: "utf8", timeout: 2000 }, + ).trim(); + return created ? `windows:${created}` : null; + } + + if (process.platform === "linux") { + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + const fields = stat + .slice(stat.lastIndexOf(") ") + 2) + .trim() + .split(/\s+/); + const startTicks = fields[19]; // field 22 overall; fields starts at process state (3) + return startTicks ? `linux:${startTicks}` : null; + } + + const started = execFileSync("ps", ["-o", "lstart=", "-p", String(pid)], { + encoding: "utf8", + timeout: 2000, + }).trim(); + return started ? `posix:${started}` : null; + } catch { + return null; + } +} + +type ParentPidLookup = (pid: number) => number | null; + +function processParentPid(pid: number): number | null { + try { + const output = + process.platform === "win32" + ? execFileSync( + "powershell.exe", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + `(Get-CimInstance Win32_Process -Filter 'ProcessId = ${pid}').ParentProcessId`, + ], + { encoding: "utf8", timeout: 2000 }, + ) + : execFileSync("ps", ["-o", "ppid=", "-p", String(pid)], { + encoding: "utf8", + timeout: 2000, + }); + const parentPid = Number(output.trim()); + return Number.isInteger(parentPid) && parentPid > 0 ? parentPid : null; + } catch { + return null; + } +} + +/** + * Prove that `childPid` currently belongs to the process tree rooted at + * `ancestorPid`. The walk fails closed on missing, invalid, or cyclic process + * metadata so a stale saved PID can never authorize terminating a new process. + */ +export function isProcessDescendant( + childPid: number, + ancestorPid: number, + parentPid: ParentPidLookup = processParentPid, +): boolean { + if (childPid <= 0 || ancestorPid <= 0 || childPid === ancestorPid) return false; + + const visited = new Set(); + let current = childPid; + for (let depth = 0; depth < 64; depth++) { + if (visited.has(current)) return false; + visited.add(current); + const parent = parentPid(current); + if (parent === ancestorPid) return true; + if (parent === null || parent <= 1) return false; + current = parent; + } + return false; +} + function getDescendants(pid: number): number[] { let children: number[]; try { - const raw = execSync(`pgrep -P ${pid}`, { encoding: "utf-8", timeout: 2000 }).trim(); + const raw = execSync(`pgrep -P ${pid}`, { + encoding: "utf-8", + timeout: 2000, + }).trim(); if (!raw) return []; children = raw .split("\n") From 0bbdf8a84cb252d6396e62d2839854c33c4cafea Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 18 Aug 2026 01:21:50 -0400 Subject: [PATCH 2/2] fix(cli): fail closed when the OS cannot confirm who owns a port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. The two halves of this change picked opposite directions for the same condition. `isProcessDescendant` fails closed by design; `activeServerOnPort` fell back to the self-reported PID whenever the OS lookup came back empty — and that is not only "unsupported platform". `lsof` may be absent (the default on many slim images), may time out, or may not see a socket owned by another user. On such a machine every scanned port silently reverted to pre-change behaviour, with nothing said. Provenance is now part of the type rather than a convention: `ActiveServer` carries `pidSource`, so a caller cannot mistake a self-report for the kernel's answer. `--kill-all` requires `"os"` and skips the rest, naming the ports it left alone and why. That is the deliberate trade — a blind sweep of a port range has no evidence beyond an unauthenticated response, so an unconfirmed PID must not be signalled. Managed previews are unaffected: they stop through their session record, which proves ownership by process birth identity. The fallback branch — the one with the security consequence — now has the coverage it lacked, via an injected lookup matching the seam `testPortOnAllHosts` and `isProcessDescendant` already use, including a live process that survives because nothing confirmed it owns the socket. Also state that `killProcessTree` honours `signal` on POSIX only: Windows always passes `/F`, deliberately, since taskkill without it posts WM_CLOSE that a console process may ignore. The caller-side comment claiming Windows cleanup is a no-op described the code before this change and now says the opposite. --- packages/cli/src/commands/preview.ts | 17 +++-- packages/cli/src/server/portUtils.test.ts | 46 +++++++++++++ packages/cli/src/server/portUtils.ts | 79 ++++++++++++++++++----- packages/cli/src/utils/orphanCleanup.ts | 7 ++ 4 files changed, 128 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 85f602a51f..826c8af05e 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -262,8 +262,17 @@ export default defineCommand({ console.log("\n No active preview servers to kill.\n"); return; } - const killed = await killActiveServers(startPort); - console.log(`\n Killed ${killed} preview server${killed === 1 ? "" : "s"}.\n`); + 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(); return; } @@ -972,8 +981,8 @@ async function runDevMode(dir: string, options?: StudioLaunchOptions): Promise` only targets this process — the child tree (Vite + Chrome) // would survive without explicit cleanup. - // On Windows, killProcessTree is a no-op (pgrep/ps unavailable); Ctrl+C - // propagates via the console process group instead. + // On Windows, killProcessTree delegates to taskkill's tree mode, which force + // kills the whole tree immediately — no grace period, unlike the POSIX path. registerChildTreeShutdown(child); return waitForChildClose(child); } diff --git a/packages/cli/src/server/portUtils.test.ts b/packages/cli/src/server/portUtils.test.ts index bad74030d2..e21a9f6a2d 100644 --- a/packages/cli/src/server/portUtils.test.ts +++ b/packages/cli/src/server/portUtils.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createServer, type Server } from "node:net"; +import { spawn } from "node:child_process"; import { resolve } from "node:path"; import { createServer as createHttpServer, type Server as HttpServer } from "node:http"; import { @@ -163,6 +164,50 @@ describe("findPortAndServe — bind host (security: F-001)", () => { }); describe("activeServerOnPort — PID provenance (security)", () => { + it("does not signal a server whose PID the OS could not confirm", async () => { + // Fail closed: the only evidence for a blind port-range sweep is an + // unauthenticated config response, so an unconfirmed PID is skipped and + // reported rather than signalled. + const alive = spawn(process.execPath, ["-e", "setTimeout(() => {}, 30000)"], { + stdio: "ignore", + }); + const port = await startConfigProbeServer({ + isHyperframes: true, + projectName: "demo-project", + projectDir: "/tmp/demo-project", + serverBuildSignature: null, + version: "0.6.42", + pid: alive.pid, + }); + + const server = await activeServerOnPort(port, async () => null); + expect(server?.pidSource).toBe("self-reported"); + + // The victim survives, because nothing verified it owns the socket. + expect(alive.killed).toBe(false); + alive.kill(); + }); + + it("tags a self-reported PID and refuses to signal it", async () => { + // The branch with the security consequence. `getProcessOnPort` returns null + // for more than "unsupported platform" — lsof absent, timed out, or unable + // to see another user's socket — and on those machines every scanned port + // used to fall back to the self-report with nothing said. + const port = await startConfigProbeServer({ + isHyperframes: true, + projectName: "demo-project", + projectDir: "/tmp/demo-project", + serverBuildSignature: null, + version: "0.6.42", + pid: 999_999, + }); + + const server = await activeServerOnPort(port, async () => null); + + expect(server?.pid).toBe("999999"); + expect(server?.pidSource).toBe("self-reported"); + }); + it("reports the PID that owns the socket, not the one the response claims", async () => { // `/__hyperframes_config` is unauthenticated and `--stop` / `--kill-all` // send signals to this field. Trusting the response let any local process @@ -180,6 +225,7 @@ describe("activeServerOnPort — PID provenance (security)", () => { const server = await activeServerOnPort(port); expect(server?.pid).toBe(String(process.pid)); + expect(server?.pidSource).toBe("os"); }); }); diff --git a/packages/cli/src/server/portUtils.ts b/packages/cli/src/server/portUtils.ts index 42d4f0cae0..44de5a60c4 100644 --- a/packages/cli/src/server/portUtils.ts +++ b/packages/cli/src/server/portUtils.ts @@ -247,6 +247,13 @@ export interface ActiveServer { projectDir: string; version: string; pid: string | null; + /** + * Where `pid` came from. `"os"` is the kernel's answer for who holds the + * listening socket; `"self-reported"` is whatever the process on the other + * end chose to put in its config response. Callers that SIGNAL the pid must + * require `"os"` — see `killActiveServers`. + */ + pidSource?: "os" | "self-reported"; browserGpuMode?: BrowserGpuMode; } @@ -326,43 +333,81 @@ export async function scanActiveServers(startPort = 3002): Promise { +export async function activeServerOnPort( + port: number, + listenerLookup: (port: number) => Promise = getProcessOnPort, +): Promise { const config = await probePort(port); if (!config) return null; - const listenerPid = await getProcessOnPort(port); - const pid = - listenerPid ?? - (Number.isInteger(config.pid) && Number(config.pid) > 0 ? String(config.pid) : null); + const listenerPid = await listenerLookup(port); + if (listenerPid) return { ...identityFrom(config, port), pid: listenerPid, pidSource: "os" }; + + // The OS lookup came back empty. That is NOT only "unsupported platform": + // `lsof` may be absent (common on slim images), may time out, or may not see + // a socket owned by another user. The value is still reported, because + // `--list` and the ownership record both have honest uses for it, but it is + // tagged so the paths that send signals can refuse it. + const selfReported = + Number.isInteger(config.pid) && Number(config.pid) > 0 ? String(config.pid) : null; + return { + ...identityFrom(config, port), + pid: selfReported, + ...(selfReported ? { pidSource: "self-reported" as const } : {}), + }; +} + +function identityFrom( + config: HyperframesConfigResponse, + port: number, +): Omit { return { port, projectName: config.projectName, projectDir: config.projectDir, version: config.version, - pid, browserGpuMode: config.browserGpuMode, }; } /** - * Kill all active HyperFrames preview servers by sending SIGTERM to their PIDs. - * Returns the number of servers killed. + * SIGTERM every active HyperFrames preview server whose PID the OS confirmed. + * + * This is a blind sweep of a port range: the only evidence that a given process + * should be killed is that it answered `/__hyperframes_config`, which is + * unauthenticated. So the decision here is deliberately FAIL CLOSED — a PID the + * OS could not confirm is skipped rather than signalled, because the alternative + * is letting any local process nominate a victim. + * + * The cost is real and bounded: where `lsof` is missing, `--kill-all` stops + * reaping unmanaged servers. Managed previews are unaffected — they stop through + * their session record, which proves ownership by process birth identity rather + * than by asking the port who it is. + * + * Skipped ports are returned so the caller can say so; a security control that + * degrades silently is one nobody knows to fix. */ -export async function killActiveServers(startPort = 3002): Promise { +export async function killActiveServers( + startPort = 3002, +): Promise<{ killed: number; unverified: number[] }> { const servers = await scanActiveServers(startPort); let killed = 0; + const unverified: number[] = []; for (const server of servers) { - if (server.pid) { - try { - process.kill(parseInt(server.pid, 10), "SIGTERM"); - killed++; - } catch { - // Process may have already exited - } + if (!server.pid) continue; + if (server.pidSource !== "os") { + unverified.push(server.port); + continue; + } + try { + process.kill(parseInt(server.pid, 10), "SIGTERM"); + killed++; + } catch { + // Process may have already exited } } - return killed; + return { killed, unverified }; } // ── Smart port selection ─────────────────────────────────────────────────── diff --git a/packages/cli/src/utils/orphanCleanup.ts b/packages/cli/src/utils/orphanCleanup.ts index ae630c0151..146dd75eec 100644 --- a/packages/cli/src/utils/orphanCleanup.ts +++ b/packages/cli/src/utils/orphanCleanup.ts @@ -36,6 +36,13 @@ export function killOrphanedProcesses(): number { * re-adoption races. * * Windows uses taskkill's tree mode because pgrep/ps are unavailable there. + * + * `signal` is honoured on POSIX only. The Windows path always passes `/F`, so a + * caller asking for SIGTERM gets a forced tree kill with no grace period, while + * the same call on POSIX gets 500 ms to flush and exit. That is deliberate — + * `taskkill` without `/F` posts WM_CLOSE, which a console process is free to + * ignore, and leaving a preview server alive is the worse failure here. Do not + * pass SIGTERM expecting a clean shutdown on Windows. */ export function killProcessTree(pid: number, signal: NodeJS.Signals = "SIGTERM"): void { if (process.platform === "win32") {