diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 954725c9cd1..8ac8f710ad5 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -51,7 +51,8 @@ jobs: - name: Verify release package bootstrap for changed manifests run: | 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[@]}" + PAPERCLIP_RELEASE_BOOTSTRAP_BASE_SHA="${{ github.event.pull_request.base.sha }}" \ + node ./scripts/check-release-package-bootstrap.mjs "${changed_paths[@]}" - name: Validate dependency resolution when manifests change run: | diff --git a/scripts/check-release-package-bootstrap.mjs b/scripts/check-release-package-bootstrap.mjs index f888385a77f..a9d792df807 100644 --- a/scripts/check-release-package-bootstrap.mjs +++ b/scripts/check-release-package-bootstrap.mjs @@ -39,7 +39,84 @@ function inspectNpmPackage(packageName) { }; } -function collectReleasePackagesForChangedPaths(changedPaths, releasePackages = buildReleasePackagePlan()) { +function readGitFileAtRevision(revision, filePath) { + const result = spawnSync("git", ["show", `${revision}:${normalizePath(filePath)}`], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + + if (result.error) { + throw result.error; + } + + if (result.status === 0) { + return result.stdout; + } + + const output = `${result.stdout ?? ""}\n${result.stderr ?? ""}`.trim(); + + if ( + /exists on disk, but not in/i.test(output) || + /does not exist in/i.test(output) + ) { + return null; + } + + throw new Error(`failed to read ${filePath} at ${revision}:\n${output || "git show failed"}`); +} + +function getBaseReleaseState( + revision, + releasePackages = buildReleasePackagePlan(), + readFileAtRevision = readGitFileAtRevision, +) { + if (!revision) return null; + + const manifestText = readFileAtRevision(revision, "scripts/release-package-manifest.json"); + + if (manifestText) { + const manifestEntries = JSON.parse(manifestText); + + if (!Array.isArray(manifestEntries)) { + throw new Error(`expected scripts/release-package-manifest.json at ${revision} to contain an array`); + } + + return { + source: "manifest", + byDir: new Map( + manifestEntries + .filter((entry) => entry?.publishFromCi === true && typeof entry.dir === "string" && typeof entry.name === "string") + .map((entry) => [entry.dir, { name: entry.name, publishFromCi: true }]), + ), + }; + } + + const byDir = new Map(); + + for (const pkg of releasePackages) { + const packageJsonText = readFileAtRevision(revision, `${pkg.dir}/package.json`); + if (!packageJsonText) continue; + + const basePackage = JSON.parse(packageJsonText); + if (basePackage.private) continue; + + byDir.set(pkg.dir, { + name: basePackage.name, + publishFromCi: true, + }); + } + + return { + source: "public-packages", + byDir, + }; +} + +function collectReleasePackagesForChangedPaths( + changedPaths, + releasePackages = buildReleasePackagePlan(), + baseReleaseState = null, +) { const normalizedChangedPaths = changedPaths.map(normalizePath); const manifestFileChanged = normalizedChangedPaths.includes("scripts/release-package-manifest.json"); const changedReleasePackages = []; @@ -48,7 +125,12 @@ function collectReleasePackagesForChangedPaths(changedPaths, releasePackages = b for (const pkg of releasePackages) { if (!pkg.publishFromCi) continue; const packageJsonPath = `${pkg.dir}/package.json`; - const isRelevant = manifestFileChanged || normalizedChangedPaths.includes(packageJsonPath); + const packageJsonChanged = normalizedChangedPaths.includes(packageJsonPath); + const basePackage = baseReleaseState?.byDir.get(pkg.dir); + const newlyReleaseEnabled = + manifestFileChanged && + (!baseReleaseState || !basePackage || basePackage.publishFromCi !== true || basePackage.name !== pkg.name); + const isRelevant = packageJsonChanged || newlyReleaseEnabled; if (!isRelevant) continue; if (seen.has(pkg.name)) continue; @@ -61,7 +143,9 @@ function collectReleasePackagesForChangedPaths(changedPaths, releasePackages = b } function main(changedPaths) { - const changedReleasePackages = collectReleasePackagesForChangedPaths(changedPaths); + const releasePackages = buildReleasePackagePlan(); + const baseReleaseState = getBaseReleaseState(process.env.PAPERCLIP_RELEASE_BOOTSTRAP_BASE_SHA, releasePackages); + const changedReleasePackages = collectReleasePackagesForChangedPaths(changedPaths, releasePackages, baseReleaseState); if (changedReleasePackages.length === 0) { process.stdout.write("No release-enabled package manifests changed in this PR.\n"); @@ -118,4 +202,5 @@ if (process.argv[1] && normalizePath(process.argv[1]).endsWith("scripts/check-re export { classifyNpmViewFailure, collectReleasePackagesForChangedPaths, + getBaseReleaseState, }; diff --git a/scripts/check-release-package-bootstrap.test.mjs b/scripts/check-release-package-bootstrap.test.mjs index 90c04cb39a2..90c923089da 100644 --- a/scripts/check-release-package-bootstrap.test.mjs +++ b/scripts/check-release-package-bootstrap.test.mjs @@ -4,9 +4,10 @@ import test from "node:test"; import { classifyNpmViewFailure, collectReleasePackagesForChangedPaths, + getBaseReleaseState, } from "./check-release-package-bootstrap.mjs"; -test("manifest changes force validation of all release-enabled packages", () => { +test("manifest changes without base state validate all release-enabled packages", () => { const releasePackages = [ { dir: "packages/a", name: "@paperclipai/a", publishFromCi: true }, { dir: "packages/b", name: "@paperclipai/b", publishFromCi: true }, @@ -24,6 +25,29 @@ test("manifest changes force validation of all release-enabled packages", () => ); }); +test("manifest changes only validate newly release-enabled packages relative to base state", () => { + 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 baseReleaseState = { + source: "manifest", + byDir: new Map([["packages/a", { name: "@paperclipai/a", publishFromCi: true }]]), + }; + + const changedPackages = collectReleasePackagesForChangedPaths( + ["scripts/release-package-manifest.json"], + releasePackages, + baseReleaseState, + ); + + assert.deepEqual( + changedPackages.map((pkg) => pkg.name), + ["@paperclipai/b"], + ); +}); + test("package-specific changes only validate affected release-enabled packages", () => { const releasePackages = [ { dir: "packages/a", name: "@paperclipai/a", publishFromCi: true }, @@ -50,3 +74,31 @@ 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"); }); + +test("base release state falls back to public packages when manifest is absent", () => { + const releasePackages = [ + { dir: "packages/a", name: "@paperclipai/a", publishFromCi: true }, + { dir: "packages/b", name: "@paperclipai/b", publishFromCi: true }, + ]; + + const baseReleaseState = getBaseReleaseState("base-sha", releasePackages, (_revision, filePath) => { + if (filePath === "scripts/release-package-manifest.json") { + return null; + } + + if (filePath === "packages/a/package.json") { + return JSON.stringify({ name: "@paperclipai/a", private: false }); + } + + if (filePath === "packages/b/package.json") { + return JSON.stringify({ name: "@paperclipai/b", private: true }); + } + + return null; + }); + + assert.equal(baseReleaseState?.source, "public-packages"); + assert.deepEqual([...baseReleaseState.byDir.entries()], [ + ["packages/a", { name: "@paperclipai/a", publishFromCi: true }], + ]); +});