Repository navigation
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 39 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Package selection and tarball matrix .github/workflows/engines.yml |
The workflow selects non-private workspace packages, validates each engines.node floor, runs available prepublishOnly scripts, packs the packages, and builds a matrix that includes workspace dependency tarballs. |
Install packages and discover exports .github/workflows/engines.yml |
For each package and Node.js floor, the workflow installs the tarballs with npm --engine-strict and discovers eligible entry points from the installed package’s exports. |
Load and validate entry points .github/workflows/engines.yml |
The workflow loads each discovered entry point. It uses dynamic import for specified require errors and fails if a load fails or returns undefined. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Other
Sequence Diagram(s)
sequenceDiagram
participant Workflow as GitHub Actions
participant Pack as pack job
participant Artifact as tarballs artifact
participant Floor as floor job
participant Npm as npm
participant Exports as package exports
Workflow->>Pack: generate package matrix and tarballs
Pack->>Artifact: upload tarballs
Artifact->>Floor: provide tarballs for each Node floor
Floor->>Npm: install packages with engine-strict
Floor->>Exports: discover and load entry points
Possibly related PRs
- microlinkhq/tinyspawn#37: PR
#37standardizes pnpm build approval through workflow environment variables; this workflow uses the related pnpm install configuration.
Merge Risk: 
ac192
-
The workflow adds a Node-floor installation and load check. No merge-blocking issue was established.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding CI coverage to check packages on their minimum supported Node.js version. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinyspawn
Details
| Benchmark suite | Current: eb22277 | Previous: 8fe68a1 | Ratio |
|---|---|---|---|
spawn node -e 0 |
339.3 ms (330-357) |
341.6 ms (327.9-351.3) |
0.99 |
spawn 50MB stdout |
95 ms (94.3-98.3) |
94.8 ms (93.6-98.2) |
1.00 |
read 50MB stdout x30 |
0 ms (0-0) |
0 ms (0-0) |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
94a6062 to
aca15fd
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aca15fd to
eb22277
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eb22277. Configure here.
|
|
||
| const OUT = path.resolve('tarballs') | ||
| const FLOOR = /^>=\s*(\d+(?:\.\d+){0,2})$/ | ||
| const WORKSPACE_DEPENDENCY_FIELDS = ['dependencies', 'optionalDependencies'] |
There was a problem hiding this comment.
Optional workspace tarballs installed as required
Medium Severity
optionalDependencies workspace packages are packed into the consumer install and passed to npm install --engine-strict as top-level required tarballs. npm skips engine mismatches for optional deps, so a floor that an optional workspace package (or its tree) does not support fails this job even though a user install of the parent would succeed and omit that optional package.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit eb22277. Configure here.
|
Closing: not pursuing the floor CI job. |
Adds
.github/workflows/engines.yml, which checks that every published package installs and loads on the lowest Node.js version itsengines.nodeallows. A dependency bump or new syntax that raises the real floor now turns CI red instead of breaking users on that version.What it does
pnpm install, then for each non-private workspace package (the root package in a single-package repo): runprepublishOnlyif present, andpnpm packit. The floor is read from that package'sengines.nodeat run time. Nothing is hardcoded in the workflow. A range that is not>= xfails the job.npm install --engine-strictof the package tarball, plus the tarballs of the workspace packages it depends on. This is what a user does: production dependencies only, lifecycle scripts included. Any package in the tree whoseenginesexcludes the floor fails the install withEBADENGINE. Without--engine-strict, npm only warns.requireevery runtime entry inexports(ormainwhen there is noexports). Entries that are import-only fall back toimport(). Type-only entries and*patterns are skipped.It runs as its own workflow on
pull_requestand on push to the default branch, so the release logic inmain.ymlis untouched.Why npm on the floor side
pnpm needs Node 18 or later, so it cannot run on older floors, and npm ships with every Node.js version. The install happens in a fresh directory from tarballs, so devDependencies never enter the tree and the test toolchain (ava, c8, standard), which needs a newer Node, is not involved.
Limits
setup-noderesolves a major-only floor such as>= 20to the latest 20.x. The job cannot tell 20.0 from 20.12. A floor written with a minor, such as>= 20.19, is checked at the latest 20.19.x.binscripts are not executed.enginesaccept the running Node. A user on the floor can get an older dependency than a user on current Node, and the job checks the tree the floor user gets. It goes red only when no version in range supports the floor, which is the case that breaks users.Proof that it catches a violation
On microlinkhq/keyvhq#294,
@keyvhq/redisdeclares>= 20and depends onioredis@6, which requires>=20.0.0. A throwaway commit lowered@keyvhq/redisto>= 18:>= 18:Node 18 @keyvhq/redisfails, the other 15 packages pass.>= 20, after dropping that commit: all 16 jobs pass.🤖 Generated with Claude Code
Note
Low Risk
Adds CI-only validation; no application, auth, or runtime deployment changes.
Overview
Adds a dedicated
enginesGitHub Actions workflow (PRs and pushes tomaster) that enforces each published package’s declaredengines.nodefloor without touching existing release CI.The Pack job uses Node LTS + pnpm to install the monorepo, run
prepublishOnlywhere defined,pnpm packevery non-private package, and build a matrix from each package’s>= xengines.node(invalid ranges fail). Workspaceworkspace:deps are resolved into tarball closures for downstream installs.Floor matrix jobs run on that minimum Node version,
npm install --engine-strictthe relevant tarballs in a clean consumer directory (production-like install), then smoke-load every runtimeexportsentry viarequirewithimport()fallback for ESM-only paths—so overstated Node support or syntax/deps that break on the floor fail CI before users do.Reviewed by Cursor Bugbot for commit eb22277. Bugbot is set up for automated code reviews on this repo. Configure here.