fix(broker-lifecycle): reject crashed brokers via PID-alive probe

isBrokerEndpointReady() only pings the socket for 150ms. If the broker
process has crashed but its socket file lingers (unix domain) or the
listener drops without the probe noticing, the existing session is
trusted and reused — but every downstream task disconnects mid-turn
because the transport subsystem behind the socket is actually gone.

Add isPidAlive() check consulted before trusting the socket ping. If
the PID is dead, tear down and respawn.

Safety around the teardown's SIGTERM:
- verifyBrokerPid() cross-checks session.pid against the on-disk
  pid-file contents AND the live process command line via `ps`
  (POSIX) before returning true.
- Windows intentionally returns false — tasklist exposes image name
  but not command line, and matching node.exe alone is too weak to
  rule out recycled-PID foreign processes. Windows rotation still
  cleans socket/pidfile; detached old broker eventually exits on its
  own since no new client reaches it.
- If verifyBrokerPid() returns false (e.g. stale pid-file, PID gone,
  ps lookup fails), killProcess falls back to null — no signal, only
  file cleanup, same as trunk behavior.

Age-based rotation for healthy-but-degrading brokers was considered
and dropped in this revision: rotating a still-serving broker can
interrupt a concurrent client's in-flight turn. A proper fix needs
an active health probe (e.g. lightweight RPC round-trip) or graceful
drain. Out of scope for this PR; filed as a follow-up.

Author: Анатолий Иванов

plugins/codex/scripts/lib/broker-lifecycle.mjs

@@ -3,7 +3,7 @@ import net from "node:net";
 import os from "node:os";
 import path from "node:path";
 import process from "node:process";
-import { spawn } from "node:child_process";
+import { spawn, execFileSync } from "node:child_process";
 import { fileURLToPath } from "node:url";
 import { createBrokerEndpoint, parseBrokerEndpoint } from "./broker-endpoint.mjs";
 import { resolveStateDir } from "./state.mjs";
@@ -110,20 +110,109 @@ async function isBrokerEndpointReady(endpoint) {
   }
 }
 
+// Observed failure: the broker process has crashed but its endpoint socket
+// file lingers (unix domain) or listener drops without `waitForBrokerEndpoint`
+// noticing in the 150ms probe window. `isBrokerEndpointReady` passes, the
+// caller trusts the existing session, and every downstream task disconnects
+// mid-turn. Add a PID-alive probe so we catch this class up-front and force
+// a fresh broker before trusting the socket.
+//
+// Age-based rotation was considered to cover slow-degradation (broker alive
+// but serving unreliably). Dropped in this revision — rotating a healthy
+// broker while it may be mid-turn for a concurrent client can interrupt that
+// turn. Proper fix for slow degradation needs a real health probe (e.g.
+// lightweight RPC round-trip) or graceful drain, which are out of scope
+// for this PR. Left as a follow-up.
+
+function isPidAlive(pid) {
+  if (!Number.isFinite(pid) || pid <= 0) return false;
+  try {
+    // signal 0 only checks existence; no actual signal delivered
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function isSessionStale(session) {
+  if (!session) return true;
+  // PID check — covers crashed-broker case
+  if (session.pid != null && !isPidAlive(session.pid)) return true;
+  return false;
+}
+
+// Default kill for stale-rotation teardown. Without this, rotating a still-alive
+// broker only removes its socket/pid files — the detached process keeps running
+// and leaks host resources. SIGTERM is best-effort; ignore missing-process errors.
+function defaultKillProcess(pid) {
+  try {
+    process.kill(pid, "SIGTERM");
+  } catch {
+    // process already gone — fine
+  }
+}
+
+// Cross-check session.pid against the actual running process before signaling.
+//
+// pid-file alone is insufficient: app-server-broker.mjs only removes it in the
+// clean shutdown() path — if the broker crashed ungracefully the pidfile
+// lingers, and when the OS recycles that PID to an unrelated process the file
+// contents will spuriously match. On POSIX we also check `ps` command line to
+// confirm the broker script name is present. On Windows we intentionally do
+// not send SIGTERM: `tasklist` exposes image name but not full command line
+// via the public CLI, and matching on image-name (`node.exe`) alone is too
+// weak to rule out recycled-PID foreign processes. Windows rotation will
+// still clean up socket/pidfile — detached old broker eventually exits on
+// its own since no new client will reach it.
+function verifyBrokerPid(session) {
+  if (!session || !Number.isFinite(session.pid) || !session.pidFile) return false;
+  if (process.platform === "win32") return false;
+  try {
+    if (!fs.existsSync(session.pidFile)) return false;
+    const content = fs.readFileSync(session.pidFile, "utf8").trim();
+    if (Number(content) !== session.pid) return false;
+    // POSIX: ps exposes the full command, so match instance-specific args.
+    // Script name alone is too broad — a recycled PID belonging to a foreign
+    // broker instance (different workspace) would also contain
+    // "app-server-broker.mjs" and cause a cross-session kill. We also require
+    // the session's unique --pid-file and --endpoint paths to appear in the
+    // live command line. Those paths are per-session (see spawnBrokerProcess —
+    // both are derived from createBrokerSessionDir()), so a recycled PID on
+    // an unrelated broker cannot match.
+    const cmd = execFileSync("ps", ["-p", String(session.pid), "-o", "command="], {
+      encoding: "utf8",
+      timeout: 1000
+    });
+    if (!cmd.includes("app-server-broker.mjs")) return false;
+    if (!cmd.includes(`--pid-file ${session.pidFile}`)) return false;
+    if (session.endpoint && !cmd.includes(`--endpoint ${session.endpoint}`)) return false;
+    return true;
+  } catch {
+    // Any lookup failure (process gone, permission denied, timeout) → skip
+    // the kill. Safe default; socket/pidfile cleanup still proceeds.
+    return false;
+  }
+}
+
 export async function ensureBrokerSession(cwd, options = {}) {
   const existing = loadBrokerSession(cwd);
-  if (existing && (await isBrokerEndpointReady(existing.endpoint))) {
+  if (existing && !isSessionStale(existing) && (await isBrokerEndpointReady(existing.endpoint))) {
     return existing;
   }
 
   if (existing) {
+    // Only send SIGTERM when the pid-file on disk still maps to session.pid —
+    // otherwise the PID may have been recycled by the OS to an unrelated
+    // process and signaling it would kill something we don't own.
+    const killAllowed = verifyBrokerPid(existing);
     teardownBrokerSession({
       endpoint: existing.endpoint ?? null,
       pidFile: existing.pidFile ?? null,
       logFile: existing.logFile ?? null,
       sessionDir: existing.sessionDir ?? null,
       pid: existing.pid ?? null,
-      killProcess: options.killProcess ?? null
+      killProcess: options.killProcess ?? (killAllowed ? defaultKillProcess : null)
     });
     clearBrokerSession(cwd);
   }