From 7a36848b98f669202d84a4475b83e326be5efa7b Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Sat, 3 Oct 2026 14:05:28 +0200 Subject: [PATCH 1/2] Release guard: the checks run in a clean env, and every per-tag check runs in the guard release-after-suite.mjs now takes the run's git ref and, from one process, checks the tag against the declared prefix and manifest version, runs the release-checks suite, the semver policy check and the package's own tests, then releases. The checks run without NODE_OPTIONS, NODE_TEST_*, or NODE_AUTH_TOKEN; only the release action gets the token. The workflow's separate tag, semver and test steps are gone, and so is the pin's continue-on-error/shell/defaults ban, which no release check needs any more. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/publish-packages.yml | 74 ++++-------- docs/semver-policy.md | 29 +++-- scripts/check-changelog-semver.test.mjs | 153 +++++++++++++++++------- scripts/release-after-suite.mjs | 90 +++++++++++--- 4 files changed, 223 insertions(+), 123 deletions(-) diff --git a/.github/workflows/publish-packages.yml b/.github/workflows/publish-packages.yml index ea38c052..b957bef9 100644 --- a/.github/workflows/publish-packages.yml +++ b/.github/workflows/publish-packages.yml @@ -10,8 +10,8 @@ name: Publish packages # # Tag a release to publish, for example `cultcache-ts-v0.13.6` or # `cultnet-py-v0.1.0`. The tag's version must match the version in the package -# manifest; the workflow refuses the publish otherwise rather than shipping a -# mislabelled artifact. +# manifest; scripts/release-after-suite.mjs refuses the release otherwise rather +# than shipping a mislabelled artifact. on: push: @@ -42,7 +42,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: - # fetch-depth: 0 so the semver check below can see every previously + # fetch-depth: 0 so the semver check in the release step can see every previously # published tag, not just the one that triggered this run. - name: Check out CultLib uses: actions/checkout@v4 @@ -64,34 +64,20 @@ jobs: - name: Install workspace dependencies run: npm ci - - name: Verify the tag matches the manifest version - if: startsWith(github.ref, 'refs/tags/cultcache-ts-v') - run: | - manifest="$(node -p "require('./packages/cultcache-ts/package.json').version")" - tagged="${GITHUB_REF_NAME#cultcache-ts-v}" - if [ "$manifest" != "$tagged" ]; then - echo "tag $GITHUB_REF_NAME does not match manifest version $manifest" >&2 - exit 1 - fi - - # docs/semver-policy.md: a breaking change under too small a bump, or a - # version that skips or reverses the previous release, fails here - # instead of shipping with a number that lies. - - name: Verify semver policy - if: startsWith(github.ref, 'refs/tags/cultcache-ts-v') - run: | - tagged="${GITHUB_REF_NAME#cultcache-ts-v}" - node scripts/check-changelog-semver.mjs --package "@gamecult/cultcache-ts" --version "$tagged" - + # Manual runs publish nothing, so they build and test here. A tag run's + # Publish runs these tests itself. - name: Build and test + if: ${{ !startsWith(github.ref, 'refs/tags/') }} working-directory: packages/cultcache-ts run: npm test - # Runs the release-checks suite, then npm publish --access public, in one - # process that publishes only if the suite passed (docs/semver-policy.md). + # One process checks that the tag is the manifest version, runs the + # release-checks suite, the semver policy check (docs/semver-policy.md) + # and npm test, and only then runs npm publish --access public. Only + # npm publish receives the token. - name: Publish if: startsWith(github.ref, 'refs/tags/cultcache-ts-v') - run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts + run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts "$GITHUB_REF" env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} @@ -123,7 +109,7 @@ jobs: echo "run=false" >> "$GITHUB_OUTPUT" fi - # fetch-depth: 0 so the semver check below can see every previously + # fetch-depth: 0 so the semver check in the release step can see every previously # published tag, not just the one that triggered this run. - name: Check out CultLib if: steps.selected.outputs.run == 'true' @@ -149,40 +135,20 @@ jobs: with: dotnet-version: 10.0.x - - name: Verify the tag matches the manifest version - if: steps.selected.outputs.run == 'true' && startsWith(github.ref, 'refs/tags/') - run: | - manifest="$(python -c "import tomllib,pathlib;print(tomllib.loads(pathlib.Path('packages/${{ matrix.package }}/pyproject.toml').read_text())['project']['version'])")" - tagged="${GITHUB_REF_NAME#${{ matrix.package }}-v}" - if [ "$manifest" != "$tagged" ]; then - echo "tag $GITHUB_REF_NAME does not match manifest version $manifest" >&2 - exit 1 - fi - - # docs/semver-policy.md: a breaking change under too small a bump, or a - # version that skips or reverses the previous release, fails here - # instead of shipping with a number that lies. - - name: Verify semver policy - if: steps.selected.outputs.run == 'true' && startsWith(github.ref, 'refs/tags/') - run: | - tagged="${GITHUB_REF_NAME#${{ matrix.package }}-v}" - node scripts/check-changelog-semver.mjs --package "${{ matrix.package }}" --version "$tagged" - # Lower layers install from source so a release of one package is tested # against the sibling revision it was cut with, not whatever PyPI has. - - name: Install and test + - name: Install if: steps.selected.outputs.run == 'true' - run: | - python -m pip install --upgrade build -e packages/cultcache-py -e packages/cultnet-py -e packages/cultmesh-py - python -m unittest discover -s packages/${{ matrix.package }}/tests + run: python -m pip install --upgrade build -e packages/cultcache-py -e packages/cultnet-py -e packages/cultmesh-py - # Runs the release-checks suite, then builds the sdist and wheel, in one - # process that builds only if the suite passed (docs/semver-policy.md). - # Publish uploads only what this step built, so it has nothing to upload - # unless the suite passed. + # One process checks, on a tag, that the tag is the manifest version, runs + # the release-checks suite, on a tag the semver policy check + # (docs/semver-policy.md), and the package's unit tests, and only then + # builds the sdist and wheel. Publish uploads only what this step built, + # so it has nothing to upload unless every check passed. - name: Build sdist and wheel if: steps.selected.outputs.run == 'true' - run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }} + run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }} "$GITHUB_REF" - name: Publish if: steps.selected.outputs.run == 'true' && startsWith(github.ref, 'refs/tags/') diff --git a/docs/semver-policy.md b/docs/semver-policy.md index 4b7a2028..a2620660 100644 --- a/docs/semver-policy.md +++ b/docs/semver-policy.md @@ -271,8 +271,8 @@ scripts/check-changelog-semver.test.mjs`). It is wired into: -- `.github/workflows/publish-packages.yml`, right after each job's existing - "tag matches manifest version" check, for `cultcache-ts`, `cultcache-py`, +- `.github/workflows/publish-packages.yml`, through + `scripts/release-after-suite.mjs`, for `cultcache-ts`, `cultcache-py`, `cultnet-py`, and `cultmesh-py`, with `fetch-depth: 0` so the checkout holds the tags; - `scripts/build-unity-package.ps1`, for `org.gamecult.cultlib`; @@ -282,16 +282,21 @@ It is wired into: Each of those runs the check's own tests before it releases. The Unity release scripts run them before they call the check, and the tests pin that order. In -`publish-packages.yml` the release step runs `scripts/release-after-suite.mjs`, -which runs the tests and then the release action from one process, and releases -only if the tests passed: `npm publish` for `cultcache-ts`, and for the Python -packages the sdist and wheel build, whose output is all the PyPI upload step -uploads. The gate is that call, not a step condition, so no `if:` can release -what the tests did not pass. The tests compare each job's release steps with -their expected text exactly, less the steps' `if:` lines, and refuse -`continue-on-error`, `shell` and `defaults` anywhere in the workflow. Removing -the call is still possible, but only as a visible edit to those steps and to the -tests. +`publish-packages.yml` the release step runs `scripts/release-after-suite.mjs` +with the run's git ref. From one process it checks that a tag ref is the +package's declared prefix and manifest version, runs these tests, runs this +check for the tagged version, runs the package's own tests (`npm test`, or the +Python package's unit tests), and only then the release action: `npm publish` +for `cultcache-ts`, which it refuses on any ref but a release tag, and for the +Python packages the sdist and wheel build, whose output is all the PyPI upload +step uploads. The checks run without `NODE_OPTIONS`, `NODE_TEST_CONTEXT`, any +other `NODE_TEST_*` variable and `NODE_AUTH_TOKEN`, so no inherited variable can +make the tests run nothing and pass; only the release action receives the token. +The gate is that call, not a step condition, so no `if:`, `continue-on-error` or +`shell` on another step can release what a check refused. The tests compare +each job's release steps with their expected text exactly, less the steps' `if:` +lines. Removing the call is still possible, but only as a visible edit to those +steps and to the tests. `org.gamecult.cultlib` compiles CultMath from source and ships beside `org.gamecult.cultmath`, so its build refuses unless it declares exactly the diff --git a/scripts/check-changelog-semver.test.mjs b/scripts/check-changelog-semver.test.mjs index 237c870f..84b4e767 100644 --- a/scripts/check-changelog-semver.test.mjs +++ b/scripts/check-changelog-semver.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { execFileSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -742,23 +742,23 @@ test("workflow: the Caching Unity prefix is its own tag family, not another pack }); // The suite is what pins the declared prefixes and the checker's rules, so nothing releases without it. -// publish-packages.yml releases only through scripts/release-after-suite.mjs, which runs the suite and then -// the release action from one process, so no if: on any step can release what the suite did not pass. This -// pin reads no condition. It compares each job's release steps whole, less their own if: line, with the -// text below, so a release step that does anything else is a mismatch, and it refuses any step or job that -// may fail without failing the job, or that runs through another shell. Each Unity release script runs the -// suite itself, before it calls the checker. +// publish-packages.yml releases only through scripts/release-after-suite.mjs, which runs the tag check, the +// suite, the semver policy check and the package's tests and then the release action from one process, so no +// if:, continue-on-error or shell on any other step can release what those checks did not pass. This pin +// reads no condition. It compares each job's release steps whole, less their own if: line, with the text +// below, so a release step that does anything else is a mismatch. Each Unity release script runs the suite +// itself, before it calls the checker. const SUITE = /node\s+--test[^\n]*check-changelog-semver\.test\.mjs/; const CHECKER = /check-changelog-semver\.mjs/; const code = (text) => text.split("\n").filter((line) => !line.trim().startsWith("#")).join("\n"); const RELEASE_STEPS = { "cultcache-ts": [ - ["- name: Publish", " run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts", " env:", " NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}"], + ["- name: Publish", " run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts \"$GITHUB_REF\"", " env:", " NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}"], ], // Publish uploads only the dist the guarded step built, so it has nothing to upload unless the suite passed. python: [ - ["- name: Build sdist and wheel", " run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }}"], + ["- name: Build sdist and wheel", " run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }} \"$GITHUB_REF\""], ["- name: Publish", " uses: pypa/gh-action-pypi-publish@release/v1", " with:", " packages-dir: packages/${{ matrix.package }}/dist"], ], }; @@ -804,9 +804,6 @@ function workflowReleaseProblems(workflow) { from = at + 1; } }); - if (/continue-on-error|\bshell\b|\bdefaults\b/.test(code(workflow))) { - problems.push("the workflow says continue-on-error, shell or defaults: no step may fail without failing its job, or run through another shell"); - } return problems; } @@ -816,54 +813,94 @@ function scriptSuiteProblems(script) { return body.search(CHECKER) < body.search(SUITE) ? ["calls the checker before the suite"] : []; } -test("workflow: each job releases only through release-after-suite.mjs, and no step may fail without failing its job", () => { +test("workflow: each job releases only through release-after-suite.mjs", () => { const workflow = workflowText(); assert.deepEqual(workflowReleaseProblems(workflow), []); - const npmRun = "run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts"; - const pythonRun = "run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }}"; + const npmRun = 'run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts "$GITHUB_REF"'; + const pythonRun = 'run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }} "$GITHUB_REF"'; const exact = /is not exactly the release call/; for (const run of [npmRun, pythonRun]) { assert.equal(workflow.split(run).length, 2, run); const edit = (replacement) => workflowReleaseProblems(workflow.replace(run, replacement)).join("\n"); - // anything else on the line, on a continuation line, or in a block; another call; another shell + // anything else on the line, on a continuation line, or in a block; another call, ref or shell assert.match(edit(`${run} || true`), exact, run); assert.match(edit(`${run}\n || true`), exact, run); assert.match(edit("run: |\n node --test scripts/check-changelog-semver.test.mjs\n true"), exact, run); assert.match(edit(run.replace("release-after-suite.mjs", "check-changelog-semver.mjs")), exact, run); - assert.match(edit(`${run}\n shell: bash`), /another shell/, run); - assert.match(edit(`${run}\n continue-on-error: true`), /no step may fail/, run); + assert.match(edit(run.replace('"$GITHUB_REF"', "refs/tags/cultcache-ts-v0.14.0")), exact, run); + assert.match(edit(`${run}\n shell: bash`), exact, run); + assert.match(edit(`${run}\n continue-on-error: true`), exact, run); } - // the guard runs the suite itself, so the release steps' conditions are free + // the guard runs every release check itself, so neither the release steps' conditions nor a job allowed + // to fail can release what a check refused assert.deepEqual(workflowReleaseProblems(workflow.replace(/if: startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\)(\r?\n\s+run: node)/, "if: always()$1")), []); - // a job allowed to fail, a release step removed, renamed or doubled, a Publish that uploads another dist, an unknown job - assert.match(workflowReleaseProblems(workflow.replace(/( runs-on: ubuntu-latest\r?\n)/, "$1 continue-on-error: true\n")).join("\n"), /no step may fail/); + assert.deepEqual(workflowReleaseProblems(workflow.replace(/( runs-on: ubuntu-latest\r?\n)/, "$1 continue-on-error: true\n")), []); + // a release step removed, renamed or doubled, a Publish that uploads another dist, an unknown job assert.match(workflowReleaseProblems(workflow.replace("- name: Build sdist and wheel", "- name: Build")).join("\n"), /needs exactly one step "- name: Build sdist and wheel"/); assert.match(workflowReleaseProblems(workflow.replace("- name: Build and test", "- name: Publish\n run: npm publish\n\n - name: Build and test")).join("\n"), /needs exactly one step "- name: Publish"/); assert.match(workflowReleaseProblems(workflow.replace("packages-dir: packages/${{ matrix.package }}/dist", "packages-dir: dist")).join("\n"), exact); assert.match(workflowReleaseProblems(`${workflow}\n other:\n runs-on: ubuntu-latest\n`).join("\n"), /job other is not a job this pin knows/); }); -test("release-after-suite runs the suite first, and the release action only when the suite passed", async () => { - const { releaseAfterSuite, suite } = await import("./release-after-suite.mjs"); - const calls = []; - const runner = (suiteStatus, actionStatus = 0) => (command, args, cwd) => { - calls.push([command, args, cwd]); - return command === process.execPath ? suiteStatus : actionStatus; +test("release-after-suite checks the tag, then runs the suite, the semver check and the package's tests, and releases only when all passed", async () => { + const { releaseAfterSuite, suite, checker } = await import("./release-after-suite.mjs"); + const npmVersion = JSON.parse(readFileSync(join(repoRoot, "packages", "cultcache-ts", "package.json"), "utf8")).version; + const npmDir = join(repoRoot, "packages", "cultcache-ts"); + const pyDir = join(repoRoot, "packages", "cultnet-py"); + const pyVersion = readFileSync(join(pyDir, "pyproject.toml"), "utf8").match(/^version = "([^"]+)"$/m)[1]; + const npmTag = `refs/tags/cultcache-ts-v${npmVersion}`; + const pyTag = `refs/tags/cultnet-py-v${pyVersion}`; + const env = { PATH: "p", NODE_AUTH_TOKEN: "t", NODE_OPTIONS: "--test-name-pattern=x", NODE_TEST_CONTEXT: "child", NODE_TEST_X: "1" }; + const checked = { PATH: "p" }; + let calls; + // fail: the index of the call that exits 1 + const runner = (fail = -1) => (command, args, cwd, childEnv) => { + calls.push([command, args, cwd, childEnv]); + return calls.length - 1 === fail ? 1 : 0; }; - const suiteCall = [process.execPath, ["--test", suite], undefined]; - assert.equal(releaseAfterSuite(["npm-publish", "packages/cultcache-ts"], runner(0)), 0); - assert.deepEqual(calls, [suiteCall, ["npm", ["publish", "--access", "public"], "packages/cultcache-ts"]]); - calls.length = 0; - assert.equal(releaseAfterSuite(["python-build", "packages/cultnet-py"], runner(0, 3)), 3); - assert.deepEqual(calls, [suiteCall, ["python", ["-m", "build", "packages/cultnet-py"], undefined]]); - for (const action of ["npm-publish", "python-build"]) { - calls.length = 0; - assert.equal(releaseAfterSuite([action, "packages/x"], runner(1)), 1); - assert.deepEqual(calls, [suiteCall], action); + const go = (argv, fail) => { + calls = []; + return releaseAfterSuite(argv, runner(fail), env); + }; + const suiteCall = [process.execPath, ["--test", suite], undefined, checked]; + const checkerCall = (name, version) => [process.execPath, [checker, "--package", name, "--version", version, "--cwd", repoRoot], undefined, checked]; + const npmCalls = [ + suiteCall, + checkerCall("@gamecult/cultcache-ts", npmVersion), + ["npm", ["test"], npmDir, checked], + ["npm", ["publish", "--access", "public"], npmDir, env], + ]; + assert.equal(go(["npm-publish", npmDir, npmTag]), 0); + assert.deepEqual(calls, npmCalls); + // any check that fails stops the release at that check + for (const fail of [0, 1, 2]) { + assert.equal(go(["npm-publish", npmDir, npmTag], fail), 1, `check ${fail}`); + assert.deepEqual(calls, npmCalls.slice(0, fail + 1), `check ${fail}`); } - for (const argv of [[], ["npm-publish"], ["constructor", "x"], ["npm-publish", "x", "y"]]) { - calls.length = 0; - assert.equal(releaseAfterSuite(argv, runner(0)), 2, argv.join(" ")); + const pyCalls = [ + suiteCall, + checkerCall("cultnet-py", pyVersion), + ["python", ["-m", "unittest", "discover", "-s", join(pyDir, "tests")], undefined, checked], + ["python", ["-m", "build", pyDir], undefined, env], + ]; + assert.equal(go(["python-build", pyDir, pyTag]), 0); + assert.deepEqual(calls, pyCalls); + // a branch ref builds Python without the tag's semver check, and never publishes npm + assert.equal(go(["python-build", pyDir, "refs/heads/main"]), 0); + assert.deepEqual(calls, [pyCalls[0], pyCalls[2], pyCalls[3]]); + // a tag that is not the manifest version's, another package's tag, or no tag at all refuses before anything runs + for (const argv of [ + ["npm-publish", npmDir, `refs/tags/cultcache-ts-v${npmVersion}9`], + ["npm-publish", npmDir, `refs/tags/cultnet-py-v${npmVersion}`], + ["npm-publish", npmDir, "refs/heads/main"], + ["python-build", pyDir, `refs/tags/cultcache-py-v${pyVersion}`], + ["npm-publish", join(repoRoot, "packages"), npmTag], + ]) { + assert.equal(go(argv), 1, argv.join(" ")); + assert.deepEqual(calls, [], argv.join(" ")); + } + for (const argv of [[], ["npm-publish"], ["npm-publish", npmDir], ["constructor", "x", npmTag], ["npm-publish", npmDir, npmTag, "y"]]) { + assert.equal(go(argv), 2, argv.join(" ")); assert.deepEqual(calls, []); } // run as a script, a usage error exits 2 before the suite runs @@ -871,6 +908,42 @@ test("release-after-suite runs the suite first, and the release action only when assert.throws(() => execFileSync(process.execPath, [script], { stdio: "pipe" }), (error) => error.status === 2); }); +// Each of these makes `node --test` on a failing suite run nothing and exit 0 (Soul, asura cut-3-gate s4). +// The guard must still refuse, and must never reach the publish. +test("release-after-suite refuses when the environment would make the failing suite pass vacuously", async () => { + const { releaseAfterSuite, suite } = await import("./release-after-suite.mjs"); + const dir = mkdtempSync(join(tmpdir(), "cultlib-release-guard-")); + try { + const failing = join(dir, "failing.test.mjs"); + writeFileSync(failing, 'import { test } from "node:test";\ntest("fails", () => { throw new Error("the suite failed"); });\n'); + const runTests = (env) => spawnSync(process.execPath, ["--test", failing], { env, stdio: "ignore" }).status ?? 1; + const base = Object.fromEntries(Object.entries(process.env).filter(([key]) => !/^NODE_(OPTIONS|TEST_)/i.test(key))); + assert.equal(runTests(base), 1, "the fixture suite fails"); + const npmDir = join(repoRoot, "packages", "cultcache-ts"); + const tag = `refs/tags/cultcache-ts-v${JSON.parse(readFileSync(join(npmDir, "package.json"), "utf8")).version}`; + for (const bypass of [ + { NODE_TEST_CONTEXT: "child" }, + { NODE_TEST_CONTEXT: "child-v8" }, + { NODE_OPTIONS: "--test-name-pattern=^zzz_nothing$" }, + { NODE_OPTIONS: "--test-skip-pattern=." }, + ]) { + const env = { ...base, ...bypass, NODE_AUTH_TOKEN: "token" }; + const label = JSON.stringify(bypass); + assert.equal(runTests(env), 0, `${label} no longer makes the failing suite pass, so this test proves nothing`); + const released = []; + const run = (command, args, cwd, childEnv) => { + if (command === process.execPath && args.length === 2 && args[1] === suite) return runTests(childEnv); + released.push([command, ...args]); + return 0; + }; + assert.equal(releaseAfterSuite(["npm-publish", npmDir, tag], run, env), 1, label); + assert.deepEqual(released, [], label); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("the Unity release scripts run the suite before the checker; removing it fails this test", () => { const scripts = ["scripts/build-unity-package.ps1", "packages/cultmath/scripts/build-unity-package.ps1", "scripts/verify-caching-unity-release.ps1"]; for (const script of scripts) { diff --git a/scripts/release-after-suite.mjs b/scripts/release-after-suite.mjs index 1bf06a3b..7832b23c 100644 --- a/scripts/release-after-suite.mjs +++ b/scripts/release-after-suite.mjs @@ -1,34 +1,90 @@ -// How publish-packages.yml releases a package: the release-checks suite runs first, and the release -// action runs only if the suite passed, both from this one process. No workflow condition can then -// release what the suite did not pass. docs/semver-policy.md, "The suite gates every release". +// How publish-packages.yml releases a package. From this one process, in order: the release tag must name +// the package's declared prefix and its manifest version, the release-checks suite must pass, the semver +// policy check must pass for that version, the package's own tests must pass, and only then the release +// action runs. No workflow condition can then release what any of those checks did not pass. +// docs/semver-policy.md, "The suite gates every release". // -// node scripts/release-after-suite.mjs npm-publish npm publish --access public in -// node scripts/release-after-suite.mjs python-build python -m build , for the PyPI step +// node scripts/release-after-suite.mjs npm-publish npm publish --access public in +// node scripts/release-after-suite.mjs python-build python -m build , for the PyPI step +// +// is the run's GITHUB_REF. A tag ref is a release and gets the tag and semver checks; npm-publish +// refuses any other ref. python-build on a branch ref builds without them, for the manual runs that publish +// nothing. import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; +import releasePackages from "./release-packages.mjs"; export const suite = fileURLToPath(new URL("./check-changelog-semver.test.mjs", import.meta.url)); +export const checker = fileURLToPath(new URL("./check-changelog-semver.mjs", import.meta.url)); +const repoRoot = fileURLToPath(new URL("..", import.meta.url)); + +// The checks run without the variables that can make `node --test` run nothing and exit 0 +// (NODE_OPTIONS test filters, NODE_TEST_CONTEXT, any NODE_TEST_*), and without the publish token, +// which only the release action receives. +const UNCHECKED = /^(NODE_OPTIONS|NODE_TEST_.*|NODE_AUTH_TOKEN)$/i; +export const checkEnv = (env) => Object.fromEntries(Object.entries(env).filter(([key]) => !UNCHECKED.test(key))); + +const pyprojectField = (text, field) => + text.split(/^\[/m).find((section) => section.startsWith("project]"))?.match(new RegExp(`^${field} = "([^"]+)"$`, "m"))?.[1]; // Scoped npm packages default to restricted, and CultLib is MIT, so the publish says public. export const actions = { - "npm-publish": (dir) => ["npm", ["publish", "--access", "public"], dir], - "python-build": (dir) => ["python", ["-m", "build", dir], undefined], + "npm-publish": { + manifest: (dir) => JSON.parse(readFileSync(join(dir, "package.json"), "utf8")), + test: (dir) => ["npm", ["test"], dir], + release: (dir) => ["npm", ["publish", "--access", "public"], dir], + tagOnly: true, + }, + "python-build": { + manifest: (dir) => { + const text = readFileSync(join(dir, "pyproject.toml"), "utf8"); + return { name: pyprojectField(text, "name"), version: pyprojectField(text, "version") }; + }, + test: (dir) => ["python", ["-m", "unittest", "discover", "-s", join(dir, "tests")], undefined], + release: (dir) => ["python", ["-m", "build", dir], undefined], + tagOnly: false, + }, }; -const spawn = (command, args, cwd) => spawnSync(command, args, { cwd, stdio: "inherit" }).status ?? 1; +const spawn = (command, args, cwd, env) => spawnSync(command, args, { cwd, env, stdio: "inherit" }).status ?? 1; -// Returns the exit code. `run(command, args, cwd)` returns a command's exit code; tests pass their own. -export function releaseAfterSuite(argv, run = spawn) { - const [action, dir, ...rest] = argv; - if (!Object.hasOwn(actions, action ?? "") || !dir || rest.length > 0) { - console.error(`usage: node scripts/release-after-suite.mjs <${Object.keys(actions).join("|")}> `); +const refuse = (message) => { + console.error(`${message}; not releasing.`); + return 1; +}; + +// Returns the exit code. `run(command, args, cwd, env)` returns a command's exit code; tests pass their own. +export function releaseAfterSuite(argv, run = spawn, env = process.env) { + const [name, dir, ref, ...rest] = argv; + if (!Object.hasOwn(actions, name ?? "") || !dir || !ref || rest.length > 0) { + console.error(`usage: node scripts/release-after-suite.mjs <${Object.keys(actions).join("|")}> `); return 2; } - if (run(process.execPath, ["--test", suite], undefined) !== 0) { - console.error("The release check suite failed (scripts/check-changelog-semver.test.mjs); not releasing."); - return 1; + const action = actions[name]; + let manifest; + try { + manifest = action.manifest(dir); + } catch { + return refuse(`${dir} has no readable manifest`); } - return run(...actions[action](dir)); + const entry = Object.hasOwn(releasePackages, manifest.name ?? "") ? releasePackages[manifest.name] : null; + if (entry === null) return refuse(`${dir} is ${manifest.name}, which scripts/release-packages.mjs does not declare`); + const isTag = ref.startsWith("refs/tags/"); + const tag = `refs/tags/${entry.tagPrefix}-v${manifest.version}`; + if (isTag && ref !== tag) return refuse(`${ref} is not ${tag}, the tag of ${manifest.name}'s manifest version`); + if (!isTag && action.tagOnly) return refuse(`${name} releases only from a release tag, and ${ref} is not one`); + + const clean = checkEnv(env); + if (run(process.execPath, ["--test", suite], undefined, clean) !== 0) { + return refuse("The release check suite failed (scripts/check-changelog-semver.test.mjs)"); + } + if (isTag && run(process.execPath, [checker, "--package", manifest.name, "--version", manifest.version, "--cwd", repoRoot], undefined, clean) !== 0) { + return refuse(`${manifest.name} ${manifest.version} fails the semver policy (docs/semver-policy.md)`); + } + if (run(...action.test(dir), clean) !== 0) return refuse(`${manifest.name}'s own tests failed`); + return run(...action.release(dir), env); } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { From 19b52b16c38240312d91ebf5d352e7f013728690 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Sat, 3 Oct 2026 14:37:38 +0200 Subject: [PATCH 2/2] Release pin: keep the shell and defaults ban, which still guards what the pinned release line runs A job or workflow defaults.run.shell can run the exact Publish line through any command, with the token in its env. Only continue-on-error leaves the pin: a check that fails inside the guard refuses the release whatever the step allows. Co-Authored-By: Claude Opus 5.5 --- docs/semver-policy.md | 9 +++++---- scripts/check-changelog-semver.test.mjs | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/docs/semver-policy.md b/docs/semver-policy.md index a2620660..b381da17 100644 --- a/docs/semver-policy.md +++ b/docs/semver-policy.md @@ -292,10 +292,11 @@ Python packages the sdist and wheel build, whose output is all the PyPI upload step uploads. The checks run without `NODE_OPTIONS`, `NODE_TEST_CONTEXT`, any other `NODE_TEST_*` variable and `NODE_AUTH_TOKEN`, so no inherited variable can make the tests run nothing and pass; only the release action receives the token. -The gate is that call, not a step condition, so no `if:`, `continue-on-error` or -`shell` on another step can release what a check refused. The tests compare -each job's release steps with their expected text exactly, less the steps' `if:` -lines. Removing the call is still possible, but only as a visible edit to those +The gate is that call, not a step condition, so no `if:` or `continue-on-error` +can release what a check refused. The tests compare each job's release steps +with their expected text exactly, less the steps' `if:` lines, and refuse `shell` +and `defaults` anywhere in the workflow, which could run a release step's line +through another command. Removing the call is still possible, but only as a visible edit to those steps and to the tests. `org.gamecult.cultlib` compiles CultMath from source and ships beside diff --git a/scripts/check-changelog-semver.test.mjs b/scripts/check-changelog-semver.test.mjs index 84b4e767..72f2afdd 100644 --- a/scripts/check-changelog-semver.test.mjs +++ b/scripts/check-changelog-semver.test.mjs @@ -744,10 +744,11 @@ test("workflow: the Caching Unity prefix is its own tag family, not another pack // The suite is what pins the declared prefixes and the checker's rules, so nothing releases without it. // publish-packages.yml releases only through scripts/release-after-suite.mjs, which runs the tag check, the // suite, the semver policy check and the package's tests and then the release action from one process, so no -// if:, continue-on-error or shell on any other step can release what those checks did not pass. This pin -// reads no condition. It compares each job's release steps whole, less their own if: line, with the text -// below, so a release step that does anything else is a mismatch. Each Unity release script runs the suite -// itself, before it calls the checker. +// if: or continue-on-error on any step can release what those checks did not pass. This pin reads no +// condition. It compares each job's release steps whole, less their own if: line, with the text below, so a +// release step that does anything else is a mismatch, and it refuses shell and defaults anywhere, which could +// run the pinned line through another command. Each Unity release script runs the suite itself, before it +// calls the checker. const SUITE = /node\s+--test[^\n]*check-changelog-semver\.test\.mjs/; const CHECKER = /check-changelog-semver\.mjs/; const code = (text) => text.split("\n").filter((line) => !line.trim().startsWith("#")).join("\n"); @@ -804,6 +805,10 @@ function workflowReleaseProblems(workflow) { from = at + 1; } }); + // A job's or the workflow's defaults.run.shell would run the pinned release line through any command. + if (/\bshell\b|\bdefaults\b/.test(code(workflow))) { + problems.push("the workflow says shell or defaults: a release step must run its pinned line through the default shell"); + } return problems; } @@ -828,14 +833,15 @@ test("workflow: each job releases only through release-after-suite.mjs", () => { assert.match(edit("run: |\n node --test scripts/check-changelog-semver.test.mjs\n true"), exact, run); assert.match(edit(run.replace("release-after-suite.mjs", "check-changelog-semver.mjs")), exact, run); assert.match(edit(run.replace('"$GITHUB_REF"', "refs/tags/cultcache-ts-v0.14.0")), exact, run); - assert.match(edit(`${run}\n shell: bash`), exact, run); + assert.match(edit(`${run}\n shell: bash`), /another command|default shell/, run); assert.match(edit(`${run}\n continue-on-error: true`), exact, run); } // the guard runs every release check itself, so neither the release steps' conditions nor a job allowed // to fail can release what a check refused assert.deepEqual(workflowReleaseProblems(workflow.replace(/if: startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\)(\r?\n\s+run: node)/, "if: always()$1")), []); assert.deepEqual(workflowReleaseProblems(workflow.replace(/( runs-on: ubuntu-latest\r?\n)/, "$1 continue-on-error: true\n")), []); - // a release step removed, renamed or doubled, a Publish that uploads another dist, an unknown job + // a job's default shell, a release step removed, renamed or doubled, a Publish that uploads another dist, an unknown job + assert.match(workflowReleaseProblems(workflow.replace(/( runs-on: ubuntu-latest\r?\n)/, "$1 defaults:\n run:\n shell: sh -c 'npm publish' {0}\n")).join("\n"), /default shell/); assert.match(workflowReleaseProblems(workflow.replace("- name: Build sdist and wheel", "- name: Build")).join("\n"), /needs exactly one step "- name: Build sdist and wheel"/); assert.match(workflowReleaseProblems(workflow.replace("- name: Build and test", "- name: Publish\n run: npm publish\n\n - name: Build and test")).join("\n"), /needs exactly one step "- name: Publish"/); assert.match(workflowReleaseProblems(workflow.replace("packages-dir: packages/${{ matrix.package }}/dist", "packages-dir: dist")).join("\n"), exact);