Skip to content

ci: migrate to npm 12 - #167

Merged
achoimet merged 1 commit into
mainfrom
chore/npm-12
Aug 11, 2026
Merged

ci: migrate to npm 12#167
achoimet merged 1 commit into
mainfrom
chore/npm-12

Conversation

@achoimet

Copy link
Copy Markdown
Member

npm 12 disables install scripts by default, including for global installs — see the npm v12.0.0 release notes.

  • Upgrade to npm 12 before the global tool installs.
  • snyk: install with --allow-scripts=snyk — its postinstall bootstraps the CLI binary (without it, snyk still works but lazily downloads the binary on first run; at release time the explicit approval is clearer). Verified both behaviors on npm 12.0.2.
  • semver: unchanged, already installed with --ignore-scripts.

This reusable workflow is consumed by all extension repos, so this single change migrates their CI too.

Part of the org-wide npm 12 CI migration.

@claude

claude Bot commented Aug 10, 2026

Copy link
Copy Markdown

Claude finished @achoimet's task in 59s —— View job


Claude finished reviewing this PR

  • Check for previously unresolved review threads to reconcile
  • Review the diff in .github/workflows/reusable-extension-ci.yml
  • Post final review feedback

No prior unresolved Claude review threads exist on this PR, so nothing to reconcile.

Review

This is a small, well-scoped CI change (npm 12 default global-install upgrade) and I didn't find any correctness bugs, security issues, or race conditions.

  • The npm install -g npm@12 step is added consistently in both places that do global installs (release Snyk step and the chart-bump-version job), before the tool install — order is correct so the upgraded npm is what performs the subsequent install.
  • --allow-scripts=snyk on the Snyk install and the pre-existing --ignore-scripts on the semver install are both intentional and explained by inline comments; behavior is documented as verified against npm 12.0.2 in the PR description.
  • No secrets handling, permissions, or scoping changes — SNYK_TOKEN usage is untouched.
  • Minor observation (not a bug): npm@12 is an unpinned floating tag, so future npm 12.x patch releases will be picked up automatically in CI runs; the existing Snyk version below it is pinned exactly for "reproducibility." This is likely intentional for this migration and consistent with wanting the org to stay current on npm 12, so I'm not flagging it as an issue — just calling it out for awareness.

Nothing else stood out — the change looks safe to merge.

@achoimet
achoimet merged commit 24ec7ba into main Aug 11, 2026
6 checks passed
@achoimet
achoimet deleted the chore/npm-12 branch August 11, 2026 05:45
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant