fix(uninstall): stop model router during teardown

[Thabhelo]

Summary

Stop the routed onboard model-router process during uninstall so port 4000 is freed before state cleanup and re-onboard does not fail with "Port 4000 already has a healthy router endpoint".

Related Issue

Fixes #5169

Changes

• Add stop-model-router to the uninstall plan's "Stopping services" step
• Implement stopModelRouter() mirroring the Ollama auth-proxy path: read routerPid from ~/.nemoclaw/onboard-session.json, verify cmdline, fall back to lsof on :4000
• Add regression tests for session PID, orphan lsof, foreign-owner, and wrong-cmdline cases

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Thabhelo 50872400+Thabhelo@users.noreply.github.com

Summary by CodeRabbit

• Bug Fixes

  • Uninstall reliably stops the model-router during cleanup (including orphaned instances bound to the router port) and avoids stopping unrelated or foreign-owned processes.
  • Onboarding recovers from mismatched PID ownership by locating/stopping orphaned model-router instances for the expected port or prompting the user when incompatible routers remain.

• New Features

  • Uninstall plan now explicitly includes a model-router stop step.

• Tests

  • Added tests for model-router shutdown, port-based discovery, ownership checks, and command-line matching.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8024d7c8-8c63-47c7-8d99-6d80d69b4f21

📥 Commits

Reviewing files that changed from the base of the PR and between 9bb6b19 and bf2e481.

📒 Files selected for processing (2)

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

🚧 Files skipped from review as they are similar to previous changes (2)

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

---

📝 Walkthrough

Walkthrough

Adds a new uninstall action to stop model-router processes; implements PID discovery and validation (cmdline and ownership), graceful SIGTERM→SIGKILL termination with lsof-based orphan scanning for port 4000, integrates stopModelRouter into uninstall, and adds tests for stored-PID, orphan, ownership, and cmdline-gating scenarios.

Changes

Model-Router Cleanup on Uninstall

Layer / File(s)	Summary
Uninstall plan schema and builder src/lib/domain/uninstall/plan.ts, src/lib/domain/uninstall/plan.test.ts	UninstallPlanAction union gains { kind: "stop-model-router" }. The uninstall plan emits this action in the "Stopping services" phase; tests updated to expect the new action in flattened steps and the stopping-services actions.
Model-router process discovery and onboarding integration src/lib/onboard/model-router-process.ts, src/lib/onboard/model-router-process.test.ts, src/lib/onboard/model-router.ts, src/lib/onboard/model-router.test.ts	Exports findModelRouterPidForPort with optional listProcPids injection, updates isModelRouterCommandLineForPort to detect model-router in argv[0] or argv[1], adds unit tests for discovery behavior, and uses the finder in reconcileModelRouter() to stop orphaned routers when a recorded PID doesn't own the port.
Model-router shutdown implementation and execution src/lib/actions/uninstall/run-plan.ts	Adds stopModelRouter helpers: reads persisted routerPid, validates existence and cmdline for port 4000 via isModelRouterCommandLineForPort, attempts SIGTERM then SIGKILL with disappearance checks, and falls back to lsof-based orphan scanning filtered by ownership and cmdline. Integrated into the uninstall "Stopping services" execution path.
Model-router shutdown test scenarios src/lib/actions/uninstall/run-plan.test.ts	Adds MODEL_ROUTER_CMDLINE constant and four Vitest cases verifying: stopping by stored routerPid, orphan discovery via lsof, refusal to stop foreign-owned processes, and cmdline-gating to avoid stopping non-matching processes. Tests assert PID killed state and expected log messages.

Sequence Diagram

sequenceDiagram
  participant User
  participant RunUninstallPlan
  participant stopModelRouter
  participant ps_cmd
  participant lsof_cmd
  participant OS
  User->>RunUninstallPlan: run uninstall
  RunUninstallPlan->>stopModelRouter: invoke stopModelRouter(paths,runtime)
  stopModelRouter->>ps_cmd: read cmdline for tracked PID / validate ownership
  ps_cmd-->>stopModelRouter: cmdline + owner info
  stopModelRouter->>OS: send SIGTERM
  OS-->>stopModelRouter: PID alive?
  stopModelRouter->>OS: send SIGKILL (if needed)
  stopModelRouter->>lsof_cmd: lsof -ti :4000 (fallback)
  lsof_cmd-->>stopModelRouter: candidate PIDs
  stopModelRouter->>ps_cmd: verify cmdline + ownership for candidates
  stopModelRouter-->>RunUninstallPlan: log results

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

"🐰 I sniffed the port and found a clue,
A router sitting on four-oh-oh-oh-two,
SIGTERM first, then SIGKILL if it stays,
Orphans found and politely chased away.
Uninstall hops on, the system breathes anew."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(uninstall): stop model router during teardown' clearly summarizes the main change: ensuring the model-router process is stopped as part of the uninstall sequence.
Linked Issues check	✅ Passed	The PR fully addresses the objective from issue #5169: it implements logic to detect and stop the model-router process during uninstall, handles both session PID and lsof discovery, validates command-line patterns including Python venv invocations, and adds comprehensive regression tests for session PID, orphan lsof, foreign-owner, and wrong-cmdline scenarios.
Out of Scope Changes check	✅ Passed	All changes are within scope: uninstall model-router cleanup, model-router process discovery, command-line validation enhancements, and onboard orphan recovery. The PR explicitly excludes Hermes gateway wait and API_SERVER_KEY injection mentioned in #5230 as out of scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Thabhelo]

Updated after @tyeth-ai-assisted's note on #5230:

• Python venv cmdline — isModelRouterCommandLineForPort now checks args[0] or args[1] (real-world: python /path/model-router proxy …)
• Onboard orphan recovery — findModelRouterPidForPort + reconcileModelRouter auto-stop when port 4000 is healthy but session PID is stale
• Tests use Python-interpreter cmdlines

Left Hermes gateway wait / API_SERVER_KEY injection from #5230 out of scope here — separate concern.

Thanks for the pointer — that cmdline gap would have made the uninstall stop silently no-op in production.

[wscurran]

✨ Thanks for adding the stop-model-router step to the uninstall plan with regression tests covering session PID, orphan lsof, foreign-owner, and wrong-cmdline cases. This proposes a way to stop the routed onboard model-router process during uninstall so port 4000 is freed before state cleanup.

---

Related open issues:

• #5169 uninstall doesn't remove model-router

[cv]

CI is green on the current head, CodeRabbit has no actionable comments on the latest review, and the PR includes focused regression coverage for the uninstall/model-router cleanup paths. I checked the available live E2E lanes: the nearest model-router E2E is not safely dispatchable against this fork head via a trusted target_ref workflow, so I am not running the secret-bearing regression workflow on the fork ref.