Skip to content

docs: process documents and release tooling for the 1.0-beta release - #519

Merged
wayfarer3130 merged 32 commits into
1.0-betafrom
release-process-docs
Oct 7, 2026
Merged

wayfarer3130 merged 32 commits into
1.0-betafrom
release-process-docs

Conversation

@awatson1978

@awatson1978 awatson1978 commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

This PR is the first step of the plan in #518. It adds the process documents and the release tooling for the 1.0-beta release. Merge of this PR publishes nothing.

Process documents

  • .github/PULL_REQUEST_TEMPLATE.md sets what a PR description must have. A small staging PR fills in the summary and the checklist only. Each package merge into 1.0-beta fills in all sections: a runnable example, test evidence, benchmarks where the PR claims a performance change, and fixture citations.
  • DATACITATION.md records the origin and the license of each committed DICOM test file. The format follows the file of the same name in OHIF. The origin of the six older test/*.dcm files is not known yet. The planned @dcmjs-org/fixtures work traces these files, or replaces them with synthetic files (RELEASE_PLAN.md, section 6, step 2).

Release tooling

All packages release together, with one shared version number:

  1. semantic-release reads the commit titles and selects the next version.
  2. scripts/stamp-workspace-versions.mjs writes that version into the root package and into each public workspace package. The script skips private packages.
  3. pnpm -r publish publishes all public packages. On 1.0-beta, the packages go to the npm beta tag.

The changes:

  • .releaserc.json sets three release branches: master, 1.0-beta (prerelease beta), and release/0.5x (range 0.x). The @semantic-release/npm plugin only sets the version and does not publish. The @semantic-release/exec plugin runs the stamp script and pnpm -r publish.
  • .github/workflows/publish-package.yml now also runs on a push to 1.0-beta and to release/0.5x. The workflow loads @semantic-release/exec, and sets NODE_AUTH_TOKEN from NPM_TOKEN, because pnpm publish reads that variable.
  • package.json marks the root dcmjs package "private": true on this branch. A recursive publish thus cannot publish the unscoped dcmjs name by accident (RELEASE_PLAN.md, section 9).
  • test/stamp-workspace-versions.test.js has two tests. The first test stamps a scratch workspace and makes sure that the script skips private packages. The second test makes sure that the script rejects a string that is not a version, and changes no file.

Corrections to RELEASE_PLAN.md

  • Section 3 now says that the plan first hoped for the @dcmjs/ scope. An earlier rename to @dcmjs-org also changed this historical sentence by mistake.
  • Section 12, item 3 now tells the repository owner to register the @dcmjs-org npm organization, if necessary. The item also tells the owner to make sure that NPM_TOKEN can publish new packages under @dcmjs-org. The old item named the @dcmjs scope.

Before the first publish

The publish GitHub environment still controls the workflow. The workflow cannot run from 1.0-beta until the repository owner does the actions in RELEASE_PLAN.md, section 12:

  1. Add branch protection to 1.0-beta.
  2. Add 1.0-beta to the allowed branches of the publish environment.
  3. Register the @dcmjs-org npm organization, if necessary, and make sure that NPM_TOKEN can publish under that scope.

An earlier version of this PR also added docs/WRITING_STYLE.md. Commit 2a5935e removed that guide, because the authoring work is complete.

🤖 Generated with Claude Code

awatson1978 and others added 9 commits September 16, 2026 18:12
Supersedes the 2026-08-07 plan. Describes the package split, the
1.0-beta release branch, per-package staging branches, the test-first
sequence for core, routing of the 33 review findings, and lockstep
publishing with semantic-release.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two trivial assertions on the committed sample-sr.dcm fixture. The
purpose is to exercise the CI checks on the 1.0-beta branch and prove
they run and report on pull requests. See RELEASE_PLAN.md section 6,
step 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e 24

fs.readFileSync can return a small file inside a larger shared memory
pool. Taking .buffer on that result hands the parser the whole pool,
with the file sitting at a nonzero offset, and the DICM marker check
then fails. Node 24 pools more aggressively, which is why
anonymizer.test.js and data.test.js fail there today (7 tests) while
Node 22 passes by luck of alignment.

This adds readFileAsArrayBuffer() to testUtils and routes all 22
file-read sites through it. Typed-array .buffer uses are untouched.
Same class of problem as issue #311.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ware helper

The first pass converted single-line fs.readFileSync(...).buffer sites.
CI on Node 24 then failed in a second population: reads assigned to a
variable first, with .buffer taken on a later line. This converts those
sites in data.test.js, data-options.test.js, lossless-read-write.test.js
and normalizers.test.js, and drops the imports that became unused.
Sites that already handle byteOffset correctly are untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ensureTestDataDir checked for the directory and then created it. Two
parallel jest workers can both pass the check, and the slower mkdir then
fails with EEXIST, which failed test_multiframe_1 on one CI leg.
mkdirSync with recursive:true succeeds whether or not the directory
exists, so the check goes away.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three process documents the release plan calls for. The style guide
defines the audience (healthcare-literate, no assumed programming or
imaging jargon) and bans machine-flavored prose. The PR template
requires plain-language descriptions, runnable examples, test evidence,
and fixture citations. DATACITATION.md records the origin and license
of every committed DICOM test file, with the legacy files honestly
marked as provenance-pending.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
semantic-release computes one version from conventional commits;
stamp-workspace-versions.mjs writes it into every publishable workspace
package.json, and pnpm -r publish releases them together under the npm
beta tag. The publish workflow now also fires on 1.0-beta and
release/0.5x, and passes NODE_AUTH_TOKEN for pnpm publish. Nothing
publishes until the repository owner adds 1.0-beta to the publish
environment allowlist. See RELEASE_PLAN.md sections 9 and 12.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings in the Node 24 buffer fix and the temp-directory race fix so
this branch's CI can pass before #518 merges. Once #518 lands, the
diff of #519 collapses back to just the process files.
Adapts the rules from Cursor's unslop skill: the fuller AI-vocabulary
list, filler phrases, fancy synonyms for is, vague attributions,
synonym cycling, false ranges, punctuation and formatting rules
(em dashes, colons, bold, sentence-case headings, straight quotes),
abstract metaphor nouns, and a plain-speech section that includes the
rule against over-compressed arrow-speak. Also states the boundary
this guide draws that unslop does not: metaphors that teach a concept
to a non-technical reader stay, metaphors that decorate go. The
guide's own prose now follows its own em-dash rule.

Source: https://github.com/cursor/plugins/blob/main/pstack/skills/unslop/SKILL.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@awatson1978 awatson1978 changed the title docs: process documents and release tooling for the 1.0-beta line docs: process documents and release tooling for the 1.0-beta release Sep 17, 2026
awatson1978 and others added 6 commits September 17, 2026 15:27
Adopts wayfarer3130's review comments: the engines split from the data
types (core = building blocks only; legacy = today's readers and
writers; parser = the new streaming reader; generator = the new
writers). The vendored dicom-parser tokenizer is dropped from the 1.0
stream entirely, and about 300 tests retire with it. @dcmjs/media is
added as a bundle over dicomdir, pdfs, and video, which all remain
individually installable. The wrapper publishes as @dcmjs/dcmjs during
the beta so the real dcmjs name stays untouched until the team is
ready. FHIR stays in the plan with its landing order marked as an open
question. Section 7 is rewritten to the simpler tests-travel-with-code
approach discussed since the first draft.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s sequencing

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@awatson1978
awatson1978 requested review from igoroctaviano, pieper and wayfarer3130 and removed request for igoroctaviano, pieper and wayfarer3130 September 30, 2026 18:41
@wayfarer3130

Copy link
Copy Markdown
Contributor

There were three things found that I htink are worth looking at:

  1. --tag beta on master after GA (Medium)
    .releaserc.json declares three branches: release/0.5x, master, and 1.0-beta. The publishCmd uses --tag beta on all of them. semantic-release reads the config from the branch that it builds, so only the master entry has an effect:

Before GA: no effect, because the file is only on 1.0-beta.
release/0.5x: no effect. That branch comes from the 0.52 line and does not have this file.
At GA, 1.0-beta merges into master. The first master publish is 1.0.0 with the beta dist-tag, and latest stays on 0.52.0. RELEASE_PLAN §10 says that GA "releases 1.0.0 to npm latest". The config does not do that. Each later master release also gets beta.
Recovery is easy: npm dist-tag add dcmjs@1.0.0 latest. Suggested fix, so that the GA PR needs no change to the config: --tag ${nextRelease.channel || 'latest'}.

  1. The version can be 0.53.0-beta.1 (High, PLAUSIBLE)
    The last tag on 1.0-beta is v0.52.0. semantic-release calculates the prerelease from the last release and the commit types. A feat: commit gives 0.53.0-beta.1. A fix: commit gives 0.52.1-beta.1. Only a BREAKING CHANGE: footer (or feat!:) gives 1.0.0-beta.1. The plan says 1.0.0-beta.N everywhere, but no document tells the author to add a breaking-change commit. I did not run semantic-release to confirm this, because a dry run needs tokens.

Suggested fix: write the step in RELEASE_PLAN §9 (for example, the first package merge has a BREAKING CHANGE: footer). Or run semantic-release --dry-run --branches 1.0-beta to confirm.

  1. Unscoped dcmjs is published during the beta (Medium)
    pnpm-workspace.yaml lists only ".". The root package is dcmjs, and it is not private. I ran a dry run in a scratch workspace with the same layout: pnpm -r publish includes the root package. So the first beta publish is dcmjs@-beta.1, with the beta tag.

RELEASE_PLAN §3 and §9 say that the beta publishes as @dcmjs/dcmjs, and that "the unscoped dcmjs name starts publishing ... when the team decides it is ready." The beta tag does not change latest, so users that do nothing are safe. But the plan and the config disagree. Ask the author which result is correct, and change the other one.

  1. No dist-tag promotion (Medium)
    @semantic-release/npm has npmPublish: false, and @semantic-release/exec has no addChannelCmd. When semantic-release adds an existing version to a new channel, it calls addChannel. With this config, that step does nothing. Example: the cleanup of the vNext and dev tags in §10 must be done by hand. If that is the intent, the plan must say so.

@wayfarer3130 wayfarer3130 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few comments,
also dcmjs is already claimed, by "dcmjs", which might be Steve, or it might be whoever registered the other dcmjs github (whcih hasn't done anything)

awatson1978 and others added 3 commits October 1, 2026 20:02
…arifications from review

Rewrites the section-1 metaphor in direct terms and retitles section 6.
Adds the breaking-change-commit requirement for the first 1.0.0-beta.1
version calculation, documents the private root package during the beta,
records dist-tag promotion as a deliberate manual step, and fixes the
appendix publishCmd to tag by channel instead of hardcoding beta.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng the beta

Review found the publish command hardcoded --tag beta, which would have
followed 1.0-beta into master at GA, and that a recursive publish would
ship the unscoped dcmjs name during the beta against the plan's intent.
The tag now derives from the release channel, and the root package is
private until the @dcmjs/dcmjs wrapper PR renames it. Dist-tag promotion
stays deliberately manual, now documented in RELEASE_PLAN section 10.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@awatson1978

Copy link
Copy Markdown
Collaborator Author

Thanks Bill — all four are addressed on this branch (and flowing to the staging branches):

  1. --tag beta after GA — took your suggested fix verbatim: the publish command now tags with ${nextRelease.channel || 'latest'}, so the 1.0-beta branch publishes under beta and master publishes under latest with no config change needed in the GA PR. Appendix A in RELEASE_PLAN.md updated to match.

  2. 0.53.0-beta.1 risk — you're right about the calculation; RELEASE_PLAN §9 now states the requirement explicitly: the release that produces the first beta must carry a BREAKING CHANGE: footer (or feat!:) so the computed version is 1.0.0-beta.1, plus a note to confirm with semantic-release --dry-run before flipping the publish switch.

  3. Unscoped dcmjs during the beta — the plan's intent was the correct side, so the config changed: the root package.json is now "private": true on this lineage, which makes pnpm -r publish skip it. The wrapper PR (step 7 of the landing order) removes the flag when it renames the root surface to @dcmjs/dcmjs. §9 documents this.

  4. No dist-tag promotion — manual is indeed the intent for now; §10 says so explicitly (channel tags come from the publish itself; promoting an existing version and deleting the stale vNext/dev tags is a hand-run npm dist-tag step, and an addChannelCmd is deferred until there's a real promotion to perform).

🤖 Generated with Claude Code

@wayfarer3130

Copy link
Copy Markdown
Contributor

@awatson1978 - can you update the notes to indicate we will publish to @dcmjs-org/ paths and will reconsider if we can get the @dcmjs path later? I've contacted the owner of that, but don't have a response yet.

The @dcmjs npm organization is not ours; per review discussion the
packages publish under @dcmjs-org (matching the GitHub organization),
and the team can reconsider the shorter scope if its owner responds.
Every package name in the plan now reads @dcmjs-org/.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@awatson1978

Copy link
Copy Markdown
Collaborator Author

Done — everything now reads @dcmjs-org/. RELEASE_PLAN's scope-name note records the decision the way you described it: we publish under @dcmjs-org/ (matching the GitHub org), you've contacted the owner of @dcmjs, and if that name becomes available the team can reconsider the shorter scope later. The rename is applied end to end, not just in the notes: the package names, workspace dependencies, and by-name imports on the staging branches (#568 for the engine packages, #569 for schemas/fixtures), the data-citation file, and the assembly PR descriptions. The wrapper's beta name in §9 is now @dcmjs-org/dcmjs accordingly. One leftover action from §3 still stands: confirming the @dcmjs-org npm organization itself is registered to us before the first publish.

🤖 Generated with Claude Code

awatson1978 and others added 3 commits October 3, 2026 10:22
Per review on #542: the guide existed to hold the staged-landing PR
descriptions to one standard, and every description is now written.
Removed rather than kept stale; git history preserves it if the team
wants it back later. The PR template keeps the one-line spirit of it.

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

@wayfarer3130 wayfarer3130 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a few changes for better security/publishing design - note that trusted publishing will become required in 2027, so we might as well go with it.

  • Raise minimumReleaseAge to 2880 (2 days).
  • Pin semantic-release and its plugins as exact devDependencies.
  • Pin each action by commit SHA.
  • Publish with OIDC trusted publishing instead of the long-lived npm token.
  • Turn off cache: pnpm in the publish job.

…publish job

Review feedback from #519 (trusted publishing becomes required in 2027,
so adopt it now):

- minimumReleaseAge 1440 -> 2880 (two days)
- semantic-release and its five plugins are exact-pinned devDependencies
  installed from the lockfile; the publish step runs pnpm exec
  semantic-release instead of cycjimmy/semantic-release-action with
  runtime-fetched extra_plugins
- every action in the publish workflow is pinned by commit SHA
- npm auth is OIDC trusted publishing: no NPM_TOKEN/NODE_AUTH_TOKEN, no
  registry-url .npmrc; RELEASE_PLAN documents the per-package trusted
  publisher setup and the first-publish bootstrap
- cache: pnpm removed from the publish job

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@awatson1978

Copy link
Copy Markdown
Collaborator Author

All five are in (fe4c87e) — agreed on getting ahead of the 2027 requirement rather than migrating later:

  1. minimumReleaseAge 2880 — raised in pnpm-workspace.yaml (strict mode already on, so resolution fails loudly rather than falling back).

  2. semantic-release pinned as exact devDependencies — semantic-release 25.0.9 plus all five plugins from .releaserc.json (commit-analyzer 13.0.1, release-notes-generator 14.1.1, npm 13.2.0, github 12.0.10, exec 7.1.0), exact like the rest of the devDependencies block, resolved through the lockfile. This also retired cycjimmy/semantic-release-action and its runtime-fetched extra_plugins — the publish step is now pnpm exec semantic-release, so nothing is fetched at release time.

  3. Actions pinned by commit SHA — checkout, pnpm/action-setup, and setup-node, each with the version as a trailing comment.

  4. OIDC trusted publishing — NPM_TOKEN/NODE_AUTH_TOKEN are gone, and so is setup-node's registry-url (its generated .npmrc expects a token env that no longer exists). The workflow already carried id-token: write for provenance; pnpm picks up the OIDC exchange from there. RELEASE_PLAN §12 now documents the owner side: register each package's trusted publisher on npmjs.com (this repo + publish-package.yml + the publish environment) — including the first-publish bootstrap, since a trusted publisher can only be configured on a package that already exists, the very first publish of each new package is a one-time npm publish from an owner workstation.

  5. cache: pnpm off in the publish job — removed; the job installs from the lockfile only.

Appendix B in RELEASE_PLAN now states the whole hardening posture in one place so it survives after this PR's conversation.

🤖 Generated with Claude Code

…them

pnpm 11 stopped reading package.json pnpm.overrides (it warned on every
install), so the security pins silently stopped applying and the audit
gate failed once the lockfile was regenerated: form-data@3 and ws@7
resolved to advisory-vulnerable versions through jest 27's jsdom chain,
with more paths added by the new semantic-release devDependencies.

Ports the fix already on release/1.0-beta-core: the overrides block
(old pins plus audit minimums for adm-zip, js-yaml, brace-expansion,
browserslist, ws@7, form-data@3) and the braces GHSA ignore (advisory
names a patched >=3.0.4 that still does not exist on the registry; all
paths are dev tooling) live in pnpm-workspace.yaml, and the dead
package.json pnpm field is removed.

pnpm audit --audit-level=high exits 0; full suite 320/320.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@awatson1978

Copy link
Copy Markdown
Collaborator Author

The audit failure after the previous push had a root cause worth recording: pnpm 11 stopped reading package.json pnpm.overrides (it had been warning on every install), so this branch's security pins had silently stopped applying — regenerating the lockfile for the semantic-release devDependencies surfaced it (form-data@3 and ws@7 resolving to advisory-vulnerable versions through jest's jsdom chain). f2a3631 ports the fix already on release/1.0-beta-core: the overrides live in pnpm-workspace.yaml where pnpm 11 reads them, the dead package.json field is removed, and the braces GHSA ignore stays (the advisory's "patched" 3.0.4 still doesn't exist on the registry — re-checked today; all paths are dev tooling). pnpm audit --audit-level=high exits 0 and the suite is 320/320; all checks green.

🤖 Generated with Claude Code

@wayfarer3130
wayfarer3130 merged commit e648878 into 1.0-beta Oct 7, 2026
10 checks passed
@wayfarer3130
wayfarer3130 deleted the release-process-docs branch October 7, 2026 13:28
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