Skip to content

feat(env): add configurable port overrides via environment variables#683

Open
jnun wants to merge 3 commits intoNVIDIA:mainfrom
jnun:feat/configurable-ports-v3
Open

feat(env): add configurable port overrides via environment variables#683
jnun wants to merge 3 commits intoNVIDIA:mainfrom
jnun:feat/configurable-ports-v3

Conversation

@jnun
Copy link

@jnun jnun commented Mar 23, 2026

Summary

  • Add bin/lib/ports.js — central port configuration that reads NEMOCLAW_GATEWAY_PORT, NEMOCLAW_DASHBOARD_PORT, NEMOCLAW_VLLM_PORT, and NEMOCLAW_OLLAMA_PORT from environment variables with validation and sensible defaults
  • Replace all hardcoded port literals in the JS source with configurable constants
  • Follows the existing process.env.NEMOCLAW_* pattern used by NEMOCLAW_MODEL, NEMOCLAW_PROVIDER, and NEMOCLAW_GPU

Problem

The default ports (8080, 8000, 18789) conflict with common services (Jenkins, Django, Tomcat, K8s dashboards). Users have no way to change them without editing source code.

Fix

One new file (bin/lib/ports.js, 23 lines), five surgical edits. No new infrastructure, no .env file loading, no shell script changes. Environment variables override defaults:

$ export NEMOCLAW_GATEWAY_PORT=8081
$ nemoclaw onboard

Validation rejects invalid values (non-integer, out of range 1024-65535).

Files changed

File Change
bin/lib/ports.js New — env var reader with validation
bin/nemoclaw.js Import + use DASHBOARD_PORT
bin/lib/onboard.js Import + replace port literals
bin/lib/local-inference.js Import + replace port literals
bin/lib/nim.js Import + replace port literals
bin/lib/preflight.js Import + replace port literals

Test plan

  • npx vitest run — 214/219 pass (5 pre-existing failures in install-preflight.test.js)
  • node bin/nemoclaw.js onboard with NEMOCLAW_GATEWAY_PORT=8081 — gateway starts on configured port
  • node bin/nemoclaw.js --help / --version — CLI smoke test
  • Default ports work when no env vars are set

Supersedes #652.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Replaced hardcoded ports with a centralized, environment-configurable port configuration (dashboard, gateway, vLLM, Ollama) with sensible defaults.
    • Health checks, startup/probe commands, dashboard URL, and port-availability checks now respect configured ports and validate port values.

jnun and others added 2 commits March 22, 2026 20:29
Allow users to override default service ports (NEMOCLAW_GATEWAY_PORT,
NEMOCLAW_DASHBOARD_PORT, NEMOCLAW_VLLM_PORT, NEMOCLAW_OLLAMA_PORT)
through environment variables. This prevents conflicts when default
ports are already in use on the host.

New file:
- bin/lib/ports.js — central port config with env var overrides
  and validation (range 1024-65535)

Modified files:
- bin/nemoclaw.js — use DASHBOARD_PORT for port forwarding
- bin/lib/onboard.js — use port constants for gateway, dashboard,
  inference detection, and Ollama startup
- bin/lib/local-inference.js — use VLLM_PORT and OLLAMA_PORT for
  health checks, base URLs, and error messages
- bin/lib/nim.js — use VLLM_PORT for NIM container port mapping
- bin/lib/preflight.js — use DASHBOARD_PORT as default check

Follows the existing process.env.NEMOCLAW_* pattern used by
NEMOCLAW_MODEL, NEMOCLAW_PROVIDER, and NEMOCLAW_GPU.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: db8b65dc-3b60-409a-9f47-7bbc7ebbb2c9

📥 Commits

Reviewing files that changed from the base of the PR and between 0d231db and 8174545.

📒 Files selected for processing (1)
  • bin/lib/ports.js
✅ Files skipped from review due to trivial changes (1)
  • bin/lib/ports.js

📝 Walkthrough

Walkthrough

Centralizes port configuration by adding bin/lib/ports.js (env-driven port constants) and replaces hardcoded ports across onboarding, local inference, preflight, and nim container logic to use those exported constants.

Changes

Cohort / File(s) Summary
Port Configuration Module
bin/lib/ports.js
New module adding parsePort() and exporting DASHBOARD_PORT, GATEWAY_PORT, VLLM_PORT, OLLAMA_PORT sourced from NEMOCLAW_* env vars with validated numeric fallbacks.
Local inference & CLI helpers
bin/lib/local-inference.js, bin/lib/nim.js, bin/lib/onboard.js
Replaced hardcoded vLLM/Ollama ports (8000/11434) with VLLM_PORT/OLLAMA_PORT in base URLs, health checks, curl commands, warmup/probe endpoints, and container reachability logic. (startNimContainer and waitForNimHealth default param updated to VLLM_PORT.)
Application / startup checks
bin/lib/preflight.js, bin/nemoclaw.js
Replaced hardcoded dashboard port (18789) with DASHBOARD_PORT in port-availability checks and OpenShell port-forward invocation. Also updated gateway arg construction to use GATEWAY_PORT where applicable.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nudged the ports from stone to vine,
Env-vars now guide each little line.
No more hard hops, I dance and sing,
Configured springs in every spring! 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: introducing environment variable-based configuration for service ports across the codebase.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bin/lib/ports.js`:
- Around line 9-13: The port validation currently uses parseInt on raw which
allows strings like "8081abc" to pass; update the validation in the function
that parses environment ports (the block creating `parsed` from `raw`) to first
ensure `raw` matches /^\d+$/ (digits-only) and only then convert using
`Number(raw)` (not parseInt), keeping the existing range check (>=1024 &&
<=65535) and throwing the same Error message when validation fails.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 31500cfe-58e2-4ff5-81fc-f8bfee7826e0

📥 Commits

Reviewing files that changed from the base of the PR and between ffa1283 and 0d231db.

📒 Files selected for processing (6)
  • bin/lib/local-inference.js
  • bin/lib/nim.js
  • bin/lib/onboard.js
  • bin/lib/ports.js
  • bin/lib/preflight.js
  • bin/nemoclaw.js

…iables

- Tighten parsePort validation: reject non-digit strings (e.g. "8081abc")
  that parseInt silently accepted; use regex + Number() instead
- Add JSDoc to parsePort and all exported constants (docstring coverage)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant