chore: add devEngines.runtime#11553
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Node runtime declaration and bumps pnpm in package.json; compile-and-lint switches to pnpm/action-setup (removes explicit Node setup); test workflow uses Changespnpm runtime & CI setup updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a pinned Node.js runtime to the repo via devEngines.runtime, and updates CI workflows/lockfile to reflect the new runtime-management approach.
Changes:
- Add
devEngines.runtime(Node 22.13.0) to the rootpackage.json. - Update
pnpm-lock.yamlto includenode@runtime:22.13.0as a locked runtime dependency with platform variants. - Adjust GitHub Actions workflows: remove explicit Node runtime setup in
ci.yml, and attempt to override runtime behavior during installs intest.yml.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds locked node@runtime:22.13.0 resolution (variations/variants) and importer devDependency entry. |
| package.json | Introduces devEngines.runtime pin for Node.js runtime management. |
| .github/workflows/test.yml | Changes install invocation to pass a runtime-on-fail override during CI tests. |
| .github/workflows/ci.yml | Removes the explicit “Setup Node” runtime step, relying on the new runtime pin instead. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This will be possible to finish when #11557 will be released |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 44-45: Replace the unsafe verification command `pn node -v` with a
reliable runtime check by using `pn exec node -v` or plain `node -v` wherever
the workflow uses `pn node -v` (notably the verification step around the `pn
install --no-runtime`/diagnostic block and the section at lines ~82–90); update
the step that runs the version check to call `pn exec node -v` (or `node -v`) so
the runner's PATH is used and the failing-call diagnostic is avoided.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c3473c76-4e7d-4d68-a11a-967debd50787
📒 Files selected for processing (1)
.github/workflows/test.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Compile & Lint
🔇 Additional comments (1)
.github/workflows/test.yml (1)
46-81: Useful Windows-only diagnostics.Keeping this step Windows-only and
continue-on-errorpreserves the existing failure signal while still capturing the extra path-resolution data.
Bumps pnpm/action-setup to a bootstrap that ships cmd-shim 9.0.3, so the `pn` shim on Windows no longer breaks under Git Bash MSYS path conversion of `cmd /C`. Also swaps the smoke job to windows-latest / Node.js 22.13.0 so Windows runs ahead of the Ubuntu matrix on branches.
| - name: Verify Node version | ||
| shell: bash | ||
| run: | | ||
| actual=$(pn node -v) | ||
| expected="v${{ inputs.node }}" | ||
| if [ "$actual" != "$expected" ]; then | ||
| echo "Expected Node version $expected but got $actual" | ||
| exit 1 |
| "packageManager": "pnpm@11.1.1", | ||
| "devEngines": { | ||
| "packageManager": { | ||
| "name": "pnpm", | ||
| "version": "11.0.7", | ||
| "version": "11.1.1", | ||
| "onFail": "download" | ||
| }, |
Reverts the Windows smoke swap now that the action-setup bump fixes the `pn` shim on Windows; ubuntu-latest / Node.js 24 is the smoke job again.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
.github/workflows/release.yml:30
- This workflow sets Node to 26.0.0 via
pn runtime -g set node 26.0.0, but withdevEngines.runtimenow set to download Node 24.6.0,pn installwill install a project-localnodebinary (from thenode@runtime:*dependency) that typically takes precedence when running scripts/binaries. That can cause the rest of the release steps to run under 24.6.0 instead of the intended 26.0.0. Consider disabling runtime dependency installation for this job (e.g. using the same mechanism as the test workflow) or otherwise ensuring the job actually runs with the intended Node version.
uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093
with:
standalone: true
- name: Setup Node
run: pn runtime -g set node 26.0.0
.github/workflows/benchmark.yml:56
- This workflow sets Node to 26.0.0, but
devEngines.runtimenow pins Node 24.6.0 and will causepnpm installto bring in a project-localnodebinary via thenode@runtime:*dependency. That can make subsequent steps (compile/bench scripts) run under 24.6.0 instead of 26.0.0. If the intent is to benchmark under the matrix-selected Node, disable runtime dependency installation for this job (same approach as in the reusable test workflow) or otherwise ensure the selected Node stays the one used for scripts.
- name: Install pnpm
uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093
with:
standalone: true
- name: Setup Node
run: pnpm runtime -g set node 26.0.0
timeout-minutes: 2
| - name: Install pnpm | ||
| uses: pnpm/action-setup@e578e19d19d31b011b841ba2aca34731a5f706a5 | ||
| uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093 | ||
| with: | ||
| standalone: true | ||
| - name: Setup Node | ||
| run: pn runtime -g set node 24.6.0 | ||
| timeout-minutes: 2 | ||
| - name: pnpm install | ||
| run: pn install | ||
| timeout-minutes: 3 |
Adds
devEngines.runtimeto pin the Node.js version (24.6.0,onFail: download) the project uses for development, so contributors don't have to manage Node versions manually.CI changes that come with it:
pnpm/action-setupto a bootstrap that ships@zkochan/cmd-shim9.0.3. The cmd-shim update is required because the previous shim'sexec cmd /Cgot mangled by Git Bash's MSYS path conversion (/C→ Windows path), which broke anypn …invocation fromshell: bashon Windows.pn install --no-runtimeso the per-test-matrix Node version chosen bypn runtime -g set node …isn't overridden by the project-pinned 24.6.0.Verify Node versionstep that assertspn node -vmatches the matrix's Node.Written by an agent (Claude Code, claude-opus-4-7).