chore: publish as @cornerstonejs/jpeg-lossless-decoder-js - #1
Merged
Conversation
This fork holds the byte-aligned-end-of-scan fix, and npm does not have the fix in any package. This commit prepares the fork for a publish under the @cornerstonejs scope, and it adds a release workflow. Manifest: - Rename the package to @cornerstonejs/jpeg-lossless-decoder-js, and set the version to 2.2.0. - Point repository.url, bugs.url and homepage at this fork. npm generates the provenance attestation from repository.url, so the URL must name the repository that publishes the package. - Add a files array. The repository has no .npmignore, so npm fell back to .gitignore, and .gitignore excludes *.cjs.map, *.js.map, *.d.ts and *.d.cts. Every publish therefore shipped no source maps and no type declarations, and it also shipped tests/data -- about 6 MB of DICOM fixtures, one file of 4.7 MB. The tarball is now 64.5 kB, and it holds the maps and the declarations. - Add publishConfig.access=public, which a scoped package needs. - Add a prepublishOnly script, so a manual publish always builds first. - Remove @rollup/rollup-linux-x64-gnu from optionalDependencies. It is a build tool, and every consumer of this package installed it. - Refresh package-lock.json. The committed lockfile named version 2.1.0 while package.json named 2.1.2, and its resolutions predate the manifest ranges, so `npm ci` cannot use it. A partial update is not possible. `npm ci`, the lint, the build and the 54 tests all pass on the new lockfile, and the build emits a byte-identical release/cjs/lossless.cjs. Build output: - Stop tracking release/. A committed release/ could never be complete, because .gitignore excludes the maps and the declarations that the same build emits. Commit 03bb80c shows the cost: its committed release/cjs/lossless.cjs is byte-identical to published 2.1.2, so the pinned commit carried a bundle without the fix that the same commit added to src/. A fresh build of that commit gives the correct bundle. Release workflow: - Add .github/workflows/release.yml. It publishes on a push to main, and only when the version in package.json is not on the registry. npm auth is OIDC trusted publishing, so the repository holds no NPM_TOKEN. - The build job installs the dependencies and holds no id-token permission. The publish job holds id-token: write, installs nothing, and publishes the release/ artifact that the build job made. No install script of a dependency runs beside the OIDC token. - npm cannot create a package that does not exist through trusted publishing, so a maintainer must do the first publish by hand. README.md gives the steps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`npm audit` reported 22 advisories on this repository: 1 critical, 16 high and 5 moderate. Every one of them is in a devDependency, and the count is the same on the lockfile that this branch replaced, so the advisories are older than this branch. The published package declares no runtime dependency, so no advisory reaches a consumer. The exposure is CI and local development. This commit takes the count to 3 moderate, 0 high and 0 critical, and the build output does not change. - Add `overrides.tsup.esbuild` at `0.19.7`. esbuild emits the bundle, so a different esbuild emits different JavaScript for the same source: a plain `npm audit fix` moved release/cjs/lossless.cjs from 32,684 to 32,590 bytes. The pin holds the artifact still while the test tools move. - Raise vitest from ^1.4.0 to ^4.1.11. vitest 1.x carries the critical advisory (GHSA-9crc-q9x8-hgqq, GHSA-5xrq-8626-4rwp), and it drags the vulnerable vite, rollup, postcss and nanoid versions with it. vitest 5.0.0 needs @types/node ^22 || >=24, and that raise breaks the declaration build, so 4.1.11 is the highest version that needs no source change. - Hold typescript at ~5.4.3 rather than ^5.4.3. A full lockfile resolve floats it to 5.7 or later, where `ArrayBufferLike` stops satisfying `ArrayBuffer` and `src/decoder.ts` line 93 fails to compile. The declaration build fails with it. - Raise the test matrix to node 20.x, 22.x and 24.x. vitest 4 needs node ^20 || ^22 || >=24, so 18.x and 16.x can no longer run the tests. Node 16 reached end of life in 2023-09 and node 18 in 2025-04. - Use `npm ci` in the test workflow, so it installs the same way release.yml does. The 3 remaining moderate advisories are all one esbuild development-server issue (GHSA about a website reaching the esbuild dev server). This repository never starts that server; it calls esbuild through tsup for one build. The advisory stays only because the esbuild pin above is what keeps the published bundle reproducible. Verified after a clean `npm ci`: the lint passes, the declaration build succeeds, all 54 tests pass, and release/cjs/lossless.cjs and release/cjs/lossless.cjs.map stay byte-identical to the files that cornerstonejs/codecs vendors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
The advisories that CI reported
Two facts first
The changes
Why esbuild is pinnedA plain Why TypeScript is held at 5.4A full lockfile resolve floats TypeScript to 5.7 or later, where The declaration build fails with it, so the package would ship no types. To Why vitest 4 and not vitest 5vitest 5.0.0 needs The 3 remaining moderate advisoriesAll three are one esbuild advisory: a website can send requests to the esbuild Verified after a clean
|
The previous commit changed the test workflow to `npm ci`, and the node 20.x
leg then failed:
npm error code EBADPLATFORM
npm error notsup Unsupported platform for @esbuild/aix-ppc64@0.28.2:
npm error wanted {"os":"aix","cpu":"ppc64"} (current: {"os":"linux","cpu":"x64"})
npm 11 writes package-lock.json, and npm 10 cannot read what it writes: `npm
ci` tries to install esbuild's optional per-platform packages instead of
skipping the ones that do not match the host. Node 20 ships npm 10.8.2 and
node 22 ships npm 10.9.8, so both legs need npm 11. Node 24 already bundles
npm 11.19.0, which is why release.yml pins node 24.20.0 and passes.
The lockfile is not at fault. A lockfile that npm 11 regenerates from scratch
inside a linux container, with no node_modules present, is byte-identical to
the committed one. `npm ci` then succeeds under npm 11 and fails under npm 10
with that same file.
Verified in a linux container on node 20, 22 and 24: `npm ci`, the build, the
lint and all 54 tests pass on each version, and release/cjs/lossless.cjs stays
byte-identical to the file that cornerstonejs/codecs vendors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
CI is green on node 20.x, 22.x and 24.xRun The node 20.x leg failed once, and the cause was the change to npm 11 writes
The lockfile is correct. A lockfile that npm 11 regenerates from nothing,
🤖 Generated with Claude Code |
Two problems that a maintainer hits on a clean checkout.
1. `npm test` needed a build before it, and it did not say so.
`tests/bundle.test.ts` imports the package root (`import esmLibrary from
'..'`), which package.json resolves to `release/`. On a checkout where
`release/` is absent, that one suite fails:
Error: Cannot find module '..' imported from tests/bundle.test.ts
The test workflow hid the problem, because it runs `npm run build` before
`npm test`. A maintainer who runs `npm ci && npm test` sees the failure.
A `pretest` script now builds first, so `npm test` is correct in any order.
`npm ci && npm test` passes on a clean clone.
2. npm 11 blocked esbuild's postinstall, and every install printed a warning:
npm warn install-scripts 1 package has install scripts not yet covered
npm warn install-scripts esbuild@0.19.7 (postinstall: node install.js)
esbuild still worked here, because @esbuild/win32-x64 and its siblings carry
the binary, and the postinstall is a fallback. The warning is still noise on
every install, and the fallback matters on a platform whose binary package
npm does not select. `allowScripts` now approves that one package, pinned to
the version that `overrides.tsup.esbuild` holds, so a change of the pin asks
for the approval again.
Verified in linux containers on node 20, 22 and 24, and on windows: `npm ci`
then `npm test` gives 11 test files and 54 tests passed, no install warning,
and release/cjs/lossless.cjs and release/cjs/lossless.cjs.map stay
byte-identical to the files that cornerstonejs/codecs vendors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lockfile carried 26 `extraneous: true` entries for esbuild 0.28.2's
per-platform packages. esbuild 0.28.2 briefly entered the tree while the
dependency work was in progress, and `overrides.tsup.esbuild` then settled the
tree on 0.19.7. The entries stayed behind, and they describe packages that no
longer belong to the dependency graph.
That is what broke `npm ci` on node 20 and node 22, not a difference of
lockfile format between npm versions. An `extraneous` entry has no
`optional: true` flag, so npm 10 treats it as a package it must install, and
it stops on the first entry that does not match the host:
npm error code EBADPLATFORM
npm error notsup Unsupported platform for @esbuild/aix-ppc64@0.28.2
npm 11 ignores an extraneous entry, which is why node 24 passed and why the
earlier diagnosis pointed at the npm version.
`npm ci` prunes these entries itself, so this commit records what `npm ci`
produces. The next commit revisits the `npm install --global npm@^11` step in
the test workflow, which is no longer the fix for this problem.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Commit ab62b35 added `npm install --global npm@^11` to each test leg, and the step is necessary, but the comment gave the wrong reason. Commit a694507 repeated that wrong reason, and it also said the step was no longer the fix. Both statements are wrong, and this commit corrects them. The step is not about a lockfile format that npm 10 cannot read. npm 10 and npm 11 resolve the nested `overrides.tsup.esbuild` differently: - npm 11 settles the whole tree on esbuild 0.19.7. - npm 10 keeps vite's own esbuild 0.28.2 beside it, so the lockfile that npm 11 wrote looks incomplete to npm 10, and `npm ci` stops with "Missing: esbuild@0.28.2 from lock file" for esbuild and its 25 per-platform packages. One lockfile cannot satisfy both resolvers, so the repository needs npm 11 everywhere. The earlier EBADPLATFORM error had a separate cause, which a694507 fixed: the lockfile held 26 stale `extraneous` entries for esbuild 0.28.2, and npm 10 treats an extraneous entry as one it must install. Verified in linux containers on node 20, 22 and 24 with the pruned lockfile: `npm ci`, the lint and all 54 tests pass on each version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…le too `overrides.tsup.esbuild` pinned esbuild for tsup alone, and npm 10 and npm 11 then built different trees from the same manifest: - npm 11 collapsed the tree onto esbuild 0.19.7. - npm 10 kept vite's own esbuild 0.28.2 beside it, so the lockfile that npm 11 wrote looked incomplete and `npm ci` stopped with "Missing: esbuild@0.28.2 from lock file", for esbuild and its 25 per-platform packages. One lockfile could not serve both resolvers, so the test workflow installed npm 11 on each leg. A contributor on node 20 or node 22 still met the failure locally, because those versions ship npm 10. `overrides.esbuild` pins the whole tree instead, and one lockfile then serves both. The workflow no longer installs npm 11, and the stock npm of each node version installs this lockfile: | node | npm | `npm ci` | | --- | --- | --- | | 20 | 10.8.2 | passes | | 22 | 10.9.8 | passes | | 24 | 11.19.0 | passes | The bundle does not change: release/cjs/lossless.cjs and release/cjs/lossless.cjs.map stay byte-identical to the files that cornerstonejs/codecs vendors, and the lint and all 54 tests pass on each node version and on windows. `npm audit` moves from 1 moderate to 4 moderate, and stays at 0 high and 0 critical. All four are the one esbuild development-server advisory, which now reaches more paths because the pin covers the whole tree. This repository never starts that server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
Correction: why
|
| node | npm | npm ci |
|---|---|---|
| 20 | 10.8.2 | passes |
| 22 | 10.9.8 | passes |
| 24 | 11.19.0 | passes |
This also removes the failure for a contributor who works on node 20 or node
22 locally, which the earlier workflow-only fix did not.
Also fixed
Commit 1484571 adds a pretest script. tests/bundle.test.ts imports the
package root, which resolves to release/, so npm ci && npm test failed on a
clean checkout with Cannot find module '..'. The test workflow hid the
problem, because it runs the build before the tests. npm test now builds
first, so it is correct in any order.
State
npm audit: 0 critical, 0 high, 4 moderate. All four are the one esbuild
development-server advisory, which this repository never starts. The count
moved from 1 to 4 because the pin now covers the whole tree.release/cjs/lossless.cjsandrelease/cjs/lossless.cjs.mapstay
byte-identical to the files thatcornerstonejs/codecsvendors, through
every change above.- The lint and all 54 tests pass on node 20, 22 and 24, and on windows.
🤖 Generated with Claude Code
The release workflow published only when the version in package.json was not on the registry, so each release needed a hand-written version bump. This commit makes the workflow choose the version. tools/version.mjs reads the commits since the last `v*` tag and maps them: | commit | bump | | --- | --- | | `feat!:`, or a `BREAKING CHANGE:` footer | major | | `feat:` | minor | | `fix:`, `perf:` | patch | | anything else | no release | `chore:`, `docs:`, `ci:`, `test:`, `refactor:`, `style:` and `build:` release nothing on their own, and a commit with no conventional prefix releases nothing either. This repository's older history is not conventional, and a guess at it would release on a commit that says only "remove map". A push that carries only such commits ends green and publishes nothing. The script imports only node builtins and it only writes files: package.json's version line and a new CHANGELOG.md section. Every git write stays in the workflow, so `node tools/version.mjs --dry-run` is safe against a dirty tree and prints exactly what a release would do. `cornerstonejs/codecs` splits tools/release/version.mjs and its release workflow the same way. The workflow now runs four jobs, and no job holds two of the three capabilities that matter: | job | permissions | installs | | --- | --- | --- | | build | contents: read | yes | | release | contents: write | no | | publish | contents: read, id-token: write | no | | github-release | contents: write | no | - build installs, lints, builds, tests, and keeps release/ as an artifact. It also fails early when the package name is absent from the registry, because trusted publishing cannot create a name, and a version commit for a publish that cannot succeed is worse than a red build. - release runs version.mjs, and it stops the run green when nothing is due. Otherwise it commits `chore(release): publish`, tags `v<version>` and pushes. - publish checks out that version commit, replays the artifact and publishes. - github-release writes the GitHub release from the CHANGELOG section. The build job skips a `chore(release): publish` commit by the actions bot, so a release cannot start another release. A push made with GITHUB_TOKEN does not start a workflow run in any case, so the guard also covers a maintainer who pushes such a commit by hand. Tags are annotated, because `git describe --tags` is what finds the previous release. Verified locally: the dry run reports a patch bump to 2.2.1 from the four `fix:` commits since v2.1; a `v2.2.0` tag at HEAD makes it report nothing to release; apply mode writes the version and a correct CHANGELOG section; and the release job's plan parsing and the github-release job's note extraction both work on that output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
wayfarer3130
added a commit
that referenced
this pull request
Sep 11, 2026
The release automation read the individual commits, but this repository squash merges, so a merge writes ONE commit to main and its subject is the pull request title. The individual commits never reach main, so they cannot decide the release. Pull request #1 showed the cost. It carried seven commits, four of them `fix:`, and it merged under the title: chore: publish as @cornerstonejs/jpeg-lossless-decoder-js (#1) `chore` releases nothing, so the release workflow published nothing. The run still reported success, because "nothing to release" is a legitimate green outcome, and `publish` and `github-release` simply skipped. Only the registry showed that 2.3.0 never existed. This commit makes the title the contract, and makes the contract visible before a merge rather than after it. - tools/conventional.mjs now holds the rules. tools/version.mjs and the new tools/check-pr-title.mjs both read it, so the check and the release cannot disagree about what a subject means. - tools/check-pr-title.mjs rejects a title that is not a conventional subject, rejects an unknown type, rejects a description under 10 characters (the description becomes the CHANGELOG entry), and otherwise states the release the title will produce. - .github/workflows/pr-title.yml runs the check on opened, edited, synchronize and reopened. `edited` matters as much as `opened`: the title is what merges, so a title corrected after review has to be re-checked. - README.md documents the contract as a table of titles, and it records how this package was bootstrapped rather than telling a reader to repeat steps that are done. The title reaches the check through the environment, never through the run script's text. A title is attacker-controlled on a fork pull request, and `${{ ... }}` interpolation would paste it into the shell, so a title containing `$(...)` would execute. Verified: `PR_TITLE='fix: $(touch /tmp/pwned) ...'` is read as literal text and nothing runs. Verified: the lint passes, all 54 tests pass, version.mjs behaves as before after the refactor, and the check gives the right answer for nine titles, including `feat!:` as major, `chore:` as no release, a title with no prefix, an unknown type, and a description that is too short. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This fork holds the byte-aligned-end-of-scan fix, and no package on npm holds
that fix.
cornerstonejs/codecsmust therefore vendor a build of thisrepository (see cornerstonejs/codecs#94). This pull request prepares the fork
for a publish under the
@cornerstonejsscope, so that the consumers can use anormal dependency.
The scoped name
@cornerstonejs/jpeg-lossless-decoder-jsis free on npm.Manifest
@cornerstonejs/jpeg-lossless-decoder-js, version →2.2.0repository.url,bugs.url,homepage→ this forkrii-mango/JPEGLosslessDecoderJS. npm builds the provenance attestation fromrepository.url, so the URL must name the repository that publishes the package.filesarraypublishConfig.access=publicprepublishOnlyscript@rollup/rollup-linux-x64-gnufromoptionalDependenciespackage-lock.jsonThe packaging defect
The repository has no
.npmignore, so npm falls back to.gitignore..gitignoreexcludes*.cjs.map,*.js.map,*.d.tsand*.d.cts. Eachpublish therefore shipped no source maps and no type declarations, and it
shipped
tests/data— about 6 MB of DICOM fixtures, one file of 4.7 MB.Published
2.1.2has this layout today.The
filesarray corrects both.npm pack --dry-runnow reports 22 files,64.5 kB packed and 303.3 kB unpacked, and the tarball holds
release/cjs/lossless.cjs.map,release/lossless.js.map,release/lossless-min.js.map,release/lossless.d.ts,release/cjs/lossless.d.ctsandrelease/lossless-min.d.ts.release/is no longer trackedA committed
release/could never be complete: the same.gitignoreexcludesthe maps and the declarations that one build emits.
Commit
03bb80c0shows the cost. Its committedrelease/cjs/lossless.cjsisbyte-identical to published
2.1.2(31,272 bytes), so the pinned commitcarried a bundle without the fix that the same commit added to
src/. Afresh build of that commit gives 32,684 bytes, and that build is byte-identical
to the file that
cornerstonejs/codecsvendors. A reviewer ofcornerstonejs/codecs#94 hit exactly this trap.
npm run buildregenerates the directory, andprepublishOnlyruns it beforeeach manual publish, so the tarball always matches the source that built it.
The lockfile
The committed
package-lock.jsonnames version2.1.0whilepackage.jsonnames
2.1.2, and its resolutions predate the manifest ranges.npm cicannotuse it, and the new workflow runs
npm ci. A partial update is not possible,so the lockfile is regenerated.
The refresh floats the build tools (
esbuild0.19.7 → 0.20.2, for example),which could change the emitted bundle. It does not: after
npm ciandnpm run build,release/cjs/lossless.cjsandrelease/cjs/lossless.cjs.mapare both byte-identical to the files that
cornerstonejs/codecsvendors. Thelint passes and all 54 tests pass.
The release workflow
.github/workflows/release.ymlpublishes on a push tomain, and only whenthe version in
package.jsonis not on the registry. Any other push tomainis a no-op, so an ordinary commit does not fail.
npm auth is OIDC trusted publishing: npm mints a short-lived token for each run
and scopes the token to this workflow file. This repository holds no
NPM_TOKEN. The name of the workflow file is part of that configuration.The work is in two jobs for one reason:
buildinstalls the dependencies, runs the lint, the build and the tests,and uploads
release/. It holds noid-tokenpermission.publishholdsid-token: write, installs nothing, and publishes therelease/artifact thatbuildmade.No install script of a dependency runs beside the OIDC token. This mirrors
cornerstonejs/codecs,.github/workflows/release.yml.What a maintainer must do
npm cannot create a package that does not exist yet through trusted publishing,
so the first publish must be manual.
README.mdgains a "Publishing"section with the steps. In short:
After that, this workflow publishes each version bump on
main.🤖 Generated with Claude Code