Skip to content

fix(dashboard): keep loopback dashboard URL on WSL2#6181

Merged
cv merged 12 commits into
mainfrom
fix/dashboard-url-wsl-loopback
Jul 3, 2026
Merged

fix(dashboard): keep loopback dashboard URL on WSL2#6181
cv merged 12 commits into
mainfrom
fix/dashboard-url-wsl-loopback

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

On WSL2, nemohermes <sandbox> dashboard-url --quiet and the onboard banner led with the WSL VM's host IP (172.x.x.x) instead of 127.0.0.1. That host IP was originally added (#1503) as a fallback for setups where WSL localhost forwarding is unavailable, but a later refactor promoted it to the primary accessUrl. This restores loopback as the primary URL while keeping the WSL host IP available as an explicit fallback.

Related Issue

Fixes #6171

Changes

  • src/lib/dashboard/contract.ts: buildChain() now always uses http://127.0.0.1:<port> as the primary accessUrl on WSL, and exposes the WSL host IP through a new fallbackUrls field instead of as the primary URL. corsOrigins still includes the fallback origin, and forwardTarget/bindAddress/shouldDisableDeviceAuth are unchanged (the forward still binds 0.0.0.0).
  • src/lib/onboard/dashboard.ts: the onboard banner prints the loopback URL first, then lists the WSL host IP under a WSL fallback label; the WSL host-address probe now honors the injected isWsl result so the behavior is testable.
  • Tests: contract.test.ts asserts loopback-primary plus fallbackUrls/CORS origins; test/onboard-dashboard.test.ts covers the rendered banner (loopback first, WSL fallback after); the verify-chain test helper is updated for the new field.

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:
  • 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:
  • 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
  • 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)
  • 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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved WSL dashboard URL behavior: primary links now consistently use 127.0.0.1, with the WSL host exposed via additional fallback URLs.
    • Updated CORS origin handling to combine loopback and fallback-derived origins with de-duplication.
    • Dashboard and agent UI links now correctly incorporate fallback-derived control URLs in WSL mode.
  • New Features
    • Added fallback-aware control UI URL generation with port rewriting and filtering of invalid/non-HTTP(S) fallback entries.
  • Tests
    • Expanded contract and onboarding tests to cover multiple WSL/fallback scenarios.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

buildChain now keeps loopback as the primary dashboard URL, records WSL host URLs as fallbacks, and updates dashboard printing and access URL derivation to use those fallbacks. Tests and fixtures were updated for the new chain shape and WSL output paths.

Changes

WSL dashboard URL flow

Layer / File(s) Summary
Chain URLs and origins
src/lib/dashboard/contract.ts
DashboardDeliveryChain adds fallbackUrls; buildChain keeps loopback as the primary access URL, adds WSL host URLs to fallbacks, and derives CORS origins from both primary and fallback URLs.
Dashboard output uses fallbacks
src/lib/onboard/dashboard.ts, src/lib/onboard/dashboard-access.ts
printDashboard resolves WSL host state, builds control UI URLs from primary and fallback bases, prints fallback dashboard URLs in both output paths, and getDashboardAccessInfo derives WSL fallback entries from fallbackUrls.
Test coverage and fixtures
src/lib/dashboard/contract.test.ts, test/helpers/onboard-final-flow-phases.ts, test/onboard-dashboard.test.ts
Contract expectations, onboarding fixtures, and dashboard output tests were updated for empty default fallbacks, WSL fallback URL printing, and fallback control UI URL generation.

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

Related issue: NVIDIA/NemoClaw#6171

Suggested labels: bug-fix, area: ui

Suggested reviewers: ericksoa

🚥 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 is concise and accurately summarizes the main WSL loopback dashboard URL fix.
Linked Issues check ✅ Passed The changes satisfy the WSL2 requirement by making loopback the primary dashboard URL and updating the onboard banner accordingly.
Out of Scope Changes check ✅ Passed The added fallback URL and CORS handling are supporting changes for the WSL dashboard fix, not unrelated scope.
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
  • Commit unit tests in branch fix/dashboard-url-wsl-loopback

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

@github-code-quality

github-code-quality Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File 96d52ae +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/blueprint/ssrf.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the branch is 69%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File 96d52ae +/-
src/lib/actions...dbox/rebuild.ts 82%
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 80%
src/lib/shields/index.ts 75%
src/lib/state/sandbox.ts 73%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/onboard/preflight.ts 69%
src/lib/actions...licy-channel.ts 59%
src/lib/policy/index.ts 56%
src/lib/onboard.ts 20%

Updated July 03, 2026 03:29 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard, dashboard-remote-bind, wsl-e2e
Optional E2E: full-e2e, device-auth-health

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard (high): Required because onboarding dashboard URL generation and final user guidance changed. This validates a clean hosted install/onboard flow reaches a ready sandbox and still produces usable dashboard access in the main hosted onboarding path.
  • dashboard-remote-bind (medium): Required because the PR changes dashboard access URL, CORS-origin, bind-target, and device-auth boundary logic. The existing remote-bind E2E is the focused live regression for opt-in remote dashboard forwarding and 0.0.0.0 exposure behavior.
  • wsl-e2e (high): Required because the PR explicitly changes WSL dashboard behavior. A real Windows/WSL run is needed to validate the loopback primary URL, WSL host-IP fallback guidance, and dashboard forwarding across the Windows-to-WSL boundary.

Optional E2E

  • full-e2e (high): Useful additional confidence for the standard Linux live user journey and final dashboard guidance under normal loopback conditions, but cloud-onboard plus dashboard-remote-bind cover the merge-blocking hosted and remote-dashboard risks.
  • device-auth-health (medium): Adjacent security confidence because shouldDisableDeviceAuth and dashboard accessibility decisions are part of the changed contract. Not merge-blocking if dashboard-remote-bind passes.

New E2E recommendations

  • WSL dashboard access (high): Existing WSL coverage runs the full flow, but there is no focused live E2E assertion that the final dashboard output presents 127.0.0.1 as primary and the WSL host IP as a fallback, nor that agent dashboard fallback URLs are rewritten to the agent port.
    • Suggested test: Add a focused wsl-dashboard-access live E2E under the WSL workflow that captures onboarding/final dashboard output and asserts loopback-primary, host-IP fallback, token redaction, and port-rewritten agent fallback URLs.
  • workflow coverage routing (medium): The WSL workflow's pull_request path filters do not include src/lib/**, even though WSL-sensitive dashboard behavior lives in src/lib/dashboard/** and src/lib/onboard/**. Future similar changes may not automatically exercise WSL E2E.
    • Suggested test: Update WSL E2E coverage routing or add a lightweight WSL dashboard contract job that is selected when dashboard/onboard source files change.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-openclaw
Optional E2E targets: device-auth-health

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-openclaw: Changes affect the dashboard delivery/access contract and onboard dashboard output used by the default OpenClaw onboarding path; run the smallest live-supported OpenClaw target to verify live onboarding, dashboard forward setup, and smoke validation still work.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Optional E2E targets

  • device-auth-health: Adjacent discrete live job that probes dashboard health/root behavior with device auth enabled; useful for extra confidence around dashboard URL/auth handling but not the primary typed target for this PR.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=device-auth-health

