-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(install): check ports 8080 and 18789 are free before starting gat… #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,47 @@ version_major() { | |
| printf '%s\n' "${1#v}" | cut -d. -f1 | ||
| } | ||
|
|
||
| # ── Port availability preflight ────────────────────────────────────── | ||
| # NemoClaw requires two ports to be free before setup starts: | ||
| # NEMOCLAW_GATEWAY_PORT (default 8080) – OpenShell gateway | ||
| # NEMOCLAW_DASHBOARD_PORT (default 18789) – OpenClaw dashboard | ||
| # Override via env var to avoid killing existing services. | ||
| NEMOCLAW_GATEWAY_PORT="${NEMOCLAW_GATEWAY_PORT:-8080}" | ||
| NEMOCLAW_DASHBOARD_PORT="${NEMOCLAW_DASHBOARD_PORT:-18789}" | ||
|
|
||
| _port_in_use() { | ||
| local p="$1" | ||
| if command -v ss &>/dev/null; then | ||
| ss -tlnH 2>/dev/null | awk '{print $4}' | grep -q ":${p}$" | ||
| return $? | ||
| elif command -v netstat &>/dev/null; then | ||
| netstat -tlnH 2>/dev/null | awk '{print $4}' | grep -q ":${p}$" | ||
| return $? | ||
| fi | ||
| return 1 | ||
| } | ||
|
|
||
| check_required_ports() { | ||
| local failed=0 | ||
| for spec in "${NEMOCLAW_GATEWAY_PORT}:gateway" "${NEMOCLAW_DASHBOARD_PORT}:dashboard"; do | ||
| local port="${spec%%:*}" label="${spec##*:}" | ||
| if _port_in_use "$port"; then | ||
| echo "[ERROR] Port $port ($label) is already in use." >&2 | ||
| echo "[ERROR] Find the process : ss -tlnp | grep :$port" >&2 | ||
| echo "[ERROR] Or override : export NEMOCLAW_${label^^}_PORT=<free>" >&2 | ||
| failed=1 | ||
| fi | ||
| done | ||
| if [ "$failed" -eq 1 ]; then | ||
| echo "" >&2 | ||
| echo "[ERROR] Free the ports above (or set override env vars), then re-run." >&2 | ||
| exit 1 | ||
| fi | ||
| echo "[INFO] Ports ${NEMOCLAW_GATEWAY_PORT} (gateway) and ${NEMOCLAW_DASHBOARD_PORT} (dashboard) are free." | ||
| } | ||
| # ───────────────────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| ensure_supported_runtime() { | ||
| command_exists node || error "${RUNTIME_REQUIREMENT_MSG} Node.js was not found on PATH." | ||
| command_exists npm || error "${RUNTIME_REQUIREMENT_MSG} npm was not found on PATH." | ||
|
|
@@ -98,6 +139,7 @@ install_nodejs() { | |
| || { rm -f "$nvm_tmp"; error "Failed to download nvm installer"; } | ||
| local actual_hash | ||
| if command_exists sha256sum; then | ||
| check_required_ports | ||
| actual_hash="$(sha256sum "$nvm_tmp" | awk '{print $1}')" | ||
|
Comment on lines
141
to
143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: The port check is nested inside the
Move 🐛 Proposed fixRemove the misplaced call from line 142: if command_exists sha256sum; then
-check_required_ports
actual_hash="$(sha256sum "$nvm_tmp" | awk '{print $1}')"Add the port check at the start of main() {
info "=== NemoClaw Installer ==="
+
+ check_required_ports
install_nodejs🤖 Prompt for AI Agents |
||
| elif command_exists shasum; then | ||
| actual_hash="$(shasum -a 256 "$nvm_tmp" | awk '{print $1}')" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 203
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 92
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 332
🌐 Web query:
netstat -H flag compatibility unix systems💡 Result:
netstat -His not a portable/standardnetstatoption across Unix-like systems (Linuxnet-toolsnetstatdoesn’t document-H, and BSDnetstatuses-hfor other purposes, not-H). (mankier.com)What is widely compatible is
Has a routing-table output flag (not a command-line option):netstat -rn, the routing table “Flags” column can includeHmeaning “route is to a host and not a network”. (docs.oracle.com)H(RTF_HOST) meaning “Host entry (net otherwise)”. (man.freebsd.org)So if you’re trying to detect “host routes” portably, do something like:
netstat -rnand then interpret the Flags column; don’t rely on a-Hoption. (docs.oracle.com)Citations:
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 1622
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 305
netstatdoes not support the-Hflag on most Unix-like systems, causing silent failures on BSD, macOS, and traditional Linux installations.The
-H(no header) flag is specific tossfrom iproute2. Traditionalnetstaton BSD, macOS, and systems using net-tools does not recognize-Hand will error or ignore it. Whennetstat -tlnHfails, the fallback returns 1 (port free), which silently misses port conflicts during deployment.Additionally, the grep pattern
:${p}$may not match all IPv6 listener formats consistently.🐛 Proposed fix
_port_in_use() { local p="$1" if command -v ss &>/dev/null; then ss -tlnH 2>/dev/null | awk '{print $4}' | grep -q ":${p}$" return $? elif command -v netstat &>/dev/null; then - netstat -tlnH 2>/dev/null | awk '{print $4}' | grep -q ":${p}$" + # netstat output varies by OS; skip header with tail, match port at end + netstat -tln 2>/dev/null | tail -n +3 | awk '{print $4}' | grep -qE "[:.]${p}$" return $? fi return 1 }📝 Committable suggestion
🤖 Prompt for AI Agents