Skip to content

PER-18047: publish on Node 24 instead of upgrading npm on Node 20 - #138

Merged
EliMoshkovich merged 2 commits into
mainfrom
eli/per-18047-publish-on-node-24
Oct 30, 2026
Merged

EliMoshkovich merged 2 commits into
mainfrom
eli/per-18047-publish-on-node-24

Conversation

@EliMoshkovich

Copy link
Copy Markdown
Collaborator

The release workflow runs npm install -g npm@latest on Node 20. npm 12 refuses Node 20 (EBADENGINE), so every release now fails at that step, before tests or publishing.

npm's Trusted Publishing now also requires Node 22.14+, so pinning npm alone on Node 20 wouldn't be enough.

Changes

  • Node 24 for the publish job. It ships npm 11.19, which meets Trusted Publishing's 11.5.1 minimum, so the upgrade step is gone and a new npm major can't break the release again.

Testing

  • Build and unit tests on Node 24: 145 passed.
  • ci.yaml still tests Node 18 and 20, and the release job runs the full test suite on Node 24 before it publishes.
  • The publish step itself can only be exercised by a release.

🤖 Generated with Claude Code

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

@zeevmoney zeevmoney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved — no CRITICAL or HIGH issues found.

Non-blocking:

  • LOW .github/workflows/node_sdk_publish.yaml:22 — publish job's actions are tag-pinned and checkout persists credentials
  • LOW .github/workflows/node_sdk_publish.yaml:24 — "Node 24 ships both" holds only for 24.5.0 and later

Release note (not a finding):

  • A release run uses the workflow file at the tagged commit, so re-running the failed 2.7.7 run will execute the old npm upgrade step again. The 2.7.7 release has to point at a commit that contains this change, or ship as a new version.

Details are in the inline comments on each line.

Comment thread .github/workflows/node_sdk_publish.yaml Resolved
Comment thread .github/workflows/node_sdk_publish.yaml Resolved
…correct the Node comment

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@EliMoshkovich
EliMoshkovich merged commit b22aa4c into main Oct 30, 2026
6 checks passed
@EliMoshkovich
EliMoshkovich deleted the eli/per-18047-publish-on-node-24 branch October 30, 2026 20:27
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.

3 participants