Skip to content

fix(release): disable pnpm 11 verifyDepsBeforeRun#162

Merged
vadimpiven merged 1 commit into
mainfrom
fix/release-bump-version-reinstall
May 24, 2026
Merged

fix(release): disable pnpm 11 verifyDepsBeforeRun#162
vadimpiven merged 1 commit into
mainfrom
fix/release-bump-version-reinstall

Conversation

@vadimpiven
Copy link
Copy Markdown
Owner

pnpm 11 (PR #156) made verifyDepsBeforeRun: install the default, which silently re-runs pnpm install whenever a workspace manifest hash changes. The release build mutates packages/node/package.json via bump-version between the initial mise install and the build script, so pnpm transparently re-installed and ran the package's slsa wget postinstall against a version that has not been published yet (slsa wget skips 0.0.0 but errors on every other un-published version). Linux failed first on the proxied DNS lookup; macOS got further and surfaced the underlying provenance check failure.

Disable the implicit reinstall — mise run setup:pnpm already tracks dependency freshness via task sources.

pnpm 11 (PR #156) made `verifyDepsBeforeRun: install` the default,
which silently re-runs `pnpm install` whenever a workspace manifest
hash changes. The release build mutates `packages/node/package.json`
via `bump-version` between the initial `mise install` and the build
script, so pnpm transparently re-installed and ran the package's
`slsa wget` postinstall against a version that has not been published
yet (slsa wget skips `0.0.0` but errors on every other un-published
version). Linux failed first on the proxied DNS lookup; macOS got
further and surfaced the underlying provenance check failure.

Disable the implicit reinstall — `mise run setup:pnpm` already tracks
dependency freshness via task `sources`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@semanticdiff-com
Copy link
Copy Markdown

semanticdiff-com Bot commented May 24, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  pnpm-workspace.yaml  0% smaller

@vadimpiven vadimpiven merged commit 9f9b2ec into main May 24, 2026
17 of 20 checks passed
@vadimpiven vadimpiven deleted the fix/release-bump-version-reinstall branch May 24, 2026 10:03
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the pnpm-workspace.yaml file by adding the verifyDepsBeforeRun: false configuration to prevent package reinstallation during version bumps. It also removes several explanatory comments regarding pnpm 11 behavior for allowBuilds and storeDir. The review feedback correctly identifies that these removed comments provide essential context for maintainability and security scan exclusions, recommending their restoration.

Comment thread pnpm-workspace.yaml
# pnpm 11 supersedes the legacy `onlyBuiltDependencies` list with a map of
# explicit per-package toggles. Anything not listed here is silently skipped
# (with `strictDepBuilds: true`, blocked outright).
allowBuilds:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The documentation explaining the allowBuilds configuration and its behavior with strictDepBuilds was removed. This context is valuable for understanding how dependencies are handled in the workspace and should be preserved for maintainability.

# pnpm 11 supersedes the legacy `onlyBuiltDependencies` list with a map of
# explicit per-package toggles. Anything not listed here is silently skipped
# (with `strictDepBuilds: true`, blocked outright).
allowBuilds:

Comment thread pnpm-workspace.yaml
# pnpm 11 stopped honoring the legacy `npm_config_store_dir` env var.
# Pin the store inside `.cache/native/pnpm` so it stays under the
# directory that's gitignored and excluded from trivy / coverage scans.
storeDir: .cache/native/pnpm
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The documentation explaining why storeDir is pinned to .cache/native/pnpm was removed. This context is important for understanding the project's caching strategy and why this specific directory is used (e.g., to avoid security scans). It should be retained.

# pnpm 11 stopped honoring the legacy `npm_config_store_dir` env var.
# Pin the store inside `.cache/native/pnpm` so it stays under the
# directory that's gitignored and excluded from trivy / coverage scans.
storeDir: .cache/native/pnpm

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 24, 2026

Greptile Summary

This PR fixes a release CI regression introduced by pnpm 11's new verifyDepsBeforeRun: install default, which transparently re-ran pnpm install after bump-version mutated packages/node/package.json, causing the slsa wget postinstall to attempt a provenance check against an as-yet-unpublished version.

  • Adds verifyDepsBeforeRun: false to pnpm-workspace.yaml, disabling the implicit reinstall globally. Dependency freshness for the workspace is already managed through mise run setup:pnpm task sources.
  • Removes two comment blocks that explained the rationale behind the allowBuilds / strictDepBuilds pairing and the storeDir path; the fix itself is correct and well-targeted.

Confidence Score: 4/5

Safe to merge — the single-line config change correctly targets the root cause and the trade-off is documented in the PR description.

The fix is minimal and well-motivated. The only notable side effect is that verifyDepsBeforeRun is now disabled for all developers, not just CI release builds, but the PR description explicitly acknowledges this and points to the existing mise task as the replacement guard. Two explanatory comment blocks were removed as a by-product of the edit, leaving the allowBuilds/strictDepBuilds relationship and the storeDir rationale undocumented.

No files require special attention.

Important Files Changed

Filename Overview
pnpm-workspace.yaml Adds verifyDepsBeforeRun: false to prevent pnpm 11's implicit reinstall from triggering the SLSA postinstall against unpublished versions during the release build; also removes two explanatory comment blocks about allowBuilds and storeDir.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
pnpm-workspace.yaml:10-14
**Comment removal loses useful context**

The two removed comment blocks explained non-obvious decisions: why `allowBuilds` replaced the legacy `onlyBuiltDependencies` list (and how it interacts with `strictDepBuilds: true`), and why `storeDir` is pinned to `.cache/native/pnpm` (excluded from trivy/coverage scans). Without those comments a future reader has no hint that these settings reflect intentional pnpm 11 migration choices rather than arbitrary values.

Reviews (1): Last reviewed commit: "fix(release): disable pnpm 11 verifyDeps..." | Re-trigger Greptile

Comment thread pnpm-workspace.yaml
Comment on lines 10 to +14

blockExoticSubdeps: true

# required for `bump-version` script to not trigger packages reinstall
verifyDepsBeforeRun: false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Comment removal loses useful context

The two removed comment blocks explained non-obvious decisions: why allowBuilds replaced the legacy onlyBuiltDependencies list (and how it interacts with strictDepBuilds: true), and why storeDir is pinned to .cache/native/pnpm (excluded from trivy/coverage scans). Without those comments a future reader has no hint that these settings reflect intentional pnpm 11 migration choices rather than arbitrary values.

Prompt To Fix With AI
This is a comment left during a code review.
Path: pnpm-workspace.yaml
Line: 10-14

Comment:
**Comment removal loses useful context**

The two removed comment blocks explained non-obvious decisions: why `allowBuilds` replaced the legacy `onlyBuiltDependencies` list (and how it interacts with `strictDepBuilds: true`), and why `storeDir` is pinned to `.cache/native/pnpm` (excluded from trivy/coverage scans). Without those comments a future reader has no hint that these settings reflect intentional pnpm 11 migration choices rather than arbitrary values.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 24, 2026

Merging this PR will not alter performance

✅ 15 untouched benchmarks


Comparing fix/release-bump-version-reinstall (93ac36e) with main (66a5ee4)

Open in CodSpeed

@codecov
Copy link
Copy Markdown

codecov Bot commented May 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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