From c4ae7483cb4d17a6b6b5e1ba4ba28a08151b5313 Mon Sep 17 00:00:00 2001 From: Akokko Date: Fri, 28 Aug 2026 21:33:10 +0800 Subject: [PATCH 1/3] fix(global-cli): bound local vite-plus resolution to the workspace root 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 (#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 --- crates/vp_global_cli/src/commands/version.rs | 39 +++---- crates/vp_global_cli/src/js_executor.rs | 110 +++++++++++++++++++ 2 files changed, 130 insertions(+), 19 deletions(-) diff --git a/crates/vp_global_cli/src/commands/version.rs b/crates/vp_global_cli/src/commands/version.rs index 26cff89b7e..9864a6fa29 100644 --- a/crates/vp_global_cli/src/commands/version.rs +++ b/crates/vp_global_cli/src/commands/version.rs @@ -9,10 +9,10 @@ use std::{ use serde::Deserialize; use vp_pm_cli::get_package_manager_type_and_version; -use vt_path::AbsolutePathBuf; +use vt_path::{AbsolutePath, AbsolutePathBuf}; use vt_workspace::find_workspace_root; -use crate::{commands::env::config::resolve_version, error::Error, help}; +use crate::{commands::env::config::resolve_version, error::Error, help, js_executor::JsExecutor}; #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -64,20 +64,16 @@ fn read_package_json(package_json_path: &Path) -> Option { serde_json::from_str(&content).ok() } -fn find_local_vite_plus(start: &Path) -> Option { - let mut current = Some(start); - while let Some(dir) = current { - let package_json_path = dir.join("node_modules").join("vite-plus").join("package.json"); - if let Some(pkg) = read_package_json(&package_json_path) { - let package_dir = package_json_path.parent()?.to_path_buf(); - // Follow symlinks (pnpm links node_modules/vite-plus -> node_modules/.pnpm/.../vite-plus) - // so parent traversal can discover colocated dependency links. - let package_dir = fs::canonicalize(&package_dir).unwrap_or(package_dir); - return Some(LocalVitePlus { version: pkg.version, package_dir }); - } - current = dir.parent(); - } - None +fn find_local_vite_plus(cwd: &AbsolutePath) -> Option { + // The workspace-bounded walk keeps this display consistent with what + // delegation would actually execute (see `local_vite_plus_install_host`). + let host = JsExecutor::local_vite_plus_install_host(cwd)?; + let package_dir = host.as_path().join("node_modules").join("vite-plus"); + let pkg = read_package_json(&package_dir.join("package.json"))?; + // Follow symlinks (pnpm links node_modules/vite-plus -> node_modules/.pnpm/.../vite-plus) + // so parent traversal can discover colocated dependency links. + let package_dir = fs::canonicalize(&package_dir).unwrap_or(package_dir); + Some(LocalVitePlus { version: pkg.version, package_dir }) } fn read_toolchain_manifest(local: &LocalVitePlus) -> Option { @@ -173,7 +169,7 @@ pub async fn execute(cwd: AbsolutePathBuf) -> Result { println!(); // Local vite-plus and tools - let local = find_local_vite_plus(cwd.as_path()); + let local = find_local_vite_plus(&cwd); print_rows( "Local vite-plus", &[("vite-plus", format_version(local.as_ref().map(|pkg| pkg.version.clone())))], @@ -228,6 +224,9 @@ mod tests { #[cfg(unix)] use std::{fs, path::Path}; + #[cfg(unix)] + use vt_path::AbsolutePath; + #[cfg(unix)] use super::{TOOL_SPECS, find_local_vite_plus, read_toolchain_manifest, resolve_tool_version}; use super::{detect_system_node_version, format_version}; @@ -307,7 +306,8 @@ mod tests { &node_modules_dir.join("vite-plus"), ); - let local = find_local_vite_plus(project).expect("expected local vite-plus to resolve"); + let local = find_local_vite_plus(AbsolutePath::new(project).unwrap()) + .expect("expected local vite-plus to resolve"); let manifest = read_toolchain_manifest(&local).expect("expected manifest to resolve"); assert_eq!( resolve_tool_version(Some(&local), Some(&manifest), TOOL_SPECS[0]).as_deref(), @@ -342,7 +342,8 @@ mod tests { &node_modules_dir.join("vite-plus"), ); - let local = find_local_vite_plus(project).expect("expected local vite-plus to resolve"); + let local = find_local_vite_plus(AbsolutePath::new(project).unwrap()) + .expect("expected local vite-plus to resolve"); assert_eq!( resolve_tool_version(Some(&local), None, TOOL_SPECS[0]).as_deref(), Some("8.0.0"), diff --git a/crates/vp_global_cli/src/js_executor.rs b/crates/vp_global_cli/src/js_executor.rs index 31459ed92b..2dda44ea7e 100644 --- a/crates/vp_global_cli/src/js_executor.rs +++ b/crates/vp_global_cli/src/js_executor.rs @@ -413,12 +413,50 @@ impl JsExecutor { Ok(output) } + /// Find the directory whose `node_modules/vite-plus` an upward walk from + /// `project_path` is allowed to use. + /// + /// Node-style resolution walks every ancestor's `node_modules`, which can + /// escape the project and silently pick up an unrelated ancestor project's + /// copy (e.g. a repo checked out inside another project's tree, whose own + /// install is missing or broken). Bound the walk at the project's workspace + /// root: within it, nearest wins (a workspace member still resolves the + /// workspace root's install); beyond it, resolution fails so callers fall + /// back to the global installation. When there is no workspace or package + /// root at all (`find_workspace_root` errors), there is no project boundary + /// to protect and the walk stays unbounded. + pub(crate) fn local_vite_plus_install_host( + project_path: &AbsolutePath, + ) -> Option { + let boundary = + vt_workspace::find_workspace_root(project_path).ok().map(|(root, _)| root.path); + + let mut current = project_path; + loop { + if current.join("node_modules/vite-plus/package.json").as_path().exists() { + return Some(current.to_absolute_path_buf()); + } + if boundary.as_deref().is_some_and(|boundary| current == boundary) { + return None; + } + match current.parent() { + Some(parent) if parent != current => current = parent, + _ => return None, + } + } + } + /// Resolve the local vite-plus package root from the project directory. pub(crate) fn resolve_local_vite_plus_package_dir( project_path: &AbsolutePath, ) -> Option { use oxc_resolver::{ResolveOptions, Resolver}; + // Only trust an install that lives within the project's workspace; the + // Node-semantics resolution below would otherwise walk past it (see + // `local_vite_plus_install_host`). + Self::local_vite_plus_install_host(project_path)?; + let resolver = Resolver::new(ResolveOptions { condition_names: vec!["import".into(), "node".into()], ..ResolveOptions::default() @@ -534,6 +572,78 @@ mod tests { dir } + /// An independent project (with its own workspace marker) checked out + /// inside another project's tree must not resolve the outer project's + /// vite-plus when its own install is missing — Node-style upward + /// resolution would otherwise silently delegate to an unrelated copy. + #[test] + fn local_resolution_stays_within_the_workspace() { + let temp = tempfile::tempdir().unwrap(); + let outer = temp.path(); + std::fs::create_dir_all(outer.join("node_modules/vite-plus/dist")).unwrap(); + std::fs::write(outer.join("node_modules/vite-plus/package.json"), r#"{"version":"0.2.1"}"#) + .unwrap(); + std::fs::write(outer.join("node_modules/vite-plus/dist/bin.js"), "").unwrap(); + std::fs::write(outer.join("pnpm-workspace.yaml"), "packages: []\n").unwrap(); + std::fs::write(outer.join("package.json"), r#"{"name":"outer"}"#).unwrap(); + + let inner = outer.join("external/inner"); + std::fs::create_dir_all(&inner).unwrap(); + std::fs::write( + inner.join("package.json"), + r#"{"name":"inner","devDependencies":{"vite-plus":"0.3.0"}}"#, + ) + .unwrap(); + std::fs::write(inner.join("pnpm-workspace.yaml"), "packages: []\n").unwrap(); + + let inner = AbsolutePath::new(inner.as_path()).unwrap(); + assert_eq!(JsExecutor::local_vite_plus_install_host(inner), None); + assert_eq!(JsExecutor::resolve_local_vite_plus_package_dir(inner), None); + assert_eq!(JsExecutor::resolve_local_vite_plus(inner), None); + } + + /// A workspace member still resolves the workspace root's install: the + /// boundary is the workspace root, not the member directory. + #[test] + fn workspace_member_resolves_the_workspace_root_install() { + let temp = tempfile::tempdir().unwrap(); + let ws = temp.path(); + std::fs::write(ws.join("pnpm-workspace.yaml"), "packages:\n - packages/*\n").unwrap(); + std::fs::write(ws.join("package.json"), r#"{"name":"ws"}"#).unwrap(); + std::fs::create_dir_all(ws.join("node_modules/vite-plus")).unwrap(); + std::fs::write(ws.join("node_modules/vite-plus/package.json"), r#"{"version":"0.3.0"}"#) + .unwrap(); + let member = ws.join("packages/app"); + std::fs::create_dir_all(&member).unwrap(); + std::fs::write(member.join("package.json"), r#"{"name":"app"}"#).unwrap(); + + let member = AbsolutePath::new(member.as_path()).unwrap(); + let host = JsExecutor::local_vite_plus_install_host(member) + .expect("workspace root install must stay resolvable"); + assert_eq!(host.as_path(), ws); + let pkg_dir = JsExecutor::resolve_local_vite_plus_package_dir(member) + .expect("workspace root install must stay resolvable"); + assert!(pkg_dir.as_path().ends_with("node_modules/vite-plus")); + } + + /// Without any project marker around (`find_workspace_root` errors) there + /// is no boundary to protect; the walk stays unbounded as before. + #[test] + fn unbounded_walk_without_project_markers() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + std::fs::create_dir_all(root.join("node_modules/vite-plus")).unwrap(); + std::fs::write(root.join("node_modules/vite-plus/package.json"), r#"{"version":"1.0.0"}"#) + .unwrap(); + let nested = root.join("a/b"); + std::fs::create_dir_all(&nested).unwrap(); + + let nested = AbsolutePath::new(nested.as_path()).unwrap(); + let host = JsExecutor::local_vite_plus_install_host(nested) + .expect("markerless directories keep the unbounded walk"); + assert_eq!(host.as_path(), root); + } + #[test] fn test_local_vite_plus_is_older() { // Older local should escalate. From ef74d996ec6a2dc5367ac4f33a6d2834e370c5db Mon Sep 17 00:00:00 2001 From: Akokko Date: Fri, 28 Aug 2026 21:51:18 +0800 Subject: [PATCH 2/3] test(global-cli): gate the markerless-walk test to unix 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 --- crates/vp_global_cli/src/js_executor.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/vp_global_cli/src/js_executor.rs b/crates/vp_global_cli/src/js_executor.rs index 2dda44ea7e..f5c914ae28 100644 --- a/crates/vp_global_cli/src/js_executor.rs +++ b/crates/vp_global_cli/src/js_executor.rs @@ -628,6 +628,12 @@ mod tests { /// Without any project marker around (`find_workspace_root` errors) there /// is no boundary to protect; the walk stays unbounded as before. + /// + /// Unix-only: the premise is that no ancestor of the tempdir carries a + /// package.json, which holds for `/tmp` / `/var/folders` but not for + /// Windows, where `%TEMP%` lives under the user profile and a stray + /// `package.json` there would create a boundary and fail the test. + #[cfg(unix)] #[test] fn unbounded_walk_without_project_markers() { let temp = tempfile::tempdir().unwrap(); From 8541a404d43882f14f7a326aea149ea5c9d346ba Mon Sep 17 00:00:00 2001 From: Akokko Date: Sun, 30 Aug 2026 13:09:54 +0800 Subject: [PATCH 3/3] fix(global-cli): only bound the walk for projects that declare vite-plus 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 --- crates/vp_global_cli/src/js_executor.rs | 84 +++++++++++++++++++------ 1 file changed, 64 insertions(+), 20 deletions(-) diff --git a/crates/vp_global_cli/src/js_executor.rs b/crates/vp_global_cli/src/js_executor.rs index f5c914ae28..100203f5ce 100644 --- a/crates/vp_global_cli/src/js_executor.rs +++ b/crates/vp_global_cli/src/js_executor.rs @@ -416,20 +416,29 @@ impl JsExecutor { /// Find the directory whose `node_modules/vite-plus` an upward walk from /// `project_path` is allowed to use. /// - /// Node-style resolution walks every ancestor's `node_modules`, which can - /// escape the project and silently pick up an unrelated ancestor project's - /// copy (e.g. a repo checked out inside another project's tree, whose own - /// install is missing or broken). Bound the walk at the project's workspace - /// root: within it, nearest wins (a workspace member still resolves the - /// workspace root's install); beyond it, resolution fails so callers fall - /// back to the global installation. When there is no workspace or package - /// root at all (`find_workspace_root` errors), there is no project boundary - /// to protect and the walk stays unbounded. + /// Walking every ancestor's `node_modules` is ordinary Node resolution + /// semantics, and layouts legitimately rely on it (hoisted installs; the + /// snapshot harness stages workspaces with no `node_modules` of their own + /// that resolve a run-root install). So the walk stays unbounded by + /// default. The exception is a project that *declares* a `vite-plus` + /// dependency (directly, or at its workspace root — the same test + /// `warn_missing_local_cli_if_project` applies): for it, escaping the + /// workspace root would silently borrow an unrelated ancestor's copy in + /// exactly the situation where "run `vp install`" is the right answer. + /// Only then is the walk bounded at the workspace root: within it, + /// nearest wins (a workspace member still resolves the workspace root's + /// install); beyond it, resolution fails so callers fall back to the + /// global installation and the install hint. pub(crate) fn local_vite_plus_install_host( project_path: &AbsolutePath, ) -> Option { - let boundary = - vt_workspace::find_workspace_root(project_path).ok().map(|(root, _)| root.path); + let boundary = vt_workspace::find_workspace_root(project_path) + .ok() + .filter(|(root, _)| { + crate::commands::has_vite_plus_dependency(project_path) + || crate::commands::has_vite_plus_dependency(root.path.as_ref()) + }) + .map(|(root, _)| root.path); let mut current = project_path; loop { @@ -452,9 +461,9 @@ impl JsExecutor { ) -> Option { use oxc_resolver::{ResolveOptions, Resolver}; - // Only trust an install that lives within the project's workspace; the - // Node-semantics resolution below would otherwise walk past it (see - // `local_vite_plus_install_host`). + // For projects that declare a vite-plus dependency, only trust an + // install within their workspace; the Node-semantics resolution below + // would otherwise walk past it (see `local_vite_plus_install_host`). Self::local_vite_plus_install_host(project_path)?; let resolver = Resolver::new(ResolveOptions { @@ -572,10 +581,11 @@ mod tests { dir } - /// An independent project (with its own workspace marker) checked out - /// inside another project's tree must not resolve the outer project's - /// vite-plus when its own install is missing — Node-style upward - /// resolution would otherwise silently delegate to an unrelated copy. + /// An independent project that *declares* a vite-plus dependency (with + /// its own workspace marker) checked out inside another project's tree + /// must not resolve the outer project's vite-plus when its own install is + /// missing — the declaration makes "run `vp install`" the right answer, + /// not silently delegating to an unrelated copy. #[test] fn local_resolution_stays_within_the_workspace() { let temp = tempfile::tempdir().unwrap(); @@ -603,13 +613,20 @@ mod tests { } /// A workspace member still resolves the workspace root's install: the - /// boundary is the workspace root, not the member directory. + /// boundary is the workspace root, not the member directory. The root + /// declares the dependency so the bounded walk is actually engaged — + /// without a declaration this case would pass trivially via the + /// unbounded default. #[test] fn workspace_member_resolves_the_workspace_root_install() { let temp = tempfile::tempdir().unwrap(); let ws = temp.path(); std::fs::write(ws.join("pnpm-workspace.yaml"), "packages:\n - packages/*\n").unwrap(); - std::fs::write(ws.join("package.json"), r#"{"name":"ws"}"#).unwrap(); + std::fs::write( + ws.join("package.json"), + r#"{"name":"ws","devDependencies":{"vite-plus":"0.3.0"}}"#, + ) + .unwrap(); std::fs::create_dir_all(ws.join("node_modules/vite-plus")).unwrap(); std::fs::write(ws.join("node_modules/vite-plus/package.json"), r#"{"version":"0.3.0"}"#) .unwrap(); @@ -626,6 +643,33 @@ mod tests { assert!(pkg_dir.as_path().ends_with("node_modules/vite-plus")); } + /// A project that does *not* declare a vite-plus dependency keeps Node's + /// unbounded upward resolution even across its own workspace marker — + /// this is the layout the snapshot harness depends on (staged workspaces + /// with no `node_modules` of their own, resolving a run-root install). + #[test] + fn undeclared_project_keeps_the_unbounded_walk() { + let temp = tempfile::tempdir().unwrap(); + let outer = temp.path(); + std::fs::create_dir_all(outer.join("node_modules/vite-plus/dist")).unwrap(); + std::fs::write(outer.join("node_modules/vite-plus/package.json"), r#"{"version":"0.2.1"}"#) + .unwrap(); + std::fs::write(outer.join("node_modules/vite-plus/dist/bin.js"), "").unwrap(); + + let inner = outer.join("cases/one/workspace"); + std::fs::create_dir_all(&inner).unwrap(); + std::fs::write(inner.join("package.json"), r#"{"name":"inner"}"#).unwrap(); + std::fs::write(inner.join("pnpm-workspace.yaml"), "packages: []\n").unwrap(); + + let inner = AbsolutePath::new(inner.as_path()).unwrap(); + let host = JsExecutor::local_vite_plus_install_host(inner) + .expect("undeclared projects keep the unbounded walk"); + assert_eq!(host.as_path(), outer); + let pkg_dir = JsExecutor::resolve_local_vite_plus_package_dir(inner) + .expect("undeclared projects keep the unbounded walk"); + assert!(pkg_dir.as_path().ends_with("node_modules/vite-plus")); + } + /// Without any project marker around (`find_workspace_root` errors) there /// is no boundary to protect; the walk stays unbounded as before. ///