Skip to content

fix(env): track injected tools across nested shim calls - #2631

Merged
fengmk2 merged 17 commits into
voidzero-dev:mainfrom
lyzno1:codex/fix-nested-package-manager-shims
Sep 10, 2026
Merged

fix(env): track injected tools across nested shim calls#2631
fengmk2 merged 17 commits into
voidzero-dev:mainfrom
lyzno1:codex/fix-nested-package-manager-shims

Conversation

@lyzno1

@lyzno1 lyzno1 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Nested package-manager calls can fail or select a system version because the boolean recursion guard assumes every tool is already on PATH.

Replace it with VP_PATH_INJECTED_TOOLS and shared child PATH preparation. Track available tools per installation, exclude other Vite+ shims, and recover missing bundled tools from the selected Node installation. Explicit vp env exec resolves a fresh selection, while nested shim calls preserve the parent's runtime and package-manager precedence through JS delegation.

Fixes #2630.

@netlify

netlify Bot commented Sep 8, 2026

Copy link
Copy Markdown

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit 5e02f12
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/6aa04421cc4bf50008abe9e4

@lyzno1 lyzno1 changed the title fix(env): resolve nested package manager shims fix(env): track injected tools across nested shim calls Sep 9, 2026
@lyzno1

lyzno1 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@liangmiQwQ I've updated this draft to track injected tools with VP_PATH_INJECTED_TOOLS and shared PATH helpers. Could you take a look and let me know whether the approach matches what you had in mind?

@lyzno1

lyzno1 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Per-tool PATH tracking is in place, but a forwarding loop remains between renamed Vite+ installations. The current shim detection cannot reliably distinguish these from shared external-manager entrypoints such as mise. This PR remains in draft while I revisit that boundary.

@liangmiQwQ

Copy link
Copy Markdown
Collaborator

Per-tool PATH tracking is in place, but a forwarding loop remains between renamed Vite+ installations. The current shim detection cannot reliably distinguish these from shared external-manager entrypoints such as mise. This PR remains in draft while I revisit that boundary.

I'll take a look soon

@liangmiQwQ
liangmiQwQ self-requested a review September 9, 2026 13:46
@liangmiQwQ

Copy link
Copy Markdown
Collaborator

Per-tool PATH tracking is in place, but a forwarding loop remains between renamed Vite+ installations. The current shim detection cannot reliably distinguish these from shared external-manager entrypoints such as mise. This PR remains in draft while I revisit that boundary.

@lyzno1 This is another problem that already exists on Vite+. It is a little bit difficult to fix and not introduced by your PR. You can mark it as ready to review if you are ready.

@lyzno1
lyzno1 marked this pull request as ready for review September 9, 2026 13:49
@liangmiQwQ

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 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-09T14:34:41.496012Z d546393 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

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: d5463937c1

ℹ️ 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".

@liangmiQwQ

liangmiQwQ commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

It would be better to update this RFC document:

- `locate_tool()` — locates a tool binary within a Node.js installation

@liangmiQwQ liangmiQwQ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Others LGTM 💯

Huge thanks for working on this!

@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.

@lyzno1 Thanks for the big refactor!

@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 10, 2026
@fengmk2

fengmk2 commented Sep 10, 2026

Copy link
Copy Markdown
Member

@lyzno1 please fix the conflicts, thanks.

@fengmk2
fengmk2 merged commit b1c41b2 into voidzero-dev:main Sep 10, 2026
125 checks passed
@fengmk2 fengmk2 added the preview-build Publish this PR's commits to the registry bridge as preview builds label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview-build Publish this PR's commits to the registry bridge as preview builds 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.

Nested npx calls to pnpm, Bun, and Yarn bypass managed resolution in vp 0.3.1

3 participants