chore: migrate to mise + bump yarn to 4.14.1 - #830
Conversation
- Pin Node and Yarn 4.14.1 in mise.toml - Drop Volta from package.json - Switch to corepack-managed yarn via packageManager field - Regenerate yarn.lock from scratch (yarn 1 v1 format -> yarn 4 v9) - enableScripts off (default); allowlist lefthook via dependenciesMeta - Replace setup-node + cache:'yarn' across 4 yarn-using workflows (integrity-check, validate-community-plugins, validate-orchestrator, validate-orchestrator-integration) with the standard cache pattern - Add yarn 4 (berry) gitignore rules
Keep PR scoped to tooling migration only; dist/ rebuild and prettier auto-wraps in build-tests-* workflows are unrelated drive-by changes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCI and repository tooling were updated to use Corepack + Yarn v4 tooling: tooling declarations and Yarn configs were added/changed, .gitignore adjusted for Yarn 4, and GitHub workflows now enable Corepack, restore explicit Yarn caches and run ChangesYarn / CI package-manager migration
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Corepack as Corepack
participant SetupNode as actions/setup-node@v4
participant Cache as actions/cache@v4
participant Yarn as Yarn (tool)
participant Repo as Repository (checkout)
GH->>Repo: checkout
GH->>Corepack: corepack enable/install
Corepack->>Yarn: install/configure specified Yarn
GH->>SetupNode: setup Node (no yarn cache)
GH->>Yarn: yarn config get cacheFolder
GH->>Cache: restore cache (cacheFolder + .yarn/install-state.gz)
GH->>Yarn: yarn --version (guard)
GH->>Yarn: YARN_ENABLE_HARDENED_MODE=false
GH->>Yarn: yarn install --immutable
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/validate-orchestrator.yml:
- Around line 86-92: The workflow runs corepack enable/install before
actions/setup-node@v4, causing Corepack to bind to the wrong Node runtime;
reorder the steps so the setup-node step (uses: actions/setup-node@v4 with
node-version: 20) appears before the Corepack bootstrap (the steps that run
"corepack enable" and "corepack install") in validate-orchestrator.yml and in
all five affected jobs in validate-orchestrator-integration.yml so Corepack
configures shims against Node 20.
In @.yarnrc.yml:
- Around line 1-2: Remove or restrict the approved git repository list in
.yarnrc.yml: locate the approvedGitRepositories entry and either delete that key
entirely (since the project uses only npm registry dependencies) or replace the
wildcard value ('**') with a scoped list of explicit trusted git repo URLs;
ensure the file no longer permits arbitrary git repositories if you choose
deletion, or document the allowed repos when scoping.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1e8500dc-67a4-401f-9807-270f14c97485
⛔ Files ignored due to path filters (4)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.github/workflows/integrity-check.yml.github/workflows/validate-orchestrator-integration.yml.github/workflows/validate-orchestrator.yml.gitignore.yarnrc.yarnrc.ymlmise.tomlpackage.json
💤 Files with no reviewable changes (1)
- .yarnrc
| - name: Install package manager (from package.json) | ||
| run: | | ||
| corepack enable | ||
| corepack install | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/validate-orchestrator.yml | sed -n '80,100p'Repository: game-ci/unity-builder
Length of output: 927
🏁 Script executed:
find .github/workflows -name "*integration*" -type fRepository: game-ci/unity-builder
Length of output: 121
🏁 Script executed:
grep -n "corepack\|setup-node" .github/workflows/validate-orchestrator-integration.yml | head -20Repository: game-ci/unity-builder
Length of output: 570
🏁 Script executed:
cat -n .github/workflows/validate-orchestrator-integration.yml | grep -A 10 -B 5 "corepack enable"Repository: game-ci/unity-builder
Length of output: 3386
Move setup-node ahead of Corepack bootstrap in validate-orchestrator.yml and integration workflows.
Currently, corepack enable/install runs against the ambient Node runtime on the runner, then actions/setup-node@v4 installs Node 20 afterward. This causes Corepack to configure package manager shims against the wrong Node version. Setup Node 20 first so Corepack binds to the correct runtime.
This ordering issue appears in validate-orchestrator.yml (lines 86–92) and in 5 separate jobs in validate-orchestrator-integration.yml. All instances should be reordered: setup-node before the Corepack steps.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/validate-orchestrator.yml around lines 86 - 92, The
workflow runs corepack enable/install before actions/setup-node@v4, causing
Corepack to bind to the wrong Node runtime; reorder the steps so the setup-node
step (uses: actions/setup-node@v4 with node-version: 20) appears before the
Corepack bootstrap (the steps that run "corepack enable" and "corepack install")
in validate-orchestrator.yml and in all five affected jobs in
validate-orchestrator-integration.yml so Corepack configures shims against Node
20.
| approvedGitRepositories: | ||
| - '**' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the .yarnrc.yml file
cat -n .yarnrc.ymlRepository: game-ci/unity-builder
Length of output: 273
🏁 Script executed:
# Check if there are any git-based dependencies in package.json
cat -n package.json | head -100Repository: game-ci/unity-builder
Length of output: 2417
🏁 Script executed:
# Look for git dependencies in yarn.lock if it exists
if [ -f yarn.lock ]; then
echo "=== Searching for git dependencies in yarn.lock ==="
rg -i "git" yarn.lock -A 2 | head -50
else
echo "No yarn.lock found"
fiRepository: game-ci/unity-builder
Length of output: 1393
🏁 Script executed:
# Check if there are workspaces or other configuration that might require git deps
fd -e "*.json" -e "*.yml" -e "*.yaml" | grep -E "(package|yarn|config)" | head -20Repository: game-ci/unity-builder
Length of output: 47
Scope or remove the git repository approval list.
The project has no git-based dependencies (all dependencies use npm registry references). The approvedGitRepositories: ['**'] setting is unnecessary and permits arbitrary git repositories; remove this setting entirely or scope it to specific trusted repositories if git dependencies become needed in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.yarnrc.yml around lines 1 - 2, Remove or restrict the approved git
repository list in .yarnrc.yml: locate the approvedGitRepositories entry and
either delete that key entirely (since the project uses only npm registry
dependencies) or replace the wildcard value ('**') with a scoped list of
explicit trusted git repo URLs; ensure the file no longer permits arbitrary git
repositories if you choose deletion, or document the allowed repos when scoping.
Yarn 4 strict layout doesn't expose @typescript-eslint/eslint-plugin or eslint-plugin-import which were resolved transitively under yarn 1. Add them as explicit devDeps. Same for @jest/globals (used by jest-fail-on-console). Disable unicorn/no-useless-undefined which now fires on existing mockResolvedValue(undefined) calls. The undefined arg is required by @types/jest 27 typings (removing it breaks tsc), so the rule and the typecheck disagree. Pre-existing under yarn 1 these calls compiled because the eslint config was effectively non-functional (plugin resolution failed silently) so the rule never ran.
@types/jest 27 typings require explicit .mockResolvedValue(undefined), which the rule wants removed; tsc and eslint disagreed. Disable the rule to match upstream behaviour (it only fires now because yarn 4 exposes the eslint plugin tree that yarn 1 silently broke).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.eslintrc.json (1)
81-83: Scope this lint disable to Jest test files instead of disabling it globally.Disabling
unicorn/no-useless-undefinedglobally weakens checks across production code. Onlysrc/index-plugin-features.test.tsusesmockResolvedValue(undefined)due to@types/jestv27 typings. Enable this rule globally and disable it only in test files where required.Proposed config change
"rules": { // ... other rules ... - // Temp disable to prevent mixing changes with other PRs - // Disabled: `@types/jest` 27 typings require .mockResolvedValue(undefined), - // contradicting this rule - "unicorn/no-useless-undefined": "off", + // Keep enabled globally; disable only in Jest test files + "unicorn/no-useless-undefined": "error", "i18n-text/no-en": "off" }, "overrides": [ { "files": ["jest.setup.js"], "rules": { "import/no-commonjs": "off" } + }, + { + "files": ["**/*.test.ts"], + "rules": { + "unicorn/no-useless-undefined": "off" + } } ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.json around lines 81 - 83, The global disable of the rule unicorn/no-useless-undefined should be removed and instead added under an ESLint overrides block scoped to the Jest test(s) that need it; restore unicorn/no-useless-undefined in the main config (e.g., set it back to "error" or remove the "off" entry) and add an overrides entry targeting "src/index-plugin-features.test.ts" (or a Jest pattern like "**/*.test.ts"), setting "unicorn/no-useless-undefined": "off" there so only the test file using mockResolvedValue(undefined) is exempted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.eslintrc.json:
- Around line 81-83: The global disable of the rule unicorn/no-useless-undefined
should be removed and instead added under an ESLint overrides block scoped to
the Jest test(s) that need it; restore unicorn/no-useless-undefined in the main
config (e.g., set it back to "error" or remove the "off" entry) and add an
overrides entry targeting "src/index-plugin-features.test.ts" (or a Jest pattern
like "**/*.test.ts"), setting "unicorn/no-useless-undefined": "off" there so
only the test file using mockResolvedValue(undefined) is exempted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ce53ccc4-d42d-4ec0-9790-fe5cf2e59a4f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
.eslintrc.jsonpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #830 +/- ##
=======================================
Coverage 70.89% 70.89%
=======================================
Files 28 28
Lines 876 876
Branches 239 239
=======================================
Hits 621 621
Misses 255 255 🚀 New features to boost your workflow:
|
The restore-key pattern matched an older cache that had @types/tar + minipass 3.x in node_modules from before the lockfile regen. Fresh installs end up with that stale tree and tsc fails on incompatible types. Versioning the key forces a clean cache.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/integrity-check.yml (1)
26-32:corepack enable/installruns beforeactions/setup-node@v4— consider reordering.The two
corepackinvocations execute against the runner's pre-installed Node.js (Node 20 onubuntu-latest) beforesetup-node@v4activates Node 18. In practice this tends to work on ubuntu-latest because corepack shims land in/usr/local/bin, which stays inPATHafter the Node 18 toolcache bin is prepended. The corepack binary store is also Node-version-agnostic, so the downloaded Yarn 4 persists across the version switch.The canonical ordering recommended by corepack documentation is: set up the target Node version first, then enable and install corepack — so shims are guaranteed to be scoped to the active Node version:
♻️ Proposed reorder
- - name: Install package manager (from package.json) - run: | - corepack enable - corepack install - uses: actions/setup-node@v4 with: node-version: '18' + - name: Install package manager (from package.json) + run: | + corepack enable + corepack installThis pattern appears across all affected workflows (
integrity-check.yml,validate-orchestrator.yml, andvalidate-orchestrator-integration.yml), so if you adopt this change, apply it consistently across the board.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/integrity-check.yml around lines 26 - 32, Move the Node setup step to run before invoking corepack so corepack is enabled/installed against the intended Node version: in the job steps reorder the actions/setup-node@v4 step (node-version: '18') to appear before the shell block that runs "corepack enable" and "corepack install"; update the same pattern in validate-orchestrator.yml and validate-orchestrator-integration.yml to keep behavior consistent across workflows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/integrity-check.yml:
- Around line 26-32: Move the Node setup step to run before invoking corepack so
corepack is enabled/installed against the intended Node version: in the job
steps reorder the actions/setup-node@v4 step (node-version: '18') to appear
before the shell block that runs "corepack enable" and "corepack install";
update the same pattern in validate-orchestrator.yml and
validate-orchestrator-integration.yml to keep behavior consistent across
workflows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0b256198-31b5-4521-8efc-4ebcc5d2d3d3
📒 Files selected for processing (3)
.github/workflows/integrity-check.yml.github/workflows/validate-orchestrator-integration.yml.github/workflows/validate-orchestrator.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/validate-orchestrator.yml
- .github/workflows/validate-orchestrator-integration.yml
Earlier revert commit only fixed workflows; dist/ stayed rebuilt. The newer ncc bundle output triggers 2 high-severity CodeQL alerts (URL substring sanitization + escape sanitization) that don't fire on the main branch's dist/. Restore main's dist/ so CodeQL passes.
Caching node_modules causes stale trees to leak across yarn.lock changes (e.g. @types/tar persisting after a regen). Cache only the yarn cacheFolder + install-state.gz; yarn install rebuilds node_modules from those (fast).
2 participants
mise.tomlpackage.jsonpackageManagerfieldyarn.lockfrom scratch (yarn 1 v1 format → yarn 4 v9)enableScriptsoff (default); allowlistlefthookviadependenciesMetasetup-node+cache: 'yarn'across 4 yarn-using workflows (integrity-check,validate-community-plugins,validate-orchestrator,validate-orchestrator-integration) with the standard cache patternSummary by CodeRabbit