fix(onboard): bypass proxies for loopback readiness

[HOYALIM]

Summary

Loopback HTTP readiness checks could still be routed through a host proxy despite the existing NO_PROXY augmentation, causing a running Ollama service to be reported unavailable. Loopback URLs now use a bounded native Node probe that does not consult proxy settings, while non-loopback URLs retain the validated curl path.

Related Issue

Fixes #6763

Changes

• Probe exact loopback hosts (localhost, 127.0.0.1, and ::1) through a short-lived Node HTTP or HTTPS client with a one-second request timeout.
• Keep non-loopback readiness probes on the existing validated curl path.
• Add a process-level regression test with a live loopback server and a failing http_proxy, plus routing-contract tests for both probe paths.

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)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: this restores the documented loopback proxy-bypass contract without changing commands or output
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: author review confirmed the direct path is restricted to exact loopback hostnames, the URL is passed as argv rather than interpolated into code, and non-loopback requests retain curl validation
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli src/lib/core/wait.test.ts src/lib/onboard/install-ollama-macos.test.ts src/lib/onboard/ollama-startup.test.ts (33 tests passed)
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — not applicable to this focused readiness probe change; npm run typecheck:cli, Biome, test-conditional scan, and npm run check:diff passed
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• 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: Ho Lim subhoya@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Improved readiness checks for local HTTP and HTTPS endpoints.
  • Localhost checks now bypass configured HTTP proxies for more reliable results.
  • Non-local endpoints continue using the existing network validation behavior.
  • Added handling for common loopback address formats, including IPv6 localhost.

[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.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

📝 Walkthrough

Walkthrough

waitForHttp now uses a native Node HTTP(S) probe for loopback URLs, avoiding proxy interference. Non-loopback URLs continue using the validated curl argument path, with tests covering both behaviors.

Changes

Loopback HTTP probe

Layer / File(s)	Summary
Native loopback probe src/lib/core/wait.ts	Adds loopback URL detection and a Node-based HTTP(S) readiness probe selected by waitForHttp.
Probe behavior coverage src/lib/core/wait.test.ts	Tests proxy-independent loopback checks, native probe invocation, and curl usage for non-loopback HTTPS targets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant waitForHttp
  participant NodeProbe
  participant LoopbackServer
  waitForHttp->>NodeProbe: pass loopback URL and timeout
  NodeProbe->>LoopbackServer: send HTTP(S) GET
  LoopbackServer-->>NodeProbe: return response status
  NodeProbe-->>waitForHttp: report readiness result

Loading

Possibly related PRs

• NVIDIA/NemoClaw#4215: Updates waitForHttp loopback probing to avoid proxy interference using a different implementation.

Suggested labels: bug-fix, platform: macos, area: networking

Suggested reviewers: copilot, apurvvkumaria, cv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: bypassing proxies for loopback readiness checks.
Linked Issues check	✅ Passed	The changes implement loopback proxy bypass for readiness probes while preserving the existing non-loopback curl path, matching #6763.
Out of Scope Changes check	✅ Passed	The patch stays focused on the readiness probe behavior and its tests, with no clear unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: gpu-double-onboard
Optional E2E: ollama-auth-proxy

Dispatch hint: gpu-double-onboard

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: target/main
Head: HEAD
Confidence: high

Required E2E

• gpu-double-onboard: Exercises real install.sh onboarding with the Ollama provider, including host loopback readiness, the persisted auth proxy, sandbox inference.local connectivity, and re-onboarding.

Optional E2E

• ollama-auth-proxy: Provides an adjacent real-host validation of Ollama and localhost auth-proxy HTTP behavior, although it does not invoke the CLI waitForHttp readiness helper.

New E2E recommendations

• installer/onboarding (medium): The new direct-probe branch is covered by a unit test but has no non-GPU end-to-end onboarding scenario that sets an unreachable HTTP proxy and verifies local Ollama readiness succeeds through the CLI.
  • Suggested test: Add a credential-free Linux local-Ollama onboarding E2E that sets http_proxy/HTTP_PROXY to an unreachable proxy, starts or installs Ollama on loopback, and verifies onboarding completes and inference.local remains reachable.

Dispatch hint

• Workflow: .github/workflows/e2e.yaml
• jobs input: gpu-double-onboard

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/core/wait.ts (1)

293-307: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate hostname-normalization logic vs. curl-args.ts.

isLoopbackHttpUrl's hostname normalization (bracket-strip, trailing-dot-strip, lowercase) duplicates normalizeHostname in src/lib/adapters/http/curl-args.ts (lines 167-171). Since one governs loopback classification and the other governs curl's --resolve/SSRF validation, a future fix to one path (e.g. handling a new normalization edge case) could silently diverge from the other. Consider exporting normalizeHostname and reusing it here.

♻️ Proposed fix to reuse the shared normalizer

+import { normalizeHostname } from "../adapters/http/curl-args";
+
 function isLoopbackHttpUrl(rawUrl: string): boolean {
   try {
     const url = new URL(rawUrl);
-    const hostname = url.hostname
-      .replace(/^\[(.*)\]$/u, "$1")
-      .replace(/\.$/u, "")
-      .toLowerCase();
+    const hostname = normalizeHostname(url.hostname);
     return (
       (url.protocol === "http:" || url.protocol === "https:") &&
       (hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1")
     );
   } catch {
     return false;
   }
 }

And in curl-args.ts, export the existing helper:

-function normalizeHostname(hostname: string): string {
+export function normalizeHostname(hostname: string): string {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/core/wait.ts` around lines 293 - 307, Export the existing
normalizeHostname helper from curl-args.ts and update isLoopbackHttpUrl to reuse
it instead of performing its own bracket removal, trailing-dot removal, and
lowercasing. Preserve the current protocol and loopback-host checks while
removing the duplicated normalization logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/lib/core/wait.ts`:
- Around line 293-307: Export the existing normalizeHostname helper from
curl-args.ts and update isLoopbackHttpUrl to reuse it instead of performing its
own bracket removal, trailing-dot removal, and lowercasing. Preserve the current
protocol and loopback-host checks while removing the duplicated normalization
logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e4d85d4a-0779-4bfa-8c56-92c38e866bb3

📥 Commits

Reviewing files that changed from the base of the PR and between 3de1de6 and 60edbd3.

📒 Files selected for processing (2)

• src/lib/core/wait.test.ts
• src/lib/core/wait.ts