Skip to content

feat: add publish-npm.yml for npm stable releases - #62

Merged
mairas merged 2 commits into
mainfrom
feat/v1-publish-npm
Sep 17, 2026
Merged

mairas merged 2 commits into
mainfrom
feat/v1-publish-npm

Conversation

@mairas

@mairas mairas commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds publish-npm.yml from the v1 interface plan (issue 49), replacing the five release.yml copies in the npm repositories (issue 29). With the release pipeline from PR 61, an npm repository's main.yml calls release-version and stage-release, and its release.yml calls publish-npm.

Behaviour

Two jobs keep dependency code away from publish credentials:

  • build, without id-token: requires a release event and a release commit on the default branch; checks the tag is v<upstream>+<N> with <upstream> equal to VERSION, and VERSION equal to package.json; skips with a warning when the version is already on npm; otherwise installs with --ignore-scripts (npm ci, or npm install with a warning when there is no lockfile), runs prepublishOnly, packs, and uploads the tarball.
  • publish, with id-token: write: requires npm 11.5.1 or later, downloads the tarball, and runs npm publish <tarball> --ignore-scripts. It serializes in the concurrency group <repository>-publish-npm-job.

Compared with the five copies: all skipped pre-releases; one read the package name from package.json (four hardcoded it); two checked VERSION against package.json; four warned on an existing version; one set persist-credentials: false; four serialized publishes. New here: the default-branch check, the tag-vs-VERSION check, and publishing a prebuilt tarball from a job that runs no dependency code. The corepack step all five copies had never took effect (runs print Preparing npm@11.14.1 and then 11.17.0), so it is gone.

Trusted publishing

npm validates the calling workflow's filename, so each repository keeps .github/workflows/release.yml and its npm trusted publisher configuration does not change. The caller grants id-token: write on the calling job, as examples/npm/.github/workflows/release.yml shows (npm docs).

Verification

  • tests/publish-npm.test.sh runs extracted steps against a fake npm and throwaway repositories: new and existing versions, v-prefixed VERSION, VERSION and package.json mismatch, tag for another version, malformed tags, release commit on and off the default branch, npm version minimum.
  • actionlint and shellcheck pass.
  • Not yet exercised: a real publish. That needs a version bump in a canary repository (skip-freeboard-panel in issue 55); the trusted-publishing path from a reusable workflow in another repository is the main thing that run proves.

Closes #53


🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: efab2333-61c9-47a4-ba30-daaf3e609dec

📥 Commits

Reviewing files that changed from the base of the PR and between 380f772 and f9848e9.

📒 Files selected for processing (4)
  • .github/workflows/publish-npm.yml
  • examples/npm/.github/workflows/main.yml
  • examples/npm/.github/workflows/release.yml
  • tests/publish-npm.test.sh

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

mairas and others added 2 commits September 17, 2026 14:34
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mairas
mairas force-pushed the feat/v1-publish-npm branch from 4bf1d6b to f9848e9 Compare September 17, 2026 11:35
@mairas

mairas commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Code Review Results

Scope: merge-base 380f772feat/v1-publish-npm at 4bf1d6b (4 files, +255)
Intent: Add publish-npm.yml replacing five npm release.yml copies, with tests and npm examples. CodeRabbit was rate-limited, so this review replaces it.
Reviewers: correctness, testing, maintainability, project-standards, api-contract, security, adversarial

  • api-contract: new frozen v1 interface, trusted publishing contract
  • security: id-token: write in a publish job
  • adversarial: five real callers migrate to it

P2 -- Moderate

# File Issue Reviewer Confidence Route
1 publish-npm.yml Job concurrency group <repo>-npm-publish equals the top-level group four callers already set; keeping it on migration deadlocks and cancels the publish. api-contract, adversarial 0.78 safe_auto -> review-fixer
2 publish-npm.yml prepublishOnly build code from node_modules runs in a job holding OIDC token request credentials. security 0.70 manual -> human
3 publish-npm.yml Without package-lock.json, dependencies resolve at publish time. security 0.72 gated_auto -> review-fixer
4 publish-npm.yml A release tag on a commit outside the default branch publishes unreviewed code. security 0.62 gated_auto -> human

P3 -- Low

# File Issue Reviewer Confidence Route
5 publish-npm.yml:78 corepack enable without npm creates no npm shim: the pin never applied (runs print Preparing npm@11.14.1 then 11.17.0). Same no-op in all five copies. correctness, testing 0.90 safe_auto -> review-fixer
6 publish-npm.yml:105 name step output has no consumer except the test. maintainability 0.85 safe_auto -> review-fixer
7 publish-npm.yml:64 actions/checkout@v6; every other workflow uses @v4. maintainability 0.72 safe_auto -> review-fixer
8 publish-npm.yml:92 Tag regex looser than apt-publish.yml's copy of the same format. maintainability 0.62 safe_auto -> review-fixer
9 tests/publish-npm.test.sh Leading v in VERSION untested. testing, maintainability 0.72 safe_auto -> review-fixer
10 publish-npm.yml Node version bumps under the moving v1 tag change every caller's build at once. api-contract 0.62 advisory -> human

Coverage

  • Suppressed: 0.
  • Dismissed: testing reviewer's note that canary merges start release.yml for their pre-releases. Pre-releases are created with the job token, whose events do not trigger workflows.
  • Residual risks:
    • Trusted publishing through a reusable workflow in another repository, and cross-org for hatlabs/signalk-halpi, is unproven until a canary publishes.
    • The tag-vs-VERSION check can only fire for drafts created by the legacy pipeline or by hand; stage-release.yml drafts target the built commit.
    • No dist-tag input; npm refuses an implicit latest behind a higher version (affects the private archive repo's staged v2.0.1+1 today, same as its current copy). Additive later.
    • Private repositories publish without provenance.
    • signalk-halpi's release event runs APT and npm publication independently; one can succeed while the other fails.

Verdict: Ready with fixes

@mairas

mairas commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Force-pushed review fixes (now f9848e9), folded into the two commits. Both commits pass all tests/*.test.sh.

  • 1: the publish job's group is <repository>-publish-npm-job; the header and examples/npm/.github/workflows/release.yml say callers must not use it.
  • 2: split into two jobs. build (no id-token) checks the release, installs with --ignore-scripts, runs npm run prepublishOnly --if-present, packs, and uploads the tarball. publish (id-token: write) downloads it and runs npm publish <tarball> --ignore-scripts; nothing from node_modules runs there.
  • 3: kept the npm install fallback, with a warning. signalk-duckdb-history-provider, signalk-questdb-history-provider and the private archive commit no lockfile, so requiring one would block their migration. After fix 2 the unlocked install runs without publish credentials; whether those repositories should commit lockfiles is a separate decision.
  • 4: the build job fails unless the release commit is an ancestor of the default branch.
  • 5: corepack removed. The publish job requires the runner's npm to be at least 11.5.1, the first release with trusted publishing, and prints the version.
  • 6, 8: name output dropped (the test checks the log line instead); tag regex matches apt-publish.yml's.
  • 7: actions/checkout@v4. setup-node@v6 stays: the five callers use it and nothing here uses an older major.
  • 9: tests added for a v-prefixed VERSION, the default-branch check (ancestor passes, side-branch commit fails), and the npm minimum (read from the workflow).
  • 10: not changed. A Node major bump should ship as a minor release of this repository with a release note.

@mairas
mairas merged commit 95d10b7 into main Sep 17, 2026
4 checks passed
@mairas
mairas deleted the feat/v1-publish-npm branch September 17, 2026 11:40
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.

v1 Unit 4: publish-npm.yml

1 participant