fix(onboard): use 127.0.0.1 instead of localhost for local inference detection#1978
Conversation
…detection Using `localhost` can resolve to IPv6 (::1) on some systems, causing health checks and probe commands to fail. Replace all host-side localhost references with explicit 127.0.0.1 for deterministic IPv4 behavior. Changes: - getLocalProviderValidationBaseUrl: localhost → 127.0.0.1 - getLocalProviderHealthEndpoint: localhost → 127.0.0.1 - validateLocalProvider error messages: localhost → 127.0.0.1 - getOllamaModelOptions curl: localhost → 127.0.0.1 - getOllamaWarmupCommand: localhost → 127.0.0.1 - getOllamaProbeCommand: localhost → 127.0.0.1 - onboard.ts detection curls: localhost → 127.0.0.1 - All test expectations updated to match Based on the original work in #1716 by @chandler-barlow, re-applied on the current TypeScript codebase after the JS→TS migration made the original PR unrebaseable. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Chandler Barlow <cbarlow@nvidia.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaced Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Missed in the initial commit: - src/lib/nim.ts: health check curls used localhost - src/lib/nim.test.ts: test mocks matched localhost - test/cli.test.ts: status output assertion expected localhost - test/e2e/test-gpu-e2e.sh: host-side Ollama curls used localhost Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Chandler Barlow <cbarlow@nvidia.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/test-gpu-e2e.sh (1)
553-553: Update stale summary text to match the new address choice.The final summary still says “starts Ollama on localhost”, which now conflicts with the script’s
127.0.0.1usage and updated phase logs.Suggested text-only tweak
-echo " - Onboard: starts Ollama on localhost, starts auth proxy, pulls model, creates sandbox" +echo " - Onboard: starts Ollama on 127.0.0.1, starts auth proxy, pulls model, creates sandbox"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/test-gpu-e2e.sh` at line 553, Update the summary string emitted by the script to reflect the actual address used; replace the phrase "starts Ollama on localhost" in the echoed message (the echo call that prints " - Onboard: starts Ollama on localhost, starts auth proxy, pulls model, creates sandbox") so it references "127.0.0.1" (or a neutral term like "loopback 127.0.0.1") to match the script’s use of 127.0.0.1 and the updated phase logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/test-gpu-e2e.sh`:
- Line 553: Update the summary string emitted by the script to reflect the
actual address used; replace the phrase "starts Ollama on localhost" in the
echoed message (the echo call that prints " - Onboard: starts Ollama on
localhost, starts auth proxy, pulls model, creates sandbox") so it references
"127.0.0.1" (or a neutral term like "loopback 127.0.0.1") to match the script’s
use of 127.0.0.1 and the updated phase logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c8fb16d-8c56-4dab-ae18-e06421ffbae4
📒 Files selected for processing (4)
src/lib/nim.test.tssrc/lib/nim.tstest/cli.test.tstest/e2e/test-gpu-e2e.sh
✅ Files skipped from review due to trivial changes (2)
- src/lib/nim.ts
- test/cli.test.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Automated PR review summaryReviewed PR #1978: fix(onboard): use 127.0.0.1 instead of localhost for local inference detection Recommendation
Installation and setup findings
What was validated
Failing tests and unresolved impact
Passing tests and why they matteredPassing test 1: Local provider validation now probes 127.0.0.1 for Ollama
Passing test 2: Ollama model query, warmup, and probe helpers consistently use 127.0.0.1
Passing test 3: vLLM validation URLs and curl-based health probe use 127.0.0.1
Passing test 4: Changed inference paths in onboard/local-inference contain the expected 127.0.0.1 replacements
Bottom line
|
Refresh user-facing docs against the 34 commits merged between v0.0.17 and v0.0.18. Highlights: - Replace the Ollama 0.0.0.0 binding guidance with the new authenticated reverse proxy on 127.0.0.1:11435 (#1922). - Document the compatible-endpoint provider defaulting to /v1/chat/completions and the NEMOCLAW_PREFERRED_API=openai-responses opt-in (#1984). - Add the new nemoclaw upgrade-sandboxes command with --check, --auto, and --yes flags (#1943). - Note the cross-sandbox messaging overlap warning and 409 detection in nemoclaw <name> status (#1953). - Document the messaging-token rotation auto-rebuild flow (#1967). - Cover new troubleshooting entries for the Ollama auth proxy, IPv6 localhost resolution, orphan SSH port-forward cleanup on re-onboard, and rotated messaging credentials (#1978, #1950). - Note tar failure exit code for nemoclaw debug --output (#1770) and the orphaned openshell process cleanup in nemoclaw uninstall (#1940). Also: - Extend docs/.docs-skip to exclude the experimental sandbox-mgmt shields and config commands (#1976). - Fix a sphinx-autobuild infinite rebuild loop in docs/conf.py by writing docs/project.json only when its contents change. - Bump the docs version switcher preferred entry to 0.0.18. - Regenerate nemoclaw-user-* agent skills from docs/. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made-with: Cursor
## Summary Refresh user-facing documentation against the 34 commits merged between v0.0.17 and v0.0.18, bump the docs version switcher to v0.0.18, and fix a `sphinx-autobuild` infinite-rebuild loop triggered by `docs/conf.py`. ## Changes - **Ollama authenticated reverse proxy** (#1922): Replace the `0.0.0.0:11434` guidance in `docs/inference/use-local-inference.md` with the new token-gated proxy on `127.0.0.1:11435`, including persisted token, health-check exemption, and sandbox provider wiring. Replace the matching troubleshooting entry in `docs/reference/troubleshooting.md`. - **Compatible-endpoint default API path** (#1984): Document that the compatible-endpoint provider now defaults to `/v1/chat/completions` and update `NEMOCLAW_PREFERRED_API` to describe `openai-responses` as the opt-in instead of `openai-completions`. Updates in `use-local-inference.md`, `switch-inference-providers.md`, and `troubleshooting.md`. - **`nemoclaw upgrade-sandboxes` command** (#1943): Add a new reference entry in `docs/reference/commands.md` covering `--check`, `--auto`, and `--yes` flags. - **Messaging token rotation auto-rebuild** (#1967, #1953): Note the automatic rebuild behavior and cross-sandbox overlap warning in `docs/deployment/set-up-telegram-bridge.md`, `commands.md`, and `troubleshooting.md`. - **Other troubleshooting additions**: - `localhost` → `127.0.0.1` IPv6 note (#1978) - Orphan SSH port-forward cleanup on re-onboard (#1950) - Orphan `openshell` process cleanup in `nemoclaw uninstall` (#1940) - Non-zero exit on tar failure in `nemoclaw debug --output` (#1770) - **Skip list**: Extend `docs/.docs-skip` to exclude the experimental sandbox-mgmt shields and config commands feature (#1976), which was explicitly merged as not-yet-documented. - **Build stability**: `docs/conf.py` now writes `docs/project.json` only when contents change, so `make docs-live` / `sphinx-autobuild` no longer detects its own generated file as a source change and enters an infinite rebuild loop. - **Version switcher**: Bump `docs/versions1.json` and `docs/project.json` preferred entry to v0.0.18 so this refresh renders under the new version. - **Agent skills**: Regenerate `nemoclaw-user-*` skills from `docs/` with `scripts/docs-to-skills.py`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes (ran via pre-commit hook on staged files) - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] 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) ## AI Disclosure - [x] AI-assisted — tool: Cursor --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added `nemoclaw upgrade-sandboxes` command to rebuild sandboxes when base-image digests change. * Introduced authenticated reverse proxy for local Ollama inference with token-based access control. * Automatic sandbox backup, recreation, and restore when messaging credentials are updated. * Cross-sandbox messaging token overlap detection with status warnings. * **Improvements** * Compatible-endpoint provider now defaults to `/v1/chat/completions` API path. * Enhanced troubleshooting documentation with new diagnostics sections. * **Documentation** * Updated onboarding and configuration guides. * Expanded version documentation to 0.0.18. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
NVIDIA#1978) The argv migration kept the old localhost URLs for Ollama and vLLM curl probes, but main switched to 127.0.0.1 in commit dbc805c. The onboard-selection tests mock 127.0.0.1, so the mismatch caused two test failures.
Summary
Supersedes #1716.
Using
localhostcan resolve to IPv6 (::1) on some systems, causing local inference health checks, probe commands, and model queries to fail silently. This replaces all host-sidelocalhostreferences with explicit127.0.0.1for deterministic IPv4 behavior.Based on the original work in #1716 by @chandler-barlow — the JS→TS migration (#1673) made the original PR unrebaseable, so the change has been re-applied on the current codebase with attribution.
Changes
src/lib/local-inference.ts: validation URLs, health endpoints, error messages, model queries, warmup/probe commandssrc/lib/nim.ts: NIM health check curlssrc/lib/onboard.ts: Ollama and vLLM detection curlssrc/lib/local-inference.test.ts: updated expectationssrc/lib/nim.test.ts: updated mock matcherstest/cli.test.ts: updated status output assertiontest/onboard-selection.test.ts: updated mock matcherstest/e2e/test-gpu-e2e.sh: host-side Ollama curlsTest plan
http://localhost:11434orhttp://localhost:8000references remain in src/ test/ scripts/Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests