Walkthrough

Adds a new pacquet bin CLI subcommand that prints the local node_modules/.bin directory or, with --global/-g, the configured global bin directory. The change includes command registration, dispatch wiring, implementation, and integration tests.

Changes

pacquet bin subcommand

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

product: pacquet

Suggested reviewers

• zkochan

• X
• Mastodon
• Reddit
• LinkedIn

• Add pacquet bin to print /node_modules/.bin anchored on canonicalized --dir.
• Support --global/-g by printing config.global_bin and reusing ERR_PNPM_NO_GLOBAL_BIN_DIR.
• Add integration tests, including a differential parity test against real pnpm in a workspace
 subdir.

graph TD
  U(["User"]) --> CLI["pacquet CLI"] --> CMD["bin handler"] --> DEC{"--global?"}
  DEC -->|"no"| LOCAL["<dir>/node_modules/.bin"] --> OUT["stdout"]
  DEC -->|"yes"| GBIN["Config.global_bin"] --> OUT
  GBIN -->|"None"| ERR["ERR_PNPM_NO_GLOBAL_BIN_DIR"]

  subgraph Legend
    direction LR
    _ext(["External"]) ~~~ _cmd["Command"] ~~~ _dec{"Decision"}
  end

Loading

The following are alternative approaches to this PR:

1. Centralize local bin-dir derivation as a shared helper

• ➕ Avoids duplicating pnpm-parity rules across commands (root and bin both hardcode node_modules leaves)
• ➕ Easier to keep workspace/cwd anchoring behavior consistent over time
• ➖ Introduces an abstraction that may not pay off until more commands need it
• ➖ Risk of over-generalizing command-specific parity quirks (e.g., intentionally ignoring modules-dir)

2. Validate global bin dir for read-only `bin -g` via `check_global_bin_dir`

• ➕ Closer behavioral parity with pnpm (PATH membership and writability validation)
• ➕ Earlier, clearer feedback when global config is broken
• ➖ Changes current pacquet policy of validating/creating global bin dir only in mutating global commands
• ➖ May be undesirable for a pure query command that should not enforce PATH invariants

Recommendation: The chosen approach is solid for a read-only query command and matches existing pacquet behavior for other read-only global queries (e.g., outdated --global). Consider a follow-up to move global-bin validation into a shared config/bootstrap step if strict pnpm parity for all -g commands is desired, and optionally factor a small helper for node_modules-leaf path construction if more pnpm query ports are added.

Enhancement (5) +64 / -0

cli_args.rsRegister new 'bin' args module +1/-0

Register new 'bin' args module

• Adds 'pub mod bin;' so the new 'bin' subcommand args can be compiled and referenced by the CLI.

pacquet/crates/cli/src/cli_args.rs

bin.rsImplement 'pacquet bin' command (local and '--global') +53/-0

Implement 'pacquet bin' command (local and '--global')

• Introduces 'BinArgs' with '--global/-g'. Local mode prints '<canonicalized --dir>/node_modules/.bin' (deliberately ignoring 'modules-dir'); global mode prints 'config.global_bin' and errors with 'ERR_PNPM_NO_GLOBAL_BIN_DIR' when missing.

pacquet/crates/cli/src/cli_args/bin.rs

cli_command.rsExpose 'bin' as a top-level CLI subcommand +3/-0

Expose 'bin' as a top-level CLI subcommand

• Adds 'CliCommand::Bin(BinArgs)' and help text so 'pacquet bin' is available via clap parsing.

pacquet/crates/cli/src/cli_args/cli_command.rs

dispatch.rsRoute 'bin' command through main dispatcher +1/-0

Route 'bin' command through main dispatcher

• Extends the router match to dispatch 'CliCommand::Bin' to the query dispatch module.

pacquet/crates/cli/src/cli_args/dispatch.rs

dispatch_query.rsAdd query dispatch entry for 'bin' +6/-0

Add query dispatch entry for 'bin'

• Adds 'dispatch_query::bin()' which loads config and invokes 'BinArgs::run', returning a ready future like other synchronous query commands.

pacquet/crates/cli/src/cli_args/dispatch_query.rs

Tests (1) +144 / -0

bin.rsAdd integration tests for 'pacquet bin' (local, global, pnpm parity) +144/-0

Add integration tests for 'pacquet bin' (local, global, pnpm parity)

• Adds tests verifying local output (with canonicalization), that 'modulesDir' does not change output, deterministic 'bin -g' via pinned env vars, and a differential test ensuring byte-identical output to real 'pnpm bin' from a workspace subdirectory (ignored on Windows due to 'pnpm.cmd' resolution).

pacquet/crates/cli/tests/bin.rs

Description

