fix(model-router): stop/recover router on uninstall & onboard; extend gateway health wait - #3
Closed
tyeth-ai-assisted wants to merge 3 commits into
Closed
fix(model-router): stop/recover router on uninstall & onboard; extend gateway health wait#3tyeth-ai-assisted wants to merge 3 commits into
tyeth-ai-assisted wants to merge 3 commits into
Conversation
Fixes NVIDIA#5169: uninstall.sh left a running model-router process on port 4000, causing reinstall to fail with "Port 4000 already has a healthy router endpoint, but its credential state is unknown." Changes: - Add stop-model-router action to the Stopping services uninstall step; reads routerPid from onboard-session.json and falls back to lsof :4000 scan, mirroring the Ollama auth proxy stop pattern (NVIDIA#2759) - Raise wait_for_hermes_gateway_internal attempts from 45 to 60 in agents/hermes/start.sh so the gateway health wait budget covers ~120s of effective curl timeout (was ~90s) on slow WSL2 hosts Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
When onboarding starts fresh (no recorded routerPid) but a model-router process is already healthy on port 4000 from a previous failed install, reconcileModelRouter now scans /proc to find and stop the orphan before starting a new router — instead of throwing "credential state is unknown" and requiring a manual stop-and-retry. See issue NVIDIA#5169. Adds findModelRouterPidForPort to model-router-process.ts with injectable deps (listProcPids, readProcCommandLine) for unit testing, guarded by the existing isModelRouterCommandLineForPort check so non-router services on the same port are never killed. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…d ps (NVIDIA#5169) model-router runs as a Python venv entry-point script, so the OS interposes the interpreter: /proc cmdline is 'python /path/model-router proxy --port …' with args[0]=python and args[1]=model-router. The previous isModelRouterCommandLineForPort check only tested args[0], causing both the uninstall orphan-stop and the onboard orphan-recovery to silently skip the running process. Fix: - isModelRouterCommandLineForPort: check args[0] OR args[1] basename === model-router - isModelRouterPid (uninstall run-plan): same dual-position check via ps - Update all relevant tests to use the realistic Python-interpreter cmdline (python /path/model-router proxy --port 4000) to exercise the fixed path Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
Retargeted to upstream as NVIDIA#5230 (same |
tyeth-ai-assisted
pushed a commit
that referenced
this pull request
Jul 16, 2026
## Summary Issue NVIDIA#5667: onboarding a Deep Agents / OpenAI-compatible sandbox without an explicit NEMOCLAW_MODEL recorded the model id as 'nvidia/nvidia/nemotron-3-super-v3' (doubled 'nvidia/' namespace). Root cause was the default fallback constant HOSTED_INFERENCE_MODEL in src/lib/onboard/providers.ts, which was hardcoded with a duplicated namespace segment (dev-triage hypothesis #3). When NEMOCLAW_MODEL is unset, stageHostedInferenceSourceSecretEnv() falls back to this constant and persists it into NEMOCLAW_MODEL / NEMOCLAW_COMPAT_MODEL, surfacing the doubled prefix everywhere (status, build ARG, dcode header). Fixed by changing the constant to the canonical single-prefix id 'nvidia/nemotron-3-super-v3'. Scope limited to the default constant; CI workflow YAML and e2e fixtures set NEMOCLAW_MODEL explicitly and are intentionally untouched. ## Related Issue Fixes NVIDIA#5667 ## Changes - Automated Claude Code fix selected by `auto_fix/auto_fix_recent_issues.py`. - See the commits on this branch for the exact file-level changes. ## Type of Change - [x] 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 <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [ ] 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](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Verification details reported by automation: - Codex review (independent, advisory) posted as a PR comment. - Addressed from codex review: Addressed the reviewer's completeness findings by sweeping the doubled 'nvidia/nvidia/nemotron-3-super-v3' id to the canonical 'nvidia/nemotron-3-super-v3' everywhere it represented this model — not just the original constant. The earlier claim that CI/fixtures were 'intentionally untouched' was wrong: those defaults reintroduced the malformed id. Changed: the hosted CI exports in .github/workflows/{e2e-script,nightly-e2e,e2e-vitest-scenarios}.yaml; the e2e fixture default DEFAULT_HOSTED_INFERENCE_MODEL (test/e2e-scenario/fixtures/hosted-inference.ts) and the live-helper/shell-fixture fallbacks (agent-turn-latency-helpers, hermes-discord, hermes-slack-e2e-helpers, rebuild-hermes, upgrade-stale-sandbox-helpers, ci-compatible-inference.sh, test-rebuild-*.sh, test-upgrade-stale-sandbox.sh); the onboard-probes probe-payload test literal; and the workflow-content guard assertions in test/e2e-script-workflow.test.ts that had locked in the doubled value. Finding #3 (regression test should use NEMOCLAW_PROVIDER_KEY) was rejected as incorrect: stageHostedInferenceSourceSecretEnv() — the function that records the model and was the root cause — gates exclusively on NVIDIA_INFERENCE_API_KEY (HOSTED_INFERENCE_SOURCE_ENV); NEMOCLAW_PROVIDER_KEY is a provider-credential hint used only by the Model Router / Hermes / deepagents config paths and never triggers model staging, so the test correctly exercises the real bug path. Verified: rebuilt dist and ran the issue-5667 regression test, e2e-script-workflow guard tests, onboard-probes, and hosted-inference support tests — 67 tests pass. - Added focused regression test test/issue-5667-hosted-inference-model-namespace.test.ts that exercises the real exported HOSTED_INFERENCE_MODEL constant and the real stageHostedInferenceSourceSecretEnv() staging path (no mocking of the unit under test). - Built dist from the unfixed source and confirmed the new test fails red: NEMOCLAW_MODEL staged as 'nvidia/nvidia/nemotron-3-super-v3'. - Applied the one-line fix to src/lib/onboard/providers.ts, rebuilt dist, and confirmed the test passes green (2/2). - Ran related suites (credential-exposure, onboard-selection, onboard-probes, e2e-script-workflow) — 129 tests pass — confirming no regression and that the explicitly-overridden CI workflow values are unaffected. - `npm install --ignore-scripts` - `npm run build:cli` - `./node_modules/.bin/vitest run test/issue-5667-hosted-inference-model-namespace.test.ts --project cli` - `./node_modules/.bin/vitest run test/credential-exposure.test.ts test/onboard-selection.test.ts src/lib/inference/onboard-probes.test.ts test/issue-5667-hosted-inference-model-namespace.test.ts` - `./node_modules/.bin/vitest run test/e2e-script-workflow.test.ts` --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Jason Ma <jama@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected hosted-inference Nemotron model identifiers to consistently use the proper single-prefix format (removing duplicated `nvidia/nvidia/`) across onboarding, live scenarios, and CI/E2E workflows. * Improved Deep Agents hosted-inference staging by adding a safe fallback for provider-key handling. * **Tests** * Updated E2E/workflow expectations and fixtures to match the corrected model identifiers. * Added/adjusted regression tests to prevent duplicated model namespaces and verify Deep Agents staging behavior. * **Documentation** * Fixed the Model Router pool example for Nemotron-3 Nano. * **Security/Quality** * Expanded secret redaction and direct-credential-env checks to include `NEMOCLAW_PROVIDER_KEY`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jason Ma <jama@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
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
2 participants
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 the model-router failing to stop/recover during uninstall and on fresh onboard, plus a longer Hermes gateway health wait. Rebased clean onto current upstream
main(3 commits).Commits
Testing
npm install && npm test && make checkunder Node 22 before merge/retarget.🤖 Generated with Claude Code