Skip to content

fix(global-cli): bound local vite-plus resolution to the workspace root - #2574

Open
Akokk0 wants to merge 14 commits into
voidzero-dev:mainfrom
Akokk0:fix/local-cli-workspace-boundary
Open

fix(global-cli): bound local vite-plus resolution to the workspace root#2574
Akokk0 wants to merge 14 commits into
voidzero-dev:mainfrom
Akokk0:fix/local-cli-workspace-boundary

Conversation

@Akokk0

@Akokk0 Akokk0 commented Aug 28, 2026

Copy link
Copy Markdown

A nested project can run an ancestor's vite-plus when its own installation is missing. vp --version can also display that unrelated installation.

Use oxc_resolver with Restriction::Fn in the shared vp_local_cli crate to enforce the package or workspace boundary. Workspace members can use the root installation. Excluded projects that declare vite-plus stop at their own package root. Undeclared projects retain ancestor lookup.

Command delegation, migration version checks, and both CLI version entrypoints use the shared resolver. The NAPI binding replaces the separate JavaScript lookup. Read versions from the manifest that Oxc has already parsed.

The restriction checks the lookup path before Oxc resolves symlinks, so local links to external package stores remain valid. Add unit tests and direct CLI output snapshots for boundaries, exports, malformed manifests, and symlinks.

Link the local CLI inside migration fixtures that need it. Use an existing shell in installer tests to avoid ETXTBSY on Linux and support BusyBox.

An existing limitation remains: malformed ancestor manifests can prevent resolution in a workspace without a root package.json.

@netlify

netlify Bot commented Aug 28, 2026

Copy link
Copy Markdown

Deploy Preview for viteplus-preview ready!

Name Link
🔨 Latest commit 8541a40
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/6a93bcdec331260008e55fd7
😎 Deploy Preview https://deploy-preview-2574--viteplus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@fengmk2 fengmk2 self-assigned this Aug 30, 2026
@Akokk0

Akokk0 commented Aug 30, 2026

Copy link
Copy Markdown
Author

The red CI is a real regression from this PR. All 75 snapshot failures have the same cause: the snapshot harness links the only real node_modules/vite-plus at the run root, and each staged workspace (which carries its own package.json/pnpm-workspace.yaml) resolves it via Node's unbounded upward walk — noted in cli_snapshots/main.rs around L1571. The workspace boundary added here stops the walk at the stage, so the install two levels up is never found.

I'll push a narrower fix: the boundary only applies when the project itself declares a vite-plus dependency (the case where the existing vp install warning is the right answer). The fixtures don't declare it, so they keep the unbounded walk.

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

fengmk2 commented Sep 7, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 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-07T12:08:21.853235Z 8541a40 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. Chef's kiss.

Reviewed commit: 8541a404d4

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

@Akokk0

Akokk0 commented Sep 11, 2026

Copy link
Copy Markdown
Author

The remaining red is this PR: the same 3 snapshot cases on every platform — migration_eslint_svelte_runes, migration_pack_tsdown_023_build, migration_pack_tsdown_023_concise_methods (the Linux shard 1 failure is apt-get, unrelated). Each differs by one line:

+warn: No project-local vite-plus installation was found. Run vp install in to install dependencies.

Cause: these fixtures declare vite-plus (pre-declared, or added by vp migrate) and then run vp lint / vp pack / vp run build without installing. The harness relies on the run-root node_modules being found by the upward walk, and this PR stops that walk at the workspace root for projects that declare vite-plus. My earlier "the fixtures don't declare it" was wrong for these three;

I don't know how to resolve this.

@fengmk2

fengmk2 commented Sep 11, 2026

Copy link
Copy Markdown
Member

@Akokk0 Let me take over.

@fengmk2
fengmk2 force-pushed the fix/local-cli-workspace-boundary branch from b38e8bf to edd2f76 Compare September 11, 2026 16:15
Akokk0 and others added 12 commits September 12, 2026 21:43
Local CLI resolution (oxc_resolver in the JS executor) and the
`vp --version` "Local vite-plus" probe both walk every ancestor
directory's node_modules, Node-style. When the project's own install is
missing or broken (e.g. after a corrupted install), resolution escapes
the project and silently picks up an unrelated ancestor project's copy:
delegation then runs another project's vite-plus, and `vp --version`
reports that copy's version and bundled tool versions as "Local".

Bound the walk at the project's workspace root via
`vt_workspace::find_workspace_root`:

- within the workspace, nearest wins - a workspace member still
  resolves the workspace root's install;
- beyond it, resolution fails, so delegation falls back to the global
  installation and the existing missing-local-cli warning (voidzero-dev#2361)
  explains the state instead of masking it;
- when there is no workspace or package root at all, the walk stays
  unbounded (unchanged behavior for markerless directories).

`find_local_vite_plus` in version.rs now derives from the same bounded
walk, so what --version displays is what delegation would execute.

Tested: unit tests cover the escape (red without the gate), the
workspace-member case, and the markerless case; verified end-to-end
with a nested-project fixture where 0.3.0 reports the outer project's
copy and the patched build reports "Not found".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test's premise is that no ancestor of the tempdir carries a
package.json. That holds for /tmp and /var/folders, but Windows' %TEMP%
lives under the user profile, where a stray package.json would create a
workspace boundary and fail the test for environmental reasons.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
the previous commit bounded local resolution at the workspace root for
every project. that breaks a layout the repo itself relies on: the
snapshot harness stages workspaces with no node_modules of their own and
resolves the run-root install through Node's unbounded upward walk, so
all three CLI snapshot jobs went red with the same signature - the
global CLI stopped seeing the project-local install (75 cases, every
diff a "does not use vite-plus" warning)

walking past the package root is ordinary Node resolution semantics and
hoisted installs depend on it, so the default stays unbounded. the
boundary now applies only when the project declares a vite-plus
dependency - directly or at its workspace root, the same test
warn_missing_local_cli_if_project uses - because that is exactly the
case where "run vp install" is the right answer rather than silently
borrowing an unrelated ancestor's copy

- new test pins the harness-shaped layout: an undeclared staged
  workspace keeps resolving the run-root install (mutation-verified:
  removing the declaration filter reds it)
- the workspace-member test's root now declares the dependency so the
  bounded walk is actually engaged rather than passing via the
  unbounded default
- snapshot fixtures do not declare vite-plus, so they take the
  unbounded path; the declared-but-missing fixture resolves its own
  install at the first hop either way

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@fengmk2
fengmk2 force-pushed the fix/local-cli-workspace-boundary branch from 556f0f5 to bf8ad42 Compare September 12, 2026 13:53
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants