ci: use Node LTS in CI workflows

[ascorbic]

Summary

• Switch CI workflows from node-version: latest to node-version: lts/*
• Add explicit node-version: lts/* to test workflow (was missing)

[claude]

Code Review for PR #9: ci: use Node LTS in CI workflows

Summary

This PR standardizes the Node.js version across CI workflows to use LTS (Long Term Support) instead of latest. Overall, this is a positive change that improves stability and predictability.

✅ Strengths

1. Better CI stability: Using lts/* instead of latest prevents unexpected breaking changes from new Node.js releases affecting builds
2. Consistency improvement: The test workflow was missing an explicit node-version specification, which this PR corrects
3. Industry best practice: Using LTS for CI is a standard recommendation for production workflows
4. Minimal risk: Very low-risk change with clear benefits

📋 Observations

Positive alignment:

• The check-latest: true flag in test.yml (.github/workflows/test.yml:20) ensures the latest patch version of the LTS is used, which is good for security
• The change aligns with the repository's pnpm version pinning approach (using pnpm@10.18.2)

Minor notes:

• No engines field in package.json files to formally declare Node version requirements (not blocking, but could be added separately)
• The release workflow doesn't have check-latest: true like the test workflow does (consider adding for consistency)

🔍 Technical Review

Code Quality: ✅ Excellent

• Clean, minimal diff
• No logic changes, only configuration updates

Potential Bugs: ✅ None identified

• This change reduces the likelihood of bugs caused by Node version discrepancies

Performance: ✅ Neutral

• No performance impact; LTS versions are well-optimized

Security: ✅ Improved

• LTS versions receive security patches longer than current releases
• check-latest: true in test workflow ensures latest patches are applied

Test Coverage: ✅ Adequate

• No new tests needed for CI configuration changes
• The workflows themselves will validate the change works

💡 Optional Enhancements (for future PRs)

1. Consider adding an engines field to package.json to document Node version requirements
2. Consider adding check-latest: true to release.yml for consistency with test.yml
3. Consider documenting the required Node version in the README if not already present

✅ Recommendation

LGTM - This PR is ready to merge. It's a straightforward, low-risk improvement that follows CI best practices.

---

Review generated by Claude Code

[claude]

PR Review: CI Node.js Version Updates

Summary

This PR improves CI workflow configuration by standardizing on Node.js LTS and modernizing the pnpm setup action. Overall, this is a solid improvement with good intentions.

✅ Positive Changes

1. Node.js LTS Standardization

• ✅ Switching from node-version: latest to node-version: lts/* is a best practice
• ✅ LTS provides better stability and predictability for CI builds
• ✅ Adds explicit node-version: lts/* to test workflow (previously missing)
• ✅ Consistent across both test.yml and release.yml

2. pnpm Action Update

• ✅ Upgrading from pnpm/action-setup@v2 to @v3 is good for security and features
• ✅ Removes hardcoded version: 8.15.9 in favor of reading from packageManager field
• ✅ Correctly leverages the packageManager: "pnpm@10.18.2" field in root package.json (line 27)

🔍 Code Quality

Excellent: The changes follow GitHub Actions best practices:

• Uses version selectors (lts/*) instead of hardcoded versions
• Leverages package.json packageManager field for version management
• Maintains consistency across workflows

🐛 Potential Issues

None identified. The changes are straightforward and low-risk:

• Both workflows should work correctly with these changes
• pnpm/action-setup@v3 automatically reads packageManager from package.json
• corepack enable in test.yml ensures pnpm version is respected

⚡ Performance Considerations

Neutral to slightly positive:

• LTS versions are well-cached by GitHub Actions runners
• No performance degradation expected
• Potential minor improvement from pnpm action v3 optimizations

🔒 Security Concerns

None. Actually improves security posture:

• Using LTS reduces exposure to bleeding-edge Node.js bugs
• Upgrading to @v3 of pnpm action includes latest security patches
• No secrets or sensitive data handling changed

🧪 Test Coverage

N/A: This PR only modifies CI configuration files:

• Changes are self-validating (CI runs will confirm they work)
• No application code modified, so no new tests needed
• Recommend verifying CI passes on this PR before merging

📝 Minor Observations

1. test.yml cleanup: Removing the hardcoded pnpm version is cleaner, but ensure corepack enable (line 22) correctly respects the packageManager field - this should work fine with action-setup@v3.

2. Consistency check: Both workflows now use:

   • pnpm/action-setup@v3 ✅
   • node-version: lts/* ✅

   This is perfect for maintainability.

🎯 Recommendation

APPROVE - This is a clean, well-scoped PR that improves CI reliability. No blocking issues found.

Suggested verification:

• Confirm CI checks pass on this PR
• Monitor that pnpm version 10.18.2 is correctly installed (should be visible in CI logs)

Great work on improving the CI setup! 🚀