`pacquet/benchmark/` (the `add.sh` hyperfine script plus its `.npmrc`,
`.yarnrc.yml`, and `bunfig.toml` companions) compared `pacquet add
fastify` against pnpm/yarn/bun's `add fastify`. It was imported once
with the pacquet merge and never wired into any CI workflow, justfile
recipe, README, or other script — `grep -r pacquet/benchmark` returns
nothing outside the files themselves.

Cross-package-manager install comparisons are already covered (and
more rigorously) by `pacquet/tasks/integrated-benchmark` via its
`--with-pnpm` flag, which is what every active PR uses today. The
three remaining benchmark systems (the TS pnpm hyperfine bench under
`benchmarks/`, and the Rust `integrated-benchmark` + `micro-benchmark`
under `pacquet/tasks/`) cover distinct audiences and layers and are
left as-is.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

Generalize the Rust orchestrator at `pacquet/tasks/integrated-benchmark/`
so it can build and benchmark pnpm revisions alongside pacquet
revisions, preparing the ground for `benchmarks/bench.sh` to become a
thin wrapper that delegates here. Existing CI invocations stay
back-compat.

What this commit changes:

- `BenchmarkScenario` gains three pnpm-only variants — `Peek`,
  `FullResolution`, `GvsWarm` — alongside the existing
  `CleanInstall`, `FrozenLockfile`, `FrozenLockfileHotCache`. Each
  scenario declares whether pacquet supports it; pacquet-unsupported
  scenarios reject pacquet targets at startup with a pointed error.
- Per-iteration cleanup grows from a `&[&str]` of paths-to-remove
  into a `Cleanup { remove, restore }` pair. `restore` overwrites
  `pnpm-lock.yaml` and `package.json` from pristine copies saved
  during `init()`, mirroring the defensive copies `bench.sh` does for
  the same scenarios. `--prepare` becomes
  `rm -rf … && cp .saved-X X && cp .saved-Y Y` per bench dir.
- Positional revisions are now `TargetSpec`s parsed from `<rev>`,
  `pacquet@<rev>`, or `pnpm@<rev>`. Bare revisions default to
  pacquet, so the existing CI invocation
  `--scenario=frozen-lockfile --verdaccio --with-pnpm HEAD main`
  parses unchanged.
- For pnpm targets, the build runs `pnpm install && pnpm run compile`
  in the cloned source and resolves the binary as
  `pnpm/dist/pnpm.mjs` (falling back to `.cjs`), matching
  `bench.sh:55-62`. The install script becomes `node <bundle>
  install …` for pnpm targets vs. `./pacquet/target/release/pacquet
  install …` for pacquet targets. `--pnpm-repository` overrides the
  pnpm checkout source; defaults to `--repository` so a monorepo
  checkout works without an extra flag.
- New `--registry={verdaccio,npm,virtual}` flag picks the registry
  the install hits. `verdaccio` keeps the existing
  spawn-or-attach-verdaccio path; `npm` skips the proxy entirely and
  points `.npmrc` at `https://registry.npmjs.org/` for live
  benchmarks; `virtual` (the default) keeps the existing mockito
  passthrough. The old `--verdaccio` short flag is a back-compat
  alias for `--registry=verdaccio`.
- `GvsWarm` flips `enableGlobalVirtualStore: true` in the synthesized
  workspace manifest and runs a pre-hyperfine warm-up of each target
  so timed iterations see a hot GVS.
- `verify::ensure_program` is now target-kind aware: `cargo` is only
  required when a pacquet target is present, `node`/`pnpm` only when
  a pnpm target (or `--with-pnpm`) is present.

What this commit does NOT change:

- The CI workflows (`.github/workflows/benchmark.yml`,
  `.github/workflows/pacquet-integrated-benchmark.yml`) are
  untouched. Their scenario sets stay identical (6 and 2
  respectively) — those land in follow-up commits.
- `benchmarks/bench.sh` is untouched in this commit; the wrapper
  rewrite is a follow-up.
- No new third-party crate dependencies.

Verification:

- `cargo check --bin=integrated-benchmark` clean
- `cargo clippy --bin=integrated-benchmark -- --deny warnings` clean
- `cargo fmt --check -p pacquet-integrated-benchmark` clean
- `cargo check --workspace --all-targets` clean
- `cargo run --bin=integrated-benchmark -- --help` lists all six
  scenarios, the three registry modes, and the new `--pnpm-repository`
  flag; positional `<TARGETS>...` doc reflects the new spec syntax.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

…ints

