Skip to content

chore: move to pnpm 12 and publish with npm trusted publishing - #137

Merged
wayfarer3130 merged 14 commits into
masterfrom
chore/pnpm-12
Oct 8, 2026
Merged

wayfarer3130 merged 14 commits into
masterfrom
chore/pnpm-12

Conversation

@wayfarer3130

@wayfarer3130 wayfarer3130 commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This PR replaces bun and lerna with pnpm 12 as the package manager. It also adds a GitHub Actions workflow that publishes the packages with npm trusted publishing (OIDC). The setup follows Cornerstone3D and OHIF.

This PR depended on #136. #136 merged, and the base branch of this PR is now master.

User requirements

  • A developer installs, builds, and tests the repository with pnpm 12: pnpm install, pnpm run build, pnpm run test.
  • Bun stays the runtime. The CLIs (mkdicomweb, dicomwebserver, dicomwebscp, createdicomweb, deploydicomweb) and the oven/bun Docker service image do not change.
  • A merge to master publishes 9 @radicalimaging/* packages to npm at one new version. @radicalimaging/healthlakestore is private, and the release does not publish it.
  • The release reads each commit message after the last version tag, and uses the highest increase:
    • type!: or a BREAKING CHANGE: footer increases the major version.
    • A message that starts with feat increases the minor version.
    • Other messages increase the patch version.
  • Change after the review: the first version of this PR read only the last commit message, and it never increased the major version.
  • A merge to master needs one approved review. A release needs no other approval.
  • When npm does not get all packages of a version, a maintainer re-runs the failed run. If a change merged after the failed run, the maintainer starts a manual run of publish.yml on the version tag.
  • The repository holds no npm token. Each package gets a provenance attestation.
  • Internal dependencies keep their >= ranges, so a consumer still accepts later releases.
  • Only a run on master, or a manual run on a v* tag, gets the npm credentials and the deploy key. A workflow on another branch gets neither.
  • Change after the second review: a release of a new version stops when pnpm audit --audit-level=high finds an advisory that auditConfig.ignoreGhsas does not hold. Before, only CI ran the audit, and only when the lockfile changed. A recover run does not run the audit, because no commit can change the tag that the run publishes.
  • When a publish fails, a maintainer uses "Re-run failed jobs" on the failed run. After 30 days, or after "Re-run all jobs" stops with an error, the maintainer starts a manual run on the version tag.
  • Change after the second review: npm never holds a package of a release before the packages that the package needs. The packages publish in dependency order, and the first failure stops the publish.
  • Change after the second review: a new package, or a package that becomes public, does not block the release flow. The next release publishes the package.
  • pnpm audit --audit-level=high gives the same result in CI and on a local machine.

Implementation requirements

  • Package manager
    • packageManager is pnpm@12.9.1, the newest release that is older than the 2-day minimumReleaseAge.
    • The PR removes lerna, lerna.json, bun.lock, bunfig.toml, and bunfig.update-lockfile.toml.
    • The root scripts use pnpm -r.
    • Package build scripts use pnpm run and node -e in place of bun run and bun -e. The bun runtime commands, bun link for example, do not change.
  • pnpm-workspace.yaml
    • nodeLinker: hoisted keeps the node_modules layout that the jest configs and the Docker build expect.
    • linkWorkspacePackages: true links the packages, because they use >= ranges and not workspace:.
    • minimumReleaseAge: 2880, with minimumReleaseAgeExclude: ['@cornerstonejs/*']. One glob replaces the two duplicate lists in the bunfig files.
    • All overrides move here from package.json. Range overrides for brace-expansion fix the version that nx pins exactly, so the audit no longer needs those 3 ignores.
    • auditConfig.ignoreGhsas holds only GHSA-vfj7-8cjw-p6xm (braces), because no patched braces release exists.
    • allowBuilds: canvas and esbuild run their install scripts. core-js-pure does not.
  • Supply-chain scripts
    • scripts/ci-supply-chain.sh uses pnpm. It runs the audit when pnpm-lock.yaml or pnpm-workspace.yaml changes. FORCE_AUDIT=1 replaces FORCE_BUN_AUDIT=1. SKIP_AUDIT=1 skips the audit. The publish workflow uses SKIP_AUDIT=1, and runs the audit in its own step in release mode.
    • scripts/check-pinned-versions.sh also checks each override value in pnpm-workspace.yaml. The check uses an allowlist: each value must be an exact semver version. An override key can still select a range.
    • scripts/check-pinned-versions.sh accepts only >=<its version> or an exact version for a dependency on a workspace package, as publish-version.mjs does.
  • CI
    • .github/workflows/ci.yml uses pnpm/action-setup@v6.1.0, actions/setup-node@v6.4.0, and Node 24.15.0, as Cornerstone3D does.
    • The job names test and build do not change.
    • Both workflows pin each action by commit SHA, with the tag in a comment.
  • Publish
    • .github/workflows/publish.yml and scripts/release/* come from the Cornerstone3D release flow, with master in place of main.
    • Change after the second review: publish.yml has 4 jobs, and each credential goes only to the job that needs it. The workflow-level permissions is {}.
      • prepare (contents: read, no secret): install, supply-chain checks with FORCE_AUDIT=1, build, test, version commit, and npm pack. Only this job runs the scripts of the dependencies.
      • push (release environment): runs verify-version-commit.mjs, then pushes the version commit and the tag with RELEASE_DEPLOY_KEY.
      • publish (npm-publish environment, id-token: write): checks out the tag, and publishes the tarballs with npm publish <tarball> --provenance and npm 11.19.0. The job installs no dependency of the repository.
      • github-release (contents: write): creates the GitHub release.
    • A git bundle and an artifact carry the version commit and the tarballs from prepare to the other jobs.
    • The publish workflow restores no cache, so a cache that another workflow wrote does not reach the release.
    • verify-version-commit.mjs uses only Node built-ins. The script accepts a version commit only if all of these are true:
      • The tag is v<new version>.
      • The new version is the version that the commits of the parent give. The push job computes that version again with release-type.mjs, so the prepare job cannot choose the version.
      • Each pair of specifier: lines in the lockfile keeps the same >= prefix.
      • The commit changes only version to the new version, and the ranges of the internal dependencies to [>=]<new version>.
      • In pnpm-lock.yaml, the commit changes only the specifier: lines of the internal dependencies.
      • A changed resolved version, a changed integrity hash, a changed third-party specifier, or a changed script fails the check.
    • publish-package.mjs publishes only the tarballs that match the publishable packages of the tag.
      • The script checks every tarball before the first npm publish. A failed check therefore leaves npm with no part of the version.
      • Each entry of a tarball must be a file or a directory under package/, in its normal spelling, and must occur once, in any case. npm drops the first directory of each entry and keeps the last copy of a path. So a second top-level directory, or a second spelling such as package/./package.json, can replace package/package.json.
      • A binding.gyp or an npm-shrinkwrap.json must also be in the tag. npm runs node-gyp for the first file at install, and installs the dependency tree of the second file.
      • No entry may be in node_modules/, because no package of this repository bundles its dependencies.
    • The publish job checks the tag output of the prepare job again. The tag must name GITHUB_SHA, or the version commit on top of GITHUB_SHA.
      • The script compares the package.json inside each tarball with the tag: name, version, the entry points, imports, bin, directories, files, scripts, gypfile, publishConfig, and the dependencies.
      • The script cannot check the other files of a tarball, for example a build output.
    • current-version.mjs reads VERSION_SOURCE for the workflow.
    • "Re-run all jobs" of a run that pushed its version commit stops with an error when master moved after the push. Before, the run did nothing and passed.
    • npm publish writes its output to the job log, so the log shows the provenance link.
    • The release artifact stays for 30 days. After that time, a re-run of the publish job fails, and a manual run on the tag is necessary.
    • version.mjs reads the commits in v<current>..HEAD. Change after the second review: if the tag of the current version does not exist, the script stops with an error. The git log -S fallback is gone, because the tag v1.7.6 now exists.
    • version.mjs stops before the push when the next tag exists already, or when npm holds a package at the next version already. Before, the publish skipped every package, and the run passed with nothing published.
    • v1.7.6 points to 83dde1c, the commit that published 1.7.6. That commit is not on master, so v1.7.6..HEAD starts at the merge base b8bbbd9, and version.mjs gives a warning.
    • publish-version.mjs stages only pnpm-lock.yaml and the package.json files, so build output and test output do not go into the release commit.
    • publish-version.mjs stops when an internal dependency range is not >=<version> or an exact version.
    • The push job pushes the version commit and the tag in one atomic push, before the publish job starts. When git reports a failure, the job reads master and the tag on GitHub:
      • GitHub holds the version commit and the tag: the push succeeded, and the publish continues with a warning.
      • GitHub holds the tag at another commit: the error names that tag.
      • master moved: the error names a race.
      • Otherwise, the error names the deploy key and the rulesets. An empty RELEASE_DEPLOY_KEY stops the job with its own error.
    • The deploy key push starts workflows. [skip ci] in the version commit message stops a release loop. The check of the chore(release): publish subject is a fallback.
    • The workflow gives the tag to the shell through env: TAG, and not through an expression in the script.
  • Recovery
    • A recover mode publishes the version that HEAD already carries when npm does not hold that version. The mode takes no new version.
    • Change after the second review: release-mode.mjs checks only the packages of the last release: the packages that carry <current> now, and that are publishable at the tag v<current>. The version at the tag does not count, because lerna left 8 packages at 1.7.4 at the tag v1.7.6.
    • release-mode.mjs checks npm again, up to 6 times at 60 s intervals, before it gives recover. The registry CDN can answer from a copy that is up to 300 s old, so a run right after a publish can see the new version as missing.
    • The step "Check that this run holds the tagged commit" is the only guard that stops a run for a later merge from publishing a tag. npm does not compare the commit of the provenance attestation.
    • A recover run publishes only when HEAD is the tagged commit, so npm gets the code of the tag and the provenance attestation names that commit.
    • A re-run of a failed run continues from its own version commit, while master holds that commit.
    • A manual run (workflow_dispatch) on a v* tag runs in recover mode. master must hold the tagged commit, and the tag name must be the same as the version in that commit.
    • A run for a later merge does not publish the missing version. The run fails, and the error names the tag for the manual run.
  • GitHub environments (change after the second review): the review of each merge to master is still the release gate, and the environments have no required reviewers. The environments limit the branches and the tags that get the credentials.
  • Package metadata
    • Each package.json now has repository.url git+https://github.com/RadicalImaging/Static-DICOMWeb.git and the correct directory. npm provenance checks this URL.
    • Before this PR, s3-deploy pointed to Radical/static-dicomweb. cs3d, healthlakestore, and static-wado-webserver had the wrong directory.
  • healthlakestore: the package is now "private": true, so the release scripts skip the package, and npm refuses a publish of the package. The version stays 1.6.5. Every other package carries 1.7.6, and npm holds 1.7.6 for each of them, so the first publish run is a normal release run.
  • Dockerfiles: the builder stages install with pnpm install --frozen-lockfile. The installer stage and the runtime stage do not change.
  • Documentation: the README now gives the pnpm commands for a source build.

Setup before the first release

An admin completed all steps on 2026-10-08.

  1. npm trusted publisher: the 9 published @radicalimaging/* packages name RadicalImaging/Static-DICOMWeb and publish.yml as the trusted publisher.
  2. Branch ruleset: a ruleset on master replaces the classic protection. The ruleset requires 1 approving review, and it blocks force pushes and branch deletion. Deploy keys and repository admins can bypass the ruleset.
  3. Deploy key: the deploy key "release (publish.yml, release environment)" has write access. The secret RELEASE_DEPLOY_KEY of the release environment holds the private key. The repository has no RELEASE_DEPLOY_KEY secret. The admin replaced the first key, which was a repository secret. A repository ruleset cannot name the GitHub Actions app as a bypass actor, so GITHUB_TOKEN cannot push to master.
  4. Environments: npm-publish allows the branch master and the tags v*. release allows the branch master only.
  5. Tag ruleset: the release-tags ruleset stops the creation, the update, and the deletion of refs/tags/v*. Deploy keys and repository admins can bypass the ruleset. Without this ruleset, a user with write access can push a v* tag on a branch and get past the npm-publish environment.
  6. Environment in the npm trusted publisher: the trusted publisher of each of the 9 packages names the environment npm-publish. The entries without an environment are gone, so npm accepts an OIDC token of publish.yml only from a job in that environment.
  7. No token publish: each of the 9 packages has "Require two-factor authentication and disallow tokens" (npm access set mfa=publish). Trusted publishing still works with this setting.

Test

  • pnpm install --frozen-lockfile passes.
  • FORCE_AUDIT=1 bash scripts/ci-supply-chain.sh exits with code 0.
  • bash scripts/check-pinned-versions.sh exits with code 0. The override check rejects >1.0.0, <2, 1.x, 1 - 2, and 1.0.0 || 2.0.0.
  • pnpm run build passes.
  • pnpm run test passes in all packages, with the same test counts as under bun.
  • The CLIs start under bun.
  • docker build . builds the x64 image, and mkdicomweb --help runs in the image.
  • A dry run of version.mjs gives 1.7.7 from 18 commits, with the tag v1.7.6.
  • pnpm run test:release passes 4 tests. The tests cover the bump rules, the publish order, verify-version-commit.mjs, and the check of the tarball entries.
  • verify-version-commit.mjs refused a version commit for v9.9.9, because the commits give 1.7.7.
  • version.mjs stopped when the tag v1.7.7 existed, and gave 1.7.7 without that tag.
  • publish-package.mjs refused a static-wado-webserver tarball with an added npm-shrinkwrap.json. That package is the last in the publish order, and the script published nothing.
  • The "Check the tag" step passed for a release run and for a tag run, and refused a tag on another commit and a tag name that is not a version.
  • actionlint 1.7.12 finds no problem in publish.yml and ci.yml.
  • In a scratch clone, publish-version.mjs made a real version commit for 1.7.7, and verify-version-commit.mjs accepted the commit. The commit went through a git bundle into a depth-1 clone of the parent, as in the push job, and the check passed there too. A version commit with an added postinstall script failed the check.
  • In the scratch clone, pack-packages.mjs packed the 9 packages. publish-package.mjs with --dry-run published them in dependency order, with cs3d first and static-wado-webserver last. An extra tarball in manifest.json stopped the script. A tarball with an added postinstall script in its package.json stopped the script.
  • verify-version-commit.mjs refused the real version commit under the tag v9.9.9. The script also refused a lockfile line +++ evil: injected in a hunk, and a version that is not above the current version.
  • publish-package.mjs refused a tarball with a second top-level directory zz/ that holds a package.json with a postinstall script.
  • release-mode.mjs at 1.7.6 gives release. With healthlakestore made public, the result is still release.
  • In a container with jq, check-pinned-versions.sh rejects workspace:*, ^1.7.6, and >=1.7.5 on an internal dependency, and accepts 1.7.6.
  • I did not build arm-Dockerfile, and I did not run publish.yml. The first merge to master after this PR is the first test of the push with the deploy key and of the recovery paths.

🤖 Generated with Claude Code

wayfarer3130 and others added 4 commits October 6, 2026 11:30
…h audit findings

- @cornerstonejs/core 4.22.3 -> 5.11.4 (adds metadata/utils peers),
  dicom-codec 1.1.6, codec-openjph 2.4.11, codec-openjpeg 1.3.6
- dcmjs 0.50.1 -> 0.52.0
- Fix high/critical bun audit findings: aws-cdk-lib 2.272.0 (cli 2.1144.0),
  ws 8.22.0, adm-zip 0.6.1, overrides for axios, fast-uri, js-yaml, pacote,
  smol-toml, tar; re-resolve other transitive deps within their ranges
- cs3d: compile with module node20 so it can import the ESM-only core 5 types
- jest: transform gl-matrix, which core 5 installs nested
- bunfig: pin linker = "hoisted" (bun.lock configVersion 1 defaults to
  isolated) and apply minimumReleaseAge to install:update-lockfile

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 5.11.5

Bun does not accept a scope wildcard in minimumReleaseAgeExcludes, so the
excludes list each @cornerstonejs package in the dependency tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fixable audit advisories

- Override http-cache-semantics to 4.3.0 to clear the lerna advisory.
- Ignore four dev-only DoS advisories in the CI audit step: braces has no
  patched release, and nx pins brace-expansion 5.0.8 exactly.
- Bump constructs to 10.8.1 to meet the aws-cdk-lib peer range ^10.5.0.
- Note in both bunfig files that the Cornerstone3D exclude lists must match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed publishing

- Replace bun and lerna with pnpm 12.9.1 for install, lockfile, scripts and audit.
  Bun stays the runtime for the CLIs and the Docker service image.
- pnpm-workspace.yaml holds the overrides, the release-age rule (with a
  @cornerstonejs/* exclude), allowBuilds and auditConfig.ignoreGhsas. Range
  overrides fix brace-expansion, so only braces (no patched release) is ignored.
- CI uses pnpm/action-setup and Node 24.15.0, as Cornerstone3D does.
- Add publish.yml and scripts/release/*, adapted from the Cornerstone3D release
  flow: version from the last commit message, atomic push of the version commit
  and tag, then npm publish --provenance with OIDC.
- Fix repository url/directory in each package.json so provenance matches.
- Align healthlakestore to 1.7.6 with the other packages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base automatically changed from chore/update-versions to master October 8, 2026 15:48
wayfarer3130 and others added 3 commits October 8, 2026 11:49
Take #136 (squash of the branch this one builds on). Keep the pnpm side of the
bun-to-pnpm conflicts: drop bun.lock and the bunfig files, keep the pnpm audit step.
Carry over the review fixes: the scp bin scripts become 100755, and the braces
ignore comment in pnpm-workspace.yaml says the advisory is unreachable, not dev-only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ruleset can allow it

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mark @radicalimaging/healthlakestore private so the release scripts skip it,
and restore its version to 1.6.5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wayfarer3130
wayfarer3130 marked this pull request as ready for review October 8, 2026 16:01
@wayfarer3130
wayfarer3130 requested a review from rleisti October 8, 2026 16:01

@rleisti rleisti left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Nice work overall — the "push before publish" ordering, the atomic push, the E404-vs-other-failure distinction in isPublished, and pinning npm for OIDC are all good calls. A few things need attention before this goes live, mostly around the failure/recovery paths, since npm can't take a version back.

🚫 Blockers

1. The failure-recovery path publishes the wrong code (or nothing).
Two issues combine here:

  • Re-run is a silent no-op. When publish-package.mjs fails partway, it tells the operator to "Re-run this workflow." But a re-run checks out the original GITHUB_SHA, while master now points at the pushed chore(release): publish commit. The tip check (publish.yml:61) sets proceed=false, every step is skipped, and the run goes green with nothing published.
  • Recover mode builds the wrong commit. If the gap is instead healed by the next merge (the other path the header comment describes), recover mode builds and tests the new merge's code and publishes it under the old version (publish.yml:106). Result: npm 1.7.7 holds commit B's code, tag v1.7.7 points at commit A, the provenance attestation names commit B, and B's changes never get a version of their own.

Suggested fix: in recover mode, check out the commit the version tag points at (git checkout "v$VERSION") before building and publishing. Then either let the tip check pass when FETCH_HEAD is the release commit whose parent is HEAD, or change the error message to say "start a manual run (workflow_dispatch) on master" instead of "re-run".

⚠️ Should fix

2. Consider gating the npm publish with a GitHub environment (publish.yml:33).
id-token: write covers the whole job, so dependency code that runs during build and test could use the trusted-publishing token. Splitting into a minimal publish job narrows that a little. But neither change helps against a malicious merged PR, which can edit the workflow itself. A stronger control: put the publish in an npm-publish environment with required reviewers and a master-only branch rule, and name that environment in each package's trusted publisher config on npmjs.com. Then npm only accepts tokens from approved runs, and that gate lives in repo settings, not in code a PR can change.

3. The bump type comes only from the tip commit's message (scripts/release/version.mjs:21).
This replaces lerna's conventional-commits analysis of everything since the last tag. Failure cases:

  • A feat lands, then a fix lands before the first run reaches the tip → the tip run releases both as a patch.
  • Merge commits ("Merge pull request #…") or non-conventional titles always give a patch.
  • feat!: / BREAKING CHANGE: gives a minor, never a major.

Suggest scanning git log v<current>..HEAD and taking the highest bump, or at least handling ! / BREAKING CHANGE.

4. The tag output is put straight into the shell (publish.yml:154, and the same in the "Create the GitHub release" step).
TAG='${{ steps.release.outputs.tag }}' puts a value from package.json straight into the script, in a job that holds the deploy key and the OIDC token. In recover mode, a crafted version string in a merged PR could break out of the quotes. Low likelihood (it needs a merged PR), but the fix costs nothing:

env:
  TAG: ${{ steps.release.outputs.tag }}

5. git add -A packages can commit build or test output (scripts/release/publish-version.mjs:75).
This runs right after build and test, so any file they write under packages/<pkg>/ that .gitignore doesn't cover ends up in the release commit. That commit is pushed to protected master with the deploy key, skipping review. Stage only what the script changed: the package.json files and pnpm-lock.yaml.

6. The header comment is wrong about what prevents a release loop (publish.yml:17).
It says GITHUB_TOKEN pushes the version commit, so the workflow can't re-trigger itself. The push actually uses RELEASE_DEPLOY_KEY, and deploy-key pushes do trigger workflows. The only loop guard is [skip ci] in the commit message (with the chore(release): publish subject check as a fallback). Please fix the comment so nobody removes [skip ci] thinking it isn't needed.

💬 Nits

7. The override check lets some ranges through (scripts/check-pinned-versions.sh:83).
is_pinned only rejects ^, ~, *, >= and <=, so overrides like >1.0.0, <2, 1.x, 1 - 2 or 1.0.0 || 2.0.0 pass as "exact", even though pnpm-workspace.yaml says the script enforces exact values. An allowlist regex for exact semver would be stricter than a denylist.

8. Non->= internal ranges are silently pinned to exact (scripts/release/publish-version.mjs:49).
Every internal dependency uses >= today, so this is latent. But a future workspace:* (which check-pinned-versions.sh allows) or ^1.7.6 would quietly become "1.7.7". Consider throwing on any unexpected prefix instead.

9. Registry lookups run one at a time, twice per release (scripts/release/workspace-packages.mjs:78).
findUnpublished and the loop in publish-package.mjs each run one npm view per package in sequence. Promise.all in findUnpublished is an easy win.

- recover mode builds the tagged commit, and a re-run continues from the
  version commit of the run instead of skipping every step
- the bump type comes from every commit since the last tag, and `type!:`
  or `BREAKING CHANGE:` gives a major
- the tag reaches the shell through `env`
- the release commit stages only the manifests and pnpm-lock.yaml
- the header comment names `[skip ci]` as the guard against a release loop
- overrides must be exact semver (allowlist), unexpected internal ranges
  stop the release, and the registry checks run in parallel

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wayfarer3130

Copy link
Copy Markdown
Collaborator Author

Thank you for the review. Commit 754851e changes points 1, 3 to 9. Point 2 does not change. The reason is below.

1. Failure recovery (blocker)

  • A re-run now continues the release. The tip check accepts a tip of master that is a chore(release): publish commit when the parent of that commit is HEAD. The run then checks out that commit, and the run goes to recover mode.
  • recover mode now checks out the tag v$VERSION before the build, the test, and the publish. npm gets the code of the tag. If the tag does not exist, the run fails and tells the operator to create the tag.
  • The error message in publish-package.mjs still says "Re-run this workflow", because a re-run now works.
  • One limit stays: the provenance attestation names GITHUB_SHA, because the OIDC token carries that commit. A re-run gives the parent of the version commit. A run for a later merge gives that later commit. In that case, the workflow writes a warning that names both commits.

3. Bump type

version.mjs now reads all commit messages in v<current>..HEAD and uses the highest bump:

  • type!: or a BREAKING CHANGE: footer gives a major.
  • feat gives a minor.
  • Other messages give a patch.

If the tag of the current version does not exist, the script stops with an error.

4. Tag in the shell

The push step and the release step now get the tag through env: TAG.

5. git add -A packages

The release commit now stages only pnpm-lock.yaml and the package.json file of each package.

6. Header comment

The comment now says that a push with the deploy key starts workflows. The comment also says that [skip ci] stops a release loop, and that the subject check is a fallback.

7. Override check

check-pinned-versions.sh now uses an allowlist for override values. A value must be an exact semver version. The check rejects >1.0.0, <2, 1.x, 1 - 2, and 1.0.0 || 2.0.0.

8. Internal ranges

publish-version.mjs now stops the release when an internal dependency range is not >=<version> or an exact version.

9. Registry lookups

findUnpublished now runs the npm view calls in parallel with Promise.all.

2. GitHub environment for the publish

We do not add an environment with required reviewers. The master ruleset requires one approved review for each merge. Each release comes from a merge to master, so a second approval for each release only repeats that gate. If the ruleset stops the requirement for a review, we will add the environment.

Test

  • bash scripts/check-pinned-versions.sh exits with code 0.
  • node --check passes for each script in scripts/release/.
  • A dry run of version.mjs gives 1.7.7 from 16 commits after v1.7.6.

I did not run publish.yml. The first merge to master is the first test of the new recovery paths.

🤖 Generated with Claude Code

@rleisti rleisti left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 754851e

Thanks — items 1, 3, 4, 5, 6, 7, 8 and 9 are resolved. I checked the override fix (>1.0.0 is now rejected and the current overrides pass) and ran the new bump logic on sample messages; both behave as intended.

🚫 Blocker (new)

The first release after merge will fail: there is no v1.7.6 tag.
version.mjs now throws when v<current version> doesn't exist, and the newest v* tag on the remote is v1.5.0 (lerna made per-package tags). The failure is harmless (it happens before any push or publish), but nothing will release until someone creates the tag. Please either push v1.7.6 on the commit that was published as 1.7.6 before merging (the first master commit carrying 1.7.6 is 7b39944 / #124; can you confirm that's the published one?), or fall back to the last commit that changed packages/create-dicomweb/package.json's version when the tag is missing.

⚠️ Should fix

Item 2 (still open): gate the publish with a GitHub environment.
id-token: write still covers the whole job. Rather than only splitting jobs, put the publish in an npm-publish environment with required reviewers and a master-only branch rule, and name that environment in each package's trusted publisher config on npmjs.com. npm then only accepts tokens from approved runs, and the gate lives in repo settings, where a PR can't change it.

Recover mode from a later merge attests the wrong commit.
When merge B heals a missing version, the job builds tag A, but the provenance attestation names B (publish.yml, "Check out the tagged commit"). The warning is good, but the published attestation is still wrong. Suggest allowing workflow_dispatch on refs/tags/v* for recover mode only, so GITHUB_SHA is the tagged commit. The next-merge run could then stop with an error telling the operator to start a manual run on the tag instead of publishing.

💬 Nit

publish-version.mjs now correctly rejects workspace:* / ^ on internal dependencies, but check-pinned-versions.sh still accepts workspace:*. So that mistake passes CI and only fails at release time, after merge. Consider running the same check in check-pinned-versions.sh.

- version.mjs falls back to the last commit that set the current version
  when no tag names it, so the first release does not need a v1.7.6 tag
- recover mode publishes only from a run that holds the tagged commit: a
  re-run, or a manual run on a v* tag that master holds. A run for a later
  merge fails and names the tag, so the attestation names the right commit
- check-pinned-versions.sh accepts only >=<version> or an exact version for
  a workspace package, as publish-version.mjs does

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wayfarer3130

Copy link
Copy Markdown
Collaborator Author

Thank you for the re-review. Commit 6609c66 changes the blocker, the attestation point, and the nit. Item 2 does not change.

Blocker: no v1.7.6 tag

  • version.mjs now uses the tag v<current> when the tag exists. When the tag does not exist, the script uses the last commit that wrote "version": "<current>" to packages/create-dicomweb/package.json. The script writes a warning that names that commit.
  • For 1.7.6, the fallback finds 7b39944 (Fix/audit 20260429 #124). A dry run without the tag gives 1.7.7 from 16 commits.
  • About your question: 7b39944 is not the published commit. npm published 1.7.6 at 2026-04-28T20:20:53Z. lerna made the version commit 83dde1c on top of b8bbbd9 (fix: Exports of type and some additional utilities #123), and 83dde1c never went to master. 7b39944 changed only package.json files. The fallback range therefore leaves out Fix/audit 20260429 #124 itself. Fix/audit 20260429 #124 is an audit fix, so the bump is a patch in all cases.
  • After the first release, the workflow pushes a v* tag for each version, so the fallback is necessary only one time.

Recover mode attests the wrong commit

I used your suggestion:

  • A manual run (workflow_dispatch) on a v* tag now runs in recover mode. master must hold the tagged commit, and the tag name must be the same as the version in that commit. Otherwise the run fails.
  • A run for master in recover mode now publishes only when HEAD is the tagged commit. A run for a later merge fails. The error names the tag and tells the operator to start a manual run on that tag.
  • The run no longer checks out a tag after the install, so the run builds and attests the same commit.
  • A re-run of the failed run still continues from its own version commit. That run attests the parent of the version commit. A normal release attests the same commit, because the push comes before the publish.

Nit: workspace:* passes CI

check-pinned-versions.sh now accepts only >=<its version> or an exact version for a workspace package. This is the rule of publish-version.mjs. I ran the check with jq in a container on a copy of the manifests:

  • The current tree passes.
  • workspace:*, ^1.7.6, and >=1.7.5 on @radicalimaging/cs3d fail.
  • 1.7.6 passes.

Item 2: GitHub environment

We keep the decision from the last reply. The master ruleset requires one approved review for each merge, and each release comes from a merge to master. We do not want a second approval for each release. If the ruleset stops the requirement for a review, we will add the npm-publish environment.

🤖 Generated with Claude Code

wayfarer3130 and others added 3 commits October 8, 2026 13:21
- prepare installs, checks the supply chain (with the audit), builds, tests,
  makes the version commit and packs the tarballs, with no credential.
- push checks the version commit and pushes it with the deploy key of the
  `release` environment. A failed push now says why it failed.
- publish publishes the tarballs in dependency order from the `npm-publish`
  environment, the only job with an OIDC token. The first failure stops it.
- Actions are pinned by commit, and the publish workflow restores no cache.
- release-mode counts only the packages of the last release, so a new or
  re-published package no longer blocks every release.
- version.mjs needs the tag of the current version.
- Tests for the bump rules, the publish order and the version commit check.

Addresses #139.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- github-release runs after a recover run, where push is skipped.
- publish uses !cancelled(), so a cancel stops it.
- The release artifact can be uploaded again, and stays for 30 days.
- verify-version-commit.mjs takes the tag, needs a version above the
  current one, and accepts only the lockfile specifiers of the packages
  of the release.
- publish-package.mjs compares the install fields of the package.json in
  each tarball with the tag, and shows the npm output again.
- release-mode.mjs no longer needs the version at the tag, because lerna
  left most packages of v1.7.6 at 1.7.4 there.
- One list of dependency types for the release scripts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- publish-package.mjs rejects a tarball entry outside package/, a link, a
  duplicate entry, and a binding.gyp that the tag does not hold. It also
  compares imports, directories, gypfile and publishConfig.
- Only a new version runs the audit. A recover run publishes a tag that no
  commit can change, so a later advisory must not block it.
  ci-supply-chain.sh gets SKIP_AUDIT for this.
- The lockfile check skips the diff header only before the first hunk.
- "Re-run all jobs" of a run that pushed its version commit fails, and
  names "Re-run failed jobs", when master moved since.
- current-version.mjs reads VERSION_SOURCE for the workflow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rleisti

rleisti commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Re-review of 59805c3 + 21e9c8e

The job split is a solid redesign, and all the earlier items are addressed. I ran the release scripts in a scratch clone (no push, no publish): the release tests pass, version.mjs gives 1.7.7, publish-version.mjs produces a commit that verify-version-commit.mjs accepts, and npm pack (npm 11.19.0) produces a package.json that matches the source in every install field for all 9 packages. The environments, deploy key and rulesets are set up as the workflow expects.

⚠️ Should fix

A new advisory can make a failed publish impossible to recover (publish.yml:127).
prepare always runs pnpm audit (FORCE_AUDIT: '1') before deciding the mode. If a publish fails and a new high advisory appears before the manual run on the tag, that tag run fails the audit every time, because the tagged code can't change. Every master run also fails on purpose until npm holds the tag, so a fix on master can't get things moving again. Suggest skipping the audit in recover mode (that code was audited when it was tagged), e.g. move the audit after "Choose the mode" and run it only when mode == 'release'.

💬 Nits

  • prepare restores no cache, so it depends on canvas's prebuilt binary download. When that times out, the fallback node-gyp build fails for lack of pixman/cairo. That's the same failure as the red build check on 59805c3. It fails safely (before the push), but installing libcairo2-dev libpango1.0-dev libjpeg-dev libgif-dev librsvg2-dev libpixman-1-dev would make releases less flaky.
  • Optional: npm-publish limits branches but has no required reviewers. Fine if a per-release approval isn't wanted.

Before merging

Please confirm that each package's trusted publisher on npmjs.com names both publish.yml and the npm-publish environment. Without the environment, npm doesn't enforce the environment gate.

- publish-package.mjs rejects a tarball entry that is not in its normal
  spelling (package/./package.json, package//package.json), because npm
  keeps the last copy of a path. findEntryProblems has a test.
- version.mjs stops before the push when the next tag exists, or when npm
  holds the next version, so a run cannot end green with nothing published.
- The push job computes the next version again from the commits, so the
  build job cannot choose the version. release-type.mjs holds the shared
  computation, with Node built-ins only.
- The lockfile check needs the same `>=` prefix on both specifier lines.
- release-mode.mjs checks npm again for up to 6 minutes before `recover`,
  because the registry CDN can answer from a copy up to 300 s old.
- After a failed push, the job reads master and the tag on GitHub. A push
  that GitHub took continues, and an existing tag gets its own error.
- The comments name the tagged-commit step as the guard, not provenance.
- readCurrentVersion replaces three reads of VERSION_SOURCE, and
  version.mjs no longer uses execa or semver.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rleisti

rleisti commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Re-review of 3460776 + 6349eca

All set from my side. The audit now runs only for a new version, so a later advisory can't block a recover run. The other hardening looks good too: the push job recalculates the version itself, version.mjs stops when the next version already exists, release-mode.mjs re-checks npm for its CDN delay, and the tarball path and field checks are new.

I re-ran the flow in a scratch clone (no push, no publish): the release tests pass, version.mjs and the push job's recalculation both give 1.7.7, verify-version-commit.mjs accepts the generated commit, and the new tarball checks pass on all 9 packages packed with npm 11.19.0.

Remaining before merge: please confirm each package's trusted publisher on npmjs.com names both publish.yml and the npm-publish environment.

Optional nit, still open: if canvas's prebuilt binary download times out, the release fails at the node-gyp fallback. It fails safely before the push and a re-run fixes it, but installing the cairo/pango/pixman dev packages in prepare would remove the flakiness.

- publish-package.mjs checks every tarball before the first publish, so a
  failed check cannot leave npm with part of a version.
- The tarball check rejects an npm-shrinkwrap.json that the tag does not
  hold, any node_modules/ entry, and entries that differ only in case.
- The publish job checks the tag output of the prepare job again: the tag
  must name the commit of the run, or its version commit.
- The recover errors tell the operator to re-run the master runs that
  failed while npm lacked the version.
- RegExp.escape replaces the local escape helper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wayfarer3130
wayfarer3130 merged commit 73f0cc4 into master Oct 8, 2026
2 checks passed
@wayfarer3130
wayfarer3130 deleted the chore/pnpm-12 branch October 8, 2026 18:53
wayfarer3130 added a commit that referenced this pull request Oct 8, 2026
Take master's tree (the squash merge of #137 and the v1.7.7 release) and keep only the Docker changes of this branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants