Skip to content

Commit 70a9b1a

Browse files
committed
fix(pm-cli): honor -g and reject -g --dry-run on the local CLI
Two local-CLI safety bugs on the new shared PM dispatcher (flagged by the codex adversarial review on PR #1495): 1. `vp install -g <pkg>` was downgraded into a project add. The shared `Install` arm dropped `global` and the `install <packages>` → `add` alias hardcoded `AddCommandOptions.global = false`, so the command silently ran `pnpm add <pkg>` (project) instead of `pnpm add -g <pkg>` (global). Worse, `run_add` always called `ensure_package_json`, so invoking `vp install -g foo` from an empty directory created a fresh `package.json` and added the package as a project dependency. Pass `global` through from `Install` to `AddCommandOptions`, and in `run_add` skip `ensure_package_json` when `options.global` is true, falling back to npm-default PM detection so we still have a binary to dispatch to in an empty directory. Same fix mirrored on `run_remove` so `vp remove -g <pkg>` works in an empty dir too. 2. `vp remove -g --dry-run <pkg>` could perform a real uninstall. The dispatcher bound `dry_run: _` and never forwarded it; the underlying `RemoveCommandOptions` has no preview field. Reject the combination in the dispatcher with a clear user message pointing the user at the globally-installed `vp` CLI. The global CLI's `run_package_manager_command` intercepts `Remove { global: true }` before reaching this path, so it keeps using its vite-plus-managed `managed_uninstall` flow with `dry_run` honored; the new error only fires on the local CLI. Verified end-to-end on the local CLI in an empty tmp dir: - `vp install -g testnpm2` installs globally, no package.json created - `vp remove -g testnpm2` removes globally, exit 0 - `vp remove -g --dry-run testnpm2` exits 1 with the new message and does not touch the global store Snap-test deltas: only the local `command-remove-pnpm10` fixture regenerates to capture the new error message; the global fixture is unchanged because the managed-uninstall path still handles `--dry-run`.
1 parent 97b89f2 commit 70a9b1a

3 files changed

Lines changed: 31 additions & 14 deletions

File tree

crates/vite_pm_cli/src/dispatch.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ pub async fn dispatch(
4242
save_peer,
4343
save_optional,
4444
save_catalog,
45-
global: _,
45+
global,
4646
node: _,
4747
packages,
4848
pass_through_args,
@@ -65,7 +65,7 @@ pub async fn dispatch(
6565
filters: filter.as_deref(),
6666
workspace_root,
6767
workspace_only: false,
68-
global: false,
68+
global,
6969
allow_build: None,
7070
pass_through_args: pass_through_args.as_deref(),
7171
};
@@ -141,10 +141,21 @@ pub async fn dispatch(
141141
workspace_root,
142142
recursive,
143143
global,
144-
dry_run: _,
144+
dry_run,
145145
packages,
146146
pass_through_args,
147147
} => {
148+
// `--dry-run` (which clap requires alongside `-g`) is honored by the
149+
// global CLI's vite-plus-managed uninstall flow. The shared
150+
// dispatcher reaches this arm only for non-managed paths
151+
// (project-scoped removes, or local-CLI global removes), and the
152+
// underlying `RemoveCommandOptions` has no preview field. Refuse
153+
// rather than silently performing a real uninstall.
154+
if dry_run {
155+
return Err(Error::UserMessage(
156+
"`--dry-run` is only supported by the globally-installed `vp` CLI for managed packages. Run the command without `--dry-run` to actually remove the package.".into(),
157+
));
158+
}
148159
let options = RemoveCommandOptions {
149160
packages: &packages,
150161
filters: filter.as_deref(),

crates/vite_pm_cli/src/handlers.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,14 @@ pub async fn run_add(
5050
cwd: &AbsolutePath,
5151
options: &AddCommandOptions<'_>,
5252
) -> Result<ExitStatus, Error> {
53-
ensure_package_json(cwd).await?;
54-
let pm = PackageManager::builder(cwd).build_with_default().await?;
53+
let pm = if options.global {
54+
// Global add doesn't need a project; fall back to npm if no package.json
55+
// is present so the underlying PM still has a binary to dispatch to.
56+
build_package_manager_or_npm_default(cwd).await?
57+
} else {
58+
ensure_package_json(cwd).await?;
59+
PackageManager::builder(cwd).build_with_default().await?
60+
};
5561
Ok(pm.run_add_command(options, cwd).await?)
5662
}
5763

@@ -68,7 +74,13 @@ pub async fn run_remove(
6874
cwd: &AbsolutePath,
6975
options: &RemoveCommandOptions<'_>,
7076
) -> Result<ExitStatus, Error> {
71-
let pm = build_package_manager(cwd).await?;
77+
let pm = if options.global {
78+
// Global remove doesn't need a project; fall back to npm if no
79+
// package.json is present.
80+
build_package_manager_or_npm_default(cwd).await?
81+
} else {
82+
build_package_manager(cwd).await?
83+
};
7284
Ok(pm.run_remove_command(options, cwd).await?)
7385
}
7486

packages/cli/snap-tests/command-remove-pnpm10/snap.txt

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,8 @@ Done in <variable>ms using pnpm v<semver>
9797
"packageManager": "pnpm@<semver>"
9898
}
9999

100-
> vp remove -g --dry-run testnpm2 && cat package.json # support remove global package with dry-run
101-
102-
up to date in <variable>ms
103-
{
104-
"name": "command-remove-pnpm10",
105-
"version": "1.0.0",
106-
"packageManager": "pnpm@<semver>"
107-
}
100+
[1]> vp remove -g --dry-run testnpm2 && cat package.json # support remove global package with dry-run
101+
error: `--dry-run` is only supported by the globally-installed `vp` CLI for managed packages. Run the command without `--dry-run` to actually remove the package.
108102

109103
[2]> vp rm --stream foo && should show tips to use pass through arguments when options are not supported
110104
error: Unexpected argument '--stream'

0 commit comments

Comments
 (0)