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 (11)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
📜 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). (6)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2026-06-04T06:04:01.216ZApplied to files:
📚 Learning: 2026-05-14T09:04:00.133ZApplied to files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis PR consolidates the pnpr install accelerator to a single gzipped ChangesConsolidate to single authorized install response
Sequence Diagram — Authorized inline install flow: sequenceDiagram
participant Client
participant serve_install
participant handle_install
participant write_cafs_worker
Client->>serve_install: POST /v1/install + Authorization
serve_install->>handle_install: runtime + policies + Identity + body
handle_install->>write_cafs_worker: send framed payload (on success)
write_cafs_worker-->>handle_install: filesWritten
handle_install-->>Client: inline header + file frames
🎯 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 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12181 +/- ##
==========================================
+ Coverage 87.46% 87.56% +0.09%
==========================================
Files 269 269
Lines 30848 30817 -31
==========================================
+ Hits 26982 26985 +3
+ Misses 3866 3832 -34 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Integrated-Benchmark Report (Linux)Each scenario has pacquet rows (direct install) and pnpr rows (the same client through the pnpr install accelerator), so pnpr@HEAD vs pacquet@HEAD is the pnpr-vs-direct ratio. Cold-store scenarios wipe the client store between runs (warm server); hot-store scenarios keep it warm. The pacquet@HEAD rows feed the pacquet Bencher testbed; the pnpr@HEAD rows feed the pnpr testbed. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.67418976078,
"stddev": 0.042367746579312575,
"median": 4.6628959092799995,
"user": 2.4641971599999994,
"system": 2.2962226600000006,
"min": 4.63225850078,
"max": 4.77229249978,
"times": [
4.6401374537799995,
4.67234263578,
4.63225850078,
4.64770088378,
4.69390129978,
4.67531999878,
4.65344918278,
4.64507981878,
4.70941533378,
4.77229249978
]
},
{
"command": "pacquet@main",
"mean": 4.65023027188,
"stddev": 0.04529855343097615,
"median": 4.64226688478,
"user": 2.4512621599999997,
"system": 2.2915428600000003,
"min": 4.60333865978,
"max": 4.76073291178,
"times": [
4.67612714778,
4.76073291178,
4.66349272278,
4.65344268678,
4.63112348778,
4.60499732178,
4.6432331287799995,
4.60333865978,
4.62451401078,
4.64130064078
]
},
{
"command": "pnpr@HEAD",
"mean": 1.70296498278,
"stddev": 0.0822687264520943,
"median": 1.6743682747800002,
"user": 2.79675926,
"system": 1.99954316,
"min": 1.6096718467800002,
"max": 1.86653490878,
"times": [
1.76665008578,
1.65423112178,
1.86653490878,
1.6464410187800003,
1.6848639327800001,
1.74979397478,
1.6096718467800002,
1.6638726167800002,
1.76999172678,
1.6175985947800002
]
},
{
"command": "pnpr@main",
"mean": 1.71960507668,
"stddev": 0.0718699978662101,
"median": 1.72077272178,
"user": 2.8122055599999998,
"system": 2.00754716,
"min": 1.6301069057800002,
"max": 1.85915291478,
"times": [
1.7755715397800003,
1.71232851278,
1.72921693078,
1.6301069057800002,
1.85915291478,
1.7367927307800002,
1.77608652078,
1.6779995277800002,
1.66745020878,
1.63134497478
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.50131690486,
"stddev": 0.049377852062072285,
"median": 0.48621977946000006,
"user": 0.36386781999999995,
"system": 0.7924559199999999,
"min": 0.47650176446000003,
"max": 0.64117550446,
"times": [
0.64117550446,
0.49415124046000003,
0.48818053046000004,
0.48501340446,
0.48603575046,
0.48539956346,
0.48994146346000006,
0.48640380846000003,
0.48036601846000004,
0.47650176446000003
]
},
{
"command": "pacquet@main",
"mean": 0.49706991136,
"stddev": 0.02973291948180692,
"median": 0.48802642196,
"user": 0.36412382,
"system": 0.7902288199999999,
"min": 0.48247816946000005,
"max": 0.5810857114600001,
"times": [
0.5810857114600001,
0.48679686046000004,
0.48592770646000005,
0.48792914446,
0.49231574746000006,
0.48890223846,
0.48812369946000006,
0.48247816946000005,
0.49394390846,
0.48319592746000006
]
},
{
"command": "pnpr@HEAD",
"mean": 0.5201138248599999,
"stddev": 0.06827915327095403,
"median": 0.49638850546,
"user": 0.36596872,
"system": 0.79141372,
"min": 0.47657036946000003,
"max": 0.7042748954600001,
"times": [
0.54342082146,
0.47657036946000003,
0.5212963204600001,
0.50025988146,
0.48002182846,
0.48359003146,
0.48138418646000003,
0.49251712946000004,
0.7042748954600001,
0.51780278446
]
},
{
"command": "pnpr@main",
"mean": 0.53934744936,
"stddev": 0.06354535239779385,
"median": 0.51725756996,
"user": 0.36559971999999996,
"system": 0.7836665199999999,
"min": 0.48467706146000006,
"max": 0.66808183846,
"times": [
0.63763420546,
0.5550521904600001,
0.49414081546000005,
0.48467706146000006,
0.50402190946,
0.5182707174600001,
0.49038302246000004,
0.51624442246,
0.66808183846,
0.52496831046
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.9281450636,
"stddev": 0.035328514324185906,
"median": 1.9112929503,
"user": 3.7479796,
"system": 1.8790861199999995,
"min": 1.8961289958,
"max": 1.9998730468,
"times": [
1.9559181218000001,
1.8976136318,
1.9091946688,
1.9510880578,
1.9998730468,
1.9561347438,
1.8994647508000002,
1.8961289958,
1.9026433868000001,
1.9133912318000001
]
},
{
"command": "pacquet@main",
"mean": 1.9349451229000003,
"stddev": 0.021057315981856145,
"median": 1.9366814213,
"user": 3.7600420999999997,
"system": 1.8973891199999997,
"min": 1.9032497438000002,
"max": 1.9683388528,
"times": [
1.9617792328,
1.9499484038000001,
1.9032497438000002,
1.9382270358,
1.9683388528,
1.9194739358,
1.9195717228,
1.9351358068000002,
1.9144405048000002,
1.9392859898
]
},
{
"command": "pnpr@HEAD",
"mean": 1.9362276846,
"stddev": 0.024667339933882176,
"median": 1.9382255373000001,
"user": 3.7428252,
"system": 1.9072701199999997,
"min": 1.8993874818,
"max": 1.9805652768,
"times": [
1.9237276448000002,
1.9805652768,
1.8993874818,
1.9320616448,
1.9230803908,
1.9528342588,
1.9535415208000002,
1.9443894298,
1.9043297628,
1.9483594348000002
]
},
{
"command": "pnpr@main",
"mean": 1.9420318833999999,
"stddev": 0.01722972315349656,
"median": 1.9441889683000002,
"user": 3.7493513999999997,
"system": 1.9198462199999997,
"min": 1.9193984678,
"max": 1.9801034558000001,
"times": [
1.9477176658000002,
1.9217799278,
1.9801034558000001,
1.9456852168,
1.9468564178,
1.9426927198000001,
1.9334223768,
1.9325159838000001,
1.9193984678,
1.9501466018
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.10760687162,
"stddev": 0.020710872600309065,
"median": 1.09871823262,
"user": 1.41505562,
"system": 1.05676006,
"min": 1.08976543462,
"max": 1.1491846266199999,
"times": [
1.09928108762,
1.10645168962,
1.14244020062,
1.09746697262,
1.09815537762,
1.10276819662,
1.08976543462,
1.09749533562,
1.09305979462,
1.1491846266199999
]
},
{
"command": "pacquet@main",
"mean": 1.1156042646200002,
"stddev": 0.017654245781451496,
"median": 1.11583422612,
"user": 1.4240138199999997,
"system": 1.0363499600000001,
"min": 1.09251425562,
"max": 1.15067350862,
"times": [
1.12704546262,
1.11258040062,
1.09908714662,
1.10748736762,
1.09731645962,
1.11908805162,
1.12933438662,
1.09251425562,
1.15067350862,
1.12091560662
]
},
{
"command": "pnpr@HEAD",
"mean": 1.10480838522,
"stddev": 0.017033052137785738,
"median": 1.10253001562,
"user": 1.3945571199999995,
"system": 1.04353916,
"min": 1.08186494162,
"max": 1.13947770462,
"times": [
1.10183521362,
1.11234069262,
1.10322481762,
1.13947770462,
1.08269024862,
1.08186494162,
1.10835091562,
1.12040231562,
1.09821749262,
1.09967950962
]
},
{
"command": "pnpr@main",
"mean": 1.0962499102199998,
"stddev": 0.017562662306849205,
"median": 1.0946012721199998,
"user": 1.40150092,
"system": 1.0357466599999998,
"min": 1.07532550362,
"max": 1.12934008862,
"times": [
1.12934008862,
1.10848664762,
1.07642743962,
1.11286060462,
1.09087865062,
1.09463189662,
1.07749963662,
1.09457064762,
1.10247798662,
1.07532550362
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12181 |
| Testbed | pacquet |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| isolated-linker.fresh-restore.cold-cache.cold-store | Latency seconds (s) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 4.67 s(+93.95%)Baseline: 2.41 s | 2.89 s (161.63%) |
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 | 1,928.15 ms(-16.08%)Baseline: 2,297.64 ms | 2,757.17 ms (69.93%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,107.61 ms(-24.71%)Baseline: 1,471.10 ms | 1,765.32 ms (62.74%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 4,674.19 ms(+93.95%)Baseline: 2,409.99 ms | 2,891.98 ms (161.63%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 501.32 ms(-22.77%)Baseline: 649.08 ms | 778.90 ms (64.36%) |
…tall A content-addressed digest in the install-accelerator store is shared across packages and says nothing about access, so the store's possession of a package's bytes is not a capability to receive them. `/v1/install` served files for any package found in the store, including ones reached only on the cache-hit / frozen-lockfile path where no access check happened — letting a caller who knows a private package's digest pull bytes the registry routes would 401 on. Check every served package against pnpr's own `packages:` policy before serving — the same decision `serve_packument` / `serve_tarball` make, in process, with no network round trip (so a warm shared server keeps its resolution advantage). `serve_install` resolves the caller's identity from `Authorization`; `deny_unauthorized_packages` denies the install (401 anonymous / 403 authenticated-but-outside-the-allowed-set) when any served package is not readable by the caller. This authorizes against pnpr's own surface, the authority for everything the store can hold today (pnpr fetches anonymously, so cached content is pnpr-hosted or publicly fetchable). When credential forwarding lands, packages the client resolved from external registries under its own token carry no pnpr policy and will need per-caller re-verification against the owning registry (TTL-cached) — noted at the check and tracked in #12184. The raw `/v1/files` endpoint is still unauthenticated; removing it (it is superseded by the inline single-response path) is a follow-up (#12184) that also ports the TS `@pnpm/pnpr.client` + worker off the two-trip path. --- Written by an agent (Claude Code, claude-opus-4-8).
Review Summary by QodoAuthorize pnpr install packages and remove unauthenticated /v1/files endpoint
WalkthroughsDescription• Authorize served packages against pnpr's access policy in /v1/install - Every package's files checked before serving (401 anonymous, 403 forbidden) - Reuses existing packages: policy, evaluated in-process with no network round trip • Remove unauthenticated /v1/files endpoint entirely - Collapse to single gzipped inline response from /v1/install - Client receives lockfile, stats, store-index entries, and file contents in one round trip • Simplify client-side file handling - Replace two-trip NDJSON streaming with single binary response parsing - Worker receives decompressed payload directly instead of making separate HTTP request Diagramflowchart LR
A["POST /v1/install<br/>with inlineFiles"] -->|resolve + diff| B["Check package<br/>access policy"]
B -->|authorized| C["Build single<br/>gzipped response"]
B -->|denied| D["401/403 error"]
C -->|header + frames| E["Client parses<br/>header"]
E -->|file frames| F["Worker writes<br/>to CAFS"]
G["POST /v1/files<br/>REMOVED"] -.->|no longer exists| H["Eliminated<br/>unauth path"]
File Changes1. pnpr/crates/pnpr/src/install_accelerator.rs
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pnpr/crates/pnpr/src/install_accelerator.rs (1)
232-264:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAuthorize against all resolved packages, not
result.package_index.This gate misses two real paths:
lockfile_onlyreturns an emptypackage_index, andcompute_diff()only populatespackage_indexfor packages the client does not already claim instore_integrities. That means an unauthorized caller can still get the resolved lockfile/metadata for private packages whenever no new index entry is emitted. Move the policy check to the fullpackagesset immediately afterresolve::collect_packages(), beforefetch_uncached().Suggested direction
let packages = resolve::collect_packages(&lockfile, &config.registry); + if let Some(denied) = deny_unauthorized_resolved_packages(policies, &identity, &packages) { + return denied; + } // `--lockfile-only`: pnpm resolves and writes the lockfile but // fetches nothing and links nothing. Skip the tarball fetch + the // file-level diff and return just the lockfile; the client writes it // and stops, so the response carries no `D`/`I` lines. @@ - if let Some(denied) = deny_unauthorized_packages(policies, &identity, &result.package_index) { - return denied; - } - let stats_json = stats_json(&result.stats); inline_response(runtime, &lockfile, &stats_json, &result)🤖 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 232 - 264, The authorization check currently calls deny_unauthorized_packages(...) against result.package_index, which misses cases (lockfile_only and packages already in store_integrities); instead, perform the policy check immediately after resolve::collect_packages() using the full resolved packages list (the `packages` Vec produced by resolve::collect_packages()) before calling resolve::fetch_uncached() or computing diffs; move or add the deny_unauthorized_packages call to use `packages` (mapping to the same identity/package identifier fields used by deny_unauthorized_packages) so unauthorized callers are rejected for any resolved package, including when request.lockfile_only is true or compute_diff omits entries.pnpr/crates/pnpr/src/server.rs (1)
1529-1534:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBump the advertised protocol version for this wire break.
The handshake still says
versions: [1], but v1 is no longer the old contract:/v1/installswitched from NDJSON +/v1/filesto a gzipped length-prefixed binary response, and/v1/filesis gone. An older client that only knows the previous v1 flow will now negotiate successfully and then fail mid-install instead of failing fast at handshake time.Minimal fix here
- (StatusCode::OK, axum::Json(serde_json::json!({ "pnpr": { "versions": [1] } }))).into_response() + (StatusCode::OK, axum::Json(serde_json::json!({ "pnpr": { "versions": [2] } }))).into_response()That needs the matching client/path rollout, or continued support for the old v1 flow during migration.
🤖 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/server.rs` around lines 1529 - 1534, The handshake in serve_pnpr_handshake currently advertises "versions: [1]" but the v1 wire contract changed; update the advertised protocol versions array (e.g., replace 1 with the new protocol number such as 2) in serve_pnpr_handshake so clients will negotiate the correct protocol; ensure this change is coordinated with client/path rollout or alternatively add explicit backward-compat handling in the server for the old v1 flow (so serve_pnpr_handshake only advertises v1 if that legacy flow is still supported).
🤖 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 `@pnpr/client/src/fetchFromPnpmRegistry.ts`:
- Around line 194-213: In fetchFromPnpmRegistry.ts inside the HTTP response
handler (the callback using res, chunks, raw and gunzip), decompress the body
first (respecting content-encoding header or gzip magic bytes) into a single
Buffer/variable, then inspect res.statusCode and either reject with the
decompressed UTF-8 string for non-200 codes or resolve with the decompressed
Buffer for 200; specifically, replace the current branch that rejects on non-200
before decompression with logic that calls gunzip when needed (handling gunzip
errors via reject), sets a decompressed variable for both compressed and
uncompressed paths, and only then checks res.statusCode to decide
resolve/reject.
In `@worker/src/start.ts`:
- Around line 504-520: The code in writeCafsFiles incorrectly re-parses a
header: remove the JSON header parsing and start reading frames at offset = 0
(do not read jsonLen or advance by 4+jsonLen) because fetchFromPnpmRegistry
already strips the length-prefixed header; update the initial payload handling
(variables payload, jsonLen, and offset) so offset is initialized to 0 and any
length checks are adjusted accordingly so the loop that scans for END_MARKER and
reads [digest,size,exec,content] frames in writeCafsFiles works against the raw
frames buffer.
---
Outside diff comments:
In `@pnpr/crates/pnpr/src/install_accelerator.rs`:
- Around line 232-264: The authorization check currently calls
deny_unauthorized_packages(...) against result.package_index, which misses cases
(lockfile_only and packages already in store_integrities); instead, perform the
policy check immediately after resolve::collect_packages() using the full
resolved packages list (the `packages` Vec produced by
resolve::collect_packages()) before calling resolve::fetch_uncached() or
computing diffs; move or add the deny_unauthorized_packages call to use
`packages` (mapping to the same identity/package identifier fields used by
deny_unauthorized_packages) so unauthorized callers are rejected for any
resolved package, including when request.lockfile_only is true or compute_diff
omits entries.
In `@pnpr/crates/pnpr/src/server.rs`:
- Around line 1529-1534: The handshake in serve_pnpr_handshake currently
advertises "versions: [1]" but the v1 wire contract changed; update the
advertised protocol versions array (e.g., replace 1 with the new protocol number
such as 2) in serve_pnpr_handshake so clients will negotiate the correct
protocol; ensure this change is coordinated with client/path rollout or
alternatively add explicit backward-compat handling in the server for the old v1
flow (so serve_pnpr_handshake only advertises v1 if that legacy flow is still
supported).
🪄 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: 7c71eb48-f0de-4ed7-929b-807ddd9ac90a
📒 Files selected for processing (11)
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.tspnpr/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/tests.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/tests/install_accelerator.rsworker/src/index.tsworker/src/start.tsworker/src/types.ts
💤 Files with no reviewable changes (2)
- pnpr/crates/pnpr/tests/install_accelerator.rs
- pnpr/crates/pnpr/src/install_accelerator/protocol.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Follow Standard Style with trailing commas, preferring functions over classes, and declaring functions after they are used (relying on hoisting)
Use a single options object instead of multiple parameters when a function needs more than two or three arguments
Follow Import Order: Standard libraries first, then external dependencies (alphabetically), then relative imports
Write self-documenting code where function names, parameters, and types explain what a function does without requiring prose comments
Do not write comments that restate what the code already says; refactor via renaming, splitting helpers, or restructuring instead
Do not repeat documentation at call sites that already exists in JSDoc on the callee; update JSDoc once for all call sites to benefit
Use JSDoc only for a function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the body
Do not record past implementation shape, refactor history, or 'the previous code did X' framing in code; use git log and git blame instead
Write comments only when: the reason for code is non-obvious (hidden invariant, workaround for known bug, deliberate exception), or the right name doesn't fit (temporary technical constraint)
Files:
worker/src/types.tsworker/src/index.tsworker/src/start.tspnpr/client/src/fetchFromPnpmRegistry.ts
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/diff.rspnpr/crates/pnpr/src/install_accelerator/tests.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/src/install_accelerator.rs
🧠 Learnings (39)
📓 Common learnings
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR
📚 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:
worker/src/types.tsworker/src/index.tsworker/src/start.tspnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-23T16:55:36.507Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: pacquet/crates/cli/tests/lockfile_verification.rs:158-162
Timestamp: 2026-05-23T16:55:36.507Z
Learning: In `pacquet/crates/cli/tests/lockfile_verification.rs`, the `trust_lockfile_skips_verification` and `trust_lockfile_cli_flag_skips_verification` tests intentionally do NOT assert `output.status.success()`. The hand-rolled fixture lockfile uses a placeholder integrity hash (`sha512-AAA…`), so the install always fails the downstream tarball integrity check regardless of the supply-chain gate. The contract being tested is "gate-skipped, not install-succeeded"; asserting success would require generating a real lockfile via the `generate_lockfile` pattern (see `hoist.rs`) which is considered not worth the extra wiring for an opt-out smoke test.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rspnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Tests that need the mocked registry should start `pnpr` through `pacquet-testing-utils`; `cargo test` / `cargo nextest run` should not require a separate `just registry-mock launch` step
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rspnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/*.rs : Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Use snapshot tests with `insta` and carefully review diffs when intentional changes alter snapshots; accept with `cargo insta review` only after careful review
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rspnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : Tests are documentation — do not duplicate test scenarios, edge cases, failure modes, or worked examples in prose when they are already captured by tests
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Prefer `#[cfg_attr(target_os = "windows", ignore = "...")]` (or matching `#[cfg(unix)]` gates) over runtime probe-and-skip helpers for platform-locked tools
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rs
📚 Learning: 2026-05-20T21:18:56.391Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11778
File: pacquet/crates/resolving-local-resolver/tests/resolve.rs:365-372
Timestamp: 2026-05-20T21:18:56.391Z
Learning: In `pacquet/crates/resolving-local-resolver/tests/resolve.rs`, the test `fail_when_resolving_from_not_existing_directory_an_injected_dependency` intentionally uses `injected: false`. The test is a verbatim port of the upstream pnpm TypeScript test (resolving/local-resolver/test/index.ts at ef87f3ccff). The `injected` flag only affects the file/link protocol choice for plain directory paths; when the `file:` scheme is explicit in the bare specifier, the flag has no effect on the resolution code path. The misleading test name is inherited from upstream.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : 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
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Prefer real fixtures over dependency-injection seams — use `tempfile::TempDir`, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Applied to files:
pnpr/crates/pnpr/src/install_accelerator/tests.rspnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: Always explicitly include 'pnpm' in changeset files with appropriate version bump (patch, minor, or major)
Applied to files:
.changeset/pnpr-inline-only-access.md
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: Version pnpm CLI patch for bug fixes, internal refactors, and changes that don't require documentation; minor for new features/settings that should be documented; major for breaking changes
Applied to files:
.changeset/pnpr-inline-only-access.md
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Do not change lockfile format, store layout, `.npmrc` semantics, or CLI surface unless pnpm changed them first
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-24T21:11:04.272Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Match how the same feature is implemented in the TypeScript pnpm CLI — any change in pacquet must match pnpm's behavior, logic, edge cases, config resolution, error messages, file/lockfile formats, and existing tests
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-20T23:08:06.093Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:08:06.093Z
Learning: Pacquet (pnpm's Rust port) has a cardinal rule: "match pnpm exactly — do not fix pnpm quirks unless the same fix has landed in pnpm first." Review comments should not suggest behavioral deviations from upstream pnpm, even when the upstream behavior appears buggy. If a real bug is identified, it must be fixed upstream first.
Applied to files:
.changeset/pnpr-inline-only-access.md
📚 Learning: 2026-05-24T21:11:04.272Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In pacquet (pnpm/pnpm repo), `ResolvedPackage.optional` AND-folding intentionally mirrors pnpm's resolveDependencies.ts:1627-1648 revisit behavior: only the directly-visited package's `optional` flag is updated on revisit, not transitive descendants. pnpm CLI corrects stale optional flags via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`. Pacquet does not yet have this pruner equivalent, so raw `node.optional` flows directly into snapshot/virtual-store via `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up issue to port `copyDependencySubGraph` is planned.
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-26T18:31:14.579Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11967
File: .changeset/git-fetcher-reject-non-sha-commits.md:2-2
Timestamp: 2026-05-26T18:31:14.579Z
Learning: In the pnpm monorepo, the `fetching/` directory contains multiple separate npm packages each with their own scoped name using a dot-separator convention, e.g., `pnpm/fetching.git-fetcher` (declared in `fetching/git-fetcher/package.json`) and `pnpm/fetching.tarball-fetcher`. There is no top-level `pnpm/fetching` package. Changesets targeting the git-fetcher should use `"pnpm/fetching.git-fetcher"`.
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-18T20:35:22.917Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11729
File: pacquet/crates/resolving-npm-resolver/src/fetch_attestation_published_at.rs:55-57
Timestamp: 2026-05-18T20:35:22.917Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/fetch_attestation_published_at.rs`, the npm attestation endpoint (`/-/npm/v1/attestations/{pkg_name}@{version}`) intentionally does NOT percent-encode the package name — the endpoint accepts literal `/` in scoped package names (e.g. `scope/pkg`). This matches upstream pnpm's `fetchAttestationPublishedAt.ts` behavior. Do not flag missing URL encoding here. By contrast, the full-metadata fetch paths (`fetch_full_metadata`, `fetch_full_metadata_cached`) DO percent-encode via the `registry_url::to_registry_url` helper, matching upstream's `toUri` behavior.
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-23T17:30:06.849Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: resolving/npm-resolver/src/createNpmResolutionVerifier.ts:381-418
Timestamp: 2026-05-23T17:30:06.849Z
Learning: In `resolving/npm-resolver/src/pickPackage.ts` (pnpm/pnpm), the resolver's `PackageMetaCache` keys by `name` (abbreviated) and `name:full` (full metadata) only — no registry component is included. This is a pre-existing limitation meaning that if two different registries serve packages of the same name in one install, the cache will only hold the first fetched entry. The `createNpmResolutionVerifier.ts` shares this same cache and inherits the limitation; a `validateSharedMeta` name-check guards against cross-package contamination but cannot distinguish same-named packages from different registries. Tightening to a registry-qualified key would require a coordinated change to the resolver's cache key shape. The Pacquet/Rust side is already registry-qualified (`{registry}\x00{name}:full`).
Applied to files:
.changeset/pnpr-inline-only-access.mdpnpr/client/src/fetchFromPnpmRegistry.tspnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-26T21:01:06.666Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.
Applied to files:
.changeset/pnpr-inline-only-access.md
📚 Learning: 2026-05-20T20:41:50.322Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11773
File: pacquet/crates/package-manager/src/install_package_from_registry.rs:111-114
Timestamp: 2026-05-20T20:41:50.322Z
Learning: In pacquet (pnpm/pnpm repo, Rust codebase), `install_package_from_registry` is the npm-only install path. The npm resolver always stamps `ResolveResult.id` (a `PkgResolutionId`) as `nameversion`. Parsing it back through `PkgNameVer` with `.expect()` is intentional — a parse failure means a mis-dispatch bug, not malformed external input. Per pacquet's CLAUDE.md: "Don't add error handling, fallbacks, or validation for scenarios that can't happen. Trust internal code and framework guarantees." Do not suggest replacing such `.expect()` calls with graceful error handling.
Applied to files:
pnpr/client/src/fetchFromPnpmRegistry.tspnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-06-01T08:59:48.719Z
Learnt from: KSXGitHub
Repo: pnpm/pnpm PR: 12093
File: pacquet/crates/cli/src/cli_args/run/recursive.rs:290-315
Timestamp: 2026-06-01T08:59:48.719Z
Learning: In pacquet's recursive run implementation (`pacquet/crates/cli/src/cli_args/run/recursive.rs`), the `pnpm-exec-summary.json` format for failed package entries correctly includes `prefix` and `message` fields in addition to `status` and `duration`. This matches pnpm's `ActionFailure` variant in `cli/utils/src/recursiveSummary.ts` and the direct serialization in `exec/commands/src/exec.ts`. There is no `ExecutionStatusInSummary` type in pnpm. The only intentional divergence is omitting the JS `error` field, whose `JSON.stringify` output is non-deterministic due to non-enumerable `Error` properties.
Applied to files:
pnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-14T07:57:23.823Z
Learnt from: mandarini
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/src/pickPackage.ts:183-221
Timestamp: 2026-05-14T07:57:23.823Z
Learning: In `resolving/npm-resolver/src/pickPackage.ts`, the error-fallback `catch` block (the `loadMeta(pkgMirror)` path that fires when the primary network fetch throws) intentionally does NOT call `maybeUpgradeAbbreviatedMetaForReleaseAge` or retry with `fullMetadata: true`. This is by design: the network is already known-iffy at that point, so an extra fetch would risk compounding the failure. The `ignoreMissingTimeField` warn-and-skip path is the accepted graceful degradation here.
Applied to files:
pnpr/client/src/fetchFromPnpmRegistry.ts
📚 Learning: 2026-05-23T09:14:43.635Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11867
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:726-730
Timestamp: 2026-05-23T09:14:43.635Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs`, the fresh-lockfile path intentionally does not run lifecycle scripts (`BuildModules` is not invoked, and `side_effects_maps_by_snapshot` from `CreateVirtualStoreOutput` is discarded). This is pre-existing behavior documented by an in-code comment mirroring upstream `link.ts:167-170`. The frozen-lockfile path wires `BuildModules` end-to-end as normal. Wiring lifecycle scripts into the fresh-lockfile path is tracked as future work, separate from this file's concern.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-23T09:14:43.635Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11867
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:726-730
Timestamp: 2026-05-23T09:14:43.635Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs`, the fresh-lockfile path intentionally does not invoke `BuildModules` and discards `side_effects_maps_by_snapshot` from `CreateVirtualStoreOutput`. This is pre-existing, documented behavior (mirroring upstream `link.ts:167-170`): `importing_done` fires once extraction and symlink linking are complete, and the fresh-lockfile path does not run lifecycle scripts. The frozen-lockfile path wires `BuildModules` end-to-end as normal. Do not flag this omission as a bug; wiring lifecycle scripts into the fresh-lockfile path is tracked as future work separate from perf refactors.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-06-02T16:13:39.456Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12144
File: pnpr/crates/pnpr/src/install_accelerator/verdict_cache.rs:57-61
Timestamp: 2026-06-02T16:13:39.456Z
Learning: In `pnpr/crates/pnpr/src/install_accelerator/verdict_cache.rs`, the `lockfile_verdicts` SQLite table intentionally uses `hash` alone (not a composite `(hash, policy)` key) as the primary key — last-write-wins per hash. This mirrors the local `lockfile-verified.jsonl` cache design in pnpm. A looser current policy can trust a stricter cached pass via `can_trust_past_check`; alternating-policy re-verification is an accepted trade-off. A composite key was explicitly rejected to maintain parity with the local cache model.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-20T20:41:30.632Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11773
File: pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs:115-117
Timestamp: 2026-05-20T20:41:30.632Z
Learning: In the pacquet Rust port of pnpm, the `is_http_url` helper in `pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs` intentionally uses `bare.starts_with("http:") || bare.starts_with("https:")` (not `"http://"` / `"https://"`) to match upstream pnpm's `startsWith('http:')` / `startsWith('https:')` check byte-for-byte. Pacquet's cardinal rule (pacquet/AGENTS.md) requires matching pnpm even on quirks; malformed non-URL inputs are rejected downstream by `reqwest::Url::parse` as a `ResolveError`.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-20T22:49:17.652Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11787
File: pacquet/crates/catalogs-resolver/src/lib.rs:156-167
Timestamp: 2026-05-20T22:49:17.652Z
Learning: In pacquet's `catalogs-resolver` crate (`pacquet/crates/catalogs-resolver/src/lib.rs`), the protocol detection pattern `catalog_lookup.split(':').next().unwrap_or("")` is an intentional byte-for-byte port of pnpm's upstream JavaScript `getProtocol`/split logic in `catalogs/resolver/src/resolveFromCatalog.ts#L95`. A bare value like `"workspace"` (without a colon) is deliberately classified as the `"workspace"` protocol, matching upstream behavior. pacquet's cardinal rule is to match upstream pnpm behavior including quirks; any behavioral change must land in pnpm first and then be ported here.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/crates/**/Cargo.toml : New registry-only crates must be placed under pnpr/crates/<short-name>/ and named pnpr-<short-name> in Cargo.toml, never using the pacquet- prefix
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to 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
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : User-facing errors go through `miette` via the `pacquet-diagnostics` crate; match pnpm's error codes and messages where pnpm defines them since error codes are part of the public contract
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-25T14:58:11.105Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11931
File: pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs:560-589
Timestamp: 2026-05-25T14:58:11.105Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`, all per-`(registry, name[, version])` caches in `NpmResolutionVerifier` (`published_at`, `full_meta`, `full_meta_for_trust`, `abbreviated_meta`, `local_meta`) intentionally use the same pattern: lock → miss-check → release lock → await fetch/load → re-acquire lock → insert. This uniform pattern is deliberate; do not flag individual caches for using it. The known follow-up improvement (replacing the pattern with `tokio::sync::OnceCell` per key inside a `Mutex<HashMap<…>>`) is tracked as a future structural change to cover all five caches simultaneously.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Follow test-logging guidance — log before non-`assert_eq!` assertions, `dbg!` complex structures, skip logging for simple scalar `assert_eq!`
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Prefer existing pacquet-* crates over writing new code; check pacquet-tarball, pacquet-crypto-hash, pacquet-crypto-shasums-file, pacquet-package-manifest, pacquet-network, pacquet-registry, pacquet-fs, and pacquet-diagnostics before implementing non-trivial functionality
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
📚 Learning: 2026-05-20T10:06:55.749Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11760
File: pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs:15-18
Timestamp: 2026-05-20T10:06:55.749Z
Learning: In `pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs`, the `DirectDep.id`, `ResolvedPackage.id`, and `ResolvedTree.packages` HashMap keys are intentionally plain `String` for now. The natural branded type would be `pacquet_lockfile::PkgNameVer`, but it cannot be used as a HashMap key because `node_semver::Version` does not derive `Hash`. The upstream parity type is `PkgResolutionId` (which carries an optional peer-dep suffix), and the branded type should be introduced alongside peer-dep resolution and lockfile generation work to avoid locking the seam too early.
Applied to files:
pnpr/crates/pnpr/src/install_accelerator.rs
🔇 Additional comments (2)
pnpr/crates/pnpr/src/install_accelerator/tests.rs (1)
1-42: LGTM!Also applies to: 44-73
.changeset/pnpr-inline-only-access.md (1)
1-7: LGTM!
`POST /v1/files` served any CAFS file by digest with no authentication and no package identity, so the access gate on `/v1/install` (which is per package) couldn't cover it — it had to be removed, not gated. It was already superseded by the single-response inline path (#12178). * Server: `/v1/install` always answers with the inline gzipped body (lockfile + stats + store-index entries + the missing files' contents); the NDJSON two-trip path, the `/v1/files` route, `handle_files`, and the `FilesRequest`/`is_valid_sha512_hex` helpers are gone. * TS client + worker: `@pnpm/pnpr.client` now does the one inline request and hands the file frames to `@pnpm/worker`'s `writeCafsFiles`, which writes them to the CAFS; the `fetchAndWriteCafsFiles` /v1/files fetcher is replaced. Error bodies are decompressed before being surfaced, since the server also gzips its JSON error responses (e.g. an access denial). Verified end to end by `pnpm/test/install/pnpmRegistry.ts` (11 tests: install / add / remove / workspace through a real pnpr server). Closes the second half of the install-accelerator access work (#12184); file-bearing responses are now both inline-only and access-gated. --- Written by an agent (Claude Code, claude-opus-4-8).
zkochan
added a commit
that referenced
this pull request
Jun 4, 2026
…ternal private registries (#12184) (#12189) Closes #12184 (part 2). #12181 shipped the per-caller access gate on `POST /v1/install`, which authorizes every served package against pnpr's own `packages:` policy — the complete answer **while pnpr fetches anonymously**. This PR adds the remaining piece: forwarding the caller's per-registry credentials so the accelerator can resolve/fetch **external private** content as the caller, and gating that content per user against the registry that actually owns it. ## Credential forwarding (issue steps 1–2) - **Wire:** `POST /v1/install` gains an `authHeaders` body map (`{ "//host/path/": "Bearer …" }`, the shape `AuthHeaders::from_map` consumes / `getAuthHeadersFromCreds` produces) plus an HTTP `Authorization` header. The body map carries the *upstream* registry tokens; the header identifies the caller to pnpr's own gate and keys the grant table. - **pacquet plumbing:** a request-scoped `Arc<AuthHeaders>` is threaded via a new `Install.auth_override` field and an `auth_override` param on `build_resolution_verifiers`, so resolution/verification run as the caller **without** baking per-user auth into the interned `&'static Config` (which would leak one config per user). - **Server:** `handle_install` builds the per-request `AuthHeaders` and threads it through resolve, verify, and `fetch_uncached` (which now returns the freshly-fetched set). - **Clients:** pacquet `pnpr-client` and `@pnpm/pnpr.client` send `registry` / `namedRegistries` / `authHeaders` + `Authorization`; the TS path sources them from the caller's registry credentials via `@pnpm/network.auth-header` (`getAuthHeadersFromCreds` is newly re-exported). `@pnpm/worker` is unchanged — downloads happen server-side. - **Credential scope:** both clients forward the caller's *full* credential map, not a subset scoped to the declared registries. The registries a dependency graph touches aren't knowable up front — a transitive package can be scope-routed to another registry or pinned to a tarball URL on a host that's in `.npmrc` but isn't a declared registry — so pnpr attaches the right token per fetched URL exactly as a local install does. These are package-fetch credentials going to the very service the caller configured to fetch its packages. ## Per-user grant table (issue steps 3–4) Externally-resolved private content carries no pnpr policy, so the store's possession of the bytes must not authorize a user the upstream never cleared. A served package is dispatched by **whether a forwarded credential was used to fetch it**: - **No forwarded cred → pnpr-as-authority:** the existing local `packages:` policy check, unchanged. - **Forwarded cred → upstream-as-authority:** gated against a persistent `(user, name@version)` grant table (SQLite, modeled on `VerdictCache`). Freshly fetched this request ⇒ record + allow (the upstream just accepted the token). Cache hit with a standing grant ⇒ allow, no upstream trip. Cache hit, no grant ⇒ re-verify against the owning registry with the caller's credential — record on success; **clear-on-discovery** (purge the user's grants for the package) + deny on `401`/`403`. TTL is the `installAccelerator.grantTtl` config knob (default: permanent). ## Public vs private (no per-user gating for public packages) A forwarded credential matching a registry doesn't mean a package is *private* — in a mixed proxy (one registry serving a company's private packages **and** public ones), the token matches everything, and gating public content per user would cost a grant row and a re-verify round trip per user for bytes anyone may read. So before the per-user path, a not-yet-classified cache hit is probed **anonymously**: a `2xx` classifies the package public in a global set (no user pays for it again, no grant, no further round trip); a `401`/`403` means it's genuinely private and falls through to the grant / re-verify path above. Public packages thus cost **one anonymous probe across the whole fleet**, not one per user. ## Tests - pnpr: grant-table + public-set mechanics, regime dispatch, the upstream-authorization paths (fresh-fetch, granted cache hit, private re-verify-and-record, denied-clears-grants, public-classified-once-then-free), and forwarded-cred-routes-around-local-policy. - pacquet `pnpr-client`: a test asserting `authHeaders` + `Authorization` travel on the wire. - Full suites green: `pnpr` (237), `pacquet-package-manager` (389), `pacquet-pnpr-client` (12), `pacquet-network`/`config` (325); clippy `-D warnings`, `cargo fmt`, rustdoc `-D warnings --document-private-items`, `typos`, and the TS compile all clean. ## Scoped follow-ups (not in this PR) - Clear-on-discovery fires at the re-verify hook only. A `401`/`403` during the cold resolve aborts the request anyway (nothing is served); threading the offending package out of the deep resolve error to also clear stale grants for *future* requests needs structured auth errors. - Per-scope external registries route via the default registry, since pacquet doesn't yet surface `@scope:registry` routing in `collect_packages`.
This was referenced Jun 5, 2026
2 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pnpr install accelerator served content-addressed store files without binding them to the caller's access. A sha512 CAFS digest is shared across packages and reveals nothing about access, so the store's possession of a package's bytes is not a capability to receive them — a caller who knows a private package's digest could pull bytes the registry routes return
401for. This PR closes both delivery paths.1. Authorize served packages (
/v1/install)Check every served package against pnpr's own
packages:policy before serving — the same decisionserve_packument/serve_tarballmake, evaluated in process with no network round trip, so a warm shared server keeps its resolution advantage.serve_installresolves the caller'sIdentityfromAuthorization;deny_unauthorized_packagesdenies the install —401anonymous,403authenticated-but-outside-the-allowed-set. The accelerator reuses the exact policy the registry routes use, so there's no separate ACL and no new way to misconfigure.Tests (
install_accelerator/tests.rs, in-process): anonymous →@private/foo⇒401; authenticated →@private/foo⇒ allowed; authenticated-but-not-in-allow-list →@team/foo⇒403; in-allow-list ⇒ allowed; anonymous → public ⇒ allowed.2. Remove the unauthenticated
/v1/filesPOST /v1/filesserved any file by digest with no auth and no package identity, so the per-package gate above can't cover it — it had to be removed, not gated. It was already superseded by the single-response inline path (#12178)./v1/installalways answers with the inline gzipped body (lockfile + stats + store-index entries + the missing files' contents); the NDJSON two-trip path, the/v1/filesroute,handle_files, andFilesRequest/is_valid_sha512_hexare gone.@pnpm/pnpr.clientnow does the one inline request and hands the file frames to@pnpm/worker'swriteCafsFiles, which writes them to the CAFS; thefetchAndWriteCafsFiles/v1/filesfetcher is replaced.Verified:
cargo test -p pnpr(138 + integration) green,cargo clippy -p pnprclean;@pnpm/worker,@pnpm/pnpr.client, and@pnpm/installing.deps-installercompile.Scope note / follow-up
This authorizes against pnpr's own surface — the authority for everything the store can hold today (pnpr fetches anonymously, so cached content is pnpr-hosted or publicly fetchable). When credential forwarding lands, packages the client resolves from external registries under its own token carry no pnpr policy and will need per-caller re-verification against the owning registry (a per-
(user, name@version)grant table, cleared on observed denial, with an optional TTL). Tracked in #12184; marked atdeny_unauthorized_packages.Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
Security Enhancements
Performance Improvements
Tests