Fix Node.js 22 V8 crash in GitHub Actions workflows

[justin808]

Summary

• Disable yarn cache for Node.js 22 in all affected workflows
• Works around V8 crash bug in Node.js 22.21.0
• Prevents "Fatal error in , line 0 # unreachable code" during CI runs

Problem

Node.js 22.21.0 has a V8 bug that causes crashes when yarn cache dir is executed during the setup-node@v4 cache detection step. This was causing CI failures in multiple workflows.

Reference: nodejs/node#56010

Solution

Disabled yarn cache for Node.js 22 by setting cache: '' in the setup-node configuration, similar to the fix already applied in main.yml.

Modified Workflows

• lint-js-and-ruby.yml
• package-js-tests.yml
• pro-integration-tests.yml (3 occurrences)
• pro-lint.yml
• pro-package-tests.yml (2 occurrences)

Test Plan

• All workflow files updated consistently
• RuboCop passes
• Pre-commit hooks pass
• CI workflows run successfully without V8 crashes

🤖 Generated with Claude Code

---

This change is 

[coderabbitai]

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 46 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between ca6366e and 43919ad.

📒 Files selected for processing (5)

• .github/workflows/lint-js-and-ruby.yml (1 hunks)
• .github/workflows/package-js-tests.yml (1 hunks)
• .github/workflows/pro-integration-tests.yml (3 hunks)
• .github/workflows/pro-lint.yml (1 hunks)
• .github/workflows/pro-package-tests.yml (2 hunks)

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/fix-node22-crash

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review - PR #1922: Fix Node.js 22 V8 crash in GitHub Actions workflows

Summary

This PR addresses a critical CI infrastructure issue by disabling yarn cache for Node.js 22 across multiple workflow files to work around a V8 crash bug in Node.js 22.21.0.

---

✅ Strengths

1. Comprehensive Coverage: All affected workflows using Node.js 22 have been updated consistently:

   • lint-js-and-ruby.yml
   • package-js-tests.yml
   • pro-integration-tests.yml (3 jobs)
   • pro-lint.yml
   • pro-package-tests.yml (2 jobs)

2. Consistent Implementation: The fix follows the same pattern already established in main.yml, maintaining consistency across the codebase.

