fix: restore routed inference and connect UX

[kjw3]

Summary

• remove inference.local from the sandbox NO_PROXY override so routed inference still goes through the OpenShell proxy path
• stop pre-starting a duplicate port forward in nemoclaw <sandbox> connect
• add regression coverage for both behaviors

Root Cause

A plain OpenShell sandbox on openshell 0.0.16 could resolve https://inference.local/v1/models, but a NemoClaw runtime sandbox could not. The difference was NemoClaw's runtime env override in scripts/nemoclaw-start.sh, which added inference.local to NO_PROXY and forced direct DNS resolution inside the sandbox. That bypassed the OpenShell proxy path that actually handles inference.local.

Separately, nemoclaw <sandbox> connect was calling openshell forward start --background 18789 ... before openshell sandbox connect, which produced noisy duplicate-forward errors on already-connected sandboxes.

Validation

npx vitest run test/service-env.test.js test/cli.test.js
npx eslint bin/nemoclaw.js test/service-env.test.js test/cli.test.js
npx tsc -p jsconfig.json --noEmit
bash -n scripts/nemoclaw-start.sh

Real-world repro

• plain OpenShell sandbox: curl -sS https://inference.local/v1/models worked
• NemoClaw runtime sandbox with NO_PROXY=...,inference.local,...: curl: (6) Could not resolve host: inference.local
• after removing inference.local from NO_PROXY, routed inference stays on the proxy path as intended

Summary by CodeRabbit

• Changes
  • Sandbox connect no longer starts a background port-forward, streamlining the connection flow and avoiding unexpected background processes.
  • Proxy bypass list updated: inference.local will now be routed through the proxy instead of being bypassed, changing which hosts are reached directly.
  • Setup prompt wording refined when multiple local inference options are detected for clearer guidance.
• Tests
  • Added/updated tests to verify connect behavior and the updated proxy environment values.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39ecf4e5-a66a-4228-a240-34fbcf863a63

📥 Commits

Reviewing files that changed from the base of the PR and between 8c27492 and 472a4a8.

📒 Files selected for processing (1)

• test/cli.test.js

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

• test/cli.test.js

---

📝 Walkthrough

Walkthrough

Removed the automatic background OpenShell port-forward before sandbox connect and removed inference.local from the proxy-bypass list; tests and an onboarding prompt message were updated to match these behavior changes.

Changes

Cohort / File(s)	Summary
Sandbox Connection bin/nemoclaw.js	Removed the pre-start of an OpenShell port-forward (openshell forward start --background 18789 <sandboxName>) from sandboxConnect(); function still calls ensureLiveSandboxOrExit() and runs openshell sandbox connect.
Proxy Configuration Script scripts/nemoclaw-start.sh	Updated _NO_PROXY_VAL to omit inference.local; exported NO_PROXY/no_proxy now contain only localhost,127.0.0.1,::1,${PROXY_HOST}.
CLI Tests test/cli.test.js	Added test for alpha connect verifying openshell sandbox get and openshell sandbox connect are invoked and that no forward start --background 18789 alpha command is emitted.
Service Environment Tests test/service-env.test.js	Adjusted assertions and expected strings so NO_PROXY/no_proxy values and persisted ~/.bashrc/~/.profile lines do not include inference.local.
Onboarding Prompt & Test bin/lib/onboard.js, test/onboard-selection.test.js	Reworded prompt in setupNim(gpu) from “Press Enter to keep NVIDIA Endpoints.” to “Select one explicitly if you want to use it.” and removed the matching test assertion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through scripts and tests today,
Snipped a forward that no longer led the way.
Kept proxies tight to loopback and gate,
Tweaked a prompt, adjusted a test's fate.
A small, precise nibble—now code is okay.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: restore routed inference and connect UX' accurately describes the main changes: removing inference.local from NO_PROXY to restore routed inference, and removing duplicate port-forward in connect to fix UX issues.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/inference-proxy-and-connect-forward

---

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

[coderabbitai]

🧹 Nitpick comments (1)

test/cli.test.js (1)

205-205: Broaden the negative assertion to avoid sandbox-name coupling.

The current check only rejects forward start --background 18789 alpha. Consider rejecting any forward start --background 18789 invocation so the test still fails if argument ordering/sandbox resolution changes.

Suggested tweak

-    expect(calls).not.toContain("forward start --background 18789 alpha");
+    expect(calls).not.toContain("forward start --background 18789");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/cli.test.js` at line 205, Replace the exact-string negative assertion so
the test rejects any invocation beginning with the forward command and port.
Instead of expect(calls).not.toContain("forward start --background 18789
alpha"), assert that no entry in the calls array starts with "forward start
--background 18789" (for example: expect(calls.some(c => c.startsWith("forward
start --background 18789"))).toBe(false)), targeting the existing calls variable
to avoid coupling to the sandbox name or argument ordering.

🤖 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/cli.test.js`:
- Line 205: Replace the exact-string negative assertion so the test rejects any
invocation beginning with the forward command and port. Instead of
expect(calls).not.toContain("forward start --background 18789 alpha"), assert
that no entry in the calls array starts with "forward start --background 18789"
(for example: expect(calls.some(c => c.startsWith("forward start --background
18789"))).toBe(false)), targeting the existing calls variable to avoid coupling
to the sandbox name or argument ordering.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dd6d8b9a-1560-4038-aac8-3e5670a4d049

📥 Commits

Reviewing files that changed from the base of the PR and between 5f692e5 and 11b62d5.

📒 Files selected for processing (4)

• bin/nemoclaw.js
• scripts/nemoclaw-start.sh
• test/cli.test.js
• test/service-env.test.js

💤 Files with no reviewable changes (1)

• bin/nemoclaw.js

[ericksoa]

Reviewed for regressions. NO_PROXY fix is correct and well-tested — inference.local should route through the OpenShell proxy, not bypass it. Port forward removal and prompt cleanup look clean. One note: confirm OpenShell's sandbox connect reliably re-establishes port 18789 forwarding on reconnect (not just first connect). Otherwise, ship it.