From cb89872c280ce03d5ae9eedd80cbf5813c5672f9 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 16 Aug 2026 18:18:03 +0800 Subject: [PATCH] fix(install): clean up a broken managed Node and guard the termux probe AI-review follow-up on #87467: - On probe failure, remove the extracted ~/.hermes/node tree and the node/npm/npx bin links so later installer steps and retry runs start clean instead of resolving node to a binary that cannot start. - The termux pkg branch had the same silent-success class: an empty version probe logged success and set HAS_NODE=true. Degrade with the binary's own error instead. --- scripts/install.sh | 19 ++++++++++++++++--- tests/test_install_sh_node_probe_87460.py | 12 ++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 8367242c38..ea69ab6329 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -980,8 +980,17 @@ install_node() { if pkg install -y nodejs >/dev/null; then local installed_ver installed_ver=$(node --version 2>/dev/null || true) - log_success "Node.js $installed_ver installed via pkg" - HAS_NODE=true + if [ -n "$installed_ver" ]; then + log_success "Node.js $installed_ver installed via pkg" + HAS_NODE=true + else + # pkg succeeded but the binary cannot start — the same + # silent-success class the managed-download probe guards + # against (#87460). Degrade instead of claiming success. + log_error "Node.js installed via pkg failed to start:" + node --version >&2 || true + HAS_NODE=false + fi else log_warn "Failed to install Node.js via pkg" HAS_NODE=false @@ -1106,10 +1115,14 @@ install_node() { # whole installer at exit 127 with the loader's explanation # discarded by 2>/dev/null — installs died mid-sentence with no # output at all (#87460). Degrade instead, and surface the real - # error the loader printed. + # error the loader printed. Remove the broken tree and the bin + # links so later steps and retry runs start clean instead of + # resolving `node` to a binary that cannot start. log_error "Downloaded Node.js failed to start:" printf '%s\n' "$installed_ver" >&2 log_info "On Debian/Ubuntu the usual fix is: sudo apt-get install -y libatomic1" + rm -rf "$HERMES_HOME/node" + rm -f "$node_link_dir/node" "$node_link_dir/npm" "$node_link_dir/npx" HAS_NODE=false return 0 fi diff --git a/tests/test_install_sh_node_probe_87460.py b/tests/test_install_sh_node_probe_87460.py index d0b91a89cc..719caa1b67 100644 --- a/tests/test_install_sh_node_probe_87460.py +++ b/tests/test_install_sh_node_probe_87460.py @@ -153,6 +153,18 @@ def test_broken_node_degrades_with_clear_error(tmp_path: Path) -> None: assert "apt-get install -y libatomic1" in stdout + stderr assert "HAS_NODE=false" in stdout assert "HAS_NODE=true" not in stdout + # AI-review follow-up: the broken tree and bin links must not linger + # — retries and later installer steps resolve `node` cleanly. + home = tmp_path / "home" + link_dir = tmp_path / "links" + assert not (home / "node").exists(), "broken managed Node tree left behind" + assert not (link_dir / "node").exists(), "broken node symlink left behind" + assert not (link_dir / "npm").exists(), "broken npm symlink left behind" + # AI-review follow-up: the broken tree and bin links must not linger + # — retries and later installer steps resolve `node` cleanly. + assert not (home / "node").exists(), "broken managed Node tree left behind" + assert not (link_dir / "node").exists(), "broken node symlink left behind" + assert not (link_dir / "npm").exists(), "broken npm symlink left behind" def test_healthy_node_reports_success(tmp_path: Path) -> None: