yarn -> pnpm, volta -> vite+ (BL-16115)#611
Conversation
Use the Vite+ (vp) CLI to manage the Node.js runtime and the pnpm package manager instead of Volta. vp reads the Node version from .node-version and the package manager from package.json's packageManager field. Bump Node 20.18.0 -> 22.22.3: pnpm 11.5.2, when run via Vite+'s bundled pnpm, requires Node >= 22.13 (it uses the node:sqlite builtin). Align @types/node to the 22.x line accordingly. CI now uses voidzero-dev/setup-vp@v1 (pinned to vp 0.1.24) plus `vp install --frozen-lockfile` and `vp run <script>`. The build/test scripts still run the pinned Vite 5 / vitest; routing build/dev/test through `vp build|dev|test` is a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR replaces the Yarn-based build system with pnpm and the Vite+ (vp) task runner, pins all dependency versions exactly, adds supply-chain security hardening, updates Node to 22.22.3, and upgrades CI/CD workflows and developer documentation to use the new toolchain. ChangesPackage Manager & CI/CD Toolchain Migration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
|
| Filename | Overview |
|---|---|
| .github/workflows/build-and-deploy.yml | Migrated from yarn/volta to vp (Vite+); install/build/test commands updated correctly. Uses mutable setup-vp@v1 tag instead of a pinned SHA. |
| pnpm-workspace.yaml | New file adding supply-chain hardening: allowBuilds allowlist, blockExoticSubdeps, and a 7-day minimumReleaseAge. The minimumReleaseAgeExclude for bloom-player only covers the package itself, not its transitive deps, which could cause master alpha-build failures in strict mode. |
| package.json | Switched packageManager to pnpm@11.5.2, removed volta config and yarn resolutions, pinned all dependencies to exact versions, and updated scripts from yarn to pnpm. build:ci:alpha correctly uses pnpm add -D bloom-player@alpha instead of the old yarn upgrade. |
| .husky/pre-commit | Replaced yarn pretty-quick/lint-staged with direct node_modules/.bin/ paths, which is correct for pnpm since it doesn't expose a pnpm exec shim by default in hooks. |
| .node-version | New file pinning Node.js to 22.22.3 for Vite+/setup-vp to consume; replaces the volta block in package.json. |
| .gitignore | Removed yarn/volta specific entries (.pnp, .yarn/, .yarnrc.yml) and replaced yarn-debug/error.log with pnpm-debug.log. |
Reviews (1): Last reviewed commit: "volta -> Vite+ for Node.js and pnpm mana..." | Re-trigger Greptile
| uses: voidzero-dev/setup-vp@v1 | ||
| with: | ||
| version: 0.1.24 | ||
| node-version-file: .node-version | ||
| cache: true |
There was a problem hiding this comment.
Mutable action tag for
setup-vp
voidzero-dev/setup-vp@v1 resolves to whatever the v1 tag currently points to — a future push to that tag by voidzero-dev would run different code on your next build without any change to this file. The setup-vp docs acknowledge this and offer an "Advanced" composition with socketdev/action@<sha> for full SHA-pinning. Note that actions/checkout@v3 has the same exposure, but that pre-dates this PR. Pinning to a commit SHA (letting Renovate bump it) is the hardened posture.
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!
| # bloom-player is our own controlled package; we want to consume newly published | ||
| # alphas immediately, so exclude it from the release-age delay above. | ||
| minimumReleaseAgeExclude: | ||
| - bloom-player | ||
|
|
There was a problem hiding this comment.
minimumReleaseAgeExclude does not cover bloom-player's transitive deps
minimumReleaseAgeExclude: - bloom-player exempts the bloom-player package itself from the 7-day delay, but pnpm's resolution will still enforce minimumReleaseAge on any new transitive dep that bloom-player's latest alpha pulls in. Because minimumReleaseAge is explicitly set, pnpm 11 defaults minimumReleaseAgeStrict to true, meaning a freshly-published transitive dep would cause pnpm add -D bloom-player@alpha to exit non-zero and break every master build until that dep ages out. Adding minimumReleaseAgeStrict: false to the workspace config would keep the delay for normal installs while letting the alpha update step succeed regardless of transitive dep ages.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-deploy.yml:
- Line 20: Replace the mutable action reference `uses: voidzero-dev/setup-vp@v1`
with the immutable commit SHA that it currently resolves to (e.g. `uses:
voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1`) so the workflow
is pinned and cannot drift; update the `uses` entry in the workflow to that SHA
and commit the change.
In @.husky/pre-commit:
- Around line 4-5: Replace direct node_modules/.bin invocations in the
pre-commit hook with pnpm exec to align with pnpm conventions: locate the two
commands where node_modules/.bin/pretty-quick --staged and
node_modules/.bin/lint-staged are invoked and change them to use pnpm exec
(i.e., run the same tools via pnpm exec pretty-quick --staged and pnpm exec
lint-staged) so pnpm resolves and runs the executables.
In `@package.json`:
- Line 10: The npm scripts "build:ci" and "test:ci" use "set CI=true&& pnpm ..."
which doesn't export CI to the command in POSIX shells; replace those with an
inline env assignment so the CI variable is set for the pnpm invocation (e.g.,
change the "build:ci" and "test:ci" script values from the current "set
CI=true&& pnpm ..." form to use "CI=true pnpm ..." so the environment variable
is applied to the subsequent command).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c23fc258-9e87-44ce-86cf-7257e6e8edb7
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.github/workflows/build-and-deploy.yml.gitignore.husky/pre-commit.node-versionREADME.mdindex.htmlpackage.jsonpnpm-workspace.yamlsrc/translations/README.mdvite.config.ts
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Install Vite+ (manages Node.js and pnpm) | ||
| uses: voidzero-dev/setup-vp@v1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Resolve the current object behind tag v1 (verify before pinning).
ref_json="$(gh api repos/voidzero-dev/setup-vp/git/ref/tags/v1)"
type="$(jq -r '.object.type' <<<"$ref_json")"
sha="$(jq -r '.object.sha' <<<"$ref_json")"
if [ "$type" = "tag" ]; then
gh api repos/voidzero-dev/setup-vp/git/tags/"$sha" --jq '.object.sha'
else
echo "$sha"
fiRepository: BloomBooks/BloomLibrary2
Length of output: 109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm the exact line content in the workflow
rg -n "voidzero-dev/setup-vp@v1" .github/workflows/build-and-deploy.yml || true
nl -ba .github/workflows/build-and-deploy.yml | sed -n '1,80p'Repository: BloomBooks/BloomLibrary2
Length of output: 177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Resolve tag v1 -> commit SHA (matches prior run)
ref_json="$(gh api repos/voidzero-dev/setup-vp/git/ref/tags/v1)"
type="$(jq -r '.object.type' <<<"$ref_json")"
sha="$(jq -r '.object.sha' <<<"$ref_json")"
if [ "$type" = "tag" ]; then
gh api repos/voidzero-dev/setup-vp/git/tags/"$sha" --jq '.object.sha'
else
echo "$sha"
fiRepository: BloomBooks/BloomLibrary2
Length of output: 109
Pin voidzero-dev/setup-vp to an immutable commit SHA (avoid mutable @v1)
In .github/workflows/build-and-deploy.yml (Line 20), uses: voidzero-dev/setup-vp@v1 is mutable and currently resolves to 2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1. Pin to that SHA to prevent CI supply-chain drift.
🔒 Suggested fix
- uses: voidzero-dev/setup-vp@v1
+ uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: voidzero-dev/setup-vp@v1 | |
| uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1 |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-and-deploy.yml at line 20, Replace the mutable
action reference `uses: voidzero-dev/setup-vp@v1` with the immutable commit SHA
that it currently resolves to (e.g. `uses:
voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1`) so the workflow
is pinned and cannot drift; update the `uses` entry in the workflow to that SHA
and commit the change.
Source: Linters/SAST tools
| node_modules/.bin/pretty-quick --staged | ||
| node_modules/.bin/lint-staged |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider using pnpm exec for better ecosystem alignment.
While the direct node_modules/.bin/ invocation is functionally correct, using pnpm exec is more idiomatic for pnpm-based projects and explicitly delegates executable resolution to pnpm.
♻️ Proposed refactor using pnpm exec
-node_modules/.bin/pretty-quick --staged
-node_modules/.bin/lint-staged
+pnpm exec pretty-quick --staged
+pnpm exec lint-staged📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| node_modules/.bin/pretty-quick --staged | |
| node_modules/.bin/lint-staged | |
| pnpm exec pretty-quick --staged | |
| pnpm exec lint-staged |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.husky/pre-commit around lines 4 - 5, Replace direct node_modules/.bin
invocations in the pre-commit hook with pnpm exec to align with pnpm
conventions: locate the two commands where node_modules/.bin/pretty-quick
--staged and node_modules/.bin/lint-staged are invoked and change them to use
pnpm exec (i.e., run the same tools via pnpm exec pretty-quick --staged and pnpm
exec lint-staged) so pnpm resolves and runs the executables.
| "build:ci": "set CI=true&& yarn build", | ||
| "build:ci:alpha": "yarn upgrade bloom-player --network-timeout 1000000000 && yarn build:ci", | ||
| "build": "pnpm copyInBloomPlayer && vite build", | ||
| "build:ci": "set CI=true&& pnpm build", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find affected scripts:"
rg -n '"(build:ci|test:ci)"\s*:\s*"set CI=true&&' package.json
echo "Demonstrate POSIX behavior:"
sh -c 'set CI=true&& [ -n "${CI:-}" ] && echo "CI is set" || echo "CI is NOT set"'
# Expected: "CI is NOT set"Repository: BloomBooks/BloomLibrary2
Length of output: 229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Current process CI env:"
printf 'CI=%q\n' "${CI-}"
echo
echo "Test 1: 'set CI=true&&' with CI explicitly unset (should show CI empty if claim is correct):"
env -u CI sh -c 'set CI=true && printf "CI=%s; positional $1=%s\n" "${CI-}" "${1-}"'
echo
echo "Test 2: check whether CI becomes set via 'set CI=true&&' by examining ${CI:-}:"
env -u CI sh -c 'set CI=true && [ -n "${CI:-}" ] && echo "CI is set" || echo "CI is NOT set"; [ -n "${1:-}" ] && echo "positional set ($1=$1)"'
echo
echo "Test 3: inline env assignment 'CI=true ...' with CI explicitly unset:"
env -u CI sh -c 'CI=true pnpm -v >/dev/null 2>&1 || true; printf "CI=%s; positional $1=%s\n" "${CI-}" "${1-}"'Repository: BloomBooks/BloomLibrary2
Length of output: 439
Fix CI env assignment in build:ci/test:ci
set CI=true&& ... does not set the CI environment variable for the following pnpm command (in POSIX sh, set changes positional parameters). Replace with inline env assignment.
💡 Suggested fix
- "build:ci": "set CI=true&& pnpm build",
+ "build:ci": "CI=true pnpm build",
...
- "test:ci": "set CI=true&& pnpm test",
+ "test:ci": "CI=true pnpm test",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "build:ci": "set CI=true&& pnpm build", | |
| "build:ci": "CI=true pnpm build", | |
| "build:ci:alpha": "bloom-player@alpha pnpm build", | |
| ... | |
| "test:ci": "CI=true pnpm test", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 10, The npm scripts "build:ci" and "test:ci" use "set
CI=true&& pnpm ..." which doesn't export CI to the command in POSIX shells;
replace those with an inline env assignment so the CI variable is set for the
pnpm invocation (e.g., change the "build:ci" and "test:ci" script values from
the current "set CI=true&& pnpm ..." form to use "CI=true pnpm ..." so the
environment variable is applied to the subsequent command).
This change is
Summary by CodeRabbit