fix(services): resolve and record a real pid when adopting a service (#8218)

ServiceSupervisor.start(), when probeBeforeSpawn detects an already-healthy
instance on the target port, "adopts" it (marks the service running without
spawning a duplicate that would die with EADDRINUSE). This adopt path calls
setToolStatus(tool, "running") with no pid argument at all, so this.pid stays
at its constructor default of null for the rest of that supervisor's life --
only the spawn path (a genuinely new child process) ever sets a real pid.

In production this "adopt" path is common, not an edge case: any embedded
sidecar service (cliproxy, 9router, bifrost, mux) whose child process
survives a `systemctl --user restart omniroute.service` gets adopted by the
new supervisor instance on the next start(), and its pid is lost from that
point on -- even though the service is genuinely healthy and running. The
observed symptom: a service shows state "running" but pid null, and
something downstream that keys liveness tracking off pid eventually treats
it as untrustworthy/stale despite nothing actually being wrong.

Fix: resolve the real pid of the process holding the port (via `lsof -ti
:<port>`, best-effort -- a lookup failure leaves pid null rather than
blocking adoption) and record it the same way the spawn path does.

An existing test asserted pid === null on adopt. That was accurate for the
old behavior but reflected a missing resolution, not a deliberate "adopted
services never get a pid" design choice -- updated its assertion to match
the corrected behavior and added a dedicated regression test proving the
resolved pid matches the real process actually bound to the port.

src/lib/services/ServiceSupervisor.ts

@@ -7,7 +7,7 @@ import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 import { getServiceRow, updateServiceField, setToolStatus } from "@/lib/db/versionManager";
 import { RingBuffer } from "./ringBuffer";
 import { HealthChecker } from "./healthCheck";
-import { decidePreSpawn, probeBeforeSpawn } from "./portProbe";
+import { decidePreSpawn, probeBeforeSpawn, resolvePortPid } from "./portProbe";
 import type { ServiceConfig, ServiceState, ServiceStatus, LogLine, HealthState } from "./types";
 
 const CRASH_FAST_THRESHOLD_MS = 5_000;
@@ -93,10 +93,17 @@ export class ServiceSupervisor extends EventEmitter {
         if (decision.action === "adopt") {
           // Something healthy already serves this port — treat it as running
           // rather than spawning a duplicate that would die with EADDRINUSE.
+          // We didn't spawn it, so there's no ChildProcess handle to read a
+          // pid from — resolve one from the OS instead. Best-effort: if
+          // resolution fails, pid stays null rather than blocking adoption,
+          // but downstream liveness checks that key off pid will only trust
+          // this instance once a real pid is on record.
+          const adoptedPid = await resolvePortPid(this.config.port);
           this.checker.start();
           this.startedAt = new Date().toISOString();
+          this.pid = adoptedPid;
           this.setState("running");
-          await setToolStatus(this.config.tool, "running");
+          await setToolStatus(this.config.tool, "running", adoptedPid ?? undefined);
           return this.getStatus();
         }
 

src/lib/services/portProbe.ts

@@ -14,6 +14,7 @@
  */
 
 import { createConnection } from "node:net";
+import { spawn } from "node:child_process";
 
 /** Result of probing the service before spawning. */
 export interface PreSpawnProbe {
@@ -25,12 +26,11 @@ export interface PreSpawnProbe {
 
 /** Outcome of the pre-spawn decision. */
 export type PreSpawnDecision =
-  | { action: "spawn" }
-  | { action: "adopt" }
-  | { action: "error"; message: string };
+  { action: "spawn" } | { action: "adopt" } | { action: "error"; message: string };
 
 const HEALTH_PROBE_TIMEOUT_MS = 3_000;
 const PORT_PROBE_TIMEOUT_MS = 1_000;
+const PID_RESOLVE_TIMEOUT_MS = 2_000;
 
 /**
  * Decide what to do before spawning, given a probe of the port + health.
@@ -102,3 +102,46 @@ export async function probeBeforeSpawn(healthUrl: string, port: number): Promise
   ]);
   return { healthy, portInUse };
 }
+
+/**
+ * Resolve the pid of whatever process is listening on `port`, if any.
+ *
+ * Used when adopting an already-healthy instance (see `decidePreSpawn`'s
+ * "adopt" outcome): the supervisor didn't spawn that process itself, so it
+ * has no pid from a `ChildProcess` handle, but tracking a real pid is still
+ * needed for downstream liveness checks to trust an adopted service the same
+ * way they trust a freshly-spawned one. Returns null if nothing is found or
+ * the lookup fails/times out (best-effort; never blocks adoption on this).
+ */
+export async function resolvePortPid(port: number): Promise<number | null> {
+  return new Promise((resolve) => {
+    const proc = spawn("lsof", ["-ti", `:${port}`]);
+    let output = "";
+    let settled = false;
+
+    const finish = (value: number | null) => {
+      if (settled) return;
+      settled = true;
+      clearTimeout(timeout);
+      resolve(value);
+    };
+
+    const timeout = setTimeout(() => {
+      proc.kill();
+      finish(null);
+    }, PID_RESOLVE_TIMEOUT_MS);
+
+    proc.stdout?.on("data", (chunk: Buffer) => {
+      output += chunk.toString("utf8");
+    });
+    proc.on("error", () => finish(null));
+    proc.on("close", () => {
+      const firstLine = output
+        .split("\n")
+        .map((line) => line.trim())
+        .find((line) => line.length > 0);
+      const parsed = firstLine ? Number.parseInt(firstLine, 10) : Number.NaN;
+      finish(Number.isFinite(parsed) ? parsed : null);
+    });
+  });
+}

tests/unit/services/ServiceSupervisor.test.ts

@@ -220,7 +220,12 @@ test("#6205: probeBeforeSpawn adopts a healthy existing instance (no spawn)", as
   try {
     const status = await sup.start();
     assert.equal(status.state, "running", "adopted instance is marked running");
-    assert.equal(status.pid, null, "no child process is spawned when adopting");
+    // No child process handle exists on adoption (nothing was spawned), but a
+    // real pid is still resolved from the OS — see the dedicated pid-tracking
+    // test below. Historically this asserted pid === null, which just
+    // reflected the missing resolution rather than a deliberate "no pid for
+    // adopted services" design choice.
+    assert.ok(status.pid !== null, "adopted instance still gets a tracked pid");
     // No child means no captured stdout ticks.
     await new Promise((r) => setTimeout(r, 300));
     assert.equal(sup.getRingBuffer().snapshot().length, 0, "no logs — nothing was spawned");
@@ -229,3 +234,31 @@ test("#6205: probeBeforeSpawn adopts a healthy existing instance (no spawn)", as
     healthServer.close();
   }
 });
+
+// Regression test: the adopt branch used to call setToolStatus(tool, "running")
+// with no pid argument at all, leaving `pid` permanently null for any service
+// adopted on a supervisor restart (common in production — e.g. after
+// `systemctl --user restart omniroute.service`, sidecar child processes can
+// outlive the restart and get adopted rather than spawned fresh). Downstream
+// consumers that key liveness tracking off pid would then treat a genuinely
+// healthy, running service as untrustworthy/stale. This asserts the resolved
+// pid on adoption matches the real process actually holding the port.
+test("adopted service resolves and records the real pid of the process holding the port", async () => {
+  const healthServer = startHealthServer(29996);
+  const cfg = { ...tickConfig("test-adopt", 29996), probeBeforeSpawn: true };
+  const sup = new ServiceSupervisor(cfg);
+
+  try {
+    const status = await sup.start();
+    assert.equal(status.state, "running");
+    // The health server above runs inline in this test process (no child
+    // process spawned for it), so the pid actually bound to port 29996 is
+    // this test process's own pid — that's exactly what resolvePortPid()
+    // should find and what the supervisor should record.
+    assert.equal(status.pid, process.pid, "resolved pid matches the process holding the port");
+    assert.equal(sup.getStatus().pid, process.pid, "in-memory supervisor state also has the pid");
+  } finally {
+    await sup.stop();
+    healthServer.close();
+  }
+});