3. Good Documentation:

   • Clear inline comments explaining the workaround
   • Reference to the upstream Node.js issue (#56010)
   • Well-written PR description with context and test plan

4. Appropriate Conditional Logic: The package-js-tests.yml correctly uses conditional syntax to disable cache only for Node 22 while preserving it for other versions in the matrix.

---

🔍 Observations

1. Workflows Not Requiring Changes (verified as correct):

   • examples.yml: Uses Node.js 20 only ✓
   • rspec-package-specs.yml: No Node.js setup step ✓
   • main.yml: Already fixed previously ✓

2. Minor inconsistency: Different approaches used:

   • Fixed Node 22: cache: '' (simple string)
   • Matrix with Node 22: cache: ${{ matrix.node-version != '22' && 'yarn' || '' }} (conditional)

   Both are valid, but this is appropriate given the different contexts.

---

🎯 Code Quality Assessment

• Best Practices: ✅ Follows GitHub Actions best practices
• Security Concerns: ✅ None identified (only disabling caching, which is a performance optimization)
• Performance Considerations: ⚠️ Minor performance impact from disabled caching, but necessary workaround
• Test Coverage: ✅ PR description includes test plan; CI runs will validate the fix

---

💡 Suggestions

1. Future Cleanup Task: Consider adding a TODO comment or GitHub issue to track reverting this workaround once Node.js fixes the V8 bug. This will help ensure the cache is re-enabled when safe to do so.

   Example:

   # TODO: Re-enable cache when Node.js 22 V8 bug is fixed
   # Track: https://github.com/nodejs/node/issues/56010

2. Monitoring: After merging, monitor the linked Node.js issue to determine when the cache can be safely re-enabled for Node.js 22.

---

✅ Verdict

LGTM - This PR correctly addresses the V8 crash issue with a safe, well-documented workaround. The implementation is consistent, comprehensive, and follows established patterns in the codebase.

The minor performance impact from disabled caching is acceptable given the alternative is CI failures. Once the upstream Node.js bug is resolved, the cache can be re-enabled.

---

Recommendation: ✅ Approve and merge after CI passes

🤖 Review generated with Claude Code

[claude]

Code Review

Summary

This PR successfully addresses the Node.js 22.21.0 V8 crash issue by disabling yarn cache for Node.js 22 in GitHub Actions workflows. The changes are well-documented and consistent with the existing pattern in main.yml.

---

✅ Strengths

1. Consistency: The fix correctly mirrors the pattern already established in main.yml:72-74, ensuring uniformity across all workflows.

2. Documentation: Excellent use of inline comments with:

   • Clear TODO for re-enabling when bug is fixed
   • Explanation of why the change is needed
   • Reference link to upstream issue tracker

3. Appropriate Scope: All affected workflow files have been identified and updated (5 files, 8 occurrences total).

4. Conditional Logic: Smart approach in package-js-tests.yml using conditional expression to only disable cache for Node 22:

   cache: ${{ matrix.node-version != '22' && 'yarn' || '' }}

   This ensures other Node versions still benefit from caching.

---

🔍 Code Quality Observations

Inconsistency in Approach: There are two different patterns used:

1. Static approach (lint-js-and-ruby.yml, pro-*.yml files):

   cache: ''

2. Conditional approach (package-js-tests.yml):

   cache: ${{ matrix.node-version != '22' && 'yarn' || '' }}

Recommendation: Consider standardizing on one approach for consistency:

• If workflows only use Node 22 → Static cache: '' is fine (cleaner, simpler)
• If workflows use matrix with multiple Node versions → Use conditional expression

Currently, all modified workflows appear to use Node 22 exclusively except package-js-tests.yml which has a matrix. The current implementation is actually correct and optimal - using the right pattern for each scenario.

---

🛡️ Security & Performance

Performance Impact: ⚠️ Minor negative impact

• Disabling cache will increase CI runtime for Node.js 22 jobs
• Each run will need to download yarn dependencies from scratch
• This is an acceptable tradeoff to prevent V8 crashes

Security: ✅ No concerns

• Changes are limited to CI configuration
• No code execution changes
• Uses empty string for cache, which is the documented way to disable caching in setup-node@v4

---

📋 Test Coverage

The PR description mentions:

• ✅ All workflow files updated consistently
• ✅ RuboCop passes
• ✅ Pre-commit hooks pass
• ⏳ CI workflows run successfully without V8 crashes (pending)

Recommendation: After merge, monitor the first few CI runs to confirm:

1. No V8 crashes occur
2. CI runtime increase is acceptable
3. Yarn install completes successfully without cache

---

🐛 Potential Issues

None identified. The implementation is sound.

---

💡 Suggestions for Improvement

1. Comment consistency: In main.yml, the comment format is slightly different (2 lines vs 3 lines). Consider making all comments match the same format for consistency:

   Current in main.yml:

   # Disable cache for Node 22 due to V8 bug in 22.21.0
   # https://github.com/nodejs/node/issues/56010

   Current in this PR:

   # TODO: Re-enable cache when Node.js 22 V8 bug is fixed
   # Disable cache for Node 22 due to V8 bug in 22.21.0
   # Track: https://github.com/nodejs/node/issues/56010

   The PR version is actually better because it includes the TODO reminder. Consider updating main.yml to match this format in a follow-up.

2. Future cleanup: Add a calendar reminder or GitHub issue to check the Node.js issue tracker in a few weeks and re-enable caching when the bug is fixed.

---

✅ Verdict

LGTM - Approve with minor suggestion

This is a well-executed workaround for a known upstream bug. The changes are:

• ✅ Correct and complete
• ✅ Well-documented
• ✅ Consistent with existing patterns
• ✅ Minimal risk
• ✅ Appropriate for the problem being solved

The only suggestion is to consider updating main.yml to use the same 3-line comment format for consistency (optional, non-blocking).

Great work! 🚀

[claude]

Code Review - PR #1922

Summary

This PR addresses a critical V8 crash bug in Node.js 22.21.0 that affects GitHub Actions workflows. The fix disables yarn caching for Node.js 22 across multiple workflow files.

✅ Strengths

1. Consistent Implementation: The fix is applied consistently across all 5 affected workflow files with identical comment patterns
2. Well-Documented: Each change includes:
   • Clear TODO comment for re-enabling cache when fixed
   • Reference to the upstream Node.js issue (#56010)
   • Explanatory comment about the V8 bug
3. Follows Existing Pattern: Matches the fix already implemented in main.yml (lines 72-74)
4. Smart Conditional Logic: In package-js-tests.yml, uses a conditional expression to only disable cache for Node 22 while keeping it enabled for other versions in the matrix
5. Complete Coverage: All workflow files using Node.js 22 are updated (verified by searching the codebase)

🎯 Code Quality

Architecture: ✅ Sound approach

• Disabling cache is the right workaround for a runtime crash
• Performance impact is acceptable given it prevents complete CI failures
• Clean separation between Node 20 and Node 22 handling

Consistency: ✅ Excellent

• All Node 22 instances use the same fix pattern
• Comments are consistent across files
• Aligns with existing fix in main.yml

Maintainability: ✅ Good

• TODO comments ensure this workaround will not be forgotten
• Clear tracking link to upstream issue
• Easy to revert when Node.js releases a fix

🔍 Detailed Analysis

package-js-tests.yml (lines 63-66)

cache: ${{ matrix.node-version \!= '22' && 'yarn' || '' }}

This is particularly elegant - it conditionally disables cache only for Node 22 while preserving performance for other Node versions in the test matrix. This is the preferred pattern when dealing with matrix strategies.

Other Workflow Files

The remaining 4 files use hardcoded Node 22, so they use the simpler cache: '' approach, which is appropriate.

🐛 Potential Issues

None identified. The implementation is solid.

⚡ Performance Considerations

Impact: Moderate but acceptable

• Disabling yarn cache will increase CI runtime for Node 22 jobs
• Each affected job will need to download dependencies from scratch
• This is a reasonable tradeoff to prevent workflow crashes

Recommendation: Monitor the upstream Node.js issue and re-enable caching as soon as a fix is released in Node.js 22.x.

🔒 Security Concerns

None. This change only affects CI caching behavior and does not introduce security risks.

🧪 Test Coverage

Status: ✅ Adequate for this change

• The PR author notes that pre-commit hooks and RuboCop pass
• The real test will be CI execution, which should now complete without crashes
• No additional tests needed for a workflow configuration change

📋 Suggestions

1. Consider adding a reminder mechanism: Since this is a temporary workaround, consider:

   • Adding a calendar reminder to check the Node.js issue in 1-2 months
   • Or creating a follow-up GitHub issue to track re-enabling the cache

2. Minor: Consider version-specific cache key: The current simple approach is easier to maintain and perfectly acceptable.

✅ Verification Checklist

• ✅ All workflow files with Node.js 22 are updated
• ✅ Consistent comment patterns across all changes
• ✅ Follows existing pattern from main.yml
• ✅ No formatting issues (YAML is properly structured)
• ✅ Conditional logic in package-js-tests.yml is correct
• ✅ TODO comments added for future cleanup
• ✅ Tracking link included

🎬 Recommendation

APPROVE - This PR is ready to merge.

The implementation is solid, well-documented, and follows established patterns. The workaround is appropriate for the severity of the issue (complete CI failures). The only thing remaining is to verify that CI runs successfully after merge, which will validate the fix in practice.

---

Review conducted following repository conventions from CLAUDE.md
🤖 Generated with Claude Code