fix(router): verify shutdown convergence

src/lib/onboard/model-router-process.test.ts

@@ -3,7 +3,9 @@
 
 import { describe, expect, it } from "vitest";
 
-import { findModelRouterPidForPort } from "./model-router-process";
+import { findModelRouterPidForPort, stopModelRouterProcess } from "./model-router-process";
+
+const ROUTER_ARGS = ["/opt/model-router", "proxy", "--port", "4000"];
 
 describe("findModelRouterPidForPort", () => {
   it("returns the PID when a model-router proxy is found via direct proc scan (#5169)", () => {
@@ -65,3 +67,124 @@ describe("findModelRouterPidForPort", () => {
     expect(pid).toBe(100);
   });
 });
+
+describe("stopModelRouterProcess", () => {
+  it("returns only after the owned process and health endpoint stop", async () => {
+    let running = true;
+    let healthy = true;
+    const signals: NodeJS.Signals[] = [];
+
+    await stopModelRouterProcess(123, 4000, {
+      isRunning: () => running,
+      readCommandLine: () => ROUTER_ARGS,
+      isHealthy: async () => healthy,
+      kill: (_pid, signal) => {
+        signals.push(signal);
+        running = false;
+        healthy = false;
+      },
+      sleep: async () => {},
+    });
+
+    expect(signals).toEqual(["SIGTERM"]);
+  });
+
+  it("refuses to signal a PID that no longer belongs to the router", async () => {
+    const signals: NodeJS.Signals[] = [];
+
+    await expect(
+      stopModelRouterProcess(123, 4000, {
+        isRunning: () => true,
+        readCommandLine: () => ["/usr/bin/unrelated-service", "--port", "4000"],
+        isHealthy: async () => true,
+        kill: (_pid, signal) => signals.push(signal),
+        sleep: async () => {},
+      }),
+    ).rejects.toThrow("it is not the model-router proxy");
+    expect(signals).toEqual([]);
+  });
+
+  it("fails closed when SIGTERM cannot be delivered", async () => {
+    await expect(
+      stopModelRouterProcess(123, 4000, {
+        isRunning: () => true,
+        readCommandLine: () => ROUTER_ARGS,
+        isHealthy: async () => true,
+        kill: () => {
+          throw new Error("EPERM");
+        },
+        sleep: async () => {},
+      }),
+    ).rejects.toThrow("Failed to send SIGTERM");
+  });
+
+  it("escalates to SIGKILL and reports a process that survives both signals", async () => {
+    const signals: NodeJS.Signals[] = [];
+
+    await expect(
+      stopModelRouterProcess(123, 4000, {
+        isRunning: () => true,
+        readCommandLine: () => ROUTER_ARGS,
+        isHealthy: async () => true,
+        kill: (_pid, signal) => signals.push(signal),
+        sleep: async () => {},
+      }),
+    ).rejects.toThrow("shutdown did not converge");
+    expect(signals).toEqual(["SIGTERM", "SIGKILL"]);
+  });
+
+  it("returns after SIGKILL removes the owned process and endpoint", async () => {
+    let running = true;
+    let healthy = true;
+    const signals: NodeJS.Signals[] = [];
+
+    await stopModelRouterProcess(123, 4000, {
+      isRunning: () => running,
+      readCommandLine: () => ROUTER_ARGS,
+      isHealthy: async () => healthy,
+      kill: (_pid, signal) => {
+        signals.push(signal);
+        running = signal !== "SIGKILL";
+        healthy = signal !== "SIGKILL";
+      },
+      sleep: async () => {},
+    });
+
+    expect(signals).toEqual(["SIGTERM", "SIGKILL"]);
+  });
+
+  it("refuses SIGKILL when PID ownership changes during graceful shutdown", async () => {
+    let ownershipChecks = 0;
+    const signals: NodeJS.Signals[] = [];
+
+    await expect(
+      stopModelRouterProcess(123, 4000, {
+        isRunning: () => true,
+        readCommandLine: () => {
+          ownershipChecks += 1;
+          return ownershipChecks === 1 ? ROUTER_ARGS : ["/usr/bin/unrelated-service"];
+        },
+        isHealthy: async () => false,
+        kill: (_pid, signal) => signals.push(signal),
+        sleep: async () => {},
+      }),
+    ).rejects.toThrow("ownership changed during shutdown");
+    expect(signals).toEqual(["SIGTERM"]);
+  });
+
+  it("does not declare success when the PID exits but the endpoint survives", async () => {
+    let running = true;
+
+    await expect(
+      stopModelRouterProcess(123, 4000, {
+        isRunning: () => running,
+        readCommandLine: () => ROUTER_ARGS,
+        isHealthy: async () => true,
+        kill: () => {
+          running = false;
+        },
+        sleep: async () => {},
+      }),
+    ).rejects.toThrow("port 4000 remains healthy");
+  });
+});

src/lib/onboard/model-router-process.ts

@@ -14,6 +14,12 @@ export type ModelRouterProcessOwnershipDeps = {
   readCommandLine?: (pid: number) => string[] | null;
 };
 
+export type StopModelRouterProcessDeps = ModelRouterProcessOwnershipDeps & {
+  isHealthy?: (port: number, timeoutMs?: number) => Promise<boolean>;
+  kill?: (pid: number, signal: NodeJS.Signals) => void;
+  sleep?: (delayMs: number) => Promise<void>;
+};
+
 type ModelRouterCommandLineReaderDeps = {
   readProcCommandLine?: (pid: number) => string[] | null;
   readPsCommandLine?: (pid: number) => string[] | null;
@@ -118,25 +124,94 @@ export function doesModelRouterProcessOwnPort(
   return Array.isArray(args) && isModelRouterCommandLineForPort(args, port);
 }
 
-export async function stopModelRouterProcess(pid: number, port: number): Promise<void> {
+/**
+ * Stop the exact router process and return only after both its PID and health
+ * endpoint disappear. Ownership is checked again before SIGKILL to avoid
+ * signaling a PID that was reused during the graceful-shutdown window.
+ */
+export async function stopModelRouterProcess(
+  pid: number,
+  port: number,
+  deps: StopModelRouterProcessDeps = {},
+): Promise<void> {
+  const isRunning = deps.isRunning ?? isProcessRunning;
+  const readCommandLine = deps.readCommandLine ?? readModelRouterProcessCommandLine;
+  const isHealthy = deps.isHealthy ?? isRouterHealthy;
+  const kill = deps.kill ?? ((targetPid, signal) => process.kill(targetPid, signal));
+  const sleep =
+    deps.sleep ?? ((delayMs) => new Promise<void>((resolve) => setTimeout(resolve, delayMs)));
+
+  if (!isRunning(pid)) {
+    if (!(await isHealthy(port, 1000))) return;
+    throw new Error(
+      `Refusing to replace model router: PID ${pid} exited but port ${port} remains healthy.`,
+    );
+  }
+  if (
+    !doesModelRouterProcessOwnPort(pid, port, {
+      isRunning,
+      readCommandLine,
+    })
+  ) {
+    throw new Error(
+      `Refusing to stop PID ${pid}: it is not the model-router proxy for port ${port}.`,
+    );
+  }
+
   try {
-    process.kill(pid, "SIGTERM");
-  } catch {
-    return;
+    kill(pid, "SIGTERM");
+  } catch (error) {
+    if (!isRunning(pid) && !(await isHealthy(port, 1000))) return;
+    throw new Error(
+      `Failed to send SIGTERM to model router PID ${pid}: ${
+        error instanceof Error ? error.message : String(error)
+      }`,
+    );
   }
   for (let _attempt = 0; _attempt < 10; _attempt++) {
-    await new Promise((resolve) => setTimeout(resolve, 500));
-    if (!isProcessRunning(pid) && !(await isRouterHealthy(port, 1000))) return;
+    await sleep(500);
+    if (!isRunning(pid) && !(await isHealthy(port, 1000))) return;
+  }
+
+  if (!isRunning(pid)) {
+    throw new Error(
+      `Model router PID ${pid} exited after SIGTERM, but port ${port} remains healthy.`,
+    );
+  }
+  if (
+    !doesModelRouterProcessOwnPort(pid, port, {
+      isRunning,
+      readCommandLine,
+    })
+  ) {
+    throw new Error(
+      `Refusing to send SIGKILL to PID ${pid}: model-router ownership changed during shutdown.`,
+    );
   }
+
   try {
-    process.kill(pid, "SIGKILL");
-  } catch {
-    // already stopped
+    kill(pid, "SIGKILL");
+  } catch (error) {
+    if (!isRunning(pid) && !(await isHealthy(port, 1000))) return;
+    throw new Error(
+      `Failed to send SIGKILL to model router PID ${pid}: ${
+        error instanceof Error ? error.message : String(error)
+      }`,
+    );
   }
   for (let _attempt = 0; _attempt < 5; _attempt++) {
-    await new Promise((resolve) => setTimeout(resolve, 500));
-    if (!isProcessRunning(pid) && !(await isRouterHealthy(port, 1000))) return;
+    await sleep(500);
+    if (!isRunning(pid) && !(await isHealthy(port, 1000))) return;
   }
+
+  const processStillRunning = isRunning(pid);
+  const endpointStillHealthy = await isHealthy(port, 1000);
+  if (!processStillRunning && !endpointStillHealthy) return;
+  const survivors = [
+    processStillRunning ? `PID ${pid}` : null,
+    endpointStillHealthy ? `health endpoint on port ${port}` : null,
+  ].filter(Boolean);
+  throw new Error(`Model router shutdown did not converge; still active: ${survivors.join(", ")}.`);
 }
 
 /**