The new local pacquet bin integration tests spawn pacquet bin without pinning HOME/XDG env
vars, but bin currently loads full configuration, so the tests can become environment-dependent or
fail on machines with unexpected user/global pnpm config state. This makes CI/local runs potentially
flaky for reasons unrelated to the test’s intent.

Code

pacquet/crates/cli/tests/bin.rs[R19-24]

+fn bin_prints_the_local_node_modules_bin_dir() {
+    let CommandTempCwd { pacquet, root, workspace, .. } = CommandTempCwd::init();
+
+    let output = pacquet.with_args(["bin"]).output().expect("run pacquet bin");
+    dbg!(&output);
+    assert!(output.status.success(), "pacquet bin should succeed");

Evidence

The tests run pacquet bin with no environment isolation, while the bin dispatch path forces a
configuration load that reads from user/environment-derived locations, making the test outcome
depend on the runner’s HOME/XDG state.

pacquet/crates/cli/tests/bin.rs[18-24]
pacquet/crates/testing-utils/src/bin.rs[23-36]
pacquet/crates/cli/src/cli_args/dispatch_query.rs[169-172]
pacquet/crates/cli/src/cli_args/dispatch.rs[158-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new local `pacquet bin` tests run the CLI without isolating user-level config locations (HOME/XDG). Because `pacquet bin` currently routes through `ctx.config()` even in local mode, these tests can depend on the developer/CI machine’s global config files and environment.
### Issue Context
- `dispatch_query::bin` calls `args.run(ctx.dir, (ctx.config)()?)`, which forces a config load.
- The config loader consults user/environment-derived config locations.
- `CommandTempCwd::init()` only sets the working directory; it doesn’t pin HOME/XDG env vars for the spawned command.
### Fix Focus Areas
- pacquet/crates/cli/tests/bin.rs[19-49]
### Suggested fix
In `bin_prints_the_local_node_modules_bin_dir` and `bin_ignores_a_custom_modules_dir`, set the same env vars you already pin in the `-g` tests (at least `HOME` and `XDG_CONFIG_HOME`; optionally also `XDG_DATA_HOME` if config lookup uses it elsewhere) to directories under `root.path()` before calling `.output()`. This makes the tests hermetic even while `pacquet bin` continues to load config.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

BinArgs::run reads PATH using std::env::var("PATH"), which returns an error for non-UTF-8
values on Unix; the code converts that to None and check_global_bin_dir then reports
ERR_PNPM_NO_PATH_ENV even though PATH is set. This can cause pacquet bin -g to fail incorrectly
(and with a misleading error) in the rare case where PATH contains non-Unicode bytes.

Code

pacquet/crates/cli/src/cli_args/bin.rs[R53-54]

+            check_global_bin_dir(&bin, std::env::var("PATH").ok().as_deref(), true)
+                .map_err(miette::Report::new)?;

Evidence

The new bin -g implementation uses std::env::var("PATH").ok() (Unicode-only), while
check_global_bin_dir treats a None/empty path_env as ERR_PNPM_NO_PATH_ENV. Elsewhere in the
CLI, PATH is handled via var_os, indicating the codebase expects to support OS-string PATH values.

pacquet/crates/cli/src/cli_args/bin.rs[49-55]
pacquet/crates/config/src/global_bin_check.rs[53-60]
pacquet/crates/cli/src/cli_args/exec.rs[242-249]
pacquet/crates/cli/src/cli_args/dlx.rs[557-564]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pacquet bin -g` passes `std::env::var("PATH").ok().as_deref()` into `check_global_bin_dir`. On Unix, `std::env::var` fails if PATH contains non-UTF-8 bytes, so `.ok()` turns that into `None`, and `check_global_bin_dir` emits `ERR_PNPM_NO_PATH_ENV` (misleading) and the command fails.
### Issue Context
Other CLI code paths treat PATH as an OS string (`var_os`) to avoid Unicode-only assumptions.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/bin.rs[53-54]
- pacquet/crates/config/src/global_bin_check.rs[53-60]
### Suggested fix
Prefer making `check_global_bin_dir` accept `Option<&std::ffi::OsStr>` (so it can call `std::env::split_paths` directly without lossy conversion), and update call sites (`bin`, and any other `check_global_bin_dir` callers) to use `std::env::var_os("PATH")`.
If you want a smaller/local change, you can use `std::env::var_os("PATH")` and convert with `to_string_lossy()` before passing it in, but that’s lossy; the `OsStr` signature is the more correct long-term shape.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pacquet bin -g calls create_dir_all() on config.global_bin without ensuring it is an absolute
path, so a relative global-bin-dir value will be created relative to the process CWD. Because
Config::current takes global-bin-dir verbatim from env/config into a PathBuf with no
normalization, this can lead to confusing/incorrect filesystem mutations and output when users (or
CI) provide a relative setting.

Code

pacquet/crates/cli/src/cli_args/bin.rs[R44-54]

+            let bin = config.global_bin.clone().ok_or(GlobalError::NoGlobalBinDir)?;
+            // pnpm's config reader `mkdir`s the global bin dir and runs
+            // `checkGlobalBinDir` for every `--global` command before the handler
+            // prints; `globalDirShouldAllowWrite` is true for all but `root`, so
+            // `bin` validates writability too.
+            std::fs::create_dir_all(&bin).map_err(|error| {
+                let bin_dir = bin.display();
+                miette::miette!("failed to create the global bin directory {bin_dir}: {error}")
+            })?;
+            check_global_bin_dir(&bin, std::env::var("PATH").ok().as_deref(), true)
+                .map_err(miette::Report::new)?;

Evidence

The bin -g handler unconditionally creates config.global_bin on disk, and Config::current
assigns global_bin from global_bin_dir sourced directly from env/config via PathBuf::from
without making it absolute; therefore a relative configured value can be used and will be created
relative to the current directory.

pacquet/crates/cli/src/cli_args/bin.rs[42-55]
pacquet/crates/config/src/lib.rs[2269-2289]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pacquet bin -g` creates the resolved global bin directory (`config.global_bin`) via `std::fs::create_dir_all(&bin)` even if `bin` is a relative path. Since `Config::current` may populate `global_bin_dir` (and thus `global_bin`) directly from `PNPM_CONFIG_GLOBAL_BIN_DIR`/`pnpm_config_global_bin_dir` without converting it to an absolute path, a relative value will create directories relative to the current working directory.
### Issue Context
This is triggered only for `--global`, but it is a user-facing command and should avoid creating directories in unexpected locations.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/bin.rs[42-55]
- pacquet/crates/config/src/lib.rs[2269-2289]
### Suggested fix
- Before `create_dir_all`, validate `bin.is_absolute()` and return a diagnostic error if not.
- Alternatively (if matching pnpm requires it), explicitly resolve relative `global-bin-dir` against a well-defined anchor (e.g., pnpm home dir) during `Config::current`, so all global commands share the same invariant.
- Add/adjust a test that sets `PNPM_CONFIG_GLOBAL_BIN_DIR` to a relative value and asserts the command errors (or resolves deterministically) rather than creating a directory under the CWD.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

BinArgs::run converts create_dir_all failures into a new miette::Report string, dropping the
original io::Error as a chained source and reducing diagnostic fidelity. This is inconsistent with
existing CLI code paths that preserve the underlying error via IntoDiagnostic + wrap_err, making
failures harder to debug.

Code

pacquet/crates/cli/src/cli_args/bin.rs[R30-33]

+            std::fs::create_dir_all(&bin).map_err(|error| {
+                let bin_dir = bin.display();
+                miette::miette!("failed to create the global bin directory {bin_dir}: {error}")
+            })?;

Evidence

The new bin command drops the underlying io::Error when wrapping create_dir_all, while
existing CLI code preserves sources via into_diagnostic().wrap_err(...) for similar filesystem
operations.

pacquet/crates/cli/src/cli_args/bin.rs[24-35]
pacquet/crates/cli/src/cli_args/global.rs[96-101]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pacquet bin -g` wraps `std::fs::create_dir_all` errors using `map_err` + `miette::miette!(...)`, which produces a new report and loses the original `io::Error` as a source in the diagnostic chain.
### Issue Context
Other CLI code (e.g. global command setup) uses `IntoDiagnostic` and `wrap_err` to keep the underlying error and add context. Matching that pattern here improves debugging without changing behavior.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/bin.rs[24-36]
### Suggested change
Replace the `map_err(|error| miette::miette!(...))` block with something that preserves the source, e.g.:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new integration tests unconditionally call dbg!(&output), which spams stderr on successful
runs and adds noise to CI logs.

Code

pacquet/crates/cli/tests/bin.rs[R23-25]

+    let output = pacquet.with_args(["bin"]).output().expect("run pacquet bin");
+    dbg!(&output);
+    assert!(output.status.success(), "pacquet bin should succeed");

Evidence

The new test file contains multiple unconditional dbg!(&output) calls in normal test flow.

pacquet/crates/cli/tests/bin.rs[23-26]
pacquet/crates/cli/tests/bin.rs[45-47]
pacquet/crates/cli/tests/bin.rs[68-81]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`dbg!(&output)` is left in the committed integration tests and prints on every successful test run.
### Issue Context
This is pure test-log noise; the assertions already provide failure context.
### Fix Focus Areas
- pacquet/crates/cli/tests/bin.rs[23-26]
- pacquet/crates/cli/tests/bin.rs[45-47]
- pacquet/crates/cli/tests/bin.rs[68-81]
### Fix
Remove the `dbg!` calls (or gate them behind an env var if occasional debugging is needed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.