From 3e82e1002572c91fc79eda8e834bbdcc6b36340f Mon Sep 17 00:00:00 2001 From: MK Date: Fri, 22 May 2026 17:18:09 +0800 Subject: [PATCH 1/6] feat(pm): add `vp pm approve-builds` subcommand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a unified `vp pm approve-builds` command that mirrors `pnpm approve-builds` and adapts to `bun pm trust`, with informative warn-and-noop fallbacks for npm and yarn. Surface (intentionally tight, matches pnpm's documented flags): vp pm approve-builds # interactive (pnpm) vp pm approve-builds esbuild # approve named packages vp pm approve-builds esbuild !core-js # pnpm >= 11.0.0 (deny syntax) vp pm approve-builds --all # pnpm >= 10.32.0 / bun vp pm approve-builds -- # forward to underlying PM Cross-PM behavior: - pnpm: pass-through. Version-gates `--all` on >= 10.32.0 and `!pkg` deny syntax on >= 11.0.0 (per pnpm PR #11030); prerelease versions compared against the lowest prerelease floor (`10.32.0-0` / `11.0.0-0`) so RCs are accepted. - bun: `bun pm trust [--all] [pkgs...]`. `!pkg` tokens emit a warn and are filtered (bun has no denylist model). When only deny tokens are given, the warn alone is enough context — no redundant note. - npm: warn and exit 0, pointing at `ignore-scripts=true` in `.npmrc`. - yarn: warn and exit 0. Yarn 1 (Classic) gets an npm-style hint (lifecycle scripts run by default); yarn Berry gets `dependenciesMeta..built: true` advice. Safety: - `--all` and positional packages are mutually exclusive at the clap layer, preventing a silent override where `--all` would otherwise drop denylisted packages on bun. - Version-gate failures render via `Error::UserMessage` (no harsh `error:` prefix). Coverage: - 23 Rust unit tests (resolver, version gates, prereleases, deny gate, yarn 1 vs Berry, pass-through args, strict unparseable-version) - 8 clap parse tests (conflicts, lone-flag, lone-packages, pass-through capture, deny conflict) - 5 snap fixtures: local pnpm10/npm/yarn + global bun/pnpm10-old - RFC at `rfcs/approve-builds-command.md` --- .../src/commands/approve_builds.rs | 504 +++++++++++ crates/vite_install/src/commands/mod.rs | 1 + crates/vite_pm_cli/src/cli.rs | 69 ++ crates/vite_pm_cli/src/handlers.rs | 16 + .../cli-helper-message/snap.txt | 39 +- .../package.json | 5 + .../command-pm-approve-builds-bun/snap.txt | 25 + .../command-pm-approve-builds-bun/steps.json | 8 + .../package.json | 5 + .../snap.txt | 8 + .../steps.json | 7 + .../package.json | 6 + .../command-pm-approve-builds-npm/snap.txt | 21 + .../command-pm-approve-builds-npm/steps.json | 8 + .../package.json | 6 + .../command-pm-approve-builds-pnpm10/snap.txt | 18 + .../steps.json | 7 + .../package.json | 6 + .../command-pm-approve-builds-yarn/snap.txt | 21 + .../command-pm-approve-builds-yarn/steps.json | 9 + rfcs/approve-builds-command.md | 836 ++++++++++++++++++ 21 files changed, 1606 insertions(+), 19 deletions(-) create mode 100644 crates/vite_install/src/commands/approve_builds.rs create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-bun/package.json create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/package.json create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/snap.txt create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/steps.json create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-npm/package.json create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-npm/steps.json create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-pnpm10/package.json create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-pnpm10/steps.json create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-yarn/package.json create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt create mode 100644 packages/cli/snap-tests/command-pm-approve-builds-yarn/steps.json create mode 100644 rfcs/approve-builds-command.md diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs new file mode 100644 index 0000000000..96c7d7d6a6 --- /dev/null +++ b/crates/vite_install/src/commands/approve_builds.rs @@ -0,0 +1,504 @@ +use std::{collections::HashMap, process::ExitStatus}; + +use semver::Version; +use vite_command::run_command; +use vite_error::Error; +use vite_path::AbsolutePath; +use vite_shared::output; + +use crate::package_manager::{ + PackageManager, PackageManagerType, ResolveCommandResult, format_path_env, +}; + +/// Options for the approve-builds command. +#[derive(Debug, Default)] +pub struct ApproveBuildsCommandOptions<'a> { + /// Packages to approve. Prefix with `!` to deny (pnpm only). + pub packages: &'a [String], + /// Approve every package that is currently pending approval. + pub all: bool, + /// Extra arguments forwarded verbatim to the underlying package manager. + pub pass_through_args: Option<&'a [String]>, +} + +impl PackageManager { + /// Run the approve-builds command with the package manager. + /// Returns `ExitStatus` with success (0) when the command is a no-op + /// (npm, yarn, or bun with only deny tokens / no positionals). + pub async fn run_approve_builds_command( + &self, + options: &ApproveBuildsCommandOptions<'_>, + cwd: impl AsRef, + ) -> Result { + let Some(resolved) = self.resolve_approve_builds_command(options)? else { + return Ok(ExitStatus::default()); + }; + run_command(&resolved.bin_path, &resolved.args, &resolved.envs, cwd).await + } + + /// Resolve the approve-builds command. + /// Returns `None` when the command is a no-op for the detected PM + /// (npm/yarn warn; bun with no approve-tokens prints a contextual hint). + /// Returns `Err(Error::InvalidArgument)` when `--all` or `!pkg` is requested + /// on a pnpm version that does not support it. + pub fn resolve_approve_builds_command( + &self, + options: &ApproveBuildsCommandOptions, + ) -> Result, Error> { + let bin_name: String; + let mut args: Vec = Vec::new(); + + match self.client { + PackageManagerType::Pnpm => { + if options.all && !pnpm_supports_approve_builds_all(&self.version) { + return Err(Error::InvalidArgument( + "`--all` requires pnpm >= 10.32.0. Upgrade pnpm or pass package names explicitly.".into(), + )); + } + if options.packages.iter().any(|p| p.starts_with('!')) + && !pnpm_supports_deny_syntax(&self.version) + { + return Err(Error::InvalidArgument( + "`!` deny syntax requires pnpm >= 11.0.0. Upgrade pnpm or omit the `!` entries.".into(), + )); + } + bin_name = "pnpm".into(); + args.push("approve-builds".into()); + if options.all { + args.push("--all".into()); + } + args.extend(options.packages.iter().cloned()); + } + PackageManagerType::Bun => { + // bun has no allow/deny model — filter `!pkg` with a warning. + let (denies, approves): (Vec<&String>, Vec<&String>) = + options.packages.iter().partition(|p| p.starts_with('!')); + let has_denies = !denies.is_empty(); + if has_denies { + let names: Vec<&str> = + denies.iter().map(|p| p.strip_prefix('!').unwrap_or(p)).collect(); + output::warn(&format!( + "bun does not support denylisting build scripts. Packages outside \ + `trustedDependencies` in package.json are already denied by default. \ + Skipping: {}", + names.join(", ") + )); + } + + // No approves and not --all: bun has no interactive picker. + if approves.is_empty() && !options.all { + // If we already warned about denies, that message is enough context. + if !has_denies { + output::note( + "bun pm trust requires package names. Run `bun pm untrusted` to see \ + which packages are pending, then pass them explicitly: \ + `vp pm approve-builds [...]` or `vp pm approve-builds --all`.", + ); + } + return Ok(None); + } + + bin_name = "bun".into(); + args.push("pm".into()); + args.push("trust".into()); + if options.all { + args.push("--all".into()); + } + args.extend(approves.into_iter().cloned()); + } + PackageManagerType::Npm => { + output::warn( + "npm runs lifecycle scripts by default. To restrict them, set \ + `ignore-scripts=true` in .npmrc and rebuild approved packages with \ + `vp pm rebuild `.", + ); + return Ok(None); + } + PackageManagerType::Yarn => { + // Yarn 1 (Classic) runs lifecycle scripts by default; Berry (2+) blocks them. + if self.version.starts_with("1.") { + output::warn( + "yarn (v1) runs lifecycle scripts by default. To restrict them, set \ + `ignore-scripts true` in .npmrc and rebuild approved packages with \ + `vp pm rebuild `.", + ); + } else { + output::warn( + "yarn does not run third-party build scripts by default. To allow a \ + package, set `dependenciesMeta[\"\"].built: true` in package.json.", + ); + } + return Ok(None); + } + } + + // Append pass-through args to the underlying PM (pnpm/bun branches only). + if let Some(extra) = options.pass_through_args { + args.extend_from_slice(extra); + } + + let envs = HashMap::from([("PATH".to_string(), format_path_env(self.get_bin_prefix()))]); + Ok(Some(ResolveCommandResult { bin_path: bin_name, args, envs })) + } +} + +fn pnpm_supports_approve_builds_all(version: &str) -> bool { + // `pnpm approve-builds --all` was added in pnpm v10.32.0. + // Compare against the lowest prerelease of 10.32.0 (`10.32.0-0`) so that + // prereleases of 10.32.0 (e.g. `10.32.0-rc.0`, `10.32.0-beta.1`) also + // satisfy the gate per semver ordering rules. + // Unparseable versions are treated as not-supported (strict), since the + // production path always populates `version` from a validated semver. + let floor = Version::parse("10.32.0-0").expect("static semver"); + Version::parse(version).is_ok_and(|v| v >= floor) +} + +fn pnpm_supports_deny_syntax(version: &str) -> bool { + // `!` deny syntax shipped in pnpm v11.0.0 (PR #11030). + // Compare against `11.0.0-0` so prereleases of v11 also satisfy. + let floor = Version::parse("11.0.0-0").expect("static semver"); + Version::parse(version).is_ok_and(|v| v >= floor) +} + +#[cfg(test)] +mod tests { + use tempfile::{TempDir, tempdir}; + use vite_path::AbsolutePathBuf; + use vite_str::Str; + + use super::*; + + fn create_temp_dir() -> TempDir { + tempdir().expect("Failed to create temp directory") + } + + fn create_mock_package_manager(pm_type: PackageManagerType, version: &str) -> PackageManager { + let temp_dir = create_temp_dir(); + let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap(); + let install_dir = temp_dir_path.join("install"); + + PackageManager { + client: pm_type, + package_name: pm_type.to_string().into(), + version: Str::from(version), + hash: None, + bin_name: pm_type.to_string().into(), + workspace_root: temp_dir_path.clone(), + is_monorepo: false, + install_dir, + } + } + + #[test] + fn pnpm_no_args_interactive() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions::default()) + .expect("resolves") + .expect("supported"); + assert_eq!(result.bin_path, "pnpm"); + assert_eq!(result.args, vec!["approve-builds"]); + } + + #[test] + fn pnpm_with_packages() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let packages = vec!["esbuild".to_string(), "fsevents".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.bin_path, "pnpm"); + assert_eq!(result.args, vec!["approve-builds", "esbuild", "fsevents"]); + } + + #[test] + fn pnpm_v11_passes_deny_syntax_through() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "11.0.0"); + let packages = vec!["esbuild".to_string(), "!core-js".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["approve-builds", "esbuild", "!core-js"]); + } + + #[test] + fn pnpm_deny_rejected_below_v11() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let packages = vec!["esbuild".to_string(), "!core-js".to_string()]; + let err = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect_err("should reject !pkg on pnpm < 11"); + assert!(matches!(err, Error::InvalidArgument(_))); + } + + #[test] + fn pnpm_deny_accepts_v11_prerelease() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "11.0.0-rc.0"); + let packages = vec!["!core-js".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["approve-builds", "!core-js"]); + } + + #[test] + fn pnpm_all_flag() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["approve-builds", "--all"]); + } + + #[test] + fn pnpm_all_flag_with_newer_version() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "11.0.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["approve-builds", "--all"]); + } + + #[test] + fn pnpm_all_rejected_below_v10_32() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.31.0"); + let err = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect_err("should reject --all on old pnpm"); + assert!(matches!(err, Error::InvalidArgument(_))); + } + + #[test] + fn pnpm_all_accepts_v10_32_prerelease() { + for version in &["10.32.0-0", "10.32.0-rc.0", "10.32.0-beta.1"] { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, version); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .unwrap_or_else(|_| panic!("{version} should accept --all")) + .unwrap_or_else(|| panic!("{version} should resolve")); + assert_eq!(result.args, vec!["approve-builds", "--all"]); + } + } + + #[test] + fn pnpm_all_accepts_v11_prerelease() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "11.0.0-rc.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect("v11 prerelease should accept --all") + .expect("should resolve"); + assert_eq!(result.args, vec!["approve-builds", "--all"]); + } + + #[test] + fn pnpm_all_rejected_at_v10_31_patch_max() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.31.999"); + let err = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect_err("10.31.999 should still reject"); + assert!(matches!(err, Error::InvalidArgument(_))); + } + + #[test] + fn pnpm_unparseable_version_rejects_all() { + // Strict gate: unparseable versions (a corruption/edge case) fail the check. + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "latest"); + let err = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect_err("unparseable version should fail the gate"); + assert!(matches!(err, Error::InvalidArgument(_))); + } + + #[test] + fn pnpm_appends_pass_through_args() { + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let extra = vec!["--workspace-root".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + pass_through_args: Some(&extra), + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["approve-builds", "--all", "--workspace-root"]); + } + + #[test] + fn bun_trust_by_name() { + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let packages = vec!["esbuild".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.bin_path, "bun"); + assert_eq!(result.args, vec!["pm", "trust", "esbuild"]); + } + + #[test] + fn bun_trust_all() { + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["pm", "trust", "--all"]); + } + + #[test] + fn bun_filters_deny_syntax() { + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let packages = vec!["esbuild".to_string(), "!core-js".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + // !core-js is filtered out, only esbuild forwarded + assert_eq!(result.args, vec!["pm", "trust", "esbuild"]); + } + + #[test] + fn bun_only_deny_becomes_noop() { + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let packages = vec!["!core-js".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves"); + // After filtering !core-js, no positionals remain → no-op. + // The deny warn fired; no redundant no-args note. + assert!(result.is_none()); + } + + #[test] + fn bun_no_args_is_noop() { + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions::default()) + .expect("resolves"); + assert!(result.is_none()); + } + + #[test] + fn bun_strips_single_bang_only() { + // `!!foo` strips exactly one leading `!` for the warning message, + // preserving the user's intent in the displayed name. + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let packages = vec!["!!core-js".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves"); + // Still classified as a deny (starts with !), but the displayed name + // in the warning retains the second `!`. We can only assert no-op here; + // visual inspection of the warn is captured in snap-tests. + assert!(result.is_none()); + } + + #[test] + fn bun_appends_pass_through_args() { + let pm = create_mock_package_manager(PackageManagerType::Bun, "1.3.0"); + let extra = vec!["--silent".to_string()]; + let packages = vec!["esbuild".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + pass_through_args: Some(&extra), + ..Default::default() + }) + .expect("resolves") + .expect("supported"); + assert_eq!(result.args, vec!["pm", "trust", "esbuild", "--silent"]); + } + + #[test] + fn npm_warns_and_noop() { + let pm = create_mock_package_manager(PackageManagerType::Npm, "11.0.0"); + let packages = vec!["esbuild".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect("resolves"); + assert!(result.is_none()); + } + + #[test] + fn yarn_berry_warns_and_noop() { + let pm = create_mock_package_manager(PackageManagerType::Yarn, "4.0.0"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect("resolves"); + assert!(result.is_none()); + } + + #[test] + fn yarn1_warns_and_noop() { + // Yarn 1 emits an npm-style warning (lifecycle scripts run by default), + // distinct from the Berry message about dependenciesMeta.built. + let pm = create_mock_package_manager(PackageManagerType::Yarn, "1.22.22"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }) + .expect("resolves"); + assert!(result.is_none()); + } +} diff --git a/crates/vite_install/src/commands/mod.rs b/crates/vite_install/src/commands/mod.rs index 948880f6cb..9b3bbcda86 100644 --- a/crates/vite_install/src/commands/mod.rs +++ b/crates/vite_install/src/commands/mod.rs @@ -1,4 +1,5 @@ pub mod add; +pub mod approve_builds; pub mod audit; pub mod cache; pub mod config; diff --git a/crates/vite_pm_cli/src/cli.rs b/crates/vite_pm_cli/src/cli.rs index 64c25cd54f..9c86f95b57 100644 --- a/crates/vite_pm_cli/src/cli.rs +++ b/crates/vite_pm_cli/src/cli.rs @@ -554,6 +554,23 @@ impl PackageManagerCommand { /// Package manager subcommands (`vp pm `). #[derive(Subcommand, Debug, Clone)] pub enum PmCommands { + /// Approve dependency lifecycle scripts (install/postinstall) to run + #[command(name = "approve-builds")] + ApproveBuilds { + /// Packages to approve. Prefix with `!` to deny (pnpm >= 11.0.0 only). + /// Omit to launch interactive mode (pnpm only). + packages: Vec, + + /// Approve every package currently pending approval (pnpm >= 10.32.0). + /// Mutually exclusive with positional packages. + #[arg(long, conflicts_with = "packages")] + all: bool, + + /// Additional arguments to pass through to the package manager + #[arg(last = true, allow_hyphen_values = true)] + pass_through_args: Option>, + }, + /// Remove unnecessary packages Prune { /// Remove devDependencies @@ -1182,4 +1199,56 @@ mod tests { assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation); } + + #[test] + fn approve_builds_all_conflicts_with_packages() { + let error = parse_pm_command(&["vp", "pm", "approve-builds", "--all", "esbuild"]) + .expect_err("expected --all + positional to fail"); + + assert_eq!(error.kind(), clap::error::ErrorKind::ArgumentConflict); + } + + #[test] + fn approve_builds_all_conflicts_with_deny_packages() { + let error = parse_pm_command(&["vp", "pm", "approve-builds", "--all", "!core-js"]) + .expect_err("expected --all + !pkg to fail"); + + assert_eq!(error.kind(), clap::error::ErrorKind::ArgumentConflict); + } + + #[test] + fn approve_builds_all_alone_parses() { + let command = parse_pm_command(&["vp", "pm", "approve-builds", "--all"]) + .expect("--all alone should parse"); + + assert!(matches!( + command, + PackageManagerCommand::Pm(PmCommands::ApproveBuilds { all: true, .. }) + )); + } + + #[test] + fn approve_builds_packages_alone_parses() { + let command = parse_pm_command(&["vp", "pm", "approve-builds", "esbuild", "fsevents"]) + .expect("positional packages should parse"); + + assert!(matches!( + command, + PackageManagerCommand::Pm(PmCommands::ApproveBuilds { all: false, .. }) + )); + } + + #[test] + fn approve_builds_pass_through_args_capture() { + let command = + parse_pm_command(&["vp", "pm", "approve-builds", "esbuild", "--", "--workspace-root"]) + .expect("pass-through args should parse"); + + let PackageManagerCommand::Pm(PmCommands::ApproveBuilds { pass_through_args, .. }) = + command + else { + panic!("expected ApproveBuilds variant"); + }; + assert_eq!(pass_through_args, Some(vec!["--workspace-root".to_string()])); + } } diff --git a/crates/vite_pm_cli/src/handlers.rs b/crates/vite_pm_cli/src/handlers.rs index 3ed2eec5a0..a03213519f 100644 --- a/crates/vite_pm_cli/src/handlers.rs +++ b/crates/vite_pm_cli/src/handlers.rs @@ -8,6 +8,7 @@ use vite_install::{ PackageManager, commands::{ add::AddCommandOptions, + approve_builds::ApproveBuildsCommandOptions, audit::AuditCommandOptions, cache::CacheCommandOptions, config::ConfigCommandOptions, @@ -194,6 +195,21 @@ pub async fn run_pm_subcommand( }; match command { + PmCommands::ApproveBuilds { packages, all, pass_through_args } => { + let options = ApproveBuildsCommandOptions { + packages: &packages, + all, + pass_through_args: pass_through_args.as_deref(), + }; + // Map `Error::InvalidArgument` from the resolver to `UserMessage` + // so the version-gate failure renders without the harsh `error:` prefix. + match pm.run_approve_builds_command(&options, cwd).await { + Ok(status) => Ok(status), + Err(vite_error::Error::InvalidArgument(msg)) => Err(Error::UserMessage(msg)), + Err(other) => Err(Error::Install(other)), + } + } + PmCommands::Prune { prod, no_optional, pass_through_args } => { let options = PruneCommandOptions { prod, diff --git a/packages/cli/snap-tests-global/cli-helper-message/snap.txt b/packages/cli/snap-tests-global/cli-helper-message/snap.txt index fbbed52b04..5d38fc9b94 100644 --- a/packages/cli/snap-tests-global/cli-helper-message/snap.txt +++ b/packages/cli/snap-tests-global/cli-helper-message/snap.txt @@ -313,25 +313,26 @@ Usage: vp pm Forward a command to the package manager Commands: - prune Remove unnecessary packages - pack Create a tarball of the package - list List installed packages [aliases: ls] - view View package information from the registry [aliases: info, show] - publish Publish package to registry - owner Manage package owners [aliases: author] - cache Manage package cache - config Manage package manager configuration [aliases: c] - login Log in to a registry [aliases: adduser] - logout Log out from a registry - whoami Show the current logged-in user - token Manage authentication tokens - audit Run a security audit - dist-tag Manage distribution tags - deprecate Deprecate a package version - search Search for packages in the registry - rebuild Rebuild native modules [aliases: rb] - fund Show funding information for installed packages - ping Ping the registry + approve-builds Approve dependency lifecycle scripts (install/postinstall) to run + prune Remove unnecessary packages + pack Create a tarball of the package + list List installed packages [aliases: ls] + view View package information from the registry [aliases: info, show] + publish Publish package to registry + owner Manage package owners [aliases: author] + cache Manage package cache + config Manage package manager configuration [aliases: c] + login Log in to a registry [aliases: adduser] + logout Log out from a registry + whoami Show the current logged-in user + token Manage authentication tokens + audit Run a security audit + dist-tag Manage distribution tags + deprecate Deprecate a package version + search Search for packages in the registry + rebuild Rebuild native modules [aliases: rb] + fund Show funding information for installed packages + ping Ping the registry Options: -h, --help Print help diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/package.json b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/package.json new file mode 100644 index 0000000000..b7a2678d88 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/package.json @@ -0,0 +1,5 @@ +{ + "name": "command-pm-approve-builds-bun", + "version": "1.0.0", + "packageManager": "bun@1.3.11" +} diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt new file mode 100644 index 0000000000..55084799a5 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt @@ -0,0 +1,25 @@ +> vp pm approve-builds --help # should show help with --all caveat +Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] + +Approve dependency lifecycle scripts (install/postinstall) to run + +Arguments: + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager + +Options: + --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + -h, --help Print help + +Documentation: https://viteplus.dev/guide/install + + +> vp pm approve-builds # bun no-args: prints contextual note, exit 0 +note: bun pm trust requires package names. Run `bun pm untrusted` to see which packages are pending, then pass them explicitly: `vp pm approve-builds [...]` or `vp pm approve-builds --all`. + +> vp pm approve-builds '!core-js' # deny-only: prints deny warn, no redundant note +warn: bun does not support denylisting build scripts. Packages outside `trustedDependencies` in package.json are already denied by default. Skipping: core-js + +[1]> vp pm approve-builds --all # forwards bun pm trust --all (nothing to trust in this empty project) +bun pm trust v (af24e281) +error: Lockfile not found diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json new file mode 100644 index 0000000000..cf5e8e08aa --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json @@ -0,0 +1,8 @@ +{ + "commands": [ + "vp pm approve-builds --help # should show help with --all caveat", + "vp pm approve-builds # bun no-args: prints contextual note, exit 0", + "vp pm approve-builds '!core-js' # deny-only: prints deny warn, no redundant note", + "vp pm approve-builds --all # forwards bun pm trust --all (nothing to trust in this empty project)" + ] +} diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/package.json b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/package.json new file mode 100644 index 0000000000..55e477ffd8 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/package.json @@ -0,0 +1,5 @@ +{ + "name": "command-pm-approve-builds-pnpm10-old", + "version": "1.0.0", + "packageManager": "pnpm@10.31.0" +} diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/snap.txt new file mode 100644 index 0000000000..87d87328dd --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/snap.txt @@ -0,0 +1,8 @@ +[1]> vp pm approve-builds --all # pnpm 10.31.0 < 10.32.0 → rejected with friendly UserMessage (no `error:` prefix) +`--all` requires pnpm >= . Upgrade pnpm or pass package names explicitly. + +[1]> vp pm approve-builds esbuild '!core-js' # pnpm 10.31.0 < 11.0.0 → !pkg deny syntax rejected +`!` deny syntax requires pnpm >= . Upgrade pnpm or omit the `!` entries. + +> vp pm approve-builds esbuild # plain positional still works on old pnpm +There are no packages awaiting approval diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/steps.json b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/steps.json new file mode 100644 index 0000000000..15634fb4ed --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm10-old/steps.json @@ -0,0 +1,7 @@ +{ + "commands": [ + "vp pm approve-builds --all # pnpm 10.31.0 < 10.32.0 → rejected with friendly UserMessage (no `error:` prefix)", + "vp pm approve-builds esbuild '!core-js' # pnpm 10.31.0 < 11.0.0 → !pkg deny syntax rejected", + "vp pm approve-builds esbuild # plain positional still works on old pnpm" + ] +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-npm/package.json b/packages/cli/snap-tests/command-pm-approve-builds-npm/package.json new file mode 100644 index 0000000000..3a6c7cfb31 --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-npm/package.json @@ -0,0 +1,6 @@ +{ + "name": "command-pm-approve-builds-npm", + "version": "1.0.0", + "private": true, + "packageManager": "npm@10.9.0" +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt new file mode 100644 index 0000000000..d3bdc9fcce --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt @@ -0,0 +1,21 @@ +> vp pm approve-builds --help # should show help +Approve dependency lifecycle scripts (install/postinstall) to run + +Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] + +Arguments: + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager + +Options: + --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + -h, --help Print help + +> vp pm approve-builds # warn and exit 0 (no-op on npm) +warn: npm runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. + +> vp pm approve-builds esbuild # warn and exit 0 (no-op on npm) +warn: npm runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. + +> vp pm approve-builds --all # warn and exit 0 (no-op on npm) +warn: npm runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. diff --git a/packages/cli/snap-tests/command-pm-approve-builds-npm/steps.json b/packages/cli/snap-tests/command-pm-approve-builds-npm/steps.json new file mode 100644 index 0000000000..489eba5df0 --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-npm/steps.json @@ -0,0 +1,8 @@ +{ + "commands": [ + "vp pm approve-builds --help # should show help", + "vp pm approve-builds # warn and exit 0 (no-op on npm)", + "vp pm approve-builds esbuild # warn and exit 0 (no-op on npm)", + "vp pm approve-builds --all # warn and exit 0 (no-op on npm)" + ] +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/package.json b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/package.json new file mode 100644 index 0000000000..85c8854142 --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/package.json @@ -0,0 +1,6 @@ +{ + "name": "command-pm-approve-builds-pnpm10", + "version": "1.0.0", + "private": true, + "packageManager": "pnpm@10.32.0" +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt new file mode 100644 index 0000000000..5018fba0bd --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt @@ -0,0 +1,18 @@ +> vp pm approve-builds --help # should show help +Approve dependency lifecycle scripts (install/postinstall) to run + +Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] + +Arguments: + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager + +Options: + --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + -h, --help Print help + +> vp pm approve-builds --all # forwards to pnpm approve-builds --all (nothing to approve) +There are no packages awaiting approval + +> vp pm approve-builds esbuild fsevents # forwards positional packages to pnpm +There are no packages awaiting approval diff --git a/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/steps.json b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/steps.json new file mode 100644 index 0000000000..5f3d32b730 --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/steps.json @@ -0,0 +1,7 @@ +{ + "commands": [ + "vp pm approve-builds --help # should show help", + "vp pm approve-builds --all # forwards to pnpm approve-builds --all (nothing to approve)", + "vp pm approve-builds esbuild fsevents # forwards positional packages to pnpm" + ] +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-yarn/package.json b/packages/cli/snap-tests/command-pm-approve-builds-yarn/package.json new file mode 100644 index 0000000000..2901b7d998 --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-yarn/package.json @@ -0,0 +1,6 @@ +{ + "name": "command-pm-approve-builds-yarn", + "version": "1.0.0", + "private": true, + "packageManager": "yarn@1.22.22" +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt new file mode 100644 index 0000000000..07b15bb17c --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt @@ -0,0 +1,21 @@ +> vp pm approve-builds --help # should show help +Approve dependency lifecycle scripts (install/postinstall) to run + +Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] + +Arguments: + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager + +Options: + --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + -h, --help Print help + +> vp pm approve-builds # warn and exit 0 (no-op on yarn) +warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts true` in .npmrc and rebuild approved packages with `vp pm rebuild `. + +> vp pm approve-builds esbuild # warn and exit 0 (no-op on yarn) +warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts true` in .npmrc and rebuild approved packages with `vp pm rebuild `. + +> vp pm approve-builds --all # warn and exit 0 (no-op on yarn) +warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts true` in .npmrc and rebuild approved packages with `vp pm rebuild `. diff --git a/packages/cli/snap-tests/command-pm-approve-builds-yarn/steps.json b/packages/cli/snap-tests/command-pm-approve-builds-yarn/steps.json new file mode 100644 index 0000000000..c247eb3429 --- /dev/null +++ b/packages/cli/snap-tests/command-pm-approve-builds-yarn/steps.json @@ -0,0 +1,9 @@ +{ + "ignoredPlatforms": ["linux", "win32"], + "commands": [ + "vp pm approve-builds --help # should show help", + "vp pm approve-builds # warn and exit 0 (no-op on yarn)", + "vp pm approve-builds esbuild # warn and exit 0 (no-op on yarn)", + "vp pm approve-builds --all # warn and exit 0 (no-op on yarn)" + ] +} diff --git a/rfcs/approve-builds-command.md b/rfcs/approve-builds-command.md new file mode 100644 index 0000000000..42ba643632 --- /dev/null +++ b/rfcs/approve-builds-command.md @@ -0,0 +1,836 @@ +# RFC: Vite+ Approve Builds Command (`vp pm approve-builds`) + +## Summary + +Add `vp pm approve-builds` subcommand under the `vp pm` command group to provide a unified, cross-package-manager interface for approving (or denying) dependency lifecycle scripts (`preinstall` / `install` / `postinstall`). The design mirrors [`pnpm approve-builds`](https://pnpm.io/cli/approve-builds) one-to-one and is adapted to [`bun pm trust`](https://bun.com/docs/pm/cli/pm#trust), with a warning-and-no-op fallback for npm and yarn (which do not have an equivalent first-class command). + +This is a sub-RFC of [pm-command-group.md](./pm-command-group.md) and extends the list of `vp pm` subcommands. + +## Motivation + +Modern package managers ship with **opt-in lifecycle script execution** to mitigate supply-chain attacks: + +- **pnpm ≥ v10** ignores all install scripts by default and requires explicit approval via the `allowBuilds` map in `pnpm-workspace.yaml`. +- **bun** blocks lifecycle scripts for any dependency not listed in `trustedDependencies` (in `package.json`) or shipped in bun's default-trusted list. +- **npm** still executes lifecycle scripts by default but can be flipped off with `ignore-scripts=true` in `.npmrc`. +- **yarn (Berry)** blocks third-party build scripts by default (`enableScripts` is `false`); per-package opt-in is via `dependenciesMeta..built: true` in `package.json`. + +This produces two parallel workflows for the **same conceptual task** — "I trust `esbuild` to run its post-install build": + +```bash +# pnpm +pnpm approve-builds # interactive +pnpm approve-builds esbuild fsevents # by name +pnpm approve-builds esbuild !core-js # approve esbuild, deny core-js +pnpm approve-builds --all # approve all pending + +# bun +bun pm trust esbuild # trust one package +bun pm trust --all # trust everything pending + +# npm / yarn +# (no equivalent command; users must edit package.json or config manually) +``` + +### Pain Points + +1. **Conceptual divergence**: pnpm stores approvals in `pnpm-workspace.yaml` (`allowBuilds:`), bun stores them in `package.json` (`trustedDependencies:`). The two are semantically similar but live in different files with different shapes. +2. **Asymmetric models**: pnpm supports both _allow_ and _deny_ (via `!pkg`), bun only supports _trust_ (deny is the default). +3. **CI portability**: A monorepo migrating between pnpm and bun today must rewrite all build-approval automation. + +### Proposed Solution + +A single Vite+ subcommand routes to the underlying package manager's idiomatic command: + +```bash +# Works for all package managers +vp pm approve-builds # interactive prompt (pnpm) / warn (bun, npm, yarn) +vp pm approve-builds esbuild fsevents # approve listed packages +vp pm approve-builds esbuild !core-js # approve esbuild, deny core-js (pnpm only) +vp pm approve-builds --all # approve every pending package +``` + +## Proposed Solution + +### Command Syntax + +```bash +vp pm approve-builds [PACKAGES...] [OPTIONS] +``` + +**Positional arguments:** + +- `PACKAGES...`: One or more package names to approve. + - Prefix with `!` to deny (`!core-js`) — pnpm only; for bun this prints a warning explaining the model doesn't support denylisting and skips the denied entries. + - Omitting all positionals (and `--all`) launches **interactive mode** on pnpm; bun has no interactive picker so we print a `note` asking the user to pass package names explicitly. + +**Options:** + +- `--all`: Approve every package that is currently pending approval. Maps to `pnpm approve-builds --all` (added in pnpm v10.32.0) and `bun pm trust --all`. + +(Intentionally matches pnpm's documented surface. `pnpm approve-builds --global` was removed in pnpm v11.0.0, so we do not expose `-g/--global`. Other ergonomics — listing pending packages, showing default-trusted lists, CI confirmation gates — are deferred to follow-up RFCs; see [Future Enhancements](#future-enhancements).) + +### Subcommand Behavior + +#### 1. Interactive mode + +```bash +$ vp pm approve-builds +Detected package manager: pnpm@10.32.0 +Running: pnpm approve-builds + +? Choose which packages to build (Press to select, to toggle all, to invert selection) + ◯ @biomejs/biome + ◯ esbuild + ◯ fsevents + ◯ sharp +``` + +- **pnpm**: forwards interactively to `pnpm approve-builds` (it owns the TUI). +- **bun**: bun has no interactive picker for `bun pm trust`. Vite+ prints: + + ``` + note bun pm trust requires package names. Run `bun pm untrusted` to see + which packages are pending, then pass them explicitly: + vp pm approve-builds [...] + vp pm approve-builds --all + ``` + + Exit 0. + +- **npm**: prints a warning and exits 0 (no-op): + + ``` + warn npm runs lifecycle scripts by default. To restrict them, set + `ignore-scripts=true` in .npmrc and rebuild approved packages with + `vp pm rebuild `. + ``` + +- **yarn**: prints a warning and exits 0 (no-op): + + ``` + warn yarn does not run third-party build scripts by default. To allow a + package, set `dependenciesMeta[""].built: true` in package.json. + ``` + +#### 2. Direct approval + +```bash +$ vp pm approve-builds esbuild fsevents +Detected package manager: pnpm@10.32.0 +Running: pnpm approve-builds esbuild fsevents +✔ esbuild approved +✔ fsevents approved +``` + +- **pnpm**: pass-through; pnpm updates `allowBuilds` in `pnpm-workspace.yaml`. +- **bun**: invokes `bun pm trust esbuild fsevents`; bun appends `trustedDependencies` in `package.json`. +- **npm / yarn**: prints the warning shown above and exits 0. + +#### 3. Deny syntax (`!pkg`) + +```bash +$ vp pm approve-builds esbuild !core-js +Detected package manager: pnpm@10.32.0 +Running: pnpm approve-builds esbuild !core-js +✔ esbuild approved +✗ core-js denied +``` + +- **pnpm**: pass-through (native syntax). +- **bun**: Vite+ prints: + + ``` + warn bun does not support denylisting build scripts. Packages outside + `trustedDependencies` in package.json are already denied by default. + Skipping: core-js + ``` + + Then forwards the non-denied positional packages (`esbuild`) to `bun pm trust`. + +- **npm / yarn**: warning as above; no-op. + +#### 4. `--all` + +```bash +$ vp pm approve-builds --all +Detected package manager: bun@1.3.0 +Running: bun pm trust --all +✔ Trusted 4 packages +``` + +- **pnpm** ≥ v10.32.0: forwards to `pnpm approve-builds --all`. +- **pnpm** < v10.32.0: errors with a usage hint asking the user to upgrade pnpm or enumerate the packages explicitly. +- **bun**: forwards to `bun pm trust --all`. +- **npm / yarn**: warning as above; no-op. + +### Command Mapping + +**pnpm references:** + +- https://pnpm.io/cli/approve-builds +- https://pnpm.io/settings#allowbuilds + +**bun references:** + +- https://bun.com/docs/pm/cli/pm#trust + +**npm references:** + +- No equivalent command. Closest configuration: [`ignore-scripts`](https://docs.npmjs.com/cli/v11/using-npm/config#ignore-scripts) and [`npm rebuild`](https://docs.npmjs.com/cli/v11/commands/npm-rebuild). + +**yarn references:** + +- No equivalent command. yarn@2+ already blocks third-party build scripts by default ([`enableScripts`](https://yarnpkg.com/configuration/yarnrc#enableScripts) defaults to `false`); per-package opt-in is via [`dependenciesMeta..built`](https://yarnpkg.com/configuration/manifest#dependenciesMeta) in `package.json`. + +| Vite+ Flag | pnpm | npm | yarn@1 | yarn@2+ | bun | Description | +| ----------------------------- | ---------------------------------------- | ---------- | ---------- | ---------- | --------------------------- | ------------------------------- | +| `vp pm approve-builds` | `pnpm approve-builds` | N/A (warn) | N/A (warn) | N/A (warn) | N/A (note) | Interactive prompt (pnpm only) | +| `vp pm approve-builds ` | `pnpm approve-builds ` | N/A (warn) | N/A (warn) | N/A (warn) | `bun pm trust ` | Approve named packages | +| `vp pm approve-builds !` | `pnpm approve-builds !` | N/A (warn) | N/A (warn) | N/A (warn) | N/A (warn — model mismatch) | Deny named packages (pnpm only) | +| `--all` | `pnpm approve-builds --all` (≥ v10.32.0) | N/A (warn) | N/A (warn) | N/A (warn) | `bun pm trust --all` | Approve every pending package | + +**Notes:** + +- **`!pkg` deny syntax is pnpm-only.** For bun the deny syntax is rejected with a warning that names the affected positionals (so users notice rather than silently get a partial approval). +- **npm and yarn never have an `approve-builds` command.** Vite+ prints a one-line `warn` and exits 0. For npm (which runs scripts by default) the warn points at `ignore-scripts`. For yarn (which blocks third-party scripts by default) the warn points at `dependenciesMeta..built`. We intentionally exit 0 (not non-zero) so monorepo scripts that run `vp pm approve-builds` opportunistically don't break on heterogeneous environments. +- **No-args mode on bun** also exits 0 with a `note` (bun's `bun pm trust` requires package names; there's no interactive picker to forward to). +- **Configuration storage differs:** pnpm writes to `pnpm-workspace.yaml` under `allowBuilds:`. bun writes to `package.json` under `trustedDependencies: []`. Vite+ does not normalize the storage location — each PM owns its own state. (See [Design Decision §2](#2-do-not-normalize-storage).) + +### Implementation Architecture + +#### 1. Command Structure + +**File**: `crates/vite_task/src/lib.rs` + +Add a new variant under `PmCommands`: + +```rust +#[derive(Subcommand, Debug)] +pub enum PmCommands { + // ... existing subcommands + + /// Approve dependency lifecycle scripts (install/postinstall) to run + ApproveBuilds { + /// Packages to approve. Prefix with `!` to deny (pnpm only). + /// Omit to launch interactive mode (pnpm only). + packages: Vec, + + /// Approve every package that is currently pending + #[arg(long)] + all: bool, + }, +} +``` + +#### 2. Package Manager Adapter + +**File**: `crates/vite_package_manager/src/commands/approve_builds.rs` (new file) + +```rust +use std::process::ExitStatus; + +use vite_error::Error; +use vite_path::AbsolutePath; +use vite_shared::output::{note, warn}; + +use crate::package_manager::{PackageManager, PackageManagerType}; + +pub struct ApproveBuildsOptions<'a> { + pub packages: &'a [String], + pub all: bool, +} + +impl PackageManager { + /// Approve dependency lifecycle scripts. + pub async fn run_approve_builds( + &self, + opts: ApproveBuildsOptions<'_>, + cwd: impl AsRef, + ) -> Result { + match self.client { + PackageManagerType::Pnpm => self.pnpm_approve_builds(opts, cwd).await, + PackageManagerType::Bun => self.bun_approve_builds(opts, cwd).await, + PackageManagerType::Npm => { + warn( + "npm runs lifecycle scripts by default. To restrict them, set \ + `ignore-scripts=true` in .npmrc and rebuild approved packages with \ + `vp pm rebuild `.", + ); + Ok(ExitStatus::default()) // exit 0 — no-op + } + PackageManagerType::Yarn => { + note( + "yarn does not run third-party build scripts by default. To allow a \ + package, set `dependenciesMeta[\"\"].built: true` in package.json.", + ); + Ok(ExitStatus::default()) // exit 0 — no-op + } + } + } + + async fn pnpm_approve_builds( + &self, + opts: ApproveBuildsOptions<'_>, + cwd: impl AsRef, + ) -> Result { + if opts.all && self.version_lt("10.32.0") { + return Err(Error::Usage( + "`--all` requires pnpm ≥ 10.32.0. Upgrade pnpm or pass package names explicitly." + .into(), + )); + } + + let mut args = vec!["approve-builds".to_string()]; + if opts.all { + args.push("--all".into()); + } + args.extend(opts.packages.iter().cloned()); + self.run_raw("pnpm", &args, cwd).await + } + + async fn bun_approve_builds( + &self, + opts: ApproveBuildsOptions<'_>, + cwd: impl AsRef, + ) -> Result { + // Reject `!pkg` deny syntax with a clear warning. + let (denies, approves): (Vec<&String>, Vec<&String>) = + opts.packages.iter().partition(|p| p.starts_with('!')); + if !denies.is_empty() { + let names: Vec = + denies.iter().map(|p| p.trim_start_matches('!').to_string()).collect(); + warn(&format!( + "bun does not support denylisting build scripts. Packages outside\n \ + `trustedDependencies` in package.json are already denied by default.\n \ + Skipping: {}", + names.join(", ") + )); + } + + // No-args mode: bun has no interactive picker. + if approves.is_empty() && !opts.all { + note( + "bun pm trust requires package names. Run `bun pm untrusted` to see\n \ + which packages are pending, then pass them explicitly:\n \ + vp pm approve-builds [...]\n \ + vp pm approve-builds --all", + ); + return Ok(ExitStatus::default()); + } + + let mut args = vec!["pm".to_string(), "trust".into()]; + if opts.all { + args.push("--all".into()); + } + args.extend(approves.iter().map(|s| s.to_string())); + self.run_raw("bun", &args, cwd).await + } +} +``` + +**File**: `crates/vite_package_manager/src/commands/mod.rs` + +```rust +pub mod add; +mod install; +pub mod remove; +pub mod update; +pub mod link; +pub mod unlink; +pub mod dedupe; +pub mod why; +pub mod outdated; +pub mod approve_builds; // <- add this +// pub mod pm; // (future; from pm-command-group RFC) +``` + +#### 3. CLI Implementation + +**File**: `crates/vite_task/src/approve_builds.rs` (new file) + +```rust +use vite_error::Error; +use vite_package_manager::{ + PackageManager, + commands::approve_builds::ApproveBuildsOptions, +}; +use vite_path::AbsolutePathBuf; +use vite_workspace::Workspace; + +pub struct ApproveBuildsCommand { + workspace_root: AbsolutePathBuf, +} + +impl ApproveBuildsCommand { + pub fn new(workspace_root: AbsolutePathBuf) -> Self { + Self { workspace_root } + } + + pub async fn execute(self, packages: Vec, all: bool) -> Result<(), Error> { + let package_manager = PackageManager::builder(&self.workspace_root).build().await?; + let workspace = Workspace::partial_load(self.workspace_root.clone())?; + + let status = package_manager + .run_approve_builds( + ApproveBuildsOptions { packages: &packages, all }, + &workspace.root, + ) + .await?; + + if !status.success() { + return Err(Error::CommandFailed { + command: "pm approve-builds".into(), + exit_code: status.code(), + }); + } + workspace.unload().await?; + Ok(()) + } +} +``` + +## Design Decisions + +### 1. Mirror pnpm's documented surface exactly + +**Decision**: The Vite+ command exposes only what `pnpm approve-builds` documents — positional packages (with `!pkg` deny prefix) and `--all`. Bun's additional commands (`bun pm untrusted`, `bun pm default-trusted`) are **not** folded in as flags. + +**Rationale**: pnpm and bun share only the "approve these packages" operation. Adding `--list`, `--default-trusted`, `-y`, or `-g` would either invent flags that don't exist in pnpm's documented surface or paper over bun's separate-command model. If `vp pm untrusted` and `vp pm default-trusted` are wanted later, they should be their own sibling subcommands under `pm` (mirroring bun) — that's a follow-up RFC, not creeping scope here. + +### 2. Do not normalize storage + +**Decision**: Vite+ does **not** rewrite `pnpm-workspace.yaml ↔ package.json#trustedDependencies` into a single shared file. + +**Rationale**: + +- The two formats encode different semantics (`allowBuilds: { core-js: false }` has no bun analog). +- Round-tripping between them on every command would mutate files the user doesn't expect. +- Migrations between package managers are rare; on-demand conversion (e.g., a future `vp migrate` step) is the right place for that translation, not the day-to-day `approve-builds` command. +- The pnpm-only `!pkg` deny syntax stays meaningful and isn't silently lost. + +### 3. `!pkg` deny syntax: pnpm-only, surface a warning + +**Decision**: Accept `!pkg` in positional args; for bun, print a `warn` naming the affected packages and continue with the approved ones. + +**Rationale**: + +- Silently dropping `!core-js` would leave users believing they had denied a package when they hadn't. +- Erroring out would break `vp pm approve-builds esbuild !core-js` for a developer who copied the command from a pnpm tutorial but happens to be on bun for one repo. +- The warning names the dropped packages so the divergence is auditable. + +### 4. npm / yarn: warn + exit 0 + +**Decision**: Print a `warn` and return exit code 0 on npm and yarn. + +**Rationale**: + +- **npm** runs lifecycle scripts by default — the warn points at how to _restrict_ them (`ignore-scripts=true`). +- **yarn (Berry)** blocks third-party build scripts by default; the per-package opt-in lives in `package.json` (`dependenciesMeta..built: true`). We `warn` pointing at that field rather than performing the edit ourselves — staying within the RFC's intentionally-tight scope. +- Both surfaces use `warn` (not `note`) for consistency: the user invoked `approve-builds` and the requested action could not be completed on this PM, so they need a visible signal and a manual workaround. +- Exit 0 lets CI scripts that conditionally run `vp pm approve-builds --all` work across heterogeneous repos. +- Exit non-zero (the alternative) would break monorepo orchestration scripts and demand per-PM conditionals. + +### 5. No-args on bun: note + exit 0 + +**Decision**: When `vp pm approve-builds` is invoked with no args (and no `--all`) on bun, print a `note` and exit 0 rather than building a Vite+-owned interactive picker. + +**Rationale**: + +- Implementing a picker requires parsing `bun pm untrusted` output and reusing the prompts module — meaningful work that should land in its own RFC if/when wanted. +- The current behavior keeps this RFC's surface area minimal and faithful to pnpm's documented flag set. + +### 6. No caching + +**Decision**: Do not cache approve-builds results. + +**Rationale**: This command mutates configuration files; caching would be incorrect. + +## Error Handling + +### No package manager detected + +``` +$ vp pm approve-builds +error No package manager detected. + Please run one of: + - vp install (to set up package manager) + - Add a `packageManager` field to package.json +``` + +### `--all` on pnpm < v10.32.0 + +``` +$ vp pm approve-builds --all +Detected package manager: pnpm@10.20.0 +error `--all` requires pnpm ≥ 10.32.0. Upgrade pnpm or pass package names explicitly. +``` + +### Deny syntax against bun + +``` +$ vp pm approve-builds esbuild !core-js +Detected package manager: bun@1.3.0 +warn bun does not support denylisting build scripts. Packages outside + `trustedDependencies` in package.json are already denied by default. + Skipping: core-js +Running: bun pm trust esbuild +✔ Trusted 1 package +``` + +### Underlying command failed + +``` +$ vp pm approve-builds esbuild +Detected package manager: pnpm@10.32.0 +Running: pnpm approve-builds esbuild +ERR_PNPM_CONFIG_WRITE_FAILED: cannot write pnpm-workspace.yaml +exit code: 1 +``` + +Exit code is propagated. + +## User Experience + +### Interactive approval (pnpm) + +``` +$ vp pm approve-builds +Detected package manager: pnpm@10.32.0 +Running: pnpm approve-builds + +? Choose which packages to build (Press to select, to toggle all, to invert selection) +❯◯ @biomejs/biome + ◯ esbuild + ◯ fsevents + ◯ sharp + +✔ Updated pnpm-workspace.yaml (allowBuilds) +``` + +### Direct approval (bun) + +``` +$ vp pm approve-builds esbuild fsevents +Detected package manager: bun@1.3.0 +Running: bun pm trust esbuild fsevents +✔ Updated package.json (trustedDependencies) +``` + +### Bulk approval + +``` +$ vp pm approve-builds --all +Detected package manager: bun@1.3.0 +Running: bun pm trust --all +✔ Trusted 4 packages +``` + +### No-args on bun + +``` +$ vp pm approve-builds +Detected package manager: bun@1.3.0 +note bun pm trust requires package names. Run `bun pm untrusted` to see + which packages are pending, then pass them explicitly: + vp pm approve-builds [...] + vp pm approve-builds --all +``` + +### No-op on npm + +``` +$ vp pm approve-builds +Detected package manager: npm@11.0.0 +warn npm runs lifecycle scripts by default. To restrict them, set + `ignore-scripts=true` in .npmrc and rebuild approved packages with + `vp pm rebuild `. +``` + +### No-op on yarn + +``` +$ vp pm approve-builds esbuild +Detected package manager: yarn@4.0.0 +warn yarn does not run third-party build scripts by default. To allow a + package, set `dependenciesMeta[""].built: true` in package.json. +``` + +## Alternative Designs Considered + +### Alternative 1: Separate `vp pm trust` / `vp pm untrusted` / `vp pm allow-build` + +```bash +vp pm trust esbuild +vp pm untrusted +vp pm allow-build esbuild +``` + +**Rejected because:** + +- Mirrors PM-specific vocabulary instead of unifying it. +- Triples the surface area users have to learn. +- The single-command shape matches existing Vite+ conventions. + +### Alternative 2: Normalize all approvals into a Vite+-owned file (e.g., `vite-plus.json`) + +```json +{ "approvedBuilds": ["esbuild", "fsevents"] } +``` + +**Rejected because:** + +- Forces Vite+ to re-implement script execution gating (today owned by pnpm/bun). +- Creates two sources of truth (`vite-plus.json` and the PM's own file) — drift inevitable. +- Loses pnpm's allow/deny distinction and version-specific entries (`nx@21.6.4 || 21.6.5: true`). + +### Alternative 3: Always shell out raw + +```bash +vp pm approve-builds -- pnpm approve-builds --all +``` + +**Rejected because:** + +- Defeats the purpose of a unified command. +- Forces the user to know which PM is active. +- No bun parity (bun has no `approve-builds` to delegate to). + +### Alternative 4: Auto-approve everything on install + +**Rejected because:** + +- Defeats the supply-chain protection that pnpm/bun designed the gating to provide. +- Would be a security regression compared to running pnpm/bun directly. + +### Alternative 5: Bundle bun's untrusted/default-trusted as flags + +```bash +vp pm approve-builds --list +vp pm approve-builds --default-trusted +``` + +**Rejected because:** + +- These flags don't exist in pnpm's documented surface; adding them invents a unified vocabulary that no PM actually uses. +- bun models them as separate commands (`bun pm untrusted`, `bun pm default-trusted`); the cleaner Vite+ mirror is also separate subcommands. +- Out of scope for this RFC; see [Future Enhancements](#future-enhancements). + +## Implementation Plan + +### Phase 1: Core plumbing + +1. Add `ApproveBuilds` variant to `PmCommands` in `crates/vite_task/src/lib.rs`. +2. Create `crates/vite_package_manager/src/commands/approve_builds.rs` with the pnpm + bun adapters. +3. Wire pass-through for pnpm (`approve-builds`, `approve-builds `, `approve-builds !`, `--all`). +4. Wire `bun pm trust` (positionals + `--all`), with `!pkg` filter + warning. +5. Wire the npm/yarn warning path (exit 0). +6. Wire the bun no-args `note` path. + +### Phase 2: Version-gating for pnpm `--all` + +1. Detect pnpm version and reject `--all` against < v10.32.0 with the usage hint. + +### Phase 3: Tests + snap tests + +1. Unit tests for command resolution (per PM × flag matrix). +2. Snap tests covering each PM in `packages/cli/snap-tests/`. + +### Phase 4: Docs + +1. Update `vp pm --help` to list the new subcommand. +2. Add a row to the [pm-command-group RFC](./pm-command-group.md) compatibility matrix. +3. Add `vp pm approve-builds` to user-facing CLI docs. + +## Testing Strategy + +### Unit tests + +```rust +#[test] +fn pnpm_basic_approve() { + let pm = PackageManager::mock(PackageManagerType::Pnpm).with_version("10.32.0"); + let opts = ApproveBuildsOptions { packages: &vec!["esbuild".into()], all: false }; + let cmd = pm.resolve_approve_builds(&opts); + assert_eq!(cmd.bin, "pnpm"); + assert_eq!(cmd.args, vec!["approve-builds", "esbuild"]); +} + +#[test] +fn pnpm_all_flag() { + let pm = PackageManager::mock(PackageManagerType::Pnpm).with_version("10.32.0"); + let opts = ApproveBuildsOptions { packages: &vec![], all: true }; + let cmd = pm.resolve_approve_builds(&opts); + assert_eq!(cmd.args, vec!["approve-builds", "--all"]); +} + +#[test] +fn pnpm_all_rejected_below_v10_32() { + let pm = PackageManager::mock(PackageManagerType::Pnpm).with_version("10.20.0"); + let opts = ApproveBuildsOptions { packages: &vec![], all: true }; + assert!(pm.resolve_approve_builds(&opts).is_err()); +} + +#[test] +fn pnpm_passes_deny_syntax_through() { + let pm = PackageManager::mock(PackageManagerType::Pnpm).with_version("10.32.0"); + let opts = ApproveBuildsOptions { + packages: &vec!["esbuild".into(), "!core-js".into()], + all: false, + }; + let cmd = pm.resolve_approve_builds(&opts); + assert_eq!(cmd.args, vec!["approve-builds", "esbuild", "!core-js"]); +} + +#[test] +fn bun_deny_syntax_filtered_with_warning() { + let pm = PackageManager::mock(PackageManagerType::Bun); + let opts = ApproveBuildsOptions { + packages: &vec!["esbuild".into(), "!core-js".into()], + all: false, + }; + let cmd = pm.resolve_approve_builds(&opts); + assert_eq!(cmd.args, vec!["pm", "trust", "esbuild"]); + assert!(cmd.warnings.iter().any(|w| w.contains("core-js"))); +} + +#[test] +fn bun_all_flag_passes_through() { + let pm = PackageManager::mock(PackageManagerType::Bun); + let opts = ApproveBuildsOptions { packages: &vec![], all: true }; + let cmd = pm.resolve_approve_builds(&opts); + assert_eq!(cmd.args, vec!["pm", "trust", "--all"]); +} + +#[test] +fn bun_no_args_emits_note() { + let pm = PackageManager::mock(PackageManagerType::Bun); + let opts = ApproveBuildsOptions { packages: &vec![], all: false }; + let result = pm.resolve_approve_builds(&opts); + assert!(result.no_op); + assert!(result.notes.iter().any(|n| n.contains("bun pm untrusted"))); +} + +#[test] +fn npm_warns_and_exits_zero() { + let pm = PackageManager::mock(PackageManagerType::Npm); + let result = pm.resolve_approve_builds(&Default::default()); + assert!(result.no_op); + assert!(result.warnings.iter().any(|w| w.contains("ignore-scripts=true"))); +} +``` + +### Snap tests + +Add fixtures under `packages/cli/snap-tests/pm-approve-builds-{pnpm,bun,npm,yarn}` covering: + +- No-op invocation (warning for npm/yarn, note for bun). +- `--all` against bun. +- `--all` against pnpm. +- `esbuild !core-js` against bun (asserts the deny warning text). +- `esbuild !core-js` against pnpm (asserts pass-through). + +## CLI Help Output + +``` +$ vp pm approve-builds --help +Approve dependency lifecycle scripts (install/postinstall) to run + +Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... + +Arguments: + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm only). + Omit all positionals to launch interactive mode (pnpm only). + +Options: + --all Approve every package currently pending approval + -h, --help Print help + +Examples: + vp pm approve-builds # interactive prompt (pnpm) + vp pm approve-builds esbuild fsevents # approve specific packages + vp pm approve-builds esbuild !core-js # approve esbuild, deny core-js (pnpm only) + vp pm approve-builds --all # approve every pending package +``` + +## Package Manager Compatibility + +| Capability | pnpm | npm | yarn@1 | yarn@2+ | bun | +| --------------------- | -------------------------------------- | ------- | ----------------------------------------------- | ----------------------------------------------- | --------------------------------------- | +| Interactive (no args) | ✅ native | ❌ warn | ❌ warn | ❌ warn | ❌ note (no picker) | +| Approve by name | ✅ `pnpm approve-builds ` | ❌ warn | ❌ warn | ❌ warn | ✅ `bun pm trust ` | +| Deny by name (`!pkg`) | ✅ `pnpm approve-builds !` | ❌ warn | ❌ warn | ❌ warn | ⚠️ warn (model mismatch) | +| `--all` | ✅ ≥ v10.32.0 (error on older) | ❌ warn | ❌ warn | ❌ warn | ✅ `bun pm trust --all` | +| Storage location | `pnpm-workspace.yaml` → `allowBuilds:` | n/a | `package.json` → `dependenciesMeta..built` | `package.json` → `dependenciesMeta..built` | `package.json` → `trustedDependencies:` | + +## Future Enhancements + +### 1. `vp pm untrusted` + +Mirror `bun pm untrusted` as a sibling subcommand. For pnpm, derive the pending list from `pnpm install --lockfile-only --reporter=ndjson` (filtering `ignored-scripts` events). For npm/yarn, warn-and-exit-0. + +### 2. `vp pm default-trusted` + +Mirror `bun pm default-trusted` as a sibling subcommand. For pnpm/npm/yarn, print a `note` explaining no such list exists. + +### 3. Cross-PM migration helper + +`vp migrate approve-builds` could read `allowBuilds:` from `pnpm-workspace.yaml` and emit a `trustedDependencies:` list for `package.json` (or vice versa). + +### 4. CI confirmation gate / `--yes` + +If user feedback shows `--all` is being run unintentionally in scripts, revisit a confirmation prompt + `-y` opt-out. Not in scope today. + +### 5. Audit integration + +`vp pm audit` (already in the [pm command group RFC](./pm-command-group.md)) could surface "this package is currently approved to run install scripts" alongside its CVE list. + +## Security Considerations + +1. **No silent denylist drops**: bun users who type `!core-js` see a warning instead of having it silently ignored. +2. **No storage normalization**: Vite+ does not introduce a new file format that could become a parallel source of truth for which scripts may run. +3. **Pass-through preserves PM-native auditing**: pnpm and bun continue to own the actual gating; Vite+ is a thin orchestration layer. + +## Backward Compatibility + +This is a new subcommand under the (also new) `vp pm` command group. No breaking changes. + +- Independent of [pm-command-group.md](./pm-command-group.md) — can ship before, after, or alongside it. +- No changes to existing commands. +- No changes to cache or task graph behavior. + +## Real-World Usage Examples + +### Approving after `vp install` + +```bash +vp install +# pnpm reports 4 packages with ignored build scripts +vp pm approve-builds esbuild fsevents sharp +vp install # re-run to actually execute the approved scripts +``` + +### Bulk approval in CI + +```yaml +- run: vp install +- run: vp pm approve-builds --all +``` + +### Mixed approve/deny on pnpm + +```bash +vp pm approve-builds esbuild fsevents !core-js !some-tracker +``` + +## Conclusion + +This RFC adds `vp pm approve-builds` as a focused mirror of `pnpm approve-builds`, adapted to `bun pm trust`. The surface area is intentionally tight: + +- ✅ Positional `[PACKAGES...]` with pnpm's `!pkg` deny prefix +- ✅ `--all` flag (matches pnpm v10.32.0+ and bun) +- ✅ pnpm interactive mode passes through; bun no-args mode emits a `note` +- ✅ `!pkg` deny syntax is preserved for pnpm; warned (not silently dropped) for bun +- ✅ npm and yarn both warn (pointing at `ignore-scripts` and `dependenciesMeta..built` respectively) and exit 0, keeping CI scripts portable +- ✅ No new storage format — each PM keeps owning its own configuration +- ✅ Auxiliary bun commands (`untrusted`, `default-trusted`) deferred to follow-up sibling subcommands rather than folded in as flags From 9da36c7fc1e4be6ccdc1d703d6716e2b9c2d7a82 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 24 May 2026 20:20:08 +0800 Subject: [PATCH 2/6] fix(pm): rename `unparseable` to `unparsable` (typos CI) `crate-ci/typos` flagged "Unparseable"/"unparseable" as misspellings. The accepted spelling is "Unparsable"/"unparsable". Renames the comment, the test name, and the assertion message. --- crates/vite_install/src/commands/approve_builds.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs index 96c7d7d6a6..8feed071a1 100644 --- a/crates/vite_install/src/commands/approve_builds.rs +++ b/crates/vite_install/src/commands/approve_builds.rs @@ -147,7 +147,7 @@ fn pnpm_supports_approve_builds_all(version: &str) -> bool { // Compare against the lowest prerelease of 10.32.0 (`10.32.0-0`) so that // prereleases of 10.32.0 (e.g. `10.32.0-rc.0`, `10.32.0-beta.1`) also // satisfy the gate per semver ordering rules. - // Unparseable versions are treated as not-supported (strict), since the + // Unparsable versions are treated as not-supported (strict), since the // production path always populates `version` from a validated semver. let floor = Version::parse("10.32.0-0").expect("static semver"); Version::parse(version).is_ok_and(|v| v >= floor) @@ -335,15 +335,15 @@ mod tests { } #[test] - fn pnpm_unparseable_version_rejects_all() { - // Strict gate: unparseable versions (a corruption/edge case) fail the check. + fn pnpm_unparsable_version_rejects_all() { + // Strict gate: unparsable versions (a corruption/edge case) fail the check. let pm = create_mock_package_manager(PackageManagerType::Pnpm, "latest"); let err = pm .resolve_approve_builds_command(&ApproveBuildsCommandOptions { all: true, ..Default::default() }) - .expect_err("unparseable version should fail the gate"); + .expect_err("unparsable version should fail the gate"); assert!(matches!(err, Error::InvalidArgument(_))); } From 7fc5b2e9ff70b31cc9a68ac25935c2f0c44c4133 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 24 May 2026 20:43:23 +0800 Subject: [PATCH 3/6] fix(pm): use `ignore-scripts=true` in yarn v1 hint Addresses PR review: the yarn v1 warning prescribed `ignore-scripts true` (space) but npm's `.npmrc` syntax requires `key=value`. Matches the npm branch's existing wording. --- crates/vite_install/src/commands/approve_builds.rs | 2 +- .../cli/snap-tests/command-pm-approve-builds-yarn/snap.txt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs index 8feed071a1..3004a3687a 100644 --- a/crates/vite_install/src/commands/approve_builds.rs +++ b/crates/vite_install/src/commands/approve_builds.rs @@ -119,7 +119,7 @@ impl PackageManager { if self.version.starts_with("1.") { output::warn( "yarn (v1) runs lifecycle scripts by default. To restrict them, set \ - `ignore-scripts true` in .npmrc and rebuild approved packages with \ + `ignore-scripts=true` in .npmrc and rebuild approved packages with \ `vp pm rebuild `.", ); } else { diff --git a/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt index 07b15bb17c..b3f189fbcd 100644 --- a/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt +++ b/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt @@ -12,10 +12,10 @@ Options: -h, --help Print help > vp pm approve-builds # warn and exit 0 (no-op on yarn) -warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts true` in .npmrc and rebuild approved packages with `vp pm rebuild `. +warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. > vp pm approve-builds esbuild # warn and exit 0 (no-op on yarn) -warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts true` in .npmrc and rebuild approved packages with `vp pm rebuild `. +warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. > vp pm approve-builds --all # warn and exit 0 (no-op on yarn) -warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts true` in .npmrc and rebuild approved packages with `vp pm rebuild `. +warn: yarn (v1) runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. From d5fb0c8ac729f43816e0fc8250190f090fcfef02 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 24 May 2026 21:30:37 +0800 Subject: [PATCH 4/6] fix(pm): address PR review findings on approve-builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review surfaced 3 high-severity issues plus several coverage/UX gaps. Fixes: HIGH severity - Local CLI (NAPI binding) lost the `Error::UserMessage` discriminator introduced in the round-1 friendly-error fix — every error variant was being wrapped in `Error::Anyhow` and printed with the harsh `error:` prefix. Now `execute_pm_command` intercepts UserMessage, prints it via `output::raw_stderr` (no prefix), and returns exit 1. Matches the global CLI's `is_user_message()` behavior. - `vp pm approve-builds --all -- esbuild` bypassed clap's `conflicts_with = "packages"` because the positional went to `pass_through_args` instead — the resolver then emitted `pnpm approve-builds --all esbuild`, which pnpm rejects with `ERR_PNPM_APPROVE_BUILDS_ALL_WITH_ARGS`. Added an up-front runtime guard that rejects `--all` combined with any non-flag pass-through token. Flag-only pass-through (`--silent`, `--loglevel=warn`) still works alongside `--all`. - `pass_through_args` were silently dropped on every no-op return path (npm, yarn, bun deny-only, bun no-args). Now each of those paths emits a `warn` listing the dropped args. UX / surface - Restored `ApproveBuilds` to the `needs_project` set in the handler. The round-1 reasoning (so educational messages can fire outside a project) didn't pan out: in an empty dir vp fell back to a synthetic npm `PackageManager` and the user got the npm `ignore-scripts=true` lecture regardless of their actual stack. approve-builds is fundamentally project-scoped; show `No package.json found.` instead, matching prune/list/audit. Coverage - bun snap fixture: added missing `"ignoredPlatforms": ["win32"]` to match the sibling bun fixtures. - Snap-test redaction (`utils.ts`): normalize the bun-style build hash (`v (af24e281)` → `v ()`) so the snap doesn't drift on every bun rebuild. - Added `command-pm-approve-builds-yarn4` snap fixture covering the yarn Berry branch's distinct `dependenciesMeta.built` warn text (previously only yarn v1 was snap-covered). - Added Rust unit tests: yarn 1.x prerelease (`1.22.22-canary`) still takes the v1 branch; deny-syntax gate rejects unparsable versions; `--all` + positional via `--` rejected at the resolver; `--all` + flag-only pass-through still accepted; npm no-op warns about dropped pass-through args. Tests: vite_install 28/28, vite_pm_cli 5/5, vp check --fix clean, all 6 snap fixtures regenerated. --- .../src/commands/approve_builds.rs | 108 ++++++++++++++++++ crates/vite_pm_cli/src/handlers.rs | 3 +- packages/cli/binding/src/cli/mod.rs | 14 ++- .../command-pm-approve-builds-bun/snap.txt | 4 +- .../command-pm-approve-builds-bun/steps.json | 3 +- .../package.json | 6 + .../command-pm-approve-builds-yarn4/snap.txt | 9 ++ .../steps.json | 8 ++ packages/tools/src/utils.ts | 3 + 9 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/package.json create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/snap.txt create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/steps.json diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs index 3004a3687a..48122949cd 100644 --- a/crates/vite_install/src/commands/approve_builds.rs +++ b/crates/vite_install/src/commands/approve_builds.rs @@ -45,6 +45,19 @@ impl PackageManager { &self, options: &ApproveBuildsCommandOptions, ) -> Result, Error> { + // Up-front guard: `--all` is incompatible with positional package names + // on both pnpm (ERR_PNPM_APPROVE_BUILDS_ALL_WITH_ARGS) and bun. clap's + // `conflicts_with` catches direct positionals, but tokens passed via + // `--` end up in `pass_through_args` and slip past — catch them here. + if options.all + && options.pass_through_args.is_some_and(|extras| extras.iter().any(is_positional_arg)) + { + return Err(Error::InvalidArgument( + "`--all` cannot be combined with positional package names (including via `--`)." + .into(), + )); + } + let bin_name: String; let mut args: Vec = Vec::new(); @@ -95,6 +108,7 @@ impl PackageManager { `vp pm approve-builds [...]` or `vp pm approve-builds --all`.", ); } + warn_dropped_pass_through(options.pass_through_args); return Ok(None); } @@ -112,6 +126,7 @@ impl PackageManager { `ignore-scripts=true` in .npmrc and rebuild approved packages with \ `vp pm rebuild `.", ); + warn_dropped_pass_through(options.pass_through_args); return Ok(None); } PackageManagerType::Yarn => { @@ -128,6 +143,7 @@ impl PackageManager { package, set `dependenciesMeta[\"\"].built: true` in package.json.", ); } + warn_dropped_pass_through(options.pass_through_args); return Ok(None); } } @@ -160,6 +176,22 @@ fn pnpm_supports_deny_syntax(version: &str) -> bool { Version::parse(version).is_ok_and(|v| v >= floor) } +fn is_positional_arg(token: &String) -> bool { + !token.starts_with('-') +} + +fn warn_dropped_pass_through(extras: Option<&[String]>) { + if let Some(extras) = extras + && !extras.is_empty() + { + output::warn(&format!( + "Ignoring pass-through args ({}): this package manager has no \ + native approve-builds command to forward them to.", + extras.join(" ") + )); + } +} + #[cfg(test)] mod tests { use tempfile::{TempDir, tempdir}; @@ -501,4 +533,80 @@ mod tests { .expect("resolves"); assert!(result.is_none()); } + + #[test] + fn yarn1_prerelease_still_v1() { + // A yarn 1.x prerelease (e.g. "1.22.22-canary") must still take the v1 + // branch, not the Berry branch. Guards against a future refactor that + // strips prereleases via Version::parse + major-check. + let pm = create_mock_package_manager(PackageManagerType::Yarn, "1.22.22-canary"); + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions::default()) + .expect("resolves"); + assert!(result.is_none()); + } + + #[test] + fn pnpm_deny_unparsable_version_rejects() { + // Strict gate: the deny-syntax helper rejects unparsable versions for + // the same reason as the `--all` gate (no semver, no support claim). + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "latest"); + let packages = vec!["!core-js".to_string()]; + let err = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + packages: &packages, + ..Default::default() + }) + .expect_err("unparsable version should fail the deny gate"); + assert!(matches!(err, Error::InvalidArgument(_))); + } + + #[test] + fn all_rejects_pass_through_positional() { + // `--all` + a positional token slipped via `--` should be rejected + // up-front (clap's conflicts_with can't catch the bypass). + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let extras = vec!["esbuild".to_string()]; + let err = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + pass_through_args: Some(&extras), + ..Default::default() + }) + .expect_err("--all + positional via -- should be rejected"); + assert!(matches!(err, Error::InvalidArgument(_))); + } + + #[test] + fn all_accepts_pass_through_flags() { + // `--all` + flag-only pass-through (`--silent`, `--loglevel=warn`) is + // legitimate — they're PM-level flags, not positionals. + let pm = create_mock_package_manager(PackageManagerType::Pnpm, "10.32.0"); + let extras = vec!["--silent".to_string(), "--loglevel=warn".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + pass_through_args: Some(&extras), + ..Default::default() + }) + .expect("flag-only extras allowed with --all") + .expect("supported"); + assert_eq!(result.args, vec!["approve-builds", "--all", "--silent", "--loglevel=warn"]); + } + + #[test] + fn npm_warns_about_dropped_pass_through() { + // No-op paths should not silently swallow user-supplied pass-through + // args; they should at least surface a warning. + let pm = create_mock_package_manager(PackageManagerType::Npm, "11.0.0"); + let extras = vec!["--silent".to_string()]; + let result = pm + .resolve_approve_builds_command(&ApproveBuildsCommandOptions { + pass_through_args: Some(&extras), + ..Default::default() + }) + .expect("resolves"); + assert!(result.is_none()); // still a no-op + // Visual inspection of the warn text is captured in snap-tests. + } } diff --git a/crates/vite_pm_cli/src/handlers.rs b/crates/vite_pm_cli/src/handlers.rs index a03213519f..45407ed189 100644 --- a/crates/vite_pm_cli/src/handlers.rs +++ b/crates/vite_pm_cli/src/handlers.rs @@ -179,7 +179,8 @@ pub async fn run_pm_subcommand( ) -> Result { let needs_project = matches!( command, - PmCommands::Prune { .. } + PmCommands::ApproveBuilds { .. } + | PmCommands::Prune { .. } | PmCommands::Pack { .. } | PmCommands::List { .. } | PmCommands::Publish { .. } diff --git a/packages/cli/binding/src/cli/mod.rs b/packages/cli/binding/src/cli/mod.rs index 8a16beffe1..872e0802e3 100644 --- a/packages/cli/binding/src/cli/mod.rs +++ b/packages/cli/binding/src/cli/mod.rs @@ -208,9 +208,17 @@ async fn execute_pm_command( "Global package operations (`-g`/`--global`) are only supported by the globally-installed `vp` CLI. See https://viteplus.dev/guide/ to install it, then run the same command via the global `vp` binary.", ))); } - let status = vite_pm_cli::dispatch(cwd, command) - .await - .map_err(|e| Error::Anyhow(anyhow::Error::new(e)))?; + let status = match vite_pm_cli::dispatch(cwd, command).await { + Ok(status) => status, + // Render `UserMessage` cleanly (no `error:` prefix) and exit non-zero — + // matches the global CLI's `is_user_message()` branch in main.rs so the + // friendly version-gate / usage errors look the same on both surfaces. + Err(vite_pm_cli::Error::UserMessage(msg)) => { + vite_shared::output::raw_stderr(&msg); + return Ok(ExitStatus(1)); + } + Err(e) => return Err(Error::Anyhow(anyhow::Error::new(e))), + }; Ok(ExitStatus(status.code().unwrap_or(1) as u8)) } diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt index 55084799a5..82158c431f 100644 --- a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt @@ -20,6 +20,6 @@ note: bun pm trust requires package names. Run `bun pm untrusted` to see which p > vp pm approve-builds '!core-js' # deny-only: prints deny warn, no redundant note warn: bun does not support denylisting build scripts. Packages outside `trustedDependencies` in package.json are already denied by default. Skipping: core-js -[1]> vp pm approve-builds --all # forwards bun pm trust --all (nothing to trust in this empty project) -bun pm trust v (af24e281) +[1]> vp pm approve-builds --all # forwards bun pm trust --all (errors on empty project — no lockfile) +bun pm trust v () error: Lockfile not found diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json index cf5e8e08aa..8276bb651c 100644 --- a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/steps.json @@ -1,8 +1,9 @@ { + "ignoredPlatforms": ["win32"], "commands": [ "vp pm approve-builds --help # should show help with --all caveat", "vp pm approve-builds # bun no-args: prints contextual note, exit 0", "vp pm approve-builds '!core-js' # deny-only: prints deny warn, no redundant note", - "vp pm approve-builds --all # forwards bun pm trust --all (nothing to trust in this empty project)" + "vp pm approve-builds --all # forwards bun pm trust --all (errors on empty project — no lockfile)" ] } diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/package.json b/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/package.json new file mode 100644 index 0000000000..606081e081 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/package.json @@ -0,0 +1,6 @@ +{ + "name": "command-pm-approve-builds-yarn4", + "version": "1.0.0", + "private": true, + "packageManager": "yarn@4.10.3" +} diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/snap.txt new file mode 100644 index 0000000000..b5a7304d53 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/snap.txt @@ -0,0 +1,9 @@ +> vp pm approve-builds # yarn Berry warn — points at dependenciesMeta[""].built +warn: yarn does not run third-party build scripts by default. To allow a package, set `dependenciesMeta[""].built: true` in package.json. + +> vp pm approve-builds esbuild # same warn (no native command on yarn) +warn: yarn does not run third-party build scripts by default. To allow a package, set `dependenciesMeta[""].built: true` in package.json. + +> vp pm approve-builds esbuild -- --silent # extras trigger the dropped-pass-through warn +warn: yarn does not run third-party build scripts by default. To allow a package, set `dependenciesMeta[""].built: true` in package.json. +warn: Ignoring pass-through args (--silent): this package manager has no native approve-builds command to forward them to. diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/steps.json b/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/steps.json new file mode 100644 index 0000000000..520d4a1564 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-yarn4/steps.json @@ -0,0 +1,8 @@ +{ + "ignoredPlatforms": ["win32"], + "commands": [ + "vp pm approve-builds # yarn Berry warn — points at dependenciesMeta[\"\"].built", + "vp pm approve-builds esbuild # same warn (no native command on yarn)", + "vp pm approve-builds esbuild -- --silent # extras trigger the dropped-pass-through warn" + ] +} diff --git a/packages/tools/src/utils.ts b/packages/tools/src/utils.ts index b9762d8ffe..4ffbf0a5bd 100644 --- a/packages/tools/src/utils.ts +++ b/packages/tools/src/utils.ts @@ -174,6 +174,9 @@ export function replaceUnstableOutput(output: string, cwd?: string) { .replaceAll(/\n\s+at .+/g, '') // replace git stash hashes: "git stash (abc1234)" => "git stash ()" .replaceAll(/git stash \([0-9a-f]+\)/g, 'git stash ()') + // replace bun-style build hashes after the semver banner: + // "bun pm trust v (af24e281)" => "bun pm trust v ()" + .replaceAll(/(v) \([0-9a-f]+\)/g, '$1 ()') // normalize cat error spacing: Windows "cat:file" vs Unix "cat: file" .replaceAll(/\bcat:(\S)/g, 'cat: $1') ); From e01ace87a7f6085b0815847359df498dca0f9531 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 24 May 2026 21:40:45 +0800 Subject: [PATCH 5/6] refactor(pm): use node-semver Range for approve-builds version gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR review, switch the version-gate helpers from manual `Version >= Version` comparison to `node_semver::Range` parsing with the readable `>=10.32.0` / `>=11.0.0` expressions. Behavior change: npm semver convention now applies — prereleases of any version do NOT satisfy a release-version range by default. So `10.32.0-rc.0` and `11.0.0-rc.0` no longer satisfy the `--all` gate; similarly for the `!pkg` deny gate against `>=11.0.0`. Users on a pnpm prerelease must bump to the corresponding release before relying on `--all` or `!pkg`. Tests flipped accordingly: - `pnpm_all_accepts_v10_32_prerelease` → `pnpm_all_rejects_v10_32_prerelease` - `pnpm_all_accepts_v11_prerelease` → `pnpm_all_rejects_v11_prerelease` - `pnpm_deny_accepts_v11_prerelease` → `pnpm_deny_rejects_v11_prerelease` Adds `node-semver = { workspace = true }` to `vite_install/Cargo.toml`. The workspace already declares the dep (v2.2.0). Tests: 28/28 still passing. --- Cargo.lock | 1 + crates/vite_install/Cargo.toml | 1 + .../src/commands/approve_builds.rs | 68 +++++++++++-------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 30af104f53..41e9e34fea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7571,6 +7571,7 @@ dependencies = [ "hex", "httpmock", "indoc", + "node-semver", "pathdiff", "reqwest", "semver 1.0.27", diff --git a/crates/vite_install/Cargo.toml b/crates/vite_install/Cargo.toml index 8719b75287..8362762f11 100644 --- a/crates/vite_install/Cargo.toml +++ b/crates/vite_install/Cargo.toml @@ -14,6 +14,7 @@ flate2 = { workspace = true } futures-util = { workspace = true } hex = { workspace = true } indoc = { workspace = true } +node-semver = { workspace = true } pathdiff = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs index 48122949cd..caa0445fba 100644 --- a/crates/vite_install/src/commands/approve_builds.rs +++ b/crates/vite_install/src/commands/approve_builds.rs @@ -1,6 +1,6 @@ use std::{collections::HashMap, process::ExitStatus}; -use semver::Version; +use node_semver::{Range, Version}; use vite_command::run_command; use vite_error::Error; use vite_path::AbsolutePath; @@ -160,20 +160,25 @@ impl PackageManager { fn pnpm_supports_approve_builds_all(version: &str) -> bool { // `pnpm approve-builds --all` was added in pnpm v10.32.0. - // Compare against the lowest prerelease of 10.32.0 (`10.32.0-0`) so that - // prereleases of 10.32.0 (e.g. `10.32.0-rc.0`, `10.32.0-beta.1`) also - // satisfy the gate per semver ordering rules. - // Unparsable versions are treated as not-supported (strict), since the - // production path always populates `version` from a validated semver. - let floor = Version::parse("10.32.0-0").expect("static semver"); - Version::parse(version).is_ok_and(|v| v >= floor) + // Uses npm-flavored range semantics (per node-semver): prereleases of + // any version do NOT satisfy `>=10.32.0` by default. Users on a pnpm + // prerelease should bump to the corresponding release before relying + // on `--all`. + version_satisfies(version, ">=10.32.0") } fn pnpm_supports_deny_syntax(version: &str) -> bool { // `!` deny syntax shipped in pnpm v11.0.0 (PR #11030). - // Compare against `11.0.0-0` so prereleases of v11 also satisfy. - let floor = Version::parse("11.0.0-0").expect("static semver"); - Version::parse(version).is_ok_and(|v| v >= floor) + // Same npm prerelease semantics as above. + version_satisfies(version, ">=11.0.0") +} + +fn version_satisfies(version: &str, range: &'static str) -> bool { + // Static range strings always parse; unparsable user-supplied versions + // are treated as not-satisfying (strict), since the production path + // populates `version` from a validated semver. + let range = range.parse::().expect("static range"); + version.parse::().is_ok_and(|v| v.satisfies(&range)) } fn is_positional_arg(token: &String) -> bool { @@ -275,17 +280,18 @@ mod tests { } #[test] - fn pnpm_deny_accepts_v11_prerelease() { + fn pnpm_deny_rejects_v11_prerelease() { + // npm semver convention: prereleases don't satisfy `>=11.0.0`. Users + // on `11.0.0-rc.0` must bump to a release before relying on `!pkg`. let pm = create_mock_package_manager(PackageManagerType::Pnpm, "11.0.0-rc.0"); let packages = vec!["!core-js".to_string()]; - let result = pm + let err = pm .resolve_approve_builds_command(&ApproveBuildsCommandOptions { packages: &packages, ..Default::default() }) - .expect("resolves") - .expect("supported"); - assert_eq!(result.args, vec!["approve-builds", "!core-js"]); + .expect_err("11.0.0-rc.0 should be rejected by deny gate (npm prerelease rule)"); + assert!(matches!(err, Error::InvalidArgument(_))); } #[test] @@ -327,31 +333,33 @@ mod tests { } #[test] - fn pnpm_all_accepts_v10_32_prerelease() { + fn pnpm_all_rejects_v10_32_prerelease() { + // npm semver convention: prereleases of any version do NOT satisfy + // `>=10.32.0`. Users on a 10.32.0-* tag must bump to 10.32.0 release. for version in &["10.32.0-0", "10.32.0-rc.0", "10.32.0-beta.1"] { let pm = create_mock_package_manager(PackageManagerType::Pnpm, version); - let result = pm - .resolve_approve_builds_command(&ApproveBuildsCommandOptions { - all: true, - ..Default::default() - }) - .unwrap_or_else(|_| panic!("{version} should accept --all")) - .unwrap_or_else(|| panic!("{version} should resolve")); - assert_eq!(result.args, vec!["approve-builds", "--all"]); + let result = pm.resolve_approve_builds_command(&ApproveBuildsCommandOptions { + all: true, + ..Default::default() + }); + assert!( + matches!(result, Err(Error::InvalidArgument(_))), + "{version}: expected rejection (npm prerelease rule), got {result:?}" + ); } } #[test] - fn pnpm_all_accepts_v11_prerelease() { + fn pnpm_all_rejects_v11_prerelease() { + // Same npm prerelease rule: 11.0.0-rc.0 does not satisfy `>=10.32.0`. let pm = create_mock_package_manager(PackageManagerType::Pnpm, "11.0.0-rc.0"); - let result = pm + let err = pm .resolve_approve_builds_command(&ApproveBuildsCommandOptions { all: true, ..Default::default() }) - .expect("v11 prerelease should accept --all") - .expect("should resolve"); - assert_eq!(result.args, vec!["approve-builds", "--all"]); + .expect_err("11.0.0-rc.0 should be rejected (npm prerelease rule)"); + assert!(matches!(err, Error::InvalidArgument(_))); } #[test] From 7d975a40a118d9d75a151a8a99a684296b23695e Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 24 May 2026 22:05:35 +0800 Subject: [PATCH 6/6] fix(snap-tests): regenerate bun + no-package-json snaps; add pnpm11 fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failure had two root causes: 1. The new bun build-hash redaction in packages/tools/src/utils.ts (added in `fix(pm): address PR review findings`) rewrote `v (af24e281)` → `v ()`. Six pre-existing bun fixtures still carried the literal hash and now diff against the redactor output. Regenerated: - command-add-bun - command-list-bun - command-outdated-bun - command-remove-bun - command-update-bun - command-why-bun 2. The local-CLI binding UserMessage intercept (round-2 fix #1) renders `Error::UserMessage` without the `error:` prefix, matching the global CLI. `command-pm-no-package-json` previously captured `error: No package.json found.` from the local CLI; now correctly captures `No package.json found.` (cleaner, matches global behavior). Also adds command-pm-approve-builds-pnpm11 snap fixture covering the green path for pnpm v11's `!pkg` deny syntax (round-3 review coverage gap finding). --- .../cli/snap-tests-global/command-add-bun/snap.txt | 8 ++++---- .../cli/snap-tests-global/command-list-bun/snap.txt | 2 +- .../snap-tests-global/command-outdated-bun/snap.txt | 6 +++--- .../command-pm-approve-builds-pnpm11/package.json | 6 ++++++ .../command-pm-approve-builds-pnpm11/snap.txt | 5 +++++ .../command-pm-approve-builds-pnpm11/steps.json | 6 ++++++ .../snap-tests-global/command-remove-bun/snap.txt | 12 ++++++------ .../snap-tests-global/command-update-bun/snap.txt | 4 ++-- .../cli/snap-tests-global/command-why-bun/snap.txt | 2 +- .../snap-tests/command-pm-no-package-json/snap.txt | 8 ++++---- 10 files changed, 38 insertions(+), 21 deletions(-) create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/package.json create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/snap.txt create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/steps.json diff --git a/packages/cli/snap-tests-global/command-add-bun/snap.txt b/packages/cli/snap-tests-global/command-add-bun/snap.txt index 8334324811..e025c8080f 100644 --- a/packages/cli/snap-tests-global/command-add-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-add-bun/snap.txt @@ -36,7 +36,7 @@ Usage: vp add ... [-- ...] For more information, try '--help'. > vp add testnpm2 -D && cat package.json # should add package as dev dependencies -bun add v (af24e281) +bun add v () installed testnpm2@ @@ -51,7 +51,7 @@ installed testnpm2@ } > vp add testnpm2 test-vite-plus-install && cat package.json # should add packages to dependencies -bun add v (af24e281) +bun add v () installed testnpm2@ installed test-vite-plus-install@ @@ -70,7 +70,7 @@ installed test-vite-plus-install@ } > vp install test-vite-plus-package@1.0.0 --save-peer && cat package.json # should install package alias for add -bun add v (af24e281) +bun add v () installed test-vite-plus-package@ @@ -91,7 +91,7 @@ installed test-vite-plus-package@ } > vp add test-vite-plus-package-optional -O && cat package.json # should add package as optional dependencies -bun add v (af24e281) +bun add v () installed test-vite-plus-package-optional@ diff --git a/packages/cli/snap-tests-global/command-list-bun/snap.txt b/packages/cli/snap-tests-global/command-list-bun/snap.txt index 37d247ea8e..fc97546e6a 100644 --- a/packages/cli/snap-tests-global/command-list-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-list-bun/snap.txt @@ -1,5 +1,5 @@ > vp install # should install packages first -bun install v (af24e281) +bun install v () + test-vite-plus-package@ + test-vite-plus-package-optional@ diff --git a/packages/cli/snap-tests-global/command-outdated-bun/snap.txt b/packages/cli/snap-tests-global/command-outdated-bun/snap.txt index edc726829d..f987d8805a 100644 --- a/packages/cli/snap-tests-global/command-outdated-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-outdated-bun/snap.txt @@ -25,7 +25,7 @@ Documentation: https://viteplus.dev/guide/install > vp install # should install packages first -bun install v (af24e281) +bun install v () + test-vite-plus-top-package@ + test-vite-plus-other-optional@ @@ -34,7 +34,7 @@ bun install v (af24e281) 4 packages installed [ms] > vp outdated testnpm2 # should show outdated package -bun outdated v (af24e281) +bun outdated v () |--------------------------------------| | Package | Current | Update | Latest | |----------|---------|--------|--------| @@ -42,7 +42,7 @@ bun outdated v (af24e281) |--------------------------------------| > vp outdated -r # should support recursive output -bun outdated v (af24e281) +bun outdated v () |---------------------------------------------------------------------------------------------| | Package | Current | Update | Latest | Workspace | |------------------------------------------|---------|--------|--------|----------------------| diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/package.json b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/package.json new file mode 100644 index 0000000000..0238e53c01 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/package.json @@ -0,0 +1,6 @@ +{ + "name": "command-pm-approve-builds-pnpm11", + "version": "1.0.0", + "private": true, + "packageManager": "pnpm@11.0.0" +} diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/snap.txt new file mode 100644 index 0000000000..30872167c4 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/snap.txt @@ -0,0 +1,5 @@ +> vp pm approve-builds --all # forwards pnpm approve-builds --all (nothing to approve) +There are no packages awaiting approval + +> vp pm approve-builds esbuild '!core-js' # pnpm 11.x supports !pkg deny syntax — forwarded as positional args +There are no packages awaiting approval diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/steps.json b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/steps.json new file mode 100644 index 0000000000..1a03e318d5 --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-pnpm11/steps.json @@ -0,0 +1,6 @@ +{ + "commands": [ + "vp pm approve-builds --all # forwards pnpm approve-builds --all (nothing to approve)", + "vp pm approve-builds esbuild '!core-js' # pnpm 11.x supports !pkg deny syntax — forwarded as positional args" + ] +} diff --git a/packages/cli/snap-tests-global/command-remove-bun/snap.txt b/packages/cli/snap-tests-global/command-remove-bun/snap.txt index b996e6a872..939bea12cf 100644 --- a/packages/cli/snap-tests-global/command-remove-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-remove-bun/snap.txt @@ -30,7 +30,7 @@ Usage: vp remove ... [-- ...] For more information, try '--help'. > vp remove testnpm2 -D && cat package.json # should error when remove not exists package from dev dependencies -bun remove v (af24e281) +bun remove v () package.json doesn't have dependencies, there's nothing to remove! { "name": "command-remove-bun", @@ -39,17 +39,17 @@ package.json doesn't have dependencies, there's nothing to remove! } > vp add testnpm2 && vp add -D test-vite-plus-install && vp add -O test-vite-plus-package-optional && cat package.json # should add packages to dependencies -bun add v (af24e281) +bun add v () installed testnpm2@ 1 package installed [ms] -bun add v (af24e281) +bun add v () installed test-vite-plus-install@ 1 package installed [ms] -bun add v (af24e281) +bun add v () installed test-vite-plus-package-optional@ @@ -70,7 +70,7 @@ installed test-vite-plus-package-optional@ } > vp remove testnpm2 test-vite-plus-install && cat package.json # should remove packages from dependencies -bun remove v (af24e281) +bun remove v () - testnpm2 - test-vite-plus-install @@ -85,7 +85,7 @@ bun remove v (af24e281) } > vp remove -O test-vite-plus-package-optional && cat package.json # should remove package from optional dependencies -bun remove v (af24e281) +bun remove v () package.json has no dependencies! Deleted empty lockfile diff --git a/packages/cli/snap-tests-global/command-update-bun/snap.txt b/packages/cli/snap-tests-global/command-update-bun/snap.txt index 494778e3a1..5e06654736 100644 --- a/packages/cli/snap-tests-global/command-update-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-update-bun/snap.txt @@ -26,7 +26,7 @@ Documentation: https://viteplus.dev/guide/install > vp update testnpm2 && cat package.json # should update package within semver range -bun update v (af24e281) +bun update v () + test-vite-plus-package@ + test-vite-plus-package-optional@ @@ -50,7 +50,7 @@ installed testnpm2@ } > vp up testnpm2 --latest && cat package.json # should update to absolute latest version -bun update v (af24e281) +bun update v () installed testnpm2@ diff --git a/packages/cli/snap-tests-global/command-why-bun/snap.txt b/packages/cli/snap-tests-global/command-why-bun/snap.txt index 5c7fe0029b..9093698e6a 100644 --- a/packages/cli/snap-tests-global/command-why-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-why-bun/snap.txt @@ -27,7 +27,7 @@ Documentation: https://viteplus.dev/guide/install > vp install # should install packages first -bun install v (af24e281) +bun install v () + test-vite-plus-package@ + test-vite-plus-package-optional@ diff --git a/packages/cli/snap-tests/command-pm-no-package-json/snap.txt b/packages/cli/snap-tests/command-pm-no-package-json/snap.txt index cca707ab25..2a604732ca 100644 --- a/packages/cli/snap-tests/command-pm-no-package-json/snap.txt +++ b/packages/cli/snap-tests/command-pm-no-package-json/snap.txt @@ -1,14 +1,14 @@ [1]> vp pm ls # should show friendly error -error: No package.json found. +No package.json found. [1]> vp pm prune # should show friendly error -error: No package.json found. +No package.json found. [1]> vp outdated # should show friendly error -error: No package.json found. +No package.json found. [1]> vp why lodash # should show friendly error -error: No package.json found. +No package.json found. > vp outdated definitely-not-installed-vite-plus-snap-pkg -g --format json # should not require package.json for global outdated {}