Skip to content

fix(env): preserve existing .nvmrc pins - #2676

Merged
fengmk2 merged 4 commits into
voidzero-dev:mainfrom
ywenhao:fix/env-pin-existing-nvmrc-2675
Sep 12, 2026
Merged

fix(env): preserve existing .nvmrc pins#2676
fengmk2 merged 4 commits into
voidzero-dev:mainfrom
ywenhao:fix/env-pin-existing-nvmrc-2675

Conversation

@ywenhao

@ywenhao ywenhao commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

A project using .nvmrc as its Node version source currently gets a second, higher-priority devEngines.runtime declaration when vp env pin changes its version. Vite+ then uses the new version while nvm and CI configured to read .nvmrc keep the old one.

Pinning now updates an effective .nvmrc in the current directory, preserving comments, reserved key/value lines, whitespace, and line endings. Higher-priority .node-version and manifest declarations keep their existing behavior, and inherited parent files are not modified. The change also adds --target nvmrc and keeps pin inspection and unpin consistent with the new target. Pin inspection stops at a nearer engines.node constraint instead of reporting a shadowed ancestor pin.

Fixes #2675.

Validation

Validated on Linux x64 / WSL2 with the repository's nightly-2026-08-02 Rust toolchain:

  • cargo build -p vp_global_cli --locked
  • cargo test -p vp_global_cli --locked: 491 passed, 2 ignored.
  • cargo clippy -p vp_global_cli --all-targets --all-features --locked -- --deny warnings with the repository's existing lint allow-list: passed.
  • cargo fmt --all --check and repository-configured vp fmt --check .: passed (491 files checked by Vite+).
  • VP_SNAP_SKIP_FLAVORS=local cargo test -p vp_cli_snapshots --locked -- command_env_pin --test-threads=1: 16 passed, including 11 new CLI cases and 5 existing package-manager cases.
  • The main new regression case fails against the unmodified base binary: it shows the unwanted devEngines.runtime write and stale .nvmrc. Snapshots were generated with the new binary and reviewed.

The initial parallel snapshot comparison encountered two Text file busy (os error 26) errors while starting the test harness's vp env setup, before the case steps. The full focused comparison passed with one test thread. Windows and macOS execution is left to CI.

@fengmk2

fengmk2 commented Sep 12, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-12T03:30:11.949228Z 7df05c0 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7df05c0ae3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/vp_global_cli/src/commands/env/pin.rs Outdated

@fengmk2 fengmk2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ywenhao Thanks!

@fengmk2 fengmk2 added test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: create-e2e Run `vp create` e2e tests test: sfw labels Sep 12, 2026
@fengmk2
fengmk2 merged commit 1639242 into voidzero-dev:main Sep 12, 2026
150 of 177 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test: create-e2e Run `vp create` e2e tests test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: sfw

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vp env pin: update an existing .nvmrc instead of adding devEngines.runtime

2 participants