fix(model-router): stop/recover router on uninstall & onboard; extend gateway health wait

agents/hermes/start.sh

@@ -676,7 +676,7 @@ start_hermes_dashboard_sandbox_user() {
 wait_for_hermes_gateway_internal() {
   local gateway_pid="$1"
   local attempts=0
-  while [ "$attempts" -lt 45 ]; do
+  while [ "$attempts" -lt 60 ]; do
     if curl -sf --max-time 2 "http://127.0.0.1:${INTERNAL_PORT}/health" >/dev/null 2>&1; then
       return 0
     fi
@@ -952,6 +952,59 @@ migrate_legacy_layout() {
   echo "[migration] Completed ${label} layout migration (${data_dir} removed)" >&2
 }
 
+# Hermes v0.16.0+ requires API_SERVER_KEY in the environment even for
+# loopback-only api_server binds. Sandboxes built before this requirement
+# have a .env without the key; generate and persist one now so existing
+# sandboxes don't fail on first startup after a Hermes version upgrade.
+ensure_hermes_api_server_key() {
+  local env_file="${HERMES_DIR}/.env"
+  local hash_file="${HERMES_HASH_FILE}"
+  local compat_hash="${HERMES_DIR}/.config-hash"
+  [ -f "$env_file" ] || return 0
+
+  grep -q "^API_SERVER_KEY=" "$env_file" 2>/dev/null && return 0
+
+  if [ -L "$env_file" ] || [ -L "$hash_file" ] || { [ -e "$compat_hash" ] && [ -L "$compat_hash" ]; }; then
+    echo "[SECURITY] Refusing API_SERVER_KEY injection — config or hash path is a symlink" >&2
+    return 1
+  fi
+
+  if [ "$(id -u)" -eq 0 ]; then
+    chown root:sandbox "$env_file" || return 1
+    chmod 640 "$env_file" || return 1
+    chmod u+w "$hash_file" || return 1
+    [ ! -f "$compat_hash" ] || chmod u+w "$compat_hash" 2>/dev/null || true
+  elif [ ! -w "$env_file" ]; then
+    echo "[config] Cannot inject API_SERVER_KEY — .env not writable (non-root mode); Hermes api_server will fail" >&2
+    return 0
+  fi
+
+  local new_key
+  new_key=$(python3 -c "import secrets; print(secrets.token_hex(32))")
+  printf 'API_SERVER_KEY=%s\n' "$new_key" >>"$env_file"
+  echo "[config] Generated missing API_SERVER_KEY for Hermes API server (Hermes v0.16.0+ requirement)" >&2
+
+  local _write_rc=0
+  if sha256sum "${HERMES_DIR}/config.yaml" "${HERMES_DIR}/.env" >"$hash_file"; then
+    chown root:root "$hash_file" 2>/dev/null || true
+    chmod 444 "$hash_file" 2>/dev/null || true
+    if [ -f "$compat_hash" ]; then
+      sha256sum "${HERMES_DIR}/config.yaml" "${HERMES_DIR}/.env" >"$compat_hash" || _write_rc=$?
+      chown sandbox:sandbox "$compat_hash" 2>/dev/null || true
+      chmod 600 "$compat_hash" 2>/dev/null || true
+    fi
+  else
+    _write_rc=$?
+  fi
+
+  if [ "$(id -u)" -eq 0 ]; then
+    chown sandbox:sandbox "$env_file" 2>/dev/null || true
+    chmod 640 "$env_file" 2>/dev/null || true
+  fi
+
+  [ "$_write_rc" -eq 0 ] || return "$_write_rc"
+}
+
 refresh_hermes_provider_placeholders() {
   local env_file="${HERMES_DIR}/.env"
   local hash_file="${HERMES_HASH_FILE}"
@@ -1092,6 +1145,7 @@ if [ "$(id -u)" -ne 0 ]; then
   apply_shields_up_runtime_env
   validate_hermes_env_secret_boundary
   validate_hermes_runtime_env_secret_boundary
+  ensure_hermes_api_server_key
   refresh_hermes_provider_placeholders
   configure_messaging_channels
   retry_tirith_marker_if_needed
@@ -1142,6 +1196,7 @@ verify_config_integrity "${HERMES_DIR}" "${HERMES_HASH_FILE}"
 apply_shields_up_runtime_env
 validate_hermes_env_secret_boundary
 validate_hermes_runtime_env_secret_boundary
+ensure_hermes_api_server_key
 refresh_hermes_provider_placeholders
 configure_messaging_channels
 retry_tirith_marker_if_needed

src/lib/actions/uninstall/run-plan.test.ts

@@ -1056,4 +1056,198 @@ describe("uninstall run plan", () => {
     expect(killed).toContain(9999887);
     expect(logs).toContain("Stopped host openshell-gateway process 9999887");
   });
+
+  // Real-world: model-router is a Python venv script so the OS interposes the
+  // interpreter — args[0]=python, args[1]=model-router (issue #5169).
+  const ROUTER_CMDLINE =
+    "/home/test/.nemoclaw/model-router-venv/bin/python /home/test/.nemoclaw/model-router-venv/bin/model-router proxy --port 4000\n";
+
+  function routerPsStub(
+    pidStr: string,
+    opts: { exited: Set<number>; cmdline?: string; owner?: string },
+  ) {
+    return (args: readonly string[]): RunResult | null => {
+      if (args[0] !== "-p" || args[1] !== pidStr || args[2] !== "-o") return null;
+      const pid = Number(pidStr);
+      if (args[3] === "pid=") return opts.exited.has(pid) ? notFound() : ok(`${pidStr}\n`);
+      if (args[3] === "user=") return ok(`${opts.owner ?? "testuser"}\n`);
+      if (args[3] === "args=") return ok(opts.cmdline ?? ROUTER_CMDLINE);
+      return null;
+    };
+  }
+
+  it("kills the model router via the onboard-session.json routerPid (#5169)", () => {
+    const logs: string[] = [];
+    const killed: number[] = [];
+    const exited = new Set<number>();
+    const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-test-5169-pid-"));
+    const stateDir = path.join(tmpHome, ".nemoclaw");
+    fs.mkdirSync(stateDir, { recursive: true });
+    fs.writeFileSync(path.join(stateDir, "onboard-session.json"), JSON.stringify({ routerPid: 54321 }));
+
+    try {
+      const stub = routerPsStub("54321", { exited });
+      const result = runUninstallPlan(
+        { assumeYes: true, deleteModels: false, keepOpenShell: true },
+        {
+          commandExists: () => true,
+          env: { HOME: tmpHome, LOGNAME: "testuser" } as NodeJS.ProcessEnv,
+          existsSync: (target) => fs.existsSync(target),
+          isTty: false,
+          kill: (pid, _signal) => {
+            killed.push(pid);
+            exited.add(pid);
+            return true;
+          },
+          log: (line) => logs.push(line),
+          rmSync: vi.fn(),
+          run: (command, args) => {
+            if (command === "ps") {
+              const result = stub(args);
+              if (result) return result;
+            }
+            if (command === "lsof" && args[0] === "-ti" && args[1] === ":4000") return ok("");
+            if (args[0] === "-c") return ok("/fake/bin/tool\n");
+            if (args[0] === "-f") return ok("");
+            return ok();
+          },
+          runDocker: () => ok(""),
+        },
+      );
+
+      expect(result.exitCode).toBe(0);
+      expect(killed).toContain(54321);
+      expect(logs).toContain("Stopped model router 54321");
+    } finally {
+      fs.rmSync(tmpHome, { recursive: true, force: true });
+    }
+  });
+
+  it("kills an orphan model router via lsof :4000 when the session file is gone (#5169)", () => {
+    const logs: string[] = [];
+    const killed: number[] = [];
+    const exited = new Set<number>();
+    const stub = routerPsStub("65432", { exited });
+    const result = runUninstallPlan(
+      { assumeYes: true, deleteModels: false, keepOpenShell: true },
+      {
+        commandExists: () => true,
+        env: {
+          HOME: "/tmp/nemoclaw-uninstall-test-5169-lsof",
+          LOGNAME: "testuser",
+        } as NodeJS.ProcessEnv,
+        existsSync: () => false,
+        isTty: false,
+        kill: (pid, _signal) => {
+          killed.push(pid);
+          exited.add(pid);
+          return true;
+        },
+        log: (line) => logs.push(line),
+        rmSync: vi.fn(),
+        run: (command, args) => {
+          if (command === "lsof" && args[0] === "-ti" && args[1] === ":4000") {
+            return ok("65432\n");
+          }
+          if (command === "ps") {
+            const result = stub(args);
+            if (result) return result;
+          }
+          if (args[0] === "-c") return ok("/fake/bin/tool\n");
+          if (args[0] === "-f") return ok("");
+          return ok();
+        },
+        runDocker: () => ok(""),
+      },
+    );
+
+    expect(result.exitCode).toBe(0);
+    expect(killed).toContain(65432);
+    expect(logs).toContain("Stopped model router 65432");
+  });
+
+  it("never stops a model router process owned by a different user (#5169)", () => {
+    const logs: string[] = [];
+    const killed: number[] = [];
+    const stub = routerPsStub("77788", { exited: new Set(), owner: "someone-else" });
+    const result = runUninstallPlan(
+      { assumeYes: true, deleteModels: false, keepOpenShell: true },
+      {
+        commandExists: () => true,
+        env: {
+          HOME: "/tmp/nemoclaw-uninstall-test-5169-foreign",
+          LOGNAME: "testuser",
+        } as NodeJS.ProcessEnv,
+        existsSync: () => false,
+        isTty: false,
+        kill: (pid) => {
+          killed.push(pid);
+          return true;
+        },
+        log: (line) => logs.push(line),
+        rmSync: vi.fn(),
+        run: (command, args) => {
+          if (command === "lsof" && args[0] === "-ti" && args[1] === ":4000") {
+            return ok("77788\n");
+          }
+          if (command === "ps") {
+            const result = stub(args);
+            if (result) return result;
+          }
+          if (args[0] === "-c") return ok("/fake/bin/tool\n");
+          if (args[0] === "-f") return ok("");
+          return ok();
+        },
+        runDocker: () => ok(""),
+      },
+    );
+
+    expect(result.exitCode).toBe(0);
+    expect(killed).not.toContain(77788);
+    expect(logs).toContain("No model router processes found");
+  });
+
+  it("never kills a process on :4000 whose cmdline is not model-router proxy (#5169)", () => {
+    const logs: string[] = [];
+    const killed: number[] = [];
+    const stub = routerPsStub("99988", {
+      exited: new Set(),
+      cmdline: "/usr/bin/python3 -m http.server 4000\n",
+    });
+    const result = runUninstallPlan(
+      { assumeYes: true, deleteModels: false, keepOpenShell: true },
+      {
+        commandExists: () => true,
+        env: {
+          HOME: "/tmp/nemoclaw-uninstall-test-5169-foreign-cmdline",
+          LOGNAME: "testuser",
+        } as NodeJS.ProcessEnv,
+        existsSync: () => false,
+        isTty: false,
+        kill: (pid) => {
+          killed.push(pid);
+          return true;
+        },
+        log: (line) => logs.push(line),
+        rmSync: vi.fn(),
+        run: (command, args) => {
+          if (command === "lsof" && args[0] === "-ti" && args[1] === ":4000") {
+            return ok("99988\n");
+          }
+          if (command === "ps") {
+            const result = stub(args);
+            if (result) return result;
+          }
+          if (args[0] === "-c") return ok("/fake/bin/tool\n");
+          if (args[0] === "-f") return ok("");
+          return ok();
+        },
+        runDocker: () => ok(""),
+      },
+    );
+
+    expect(result.exitCode).toBe(0);
+    expect(killed).not.toContain(99988);
+    expect(logs).toContain("No model router processes found");
+  });
 });

src/lib/actions/uninstall/run-plan.ts

@@ -481,6 +481,84 @@ function stopOllamaAuthProxy(paths: UninstallPaths, runtime: UninstallRuntime):
   if (stopped.size === 0) runtime.log("No Ollama auth proxy processes found");
 }
 
