From 9d71c04d1a0f1343351e6094d2f9855cf3932ff4 Mon Sep 17 00:00:00 2001
From: Paul Logan <pauljflogan@gmail.com>
Date: Sun, 30 Aug 2026 15:25:16 -0700
Subject: [PATCH] fix(extension): auto-start must not kill a running server
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`ensurePiWebRunning()` runs on every extension load — i.e. once per pi
process, including the `pi --mode rpc` workers that pi-web itself spawns.
When its health check failed, it called `startPiWeb()`, which ran
`launchctl kickstart -k`. The `-k` flag stops the running instance first,
and pi-web handles SIGTERM by exiting 0, so launchd recorded a *successful*
exit and the kill was invisible in `last exit code`.

That single kill briefly takes the port down, so every other pi process
starting in that window fails its own check and issues its own kickstart.
Each restart manufactures more failures: a self-sustained restart storm
(observed here at ~6 exits/min, `runs = 582`, with 502s from
`tailscale serve` making the phone PWA unreachable).

- `startPiWeb()` no longer kills: if something is already bound to the
  port it is not "not running", so exit without touching the job;
  otherwise plain `launchctl start`.
- `ensurePiWebRunning()` confirms with a second check before acting,
  because one fetch is not proof that a server other processes are using
  is dead.
- Health-check timeout 1s -> 4s: a healthy server answers in ~3ms, but
  401/403 already count as up, so the only way to fail is a timeout —
  exactly the transient state a loaded machine produces.

`/pi-web restart` keeps `kickstart -k`: an explicit user request to
restart should still kill.

Refs #112
---
 .pi/extensions/pi-web.ts | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/.pi/extensions/pi-web.ts b/.pi/extensions/pi-web.ts
index 076f661e..efa694e6 100644
--- a/.pi/extensions/pi-web.ts
+++ b/.pi/extensions/pi-web.ts
@@ -203,7 +203,9 @@ export function openBrowser(pi: ExtensionAPI, url: string): Promise<void> {
 async function healthCheck(host: string, port: string): Promise<boolean> {
   try {
     const res = await fetch(`http://${host}:${port}`, {
-      signal: AbortSignal.timeout(1000),
+      // 1s was tight enough that a busy machine reported a healthy server as
+      // down. A false negative here is expensive: it triggers service restarts.
+      signal: AbortSignal.timeout(4000),
     });
     // 401/403 means pi-web is running with auth enabled.
     return res.ok || res.status === 401 || res.status === 403;
@@ -218,7 +220,17 @@ function windowsLauncher(): string {
   return join(homedir(), ".config", "pi-web", "pi-web-start.vbs");
 }
 
-async function startPiWeb(pi: ExtensionAPI): Promise<void> {
+// startPiWeb must only ever *start* the service. It used to run
+// `launchctl kickstart -k`, which SIGTERMs a running instance (pi-web exits 0
+// on SIGTERM). This function is reached from the opportunistic load-time check
+// that every pi process runs, so one transient health-check failure killed a
+// healthy server, which then made every other concurrent check fail and kickstart
+// too -- a self-sustaining restart storm. Killing is left to /pi-web restart.
+async function startPiWeb(
+  pi: ExtensionAPI,
+  host = "127.0.0.1",
+  port = "31415",
+): Promise<void> {
   if (process.platform === "win32") {
     const launcher = windowsLauncher();
     if (!existsSync(launcher)) {
@@ -233,7 +245,11 @@ async function startPiWeb(pi: ExtensionAPI): Promise<void> {
   if (process.platform === "darwin") {
     await pi.exec("sh", [
       "-lc",
-      `plist="$HOME/Library/LaunchAgents/com.pi-web.plist"; if [ ! -f "$plist" ]; then exit 127; fi; launchctl bootstrap "gui/$(id -u)" "$plist" 2>/dev/null || launchctl load "$plist" 2>/dev/null || true; launchctl kickstart -k "gui/$(id -u)/com.pi-web" 2>/dev/null || launchctl start com.pi-web`,
+      `plist="$HOME/Library/LaunchAgents/com.pi-web.plist"; if [ ! -f "$plist" ]; then exit 127; fi; ` +
+        `launchctl bootstrap "gui/$(id -u)" "$plist" 2>/dev/null || launchctl load "$plist" 2>/dev/null || true; ` +
+        // Something already bound to the port is not "not running"; leave it alone.
+        `if nc -z '${host}' '${port}' 2>/dev/null; then exit 0; fi; ` +
+        `launchctl start com.pi-web 2>/dev/null || true`,
     ]);
     return;
   }
@@ -318,8 +334,13 @@ async function ensurePiWebRunning(
 ): Promise<boolean> {
   if (await healthCheck(host, port)) return true;
 
+  // Confirm before touching a service other processes may be using: concurrent
+  // pi starts amplify a single slow response into restarts of a healthy server.
+  await new Promise((resolve) => setTimeout(resolve, 1500));
+  if (await healthCheck(host, port)) return true;
+
   try {
-    await startPiWeb(pi);
+    await startPiWeb(pi, host, port);
   } catch {
     return false;
   }
@@ -825,7 +846,7 @@ export default function (pi: ExtensionAPI) {
           return;
         }
         try {
-          await startPiWeb(pi);
+          await startPiWeb(pi, host, port);
           let started = false;
           for (let i = 0; i < 10; i++) {
             await new Promise((resolve) => setTimeout(resolve, 300));