feat(lockfile): emit pnpmfileChecksum in pacquet lockfiles by zkochan · Pull Request #12280 · pnpm/pnpm

zkochan · 2026-06-09T05:26:16Z

PR Summary by Qodo

Emit pnpmfileChecksum in pacquet-generated pnpm-lock.yaml (pnpm parity)
✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Walkthroughs

User Description

What

Ports pnpm's calculatePnpmfileChecksum so a pacquet install records the pnpmfileChecksum field in pnpm-lock.yaml when the project's .pnpmfile.{cjs,mjs} exports a hooks object.

Addresses item 5 of #12266 (lockfile-parity gaps vs the pnpm CLI). Previously pacquet ran pnpmfile hooks but never wrote the checksum, so its lockfile diverged from pnpm whenever a project had a pnpmfile.

How

crypto-hash — ports the two missing helpers from @pnpm/crypto.hash: create_hash (sha256-<base64>) and create_hash_from_file (UTF-8 read + CRLF→LF normalization, the cross-platform-stability rule pnpm relies on).
hooks — adds PnpmfileHooks::calculate_pnpmfile_checksum(). The checksum value is pure file hashing; only pnpm's entries.some(entry => entry.hooks != null) gate consults the evaluated module, answered by a new lightweight hasHooks query on the existing long-lived pnpmfile worker (already spawned during resolution). A pnpmfile that exports no hooks records no checksum, matching pnpm.
lockfile — adds the pnpmfile_checksum top-level field to Lockfile, serialized right after packageExtensionsChecksum per pnpm's ROOT_KEYS order (yaml_emit::ROOT_KEYS already listed it).
package-manager — threads the computed checksum through GraphToLockfileOptions → dependencies_graph_to_lockfile → both fresh-lockfile build paths; current_lockfile carries it over verbatim.

Verification

New unit tests (hooks gate + checksum value, CRLF normalization) and install-level tests (with-hooks emits / without-hooks omits).
Parity proof: create_hash of this monorepo's real .pnpmfile.cjs reproduces the committed pnpmfileChecksum: sha256-FyKtfMeHqu9zq1rXAr8P3zdA0NAXUioGdY0esg+SB5s= byte-for-byte.
cargo check --workspace --all-targets, targeted nextest suites, clippy -D warnings, cargo fmt --check, cargo doc -D warnings, and typos all pass.

Scope