The dylint job runs the `perfectionist` lint family with `-D warnings`,
which forbids single-letter function parameters, generic parameters,
and closure parameters. The previous commit introduced five
violations:

- `TargetSpec::from_str(s: &str)` — single-letter fn param `s` →
  rename to `input`.
- `build_cleanup_command<'a, I, BenchDir>` — single-letter generic
  `I` → rename to `Ids` (a noun matching the parameter's role).
- Three `|t|` closures in `main.rs` that filter/map over `targets` →
  rename to `|target|`.

Pure rename, no behavior change. Verified `cargo clippy --bin=integrated-benchmark
-- --deny warnings` and `cargo fmt --check` clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

Clap surfaces field rustdocs verbatim as `--help` text, so verbose
rustdocs that read like architecture notes blow up the help output.
Per review on #11741, shorten each clap-visible doc to one
line / one short paragraph that reads cleanly as CLI help:

- `--verdaccio`: drop the "kept for back-compat" historical note —
  not useful to a user reading help. The compat behaviour is still
  the same (`-V` is a short alias and overrides `--registry`).
- `--pnpm-repository`: collapse the rationale ("monorepo checkout
  containing both works out of the box") into the existing
  "Defaults to `--repository`" line; the rationale is for code
  readers, not CLI users.
- `--with-pnpm`: collapse to one line — "Also benchmark the
  system-installed pnpm." "System-installed" disambiguates from the
  new `pnpm@<rev>` target syntax (a CLI user could otherwise wonder
  whether `--with-pnpm` and `pnpm@HEAD` are equivalent), so it
  stays in the wording even at one line.
- `targets`: collapse the bulleted variant list into one inline
  enumeration. The exhaustive bullets came out poorly in clap's
  help renderer anyway (run-together dashes).
- `RegistryMode` / `BenchmarkScenario` value-enum variants: one
  line each. The verbose verdaccio explanation, the "frozen
  lockfile, warm local store" phrasing, the "no lockfile and
  without local cache" expansion etc. are all redundant with the
  scenario / mode names. Keep the "Pnpm-only" marker on the three
  pnpm-only scenarios — that's the load-bearing detail.

`Cleanup` struct docs (`Cleanup` itself is not clap-visible, used
internally) are also trimmed in the same pass, per the same review
question on line 152: the `restore` field doc explained `(dst,
src)` semantics three times. Reduced to one line plus a brief "why"
clause.

Verified `cargo run --bin=integrated-benchmark -- --help` renders
each flag and value-enum variant on one line; `cargo clippy
--deny warnings` and `cargo fmt --check` still clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

Round of fixes raised by my own /review pass. None affect what the
benchmarks measure — they tighten contracts the previous commits left
loose, drop dead init work, and correct three small documentation /
comment errors.

- `verify::ensure_git_repo` was checked unconditionally and asserted
  `Cargo.toml` + `Cargo.lock` exist. That's a pacquet-shape check;
  pointing `--pnpm-repository` at a standalone pnpm clone would panic.
  Split into `ensure_pacquet_git_repo` and `ensure_pnpm_git_repo`
  (the latter looks for `pnpm/package.json` or `pnpm-workspace.yaml`)
  and call each only for the target kinds that need them.
- `pnpm` was only added to the ensure-program list when a pnpm target
  or `--with-pnpm` was in play. The proxy-cache populator that runs
  for verdaccio/virtual modes also shells out to `pnpm install`, so a
  pacquet-only run with `--registry=virtual` (the default) could miss
  the prerequisite check. Require `pnpm` for any non-npm registry mode.
- `validate_revision_list`'s allowed alphabet rejected `@` and `/`,
  even though `TargetSpec::from_str`'s "unknown prefix keeps full
  string as pacquet rev" branch (and its unit test) explicitly accepts
  revs like `v1.0.0@sha`. Widen the alphabet so the documented
  contract holds end-to-end. `@` and `/` are not shell metacharacters
  in the contexts we pass revs to (git CLI args, bench-dir names).
- `init()` was synthesizing files for the `.init-proxy-cache` bench
  dir regardless of registry mode. With `--registry=npm` those files
  are never read. Gate the whole `INIT_PROXY_CACHE` entry in the
  init id-list on `populate_proxy_cache` so npm runs skip the work
  entirely.
- `--registry-port` doc: note that it's ignored when `--registry=npm`.
- `benchmarks/bench.sh`: defensive `git fetch origin main:main` so
  `pnpm@main` resolves on dispatch runs where `actions/checkout` only
  created a local ref for the head branch. Move the `read_cell`
  helper above the scenario loop instead of redefining it every
  iteration. Switch jq from `floor` to `round` so cell values match
  the rounding the deleted `generate-results.js` did with
  `toFixed(3)`. Drop dead `${main_cell:-n/a}` fallbacks (read_cell
  always returns a non-empty string). Rewrite the misleading
  "the per-scenario build step is a no-op" comment — the bundle
  step is not incremental and re-runs each time. Six extra bundle
  runs is accepted overhead in exchange for one consistent build
  path shared with the pacquet bench.
- `benchmarks/README.md`: drop the false "Rust toolchain is
  bootstrapped automatically by `cargo build`" sentence.

Verified `cargo clippy --bin=integrated-benchmark -- --deny warnings`
and `cargo fmt --check -p pacquet-integrated-benchmark` clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

CodeRabbit / Copilot flagged that `WorkEnv::pnpm_bundle_path()` checked
`pnpm.mjs.exists()` at `init()` time, but the pnpm source dir doesn't
exist yet at that point — `build()` runs after `init()` and is what
clones the repo and runs `pnpm install && pnpm run compile`. So the
existence check was always false at the call site, and the generated
`install.bash` always hardcoded `pnpm.cjs`. Modern pnpm produces both
bundles so the script didn't crash, but the "prefer mjs, fall back to
cjs" intent (mirroring `benchmarks/bench.sh`'s `resolve_pnpm_bin`) was
silently broken — measurements always exercised the CJS startup path.

The fix moves the `[ -f X ]` check into the install script itself, so
it runs after `build()` has produced the bundle:

    node "$([ -f ./pnpm-source/pnpm/dist/pnpm.mjs ] \
            && echo ./pnpm-source/pnpm/dist/pnpm.mjs \
            || echo ./pnpm-source/pnpm/dist/pnpm.cjs)"

Two side wins:

- The path is now relative to the bench dir (which the script `cd`s into
  at the top), so it doesn't depend on a `--work-env` value the orchestrator
  embeds as an absolute prefix. Resolves Copilot's separate concern about
  unquoted absolute paths breaking if the work-env contains shell
  metacharacters.
- `WorkEnv::pnpm_bundle_path` was only ever called from this path and is
  now dead — deleted. `install_command` no longer needs `&self` since none
  of its three branches read anything off the receiver; it's a free function
  on `WorkEnv` now.

Also tightens the `validate_revision_list` comment in `verify.rs`. The
previous wording cited `HEAD@{1}` as a reason for allowing `@`, but `{`
and `}` are still rejected — the comment was misleading. Reflog
revisions aren't supported (and shouldn't be: `{` is a shell
metacharacter we don't want in bench-dir names). The real reason `@` is
allowed is `v1.0.0@sha`-style tag suffixes.

Verified `cargo clippy --locked --workspace --all-targets -- -D warnings`,
`cargo fmt --all -- --check`, and `cargo test -p pacquet-integrated-benchmark`
(5/5 pass) clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

…s-warm

zkochan noted on PR #11741 that pacquet now implements all three of the
"pnpm-only" scenarios, so the startup rejection in main.rs is no longer
warranted. Confirmed on origin/main:

- **peek (`pacquet add <pkg>`)** — `cli_args/add.rs` exposes `Add` with
  full `--save-prod/dev/optional/peer` flag set; `pacquet_package_manager::Add`
  is the install pipeline behind it.
- **full-resolution (`pacquet install` without `--frozen-lockfile`)** —
  `install.rs:594-623` dispatches to `InstallWithoutLockfile::run`,
  which runs the full resolver chain (`NpmResolver` →
  `GitResolver` → `TarballResolver` → local / runtime / named-registry
  resolvers) against the manifest, downloading and installing real
  packages. Gated only by `config.lockfile = false` (which the bench
  fixture sets via `lockfile=false` in `.npmrc` for the FullResolution
  scenario).
- **gvs-warm (`pacquet install --frozen-lockfile` + `enableGlobalVirtualStore:
  true`)** — `install_frozen_lockfile.rs:540` and `install.rs:556-568`
  read `config.enable_global_virtual_store` and route slot layout
  through the GVS shape (`virtual_store_layout.rs:170-175`). The
  bench's workspace-manifest synthesizer already flips
  `enable_global_virtual_store: true` for `GvsWarm`.

Changes:

- Drop `BenchmarkScenario::supports_pacquet()` — every scenario now
  supports both kinds. The method was only called from one site.
- Drop the startup panic in `main.rs` that rejected pacquet targets
  for the three scenarios above.
- Drop the "Pnpm-only" suffix from the three rustdoc lines on
  `BenchmarkScenario::{Peek, FullResolution, GvsWarm}` — the CLI
  `--help` no longer claims a restriction that doesn't exist.

Verified `cargo clippy --locked --workspace --all-targets -- -D warnings`,
`cargo fmt --all -- --check`, and `cargo test -p pacquet-integrated-benchmark`
(5/5 pass) clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

Four follow-up findings from Copilot, all valid:

- **bash_command quoting (work_env.rs:77)** — hyperfine runs each
  benchmarked command through a shell, so the `bash {path}` string
  must survive shell-tokenization. `path` is `<work_env>/<bench_id>
  /install.bash` and `--work-env` is user-supplied, so a path with
  spaces / metacharacters would break the run. Wrap the script path
  in `os_display::Quotable::maybe_quote()` so the result is
  shell-safe (and bare for the common alphanumeric/slash/dash case).

- **`pnpm run compile` is too heavy (work_env.rs:236)** — the root
  `compile` script is `pn compile-only && pn update-manifests`, and
  `update-manifests` fires an extra `pnpm install` plus `pn
  meta-updater` (which rewrites tracked manifest files). The bench
  doesn't need any of that — it only needs `pnpm/dist/pnpm.{mjs,cjs}`,
  which `compile-only` already produces via `tsgo --build` →
  `typecheck-only` → `pn -F=pnpm compile`. Switching to
  `compile-only` drops the redundant install on every revision build.
  As a side benefit the second-invocation `git reset --hard` in
  `sync_bench_repo` will have less to discard (lockfile / generated
  metadata won't drift). Comment references that mentioned
  `pnpm run compile` updated to track the new name.

- **`--pnpm-repository` help text was misleading (cli_args.rs:22-24)** —
  the old "Required when a target is `pnpm@<rev>`" was inaccurate
  because the orchestrator falls back to `--repository`, which is
  exactly what you want for the pnpm/pnpm monorepo case (both
  pacquet and pnpm live in the same repo). Rewrote as: "Only set
  this if pnpm and pacquet live in separate clones; defaults to
  `--repository`, which is correct for the `pnpm/pnpm` monorepo."
  No clap-level constraint added — the contextual rule ("required
  iff `--repository` isn't a pnpm checkout") can't be expressed in
  `required_if_eq` / `requires`, and `ensure_pnpm_git_repo` already
  panics with an actionable message when the fallback isn't a pnpm
  checkout.

- **`ensure_pnpm_git_repo` docstring (verify.rs:31-37)** — the
  "standalone pnpm clone" phrasing was confusing: pnpm doesn't ship
  a layout where its source lives at the repo root, so "standalone"
  is misleading. The function actually recognizes the pnpm/pnpm
  monorepo via either `pnpm/package.json` (CLI manifest) or
  top-level `pnpm-workspace.yaml`. Reworded to say so explicitly
  and to call out that the no-`pnpm/`-subdir shape isn't supported
  because pnpm doesn't have one.

Verified `cargo clippy --locked --workspace --all-targets -- -D warnings`,
`cargo fmt --all -- --check`, and `cargo test -p pacquet-integrated-benchmark`
(5/5 pass) clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

KSX suggested rewriting the imperative `if … { Some(…) } else { None }`
as a functional chain. The exact suggested form
(`.then_some(()).map(|()| f()).map(String::from)`) doesn't compile under
the existing `Text: Into<String>` bound — `String::from(t)` requires the
inverse `String: From<T>`, which Rust's trait solver doesn't infer from
`T: Into<String>` despite the blanket. The fix that keeps the bound
intact is `.map(Into::into)` (or, more compactly, fold the whole
gate-on-bool into `Option::then`):

    self.lockfile_enabled().then(|| load_lockfile().into())

That's the idiomatic Rust for "produce `Some(value)` iff bool", drops
the `.then_some(()).map(|()| …)` two-step destructure pattern that's
unusual, and lines up with the original's single-expression shape.

Verified `cargo clippy --locked --workspace --all-targets -- -D warnings`,
`cargo fmt --all -- --check`, and `cargo test -p pacquet-integrated-benchmark`
(5/5 pass) clean.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5

Followup to 91712f4, which changed the per-revision build step from
`pnpm run compile` to `pnpm run compile-only` (to drop the redundant
`update-manifests` pass). The README still said `pnpm run compile`,
which Copilot flagged on PR #11741. Update the step description and
add a one-sentence note explaining why `compile-only` (not the full
`compile`) is the right level.

https://claude.ai/code/session_01Ya2xTvnu5ycwqux8tfPbn5