Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a workspace/CLI ChangesTrustLockfile + verifier memory reductions
Sequence DiagramsequenceDiagram
participant Client as Client (createClient)
participant Resolver as NpmResolver
participant SharedCache as PackageMetaCache (meta_cache)
participant Verifier as ResolutionVerifier
Client->>SharedCache: createDefaultPackageMetaCache()
Client->>Resolver: create resolver with meta_cache
Resolver->>SharedCache: populate packument entries on fetch
Client->>Verifier: create verifiers with same meta_cache
Verifier->>SharedCache: lookup {name}:full (fast-path)
alt cache hit
SharedCache-->>Verifier: projected trust-meta
Verifier->>Verifier: run trust checks (fail_if_trust_downgraded)
else cache miss
Verifier->>Resolver: fetch full packument
Resolver-->>SharedCache: store fetched packument
SharedCache-->>Verifier: projected trust-meta
Verifier->>Verifier: run trust checks
end
Estimated code review effort: 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues:
Possibly related PRs:
Suggested reviewers:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11878 +/- ##
==========================================
+ Coverage 87.81% 87.83% +0.02%
==========================================
Files 205 205
Lines 24424 24497 +73
==========================================
+ Hits 21448 21518 +70
- Misses 2976 2979 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.40377371826,
"stddev": 0.19228803740217565,
"median": 2.34641842586,
"user": 2.7906079599999996,
"system": 3.43932896,
"min": 2.22076645236,
"max": 2.89027485436,
"times": [
2.32268717636,
2.34220644636,
2.3087577653599998,
2.55932490536,
2.89027485436,
2.39632965236,
2.35063040536,
2.22076645236,
2.28337084036,
2.36338868436
]
},
{
"command": "pacquet@main",
"mean": 2.28058998566,
"stddev": 0.052823463354859934,
"median": 2.28494016286,
"user": 2.76117496,
"system": 3.45498956,
"min": 2.18677316736,
"max": 2.38176091936,
"times": [
2.18677316736,
2.23537420636,
2.29766155836,
2.29267593936,
2.28056638036,
2.28931394536,
2.25161082636,
2.38176091936,
2.32868079136,
2.26148212236
]
},
{
"command": "pnpm",
"mean": 4.58431261876,
"stddev": 0.044621554018339095,
"median": 4.56495212386,
"user": 7.78718866,
"system": 3.9940413599999998,
"min": 4.530680072360001,
"max": 4.66374276336,
"times": [
4.64230032036,
4.61537068236,
4.548295615360001,
4.6052895643600005,
4.557302984360001,
4.568798597360001,
4.66374276336,
4.550239937360001,
4.530680072360001,
4.56110565036
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6720390981000001,
"stddev": 0.025411889799812012,
"median": 0.6648045038999999,
"user": 0.37363957999999997,
"system": 1.4676017999999997,
"min": 0.6514459599,
"max": 0.7362525419,
"times": [
0.7362525419,
0.6514459599,
0.6903688939,
0.6516604889000001,
0.6639191519,
0.6714124829,
0.6656898559,
0.6541226789,
0.6628149319,
0.6727039949
]
},
{
"command": "pacquet@main",
"mean": 0.7005922838,
"stddev": 0.0630947475983233,
"median": 0.6720925829,
"user": 0.37687008,
"system": 1.4747907,
"min": 0.6628618009,
"max": 0.8623077119,
"times": [
0.7521508719,
0.6736980179000001,
0.6650110099,
0.7043575809,
0.6825550149,
0.6704871479,
0.6628618009,
0.8623077119,
0.6658107719,
0.6666829099
]
},
{
"command": "pnpm",
"mean": 2.4047930294000004,
"stddev": 0.06814997412943753,
"median": 2.3950521499,
"user": 3.01940468,
"system": 2.1774706999999998,
"min": 2.2819860889,
"max": 2.5268008629,
"times": [
2.4931380339,
2.4097997539000002,
2.3687558589,
2.2819860889,
2.3860722119,
2.3660539059000003,
2.4262674319000004,
2.4040320879,
2.3850240579000004,
2.5268008629
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 5.08409963416,
"stddev": 0.22284140667833519,
"median": 5.137986423659999,
"user": 6.79717828,
"system": 3.5132990800000004,
"min": 4.74677052266,
"max": 5.39200214066,
"times": [
5.20482459566,
5.19222354566,
4.74677052266,
4.82078996866,
5.3504679696599995,
5.16938952666,
4.83988935666,
5.01805539466,
5.1065833206599995,
5.39200214066
]
},
{
"command": "pacquet@main",
"mean": 4.96440225846,
"stddev": 0.09934675902625842,
"median": 4.94841612516,
"user": 6.726338980000001,
"system": 3.4828615800000002,
"min": 4.82064076266,
"max": 5.0973140276599995,
"times": [
4.84738705666,
4.92321360066,
5.05821225466,
4.82064076266,
5.08379671266,
4.97361864966,
5.03200087866,
4.91023595666,
5.0973140276599995,
4.89760268466
]
},
{
"command": "pnpm",
"mean": 6.208396699860001,
"stddev": 0.09987319847265061,
"median": 6.18978656716,
"user": 10.205546079999998,
"system": 4.367753379999999,
"min": 6.0770756086599995,
"max": 6.37964838266,
"times": [
6.18855050266,
6.21435702166,
6.1889298856599995,
6.2644471486599995,
6.35073755866,
6.13875287966,
6.37964838266,
6.19064324866,
6.0770756086599995,
6.0908247616599995
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.95827741838,
"stddev": 0.0904352026434587,
"median": 3.96278279638,
"user": 4.270983719999999,
"system": 2.2449105799999995,
"min": 3.80604875788,
"max": 4.1263423668799994,
"times": [
3.80604875788,
4.1263423668799994,
3.98366544788,
3.91781465888,
4.01729507288,
4.01176821688,
3.8623402158799998,
4.00301351188,
3.91258578988,
3.94190014488
]
},
{
"command": "pacquet@main",
"mean": 4.01568185638,
"stddev": 0.12878035000295052,
"median": 3.98505016888,
"user": 4.31665972,
"system": 2.23357218,
"min": 3.86202736288,
"max": 4.25416170588,
"times": [
4.15931798888,
3.9835853758799997,
3.96156061888,
3.86202736288,
3.89863262988,
4.25416170588,
4.13451237188,
4.02502969488,
3.98651496188,
3.89147585288
]
},
{
"command": "pnpm",
"mean": 4.1922593042799985,
"stddev": 0.04534405424030694,
"median": 4.17800703088,
"user": 5.1797368200000005,
"system": 2.6452055799999994,
"min": 4.12446297688,
"max": 4.27606120588,
"times": [
4.24709740188,
4.21068339788,
4.12446297688,
4.16443668388,
4.17225529788,
4.18375876388,
4.27606120588,
4.16348940388,
4.16319190388,
4.21715600688
]
}
]
} |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Bencher Report| Branch | pr/11878 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 5,084.10 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 3,958.28 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,403.77 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 672.04 ms |
7a5bcd3 to
cf77b15
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
installing/commands/src/install.ts (1)
68-68: ⚡ Quick winKeep
InstallCommandOptionsin sync with the new flag.
rcOptionsTypes()now admitstrust-lockfile, butInstallCommandOptionsbelow still omitstrustLockfile. That leaves the handler boundary out of sync with the option surface and makes it easy to drop the flag in later refactors without the compiler noticing.Suggested fix
export type InstallCommandOptions = Pick<Config, | 'autoInstallPeers' | 'bail' | 'bin' | 'catalogs' @@ | 'tag' +| 'trustLockfile' | 'allowBuilds' | 'optional' | 'virtualStoreDir'🤖 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 `@installing/commands/src/install.ts` at line 68, The InstallCommandOptions type is missing the trustLockfile property that corresponds to the new 'trust-lockfile' rc option allowed by rcOptionsTypes(); update the InstallCommandOptions definition to include trustLockfile?: boolean (camelCase) and ensure any code that builds or destructures InstallCommandOptions (e.g., the handler that receives options from rcOptionsTypes()/parse logic and any uses in installCommand/handleInstall) maps the 'trust-lockfile' flag into that property so the handler boundary stays in sync with the CLI option surface.
🤖 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.
Inline comments:
In `@pacquet/crates/cli/tests/lockfile_verification.rs`:
- Around line 148-152: Add an assertion that the command actually succeeded
before checking stderr: after the existing `let stderr =
String::from_utf8_lossy(&output.stderr).into_owned();` (and before
`assert!(!stderr.contains("ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION"), ...)`) add
`assert!(output.status.success(), "pacquet install failed: {:?}", output);` to
both trust-lockfile happy-path tests that use the `output` and `stderr`
variables (the blocks containing the `stderr`/`assert!(!stderr.contains(...))`
checks at the shown location and the similar block at lines ~202-206) so the
test fails if the command failed for any other reason.
In `@pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`:
- Around line 555-558: Change project_trust_meta to take a borrowed &Package
rather than owning Package and update its implementation to clone only the
retained fields; in the fast path where you currently do
project_trust_meta(meta.as_ref().clone()) (inside the shared-cache branch using
self.meta_cache / shared / meta) call project_trust_meta(meta.as_ref()) so you
avoid cloning the full packument, and update any other callers of
project_trust_meta to pass a reference or explicitly clone only needed fields.
---
Nitpick comments:
In `@installing/commands/src/install.ts`:
- Line 68: The InstallCommandOptions type is missing the trustLockfile property
that corresponds to the new 'trust-lockfile' rc option allowed by
rcOptionsTypes(); update the InstallCommandOptions definition to include
trustLockfile?: boolean (camelCase) and ensure any code that builds or
destructures InstallCommandOptions (e.g., the handler that receives options from
rcOptionsTypes()/parse logic and any uses in installCommand/handleInstall) maps
the 'trust-lockfile' flag into that property so the handler boundary stays in
sync with the CLI option surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a624ef34-9257-4440-9542-d08f4cf3b537
📒 Files selected for processing (31)
.changeset/trust-lockfile-and-verifier-memory.mdconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsconfig/reader/src/types.tsinstalling/client/src/index.tsinstalling/commands/src/add.tsinstalling/commands/src/install.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/test/install/trustLockfile.tspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/src/index.ts
📜 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). (3)
- GitHub Check: ubuntu-latest / Node.js 24 / Test
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
config/reader/src/configFileKey.tsconfig/reader/src/types.tsinstalling/commands/src/install.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/commands/src/installDeps.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/client/src/index.tsinstalling/deps-installer/test/install/trustLockfile.tsconfig/reader/src/Config.tsinstalling/commands/src/add.tsresolving/npm-resolver/src/index.tsinstalling/deps-installer/src/install/index.tsresolving/default-resolver/src/index.tsconfig/reader/src/localConfig.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/config/src/env_overlay.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install/tests.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/cli/tests/lockfile_verification.rs
🧠 Learnings (4)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
config/reader/src/configFileKey.tsconfig/reader/src/types.tsinstalling/commands/src/install.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/commands/src/installDeps.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/client/src/index.tsinstalling/deps-installer/test/install/trustLockfile.tsconfig/reader/src/Config.tsinstalling/commands/src/add.tsresolving/npm-resolver/src/index.tsinstalling/deps-installer/src/install/index.tsresolving/default-resolver/src/index.tsconfig/reader/src/localConfig.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 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/config/src/env_overlay.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install/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/config/src/env_overlay.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install/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/config/src/env_overlay.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install/tests.rs
🔇 Additional comments (30)
installing/deps-installer/test/install/trustLockfile.ts (1)
1-55: LGTM!pacquet/crates/cli/tests/lockfile_verification.rs (1)
97-147: LGTM!Also applies to: 154-200, 208-209
pacquet/crates/package-manager/src/install/tests.rs (1)
64-64: LGTM!Also applies to: 132-132, 204-204, 269-269, 351-351, 418-418, 505-505, 649-649, 749-749, 866-866, 988-988, 1085-1085, 1190-1190, 1239-1239, 1330-1330, 1423-1423, 1486-1486, 1550-1550, 1640-1640, 1746-1746, 1806-1806, 1893-1893, 1970-1970, 2053-2053, 2254-2254, 2377-2377, 2476-2476, 2581-2581, 2671-2671, 2764-2764, 2854-2854, 2941-2941, 3034-3034, 3125-3125, 3222-3222, 3321-3321, 3406-3406, 3489-3489, 3558-3558, 3623-3623, 3687-3687, 3754-3754, 3837-3837, 3898-3898, 3982-3982, 4032-4032, 4102-4102, 4173-4173, 4238-4238
pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
78-78: LGTM!pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs (1)
50-50: LGTM!.changeset/trust-lockfile-and-verifier-memory.md (1)
1-11: LGTM!config/reader/src/Config.ts (1)
270-270: LGTM!config/reader/src/configFileKey.ts (1)
64-64: LGTM!config/reader/src/localConfig.ts (1)
89-89: LGTM!config/reader/src/types.ts (1)
122-122: LGTM!pacquet/crates/config/src/lib.rs (1)
790-807: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
287-293: LGTM!Also applies to: 616-618
pacquet/crates/config/src/workspace_yaml/tests.rs (1)
868-869: LGTM!Also applies to: 883-884, 899-900
installing/commands/src/recursive.ts (1)
91-91: LGTM!installing/commands/src/remove.ts (1)
148-148: LGTM!installing/deps-installer/src/install/extendInstallOptions.ts (1)
216-233: LGTM!resolving/npm-resolver/src/index.ts (1)
217-217: LGTM!Also applies to: 1075-1087
resolving/default-resolver/src/index.ts (1)
11-23: LGTM!Also applies to: 36-38, 190-190, 233-233
installing/client/src/index.ts (1)
13-20: LGTM!Also applies to: 73-85
pacquet/crates/package-manager/src/build_resolution_verifiers.rs (1)
28-29: LGTM!Also applies to: 61-72, 122-123
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs (1)
100-107: LGTM!Also applies to: 136-137, 218-219
resolving/npm-resolver/src/createNpmResolutionVerifier.ts (7)
83-92: LGTM!Also applies to: 165-165
381-413: LGTM!
415-452: LGTM!
468-490: LGTM!
610-611: LGTM!
618-641: LGTM!
643-675: LGTM!pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (2)
129-135: LGTM!
277-278: LGTM!Also applies to: 341-341, 382-382, 577-577
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@installing/commands/src/install.ts`:
- Line 68: The help output for the install command is missing the new flag even
though parsing includes 'trust-lockfile'; update the help/options list used by
the install command to include the '--trust-lockfile' entry so it shows in `pnpm
install --help` — locate the options/help array or object in
installing/commands/src/install.ts (the same module that defines the parsed
option 'trust-lockfile') and add a brief description and flag name for
'--trust-lockfile' alongside the other flags so it is displayed in the CLI help.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 325146c1-6a28-4a58-b019-02d0e1ce4e9a
📒 Files selected for processing (31)
.changeset/trust-lockfile-and-verifier-memory.mdconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsconfig/reader/src/types.tsinstalling/client/src/index.tsinstalling/commands/src/add.tsinstalling/commands/src/install.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/test/install/trustLockfile.tspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/src/index.ts
📜 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). (1)
- GitHub Check: ubuntu-latest / Node.js 24 / Test
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
installing/commands/src/add.tsinstalling/commands/src/remove.tsconfig/reader/src/localConfig.tsinstalling/commands/src/installDeps.tsconfig/reader/src/types.tsconfig/reader/src/Config.tsresolving/default-resolver/src/index.tsinstalling/deps-installer/src/install/index.tsresolving/npm-resolver/src/index.tsconfig/reader/src/configFileKey.tsinstalling/commands/src/recursive.tsinstalling/commands/src/install.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/test/install/trustLockfile.tsinstalling/client/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/cli/tests/lockfile_verification.rs
🧠 Learnings (4)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
installing/commands/src/add.tsinstalling/commands/src/remove.tsconfig/reader/src/localConfig.tsinstalling/commands/src/installDeps.tsconfig/reader/src/types.tsconfig/reader/src/Config.tsresolving/default-resolver/src/index.tsinstalling/deps-installer/src/install/index.tsresolving/npm-resolver/src/index.tsconfig/reader/src/configFileKey.tsinstalling/commands/src/recursive.tsinstalling/commands/src/install.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/test/install/trustLockfile.tsinstalling/client/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 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-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.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-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.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-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
🔇 Additional comments (39)
installing/commands/src/add.ts (1)
82-82: LGTM!installing/commands/src/remove.ts (1)
148-148: LGTM!pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs (1)
50-50: LGTM!pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
78-78: LGTM!config/reader/src/localConfig.ts (1)
89-90: LGTM!pacquet/crates/config/src/env_overlay.rs (1)
173-173: LGTM!installing/commands/src/installDeps.ts (1)
108-108: LGTM!pacquet/crates/package-manager/src/add.rs (1)
108-108: LGTM!config/reader/src/types.ts (1)
122-122: LGTM!.changeset/trust-lockfile-and-verifier-memory.md (1)
1-12: LGTM!config/reader/src/Config.ts (1)
270-270: LGTM!resolving/default-resolver/src/index.ts (1)
11-11: LGTM!Also applies to: 36-38, 190-190, 233-233
installing/deps-installer/src/install/index.ts (1)
386-386: LGTM!resolving/npm-resolver/src/index.ts (1)
217-217: LGTM!Also applies to: 1075-1087
pacquet/crates/package-manager/src/build_resolution_verifiers.rs (1)
28-28: LGTM!Also applies to: 61-67, 71-71, 122-122
config/reader/src/configFileKey.ts (1)
64-64: LGTM!installing/commands/src/recursive.ts (1)
91-91: LGTM!pacquet/crates/config/src/lib.rs (1)
790-806: LGTM!installing/deps-installer/src/install/extendInstallOptions.ts (1)
216-233: LGTM!pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
129-134: LGTM!Also applies to: 277-277, 577-577
installing/deps-installer/test/install/trustLockfile.ts (1)
1-55: LGTM!installing/client/src/index.ts (1)
13-13: LGTM!Also applies to: 73-79, 84-84
pacquet/crates/package-manager/src/install.rs (1)
31-31: LGTM!Also applies to: 101-109, 300-300, 399-423, 678-678
pacquet/crates/package-manager/src/install/tests.rs (1)
64-4238: LGTM!pacquet/crates/cli/src/cli_args/install.rs (1)
170-175: LGTM!Also applies to: 193-193, 224-229, 255-255
pacquet/crates/config/src/workspace_yaml.rs (1)
287-293: LGTM!Also applies to: 616-618
pacquet/crates/cli/tests/lockfile_verification.rs (2)
148-152: Assert command success before checking stderr contents.Both trust-lockfile tests only verify that stderr doesn't contain the violation code, but don't assert that the command actually succeeded. This could mask failures from unrelated causes.
Also applies to: 202-206
96-147: LGTM!Also applies to: 157-201
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs (4)
555-558: Avoid cloning the full packument in the shared-cache trust fast path.Line 557 clones the entire
Packageviameta.as_ref().clone()to satisfyproject_trust_meta(meta: Package). The projection immediately discards most fields (dependency graphs, scripts, READMEs). Changingproject_trust_metato take&Packagewould let it clone only the retained fields (name,time,modified,etag, and per-version trust evidence).
32-32: LGTM!Also applies to: 44-44
100-107: LGTM!Also applies to: 136-136, 218-218
680-736: LGTM!pacquet/crates/config/src/workspace_yaml/tests.rs (1)
868-868: LGTM!Also applies to: 883-883, 899-899
resolving/npm-resolver/src/createNpmResolutionVerifier.ts (6)
5-5: LGTM!Also applies to: 21-22
83-92: LGTM!
165-165: LGTM!
381-413: LGTM!
415-452: LGTM!
468-490: LGTM!Also applies to: 610-676
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@resolving/npm-resolver/src/createNpmResolutionVerifier.ts`:
- Around line 381-409: The shared packument cache is being looked up using only
`${name}:full` which can return metadata from the wrong registry; update the
cache key usages in createNpmResolutionVerifier (where it does
sharedMetaCache?.get(`${name}:full`) and any corresponding set/lookup sites
around the cachedPromise / fetchFullMetadataCached -> projectTrustMeta flow) to
include the registry (e.g. `${registry}\x00${name}:full`) or otherwise validate
the cached entry’s source registry before reuse so that projectTrustMeta runs
against the correct registry's history and minimumReleaseAge/trustPolicy.
In `@resolving/npm-resolver/src/index.ts`:
- Line 217: The current assignment const metaCache: PackageMetaCache =
opts.metaCache ?? createDefaultPackageMetaCache() makes it impossible to know
whether this instance owns the cache and causes clearCache() to clear
caller-provided caches; change to track ownership by creating a boolean like
metaCacheOwned = opts.metaCache ? false : true and assign metaCache =
opts.metaCache ?? createDefaultPackageMetaCache(); then update the clearCache()
implementation to only call metaCache.clear() when metaCacheOwned is true (leave
caller-provided metaCache untouched). Ensure symbols referenced: metaCache,
opts.metaCache, createDefaultPackageMetaCache, and clearCache are used so
reviewers can locate and verify the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e66b715-9ec6-44e4-a2b0-450e6c7fecfd
📒 Files selected for processing (31)
.changeset/trust-lockfile-and-verifier-memory.mdconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsconfig/reader/src/types.tsinstalling/client/src/index.tsinstalling/commands/src/add.tsinstalling/commands/src/install.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/test/install/trustLockfile.tspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/src/index.ts
✅ Files skipped from review due to trivial changes (3)
- pacquet/crates/package-manager/src/add.rs
- pacquet/crates/config/src/workspace_yaml/tests.rs
- .changeset/trust-lockfile-and-verifier-memory.md
📜 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). (8)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Dylint
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
config/reader/src/types.tsconfig/reader/src/Config.tsinstalling/commands/src/remove.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsinstalling/commands/src/install.tsinstalling/deps-installer/test/install/trustLockfile.tsinstalling/commands/src/add.tsinstalling/commands/src/recursive.tsinstalling/commands/src/installDeps.tsresolving/npm-resolver/src/index.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsinstalling/client/src/index.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/cli/tests/lockfile_verification.rs
🧠 Learnings (4)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
config/reader/src/types.tsconfig/reader/src/Config.tsinstalling/commands/src/remove.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsinstalling/commands/src/install.tsinstalling/deps-installer/test/install/trustLockfile.tsinstalling/commands/src/add.tsinstalling/commands/src/recursive.tsinstalling/commands/src/installDeps.tsresolving/npm-resolver/src/index.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsinstalling/client/src/index.ts
📚 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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/lib.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
🔇 Additional comments (16)
config/reader/src/types.ts (1)
122-122: LGTM!config/reader/src/Config.ts (1)
270-270: LGTM!installing/commands/src/remove.ts (1)
148-148: LGTM!pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
78-78: LGTM!config/reader/src/configFileKey.ts (1)
64-64: LGTM!config/reader/src/localConfig.ts (1)
89-89: LGTM!installing/commands/src/install.ts (1)
68-68: LGTM!Also applies to: 234-237, 332-332
installing/deps-installer/test/install/trustLockfile.ts (1)
1-55: LGTM!pacquet/crates/config/src/env_overlay.rs (1)
173-173: LGTM!installing/commands/src/add.ts (1)
82-82: LGTM!installing/commands/src/recursive.ts (1)
91-91: LGTM!installing/commands/src/installDeps.ts (1)
108-108: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
287-293: LGTM!Also applies to: 616-618
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs (1)
50-50: LGTM!pacquet/crates/package-manager/src/build_resolution_verifiers.rs (1)
28-28: LGTM!Also applies to: 61-67, 71-71, 122-122
pacquet/crates/config/src/lib.rs (1)
790-807: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@pacquet/crates/cli/tests/lockfile_verification.rs`:
- Around line 97-103: Update the doc comments above the #[test] that currently
read "installs cleanly" to state the narrower contract: this test verifies that
setting trustLockfile: true short-circuits the verification gate (i.e., the
verification step is skipped) and that no MINIMUM_RELEASE_AGE_VIOLATION error is
emitted to stderr, rather than asserting a full successful install; apply the
same rewording for the similar comment at the other location (lines 167-170) so
the docs document the test precondition and the exact postcondition
(verification skipped and no specific verification error).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 05a5a85e-b0e4-46b2-a954-dbb812fe5a05
📒 Files selected for processing (31)
.changeset/trust-lockfile-and-verifier-memory.mdconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsconfig/reader/src/types.tsinstalling/client/src/index.tsinstalling/commands/src/add.tsinstalling/commands/src/install.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/test/install/trustLockfile.tspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/src/index.ts
✅ Files skipped from review due to trivial changes (4)
- .changeset/trust-lockfile-and-verifier-memory.md
- pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs
- installing/commands/src/installDeps.ts
- installing/commands/src/remove.ts
📜 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). (8)
- GitHub Check: Audit dependencies
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Analyze (javascript)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
config/reader/src/Config.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsconfig/reader/src/localConfig.tsresolving/default-resolver/src/index.tsconfig/reader/src/types.tsinstalling/commands/src/recursive.tsinstalling/commands/src/add.tsresolving/npm-resolver/src/index.tsinstalling/commands/src/install.tsinstalling/client/src/index.tsinstalling/deps-installer/test/install/trustLockfile.tsconfig/reader/src/configFileKey.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/config/src/env_overlay.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install/tests.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/cli/tests/lockfile_verification.rs
🧠 Learnings (5)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
config/reader/src/Config.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsconfig/reader/src/localConfig.tsresolving/default-resolver/src/index.tsconfig/reader/src/types.tsinstalling/commands/src/recursive.tsinstalling/commands/src/add.tsresolving/npm-resolver/src/index.tsinstalling/commands/src/install.tsinstalling/client/src/index.tsinstalling/deps-installer/test/install/trustLockfile.tsconfig/reader/src/configFileKey.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 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/config/src/env_overlay.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install/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/config/src/env_overlay.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install/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/config/src/env_overlay.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install/tests.rs
📚 Learning: 2026-05-23T17:29:56.247Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: resolving/npm-resolver/src/createNpmResolutionVerifier.ts:381-418
Timestamp: 2026-05-23T17:29:56.247Z
Learning: When reviewing the npm resolver code, note that `PackageMetaCache` is intentionally keyed only by `name` and `name:full` (no registry component). As a result, code that shares this cache (e.g., `createNpmResolutionVerifier.ts` via the shared `validateSharedMeta` guard) can prevent cross-package contamination by matching names, but it cannot distinguish two different registries that serve packages with the same name within a single install. Don’t flag (or attempt to fix) this as a local issue in the verifier alone—correctly distinguishing registries would require a coordinated change to the resolver cache key shape. (The Pacquet/Rust cache is already registry-qualified, unlike the npm-resolver cache.)
Applied to files:
resolving/npm-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
🔇 Additional comments (27)
config/reader/src/Config.ts (1)
270-270: LGTM!pacquet/crates/config/src/env_overlay.rs (1)
173-173: LGTM!pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
78-78: LGTM!installing/deps-installer/src/install/index.ts (1)
378-387: LGTM!installing/deps-installer/src/install/extendInstallOptions.ts (1)
216-233: LGTM!config/reader/src/localConfig.ts (1)
84-93: LGTM!resolving/default-resolver/src/index.ts (1)
11-12: LGTM!Also applies to: 36-38, 180-191, 220-234
config/reader/src/types.ts (1)
122-123: LGTM!pacquet/crates/config/src/lib.rs (1)
790-806: LGTM!pacquet/crates/config/src/workspace_yaml/tests.rs (1)
868-868: LGTM!Also applies to: 883-883, 899-899
installing/commands/src/recursive.ts (1)
91-91: LGTM!installing/commands/src/add.ts (1)
82-82: LGTM!resolving/npm-resolver/src/index.ts (1)
217-225: LGTM!Also applies to: 288-290, 1083-1095
installing/commands/src/install.ts (1)
68-68: LGTM!Also applies to: 234-237, 332-332
pacquet/crates/package-manager/src/add.rs (1)
108-108: LGTM!installing/client/src/index.ts (1)
13-13: LGTM!Also applies to: 73-80, 84-84
installing/deps-installer/test/install/trustLockfile.ts (1)
1-55: LGTM!pacquet/crates/package-manager/src/build_resolution_verifiers.rs (1)
27-28: LGTM!Also applies to: 58-72, 119-124
config/reader/src/configFileKey.ts (1)
64-64: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
287-292: LGTM!Also applies to: 616-618
pacquet/crates/cli/src/cli_args/install.rs (1)
169-176: LGTM!Also applies to: 193-194, 224-229, 255-255
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
129-135: LGTM!Also applies to: 277-278, 341-341, 382-382, 577-577
pacquet/crates/package-manager/src/install.rs (1)
31-31: LGTM!Also applies to: 101-109, 300-300, 406-435, 678-678
resolving/npm-resolver/src/createNpmResolutionVerifier.ts (1)
5-5: LGTM!Also applies to: 21-22, 29-95, 160-171, 373-421, 424-461, 463-526, 603-650, 652-709
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs (1)
32-32: LGTM!Also applies to: 44-44, 100-107, 136-136, 218-218, 548-571, 683-744
pacquet/crates/cli/tests/lockfile_verification.rs (1)
104-165: LGTM!Also applies to: 171-223
pacquet/crates/package-manager/src/install/tests.rs (1)
64-4032: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`:
- Around line 716-743: The projection currently clones the entire npm_user into
project_trust_package_version which retains unnecessary per-version publisher
metadata; instead construct a new/npm_user value that preserves only the trust
signal (trusted_publisher) from version.npm_user and omit or set all other
npm_user fields to empty/None/default so the projected PackageVersion keeps only
the trust flag used by fail_if_trust_downgraded (update the npm_user assignment
in project_trust_package_version to map version.npm_user -> a new
pacquet_registry::NpmUser containing only trusted_publisher cloned from the
source).
In `@resolving/npm-resolver/src/createNpmResolutionVerifier.ts`:
- Around line 448-460: The projectTrustManifest function currently keeps the
entire _npmUser object which retains per-version data/PII; update it so _npmUser
is projected to only include the trustedPublisher field (e.g. set _npmUser:
manifest._npmUser ? { trustedPublisher: manifest._npmUser.trustedPublisher } :
undefined) while preserving the existing dist.attestations.provenance logic and
the overall cast back to PackageInRegistry; modify the return shape in
projectTrustManifest accordingly so only trustedPublisher is carried into the
trust cache.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 74a98022-78f8-47d6-afcb-1e6e340ee70b
📒 Files selected for processing (31)
.changeset/trust-lockfile-and-verifier-memory.mdconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsconfig/reader/src/types.tsinstalling/client/src/index.tsinstalling/commands/src/add.tsinstalling/commands/src/install.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/test/install/trustLockfile.tspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/src/index.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/trust-lockfile-and-verifier-memory.md
📜 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). (6)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Code Coverage
- GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
installing/commands/src/remove.tsconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/types.tsinstalling/commands/src/add.tsconfig/reader/src/localConfig.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsresolving/npm-resolver/src/index.tsinstalling/client/src/index.tsinstalling/deps-installer/test/install/trustLockfile.tsresolving/default-resolver/src/index.tsinstalling/commands/src/install.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/cli/tests/lockfile_verification.rs
🧠 Learnings (5)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
installing/commands/src/remove.tsconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/types.tsinstalling/commands/src/add.tsconfig/reader/src/localConfig.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsresolving/npm-resolver/src/index.tsinstalling/client/src/index.tsinstalling/deps-installer/test/install/trustLockfile.tsresolving/default-resolver/src/index.tsinstalling/commands/src/install.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
📚 Learning: 2026-05-23T17:29:56.247Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: resolving/npm-resolver/src/createNpmResolutionVerifier.ts:381-418
Timestamp: 2026-05-23T17:29:56.247Z
Learning: When reviewing the npm resolver code, note that `PackageMetaCache` is intentionally keyed only by `name` and `name:full` (no registry component). As a result, code that shares this cache (e.g., `createNpmResolutionVerifier.ts` via the shared `validateSharedMeta` guard) can prevent cross-package contamination by matching names, but it cannot distinguish two different registries that serve packages with the same name within a single install. Don’t flag (or attempt to fix) this as a local issue in the verifier alone—correctly distinguishing registries would require a coordinated change to the resolver cache key shape. (The Pacquet/Rust cache is already registry-qualified, unlike the npm-resolver cache.)
Applied to files:
resolving/npm-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
🔇 Additional comments (25)
installing/commands/src/remove.ts (1)
148-148: LGTM!pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
78-78: LGTM!config/reader/src/Config.ts (1)
270-270: LGTM!config/reader/src/configFileKey.ts (1)
64-64: LGTM!pacquet/crates/package-manager/src/add.rs (1)
108-108: LGTM!config/reader/src/types.ts (1)
122-122: LGTM!installing/commands/src/add.ts (1)
82-82: LGTM!config/reader/src/localConfig.ts (1)
89-89: LGTM!pacquet/crates/config/src/lib.rs (1)
790-807: LGTM!installing/deps-installer/src/install/index.ts (1)
386-386: LGTM!pacquet/crates/config/src/env_overlay.rs (1)
173-173: LGTM!installing/deps-installer/src/install/extendInstallOptions.ts (1)
216-233: LGTM!installing/commands/src/installDeps.ts (1)
108-108: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
287-292: LGTM!Also applies to: 616-618
pacquet/crates/package-manager/src/build_resolution_verifiers.rs (1)
28-29: LGTM!Also applies to: 61-72, 122-123
installing/commands/src/recursive.ts (1)
91-91: LGTM!resolving/npm-resolver/src/index.ts (1)
217-225: LGTM!Also applies to: 288-289, 1083-1095
installing/client/src/index.ts (1)
13-13: LGTM!Also applies to: 73-85
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs (1)
50-50: LGTM!pacquet/crates/config/src/workspace_yaml/tests.rs (1)
868-868: LGTM!Also applies to: 883-883, 899-899
pacquet/crates/cli/tests/lockfile_verification.rs (1)
97-167: LGTM!Also applies to: 169-228
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
129-134: LGTM!Also applies to: 277-277, 341-343, 382-384, 575-579
pacquet/crates/cli/src/cli_args/install.rs (1)
170-175: LGTM!Also applies to: 193-194, 224-229, 255-255
installing/deps-installer/test/install/trustLockfile.ts (1)
1-55: LGTM!pacquet/crates/package-manager/src/install.rs (1)
412-421: ⚡ Quick winRemove the proposed
meta_cacheearly-drop change: the outerArcisn’t kept alive past the fresh-installer handoff
pacquet/crates/package-manager/src/install.rsonly usesmeta_cachefor verifier construction and forInstallWithFreshLockfile { meta_cache: Arc::clone(&meta_cache), ... }(e.g., around line 678). There are no furthermeta_cachereferences after that handoff, so the localmeta_cacheArcis eligible to be dropped once the clone is taken; the earlydrop(meta_cache)insideinstall_with_fresh_lockfile.rsshould still be effective when no other strong refs remain.> Likely an incorrect or invalid review comment.
Verifying a multi-thousand-entry lockfile against `minimumReleaseAge` or `trustPolicy: no-downgrade` retained every fetched packument in a per-install cache for the entire install. On large workspaces this OOM'd CI runners with a 2GB heap cap. Project both caches down to just the fields each check reads (per-version trust evidence + the `time` map for trust; package-level `modified` + version-name set for the abbreviated shortcut) so the bulk packument is GC'd as soon as the fetch returns. Also adds a `trustLockfile` setting (default `false`) that skips the verification pass entirely for environments where the lockfile is already part of the trusted base. Mirrored in pacquet. Closes #11860.
The verifier kept its own per-install dedup Maps and re-fetched every packument the resolver had already pulled during the same install. Plumb the resolver's per-install `PackageMetaCache` through to the verifier (via `createNpmResolutionVerifier` / `build_resolution_verifiers`) so a name already in the resolver's LRU short-circuits the verifier's disk/network round-trip — fast path only, the cached document is projected for the trust check so the verifier's memory footprint stays bounded. In pnpm, `installing/client` now constructs one LRU and hands it to both `createResolver` and `createResolutionVerifiers`. In pacquet, the `InMemoryPackageMetaCache` is lifted to `Install::dispatch` and passed to both `build_resolution_verifiers` and `InstallWithFreshLockfile`.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
installing/deps-installer/test/install/trustLockfile.ts (1)
8-16: ⚡ Quick winAssert that the verifier is never invoked when
trustLockfileis enabled.Right now the second test only checks that install resolves. It would still pass if the implementation kept calling the verifier and merely swallowed its failure, which wouldn't protect the OOM regression this flag is meant to avoid. Track calls on
canTrustPastCheck/verifyand assert they stay at zero in thetrustLockfile: truecase.Also applies to: 37-55
🤖 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 `@installing/deps-installer/test/install/trustLockfile.ts` around lines 8 - 16, The test must assert that the verifier is never invoked when trustLockfile is enabled: wrap the rejectingVerifier methods (canTrustPastCheck and verify) with call counters or spies and add assertions that their call counts remain zero in the test case that sets trustLockfile: true; update both the earlier test block and the similar case around lines 37-55 to track rejectingVerifier.canTrustPastCheck and rejectingVerifier.verify and assert zero calls after install resolves, ensuring the verifier isn't called or swallowed.
🤖 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 `@installing/deps-installer/test/install/trustLockfile.ts`:
- Around line 8-16: The test must assert that the verifier is never invoked when
trustLockfile is enabled: wrap the rejectingVerifier methods (canTrustPastCheck
and verify) with call counters or spies and add assertions that their call
counts remain zero in the test case that sets trustLockfile: true; update both
the earlier test block and the similar case around lines 37-55 to track
rejectingVerifier.canTrustPastCheck and rejectingVerifier.verify and assert zero
calls after install resolves, ensuring the verifier isn't called or swallowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 18f1320b-9067-4712-8112-5fdc04d26b26
📒 Files selected for processing (31)
.changeset/trust-lockfile-and-verifier-memory.mdconfig/reader/src/Config.tsconfig/reader/src/configFileKey.tsconfig/reader/src/localConfig.tsconfig/reader/src/types.tsinstalling/client/src/index.tsinstalling/commands/src/add.tsinstalling/commands/src/install.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/recursive.tsinstalling/commands/src/remove.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/test/install/trustLockfile.tspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/package-manager/src/add.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/default-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/src/index.ts
✅ Files skipped from review due to trivial changes (2)
- installing/commands/src/recursive.ts
- .changeset/trust-lockfile-and-verifier-memory.md
📜 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). (7)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Code Coverage
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (3)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/add.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
installing/commands/src/remove.tsconfig/reader/src/localConfig.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/add.tsconfig/reader/src/configFileKey.tsconfig/reader/src/types.tsinstalling/client/src/index.tsconfig/reader/src/Config.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/test/install/trustLockfile.tsresolving/default-resolver/src/index.tsinstalling/commands/src/install.tsresolving/npm-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/cli/tests/lockfile_verification.rs
🧠 Learnings (5)
📚 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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/add.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/add.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.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/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/workspace_yaml/tests.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/package-manager/src/add.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/cli/tests/lockfile_verification.rspacquet/crates/package-manager/src/build_resolution_verifiers.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
installing/commands/src/remove.tsconfig/reader/src/localConfig.tsinstalling/commands/src/installDeps.tsinstalling/commands/src/add.tsconfig/reader/src/configFileKey.tsconfig/reader/src/types.tsinstalling/client/src/index.tsconfig/reader/src/Config.tsinstalling/deps-installer/src/install/index.tsinstalling/deps-installer/src/install/extendInstallOptions.tsinstalling/deps-installer/test/install/trustLockfile.tsresolving/default-resolver/src/index.tsinstalling/commands/src/install.tsresolving/npm-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 Learning: 2026-05-23T17:29:56.247Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: resolving/npm-resolver/src/createNpmResolutionVerifier.ts:381-418
Timestamp: 2026-05-23T17:29:56.247Z
Learning: When reviewing the npm resolver code, note that `PackageMetaCache` is intentionally keyed only by `name` and `name:full` (no registry component). As a result, code that shares this cache (e.g., `createNpmResolutionVerifier.ts` via the shared `validateSharedMeta` guard) can prevent cross-package contamination by matching names, but it cannot distinguish two different registries that serve packages with the same name within a single install. Don’t flag (or attempt to fix) this as a local issue in the verifier alone—correctly distinguishing registries would require a coordinated change to the resolver cache key shape. (The Pacquet/Rust cache is already registry-qualified, unlike the npm-resolver cache.)
Applied to files:
resolving/npm-resolver/src/index.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
🔇 Additional comments (21)
pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
78-78: LGTM!installing/commands/src/remove.ts (1)
148-148: LGTM!config/reader/src/localConfig.ts (1)
89-89: LGTM!pacquet/crates/config/src/workspace_yaml/tests.rs (1)
868-868: LGTM!Also applies to: 883-883, 899-899
installing/commands/src/installDeps.ts (1)
108-108: LGTM!installing/commands/src/add.ts (1)
82-82: LGTM!config/reader/src/configFileKey.ts (1)
64-64: LGTM!config/reader/src/types.ts (1)
122-122: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
287-293: LGTM!Also applies to: 616-618
pacquet/crates/config/src/lib.rs (1)
790-807: LGTM!installing/client/src/index.ts (1)
13-20: LGTM!Also applies to: 73-85
config/reader/src/Config.ts (1)
270-270: LGTM!installing/deps-installer/src/install/index.ts (1)
378-387: LGTM!pacquet/crates/config/src/env_overlay.rs (1)
173-173: LGTM!pacquet/crates/package-manager/src/install/tests.rs (1)
64-64: LGTM!Also applies to: 132-132, 204-204, 269-269, 351-351, 418-418, 505-505, 649-649, 749-749, 866-866, 988-988, 1085-1085, 1190-1190, 1239-1239, 1330-1330, 1423-1423, 1486-1486, 1550-1550, 1640-1640, 1746-1746, 1806-1806, 1893-1893, 1970-1970, 2053-2053, 2254-2254, 2377-2377, 2476-2476, 2581-2581, 2671-2671, 2764-2764, 2854-2854, 2941-2941, 3034-3034, 3125-3125, 3222-3222, 3321-3321, 3406-3406, 3489-3489, 3558-3558, 3623-3623, 3687-3687, 3754-3754, 3837-3837, 3898-3898, 3982-3982, 4032-4032, 4102-4102, 4173-4173, 4238-4238
pacquet/crates/cli/tests/lockfile_verification.rs (1)
97-167: LGTM!Also applies to: 169-228
pacquet/crates/package-manager/src/build_resolution_verifiers.rs (1)
28-29: LGTM!Also applies to: 61-67, 68-72, 122-123
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
129-135: LGTM!Also applies to: 277-278
resolving/npm-resolver/src/index.ts (1)
217-225: LGTM!Also applies to: 288-289, 1083-1095
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs (1)
32-32: LGTM!Also applies to: 44-44, 100-107, 136-136, 218-218, 548-570, 683-752
resolving/npm-resolver/src/createNpmResolutionVerifier.ts (1)
5-5: LGTM!Also applies to: 21-21, 83-92, 165-165, 381-418, 424-464, 480-502, 622-622, 630-712
|
@zkochan I feel that with the new option |
Summary
Fixes #11860 —
pnpm install --frozen-lockfileOOMs CI runners (2GB heap) on workspaces with thousands of locked entries whenminimumReleaseAge/trustPolicy: no-downgradeare active.Memory fix (primary)
Verification already capped per-entry concurrency at 16; the actual OOM source was two unbounded per-install caches that retained the full registry packument (dependency graph, scripts, README, every per-version manifest) for every
(registry, name)pair seen — easily multiple GB across a 4k-entry lockfile. Both caches now store a projection of just the fields the corresponding check reads:fullMetaForTrustCache→name+time+ per-version_npmUser.trustedPublisher+dist.attestations.provenanceabbreviatedMetaCache→modified+Set<string>of currently-listed version namesThe full document is GC-able as soon as the fetch resolves.
trustLockfileopt-outNew
trustLockfilesetting (defaultfalse). Whentrue,pnpm installskips the verification pass entirely and trusts the on-disk lockfile as-is. Intended for environments where every commit comes from a trusted author (closed-source projects, CI runs against an already-verified lockfile) — outside contributors can still poison a lockfile under a weaker policy, so leave the defaultfalsefor projects with external collaborators.Shared resolver ↔ verifier packument cache
A separate follow-up commit plumbs the resolver's per-install
PackageMetaCacheLRU into the verifier (createNpmResolutionVerifier/ pacquet'sbuild_resolution_verifiers). The verifier'sfetchAbbreviatedMeta/fetchFullMetaForTrustconsult it before fetching: a(registry, name)the resolver already pulled in the same install yields the cached packument instead of a fresh disk/network round-trip. The verifier's local cache still stores only the projected form, so the memory fix is unaffected.The shared cache is cold when verification runs on the first install of a given workspace (verification runs before resolution), so the win primarily shows up in long-lived processes or in repeat installs that hit the same
(registry, name). Doesn't help the frozen-install path (no resolver runs) — which is the path the OOM hit; the memory fix above is what fixes that case.Mirrored in pacquet
Config.trust_lockfile+pnpm-workspace.yaml(trustLockfile) +PNPM_CONFIG_TRUST_LOCKFILEenvpacquet/crates/package-manager/src/install.rsshort-circuits the verifier whentrustLockfile: truecreate_npm_resolution_verifier.rsprojects to a strippedPackageviaproject_trust_metaInMemoryPackageMetaCachelifted toInstall::runand passed to bothbuild_resolution_verifiersandInstallWithFreshLockfileTest plan
cargo nextest run -p pacquet-resolving-npm-resolver— passes (verifier tests cover the projection round-trip via mocked registry)cargo nextest run -p pacquet-package-manager -p pacquet-lockfile-verification— passescargo nextest run -p pacquet-config— passes (extendedpnpm-workspace.yamlround-trip withtrustLockfile)cargo nextest run -p pacquet-cli --test lockfile_verification— new end-to-end testtrust_lockfile_skips_verificationpasses alongside the existing rejection testcargo clippy --workspace --all-targets -- --deny warningsRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-depspnpm --filter pnpm run compile(bundle rebuilds clean)minimumReleaseAgeto confirm heap stays bounded (would appreciate a tester running this against the failing fixture if reachable)Written by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit