From 777f7409d8140207fd72a1b9a7cca2d223fb48e7 Mon Sep 17 00:00:00 2001 From: mushuiyu_xydt Date: Thu, 11 Jun 2026 21:58:43 +0800 Subject: [PATCH] fix(installer): stop after failed Node package installs Linux Node package-manager setup/install failures now fail the installer immediately instead of falling through to a misleading success path. Adds regression coverage for NodeSource setup and apt nodejs install failures under conditional shell invocation.\n\nFixes #73837\n\nProof: bash -n scripts/install.sh; node scripts/run-vitest.mjs test/scripts/install-sh.test.ts; node scripts/run-oxlint.mjs test/scripts/install-sh.test.ts; git diff --check origin/main...HEAD; autoreview clean; Azure Crabbox check:changed cbx_6286dc1e287b passed. --- scripts/install.sh | 51 +++++++++++-------- test/scripts/install-sh.test.ts | 88 +++++++++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 25 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index f28b3b912d71..57ad560d22d3 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -524,6 +524,15 @@ run_quiet_step() { return 1 } +run_required_step() { + local title="$1" + shift + if run_quiet_step "$title" "$@"; then + return 0 + fi + exit 1 +} + cleanup_legacy_submodules() { local repo_dir="$1" local legacy_dir="$repo_dir/Peekaboo" @@ -1747,9 +1756,9 @@ finish_linux_node_install() { install_node_with_apk() { ui_info "Installing Node.js via apk (Alpine Linux detected)" if is_root; then - run_quiet_step "Installing Node.js" apk add --no-cache nodejs npm + run_required_step "Installing Node.js" apk add --no-cache nodejs npm else - run_quiet_step "Installing Node.js" sudo apk add --no-cache nodejs npm + run_required_step "Installing Node.js" sudo apk add --no-cache nodejs npm fi activate_supported_node_on_path || true @@ -1763,9 +1772,9 @@ install_node_with_apk() { ui_warn "Alpine nodejs package installed ${apk_node_version}, below required v${NODE_MIN_VERSION}+" ui_info "Trying Alpine nodejs-current package" if is_root; then - run_quiet_step "Installing nodejs-current" apk add --no-cache nodejs-current npm + run_required_step "Installing nodejs-current" apk add --no-cache nodejs-current npm else - run_quiet_step "Installing nodejs-current" sudo apk add --no-cache nodejs-current npm + run_required_step "Installing nodejs-current" sudo apk add --no-cache nodejs-current npm fi activate_supported_node_on_path || true @@ -1810,9 +1819,9 @@ install_node() { if command -v pacman &> /dev/null || is_arch_linux; then ui_info "Installing Node.js via pacman (Arch-based distribution detected)" if is_root; then - run_quiet_step "Installing Node.js" pacman -Sy --noconfirm nodejs npm + run_required_step "Installing Node.js" pacman -Sy --noconfirm nodejs npm else - run_quiet_step "Installing Node.js" sudo pacman -Sy --noconfirm nodejs npm + run_required_step "Installing Node.js" sudo pacman -Sy --noconfirm nodejs npm fi finish_linux_node_install return 0 @@ -1827,35 +1836,35 @@ install_node() { if command -v apt-get &> /dev/null; then local tmp tmp="$(mktempfile)" - run_quiet_step "Downloading NodeSource setup script" download_file "https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp" + run_required_step "Downloading NodeSource setup script" download_file "https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp" if is_root; then - run_quiet_step "Configuring NodeSource repository" bash "$tmp" - run_quiet_step "Installing Node.js" apt_get_install nodejs + run_required_step "Configuring NodeSource repository" bash "$tmp" + run_required_step "Installing Node.js" apt_get_install nodejs else - run_quiet_step "Configuring NodeSource repository" sudo -E bash "$tmp" - run_quiet_step "Installing Node.js" apt_get_install nodejs + run_required_step "Configuring NodeSource repository" sudo -E bash "$tmp" + run_required_step "Installing Node.js" apt_get_install nodejs fi elif command -v dnf &> /dev/null; then local tmp tmp="$(mktempfile)" - run_quiet_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp" + run_required_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp" if is_root; then - run_quiet_step "Configuring NodeSource repository" bash "$tmp" - run_quiet_step "Installing Node.js" dnf install -y -q nodejs + run_required_step "Configuring NodeSource repository" bash "$tmp" + run_required_step "Installing Node.js" dnf install -y -q nodejs else - run_quiet_step "Configuring NodeSource repository" sudo bash "$tmp" - run_quiet_step "Installing Node.js" sudo dnf install -y -q nodejs + run_required_step "Configuring NodeSource repository" sudo bash "$tmp" + run_required_step "Installing Node.js" sudo dnf install -y -q nodejs fi elif command -v yum &> /dev/null; then local tmp tmp="$(mktempfile)" - run_quiet_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp" + run_required_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp" if is_root; then - run_quiet_step "Configuring NodeSource repository" bash "$tmp" - run_quiet_step "Installing Node.js" yum install -y -q nodejs + run_required_step "Configuring NodeSource repository" bash "$tmp" + run_required_step "Installing Node.js" yum install -y -q nodejs else - run_quiet_step "Configuring NodeSource repository" sudo bash "$tmp" - run_quiet_step "Installing Node.js" sudo yum install -y -q nodejs + run_required_step "Configuring NodeSource repository" sudo bash "$tmp" + run_required_step "Installing Node.js" sudo yum install -y -q nodejs fi else ui_error "Could not detect package manager" diff --git a/test/scripts/install-sh.test.ts b/test/scripts/install-sh.test.ts index c1b4b3dfbdf9..16d41e198ef8 100644 --- a/test/scripts/install-sh.test.ts +++ b/test/scripts/install-sh.test.ts @@ -160,12 +160,14 @@ describe("install.sh", () => { expect(script).toContain("is_alpine_linux()"); expect(script).toContain("install_node_with_apk()"); expect(script).toContain('ui_info "Installing Node.js via apk (Alpine Linux detected)"'); - expect(script).toContain('run_quiet_step "Installing Node.js" apk add --no-cache nodejs npm'); expect(script).toContain( - 'run_quiet_step "Installing Node.js" sudo apk add --no-cache nodejs npm', + 'run_required_step "Installing Node.js" apk add --no-cache nodejs npm', ); expect(script).toContain( - 'run_quiet_step "Installing nodejs-current" apk add --no-cache nodejs-current npm', + 'run_required_step "Installing Node.js" sudo apk add --no-cache nodejs npm', + ); + expect(script).toContain( + 'run_required_step "Installing nodejs-current" apk add --no-cache nodejs-current npm', ); expect(script).toContain("if ! node_is_at_least_required; then"); @@ -286,6 +288,84 @@ describe("install.sh", () => { expect(result.stdout).toContain("Use Alpine 3.21+ or install Node.js 24 manually"); }); + it("stops when NodeSource repository setup fails", () => { + const result = runInstallShell(` + set -euo pipefail + source "${SCRIPT_PATH}" + OS=linux + require_sudo() { :; } + install_build_tools_linux() { return 0; } + is_root() { return 0; } + is_alpine_linux() { return 1; } + apt-get() { :; } + download_file() { :; } + ui_info() { printf 'info:%s\\n' "$*"; } + ui_success() { printf 'success:%s\\n' "$*"; } + ui_error() { printf 'error:%s\\n' "$*"; } + run_quiet_step() { + printf 'step:%s|%s\\n' "$1" "\${*:2}" + if [[ "$1" == "Configuring NodeSource repository" ]]; then + return 64 + fi + return 0 + } + node() { + if [[ "\${1:-}" == "-v" ]]; then + printf 'v24.0.0\\n' + fi + } + activate_supported_node_on_path() { :; } + if install_node; then + echo "install_node returned success" + fi + `); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("step:Configuring NodeSource repository|bash"); + expect(result.stdout).not.toContain("step:Installing Node.js|apt_get_install nodejs"); + expect(result.stdout).not.toContain("success:Node.js v24.0.0 installed"); + expect(result.stdout).not.toContain("install_node returned success"); + }); + + it("stops when apt cannot install the Node.js package", () => { + const result = runInstallShell(` + set -euo pipefail + source "${SCRIPT_PATH}" + OS=linux + require_sudo() { :; } + install_build_tools_linux() { return 0; } + is_root() { return 0; } + is_alpine_linux() { return 1; } + apt-get() { :; } + download_file() { :; } + ui_info() { printf 'info:%s\\n' "$*"; } + ui_success() { printf 'success:%s\\n' "$*"; } + ui_error() { printf 'error:%s\\n' "$*"; } + run_quiet_step() { + printf 'step:%s|%s\\n' "$1" "\${*:2}" + if [[ "$1" == "Installing Node.js" ]]; then + return 65 + fi + return 0 + } + node() { + if [[ "\${1:-}" == "-v" ]]; then + printf 'v24.0.0\\n' + fi + } + activate_supported_node_on_path() { :; } + if install_node; then + echo "install_node returned success" + fi + `); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("step:Configuring NodeSource repository|bash"); + expect(result.stdout).toContain("step:Installing Node.js|apt_get_install nodejs"); + expect(result.stdout).not.toContain("success:Node.js v24.0.0 installed"); + expect(result.stdout).not.toContain("install_node returned success"); + }); + it("installs Git with apk on Alpine", () => { const tmp = mkdtempSync(join(tmpdir(), "openclaw-install-git-apk-")); const bin = join(tmp, "bin"); @@ -769,7 +849,7 @@ describe("install.sh", () => { /detect_os_or_die\s+if \[\[ "\$OS" == "linux" \]\]; then\s+export DEBIAN_FRONTEND="\$\{DEBIAN_FRONTEND:-noninteractive\}"\s+export NEEDRESTART_MODE="\$\{NEEDRESTART_MODE:-a\}"\s+fi/m, ); expect(script).toContain( - 'run_quiet_step "Configuring NodeSource repository" sudo -E bash "$tmp"', + 'run_required_step "Configuring NodeSource repository" sudo -E bash "$tmp"', ); });