This covers the write side (the issue's stated gap). The matching read side — frozen-install drift detection via getOutdatedLockfileSetting — is separate; freshness.rs still lists pnpmfileChecksum as a pending settings-checker variant and is not part of item 5.

Written by an agent (Claude Code, claude-opus-4-8).

AI Description

• Record pnpmfileChecksum in pnpm-lock.yaml when a project pnpmfile exports hooks.
• Add pnpm-compatible sha256- hashing with CRLF→LF normalization.
• Extend pnpmfile worker with a hasHooks query and add parity-focused tests.

Diagram

graph TD
  A["Fresh install"] -->|"calls"| B["Pnpmfile hooks"] -->|"hasHooks query"| C["Node worker"] -->|"hash pnpmfile"| D["crypto-hash"] -->|"set checksum"| E["Lockfile model"] -->|"emit"| F["yaml_emit"] -->|"writes"| G["pnpm-lock.yaml"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Always hash when pnpmfile exists (no hooks gate)

➕ Simpler implementation; no worker protocol change
➕ No need to consult the evaluated module
➖ Diverges from pnpm behavior (pnpm omits key when no hooks)
➖ Would introduce lockfile churn for projects with pnpmfile but no hooks export

2. Heuristic/static detection of hooks export (parse/regex)

➕ Avoids worker query/IPC round trip
➕ No need to load/execute the pnpmfile to decide inclusion
➖ Brittle across CJS/MJS patterns and dynamic exports
➖ Likely to disagree with pnpm for non-trivial pnpmfiles; parity risk is high

Recommendation: Keep the PR’s approach: query the already-running pnpmfile worker for the exact hooks-export gate (matching pnpm semantics), and compute the checksum purely from normalized file bytes. This preserves pnpm parity while keeping runtime cost low (one cheap query) and avoids brittle static analysis.

File Changes

Enhancement (8)

lib.rs Implement pnpm-compatible sha256-base64 hashing helpers +27/-0
Implement pnpm-compatible sha256-base64 hashing helpers
• Introduces 'create_hash' to emit 'sha256-<base64>' and 'create_hash_from_file' to hash UTF-8 file contents after CRLF→LF normalization. These helpers match pnpm’s hashing behavior used for 'pnpmfileChecksum'.
pacquet/crates/crypto-hash/src/lib.rs

lib.rs Extend PnpmfileHooks trait with checksum calculation API +15/-0
Extend PnpmfileHooks trait with checksum calculation API
• Adds a default async 'calculate_pnpmfile_checksum()' method returning 'Option<String>' to represent pnpm’s conditional checksum semantics (only when hooks are exported).
pacquet/crates/hooks/src/lib.rs

node_runtime.rs Compute pnpmfileChecksum for NodeJsHooks via worker gate + file hash +12/-0
Compute pnpmfileChecksum for NodeJsHooks via worker gate + file hash
• Implements 'calculate_pnpmfile_checksum()' by first querying the worker for 'hasHooks', then hashing the pnpmfile with CRLF normalization when applicable.
pacquet/crates/hooks/src/node_runtime.rs

worker.rs Add hasHooks query to pnpmfile worker protocol +24/-3
Add hasHooks query to pnpmfile worker protocol
• Extends the JSON line protocol to support a 'hasHooks' query, refactors request sending into a shared 'request()' helper, and updates the embedded Node script to return whether 'mod.hooks' is present.
pacquet/crates/hooks/src/worker.rs

lib.rs Add optional pnpmfile_checksum to lockfile model +14/-0
Add optional pnpmfile_checksum to lockfile model
• Introduces 'pnpmfile_checksum: Option<String>' with 'skip_serializing_if' so the 'pnpmfileChecksum' key is omitted when absent, preserving pnpm’s wire shape and byte-for-byte round-tripping behavior.
pacquet/crates/lockfile/src/lib.rs

current_lockfile.rs Preserve pnpmfile_checksum when filtering wanted→current lockfile +3/-0
Preserve pnpmfile_checksum when filtering wanted→current lockfile
• Threads 'pnpmfile_checksum' through the current lockfile filter so the field round-trips unchanged when producing the filtered view.
pacquet/crates/package-manager/src/current_lockfile.rs

dependencies_graph_to_lockfile.rs Thread pnpmfile_checksum through graph-to-lockfile options and output +8/-0
Thread pnpmfile_checksum through graph-to-lockfile options and output
• Extends 'GraphToLockfileOptions' and 'dependencies_graph_to_lockfile' to accept and set the optional 'pnpmfile_checksum' on the generated lockfile.
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs

install_with_fresh_lockfile.rs Compute pnpmfileChecksum once and include it in fresh lockfile builds +15/-0
Compute pnpmfileChecksum once and include it in fresh lockfile builds
• Computes the optional checksum via 'after_all_resolved_hook.calculate_pnpmfile_checksum()' and threads it into both fresh lockfile build paths so the emitted lockfile records the checksum when applicable.
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs

Tests (9)

tests.rs Add test vectors for base64 hash format and CRLF normalization +28/-1
Add test vectors for base64 hash format and CRLF normalization
• Adds unit tests validating the 'sha256-<base64>' prefix/encoding and ensuring CRLF-normalized file hashing is stable across line-ending variants.
pacquet/crates/crypto-hash/src/tests.rs

node_hooks.rs Add node hook tests for checksum gating and value parity +24/-0
Add node hook tests for checksum gating and value parity
• Adds async tests verifying checksum is produced when 'hooks' is exported and omitted when the module exports no hooks object, matching pnpm’s gate.
pacquet/crates/hooks/tests/node_hooks.rs

tests.rs Update lockfile save tests for new pnpmfile_checksum field +2/-0
Update lockfile save tests for new pnpmfile_checksum field
• Updates lockfile construction in save-lockfile tests to include 'pnpmfile_checksum: None' to match the updated struct shape.
pacquet/crates/lockfile/src/save_lockfile/tests.rs

tests.rs Adjust current lockfile tests for pnpmfile_checksum +1/-0
Adjust current lockfile tests for pnpmfile_checksum
• Updates test lockfile fixtures to include 'pnpmfile_checksum: None' after struct expansion.
pacquet/crates/package-manager/src/current_lockfile/tests.rs

tests.rs Update graph-to-lockfile tests for pnpmfile_checksum option +7/-0
Update graph-to-lockfile tests for pnpmfile_checksum option
• Updates multiple lockfile option fixtures to include 'pnpmfile_checksum: None' to match the new options struct field.
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs

tests.rs Update hoisted dep graph tests for pnpmfile_checksum field +5/-0
Update hoisted dep graph tests for pnpmfile_checksum field
• Adjusts test lockfile fixtures to include 'pnpmfile_checksum: None' after the lockfile struct gains the new field.
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs

tests.rs Add install-level tests ensuring pnpmfileChecksum emit/omit behavior +52/-0
Add install-level tests ensuring pnpmfileChecksum emit/omit behavior
• Adds two end-to-end install tests: one asserting 'pnpmfileChecksum' is written when hooks are exported and one asserting it’s omitted when no hooks are exported.
pacquet/crates/package-manager/src/install/tests.rs

tests.rs Update real-hoist tests for pnpmfile_checksum field +19/-0
Update real-hoist tests for pnpmfile_checksum field
• Updates lockfile fixtures in hoisting tests to include 'pnpmfile_checksum: None' to satisfy the updated lockfile struct.
pacquet/crates/real-hoist/src/tests.rs

tests.rs Update lockfile reuse tests for pnpmfile_checksum field +1/-0
Update lockfile reuse tests for pnpmfile_checksum field
• Adjusts the lockfile test fixture to include 'pnpmfile_checksum: None' to reflect the new lockfile field.
pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs

Other (2)

Cargo.toml Add base64 dependency for pnpm-style hash output +2/-1
Add base64 dependency for pnpm-style hash output
• Adds the workspace 'base64' crate dependency so crypto-hash can emit 'sha256-<base64>' digests matching pnpm’s format.
pacquet/crates/crypto-hash/Cargo.toml

Cargo.toml Wire hooks crate to crypto-hash for pnpmfile checksum calculation +5/-4
Wire hooks crate to crypto-hash for pnpmfile checksum calculation
• Adds a dependency on 'pacquet-crypto-hash' so hook implementations can compute 'pnpmfileChecksum'.
pacquet/crates/hooks/Cargo.toml

Summary by CodeRabbit

New Features
- Lockfiles now carry an optional pnpmfile checksum; installs will write pnpmfileChecksum when a pnpmfile exports hooks and omit it when not present.
- New hook API lets runtimes compute that checksum; checksum computation normalizes line endings for stable, pnpm-compatible SHA-256/base64 digests.
Tests
- Expanded tests cover checksum generation, CRLF→LF normalization, worker hook detection, and lockfile write/omit behavior.

coderabbitai · 2026-06-09T05:26:23Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 19d1b646-2dba-4053-9546-9a70a5df807b

📥 Commits

Reviewing files that changed from the base of the PR and between 526ca1a and dc9b96f.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

pacquet/crates/crypto-hash/Cargo.toml
pacquet/crates/crypto-hash/src/tests.rs

🚧 Files skipped from review as they are similar to previous changes (2)

pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/crypto-hash/Cargo.toml

📝 Walkthrough

Walkthrough

Adds pnpmfile checksum support: SHA256+base64 helpers, worker protocol to detect pnpmfile hooks, checksum calculation in NodeJsHooks, threading of checksum into lockfile generation and fresh-install, lockfile schema addition, and tests.

Changes

pnpmfile Checksum Support

Layer / File(s)	Summary
Crypto hash helpers for pnpm-compatible SHA256 `pacquet/crates/crypto-hash/Cargo.toml`, `pacquet/crates/crypto-hash/src/lib.rs`, `pacquet/crates/crypto-hash/src/tests.rs`	Added `base64` dependency and `tempfile` dev-dep; implemented `create_hash(input: &str) -> String` (SHA256 in `sha256-<base64>` format) and `create_hash_from_file(path: &Path)` (reads UTF‑8, normalizes CRLF→LF before hashing); tests validate exact output and line-ending normalization.
Hook protocol and pnpmfile checksum calculation `pacquet/crates/hooks/Cargo.toml`, `pacquet/crates/hooks/src/lib.rs`, `pacquet/crates/hooks/src/worker.rs`, `pacquet/crates/hooks/src/node_runtime.rs`, `pacquet/crates/hooks/tests/node_hooks.rs`	Added `PnpmfileHooks::calculate_pnpmfile_checksum()` (default `None`); extended Node worker protocol with `hasHooks` query and generalized request helper; Node runner handles `hasHooks`; `NodeJsHooks` computes checksum via `create_hash_from_file` only when `hooks` export exists; tests added for hook/no-hook cases.
Lockfile schema with pnpmfile_checksum field `pacquet/crates/lockfile/src/lib.rs`, `pacquet/crates/lockfile/src/save_lockfile/tests.rs`	Added optional `pnpmfile_checksum: Option<String>` to `Lockfile` with `serde(skip_serializing_if = "Option::is_none")`; updated tests to set field to `None`.
Thread checksum through lockfile generation `pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs`, `pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs`, `pacquet/crates/package-manager/src/current_lockfile.rs`, `pacquet/crates/package-manager/src/current_lockfile/tests.rs`	`GraphToLockfileOptions` gains `pnpmfile_checksum`; `dependencies_graph_to_lockfile` sets produced `Lockfile.pnpmfile_checksum` from options; `filter_lockfile_for_current` preserves the checksum; tests updated to include the option.
Compute checksum once in fresh-install pipeline `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs`	Computes `pnpmfile_checksum` once via `afterAllResolved` hook and threads it into `build_fresh_lockfile(pnpmfile_checksum: Option<&str>)` and then into `GraphToLockfileOptions` for both `lockfile_only` and normal flows.
Integration tests and test fixture updates `pacquet/crates/package-manager/src/install/tests.rs`, `pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs`, `pacquet/crates/real-hoist/src/tests.rs`, `pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs`	Added integration tests verifying `pnpm-lock.yaml` contains `pnpmfileChecksum` when hooks exist and omits it otherwise; updated many lockfile fixtures to include `pnpmfile_checksum: None`.

Sequence Diagram

sequenceDiagram
  participant CLI
  participant Install as Install/FreshLockfile
  participant NodeJsHooks
  participant NodeWorker
  participant CryptoHash
  participant LockfileGen as GraphToLockfileOptions

  CLI->>Install: start fresh install
  Install->>NodeJsHooks: call afterAllResolved hook
  NodeJsHooks->>NodeWorker: ensure worker & query has_hooks
  NodeWorker->>NodeWorker: send hasHooks request to runner
  NodeWorker-->>NodeJsHooks: bool(hasHooks)
  alt has hooks
    NodeJsHooks->>CryptoHash: create_hash_from_file(pnpmfile)
    CryptoHash-->>NodeJsHooks: "sha256-<base64>"
    NodeJsHooks-->>Install: Some(checksum)
    Install->>LockfileGen: build_fresh_lockfile(..., pnpmfile_checksum)
  else no hooks
    NodeJsHooks-->>Install: None
    Install->>LockfileGen: build_fresh_lockfile(..., pnpmfile_checksum=None)
  end
  LockfileGen->>LockfileGen: include pnpmfile_checksum in Lockfile
  LockfileGen-->>CLI: write pnpm-lock.yaml (with/without pnpmfileChecksum)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

pacquet: lockfile-parity gaps vs pnpm CLI (npm: aliases, minimumReleaseAge, peer suffixes) #12266: The PR directly addresses the pnpmfile checksum omission by implementing hash computation, hook detection, lockfile field, and pipeline wiring.

Possibly related PRs

pnpm/pnpm#11905: Overlaps with fresh-lockfile/lockfile construction changes that may need reconciliation with GraphToLockfileOptions edits.
pnpm/pnpm#12046: Related install/lockfile generation pipeline changes that touch lockfile emission and lockfile-only flows.

Poem

"I’m a rabbit with a tiny pad,
I hash your pnpmfile — never sad,
CRLF to LF I gently sweep,
SHA256 in base64 to keep,
Hop — a checksum nestles in YAML bed." 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'feat(lockfile): emit pnpmfileChecksum in pacquet lockfiles' clearly and concisely describes the main change: adding support for emitting the pnpmfileChecksum field to lockfiles, which is the core objective of this changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch pacquet-pnpmfile-checksum

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Port pnpm's `calculatePnpmfileChecksum` so a pacquet install records the `pnpmfileChecksum` field in `pnpm-lock.yaml` when the project's `.pnpmfile.{cjs,mjs}` exports a `hooks` object — matching pnpm's value byte-for-byte (sha256-base64 of the pnpmfile's CRLF-normalized contents) and its position in the lockfile (right after `packageExtensionsChecksum`). The checksum value is pure file hashing; only pnpm's `entries.some(entry => entry.hooks != null)` gate consults the evaluated module, answered here by a new `hasHooks` query on the existing long-lived pnpmfile worker. A pnpmfile that exports no hooks records no checksum, matching pnpm. Addresses item 5 of #12266.

github-actions · 2026-06-09T05:32:06Z

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00      7.3±0.11ms   590.4 KB/sec    1.05      7.7±0.18ms   563.0 KB/sec

codecov-commenter · 2026-06-09T05:41:29Z

Codecov Report

❌ Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.54%. Comparing base (a06adee) to head (dc9b96f).

Files with missing lines	Patch %	Lines
pacquet/crates/hooks/src/lib.rs	0.00%	2 Missing ⚠️
pacquet/crates/hooks/src/worker.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12280      +/-   ##
==========================================
+ Coverage   87.53%   87.54%   +0.01%     
==========================================
  Files         280      280              
  Lines       33441    33476      +35     
==========================================
+ Hits        29271    29307      +36     
+ Misses       4170     4169       -1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ 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.

qodo-free-for-open-source-projects · 2026-06-09T06:02:58Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

1. Checksum failures silently dropped 🐞 Bug ≡ Correctness

Description

NodeJsHooks::calculate_pnpmfile_checksum converts worker/query failures and file-read/hash
failures into None, so the install can write a lockfile that omits pnpmfileChecksum without
surfacing an error. This hides real failures and can break pnpm lockfile parity because the lockfile
writer interprets None as “no checksum to record”.

Code

pacquet/crates/hooks/src/node_runtime.rs[R175-185]

+    async fn calculate_pnpmfile_checksum(&self) -> Option<String> {
+        // Gate on the loaded module exporting `hooks`, mirroring pnpm's
+        // `entries.some(entry => entry.hooks != null)`. The checksum
+        // value itself is a pure hash of the pnpmfile's normalized
+        // bytes — only this gate needs to consult the evaluated module.
+        let worker = self.worker().await.ok()?;
+        if !worker.has_hooks().await {
+            return None;
+        }
+        pacquet_crypto_hash::create_hash_from_file(&self.file).ok()
+    }

Evidence
The checksum method uses .ok()?/.ok() to drop errors into None, and the install pipeline
directly uses that Option when building the lockfile; meanwhile, other pnpmfile hook failures are
explicitly documented/handled as install-fatal.
pacquet/crates/hooks/src/node_runtime.rs[175-185]
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1078-1087]
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1696-1723]

