Skip to content

Commit 777f740

Browse files
mushuiyu886mushuiyu886
authored
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.
1 parent 2bec2ca commit 777f740

2 files changed

Lines changed: 114 additions & 25 deletions

File tree

‎scripts/install.sh‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,15 @@ run_quiet_step() {
524524
return 1
525525
}
526526

527+
run_required_step() {
528+
local title="$1"
529+
shift
530+
if run_quiet_step "$title" "$@"; then
531+
return 0
532+
fi
533+
exit 1
534+
}
535+
527536
cleanup_legacy_submodules() {
528537
local repo_dir="$1"
529538
local legacy_dir="$repo_dir/Peekaboo"
@@ -1747,9 +1756,9 @@ finish_linux_node_install() {
17471756
install_node_with_apk() {
17481757
ui_info "Installing Node.js via apk (Alpine Linux detected)"
17491758
if is_root; then
1750-
run_quiet_step "Installing Node.js" apk add --no-cache nodejs npm
1759+
run_required_step "Installing Node.js" apk add --no-cache nodejs npm
17511760
else
1752-
run_quiet_step "Installing Node.js" sudo apk add --no-cache nodejs npm
1761+
run_required_step "Installing Node.js" sudo apk add --no-cache nodejs npm
17531762
fi
17541763

17551764
activate_supported_node_on_path || true
@@ -1763,9 +1772,9 @@ install_node_with_apk() {
17631772
ui_warn "Alpine nodejs package installed ${apk_node_version}, below required v${NODE_MIN_VERSION}+"
17641773
ui_info "Trying Alpine nodejs-current package"
17651774
if is_root; then
1766-
run_quiet_step "Installing nodejs-current" apk add --no-cache nodejs-current npm
1775+
run_required_step "Installing nodejs-current" apk add --no-cache nodejs-current npm
17671776
else
1768-
run_quiet_step "Installing nodejs-current" sudo apk add --no-cache nodejs-current npm
1777+
run_required_step "Installing nodejs-current" sudo apk add --no-cache nodejs-current npm
17691778
fi
17701779

17711780
activate_supported_node_on_path || true
@@ -1810,9 +1819,9 @@ install_node() {
18101819
if command -v pacman &> /dev/null || is_arch_linux; then
18111820
ui_info "Installing Node.js via pacman (Arch-based distribution detected)"
18121821
if is_root; then
1813-
run_quiet_step "Installing Node.js" pacman -Sy --noconfirm nodejs npm
1822+
run_required_step "Installing Node.js" pacman -Sy --noconfirm nodejs npm
18141823
else
1815-
run_quiet_step "Installing Node.js" sudo pacman -Sy --noconfirm nodejs npm
1824+
run_required_step "Installing Node.js" sudo pacman -Sy --noconfirm nodejs npm
18161825
fi
18171826
finish_linux_node_install
18181827
return 0
@@ -1827,35 +1836,35 @@ install_node() {
18271836
if command -v apt-get &> /dev/null; then
18281837
local tmp
18291838
tmp="$(mktempfile)"
1830-
run_quiet_step "Downloading NodeSource setup script" download_file "https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
1839+
run_required_step "Downloading NodeSource setup script" download_file "https://deb.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
18311840
if is_root; then
1832-
run_quiet_step "Configuring NodeSource repository" bash "$tmp"
1833-
run_quiet_step "Installing Node.js" apt_get_install nodejs
1841+
run_required_step "Configuring NodeSource repository" bash "$tmp"
1842+
run_required_step "Installing Node.js" apt_get_install nodejs
18341843
else
1835-
run_quiet_step "Configuring NodeSource repository" sudo -E bash "$tmp"
1836-
run_quiet_step "Installing Node.js" apt_get_install nodejs
1844+
run_required_step "Configuring NodeSource repository" sudo -E bash "$tmp"
1845+
run_required_step "Installing Node.js" apt_get_install nodejs
18371846
fi
18381847
elif command -v dnf &> /dev/null; then
18391848
local tmp
18401849
tmp="$(mktempfile)"
1841-
run_quiet_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
1850+
run_required_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
18421851
if is_root; then
1843-
run_quiet_step "Configuring NodeSource repository" bash "$tmp"
1844-
run_quiet_step "Installing Node.js" dnf install -y -q nodejs
1852+
run_required_step "Configuring NodeSource repository" bash "$tmp"
1853+
run_required_step "Installing Node.js" dnf install -y -q nodejs
18451854
else
1846-
run_quiet_step "Configuring NodeSource repository" sudo bash "$tmp"
1847-
run_quiet_step "Installing Node.js" sudo dnf install -y -q nodejs
1855+
run_required_step "Configuring NodeSource repository" sudo bash "$tmp"
1856+
run_required_step "Installing Node.js" sudo dnf install -y -q nodejs
18481857
fi
18491858
elif command -v yum &> /dev/null; then
18501859
local tmp
18511860
tmp="$(mktempfile)"
1852-
run_quiet_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
1861+
run_required_step "Downloading NodeSource setup script" download_file "https://rpm.nodesource.com/setup_${NODE_DEFAULT_MAJOR}.x" "$tmp"
18531862
if is_root; then
1854-
run_quiet_step "Configuring NodeSource repository" bash "$tmp"
1855-
run_quiet_step "Installing Node.js" yum install -y -q nodejs
1863+
run_required_step "Configuring NodeSource repository" bash "$tmp"
1864+
run_required_step "Installing Node.js" yum install -y -q nodejs
18561865
else
1857-
run_quiet_step "Configuring NodeSource repository" sudo bash "$tmp"
1858-
run_quiet_step "Installing Node.js" sudo yum install -y -q nodejs
1866+
run_required_step "Configuring NodeSource repository" sudo bash "$tmp"
1867+
run_required_step "Installing Node.js" sudo yum install -y -q nodejs
18591868
fi
18601869
else
18611870
ui_error "Could not detect package manager"

‎test/scripts/install-sh.test.ts‎

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,14 @@ describe("install.sh", () => {
160160
expect(script).toContain("is_alpine_linux()");
161161
expect(script).toContain("install_node_with_apk()");
162162
expect(script).toContain('ui_info "Installing Node.js via apk (Alpine Linux detected)"');
163-
expect(script).toContain('run_quiet_step "Installing Node.js" apk add --no-cache nodejs npm');
164163
expect(script).toContain(
165-
'run_quiet_step "Installing Node.js" sudo apk add --no-cache nodejs npm',
164+
'run_required_step "Installing Node.js" apk add --no-cache nodejs npm',
166165
);
167166
expect(script).toContain(
168-
'run_quiet_step "Installing nodejs-current" apk add --no-cache nodejs-current npm',
167+
'run_required_step "Installing Node.js" sudo apk add --no-cache nodejs npm',
168+
);
169+
expect(script).toContain(
170+
'run_required_step "Installing nodejs-current" apk add --no-cache nodejs-current npm',
169171
);
170172
expect(script).toContain("if ! node_is_at_least_required; then");
171173

@@ -286,6 +288,84 @@ describe("install.sh", () => {
286288
expect(result.stdout).toContain("Use Alpine 3.21+ or install Node.js 24 manually");
287289
});
288290

291+
it("stops when NodeSource repository setup fails", () => {
292+
const result = runInstallShell(`
293+
set -euo pipefail
294+
source "${SCRIPT_PATH}"
295+
OS=linux
296+
require_sudo() { :; }
297+
install_build_tools_linux() { return 0; }
298+
is_root() { return 0; }
299+
is_alpine_linux() { return 1; }
300+
apt-get() { :; }
301+
download_file() { :; }
302+
ui_info() { printf 'info:%s\\n' "$*"; }
303+
ui_success() { printf 'success:%s\\n' "$*"; }
304+
ui_error() { printf 'error:%s\\n' "$*"; }
305+
run_quiet_step() {
306+
printf 'step:%s|%s\\n' "$1" "\${*:2}"
307+
if [[ "$1" == "Configuring NodeSource repository" ]]; then
308+
return 64
309+
fi
310+
return 0
311+
}
312+
node() {
313+
if [[ "\${1:-}" == "-v" ]]; then
314+
printf 'v24.0.0\\n'
315+
fi
316+
}
317+
activate_supported_node_on_path() { :; }
318+
if install_node; then
319+
echo "install_node returned success"
320+
fi
321+
`);
322+
323+
expect(result.status).toBe(1);
324+
expect(result.stdout).toContain("step:Configuring NodeSource repository|bash");
325+
expect(result.stdout).not.toContain("step:Installing Node.js|apt_get_install nodejs");
326+
expect(result.stdout).not.toContain("success:Node.js v24.0.0 installed");
327+
expect(result.stdout).not.toContain("install_node returned success");
328+
});
329+
330+
it("stops when apt cannot install the Node.js package", () => {
331+
const result = runInstallShell(`
332+
set -euo pipefail
333+
source "${SCRIPT_PATH}"
334+
OS=linux
335+
require_sudo() { :; }
336+
install_build_tools_linux() { return 0; }
337+
is_root() { return 0; }
338+
is_alpine_linux() { return 1; }
339+
apt-get() { :; }
340+
download_file() { :; }
341+
ui_info() { printf 'info:%s\\n' "$*"; }
342+
ui_success() { printf 'success:%s\\n' "$*"; }
343+
ui_error() { printf 'error:%s\\n' "$*"; }
344+
run_quiet_step() {
345+
printf 'step:%s|%s\\n' "$1" "\${*:2}"
346+
if [[ "$1" == "Installing Node.js" ]]; then
347+
return 65
348+
fi
349+
return 0
350+
}
351+
node() {
352+
if [[ "\${1:-}" == "-v" ]]; then
353+
printf 'v24.0.0\\n'
354+
fi
355+
}
356+
activate_supported_node_on_path() { :; }
357+
if install_node; then
358+
echo "install_node returned success"
359+
fi
360+
`);
361+
362+
expect(result.status).toBe(1);
363+
expect(result.stdout).toContain("step:Configuring NodeSource repository|bash");
364+
expect(result.stdout).toContain("step:Installing Node.js|apt_get_install nodejs");
365+
expect(result.stdout).not.toContain("success:Node.js v24.0.0 installed");
366+
expect(result.stdout).not.toContain("install_node returned success");
367+
});
368+
289369
it("installs Git with apk on Alpine", () => {
290370
const tmp = mkdtempSync(join(tmpdir(), "openclaw-install-git-apk-"));
291371
const bin = join(tmp, "bin");
@@ -769,7 +849,7 @@ describe("install.sh", () => {
769849
/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,
770850
);
771851
expect(script).toContain(
772-
'run_quiet_step "Configuring NodeSource repository" sudo -E bash "$tmp"',
852+
'run_required_step "Configuring NodeSource repository" sudo -E bash "$tmp"',
773853
);
774854
});
775855

0 commit comments

Comments
 (0)