fix: require validated registry lockfile warmups before timing

.github/workflows/benchmark.yaml

@@ -203,6 +203,7 @@ jobs:
             --fixtures="${{ matrix.fixture }}" \
             --variation="${{ matrix.variation }}"
       - name: Upload Benchmark Results
+        if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v7
         with:
           name: results-${{ matrix.fixture }}-${{ matrix.variation }}

.github/workflows/registry-lockfile-tests.yml

@@ -0,0 +1,26 @@
+name: Registry lockfile tests
+on:
+  push:
+    paths:
+      - 'scripts/registry/**'
+      - 'scripts/variations/registry-lockfile.sh'
+      - '.github/workflows/registry-lockfile-tests.yml'
+  pull_request:
+    paths:
+      - 'scripts/registry/**'
+      - 'scripts/variations/registry-lockfile.sh'
+      - '.github/workflows/registry-lockfile-tests.yml'
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v6
+      - uses: actions/setup-node@v6
+        with:
+          node-version: '24'
+          package-manager-cache: false
+      - name: Install hyperfine 1.19.0 (supports --conclude)
+        run: |
+          curl -fsSL https://github.com/sharkdp/hyperfine/releases/download/v1.19.0/hyperfine_1.19.0_amd64.deb -o /tmp/hyperfine.deb
+          sudo dpkg -i /tmp/hyperfine.deb
+      - run: node --test scripts/registry/lockfile.test.js

README.md

@@ -96,6 +96,33 @@ Auth notes:
 
 - `aws` requires `CODEARTIFACT_AUTH_TOKEN`.
 
+### Registry lockfile warmups
+
+`registry-lockfile` resolves and validates a fresh `package-lock.json` for each
+registry before starting timed runs. Warmups run in a checked prepare hook;
+a failed install, timeout, missing/invalid lockfile, or changed lockfile stops
+the job even though timed install failures are collected with `--ignore-failure`.
+The results artifact contains separate `<registry>-warmup-<n>.log` files,
+preparation logs, and the validated lockfile. GitHub's job summary reports warmup
+success or failure. `BENCH_WARMUP=0` still performs one required resolution.
+
+Warmups default to **600 seconds** per install (`BENCH_WARMUP_TIMEOUT`), while
+timed installs retain the **300-second** `BENCH_TIMEOUT` default. Third-party
+registries observed taking 300–380 seconds to resolve a cold graph can therefore
+finish preparation without raising the budget for timed tarball serving. A
+warmup exceeding 600 seconds fails visibly; it never becomes timed run 0.
+Every timed run clears caches and `node_modules`, checks the configured registry,
+and requires an unchanged validated lockfile. Successful lockfile installs fetch
+tarballs without resolving packuments again.
+
+```bash
+BENCH_WARMUP_TIMEOUT=600 BENCH_TIMEOUT=300 \
+  ./bench run --variation=registry-lockfile --fixtures=next --registries=npm
+```
+
+Run the lockfile regression tests with Node.js, npm, GNU `timeout`, and hyperfine
+installed: `node --test scripts/registry/lockfile.test.js`.
+
 ## Testing Script Execution
 
 This suite also tests the performance of basic script execution (ex. `npm run foo`). Notably, for any given build, test or deployment task the spawning of the process is a fraction of the overall execution time. That said, this is a commonly tracked workflow by various developer tools as it involves the common set of tasks: startup, filesystem read (`package.json`) & finally, spawning the process/command.

scripts/registry/lockfile.test.js

