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

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

3 participants