feat(pnpr): offload lockfile verification to the server (#12139)#12144
Conversation
When a pnpr server is configured, the client no longer runs verifyLockfileResolutions locally. It sends its on-disk lockfile plus its full verification policy to /v1/install; pnpr verifies that input lockfile under the client's policy before resolving and streams back any violations so the client aborts with the identical ERR_PNPM_* code. - Protocol: /v1/install carries `lockfile`, `frozenLockfile`, and the full policy (minimumReleaseAge[Exclude|IgnoreMissingTime], trustPolicy[Exclude|IgnoreAfter]). - pnpr verifies the input lockfile before resolving; violations stream as a 200 NDJSON `E` line the client rebuilds into a VerifyError. - frozenLockfile governs server-side resolution reuse (frozen → as-is; non-frozen → reuse pins + resolve new). - Whole-lockfile verdict cache on pnpr (SQLite, keyed by lockfile hash + policy snapshot, reusing can_trust_past_check) short-circuits repeats. - pacquet client sends the lockfile + policy, drops the no-downgrade guard (pnpr enforces it now), and treats the server-produced lockfile as trusted during local materialization. Rust-only change (pacquet client + pnpr server); the TS agent server is deprecated and the TS client already skips local verification.
|
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:
📝 WalkthroughWalkthroughThis PR offloads input-lockfile verification to pnpr: the client sends its on-disk lockfile and full verification policy to POST /v1/install; pnpr verifies (with optional SQLite verdict cache) before resolving, returns rendered violations when present, and the client reconstructs VerifyError or trusts the server-produced lockfile for local materialization. ChangesVerification offload to pnpr
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Client
participant Server
participant Cache
CLI->>Client: prepare install options with lockfile + policy
Client->>Server: POST /v1/install (lockfile + policy)
Server->>Cache: lookup verdict by lockfile-hash + policy
alt cached compatible verdict
Cache-->>Server: cached verified
Server-->>Client: resolved lockfile (OK)
Client-->>CLI: proceed (trust server lockfile)
else cache miss or incompatible policy
Server->>Server: verify input lockfile using packument cache
alt verification fails
Server-->>Client: NDJSON E line with rendered violations
Client-->>CLI: surface VerifyError reconstructed from violations
else verification passes
Server->>Cache: record merged policy verdict
Server-->>Client: resolved lockfile (OK)
Client-->>CLI: proceed (trust server lockfile)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.06579634466,
"stddev": 0.05903169971631456,
"median": 2.05150971546,
"user": 2.5957056799999996,
"system": 3.3569274199999994,
"min": 1.9747862124600002,
"max": 2.16590949146,
"times": [
2.02600964746,
2.0407175514600002,
1.9747862124600002,
2.04153730146,
2.06148212946,
2.03323969846,
2.16590949146,
2.08814785746,
2.06718849446,
2.15894506246
]
},
{
"command": "pacquet@main",
"mean": 2.07503379686,
"stddev": 0.07542244545656995,
"median": 2.04920794496,
"user": 2.58486628,
"system": 3.4114705199999995,
"min": 1.97689497046,
"max": 2.22933651646,
"times": [
2.22933651646,
2.02846217446,
2.0931214534600002,
2.12707200046,
2.02448489846,
2.06904664146,
2.02453656746,
1.97689497046,
2.02936924846,
2.14801349746
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.66204822524,
"stddev": 0.015522979727966695,
"median": 0.65948955064,
"user": 0.36278853999999994,
"system": 1.3501168,
"min": 0.64608236314,
"max": 0.6997877421400001,
"times": [
0.6997877421400001,
0.66966507914,
0.66350812614,
0.66773822714,
0.65274291814,
0.66414788014,
0.64929967314,
0.65203926814,
0.64608236314,
0.65547097514
]
},
{
"command": "pacquet@main",
"mean": 0.66244970484,
"stddev": 0.02128315108461096,
"median": 0.65609214914,
"user": 0.36264474,
"system": 1.3482368,
"min": 0.64439831914,
"max": 0.70967623014,
"times": [
0.70967623014,
0.67415957814,
0.65586001514,
0.64578185114,
0.64781839514,
0.64759520214,
0.65632428314,
0.64439831914,
0.65680887014,
0.68607430414
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.3499057074,
"stddev": 0.02483031448252493,
"median": 2.350092598,
"user": 3.7337521800000006,
"system": 3.15154184,
"min": 2.3047442025000002,
"max": 2.3964318555000004,
"times": [
2.3047442025000002,
2.3569625535000003,
2.3399037155,
2.3284747705,
2.3507324545,
2.3441761275,
2.3518527865000003,
2.3964318555000004,
2.3763258665,
2.3494527415
]
},
{
"command": "pacquet@main",
"mean": 2.3575551693000003,
"stddev": 0.02170778213382734,
"median": 2.352548232,
"user": 3.71612468,
"system": 3.16336494,
"min": 2.3221081685000002,
"max": 2.3987317255000002,
"times": [
2.3514601975000002,
2.3536362665,
2.3480804115000002,
2.3722780265,
2.3987317255000002,
2.3781675855,
2.3221081685000002,
2.3380578245,
2.3486788885000003,
2.3643525985
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.5233950251600001,
"stddev": 0.028614280378405188,
"median": 1.5360007907600002,
"user": 1.7410067999999999,
"system": 1.86327566,
"min": 1.48443418176,
"max": 1.55350516276,
"times": [
1.54986407876,
1.54203432876,
1.54846114976,
1.51086048476,
1.48772863576,
1.55350516276,
1.54107717976,
1.4850606477600001,
1.48443418176,
1.53092440176
]
},
{
"command": "pacquet@main",
"mean": 1.5241075086600002,
"stddev": 0.028279198832270144,
"median": 1.51611585576,
"user": 1.7191918999999998,
"system": 1.8690156599999999,
"min": 1.50421147876,
"max": 1.60139687176,
"times": [
1.50421147876,
1.50998412976,
1.51715395676,
1.5056389697600001,
1.60139687176,
1.51682678976,
1.51539175776,
1.52986053476,
1.51540492176,
1.52520567576
]
}
]
} |
|
Bencher Report| Branch | pr/12144 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,349.91 ms(-0.46%)Baseline: 2,360.87 ms | 2,833.05 ms (82.95%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,523.40 ms(-0.09%)Baseline: 1,524.79 ms | 1,829.75 ms (83.26%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,065.80 ms(-0.59%)Baseline: 2,078.05 ms | 2,493.67 ms (82.84%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 662.05 ms(+0.51%)Baseline: 658.70 ms | 790.44 ms (83.76%) |
Review Summary by QodoOffload lockfile verification to pnpr server with policy-driven validation
WalkthroughsDescription• Offload lockfile verification from client to pnpr server • Send client's verification policy to server for input lockfile validation • Implement SQLite-backed whole-lockfile verdict cache on server • Support frozen vs reuse-and-update resolution modes via frozenLockfile • Remove client-side trustPolicy: no-downgrade guard; enforce server-side Diagramflowchart LR
Client["Client<br/>(pacquet)"]
Server["Server<br/>(pnpr)"]
Cache["Verdict Cache<br/>(SQLite)"]
Client -- "Send lockfile +<br/>verification policy" --> Server
Server -- "Verify input lockfile<br/>under client policy" --> Cache
Cache -- "Cache hit?<br/>O(1) lookup" --> Server
Server -- "Resolve + stream<br/>lockfile or violations" --> Client
Client -- "Rebuild VerifyError<br/>if violations" --> Client
File Changes1. pacquet/crates/cli/src/cli_args/install.rs
|
There was a problem hiding this comment.
Pull request overview
This PR moves lockfile-resolution verification off the pacquet client and into the pnpr server: the client sends its on-disk lockfile plus its full verification policy to /v1/install, the server verifies the input lockfile under the client’s policy before resolving, and verification failures are streamed back so the client can abort with the same ERR_PNPM_* diagnostic.
Changes:
- Extend the
/v1/installrequest to include the input lockfile,frozenLockfile, and the full verification policy; perform server-side input-lockfile verification and stream violations to the client. - Add a SQLite-backed whole-lockfile “pass verdict” cache on
pnprto short-circuit repeated verifications. - Update the pacquet
pnpr-client+ CLI to send lockfile/policy, reconstructVerifyErrorfrom server violations, and skip local lockfile verification on the pnpr path.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pnpr/crates/pnpr/src/install_accelerator/verdict_cache.rs |
Adds SQLite-backed whole-lockfile verification verdict cache used by the server to skip repeated verification. |
pnpr/crates/pnpr/src/install_accelerator/verdict_cache/tests.rs |
Unit tests for the new verdict cache behavior. |
pnpr/crates/pnpr/src/install_accelerator/resolve.rs |
Seeds resolution from an input lockfile and forces local materialization to trust the (server-verified) lockfile. |
pnpr/crates/pnpr/src/install_accelerator/protocol.rs |
Extends the install request wire schema with lockfile, frozenLockfile, and full verification policy fields. |
pnpr/crates/pnpr/src/install_accelerator.rs |
Implements server-side input-lockfile verification (with NDJSON violation streaming) and verdict-cache integration. |
pnpr/crates/pnpr/Cargo.toml |
Adds dependencies needed for verification and resolver/verifier integration in pnpr. |
pacquet/crates/pnpr-client/src/lib.rs |
Sends lockfile + policy to server; parses streamed violations and reconstructs VerifyError. |
pacquet/crates/pnpr-client/src/tests.rs |
Adds tests ensuring E-line violations rebuild into the correct VerifyError variants. |
pacquet/crates/pnpr-client/tests/integration.rs |
E2E tests for “clean lockfile accepted” and “policy-violating lockfile rejected with verification error”. |
pacquet/crates/pnpr-client/Cargo.toml |
Adds deps needed to serialize policy and reconstruct verification errors. |
pacquet/crates/lockfile-verification/src/lib.rs |
Re-exports RenderedViolation for client-side VerifyError reconstruction. |
pacquet/crates/config/src/lib.rs |
Makes TrustPolicy serializable so it can be sent over the wire. |
pacquet/crates/cli/src/cli_args/install.rs |
Updates pnpr install path to send lockfile + full policy, and surface server verification failures as local-style diagnostics. |
Cargo.lock |
Lockfile updates for added Rust dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pacquet/crates/cli/src/cli_args/install.rs (1)
320-327:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreserve the install-mode flags on the pnpr fast path.
Only
frozen_lockfileis threaded intoPnprLink. The downstream install still hardcodeslockfile_only: falseandignore_manifest_check: false, and the server never seesprefer_frozen_lockfile, so--lockfile-only,--ignore-manifest-check, and--prefer-frozen-lockfilesilently stop working wheneverpnpr_serveris enabled.Also applies to: 389-393
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pacquet/crates/cli/src/cli_args/install.rs` around lines 320 - 327, The Pnpr fast path only threads frozen_lockfile into the PnprLink payload, causing --lockfile-only, --ignore-manifest-check, and --prefer-frozen-lockfile to be dropped; update the PnprLink construction (the PnprLink initializer shown around the dependency_groups/frozen_lockfile block and the analogous block at lines ~389-393) to include the missing install-mode flags (lockfile_only, ignore_manifest_check, prefer_frozen_lockfile or whatever flag names the CLI uses) and pass the corresponding variables used elsewhere in install.rs so the server and downstream install honor those CLI flags when pnpr_server is enabled.pnpr/crates/pnpr/src/install_accelerator/resolve.rs (1)
100-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
frozen_lockfilewhen the request has no lockfile.
request.frozen_lockfile && input_lockfile.is_some()downgrades a frozen install into a normal resolve whenever the client has no on-disk lockfile. That changes the contract for--frozen-lockfile: instead of failing, the server can synthesize a new lockfile. Pass the client'sfrozen_lockfilethrough unchanged and let pacquet surface the expected frozen-lockfile error for the missing-lockfile case.🤖 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 `@pnpr/crates/pnpr/src/install_accelerator/resolve.rs` around lines 100 - 121, The code currently computes frozen_lockfile as request.frozen_lockfile && input_lockfile.is_some(), which clears the client's --frozen-lockfile flag when no on-disk lockfile exists; change the logic to preserve the client's setting by assigning frozen_lockfile = request.frozen_lockfile and pass that into the Install construction (identify the frozen_lockfile local and the Install { ... frozen_lockfile, ... } initializer) so the resolver will not synthesize a lockfile and will let pacquet surface the proper missing-lockfile error.
🤖 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/src/cli_args/install.rs`:
- Around line 428-453: The install call always forces server-side lockfile
verification; honor the effective trust_lockfile from run() instead. In the
PnprClient::new(...).install(InstallOptions { ... }) call (and surrounding logic
that computes dependencies, lockfile, and link.frozen_lockfile), short-circuit
by detecting the effective trust_lockfile flag and either (A) skip
sending/asking the server to verify the input lockfile (i.e. call the local
non-pnpr install path or pass an InstallOptions variant that indicates "skip
verification"), or (B) extend InstallOptions / the pnpr protocol to include a
trust_lockfile boolean and forward that to the server so it can skip
verification; update the call-site to pass the effective flag from run() and
adjust PnprClient::install handling accordingly. Ensure the symbol references to
PnprClient::new, PnprClient::install, InstallOptions, and the run()-computed
trust_lockfile are used to locate and change the code.
In `@pacquet/crates/lockfile-verification/src/lib.rs`:
- Line 34: The crate-level docs (the top-of-file `//!` "Public surface today"
list) are now stale because `RenderedViolation` was re-exported via `pub use
errors::{RenderedViolation, VerifyError};`; update the crate-level public API
documentation in lib.rs to reflect the new exports by adding `RenderedViolation`
(and `VerifyError` if missing) to that `//! Public surface today` list or
replace the list with an accurate prose description of the public contract;
locate the crate-level doc comment near the top of lib.rs and ensure the docs
match the actual public symbols (`RenderedViolation`, `VerifyError`) exported by
the module.
In `@pacquet/crates/pnpr-client/src/lib.rs`:
- Around line 66-73: The InstallOptions wire type currently allows
minimum_release_age_ignore_missing_time to be None which serializes to null;
change the InstallOptions field minimum_release_age_ignore_missing_time from
Option<bool> to bool so the client always sends an explicit true/false (matching
pacquet_config::Config's concrete bool). Update any constructors/builders that
create InstallOptions to supply a boolean (derive it from pacquet_config::Config
or choose the appropriate default), and ensure serialization to /v1/install no
longer emits null for minimum_release_age_ignore_missing_time; keep the
server-side verifier expecting a concrete bool.
In `@pnpr/crates/pnpr/src/install_accelerator/verdict_cache.rs`:
- Around line 52-56: The table lockfile_verdicts and its accessors currently use
hash as the primary key causing overwrites across policies; change the schema to
make the compound primary key (hash, policy) and update the upsert in record()
to upsert by (hash, policy) instead of hash alone, and similarly adjust any
SELECT/DELETE/lookup logic that assumes a single-row-per-hash (see the table
creation and the code paths referenced around record() and the block referenced
at lines ~96-100) so verdicts are stored and retrieved per (hash, policy) pair.
---
Outside diff comments:
In `@pacquet/crates/cli/src/cli_args/install.rs`:
- Around line 320-327: The Pnpr fast path only threads frozen_lockfile into the
PnprLink payload, causing --lockfile-only, --ignore-manifest-check, and
--prefer-frozen-lockfile to be dropped; update the PnprLink construction (the
PnprLink initializer shown around the dependency_groups/frozen_lockfile block
and the analogous block at lines ~389-393) to include the missing install-mode
flags (lockfile_only, ignore_manifest_check, prefer_frozen_lockfile or whatever
flag names the CLI uses) and pass the corresponding variables used elsewhere in
install.rs so the server and downstream install honor those CLI flags when
pnpr_server is enabled.
In `@pnpr/crates/pnpr/src/install_accelerator/resolve.rs`:
- Around line 100-121: The code currently computes frozen_lockfile as
request.frozen_lockfile && input_lockfile.is_some(), which clears the client's
--frozen-lockfile flag when no on-disk lockfile exists; change the logic to
preserve the client's setting by assigning frozen_lockfile =
request.frozen_lockfile and pass that into the Install construction (identify
the frozen_lockfile local and the Install { ... frozen_lockfile, ... }
initializer) so the resolver will not synthesize a lockfile and will let pacquet
surface the proper missing-lockfile 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: 5a577c66-c107-4778-89e1-53ceca302633
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
pacquet/crates/cli/src/cli_args/install.rspacquet/crates/config/src/lib.rspacquet/crates/lockfile-verification/src/lib.rspacquet/crates/pnpr-client/Cargo.tomlpacquet/crates/pnpr-client/src/lib.rspacquet/crates/pnpr-client/src/tests.rspacquet/crates/pnpr-client/tests/integration.rspnpr/crates/pnpr/Cargo.tomlpnpr/crates/pnpr/src/install_accelerator.rspnpr/crates/pnpr/src/install_accelerator/protocol.rspnpr/crates/pnpr/src/install_accelerator/resolve.rspnpr/crates/pnpr/src/install_accelerator/verdict_cache.rspnpr/crates/pnpr/src/install_accelerator/verdict_cache/tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/lockfile-verification/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspacquet/crates/config/src/lib.rspacquet/crates/pnpr-client/src/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.rs
pnpr/**/pnpr/**/*.rs
📄 CodeRabbit inference engine (pnpr/AGENTS.md)
pnpr/**/pnpr/**/*.rs: Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, and error handling
Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions
Files:
pnpr/crates/pnpr/src/install_accelerator/verdict_cache/tests.rspnpr/crates/pnpr/src/install_accelerator/resolve.rspnpr/crates/pnpr/src/install_accelerator.rspnpr/crates/pnpr/src/install_accelerator/verdict_cache.rspnpr/crates/pnpr/src/install_accelerator/protocol.rs
pacquet/**/Cargo.toml
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/Cargo.toml: Check whether the workspace already depends on something suitable in[workspace.dependencies]in the rootCargo.tomlbefore adding a new dependency
Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it
Files:
pacquet/crates/pnpr-client/Cargo.toml
pacquet/**/tests/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — usetempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or matching#[cfg(unix)]gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests withinstaand carefully review diffs when intentional changes alter snapshots; accept withcargo insta reviewonly after careful review
Tests that need the mocked registry should startpnprthroughpacquet-testing-utils;cargo test/cargo nextest runshould not require a separatejust registry-mock launchstep
Files:
pacquet/crates/pnpr-client/tests/integration.rs
pnpr/**/pnpr/**/Cargo.toml
📄 CodeRabbit inference engine (pnpr/AGENTS.md)
Declare new shared dependencies in the root [workspace.dependencies] and use { workspace = true } in pnpr crate's Cargo.toml
Files:
pnpr/crates/pnpr/Cargo.toml
🧠 Learnings (3)
📚 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/lockfile-verification/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspacquet/crates/config/src/lib.rspacquet/crates/pnpr-client/src/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.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/lockfile-verification/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspacquet/crates/config/src/lib.rspacquet/crates/pnpr-client/src/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.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/lockfile-verification/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspacquet/crates/config/src/lib.rspacquet/crates/pnpr-client/src/tests.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.rs
🔇 Additional comments (3)
pacquet/crates/config/src/lib.rs (1)
17-17: LGTM!Also applies to: 98-104
pacquet/crates/pnpr-client/Cargo.toml (1)
14-25: LGTM!pacquet/crates/pnpr-client/src/tests.rs (1)
1-43: LGTM!
…iew) Address PR review feedback: - Forward the effective `trustLockfile` to /v1/install; the server skips the input-lockfile verify gate when set, so `--trust-lockfile` / `trustLockfile` keep skipping verification on the pnpr path (parity with local). - Stop masking `frozenLockfile` when the request has no lockfile — pass it through so a frozen install with a missing lockfile surfaces the frozen-lockfile error instead of silently resolving fresh. - Send `minimumReleaseAgeIgnoreMissingTime` as a concrete bool (not Option) so the full client policy always crosses the wire explicitly. - Clarify the verdict-cache docs: one row per lockfile hash, policy snapshot checked via can_trust_past_check (mirrors the local jsonl cache); not a per-(hash,policy) key. - Add RenderedViolation to the lockfile-verification public-surface docs. - Test: trustLockfile makes the server skip verification end-to-end.
|
Re: the outside-diff note that The protocol-version bump is intentionally skipped per @zkochan (pnpr is pre-production, client + server move in lockstep). Written by an agent (Claude Code, claude-opus-4-8). |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pnpr/crates/pnpr/src/install_accelerator/resolve.rs (1)
91-101: ⚡ Quick winAdd a regression test for
frozen_lockfile = truewith no input lockfile.Line 101 is the behavior change that restores pnpm parity, but the touched integration coverage here only pins reuse/verification/trust-lockfile. A targeted end-to-end test for
lockfile = None+frozen_lockfile = truewould keep this from silently regressing back to synthesizing a new lockfile. As per coding guidelines, "Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm".🤖 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 `@pnpr/crates/pnpr/src/install_accelerator/resolve.rs` around lines 91 - 101, Add a regression integration test that exercises the code path in resolve.rs where request.lockfile is None and request.frozen_lockfile is true: construct a Request with lockfile = None and frozen_lockfile = true, invoke the resolve/install path that uses the local variables input_lockfile, lockfile_path and Lockfile::FILE_NAME, and assert that the call returns the expected frozen-lockfile error (not a successful synthesis) and that no lockfile was written to lockfile_path; this test should fail if the code ever synthesizes a new lockfile instead of surfacing pacquet's frozen-lockfile error.
🤖 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 `@pnpr/crates/pnpr/src/install_accelerator/resolve.rs`:
- Around line 91-101: Add a regression integration test that exercises the code
path in resolve.rs where request.lockfile is None and request.frozen_lockfile is
true: construct a Request with lockfile = None and frozen_lockfile = true,
invoke the resolve/install path that uses the local variables input_lockfile,
lockfile_path and Lockfile::FILE_NAME, and assert that the call returns the
expected frozen-lockfile error (not a successful synthesis) and that no lockfile
was written to lockfile_path; this test should fail if the code ever synthesizes
a new lockfile instead of surfacing pacquet's frozen-lockfile error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c472bd5-5df0-495a-90af-f7a20680c4e3
📒 Files selected for processing (8)
pacquet/crates/cli/src/cli_args/install.rspacquet/crates/lockfile-verification/src/lib.rspacquet/crates/pnpr-client/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspnpr/crates/pnpr/src/install_accelerator.rspnpr/crates/pnpr/src/install_accelerator/protocol.rspnpr/crates/pnpr/src/install_accelerator/resolve.rspnpr/crates/pnpr/src/install_accelerator/verdict_cache.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- pacquet/crates/lockfile-verification/src/lib.rs
- pnpr/crates/pnpr/src/install_accelerator/protocol.rs
- pnpr/crates/pnpr/src/install_accelerator/verdict_cache.rs
- pnpr/crates/pnpr/src/install_accelerator.rs
- pacquet/crates/cli/src/cli_args/install.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Analyze (javascript)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (3)
pnpr/**/pnpr/**/*.rs
📄 CodeRabbit inference engine (pnpr/AGENTS.md)
pnpr/**/pnpr/**/*.rs: Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, and error handling
Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions
Files:
pnpr/crates/pnpr/src/install_accelerator/resolve.rs
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/pnpr-client/tests/integration.rspacquet/crates/pnpr-client/src/lib.rs
pacquet/**/tests/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — usetempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or matching#[cfg(unix)]gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests withinstaand carefully review diffs when intentional changes alter snapshots; accept withcargo insta reviewonly after careful review
Tests that need the mocked registry should startpnprthroughpacquet-testing-utils;cargo test/cargo nextest runshould not require a separatejust registry-mock launchstep
Files:
pacquet/crates/pnpr-client/tests/integration.rs
🧠 Learnings (3)
📚 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/pnpr-client/tests/integration.rspacquet/crates/pnpr-client/src/lib.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/pnpr-client/tests/integration.rspacquet/crates/pnpr-client/src/lib.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/pnpr-client/tests/integration.rspacquet/crates/pnpr-client/src/lib.rs
🔇 Additional comments (2)
pacquet/crates/pnpr-client/src/lib.rs (1)
66-74: LGTM!Also applies to: 189-208
pacquet/crates/pnpr-client/tests/integration.rs (1)
67-70: LGTM!Also applies to: 209-209, 220-246
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12144 +/- ##
=======================================
Coverage ? 87.47%
=======================================
Files ? 262
Lines ? 29903
Branches ? 0
=======================================
Hits ? 26157
Misses ? 3746
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Address the outside-diff review note that the pnpr fast path dropped several install-mode flags: - Forward `preferFrozenLockfile` to /v1/install; the server's resolve uses it (so `--no-prefer-frozen-lockfile` forces a fresh re-resolve) instead of hardcoding reuse. - Forward `ignoreManifestCheck`; the server's frozen freshness check and the local materialization both honor it now. - Error on `--lockfile-only` + pnprServer instead of silently fetching + linking (the protocol bundles resolution with file distribution, so there's no resolve-only mode yet). Tracked by #12146.
|
Update on the outside-diff note about dropped install-mode flags (
Written by an agent (Claude Code, claude-opus-4-8). |
| let key = serde_json::json!({ | ||
| "registry": registry, | ||
| "namedRegistries": request.named_registries, | ||
| "overrides": overrides, | ||
| "minimumReleaseAge": request.minimum_release_age, |
There was a problem hiding this comment.
Real concern, but it predates this PR and isn't specific to the verification fields: config_for's leak key already included arbitrary client-controlled overrides (a free-form map) and minimumReleaseAge, so the distinct-key space was already unbounded on an endpoint with no auth gate. This PR widens the key but doesn't change the fundamental property.
Properly bounding it (a cap + rejection, or avoiding the 'static leak the install path requires) is a server-hardening task that belongs with adding auth/rate-limiting to /v1/install generally — pnpr is pre-production and currently unauthenticated, so a cap here alone would be incomplete. Happy to add a bounded-config cap in this PR if you'd prefer; otherwise I'll leave this for the broader pnpr hardening pass. @zkochan — your call.
Written by an agent (Claude Code, claude-opus-4-8).
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/src/cli_args/install.rs`:
- Around line 326-327: The tri-state prefer_frozen_lockfile flag is being
forwarded directly to the pnpr payload instead of the resolved/effective boolean
from the client config; compute the effective value and send that. Locate where
the prefer_frozen_lockfile variable is passed into the Install/pnpr
serialization (the places building the /v1/install request) and replace it with
an effective boolean computed from the CLI tri-state and the resolved client
config (e.g. match prefer_frozen_lockfile { Some(b) => b, None =>
state.config.prefer_frozen_lockfile.unwrap_or(false) } or equivalent), and apply
the same change to the other occurrences noted (the other places building pnpr
install requests). Ensure the serialized payload contains the resolved boolean,
not the Option/tri-state.
🪄 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: 182a5660-6908-407a-b1f2-63c49651aa48
📒 Files selected for processing (5)
pacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspnpr/crates/pnpr/src/install_accelerator/protocol.rspnpr/crates/pnpr/src/install_accelerator/resolve.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- pnpr/crates/pnpr/src/install_accelerator/resolve.rs
- pacquet/crates/pnpr-client/tests/integration.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Agent
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Code Coverage
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/pnpr-client/src/lib.rspacquet/crates/cli/src/cli_args/install.rs
pnpr/**/pnpr/**/*.rs
📄 CodeRabbit inference engine (pnpr/AGENTS.md)
pnpr/**/pnpr/**/*.rs: Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, and error handling
Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions
Files:
pnpr/crates/pnpr/src/install_accelerator/protocol.rs
🧠 Learnings (3)
📚 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/pnpr-client/src/lib.rspacquet/crates/cli/src/cli_args/install.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/pnpr-client/src/lib.rspacquet/crates/cli/src/cli_args/install.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/pnpr-client/src/lib.rspacquet/crates/cli/src/cli_args/install.rs
🔇 Additional comments (3)
pacquet/crates/pnpr-client/src/lib.rs (1)
66-71: LGTM!Also applies to: 207-208
pnpr/crates/pnpr/src/install_accelerator/protocol.rs (1)
60-68: LGTM!pacquet/crates/cli/src/cli_args/install.rs (1)
328-328: LGTM!Also applies to: 403-413, 433-442, 529-529
…review) The pnpr path forwarded the raw CLI tri-state, so `None` serialized and the server defaulted to reuse — dropping a yaml `preferFrozenLockfile: false` unless the user also passed --no-prefer-frozen-lockfile. Resolve it against config.prefer_frozen_lockfile (as the local Install does) and send the effective value.
A verdict row whose policy blob no longer parses would miss forever. Delete it on the parse failure so the next install re-verifies and re-records a clean row. Adds a regression test.
…t (review) - evict_overflow doc no longer claims LRU-by-access; eviction is by last-verification time (verified_at_ms is set on record, not on a hit, so is_verified stays a pure read). - the overwrite test now asserts is_verified returned true so the in-closure assertion can't be skipped on an unexpected miss.
Closes #12139.
What
When a
pnprserver is configured, the client no longer runsverifyLockfileResolutionslocally. It sends its on-disk lockfile plus its full verification policy to/v1/install; pnpr verifies that input lockfile under the client's policy before resolving, and streams back any violations so the client aborts with the identicalERR_PNPM_*diagnostic the local gate would have produced. This is faster (pnpr's packument cache is warm + shared) and removes the client's own registry-reachability requirement — it adds no new trust (the client already trusts pnpr to resolve and serve bytes).All three phases from the issue, delivered together. Rust-only:
pacquetclient +pnprserver. The TS agent server is deprecated and the TS client already skips local verification, so no TS changes were needed.How
Phase 1 — send lockfile + policy; pnpr verifies; client skips local verify
install_accelerator/protocol.rs, mirrored inpnpr-client):/v1/installnow carrieslockfile,frozenLockfile, and the full policy (minimumReleaseAge[Exclude|IgnoreMissingTime],trustPolicy[Exclude|IgnoreAfter]).handle_installverifies the input lockfile viabuild_resolution_verifiers+collect_resolution_policy_violations(under the client policy threaded into the serverconfig_for) before resolving. On a violation it streams a200NDJSONEline of rendered violations; the client rebuilds the identicalVerifyError(PnprClientError::Verification).state.lockfile+ policy, drops thetrustPolicy: no-downgradeguard (pnpr enforces it now — input-lockfile verifier for reused entries + resolver pick-time check for new ones), and setstrust_lockfile: trueon the local materialization so it never re-verifies or touches the locallockfile-verified.jsonl.Phase 2 —
frozenLockfilegoverns resolution reuseresolve.rsseeds resolution from the input lockfile (frozen → as-is; non-frozen → reuse pins + resolve new).Phase 3 — SQLite whole-lockfile verdict cache on pnpr
install_accelerator/verdict_cache.rs: SQLite-backed (reuses the existingrusqlitedep), keyed by(lockfile hash, merged policy snapshot), hit = all verifierscan_trust_past_check. Only passes are cached (monotonic age + hash pins versions → time-correct without a cutoff, same property as the local cache); LRU cap, no TTL.Notes for review
Tarballresolutions (not plainRegistryresolutions, whose URL is derived), so the e2e violation test forces aminimumReleaseAgerejection instead..); a mismatch correctly errors under frozen.Testing
just check,just lint,cargo dylint --all,cargo doc -D warnings,just fmtall clean. Touched-crate suites (718 tests) + full pnpr/pnpr-client suites (216) pass, including 10 new tests:VerdictCacheunit tests,parse_install_responsewire→error reconstruction, and two end-to-end tests (a clean lockfile round-trips; a policy-violating lockfile is rejected with the rightVerifyError).Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Bug Fixes
Tests