Relevant changed files

  • src/lib/dashboard/contract.ts
  • src/lib/onboard/dashboard-access.ts
  • src/lib/onboard/dashboard.ts
  • test/helpers/onboard-final-flow-phases.ts

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: WSL fallback host validation before URL and CORS construction.
Open items: 0 required · 3 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 3 still apply · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: WSL fallback host validation before URL and CORS construction
  • PRA-2 Resolve or justify: Validate WSL fallback hosts before URL and CORS construction in src/lib/dashboard/contract.ts:112
  • PRA-3 Resolve or justify: Add direct quiet-command coverage for the reported WSL URL regression in src/lib/dashboard-url-command.test.ts:53
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Add direct quiet-command coverage for the reported WSL URL regression
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify security src/lib/dashboard/contract.ts:112 Normalize and validate the WSL fallback host at the source boundary or in `buildChain()` before appending to `fallbackUrls` or deriving `corsOrigins`. Prefer accepting only valid IP literals with `node:net.isIP`, bracket-normalizing IPv6 if supported, and dropping unsupported or malformed values.
PRA-3 Resolve/justify tests src/lib/dashboard-url-command.test.ts:53 Add a mocked command/bridge test that simulates WSL with `hostname -I` returning a `172.x` address and asserts quiet output uses the sandbox dashboard port on `127.0.0.1`, not the fallback host IP.
Review findings by urgency: 0 required fixes, 3 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: WSL fallback host validation before URL and CORS construction

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Existing tests cover valid WSL host fallback behavior but not malformed host rejection from `fallbackUrls`, `corsOrigins`, or `getDashboardAccessInfo()`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `buildChain()` still executes `fallbackUrls.push(`http://${h.wslHostAddress}:${port}`)` and then derives `extraOrigins` from `[accessUrl, ...fallbackUrls]`.

PRA-2 Resolve/justify — Validate WSL fallback hosts before URL and CORS construction

  • Location: src/lib/dashboard/contract.ts:112
  • Category: security
  • Problem: `buildChain()` still interpolates `h.wslHostAddress` directly into `fallbackUrls`, and `corsOrigins` is derived from `accessUrl` plus those fallback URLs. The value comes from `getWslHostAddress()`, which returns an injected `wslHostAddress` or the first whitespace-delimited `hostname -I` token without verifying that it is a valid URL-safe IP address. The new `buildFallbackControlUiUrls()` parser filters malformed URLs only for one display helper, after the invalid fallback URL has already been created and after CORS origins can be derived from it.
  • Impact: A malformed or spoofed WSL host token can produce misleading browser guidance and can broaden the dashboard CORS allowlist if the constructed URL parses as an unexpected HTTP origin. This is not a confirmed sandbox escape, but it expands an unvalidated host-network boundary value into token-bearing dashboard URL handling and CORS policy.
  • Recommended action: Normalize and validate the WSL fallback host at the source boundary or in `buildChain()` before appending to `fallbackUrls` or deriving `corsOrigins`. Prefer accepting only valid IP literals with `node:net.isIP`, bracket-normalizing IPv6 if supported, and dropping unsupported or malformed values.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/dashboard-access.ts:getWslHostAddress()` and `src/lib/dashboard/contract.ts` around `fallbackUrls.push(...)` and `extraOrigins`; confirm invalid `wslHostAddress` values cannot enter either `fallbackUrls`, `corsOrigins`, or `getDashboardAccessInfo()` output.
  • Missing regression test: Add contract/access tests such as `buildChain drops malformed WSL host addresses from fallbackUrls and corsOrigins`, `buildChain accepts only normalized WSL IP fallback hosts and bracket-normalizes IPv6 or drops unsupported IPv6`, and `getDashboardAccessInfo does not emit a WSL fallback entry for malformed host probe output`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/dashboard-access.ts:getWslHostAddress()` and `src/lib/dashboard/contract.ts` around `fallbackUrls.push(...)` and `extraOrigins`; confirm invalid `wslHostAddress` values cannot enter either `fallbackUrls`, `corsOrigins`, or `getDashboardAccessInfo()` output.
  • Evidence: `fallbackUrls.push(`http://${h.wslHostAddress}:${port}`)` is followed by `const extraOrigins = [accessUrl, ...fallbackUrls].map(toOrigin)` with no allowlist or IP normalization in this changed path.

