fix(cli): stamp package-manager lifecycle env for vp run scripts - #2385
Open
tarikermis wants to merge 5 commits into
Open
fix(cli): stamp package-manager lifecycle env for vp run scripts#2385tarikermis wants to merge 5 commits into
tarikermis wants to merge 5 commits into
Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Member
|
@tarikermis can you also add a new snapshot test to cover this bug fix? |
Contributor
Author
|
Good call, on it - adding a snapshot test for the fix now. Best Regards, Tarik |
Covers voidzero-dev#2317: snapshot the session-constant lifecycle env computed for fixture package-manager install layouts (pnpm/npm/yarn JS CLI entries, native pnpm binary, shim fallback, and bun's empty stamp) so a regression that drops the stamp or changes exec-path resolution fails the test.
Member
|
@tarikermis I mean this snapshot tests https://github.com/voidzero-dev/vite-plus/tree/main/crates/vp_cli_snapshots/tests/cli_snapshots . |
Contributor
Author
|
Ah my bad, got it - adding a CLI snapshot test in vp_cli_snapshots now. Best Regards, Tarik |
CLI-level regression test for voidzero-dev#2317: a fixture pnpm project runs a package.json script via vp run that surfaces npm_execpath, npm_config_user_agent, and INIT_CWD. Pre-fix all three were undefined in the script process, so child tooling like npm-run-all fell back to npm. A fake managed pnpm install under VP_HOME keeps the case offline.
fengmk2
reviewed
Aug 10, 2026
| --- | ||
| source: crates/vp_pm_cli/src/lifecycle_env.rs | ||
| expression: "render_lifecycle_stamp(PackageManagerType::Pnpm, \"11.20.0\", &[])" | ||
| --- |
fengmk2
reviewed
Aug 10, 2026
|
|
||
| if let Some(node_execpath) = &context.node_execpath { | ||
| vars.push(("npm_node_execpath", node_execpath.as_os_str().to_os_string())); | ||
| vars.push(("NODE", node_execpath.as_os_str().to_os_string())); |
Member
There was a problem hiding this comment.
Is there source documentation for the naming of these environment variables? Or reference code? Need to add code comments to explain them.
Contributor
Author
|
Sure - reverting the insta snapshots and adding source comments explaining the env var naming. Best Regards, Tarik |
Revert the crate-level insta snapshot tests (the CLI-level vp_cli_snapshots case covers the bug fix end to end), and document where each lifecycle env var name and format comes from: npm's set-envs.js and user-agent definition, pnpm's @pnpm/npm-lifecycle and config userAgent, verified against pnpm 11.21.0 and npm 10.9.8.
# Conflicts: # packages/cli/binding/index.d.cts # packages/cli/binding/src/cli/mod.rs # packages/cli/binding/src/lib.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2317
What changed
vp runandvprnow set the session-level package-manager lifecycle variables beforevt::Session::initsnapshots the environment:npm_execpathnpm_config_user_agentINIT_CWDnpm_node_execpath/NODEThis mirrors the values pnpm, npm, and Yarn set when they run package scripts themselves.
Why
vp runcurrently addsnode_modules/.bintoPATHand setsVP_RUN=1, but leaves the lifecycle environment empty. Child runners such as npm-run-all2 then fall back tonpm runinside pnpm projects, which can fail withEBADDEVENGINESwhendevEngines.packageManagerrequires pnpm.The new helper in
vp_pm_cliresolves the JS CLI entry used by the package-manager shims and builds the matching user-agent string. The CLI binding stamps those values before the session starts. Node's realprocess.versionandprocess.execPathare passed through napi so version-manager symlinks are not resolved to the wrong executable.Per-script fields such as
npm_lifecycle_event,npm_lifecycle_script,npm_package_*, andPNPM_SCRIPT_SRC_DIRare deliberately left out because they cannot be set correctly once per session. Bun is also left unchanged because I could not verify its lifecycle contract.Verification
The repro pins pnpm in
devEngines.packageManager, runs npm-run-all2, and places a failing npm stub onPATH.main, the lifecycle variables are unset and npm-run-all2 invokes the npm stub.cargo test -p vp_pm_cli: 746 passed.cargo test -p vite-plus-cli: 37 passed.pnpm fmtandpnpm lintreport no findings in changed files. The repo-wide lint still reports 18 existing errors in untoucheddocs/.vitepress/*files.Notes
Windows was not available for a local run, but the JS-entry lookup and pnpm native
.exefallback are covered by platform-aware tests.vpandvprshare the samerun()call site, so both use the new environment.Best Regards, Tarik