@@ -0,0 +1,196 @@
+const assert = require("node:assert/strict");
+const { spawn, spawnSync } = require("node:child_process");
+const fs = require("node:fs");
+const http = require("node:http");
+const os = require("node:os");
+const path = require("node:path");
+const test = require("node:test");
+const scriptsDir = path.resolve(__dirname, "..");
+
+const quote = value => `'${String(value).replaceAll("'", "'\\''")}'`;
+const run = (command, args, options) => new Promise((resolve, reject) => {
+  const child = spawn(command, args, options);
+  let output = "";
+  child.stdout.on("data", data => { output += data; });
+  child.stderr.on("data", data => { output += data; });
+  child.on("error", reject);
+  child.on("close", status => resolve({ status, output }));
+});
+
+function fixture(t, { realNpm = false } = {}) {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), "registry-lockfile-test-"));
+  t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
+  const scripts = path.join(dir, "scripts");
+  const output = path.join(dir, "results");
+  fs.mkdirSync(scripts);
+  fs.mkdirSync(output);
+  for (const file of ["registry-package-count.sh", "collect-package-count.js"]) {
+    fs.copyFileSync(path.join(scriptsDir, file), path.join(scripts, file));
+  }
+  // Isolate destructive benchmark cleanup to this test's fixture/cache.
+  fs.writeFileSync(path.join(scripts, "clean-helpers.sh"), `set -eu
+for action in "$@"; do
+  case "$action" in
+    clean_all) rm -rf node_modules package-lock.json .npmrc .npm-cache ;;
+    clean_node_modules) rm -rf node_modules ;;
+    clean_all_cache) rm -rf .npm-cache ;;
+    clean_npmrc) rm -f .npmrc ;;
+  esac
+done
+`);
+  fs.cpSync(__dirname, path.join(scripts, "registry"), { recursive: true });
+  fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ name: "fixture", version: "1.0.0" }));
+  const validLock = path.join(dir, "valid-lock.json");
+  fs.writeFileSync(validLock, JSON.stringify({ lockfileVersion: 3, packages: { "": { name: "fixture", version: "1.0.0" } } }));
+  const env = {
+    ...process.env,
+    GITHUB_STEP_SUMMARY: path.join(dir, "summary.md"),
+    npm_config_cache: path.join(dir, ".npm-cache"),
+    npm_config_userconfig: path.join(dir, "user.npmrc"),
+  };
+  if (!realNpm) {
+    const bin = path.join(dir, "bin");
+    fs.mkdirSync(bin);
+    fs.writeFileSync(path.join(bin, "npm"), `#!/bin/sh
+if [ "$1" = install ]; then
+  if [ -f package-lock.json ]; then printf 'existing ' >> installs; else printf 'fresh ' >> installs; fi
+  cat .npmrc >> installs
+  cp "${validLock}" package-lock.json
+  mkdir -p node_modules/example
+  printf '{"name":"example"}' > node_modules/example/package.json
+elif [ "$2" = set ]; then
+  if [ "$3" = registry ]; then printf '%s\\n' "$4" > .npmrc; fi
+else
+  cat .npmrc
+fi
+`, { mode: 0o755 });
+    env.PATH = `${bin}:${env.PATH}`;
+  }
+  return {
+    dir, scripts, output, env, validLock,
+    async benchmark({ install = `cp ${quote(validLock)} package-lock.json`, timeout = "2", warmups = "1", conclude, registry = "http://127.0.0.1/", command = "echo timed >> timed-runs" } = {}) {
+      const setup = `npm config set registry ${quote(registry)} --location=project`;
+      const prepare = ["bash", path.join(scripts, "registry/prepare-lockfile.sh"), scripts, output, "npm", registry, setup, install, warmups, timeout].map(quote).join(" ");
+      const args = ["--ignore-failure", "--warmup=0", "--runs=2", `--export-json=${output}/benchmarks.json`, `--prepare=${prepare}`];
+      if (conclude) args.push(`--conclude=${conclude}`);
+      args.push(command);
+      return run("hyperfine", args, { cwd: dir, env });
+    },
+  };
+}
+
+for (const scenario of [
+  { name: "timeout", install: "sleep 1", timeout: "0.05", pattern: /timeout 0.05s.*exit 124/ },
+  { name: "nonzero exit", install: "exit 23", pattern: /exit 23/ },
+  { name: "missing lockfile", install: "true", pattern: /lockfile validation/ },
+  { name: "malformed lockfile", install: "echo '{' > package-lock.json", pattern: /lockfile validation/ },
+  { name: "invalid lockfile", install: "echo '{}' > package-lock.json", pattern: /lockfile validation/ },
+]) {
+  test(`failed ${scenario.name} warmup stops timing and reports in the job summary`, async t => {
+    const f = fixture(t);
+    // A stale lockfile must not make a failed warmup look successful.
+    fs.copyFileSync(f.validLock, path.join(f.dir, "package-lock.json"));
+    const result = await f.benchmark(scenario);
+    assert.notEqual(result.status, 0, result.output);
+    assert.equal(fs.existsSync(path.join(f.dir, "timed-runs")), false);
+    assert.match(fs.readFileSync(f.env.GITHUB_STEP_SUMMARY, "utf8"), scenario.pattern);
+    assert.equal(fs.existsSync(path.join(f.output, "npm-package-lock.json")), false);
+  });
+}
+
+test("successful slow warmup has its own timeout and both timed runs start with its lockfile", async t => {
+  const f = fixture(t);
+  const result = await f.benchmark({
+    install: `sleep 0.2; cp ${quote(f.validLock)} package-lock.json`,
+    timeout: "1",
+    command: "timeout 0.05 sh -c 'test -s package-lock.json && echo timed >> timed-runs'",
+  });
+  assert.equal(result.status, 0, result.output);
+  assert.equal(fs.readFileSync(path.join(f.dir, "timed-runs"), "utf8"), "timed\ntimed\n");
+  assert.match(fs.readFileSync(f.env.GITHUB_STEP_SUMMARY, "utf8"), /warmup\(s\) succeeded/);
+});
+
+test("BENCH_WARMUP=0 still creates a required fresh lockfile", async t => {
+  const f = fixture(t);
+  const result = await f.benchmark({ warmups: "0" });
+  assert.equal(result.status, 0, result.output);
+  assert.equal(fs.existsSync(path.join(f.output, "npm-warmup-0.log")), true);
+});
+
+test("a failed later warmup cannot reuse the first warmup's valid lockfile", async t => {
+  const f = fixture(t);
+  const result = await f.benchmark({
+    warmups: "2",
+    install: `if [ -f warmed ]; then exit 24; fi; touch warmed; cp ${quote(f.validLock)} package-lock.json`,
+  });
+  assert.notEqual(result.status, 0, result.output);
+  assert.equal(fs.existsSync(path.join(f.dir, "timed-runs")), false);
+  assert.match(fs.readFileSync(f.env.GITHUB_STEP_SUMMARY, "utf8"), /warmup 1.*exit 24/);
+});
+
+test("a replaced lockfile stops the next timed run", async t => {
+  const f = fixture(t);
+  const result = await f.benchmark({ conclude: "echo '{}' > package-lock.json" });
+  assert.notEqual(result.status, 0, result.output);
+  assert.equal(fs.readFileSync(path.join(f.dir, "timed-runs"), "utf8"), "timed\n");
+  assert.match(fs.readFileSync(f.env.GITHUB_STEP_SUMMARY, "utf8"), /timed-run lockfile validation/);
+});
+
+test("real npm timed installs fetch tarballs and no packuments after warmup", async t => {
+  const f = fixture(t, { realNpm: true });
+  const tarDir = path.join(f.dir, "tar");
+  fs.mkdirSync(path.join(tarDir, "package"), { recursive: true });
+  fs.writeFileSync(path.join(tarDir, "package/package.json"), JSON.stringify({ name: "test-package", version: "1.0.0" }));
+  const tarball = path.join(f.dir, "test-package.tgz");
+  const packed = spawnSync("tar", ["-czf", tarball, "-C", tarDir, "package"], { encoding: "utf8" });
+  assert.equal(packed.status, 0, packed.stderr);
+  const bytes = fs.readFileSync(tarball);
+  const requests = [];
+  const server = http.createServer((req, res) => {
+    requests.push({ url: req.url, timed: fs.existsSync(path.join(f.dir, "timing")) });
+    if (req.url === "/test-package") {
+      res.setHeader("content-type", "application/json");
+      res.end(JSON.stringify({ name: "test-package", "dist-tags": { latest: "1.0.0" }, versions: { "1.0.0": { name: "test-package", version: "1.0.0", dist: { tarball: `${registry}test-package/-/test-package-1.0.0.tgz` } } } }));
+    } else if (req.url === "/test-package/-/test-package-1.0.0.tgz") {
+      res.setHeader("content-type", "application/octet-stream");
+      res.end(bytes);
+    } else {
+      res.statusCode = 404;
+      res.end();
+    }
+  });
+  await new Promise(resolve => server.listen(0, "127.0.0.1", resolve));
+  t.after(() => new Promise(resolve => server.close(resolve)));
+  const registry = `http://127.0.0.1:${server.address().port}/`;
+  fs.writeFileSync(path.join(f.dir, "package.json"), JSON.stringify({ name: "fixture", version: "1.0.0", dependencies: { "test-package": "^1.0.0" } }));
+  const install = "npm install --prefer-online --no-audit --no-fund --no-update-notifier --ignore-scripts --loglevel=http";
+  const result = await f.benchmark({ registry, install, timeout: "20", command: `touch timing; timeout 20 ${install}; status=$?; rm timing; exit $status` });
+  assert.equal(result.status, 0, result.output);
+  assert.ok(requests.some(req => !req.timed && req.url === "/test-package"));
+  const timed = requests.filter(req => req.timed);
+  assert.equal(timed.length, 2, JSON.stringify(requests));
+  assert.ok(timed.every(req => req.url.endsWith(".tgz")), JSON.stringify(timed));
+  const results = JSON.parse(fs.readFileSync(path.join(f.output, "benchmarks.json")));
+  assert.deepEqual(results.results[0].exit_codes, [0, 0]);
+});
+
+
+test("the full variation creates a separate fresh warmup for each registry", async t => {
+  const f = fixture(t);
+  const result = await run("bash", [path.join(scriptsDir, "variations/registry-lockfile.sh"), f.scripts, f.output, "fixture", "registry-lockfile"], {
+    cwd: f.dir,
+    env: { ...f.env, BENCH_INCLUDE_REGISTRY: "npm,vlt", BENCH_WARMUP: "1", BENCH_RUNS: "2", VLT_TOKEN: "test", CLOUDSMITH_REGISTRY: "", GH_REGISTRY: "", JFROG_REGISTRY: "" },
+  });
+  assert.equal(result.status, 0, result.output);
+  const installs = fs.readFileSync(path.join(f.dir, "installs"), "utf8").trim().split("\n");
+  assert.deepEqual(installs, [
+    "fresh https://registry.npmjs.org/",
+    "existing https://registry.npmjs.org/",
+    "existing https://registry.npmjs.org/",
+    "fresh https://registry.vlt.io/vlt-benchmarks/npm/",
+    "existing https://registry.vlt.io/vlt-benchmarks/npm/",
+    "existing https://registry.vlt.io/vlt-benchmarks/npm/",
+  ]);
+  const results = JSON.parse(fs.readFileSync(path.join(f.output, "fixture/registry-lockfile/benchmarks.json")));
+  assert.deepEqual(results.results.map(row => [row.command, row.exit_codes]), [["npm", [0, 0]], ["vlt", [0, 0]]]);
+});

