fix(cli): bind dashboard forward to all interfaces under WSL2#1503
fix(cli): bind dashboard forward to all interfaces under WSL2#1503Tempuskg wants to merge 4 commits intoNVIDIA:mainfrom
Conversation
The dashboard port forward bound to 127.0.0.1, which is not reachable from Windows browsers when NemoClaw runs inside WSL2. Detect WSL and bind to 0.0.0.0 instead so the dashboard is accessible via the WSL host IP. Also adds a VS Code/WSL-friendly URL to the dashboard access info that uses the WSL host IP from hostname -I. Signed-off-by: Darren McLeod <tempuskg@gmail.com>
📝 WalkthroughWalkthroughThe onboarding module is updated to improve dashboard forwarding and access information, particularly for WSL environments. Five new helper functions standardize dashboard URL generation, authentication token handling, and environment-specific guidance. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds WSL-specific handling for the CLI dashboard port-forward and access URL generation so the dashboard remains reachable from the Windows host when NemoClaw runs inside WSL2.
Changes:
- Introduces helpers to (a) choose a WSL-friendly forward bind target and (b) generate dashboard access URLs, including a WSL host-IP URL derived from
hostname -I. - Adds guidance-string helper(s) intended to improve user-facing instructions for dashboard access under WSL.
- Adds unit tests covering WSL bind target selection and WSL URL generation.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
bin/lib/onboard.js |
Adds new dashboard/WSL helper functions and exports them for use by the CLI. |
test/onboard.test.js |
Adds tests validating the new helper behavior for WSL binding and URL generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/onboard.test.js
Outdated
| release: "6.6.87.2-microsoft-standard-WSL2", | ||
| }); | ||
|
|
||
| expect(command).toContain("'forward' 'start' '--background' '0.0.0.0:18789' 'the-crucible'"); |
There was a problem hiding this comment.
This test assertion depends on the exact quoting format produced by openshellShellCommand (single quotes and argument ordering). That makes it brittle to unrelated changes in shell quoting. Consider asserting on the presence of the key argument(s) (e.g., 0.0.0.0:18789 and the sandbox name) rather than an exact quoted substring.
| expect(command).toContain("'forward' 'start' '--background' '0.0.0.0:18789' 'the-crucible'"); | |
| expect(command).toContain("forward"); | |
| expect(command).toContain("start"); | |
| expect(command).toContain("--background"); | |
| expect(command).toContain("0.0.0.0:18789"); | |
| expect(command).toContain("the-crucible"); |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 3759-3821: Ensure the runtime onboarding flow uses the new
helpers: update ensureDashboardForward() so it no longer computes the bind
target directly with resolveDashboardForwardTarget(chatUiUrl) or hardcode
127.0.0.1; instead call getDashboardForwardStartCommand(sandboxName, {
...options, /* preserve chatUiUrl when remote-host */ }) or otherwise use
getWslHostAddress/options to pick the correct forwardTarget so WSL binds to the
WSL host IP when appropriate; update getDashboardForwardStartCommand to
accept/preserve a chatUiUrl/remote-host flag if needed rather than always using
127.0.0.1. Also change printDashboard() to render the URLs from
getDashboardAccessInfo(sandboxName, options) and the lines from
getDashboardGuidanceLines(dashboardAccess) instead of directly calling
buildControlUiUrls(), so the VS Code/WSL URL generated by
buildAuthenticatedDashboardUrl shows up to users.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eef2eaaa-811a-467c-b806-4b874a755f12
📒 Files selected for processing (2)
bin/lib/onboard.jstest/onboard.test.js
Route ensureDashboardForward() through isWsl() so the port forward actually binds to 0.0.0.0 under WSL2. Update printDashboard() to use getDashboardAccessInfo() and getDashboardGuidanceLines() so users see the VS Code/WSL URL at onboard time. Split the brittle forward-command test assertion into individual toContain checks per Copilot review feedback, and make the createSandbox integration test WSL-aware. Signed-off-by: Darren McLeod <tempuskg@gmail.com>
|
✨ Thanks for submitting this fix, which proposes a way to bind the dashboard forward to all interfaces under WSL2, making it accessible from Windows browsers. |
cv
left a comment
There was a problem hiding this comment.
LGTM — security review PASS.
0.0.0.0bind correctly gated behindisWsl()(4-signal detection)- Non-WSL systems unaffected — stay on loopback
- WSL2 network exposure is an accepted trade-off, mitigated by token auth
hostname -Ioutput is display-only (console URL for user)- Good test coverage with injected options for deterministic WSL simulation
No concerns.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/onboard.test.ts`:
- Around line 401-413: The test is calling getDashboardForwardStartCommand which
triggers the openshell binary resolution and may call process.exit(1) when the
binary is missing; to make the test hermetic, mock/stub the openshell-resolution
path so it returns a valid dummy path (or stub the specific resolver function
that checks for the openshell binary) before invoking
getDashboardForwardStartCommand, or alternatively spy on process.exit to prevent
exiting and assert it was not called; ensure you restore the stub/spy after the
test. Use the symbol getDashboardForwardStartCommand to locate the call site and
the resolver function (the module that looks up the openshell binary) to apply
the mock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8c4ab6c8-a595-4a35-b2de-075105f86feb
📒 Files selected for processing (2)
src/lib/onboard.tstest/onboard.test.ts
| it("binds the dashboard forward to all interfaces under WSL", () => { | ||
| const command = getDashboardForwardStartCommand("the-crucible", { | ||
| env: { WSL_DISTRO_NAME: "Ubuntu" }, | ||
| platform: "linux", | ||
| release: "6.6.87.2-microsoft-standard-WSL2", | ||
| }); | ||
|
|
||
| expect(command).toContain("forward"); | ||
| expect(command).toContain("start"); | ||
| expect(command).toContain("--background"); | ||
| expect(command).toContain("0.0.0.0:18789"); | ||
| expect(command).toContain("the-crucible"); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify call chain that causes process exit when binary resolution fails.
rg -n -C2 "function getDashboardForwardStartCommand|openshellShellCommand|getOpenshellBinary|process\.exit\(1\)" src/lib/onboard.ts
rg -n -C3 "binds the dashboard forward to all interfaces under WSL|getDashboardForwardStartCommand\(" test/onboard.test.tsRepository: NVIDIA/NemoClaw
Length of output: 14327
🏁 Script executed:
#!/bin/bash
# Check test file imports and mocking infrastructure
head -50 test/onboard.test.ts
# Look for other tests that mock dependencies
rg -n "jest\.mock|sinon|mock|Mock|spy|Spy" test/onboard.test.ts | head -20
# Find CONTROL_UI_PORT constant
rg -n "CONTROL_UI_PORT" src/lib/onboard.ts | head -5
# Check resolveDashboardForwardTarget to understand the function
rg -n -A5 "function resolveDashboardForwardTarget" src/lib/onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 1997
🏁 Script executed:
#!/bin/bash
# Check if there's a vitest setup file or mocking configuration
rg -n "vi\.mock|vitest.*mock" test/
# Look for setup files
ls -la test/setup* test/*.config* 2>/dev/null || echo "No setup files found"
# Check if getDashboardForwardStartCommand is mocked in tests
rg -n "getDashboardForwardStartCommand" test/onboard.test.ts
# Look at the actual test and surrounding context more carefully
sed -n '395,420p' test/onboard.test.ts
# Check resolveDashboardForwardTarget function
rg -n -B2 -A8 "function resolveDashboardForwardTarget" src/lib/onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 1295
🏁 Script executed:
#!/bin/bash
# Look at resolveDashboardForwardTarget implementation
rg -n -B2 -A10 "function resolveDashboardForwardTarget" src/lib/onboard.ts
# Also check isWsl function since that's used in getDashboardForwardStartCommand
rg -n -B2 -A5 "function isWsl" src/lib/onboard.ts
# Verify the exact line 4205 context
sed -n '4202,4210p' src/lib/onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 479
🏁 Script executed:
#!/bin/bash
# Find isWsl function
rg -n -B2 -A8 "function isWsl" src/lib/onboard.ts
# Find resolveDashboardForwardTarget function
rg -n -B2 -A15 "function resolveDashboardForwardTarget" src/lib/onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Search more broadly for isWsl and resolveDashboardForwardTarget
rg "isWsl|resolveDashboardForwardTarget" src/lib/onboard.ts | head -20Repository: NVIDIA/NemoClaw
Length of output: 558
🏁 Script executed:
#!/bin/bash
# Check what's imported at top of onboard.ts
head -40 src/lib/onboard.ts | grep -E "require|import"
# Check if dashboard and platform modules are external or internal
ls -la src/lib/platform.ts src/lib/dashboard.ts 2>/dev/null || echo "Check dist or other locations"
# Look for any mocking or stubbing in the test for openshellShellCommand, getOpenshellBinary, or related
rg -n "openshellShellCommand|getOpenshellBinary|vitest\.mock\|vi\.mock" test/onboard.test.ts
# Check test setup - look at imports and any beforeEach/beforeAll
sed -n '1,100p' test/onboard.test.ts | grep -E "import|beforeEach|beforeAll|describe|vi\.|mock"Repository: NVIDIA/NemoClaw
Length of output: 1081
🏁 Script executed:
#!/bin/bash
# Look at the isWsl implementation - it's in platform module
cat src/lib/platform.ts | grep -A10 "function isWsl\|export.*isWsl"
# Also check dashboard.ts for resolveDashboardForwardTarget
cat src/lib/dashboard.ts | grep -A10 "function resolveDashboardForwardTarget\|export.*resolveDashboardForwardTarget"Repository: NVIDIA/NemoClaw
Length of output: 1031
🏁 Script executed:
#!/bin/bash
# Get full isWsl function to understand release parameter handling
rg -n -A20 "function isWsl" src/lib/platform.ts
# Verify resolveOpenshell function to confirm the exit behavior
rg -n -B2 -A10 "function resolveOpenshell" src/lib/onboard.tsRepository: NVIDIA/NemoClaw
Length of output: 961
🏁 Script executed:
#!/bin/bash
# Check resolveOpenshell function for what it does
rg -n -B5 -A15 "function resolveOpenshell" src/lib/onboard.ts
# Check if there are any other tests that successfully mock or work around this issue
rg -n -B3 -A3 "getDashboardForwardStartCommand\|openshellShellCommand\|getOpenshellBinary" test/onboard.test.ts | head -40Repository: NVIDIA/NemoClaw
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Let me check resolveOpenshell more carefully
rg -n "resolveOpenshell" src/lib/onboard.ts
# Look for the function definition
sed -n '430,460p' src/lib/onboard.ts
# Check if there's any vi.mock or mocking of external processes in the test file
grep -n "vi\.mock\|jest\.mock\|mock(" test/onboard.test.ts || echo "No vi.mock found"
# See what the test file actually imports from the compiled onboard
sed -n '12,50p' test/onboard.test.tsRepository: NVIDIA/NemoClaw
Length of output: 2291
🏁 Script executed:
#!/bin/bash
# Final verification - check that getDashboardForwardStartCommand indeed calls openshellShellCommand
sed -n '4200,4210p' src/lib/onboard.ts
# Verify the call chain one more time from compiled dist
grep -A3 "function getDashboardForwardStartCommand" dist/lib/onboard.js 2>/dev/null || echo "dist not available, checking source"
# Check if there are any other tests in the file that deal with openshell binary mocking
rg -n "openshell|OPENSHELL_BIN|resolveOpenshell" test/onboard.test.tsRepository: NVIDIA/NemoClaw
Length of output: 6107
🏁 Script executed:
#!/bin/bash
# Check one of the existing tests that properly mocks openshell to understand the pattern
sed -n '1230,1260p' test/onboard.test.tsRepository: NVIDIA/NemoClaw
Length of output: 1325
🏁 Script executed:
#!/bin/bash
# Verify the test at 401-413 doesn't use the fake binary pattern like other tests
sed -n '395,430p' test/onboard.test.tsRepository: NVIDIA/NemoClaw
Length of output: 1582
WSL forward-bind unit test is currently host-dependent and fails CI.
At line 402, getDashboardForwardStartCommand() triggers openshell binary resolution and calls process.exit(1) when the binary is unavailable, making this test non-hermetic.
🔧 Suggested stabilization (mock the openshell binary dependency)
it("binds the dashboard forward to all interfaces under WSL", () => {
+ const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-wsl-forward-"));
+ const fakeBin = path.join(tmpDir, "bin");
+ fs.mkdirSync(fakeBin, { recursive: true });
+ fs.writeFileSync(path.join(fakeBin, "openshell"), "#!/usr/bin/env bash\nexit 0\n", {
+ mode: 0o755,
+ });
+ const previousPath = process.env.PATH;
+ process.env.PATH = `${fakeBin}:${previousPath || ""}`;
+ try {
const command = getDashboardForwardStartCommand("the-crucible", {
env: { WSL_DISTRO_NAME: "Ubuntu" },
platform: "linux",
release: "6.6.87.2-microsoft-standard-WSL2",
});
expect(command).toContain("forward");
expect(command).toContain("start");
expect(command).toContain("--background");
expect(command).toContain("0.0.0.0:18789");
expect(command).toContain("the-crucible");
+ } finally {
+ process.env.PATH = previousPath;
+ fs.rmSync(tmpDir, { recursive: true, force: true });
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("binds the dashboard forward to all interfaces under WSL", () => { | |
| const command = getDashboardForwardStartCommand("the-crucible", { | |
| env: { WSL_DISTRO_NAME: "Ubuntu" }, | |
| platform: "linux", | |
| release: "6.6.87.2-microsoft-standard-WSL2", | |
| }); | |
| expect(command).toContain("forward"); | |
| expect(command).toContain("start"); | |
| expect(command).toContain("--background"); | |
| expect(command).toContain("0.0.0.0:18789"); | |
| expect(command).toContain("the-crucible"); | |
| }); | |
| it("binds the dashboard forward to all interfaces under WSL", () => { | |
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-wsl-forward-")); | |
| const fakeBin = path.join(tmpDir, "bin"); | |
| fs.mkdirSync(fakeBin, { recursive: true }); | |
| fs.writeFileSync(path.join(fakeBin, "openshell"), "#!/usr/bin/env bash\nexit 0\n", { | |
| mode: 0o755, | |
| }); | |
| const previousPath = process.env.PATH; | |
| process.env.PATH = `${fakeBin}:${previousPath || ""}`; | |
| try { | |
| const command = getDashboardForwardStartCommand("the-crucible", { | |
| env: { WSL_DISTRO_NAME: "Ubuntu" }, | |
| platform: "linux", | |
| release: "6.6.87.2-microsoft-standard-WSL2", | |
| }); | |
| expect(command).toContain("forward"); | |
| expect(command).toContain("start"); | |
| expect(command).toContain("--background"); | |
| expect(command).toContain("0.0.0.0:18789"); | |
| expect(command).toContain("the-crucible"); | |
| } finally { | |
| process.env.PATH = previousPath; | |
| fs.rmSync(tmpDir, { recursive: true, force: true }); | |
| } | |
| }); |
🧰 Tools
🪛 GitHub Check: checks
[failure] 402-402: [cli] test/onboard.test.ts > onboard helpers > binds the dashboard forward to all interfaces under WSL
Error: process.exit unexpectedly called with "1"
❯ getOpenshellBinary src/lib/onboard.ts:454:13
❯ openshellShellCommand src/lib/onboard.ts:461:22
❯ getDashboardForwardStartCommand src/lib/onboard.ts:4206:13
❯ test/onboard.test.ts:402:21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/onboard.test.ts` around lines 401 - 413, The test is calling
getDashboardForwardStartCommand which triggers the openshell binary resolution
and may call process.exit(1) when the binary is missing; to make the test
hermetic, mock/stub the openshell-resolution path so it returns a valid dummy
path (or stub the specific resolver function that checks for the openshell
binary) before invoking getDashboardForwardStartCommand, or alternatively spy on
process.exit to prevent exiting and assert it was not called; ensure you restore
the stub/spy after the test. Use the symbol getDashboardForwardStartCommand to
locate the call site and the resolver function (the module that looks up the
openshell binary) to apply the mock.
Summary
The dashboard port-forward command binds to
127.0.0.1, making it unreachable from Windows browsers when NemoClaw runs inside WSL2. This PR detects WSL and binds to0.0.0.0instead, and adds a VS Code/WSL-friendly URL using the WSL host IP fromhostname -I.Related Issue
None.
Changes
getDashboardForwardStartCommand()helper that detects WSL viaisWsl()and binds to0.0.0.0:18789instead of the default loopback address.getDashboardAccessInfo()helper that appends aVS Code/WSLdashboard URL entry when running under WSL.buildAuthenticatedDashboardUrl(),getWslHostAddress(),getDashboardForwardPort(), andgetDashboardGuidanceLines()helpers.Type of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.make docsbuilds without warnings. (for doc-only changes)52/52 onboard tests pass. Full suite: 960 passed with 6 pre-existing failures that also occur on upstream
main(cli.test.js, install-preflight.test.js, nemoclaw-cli-recovery.test.js).Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Signed-off-by: Darren Kattan darren@kattan.dev
Summary by CodeRabbit
New Features
Bug Fixes
Tests