From 0bba2cbab8ed4200ab96db73daf0c8f27abbbd46 Mon Sep 17 00:00:00 2001 From: phantomptr Date: Mon, 28 Sep 2026 12:23:54 -0700 Subject: [PATCH] fix(scripts): make run-client stops only the engine on the app's port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kill-stale-client did `pkill -f ps5upload-engine`, which SIGTERMed every engine on the machine — a standalone one on another port died 70 minutes into a package build. It now stops only an engine listening on 19113 (an engine whose app is gone exits by itself). The decision is a small pure module with node:test cases, which scripts:check now runs. Co-Authored-By: Claude Opus 5.5 --- scripts/check-scripts.mjs | 3 +++ scripts/kill-stale-client.mjs | 33 ++++++++++++++++++++++++++----- scripts/stale-client-lib.mjs | 21 ++++++++++++++++++++ scripts/stale-client-lib.test.mjs | 21 ++++++++++++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 scripts/stale-client-lib.mjs create mode 100644 scripts/stale-client-lib.test.mjs diff --git a/scripts/check-scripts.mjs b/scripts/check-scripts.mjs index 9a1d6a46..60345704 100755 --- a/scripts/check-scripts.mjs +++ b/scripts/check-scripts.mjs @@ -149,6 +149,9 @@ const psFiles = files.filter((f) => /\.ps1$/i.test(f)); let ok = true; ok = checkSdkPin() && ok; for (const f of nodeFiles) ok = run(path.relative(repoRoot, f), "node", ["--check", f]) && ok; +// Scripts with logic worth pinning carry a node:test file beside them. +const nodeTests = nodeFiles.filter((f) => f.endsWith(".test.mjs")); +if (nodeTests.length) ok = run("node --test", "node", ["--test", ...nodeTests]) && ok; if (hasCommand("bash")) { for (const f of shellFiles) ok = run(path.relative(repoRoot, f), "bash", ["-n", f]) && ok; } else if (shellFiles.length > 0) { diff --git a/scripts/kill-stale-client.mjs b/scripts/kill-stale-client.mjs index f2b77a1c..b2cc89e0 100644 --- a/scripts/kill-stale-client.mjs +++ b/scripts/kill-stale-client.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node -// Best-effort cleanup of stale ps5upload-desktop / ps5upload-engine processes -// plus any Vite dev server holding :1420. Invoked as a `make run-client` +// Best-effort cleanup of a stale ps5upload-desktop, an engine still holding the +// app's port (19113), and a Vite dev server holding :1420. Engines elsewhere are +// left alone (see stale-client-lib.mjs). Invoked as a `make run-client` // dependency so a previously mis-terminated session doesn't fail the next // launch with "Port 1420 already in use" or two engines fighting for ports. // @@ -15,6 +16,8 @@ import { execSync } from 'node:child_process'; +import { APP_ENGINE_PORT, staleEnginePids } from './stale-client-lib.mjs'; + const isWin = process.platform === 'win32'; const PORT = 1420; @@ -26,12 +29,12 @@ function silent(cmd) { try { execSync(cmd, { stdio: 'ignore' }); } catch { /* expected: no match */ } } +// The dev app itself. Its engine exits with it (the engine's parent watch); an +// engine is NOT killed by name here — see stale-client-lib.mjs. if (isWin) { silent('taskkill /IM ps5upload-desktop.exe /F'); - silent('taskkill /IM ps5upload-engine.exe /F'); } else { silent("pkill -f '[p]s5upload-desktop'"); - silent("pkill -f '[p]s5upload-engine'"); } function pidsOnPort(port) { @@ -46,7 +49,7 @@ function pidsOnPort(port) { } return [...pids]; } - const out = run(`lsof -ti :${port}`); + const out = run(`lsof -ti tcp:${port} -sTCP:LISTEN`); return out.trim().split(/\s+/).filter(Boolean); } catch { return []; @@ -68,6 +71,26 @@ function looksLikeOurVite(pid) { } } +function commandOf(pid) { + try { + if (isWin) { + return run( + `powershell -NoProfile -Command "(Get-CimInstance Win32_Process -Filter 'ProcessId=${pid}').CommandLine"`, + ).trim(); + } + return run(`ps -o command= -p ${pid}`).trim(); + } catch { + return ''; + } +} + +// Only an engine still holding the app's port (an orphan the app can't bind past). +const listeners = pidsOnPort(APP_ENGINE_PORT).map((pid) => ({ pid, command: commandOf(pid) })); +for (const pid of staleEnginePids(listeners)) { + silent(isWin ? `taskkill /PID ${pid} /F` : `kill ${pid}`); + console.log(`✓ stopped the stale engine on :${APP_ENGINE_PORT} (pid ${pid})`); +} + for (const pid of pidsOnPort(PORT)) { if (looksLikeOurVite(pid)) { silent(isWin ? `taskkill /PID ${pid} /F` : `kill -9 ${pid}`); diff --git a/scripts/stale-client-lib.mjs b/scripts/stale-client-lib.mjs new file mode 100644 index 00000000..d6c3a6a4 --- /dev/null +++ b/scripts/stale-client-lib.mjs @@ -0,0 +1,21 @@ +// What `make run-client` may clean up before starting the dev app (see kill-stale-client.mjs). +// +// Only an engine holding the desktop app's own port is stale. A blanket +// `pkill -f ps5upload-engine` also killed engines that had nothing to do with the +// dev app — a standalone one on another port running a 70-minute conversion +// died mid-build. An engine whose app is gone exits by itself (its parent watch), +// so the port holder is the only one that can be in the way. + +/** The engine's port the desktop app binds first. */ +export const APP_ENGINE_PORT = 19113; + +/** The executable is the engine itself, not a command that merely mentions it. */ +function isEngine(command) { + const exe = command.trim().split(/\s+(?=-)/)[0]; + return /(^|[\\/])ps5upload-engine(\.exe)?$/i.test(exe); +} + +/** Of the processes listening on the app's port, the pids that are a ps5upload engine. */ +export function staleEnginePids(listeners) { + return listeners.filter((l) => isEngine(l.command)).map((l) => l.pid); +} diff --git a/scripts/stale-client-lib.test.mjs b/scripts/stale-client-lib.test.mjs new file mode 100644 index 00000000..3ddd5dfa --- /dev/null +++ b/scripts/stale-client-lib.test.mjs @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { staleEnginePids } from "./stale-client-lib.mjs"; + +test("only the engine holding the app's port is stale", () => { + const listeners = [ + { pid: 10, command: "/Users/me/Library/Application Support/com.phantomptr.ps5upload/engine/ps5upload-engine" }, + { pid: 11, command: "/usr/bin/python3 -m http.server 19113" }, + ]; + assert.deepEqual(staleEnginePids(listeners), [10]); +}); + +test("an engine elsewhere — a conversion on another port — is never in the list to kill", () => { + // The caller passes only what listens on the app's port; nothing else can come back. + assert.deepEqual(staleEnginePids([]), []); +}); + +test("a name that merely contains the words is not the engine", () => { + assert.deepEqual(staleEnginePids([{ pid: 12, command: "vim notes-about-ps5upload-engine.txt" }]), []); +});