Agent prompt

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

## Issue description
`NodeJsHooks::calculate_pnpmfile_checksum()` currently swallows failures (worker spawn/request errors and file read/hash errors) by mapping them to `None`. That makes checksum omissions indistinguishable from the intended “no hooks exported” gate, and can silently produce a lockfile missing `pnpmfileChecksum`.
### Issue Context
The install path uses `calculate_pnpmfile_checksum()` to populate the top-level lockfile field, and hook failures elsewhere (e.g. `afterAllResolved`) are treated as install-fatal.
### Fix Focus Areas
- pacquet/crates/hooks/src/node_runtime.rs[175-185]
- pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1078-1087]
- pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1696-1723]
### Suggested fix
- Change the checksum API to surface errors distinctly from “no hooks”, e.g.:
- `async fn calculate_pnpmfile_checksum(&self) -> Result<Option<String>, HookError>`
- propagate `worker().await?`, propagate/convert `create_hash_from_file` IO errors into `HookError::Execution` (or introduce a dedicated error variant).
- Update the call site in `install_with_fresh_lockfile` to propagate failures (abort install) or explicitly log + abort only on IO/worker errors.
- If you must keep `Option<String>` for trait compatibility, at minimum emit a warning/error log when worker/file hashing fails so silent parity breakage is diagnosable.

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