scripts/registry/prepare-lockfile.sh

@@ -0,0 +1,61 @@
+# A failing prepare hook stops hyperfine even when --ignore-failure is enabled.
+set -Eeuo pipefail
+
+scripts=$1
+output=$2
+registry=$3
+registry_url=$4
+setup=$5
+install=$6
+warmups=$7
+warmup_timeout=$8
+snapshot="$output/$registry-package-lock.json"
+stage="lockfile preparation"
+
+report_failure() {
+  local status=$?
+  local message="$registry registry-lockfile: $stage failed (exit $status). Timed runs stopped; see $registry-prepare.log and $registry-warmup-*.log."
+  printf '%s\n' "$message" | tee "$output/$registry-failure.log" >&2
+  if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
+    printf '\n### Registry lockfile failure\n\n%s\n' "$message" >> "$GITHUB_STEP_SUMMARY"
+  fi
+  exit "$status"
+}
+trap report_failure ERR
+
+prepare() {
+  sleep 1
+  # Corepack cache cleans can re-pin devEngines.packageManager, so remove it last.
+  bash "$scripts/clean-helpers.sh" clean_all_cache clean_package_manager_field clean_node_modules clean_package_manager_files clean_npmrc
+  bash -c "$setup"
+  test "$(npm config get registry)" = "$registry_url"
+}
+
+if [ ! -f "$snapshot" ]; then
+  # Never accept a fixture lockfile or a previous registry's lockfile as warmup.
+  bash "$scripts/clean-helpers.sh" clean_all clean_npmrc
+  if [[ ! "$warmups" =~ ^[0-9]+$ ]]; then
+    stage="invalid BENCH_WARMUP=$warmups"
+    false
+  fi
+  # Even --warmup=0 must resolve a graph before any lockfile timing begins.
+  if [ "$warmups" -eq 0 ]; then warmups=1; fi
+  for ((iteration=0; iteration<warmups; iteration++)); do
+    stage="warmup $iteration preparation"
+    prepare
+    stage="warmup $iteration (timeout ${warmup_timeout}s)"
+    timeout "$warmup_timeout" bash -c "$install" > "$output/$registry-warmup-$iteration.log" 2>&1
+    stage="warmup $iteration lockfile validation"
+    node "$scripts/registry/validate-lockfile.js"
+  done
+  cp package-lock.json "$snapshot"
+  if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
+    printf '\n- %s registry-lockfile: %s warmup(s) succeeded (timeout %ss); lockfile validated.\n' "$registry" "$warmups" "$warmup_timeout" >> "$GITHUB_STEP_SUMMARY"
+  fi
+fi
+
+stage="timed-run lockfile validation"
+prepare
+# The immutable, registry-specific snapshot also detects a lockfile replaced or
+# modified between runs. No validation or preparation is included in the timing.
+cmp package-lock.json "$snapshot"

