Keep pnpm clean from removing pnpm-lock.yaml just because the workspace config sets lockfile: true. The lockfile cleanup now follows the command-line --lockfile option.
Co-authored-by: cyphercodes <cyphercodes@users.noreply.github.com>
Closes#346.
Adds a **Reporter / log events** section to `CODE_STYLE_GUIDE.md` plus a pointer in `AGENTS.md`'s cardinal-rule section so the "match pnpm" mandate explicitly covers log emissions.
The section walks a porter through the full lifecycle:
- **Finding the upstream emit** — three call shapes to grep for in `pnpm/pnpm` (`globalLogger`, `logger.debug({ name: ... })`, `streamParser.write(...)`).
- **Channel mapping** — `LogEvent` is the channels pacquet currently emits, *not* pnpm's full set; the convention for adding a new variant when an upstream channel is missing (mirror the TS shape field-for-field, add a wire-shape unit test) lives in the section. Notes the type-vs-emit-site permalink distinction: single-site channels link both, multi-site channels (`Stage`, `Progress`) only pin the type.
- **Threading the reporter** — `<R: Reporter>` generic on every fn that emits, turbofish at the production entry point, install-scoped state via `&AtomicU8`-style parameters where dedup matters (`link_file::log_method_once` cited as the example).
- **Where to put the emit** — match the upstream call site's position relative to side effects; cite the upstream permalink at a pinned SHA in the code comment.
- **Testing** — recording-fake reporter (unit-struct + `static Mutex<Vec<LogEvent>>` declared inside the test fn); sequence assertion via `matches!`. Verify the test catches the regression by temporarily commenting out the emit.
- **What not to do** — don't reformat upstream messages, don't invent channels, don't change emit granularity (cites `pnpm:fetching-progress in_progress`'s `Content-Length` + `>= 5 MB` + 500ms gate as the worked counter-example, with the pinned upstream permalink to `remoteTarballFetcher.ts`).
- **Worked example** — `pnpm:summary` in `Install::run`. Shows the upstream permalink, the pacquet emit with code comment, and the test that pins the sequence.
The doc is self-contained — no GitHub-issue back-references in the body — so a porter reads it linearly without context-switching.
Pure docs, no code changes.
Backfills two channels from #347 in one PR — both are "progress"
shaped so they share the tarball-side reporter threading.
### `pnpm:progress` — per-package status transitions
Mirrors pnpm's emit at
[`installing/package-requester/src/packageRequester.ts:435`](https://github.com/pnpm/pnpm/blob/086c5e91e8/installing/package-requester/src/packageRequester.ts#L435).
Four `status` values fire per package:
- **`resolved`** — emitted before the tarball download in both
lockfile paths. In the no-lockfile path,
`install_package_from_registry::run` emits once the registry has
picked a version. In the frozen-lockfile path,
`create_virtual_store::run`'s warm batch and per-snapshot
`install_package_by_snapshot::run`'s entry emit on each snapshot
(the lockfile *is* the resolution).
- **`fetched`** — emitted from `fetch_and_extract_with_retry` on
the successful `Ok` branch. Failed attempts that retry don't fire
it; cache-hit early returns don't fire it.
- **`found_in_store`** — emitted from `run_without_mem_cache`'s
cache-hit early returns (prefetched-cas + `load_cached_cas_paths`),
from `create_virtual_store::run`'s warm batch where prefetch
already settled the bytes, and from `run_with_mem_cache`'s
in-process dedup hits (so the second requester of a shared URL
also ticks the per-package counter).
- **`imported`** — emitted after `create_cas_files` returns `Ok` in
both `create_virtual_dir_by_snapshot::run` and
`install_package_from_registry::install_package_version`. `method`
is the optimistic wire-mapped configured method
(`Auto`/`CloneOrCopy` → `clone`); pacquet doesn't surface the
per-package resolved method past `link_file`'s install-scoped
atomic. Tracked under #347 for refinement once that becomes
per-package observable.
### `pnpm:fetching-progress` — per-tarball download progress
Mirrors pnpm's emit at
[`installing/package-requester/src/packageRequester.ts:560`](https://github.com/pnpm/pnpm/blob/086c5e91e8/installing/package-requester/src/packageRequester.ts#L560)
and
[`fetching/tarball-fetcher/src/remoteTarballFetcher.ts:143`](https://github.com/pnpm/pnpm/blob/086c5e91e8/fetching/tarball-fetcher/src/remoteTarballFetcher.ts#L143).
Two `status` values:
- **`started`** — fires exactly once per HTTP attempt, including
attempts that fail before the response head arrives (DNS /
connect / timeout) so retried attempts stay visible in the
reporter. `size` is the response's `Content-Length` when a
response head arrives, and JSON `null` (i.e. `None`) when there
is no response, or when the response is chunked / unknown-length.
pnpm's reporter checks `size != null` before rendering a percent
gauge.
- **`in_progress`** — gated and throttled to match pnpm exactly:
- Skip entirely when `Content-Length` is unknown (chunked /
streaming) or the tarball is smaller than `BIG_TARBALL_SIZE = 5 MB`.
Most npm packages are well under this; per-byte progress for
tiny tarballs floods the JS reporter with values that would
render as 100% before any UI tick can show them.
- Throttle to 500ms (matches `lodash.throttle(opts.onProgress, 500)`).
- Trailing edge: a final emit after the body finishes if the
last in-flight `downloaded` value differs from the last
reported one. Matches `lodash.throttle`'s default
`{leading: true, trailing: true}` so the consumer sees the
actual end-of-download byte count.
### `run_with_mem_cache` deadlock fix
Pre-existing bug exposed by review: when the *owning* requester's
`run_without_mem_cache` errored, the function returned without
flipping the cache slot to `Available` or notifying waiters.
Concurrent siblings parked on `notify.notified().await` forever
and the install hung — a single 404 could deadlock every later
consumer of the same URL.
Fixed by adding `CacheValue::Failed`. The owner uses `match` on
the result; on `Err` it sets `Failed`, removes the entry from
`mem_cache` (so a fresh request can retry without inheriting the
failure), and notifies waiters. Waiters check for `Failed` on
wake and surface a new `TarballError::SiblingFetchFailed { url }`
instead of the `unreachable!` the previous code would have hit.
### Threading
`pacquet-tarball` gains a `pacquet-reporter` dependency.
`DownloadTarballToStore` carries a new `requester: &'a str` field
(the install root, same value as the `prefix` in
`pacquet_reporter::StageLog`). `<R: Reporter>` is threaded through
`run_with_mem_cache`, `run_without_mem_cache`,
`fetch_and_extract_with_retry`, and `fetch_and_extract_once`. The
generic monomorphises away — zero runtime cost. `requester` and
`<R>` cascade up through `InstallPackageBySnapshot`,
`InstallPackageFromRegistry`, `CreateVirtualDirBySnapshot`,
`CreateVirtualStore`, `InstallFrozenLockfile`,
`InstallWithoutLockfile`, and `Install::run`.
* refactor: extract all inline test modules into dedicated files
Move every inline `#[cfg(test)] mod tests { ... }` block across the
workspace into external `tests.rs` files and update CODE_STYLE_GUIDE.md
to require external test files unconditionally (no threshold).
28 files extracted across 8 crates: store-dir, package-manager, fs,
package-manifest, lockfile, npmrc, cli, registry.
Closes#358.
* fix(package-manager): relocate snapshot to match new test file location
When `install::tests::should_install_dependencies` moved from the
inline `mod tests` in `install.rs` to the external
`install/tests.rs`, insta's snapshot lookup directory shifted from
`crates/package-manager/src/snapshots/` to
`crates/package-manager/src/install/snapshots/`. CI failed on every
platform because no snapshot existed at the new location. Move the
existing `.snap` file to its new home.
* test: address review feedback on extracted test files
Apply the actionable review comments on the test-extraction PR:
- save_lockfile: replace hardcoded `/nonexistent-pacquet-dir/...`
with a tempdir-based missing path so the invalid-path test is
deterministic across environments; add diagnostic message to the
`matches!` assertion.
- snapshot_dep_ref: convert `looks_like_alias_rules` to a
table-driven `assert_eq!` form with per-case logging.
- npmrc/workspace_yaml: compare `StoreDir` values directly instead
of `display().to_string()`, which produces backslashes on Windows
and would fail the `apply_overrides_npmrc_defaults` test there.
- package-manager/install: clear the static `EVENTS` mutex at the
start of `install_emits_pnpm_event_sequence` so retries don't
inherit stale events; add per-path logging before the
`is_symlink_or_junction` / `exists` / `is_dir` assertions.
- package-manager/link_file: same `EVENTS.clear()` defense in
`log_method_once_emits_first_call_per_method_only`; add
diagnostics before nlink and symlink-metadata assertions.
- package-manager/build_snapshot: log the relevant fields before
the `matches!` and `is_none` assertions.
- package-manager/install_package_from_registry: log
`virtual_store_path` state before the `is_dir` assertion.
- package-manifest: assert the actual return value of
`manifest.script("test", false)` (was `Some("echo")`) and
`manifest.script("invalid", true)` (was `None`); add `dbg!` /
`eprintln!` diagnostics around the bare `assert!` checks per
CODE_STYLE_GUIDE.md.
- store-dir/check_pkg_files_integrity: add `dbg!(&result)` and
path-existence logging before each non-`assert_eq!` assertion
across all 11 tests.
---------
Co-authored-by: Zoltan Kochan <z@kochan.io>
* feat(audit): add registry signature verification
* chore: add registry signature terms to cspell
* chore: sort cspell registry terms
* refactor(audit): use repo concurrency and error helpers
* refactor(audit): use registry fetch helper for signatures
* refactor(audit): share audit command context
* fix(audit): respect scoped registries for signatures
* fix(audit): handle missing signature metadata gracefully
* docs(audit): document signature verification
* test(audit): avoid signature spellcheck false positives
* chore(audit): add scoped registry project reference
* refactor(audit): clarify signature verification fetching
* style(audit): align signature verifier formatting
* fix(audit): validate signature metadata shape and report cleanly
* fix(audit): handle crypto.verify throws on malformed registry keys
A registry returning malformed PEM key material made verifier.verify throw
synchronously, rejecting the Promise.all and crashing the whole audit run.
Treat any verify failure as an invalid signature for that single package.
* refactor(audit): extract parseJsonResponse helper
Both fetchRegistryKeys and fetchPackument repeated the same JSON.parse +
PnpmError wrapping pattern. Collapse into a single helper.
* refactor(audit): split signature verification into its own package
Move verifySignatures from @pnpm/deps.compliance.audit into a new
@pnpm/deps.compliance.signatures package. Vulnerability auditing and
signature verification are conceptually distinct trust subsystems, and
sigstore provenance verification is in scope for a future change — keeping
all signature work in its own package avoids growing the audit module into
two unrelated concerns.
* docs(audit): drop signature verification section
The signature verification implementation moved to
@pnpm/deps.compliance.signatures; that package's README documents the
behavior. The audit package no longer needs to mention it.
* refactor(signatures): move package to deps/security
Place the new signature verification package under deps/security/ rather
than deps/compliance/. Compliance is a fuzzy fit for tamper detection;
security is the right home, and sigstore provenance verification (future
scope) will live alongside it. Existing audit/license/sbom packages stay
where they are — this only changes where the new package lands.
---------
Co-authored-by: Colin Fristoe <47856231+ctfristoe@users.noreply.github.com>
Co-authored-by: Zoltan Kochan <z@kochan.io>
Third channel from the backfill (#347), and a course-correction on a
fourth.
**`pnpm:package-import-method`.** Adds the `PackageImportMethod`
variant to `LogEvent` and emits it from `link_file`'s
`log_method_once` helper, mirroring pnpm's emit at
[`fs/indexed-pkg-importer/src/index.ts:32`](https://github.com/pnpm/pnpm/blob/086c5e91e8/fs/indexed-pkg-importer/src/index.ts#L32).
The structured event fires alongside the existing `tracing::info!`
diagnostic the first time each resolved method (`clone` / `hardlink`
/ `copy`) actually *succeeds*, so for `Auto` and `CloneOrCopy` the
wire value is the post-fallback method rather than the optimistic
configured one.
The wire-format enum is `clone | hardlink | copy` exactly. Pacquet's
`Npmrc::package_import_method` carries `Auto` and `CloneOrCopy` on
top of those, but those are dispatched-on by the auto-importer's
fallback chain, not emitted, so the wire enum stays in lockstep with
pnpm's. Threading the reporter through the link path required adding
`<R: Reporter>` to `link_file`, `auto_link`, `clone_or_copy_link`,
and the call chain above (`create_cas_files`,
`create_virtual_dir_by_snapshot`, `install_package_by_snapshot`,
`create_virtual_store`, `install_package_from_registry`,
`install_frozen_lockfile`, `install_without_lockfile`). The reporter
type monomorphises away — zero runtime cost.
**Install-scoped dedupe.** The bitfield atomic that gates
once-per-method emission is install-scoped — created in `Install::run`
and threaded down as a `&'a AtomicU8` field on each install-layer
struct, matching the existing `verified_files_cache` pattern.
Mirroring upstream pnpm's per-importer closure capture (a process-
static would suppress emits on every install after the first in the
same process — fine for the one-shot CLI today but a footgun for
tests and any future embedded use).
**Emit-only-on-success.** All four `fs::copy`-bearing branches
(explicit `Copy`, explicit `Hardlink`'s EXDEV fallback to copy, and
the copy fallbacks in both `auto_link` and `clone_or_copy_link`)
emit *after* the copy returns `Ok`, so a copy failure can't record a
method that never succeeded.
**Deferral on `pnpm:stage resolution_started/done`.** Investigated
this channel before picking the current one and concluded it can't
land cleanly today: pacquet's no-lockfile flow interleaves resolution
and import per dependency, so emitting `resolution_done` after import
work has already happened would break the JS reporter's per-package
state. Frozen-lockfile installs (the primary path today) don't need
the events — pnpm itself skips them in `deps-restorer`. Recorded the
deferral on #347 alongside the dependency on a phase-separated
resolution pass.
* feat(reporter): emit pnpm:summary at install end
Adds the `Summary` variant to `LogEvent` and emits it from
`Install::run` immediately after `pnpm:stage importing_done`. Mirrors
pnpm's emit at
https://github.com/pnpm/pnpm/blob/086c5e91e8/installing/deps-installer/src/install/index.ts#L1663.
The payload carries only `prefix`; the reporter combines this with the
accumulated `pnpm:root` events (still TODO under #347) to render the
final "+N -M" diff block.
Test: extends the install integration test (renamed to
`install_emits_pnpm_event_sequence` to match the growing scope) to
assert the four-event success sequence (`Context`, `ImportingStarted`,
`ImportingDone`, `Summary`) and spot-checks summary's `prefix`.
Verified the test catches the regression by temporarily commenting
out the emit and confirming it failed with a Summary-shaped gap.
Refs #347. Engine: #345. Previous channel: #354.
* docs(reporter): clarify SummaryLog payload comment
Per review feedback on #355: the original wording said the `pnpm:summary`
payload "is just `prefix`", which omitted the `level` field that's
also part of the serialized record. `level` is the bunyan-envelope
severity carried by every channel, not channel-specific to summary;
say so explicitly so a reader doesn't try to "fix" the struct based on
the comment.
Same fix applied to the wire-shape test's doc comment.
Adds the `Context` variant to `LogEvent` (camelCase serde so the wire
shape matches `@pnpm/cli.default-reporter`'s expectations) and emits it
once from `Install::run`, immediately before `pnpm:stage importing_started`.
Mirrors pnpm's emit at
https://github.com/pnpm/pnpm/blob/086c5e91e8/installing/context/src/index.ts#L196.
`currentLockfileExists` is hard-coded `false` for now because pacquet
doesn't read or write `node_modules/.pnpm/lock.yaml` yet. The site
carries a TODO so the value gets flipped when that path lands.
Test: extends the existing recording-fake DI test (per #339) to assert
the full event sequence (`Context`, `ImportingStarted`, `ImportingDone`)
and spot-checks the context payload's directory fields. Verified the
test catches the regression by temporarily commenting out the emit and
confirming a fail; restored.
Refs #347. Engine: #345.
#364's `use super::*;` removal stripped the test module's view of
`LogEvent`, `Reporter`, `Stage`, `StageLog`, and `Lockfile`, but the
test bodies still reference them — so `cargo nextest run -p
pacquet-package-manager` failed to compile on `main`.
Promote the inline `use pacquet_lockfile::Lockfile` from
`frozen_lockfile_flag_overrides_config_lockfile_false` to the module-
level `use` block, and add the missing reporter symbols alongside the
existing `SilentReporter` import. The two `Lockfile`-using tests now
share one import.
* ci(release): build artifacts on macos-latest to fix darwin-x64 signing
Cross-signing darwin Mach-O binaries on Linux with the saurik fork of
ldid produces an ad-hoc signature whose page hashes don't match the
post-postject layout for Node.js 25's chained fixups, leaving fixups
unapplied at load and crashing the binary in __cxx_global_var_init
(EXC_BAD_ACCESS at 0x3 — the unprocessed chain-entry tag).
Running the release on macos-latest lets pack-app's adHocSignMacBinary
use native codesign, which understands chained fixups. Drops the entire
ldid build step.
* ci(release): document why release runs on macos-latest
* feat(reporter): add NDJSON reporting engine with pnpm:stage smoke test
Introduces `pacquet-reporter` carrying the typed `LogEvent` enum, the
no-`self` `Reporter` capability trait per #339, and `NdjsonReporter` /
`SilentReporter` implementations. Adds the `--reporter=ndjson|silent`
flag and threads a `R: Reporter` generic through `Install::run`,
emitting `pnpm:stage` `importing_started` / `importing_done` to
bracket the install — one channel wired end-to-end as a smoke test
for the schema-mirror approach.
The wire format mirrors what `@pnpm/core-loggers` defines and
`@pnpm/cli.default-reporter` consumes under `pnpm --reporter=ndjson`.
Engine only. The JS-reporter spawn-and-pipe (`--reporter=default`),
the backfill of missing log events in already-ported code (#347), and
the porting convention in AGENTS.md (#346) land in follow-up PRs. The
default reporter is `silent` until the spawn-and-pipe exists, so user-
facing output is unchanged.
Refs #345. Design: #344.
* docs(package-manager): name pnpm's `lockfileDir` and link findWorkspaceDir
Upstream emits stage events with `prefix: lockfileDir` — the directory
holding `pnpm-lock.yaml`, which collapses to the workspace root when a
workspace is present. The previous comment described the *result* in
pacquet's no-workspace world but didn't name the upstream concept or
point at the helper to mirror once workspaces land. Replacing the
comment with a TODO that links `findWorkspaceDir` makes the migration
path explicit so the next person editing this site doesn't have to
re-derive it.
No behavior change.
* fix(cli): make --reporter accepted on either side of the subcommand
Adding `global = true` to the clap attribute lets users write either
`pacquet --reporter=ndjson install` or `pacquet install --reporter=ndjson`,
matching pnpm. Without it the flag is parsed by the top-level `CliArgs`
only, so a sub-command-side use ("pacquet install --reporter=...")
produced "unexpected argument '--reporter' found".
* refactor(reporter,package-manager): apply review fixes from #349
Address feedback on the reporter PR.
reporter:
- Drop the dependency-injection issue link (#339) from `Reporter`,
the recording-fake test, and the module-level docs; convention is
visible from the code.
- Wrap `bunyan` and `bole` references as markdown links.
- Demote `Envelope`'s `#[serde(flatten)]` rationale to a regular `//`
comment so private impl detail isn't part of `///` rustdoc.
- Introduce `pub trait EnvVar` + `pub struct RealEnvVar` (reporter-
local capability trait) and split `hostname` into a generic
`resolve_hostname<E>` plus a non-generic cached production wrapper.
Add four unit tests covering `HOSTNAME`, `COMPUTERNAME` fallback,
precedence, and the unset case.
- Switch the wire-shape test to `pipe-trait`; add `pipe-trait` to
dev-dependencies.
- Fix intra-link rule violation in the `SilentReporter` test doc.
package-manager:
- Tighten the install `prefix` extraction to fail loudly on
non-UTF-8 / parentless manifest paths instead of silently emitting
an empty string. Workspace-aware resolution tracked at #357.
- Replace `concat!` YAML literals in two tests with
`text_block_macros::text_block!`; add as dev-dependency.
- Convert bare-prose identifiers in
`install_emits_stage_events_bracketing_the_run` to intra-doc
links; drop the recording-fake DI prose.
Long inline `mod tests` extraction tracked at #358.
* fix(reporter): drop private-item link from `EnvVar` public doc
`EnvVar` is `pub` but `resolve_hostname` is private; rustdoc rejects
the intra-link with `-D rustdoc::private-intra-doc-links` (the rule
added to `CODE_STYLE_GUIDE.md` in #351). Rephrase the doc to talk
about "env-dependent helpers" generally without naming the private
function.
* refactor(reporter): switch hostname to gethostname syscall + DI capability
Address the second round of review feedback on #349.
reporter:
- Replace env-var-based hostname (HOSTNAME / COMPUTERNAME) with the
real gethostname(2) syscall via the `gethostname` crate. The
env-var approach was unsound: shells set `HOST` (zsh) or
`HOSTNAME` (bash) as private variables that don't propagate to
spawned binaries, so a packaged `pacquet` would have observed an
empty hostname in most production environments.
- Introduce `pub trait GetHostName` + `pub struct RealApi`. Real
resolution lives in `RealApi::get_host_name`. Tests can supply
their own impl.
- Cache the production value in `static HOSTNAME: LazyLock<String>
= LazyLock::new(RealApi::get_host_name)`. The wire emit path
reads `&HOSTNAME` directly; the previous `hostname()` wrapper is
no longer needed.
- Drop the now-redundant `EnvVar` trait, `RealEnvVar`,
`resolve_hostname`, and the four env-resolution unit tests (those
scenarios no longer exist after the syscall switch).
reporter test layout:
- Move the tests module into its own file at
`crates/reporter/src/tests.rs` per the unit-test-layout rule in
`CODE_STYLE_GUIDE.md`. Drop `use super::*` in favor of explicit
merged imports of just the items the tests touch.
- Two new tests: `get_host_name_capability_is_mockable` (proves the
trait dispatch works for fakes) and
`real_api_returns_a_non_empty_host_name` (smoke-checks the real
syscall path).
Documentation style:
- Restructure the workspace-prefix and stage-bracketing comments to
drop em dashes per `CONTRIBUTING.md`'s writing-style guidance and
to render `bunyan` as a markdown link.
- Fix the `@pnpm/core-loggers` link to point at the package
directory rather than its `src` subdirectory.
Imports:
- Replace seven occurrences of `pacquet_reporter::SilentReporter`
qualified at the call site with a single `use pacquet_reporter::
SilentReporter` in the test module.
* feat(fs.graceful-fs): expose promisified chmod and unlink
So callers can perform mode changes and removals through the same
EMFILE/ENFILE-queueing layer as the other operations.
* chore: remove ENFILE word to satisfy cspell
* test: make checkPlatform negation tests platform-independent
The two multi-valued supportedArchitectures tests added in #11375 used
'current' alongside a value that the negation in the wanted platform
matched on some hosts (e.g. ['linux', 'current'] on Windows expands to
['linux', 'win32'], which is correctly rejected by ['!win32']). Replace
'current' with fixed second values so the multi-value code path is still
exercised without depending on process.platform / process.arch.
* test: mock process.platform / process.arch instead of avoiding 'current'
Restores the more realistic scenario from #11375 where supportedArchitectures
mixes a fixed value with 'current'. Mock process.platform / process.arch
explicitly per test so the result no longer depends on the host CI runner.
#11399 fixed the fs.cpSync call in pnpm/artifacts/exe/scripts/build-artifacts.ts,
which controls the dist/ shipped inside the npm-published @pnpm/exe package.
But the GitHub release tarballs (pnpm-{darwin,linux}-{x64,arm64}.tar.gz) are
produced by a different script — __utils__/scripts/src/copy-artifacts.ts, run
via 'pn copy-artifacts' in the release workflow. That script has the same
fs.cpSync(...) call without verbatimSymlinks: true, so the broken absolute
symlinks under dist/node_modules/.bin/ pointing at /home/runner/work/pnpm/
pnpm/... still made it into the v11.0.2 GitHub release tarballs.
Apply the same one-line fix to that script so the next release ships clean
relative symlinks.
Follow-up to #11398.
🤖 Generated with [Amp](https://ampcode.com)
Amp-Thread-ID: https://ampcode.com/threads/T-019dda79-b947-742f-8711-b6f83bcda9ff
Co-authored-by: Amp <amp@ampcode.com>
* feat(fs.graceful-fs): expose promisified chmod and unlink
So callers can perform mode changes and removals through the same
EMFILE/ENFILE-queueing layer as the other operations.
* chore: remove ENFILE word to satisfy cspell
* test: make checkPlatform negation tests platform-independent
The two multi-valued supportedArchitectures tests added in #11375 used
'current' alongside a value that the negation in the wanted platform
matched on some hosts (e.g. ['linux', 'current'] on Windows expands to
['linux', 'win32'], which is correctly rejected by ['!win32']). Replace
'current' with fixed second values so the multi-value code path is still
exercised without depending on process.platform / process.arch.
* test: mock process.platform / process.arch instead of avoiding 'current'
Restores the more realistic scenario from #11375 where supportedArchitectures
mixes a fixed value with 'current'. Mock process.platform / process.arch
explicitly per test so the result no longer depends on the host CI runner.
* docs(style): warn against referencing private items from public docs
* docs(style): rewrite doc-comment rule to match writing style
* docs(style): correct doc-comment visibility framing
* docs(style): drop misleading comment on private helper in example
* docs(style): clarify that the rule targets more-private references
---------
Co-authored-by: Claude <noreply@anthropic.com>
* docs(plans/test-porting): allow tests beyond pnpm's coverage
Clarify that the porting plan is a floor, not a ceiling: pacquet-only
tests beyond what upstream pnpm has are welcome.
* docs(plans/test-porting): clarify extra tests are encouraged, not required
Reword the "more tests than pnpm" note so it's clear that pacquet-only
coverage is a plus but not an obligation.
* docs(plans/test-porting): align extra-tests note with writing style
Remove the em dash and contractions, and split the clauses into
standalone sentences to match the style guidance in CONTRIBUTING.md.
* docs(plans/test-porting): drop redundant clause about not inventing coverage
The earlier sentences already state that extra coverage is not required,
so the trailing clause repeats the point without adding new guidance.
* docs(plans/test-porting): use "just to" for the symmetry clause
"Just to keep" conveys that symmetry alone is the disallowed reason,
which "in order to" leaves ambiguous.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* docs(style): require intra-doc links for identifier mentions
Add a CODE_STYLE_GUIDE.md section telling contributors (and AI agents)
to turn identifier mentions inside `///` and `//!` doc comments into
rustdoc intra-doc links when the identifier is reachable from scope.
Intra-links give readers one-click navigation and let rustdoc surface
broken-link warnings when items are renamed or removed.
* docs(style): tighten intra-doc link guidance
Drop the redundant `//!` callout — saying "doc comment (`///` or `//!`)"
once at the top is enough; the second example only restated the rule.
---------
Co-authored-by: Claude <noreply@anthropic.com>
#11399 fixed the fs.cpSync call in pnpm/artifacts/exe/scripts/build-artifacts.ts,
which controls the dist/ shipped inside the npm-published @pnpm/exe package.
But the GitHub release tarballs (pnpm-{darwin,linux}-{x64,arm64}.tar.gz) are
produced by a different script — __utils__/scripts/src/copy-artifacts.ts, run
via 'pn copy-artifacts' in the release workflow. That script has the same
fs.cpSync(...) call without verbatimSymlinks: true, so the broken absolute
symlinks under dist/node_modules/.bin/ pointing at /home/runner/work/pnpm/
pnpm/... still made it into the v11.0.2 GitHub release tarballs.
Apply the same one-line fix to that script so the next release ships clean
relative symlinks.
Follow-up to #11398.
🤖 Generated with [Amp](https://ampcode.com)
Amp-Thread-ID: https://ampcode.com/threads/T-019dda79-b947-742f-8711-b6f83bcda9ff
Co-authored-by: Amp <amp@ampcode.com>
* fix(global): avoid doubled modulesDir when approving builds in global add
The global add → approve-builds flow used to forward an absolute
`modulesDir` (`<installDir>/node_modules`) into the install run by
`approve-builds`. The install layer treats `modulesDir` as a path
relative to `lockfileDir` and joins it again — producing a doubled
path on Windows because `path.join` does not collapse an embedded
absolute path. The hoist step then failed with `ENOENT` while trying
to symlink under `<installDir>\<installDir>\node_modules\.pnpm\...`.
Closes#11403.
* test: type test fixtures correctly
* fix(install): tolerate absolute modulesDir in headless install context
Replace the prior unit test (which only checked the call shape) with an
integration test that exercises `install()` with an absolute `modulesDir`
through both the regular and frozen-lockfile paths — the failure mode the
global add → approve-builds chain originally hit on Windows.
`headlessInstall` and `readProjectsContext` now resolve `modulesDir` via
`pathAbsolute` instead of `path.join(lockfileDir, modulesDir)`, so an
absolute value no longer produces a doubled prefix. The
`promptApproveGlobalBuilds` change from the previous commit is retained
as the contract-level fix.
* test: add e2e test driving the pnpm CLI with --modules-dir=<abs>
Replace the programmatic install() regression test with an e2e test in
pnpm/test/install/absoluteModulesDir.ts that runs the bundled pnpm
binary with `pnpm install --modules-dir=<abs>` (regular and frozen).
This is the closest CLI-level reproduction of the doubled-prefix path
bug from #11403 — the bug fired specifically in the headless install
path that --frozen-lockfile triggers.
* test(global): drive add -g + approve-builds chain end-to-end
Add an e2e test that runs the bundled pnpm CLI through the full
`pnpm add -g <pkg-with-build>` → approve-builds → install chain that
produced the doubled-prefix `ENOENT` in #11403.
The chain only fires when `process.stdin.isTTY` is true, which CI
subprocesses don't satisfy. Add a test-only env var
`PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS` that bypasses the TTY guard in
`promptApproveGlobalBuilds` and forwards `all: true` so `approve-builds`
skips its multiselect and confirm prompts. The post-approval install
then runs the same code path a real user hit, and the test asserts the
build artifacts ended up in the global install dir.
Replaces the narrower `--modules-dir=<abs>` regression test, which
only exercised the install layer and not the global-add flow that
originally surfaced the bug.
* test: enable global add -g + approve-builds e2e test on Windows
- Switch to @pnpm.e2e/install-script-example which is cross-platform.
- Use pathAbsolute for modulesDir to prevent doubled path bugs on Windows.
- Add path-absolute dependency to affected packages.
The package-manager handling block in main.ts was guarded by
`!isExecutedByCorepack()`, which skipped the entire block — including
syncEnvLockfile and checkPackageManager — when COREPACK_ROOT was set.
The lockfile's packageManagerDependencies entry would drift stale, and
devEngines.packageManager mismatches were silently ignored.
Move the corepack guard onto switchCliVersion only (corepack owns
version selection), so that checkPackageManager and syncEnvLockfile run
regardless of how pnpm was invoked. syncEnvLockfile self-gates via
shouldPersistLockfile, so projects that only use the legacy
packageManager field still won't have the lockfile rewritten.
When the check fires under corepack, augment the message and hint to
explain that pnpm cannot switch versions under corepack and point to
the two ways out (align packageManager with devEngines.packageManager,
or invoke pnpm directly).
Closes#11397
* fix(global): avoid doubled modulesDir when approving builds in global add
The global add → approve-builds flow used to forward an absolute
`modulesDir` (`<installDir>/node_modules`) into the install run by
`approve-builds`. The install layer treats `modulesDir` as a path
relative to `lockfileDir` and joins it again — producing a doubled
path on Windows because `path.join` does not collapse an embedded
absolute path. The hoist step then failed with `ENOENT` while trying
to symlink under `<installDir>\<installDir>\node_modules\.pnpm\...`.
Closes#11403.
* test: type test fixtures correctly
* fix(install): tolerate absolute modulesDir in headless install context
Replace the prior unit test (which only checked the call shape) with an
integration test that exercises `install()` with an absolute `modulesDir`
through both the regular and frozen-lockfile paths — the failure mode the
global add → approve-builds chain originally hit on Windows.
`headlessInstall` and `readProjectsContext` now resolve `modulesDir` via
`pathAbsolute` instead of `path.join(lockfileDir, modulesDir)`, so an
absolute value no longer produces a doubled prefix. The
`promptApproveGlobalBuilds` change from the previous commit is retained
as the contract-level fix.
* test: add e2e test driving the pnpm CLI with --modules-dir=<abs>
Replace the programmatic install() regression test with an e2e test in
pnpm/test/install/absoluteModulesDir.ts that runs the bundled pnpm
binary with `pnpm install --modules-dir=<abs>` (regular and frozen).
This is the closest CLI-level reproduction of the doubled-prefix path
bug from #11403 — the bug fired specifically in the headless install
path that --frozen-lockfile triggers.
* test(global): drive add -g + approve-builds chain end-to-end
Add an e2e test that runs the bundled pnpm CLI through the full
`pnpm add -g <pkg-with-build>` → approve-builds → install chain that
produced the doubled-prefix `ENOENT` in #11403.
The chain only fires when `process.stdin.isTTY` is true, which CI
subprocesses don't satisfy. Add a test-only env var
`PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS` that bypasses the TTY guard in
`promptApproveGlobalBuilds` and forwards `all: true` so `approve-builds`
skips its multiselect and confirm prompts. The post-approval install
then runs the same code path a real user hit, and the test asserts the
build artifacts ended up in the global install dir.
Replaces the narrower `--modules-dir=<abs>` regression test, which
only exercised the install layer and not the global-add flow that
originally surfaced the bug.
* test: enable global add -g + approve-builds e2e test on Windows
- Switch to @pnpm.e2e/install-script-example which is cross-platform.
- Use pathAbsolute for modulesDir to prevent doubled path bugs on Windows.
- Add path-absolute dependency to affected packages.
The package-manager handling block in main.ts was guarded by
`!isExecutedByCorepack()`, which skipped the entire block — including
syncEnvLockfile and checkPackageManager — when COREPACK_ROOT was set.
The lockfile's packageManagerDependencies entry would drift stale, and
devEngines.packageManager mismatches were silently ignored.
Move the corepack guard onto switchCliVersion only (corepack owns
version selection), so that checkPackageManager and syncEnvLockfile run
regardless of how pnpm was invoked. syncEnvLockfile self-gates via
shouldPersistLockfile, so projects that only use the legacy
packageManager field still won't have the lockfile rewritten.
When the check fires under corepack, augment the message and hint to
explain that pnpm cannot switch versions under corepack and point to
the two ways out (align packageManager with devEngines.packageManager,
or invoke pnpm directly).
Closes#11397
* docs(plans): note pnpm/test integration coverage in test porting plan
The TEST_PORTING.md plan cited specific pnpm/test/... files inline but
never explained that pnpm/test/ is a parallel CLI-level integration test
tree covering nearly every feature in pnpm. Agents porting features kept
auditing only the per-package test/ directories and missing CLI-level
coverage. Add an explicit instruction in the intro to audit pnpm/test/
alongside the per-package tests for every feature.
https://claude.ai/code/session_01EXbEDL1UcPmvg1jZeT6XRK
* docs(plans): drop em dashes in TEST_PORTING per writing style
CONTRIBUTING.md flags em dashes as a symptom of loose phrasing and asks
that prose be restructured rather than have the em dash swapped for
another punctuation mark. Restructure the two prose paragraphs added in
the previous commit so each clause stands as its own complete sentence.
The em dash on line 303 is inside a literal upstream TypeScript test
description and is left alone.
https://claude.ai/code/session_01EXbEDL1UcPmvg1jZeT6XRK
---------
Co-authored-by: Claude <noreply@anthropic.com>
The test-porting plan in plans/TEST_PORTING.md tracks the upstream pnpm
tests scheduled for porting and the conventions expected of the ports,
but neither AGENTS.md nor CONTRIBUTING.md pointed at it. Add references
from both so contributors find the plan when working on ported tests.
https://claude.ai/code/session_01TESBtd59bbwcaNu6jsCaiU
Co-authored-by: Claude <noreply@anthropic.com>
