Repository navigation
fix(cli): update flow now says whether the running process needs a restart - #12005
Merged
diegosouzapw merged 1 commit intoAug 29, 2026
Merged
Conversation
…start (#11885) Three defects in the same known bug class produced the reported symptom (dashboard stuck on an old version while `omniroute update` says "already up to date"): - `omniroute update --apply` installed files and re-verified them on disk, but never restarted anything and its success message implied the running process was already current. It now checks the CLI's own PID file and prints an explicit instruction (`omniroute restart`, or `omniroute serve` when no server is detected) instead of implying the update is live. - The dashboard's npm-mode Update flow (`/api/system/version`) hardcoded `pm2 restart omniroute` at two duplicated call sites and silently degraded to a "skipped" status — reading like a completed update — whenever pm2 wasn't the process manager. A new `restartRunningServer()` helper (`src/lib/system/processManagerRestart.ts`) tries OmniRoute's own PID-file-managed supervisor first, then pm2, and reports an honest "restart-required" outcome otherwise. Both call sites now share one `sendRestartStep()` wrapper instead of duplicating the logic a third time. - `getLatestVersionFromNpmCli()` (backing the dashboard's update banner) ran `npm info` without `--prefer-online`, the same stale-npm-cache class already fixed in the CLI's own `getLatestVersion()` for #4376 but never mirrored here. The reporter's exact environment (install method, process manager) was never confirmed, so this fixes the mechanism, not a proven root cause for that specific user.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…start (cheaper-inference#11885) (cheaper-inference#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 cheaper-inference#4376). TDD throughout, 63/63 targeted regression tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three defects in the same known bug class that together produce the reported symptom (a dashboard stuck reporting an old version while
omniroute updateclaims "already up to date"): the update mechanism, not the reporter's specific environment, is what this PR proves and fixes — their exact install method / process manager was never confirmed (see "Pending live validation" below).bin/cli/commands/update.mjs—omniroute update --applyinstalls the new files (npm install -g) and re-readspackage.jsonfrom disk to verify, but never restarted anything, and its old success message ("Runomniroute --versionto verify.") implied the running process was already current. A lateromniroute updatethen correctly says "already up to date" (the files are current) while the long-lived server process — which cached itsrequire()dpackage.jsonat startup — keeps serving the old build. This is exactly the reported symptom pair.stop.mjs/restart.mjsalready trust) and print an explicit, prominent instruction —omniroute restartif one is running,omniroute serveotherwise — instead of implying the update is live. Deliberately does not auto-invokerestart.mjs'srunRestartCommand(): it stops then re-spawns the server in the foreground, which can block the calling terminal — a materially bigger, riskier behavior change than warranted for an unattended--applyflow.src/app/api/system/version/route.ts(dashboard's own Update button, npm mode) — hardcodedpm2 restart omnirouteat two near-identical branches (~264, ~347 pre-fix). When pm2 isn't the process manager it silently degraded tostatus: "skipped"while the install step still said "done" — reading like a completed live update. OmniRoute ships its own supervisor (omniroute serve/--daemon, PID-file managed) and plainnpm run startas pm2 alternatives, so this was the common case, not the edge case.src/lib/system/processManagerRestart.ts::restartRunningServer()tries OmniRoute's own PID-file-managed supervisor first (SIGTERM only the supervised server child, never the supervisor — killing the supervisor sets itsisShuttingDownflag and intentionally skips the respawn, perstop.mjs; killing the child lets the supervisor's own crash-recovery respawn it, which is the restart we want), then falls back to pm2, and returns an honestrestart-requiredoutcome instead of a silentskippedwhen neither applies. Both route.ts call sites now share onesendRestartStep()helper instead of duplicating the pm2-only logic a third time.src/lib/system/versionCheck.ts::getLatestVersionFromNpmCli()(backs the dashboard's "Update Available" banner) — rannpm info omniroute version --jsonwithout--prefer-online, the identical stale-npm-cache bug already fixed in the CLI's owngetLatestVersion()for [BUG] Failed Update From Commandomniroute update#4376, never mirrored here. Added--prefer-onlineplus an injectableexecFn(matching the CLI's fix pattern) for testability.Test plan (TDD — failing test written first for each defect, then the fix)
tests/unit/system-version-check-prefer-online-11885.test.ts— provesgetLatestVersionFromNpmClipasses--prefer-onlineand is exec-injectable.tests/unit/cli-update-restart-guidance-11885.test.ts— provesisServerProcessRunning/printPostApplyGuidancedetect a live server via PID file and printomniroute restartguidance instead of implying the update is live.tests/unit/system-process-manager-restart-11885.test.ts— provesrestartRunningServer()prefers the own-supervisor path (killing only the server pid, never pm2), falls back to pm2 when no supervisor is detected/stale, and returns an honestrestart-requiredoutcome when neither applies.cli-update-*,system-version-check-4100,system-version-cache-8278,auto-update*,home-update-error-render-5991).npm install,pm2, or process kill is ever executed by a test.Pending live validation
Could not verify locally / no VPS access in this session:
omniroute serve/plain terminal?) — this PR fixes the mechanism that produces the reported symptom class, not a confirmed root cause for that specific user's box.restartRunningServer()'s own-supervisor path (SIGTERM to the server pid actually being respawned by a liveServerSupervisor) — unit-tested with injectedkillProcess/readPidFile/isPidRunning, never exercised against a real spawned supervisor process.omniroute serve --daemoninstall.omniroute update --applyagainst a real global npm install (only the pre-existing#9475shadow-install test exercises the surrounding flow via a fakednpmbinary onPATH).Both of the above need
root@192.168.0.15or an equivalent real install to runomniroute serve/omniroute serve --daemon/pm2 and observe an actual restart — not available in this session.Closes #11885