fix(cli): update flow now says whether the running process needs a restart (#11885) (#12005)

Fixes three defects in the "update doesn't restart the running process" bug class: CLI update guidance now detects a live server and tells the operator to restart instead of implying the update is already live; the dashboard's Update button tries OmniRoute's own PID-file supervisor before falling back to pm2 instead of hardcoding pm2 and silently skipping; getLatestVersionFromNpmCli now uses --prefer-online (same fix pattern as #4376). TDD throughout, 63/63 targeted regression tests pass.

bin/cli/commands/update.mjs

@@ -1,4 +1,4 @@
-import { printHeading, printInfo, printSuccess, printError } from "../io.mjs";
+import { printHeading, printInfo, printSuccess, printError, printWarning } from "../io.mjs";
 import { homedir } from "node:os";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
@@ -6,6 +6,7 @@ import { execFile } from "node:child_process";
 import { promisify } from "node:util";
 import { t } from "../i18n.mjs";
 import { npmBin, npmExecOptions } from "../npm-exec.mjs";
+import { readPidFile, isPidRunning } from "../utils/pid.mjs";
 
 const execFileAsync = promisify(execFile);
 
@@ -79,6 +80,39 @@ export async function createBackup() {
   }
 }
 
+// #11885: `--apply` installs the new files (npm install -g) and re-reads
+// package.json from disk to confirm it, but a long-lived server process keeps
+// serving whatever it loaded at its last start — Node caches a `require()`d
+// package.json per resolved path for the life of the process. A later
+// `omniroute update` then correctly reports "already up to date" (the files
+// ARE current) while the running server is still stale, matching the reported
+// symptom. `--apply` never restarted anything and its success message ("Run
+// `omniroute --version` to verify.") implied the update was already live.
+//
+// `restart.mjs`'s `runRestartCommand()` stops then re-spawns the server in the
+// foreground (via `serve.mjs::runServe`), which can block the calling terminal
+// and is a materially bigger behavior change than this fix warrants to invoke
+// unconditionally and unattended from `--apply`. Instead, detect whether a
+// CLI-managed server is currently running (the same PID file `stop.mjs`/
+// `restart.mjs` already trust) and print an explicit, prominent instruction —
+// honest about what did and didn't happen — rather than silently assuming.
+export async function isServerProcessRunning(deps = { readPidFile, isPidRunning }) {
+  const pid = deps.readPidFile("server");
+  return Boolean(pid && deps.isPidRunning(pid));
+}
+
+export async function printPostApplyGuidance(latest, deps = { readPidFile, isPidRunning }) {
+  const running = await isServerProcessRunning(deps);
+  if (running) {
+    printWarning(`Files updated to ${latest}, but the running server is still on the old version.`);
+    printInfo("  Run `omniroute restart` now to apply this update.");
+  } else {
+    printInfo(`No running OmniRoute server was detected via the CLI's PID file.`);
+    printInfo(`  Start it with \`omniroute serve\` (or restart your existing process) to run ${latest}.`);
+  }
+  printInfo("`omniroute --version` will keep reporting the old version until the process restarts.");
+}
+
 export function registerUpdate(program) {
   program
     .command("update")
@@ -210,8 +244,8 @@ export async function runUpdateCommand(opts = {}) {
       console.log("  or reorder PATH so the global bin comes first.");
       return 1;
     }
-    printSuccess(`Updated to version ${latest}`);
-    printInfo("Run `omniroute --version` to verify.");
+    printSuccess(`Installed omniroute@${latest} to disk.`);
+    await printPostApplyGuidance(latest);
     return 0;
   } catch (err) {
     printError(`Update failed: ${err.message}`);

bin/cli/io.mjs

@@ -81,3 +81,7 @@ export function printInfo(message) {
 export function printError(message) {
   console.log(`\x1b[31m✖ ${message}\x1b[0m`);
 }
+
+export function printWarning(message) {
+  console.log(`\x1b[33m⚠ ${message}\x1b[0m`);
+}

changelog.d/fixes/11885-update-restart-live-process.md

@@ -0,0 +1 @@
+- **fix(cli):** `omniroute update --apply` now tells you explicitly whether a running server was detected and, if so, that you must run `omniroute restart` to apply the update — it never restarted anything and previously implied the update was already live once files were installed. The dashboard's npm-mode Update flow (`/api/system/version`) now tries OmniRoute's own PID-file-managed supervisor before falling back to pm2, and reports an honest "restart required" step instead of a silent pm2-only "skipped" that read like a completed update. The server-side latest-version lookup backing the dashboard's update banner also gained `--prefer-online`, closing the same stale-npm-cache class already fixed in the CLI's own copy for #4376 ([#11885](https://github.com/diegosouzapw/OmniRoute/issues/11885)).

src/app/api/system/version/route.ts

@@ -24,6 +24,7 @@ import {
   resolveLatestVersionCached,
 } from "@/lib/system/versionCheck";
 import { resolveGlobalOmniroutePath } from "@/lib/system/globalPackagePath";
+import { restartRunningServer } from "@/lib/system/processManagerRestart";
 // #5542 — On Windows npm is `npm.cmd`; Node ≥24 refuses to execFile a `.cmd` without
 // a shell (nodejs/node#52554 → "spawn npm ENOENT"). buildNpmExecOptions enables the
 // shell on win32 only; SERVICE_VERSION_PATTERN keeps the shell-joined version safe.
@@ -41,6 +42,20 @@ function getCurrentVersion(): string {
   }
 }
 
+/**
+ * Shared restart step for both npm-mode update flows (source-checkout and global-install
+ * below). #11885: this used to hardcode `pm2 restart omniroute` in each branch separately
+ * and silently report "skipped" — reading like a completed update — whenever pm2 wasn't
+ * the process manager. `restartRunningServer()` tries OmniRoute's own PID-file-managed
+ * supervisor first, then pm2, and this wrapper turns its honest "restart-required" outcome
+ * into an SSE step the dashboard renders as a warning instead of a false "done".
+ */
+async function sendRestartStep(send: (data: Record<string, unknown>) => void): Promise<void> {
+  send({ step: "restart", status: "running", message: "Restarting service..." });
+  const outcome = await restartRunningServer();
+  send({ step: "restart", status: outcome.status, message: outcome.message });
+}
+
 async function getNews() {
   try {
     const res = await fetch(NEWS_JSON_URL, { next: { revalidate: 3600 } });
@@ -259,20 +274,7 @@ export async function POST(req: NextRequest) {
           );
           send({ step: "rebuild", status: "done", message: "Build complete" });
 
-          send({ step: "restart", status: "running", message: "Restarting service..." });
-          try {
-            await execFileAsync("pm2", ["restart", "omniroute", "--update-env"], {
-              timeout: 30_000,
-              cwd: PROJECT_ROOT,
-            });
-            send({ step: "restart", status: "done", message: "Service restarted" });
-          } catch {
-            send({
-              step: "restart",
-              status: "skipped",
-              message: "PM2 not available — manual restart needed",
-            });
-          }
+          await sendRestartStep(send);
 
           send({
             step: "complete",
@@ -341,22 +343,8 @@ export async function POST(req: NextRequest) {
         );
         send({ step: "rebuild", status: "done", message: "Native modules rebuilt" });
 
-        // Step 3: Restart PM2
-        send({ step: "restart", status: "running", message: "Restarting service via PM2..." });
-          try {
-            await execFileAsync("pm2", ["restart", "omniroute", "--update-env"], {
-              timeout: 30000,
-              cwd: PROJECT_ROOT,
-            });
-            send({ step: "restart", status: "done", message: "Service restarted" });
-          } catch {
-            // PM2 may not be available (Docker/manual setups)
-            send({
-              step: "restart",
-              status: "skipped",
-              message: "PM2 not available — manual restart needed",
-            });
-          }
+        // Step 3: Restart
+        await sendRestartStep(send);
 
         clearLatestVersionCache();
         send({

src/lib/system/processManagerRestart.ts

@@ -0,0 +1,123 @@
+/**
+ * Restart mechanism detection for the npm-mode dashboard "Update" flow.
+ *
+ * #11885: `src/app/api/system/version/route.ts` hardcoded `pm2 restart omniroute` as the
+ * ONLY restart mechanism, at two near-identical branches. OmniRoute ships its OWN
+ * supervisor (`bin/cli/runtime/processSupervisor.mjs`, started by `omniroute serve` /
+ * `omniroute serve --daemon`) with PID-file management (`bin/cli/utils/pid.mjs`) as an
+ * alternative to pm2 — so on any install that isn't pm2-managed, the restart step
+ * silently degraded to a "skipped" status while the install step still reported "done",
+ * which reads like a completed live update even though nothing restarted.
+ *
+ * This module tries OmniRoute's own PID-file-managed supervisor first, then falls back
+ * to pm2, and returns an honest "restart-required" outcome instead of a silent no-op
+ * when neither is detected.
+ */
+import path from "node:path";
+import fs from "node:fs";
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
+import { resolveDataDir } from "@/lib/dataPaths";
+
+const execFileAsync = promisify(execFile);
+
+export type RestartMethod = "own-supervisor" | "pm2" | "none";
+export type RestartStatus = "done" | "restart-required";
+
+export interface RestartOutcome {
+  method: RestartMethod;
+  status: RestartStatus;
+  message: string;
+}
+
+type PidService = "server" | "supervisor";
+
+export interface RestartManagerDeps {
+  readPidFile?: (service: PidService) => number | null;
+  isPidRunning?: (pid: number) => boolean;
+  killProcess?: (pid: number) => void;
+  execPm2?: (args: string[]) => Promise<unknown>;
+}
+
+function defaultReadPidFile(service: PidService): number | null {
+  try {
+    const file = path.join(resolveDataDir(), service, ".pid");
+    if (!fs.existsSync(file)) return null;
+    const raw = fs.readFileSync(file, "utf8").trim();
+    const pid = parseInt(raw, 10);
+    return Number.isFinite(pid) ? pid : null;
+  } catch {
+    return null;
+  }
+}
+
+function defaultIsPidRunning(pid: number): boolean {
+  if (!pid) return false;
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function defaultKillProcess(pid: number): void {
+  process.kill(pid, "SIGTERM");
+}
+
+function defaultExecPm2(args: string[]): Promise<unknown> {
+  return execFileAsync("pm2", args, { timeout: 30_000 });
+}
+
+/**
+ * Attempt to restart the running OmniRoute server.
+ *
+ * Order:
+ *  1. OmniRoute's own PID-file-managed supervisor — SIGTERM the supervised "server"
+ *     child ONLY, never the supervisor process itself. The supervisor's exit handler
+ *     (`ServerSupervisor.handleExit`) treats an unexpected child exit as a crash and
+ *     respawns it, which IS the restart we want. SIGTERM to the SUPERVISOR instead
+ *     sets its `isShuttingDown` flag and intentionally skips the respawn (see
+ *     `bin/cli/commands/stop.mjs`) — that would stop the service, not restart it.
+ *  2. pm2, for installs that use it instead of the CLI's own supervisor.
+ *  3. Neither detected — return an honest "restart-required" outcome rather than a
+ *     silently-skipped step that reads like nothing was wrong.
+ */
+export async function restartRunningServer(
+  deps: RestartManagerDeps = {}
+): Promise<RestartOutcome> {
+  const readPidFile = deps.readPidFile ?? defaultReadPidFile;
+  const isPidRunning = deps.isPidRunning ?? defaultIsPidRunning;
+  const killProcess = deps.killProcess ?? defaultKillProcess;
+  const execPm2 = deps.execPm2 ?? defaultExecPm2;
+
+  const supervisorPid = readPidFile("supervisor");
+  const serverPid = readPidFile("server");
+  const supervisorAlive = Boolean(supervisorPid && isPidRunning(supervisorPid));
+  const serverAlive = Boolean(serverPid && isPidRunning(serverPid));
+
+  if (supervisorAlive && serverAlive && serverPid) {
+    try {
+      killProcess(serverPid);
+      return {
+        method: "own-supervisor",
+        status: "done",
+        message: "Restarted via the OmniRoute supervisor (server process recycled).",
+      };
+    } catch {
+      // Fall through to pm2 / restart-required below.
+    }
+  }
+
+  try {
+    await execPm2(["restart", "omniroute", "--update-env"]);
+    return { method: "pm2", status: "done", message: "Service restarted via pm2." };
+  } catch {
+    return {
+      method: "none",
+      status: "restart-required",
+      message:
+        "Files were updated, but no supported process manager (OmniRoute's own supervisor or pm2) was detected — restart the server manually to apply the update.",
+    };
+  }
+}

src/lib/system/versionCheck.ts

@@ -53,14 +53,26 @@ let latestVersionCacheGeneration = 0;
 // for back-compat with existing server-side importers.
 export { normalizeVersion, isNewer } from "./versionCompare";
 
-/** Latest published version via the `npm` CLI (fast when npm is on PATH, e.g. source installs). */
-export async function getLatestVersionFromNpmCli(): Promise<string | null> {
+/**
+ * Latest published version via the `npm` CLI (fast when npm is on PATH, e.g. source installs).
+ *
+ * `execFn` is injectable for tests (same pattern as the CLI's own
+ * `bin/cli/commands/update.mjs::getLatestVersion()`).
+ */
+export async function getLatestVersionFromNpmCli(
+  execFn: typeof execFileAsync = execFileAsync
+): Promise<string | null> {
   try {
     // #5542 — win32 npm is npm.cmd; execFile without a shell throws "spawn npm ENOENT"
     // on Node ≥24 (nodejs/node#52554). buildNpmExecOptions enables the shell on win32.
-    const { stdout } = await execFileAsync(
+    // #11885 — `--prefer-online` forces npm to revalidate its HTTP cache against the
+    // registry. Without it `npm info` can return a stale cached version, the same known
+    // bug class already fixed in the CLI's own copy for #4376 (see that fix's comment in
+    // bin/cli/commands/update.mjs::getLatestVersion()) but never mirrored here — this is
+    // the function backing the dashboard's "Update Available" banner.
+    const { stdout } = await execFn(
       "npm",
-      ["info", "omniroute", "version", "--json"],
+      ["info", "omniroute", "version", "--json", "--prefer-online"],
       buildNpmExecOptions(process.platform, { timeoutMs: LOOKUP_TIMEOUT_MS })
     );
     const parsed = JSON.parse(String(stdout).trim());

tests/unit/cli-update-restart-guidance-11885.test.ts

@@ -0,0 +1,75 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const update = await import("../../bin/cli/commands/update.mjs");
+
+// #11885: a user's dashboard reported an old version, running `omniroute update`
+// said "already up to date" (the on-disk package.json HAD been updated by an
+// earlier `--apply` run), but the long-lived server process kept serving the old
+// version because `--apply` never restarts anything — it only re-reads the
+// package.json on disk and prints "Run `omniroute --version` to verify", which
+// implies the running install is now current when it is not.
+
+test("isServerProcessRunning: true when the CLI-managed server pid is alive (#11885)", async () => {
+  const running = await update.isServerProcessRunning({
+    readPidFile: (service: string) => (service === "server" ? 12345 : null),
+    isPidRunning: (pid: number) => pid === 12345,
+  });
+  assert.equal(running, true);
+});
+
+test("isServerProcessRunning: false when there is no pid file (#11885)", async () => {
+  const running = await update.isServerProcessRunning({
+    readPidFile: () => null,
+    isPidRunning: () => true,
+  });
+  assert.equal(running, false);
+});
+
+test("isServerProcessRunning: false when the pid file is stale (#11885)", async () => {
+  const running = await update.isServerProcessRunning({
+    readPidFile: () => 999,
+    isPidRunning: () => false,
+  });
+  assert.equal(running, false);
+});
+
+function captureLogs(fn: () => Promise<void>) {
+  const logs: string[] = [];
+  const origLog = console.log;
+  console.log = (...args: unknown[]) => {
+    logs.push(args.map(String).join(" "));
+  };
+  return fn()
+    .then(() => logs)
+    .finally(() => {
+      console.log = origLog;
+    });
+}
+
+test("printPostApplyGuidance tells the user to run `omniroute restart` when a server is running, and stops implying the update is already live (#11885)", async () => {
+  const logs = await captureLogs(() =>
+    update.printPostApplyGuidance("3.9.0", {
+      readPidFile: () => 42,
+      isPidRunning: () => true,
+    })
+  );
+  const joined = logs.join("\n");
+  assert.match(joined, /omniroute restart/);
+  assert.doesNotMatch(
+    joined,
+    /^.*Updated to version 3\.9\.0.*$/m,
+    "must not claim the running process is already updated"
+  );
+});
+
+test("printPostApplyGuidance tells the user to start the server when none is detected (#11885)", async () => {
+  const logs = await captureLogs(() =>
+    update.printPostApplyGuidance("3.9.0", {
+      readPidFile: () => null,
+      isPidRunning: () => false,
+    })
+  );
+  const joined = logs.join("\n");
+  assert.match(joined, /omniroute serve/);
+});

tests/unit/system-process-manager-restart-11885.test.ts

@@ -0,0 +1,80 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { restartRunningServer } from "@/lib/system/processManagerRestart";
+
+// #11885: the dashboard's npm-mode Update flow (src/app/api/system/version/route.ts)
+// hardcoded `pm2 restart omniroute` as the ONLY restart mechanism, at two
+// near-identical branches. When pm2 isn't the process manager (OmniRoute's own
+// `omniroute serve --daemon` supervisor, or plain `npm run start`) it silently
+// degraded to a "skipped" status while the install step still reported "done" —
+// reading like a completed live update when the running server never restarted.
+
+test("restartRunningServer: uses OmniRoute's own supervisor when both the supervisor and server pids are alive", async () => {
+  const killed = [];
+  const pm2Calls = [];
+  const outcome = await restartRunningServer({
+    readPidFile: (service) => (service === "supervisor" ? 111 : service === "server" ? 222 : null),
+    isPidRunning: (pid) => pid === 111 || pid === 222,
+    killProcess: (pid) => killed.push(pid),
+    execPm2: async (args) => {
+      pm2Calls.push(args);
+    },
+  });
+
+  assert.equal(outcome.method, "own-supervisor");
+  assert.equal(outcome.status, "done");
+  // Must SIGTERM the supervised SERVER child, never the supervisor itself — killing
+  // the supervisor sets its isShuttingDown flag and intentionally skips the respawn
+  // (see bin/cli/commands/stop.mjs), which would NOT produce a restart.
+  assert.deepEqual(killed, [222]);
+  assert.equal(pm2Calls.length, 0, "must not fall back to pm2 when the own supervisor path succeeds");
+});
+
+test("restartRunningServer: falls back to pm2 when no OmniRoute supervisor is detected", async () => {
+  const pm2Calls = [];
+  const outcome = await restartRunningServer({
+    readPidFile: () => null,
+    isPidRunning: () => false,
+    killProcess: () => {
+      throw new Error("must not be called");
+    },
+    execPm2: async (args) => {
+      pm2Calls.push(args);
+    },
+  });
+
+  assert.equal(outcome.method, "pm2");
+  assert.equal(outcome.status, "done");
+  assert.equal(pm2Calls.length, 1);
+  assert.deepEqual(pm2Calls[0], ["restart", "omniroute", "--update-env"]);
+});
+
+test("restartRunningServer: falls back to pm2 when the supervisor pid is stale", async () => {
+  const pm2Calls = [];
+  const outcome = await restartRunningServer({
+    readPidFile: (service) => (service === "supervisor" ? 999 : null),
+    isPidRunning: () => false,
+    killProcess: () => {
+      throw new Error("must not be called");
+    },
+    execPm2: async (args) => {
+      pm2Calls.push(args);
+    },
+  });
+
+  assert.equal(outcome.method, "pm2");
+});
+
+test("restartRunningServer: returns an honest restart-required outcome when neither is available (no silent 'skipped')", async () => {
+  const outcome = await restartRunningServer({
+    readPidFile: () => null,
+    isPidRunning: () => false,
+    execPm2: async () => {
+      throw new Error("pm2: command not found");
+    },
+  });
+
+  assert.equal(outcome.method, "none");
+  assert.equal(outcome.status, "restart-required");
+  assert.match(outcome.message, /restart/i);
+});

tests/unit/system-version-check-prefer-online-11885.test.ts

@@ -0,0 +1,34 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { getLatestVersionFromNpmCli } from "@/lib/system/versionCheck";
+
+// #11885: a user's dashboard showed 3.8.10 while the "3.8.49 available" banner kept
+// firing, yet `omniroute update` on the box said "already up to date". Part of the
+// same known bug class as #4376 (already fixed in bin/cli/commands/update.mjs's own
+// getLatestVersion()): `npm info omniroute version --json` without `--prefer-online`
+// can serve a stale cached "latest" from npm's local HTTP cache. The server-side copy
+// used by the dashboard's update banner never got the same fix.
+test("getLatestVersionFromNpmCli passes --prefer-online to bypass the stale npm cache (#11885, same class as #4376)", async () => {
+  let capturedArgs: string[] | null = null;
+  const fakeExec = async (_cmd: string, args: string[]) => {
+    capturedArgs = args;
+    return { stdout: JSON.stringify("3.8.49"), stderr: "" };
+  };
+  const latest = await getLatestVersionFromNpmCli(fakeExec as never);
+  assert.equal(latest, "3.8.49");
+  assert.ok(capturedArgs, "exec must be invoked");
+  assert.ok(
+    (capturedArgs as string[]).includes("--prefer-online"),
+    `expected --prefer-online in npm args, got: ${JSON.stringify(capturedArgs)}`
+  );
+  assert.ok((capturedArgs as string[]).includes("info"));
+  assert.ok((capturedArgs as string[]).includes("omniroute"));
+  assert.ok((capturedArgs as string[]).includes("version"));
+});
+
+test("getLatestVersionFromNpmCli returns null when npm is unavailable", async () => {
+  const latest = await getLatestVersionFromNpmCli(async () => {
+    throw new Error("npm not found");
+  });
+  assert.equal(latest, null);
+});