From 6e42909a22b094d4f2b5652c48e9ecc1e239f3a2 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 3 May 2026 18:35:45 -0700 Subject: [PATCH] fix(ci): tighten release bootstrap guard --- .github/workflows/pr.yml | 4 +- package.json | 2 +- scripts/check-release-package-bootstrap.mjs | 90 +++++++++++++++---- .../check-release-package-bootstrap.test.mjs | 52 +++++++++++ scripts/release-package-map.test.mjs | 13 +-- 5 files changed, 133 insertions(+), 28 deletions(-) create mode 100644 scripts/check-release-package-bootstrap.test.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 5ee1ece454e..954725c9cd1 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -50,8 +50,8 @@ jobs: - name: Verify release package bootstrap for changed manifests run: | - changed="$(git diff --name-only "${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}")" - node ./scripts/check-release-package-bootstrap.mjs $changed + mapfile -t changed_paths < <(git diff --name-only "${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}") + node ./scripts/check-release-package-bootstrap.mjs "${changed_paths[@]}" - name: Validate dependency resolution when manifests change run: | diff --git a/package.json b/package.json index 9f7186bab2b..5a7d38fd3ef 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "smoke:openclaw-docker-ui": "./scripts/smoke/openclaw-docker-ui.sh", "smoke:openclaw-sse-standalone": "./scripts/smoke/openclaw-sse-standalone.sh", "smoke:terminal-bench-loop-skill": "node scripts/smoke/terminal-bench-loop-skill-smoke.mjs", - "test:release-registry": "node --test scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs", + "test:release-registry": "node --test scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs", "test:e2e": "npx playwright test --config tests/e2e/playwright.config.ts", "test:e2e:headed": "npx playwright test --config tests/e2e/playwright.config.ts --headed", "test:e2e:multiuser-authenticated": "npx playwright test --config tests/e2e/playwright-multiuser-authenticated.config.ts", diff --git a/scripts/check-release-package-bootstrap.mjs b/scripts/check-release-package-bootstrap.mjs index d5ebd603875..f888385a77f 100644 --- a/scripts/check-release-package-bootstrap.mjs +++ b/scripts/check-release-package-bootstrap.mjs @@ -1,6 +1,6 @@ #!/usr/bin/env node -import { execFileSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import { buildReleasePackagePlan } from "./release-package-map.mjs"; @@ -8,41 +8,81 @@ function normalizePath(filePath) { return filePath.replace(/\\/g, "/"); } -function npmPackageExists(packageName) { - try { - execFileSync("npm", ["view", packageName, "name", "--json"], { - stdio: ["ignore", "pipe", "pipe"], - encoding: "utf8", - }); - return true; - } catch { - return false; - } +function classifyNpmViewFailure(output) { + return /\bE404\b|404 Not Found|could not be found/i.test(output) ? "missing" : "registry_error"; } -function main(changedPaths) { +function inspectNpmPackage(packageName) { + const result = spawnSync("npm", ["view", packageName, "name", "--json"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + + if (result.error) { + throw result.error; + } + + if (result.status === 0) { + return { status: "exists" }; + } + + const output = `${result.stdout ?? ""}\n${result.stderr ?? ""}`.trim(); + const failureType = classifyNpmViewFailure(output); + + if (failureType === "missing") { + return { status: "missing" }; + } + + return { + status: "registry_error", + detail: output || `npm view exited with status ${result.status ?? "unknown"}`, + }; +} + +function collectReleasePackagesForChangedPaths(changedPaths, releasePackages = buildReleasePackagePlan()) { const normalizedChangedPaths = changedPaths.map(normalizePath); - const releasePackages = buildReleasePackagePlan(); + const manifestFileChanged = normalizedChangedPaths.includes("scripts/release-package-manifest.json"); const changedReleasePackages = []; const seen = new Set(); for (const pkg of releasePackages) { if (!pkg.publishFromCi) continue; const packageJsonPath = `${pkg.dir}/package.json`; + const isRelevant = manifestFileChanged || normalizedChangedPaths.includes(packageJsonPath); - if (!normalizedChangedPaths.includes(packageJsonPath)) continue; + if (!isRelevant) continue; if (seen.has(pkg.name)) continue; changedReleasePackages.push(pkg); seen.add(pkg.name); } + return changedReleasePackages; +} + +function main(changedPaths) { + const changedReleasePackages = collectReleasePackagesForChangedPaths(changedPaths); + if (changedReleasePackages.length === 0) { process.stdout.write("No release-enabled package manifests changed in this PR.\n"); return; } - const missingPackages = changedReleasePackages.filter((pkg) => !npmPackageExists(pkg.name)); + const missingPackages = []; + const registryFailures = []; + + for (const pkg of changedReleasePackages) { + const npmStatus = inspectNpmPackage(pkg.name); + + if (npmStatus.status === "missing") { + missingPackages.push(pkg); + continue; + } + + if (npmStatus.status === "registry_error") { + registryFailures.push({ pkg, detail: npmStatus.detail }); + } + } if (missingPackages.length > 0) { const details = missingPackages @@ -55,9 +95,27 @@ function main(changedPaths) { throw new Error(`release package bootstrap check failed:\n- ${details}`); } + if (registryFailures.length > 0) { + const details = registryFailures + .map( + ({ pkg, detail }) => + `${pkg.name} (${pkg.dir}) could not be checked against npm due to a registry error:\n${detail}`, + ) + .join("\n- "); + + throw new Error(`release package bootstrap check could not verify npm state:\n- ${details}`); + } + process.stdout.write( `Release bootstrap OK for changed manifests: ${changedReleasePackages.map((pkg) => pkg.name).join(", ")}\n`, ); } -main(process.argv.slice(2)); +if (process.argv[1] && normalizePath(process.argv[1]).endsWith("scripts/check-release-package-bootstrap.mjs")) { + main(process.argv.slice(2)); +} + +export { + classifyNpmViewFailure, + collectReleasePackagesForChangedPaths, +}; diff --git a/scripts/check-release-package-bootstrap.test.mjs b/scripts/check-release-package-bootstrap.test.mjs new file mode 100644 index 00000000000..90c04cb39a2 --- /dev/null +++ b/scripts/check-release-package-bootstrap.test.mjs @@ -0,0 +1,52 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + classifyNpmViewFailure, + collectReleasePackagesForChangedPaths, +} from "./check-release-package-bootstrap.mjs"; + +test("manifest changes force validation of all release-enabled packages", () => { + const releasePackages = [ + { dir: "packages/a", name: "@paperclipai/a", publishFromCi: true }, + { dir: "packages/b", name: "@paperclipai/b", publishFromCi: true }, + { dir: "packages/c", name: "@paperclipai/c", publishFromCi: false }, + ]; + + const changedPackages = collectReleasePackagesForChangedPaths( + ["scripts/release-package-manifest.json"], + releasePackages, + ); + + assert.deepEqual( + changedPackages.map((pkg) => pkg.name), + ["@paperclipai/a", "@paperclipai/b"], + ); +}); + +test("package-specific changes only validate affected release-enabled packages", () => { + const releasePackages = [ + { dir: "packages/a", name: "@paperclipai/a", publishFromCi: true }, + { dir: "packages/b", name: "@paperclipai/b", publishFromCi: true }, + ]; + + const changedPackages = collectReleasePackagesForChangedPaths( + ["packages/b/package.json", "README.md"], + releasePackages, + ); + + assert.deepEqual( + changedPackages.map((pkg) => pkg.name), + ["@paperclipai/b"], + ); +}); + +test("npm E404 failures are treated as missing packages", () => { + assert.equal(classifyNpmViewFailure("npm error code E404"), "missing"); + assert.equal(classifyNpmViewFailure("404 Not Found"), "missing"); +}); + +test("non-404 npm failures are treated as registry errors", () => { + assert.equal(classifyNpmViewFailure("npm error code EAI_AGAIN"), "registry_error"); + assert.equal(classifyNpmViewFailure("npm error code E429"), "registry_error"); +}); diff --git a/scripts/release-package-map.test.mjs b/scripts/release-package-map.test.mjs index 19bc86d79eb..704dd3ddbec 100644 --- a/scripts/release-package-map.test.mjs +++ b/scripts/release-package-map.test.mjs @@ -13,15 +13,10 @@ test("release package manifest covers all public packages with explicit CI enrol assert.ok(packages.every((pkg) => typeof pkg.publishFromCi === "boolean")); }); -test("ACPX adapter remains enrolled for CI publishing", () => { - const packages = buildReleasePackagePlan(); - const acpxPackage = packages.find((pkg) => pkg.name === "@paperclipai/adapter-acpx-local"); - assert.ok(acpxPackage); - assert.equal(acpxPackage.publishFromCi, true); - - const enabledNames = new Set(getReleasePackages().map((pkg) => pkg.name)); - assert.equal(enabledNames.has("@paperclipai/adapter-acpx-local"), true); - assert.equal(enabledNames.has("@paperclipai/adapter-utils"), true); +test("release package list only contains CI-enrolled packages", () => { + const enabledPackages = getReleasePackages(); + assert.ok(enabledPackages.length > 0); + assert.ok(enabledPackages.every((pkg) => pkg.publishFromCi === true)); }); test("release package configuration validates successfully", () => {