+// Default port for the model-router proxy (matches model-router.ts).
+const DEFAULT_MODEL_ROUTER_PORT = 4000;
+
+function isModelRouterPid(pid: number, runtime: UninstallRuntime): boolean {
+  if (!Number.isInteger(pid) || pid <= 0) return false;
+  const result = runtime.run("ps", ["-p", String(pid), "-o", "args="], { env: runtime.env });
+  if (result.status !== 0) return false;
+  const args = result.stdout.trim().split(/\s+/).filter(Boolean);
+  // model-router runs as a Python venv script: args[0]=python, args[1]=model-router.
+  // Check both positions, mirroring isModelRouterCommandLineForPort.
+  const name0 = path.basename(args[0] || "");
+  const name1 = path.basename(args[1] || "");
+  return (name0 === "model-router" || name1 === "model-router") && args.includes("proxy");
+}
+
+function tryStopModelRouterPid(pid: number, runtime: UninstallRuntime): boolean {
+  runtime.kill(pid);
+  if (waitForPidExit(pid, runtime, 1500)) {
+    runtime.log(`Stopped model router ${pid}`);
+    return true;
+  }
+  runtime.kill(pid, "SIGKILL");
+  if (waitForPidExit(pid, runtime, 1500)) {
+    runtime.log(`Stopped model router ${pid}`);
+    return true;
+  }
+  runtime.warn(`Failed to stop model router ${pid}`);
+  return false;
+}
+
+function stopModelRouter(paths: UninstallPaths, runtime: UninstallRuntime): void {
+  const stopped = new Set<number>();
+
+  // 1. Try the PID recorded in the onboard session. This is the most reliable
+  //    signal since it was written by the same process that started the router.
+  //    Path mirrors SESSION_FILE in src/lib/state/onboard-session.ts.
+  const sessionFile = path.join(paths.nemoclawStateDir, "onboard-session.json");
+  if (runtime.existsSync(sessionFile)) {
+    try {
+      const raw: unknown = JSON.parse(fs.readFileSync(sessionFile, "utf-8"));
+      const pid = Number.parseInt(
+        String((raw as Record<string, unknown>)?.routerPid ?? ""),
+        10,
+      );
+      if (
+        Number.isFinite(pid) &&
+        pid > 0 &&
+        pidOwnedByCurrentUser(pid, runtime) &&
+        isModelRouterPid(pid, runtime)
+      ) {
+        if (tryStopModelRouterPid(pid, runtime)) stopped.add(pid);
+      }
+    } catch {
+      /* session file absent or malformed — fall through to port scan */
+    }
+  }
+
+  // 2. Fallback: scan the default router port for orphans whose session file is
+  //    already gone (e.g. a previous uninstall partially cleaned state but the
+  //    process survived). Filter via cmdline so we never kill unrelated listeners.
+  if (!runtime.commandExists("lsof")) {
+    if (stopped.size === 0) runtime.warn("lsof not found; skipping orphan model router scan.");
+    return;
+  }
+  const lsof = runtime.run("lsof", ["-ti", `:${DEFAULT_MODEL_ROUTER_PORT}`], {
+    env: runtime.env,
+  });
+  const pids = splitNonEmptyLines(lsof.stdout).map(Number).filter(Number.isFinite);
+  for (const pid of pids) {
+    if (stopped.has(pid)) continue;
+    if (!pidOwnedByCurrentUser(pid, runtime)) continue;
+    if (!isModelRouterPid(pid, runtime)) continue;
+    if (tryStopModelRouterPid(pid, runtime)) stopped.add(pid);
+  }
+
+  if (stopped.size === 0) runtime.log("No model router processes found");
+}
+
 function stopOrphanedOpenShell(runtime: UninstallRuntime): void {
   if (!runtime.commandExists("pgrep")) {
     runtime.warn("pgrep not found; skipping orphaned openshell process cleanup.");
@@ -788,6 +866,7 @@ function executePlan(
         { logNoProcesses: true },
       );
       stopOllamaAuthProxy(paths, runtime);
+      stopModelRouter(paths, runtime);
     } else if (step.name === "OpenShell resources") {
       removeOpenShellResources(options, runtime);
     } else if (step.name === "NemoClaw CLI") {