2. Pending entries not cleaned 🐞 Bug ☼ Reliability

Description

NodeWorker::request inserts a pending entry before serializing/writing the request, but if
serialization or stdin write/flush fails it returns without removing that entry. This can leave
stale pending entries in memory and makes worker failure modes harder to recover from cleanly.

Code

pacquet/crates/hooks/src/worker.rs[R133-143]

+    async fn request(&self, label: &str, mut body: Value, log: LogFn) -> Result<Value, HookError> {
     let id = self.next_id.fetch_add(1, Ordering::Relaxed);
     let (done, rx) = oneshot::channel();
     self.pending.lock().await.insert(id, Pending { log, done });
-        let request = serde_json::json!({ "id": id, "hook": hook, "payload": payload });
+        body["id"] = serde_json::json!(id);
     let mut line =
-            serde_json::to_string(&request).map_err(|err| self.exec_err(err.to_string()))?;
+            serde_json::to_string(&body).map_err(|err| self.exec_err(err.to_string()))?;
     line.push('\n');
     {
         let mut stdin = self.stdin.lock().await;

Evidence

The code inserts into pending and then performs multiple ? operations that can return early
without removing the entry; removal is only present in the timeout branch.

pacquet/crates/hooks/src/worker.rs[133-156]

Agent prompt

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

## Issue description
`NodeWorker::request()` inserts `Pending { log, done }` into `self.pending` and then performs fallible steps (mutating body, JSON serialization, stdin write/flush). On any of those early failures it returns an error without removing the pending entry; only the timeout branch removes it.
### Issue Context
This is the shared request path for both hook calls and the new `hasHooks` query. If the worker pipe breaks (or serialization fails), the map can retain stale entries.
### Fix Focus Areas
- pacquet/crates/hooks/src/worker.rs[133-156]
### Suggested fix
- Ensure `self.pending` is cleaned up on *every* error path after insertion:
- After any failure in `to_string`, `write_all`, or `flush`, remove `id` from `self.pending` before returning.
- Consider an RAII guard pattern (a small struct with `Drop` that removes `id` unless explicitly disarmed after the request is successfully sent).
- Keep the insertion-before-write ordering (to avoid a fast worker response racing ahead of map insertion), but add explicit cleanup on send/serialization errors.

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

3. ~~Flaky temp-dir test~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

hash_from_file_normalizes_crlf writes into a fixed std::env::temp_dir() subdirectory, so
concurrent/parallel test runs can race on create/write/remove and intermittently fail or delete
another run’s files.

Code

pacquet/crates/crypto-hash/src/tests.rs[R19-28]

+    let dir = std::env::temp_dir().join("pacquet-crypto-hash-crlf-test");
+    std::fs::create_dir_all(&dir).unwrap();
+    let crlf = dir.join("crlf.txt");
+    let lf = dir.join("lf.txt");
+    std::fs::write(&crlf, "a\r\nb\r\n").unwrap();
+    std::fs::write(&lf, "a\nb\n").unwrap();
+    assert_eq!(create_hash_from_file(&crlf).unwrap(), create_hash_from_file(&lf).unwrap());
+    assert_eq!(create_hash_from_file(&lf).unwrap(), create_hash("a\nb\n"));
+    std::fs::remove_dir_all(&dir).ok();
+}

Evidence
The test uses a constant temp path and removes it at the end, which is not safe under parallel
execution. Nearby hook tests demonstrate the preferred pattern of using TempDir for isolation.
pacquet/crates/crypto-hash/src/tests.rs[19-28]
pacquet/crates/hooks/tests/node_hooks.rs[8-10]

Agent prompt

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

## Issue description
`hash_from_file_normalizes_crlf` uses a constant directory name under the OS temp directory. When tests execute in parallel (common in Rust), multiple invocations can contend on the same path (create, write, remove), causing flaky failures.
## Issue Context
Other tests in this repo already use `tempfile::TempDir` to ensure per-test isolation.
## Fix Focus Areas
- pacquet/crates/crypto-hash/src/tests.rs[19-28]
## Suggested fix
- Replace the fixed `std::env::temp_dir().join("pacquet-crypto-hash-crlf-test")` directory with a unique temporary directory created via `tempfile::TempDir::new()` (or `tempfile::Builder`).
- Write `crlf.txt` and `lf.txt` under that `TempDir`.
- Drop explicit `remove_dir_all`; rely on `TempDir` cleanup (or keep it only if you have a strong reason, but avoid deleting a shared path).

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

coderabbitai

🧹 Nitpick comments (1)

pacquet/crates/crypto-hash/src/tests.rs (1)

19-28: ⚡ Quick win

Use tempfile::TempDir for automatic cleanup.

The test manually creates and cleans up a temp directory, but cleanup can be skipped on panic and the path may not be unique across concurrent test runs. The hooks tests (e.g., pacquet/crates/hooks/tests/node_hooks.rs) use tempfile::TempDir::new() which ensures uniqueness and automatic cleanup.

♻️ Refactor to use tempfile::TempDir

Add tempfile to [dev-dependencies] in pacquet/crates/crypto-hash/Cargo.toml:

[dev-dependencies]
tempfile = { workspace = true }

Then update the test:

+use tempfile::TempDir;
+
 #[test]
 fn hash_from_file_normalizes_crlf() {
-    let dir = std::env::temp_dir().join("pacquet-crypto-hash-crlf-test");
-    std::fs::create_dir_all(&dir).unwrap();
+    let dir = TempDir::new().unwrap();
+    let dir = dir.path();
     let crlf = dir.join("crlf.txt");
     let lf = dir.join("lf.txt");
     std::fs::write(&crlf, "a\r\nb\r\n").unwrap();
     std::fs::write(&lf, "a\nb\n").unwrap();
     assert_eq!(create_hash_from_file(&crlf).unwrap(), create_hash_from_file(&lf).unwrap());
     assert_eq!(create_hash_from_file(&lf).unwrap(), create_hash("a\nb\n"));
-    std::fs::remove_dir_all(&dir).ok();
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pacquet/crates/crypto-hash/src/tests.rs` around lines 19 - 28, Replace the
manual temp-dir creation and cleanup in the test that uses create_hash_from_file
and create_hash with tempfile::TempDir to ensure unique, automatically cleaned
directories: add tempfile to [dev-dependencies] in
pacquet/crates/crypto-hash/Cargo.toml (tempfile = { workspace = true }), then in
the test use tempfile::TempDir::new() to create the directory, use
tempdir.path().join(...) for crlf.txt and lf.txt, write the files there, run the
same assertions against create_hash_from_file(&crlf) and create_hash(&lf) and
remove the explicit std::fs::remove_dir_all call so cleanup is automatic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pacquet/crates/crypto-hash/src/tests.rs`:
- Around line 19-28: Replace the manual temp-dir creation and cleanup in the
test that uses create_hash_from_file and create_hash with tempfile::TempDir to
ensure unique, automatically cleaned directories: add tempfile to
[dev-dependencies] in pacquet/crates/crypto-hash/Cargo.toml (tempfile = {
workspace = true }), then in the test use tempfile::TempDir::new() to create the
directory, use tempdir.path().join(...) for crlf.txt and lf.txt, write the files
there, run the same assertions against create_hash_from_file(&crlf) and
create_hash(&lf) and remove the explicit std::fs::remove_dir_all call so cleanup
is automatic.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f65f3115-c001-40a2-ad0b-139be028ea6a

📥 Commits

Reviewing files that changed from the base of the PR and between a06adee and e556028.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (19)

pacquet/crates/crypto-hash/Cargo.toml
pacquet/crates/crypto-hash/src/lib.rs
pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/hooks/Cargo.toml
pacquet/crates/hooks/src/lib.rs
pacquet/crates/hooks/src/node_runtime.rs
pacquet/crates/hooks/src/worker.rs
pacquet/crates/hooks/tests/node_hooks.rs
pacquet/crates/lockfile/src/lib.rs
pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/package-manager/src/current_lockfile.rs
pacquet/crates/package-manager/src/current_lockfile/tests.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/crates/real-hoist/src/tests.rs
pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Lint and Test (macos-latest)
GitHub Check: Run benchmark on ubuntu-latest

🧰 Additional context used

📓 Path-based instructions (3)

pacquet/**/Cargo.toml

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/Cargo.toml: Check whether the workspace already depends on something suitable in [workspace.dependencies] in the root Cargo.toml before adding a new dependency
Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it

Files:

pacquet/crates/crypto-hash/Cargo.toml
pacquet/crates/hooks/Cargo.toml

pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization
Derive simple conversions for branded strings using #[derive(derive_more::From)] and #[derive(derive_more::Into)] instead of handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like `${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs
pacquet/crates/package-manager/src/current_lockfile/tests.rs
pacquet/crates/package-manager/src/current_lockfile.rs
pacquet/crates/hooks/tests/node_hooks.rs
pacquet/crates/crypto-hash/src/lib.rs
pacquet/crates/hooks/src/node_runtime.rs
pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs
pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/lockfile/src/lib.rs
pacquet/crates/hooks/src/lib.rs
pacquet/crates/hooks/src/worker.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/crates/real-hoist/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs

pacquet/**/tests/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — use tempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq! assertions, dbg! complex structures, skip logging for simple scalar assert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer #[cfg_attr(target_os = "windows", ignore = "...")] (or matching #[cfg(unix)] gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests with insta and carefully review diffs when intentional changes alter snapshots; accept with cargo insta review only after careful review
Tests that need the mocked registry should start pnpr through pacquet-testing-utils; cargo test / cargo nextest run should not require a separate just registry-mock launch step

Files:

pacquet/crates/hooks/tests/node_hooks.rs

🧠 Learnings (7)

📚 Learning: 2026-05-20T19:40:55.051Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.

Applied to files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs
pacquet/crates/package-manager/src/current_lockfile/tests.rs
pacquet/crates/package-manager/src/current_lockfile.rs
pacquet/crates/hooks/tests/node_hooks.rs
pacquet/crates/crypto-hash/src/lib.rs
pacquet/crates/hooks/src/node_runtime.rs
pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs
pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/lockfile/src/lib.rs
pacquet/crates/hooks/src/lib.rs
pacquet/crates/hooks/src/worker.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/crates/real-hoist/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs

📚 Learning: 2026-05-22T00:08:44.646Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs
pacquet/crates/package-manager/src/current_lockfile/tests.rs
pacquet/crates/package-manager/src/current_lockfile.rs
pacquet/crates/hooks/tests/node_hooks.rs
pacquet/crates/crypto-hash/src/lib.rs
pacquet/crates/hooks/src/node_runtime.rs
pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs
pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/lockfile/src/lib.rs
pacquet/crates/hooks/src/lib.rs
pacquet/crates/hooks/src/worker.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/crates/real-hoist/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs

📚 Learning: 2026-05-20T23:07:58.444Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.

Applied to files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs
pacquet/crates/package-manager/src/current_lockfile/tests.rs
pacquet/crates/package-manager/src/current_lockfile.rs
pacquet/crates/hooks/tests/node_hooks.rs
pacquet/crates/crypto-hash/src/lib.rs
pacquet/crates/hooks/src/node_runtime.rs
pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs
pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/lockfile/src/lib.rs
pacquet/crates/hooks/src/lib.rs
pacquet/crates/hooks/src/worker.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/crates/real-hoist/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs

📚 Learning: 2026-05-24T21:10:50.292Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: When implementing dependency “revisit”/optional-folding in pacquet, mirror pnpm’s `resolveDependencies.ts` behavior: on revisit, update the `optional` flag only for the directly visited package, and do not automatically change `optional` for transitive descendants. pnpm corrects stale optional flags later via `copyDependencySubGraph` (a BFS in `lockfile/pruner/src/index.ts`/`copyDependencySubGraph`). Until pacquet has an equivalent pruner/copy-subgraph step, be aware that the raw `node.optional` can flow through to the lockfile/virtual store via `dependencies_graph_to_lockfile.rs` → `create_virtual_store.rs` → `installability.rs`, so review changes that affect optional flag propagation for this mismatch.

Applied to files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs

📚 Learning: 2026-05-24T21:10:50.292Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: In pacquet’s Rust resolver layer, the behavior of `ResolvedPackage.optional` “AND-folding”/revisit updates is intentionally aligned with pnpm: when a package is directly revisited, only that package’s `optional` flag is updated; transitive descendants are not re-walked/rescored in this stage. pnpm later corrects any stale optional flags downstream via its pruner/BFS logic (`copyDependencySubGraph` + `nonOptional` reachability stamping in the lockfile pruner). Since pacquet does not yet have that pruner equivalent, review should NOT flag the downstream/stale optional propagation as a bug in pacquet PRs; treat the resolver-layer optional propagation gap as expected parity until the pruning/stamping step is ported.

Applied to files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs

📚 Learning: 2026-06-06T18:58:37.156Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 12243
File: pacquet/crates/package-manager/src/install_package_by_snapshot.rs:319-322
Timestamp: 2026-06-06T18:58:37.156Z
Learning: When reviewing Rust code, do not assume `matches!(expr, Pattern(_))` will move out of `expr` if `Pattern(_)` contains no by-value bindings. `matches!` desugars to a `match` that auto-borrows the scrutinee for discrimination, so even if `expr` is a non-`Copy` value behind a shared reference (e.g., `&T`), the macro should not move-out of the borrowed data purely due to `matches!`. Treat `matches!(&expr, Pattern(_))` as a readability/clarity improvement, not a correctness requirement. Only flag potential move-out-of-borrow risks when the pattern includes by-value bindings that would require moving the matched value.

Applied to files:

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs
pacquet/crates/package-manager/src/current_lockfile/tests.rs
pacquet/crates/package-manager/src/current_lockfile.rs
pacquet/crates/hooks/tests/node_hooks.rs
pacquet/crates/crypto-hash/src/lib.rs
pacquet/crates/hooks/src/node_runtime.rs
pacquet/crates/crypto-hash/src/tests.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs
pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/lockfile/src/lib.rs
pacquet/crates/hooks/src/lib.rs
pacquet/crates/hooks/src/worker.rs
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/crates/real-hoist/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs

📚 Learning: 2026-05-20T23:23:40.606Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11788
File: pacquet/crates/lockfile/src/snapshot_dep_ref.rs:52-56
Timestamp: 2026-05-20T23:23:40.606Z
Learning: In the pacquet lockfile crate, the link target is intentionally represented as a raw `Link(String)` (not a `LinkTarget` newtype) for both `SnapshotDepRef::Link` and `ImporterDepVersion::Link`. pnpm upstream preserves `link:` targets verbatim with only a prefix check (e.g., `refToRelative`), so do not suggest introducing a `LinkTarget` newtype in a bug-fix/targeted PR. If type branding is ever desired, it should be done together for both variants in a separate dedicated refactor PR rather than as part of a small fix.

Applied to files:

pacquet/crates/lockfile/src/save_lockfile/tests.rs
pacquet/crates/lockfile/src/lib.rs

🔇 Additional comments (26)

pacquet/crates/lockfile/src/lib.rs (1)

140-152: LGTM!

pacquet/crates/lockfile/src/save_lockfile/tests.rs (1)

205-205: LGTM!

Also applies to: 243-243

pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs (1)

103-108: LGTM!

Also applies to: 153-153, 188-188

pacquet/crates/package-manager/src/current_lockfile.rs (1)

92-94: LGTM!

pacquet/crates/package-manager/src/current_lockfile/tests.rs (1)

61-61: LGTM!

pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs (1)

50-50: LGTM!

Also applies to: 222-222, 248-248, 365-365, 1038-1038, 1166-1166, 1323-1323

pacquet/crates/crypto-hash/src/lib.rs (2)

18-28: LGTM!

30-41: LGTM!

pacquet/crates/hooks/Cargo.toml (1)

14-14: LGTM!

pacquet/crates/hooks/src/lib.rs (1)

83-96: LGTM!

pacquet/crates/hooks/src/worker.rs (4)

114-116: LGTM!

118-127: LGTM!

129-157: LGTM!

208-208: LGTM!

pacquet/crates/hooks/src/node_runtime.rs (1)

175-185: LGTM!

pacquet/crates/hooks/tests/node_hooks.rs (2)

32-43: LGTM!

45-54: LGTM!

pacquet/crates/crypto-hash/Cargo.toml (1)

14-14: base64 is already declared in the root workspace dependencies (root Cargo.toml line 93), so base64 = { workspace = true } in pacquet/crates/crypto-hash/Cargo.toml complies with the workspace-dependency guideline.

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (4)

1078-1087: LGTM!

1102-1102: LGTM!

1233-1233: LGTM!

1753-1753: LGTM!

Also applies to: 1779-1779

pacquet/crates/package-manager/src/install/tests.rs (1)

6489-6540: LGTM!

pacquet/crates/package-manager/src/hoisted_dep_graph/tests.rs (1)

181-181: LGTM!

Also applies to: 200-200, 651-651, 691-691, 826-826

pacquet/crates/real-hoist/src/tests.rs (1)

39-39: LGTM!

Also applies to: 70-70, 123-123, 178-178, 258-258, 331-331, 372-372, 427-427, 534-534, 619-619, 686-686, 764-764, 818-818, 875-875, 934-934, 987-987, 1043-1043, 1082-1082, 1128-1128

pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs (1)

35-35: LGTM!

github-actions · 2026-06-09T06:13:20Z

Integrated-Benchmark Report (Linux)

Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD.

Scenario: Isolated linker: fresh restore, cold cache + cold store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	9.983 ± 0.136	9.857	10.256	1.84 ± 0.04
`pacquet@main`	9.909 ± 0.107	9.847	10.206	1.83 ± 0.03
`pnpr@HEAD`	5.416 ± 0.080	5.282	5.510	1.00
`pnpr@main`	5.509 ± 0.170	5.355	5.831	1.02 ± 0.03

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 9.9832617432,
      "stddev": 0.13634004699648122,
      "median": 9.9306405737,
      "user": 3.08201258,
      "system": 3.358720880000001,
      "min": 9.8567486807,
      "max": 10.2557052427,
      "times": [
        10.2557052427,
        10.1053012767,
        10.1432183207,
        9.8949357157,
        9.9384038327,
        9.8936032347,
        9.8567486807,
        9.922877314700001,
        9.9589694847,
        9.862854328700001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 9.908926701999999,
      "stddev": 0.10714037630937995,
      "median": 9.8738077122,
      "user": 3.09710488,
      "system": 3.36731198,
      "min": 9.8470628167,
      "max": 10.2063807127,
      "times": [
        10.2063807127,
        9.8800836057,
        9.8675318187,
        9.8970813257,
        9.9252905897,
        9.8640043557,
        9.8470628167,
        9.8617378967,
        9.8502493217,
        9.8898445767
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 5.416131193000001,
      "stddev": 0.08016063786197437,
      "median": 5.4251777732,
      "user": 2.5153872799999997,
      "system": 3.0333097799999997,
      "min": 5.281901098700001,
      "max": 5.5099710687000005,
      "times": [
        5.4625021217,
        5.3424196557000005,
        5.371567943700001,
        5.281901098700001,
        5.4512345837,
        5.3991209627000005,
        5.497923198700001,
        5.5034215737,
        5.5099710687000005,
        5.3412497227
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 5.508535795799999,
      "stddev": 0.1699532051797275,
      "median": 5.4480172467,
      "user": 2.5332630800000002,
      "system": 3.0292811799999995,
      "min": 5.3552728687,
      "max": 5.8308583057000005,
      "times": [
        5.5168750717,
        5.7096266557,
        5.3791594217,
        5.372603717700001,
        5.3681556707,
        5.3626434567,
        5.3552728687,
        5.6160720277000005,
        5.8308583057000005,
        5.5740907617
      ]
    }
  ]
}

Scenario: Isolated linker: fresh restore, hot cache + hot store

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`pacquet@HEAD`	645.4 ± 10.5	632.9	669.6	1.00
`pacquet@main`	670.1 ± 57.5	637.3	827.9	1.04 ± 0.09
`pnpr@HEAD`	781.5 ± 47.8	731.9	872.3	1.21 ± 0.08
`pnpr@main`	750.6 ± 17.2	727.8	778.7	1.16 ± 0.03

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.6454477131,
      "stddev": 0.010505140063458774,
      "median": 0.6437139991,
      "user": 0.37901878,
      "system": 1.32881896,
      "min": 0.6328604646000001,
      "max": 0.6696031546000001,
      "times": [
        0.6418006016000001,
        0.6457633666000001,
        0.6447760286,
        0.6696031546000001,
        0.6328604646000001,
        0.6546219166000001,
        0.6469618896000001,
        0.6426519696,
        0.6411612776000001,
        0.6342764616000001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6701470952000002,
      "stddev": 0.05746860307523407,
      "median": 0.6498552686000001,
      "user": 0.36905598000000006,
      "system": 1.3379893600000001,
      "min": 0.6372513446000001,
      "max": 0.8278510976000001,
      "times": [
        0.6374130656000001,
        0.6372513446000001,
        0.6432630766,
        0.8278510976000001,
        0.6554114156,
        0.6390301266,
        0.6794292166000001,
        0.6646813876000001,
        0.6442991216000001,
        0.6728410996
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.7815292248000001,
      "stddev": 0.0477518935674792,
      "median": 0.7779616576000001,
      "user": 0.40771298,
      "system": 1.32410856,
      "min": 0.7319468936000001,
      "max": 0.8722978106000001,
      "times": [
        0.8234746496,
        0.7674032486000001,
        0.7407094696000001,
        0.7329261536000001,
        0.8722978106000001,
        0.7319468936000001,
        0.7885200666000001,
        0.7381090536000001,
        0.8219405866000001,
        0.7979643156000001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.7506389971,
      "stddev": 0.01723659619782505,
      "median": 0.7464271966,
      "user": 0.38966038000000003,
      "system": 1.3396560599999998,
      "min": 0.7278054656,
      "max": 0.7786678496000001,
      "times": [
        0.7494149766000001,
        0.7779090926000001,
        0.7465694216000001,
        0.7403459526000001,
        0.7633532716000001,
        0.7377582826000001,
        0.7278054656,
        0.7382806866000001,
        0.7462849716000001,
        0.7786678496000001
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + cold store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	9.243 ± 0.053	9.159	9.324	1.81 ± 0.01
`pacquet@main`	9.257 ± 0.060	9.185	9.365	1.82 ± 0.02
`pnpr@HEAD`	5.121 ± 0.080	5.032	5.308	1.01 ± 0.02
`pnpr@main`	5.093 ± 0.028	5.068	5.146	1.00

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 9.24331068378,
      "stddev": 0.05346759522423389,
      "median": 9.25121832658,
      "user": 3.537384839999999,
      "system": 3.32912914,
      "min": 9.15869387808,
      "max": 9.32413648808,
      "times": [
        9.213948909079999,
        9.20399552408,
        9.26784571008,
        9.24701413008,
        9.26634333908,
        9.31225150808,
        9.18345482808,
        9.25542252308,
        9.15869387808,
        9.32413648808
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 9.256965807780002,
      "stddev": 0.05985070233802738,
      "median": 9.24717619808,
      "user": 3.59277644,
      "system": 3.34216614,
      "min": 9.18495978408,
      "max": 9.365052670079999,
      "times": [
        9.20405280108,
        9.18495978408,
        9.20590303308,
        9.203104636079999,
        9.30952076108,
        9.30376821008,
        9.23832206708,
        9.365052670079999,
        9.25603032908,
        9.298943786079999
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 5.120680822079999,
      "stddev": 0.079623850882933,
      "median": 5.09943497808,
      "user": 2.30208684,
      "system": 2.91443154,
      "min": 5.03200230308,
      "max": 5.3075110280799995,
      "times": [
        5.1384227320799996,
        5.10966999408,
        5.3075110280799995,
        5.10026934208,
        5.0986006140799995,
        5.06875821708,
        5.19951699908,
        5.08109184508,
        5.07096514608,
        5.03200230308
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 5.09342903858,
      "stddev": 0.02826087065331442,
      "median": 5.08554062408,
      "user": 2.32269364,
      "system": 2.9073443399999994,
      "min": 5.0678015920799995,
      "max": 5.14591145808,
      "times": [
        5.0684118930799995,
        5.114412125079999,
        5.0892614930799995,
        5.08204391608,
        5.070013331079999,
        5.13357564408,
        5.07382160108,
        5.14591145808,
        5.089037332079999,
        5.0678015920799995
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, hot cache + hot store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	1.386 ± 0.012	1.360	1.400	2.06 ± 0.08
`pacquet@main`	1.407 ± 0.011	1.394	1.424	2.09 ± 0.08
`pnpr@HEAD`	0.674 ± 0.025	0.654	0.728	1.00
`pnpr@main`	0.693 ± 0.088	0.645	0.941	1.03 ± 0.14

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.3860114511800001,
      "stddev": 0.012097854630041057,
      "median": 1.38774608648,
      "user": 1.54290028,
      "system": 1.7695706999999998,
      "min": 1.36019993198,
      "max": 1.4003177769800002,
      "times": [
        1.3906060229800001,
        1.39378700798,
        1.39826212198,
        1.36019993198,
        1.38580787598,
        1.37203434398,
        1.4003177769800002,
        1.38361715498,
        1.38968429698,
        1.38579797798
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.40728049928,
      "stddev": 0.011377408882940865,
      "median": 1.4049762509800001,
      "user": 1.55856238,
      "system": 1.8153241999999998,
      "min": 1.39421910098,
      "max": 1.42394545598,
      "times": [
        1.39669331998,
        1.4226374829800001,
        1.42394545598,
        1.4095822639800002,
        1.4123586429800001,
        1.40037023798,
        1.39421910098,
        1.41744176398,
        1.3960329719800002,
        1.3995237519800001
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.6736389228800002,
      "stddev": 0.025326070170085434,
      "median": 0.6627966814800001,
      "user": 0.33198258,
      "system": 1.2709217999999998,
      "min": 0.6538189489800001,
      "max": 0.7284719209800001,
      "times": [
        0.7284719209800001,
        0.7126296939800001,
        0.6691180549800001,
        0.6605440879800001,
        0.6594933439800001,
        0.6628048119800001,
        0.6538189489800001,
        0.6627885509800001,
        0.6660510039800001,
        0.6606688109800001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.6932302469800001,
      "stddev": 0.08754233751205424,
      "median": 0.6676783679800001,
      "user": 0.33891298,
      "system": 1.2655534999999998,
      "min": 0.6447194639800001,
      "max": 0.9409071619800001,
      "times": [
        0.6797650839800001,
        0.6686329609800001,
        0.6647212389800001,
        0.6705116399800001,
        0.6750820599800001,
        0.9409071619800001,
        0.6667237749800001,
        0.6596987159800001,
        0.6447194639800001,
        0.6615403689800001
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + hot store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	4.973 ± 0.031	4.942	5.026	7.36 ± 0.41
`pacquet@main`	5.007 ± 0.022	4.966	5.037	7.41 ± 0.41
`pnpr@HEAD`	0.675 ± 0.037	0.651	0.776	1.00
`pnpr@main`	0.702 ± 0.067	0.668	0.890	1.04 ± 0.11

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 4.973146718780001,
      "stddev": 0.031073116923122753,
      "median": 4.95913319688,
      "user": 1.70364666,
      "system": 1.9008816199999998,
      "min": 4.94195778288,
      "max": 5.0257645148800005,
      "times": [
        4.947923916880001,
        5.021870609880001,
        4.94195778288,
        4.94563518488,
        5.0257645148800005,
        4.95561389488,
        4.962652498880001,
        4.98730606088,
        4.95468223288,
        4.988060490880001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 5.0065043543800005,
      "stddev": 0.0220028247436783,
      "median": 5.00845822488,
      "user": 1.75131716,
      "system": 1.9239037199999995,
      "min": 4.9658012968800005,
      "max": 5.03735091588,
      "times": [
        5.03735091588,
        5.01425144488,
        5.0311837738800005,
        5.00673987488,
        5.02400369188,
        5.01017657488,
        4.994366539880001,
        4.99785844988,
        4.983310980880001,
        4.9658012968800005
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.67529925138,
      "stddev": 0.03729556452272575,
      "median": 0.6623729598800001,
      "user": 0.33613916,
      "system": 1.28424262,
      "min": 0.65092948388,
      "max": 0.7755267088800001,
      "times": [
        0.7755267088800001,
        0.66860721588,
        0.6551142458800001,
        0.69000793788,
        0.6537076718800001,
        0.6669963418800001,
        0.6577495778800001,
        0.65092948388,
        0.6780293528800001,
        0.6563239768800001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.7021361152800001,
      "stddev": 0.06723310026428778,
      "median": 0.67800935388,
      "user": 0.34224426,
      "system": 1.2880938199999998,
      "min": 0.6678710478800001,
      "max": 0.89031461588,
      "times": [
        0.6993053858800001,
        0.6678710478800001,
        0.67079111688,
        0.6709917228800001,
        0.89031461588,
        0.6777916788800001,
        0.6926466048800001,
        0.6782270288800001,
        0.7009702388800001,
        0.6724517118800001
      ]
    }
  ]
}

github-actions · 2026-06-09T06:13:22Z

Bencher Report

Branch	pr/12280
Testbed	pacquet

🚨 2 Alerts

Benchmark	Measure Units	View	Benchmark Result (Result Δ%)	Upper Boundary (Limit %)
isolated-linker.fresh-install.cold-cache.cold-store	Latency seconds (s)	📈 plot 🚷 threshold 🚨 alert (🔔)	9.24 s (+54.68%) Baseline: 5.98 s	7.17 s (128.90%)
isolated-linker.fresh-restore.cold-cache.cold-store	Latency seconds (s)	📈 plot 🚷 threshold 🚨 alert (🔔)	9.98 s (+20.57%) Baseline: 8.28 s	9.94 s (100.48%)

Click to view all benchmark results

Benchmark	Latency	Benchmark Result milliseconds (ms) (Result Δ%)	Upper Boundary milliseconds (ms) (Limit %)
isolated-linker.fresh-install.cold-cache.cold-store	📈 view plot 🚷 view threshold 🚨 view alert (🔔)	9,243.31 ms (+54.68%) Baseline: 5,975.75 ms	7,170.90 ms (128.90%)
isolated-linker.fresh-install.cold-cache.hot-store	📈 view plot 🚷 view threshold	4,973.15 ms (-0.66%) Baseline: 5,006.34 ms	6,007.60 ms (82.78%)
isolated-linker.fresh-install.hot-cache.hot-store	📈 view plot 🚷 view threshold	1,386.01 ms (-0.15%) Baseline: 1,388.16 ms	1,665.79 ms (83.20%)
isolated-linker.fresh-restore.cold-cache.cold-store	📈 view plot 🚷 view threshold 🚨 view alert (🔔)	9,983.26 ms (+20.57%) Baseline: 8,279.96 ms	9,935.95 ms (100.48%)
isolated-linker.fresh-restore.hot-cache.hot-store	📈 view plot 🚷 view threshold	645.45 ms (-0.96%) Baseline: 651.73 ms	782.07 ms (82.53%)

🐰 View full continuous benchmarking report in Bencher

qodo-free-for-open-source-projects · 2026-06-09T06:16:39Z

Code review by qodo was updated up to the latest commit 526ca1a

`hash_from_file_normalizes_crlf` used a fixed directory under the OS temp dir, so parallel test runs could contend on the same create/write/remove path and flake. Use a per-test `tempfile::TempDir` instead and drop the explicit cleanup.

qodo-free-for-open-source-projects · 2026-06-09T07:00:24Z

Code review by qodo was updated up to the latest commit dc9b96f

zkochan force-pushed the pacquet-pnpmfile-checksum branch from 4d305bc to e556028 Compare June 9, 2026 05:33

zkochan marked this pull request as ready for review June 9, 2026 06:02

coderabbitai Bot reviewed Jun 9, 2026

View reviewed changes

coderabbitai Bot approved these changes Jun 9, 2026

View reviewed changes

zkochan force-pushed the pacquet-pnpmfile-checksum branch from 526ca1a to dc9b96f Compare June 9, 2026 07:00

zkochan merged commit b730835 into main Jun 9, 2026
28 checks passed

zkochan deleted the pacquet-pnpmfile-checksum branch June 9, 2026 07:14

zkochan mentioned this pull request Jun 9, 2026

pacquet: lockfile-parity gaps vs pnpm CLI (npm: aliases, minimumReleaseAge, peer suffixes) #12266

Closed

Uh oh!

Uh oh!

Conversation

zkochan commented Jun 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Walkthroughs

What

How

Verification

Scope

File Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

Review ran into problems

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Micro-Benchmark Results

Linux

Uh oh!

codecov-commenter commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integrated-Benchmark Report (Linux)

Scenario: Isolated linker: fresh restore, cold cache + cold store

Scenario: Isolated linker: fresh restore, hot cache + hot store

Scenario: Isolated linker: fresh install, cold cache + cold store

Scenario: Isolated linker: fresh install, hot cache + hot store

Scenario: Isolated linker: fresh install, cold cache + hot store

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bencher Report

🚨 2 Alerts

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented Jun 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading

codecov-commenter commented Jun 9, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 •

edited

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 •

edited

Loading