From 7d536bb4bb0381499e13743d79d7e019db424700 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 19 Oct 2026 11:15:54 +0900 Subject: [PATCH 1/6] fix(shim): preserve npm global links during uninstall dry runs Only clean up links after the package disappears from the selected npm prefix. Collect uninstall binaries from that prefix without falling back to another Node installation. Cover dry runs configured through CLI flags, environment variables, and user npmrc, while verifying that real uninstalls still remove links. --- .../snapshots.toml | 32 ++++ .../snapshots/npm_global_uninstall_dry_run.md | 171 ++++++++++++++++++ crates/vp_global_cli/src/shim/dispatch.rs | 70 ++++++- 3 files changed, 268 insertions(+), 5 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_dry_run.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml index 8a5c06a549..b6c3bc84eb 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml @@ -11,3 +11,35 @@ steps = [ { argv = ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli"], comment = "Should be gone", continue-on-failure = true }, { argv = ["vpt", "rm", "-rf", "custom-prefix"], comment = "Cleanup", continue-on-failure = true }, ] + +[[case]] +name = "npm_global_uninstall_dry_run" +vp = "global" +skip-platforms = ["windows"] +steps = [ + { tty = false, argv = ["npm", "install", "-g", "--prefix", "./custom-prefix", "./npm-global-prefix-pkg"], comment = "Install a local package and create its Vite+ link" }, + { tty = false, argv = ["npm", "uninstall", "-g", "--prefix", "./custom-prefix", "npm-global-prefix-pkg", "--dry-run"], comment = "CLI dry-run must preserve the package, npm binary, link, and BinConfig" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + ["npm-global-prefix-cli"], + { tty = false, argv = ["npm", "uninstall", "-g", "--prefix", "./custom-prefix", "npm-global-prefix-pkg"], envs = [["npm_config_dry_run", "true"]], comment = "Environment-configured dry-run must also preserve installed state" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + ["npm-global-prefix-cli"], + ["vpt", "write-file", "dry-run.npmrc", "dry-run=true\n"], + { tty = false, argv = ["npm", "uninstall", "-g", "--prefix", "./custom-prefix", "npm-global-prefix-pkg"], envs = [["npm_config_userconfig", "${workspace}/dry-run.npmrc"]], comment = "User npmrc dry-run must also preserve installed state" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + ["npm-global-prefix-cli"], + { tty = false, argv = ["npm", "uninstall", "-g", "--prefix", "./custom-prefix", "npm-global-prefix-pkg"], comment = "A real uninstall must still clean up the package, npm binary, link, and BinConfig" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_dry_run.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_dry_run.md new file mode 100644 index 0000000000..ade04a31ea --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_dry_run.md @@ -0,0 +1,171 @@ +# npm_global_uninstall_dry_run + +## `npm install -g --prefix ./custom-prefix ./npm-global-prefix-pkg` + +Install a local package and create its Vite+ link + +``` + +added 1 package in +Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli +``` + +## `npm uninstall -g --prefix ./custom-prefix npm-global-prefix-pkg --dry-run` + +CLI dry-run must preserve the package, npm binary, link, and BinConfig + +``` +remove npm-global-prefix-pkg 1.0.0 + +removed 1 package in +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert symlink` + +``` +custom-prefix/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `npm_config_dry_run=true npm uninstall -g --prefix ./custom-prefix npm-global-prefix-pkg` + +Environment-configured dry-run must also preserve installed state + +``` +remove npm-global-prefix-pkg 1.0.0 + +removed 1 package in +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert symlink` + +``` +custom-prefix/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `vpt write-file dry-run.npmrc 'dry-run=true +'` + +``` +``` + +## `npm_config_userconfig=${workspace}/dry-run.npmrc npm uninstall -g --prefix ./custom-prefix npm-global-prefix-pkg` + +User npmrc dry-run must also preserve installed state + +``` +remove npm-global-prefix-pkg 1.0.0 + +removed 1 package in +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert symlink` + +``` +custom-prefix/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `npm uninstall -g --prefix ./custom-prefix npm-global-prefix-pkg` + +A real uninstall must still clean up the package, npm binary, link, and BinConfig + +``` + +removed 1 package in +Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg: missing +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert missing` + +``` +custom-prefix/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing` + +``` +/.vite-plus/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert missing` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: missing +``` diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 7a12df4f7a..0c53a7ce48 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -510,6 +510,12 @@ fn remove_npm_global_uninstall_links(bin_entries: &[(String, String)], npm_prefi continue; } + // A successful npm command may be a dry-run configured through flags, env, or npmrc. + let package_dir = config::get_node_modules_dir(npm_prefix, package_name); + if package_dir.as_path().try_exists().unwrap_or(true) { + continue; + } + let config = match BinConfig::load_sync(bin_name) { Ok(Some(c)) if c.source == BinSource::Npm => c, _ => continue, @@ -608,15 +614,14 @@ fn read_npm_package_json( fn collect_bin_names_from_npm( packages: &[String], npm_prefix: &AbsolutePath, - node_dir: &AbsolutePath, ) -> Vec<(String, String)> { let mut all_bins = Vec::new(); for spec in packages { let Some(package_name) = resolve_package_name(spec) else { continue }; - let Some(content) = read_npm_package_json(npm_prefix, node_dir, &package_name) else { - continue; - }; + let package_json_path = + config::get_node_modules_dir(npm_prefix, &package_name).join("package.json"); + let Ok(content) = std::fs::read_to_string(package_json_path.as_path()) else { continue }; let Ok(package_json) = serde_json::from_str::(&content) else { continue; }; @@ -911,7 +916,7 @@ pub async fn dispatch(tool: &str, args: &[String], env: ToolPathEnv) -> i32 { // Collect bin names before uninstall (package.json will be gone after) let node_dir = node_prefix_from_binary(&node_path); let npm_prefix = resolve_npm_prefix(&parsed, &tool_path, &node_dir, &child_env); - let bin_names = collect_bin_names_from_npm(&parsed.packages, &npm_prefix, &node_dir); + let bin_names = collect_bin_names_from_npm(&parsed.packages, &npm_prefix); let exit_code = exec::spawn_tool(&tool_path, args, child_env); if exit_code == 0 { remove_npm_global_uninstall_links(&bin_names, &npm_prefix); @@ -1894,6 +1899,61 @@ mod tests { assert!(result.is_none(), "URLs should be filtered"); } + #[test] + fn test_npm_uninstall_preserves_links_until_package_is_removed() { + let temp = TempDir::new().unwrap(); + let npm_prefix = AbsolutePathBuf::new(temp.path().join("npm-prefix")).unwrap(); + let package_dir = config::get_node_modules_dir(&npm_prefix, "test-package"); + std::fs::create_dir_all(&package_dir).unwrap(); + let source = package_dir.join("cli.js"); + std::fs::write(&source, "console.log('test');").unwrap(); + + vp_shared::EnvConfig::with_vars([(env_vars::VP_HOME, temp.path())], |_| { + let bin_dir = config::get_bin_dir().unwrap(); + std::fs::create_dir_all(&bin_dir).unwrap(); + create_bin_link(&bin_dir, "test-cli", &source, "test-package", "22.18.0"); + let link = bin_dir.join("test-cli"); + let metadata_path = BinConfig::path("test-cli").unwrap(); + let metadata = std::fs::read(&metadata_path).unwrap(); + assert!(link.as_path().exists()); + + let bins = vec![("test-cli".to_string(), "test-package".to_string())]; + remove_npm_global_uninstall_links(&bins, &npm_prefix); + + assert!(link.as_path().exists(), "dry-run must preserve the installed command"); + assert_eq!(std::fs::read(&metadata_path).unwrap(), metadata); + #[cfg(windows)] + assert!(bin_dir.join("test-cli.cmd").as_path().exists()); + + std::fs::remove_dir_all(&package_dir).unwrap(); + remove_npm_global_uninstall_links(&bins, &npm_prefix); + + assert!(std::fs::symlink_metadata(&link).is_err()); + assert!(!metadata_path.as_path().exists()); + #[cfg(windows)] + assert!(!bin_dir.join("test-cli.cmd").as_path().exists()); + }); + } + + #[test] + fn test_npm_uninstall_does_not_collect_bins_from_another_prefix() { + let temp = TempDir::new().unwrap(); + let npm_prefix = AbsolutePathBuf::new(temp.path().join("npm-prefix")).unwrap(); + let node_dir = AbsolutePathBuf::new(temp.path().join("node")).unwrap(); + let package_dir = config::get_node_modules_dir(&node_dir, "test-package"); + std::fs::create_dir_all(&package_dir).unwrap(); + std::fs::write( + package_dir.join("package.json"), + r#"{"name":"test-package","bin":{"test-cli":"cli.js"}}"#, + ) + .unwrap(); + + assert!( + collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix).is_empty(), + "uninstall must only collect binaries installed in the selected npm prefix" + ); + } + // --- parse_npm_global_uninstall tests --- #[test] From 1d94400848044e267fd2f513ab8a54408fe9685c Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 19 Oct 2026 11:41:48 +0900 Subject: [PATCH 2/6] fix(shim): honor npm prefix shorthand in global commands Recognize -C and -C= so link creation and cleanup use the same prefix as npm when npmrc or NPM_CONFIG_PREFIX points elsewhere. Cover argument ordering, mixed-prefix precedence, configuration overrides, and dry-run preservation. --- .../snapshots.toml | 29 ++++ .../npm_global_uninstall_prefix_shorthand.md | 145 ++++++++++++++++++ crates/vp_global_cli/src/shim/dispatch.rs | 29 +++- 3 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_prefix_shorthand.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml index b6c3bc84eb..4cdaf7a014 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml @@ -43,3 +43,32 @@ steps = [ ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], ] + +[[case]] +name = "npm_global_uninstall_prefix_shorthand" +vp = "global" +skip-platforms = ["windows"] +unset-env = ["NPM_CONFIG_PREFIX"] +steps = [ + { tty = false, argv = ["npm", "install", "-g", "--prefix", "./custom-prefix", "./npm-global-prefix-pkg"] }, + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "uninstall", "-g", "-C", "./custom-prefix", "npm-global-prefix-pkg", "--dry-run"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/other-prefix"]], comment = "A shorthand prefix override must still preserve links during dry-run" }, + ["npm-global-prefix-cli"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "uninstall", "-g", "-C", "./custom-prefix", "npm-global-prefix-pkg"], envs = [["NPM_CONFIG_PREFIX", "${workspace}/other-prefix"]], comment = "-C must override the environment prefix for uninstall link cleanup" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], + ["vpt", "write-file", "prefix.npmrc", "prefix=${HOME}/npm-prefix\n"], + { tty = false, argv = ["npm", "install", "-g", "-C=./custom-prefix", "./npm-global-prefix-pkg"], envs = [["npm_config_userconfig", "${workspace}/prefix.npmrc"]], comment = "-C= must override the user npmrc prefix when creating links" }, + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + ["npm-global-prefix-cli"], + { tty = false, argv = ["npm", "-C", "./custom-prefix", "uninstall", "-g", "npm-global-prefix-pkg"], envs = [["npm_config_userconfig", "${workspace}/prefix.npmrc"]], comment = "-C before the subcommand must also override the user npmrc prefix for cleanup" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_prefix_shorthand.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_prefix_shorthand.md new file mode 100644 index 0000000000..77f77aaaaf --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_prefix_shorthand.md @@ -0,0 +1,145 @@ +# npm_global_uninstall_prefix_shorthand + +## `npm install -g --prefix ./custom-prefix ./npm-global-prefix-pkg` + +``` + +added 1 package in +Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `NPM_CONFIG_PREFIX=${workspace}/other-prefix npm uninstall -g -C ./custom-prefix npm-global-prefix-pkg --dry-run` + +A shorthand prefix override must still preserve links during dry-run + +``` +remove npm-global-prefix-pkg 1.0.0 + +removed 1 package in +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `NPM_CONFIG_PREFIX=${workspace}/other-prefix npm uninstall -g -C ./custom-prefix npm-global-prefix-pkg` + +-C must override the environment prefix for uninstall link cleanup + +``` + +removed 1 package in +Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg: missing +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert missing` + +``` +custom-prefix/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing` + +``` +/.vite-plus/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert missing` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: missing +``` + +## `vpt write-file prefix.npmrc 'prefix=${HOME}/npm-prefix +'` + +``` +``` + +## `npm_config_userconfig=${workspace}/prefix.npmrc npm install -g -C=./custom-prefix ./npm-global-prefix-pkg` + +-C= must override the user npmrc prefix when creating links + +``` + +added 1 package in +Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `npm_config_userconfig=${workspace}/prefix.npmrc npm -C ./custom-prefix uninstall -g npm-global-prefix-pkg` + +-C before the subcommand must also override the user npmrc prefix for cleanup + +``` + +removed 1 package in +Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg: missing +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert missing` + +``` +custom-prefix/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing` + +``` +/.vite-plus/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert missing` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: missing +``` diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 0c53a7ce48..5b23827fd9 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -30,12 +30,12 @@ use crate::{ struct NpmGlobalCommand { /// Package names/specs extracted from args (e.g., ["codex", "typescript@5"]) packages: Vec, - /// Explicit `--prefix ` from the CLI args, if present. + /// Explicit `--prefix ` or `-C ` from the CLI args, if present. explicit_prefix: Option, } /// Value-bearing npm flags whose next arg should be skipped during package extraction. -/// Note: `--prefix` is handled separately to capture its value. +/// Note: `--prefix` and `-C` are handled separately to capture their values. const NPM_VALUE_FLAGS: &[&str] = &["--registry", "--tag", "--cache", "--tmp"]; /// Install subcommands recognized by npm. @@ -59,7 +59,7 @@ fn parse_npm_global_command(args: &[String], subcommands: &[&str]) -> Option Option Date: Mon, 19 Oct 2026 12:09:11 +0900 Subject: [PATCH 3/6] fix(shim): check the original npm installation path after uninstall Keep the package path collected before uninstall so a surviving local copy cannot be mistaken for the removed global installation. Verify that real uninstall cleans up global links while preserving local packages, and that dry runs still preserve the installed command. --- .../snapshots.toml | 25 ++++ ...m_global_uninstall_preserves_local_copy.md | 122 ++++++++++++++++++ crates/vp_global_cli/src/shim/dispatch.rs | 32 +++-- 3 files changed, 170 insertions(+), 9 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_preserves_local_copy.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml index 4cdaf7a014..dff26bdb8a 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml @@ -44,6 +44,31 @@ steps = [ ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], ] +[[case]] +name = "npm_global_uninstall_preserves_local_copy" +vp = "global" +skip-platforms = ["windows"] +steps = [ + ["vpt", "mkdir", "-p", "custom-prefix"], + ["vpt", "write-file", "custom-prefix/package.json", "{\"private\":true}"], + { tty = false, argv = ["npm", "install", "--prefix", "./custom-prefix", "--ignore-scripts", "--no-save", "--no-package-lock", "--no-audit", "./npm-global-prefix-pkg"], comment = "Install a local copy under the same prefix" }, + { tty = false, argv = ["npm", "install", "-g", "--prefix", "./custom-prefix", "./npm-global-prefix-pkg"], comment = "Install a separate global copy and create its Vite+ link" }, + ["vpt", "stat-file", "custom-prefix/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "uninstall", "-g", "--prefix", "./custom-prefix", "npm-global-prefix-pkg", "--dry-run"], comment = "Dry-run must preserve the global command even when a local copy also exists" }, + ["npm-global-prefix-cli"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "uninstall", "-g", "--prefix", "./custom-prefix", "npm-global-prefix-pkg"], comment = "Real uninstall must remove the global links without confusing the remaining local copy for the global installation" }, + ["vpt", "stat-file", "custom-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "custom-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], + ["vpt", "stat-file", "custom-prefix/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["node", "custom-prefix/node_modules/npm-global-prefix-pkg/cli.js"], +] + [[case]] name = "npm_global_uninstall_prefix_shorthand" vp = "global" diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_preserves_local_copy.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_preserves_local_copy.md new file mode 100644 index 0000000000..4cd12b5e3d --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_preserves_local_copy.md @@ -0,0 +1,122 @@ +# npm_global_uninstall_preserves_local_copy + +## `vpt mkdir -p custom-prefix` + +``` +``` + +## `vpt write-file custom-prefix/package.json '{"private":true}'` + +``` +``` + +## `npm install --prefix ./custom-prefix --ignore-scripts --no-save --no-package-lock --no-audit ./npm-global-prefix-pkg` + +Install a local copy under the same prefix + +``` + +added 1 package in +``` + +## `npm install -g --prefix ./custom-prefix ./npm-global-prefix-pkg` + +Install a separate global copy and create its Vite+ link + +``` + +added 1 package in +Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file custom-prefix/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +custom-prefix/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm uninstall -g --prefix ./custom-prefix npm-global-prefix-pkg --dry-run` + +Dry-run must preserve the global command even when a local copy also exists + +``` +remove npm-global-prefix-pkg 1.0.0 + +removed 1 package in +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm uninstall -g --prefix ./custom-prefix npm-global-prefix-pkg` + +Real uninstall must remove the global links without confusing the remaining local copy for the global installation + +``` + +removed 1 package in +Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file custom-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` + +``` +custom-prefix/lib/node_modules/npm-global-prefix-pkg: missing +``` + +## `vpt stat-file custom-prefix/bin/npm-global-prefix-cli --assert missing` + +``` +custom-prefix/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing` + +``` +/.vite-plus/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert missing` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: missing +``` + +## `vpt stat-file custom-prefix/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +custom-prefix/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `node custom-prefix/node_modules/npm-global-prefix-pkg/cli.js` + +``` +npm-global-prefix-cli works +``` diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 5b23827fd9..04731ab14f 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -500,10 +500,13 @@ fn dedup_missing_bins( /// When a bin is owned by a **different** npm package (not being uninstalled), npm may /// still delete its binary from `npm_bin_dir`, leaving our symlink dangling. In that /// case we repair the link by pointing directly at the surviving package's binary. -fn remove_npm_global_uninstall_links(bin_entries: &[(String, String)], npm_prefix: &AbsolutePath) { +fn remove_npm_global_uninstall_links( + bin_entries: &[(String, String, AbsolutePathBuf)], + npm_prefix: &AbsolutePath, +) { let Ok(bin_dir) = config::get_bin_dir() else { return }; - for (bin_name, package_name) in bin_entries { + for (bin_name, package_name, package_dir) in bin_entries { // Skip protected shims: a stale Npm BinConfig must not let // `npm uninstall -g` delete a default shim that `vp env setup` owns. if is_protected_shim(bin_name, false) { @@ -511,7 +514,6 @@ fn remove_npm_global_uninstall_links(bin_entries: &[(String, String)], npm_prefi } // A successful npm command may be a dry-run configured through flags, env, or npmrc. - let package_dir = config::get_node_modules_dir(npm_prefix, package_name); if package_dir.as_path().try_exists().unwrap_or(true) { continue; } @@ -610,23 +612,23 @@ fn read_npm_package_json( }) } -/// Collect (bin_name, package_name) pairs from packages by reading their installed package.json files. +/// Collect bin names, package names, and their pre-uninstall installation paths. fn collect_bin_names_from_npm( packages: &[String], npm_prefix: &AbsolutePath, -) -> Vec<(String, String)> { +) -> Vec<(String, String, AbsolutePathBuf)> { let mut all_bins = Vec::new(); for spec in packages { let Some(package_name) = resolve_package_name(spec) else { continue }; - let package_json_path = - config::get_node_modules_dir(npm_prefix, &package_name).join("package.json"); + let package_dir = config::get_node_modules_dir(npm_prefix, &package_name); + let package_json_path = package_dir.join("package.json"); let Ok(content) = std::fs::read_to_string(package_json_path.as_path()) else { continue }; let Ok(package_json) = serde_json::from_str::(&content) else { continue; }; for bin_name in extract_bin_names(&package_json) { - all_bins.push((bin_name, package_name.clone())); + all_bins.push((bin_name, package_name.clone(), package_dir.clone())); } } @@ -1907,6 +1909,11 @@ mod tests { std::fs::create_dir_all(&package_dir).unwrap(); let source = package_dir.join("cli.js"); std::fs::write(&source, "console.log('test');").unwrap(); + std::fs::write( + package_dir.join("package.json"), + r#"{"name":"test-package","bin":{"test-cli":"cli.js"}}"#, + ) + .unwrap(); vp_shared::EnvConfig::with_vars([(env_vars::VP_HOME, temp.path())], |_| { let bin_dir = config::get_bin_dir().unwrap(); @@ -1917,7 +1924,12 @@ mod tests { let metadata = std::fs::read(&metadata_path).unwrap(); assert!(link.as_path().exists()); - let bins = vec![("test-cli".to_string(), "test-package".to_string())]; + let bins = collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix); + assert_eq!(bins.len(), 1); + #[cfg(unix)] + let local_dir = npm_prefix.join("node_modules").join("test-package"); + #[cfg(unix)] + std::fs::create_dir_all(&local_dir).unwrap(); remove_npm_global_uninstall_links(&bins, &npm_prefix); assert!(link.as_path().exists(), "dry-run must preserve the installed command"); @@ -1930,6 +1942,8 @@ mod tests { assert!(std::fs::symlink_metadata(&link).is_err()); assert!(!metadata_path.as_path().exists()); + #[cfg(unix)] + assert!(local_dir.as_path().exists()); #[cfg(windows)] assert!(!bin_dir.join("test-cli.cmd").as_path().exists()); }); From 388f19eeed0eedf9b6c9a32bec3e0b554c57dd88 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 19 Oct 2026 12:26:36 +0900 Subject: [PATCH 4/6] fix(shim): restore safe npm uninstall fallback lookup Restore lookup in the selected Node installation when the configured npm prefix has no readable package manifest. Retain the actual installation path so dry runs and no-op uninstalls preserve links to surviving packages. Reuse the existing package lookup helper without adding npm configuration parsing. Cover userconfig overrides with a private runtime copy so tests do not modify the shared runtime seed. --- .../private-runtime.mjs | 15 +++ .../snapshots.toml | 26 ++++ ...pm_global_uninstall_userconfig_fallback.md | 123 ++++++++++++++++++ crates/vp_global_cli/src/shim/dispatch.rs | 87 ++++++++++--- 4 files changed, 232 insertions(+), 19 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs new file mode 100644 index 0000000000..11805b87c7 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs @@ -0,0 +1,15 @@ +import assert from 'node:assert/strict'; +import { cpSync, lstatSync, mkdirSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; + +// Keep global npm installs out of the runner's shared runtime seed. +const runtimeLink = join(process.env.VP_HOME, 'js_runtime'); +assert(lstatSync(runtimeLink).isSymbolicLink()); +const privateRuntime = resolve('private-runtime'); +const nodePrefix = join(privateRuntime, 'node', process.versions.node); +mkdirSync(dirname(nodePrefix), { recursive: true }); +cpSync(dirname(dirname(process.execPath)), nodePrefix, { recursive: true, verbatimSymlinks: true }); +unlinkSync(runtimeLink); +symlinkSync(privateRuntime, runtimeLink, 'dir'); +symlinkSync(nodePrefix, 'node-prefix', 'dir'); +writeFileSync('.node-version', process.versions.node); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml index dff26bdb8a..56eabd5d98 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml @@ -69,6 +69,32 @@ steps = [ ["node", "custom-prefix/node_modules/npm-global-prefix-pkg/cli.js"], ] +[[case]] +name = "npm_global_uninstall_userconfig_fallback" +vp = "global" +skip-platforms = ["windows"] +unset-env = ["NPM_CONFIG_PREFIX"] +steps = [ + ["node", "private-runtime.mjs"], + { tty = false, argv = ["npm", "install", "-g", "./npm-global-prefix-pkg"], comment = "Install into a private copy of the selected Node runtime" }, + ["vpt", "stat-file", "node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "config", "set", "prefix", "./other-prefix", "--location=user"], comment = "Make the normal user config point to another prefix" }, + ["vpt", "write-file", "empty.npmrc", ""], + { tty = false, argv = ["npm", "uninstall", "-g", "--userconfig=empty.npmrc", "npm-global-prefix-pkg", "--dry-run"], comment = "A config override dry-run must preserve the fallback installation" }, + ["npm-global-prefix-cli"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "uninstall", "-g", "npm-global-prefix-pkg"], comment = "Uninstalling from an empty prefix must not remove links to the surviving fallback package" }, + ["npm-global-prefix-cli"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], + { tty = false, argv = ["npm", "uninstall", "-g", "--userconfig", "empty.npmrc", "npm-global-prefix-pkg"], comment = "A real uninstall using the override must clean up the fallback package's links" }, + ["vpt", "stat-file", "node-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "node-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], +] + [[case]] name = "npm_global_uninstall_prefix_shorthand" vp = "global" diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md new file mode 100644 index 0000000000..f21183071f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md @@ -0,0 +1,123 @@ +# npm_global_uninstall_userconfig_fallback + +## `node private-runtime.mjs` + +``` +``` + +## `npm install -g ./npm-global-prefix-pkg` + +Install into a private copy of the selected Node runtime + +``` + +added 1 package in +Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` + +``` +node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` + +``` +/.vite-plus/bin/npm-global-prefix-cli: symlink +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm config set prefix ./other-prefix --location=user` + +Make the normal user config point to another prefix + +``` +``` + +## `vpt write-file empty.npmrc ` + +``` +``` + +## `npm uninstall -g --userconfig=empty.npmrc npm-global-prefix-pkg --dry-run` + +A config override dry-run must preserve the fallback installation + +``` +remove npm-global-prefix-pkg 1.0.0 + +removed 1 package in +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm uninstall -g npm-global-prefix-pkg` + +Uninstalling from an empty prefix must not remove links to the surviving fallback package + +``` + +up to date in +``` + +## `npm-global-prefix-cli` + +``` +npm-global-prefix-cli works +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert file` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: file +``` + +## `npm uninstall -g --userconfig empty.npmrc npm-global-prefix-pkg` + +A real uninstall using the override must clean up the fallback package's links + +``` + +removed 1 package in +Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli +``` + +## `vpt stat-file node-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` + +``` +node-prefix/lib/node_modules/npm-global-prefix-pkg: missing +``` + +## `vpt stat-file node-prefix/bin/npm-global-prefix-cli --assert missing` + +``` +node-prefix/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing` + +``` +/.vite-plus/bin/npm-global-prefix-cli: missing +``` + +## `vpt stat-file $VP_HOME/bins/npm-global-prefix-cli.json --assert missing` + +``` +/.vite-plus/bins/npm-global-prefix-cli.json: missing +``` diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 04731ab14f..90a9429928 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -240,7 +240,7 @@ fn check_npm_global_install_result( for spec in packages { let Some(package_name) = resolve_package_name(spec) else { continue }; - let Some(content) = read_npm_package_json(npm_prefix, node_dir, &package_name) else { + let Some((_, content)) = read_npm_package_json(npm_prefix, node_dir, &package_name) else { continue; }; let Ok(package_json) = serde_json::from_str::(&content) else { @@ -595,20 +595,18 @@ fn remove_npm_global_uninstall_links( } } -/// Read the installed package.json from npm's node_modules directory. +/// Read the installed package.json and return its installation directory with the contents. /// Tries the npm prefix first (handles custom prefix), then falls back to node_dir. fn read_npm_package_json( npm_prefix: &AbsolutePath, node_dir: &AbsolutePath, package_name: &str, -) -> Option { - std::fs::read_to_string( - config::get_node_modules_dir(npm_prefix, package_name).join("package.json").as_path(), - ) - .ok() - .or_else(|| { - let dir = config::get_node_modules_dir(node_dir, package_name); - std::fs::read_to_string(dir.join("package.json").as_path()).ok() +) -> Option<(AbsolutePathBuf, String)> { + [npm_prefix, node_dir].into_iter().find_map(|prefix| { + let dir = config::get_node_modules_dir(prefix, package_name); + std::fs::read_to_string(dir.join("package.json").as_path()) + .ok() + .map(|content| (dir, content)) }) } @@ -616,14 +614,17 @@ fn read_npm_package_json( fn collect_bin_names_from_npm( packages: &[String], npm_prefix: &AbsolutePath, + node_dir: &AbsolutePath, ) -> Vec<(String, String, AbsolutePathBuf)> { let mut all_bins = Vec::new(); for spec in packages { let Some(package_name) = resolve_package_name(spec) else { continue }; - let package_dir = config::get_node_modules_dir(npm_prefix, &package_name); - let package_json_path = package_dir.join("package.json"); - let Ok(content) = std::fs::read_to_string(package_json_path.as_path()) else { continue }; + let Some((package_dir, content)) = + read_npm_package_json(npm_prefix, node_dir, &package_name) + else { + continue; + }; let Ok(package_json) = serde_json::from_str::(&content) else { continue; }; @@ -918,7 +919,7 @@ pub async fn dispatch(tool: &str, args: &[String], env: ToolPathEnv) -> i32 { // Collect bin names before uninstall (package.json will be gone after) let node_dir = node_prefix_from_binary(&node_path); let npm_prefix = resolve_npm_prefix(&parsed, &tool_path, &node_dir, &child_env); - let bin_names = collect_bin_names_from_npm(&parsed.packages, &npm_prefix); + let bin_names = collect_bin_names_from_npm(&parsed.packages, &npm_prefix, &node_dir); let exit_code = exec::spawn_tool(&tool_path, args, child_env); if exit_code == 0 { remove_npm_global_uninstall_links(&bin_names, &npm_prefix); @@ -1924,7 +1925,7 @@ mod tests { let metadata = std::fs::read(&metadata_path).unwrap(); assert!(link.as_path().exists()); - let bins = collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix); + let bins = collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix, &npm_prefix); assert_eq!(bins.len(), 1); #[cfg(unix)] let local_dir = npm_prefix.join("node_modules").join("test-package"); @@ -1950,7 +1951,31 @@ mod tests { } #[test] - fn test_npm_uninstall_does_not_collect_bins_from_another_prefix() { + fn test_npm_uninstall_prefers_configured_prefix() { + let temp = TempDir::new().unwrap(); + let npm_prefix = AbsolutePathBuf::new(temp.path().join("npm-prefix")).unwrap(); + let node_dir = AbsolutePathBuf::new(temp.path().join("node")).unwrap(); + for prefix in [&npm_prefix, &node_dir] { + let package_dir = config::get_node_modules_dir(prefix, "test-package"); + std::fs::create_dir_all(&package_dir).unwrap(); + std::fs::write( + package_dir.join("package.json"), + r#"{"name":"test-package","bin":{"test-cli":"cli.js"}}"#, + ) + .unwrap(); + } + assert_eq!( + collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix, &node_dir), + vec![( + "test-cli".to_string(), + "test-package".to_string(), + config::get_node_modules_dir(&npm_prefix, "test-package"), + )] + ); + } + + #[test] + fn test_npm_uninstall_preserves_fallback_links_until_package_is_removed() { let temp = TempDir::new().unwrap(); let npm_prefix = AbsolutePathBuf::new(temp.path().join("npm-prefix")).unwrap(); let node_dir = AbsolutePathBuf::new(temp.path().join("node")).unwrap(); @@ -1962,10 +1987,34 @@ mod tests { ) .unwrap(); - assert!( - collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix).is_empty(), - "uninstall must only collect binaries installed in the selected npm prefix" + let source = package_dir.join("cli.js"); + std::fs::write(&source, "console.log('test');").unwrap(); + let bins = collect_bin_names_from_npm(&s(&["test-package"]), &npm_prefix, &node_dir); + assert_eq!( + bins, + vec![("test-cli".to_string(), "test-package".to_string(), package_dir.clone())] ); + + vp_shared::EnvConfig::with_vars([(env_vars::VP_HOME, temp.path())], |_| { + let bin_dir = config::get_bin_dir().unwrap(); + std::fs::create_dir_all(&bin_dir).unwrap(); + create_bin_link(&bin_dir, "test-cli", &source, "test-package", "22.18.0"); + let link = bin_dir.join("test-cli"); + let metadata_path = BinConfig::path("test-cli").unwrap(); + let metadata = std::fs::read(&metadata_path).unwrap(); + + remove_npm_global_uninstall_links(&bins, &npm_prefix); + assert!( + link.as_path().exists(), + "a surviving fallback installation must keep its link" + ); + assert_eq!(std::fs::read(&metadata_path).unwrap(), metadata); + + std::fs::remove_dir_all(&package_dir).unwrap(); + remove_npm_global_uninstall_links(&bins, &npm_prefix); + assert!(std::fs::symlink_metadata(&link).is_err()); + assert!(!metadata_path.as_path().exists()); + }); } // --- parse_npm_global_uninstall tests --- From 0bdd2c8c13aac53d28dd7607b9b98a5136385b52 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 19 Oct 2026 13:45:57 +0900 Subject: [PATCH 5/6] test(snapshots): use an unseeded runtime for npm fallback coverage Replace the private runtime copy script with seed-runtime = false and a pinned Node version. Keep the package and link assertions and verify that the runtime directory is not a shared symlink before global npm writes. This case now downloads its own runtime rather than copying the shared seed. --- .../private-runtime.mjs | 15 ------------- .../snapshots.toml | 14 +++++++----- ...pm_global_uninstall_userconfig_fallback.md | 22 ++++++++++++------- 3 files changed, 23 insertions(+), 28 deletions(-) delete mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs deleted file mode 100644 index 11805b87c7..0000000000 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs +++ /dev/null @@ -1,15 +0,0 @@ -import assert from 'node:assert/strict'; -import { cpSync, lstatSync, mkdirSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; - -// Keep global npm installs out of the runner's shared runtime seed. -const runtimeLink = join(process.env.VP_HOME, 'js_runtime'); -assert(lstatSync(runtimeLink).isSymbolicLink()); -const privateRuntime = resolve('private-runtime'); -const nodePrefix = join(privateRuntime, 'node', process.versions.node); -mkdirSync(dirname(nodePrefix), { recursive: true }); -cpSync(dirname(dirname(process.execPath)), nodePrefix, { recursive: true, verbatimSymlinks: true }); -unlinkSync(runtimeLink); -symlinkSync(privateRuntime, runtimeLink, 'dir'); -symlinkSync(nodePrefix, 'node-prefix', 'dir'); -writeFileSync('.node-version', process.versions.node); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml index 56eabd5d98..5bbc43abd0 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml @@ -72,12 +72,16 @@ steps = [ [[case]] name = "npm_global_uninstall_userconfig_fallback" vp = "global" +comment = "This case downloads its own pinned Node runtime so global npm writes stay isolated from the shared seed." skip-platforms = ["windows"] +seed-runtime = false +env = { VP_NODE_VERSION = "22.18.0" } unset-env = ["NPM_CONFIG_PREFIX"] steps = [ - ["node", "private-runtime.mjs"], - { tty = false, argv = ["npm", "install", "-g", "./npm-global-prefix-pkg"], comment = "Install into a private copy of the selected Node runtime" }, - ["vpt", "stat-file", "node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + { argv = ["node", "--version"], snapshot = false }, + ["vpt", "stat-file", "$VP_HOME/js_runtime", "--assert", "dir"], + { tty = false, argv = ["npm", "install", "-g", "./npm-global-prefix-pkg"], comment = "Install into the test-owned Node runtime" }, + ["vpt", "stat-file", "$VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], { tty = false, argv = ["npm", "config", "set", "prefix", "./other-prefix", "--location=user"], comment = "Make the normal user config point to another prefix" }, @@ -89,8 +93,8 @@ steps = [ ["npm-global-prefix-cli"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], { tty = false, argv = ["npm", "uninstall", "-g", "--userconfig", "empty.npmrc", "npm-global-prefix-pkg"], comment = "A real uninstall using the override must clean up the fallback package's links" }, - ["vpt", "stat-file", "node-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], - ["vpt", "stat-file", "node-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "$VP_HOME/js_runtime/node/22.18.0/bin/npm-global-prefix-cli", "--assert", "missing"], ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], ] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md index f21183071f..915109a3ba 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md @@ -1,13 +1,19 @@ # npm_global_uninstall_userconfig_fallback -## `node private-runtime.mjs` +This case downloads its own pinned Node runtime so global npm writes stay isolated from the shared seed. + +## `node --version` + + +## `vpt stat-file $VP_HOME/js_runtime --assert dir` ``` +/.vite-plus/js_runtime: dir ``` ## `npm install -g ./npm-global-prefix-pkg` -Install into a private copy of the selected Node runtime +Install into the test-owned Node runtime ``` @@ -15,10 +21,10 @@ added 1 package in Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli ``` -## `vpt stat-file node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` +## `vpt stat-file $VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` ``` -node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file +/.vite-plus/js_runtime/node//lib/node_modules/npm-global-prefix-pkg/package.json: file ``` ## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` @@ -98,16 +104,16 @@ removed 1 package in Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli ``` -## `vpt stat-file node-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` +## `vpt stat-file $VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg --assert missing` ``` -node-prefix/lib/node_modules/npm-global-prefix-pkg: missing +/.vite-plus/js_runtime/node//lib/node_modules/npm-global-prefix-pkg: missing ``` -## `vpt stat-file node-prefix/bin/npm-global-prefix-cli --assert missing` +## `vpt stat-file $VP_HOME/js_runtime/node/22.18.0/bin/npm-global-prefix-cli --assert missing` ``` -node-prefix/bin/npm-global-prefix-cli: missing +/.vite-plus/js_runtime/node//bin/npm-global-prefix-cli: missing ``` ## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing` From 93385e58bb050084157f9cdabf5acf3ed3a524d4 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Mon, 19 Oct 2026 14:14:37 +0900 Subject: [PATCH 6/6] test(snapshots): isolate npm fallback coverage without runtime downloads Restore a private copy of the seeded runtime so global npm writes stay isolated without downloading Node for each case. Point the Node distribution mirror at a closed local endpoint to catch accidental download dependencies. Keep the existing uninstall assertions and regenerate the snapshot. --- .../private-runtime.mjs | 15 +++++++++++++ .../snapshots.toml | 16 ++++++-------- ...pm_global_uninstall_userconfig_fallback.md | 22 +++++++------------ 3 files changed, 30 insertions(+), 23 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs new file mode 100644 index 0000000000..11805b87c7 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/private-runtime.mjs @@ -0,0 +1,15 @@ +import assert from 'node:assert/strict'; +import { cpSync, lstatSync, mkdirSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; + +// Keep global npm installs out of the runner's shared runtime seed. +const runtimeLink = join(process.env.VP_HOME, 'js_runtime'); +assert(lstatSync(runtimeLink).isSymbolicLink()); +const privateRuntime = resolve('private-runtime'); +const nodePrefix = join(privateRuntime, 'node', process.versions.node); +mkdirSync(dirname(nodePrefix), { recursive: true }); +cpSync(dirname(dirname(process.execPath)), nodePrefix, { recursive: true, verbatimSymlinks: true }); +unlinkSync(runtimeLink); +symlinkSync(privateRuntime, runtimeLink, 'dir'); +symlinkSync(nodePrefix, 'node-prefix', 'dir'); +writeFileSync('.node-version', process.versions.node); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml index 5bbc43abd0..68e2e17940 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots.toml @@ -72,16 +72,14 @@ steps = [ [[case]] name = "npm_global_uninstall_userconfig_fallback" vp = "global" -comment = "This case downloads its own pinned Node runtime so global npm writes stay isolated from the shared seed." skip-platforms = ["windows"] -seed-runtime = false -env = { VP_NODE_VERSION = "22.18.0" } +# Require the seeded runtime instead of downloading one for this case. +env = { VP_NODE_DIST_MIRROR = "http://127.0.0.1:9" } unset-env = ["NPM_CONFIG_PREFIX"] steps = [ - { argv = ["node", "--version"], snapshot = false }, - ["vpt", "stat-file", "$VP_HOME/js_runtime", "--assert", "dir"], - { tty = false, argv = ["npm", "install", "-g", "./npm-global-prefix-pkg"], comment = "Install into the test-owned Node runtime" }, - ["vpt", "stat-file", "$VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], + ["node", "private-runtime.mjs"], + { tty = false, argv = ["npm", "install", "-g", "./npm-global-prefix-pkg"], comment = "Install into a private copy of the selected Node runtime" }, + ["vpt", "stat-file", "node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json", "--assert", "file"], ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "symlink"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], { tty = false, argv = ["npm", "config", "set", "prefix", "./other-prefix", "--location=user"], comment = "Make the normal user config point to another prefix" }, @@ -93,8 +91,8 @@ steps = [ ["npm-global-prefix-cli"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "file"], { tty = false, argv = ["npm", "uninstall", "-g", "--userconfig", "empty.npmrc", "npm-global-prefix-pkg"], comment = "A real uninstall using the override must clean up the fallback package's links" }, - ["vpt", "stat-file", "$VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], - ["vpt", "stat-file", "$VP_HOME/js_runtime/node/22.18.0/bin/npm-global-prefix-cli", "--assert", "missing"], + ["vpt", "stat-file", "node-prefix/lib/node_modules/npm-global-prefix-pkg", "--assert", "missing"], + ["vpt", "stat-file", "node-prefix/bin/npm-global-prefix-cli", "--assert", "missing"], ["vpt", "stat-file", "$VP_HOME/bin/npm-global-prefix-cli", "--assert", "missing"], ["vpt", "stat-file", "$VP_HOME/bins/npm-global-prefix-cli.json", "--assert", "missing"], ] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md index 915109a3ba..f21183071f 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/npm_global_uninstall_prefix/snapshots/npm_global_uninstall_userconfig_fallback.md @@ -1,19 +1,13 @@ # npm_global_uninstall_userconfig_fallback -This case downloads its own pinned Node runtime so global npm writes stay isolated from the shared seed. - -## `node --version` - - -## `vpt stat-file $VP_HOME/js_runtime --assert dir` +## `node private-runtime.mjs` ``` -/.vite-plus/js_runtime: dir ``` ## `npm install -g ./npm-global-prefix-pkg` -Install into the test-owned Node runtime +Install into a private copy of the selected Node runtime ``` @@ -21,10 +15,10 @@ added 1 package in Linked 'npm-global-prefix-cli' to /.vite-plus/bin/npm-global-prefix-cli ``` -## `vpt stat-file $VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` +## `vpt stat-file node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json --assert file` ``` -/.vite-plus/js_runtime/node//lib/node_modules/npm-global-prefix-pkg/package.json: file +node-prefix/lib/node_modules/npm-global-prefix-pkg/package.json: file ``` ## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert symlink` @@ -104,16 +98,16 @@ removed 1 package in Removed link 'npm-global-prefix-cli' from /.vite-plus/bin/npm-global-prefix-cli ``` -## `vpt stat-file $VP_HOME/js_runtime/node/22.18.0/lib/node_modules/npm-global-prefix-pkg --assert missing` +## `vpt stat-file node-prefix/lib/node_modules/npm-global-prefix-pkg --assert missing` ``` -/.vite-plus/js_runtime/node//lib/node_modules/npm-global-prefix-pkg: missing +node-prefix/lib/node_modules/npm-global-prefix-pkg: missing ``` -## `vpt stat-file $VP_HOME/js_runtime/node/22.18.0/bin/npm-global-prefix-cli --assert missing` +## `vpt stat-file node-prefix/bin/npm-global-prefix-cli --assert missing` ``` -/.vite-plus/js_runtime/node//bin/npm-global-prefix-cli: missing +node-prefix/bin/npm-global-prefix-cli: missing ``` ## `vpt stat-file $VP_HOME/bin/npm-global-prefix-cli --assert missing`