PRA-3 Resolve/justify — Add direct quiet-command coverage for the reported WSL URL regression

  • Location: src/lib/dashboard-url-command.test.ts:53
  • Category: tests
  • Problem: The linked issue specifically reports `nemohermes <sandbox> dashboard-url --quiet` returning a `172.x.x.x` URL. The PR adds useful pure-contract and onboard banner coverage, but no command-level test exercises the runtime bridge path where `src/commands/sandbox/dashboard-url.ts` resolves `buildDashboardChain(...).accessUrl` under WSL.
  • Impact: A future refactor could make `dashboard-url --quiet` print the WSL fallback host as the primary URL without the current tests failing, even though that is the headline user-visible failure and acceptance criterion for this PR.
  • Recommended action: Add a mocked command/bridge test that simulates WSL with `hostname -I` returning a `172.x` address and asserts quiet output uses the sandbox dashboard port on `127.0.0.1`, not the fallback host IP.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Review `src/lib/dashboard-url-command.test.ts` and any `src/commands/sandbox/dashboard-url` tests; confirm there is an explicit WSL quiet-mode case where `getAccessUrl` is backed by `buildDashboardChain()` and the mocked host probe returns a `172.x` address.
  • Missing regression test: Add `dashboard-url --quiet on WSL prints the 127.0.0.1 sandbox dashboardPort primary URL when hostname -I returns a 172.x address`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Review `src/lib/dashboard-url-command.test.ts` and any `src/commands/sandbox/dashboard-url` tests; confirm there is an explicit WSL quiet-mode case where `getAccessUrl` is backed by `buildDashboardChain()` and the mocked host probe returns a `172.x` address.
  • Evidence: Existing command tests cover generic quiet output and an injected alternate access URL, while the new PR tests cover `buildChain()` and onboard rendering; none directly exercise the reported quiet command path.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add `dashboard-url --quiet on WSL prints the 127.0.0.1 sandbox dashboardPort primary URL when hostname -I returns a 172.x address`.. The changed behavior affects WSL host probing, browser-facing dashboard URL selection, CORS origins, onboard output, and agent dashboard fallback links. Unit coverage is good for the pure contract and onboard banner, but the reported quiet command path and malformed host negative paths still need targeted validation.
  • PRA-T2 Runtime validation — Add `buildChain drops malformed WSL host addresses from fallbackUrls and corsOrigins`.. The changed behavior affects WSL host probing, browser-facing dashboard URL selection, CORS origins, onboard output, and agent dashboard fallback links. Unit coverage is good for the pure contract and onboard banner, but the reported quiet command path and malformed host negative paths still need targeted validation.
  • PRA-T3 Runtime validation — Add `buildChain accepts only normalized WSL IP fallback hosts and bracket-normalizes IPv6 or drops unsupported IPv6 values`.. The changed behavior affects WSL host probing, browser-facing dashboard URL selection, CORS origins, onboard output, and agent dashboard fallback links. Unit coverage is good for the pure contract and onboard banner, but the reported quiet command path and malformed host negative paths still need targeted validation.
  • PRA-T4 Runtime validation — Add `getDashboardAccessInfo omits WSL fallback entries when the host probe returns a malformed token`.. The changed behavior affects WSL host probing, browser-facing dashboard URL selection, CORS origins, onboard output, and agent dashboard fallback links. Unit coverage is good for the pure contract and onboard banner, but the reported quiet command path and malformed host negative paths still need targeted validation.
  • PRA-T5 Runtime validation — Perform targeted WSL runtime validation that `nemohermes <sandbox> dashboard-url --quiet` and the onboard banner lead with `127.0.0.1:<dashboardPort>` while any `172.x` address is displayed only as a labeled fallback.. The changed behavior affects WSL host probing, browser-facing dashboard URL selection, CORS origins, onboard output, and agent dashboard fallback links. Unit coverage is good for the pure contract and onboard banner, but the reported quiet command path and malformed host negative paths still need targeted validation.
  • PRA-T6 Add direct quiet-command coverage for the reported WSL URL regression — Add a mocked command/bridge test that simulates WSL with `hostname -I` returning a `172.x` address and asserts quiet output uses the sandbox dashboard port on `127.0.0.1`, not the fallback host IP.
  • PRA-T7 Acceptance clause — On WSL2 (x86), `nemohermes <sandbox> dashboard-url --quiet` returns the WSL2 internal network IP address (`172.24.173.136:1879x`) instead of the expected `127.0.0.1` loopback URL. — add test evidence or identify existing coverage. `buildChain()` now keeps WSL `accessUrl` as `http://127.0.0.1:&lt;port&gt;\` and stores the WSL host IP in `fallbackUrls`; `contract.test.ts` covers that pure function. No direct quiet-command WSL regression test covers `nemohermes <sandbox> dashboard-url --quiet`.
  • PRA-T8 Acceptance clause — The Hermes dashboard should always be served on a `127.0.0.1` loopback port so users can access it directly from the Windows browser. — add test evidence or identify existing coverage. The primary dashboard access URL is loopback on WSL in `buildChain()`, while `forwardTarget` remains `0.0.0.0:<port>` for WSL. The quiet Hermes/session-auth command path is not directly tested.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: WSL fallback host validation before URL and CORS construction

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Existing tests cover valid WSL host fallback behavior but not malformed host rejection from `fallbackUrls`, `corsOrigins`, or `getDashboardAccessInfo()`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `buildChain()` still executes `fallbackUrls.push(`http://${h.wslHostAddress}:${port}`)` and then derives `extraOrigins` from `[accessUrl, ...fallbackUrls]`.

PRA-2 Resolve/justify — Validate WSL fallback hosts before URL and CORS construction

  • Location: src/lib/dashboard/contract.ts:112
  • Category: security
  • Problem: `buildChain()` still interpolates `h.wslHostAddress` directly into `fallbackUrls`, and `corsOrigins` is derived from `accessUrl` plus those fallback URLs. The value comes from `getWslHostAddress()`, which returns an injected `wslHostAddress` or the first whitespace-delimited `hostname -I` token without verifying that it is a valid URL-safe IP address. The new `buildFallbackControlUiUrls()` parser filters malformed URLs only for one display helper, after the invalid fallback URL has already been created and after CORS origins can be derived from it.
  • Impact: A malformed or spoofed WSL host token can produce misleading browser guidance and can broaden the dashboard CORS allowlist if the constructed URL parses as an unexpected HTTP origin. This is not a confirmed sandbox escape, but it expands an unvalidated host-network boundary value into token-bearing dashboard URL handling and CORS policy.
  • Recommended action: Normalize and validate the WSL fallback host at the source boundary or in `buildChain()` before appending to `fallbackUrls` or deriving `corsOrigins`. Prefer accepting only valid IP literals with `node:net.isIP`, bracket-normalizing IPv6 if supported, and dropping unsupported or malformed values.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/dashboard-access.ts:getWslHostAddress()` and `src/lib/dashboard/contract.ts` around `fallbackUrls.push(...)` and `extraOrigins`; confirm invalid `wslHostAddress` values cannot enter either `fallbackUrls`, `corsOrigins`, or `getDashboardAccessInfo()` output.
  • Missing regression test: Add contract/access tests such as `buildChain drops malformed WSL host addresses from fallbackUrls and corsOrigins`, `buildChain accepts only normalized WSL IP fallback hosts and bracket-normalizes IPv6 or drops unsupported IPv6`, and `getDashboardAccessInfo does not emit a WSL fallback entry for malformed host probe output`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/dashboard-access.ts:getWslHostAddress()` and `src/lib/dashboard/contract.ts` around `fallbackUrls.push(...)` and `extraOrigins`; confirm invalid `wslHostAddress` values cannot enter either `fallbackUrls`, `corsOrigins`, or `getDashboardAccessInfo()` output.
  • Evidence: `fallbackUrls.push(`http://${h.wslHostAddress}:${port}`)` is followed by `const extraOrigins = [accessUrl, ...fallbackUrls].map(toOrigin)` with no allowlist or IP normalization in this changed path.

PRA-3 Resolve/justify — Add direct quiet-command coverage for the reported WSL URL regression

  • Location: src/lib/dashboard-url-command.test.ts:53
  • Category: tests
  • Problem: The linked issue specifically reports `nemohermes <sandbox> dashboard-url --quiet` returning a `172.x.x.x` URL. The PR adds useful pure-contract and onboard banner coverage, but no command-level test exercises the runtime bridge path where `src/commands/sandbox/dashboard-url.ts` resolves `buildDashboardChain(...).accessUrl` under WSL.
  • Impact: A future refactor could make `dashboard-url --quiet` print the WSL fallback host as the primary URL without the current tests failing, even though that is the headline user-visible failure and acceptance criterion for this PR.
  • Recommended action: Add a mocked command/bridge test that simulates WSL with `hostname -I` returning a `172.x` address and asserts quiet output uses the sandbox dashboard port on `127.0.0.1`, not the fallback host IP.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Review `src/lib/dashboard-url-command.test.ts` and any `src/commands/sandbox/dashboard-url` tests; confirm there is an explicit WSL quiet-mode case where `getAccessUrl` is backed by `buildDashboardChain()` and the mocked host probe returns a `172.x` address.
  • Missing regression test: Add `dashboard-url --quiet on WSL prints the 127.0.0.1 sandbox dashboardPort primary URL when hostname -I returns a 172.x address`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Review `src/lib/dashboard-url-command.test.ts` and any `src/commands/sandbox/dashboard-url` tests; confirm there is an explicit WSL quiet-mode case where `getAccessUrl` is backed by `buildDashboardChain()` and the mocked host probe returns a `172.x` address.
  • Evidence: Existing command tests cover generic quiet output and an injected alternate access URL, while the new PR tests cover `buildChain()` and onboard rendering; none directly exercise the reported quiet command path.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: Exported buildFallbackControlUiUrls accepts arbitrary hostnames without validation; then add or justify PRA-T1.
Open items: 3 required · 6 warnings · 4 suggestions · 8 test follow-ups
Since last review: 1 prior item resolved · 13 still apply · 5 new items found

Action checklist

  • PRA-2 Fix: Exported buildFallbackControlUiUrls accepts arbitrary hostnames without validation in src/lib/dashboard/contract.ts:178
  • PRA-3 Fix: Monolith growth: onboard/dashboard.ts at 548 lines exceeds 500-line threshold in src/lib/onboard/dashboard.ts:1
  • PRA-4 Fix: PR fix(cli): show SSH port-forward hint for remote dashboard access (#5925) #5929 overlaps same file — merge conflict risk in src/lib/onboard/dashboard.ts:1
  • PRA-1 Resolve or justify: Source-of-truth review needed: wslHostAddress in PlatformHints (legacy)
  • PRA-5 Resolve or justify: buildVerifyChain calls getWslHostAddress() without runCapture — ignores test override in src/lib/onboard.ts:5225
  • PRA-6 Resolve or justify: printDashboard omits gatewayPort/gatewayHealthEndpoint for agent dashboards in src/lib/onboard/dashboard.ts:451
  • PRA-7 Resolve or justify: Missing IPv6 fallback URL test for buildFallbackControlUiUrls in src/lib/dashboard/contract.test.ts:179
  • PRA-8 Resolve or justify: Missing negative test: buildFallbackControlUiUrls rejects public hostnames in src/lib/dashboard/contract.test.ts:179
  • PRA-9 Resolve or justify: Missing test verifying mocked runCapture is used for WSL host resolution in test/onboard-dashboard.test.ts:229
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Missing IPv6 fallback URL test for buildFallbackControlUiUrls
  • PRA-T7 Add or justify test follow-up: Missing negative test: buildFallbackControlUiUrls rejects public hostnames
  • PRA-T8 Add or justify test follow-up: Missing test verifying mocked runCapture is used for WSL host resolution
  • PRA-10 In-scope improvement: printWslFallback helper duplicates dashboard-access.ts fallback printing in src/lib/onboard/dashboard.ts:171
  • PRA-11 In-scope improvement: wslHostAddress retained in PlatformHints after fallbackUrls consolidation in src/lib/dashboard/contract.ts:16
  • PRA-12 In-scope improvement: buildFallbackControlUiUrls only used internally — should be non-exported in src/lib/dashboard/contract.ts:178
  • PRA-13 In-scope improvement: Test helper createTokenDownloadRunOpenshell well-extracted — consider sharing across test files in test/onboard-dashboard.test.ts:1

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Required security src/lib/dashboard/contract.ts:178 Add hostname allowlist validation (permit only private IP ranges 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.1, ::1). Alternatively, make function non-exported (internal) since only dashboard.ts:482 uses it, or add JSDoc @param contract requiring callers to pass only URLs from buildChain.
PRA-3 Required architecture src/lib/onboard/dashboard.ts:1 Extract printDashboard sub-sections: printAgentDashboardAccess, printNonAgentDashboardAccess, printFallbackUrls (shared with dashboard-access.ts), printManageLater. Or add TODO at top of file linking to follow-up refactoring issue with target milestone.
PRA-4 Required architecture src/lib/onboard/dashboard.ts:1 Coordinate with PR #5929 author; review both diffs for overlapping hunk ranges in printDashboard or forward handling. Rebase sequentially if needed.
PRA-5 Resolve/justify correctness src/lib/onboard.ts:5225 Change to: wslHostAddress: getWslHostAddress({ runCapture }), ensuring runCapture is available in scope. Also verify chatUiUrl is passed to buildChain (already done).
PRA-6 Resolve/justify correctness src/lib/onboard/dashboard.ts:451 When agent is present, pass gatewayPort: agent.healthProbe?.port, gatewayHealthEndpoint: agent.healthProbe?.url to buildChain.
PRA-7 Resolve/justify tests src/lib/dashboard/contract.test.ts:179 Add test case: buildFallbackControlUiUrls correctly parses and rewrites port for IPv6 fallback URLs.
PRA-8 Resolve/justify tests src/lib/dashboard/contract.test.ts:179 Add negative test case verifying public hostnames are rejected.
PRA-9 Resolve/justify tests test/onboard-dashboard.test.ts:229 Add test in onboard-dashboard.test.ts that injects runCapture mock and verifies printDashboard uses it for WSL host address resolution.
PRA-10 Improvement architecture src/lib/onboard/dashboard.ts:171 Move printWslFallback to dashboard-access.ts (or create shared util) and import in both files. Update getDashboardGuidanceLines or add new export for consistent fallback URL display.
PRA-11 Improvement architecture src/lib/dashboard/contract.ts:16 Deprecate wslHostAddress in PlatformHints with JSDoc @deprecated; follow-up PR to remove after confirming all callers use chain.fallbackUrls.
PRA-12 Improvement architecture src/lib/dashboard/contract.ts:178 Remove 'export' keyword from buildFallbackControlUiUrls. Update dashboard.ts import accordingly (already uses relative import).
PRA-13 Improvement tests test/onboard-dashboard.test.ts:1 Keep as-is; if other test files need similar mock, move to test/helpers/token-download.ts.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-2 Required — Exported buildFallbackControlUiUrls accepts arbitrary hostnames without validation

  • Location: src/lib/dashboard/contract.ts:178
  • Category: security
  • Problem: buildFallbackControlUiUrls is exported and accepts fallbackUrls parameter. It validates protocol (http/https) but not hostname. A caller could pass http://internal-service:8080 or http://8.8.8.8:8080 and generate dashboard URLs pointing to internal/external services.
  • Impact: SSRF-adjacent surface: if future caller passes untrusted fallback URLs, generated dashboard links could point to internal metadata services (169.254.169.254), private APIs, or external hosts. Exported function expands attack surface beyond buildChain's validated outputs.
  • Required action: Add hostname allowlist validation (permit only private IP ranges 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.1, ::1). Alternatively, make function non-exported (internal) since only dashboard.ts:482 uses it, or add JSDoc @param contract requiring callers to pass only URLs from buildChain.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read contract.ts:178-195 — buildFallbackControlUiUrls parses URL, checks protocol, rewrites port, returns URL with attacker-controlled hostname. No hostname validation present.
  • Missing regression test: Add test case: buildFallbackControlUiUrls rejects fallback URLs with public/non-private hostnames (e.g., example.com, 8.8.8.8, internal-service.local).
  • Done when: The required change is committed and verification passes: Read contract.ts:178-195 — buildFallbackControlUiUrls parses URL, checks protocol, rewrites port, returns URL with attacker-controlled hostname. No hostname validation present.
  • Evidence: contract.ts:183-190 shows URL parsing with protocol check only; line 193 returns buildControlUiUrls with attacker-controlled hostname.

PRA-3 Required — Monolith growth: onboard/dashboard.ts at 548 lines exceeds 500-line threshold

  • Location: src/lib/onboard/dashboard.ts:1
  • Category: architecture
  • Problem: File grew from 526 to 548 lines (+22) in this PR. New helper printWslFallback added but printDashboard agent/non-agent branches, token display, forward handling not extracted. 500-line guardrail requires extract-or-offset-before-merge.
  • Impact: Degraded maintainability and auditability. Harder to review security boundaries. Future changes more likely to introduce regressions.
  • Required action: Extract printDashboard sub-sections: printAgentDashboardAccess, printNonAgentDashboardAccess, printFallbackUrls (shared with dashboard-access.ts), printManageLater. Or add TODO at top of file linking to follow-up refactoring issue with target milestone.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: wc -l src/lib/onboard/dashboard.ts shows 548 lines.
  • Missing regression test: N/A - architectural guardrail.
  • Done when: The required change is committed and verification passes: wc -l src/lib/onboard/dashboard.ts shows 548 lines.
  • Evidence: File grew +22 lines in this PR. Monolith delta severity: blocker per drift context.

PRA-4 Required — PR #5929 overlaps same file — merge conflict risk

Review findings by urgency: 3 required fixes, 6 items to resolve/justify, 4 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: wslHostAddress in PlatformHints (legacy)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Contract tests pass wslHostAddress -> verify fallbackUrls output
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: contract.ts:16, 109, 112; dashboard-access.ts:82; dashboard.ts:451

PRA-5 Resolve/justify — buildVerifyChain calls getWslHostAddress() without runCapture — ignores test override

  • Location: src/lib/onboard.ts:5225
  • Category: correctness
  • Problem: buildVerifyChain in onboard.ts:5225 calls getWslHostAddress() with zero arguments, ignoring the runCapture injection that tests and printDashboard now use. This breaks testability and causes flakiness in CI without WSL.
  • Impact: WSL tests may pass incorrectly using real hostname -I; test flakiness in CI without WSL; mock injection broken for verify-chain WSL path.
  • Recommended action: Change to: wslHostAddress: getWslHostAddress({ runCapture }), ensuring runCapture is available in scope. Also verify chatUiUrl is passed to buildChain (already done).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read onboard.ts:5225 — buildVerifyChain calls getWslHostAddress() with no arguments while dashboard.ts:451 correctly passes { isWsl: deps.isWsl(), runCapture: deps.runCapture }.
  • Missing regression test: Add test in onboard test suite that injects runCapture mock and verifies buildVerifyChain uses it for WSL host address resolution.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read onboard.ts:5225 — buildVerifyChain calls getWslHostAddress() with no arguments while dashboard.ts:451 correctly passes { isWsl: deps.isWsl(), runCapture: deps.runCapture }.
  • Evidence: onboard.ts:5225 shows getWslHostAddress() called with no args; dashboard.ts:451 shows correct pattern.

PRA-6 Resolve/justify — printDashboard omits gatewayPort/gatewayHealthEndpoint for agent dashboards

  • Location: src/lib/onboard/dashboard.ts:451
  • Category: correctness
  • Problem: When agent is present, printDashboard calls buildChain without gatewayPort/gatewayHealthEndpoint from agent.healthProbe, but these are available and used elsewhere (e.g., buildVerifyChain).
  • Impact: Agent dashboard health probes may use wrong port/endpoint if they differ from primary dashboard port.
  • Recommended action: When agent is present, pass gatewayPort: agent.healthProbe?.port, gatewayHealthEndpoint: agent.healthProbe?.url to buildChain.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check dashboard.ts:451 buildChain call vs onboard.ts:5225 buildVerifyChain call — latter includes gatewayPort/gatewayHealthEndpoint.
  • Missing regression test: Add test verifying printDashboard passes agent healthProbe port/URL to buildChain for agent dashboards.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check dashboard.ts:451 buildChain call vs onboard.ts:5225 buildVerifyChain call — latter includes gatewayPort/gatewayHealthEndpoint.
  • Evidence: dashboard.ts:451 buildChain call lacks gatewayPort/gatewayHealthEndpoint; agent.healthProbe available in scope.

PRA-7 Resolve/justify — Missing IPv6 fallback URL test for buildFallbackControlUiUrls

  • Location: src/lib/dashboard/contract.test.ts:179
  • Category: tests
  • Problem: No test covers IPv6 fallback URL parsing and port rewriting (e.g., http://[2001:db8::1]:18789 -> http://[2001:db8::1]:8642).
  • Impact: IPv6 fallback URLs may break silently; no regression protection for bracket syntax handling.
  • Recommended action: Add test case: buildFallbackControlUiUrls correctly parses and rewrites port for IPv6 fallback URLs.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check contract.test.ts:179-198 — tests cover IPv4 but not IPv6 bracket notation.
  • Missing regression test: Add test: buildFallbackControlUiUrls('tok', 8642, ['http://[2001:db8::1]:18789']) returns ['http://[2001:db8::1]:8642/#token=tok'].
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check contract.test.ts:179-198 — tests cover IPv4 but not IPv6 bracket notation.
  • Evidence: Contract tests have IPv6 tests for buildChain forwardTarget but none for buildFallbackControlUiUrls.

PRA-8 Resolve/justify — Missing negative test: buildFallbackControlUiUrls rejects public hostnames

  • Location: src/lib/dashboard/contract.test.ts:179
  • Category: tests
  • Problem: No test verifies that buildFallbackControlUiUrls drops fallback URLs with public/non-private hostnames (example.com, 8.8.8.8, internal-service.local).
  • Impact: Security regression could go undetected if hostname validation is added but later removed or bypassed.
  • Recommended action: Add negative test case verifying public hostnames are rejected.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check contract.test.ts:179-198 — tests cover protocol filtering but not hostname filtering.
  • Missing regression test: Add test: buildFallbackControlUiUrls(null, 8642, ['http://example.com:18789', 'http://8.8.8.8:18789'\]\) returns [].
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check contract.test.ts:179-198 — tests cover protocol filtering but not hostname filtering.
  • Evidence: Security FAIL finding shows no hostname validation exists.

PRA-9 Resolve/justify — Missing test verifying mocked runCapture is used for WSL host resolution

  • Location: test/onboard-dashboard.test.ts:229
  • Category: tests
  • Problem: WSL tests inject runCapture mock but no assertion verifies printDashboard actually uses it for WSL host address resolution.
  • Impact: Test could pass while real code uses real hostname -I; mock injection silently broken.
  • Recommended action: Add test in onboard-dashboard.test.ts that injects runCapture mock and verifies printDashboard uses it for WSL host address resolution.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check onboard-dashboard.test.ts:229 — test injects runCapture: vi.fn(() => '172.22.1.1 10.0.0.2\n') but no assertion on mock calls.
  • Missing regression test: Add test: verify runCapture mock called with ['hostname', '-I'] when isWsl() returns true.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check onboard-dashboard.test.ts:229 — test injects runCapture: vi.fn(() => '172.22.1.1 10.0.0.2\n') but no assertion on mock calls.
  • Evidence: PRA-5 verification requirement unmet.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-10 Improvement — printWslFallback helper duplicates dashboard-access.ts fallback printing

  • Location: src/lib/onboard/dashboard.ts:171
  • Category: architecture
  • Problem: printWslFallback (dashboard.ts:171) and dashboard-access.ts:110 both loop over chain.fallbackUrls to print WSL fallback URLs with different labels ('WSL fallback' vs 'Browser (WSL fallback, if 127.0.0.1 is unreachable from Windows)').
  • Impact: Inconsistent user-facing messages; duplicate logic to maintain.
  • Suggested action: Move printWslFallback to dashboard-access.ts (or create shared util) and import in both files. Update getDashboardGuidanceLines or add new export for consistent fallback URL display.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare dashboard.ts:171-176 with dashboard-access.ts:147-155 — both iterate fallbackUrls to build authenticated URLs.
  • Missing regression test: N/A - code deduplication.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Two separate implementations for same fallback URL display logic.

PRA-11 Improvement — wslHostAddress retained in PlatformHints after fallbackUrls consolidation

  • Location: src/lib/dashboard/contract.ts:16
  • Category: architecture
  • Problem: wslHostAddress field in PlatformHints is now deprecated — fallbackUrls is the source of truth. Keeping it creates confusion and dual maintenance.
  • Impact: API confusion; callers may pass wslHostAddress expecting primary URL behavior (old behavior) instead of fallback.
  • Suggested action: Deprecate wslHostAddress in PlatformHints with JSDoc @deprecated; follow-up PR to remove after confirming all callers use chain.fallbackUrls.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check contract.ts:16 — wslHostAddress field still in PlatformHints; buildChain uses it only to populate fallbackUrls.
  • Missing regression test: N/A - API deprecation.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Contract comment [WSL2][CLI&UX] nemohermes dashboard-url returns WSL2 network IP (172.x.x.x) instead of 127.0.0.1 loopback #6171 notes fallbackUrls is the new mechanism; wslHostAddress only used to seed fallbackUrls.

PRA-12 Improvement — buildFallbackControlUiUrls only used internally — should be non-exported

  • Location: src/lib/dashboard/contract.ts:178
  • Category: architecture
  • Problem: buildFallbackControlUiUrls is exported but only called from dashboard.ts:482. Exporting expands attack surface (security FAIL) and API surface unnecessarily.
  • Impact: Security surface expansion (see blocker finding); violates least privilege / minimal API surface.
  • Suggested action: Remove 'export' keyword from buildFallbackControlUiUrls. Update dashboard.ts import accordingly (already uses relative import).
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: grep -r buildFallbackControlUiUrls src/ — only contract.ts (definition) and dashboard.ts:482 (call site).
  • Missing regression test: N/A - visibility change.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Only internal caller is dashboard.ts:482 which passes chain.fallbackUrls (validated by buildChain).

PRA-13 Improvement — Test helper createTokenDownloadRunOpenshell well-extracted — consider sharing across test files

  • Location: test/onboard-dashboard.test.ts:1
  • Category: tests
  • Problem: createTokenDownloadRunOpenshell helper extracted in test file reduces duplication. Could be moved to shared test utility if used elsewhere.
  • Impact: Positive pattern — reduces test boilerplate, improves maintainability.
  • Suggested action: Keep as-is; if other test files need similar mock, move to test/helpers/token-download.ts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test/onboard-dashboard.test.ts:22-40 — helper used in 3 test cases.
  • Missing regression test: N/A - positive pattern.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Helper replaces inline vi.fn sandbox download mock in multiple tests.
Simplification opportunities: 4 possible cuts, net -50 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-10 shrink (src/lib/onboard/dashboard.ts:171): printWslFallback function (lines 171-176) and its three call sites (lines 497, 511, 522)
    • Replacement: Import shared fallback printing from dashboard-access.ts
    • Net: -15 lines
    • Safety boundary: Must preserve token redaction via dashboardUrlForDisplay and authenticated URL construction
  • PRA-11 delete (src/lib/dashboard/contract.ts:16): wslHostAddress?: string | null from PlatformHints interface (line 16) and its usage in buildChain (lines 109, 112)
    • Replacement: Callers pass fallbackUrls directly or use buildChain output; internal getWslHostAddress feeds fallbackUrls at boundary
    • Net: -5 lines
    • Safety boundary: Must verify no external callers depend on wslHostAddress input — only internal dashboard-access.ts and dashboard.ts use it
  • PRA-12 shrink (src/lib/dashboard/contract.ts:178): export keyword from function declaration (line 178)
    • Replacement: function buildFallbackControlUiUrls(...) (non-exported)
    • Net: 0 lines
    • Safety boundary: No external callers; dashboard.ts import path unchanged
  • PRA-13 stdlib (test/onboard-dashboard.test.ts:1): Inline sandbox download mock duplication across tests
    • Replacement: createTokenDownloadRunOpenshell factory function
    • Net: -30 lines
    • Safety boundary: Test-only utility; no production code impact
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — buildFallbackControlUiUrls rejects public hostname (example.com, 8.8.8.8, internal-service.local). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/dashboard/contract.ts, src/lib/onboard/dashboard-access.ts, src/lib/onboard/dashboard.ts. Unit tests cover pure logic but WSL networking behavior requires integration validation.
  • PRA-T2 Runtime validation — buildFallbackControlUiUrls rewrites IPv6 fallback port ([2001:db8::1]:18789 -> [2001:db8::1]:8642). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/dashboard/contract.ts, src/lib/onboard/dashboard-access.ts, src/lib/onboard/dashboard.ts. Unit tests cover pure logic but WSL networking behavior requires integration validation.
  • PRA-T3 Runtime validation — buildVerifyChain uses mocked runCapture (onboard.ts:5225). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/dashboard/contract.ts, src/lib/onboard/dashboard-access.ts, src/lib/onboard/dashboard.ts. Unit tests cover pure logic but WSL networking behavior requires integration validation.
  • PRA-T4 Runtime validation — printDashboard passes agent healthProbe to buildChain (gatewayPort, gatewayHealthEndpoint). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/dashboard/contract.ts, src/lib/onboard/dashboard-access.ts, src/lib/onboard/dashboard.ts. Unit tests cover pure logic but WSL networking behavior requires integration validation.
  • PRA-T5 Runtime validation — Runtime: WSL end-to-end — loopback URL actually reachable from Windows browser via forwarded port. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/dashboard/contract.ts, src/lib/onboard/dashboard-access.ts, src/lib/onboard/dashboard.ts. Unit tests cover pure logic but WSL networking behavior requires integration validation.
  • PRA-T6 Missing IPv6 fallback URL test for buildFallbackControlUiUrls — Add test case: buildFallbackControlUiUrls correctly parses and rewrites port for IPv6 fallback URLs.
  • PRA-T7 Missing negative test: buildFallbackControlUiUrls rejects public hostnames — Add negative test case verifying public hostnames are rejected.
  • PRA-T8 Missing test verifying mocked runCapture is used for WSL host resolution — Add test in onboard-dashboard.test.ts that injects runCapture mock and verifies printDashboard uses it for WSL host address resolution.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: wslHostAddress in PlatformHints (legacy)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Contract tests pass wslHostAddress -> verify fallbackUrls output
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: contract.ts:16, 109, 112; dashboard-access.ts:82; dashboard.ts:451

PRA-2 Required — Exported buildFallbackControlUiUrls accepts arbitrary hostnames without validation

  • Location: src/lib/dashboard/contract.ts:178
  • Category: security
  • Problem: buildFallbackControlUiUrls is exported and accepts fallbackUrls parameter. It validates protocol (http/https) but not hostname. A caller could pass http://internal-service:8080 or http://8.8.8.8:8080 and generate dashboard URLs pointing to internal/external services.
  • Impact: SSRF-adjacent surface: if future caller passes untrusted fallback URLs, generated dashboard links could point to internal metadata services (169.254.169.254), private APIs, or external hosts. Exported function expands attack surface beyond buildChain's validated outputs.
  • Required action: Add hostname allowlist validation (permit only private IP ranges 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.1, ::1). Alternatively, make function non-exported (internal) since only dashboard.ts:482 uses it, or add JSDoc @param contract requiring callers to pass only URLs from buildChain.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read contract.ts:178-195 — buildFallbackControlUiUrls parses URL, checks protocol, rewrites port, returns URL with attacker-controlled hostname. No hostname validation present.
  • Missing regression test: Add test case: buildFallbackControlUiUrls rejects fallback URLs with public/non-private hostnames (e.g., example.com, 8.8.8.8, internal-service.local).
  • Done when: The required change is committed and verification passes: Read contract.ts:178-195 — buildFallbackControlUiUrls parses URL, checks protocol, rewrites port, returns URL with attacker-controlled hostname. No hostname validation present.
  • Evidence: contract.ts:183-190 shows URL parsing with protocol check only; line 193 returns buildControlUiUrls with attacker-controlled hostname.

PRA-3 Required — Monolith growth: onboard/dashboard.ts at 548 lines exceeds 500-line threshold

  • Location: src/lib/onboard/dashboard.ts:1
  • Category: architecture
  • Problem: File grew from 526 to 548 lines (+22) in this PR. New helper printWslFallback added but printDashboard agent/non-agent branches, token display, forward handling not extracted. 500-line guardrail requires extract-or-offset-before-merge.
  • Impact: Degraded maintainability and auditability. Harder to review security boundaries. Future changes more likely to introduce regressions.
  • Required action: Extract printDashboard sub-sections: printAgentDashboardAccess, printNonAgentDashboardAccess, printFallbackUrls (shared with dashboard-access.ts), printManageLater. Or add TODO at top of file linking to follow-up refactoring issue with target milestone.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: wc -l src/lib/onboard/dashboard.ts shows 548 lines.
  • Missing regression test: N/A - architectural guardrail.
  • Done when: The required change is committed and verification passes: wc -l src/lib/onboard/dashboard.ts shows 548 lines.
  • Evidence: File grew +22 lines in this PR. Monolith delta severity: blocker per drift context.

PRA-4 Required — PR #5929 overlaps same file — merge conflict risk

PRA-5 Resolve/justify — buildVerifyChain calls getWslHostAddress() without runCapture — ignores test override

  • Location: src/lib/onboard.ts:5225
  • Category: correctness
  • Problem: buildVerifyChain in onboard.ts:5225 calls getWslHostAddress() with zero arguments, ignoring the runCapture injection that tests and printDashboard now use. This breaks testability and causes flakiness in CI without WSL.
  • Impact: WSL tests may pass incorrectly using real hostname -I; test flakiness in CI without WSL; mock injection broken for verify-chain WSL path.
  • Recommended action: Change to: wslHostAddress: getWslHostAddress({ runCapture }), ensuring runCapture is available in scope. Also verify chatUiUrl is passed to buildChain (already done).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read onboard.ts:5225 — buildVerifyChain calls getWslHostAddress() with no arguments while dashboard.ts:451 correctly passes { isWsl: deps.isWsl(), runCapture: deps.runCapture }.
  • Missing regression test: Add test in onboard test suite that injects runCapture mock and verifies buildVerifyChain uses it for WSL host address resolution.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read onboard.ts:5225 — buildVerifyChain calls getWslHostAddress() with no arguments while dashboard.ts:451 correctly passes { isWsl: deps.isWsl(), runCapture: deps.runCapture }.
  • Evidence: onboard.ts:5225 shows getWslHostAddress() called with no args; dashboard.ts:451 shows correct pattern.

PRA-6 Resolve/justify — printDashboard omits gatewayPort/gatewayHealthEndpoint for agent dashboards

  • Location: src/lib/onboard/dashboard.ts:451
  • Category: correctness
  • Problem: When agent is present, printDashboard calls buildChain without gatewayPort/gatewayHealthEndpoint from agent.healthProbe, but these are available and used elsewhere (e.g., buildVerifyChain).
  • Impact: Agent dashboard health probes may use wrong port/endpoint if they differ from primary dashboard port.
  • Recommended action: When agent is present, pass gatewayPort: agent.healthProbe?.port, gatewayHealthEndpoint: agent.healthProbe?.url to buildChain.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check dashboard.ts:451 buildChain call vs onboard.ts:5225 buildVerifyChain call — latter includes gatewayPort/gatewayHealthEndpoint.
  • Missing regression test: Add test verifying printDashboard passes agent healthProbe port/URL to buildChain for agent dashboards.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check dashboard.ts:451 buildChain call vs onboard.ts:5225 buildVerifyChain call — latter includes gatewayPort/gatewayHealthEndpoint.
  • Evidence: dashboard.ts:451 buildChain call lacks gatewayPort/gatewayHealthEndpoint; agent.healthProbe available in scope.

PRA-7 Resolve/justify — Missing IPv6 fallback URL test for buildFallbackControlUiUrls

  • Location: src/lib/dashboard/contract.test.ts:179
  • Category: tests
  • Problem: No test covers IPv6 fallback URL parsing and port rewriting (e.g., http://[2001:db8::1]:18789 -> http://[2001:db8::1]:8642).
  • Impact: IPv6 fallback URLs may break silently; no regression protection for bracket syntax handling.
  • Recommended action: Add test case: buildFallbackControlUiUrls correctly parses and rewrites port for IPv6 fallback URLs.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check contract.test.ts:179-198 — tests cover IPv4 but not IPv6 bracket notation.
  • Missing regression test: Add test: buildFallbackControlUiUrls('tok', 8642, ['http://[2001:db8::1]:18789']) returns ['http://[2001:db8::1]:8642/#token=tok'].
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check contract.test.ts:179-198 — tests cover IPv4 but not IPv6 bracket notation.
  • Evidence: Contract tests have IPv6 tests for buildChain forwardTarget but none for buildFallbackControlUiUrls.

PRA-8 Resolve/justify — Missing negative test: buildFallbackControlUiUrls rejects public hostnames

  • Location: src/lib/dashboard/contract.test.ts:179
  • Category: tests
  • Problem: No test verifies that buildFallbackControlUiUrls drops fallback URLs with public/non-private hostnames (example.com, 8.8.8.8, internal-service.local).
  • Impact: Security regression could go undetected if hostname validation is added but later removed or bypassed.
  • Recommended action: Add negative test case verifying public hostnames are rejected.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check contract.test.ts:179-198 — tests cover protocol filtering but not hostname filtering.
  • Missing regression test: Add test: buildFallbackControlUiUrls(null, 8642, ['http://example.com:18789', 'http://8.8.8.8:18789'\]\) returns [].
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check contract.test.ts:179-198 — tests cover protocol filtering but not hostname filtering.
  • Evidence: Security FAIL finding shows no hostname validation exists.

PRA-9 Resolve/justify — Missing test verifying mocked runCapture is used for WSL host resolution

  • Location: test/onboard-dashboard.test.ts:229
  • Category: tests
  • Problem: WSL tests inject runCapture mock but no assertion verifies printDashboard actually uses it for WSL host address resolution.
  • Impact: Test could pass while real code uses real hostname -I; mock injection silently broken.
  • Recommended action: Add test in onboard-dashboard.test.ts that injects runCapture mock and verifies printDashboard uses it for WSL host address resolution.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check onboard-dashboard.test.ts:229 — test injects runCapture: vi.fn(() => '172.22.1.1 10.0.0.2\n') but no assertion on mock calls.
  • Missing regression test: Add test: verify runCapture mock called with ['hostname', '-I'] when isWsl() returns true.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check onboard-dashboard.test.ts:229 — test injects runCapture: vi.fn(() => '172.22.1.1 10.0.0.2\n') but no assertion on mock calls.
  • Evidence: PRA-5 verification requirement unmet.

PRA-10 Improvement — printWslFallback helper duplicates dashboard-access.ts fallback printing

  • Location: src/lib/onboard/dashboard.ts:171
  • Category: architecture
  • Problem: printWslFallback (dashboard.ts:171) and dashboard-access.ts:110 both loop over chain.fallbackUrls to print WSL fallback URLs with different labels ('WSL fallback' vs 'Browser (WSL fallback, if 127.0.0.1 is unreachable from Windows)').
  • Impact: Inconsistent user-facing messages; duplicate logic to maintain.
  • Suggested action: Move printWslFallback to dashboard-access.ts (or create shared util) and import in both files. Update getDashboardGuidanceLines or add new export for consistent fallback URL display.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare dashboard.ts:171-176 with dashboard-access.ts:147-155 — both iterate fallbackUrls to build authenticated URLs.
  • Missing regression test: N/A - code deduplication.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Two separate implementations for same fallback URL display logic.

PRA-11 Improvement — wslHostAddress retained in PlatformHints after fallbackUrls consolidation

  • Location: src/lib/dashboard/contract.ts:16
  • Category: architecture
  • Problem: wslHostAddress field in PlatformHints is now deprecated — fallbackUrls is the source of truth. Keeping it creates confusion and dual maintenance.
  • Impact: API confusion; callers may pass wslHostAddress expecting primary URL behavior (old behavior) instead of fallback.
  • Suggested action: Deprecate wslHostAddress in PlatformHints with JSDoc @deprecated; follow-up PR to remove after confirming all callers use chain.fallbackUrls.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check contract.ts:16 — wslHostAddress field still in PlatformHints; buildChain uses it only to populate fallbackUrls.
  • Missing regression test: N/A - API deprecation.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Contract comment [WSL2][CLI&UX] nemohermes dashboard-url returns WSL2 network IP (172.x.x.x) instead of 127.0.0.1 loopback #6171 notes fallbackUrls is the new mechanism; wslHostAddress only used to seed fallbackUrls.

PRA-12 Improvement — buildFallbackControlUiUrls only used internally — should be non-exported

  • Location: src/lib/dashboard/contract.ts:178
  • Category: architecture
  • Problem: buildFallbackControlUiUrls is exported but only called from dashboard.ts:482. Exporting expands attack surface (security FAIL) and API surface unnecessarily.
  • Impact: Security surface expansion (see blocker finding); violates least privilege / minimal API surface.
  • Suggested action: Remove 'export' keyword from buildFallbackControlUiUrls. Update dashboard.ts import accordingly (already uses relative import).
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: grep -r buildFallbackControlUiUrls src/ — only contract.ts (definition) and dashboard.ts:482 (call site).
  • Missing regression test: N/A - visibility change.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Only internal caller is dashboard.ts:482 which passes chain.fallbackUrls (validated by buildChain).

PRA-13 Improvement — Test helper createTokenDownloadRunOpenshell well-extracted — consider sharing across test files

  • Location: test/onboard-dashboard.test.ts:1
  • Category: tests
  • Problem: createTokenDownloadRunOpenshell helper extracted in test file reduces duplication. Could be moved to shared test utility if used elsewhere.
  • Impact: Positive pattern — reduces test boilerplate, improves maintainability.
  • Suggested action: Keep as-is; if other test files need similar mock, move to test/helpers/token-download.ts.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test/onboard-dashboard.test.ts:22-40 — helper used in 3 test cases.
  • Missing regression test: N/A - positive pattern.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Helper replaces inline vi.fn sandbox download mock in multiple tests.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng added NV QA Bugs found by the NVIDIA QA Team area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Jul 2, 2026
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Comment thread src/lib/dashboard/contract.ts Fixed
Comment thread src/lib/dashboard/contract.ts Fixed
…UiUrls

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…lback

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…iUrls

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng added v0.0.72 Release target v0.0.73 Release target and removed v0.0.72 Release target labels Jul 2, 2026
@cv cv added v0.0.74 Release target and removed v0.0.73 Release target labels Jul 2, 2026
@cv cv merged commit 5a3008c into main Jul 3, 2026
53 of 56 checks passed
@cv cv deleted the fix/dashboard-url-wsl-loopback branch July 3, 2026 07:34
@ericksoa ericksoa mentioned this pull request Jul 4, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 4, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [#6020](#6020) and
[#5876](#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [#6251](#6251) and
[#5989](#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [#6232](#6232),
[#6082](#6082),
[#6219](#6219),
[#6214](#6214),
[#6215](#6215),
[#6230](#6230), and
[#6260](#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [#6166](#6166),
[#6254](#6254),
[#6265](#6265),
[#6164](#6164), and
[#6017](#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [#6150](#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [#6234](#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [#6129](#6129),
[#5987](#5987),
[#5955](#5955), and
[#6220](#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [#5963](#5963),
[#6050](#6050),
[#6094](#6094),
[#6238](#6238),
[#5988](#5988),
[#6235](#6235),
[#6181](#6181), and
[#5986](#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [#6200](#6200),
[#6248](#6248),
[#6168](#6168),
[#6270](#6270), and
[#5649](#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
On WSL2, `nemohermes <sandbox> dashboard-url --quiet` and the onboard
banner led with the WSL VM's host IP (`172.x.x.x`) instead of
`127.0.0.1`. That host IP was originally added (NVIDIA#1503) as a *fallback*
for setups where WSL localhost forwarding is unavailable, but a later
refactor promoted it to the primary `accessUrl`. This restores loopback
as the primary URL while keeping the WSL host IP available as an
explicit fallback.

## Related Issue
Fixes NVIDIA#6171

## Changes
- `src/lib/dashboard/contract.ts`: `buildChain()` now always uses
`http://127.0.0.1:<port>` as the primary `accessUrl` on WSL, and exposes
the WSL host IP through a new `fallbackUrls` field instead of as the
primary URL. `corsOrigins` still includes the fallback origin, and
`forwardTarget`/`bindAddress`/`shouldDisableDeviceAuth` are unchanged
(the forward still binds `0.0.0.0`).
- `src/lib/onboard/dashboard.ts`: the onboard banner prints the loopback
URL first, then lists the WSL host IP under a `WSL fallback` label; the
WSL host-address probe now honors the injected `isWsl` result so the
behavior is testable.
- Tests: `contract.test.ts` asserts loopback-primary plus
`fallbackUrls`/CORS origins; `test/onboard-dashboard.test.ts` covers the
rendered banner (loopback first, WSL fallback after); the verify-chain
test helper is updated for the new field.

