fix(dashboard): inject NEMOCLAW_DASHBOARD_PORT into sandbox so gateway uses configured port (#1925)#1939
Open
Dongni-Yang wants to merge 9 commits intoNVIDIA:mainfrom
Open
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWhen Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Host (env)
participant CLI as Onboard CLI
participant Sandbox as Sandbox (container)
participant Script as nemoclaw-start.sh
participant SSH as OpenShell/SSH
Host->>CLI: export NEMOCLAW_DASHBOARD_PORT=19000
Host->>CLI: run `onboard create` (compute DASHBOARD_PORT)
CLI->>Sandbox: `sandbox create` with envArgs -> CHAT_UI_URL, (NEMOCLAW_DASHBOARD_PORT=19000)
Sandbox->>Script: start -> script reads NEMOCLAW_DASHBOARD_PORT
Script->>Script: set CHAT_UI_URL="http://127.0.0.1:19000"
Host->>SSH: start port forward bind 127.0.0.1:19000 -> 127.0.0.1:19000
Host->>Sandbox: HTTP request 127.0.0.1:19000 -> forwarded to sandbox dashboard
Sandbox-->>Host: HTTP response (dashboard UI)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/ports.ts (1)
34-41: De-duplicate the dashboard default port constant.
DASHBOARD_PORTfallback andSANDBOX_DASHBOARD_PORTboth hardcode18789. Using one source of truth avoids future drift.♻️ Suggested refactor
/** OpenShell gateway port (default 8080, override via NEMOCLAW_GATEWAY_PORT). */ export const GATEWAY_PORT = parsePort("NEMOCLAW_GATEWAY_PORT", 8080); -/** Dashboard UI port (default 18789, override via NEMOCLAW_DASHBOARD_PORT). This is the host-side port. */ -export const DASHBOARD_PORT = parsePort("NEMOCLAW_DASHBOARD_PORT", 18789); /** * The port the OpenClaw dashboard always listens on *inside* the sandbox. * This is baked into the sandbox image at build time (via CHAT_UI_URL ARG in the Dockerfile) * and cannot be changed at runtime. NEMOCLAW_DASHBOARD_PORT controls only the host-side port; * the tunnel must map host:DASHBOARD_PORT → sandbox:SANDBOX_DASHBOARD_PORT when they differ. */ export const SANDBOX_DASHBOARD_PORT = 18789; +/** Dashboard UI port (default 18789, override via NEMOCLAW_DASHBOARD_PORT). This is the host-side port. */ +export const DASHBOARD_PORT = parsePort("NEMOCLAW_DASHBOARD_PORT", SANDBOX_DASHBOARD_PORT);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ports.ts` around lines 34 - 41, DASHBOARD_PORT and SANDBOX_DASHBOARD_PORT both hardcode 18789 causing duplicated source-of-truth; change the code so SANDBOX_DASHBOARD_PORT is the single constant (export const SANDBOX_DASHBOARD_PORT = 18789) and make DASHBOARD_PORT call parsePort("NEMOCLAW_DASHBOARD_PORT", SANDBOX_DASHBOARD_PORT) instead of hardcoding 18789, referencing the existing parsePort, DASHBOARD_PORT and SANDBOX_DASHBOARD_PORT symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nemoclaw.ts`:
- Around line 288-295: The current forwardTarget assignment drops the WSL host
bind when restoring forwards; update the construction used before calling
runOpenshell so it matches the onboard logic: compute sandboxPort and localPort
as now, but set forwardTarget to include the explicit host bind (e.g., if
localPort !== sandboxPort use `0.0.0.0:${localPort}:${sandboxPort}` else
`0.0.0.0:${localPort}`) and then pass that forwardTarget into
runOpenshell(["forward", "start", ...], ...) so recovered sandboxes on WSL
preserve the 0.0.0.0 bind (referencing sandboxPort, localPort, forwardTarget,
and the runOpenshell calls).
---
Nitpick comments:
In `@src/lib/ports.ts`:
- Around line 34-41: DASHBOARD_PORT and SANDBOX_DASHBOARD_PORT both hardcode
18789 causing duplicated source-of-truth; change the code so
SANDBOX_DASHBOARD_PORT is the single constant (export const
SANDBOX_DASHBOARD_PORT = 18789) and make DASHBOARD_PORT call
parsePort("NEMOCLAW_DASHBOARD_PORT", SANDBOX_DASHBOARD_PORT) instead of
hardcoding 18789, referencing the existing parsePort, DASHBOARD_PORT and
SANDBOX_DASHBOARD_PORT symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 47a06aad-9fa4-471b-a503-d04608799c6e
📒 Files selected for processing (4)
src/lib/onboard.tssrc/lib/ports.tssrc/nemoclaw.tstest/onboard.test.ts
Dongni-Yang
force-pushed
the
fix/dashboard-port-forward-1925
branch
2 times, most recently
from
b10a45d to
0030bab
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/ports.ts (1)
34-42: Avoid duplicating the dashboard default literal.
18789is now defined in two places (DASHBOARD_PORTfallback andSANDBOX_DASHBOARD_PORT). Using one source of truth will prevent silent drift later.♻️ Suggested refactor
-export const DASHBOARD_PORT = parsePort("NEMOCLAW_DASHBOARD_PORT", 18789); +export const SANDBOX_DASHBOARD_PORT = 18789; +export const DASHBOARD_PORT = parsePort("NEMOCLAW_DASHBOARD_PORT", SANDBOX_DASHBOARD_PORT); -/** - * The default port the OpenClaw dashboard listens on inside the sandbox. - * The sandbox image is built with CHAT_UI_URL=http://127.0.0.1:DASHBOARD_PORT (patched - * by patchStagedDockerfile), so the gateway starts on whichever port was configured - * via NEMOCLAW_DASHBOARD_PORT at onboard time. This constant represents the hardcoded - * default when no override is set. - */ -export const SANDBOX_DASHBOARD_PORT = 18789; +/** + * The default port the OpenClaw dashboard listens on inside the sandbox. + */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ports.ts` around lines 34 - 42, The literal 18789 is duplicated; consolidate into a single constant and use it as the fallback for parsePort. Replace the numeric fallback in the DASHBOARD_PORT declaration so it references the SANDBOX_DASHBOARD_PORT (or create a single DEFAULT_DASHBOARD_PORT and use that in both places), ensuring parsePort("NEMOCLAW_DASHBOARD_PORT", SANDBOX_DASHBOARD_PORT) and any other references use the single constant (affecting DASHBOARD_PORT, SANDBOX_DASHBOARD_PORT and the parsePort call).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/ports.ts`:
- Around line 34-42: The literal 18789 is duplicated; consolidate into a single
constant and use it as the fallback for parsePort. Replace the numeric fallback
in the DASHBOARD_PORT declaration so it references the SANDBOX_DASHBOARD_PORT
(or create a single DEFAULT_DASHBOARD_PORT and use that in both places),
ensuring parsePort("NEMOCLAW_DASHBOARD_PORT", SANDBOX_DASHBOARD_PORT) and any
other references use the single constant (affecting DASHBOARD_PORT,
SANDBOX_DASHBOARD_PORT and the parsePort call).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ea22061e-3d40-4217-882a-adbe542d2f7e
📒 Files selected for processing (4)
scripts/nemoclaw-start.shsrc/lib/onboard.tssrc/lib/ports.tstest/onboard.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/onboard.ts
Dongni-Yang
force-pushed
the
fix/dashboard-port-forward-1925
branch
from
0030bab to
db451f6
Compare
Dongni-Yang
changed the title
Dongni-Yang
changed the title
Dongni-Yang
force-pushed
the
fix/dashboard-port-forward-1925
branch
from
2b402b5 to
b61331f
Compare
When NEMOCLAW_DASHBOARD_PORT is set to a custom value, the SSH tunnel correctly forwards that host port, but the gateway inside the sandbox was still listening on the baked-in Docker ENV port (18789 by default). This meant the tunnel found nothing on the other end. Two-part fix: - onboard.ts: pass NEMOCLAW_DASHBOARD_PORT into the container via envArgs so the runtime script can act on it - nemoclaw-start.sh: unconditionally override CHAT_UI_URL when NEMOCLAW_DASHBOARD_PORT is injected, ensuring the gateway starts on the correct port regardless of what the Docker image has baked in The build-time fix (patchStagedDockerfile baking CHAT_UI_URL at onboard time) is already in place; this runtime fix handles containers that were built with a different default or where the Docker ENV takes precedence. Signed-off-by: Dongni Yang <dongniy@nvidia.com>
- dashboard.test.ts: assert buildControlUiUrls produces the correct URL when called with a custom port (the path getDashboardAccessInfo takes when NEMOCLAW_DASHBOARD_PORT is set) - onboard.test.ts: integration test that NEMOCLAW_DASHBOARD_PORT=19000 is injected into sandbox create envArgs (Part 1 of fix) and that the openshell forward command uses same-port mapping - nemoclaw-start.test.ts: static-source assertions that the shell script unconditionally overrides CHAT_UI_URL when NEMOCLAW_DASHBOARD_PORT is injected (Part 2 of fix) and falls back to the baked-in value otherwise - ports.ts: consolidate the 18789 default into SANDBOX_DASHBOARD_PORT so DASHBOARD_PORT references it instead of duplicating the literal Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Explain that each sandbox needs a distinct dashboard port and show how to assign one at onboard time with NEMOCLAW_DASHBOARD_PORT. Signed-off-by: Dongni Yang <dongniy@nvidia.com>
…box section Add language tag to the URL example block introduced in the multi-sandbox port section to satisfy markdownlint MD040. Regenerate skills file. Signed-off-by: Dongni Yang <dongniy@nvidia.com>
…e listen port openclaw gateway run accepts a --port flag that controls the WebSocket bind port. Without it the gateway always defaults to 18789 regardless of CHAT_UI_URL or openclaw.json. Pass _DASHBOARD_PORT (already validated at lines 159-184) so the gateway binds to the user-configured port when NEMOCLAW_DASHBOARD_PORT is set. Verified locally: ss -tlnp shows ws://127.0.0.1:19000 when started with NEMOCLAW_DASHBOARD_PORT=19000; gateway.log confirms the listen address. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…not reachable (NVIDIA#1925) Add a Runtime section entry explaining the symptom, root cause, and fix for when the dashboard is unreachable after setting NEMOCLAW_DASHBOARD_PORT. The symptom — SSH tunnel forwarding the custom port while the gateway listens on 18789 — was a bug fixed in NVIDIA#1925. Users on older images need to re-run nemoclaw onboard on the current release. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…shooting update Auto-generated from docs/ via scripts/docs-to-skills.py. Reflects the new "Dashboard not reachable after setting NEMOCLAW_DASHBOARD_PORT" entry added to docs/reference/troubleshooting.md. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…th --port The initial --port fix (cc94c0d) only patched the root path (line 963, gosu gateway). openshell sandbox create always takes the non-root path (line 904) because it passes CMD args that make NEMOCLAW_CMD non-empty, re-execing as the sandbox user. Line 904 was still missing --port, so the gateway continued listening on 18789 despite the tunnel forwarding the configured port. Also patch recovery scripts so a crashed gateway restarts on the correct port rather than falling back to the hardcoded default: - buildRecoveryScript() in agent-runtime.ts now takes a port parameter - recoverSandboxProcesses() in nemoclaw.ts passes DASHBOARD_PORT to buildRecoveryScript and to the inline OpenClaw fallback script Tests: add root-path --port assertion to nemoclaw-start.test.ts and new agent-runtime.test.ts covering buildRecoveryScript port embedding. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Dongni-Yang
force-pushed
the
fix/dashboard-port-forward-1925
branch
from
85fc4dd to
9c92f34
Compare
Contributor
Author
|
locally verified that I can open html with 19000 port with this patch |
…conflict openshell forward start --background fails silently when the host port is already bound by another process (e.g. a local Docker test container started with -p PORT:PORT). With ignoreError:true + stdio:ignore, the failure was completely invisible — the dashboard URL appeared in the output but was unreachable with no explanation. The parent process of openshell forward start exits non-zero immediately when it cannot bind the port, before forking the background child. Capture that exit code and surface a diagnostic warning with a docker ps command to identify the conflicting process. Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1925.
Setting
NEMOCLAW_DASHBOARD_PORTto a custom value had no effect — the SSH tunnel forwarded the correct host port, but the OpenClaw gateway inside the sandbox was still listening on the default port (18789), so the tunnel connected to nothing.The bug has three independent failure points, all of which must be fixed.
Failure point 1 —
NEMOCLAW_DASHBOARD_PORTwas never passed into the containercreateSandbox()builds anenvArgsarray injected at container start time. OnlyCHAT_UI_URLwas in that array;NEMOCLAW_DASHBOARD_PORTwas absent, so the script inside the container had no way to know a custom port was requested.Failure point 2 — the shell fallback could not override the Docker
ENVnemoclaw-start.shusedCHAT_UI_URL=${CHAT_UI_URL:-http://127.0.0.1:${_DASHBOARD_PORT}}. The:-operator only fires when the variable is unset or empty. When the Docker image was built,CHAT_UI_URLwas baked in as anENVdirective bypatchStagedDockerfile, so the fallback never fired and the gateway always started on the port from the image.Failure point 3 —
openclaw gateway runwas not told which port to bindThe gateway binary reads its listen port from the
--portflag, not fromCHAT_UI_URLoropenclaw.json. Without--port, the gateway always defaulted to18789regardless of any env var.nemoclaw-start.shhas two gateway launch lines (non-root path line 904 and root path line 963); both were missing--port. Recovery scripts innemoclaw.tsandagent-runtime.tsalso lacked--port, meaning a crashed gateway would restart on the wrong port even after the initial start was correct.Fix
src/lib/onboard.ts— injectNEMOCLAW_DASHBOARD_PORTinto the container viaenvArgs:scripts/nemoclaw-start.sh— unconditionally overrideCHAT_UI_URLwhenNEMOCLAW_DASHBOARD_PORTis present; pass--porton both gateway launch lines:src/lib/agent-runtime.tsandsrc/nemoclaw.ts— pass port to recovery scripts so a crashed gateway restarts on the configured port, not the hardcoded default.Data flow after fix:
Changes
src/lib/ports.ts18789default intoSANDBOX_DASHBOARD_PORT; remove duplicate literalsrc/lib/onboard.tsNEMOCLAW_DASHBOARD_PORTinto sandboxenvArgswhen setscripts/nemoclaw-start.shCHAT_UI_URL; (2)--port "${_DASHBOARD_PORT}"on both gateway launch lines (904 non-root, 963 root)src/lib/agent-runtime.tsportparameter tobuildRecoveryScript; embed--portin recovery commandsrc/nemoclaw.tsDASHBOARD_PORTtobuildRecoveryScript; add--portto inline OpenClaw recovery scripttest/onboard.test.tsenvArgsinjectiontest/nemoclaw-start.test.tsCHAT_UI_URLoverride logic;--portassertions for both non-root (line 904) and root (line 963) gateway launch linessrc/lib/agent-runtime.test.tsbuildRecoveryScriptport embeddingsrc/lib/dashboard.test.tsbuildControlUiUrlswith custom portdocs/reference/troubleshooting.mdNEMOCLAW_DASHBOARD_PORTnot reachableTest plan
NEMOCLAW_DASHBOARD_PORT=19000 nemoclaw onboard—openshell sandbox exec -- grep "listening on" /tmp/gateway.logshowsws://127.0.0.1:19000; dashboard URL opens in browseropenshell forward listshows port 19000 forwarded after onboardopenshell forward listshows bothnpx vitest run --project cli test/onboard.test.ts— all cases passnpx vitest run --project cli test/nemoclaw-start.test.ts— all cases passnpx vitest run --project cli src/lib/agent-runtime.test.ts— all cases passnpx vitest run --project plugin src/lib/dashboard.test.ts— custom-port URL test passesSigned-off-by: Dongni Yang dongniy@nvidia.com