feat(pnpr): server-accelerated installs via pnprServer (endpoints + client + CLI)#12077
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (1)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-22T00:08:44.646ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughThis PR adds an opt-in pnpr install-accelerator: a new Changespnpr Server Acceleration Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested reviewers
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 docstrings
🧪 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 |
0fcc534 to
96d67d5
Compare
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12077 +/- ##
==========================================
+ Coverage 87.19% 87.23% +0.04%
==========================================
Files 243 248 +5
Lines 26798 27605 +807
==========================================
+ Hits 23366 24081 +715
- Misses 3432 3524 +92 ☔ 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.06275242412,
"stddev": 0.08075686532229703,
"median": 2.0334752639199998,
"user": 2.77796356,
"system": 3.2329548600000004,
"min": 1.98465350542,
"max": 2.2513643864199997,
"times": [
2.0369900614199996,
2.2513643864199997,
1.99238189342,
2.02086960242,
2.11880624842,
2.0126118304199996,
2.02996046642,
1.98465350542,
2.1167867024199998,
2.06309954442
]
},
{
"command": "pacquet@main",
"mean": 2.05276876402,
"stddev": 0.04568169721528983,
"median": 2.05392593742,
"user": 2.74037766,
"system": 3.28738296,
"min": 1.9830031644200001,
"max": 2.12887006142,
"times": [
2.0434870194199997,
2.08603432842,
2.02831962842,
2.05876425142,
2.09870160842,
2.06251027142,
2.0490876234199997,
2.12887006142,
1.9830031644200001,
1.98890968342
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.67508809484,
"stddev": 0.0305493837028722,
"median": 0.66692313674,
"user": 0.37154506,
"system": 1.3477522999999998,
"min": 0.6523063477400001,
"max": 0.7594022397400001,
"times": [
0.7594022397400001,
0.67197662674,
0.67487170974,
0.65817893374,
0.66776125974,
0.6621858957400001,
0.67619392674,
0.66191899474,
0.6660850137400001,
0.6523063477400001
]
},
{
"command": "pacquet@main",
"mean": 0.6841243653400001,
"stddev": 0.023553438405406437,
"median": 0.67551637024,
"user": 0.37504935999999994,
"system": 1.3506439,
"min": 0.66660196474,
"max": 0.74122342574,
"times": [
0.7086966867400001,
0.66660196474,
0.67174452074,
0.66765001674,
0.68337200874,
0.6688594817400001,
0.68206280774,
0.6731656957400001,
0.67786704474,
0.74122342574
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.40715145086,
"stddev": 0.01696333218036573,
"median": 2.40586871736,
"user": 3.96705924,
"system": 3.0847441399999997,
"min": 2.37155888986,
"max": 2.4287403588600003,
"times": [
2.42727318986,
2.4028895348600003,
2.4287403588600003,
2.39645544186,
2.41534128386,
2.40884789986,
2.3986056108600002,
2.4022894038600002,
2.4195128948600004,
2.37155888986
]
},
{
"command": "pacquet@main",
"mean": 2.4033493369600003,
"stddev": 0.028295386755770684,
"median": 2.4054768708600003,
"user": 3.9491732399999995,
"system": 3.09790254,
"min": 2.3561418458600003,
"max": 2.44638897986,
"times": [
2.41109050386,
2.39124326586,
2.3932214378600003,
2.43140440186,
2.39986323786,
2.4254984308600003,
2.3561418458600003,
2.4131136378600004,
2.44638897986,
2.36552762786
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.6112736189399999,
"stddev": 0.02672856997099809,
"median": 1.61348819054,
"user": 1.77192512,
"system": 1.91659488,
"min": 1.5661993675399999,
"max": 1.66048925354,
"times": [
1.66048925354,
1.63341357154,
1.6151642155400001,
1.62355601554,
1.62271970154,
1.61181216554,
1.59119383554,
1.5661993675399999,
1.5842215255399998,
1.6039665375399998
]
},
{
"command": "pacquet@main",
"mean": 1.6123429771400002,
"stddev": 0.04269950907911716,
"median": 1.60699018804,
"user": 1.77721722,
"system": 1.90627748,
"min": 1.57094745154,
"max": 1.71140861354,
"times": [
1.62225132754,
1.71140861354,
1.5739423375400001,
1.59172904854,
1.57840524554,
1.57094745154,
1.62307770554,
1.58586573454,
1.62695983154,
1.63884247554
]
}
]
} |
|
| Branch | pr/12077 |
| 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,407.15 ms(+0.91%)Baseline: 2,385.45 ms | 2,862.54 ms (84.09%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,611.27 ms(+6.38%)Baseline: 1,514.66 ms | 1,817.59 ms (88.65%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,062.75 ms(-2.53%)Baseline: 2,116.27 ms | 2,539.52 ms (81.23%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 675.09 ms(-0.02%)Baseline: 675.22 ms | 810.27 ms (83.32%) |
Port the pnpm-agent proof of concept from TypeScript onto pacquet's resolver and content-addressable store, exposed as additive, opt-in, versioned endpoints alongside pnpr's npm-compatible API: - POST /v1/install resolves a project server-side (pacquet Install in lockfile-only mode against a throwaway temp project), fetches only the uncached packages into the shared store, and streams an NDJSON response: D lines (file digests the client is missing), I lines (pre-packed msgpackr-records store-index entries), and a final L line with the lockfile and stats. - POST /v1/files serves a batch of files by digest as a gzip binary stream the client writes straight into its CAFS. The pacquet runtime (a single leaked Config plus a shared HTTP client) is held lazily per server in router state, so servers that never receive an agent request pay nothing and each server in a multi-server test process keeps its own store. Adds a public Lockfile::load_wanted_from_dir mirroring pnpm's readWantedLockfile(dir). Deferred (documented in the module): multi-project workspaces, incremental resolution from a client lockfile, overrides, per-request minimumReleaseAge, and true response streaming. Refs pnpm/rfcs#9 --- Written by an agent (Claude Code, claude-opus-4-8).
Rename src/agent/mod.rs to the sibling src/agent.rs form and add the trailing comma the perfectionist dylint rules require. --- Written by an agent (Claude Code, claude-opus-4-8).
Satisfy the perfectionist dylint rule (updated on main) that requires a single `use` statement per crate root. --- Written by an agent (Claude Code, claude-opus-4-8).
Port the TypeScript `@pnpm/agent.client` (`fetchFromPnpmRegistry`) plus the `fetch-and-write-cafs` worker to Rust as a new `pacquet-agent-client` crate. Given a set of dependencies and the client's content-addressable store it: - reads the integrities already in the local store index, - POSTs them with the dependencies to `/v1/install` and parses the NDJSON response (D missing-file digests, I store-index entries, L lockfile + stats, E error), - downloads the missing files from `/v1/files`, decodes the gzip binary framing, and writes each file into the local CAFS by digest (no re-hashing), rejecting any entry that wasn't requested, - writes the forwarded store-index entries, skipping keys already present, - and returns the resolved lockfile for a headless install. Integration tests drive the real round-trip: a shared TestRegistry serves the package fixtures, a per-test in-process pnpr hosts the agent endpoints and resolves from it, and the client materializes a package into a fresh store. Covers a single-file package, a multi-file package, and warm-store dedup (second run downloads nothing). Deferred (matching the server): true streaming and batched /v1/files. --- Written by an agent (Claude Code, claude-opus-4-8).
…ured Add an `agent` setting (CLI `--agent`, `agent:` in pnpm-workspace.yaml, `PNPM_CONFIG_AGENT`) and wire it into the install command, mirroring pnpm's `install()` delegating to `installFromPnpmRegistry`. When `agent` is set, `pacquet install` resolves the project server-side through `pacquet-agent-client` (which populates the local store and returns the lockfile), writes the lockfile, then runs a frozen install to link `node_modules` from it — the equivalent of pnpm handing the agent-produced lockfile to `headlessInstall`. `trustPolicy: no-downgrade` is refused on this path, matching pnpm's `TRUST_POLICY_INCOMPATIBLE_WITH_AGENT`. Adds `Config.agent` plus its yaml/env/CLI wiring and an end-to-end test that runs the real `pacquet install --agent <url>` binary against an in-process pnpr agent (resolving from the mocked fixtures registry) and asserts node_modules is linked from the agent-produced lockfile. Deferred (matching the client/server): `pacquet add` / `remove` via the agent, multi-project workspaces, overrides, and per-request minimumReleaseAge. --- Written by an agent (Claude Code, claude-opus-4-8).
…add handshake Rework the fast-path opt-in around how it's actually used: - Rename the `agent` setting to `pnprServer` (CLI `--pnpr-server`, `pnprServer:` in pnpm-workspace.yaml, `PNPM_CONFIG_PNPR_SERVER`). The value is a plain URL of a pnpr server, not the registry — the server is a resolution/fetch service, not "a registry that resolves from itself". - The client now sends its own resolution config (default `registry`, `namedRegistries`, `overrides`, `minimumReleaseAge`) in the `/v1/install` request, and the server resolves against *those* registries instead of its own uplink. This decouples "who computes" from "the source of truth", so the same pnpr server works for any client's registry setup. The server interns a `&'static Config` per distinct client registry configuration, bounding the leak. - Add a `GET /-/pnpr` capability handshake advertising supported protocol versions. The client probes it before installing and fails fast with a clear error if the configured server isn't a pnpr or speaks no common version (rather than silently falling back). Tests: the agent-client + cli e2e tests now leave the pnpr server's own uplink at the default and rely on the client-supplied registry, proving resolution uses the forwarded config; plus a handshake-rejection test. Deferred: auth/credential forwarding so private registries resolve. --- Written by an agent (Claude Code, claude-opus-4-8).
Complete the move away from "agent" terminology now that the user-facing surface is `pnprServer`. No behavior change. - crate `pacquet-agent-client` -> `pacquet-pnpr-client` (`AgentClient` -> `PnprClient`, `AgentClientError` -> `PnprClientError`) - pnpr module `agent` -> `fast_path` (`AgentRuntime` -> `FastPathRuntime`, `serve_agent_*` -> `serve_fast_path_*`, `agent_runtime` field, the `agent/` source dir and `tests/agent.rs`) - the CLI's `AgentLink` helper, the `AgentInstallOptions` import alias, user-visible error strings, and the server's on-disk `agent-store` / `agent-cache` dirs (now `pnpr-store` / `pnpr-cache`) Upstream citations keep their real names (`@pnpm/agent.client`, the pnpm-agent proof of concept, pnpm's `TRUST_POLICY_INCOMPATIBLE_WITH_AGENT` error code). --- Written by an agent (Claude Code, claude-opus-4-8).
Replace the vague, repetitive "Client for the pnpr fast-path endpoints exposed by pnpr" with "Client for pnpr's server-accelerated installs": no doubled `pnpr`, and "server-accelerated" credits the server with the speedup without claiming the install (linking) runs there. --- Written by an agent (Claude Code, claude-opus-4-8).
It's the per-server engine that resolves a client's project and serves the files its store is missing — i.e. what accelerates the install. "FastPath" was jargon and "Runtime" was vague; the new name matches the crate's "server-accelerated installs" framing. Field renamed to `install_accelerator`. No behavior change. --- Written by an agent (Claude Code, claude-opus-4-8).
Match the `InstallAccelerator` type: `mod fast_path` -> `mod install_accelerator` (source file, `fast_path/` dir, and the integration test), the `serve_fast_path_*` route handlers -> `serve_install` / `serve_files`, and the lingering "fast path" prose. "fast path" is fully gone from the crate. No behavior change. --- Written by an agent (Claude Code, claude-opus-4-8).
`pnpr` and `pnpr-fixtures` are licensed under PolyForm Shield (see `pnpr/LICENSE.md`), not the workspace's MIT, and `[licenses.private] ignore = false` makes cargo-deny audit first-party crates — so the license check fails them as "unlicensed" (the LICENSE.md only matches PolyForm-Noncommercial at 0.74, below the confidence threshold). Pin the license by file hash via `clarify` and allow it for just those two crates via `exceptions`. cargo-deny doesn't recognize a `PolyForm-Shield` identifier, so the closest recognized id (`PolyForm-Noncommercial-1.0.0`) is used. `cargo deny check` is green. --- Written by an agent (Claude Code, claude-opus-4-8).
3723590 to
a6ab49f
Compare
Review Summary by QodoAdd pnpr server-accelerated installs with client integration
WalkthroughsDescription• Adds pnpr server-accelerated install endpoints (/v1/install, /v1/files) - Offloads dependency resolution to server, streams missing files - Client sends registry config; server resolves against client's registries • Implements pacquet client (pacquet-pnpr-client) for pnpr communication - Handshake negotiation, NDJSON response parsing, binary file download - Writes files to CAFS by digest, forwards store-index entries • Integrates pnpr fast-path into CLI via --pnpr-server flag - Routes through client when configured, then links node_modules locally - Rejects trustPolicy: no-downgrade (server can't enforce it) • Comprehensive test coverage for both server and client sides Diagramflowchart LR
A["pacquet install<br/>--pnpr-server URL"] -->|sends registry config| B["pnpr server<br/>install_accelerator"]
B -->|resolves dependencies| C["pacquet resolver"]
B -->|fetches uncached| D["shared store"]
B -->|streams NDJSON| E["PnprClient"]
E -->|downloads files| F["POST /v1/files"]
F -->|binary payload| E
E -->|writes CAFS| G["client store"]
E -->|frozen install| H["link node_modules"]
File Changes1. pacquet/crates/cli/src/cli_args.rs
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
pnpr/crates/pnpr/src/install_accelerator.rs (1)
86-87: 💤 Low valueSilent directory creation failures may hide important errors.
If
create_dir_allfails (e.g., permission denied, disk full), the error is silently swallowed. The subsequentStoreDir::newand file operations will fail with confusing errors. Consider logging a warning on failure.Proposed fix
- let _ = std::fs::create_dir_all(&store_dir); - let _ = std::fs::create_dir_all(&cache_dir); + if let Err(err) = std::fs::create_dir_all(&store_dir) { + tracing::warn!(?err, ?store_dir, "failed to create pnpr store directory"); + } + if let Err(err) = std::fs::create_dir_all(&cache_dir) { + tracing::warn!(?err, ?cache_dir, "failed to create pnpr cache directory"); + }🤖 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.rs` around lines 86 - 87, The current calls to std::fs::create_dir_all for store_dir and cache_dir silently ignore errors; change them to handle the Result and surface failures (e.g., with a warning log) before continuing to call StoreDir::new. Concretely, replace the two let _ = std::fs::create_dir_all(&store_dir) / (&cache_dir) with code that matches on the Result and on Err emits a warning (using your project logger or tracing::warn!/log::warn! or at minimum eprintln!) including the path and the error, then continue or bail as appropriate so subsequent StoreDir::new and file operations get clearer diagnostics. Ensure you reference the same variables store_dir and cache_dir so behavior remains identical except for logging.pacquet/crates/config/src/workspace_yaml.rs (1)
721-723: ⚡ Quick winConsider normalizing
pnpr_serverURLs for consistent path joining.The
registryfield (lines 718-720) ensures a trailing slash is present. Sincepnpr_serverwill be used with path-based endpoints (/-/pnpr,/v1/install,/v1/files), you should either:
- Normalize to remove any user-supplied trailing slash (so the client can safely append paths with a leading slash), or
- Document the expected URL format and ensure the client handles both cases.
Without normalization, a user providing
http://server/vshttp://servercould lead to malformed URLs (http://server//v1/installvshttp://server/v1/install).♻️ Proposed normalization to strip trailing slashes
if let Some(v) = self.pnpr_server { - config.pnpr_server = Some(v); + config.pnpr_server = Some(v.trim_end_matches('/').to_string()); }🤖 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/config/src/workspace_yaml.rs` around lines 721 - 723, The assignment for pnpr_server can produce double slashes when clients append paths; normalize by stripping any trailing slashes before setting config.pnpr_server (similar to how registry is normalized) so consumers can safely append leading-path segments; update the block that reads self.pnpr_server -> config.pnpr_server to trim trailing '/' (handle None unchanged) and add a short comment referencing pnpr_server and registry normalization for clarity.pacquet/crates/pnpr-client/src/lib.rs (1)
232-239: ⚡ Quick winAvoid unnecessary clone in the non-gzip path.
When
rawdoesn't start with gzip magic bytes,raw.to_vec()clones the entire payload. Sincewrite_files_payloadaccepts&[u8], you can avoid the allocation by binding a temporary:let decompressed; let payload: &[u8] = if raw.starts_with(&[0x1f, 0x8b]) { decompressed = { let mut decoder = GzDecoder::new(&raw[..]); let mut out = Vec::new(); decoder.read_to_end(&mut out)?; out }; &decompressed } else { &raw };🤖 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/pnpr-client/src/lib.rs` around lines 232 - 239, The current logic allocates an owned Vec via raw.to_vec() for the non-gzip branch which is unnecessary because write_files_payload accepts a &[u8]; refactor the payload handling so payload is a slice (&[u8]) and only allocate when actually decompressing: keep the GzDecoder-based path producing an owned Vec (store it in a temporary like decompressed) and set payload to a slice of that Vec, otherwise set payload to &raw; update uses of payload accordingly (referencing the payload variable, raw, and GzDecoder in pnpr-client::lib.rs).
🤖 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 425-426: The current code silently swallows serialization errors
by using serde_json::to_value(...).ok(), causing missing overrides to be treated
as None; change the computation of overrides so serialization errors are
propagated (e.g. use state.config.overrides.as_ref().map(|o|
serde_json::to_value(o)).transpose() and propagate the Result with ?), and
adjust the surrounding function signature to return a Result if necessary;
reference the overrides variable, state.config.overrides, and
serde_json::to_value when making this change.
In `@pnpr/crates/pnpr/src/install_accelerator.rs`:
- Around line 260-263: The payload construction casts content.len() to u32 which
will silently truncate for files > u32::MAX; update the code around
payload.extend_from_slice(&(content.len() as u32).to_be_bytes()) to validate the
size before casting (e.g. if content.len() > u32::MAX return an error) or use
content.len().try_into() and propagate the failure; ensure the surrounding
function in install_accelerator.rs returns a Result so you can return a
descriptive error when the size exceeds u32::MAX, then only push the length
after a successful conversion and proceed with
payload.extend_from_slice(&content) and payload.push(u8::from(file.executable)).
---
Nitpick comments:
In `@pacquet/crates/config/src/workspace_yaml.rs`:
- Around line 721-723: The assignment for pnpr_server can produce double slashes
when clients append paths; normalize by stripping any trailing slashes before
setting config.pnpr_server (similar to how registry is normalized) so consumers
can safely append leading-path segments; update the block that reads
self.pnpr_server -> config.pnpr_server to trim trailing '/' (handle None
unchanged) and add a short comment referencing pnpr_server and registry
normalization for clarity.
In `@pacquet/crates/pnpr-client/src/lib.rs`:
- Around line 232-239: The current logic allocates an owned Vec via raw.to_vec()
for the non-gzip branch which is unnecessary because write_files_payload accepts
a &[u8]; refactor the payload handling so payload is a slice (&[u8]) and only
allocate when actually decompressing: keep the GzDecoder-based path producing an
owned Vec (store it in a temporary like decompressed) and set payload to a slice
of that Vec, otherwise set payload to &raw; update uses of payload accordingly
(referencing the payload variable, raw, and GzDecoder in pnpr-client::lib.rs).
In `@pnpr/crates/pnpr/src/install_accelerator.rs`:
- Around line 86-87: The current calls to std::fs::create_dir_all for store_dir
and cache_dir silently ignore errors; change them to handle the Result and
surface failures (e.g., with a warning log) before continuing to call
StoreDir::new. Concretely, replace the two let _ =
std::fs::create_dir_all(&store_dir) / (&cache_dir) with code that matches on the
Result and on Err emits a warning (using your project logger or
tracing::warn!/log::warn! or at minimum eprintln!) including the path and the
error, then continue or bail as appropriate so subsequent StoreDir::new and file
operations get clearer diagnostics. Ensure you reference the same variables
store_dir and cache_dir so behavior remains identical except for logging.
🪄 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: 8137deb7-e2e3-4786-b681-1435d29d73f4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
Cargo.tomldeny.tomlpacquet/crates/cli/Cargo.tomlpacquet/crates/cli/src/cli_args.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/cli/tests/pnpr_install.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/lockfile/src/load_lockfile.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/pnpr-client/Cargo.tomlpacquet/crates/pnpr-client/src/lib.rspacquet/crates/pnpr-client/tests/integration.rspnpr/crates/pnpr/Cargo.tomlpnpr/crates/pnpr/src/install_accelerator.rspnpr/crates/pnpr/src/install_accelerator/diff.rspnpr/crates/pnpr/src/install_accelerator/protocol.rspnpr/crates/pnpr/src/install_accelerator/resolve.rspnpr/crates/pnpr/src/lib.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/tests/install_accelerator.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
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/lib.rspnpr/crates/pnpr/src/install_accelerator/protocol.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/src/install_accelerator.rspnpr/crates/pnpr/tests/install_accelerator.rspnpr/crates/pnpr/src/install_accelerator/resolve.rspnpr/crates/pnpr/src/install_accelerator/diff.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/lockfile/src/load_lockfile.rspacquet/crates/cli/src/cli_args.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/tests/pnpr_install.rspacquet/crates/pnpr-client/tests/integration.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.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.tomlpacquet/crates/cli/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/cli/tests/pnpr_install.rspacquet/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 (4)
📚 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/src/load_lockfile.rspacquet/crates/cli/src/cli_args.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/tests/pnpr_install.rspacquet/crates/pnpr-client/tests/integration.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/src/load_lockfile.rspacquet/crates/cli/src/cli_args.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/tests/pnpr_install.rspacquet/crates/pnpr-client/tests/integration.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/src/load_lockfile.rspacquet/crates/cli/src/cli_args.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/config/src/env_overlay.rspacquet/crates/cli/tests/pnpr_install.rspacquet/crates/pnpr-client/tests/integration.rspacquet/crates/cli/src/cli_args/install.rspacquet/crates/pnpr-client/src/lib.rs
📚 Learning: 2026-05-20T23:23:40.606Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11788
File: pacquet/crates/lockfile/src/snapshot_dep_ref.rs:52-56
Timestamp: 2026-05-20T23:23:40.606Z
Learning: In the pacquet lockfile crate, the link target is intentionally represented as a raw `Link(String)` (not a `LinkTarget` newtype) for both `SnapshotDepRef::Link` and `ImporterDepVersion::Link`. pnpm upstream preserves `link:` targets verbatim with only a prefix check (e.g., `refToRelative`), so do not suggest introducing a `LinkTarget` newtype in a bug-fix/targeted PR. If type branding is ever desired, it should be done together for both variants in a separate dedicated refactor PR rather than as part of a small fix.
Applied to files:
pacquet/crates/lockfile/src/load_lockfile.rs
🔇 Additional comments (21)
pnpr/crates/pnpr/Cargo.toml (1)
22-56: LGTM!pnpr/crates/pnpr/src/lib.rs (1)
14-14: LGTM!pnpr/crates/pnpr/src/install_accelerator.rs (1)
137-217: LGTM!Also applies to: 267-291, 293-313
pnpr/crates/pnpr/src/install_accelerator/diff.rs (1)
1-162: LGTM!pnpr/crates/pnpr/src/install_accelerator/protocol.rs (1)
1-99: LGTM!pnpr/crates/pnpr/src/install_accelerator/resolve.rs (1)
1-252: LGTM!pnpr/crates/pnpr/src/server.rs (1)
60-63: LGTM!Also applies to: 91-99, 102-107, 1507-1529
pnpr/crates/pnpr/tests/install_accelerator.rs (1)
1-117: LGTM!Cargo.toml (1)
16-16: LGTM!deny.toml (1)
68-86: LGTM!pacquet/crates/config/src/lib.rs (1)
866-874: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
127-127: LGTM!pacquet/crates/config/src/env_overlay.rs (1)
140-140: LGTM!pacquet/crates/pnpr-client/Cargo.toml (1)
1-34: LGTM!pacquet/crates/cli/Cargo.toml (1)
18-18: LGTM!Also applies to: 39-39, 45-45
pacquet/crates/lockfile/src/load_lockfile.rs (1)
56-63: LGTM!pacquet/crates/pnpr-client/tests/integration.rs (1)
1-167: LGTM!pacquet/crates/cli/src/cli_args.rs (1)
217-219: LGTM!pacquet/crates/cli/tests/pnpr_install.rs (1)
64-89: LGTM!pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
47-47: LGTM!pacquet/crates/cli/src/cli_args/install.rs (1)
414-423: ⚡ Quick winHandle
optionalDependenciesin pnpr install payload
pacquet/crates/cli/src/cli_args/install.rscollects onlyDependencyGroup::ProdandDependencyGroup::Devfor the pnpr request payload; it does not sendDependencyGroup::Optional. On the pnpr side,pnpr/crates/pnpr/src/install_accelerator/resolve.rsincludesDependencyGroup::Optionalin itsdependency_groupslist, so optional deps may be derived server-side. Please confirm whether:
- optional dependencies are gathered server-side (e.g., from the manifest) without needing them in the request, or
- the protocol expects a separate
optionalDependencies/optional_dependenciesfield in the request.
- propagate `overrides` serialization errors instead of swallowing them with `.ok()` (a failure would silently drop override-driven resolutions) - guard the `/v1/files` size field against >4 GiB truncation (`u32::try_from` -> clean 500 instead of a corrupt stream) - borrow the response bytes directly on the already-decompressed path instead of cloning - document the best-effort `create_dir_all` for the store/cache dirs From CodeRabbit review on #12077. --- Written by an agent (Claude Code, claude-opus-4-8).
|
Thanks for the review — addressed the nitpicks in 60bb425:
Written by an agent (Claude Code, claude-opus-4-8). |
What
Adds an opt-in
pnprServersetting that offloads the slow part of an install — dependency resolution and computing which files the local store is missing — to a pnpr server, which streams back only the missing files.node_modulesis still linked locally from the server-produced lockfile (like server-side rendering: the compute runs remotely, the result is materialized locally).Realizes the agent concept from RFC #9, reworked around how it's actually used and rewritten in Rust on pacquet + pnpr.
How it works
pacquet install(withpnprServerset) handshakes the server —GET /-/pnpr— to negotiate a protocol version.POSTs/v1/installwith the project's dependencies, the integrities already in its store, and its own registry config (defaultregistry,namedRegistries,overrides,minimumReleaseAge).D(missing file digests),I(pre-packed store-index entries),L(lockfile + stats)./v1/files(gzip binary), writes them into its CAFS by digest (no re-hashing), writes the index entries, and runs a frozen install to linknode_modulesfrom the server's lockfile.Pieces
pnpr) —GET /-/pnprhandshake +POST /v1/install(NDJSON) +POST /v1/files(gzip), additive and opt-in alongside the npm-compatible API. Resolves against the client-sent registries, interning a&'static Configper distinct client config to bound the leak.pacquet-pnpr-client) —PnprClient: reads store integrities, negotiates the protocol version, sends the registry config, parses the stream, materializes files + index entries, returns the lockfile. Rejects unrequested file entries and repairs truncated CAFS files.pnprServersetting (--pnpr-server,pnprServer:inpnpm-workspace.yaml,PNPM_CONFIG_PNPR_SERVER). When set,pacquet installroutes through the client and then links locally — pnpm'sinstall()→installFromPnpmRegistryshape.trustPolicy: no-downgradeis refused (the server can't enforce it), matching pnpm'sTRUST_POLICY_INCOMPATIBLE_WITH_AGENT.Design notes
pnprServerURL rather than reusingregistry. The same server works for any client's registry setup, and a single pnpr can be both registry andpnprServer.pnpr; "agent" survives only in upstream citations (@pnpm/agent.client, the pnpm-agent PoC, pnpm'sTRUST_POLICY_INCOMPATIBLE_WITH_AGENTerror code).Tests
pacquet-pnpr-client: resolve + download, multi-file package, warm-store no-op, and handshake rejection. The pnpr server's own uplink is left at the default, so resolution provably uses the client-sent registry.pacquet-cli: a realpacquet install --pnpr-server <url>against an in-process pnpr (resolving from the mocked fixtures registry) linksnode_modules.pnpr:/v1/filesbinary-framing round-trip + handshake route.Full suites green; clippy / dylint (Perfectionist) / fmt / taplo /
cargo doc -D warningsclean.Deferred
Auth/credential forwarding (so private/scoped registries resolve),
pacquet add/removeviapnprServer, multi-project workspaces, and true streaming (responses are buffered today).Refs pnpm/rfcs#9
Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Tests
Chores