scripts/registry/validate-lockfile.js

@@ -0,0 +1,24 @@
+const assert = require("node:assert/strict");
+const fs = require("node:fs");
+
+try {
+  const lock = JSON.parse(fs.readFileSync("package-lock.json", "utf8"));
+  const manifest = JSON.parse(fs.readFileSync("package.json", "utf8"));
+  assert.ok([2, 3].includes(lock.lockfileVersion), "expected npm lockfile v2/v3");
+  assert.ok(lock.packages && !Array.isArray(lock.packages), "missing packages");
+  const root = lock.packages[""];
+  assert.ok(root && typeof root === "object", "missing root package");
+  for (const field of ["dependencies", "devDependencies", "optionalDependencies"]) {
+    assert.deepEqual(root[field] || {}, manifest[field] || {}, `${field} differ from package.json`);
+  }
+  for (const [location, pkg] of Object.entries(lock.packages)) {
+    assert.ok(pkg && typeof pkg === "object", `invalid package: ${location}`);
+    if (location.includes("node_modules/") && !pkg.link) {
+      assert.equal(typeof pkg.version, "string", `missing version: ${location}`);
+      assert.equal(typeof pkg.resolved, "string", `missing resolved URL: ${location}`);
+    }
+  }
+} catch (error) {
+  console.error(`Invalid registry warmup package-lock.json: ${error.message}`);
+  process.exitCode = 1;
+}