## 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)

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [x] 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:
- [ ] 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:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## 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. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `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)

---
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved WSL dashboard URL behavior: primary links now consistently
use `127.0.0.1`, with the WSL host exposed via additional fallback URLs.
* Updated CORS origin handling to combine loopback and fallback-derived
origins with de-duplication.
* Dashboard and agent UI links now correctly incorporate
fallback-derived control URLs in WSL mode.
* **New Features**
* Added fallback-aware control UI URL generation with port rewriting and
filtering of invalid/non-HTTP(S) fallback entries.
* **Tests**
* Expanded contract and onboarding tests to cover multiple WSL/fallback
scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [NVIDIA#6020](NVIDIA#6020) and
[NVIDIA#5876](NVIDIA#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [NVIDIA#6251](NVIDIA#6251) and
[NVIDIA#5989](NVIDIA#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [NVIDIA#6232](NVIDIA#6232),
[NVIDIA#6082](NVIDIA#6082),
[NVIDIA#6219](NVIDIA#6219),
[NVIDIA#6214](NVIDIA#6214),
[NVIDIA#6215](NVIDIA#6215),
[NVIDIA#6230](NVIDIA#6230), and
[NVIDIA#6260](NVIDIA#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [NVIDIA#6166](NVIDIA#6166),
[NVIDIA#6254](NVIDIA#6254),
[NVIDIA#6265](NVIDIA#6265),
[NVIDIA#6164](NVIDIA#6164), and
[NVIDIA#6017](NVIDIA#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [NVIDIA#6150](NVIDIA#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [NVIDIA#6234](NVIDIA#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [NVIDIA#6129](NVIDIA#6129),
[NVIDIA#5987](NVIDIA#5987),
[NVIDIA#5955](NVIDIA#5955), and
[NVIDIA#6220](NVIDIA#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [NVIDIA#5963](NVIDIA#5963),
[NVIDIA#6050](NVIDIA#6050),
[NVIDIA#6094](NVIDIA#6094),
[NVIDIA#6238](NVIDIA#6238),
[NVIDIA#5988](NVIDIA#5988),
[NVIDIA#6235](NVIDIA#6235),
[NVIDIA#6181](NVIDIA#6181), and
[NVIDIA#5986](NVIDIA#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [NVIDIA#6200](NVIDIA#6200),
[NVIDIA#6248](NVIDIA#6248),
[NVIDIA#6168](NVIDIA#6168),
[NVIDIA#6270](NVIDIA#6270), and
[NVIDIA#5649](NVIDIA#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression NV QA Bugs found by the NVIDIA QA Team v0.0.74 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WSL2][CLI&UX] nemohermes dashboard-url returns WSL2 network IP (172.x.x.x) instead of 127.0.0.1 loopback

3 participants