diff --git a/.github/scripts/set-release-version.mjs b/.github/scripts/set-release-version.mjs new file mode 100644 index 000000000..1baf9ca4c --- /dev/null +++ b/.github/scripts/set-release-version.mjs @@ -0,0 +1,87 @@ +#!/usr/bin/env node +// Sets the release version in package.json AND package-lock.json. +// +// prerelease.yml and promote.yml used to `sed` package.json alone, so every +// release shipped a lockfile whose root version disagreed with the package it +// locks: +// +// v1.7.0 package.json=1.7.0 lock=1.6.0 +// v1.8.0 package.json=1.8.0 lock=1.8.0-rc.4 +// v1.9.0 package.json=1.9.0 lock=1.8.0 +// +// Nothing caught it for three releases because `npm ci` only fails on +// dependency drift, never on this field — the mismatch is inert until someone +// reads the diff, which is how it was eventually noticed. +// +// Both files are tab-indented JSON that JSON.stringify round-trips byte for +// byte, so rewriting them whole still produces a one-line-per-file diff. The +// test pins that: if npm ever changes how it formats a lockfile, a release +// commit would otherwise silently become a 40k-line reformat. + +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { argv } from "node:process"; + +/** + * @param {string} version Version to write, e.g. "1.9.0" or "1.9.0-rc.2". + * @param {string} dir Directory holding package.json and package-lock.json. + */ +// MAJOR.MINOR.PATCH with the optional prerelease suffix promote.yml and +// prerelease.yml actually produce ("1.9.0", "2.0.0-rc.3"). Deliberately not full +// semver: this is a gate on what may be written into a published manifest, and +// build metadata or a leading "v" would be a caller bug, not a version to honour. +const RELEASE_VERSION = /^\d+\.\d+\.\d+(-[0-9A-Za-z.-]+)?$/; + +export function setReleaseVersion(version, dir) { + if (!version) throw new Error("a version is required"); + // Truthiness alone let 123, " " and "not-a-version" through into both + // manifests. The callers compute this from a validated tag, so it is a + // defence-in-depth check — but a script whose whole purpose is to stop bad + // version metadata should not be the thing that writes it. + if (typeof version !== "string" || !RELEASE_VERSION.test(version)) { + throw new Error( + `invalid version ${JSON.stringify(version)}; expected MAJOR.MINOR.PATCH with an optional prerelease suffix`, + ); + } + + // Both manifests are read and validated before a single byte is written. The + // obvious order — write package.json, then validate the lockfile — left + // package.json bumped and the lockfile untouched whenever validation failed, + // which is precisely the half-bump this script exists to end. Its own error + // path must not reproduce the bug it fixes. + const load = (name) => { + const file = join(dir, name); + return { file, json: JSON.parse(readFileSync(file, "utf8")) }; + }; + + const pkg = load("package.json"); + const lock = load("package-lock.json"); + + // lockfileVersion 3 repeats the root version inside packages[""]. Optional + // chaining would quietly skip it if the shape ever changed — the same + // silent half-bump this script exists to end — so demand it instead. + if (!lock.json.packages?.[""]) { + throw new Error( + 'package-lock.json has no packages[""] entry; the lockfile format changed and this script needs updating', + ); + } + + pkg.json.version = version; + lock.json.version = version; + lock.json.packages[""].version = version; + + for (const { file, json } of [pkg, lock]) { + writeFileSync(file, `${JSON.stringify(json, null, "\t")}\n`); + } +} + +// Only run when invoked directly, so the test can import the function. +if (import.meta.filename === argv[1]) { + const version = argv[2]; + if (!version) { + console.error("usage: node .github/scripts/set-release-version.mjs "); + process.exit(1); + } + setReleaseVersion(version, process.cwd()); + console.log(`version ${version} set in package.json and package-lock.json`); +} diff --git a/.github/scripts/set-release-version.test.mjs b/.github/scripts/set-release-version.test.mjs new file mode 100644 index 000000000..77d69e64e --- /dev/null +++ b/.github/scripts/set-release-version.test.mjs @@ -0,0 +1,154 @@ +import { execFileSync } from "node:child_process"; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { setReleaseVersion } from "./set-release-version.mjs"; + +let dir; + +// Tab-indented, like the real files, and shaped like lockfileVersion 3 — the +// point of most of these assertions is formatting, so the fixtures have to be +// byte-faithful rather than merely structurally right. +const pkg = [ + "{", + '\t"name": "openscreen",', + '\t"version": "1.8.0",', + '\t"private": true', + "}", + "", +].join("\n"); + +const lock = [ + "{", + '\t"name": "openscreen",', + '\t"version": "1.8.0",', + '\t"lockfileVersion": 3,', + '\t"requires": true,', + '\t"packages": {', + '\t\t"": {', + '\t\t\t"name": "openscreen",', + '\t\t\t"version": "1.8.0",', + '\t\t\t"dependencies": {', + '\t\t\t\t"zod": "^4.0.0"', + "\t\t\t}", + "\t\t},", + '\t\t"node_modules/zod": {', + '\t\t\t"version": "4.0.0"', + "\t\t}", + "\t}", + "}", + "", +].join("\n"); + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "set-release-version-")); + writeFileSync(join(dir, "package.json"), pkg); + writeFileSync(join(dir, "package-lock.json"), lock); +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +const read = (name) => readFileSync(join(dir, name), "utf8"); + +describe("setReleaseVersion", () => { + it("sets the version in package.json and both lockfile roots", () => { + setReleaseVersion("1.9.0", dir); + + expect(JSON.parse(read("package.json")).version).toBe("1.9.0"); + const written = JSON.parse(read("package-lock.json")); + expect(written.version).toBe("1.9.0"); + expect(written.packages[""].version).toBe("1.9.0"); + }); + + it("accepts a prerelease version", () => { + setReleaseVersion("2.0.0-rc.3", dir); + + expect(JSON.parse(read("package.json")).version).toBe("2.0.0-rc.3"); + expect(JSON.parse(read("package-lock.json")).packages[""].version).toBe("2.0.0-rc.3"); + }); + + // The reason the script may rewrite these files wholesale: anything else in + // them must come back out byte for byte. If npm changes its lockfile + // formatting, this fails here rather than turning a release commit into a + // 40k-line reformat nobody reviews. + it("changes only the version lines, leaving formatting untouched", () => { + setReleaseVersion("1.9.0", dir); + + const diff = (before, after) => { + const a = before.split("\n"); + const b = after.split("\n"); + expect(b.length).toBe(a.length); + return a.map((line, i) => [line, b[i]]).filter(([x, y]) => x !== y); + }; + + expect(diff(pkg, read("package.json"))).toEqual([ + ['\t"version": "1.8.0",', '\t"version": "1.9.0",'], + ]); + expect(diff(lock, read("package-lock.json"))).toEqual([ + ['\t"version": "1.8.0",', '\t"version": "1.9.0",'], + ['\t\t\t"version": "1.8.0",', '\t\t\t"version": "1.9.0",'], + ]); + }); + + it("leaves dependency versions alone", () => { + setReleaseVersion("1.9.0", dir); + + const written = JSON.parse(read("package-lock.json")); + expect(written.packages["node_modules/zod"].version).toBe("4.0.0"); + }); + + // A lockfile format change must stop the release, not half-bump it. + it("throws rather than half-bumping when the lockfile shape is unknown", () => { + writeFileSync( + join(dir, "package-lock.json"), + `${JSON.stringify({ name: "openscreen", version: "1.8.0" }, null, "\t")}\n`, + ); + + expect(() => setReleaseVersion("1.9.0", dir)).toThrow(/packages/); + + // The throw alone was never the property this test claims. package.json + // used to be written before the lockfile was validated, so this case left + // exactly the half-bump the name promises it prevents. + expect(JSON.parse(read("package.json")).version).toBe("1.8.0"); + expect(read("package-lock.json")).not.toContain("1.9.0"); + }); + + it("requires a version", () => { + expect(() => setReleaseVersion("", dir)).toThrow(/version is required/); + }); + + // Truthiness alone let all of these reach both manifests. A number in + // particular writes `"version": 123`, which is not even a legal package.json. + it.each([ + [123, "a number"], + [" ", "whitespace"], + ["not-a-version", "a malformed version"], + ["v1.9.0", "a leading v"], + ])("rejects %o (%s)", (bad) => { + expect(() => setReleaseVersion(bad, dir)).toThrow(/invalid version/); + expect(JSON.parse(read("package.json")).version).toBe("1.8.0"); + expect(read("package-lock.json")).not.toContain("1.9.0"); + }); + + // The workflows invoke this with a repository-relative path, and the + // direct-invocation guard compares against `import.meta.filename`, which is + // absolute. Node resolves argv[1] before exposing it, so the two match — but + // nothing pinned that, and the whole script is dead code if it ever stops + // being true. Invoked here the way promote.yml and prerelease.yml do. + it("runs when invoked directly through a relative path", () => { + const scripts = join(dir, "scripts"); + mkdirSync(scripts); + copyFileSync( + join(import.meta.dirname, "set-release-version.mjs"), + join(scripts, "set-release-version.mjs"), + ); + + execFileSync("node", ["scripts/set-release-version.mjs", "1.9.0"], { cwd: dir }); + + expect(JSON.parse(read("package.json")).version).toBe("1.9.0"); + expect(JSON.parse(read("package-lock.json")).packages[""].version).toBe("1.9.0"); + }); +}); diff --git a/.github/workflows/prerelease.yml b/.github/workflows/prerelease.yml index 8723005fe..5a3e1c56d 100644 --- a/.github/workflows/prerelease.yml +++ b/.github/workflows/prerelease.yml @@ -106,10 +106,10 @@ jobs: echo "Creating release branch ${BRANCH} from ${GITHUB_REF_NAME}" git checkout -b "$BRANCH" fi - sed -i -E "s|(\"version\"[[:space:]]*:[[:space:]]*\")[^\"]*(\")|\1${PRERELEASE}\2|" package.json - echo "package.json version:" - grep '"version"' package.json - git add package.json + # Writes package-lock.json too. A sed over package.json alone left the + # lockfile behind on every release up to 1.9.0; see the script header. + node .github/scripts/set-release-version.mjs "${PRERELEASE}" + git add package.json package-lock.json git commit -m "chore(release): bump to ${PRERELEASE} [skip ci]" || echo "(version already at ${PRERELEASE})" git push "$REMOTE" "$BRANCH" diff --git a/.github/workflows/promote.yml b/.github/workflows/promote.yml index 7e76a6c79..0e01f4b8e 100644 --- a/.github/workflows/promote.yml +++ b/.github/workflows/promote.yml @@ -59,25 +59,25 @@ jobs: STABLE_VERSION: ${{ steps.version.outputs.stable_version }} run: node .github/scripts/release-milestone-close.mjs - - name: Bump package.json to stable version on the release branch + - name: Bump the version to stable on the release branch env: TOKEN: ${{ secrets.OPENSCREEN_RELEASE_TOKEN }} STABLE_VERSION: ${{ steps.version.outputs.stable_version }} run: | set -euo pipefail # Promote checks out the FROZEN release branch (created by prerelease.yml) and - # rewrites package.json there. This guarantees the stable tag points at the + # rewrites the version there. This guarantees the stable tag points at the # same code that was tested as the RC plus any cherry-picked bugfixes. BRANCH="release/v${STABLE_VERSION}" git fetch origin "$BRANCH" git checkout "$BRANCH" git reset --hard "origin/${BRANCH}" - sed -i -E "s|(\"version\"[[:space:]]*:[[:space:]]*\")[^\"]*(\")|\1${STABLE_VERSION}\2|" package.json - echo "package.json version:" - grep '"version"' package.json + # Writes package-lock.json too. A sed over package.json alone left the + # lockfile behind on every release up to 1.9.0; see the script header. + node .github/scripts/set-release-version.mjs "${STABLE_VERSION}" git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add package.json + git add package.json package-lock.json git commit --allow-empty -m "chore(release): bump to ${STABLE_VERSION} [skip ci]" || true git push "https://x-access-token:${TOKEN}@github.com/${GITHUB_REPOSITORY}.git" "$BRANCH"