Skip to content

ci(publish): use npx for npm publish, drop broken in-place npm self-upgrade#163

Merged
naorsabag merged 1 commit into
masterfrom
fix-npm-self-upgrade
May 17, 2026
Merged

ci(publish): use npx for npm publish, drop broken in-place npm self-upgrade#163
naorsabag merged 1 commit into
masterfrom
fix-npm-self-upgrade

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 17, 2026

Follow-up to #162

The OIDC migration landed but the first publish run 25986313074 failed at the new `Upgrade npm for OIDC trusted publishing` step:

```
npm error code MODULE_NOT_FOUND
npm error Cannot find module 'promise-retry'
npm error - /opt/hostedtoolcache/node/22.22.2/x64/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/rebuild.js
```

This is the well-known npm in-place self-upgrade bug — `npm install -g npm` replaces `/opt/hostedtoolcache/.../node_modules/npm/` while npm is still executing out of it, so arborist's mid-flight `require('promise-retry')` blows up as the file is overwritten. Hits any major-version self-upgrade on GHA's Ubuntu runners; not specific to the `^11.5.1` pin.

Fix

Don't upgrade the system npm at all. Invoke npm 11.5.1+ via `npx -y npm@^11.5.1` from each publish step. npx fetches npm 11.x into its cache and runs it as a one-shot — the system npm 10.x stays untouched, so the in-place self-clobber is impossible. npx caches between invocations, so the three publish steps share the same downloaded npm.

The OIDC preflight no longer checks the system npm version (the `npx` specifier enforces `>=11.5.1` at the point of use); it just verifies `id-token: write` is granted.

Why npx and not Node 24

Considered bumping the publish workflow to Node 24 (which bundles npm 11.x). Rejected to keep the publish runtime aligned with the test/CI workflows (Node 22). Publishing on a Node version different from CI introduces a "we test on X but ship from Y" gap that npx neatly avoids — and the published artifacts are byte-identical either way (esbuild has no `--target` flag, tsc respects `tsconfig`, no `engines` field).

Test plan

  • Merge.
  • Re-run the publish workflow via `workflow_dispatch` (or push an empty bump). Expect: `Verify OIDC trusted publishing is wired up` passes; each publish step downloads npm 11.x via npx, exchanges the GitHub OIDC token, and publishes `0.3.3` for server / web / cli.
  • Confirm `0.3.3` lands on https://www.npmjs.com/package/@openhop/server, /@openhop/web, /openhop.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Optimized the package publishing workflow to use a pinned npm version during each publish step, improving reliability and consistency of the release process.

Review Change Stack

The previous workflow upgraded the system npm in place
(`npm install -g npm@^11.5.1`) so that `npm publish` would use an
OIDC-capable version. That step hit a long-standing npm self-upgrade
bug:

  npm error code MODULE_NOT_FOUND
  npm error Cannot find module 'promise-retry'
  npm error - .../npm/node_modules/@npmcli/arborist/lib/arborist/rebuild.js

`npm install -g npm` replaces /opt/hostedtoolcache/.../node_modules/npm/
in place while npm is still executing out of that same directory, so
arborist's mid-flight `require('promise-retry')` fails as the file is
overwritten. The bug is independent of which version we pin to and
shows up across major-version self-upgrades on GHA Ubuntu runners.

Fix: stop upgrading the system npm. Invoke npm 11.5.1+ via
`npx -y npm@^11.5.1` from each publish step instead. npx fetches
npm 11.x into its cache and runs it as a one-shot — the system npm
10.x stays untouched, so the in-place self-clobber is impossible.
npx caches between invocations, so the three publish steps share the
same downloaded npm.

The OIDC preflight no longer checks the system npm version (the npx
specifier enforces >=11.5.1 at the point of use); it just verifies
`id-token: write` is granted.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a3693d79-8197-4b04-9225-bf4e47b3af09

📥 Commits

Reviewing files that changed from the base of the PR and between 80f4170 and 991c045.

📒 Files selected for processing (1)
  • .github/workflows/publish.yml

Walkthrough

The publish GitHub Actions workflow refactors npm version management by replacing global npm upgrades with inline npx -y npm@^11.5.1 commands during package publication. The OIDC setup is simplified to verify token URL presence, and all three package publish steps are updated with the new npx pattern and OIDC documentation.

Changes

npm/OIDC publish refactoring

Layer / File(s) Summary
npm version enforcement via npx at publish time
.github/workflows/publish.yml
OIDC setup section is simplified to document that npm version checking occurs at publish time; OIDC verification step is shortened to check only token URL presence; and all three package publish steps (@openhop/server, @openhop/web, openhop CLI) switch from npm publish to npx -y npm@^11.5.1 publish with inline comments documenting OIDC token handling and npx caching behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • naorsabag/openhop#103: Introduced the publish workflow logic that this PR refactors to use npx -y npm@^11.5.1 publish.
  • naorsabag/openhop#104: Modified the same .github/workflows/publish.yml publish steps with stable step ids and release/tag gating logic.
  • naorsabag/openhop#162: Modified the same publish preflight logic with alternative npm authentication checking via npm whoami.

Suggested labels

github_actions

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: replacing npm self-upgrade with npx-based npm publish approach.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-npm-self-upgrade

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 and usage tips.

@naorsabag naorsabag merged commit 52e6efb into master May 17, 2026
8 checks passed
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.

1 participant