chore: lower minimum Node version to 18#14
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
✨ 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR lowers the declared minimum supported Node.js version from Node 20 to Node 18 across the repo’s root package and the cli/ and mcp/ workspaces, aiming to broaden compatibility for contributors and downstream users.
Changes:
- Updated
engines.nodein the rootpackage.jsonand thecli/andmcp/workspacepackage.jsonfiles to require Node>=18. - Updated
mcp/package-lock.jsonto reflect the newengines.nodevalue.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| package.json | Lowers the root package’s declared Node engine requirement to >=18.0.0. |
| cli/package.json | Lowers the CLI package’s declared Node engine requirement to >=18.0.0. |
| mcp/package.json | Lowers the MCP package’s declared Node engine requirement to >=18. |
| mcp/package-lock.json | Syncs MCP lockfile metadata to the lowered Node engine requirement. |
Files not reviewed (1)
- mcp/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| "type": "module", | ||
| "engines": { "node": ">=20.0.0" }, | ||
| "engines": { "node": ">=18.0.0" }, |
| "engines": { | ||
| "node": ">=20.0.0" | ||
| "node": ">=18.0.0" | ||
| }, |
| }, | ||
| "type": "module", | ||
| "engines": { "node": ">=20.0.0" }, | ||
| "engines": { "node": ">=18.0.0" }, |
| "type": "module", | ||
| "engines": { | ||
| "node": ">=20" | ||
| "node": ">=18" |
| "engines": { | ||
| "node": ">=20" | ||
| "node": ">=18" | ||
| }, |
263c2b8 to
63be57d
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Updates engines from >=20.0.0 to >=18.0.0 across root, cli, and mcp packages to broaden contributor and user reach. All 166 tests pass on the existing Node 22 dev environment. Note: c8 v11 (dev-only, used by 'test:coverage') still requires Node 20+; contributors on Node 18 can run 'npm test' but not 'npm run test:coverage'.
- Sync engines metadata in package-lock.json (root + cli) to match the lowered package.json minimum (>=18.0.0). - Use full semver '>=18.0.0' in mcp/package.json for consistency with root and cli (was '>=18'). - Lower @types/node to ^18.19.0 in mcp to match the runtime minimum, preventing accidental use of Node 20-only APIs in TypeScript. - All 27 mcp tests still pass.
63be57d to
70981b2
Compare
|
Triage of the 5 Copilot review comments — 4 addressed in
Generated by Claude Code |
* ci: add Node matrix workflow (18.x, 20.x, 22.x) Complements validate.yml (which pins to .nvmrc=20) by smoke-testing 'npm ci && npm test' on the full engine band declared in package.json (>= 18.0.0 after #14). Catches breakage on newer Node versions before downstream users hit it. Originally proposed by @cryptofixyup in #13. Refinements vs the original: - Renamed to node-matrix.yml so the intent is explicit alongside validate.yml, smoke.yml, etc. - Pinned actions to the versions used elsewhere in the repo (checkout@v6, setup-node@v6) per the actions group bump in #11. - Dropped 'npm run build --if-present' (no build script in root). - Added 'permissions: contents: read' to follow the principle of least privilege. - Added concurrency group to cancel superseded runs. - Added 'fail-fast: false' so all matrix legs report independently rather than aborting on the first failure. Co-authored-by: cryptofixyup <199330534+cryptofixyup@users.noreply.github.com> * fix(ci): Node 18/20 glob expansion + add publish-time version regression test - npm test/test:coverage scripts now use bash glob expansion via `bash -c 'shopt -s nullglob; node --test scripts/__tests__/*.test.mjs'` so Node 18/20 (which don't support glob in --test) work alongside Node 22/24. - New scripts/check-versions.mjs runs before each publish workflow (cli, mcp, pysdk) and asserts that the package.json version matches the tag-implied version. Catches the class of bug where a tag like cli-v0.2.0 publishes a 0.1.1 package because version bump was missed. 12 regression tests cover semver malformedness, tag/version mismatch, refs/tags/ prefix tolerance, pyproject.toml read path, and unknown-prefix tolerance. Closes #15. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: cryptofixyup <199330534+cryptofixyup@users.noreply.github.com>
Summary
Lowers the declared minimum Node version from
>=20.0.0to>=18.0.0across the three packages (/,/cli,/mcp). Broadens contributor and downstream-user reach — Node 18 is still in maintenance until April 2025 and is the default in many CI presets.Changes
package.json—engines.node:>=20.0.0→>=18.0.0cli/package.json—engines.node:>=20.0.0→>=18.0.0mcp/package.json—engines.node:>=20→>=18mcp/package-lock.json— synced to matchVerification
All 166 tests green on the dev environment (Node 22):
npm test— 114/114 (root)npm test --prefix cli— 25/25npm test --prefix mcp— 27/27Code uses only APIs available in Node 18 (global
fetchis experimental in 18, stable in 21 — works in both).Caveat
c8v11 (just merged in #9) declaresengines: ^20.18.0 || >=22.10.0. It's a dev-only coverage tool, so:npm testworks on Node 18npm run test:coveragewill warn and likely fail on Node 18If full Node 18 parity is desired we could pin
c8to^10(loses the minimatch CVE-2026-26996 fix from v11). Open to either direction.Test plan
npm ci+npm testworks on Node 18 in CI (would benefit from a Node-matrix workflow — see also Add Node.js CI workflow #13)🤖 Generated with Claude Code
Generated by Claude Code