diff --git a/src/lib/actions/uninstall/run-plan.test.ts b/src/lib/actions/uninstall/run-plan.test.ts index 6e01cd585c9..153cf502ff9 100644 --- a/src/lib/actions/uninstall/run-plan.test.ts +++ b/src/lib/actions/uninstall/run-plan.test.ts @@ -18,6 +18,10 @@ function notFound(): RunResult { } const PROXY_CMDLINE = "/usr/bin/node /opt/nemoclaw/scripts/ollama-auth-proxy.js\n"; +// 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 MODEL_ROUTER_CMDLINE = + "/home/test/.nemoclaw/model-router-venv/bin/python /home/test/.nemoclaw/model-router-venv/bin/model-router proxy --port 4000\n"; function psStub(pidStr: string, opts: { exited: Set; cmdline?: string; owner?: string }) { return (args: readonly string[]): RunResult | null => { @@ -517,6 +521,195 @@ describe("uninstall run plan", () => { expect(logs).toContain("No Ollama auth proxy processes found"); }); + it("kills the model router via onboard-session routerPid (#5169)", () => { + const logs: string[] = []; + const killed: number[] = []; + const exited = new Set(); + const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-test-5169-session-")); + const stateDir = path.join(tmpHome, ".nemoclaw"); + const sessionFile = path.join(stateDir, "onboard-session.json"); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(sessionFile, JSON.stringify({ routerPid: 55432 })); + + try { + const stub = psStub("55432", { exited, cmdline: MODEL_ROUTER_CMDLINE }); + const result = runUninstallPlan( + { assumeYes: true, deleteModels: false, keepOpenShell: true }, + { + commandExists: () => true, + env: { HOME: tmpHome, 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 === "ps") { + const result = stub(args); + if (result) return result; + } + if (command === "lsof") 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(55432); + expect(logs).toContain("Stopped model router 55432"); + } finally { + fs.rmSync(tmpHome, { recursive: true, force: true }); + } + }); + + it("kills an orphan model router via lsof :4000 when onboard-session is gone", () => { + const logs: string[] = []; + const killed: number[] = []; + const exited = new Set(); + const stub = psStub("55679", { exited, cmdline: MODEL_ROUTER_CMDLINE }); + 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("55679\n"); + } + if (command === "lsof" && args[0] === "-ti" && args[1] === ":11435") { + return ok(""); + } + 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(55679); + expect(logs).toContain("Stopped model router 55679"); + }); + + it("never stops a foreign-owned model router on :4000 even if cmdline matches", () => { + const logs: string[] = []; + const killed: number[] = []; + const stub = psStub("77888", { + exited: new Set(), + owner: "someone-else", + cmdline: MODEL_ROUTER_CMDLINE, + }); + const result = runUninstallPlan( + { assumeYes: true, deleteModels: false, keepOpenShell: true }, + { + commandExists: () => true, + env: { + HOME: "/tmp/nemoclaw-uninstall-test-5169-foreign-owner", + 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("77888\n"); + } + if (command === "lsof" && args[0] === "-ti" && args[1] === ":11435") { + return ok(""); + } + 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(77888); + expect(logs).toContain("No model router processes found"); + }); + + it("never kills a process on :4000 whose cmdline is not the model router", () => { + const logs: string[] = []; + const killed: number[] = []; + const stub = psStub("88888", { + exited: new Set(), + cmdline: "/usr/sbin/nginx -g daemon off;\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("88888\n"); + } + if (command === "lsof" && args[0] === "-ti" && args[1] === ":11435") { + return ok(""); + } + 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(88888); + expect(logs).toContain("No model router processes found"); + }); + it("escalates to SIGKILL and reports failure when SIGTERM is ignored", () => { const logs: string[] = []; const warnings: string[] = []; diff --git a/src/lib/actions/uninstall/run-plan.ts b/src/lib/actions/uninstall/run-plan.ts index 9cd60a0578d..c1cd3fbe822 100644 --- a/src/lib/actions/uninstall/run-plan.ts +++ b/src/lib/actions/uninstall/run-plan.ts @@ -17,6 +17,7 @@ import { type UninstallPaths, } from "../../domain/uninstall/paths"; import { buildUninstallPlan, type UninstallPlan } from "../../domain/uninstall/plan"; +import { isModelRouterCommandLineForPort } from "../../onboard/model-router-process"; import { stopHostGatewayProcesses } from "../../onboard/host-gateway-process"; import { stopStaleDashboardListeners } from "../../onboard/stale-gateway-cleanup"; import { classifyShimPath, type FileSystemDeps } from "./plan"; @@ -481,6 +482,87 @@ function stopOllamaAuthProxy(paths: UninstallPaths, runtime: UninstallRuntime): if (stopped.size === 0) runtime.log("No Ollama auth proxy processes found"); } +const DEFAULT_MODEL_ROUTER_PORT = 4000; + +function resolveModelRouterPort(_runtime: UninstallRuntime): number { + // Routed onboard profiles use blueprint port 4000 by default; a custom port + // would require reading the blueprint, which uninstall does not do today. + return DEFAULT_MODEL_ROUTER_PORT; +} + +function readOnboardSessionRouterPid(paths: UninstallPaths): number | null { + const sessionFile = path.join(paths.nemoclawStateDir, "onboard-session.json"); + try { + const raw = fs.readFileSync(sessionFile, "utf-8"); + const data = JSON.parse(raw) as { routerPid?: unknown }; + const pid = data.routerPid; + if (typeof pid === "number" && Number.isInteger(pid) && pid > 0) return pid; + } catch { + /* ignore — State step deletes the file shortly anyway */ + } + return null; +} + +function isModelRouterPid(pid: number, port: number, runtime: UninstallRuntime): boolean { + if (!Number.isInteger(pid) || pid <= 0) return false; + if (!pidExists(pid, runtime)) 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); + return isModelRouterCommandLineForPort(args, port); +} + +function tryStopModelRouterPid(pid: number, runtime: UninstallRuntime): boolean { + runtime.kill(pid); + if (waitForPidExit(pid, runtime, 1000)) { + runtime.log(`Stopped model router ${pid}`); + return true; + } + runtime.kill(pid, "SIGKILL"); + if (waitForPidExit(pid, runtime, 1000)) { + 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 { + // The model router is a detached child started during routed onboard that + // listens on port 4000 by default. Without this cleanup, uninstall + + // reinstall fails with "Port 4000 already has a healthy router endpoint". + // The tracked PID lives in ~/.nemoclaw/onboard-session.json (routerPid), not + // a dedicated .pid file. Mirrors stopOllamaAuthProxy() and issue #5169. + const stopped = new Set(); + const routerPort = resolveModelRouterPort(runtime); + + const recordedPid = readOnboardSessionRouterPid(paths); + if ( + recordedPid !== null && + pidOwnedByCurrentUser(recordedPid, runtime) && + isModelRouterPid(recordedPid, routerPort, runtime) + ) { + if (tryStopModelRouterPid(recordedPid, runtime)) stopped.add(recordedPid); + } + + 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", `:${routerPort}`], { 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, routerPort, 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 +870,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") { diff --git a/src/lib/domain/uninstall/plan.test.ts b/src/lib/domain/uninstall/plan.test.ts index 024a545ff85..b7a18f74bc3 100644 --- a/src/lib/domain/uninstall/plan.test.ts +++ b/src/lib/domain/uninstall/plan.test.ts @@ -40,16 +40,18 @@ describe("uninstall plan", () => { path: path.join("/usr/local/bin", binary), })), { kind: "stop-ollama-auth-proxy" }, + { kind: "stop-model-router" }, ]), ); - // The Ollama auth proxy must be stopped during the "Stopping services" - // step, before any "State and binaries" cleanup deletes the PID file. - // Otherwise a stale proxy on :11435 blocks reinstall (issue #2759). + // Both the Ollama auth proxy and the model router must be stopped during + // the "Stopping services" step, before "State and binaries" deletes PID + // files. A stale proxy on :11435 blocks reinstall (#2759); a stale router + // on :4000 blocks reinstall (#5169). const stoppingServicesStep = plan.steps.find((step) => step.name === "Stopping services"); expect(stoppingServicesStep).toBeTruthy(); expect(stoppingServicesStep?.actions).toEqual( - expect.arrayContaining([{ kind: "stop-ollama-auth-proxy" }]), + expect.arrayContaining([{ kind: "stop-ollama-auth-proxy" }, { kind: "stop-model-router" }]), ); }); diff --git a/src/lib/domain/uninstall/plan.ts b/src/lib/domain/uninstall/plan.ts index 7747de2c6b1..758be879f50 100644 --- a/src/lib/domain/uninstall/plan.ts +++ b/src/lib/domain/uninstall/plan.ts @@ -34,6 +34,7 @@ export type UninstallPlanAction = | { kind: "preserve-openshell-install-paths"; paths: string[] } | { kind: "preserve-shim"; reason: string } | { kind: "stop-helper-services" } + | { kind: "stop-model-router" } | { kind: "stop-ollama-auth-proxy" } | { kind: "stop-openshell-forward-processes" } | { kind: "stop-orphaned-openshell-processes" } @@ -76,6 +77,7 @@ export function buildUninstallPlan( { kind: "stop-openshell-forward-processes" }, { kind: "stop-orphaned-openshell-processes" }, { kind: "stop-ollama-auth-proxy" }, + { kind: "stop-model-router" }, ], }, { diff --git a/src/lib/onboard/model-router-process.test.ts b/src/lib/onboard/model-router-process.test.ts new file mode 100644 index 00000000000..054dac3fe61 --- /dev/null +++ b/src/lib/onboard/model-router-process.test.ts @@ -0,0 +1,67 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { findModelRouterPidForPort } from "./model-router-process"; + +describe("findModelRouterPidForPort", () => { + it("returns the PID when a model-router proxy is found via proc scan (direct, #5169)", () => { + const pid = findModelRouterPidForPort(4000, { + readProcCommandLine: (p) => + p === 12345 + ? ["/home/user/.nemoclaw/model-router-venv/bin/model-router", "proxy", "--port", "4000"] + : null, + listProcPids: () => [1, 100, 12345, 99999], + }); + expect(pid).toBe(12345); + }); + + it("returns the PID when model-router is Python-interpreted (args[1], #5169)", () => { + const pid = findModelRouterPidForPort(4000, { + readProcCommandLine: (p) => + p === 12345 + ? [ + "/home/user/.nemoclaw/model-router-venv/bin/python", + "/home/user/.nemoclaw/model-router-venv/bin/model-router", + "proxy", + "--port", + "4000", + ] + : null, + listProcPids: () => [1, 100, 12345, 99999], + }); + expect(pid).toBe(12345); + }); + + it("returns null when no model-router is found on that port", () => { + const pid = findModelRouterPidForPort(4000, { + readProcCommandLine: (p) => + p === 12345 + ? ["/home/user/.nemoclaw/model-router-venv/bin/model-router", "proxy", "--port", "9999"] + : null, + listProcPids: () => [12345], + }); + expect(pid).toBe(null); + }); + + it("returns null when listProcPids returns an empty list", () => { + const pid = findModelRouterPidForPort(4000, { + readProcCommandLine: () => null, + listProcPids: () => [], + }); + expect(pid).toBe(null); + }); + + it("returns the first matching PID when multiple model-routers are present", () => { + const pid = findModelRouterPidForPort(4000, { + readProcCommandLine: (p) => { + if (p === 100) return ["/opt/model-router", "proxy", "--port", "4000"]; + if (p === 200) return ["/opt/model-router", "proxy", "--port", "4000"]; + return null; + }, + listProcPids: () => [50, 100, 200], + }); + expect(pid).toBe(100); + }); +}); diff --git a/src/lib/onboard/model-router-process.ts b/src/lib/onboard/model-router-process.ts index bb17bc5638f..ab37efc0be7 100644 --- a/src/lib/onboard/model-router-process.ts +++ b/src/lib/onboard/model-router-process.ts @@ -17,6 +17,8 @@ export type ModelRouterProcessOwnershipDeps = { type ModelRouterCommandLineReaderDeps = { readProcCommandLine?: (pid: number) => string[] | null; readPsCommandLine?: (pid: number) => string[] | null; + /** Override the /proc PID enumeration (injectable for tests). */ + listProcPids?: () => number[]; }; export async function isRouterHealthy( @@ -54,8 +56,12 @@ export function isProcessRunning(pid: number | null | undefined): boolean { } export function isModelRouterCommandLineForPort(args: readonly string[], port: number): boolean { - const commandName = path.basename(args[0] || ""); - if (commandName !== "model-router") return false; + // model-router may run as a Python venv script, where the OS interposes the + // interpreter: args[0]=python, args[1]=/path/to/model-router. Check both + // positions so /proc-based detection works regardless of execution mode. + const name0 = path.basename(args[0] || ""); + const name1 = path.basename(args[1] || ""); + if (name0 !== "model-router" && name1 !== "model-router") return false; if (!args.includes("proxy")) return false; return args.some((arg, index) => { if (arg === "--port") return args[index + 1] === String(port); @@ -133,6 +139,39 @@ export async function stopModelRouterProcess(pid: number, port: number): Promise } } +/** + * Scan /proc for a model-router process bound to `port`. + * + * Used by reconcileModelRouter to auto-recover orphaned routers whose PID + * was not recorded in the current session (e.g. after a failed install left a + * running router and the next session starts fresh). Returns null when /proc + * is unavailable (macOS) or no matching process is found. + */ +export function findModelRouterPidForPort( + port: number, + deps: ModelRouterCommandLineReaderDeps = {}, +): number | null { + let pids: number[]; + if (deps.listProcPids) { + pids = deps.listProcPids(); + } else { + try { + pids = fs + .readdirSync("/proc") + .map(Number) + .filter((n) => Number.isFinite(n) && n > 0); + } catch { + return null; + } + } + const readCmdLine = deps.readProcCommandLine ?? readProcCommandLine; + for (const pid of pids) { + const args = readCmdLine(pid); + if (args && isModelRouterCommandLineForPort(args, port)) return pid; + } + return null; +} + export async function stopTrackedModelRouterForAgentChange( session: Pick | null, port: number, diff --git a/src/lib/onboard/model-router.test.ts b/src/lib/onboard/model-router.test.ts index 35a4019dec8..d654b652f0a 100644 --- a/src/lib/onboard/model-router.test.ts +++ b/src/lib/onboard/model-router.test.ts @@ -26,6 +26,21 @@ describe("model-router process ownership checks", () => { ).toBe(true); }); + it("recognizes Python-interpreted model-router venv command lines (#5169)", () => { + expect( + isModelRouterCommandLineForPort( + [ + "/home/user/.nemoclaw/model-router-venv/bin/python", + "/home/user/.nemoclaw/model-router-venv/bin/model-router", + "proxy", + "--port", + "4000", + ], + 4000, + ), + ).toBe(true); + }); + it("falls back to ps-style command lines when /proc is unavailable", () => { expect( readModelRouterProcessCommandLine(1234, { diff --git a/src/lib/onboard/model-router.ts b/src/lib/onboard/model-router.ts index cd95fdac01c..5ff7fccf29c 100644 --- a/src/lib/onboard/model-router.ts +++ b/src/lib/onboard/model-router.ts @@ -24,6 +24,7 @@ import { } from "./host-service-reachability"; import { doesModelRouterProcessOwnPort, + findModelRouterPidForPort, isRouterHealthy, stopModelRouterProcess, } from "./model-router-process"; @@ -506,9 +507,20 @@ export async function reconcileModelRouter(): Promise { routerPort, ); } else { - throw new Error( - `Port ${routerPort} already has a healthy router endpoint, but its credential state is unknown. Stop the existing model-router process and rerun onboarding.`, - ); + // The recorded PID doesn't own the port (stale session or fresh start). + // Try to locate the orphaned router via /proc so we can recover without + // requiring a manual stop-and-retry. Only stop it if the cmdline + // confirms it is actually model-router proxy — never kill an unrelated + // service that happens to occupy the port. See issue #5169. + const orphanPid = findModelRouterPidForPort(routerPort); + if (orphanPid !== null) { + console.log(` Stopping orphaned model router (PID ${orphanPid})...`); + await stopModelRouterProcess(orphanPid, routerPort); + } else { + throw new Error( + `Port ${routerPort} already has a healthy router endpoint, but its credential state is unknown. Stop the existing model-router process and rerun onboarding.`, + ); + } } }