diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 9add452d..04fc7075 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -30,12 +30,49 @@ on: # the same commit (every main commit has one, from its push run). workflow_dispatch: -# Cancel in-flight benches when a new push lands on the same PR / branch. -# Killing a bench mid-run is safe for the shared-box mutex: flock releases -# with the process (see tools/ci/with-nashua-lock.sh). +# Cancel in-flight benches when a new push lands on the same PR. Killing a +# bench mid-run is safe for the shared-box mutex: flock releases with the +# process (see tools/ci/with-nashua-lock.sh). +# +# NEVER on a push to main, because on a push `head_ref` is empty and every main +# push therefore shares the group `bench-refs/heads/main`. With +# cancel-in-progress: true, the release workflow's own version commit — pushed +# about five minutes after the merge that triggered it, into a bench that takes +# eleven — entered that group, CANCELLED the merge commit's bench, and was then +# skipped itself by the gate below. The merge produced no baseline at all: +# +# 21:45 16f50e3 Expand `hrtime` utility... (#70) cancelled +# 21:50 91d91bc chore(release): publish skipped +# 16:56 21d4749 fix: consolidated codec fixes (#73) cancelled +# 17:01 7abaaa9 chore(release): publish skipped +# +# The gate's guard exists to stop the version commit seeding a DUPLICATE +# baseline; paired with unconditional cancellation it destroyed the real one and +# supplied nothing in its place. Every PR opened afterwards compared against +# whatever CodSpeed still held for each benchmark, which is how a diff touching +# no runtime code drew a two-fold "regression". +# +# This was masked while releases were broken: a release that dies before the +# push never cancels anything, so `bac71dd` kept its baseline by accident. Fixing +# the release makes the version commit land reliably, which would have made this +# fire on most merges from now on. +# +# cancel-in-progress: false is NOT sufficient on its own, which is why the group +# is per-commit off a PR rather than per-branch. A concurrency group holds one +# running run and at most one PENDING run; queueing a third CANCELS the pending +# one, whatever cancel-in-progress says. A bench takes eleven minutes, so two +# merges inside that window would have put the second in the pending slot and +# let a third evict it -- the same lost baseline this is meant to fix, reached +# by a different route. +# +# Keying non-PR runs by github.sha gives every main commit its own group, so no +# main bench can ever cancel another. They serialise anyway: the bench box takes +# a mutex (tools/ci/with-nashua-lock.sh), which queues them without discarding +# any. PRs keep the branch-level group, where superseding an in-flight bench with +# a newer push is exactly what is wanted. concurrency: - group: bench-${{ github.head_ref || github.ref }} - cancel-in-progress: true + group: bench-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} permissions: contents: read diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 64b63b87..192a9adf 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -378,14 +378,20 @@ jobs: # Exercises the release scripts on every PR. They are otherwise only # ever executed by a live release — after the 8-job build matrix, and # partway through mutating manifests, which is a poor place to discover - # a syntax error or a bad tag walk. Mutates nothing (--dry-run), and - # publish-order.mjs additionally re-checks the topological order and - # that every package shipping dist/ actually has one. + # a syntax error or a bad tag walk. Mutates nothing and publishes + # nothing. + # + # release:preflight re-checks the topological order and that every + # package shipping dist/ actually has one, then asks the registry + # whether each package can actually be published. A package that is not + # on npm at all is only a WARNING here — the PR adding a codec is + # exactly when it legitimately does not exist yet — but it is the + # warning that was missing when libjxl was merged in #88 and broke the + # next four releases. In the release workflow the same finding is fatal. run: | set -euo pipefail - node tools/release/version.mjs --dry-run - echo "Publish order:" - node tools/release/publish-order.mjs + pnpm run release:plan + pnpm run release:preflight dist-size: # Binary-size regression gate: compares every shipped dist artifact diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7e09ca72..f1f57142 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,7 +5,7 @@ name: Release # THE FILENAME OF THIS WORKFLOW IS LOAD-BEARING. Each package's npm trusted # publisher is registered against `release.yml` in cornerstonejs/codecs, so npm # will reject an OIDC publish coming from any other workflow file. Renaming -# this file means re-running tools/release/setup-trusted-publishing.sh. +# this file means re-running `npm run release:trust`. # # This replaces the old CircleCI NPM_PUBLISH job, which needed two long-lived # personal credentials: an NPM_TOKEN written to ~/.npmrc, and a maintainer's @@ -509,24 +509,16 @@ jobs: cp -r "$d"/* "packages/$pkg/dist/" 2>/dev/null || true done ls packages/*/dist 2>/dev/null | head - - name: Compute publish order - # Generated as its own command on purpose. `done < <(cmd)` is process - # substitution, whose exit status `set -e` does not see: if - # publish-order.mjs refused an empty dist, the loop below would read - # nothing and the step would still go green, reporting "Published 0 - # packages" after the version commit and tags had already landed on - # main. Running it here makes that failure fail the job. - # - # publish-order.mjs emits the packages in dependency order, so - # dicom-codec goes out after the six siblings whose ranges it carries — - # publishing it alphabetically (third) left a window where those ranges - # resolved to versions that did not exist yet. It also refuses to emit a - # package that claims to ship dist/ but has an empty one. - run: | - set -euo pipefail - node tools/release/publish-order.mjs > publish-order.txt - cat publish-order.txt - name: Publish to npm + # One node script rather than a bash loop, for a reason the 2026-09-01 + # release paid for: `@cornerstonejs/codec-libjxl` was new, npm's OIDC + # trusted publishing cannot create a package that does not exist yet, so + # `npm publish` failed ENEEDAUTH — and `set -e` killed the loop where it + # stood, stranding the four already-registered packages queued behind it. + # publish.mjs resolves every package's registry state BEFORE it publishes + # anything, so a release that cannot fully succeed publishes nothing at + # all and says exactly what a human has to do. See its header. + # # Deliberately NOT gated on the plan. The source of truth is "every # workspace package whose current version is not yet on the registry", # which covers both the normal path (the versions version.mjs just @@ -534,30 +526,21 @@ jobs: # died partway through publishing, version.mjs correctly finds nothing # new to release on the re-run, and this step still finishes the job. # + # It also emits publish-order.txt for the github-releases job, in + # dependency order, so dicom-codec goes out after the six siblings whose + # ranges it carries — publishing it alphabetically (third) left a window + # where those ranges resolved to versions that did not exist yet — and + # refuses a package that claims to ship dist/ but has an empty one. + # # No .npmrc and no NODE_AUTH_TOKEN: `npm publish` performs the OIDC # exchange itself using the id-token permission above. Provenance is # generated automatically for trusted publishes, which is why every # package's repository.url must point at this repo. # - # --ignore-scripts: prepublishOnly re-runs `bash build.sh`, and this - # runner has no emscripten toolchain. The dist being published is the - # artifact the build job produced from this same commit. - run: | - set -euo pipefail - published=0 - skipped=0 - while read -r name version dir; do - if npm view "$name@$version" version >/dev/null 2>&1; then - echo "$name@$version is already on npm; skipping." - skipped=$((skipped + 1)) - continue - fi - - echo "Publishing $name@$version" - (cd "packages/$dir" && npm publish --ignore-scripts) - published=$((published + 1)) - done < publish-order.txt - echo "::notice::Published $published package(s); $skipped already on npm." + # `npm run` needs no node_modules — npm ships with node, and publish.mjs + # imports nothing but node builtins — so this job still installs no + # dependencies and the OIDC token still has no third-party code beside it. + run: npm run release:publish -- --out publish-order.txt - name: Upload publish order # Handed to the github-releases job so it can enumerate the same # packages without needing the dists (publish-order.mjs inspects them) diff --git a/BENCHMARKING.md b/BENCHMARKING.md index 94792eb5..6753dd6f 100644 --- a/BENCHMARKING.md +++ b/BENCHMARKING.md @@ -144,6 +144,38 @@ decode is the time spent" for wasm code — that's a fundamental limitation of wasm-via-V8-via-Cachegrind. For wasm hotspot analysis, profile the native build with `perf` / Instruments / VTune instead. +### "N benchmarks were skipped, so the baseline results were used instead" +The CodSpeed project holds more benchmark entries than the repo now +defines — as of 2026-09-04, 133 registered against 67 that actually run. +The surplus entries are orphans left by benches that were renamed or deleted +(the cold/warm split, the openjph upstream rework), plus +`libjpeg-turbo-12bit`, whose `bench` script is deliberately a no-op +(`.51` transfer syntax is disabled) so its 9th bench file never executes. + +**What to do**: nothing, for correctness. A skipped benchmark reuses its +baseline on *both* sides of the comparison, so its delta is zero and it +can never trigger a regression — the only cost is a misleading total. +Archiving them is **dashboard-only** (Settings → the benchmark → Archive); +there is no repo config, config file or code annotation that clears them, +so this cannot be fixed in a commit. + +### A regression on a diff that changes no runtime code +Almost always a stale baseline rather than a real change. The usual cause +was a main baseline that never got measured: until 2026-09-04 the release +workflow's version commit shared `bench.yml`'s push concurrency group with +the merge commit it followed, so it cancelled that bench and then skipped +its own — see the comment on `concurrency:` in +[.github/workflows/bench.yml](.github/workflows/bench.yml). Merges of #70 +and #73 produced no baseline at all. + +**What to do**: confirm the diff cannot affect the measured path, then let +the next successful **push to main** re-seed the baseline for all live +benchmarks — that happens on its own and needs no dashboard access. +Acknowledging a regression directly *is* dashboard-only and admin-only, but +it is not required to merge: main's ruleset lists no required status checks +(only 1 approving review and code-owner review), so a red CodSpeed check +never blocks a pull request. + ### Tier-related regression in `instantiate+destroy X` The instantiation bench measures whatever V8 tier the embind helpers (`makeClassHandle`, `RegisteredPointer_fromWireType`, etc.) happen to diff --git a/package.json b/package.json index 039bd742..b77c6847 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,10 @@ "build:publish": "pnpm -r --parallel run build:ci", "csp:source": "node tools/csp/check-source-js.js", "docker:build": "bash tools/docker/build.sh", - "release:plan": "node tools/release/version.mjs --dry-run" + "release:plan": "node tools/release/version.mjs --dry-run", + "release:order": "node tools/release/publish-order.mjs", + "release:preflight": "node tools/release/publish.mjs --preflight", + "release:publish": "node tools/release/publish.mjs", + "release:trust": "node tools/release/setup-trusted-publishing.mjs" } } diff --git a/packages/libjxl/package.json b/packages/libjxl/package.json index 6012f264..8f3c71bd 100644 --- a/packages/libjxl/package.json +++ b/packages/libjxl/package.json @@ -43,6 +43,7 @@ ], "repository": { "type": "git", - "url": "git+https://github.com/cornerstonejs/codecs.git" + "url": "git+https://github.com/cornerstonejs/codecs.git", + "directory": "packages/libjxl" } } diff --git a/tools/release/README.md b/tools/release/README.md index 37e2ef55..405e9500 100644 --- a/tools/release/README.md +++ b/tools/release/README.md @@ -27,9 +27,21 @@ Preview what the next release would do, at any time, from a clean checkout with ```bash pnpm release:plan # node tools/release/version.mjs --dry-run +pnpm release:preflight # what would publish, and whether npm will accept it +pnpm release:order # just the dependency order node tools/release/version.mjs --dry-run --json # machine-readable ``` +Every release entry point is a root `package.json` script, so none of them depend on a shell: + +| Script | Does | +| --- | --- | +| `release:plan` | version bumps that the next release would make, mutating nothing | +| `release:order` | publishable packages in dependency order | +| `release:preflight` | the order, plus each package's registry state; publishes nothing | +| `release:publish` | the above, then publishes what is missing (used by the workflow) | +| `release:trust` | one-time trusted-publisher registration (run by a human) | + `--dry-run` writes nothing. Without it the script rewrites manifests and changelogs and emits `release-plan.json` (gitignored); the workflow is what commits, tags and pushes. @@ -47,7 +59,8 @@ third-party code that runs in it — the `actions/*` it calls and any dependency | `github-releases` | `contents: write` | **no** | `pnpm install` therefore never runs in a job that can publish to npm, and the job holding the OIDC -token runs nothing but `npm`, the pinned actions and `publish-order.mjs` (node builtins only). +token runs nothing but `npm`, the pinned actions and the scripts in this directory (node builtins +only — `npm run` needs no `node_modules`, since npm ships with node). 1. **`build`** — every package's `dist`, in the emscripten container (matrix job). Read-only, but its artifacts are what reaches npm, so its actions are SHA-pinned like the rest. @@ -64,9 +77,12 @@ token runs nothing but `npm`, the pinned actions and `publish-order.mjs` (node b run [32733067241](https://github.com/cornerstonejs/codecs/actions/runs/32733067241) left eight version tags on a commit that never reached `main` and wedged every later release at `git tag -a` with "tag already exists". -3. **`publish`** — checks out that SHA, replays the dists, and publishes each package with - `npm publish --ignore-scripts` in the dependency order `publish-order.mjs` computes — dicom-codec - goes out after the six siblings whose ranges it carries. `--ignore-scripts` is deliberate: +3. **`publish`** — checks out that SHA, replays the dists, and runs `npm run release:publish`, which + publishes each package with `npm publish --ignore-scripts` in the dependency order + `publish-order.mjs` computes — dicom-codec goes out after the six siblings whose ranges it + carries. Before publishing anything it resolves every package's registry state, so a release that + cannot fully succeed publishes nothing (see [Adding a new package](#adding-a-new-package)). + `--ignore-scripts` is deliberate: `prepublishOnly` re-runs `bash build.sh`, and this job has no emscripten toolchain — the dist being published is the artifact built in step 1 from the same commit. npm's version comes from the exactly-pinned `node-version` (v24.20.0 → npm 11.19.0, past the 11.5.1 OIDC floor), so there @@ -80,8 +96,9 @@ token runs nothing but `npm`, the pinned actions and `publish-order.mjs` (node b which is the only thing standing between a dropped build artifact and an empty package on npm for `libjpeg-turbo-12bit` (it has no vitest config, so the test gate never touches it). -Both scripts run on every PR as a dry-run step in `pr-checks.yml`, so they are not first executed -mid-release. +`release:plan` and `release:preflight` run on every PR in `pr-checks.yml`, so these scripts are not +first executed mid-release. Preflight is also where a newly added package gets flagged — as a +warning on the PR, and as a hard failure in the release. Every step is idempotent. If a run dies partway through publishing, re-run the workflow from the Actions tab (`workflow_dispatch`) and it finishes the job rather than double-publishing: `version.mjs` @@ -97,13 +114,21 @@ Both scripts are run by a human, once, and need credentials no CI job has. ### 1. npm trusted publishing ```bash -npm install --global npm@latest # needs >= 11.15.0 for `npm trust` -npm login # account with publish rights on @cornerstonejs, 2FA enabled -bash tools/release/setup-trusted-publishing.sh +npm login # account with publish rights on @cornerstonejs, 2FA enabled +pnpm release:trust ``` -This registers `cornerstonejs/codecs` + `release.yml` as the trusted publisher for all eight -packages. The first call prompts for a 2FA one-time password. +This registers `cornerstonejs/codecs` + `release.yml` as the trusted publisher for every package in +the workspace. The first call prompts for a 2FA one-time password. Re-running is safe: a package +that already has a config is reported and skipped. + +`npm trust` needs npm >= 11.15.0. The way to get it is **Node 24.20.0**, which bundles npm 11.19.0 — +the same version `release.yml` pins. Do not reach for `npm install --global npm@latest`: on an older +Node it refuses outright, because npm 12 requires `^22.22.2 || ^24.15.0 || >=26.0.0`. + +`npm trust` also needs a **web-login session**, not a token. A granular or classic token in +`~/.npmrc` publishes fine but fails here with `401 ... Bearer token authorization is required`, so +`npm login` is not optional even on a machine that can already publish. **The workflow's filename is part of the trust relationship.** Renaming `release.yml` breaks every publish until the script is re-run against the new name. @@ -116,6 +141,45 @@ After the first green release, harden on npmjs.com: set each package's *Publishi "Require two-factor authentication and disallow tokens", and delete the old `NPM_TOKEN` from the CircleCI project (CircleCI no longer runs anything for this repo — the project should be disabled). +## Adding a new package + +**A new package cannot be released by CI until a human has published it once.** npm's OIDC trusted +publishing is configured *per package, on the registry*, so there is nothing to configure until the +package exists — and `npm trust` cannot create it ([npm/cli#8544](https://github.com/npm/cli/issues/8544)). +CI has no other npm credential by design, so its first `npm publish` fails with `ENEEDAUTH`. + +This is not hypothetical. `@cornerstonejs/codec-libjxl` was merged in +[#88](https://github.com/cornerstonejs/codecs/pull/88) and every release for the next three days +failed on it, each one leaving `main` tagged for versions that were not on npm. Because the old +publish step was a bash loop under `set -e`, it died where it stood — and libjxl sits fifth in +dependency order, so `little-endian`, `openjpeg`, `openjph` and `dicom-codec` were never even +attempted. Four packages that would have published fine sat stranded behind one that could not. + +So, after merging a new codec: + +```bash +cd packages/ +npm publish --ignore-scripts # --ignore-scripts: prepublishOnly wants the emscripten toolchain +cd - +pnpm release:trust # registers release.yml for it; skips the rest +``` + +Then re-run the Release workflow. That one bootstrap version ships **without a provenance +attestation** — there was no trusted publisher to key it to — and every version after it has one. +Check with `npm view @ dist.attestations`. + +Two guards now make this loud instead of silent: + +- `release:preflight` runs on every PR and warns that the package is not on npm yet. It warns rather + than fails, because on the PR that adds the codec that is simply true. +- `release:publish` resolves every package's registry state *before* publishing anything, and aborts + the release with these instructions if one needs bootstrapping. Nothing is published, so no + release half-lands. + +Fail-fast is deliberate here, rather than skipping the bad package and continuing: publishing +`dicom-codec` while a sibling whose range it carries has just failed is precisely the window +`publish-order.mjs` exists to close. + ### 2. A push credential for `main` + the branch ruleset Two routes. **The deploy-key route needs only repo admin**; the App route is better hygiene but diff --git a/tools/release/npm.mjs b/tools/release/npm.mjs new file mode 100644 index 00000000..e2a79e4a --- /dev/null +++ b/tools/release/npm.mjs @@ -0,0 +1,92 @@ +// Spawning npm, correctly, on every platform the release scripts run on. +// +// Shared by publish.mjs and setup-trusted-publishing.mjs. Node builtins only — +// release.yml's publish job installs no dependencies on purpose (see its +// comments), so nothing in this directory may import from node_modules. +// +// Windows needs care twice over: +// +// 1. npm is a .cmd shim there, and since the fix for CVE-2024-27980 node +// refuses to spawn .bat/.cmd without a shell — it fails with EINVAL. A +// script that read that failure as "npm says no" would be badly wrong; +// the first draft of publish.mjs did exactly that and reported all nine +// published packages as brand-new. +// 2. Passing an args ARRAY together with `shell: true` is deprecated +// (DEP0190) because the args are concatenated without escaping. So build +// the one command string deliberately instead of letting node do it +// silently. Every argument the callers pass is a fixed flag or a registry +// spec — no spaces, no shell metacharacters, nothing from user input — so +// concatenation is safe here, and `assertShellSafe` keeps it that way if +// someone later passes something with a space in it. + +import { spawnSync } from 'node:child_process'; + +const NPM = process.platform === 'win32' ? 'npm.cmd' : 'npm'; +const NEEDS_SHELL = process.platform === 'win32'; + +function assertShellSafe(args) { + for (const arg of args) { + if (!/^[A-Za-z0-9@/._=-]+$/.test(arg)) { + throw new Error( + `refusing to pass ${JSON.stringify(arg)} to npm through a shell: ` + + 'it is not a plain flag or registry spec. Quote it properly, or spawn npm without a shell.' + ); + } + } +} + +/** + * Run npm and hand back the raw spawnSync result — status and stderr included, + * so callers can tell an expected non-zero exit (a 404 from `npm view`) from a + * genuine failure. Throws only when npm could not be started at all. + */ +export function runNpm(args, options = {}) { + const spawnOptions = { encoding: 'utf8', ...options }; + + const result = NEEDS_SHELL + ? (assertShellSafe(args), spawnSync([NPM, ...args].join(' '), { ...spawnOptions, shell: true })) + : spawnSync(NPM, args, spawnOptions); + + if (result.error) { + throw new Error(`could not run \`npm ${args.join(' ')}\`: ${result.error.message}`); + } + + return result; +} + +/** npm's own version, e.g. "11.19.0". */ +export function npmVersion() { + const result = runNpm(['--version'], { stdio: ['ignore', 'pipe', 'pipe'] }); + + if (result.status !== 0) { + throw new Error(`\`npm --version\` failed with exit ${result.status}`); + } + + return `${result.stdout}`.trim(); +} + +/** + * Whether a registry spec resolves, distinguishing "npm says it is not there" + * from "npm could not tell us". + * + * That distinction is a safety property, not pedantry: treating any non-zero + * exit as "not published" turns a network blip or an expired session into + * "this is a brand-new package", which is a conclusion the release acts on. + */ +export function resolvesOnRegistry(spec) { + const result = runNpm(['view', spec, 'version'], { stdio: ['ignore', 'pipe', 'pipe'] }); + + if (result.status === 0) { + return true; + } + + const stderr = `${result.stderr ?? ''}`; + if (/E404|404 Not Found|is not in this registry/.test(stderr)) { + return false; + } + + throw new Error( + `\`npm view ${spec} version\` failed with exit ${result.status}, and not with a 404. ` + + `Refusing to guess whether it is published.\n${stderr.trim()}` + ); +} diff --git a/tools/release/publish-order.mjs b/tools/release/publish-order.mjs index 539a3634..b623fe8c 100644 --- a/tools/release/publish-order.mjs +++ b/tools/release/publish-order.mjs @@ -23,11 +23,15 @@ // package after it. One stream of complete lines has no such edge. // // Usage: -// node tools/release/publish-order.mjs +// node tools/release/publish-order.mjs (or: npm run release:order) +// +// Also imported by publish.mjs, which needs the same order and the same dist +// check but drives the publishing itself. The CLI behaviour below runs only +// when this file is executed directly, so importing it has no side effects. import fs from 'node:fs'; import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); const PACKAGES_DIR = path.join(ROOT, 'packages'); @@ -116,21 +120,35 @@ function assertShippableContents(pkg) { } } -function main() { - const packages = readWorkspace(); - const ordered = topologicallySorted(packages); +/** + * The workspace's publishable packages, in dependency order, each verified to + * have the dist/ it claims to ship. Throws on a cycle or an empty dist. + */ +export function publishOrder() { + const ordered = topologicallySorted(readWorkspace()); for (const pkg of ordered) { assertShippableContents(pkg); } - process.stdout.write(ordered.map((p) => `${p.name} ${p.version} ${p.dir}`).join('\n')); - process.stdout.write('\n'); + return ordered; +} + +/** The ` ` lines the workflow's publish-order.txt holds. */ +export function formatOrder(ordered) { + return `${ordered.map((p) => `${p.name} ${p.version} ${p.dir}`).join('\n')}\n`; } -try { - main(); -} catch (error) { - console.error(`::error::${error.message}`); - process.exit(1); +function main() { + process.stdout.write(formatOrder(publishOrder())); +} + +// Executed directly, not imported. +if (process.argv[1] && pathToFileURL(process.argv[1]).href === import.meta.url) { + try { + main(); + } catch (error) { + console.error(`::error::${error.message}`); + process.exit(1); + } } diff --git a/tools/release/publish.mjs b/tools/release/publish.mjs new file mode 100644 index 00000000..60f9b3a4 --- /dev/null +++ b/tools/release/publish.mjs @@ -0,0 +1,208 @@ +#!/usr/bin/env node +// +// Publishes the workspace to npm, in dependency order, skipping versions that +// are already on the registry. +// +// This replaces the inline bash loop that used to live in release.yml's publish +// step. The reason it moved is the 2026-09-01 release: `@cornerstonejs/codec-libjxl` +// was a brand-new package, and npm's OIDC trusted publishing CANNOT create a +// package that does not exist yet -- a trusted publisher is configured per +// package, on the registry, so there is nothing to configure until the first +// version is there (npm/cli#8544). `npm publish` therefore failed with +// ENEEDAUTH, and because the loop ran under `set -euo pipefail` the step died on +// the spot -- stranding little-endian, openjpeg, openjph and dicom-codec, four +// packages that were already registered and would have published fine. Every +// release for the next three days failed the same way, each one leaving main +// tagged for versions that were not on npm. +// +// So the fix is not "keep going on error" -- publishing dicom-codec when a +// sibling whose range it carries has just failed is exactly the window +// publish-order.mjs exists to close. The fix is to ask the question BEFORE +// publishing anything: is every package we are about to publish actually +// publishable? A package that npm cannot accept is knowable up front, and a +// release that cannot fully succeed should decline to half-succeed. +// +// Usage: +// node tools/release/publish.mjs (npm run release:publish) +// node tools/release/publish.mjs --preflight (npm run release:preflight) +// node tools/release/publish.mjs --out FILE also write the publish order +// +// --preflight checks and reports without publishing, and treats a +// needs-bootstrap package as a WARNING rather than an error: on a pull request +// the package legitimately does not exist yet, and failing the PR that adds a +// codec would be wrong. Without it, needs-bootstrap is fatal and nothing is +// published. +// +// NODE BUILTINS ONLY, and it must stay that way. release.yml's publish job +// installs no dependencies on purpose, so that the only code running next to a +// credential which can publish to npm is npm itself and the dependency-free +// scripts in this directory. An import from node_modules here would quietly +// undo that boundary. + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { publishOrder, formatOrder } from './publish-order.mjs'; +import { runNpm, resolvesOnRegistry } from './npm.mjs'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PACKAGES_DIR = path.join(ROOT, 'packages'); + +/** + * What npm will let us do with this package, in one of three states: + * + * published this exact version is already on the registry -> skip it + * publishable the package exists, this version does not -> publish it + * needs-bootstrap the package NAME is not on the registry at all + * + * The third is the one that broke the release. `npm view` reports a missing + * version and a missing package the same way (E404), so the two lookups have + * to be separate to tell them apart. + */ +function registryState(pkg) { + if (resolvesOnRegistry(`${pkg.name}@${pkg.version}`)) { + return 'published'; + } + + return resolvesOnRegistry(pkg.name) ? 'publishable' : 'needs-bootstrap'; +} + +/** + * The one-line form, for a `::error::`/`::warning::` annotation. + * + * GitHub renders only the FIRST line of an annotation's message in the Checks + * tab, so emitting the multi-line instructions there showed them cut off + * mid-sentence ("...and npm's OIDC trusted"). The annotation therefore has to + * say the whole thing in one line and point at the log for the commands, which + * is why this is separate from bootstrapInstructions rather than its first + * line. Naming the packages here matters: the annotation is the part a + * maintainer sees without opening the job. + */ +function bootstrapSummary(packages) { + const names = packages.map((p) => p.name).join(', '); + + return ( + `${packages.length} package(s) have never been published and need one manual ` + + `publish before trusted publishing can release them: ${names}. ` + + 'Nothing was published; the job log has the exact commands.' + ); +} + +function bootstrapInstructions(packages) { + const lines = [ + `${packages.length} package(s) have never been published, and npm's OIDC trusted`, + 'publishing cannot create a package that does not exist yet -- the trusted publisher is', + 'configured per package on the registry, so the first version has to come from a human.', + 'Nothing was published; the release declined rather than publishing a partial set.', + '', + 'A maintainer with publish rights on @cornerstonejs and 2FA enabled should run, once:', + '', + ]; + + for (const pkg of packages) { + lines.push(` cd packages/${pkg.dir} && npm publish --ignore-scripts && cd -`); + } + + lines.push( + '', + ' npm run release:trust # registers release.yml as the trusted publisher', + '', + 'then re-run this workflow. See tools/release/README.md, "Adding a new package".' + ); + + return lines.join('\n'); +} + +function main() { + const args = process.argv.slice(2); + const preflightOnly = args.includes('--preflight'); + const outIndex = args.indexOf('--out'); + const outFile = outIndex === -1 ? null : args[outIndex + 1]; + + // Throws on a dependency cycle, or on a package that says it ships dist/ but + // has an empty one -- both before any network call or any publish. + const ordered = publishOrder(); + + if (outFile) { + fs.writeFileSync(outFile, formatOrder(ordered)); + } + + console.log(`Publish order (${ordered.length} packages):`); + for (const pkg of ordered) { + console.log(` ${pkg.name} ${pkg.version} (${pkg.dir})`); + } + console.log(''); + + // PREFLIGHT: resolve every package's state before touching the registry with + // a write. This is the whole point of the file -- see the header. + const state = new Map(); + for (const pkg of ordered) { + state.set(pkg.name, registryState(pkg)); + } + + const needsBootstrap = ordered.filter((p) => state.get(p.name) === 'needs-bootstrap'); + + if (needsBootstrap.length > 0) { + // One line for the annotation, the instructions as ordinary log lines -- + // see bootstrapSummary for why they cannot be the same string. Each pair + // goes to a single stream so the two stay adjacent in the log. + const summary = bootstrapSummary(needsBootstrap); + const instructions = bootstrapInstructions(needsBootstrap); + + if (!preflightOnly) { + console.error(`::error::${summary}`); + console.error(instructions); + process.exit(1); + } + + // Advisory on a PR: the package really does not exist yet, and the PR that + // adds a codec should not fail for it. It should say so loudly, though -- + // silence here is what let libjxl reach main and break four releases. + console.log(`::warning::${summary}`); + console.log(instructions); + } + + const toPublish = ordered.filter((p) => state.get(p.name) === 'publishable'); + const alreadyOn = ordered.filter((p) => state.get(p.name) === 'published'); + + for (const pkg of alreadyOn) { + console.log(`${pkg.name}@${pkg.version} is already on npm; skipping.`); + } + + if (preflightOnly) { + console.log(''); + console.log( + `Preflight: ${toPublish.length} to publish, ${alreadyOn.length} already on npm, ` + + `${needsBootstrap.length} awaiting a first manual publish.` + ); + return; + } + + // Fail-fast, deliberately. If a publish fails here it is not a case we + // predicted, and continuing would publish dicom-codec over ranges pointing at + // a sibling that did not make it. + for (const pkg of toPublish) { + console.log(`Publishing ${pkg.name}@${pkg.version}`); + // --ignore-scripts: prepublishOnly re-runs the emscripten build, and the + // publish job has no toolchain. The dist here is the build job's artifact. + const result = runNpm(['publish', '--ignore-scripts'], { + cwd: path.join(PACKAGES_DIR, pkg.dir), + stdio: 'inherit', + }); + + if (result.status !== 0) { + throw new Error(`npm publish failed for ${pkg.name}@${pkg.version} (exit ${result.status})`); + } + } + + console.log( + `::notice::Published ${toPublish.length} package(s); ${alreadyOn.length} already on npm.` + ); +} + +try { + main(); +} catch (error) { + console.error(`::error::${error.message}`); + process.exit(1); +} diff --git a/tools/release/setup-trusted-publishing.mjs b/tools/release/setup-trusted-publishing.mjs new file mode 100644 index 00000000..35190512 --- /dev/null +++ b/tools/release/setup-trusted-publishing.mjs @@ -0,0 +1,279 @@ +#!/usr/bin/env node +// +// One-time setup: register .github/workflows/release.yml as the npm trusted +// publisher for every package in this workspace. +// +// After this runs, the release workflow authenticates to npm with a short-lived +// OIDC token minted per run and scoped to that workflow, so the release itself +// needs no NPM_TOKEN. +// +// It does NOT make a leaked token harmless. `npm trust github` ADDS an +// authorized publishing path; it revokes nothing. Every access token that could +// publish these packages before can still publish them afterwards. Token +// publishing stops only when each package's Publishing access on npmjs.com is +// set to "Require two-factor authentication and disallow tokens" -- a manual, +// per-package step that this script cannot perform and does not verify. It is +// printed as a next step at the end of a successful run; until it is done for +// every package, treat the old tokens as live credentials. +// +// Usage: +// npm run release:trust +// REPO=owner/name WORKFLOW=release.yml npm run release:trust +// +// PREREQUISITES +// 1. npm >= 11.15.0. The simplest way to get it is Node 24.20.0, which +// bundles npm 11.19.0 -- the same pin release.yml uses. `npm install +// --global npm@latest` will refuse on an older Node (npm 12 wants +// ^22.22.2 || ^24.15.0 || >=26.0.0). +// 2. Logged in interactively: npm login +// The account needs publish rights on @cornerstonejs and must have 2FA +// enabled. `npm trust` talks to an endpoint that only accepts a web-login +// SESSION token: a granular or classic access token sitting in ~/.npmrc +// fails with `401 ... Bearer token authorization is required`, even though +// the same token publishes fine. That is why this cannot run unattended. +// If you hit that 401, clear the stored token and log in again: +// npm logout # or delete the //registry.npmjs.org/:_authToken +// npm login # line from ~/.npmrc by hand if logout fails +// Note this replaces whatever token was in ~/.npmrc. +// +// Re-running is safe. npm allows exactly one publisher config per package, and +// `npm trust github` fails rather than updating an existing one -- including +// when the existing config is byte-for-byte what we would have created. So each +// package's current config is read first, and one that already points at +// REPO/WORKFLOW is skipped as done rather than retried and counted as a +// failure. Without that check a fully configured workspace exited 1 on every +// re-run. +// +// A config that exists but points somewhere ELSE is still a failure: that is a +// real conflict, and resolving it means deciding which one is right. Revoke it +// first if the answer is ours: +// npm trust list +// npm trust revoke --id +// +// A package that is not yet ON the registry cannot be configured at all -- see +// "Adding a new package" in README.md. Such packages are reported and skipped +// here rather than failing the run, since the fix is a manual publish. +// +// WHY THIS IS NODE AND NOT BASH. It used to be setup-trusted-publishing.sh, +// which computed the repo root with `cd ... && pwd` and passed it as argv to +// node. Under Cygwin `pwd` yields /cygdrive/z/src/codecs, and a *Windows* +// node.exe resolves that leading slash against the current drive -- so the +// scan died with `ENOENT: scandir 'Z:\cygdrive\z\src\codecs\packages'`. There +// is no path handed across a shell boundary here, and npm is spawned via its +// platform-correct executable name, so this runs the same from cmd, +// PowerShell, Git Bash, Cygwin and CI. + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { runNpm, npmVersion, resolvesOnRegistry } from './npm.mjs'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PACKAGES_DIR = path.join(ROOT, 'packages'); + +const REPO = process.env.REPO || 'cornerstonejs/codecs'; +const WORKFLOW = process.env.WORKFLOW || 'release.yml'; +const REQUIRED_NPM = '11.15.0'; + +/** -1 / 0 / 1, on dotted numeric versions. Avoids a semver dependency. */ +function compareVersions(a, b) { + const pa = a.split('.').map(Number); + const pb = b.split('.').map(Number); + + for (let i = 0; i < Math.max(pa.length, pb.length); i += 1) { + const diff = (pa[i] ?? 0) - (pb[i] ?? 0); + if (diff !== 0) { + return Math.sign(diff); + } + } + + return 0; +} + +function requireNpmVersion() { + const current = npmVersion(); + + if (compareVersions(current, REQUIRED_NPM) < 0) { + console.error(`npm ${current} is too old for \`npm trust\` (need >= ${REQUIRED_NPM}).`); + console.error('Install Node 24.20.0, which bundles npm 11.19.0:'); + console.error(' https://nodejs.org/dist/v24.20.0/node-v24.20.0-x64.msi'); + process.exit(1); + } + + return current; +} + +/** + * Every publishable package in the workspace. + * + * Read from the workspace rather than hardcoded, so the list cannot drift. A + * package added or renamed without a trusted publisher of its own does not + * fail here -- it fails at `npm publish`, partway through a live release. + */ +function publishablePackages() { + const packages = []; + + for (const entry of fs.readdirSync(PACKAGES_DIR).sort()) { + const manifestPath = path.join(PACKAGES_DIR, entry, 'package.json'); + if (!fs.existsSync(manifestPath)) { + continue; + } + + const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + if (manifest.private || !manifest.name) { + continue; + } + + packages.push(manifest.name); + } + + return packages; +} + +/** + * Whether `name` already trusts REPO/WORKFLOW. + * + * Returns 'match' (ours, nothing to do), 'other' (a config exists but is not + * ours) or 'unknown'. 'unknown' covers no config, no permission to read one, + * and output this cannot parse. + * + * Deliberately biased towards 'unknown': the caller attempts the create on + * anything that is not a confident 'match', so a shape this does not recognise + * costs a redundant `npm trust github` and the error it already handled. The + * opposite bias would silently skip a package that has no trusted publisher at + * all, and that is not discovered until a release tries to publish it. + * + * `npm trust list --json` is not covered by npm's documented output contract, + * hence the tolerance about field names. + */ +function existingTrust(name) { + const result = runNpm(['trust', 'list', name, '--json'], { stdio: ['ignore', 'pipe', 'pipe'] }); + + if (result.status !== 0) { + return 'unknown'; + } + + let parsed; + try { + parsed = JSON.parse(`${result.stdout}`); + } catch { + return 'unknown'; + } + + // Accept a bare array or any single-key wrapper around one. + const entries = Array.isArray(parsed) + ? parsed + : Object.values(parsed ?? {}).find(Array.isArray) ?? []; + + if (entries.length === 0) { + return 'unknown'; + } + + const matches = entries.some((entry) => { + if (!entry || typeof entry !== 'object') { + return false; + } + + const repo = entry.repository ?? entry.repo ?? entry.project; + const file = entry.file ?? entry.workflow ?? entry.workflowFilename; + + return repo === REPO && file === WORKFLOW; + }); + + return matches ? 'match' : 'other'; +} + +function main() { + const npmCurrent = requireNpmVersion(); + const packages = publishablePackages(); + + if (packages.length === 0) { + console.error(`No publishable packages found under ${PACKAGES_DIR}.`); + process.exit(1); + } + + console.log(`npm ${npmCurrent}`); + console.log( + `Registering ${REPO} / ${WORKFLOW} as trusted publisher for ${packages.length} packages.` + ); + console.log('The first package will prompt for your 2FA one-time password.'); + console.log(''); + + const failed = []; + const unpublished = []; + const alreadyConfigured = []; + + for (const name of packages) { + console.log(`==> ${name}`); + + if (!resolvesOnRegistry(name)) { + console.log(' NOT ON NPM YET — skipping (needs a first manual publish; see README.md)'); + unpublished.push(name); + console.log(''); + continue; + } + + if (existingTrust(name) === 'match') { + console.log(` already trusts ${REPO} / ${WORKFLOW} — skipping`); + alreadyConfigured.push(name); + console.log(''); + continue; + } + + // stdio: inherit so npm's 2FA prompt reaches the terminal. + const result = runNpm( + ['trust', 'github', name, '--repo', REPO, '--file', WORKFLOW, '--allow-publish', '--yes'], + { stdio: 'inherit' } + ); + + if (result.status === 0) { + console.log(' ok'); + } else { + console.error(' FAILED — see the message above (a conflicting config must be revoked first)'); + failed.push(name); + } + + console.log(''); + } + + console.log('Current configuration:'); + for (const name of packages) { + console.log(`==> ${name}`); + // A package with no config exits non-zero; that is reported above, and is + // not worth failing this summary over. + runNpm(['trust', 'list', name], { stdio: 'inherit' }); + } + + if (alreadyConfigured.length > 0) { + console.log(''); + console.log( + `${alreadyConfigured.length} package(s) already trusted ${REPO} / ${WORKFLOW}: ` + + `${alreadyConfigured.join(', ')}` + ); + } + + if (unpublished.length > 0) { + console.log(''); + console.log(`${unpublished.length} package(s) are not on npm yet: ${unpublished.join(', ')}`); + console.log('Publish each once by hand, then re-run this script. See README.md.'); + } + + if (failed.length > 0) { + console.error(''); + console.error(`${failed.length} package(s) were not configured: ${failed.join(', ')}`); + process.exit(1); + } + + console.log(''); + console.log('Done. Next steps:'); + console.log(' - Merge the release workflow and watch the first run publish with provenance.'); + console.log(" - Then, on npmjs.com, set each package's Publishing access to"); + console.log(" 'Require two-factor authentication and disallow tokens'."); +} + +try { + main(); +} catch (error) { + console.error(error.message); + process.exit(1); +} diff --git a/tools/release/setup-trusted-publishing.sh b/tools/release/setup-trusted-publishing.sh deleted file mode 100644 index 15d3a085..00000000 --- a/tools/release/setup-trusted-publishing.sh +++ /dev/null @@ -1,124 +0,0 @@ -#!/usr/bin/env bash -# -# One-time setup: register .github/workflows/release.yml as the npm trusted -# publisher for every package in this workspace. -# -# After this runs, the release workflow authenticates to npm with a short-lived -# OIDC token minted per run and scoped to that workflow — no NPM_TOKEN, and a -# leaked token from anywhere else cannot publish these packages. -# -# PREREQUISITES -# 1. npm >= 11.15.0: npm install --global npm@latest -# 2. Logged in interactively: npm login -# The account needs publish rights on @cornerstonejs and must have 2FA -# enabled. `npm trust` talks to an endpoint that only accepts a web-login -# SESSION token: a granular or classic access token sitting in ~/.npmrc -# fails with `401 ... Bearer token authorization is required`, even though -# the same token publishes fine. That is why this cannot run unattended. -# If you hit that 401, clear the stored token and log in again: -# npm logout # or delete the //registry.npmjs.org/:_authToken -# npm login # line from ~/.npmrc by hand if logout fails -# Note this replaces whatever token was in ~/.npmrc. -# -# Re-running is safe: npm allows exactly one publisher config per package, so a -# package that already has one is reported and skipped rather than duplicated. -# To replace an existing config, revoke it first: -# npm trust list -# npm trust revoke --id - -set -euo pipefail - -REPO="${REPO:-cornerstonejs/codecs}" -WORKFLOW="${WORKFLOW:-release.yml}" - -ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" - -# Read from the workspace rather than hardcoded, so the list cannot drift. A -# package added or renamed without a trusted publisher of its own does not fail -# here — it fails at `npm publish`, partway through a live release, after the -# version commit and tags have already landed on main. -# -# node rather than jq: npm is a prerequisite of this script, so node is -# guaranteed present and jq is not. -# -# Via a temp file rather than `mapfile < <(node ...)`: process substitution's -# exit status is invisible to `set -e`, and the scanner prints as it walks, so a -# manifest that fails to parse halfway through would leave mapfile holding the -# packages emitted before the throw — and this script would go on to report -# success having configured only some of them. -SCAN_OUTPUT=$(mktemp) -trap 'rm -f "$SCAN_OUTPUT"' EXIT - -node -e ' - const fs = require("fs"); - const path = require("path"); - const dir = path.join(process.argv[1], "packages"); - - for (const entry of fs.readdirSync(dir).sort()) { - const manifest = path.join(dir, entry, "package.json"); - if (!fs.existsSync(manifest)) continue; - const pkg = JSON.parse(fs.readFileSync(manifest, "utf8")); - if (pkg.private || !pkg.name) continue; - console.log(pkg.name); - } -' "$ROOT" >"$SCAN_OUTPUT" - -mapfile -t PACKAGES <"$SCAN_OUTPUT" - -if [ ${#PACKAGES[@]} -eq 0 ]; then - echo "No publishable packages found under $ROOT/packages." >&2 - exit 1 -fi - -require_npm_version() { - local current required="11.15.0" - current=$(npm --version) - - # Sort the two versions and check the required one comes first. - if [ "$(printf '%s\n%s\n' "$required" "$current" | sort -V | head -n1)" != "$required" ]; then - echo "npm $current is too old for \`npm trust\` (need >= $required)." >&2 - echo "Run: npm install --global npm@latest" >&2 - exit 1 - fi -} - -require_npm_version - -echo "Registering $REPO / $WORKFLOW as trusted publisher for ${#PACKAGES[@]} packages." -echo "The first package will prompt for your 2FA one-time password." -echo - -failed=() -for pkg in "${PACKAGES[@]}"; do - echo "==> $pkg" - if npm trust github "$pkg" \ - --repo "$REPO" \ - --file "$WORKFLOW" \ - --allow-publish \ - --yes; then - echo " ok" - else - echo " FAILED — see the message above (an existing config must be revoked first)" >&2 - failed+=("$pkg") - fi - echo -done - -echo "Current configuration:" -for pkg in "${PACKAGES[@]}"; do - echo "==> $pkg" - npm trust list "$pkg" || true -done - -if [ ${#failed[@]} -gt 0 ]; then - echo >&2 - echo "${#failed[@]} package(s) were not configured: ${failed[*]}" >&2 - exit 1 -fi - -echo -echo "Done. Next steps:" -echo " - Merge the release workflow and watch the first run publish with provenance." -echo " - Then, on npmjs.com, set each package's Publishing access to" -echo " 'Require two-factor authentication and disallow tokens', and delete" -echo " the old NPM_TOKEN from the CircleCI project." diff --git a/tools/release/version.mjs b/tools/release/version.mjs index d07d9de0..2b7af1a7 100644 --- a/tools/release/version.mjs +++ b/tools/release/version.mjs @@ -118,7 +118,7 @@ const RECORD_SEP = String.fromCharCode(0x1e); // git emits it as %x1e // Carried over verbatim from lerna.json's command.publish.ignoreChanges. That // key looked like it applied only to `lerna publish`, but VersionCommand // declares publish as an "other command config" and read it too — so this is -// what stopped a docs-only commit from releasing all eight packages. A commit +// what stopped a docs-only commit from releasing every package. A commit // counts as releasable only if it touches at least one path these globs do // NOT match. const IGNORE_CHANGES = ['*.md', '*.yml', '*.spec.js', '*.test.js'];