feat(env): add configurable port overrides via .env files#652
Closed
jnun wants to merge 2 commits intoNVIDIA:mainfrom
Closed
feat(env): add configurable port overrides via .env files#652jnun wants to merge 2 commits intoNVIDIA:mainfrom
jnun wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
Allow users to override default service ports (GATEWAY_PORT, REDIS_PORT, SANDBOX_PORT) through .env and .env.local files. This prevents conflicts when default ports are already in use on the host, without relying on ephemeral shell memory. New files: - bin/lib/env.js — .env/.env.local loader (dotenv) - bin/lib/ports.js — port defaults and validation - scripts/check-ports.sh — port diagnostic utility - docs/reference/port-configuration.md — user-facing reference - test/env.test.js — env loading tests - .env.example — documents available overrides Modified files: - bin/nemoclaw.js — load env at CLI entry point - bin/lib/onboard.js — port-conflict detection and .env bootstrap - bin/lib/preflight.js — pass port env vars to preflight checks - bin/lib/local-inference.js — use configured ports - bin/lib/nim.js — use configured ports - nemoclaw-blueprint/blueprint.yaml — parameterize service ports - nemoclaw-blueprint/orchestrator/runner.py — read port env vars - scripts/brev-setup.sh — source .env before start - scripts/debug.sh — source .env before start - scripts/lib/runtime.sh — source .env before start - scripts/nemoclaw-start.sh — source .env before start - scripts/setup.sh — source .env before start - scripts/start-services.sh — source .env before start - scripts/telegram-bridge.js — remove unused import - uninstall.sh — load env, use configured ports - README.md — add port configuration section - docs/reference/troubleshooting.md — add port override guidance - package-lock.json — lockfile update for dotenv - test/e2e/test-double-onboard.sh — use configurable ports - test/local-inference.test.js — use configurable ports - test/onboard-readiness.test.js — use configurable ports - test/onboard-selection.test.js — use configurable ports - test/preflight.test.js — use configurable ports - test/runtime-shell.test.js — use configurable ports
Aligns CLI examples with project documentation style guidelines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces a centralized, configurable port system for NemoClaw by replacing hardcoded port values with environment variable-driven constants. It adds environment loading infrastructure ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
uninstall.sh (1)
281-286:⚠️ Potential issue | 🟡 MinorHonor the full dashboard fallback chain during cleanup.
bin/lib/ports.jsstill acceptsDASHBOARD_PORTandPUBLIC_PORT, but Line 286 only checksNEMOCLAW_DASHBOARD_PORT. On legacy setups, the uninstaller falls back to18789here and can leave the forwarder running.Proposed fix
local -a pids=() + local dashboard_port="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}" local pid while IFS= read -r pid; do [ -n "$pid" ] || continue pids+=("$pid") - done < <(pgrep -f "openshell.*forward.*${NEMOCLAW_DASHBOARD_PORT:-18789}" 2>/dev/null || true) + done < <(pgrep -f "openshell.*forward.*${dashboard_port}" 2>/dev/null || true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@uninstall.sh` around lines 281 - 286, The cleanup currently only searches for processes matching NEMOCLAW_DASHBOARD_PORT and misses legacy fallbacks; update the pgrep invocation so it checks the full fallback chain (NEMOCLAW_DASHBOARD_PORT, DASHBOARD_PORT, PUBLIC_PORT, and the default 18789). Build a list/regex of ports (e.g. collect ${NEMOCLAW_DASHBOARD_PORT}, ${DASHBOARD_PORT}, ${PUBLIC_PORT}, and 18789, dedupe/ignore empty) and change the pgrep -f pattern "openshell.*forward.*${NEMOCLAW_DASHBOARD_PORT:-18789}" to match any of those ports (e.g. use a grouped alternation like "(portA|portB|portC)"). Ensure the surrounding variable handling (the pids array population using the while read loop) remains the same.
🧹 Nitpick comments (5)
docs/reference/troubleshooting.md (1)
112-115: Format default port values as inline code in the table.The default numeric values should be wrapped in inline code formatting for consistency with docs style (for example,
8080instead of plain8080).As per coding guidelines, "CLI commands, file paths, flags, parameter names, and values must use inline
codeformatting."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 112 - 115, Update the table rows so the numeric default port values are formatted as inline code: wrap the plain numbers (e.g., 8080, 8000, 18789, 11434) in backticks where they appear next to the environment variable names like NEMOCLAW_GATEWAY_PORT, NEMOCLAW_VLLM_PORT, NEMOCLAW_DASHBOARD_PORT, and NEMOCLAW_OLLAMA_PORT to match the docs style guide for CLI/parameter values.docs/reference/port-configuration.md (2)
26-33: Start each H2 with an introductory sentence.
Default PortsandNext Stepsboth jump straight into structured content. Add a short lead-in sentence under each heading to match the docs page-structure rule.
As per coding guidelines,Sections use H2 and H3, each starting with an introductory sentence.Also applies to: 115-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/port-configuration.md` around lines 26 - 33, The H2 headings "Default Ports" and "Next Steps" are missing the required introductory sentences; add a short lead-in sentence immediately under each H2 that briefly describes what the section covers (e.g., for "Default Ports" add a sentence like "The following table lists the default service ports and their environment variable overrides." and for "Next Steps" add a sentence summarizing the actions or links that follow) so the documentation follows the rule that every H2/H3 starts with an introductory sentence; update the content near the "Default Ports" and "Next Steps" headings accordingly.
6-8: Use canonical product casing in the frontmatter metadata.The new
keywordsandtagsentries lowercaseNemoClaw,OpenClaw, andOpenShell. Keep the canonical casing in metadata too.
As per coding guidelines,NemoClaw, OpenClaw, and OpenShell must use correct casing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/port-configuration.md` around lines 6 - 8, Update the frontmatter values so product names use canonical casing: change the entries in keywords and tags to use "NemoClaw", "OpenClaw", and "OpenShell" (e.g., replace "nemoclaw ports" with "NemoClaw ports" and lowercase tag values like "openclaw" and "openshell" with "OpenClaw" and "OpenShell") ensuring the keywords and tags arrays reflect the correct casing.README.md (1)
184-190: Drop emoji from the new callout labels.The added
ℹ️and⚠️markers violate the Markdown rule for technical prose. Use plainNoteandWarninglabels here.
As per coding guidelines,No emoji in technical prose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 184 - 190, Replace the emoji-prefixed callout labels with plain text labels: change the "ℹ️ Note" heading to "Note" and the "⚠️ Network exposure" heading to "Warning", leaving the surrounding callout content unchanged so the guidance about rebuilding the sandbox image and network exposure remains intact.bin/lib/onboard.js (1)
551-559: Fail fast if the Dockerfile patch doesn't apply.If
ARG CHAT_UI_URL=...is renamed or reformatted, this replace becomes a silent no-op and the image is built with the stale dashboard origin even though onboarding keeps going.🧩 Suggested hardening
const dockerfilePath = path.join(buildCtx, "Dockerfile"); const dockerfileContent = fs.readFileSync(dockerfilePath, "utf-8"); - fs.writeFileSync( - dockerfilePath, - dockerfileContent.replace( - /^ARG CHAT_UI_URL=.*/m, - `ARG CHAT_UI_URL=${chatUiUrl}` - ) - ); + const patchedDockerfile = dockerfileContent.replace( + /^ARG CHAT_UI_URL=.*/m, + `ARG CHAT_UI_URL=${chatUiUrl}` + ); + if (patchedDockerfile === dockerfileContent) { + fs.rmSync(buildCtx, { recursive: true, force: true }); + console.error(" Failed to patch Dockerfile: missing ARG CHAT_UI_URL."); + process.exit(1); + } + fs.writeFileSync(dockerfilePath, patchedDockerfile);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/onboard.js` around lines 551 - 559, The current replacement of ARG CHAT_UI_URL in dockerfileContent via dockerfileContent.replace(...) can silently no-op if the Dockerfile line was renamed or reformatted; modify the logic around dockerfilePath/dockerfileContent so you first test whether the regex /^ARG CHAT_UI_URL=.*/m matches (or compare before/after content) and if no match occurs, throw or exit with a clear error (including the expected token ARG CHAT_UI_URL) to fail fast; only call fs.writeFileSync to overwrite when the replacement actually changed the content, otherwise abort the onboarding flow with a descriptive error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/ports.js`:
- Around line 26-31: DASHBOARD_PORT is being computed eagerly via nested
parsePort calls which will throw if a lower-priority env (e.g., PUBLIC_PORT) is
invalid even when a higher-priority env (NEMOCLAW_DASHBOARD_PORT) is set; change
the logic that initializes DASHBOARD_PORT to lazily evaluate in priority order
using parsePort only on the first defined environment variable: check
process.env["NEMOCLAW_DASHBOARD_PORT"], then process.env["DASHBOARD_PORT"], then
process.env["PUBLIC_PORT"], and finally fall back to 18789, calling parsePort
only for the selected value (keep referencing parsePort and the env var names
exactly as used).
- Around line 14-23: The parsePort function currently uses parseInt which
accepts strings like "8000abc"; update parsePort to reject any non-numeric
characters by validating raw against a strict digits-only regex (e.g. /^\d+$/)
before parsing, keep the existing range check (1024-65535) and fallback
behavior, and throw the same Error when the format is invalid so malformed env
values (envVar) no longer silently truncate to a valid port.
In `@scripts/check-ports.sh`:
- Around line 111-116: The loop that iterates custom ports (for p in "$@") must
validate each argument before calling check_port to avoid passing non-numeric
strings like "abc" to lsof; add a small validation (e.g., is_valid_port or
inline check using a regex/isdigit and range 1-65535) that logs and
exits/non-zero or skips invalid entries, and only call check_port "$p" for
validated numeric ports—update the script's parameter handling around the for p
in "$@" block to perform this validation and return a clear error for bad
arguments.
- Around line 51-54: The environment fallback for port variables is
inconsistent: update the assignments in check-ports.sh so GATEWAY_PORT,
VLLM_PORT, and OLLAMA_PORT follow the same fallback chain pattern used by
DASHBOARD_PORT (i.e., NEMOCLAW_<NAME>_PORT → <NAME>_PORT → PUBLIC_PORT →
default) by changing the expansion for GATEWAY_PORT, VLLM_PORT, and OLLAMA_PORT
to include the plain <NAME>_PORT and PUBLIC_PORT fallbacks; also review and, if
needed, mirror the same change in bin/lib/ports.js to keep behavior consistent
across scripts.
In `@scripts/debug.sh`:
- Around line 31-33: The debug.sh script's lsof-dashboard checks currently only
read NEMOCLAW_DASHBOARD_PORT so they miss alternate aliases; update the two
places that build/check the dashboard port (the lsof-dashboard checks) to
resolve the port using a fallback chain (use NEMOCLAW_DASHBOARD_PORT, then
DASHBOARD_PORT, then PUBLIC_PORT) instead of only NEMOCLAW_DASHBOARD_PORT;
change the port variable resolution wherever lsof-dashboard is invoked (the
occurrence around the initial env loading and the similar occurrence later near
line ~294) to compute the port once via that fallback chain and use that
resolved variable in the lsof check.
In `@scripts/lib/runtime.sh`:
- Around line 209-210: The health-check/base-URL cases vllm-local and
ollama-local use only NEMOCLAW_VLLM_PORT and NEMOCLAW_OLLAMA_PORT and therefore
ignore unprefixed VLLM_PORT/OLLAMA_PORT; update the printf calls in the
vllm-local and ollama-local branches (and the other similar branches around the
other host.openshell.internal cases) to fall back to the unprefixed env vars
before the hardcoded default (i.e. use nested parameter expansion so
NEMOCLAW_VLLM_PORT -> VLLM_PORT -> 8000 and NEMOCLAW_OLLAMA_PORT -> OLLAMA_PORT
-> 11434) so alias-based overrides work for shell health checks and base URL
construction.
In `@scripts/nemoclaw-start.sh`:
- Around line 21-22: PUBLIC_PORT is currently set from NEMOCLAW_DASHBOARD_PORT
then PUBLIC_PORT then 18789, but omits the legacy DASHBOARD_PORT env var; update
the PUBLIC_PORT assignment to also fallback to DASHBOARD_PORT (so it reads
NEMOCLAW_DASHBOARD_PORT -> DASHBOARD_PORT -> PUBLIC_PORT -> 18789) so existing
setups honored by bin/lib/ports.js keep the intended port, and leave CHAT_UI_URL
constructed from PUBLIC_PORT unchanged.
- Around line 14-18: Update the PUBLIC_PORT fallback chain to include
DASHBOARD_PORT so it matches the resolution in bin/lib/ports.js: change the
assignment of PUBLIC_PORT (currently using NEMOCLAW_DASHBOARD_PORT →
PUBLIC_PORT) to use NEMOCLAW_DASHBOARD_PORT → DASHBOARD_PORT → PUBLIC_PORT (with
the existing default 18789). Locate the PUBLIC_PORT assignment in
scripts/nemoclaw-start.sh and modify it to reference NEMOCLAW_DASHBOARD_PORT,
then DASHBOARD_PORT, then PUBLIC_PORT as the fallback order so CHAT_UI_URL and
other consumers stay consistent.
In `@scripts/setup.sh`:
- Around line 35-37: After sourcing .env and .env.local, add normalization that
maps the unprefixed alias env vars GATEWAY_PORT and OLLAMA_PORT into the
canonical prefixed env vars the rest of the script expects (i.e., if
GATEWAY_PORT is set and the prefixed variant is not, export the prefixed
variable with GATEWAY_PORT's value; do the same for OLLAMA_PORT), so the
overrides are honored regardless of whether callers set the aliased or prefixed
names.
---
Outside diff comments:
In `@uninstall.sh`:
- Around line 281-286: The cleanup currently only searches for processes
matching NEMOCLAW_DASHBOARD_PORT and misses legacy fallbacks; update the pgrep
invocation so it checks the full fallback chain (NEMOCLAW_DASHBOARD_PORT,
DASHBOARD_PORT, PUBLIC_PORT, and the default 18789). Build a list/regex of ports
(e.g. collect ${NEMOCLAW_DASHBOARD_PORT}, ${DASHBOARD_PORT}, ${PUBLIC_PORT}, and
18789, dedupe/ignore empty) and change the pgrep -f pattern
"openshell.*forward.*${NEMOCLAW_DASHBOARD_PORT:-18789}" to match any of those
ports (e.g. use a grouped alternation like "(portA|portB|portC)"). Ensure the
surrounding variable handling (the pids array population using the while read
loop) remains the same.
---
Nitpick comments:
In `@bin/lib/onboard.js`:
- Around line 551-559: The current replacement of ARG CHAT_UI_URL in
dockerfileContent via dockerfileContent.replace(...) can silently no-op if the
Dockerfile line was renamed or reformatted; modify the logic around
dockerfilePath/dockerfileContent so you first test whether the regex /^ARG
CHAT_UI_URL=.*/m matches (or compare before/after content) and if no match
occurs, throw or exit with a clear error (including the expected token ARG
CHAT_UI_URL) to fail fast; only call fs.writeFileSync to overwrite when the
replacement actually changed the content, otherwise abort the onboarding flow
with a descriptive error.
In `@docs/reference/port-configuration.md`:
- Around line 26-33: The H2 headings "Default Ports" and "Next Steps" are
missing the required introductory sentences; add a short lead-in sentence
immediately under each H2 that briefly describes what the section covers (e.g.,
for "Default Ports" add a sentence like "The following table lists the default
service ports and their environment variable overrides." and for "Next Steps"
add a sentence summarizing the actions or links that follow) so the
documentation follows the rule that every H2/H3 starts with an introductory
sentence; update the content near the "Default Ports" and "Next Steps" headings
accordingly.
- Around line 6-8: Update the frontmatter values so product names use canonical
casing: change the entries in keywords and tags to use "NemoClaw", "OpenClaw",
and "OpenShell" (e.g., replace "nemoclaw ports" with "NemoClaw ports" and
lowercase tag values like "openclaw" and "openshell" with "OpenClaw" and
"OpenShell") ensuring the keywords and tags arrays reflect the correct casing.
In `@docs/reference/troubleshooting.md`:
- Around line 112-115: Update the table rows so the numeric default port values
are formatted as inline code: wrap the plain numbers (e.g., 8080, 8000, 18789,
11434) in backticks where they appear next to the environment variable names
like NEMOCLAW_GATEWAY_PORT, NEMOCLAW_VLLM_PORT, NEMOCLAW_DASHBOARD_PORT, and
NEMOCLAW_OLLAMA_PORT to match the docs style guide for CLI/parameter values.
In `@README.md`:
- Around line 184-190: Replace the emoji-prefixed callout labels with plain text
labels: change the "ℹ️ Note" heading to "Note" and the "⚠️ Network exposure"
heading to "Warning", leaving the surrounding callout content unchanged so the
guidance about rebuilding the sandbox image and network exposure remains intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e5ac7b1e-a0a1-41a0-93bc-b588aea0b01a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
.env.exampleREADME.mdbin/lib/env.jsbin/lib/local-inference.jsbin/lib/nim.jsbin/lib/onboard.jsbin/lib/ports.jsbin/lib/preflight.jsbin/nemoclaw.jsdocs/reference/port-configuration.mddocs/reference/troubleshooting.mdnemoclaw-blueprint/blueprint.yamlnemoclaw-blueprint/orchestrator/runner.pyscripts/brev-setup.shscripts/check-ports.shscripts/debug.shscripts/lib/runtime.shscripts/nemoclaw-start.shscripts/setup.shscripts/start-services.shscripts/telegram-bridge.jstest/e2e/test-double-onboard.shtest/env.test.jstest/local-inference.test.jstest/onboard-readiness.test.jstest/onboard-selection.test.jstest/preflight.test.jstest/runtime-shell.test.jsuninstall.sh
💤 Files with no reviewable changes (1)
- scripts/telegram-bridge.js
Comment on lines
+14
to
+23
| function parsePort(envVar, fallback) { | ||
| const raw = process.env[envVar]; | ||
| if (raw === undefined || raw === "") return fallback; | ||
| const parsed = parseInt(raw, 10); | ||
| if (Number.isNaN(parsed) || parsed < 1024 || parsed > 65535) { | ||
| throw new Error( | ||
| `Invalid port: ${envVar}="${raw}" — must be an integer between 1024 and 65535` | ||
| ); | ||
| } | ||
| return parsed; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n bin/lib/ports.jsRepository: NVIDIA/NemoClaw
Length of output: 1806
🏁 Script executed:
node -e "
console.log('parseInt(\"8000abc\", 10):', parseInt('8000abc', 10));
console.log('parseInt(\"9000#tmp\", 10):', parseInt('9000#tmp', 10));
console.log('parseInt(\"abc8000\", 10):', parseInt('abc8000', 10));
console.log('Number(\"8000abc\"):', Number('8000abc'));
console.log('Number(\"8000\"):', Number('8000'));
console.log('/^\\d+$/.test(\"8000abc\"):', /^\d+$/.test('8000abc'));
console.log('/^\\d+$/.test(\"8000\"):', /^\d+$/.test('8000'));
"Repository: NVIDIA/NemoClaw
Length of output: 248
🏁 Script executed:
fd -e test -e spec | head -20Repository: NVIDIA/NemoClaw
Length of output: 41
🏁 Script executed:
rg "parsePort" --type js -A 5 -B 5Repository: NVIDIA/NemoClaw
Length of output: 1534
🏁 Script executed:
find . -name "*test*" -o -name "*spec*" | grep -i portRepository: NVIDIA/NemoClaw
Length of output: 41
🏁 Script executed:
rg "DASHBOARD_PORT|GATEWAY_PORT|VLLM_PORT|OLLAMA_PORT" --type js -B 2 -A 2Repository: NVIDIA/NemoClaw
Length of output: 22271
Reject malformed port strings instead of silently truncating them.
Line 17 uses parseInt, so values like 8000abc or 9000#tmp validate as 8000/9000 instead of failing. Typos in environment variables silently bind the wrong port. Use a strict whitelist pattern before parsing:
Proposed fix
function parsePort(envVar, fallback) {
const raw = process.env[envVar];
if (raw === undefined || raw === "") return fallback;
- const parsed = parseInt(raw, 10);
- if (Number.isNaN(parsed) || parsed < 1024 || parsed > 65535) {
+ const normalized = raw.trim();
+ if (!/^\d+$/.test(normalized)) {
+ throw new Error(
+ `Invalid port: ${envVar}="${raw}" — must be an integer between 1024 and 65535`
+ );
+ }
+ const parsed = Number(normalized);
+ if (parsed < 1024 || parsed > 65535) {
throw new Error(
`Invalid port: ${envVar}="${raw}" — must be an integer between 1024 and 65535`
);
}
return parsed;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/lib/ports.js` around lines 14 - 23, The parsePort function currently uses
parseInt which accepts strings like "8000abc"; update parsePort to reject any
non-numeric characters by validating raw against a strict digits-only regex
(e.g. /^\d+$/) before parsing, keep the existing range check (1024-65535) and
fallback behavior, and throw the same Error when the format is invalid so
malformed env values (envVar) no longer silently truncate to a valid port.
Comment on lines
+26
to
+31
| // Dashboard port supports legacy env var fallback chain: | ||
| // NEMOCLAW_DASHBOARD_PORT → DASHBOARD_PORT → PUBLIC_PORT → 18789 | ||
| const DASHBOARD_PORT = parsePort( | ||
| "NEMOCLAW_DASHBOARD_PORT", | ||
| parsePort("DASHBOARD_PORT", parsePort("PUBLIC_PORT", 18789)) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n bin/lib/ports.js | head -50Repository: NVIDIA/NemoClaw
Length of output: 1806
Make the dashboard fallback chain lazy.
Lines 28–30 eagerly evaluate DASHBOARD_PORT and PUBLIC_PORT via nested function arguments. If either legacy variable contains an invalid value, the module throws before NEMOCLAW_DASHBOARD_PORT is checked, preventing the primary configuration from being honored.
For example, if NEMOCLAW_DASHBOARD_PORT=9000 (valid) but PUBLIC_PORT=invalid, the nested call parsePort("PUBLIC_PORT", 18789) throws immediately, blocking the valid primary setting.
Implement lazy evaluation to check environment variables in priority order and only attempt to parse the first defined one:
Proposed fix
+function parseFirstDefinedPort(envVars, fallback) {
+ for (const envVar of envVars) {
+ if (process.env[envVar] !== undefined && process.env[envVar] !== "") {
+ return parsePort(envVar, fallback);
+ }
+ }
+ return fallback;
+}
+
// Dashboard port supports legacy env var fallback chain:
// NEMOCLAW_DASHBOARD_PORT → DASHBOARD_PORT → PUBLIC_PORT → 18789
-const DASHBOARD_PORT = parsePort(
- "NEMOCLAW_DASHBOARD_PORT",
- parsePort("DASHBOARD_PORT", parsePort("PUBLIC_PORT", 18789))
-);
+const DASHBOARD_PORT = parseFirstDefinedPort(
+ ["NEMOCLAW_DASHBOARD_PORT", "DASHBOARD_PORT", "PUBLIC_PORT"],
+ 18789
+);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/lib/ports.js` around lines 26 - 31, DASHBOARD_PORT is being computed
eagerly via nested parsePort calls which will throw if a lower-priority env
(e.g., PUBLIC_PORT) is invalid even when a higher-priority env
(NEMOCLAW_DASHBOARD_PORT) is set; change the logic that initializes
DASHBOARD_PORT to lazily evaluate in priority order using parsePort only on the
first defined environment variable: check
process.env["NEMOCLAW_DASHBOARD_PORT"], then process.env["DASHBOARD_PORT"], then
process.env["PUBLIC_PORT"], and finally fall back to 18789, calling parsePort
only for the selected value (keep referencing parsePort and the env var names
exactly as used).
Comment on lines
+51
to
+54
| DASHBOARD_PORT="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}" | ||
| GATEWAY_PORT="${NEMOCLAW_GATEWAY_PORT:-8080}" | ||
| VLLM_PORT="${NEMOCLAW_VLLM_PORT:-8000}" | ||
| OLLAMA_PORT="${NEMOCLAW_OLLAMA_PORT:-11434}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pattern='NEMOCLAW_(DASHBOARD|GATEWAY|VLLM|OLLAMA)_PORT|\b(DASHBOARD|GATEWAY|VLLM|OLLAMA)_PORT\b'
for file in .env.example bin/lib/ports.js docs/reference/port-configuration.md scripts/check-ports.sh; do
echo "=== $file ==="
rg -n "$pattern" "$file" || true
echo
doneRepository: NVIDIA/NemoClaw
Length of output: 1906
Align port environment variable fallbacks with DASHBOARD_PORT pattern.
The script treats DASHBOARD_PORT differently from GATEWAY_PORT, VLLM_PORT, and OLLAMA_PORT: only dashboard respects a fallback chain (NEMOCLAW_DASHBOARD_PORT → DASHBOARD_PORT → PUBLIC_PORT → default), while the others skip straight to NEMOCLAW_* without checking for plain environment keys. This is inconsistent within the same script.
If the .env uses plain keys like GATEWAY_PORT, VLLM_PORT, or OLLAMA_PORT (whether explicitly set or inherited), those values are ignored in favor of hardcoded defaults—unlike the dashboard port.
Fix
DASHBOARD_PORT="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}"
-GATEWAY_PORT="${NEMOCLAW_GATEWAY_PORT:-8080}"
-VLLM_PORT="${NEMOCLAW_VLLM_PORT:-8000}"
-OLLAMA_PORT="${NEMOCLAW_OLLAMA_PORT:-11434}"
+GATEWAY_PORT="${NEMOCLAW_GATEWAY_PORT:-${GATEWAY_PORT:-8080}}"
+VLLM_PORT="${NEMOCLAW_VLLM_PORT:-${VLLM_PORT:-8000}}"
+OLLAMA_PORT="${NEMOCLAW_OLLAMA_PORT:-${OLLAMA_PORT:-11434}}"Note: bin/lib/ports.js uses the same pattern for gateway/vLLM/ollama (no fallback to plain keys), so verify whether that file should also be updated for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check-ports.sh` around lines 51 - 54, The environment fallback for
port variables is inconsistent: update the assignments in check-ports.sh so
GATEWAY_PORT, VLLM_PORT, and OLLAMA_PORT follow the same fallback chain pattern
used by DASHBOARD_PORT (i.e., NEMOCLAW_<NAME>_PORT → <NAME>_PORT → PUBLIC_PORT →
default) by changing the expansion for GATEWAY_PORT, VLLM_PORT, and OLLAMA_PORT
to include the plain <NAME>_PORT and PUBLIC_PORT fallbacks; also review and, if
needed, mirror the same change in bin/lib/ports.js to keep behavior consistent
across scripts.
Comment on lines
+111
to
+116
| if [[ $# -gt 0 ]]; then | ||
| echo "" | ||
| echo "Custom ports:" | ||
| for p in "$@"; do | ||
| check_port "$p" || true | ||
| done |
There was a problem hiding this comment.
Validate custom port arguments before checking them.
A typo like scripts/check-ports.sh abc currently falls through to lsof failure and gets reported as ok, so manual checks can return a false negative.
🛠️ Suggested fix
if [[ $# -gt 0 ]]; then
echo ""
echo "Custom ports:"
for p in "$@"; do
+ validate_port "custom port" "$p"
check_port "$p" || true
done
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check-ports.sh` around lines 111 - 116, The loop that iterates custom
ports (for p in "$@") must validate each argument before calling check_port to
avoid passing non-numeric strings like "abc" to lsof; add a small validation
(e.g., is_valid_port or inline check using a regex/isdigit and range 1-65535)
that logs and exits/non-zero or skips invalid entries, and only call check_port
"$p" for validated numeric ports—update the script's parameter handling around
the for p in "$@" block to perform this validation and return a clear error for
bad arguments.
Comment on lines
+31
to
+33
| [ -f "${SCRIPT_DIR}/../.env" ] && set -a && . "${SCRIPT_DIR}/../.env" && set +a | ||
| # shellcheck source=/dev/null | ||
| [ -f "${SCRIPT_DIR}/../.env.local" ] && set -a && . "${SCRIPT_DIR}/../.env.local" && set +a |
There was a problem hiding this comment.
Honor DASHBOARD_PORT / PUBLIC_PORT fallbacks in debug diagnostics.
This script only uses NEMOCLAW_DASHBOARD_PORT, so users relying on alias vars supported by bin/lib/ports.js can get incorrect lsof-dashboard checks.
Suggested patch
[ -f "${SCRIPT_DIR}/../.env" ] && set -a && . "${SCRIPT_DIR}/../.env" && set +a
[ -f "${SCRIPT_DIR}/../.env.local" ] && set -a && . "${SCRIPT_DIR}/../.env.local" && set +a
+DEBUG_DASHBOARD_PORT="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}"
@@
- collect "lsof-dashboard" lsof -i :"${NEMOCLAW_DASHBOARD_PORT:-18789}"
+ collect "lsof-dashboard" lsof -i :"${DEBUG_DASHBOARD_PORT}"Also applies to: 294-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/debug.sh` around lines 31 - 33, The debug.sh script's lsof-dashboard
checks currently only read NEMOCLAW_DASHBOARD_PORT so they miss alternate
aliases; update the two places that build/check the dashboard port (the
lsof-dashboard checks) to resolve the port using a fallback chain (use
NEMOCLAW_DASHBOARD_PORT, then DASHBOARD_PORT, then PUBLIC_PORT) instead of only
NEMOCLAW_DASHBOARD_PORT; change the port variable resolution wherever
lsof-dashboard is invoked (the occurrence around the initial env loading and the
similar occurrence later near line ~294) to compute the port once via that
fallback chain and use that resolved variable in the lsof check.
Comment on lines
+209
to
+210
| vllm-local) printf 'http://host.openshell.internal:%s/v1\n' "${NEMOCLAW_VLLM_PORT:-8000}" ;; | ||
| ollama-local) printf 'http://host.openshell.internal:%s/v1\n' "${NEMOCLAW_OLLAMA_PORT:-11434}" ;; |
There was a problem hiding this comment.
Add unprefixed port fallbacks for shell/runtime parity.
These paths ignore VLLM_PORT and OLLAMA_PORT, so alias-based overrides can silently fail in shell health checks and base URL construction.
Suggested patch
get_local_provider_base_url() {
local provider="${1:-}"
+ local vllm_port="${NEMOCLAW_VLLM_PORT:-${VLLM_PORT:-8000}}"
+ local ollama_port="${NEMOCLAW_OLLAMA_PORT:-${OLLAMA_PORT:-11434}}"
case "$provider" in
- vllm-local) printf 'http://host.openshell.internal:%s/v1\n' "${NEMOCLAW_VLLM_PORT:-8000}" ;;
- ollama-local) printf 'http://host.openshell.internal:%s/v1\n' "${NEMOCLAW_OLLAMA_PORT:-11434}" ;;
+ vllm-local) printf 'http://host.openshell.internal:%s/v1\n' "${vllm_port}" ;;
+ ollama-local) printf 'http://host.openshell.internal:%s/v1\n' "${ollama_port}" ;;
*) return 1 ;;
esac
}
check_local_provider_health() {
local provider="${1:-}"
+ local vllm_port="${NEMOCLAW_VLLM_PORT:-${VLLM_PORT:-8000}}"
+ local ollama_port="${NEMOCLAW_OLLAMA_PORT:-${OLLAMA_PORT:-11434}}"
case "$provider" in
vllm-local)
- curl -sf "http://localhost:${NEMOCLAW_VLLM_PORT:-8000}/v1/models" > /dev/null 2>&1
+ curl -sf "http://localhost:${vllm_port}/v1/models" > /dev/null 2>&1
;;
ollama-local)
- curl -sf "http://localhost:${NEMOCLAW_OLLAMA_PORT:-11434}/api/tags" > /dev/null 2>&1
+ curl -sf "http://localhost:${ollama_port}/api/tags" > /dev/null 2>&1
;;Also applies to: 220-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/lib/runtime.sh` around lines 209 - 210, The health-check/base-URL
cases vllm-local and ollama-local use only NEMOCLAW_VLLM_PORT and
NEMOCLAW_OLLAMA_PORT and therefore ignore unprefixed VLLM_PORT/OLLAMA_PORT;
update the printf calls in the vllm-local and ollama-local branches (and the
other similar branches around the other host.openshell.internal cases) to fall
back to the unprefixed env vars before the hardcoded default (i.e. use nested
parameter expansion so NEMOCLAW_VLLM_PORT -> VLLM_PORT -> 8000 and
NEMOCLAW_OLLAMA_PORT -> OLLAMA_PORT -> 11434) so alias-based overrides work for
shell health checks and base URL construction.
Comment on lines
+14
to
+18
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| # shellcheck source=/dev/null | ||
| [ -f "${SCRIPT_DIR}/../.env" ] && set -a && . "${SCRIPT_DIR}/../.env" && set +a | ||
| # shellcheck source=/dev/null | ||
| [ -f "${SCRIPT_DIR}/../.env.local" ] && set -a && . "${SCRIPT_DIR}/../.env.local" && set +a |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the full nemoclaw-start.sh script
cd /dev/null || true
find . -name "nemoclaw-start.sh" -type fRepository: NVIDIA/NemoClaw
Length of output: 153
🏁 Script executed:
# Also look for ports.js mentioned in the comment
find . -name "ports.js" -type fRepository: NVIDIA/NemoClaw
Length of output: 77
🏁 Script executed:
# Search for .env sourcing patterns elsewhere in the codebase
rg 'source.*\.env|\..*\.env' --type sh --type bash -B2 -A2Repository: NVIDIA/NemoClaw
Length of output: 87
🏁 Script executed:
cat -n ./scripts/nemoclaw-start.shRepository: NVIDIA/NemoClaw
Length of output: 5437
🏁 Script executed:
cat -n ./bin/lib/ports.jsRepository: NVIDIA/NemoClaw
Length of output: 1806
🏁 Script executed:
# Search for .env sourcing patterns in sh/bash files
rg 'source.*\.env|\. .*\.env' -A2 -B2Repository: NVIDIA/NemoClaw
Length of output: 5735
Add DASHBOARD_PORT to the fallback chain for PUBLIC_PORT.
Line 21 uses NEMOCLAW_DASHBOARD_PORT → PUBLIC_PORT, but the JavaScript port resolution in bin/lib/ports.js includes an additional fallback: NEMOCLAW_DASHBOARD_PORT → DASHBOARD_PORT → PUBLIC_PORT. Update line 21 to match:
PUBLIC_PORT="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}"
This ensures consistency with the port resolution chain used elsewhere in the PR and prevents desynchronization between PUBLIC_PORT and CHAT_UI_URL when DASHBOARD_PORT is set but PUBLIC_PORT is not.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/nemoclaw-start.sh` around lines 14 - 18, Update the PUBLIC_PORT
fallback chain to include DASHBOARD_PORT so it matches the resolution in
bin/lib/ports.js: change the assignment of PUBLIC_PORT (currently using
NEMOCLAW_DASHBOARD_PORT → PUBLIC_PORT) to use NEMOCLAW_DASHBOARD_PORT →
DASHBOARD_PORT → PUBLIC_PORT (with the existing default 18789). Locate the
PUBLIC_PORT assignment in scripts/nemoclaw-start.sh and modify it to reference
NEMOCLAW_DASHBOARD_PORT, then DASHBOARD_PORT, then PUBLIC_PORT as the fallback
order so CHAT_UI_URL and other consumers stay consistent.
Comment on lines
+21
to
+22
| PUBLIC_PORT="${NEMOCLAW_DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}" | ||
| CHAT_UI_URL="${CHAT_UI_URL:-http://127.0.0.1:${PUBLIC_PORT}}" |
There was a problem hiding this comment.
Restore the legacy DASHBOARD_PORT fallback here.
Line 21 skips DASHBOARD_PORT, even though bin/lib/ports.js still honors it. Existing setups that only export the legacy name will default PUBLIC_PORT back to 18789, which can skew CHAT_UI_URL and the printed dashboard links.
Proposed fix
-PUBLIC_PORT="${NEMOCLAW_DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}"
+PUBLIC_PORT="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}"
CHAT_UI_URL="${CHAT_UI_URL:-http://127.0.0.1:${PUBLIC_PORT}}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| PUBLIC_PORT="${NEMOCLAW_DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}" | |
| CHAT_UI_URL="${CHAT_UI_URL:-http://127.0.0.1:${PUBLIC_PORT}}" | |
| PUBLIC_PORT="${NEMOCLAW_DASHBOARD_PORT:-${DASHBOARD_PORT:-${PUBLIC_PORT:-18789}}}" | |
| CHAT_UI_URL="${CHAT_UI_URL:-http://127.0.0.1:${PUBLIC_PORT}}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/nemoclaw-start.sh` around lines 21 - 22, PUBLIC_PORT is currently set
from NEMOCLAW_DASHBOARD_PORT then PUBLIC_PORT then 18789, but omits the legacy
DASHBOARD_PORT env var; update the PUBLIC_PORT assignment to also fallback to
DASHBOARD_PORT (so it reads NEMOCLAW_DASHBOARD_PORT -> DASHBOARD_PORT ->
PUBLIC_PORT -> 18789) so existing setups honored by bin/lib/ports.js keep the
intended port, and leave CHAT_UI_URL constructed from PUBLIC_PORT unchanged.
Comment on lines
+35
to
+37
| # Load port overrides if present (.env.local takes precedence) | ||
| [ -f "${REPO_DIR}/.env" ] && set -a && . "${REPO_DIR}/.env" && set +a | ||
| [ -f "${REPO_DIR}/.env.local" ] && set -a && . "${REPO_DIR}/.env.local" && set +a |
There was a problem hiding this comment.
Setup path should normalize alias env vars before use.
GATEWAY_PORT / OLLAMA_PORT overrides are not honored here unless prefixed variants are set, which can break expected behavior from the new port configuration flow.
Suggested patch
# Load port overrides if present (.env.local takes precedence)
[ -f "${REPO_DIR}/.env" ] && set -a && . "${REPO_DIR}/.env" && set +a
[ -f "${REPO_DIR}/.env.local" ] && set -a && . "${REPO_DIR}/.env.local" && set +a
+NEMOCLAW_GATEWAY_PORT="${NEMOCLAW_GATEWAY_PORT:-${GATEWAY_PORT:-8080}}"
+NEMOCLAW_OLLAMA_PORT="${NEMOCLAW_OLLAMA_PORT:-${OLLAMA_PORT:-11434}}"Also applies to: 114-114, 164-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/setup.sh` around lines 35 - 37, After sourcing .env and .env.local,
add normalization that maps the unprefixed alias env vars GATEWAY_PORT and
OLLAMA_PORT into the canonical prefixed env vars the rest of the script expects
(i.e., if GATEWAY_PORT is set and the prefixed variant is not, export the
prefixed variable with GATEWAY_PORT's value; do the same for OLLAMA_PORT), so
the overrides are honored regardless of whether callers set the aliased or
prefixed names.
4 tasks
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
GATEWAY_PORT,DASHBOARD_PORT,OLLAMA_PORT,VLLM_PORT) via.env/.env.localfilesbin/lib/env.js(env loader with git-root + CWD search) andbin/lib/ports.js(central port config)preflight()with.env.localoverride guidancescripts/check-ports.shdiagnostic utility,docs/reference/port-configuration.md, andtest/env.test.jsBug fixes included
bin/lib/env.js—.env.localnot found on fresh installs (only searched__dirname; now also searches git root and CWD)bin/lib/onboard.js— No.envbootstrapped on fresh clone; unhelpful port-conflict error (now copies.env.example→.envand shows.env.localoverride instructions)nemoclaw-start.sh,debug.sh,brev-setup.sh, etc.) —.envnever loaded, so port overrides had no effectscripts/telegram-bridge.js— Removed unusedcryptoimporttest/onboard-readiness.test.js— Hardcoded port 8080; now reads from ports configbin/lib/env.js/scripts/check-ports.sh/uninstall.sh— Inline comment stripping corrupted quoted values containing#(fixed parse order per CodeRabbit review)bin/lib/onboard.js— Custom dashboard port not reaching build-time Dockerfile ARG (now patches Dockerfile copy before build)bin/lib/onboard.js— Ollama startup accepted without health verification (now retries with curl probe)CodeRabbit review status
All actionable findings from PR #639 are addressed. One documented first-run edge case remains (ports resolved at module load before
.env.examplecopy — defaults match, so behavior is identical).Test plan
npm test— 311 tests (81 TS + 230 JS), all passing (5 pre-existing upstream failures ininstall-preflight.test.js)node --test test/env.test.js test/preflight.test.js test/onboard-readiness.test.js— 46/46 port-config tests passnode bin/nemoclaw.js --version/--help— CLI smoke testos.environsecurity fix (C-2)Supersedes #639.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests