From 5b42d8a0485ceba17d7d84504496207fc7e38dbc Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 21 May 2026 21:07:02 -0400 Subject: [PATCH] fix(installer): preserve npm lockfiles during install Signed-off-by: Julie Yaunches --- .github/actions/basic-checks/action.yaml | 12 +++++- scripts/install.sh | 37 ++++++++++------ test/install-preflight.test.ts | 33 +++++++++++--- test/lockfile-ci-guard.test.ts | 55 ++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 21 deletions(-) create mode 100644 test/lockfile-ci-guard.test.ts diff --git a/.github/actions/basic-checks/action.yaml b/.github/actions/basic-checks/action.yaml index ce2ab3b32a..65e0a692cf 100644 --- a/.github/actions/basic-checks/action.yaml +++ b/.github/actions/basic-checks/action.yaml @@ -24,11 +24,19 @@ runs: [ "$EXPECTED" = "$ACTUAL" ] || { echo "::error::hadolint checksum mismatch"; exit 1; } chmod +x /usr/local/bin/hadolint + - name: Validate npm lockfiles + shell: bash + run: | + npm ci --ignore-scripts --dry-run + cd nemoclaw + npm ci --ignore-scripts --dry-run + - name: Install dependencies shell: bash run: | - npm install --ignore-scripts - cd nemoclaw && npm install + npm ci --ignore-scripts + cd nemoclaw + npm ci --ignore-scripts - name: Build TypeScript plugin shell: bash diff --git a/scripts/install.sh b/scripts/install.sh index 9d9516b065..e21adc020f 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1254,8 +1254,8 @@ fix_npm_permissions() { # Work around openclaw tarball missing directory entries (GH-503). # npm's tar extractor hard-fails because the tarball is missing directory # entries for extensions/, skills/, and dist/plugin-sdk/config/. System tar -# handles this fine. We pre-extract openclaw into node_modules BEFORE npm -# install so npm sees the dependency is already satisfied and skips it. +# handles this fine. npm ci removes node_modules before installing, so restore +# the package after ci and before build steps that need the unpacked contents. pre_extract_openclaw() { local install_dir="$1" local openclaw_version @@ -1294,6 +1294,17 @@ pre_extract_openclaw() { rm -rf "$tmpdir" } +restore_pre_extracted_openclaw() { + local install_dir="$1" + + if [[ -n "${NEMOCLAW_AGENT:-}" && "${NEMOCLAW_AGENT}" != "openclaw" ]]; then + return 0 + fi + + spin "Restoring OpenClaw package" bash -c "$(declare -f info warn resolve_openclaw_version pre_extract_openclaw); pre_extract_openclaw \"\$1\"" _ "$install_dir" \ + || warn "OpenClaw package restore failed - subsequent build steps may fail" +} + resolve_openclaw_version() { local install_dir="$1" local package_json dockerfile_base resolved_version @@ -1357,13 +1368,12 @@ install_nemoclaw() { if is_source_checkout "$repo_root"; then info "${_CLI_DISPLAY} package.json found in the selected source checkout — installing from source…" NEMOCLAW_SOURCE_ROOT="$repo_root" - if [[ -z "${NEMOCLAW_AGENT:-}" || "${NEMOCLAW_AGENT}" == "openclaw" ]]; then - spin "Preparing OpenClaw package" bash -c "$(declare -f info warn resolve_openclaw_version pre_extract_openclaw); pre_extract_openclaw \"\$1\"" _ "$NEMOCLAW_SOURCE_ROOT" \ - || warn "Pre-extraction failed — npm install may fail if openclaw tarball is broken" - fi - spin "Installing ${_CLI_DISPLAY} dependencies" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\" && npm install --ignore-scripts" + spin "Installing ${_CLI_DISPLAY} dependencies" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\" && npm ci --ignore-scripts" + restore_pre_extracted_openclaw "$NEMOCLAW_SOURCE_ROOT" spin "Building ${_CLI_DISPLAY} CLI modules" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\" && npm run --if-present build:cli" - spin "Building ${_CLI_DISPLAY} plugin" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\"/nemoclaw && npm install --ignore-scripts && npm run build" + spin "Installing ${_CLI_DISPLAY} plugin dependencies" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\"/nemoclaw && npm ci --ignore-scripts" + restore_pre_extracted_openclaw "$NEMOCLAW_SOURCE_ROOT" + spin "Building ${_CLI_DISPLAY} plugin" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\"/nemoclaw && npm run build" spin "Linking ${_CLI_DISPLAY} CLI" bash -c "cd \"$NEMOCLAW_SOURCE_ROOT\" && npm link" else if [[ -f "$package_json" ]]; then @@ -1390,13 +1400,12 @@ install_nemoclaw() { # unavailable or tags are pruned later. git -C "$nemoclaw_src" describe --tags --match 'v*' 2>/dev/null \ | sed 's/^v//' >"$nemoclaw_src/.version" || true - if [[ -z "${NEMOCLAW_AGENT:-}" || "${NEMOCLAW_AGENT}" == "openclaw" ]]; then - spin "Preparing OpenClaw package" bash -c "$(declare -f info warn resolve_openclaw_version pre_extract_openclaw); pre_extract_openclaw \"\$1\"" _ "$nemoclaw_src" \ - || warn "Pre-extraction failed — npm install may fail if openclaw tarball is broken" - fi - spin "Installing ${_CLI_DISPLAY} dependencies" bash -c "cd \"$nemoclaw_src\" && npm install --ignore-scripts" + spin "Installing ${_CLI_DISPLAY} dependencies" bash -c "cd \"$nemoclaw_src\" && npm ci --ignore-scripts" + restore_pre_extracted_openclaw "$nemoclaw_src" spin "Building ${_CLI_DISPLAY} CLI modules" bash -c "cd \"$nemoclaw_src\" && npm run --if-present build:cli" - spin "Building ${_CLI_DISPLAY} plugin" bash -c "cd \"$nemoclaw_src\"/nemoclaw && npm install --ignore-scripts && npm run build" + spin "Installing ${_CLI_DISPLAY} plugin dependencies" bash -c "cd \"$nemoclaw_src\"/nemoclaw && npm ci --ignore-scripts" + restore_pre_extracted_openclaw "$nemoclaw_src" + spin "Building ${_CLI_DISPLAY} plugin" bash -c "cd \"$nemoclaw_src\"/nemoclaw && npm run build" spin "Linking ${_CLI_DISPLAY} CLI" bash -c "cd \"$nemoclaw_src\" && npm link" # Install/upgrade the OpenShell CLI on the GitHub-clone path (curl|bash). diff --git a/test/install-preflight.test.ts b/test/install-preflight.test.ts index 510b3d9176..a80111b43d 100644 --- a/test/install-preflight.test.ts +++ b/test/install-preflight.test.ts @@ -95,7 +95,7 @@ function writeNpmStub(fakeBin: string, installSnippet: string = "exit 0") { set -euo pipefail if [ "$1" = "--version" ]; then echo "10.9.2"; exit 0; fi if [ "$1" = "config" ] && [ "$2" = "get" ] && [ "$3" = "prefix" ]; then echo "$NPM_PREFIX"; exit 0; fi -if [ "$1" = "install" ] || [ "$1" = "link" ] || [ "$1" = "uninstall" ] || [ "$1" = "pack" ] || [ "$1" = "run" ]; then +if [ "$1" = "ci" ] || [ "$1" = "install" ] || [ "$1" = "link" ] || [ "$1" = "uninstall" ] || [ "$1" = "pack" ] || [ "$1" = "run" ]; then ${installSnippet} fi echo "unexpected npm invocation: $*" >&2; exit 98`, @@ -552,7 +552,7 @@ exit 98 expect(output).not.toMatch(/0\.1\.0/); }); - it("uses npm install + npm link for a source checkout (no -g)", { timeout: 20000 }, () => { + it("uses npm ci + npm link for a source checkout (no -g)", { timeout: 20000 }, () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-install-source-")); const fakeBin = path.join(tmp, "bin"); const prefix = path.join(tmp, "prefix"); @@ -594,8 +594,16 @@ if [ "$1" = "pack" ]; then tar -czf "$tmpdir/openclaw-2026.3.11.tgz" -C "$tmpdir" package exit 0 fi +if [ "$1" = "ci" ]; then rm -rf node_modules; exit 0; fi if [ "$1" = "install" ]; then exit 0; fi -if [ "$1" = "run" ] && { [ "$2" = "build" ] || [ "$2" = "build:cli" ] || [ "$2" = "--if-present" ]; }; then exit 0; fi +if [ "$1" = "run" ] && [ "$2" = "--if-present" ] && [ "$3" = "build:cli" ]; then + test -d node_modules/openclaw || { echo "missing root openclaw before build:cli" >&2; exit 70; } + exit 0 +fi +if [ "$1" = "run" ] && [ "$2" = "build" ]; then + test -d ../node_modules/openclaw || { echo "missing root openclaw before plugin build" >&2; exit 71; } + exit 0 +fi if [ "$1" = "link" ]; then cat > "$NPM_PREFIX/bin/nemoclaw" <<'EOS' #!/usr/bin/env bash @@ -619,6 +627,9 @@ fi`, path.join(tmp, "nemoclaw", "package.json"), JSON.stringify({ name: "nemoclaw-plugin", version: "0.1.0" }, null, 2), ); + fs.mkdirSync(path.join(tmp, "bin", "lib"), { recursive: true }); + fs.writeFileSync(path.join(tmp, "bin", "lib", "usage-notice.js"), "process.exit(0);\n"); + fs.writeFileSync(path.join(tmp, "Dockerfile.base"), "ARG OPENCLAW_VERSION=2026.3.11\n"); fs.mkdirSync(path.join(tmp, "nemoclaw-blueprint", "router", "llm-router"), { recursive: true, }); @@ -637,6 +648,7 @@ fi`, NEMOCLAW_NON_INTERACTIVE: "1", NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1", NPM_PREFIX: prefix, + NEMOCLAW_REPO_ROOT: tmp, NPM_LOG_PATH: npmLog, PYTHON_LOG_PATH: pythonLog, GIT_LOG_PATH: gitLog, @@ -645,8 +657,19 @@ fi`, expect(result.status).toBe(0); const log = fs.readFileSync(npmLog, "utf-8"); - // install (no -g) and link must both have been called - expect(log).toMatch(/^install(?!\s+-g)/m); + // Root and sandbox payload installs must use npm ci so host installs do not rewrite lockfiles. + const npmCalls = log.trim().split("\n"); + const ciIndexes = npmCalls.flatMap((line, index) => (line === "ci --ignore-scripts" ? [index] : [])); + const packIndexes = npmCalls.flatMap((line, index) => + line.startsWith("pack openclaw@2026.3.11 --pack-destination ") ? [index] : [], + ); + expect(ciIndexes).toHaveLength(2); + expect(packIndexes).toHaveLength(2); + expect(ciIndexes[0] ?? -1).toBeLessThan(packIndexes[0] ?? -1); + expect(ciIndexes[1] ?? -1).toBeLessThan(packIndexes[1] ?? -1); + expect(packIndexes[0] ?? -1).toBeLessThan(npmCalls.indexOf("run --if-present build:cli")); + expect(packIndexes[1] ?? -1).toBeLessThan(npmCalls.indexOf("run build")); + expect(log).not.toMatch(/^install(?!\s+-g)/m); expect(log).toMatch(/^link/m); // the GitHub URL must NOT appear — this is a local install expect(log).not.toMatch(new RegExp(GITHUB_INSTALL_URL.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"))); diff --git a/test/lockfile-ci-guard.test.ts b/test/lockfile-ci-guard.test.ts new file mode 100644 index 0000000000..eb43772f0b --- /dev/null +++ b/test/lockfile-ci-guard.test.ts @@ -0,0 +1,55 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import YAML from "yaml"; + +const REPO_ROOT = join(import.meta.dirname, ".."); + +type CompositeAction = { + runs?: { + steps?: Array<{ + name?: string; + run?: string; + }>; + }; +}; + +function loadBasicChecksAction(): CompositeAction { + return YAML.parse( + readFileSync(join(REPO_ROOT, ".github/actions/basic-checks/action.yaml"), "utf-8"), + ) as CompositeAction; +} + +describe("lockfile CI guards", () => { + it("validates root and sandbox payload lockfiles before install can rewrite them", () => { + const action = loadBasicChecksAction(); + const steps = action.runs?.steps ?? []; + const validateIndex = steps.findIndex((step) => step.name === "Validate npm lockfiles"); + const installIndex = steps.findIndex((step) => step.name === "Install dependencies"); + const validateStep = steps[validateIndex]; + + expect(validateIndex).toBeGreaterThanOrEqual(0); + expect(installIndex).toBeGreaterThanOrEqual(0); + expect(validateIndex).toBeLessThan(installIndex); + expect(validateStep?.run?.trimEnd()).toBe( + [ + "npm ci --ignore-scripts --dry-run", + "cd nemoclaw", + "npm ci --ignore-scripts --dry-run", + ].join("\n"), + ); + }); + + it("uses npm ci for root and sandbox payload installs in basic checks", () => { + const action = loadBasicChecksAction(); + const installStep = action.runs?.steps?.find((step) => step.name === "Install dependencies"); + + expect(installStep?.run?.trimEnd()).toBe( + ["npm ci --ignore-scripts", "cd nemoclaw", "npm ci --ignore-scripts"].join("\n"), + ); + expect(installStep?.run).not.toContain("npm install"); + }); +});