Repository navigation
Fix pnpm 12.0–12.12 scoped tarball: restore (#1619) - #1678
Mikola Lysenko (mikolalysenko) merged 3 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
pnpm 12.0.0 through 12.12.x record the tarball URL of every package installed from a scoped registry, because they compare it with the default registry's conventional URL instead of the scope's. Rollback, remove and the hosted to vendored takeover dropped that URL from the restored pnpm-lock.yaml, so the lock no longer matched what pnpm wrote. The restore now reads the pnpm minor from the same install record, package.json pin or rush.json pnpmVersion it already uses, and for those releases judges a scoped package's URL against the default registry, both when restoring the entry and when reading the lock's lockfileIncludeTarballUrl evidence. pnpm 11 and 12.13+ are unchanged. Fixes #1619 Assisted-by: Claude Code:claude-opus-5-5
Fold the two derivability calls in the pnpm restore policy into one, share the rule for whether a pnpm-named registry is the project's own with the lock-evidence check, and unit-test the policy with a non-npmjs default registry next to the scope registry. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 87750af. Configure here.
|
Ready for review at Generated by Claude Code |
Final review briefWhat it does. pnpm 12.0–12.12 record Risk: low. Behavior changes only when the pnpm version is known to be 12.0–12.12 and the package is scoped; every other version takes the old path ( Look here
Verified. Read the full diff and Changes I made. None. Open questions. None blocking. Known limits (documented in the PR): no version evidence → old behavior; Auto-merge is armed, so approving sends it straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1619
Summary
pnpm 12.0.0 through 12.12.x write
tarball:for every package from a scoped registry (@scope:registry=in.npmrc, orregistries."@scope"in pnpm-workspace.yaml), even withlockfileIncludeTarballUrloff. Until now, a hostedrollback, aremoveof the scoped pin and the hosted → vendored takeover all wrote those entries back withouttarball:, so the restoredpnpm-lock.yamlno longer matched what pnpm wrote. With this change they write the URL back. pnpm ≤ 11 and pnpm ≥ 12.13 behave as before.Root cause
pnpm 12.0–12.12 decide whether to record a tarball by comparing it with the conventional URL under the default registry, not under the package's scope registry. pnpm 12.13.0 fixed this upstream (pnpm#16866).
restore_pnpm_locks(patch/redirect/upstream/npm.rs) always compared with the scope registry, so it treated the URL as one pnpm derives and dropped it.pnpm_lock_tarball_evidencehad the same blind spot: in a lock with only scoped entries, it would read such a URL as proof thatlockfileIncludeTarballUrlis on.All three paths (rollback, remove, takeover) go through
upstream::restore_upstream→restore_pnpm_locks, so one fix covers them.Change
tarball:URLs the lock never had whenlockfileIncludeTarballUrlsits in a settings file the installed pnpm ignores (workspace file on pnpm 9,.npmrcon pnpm 11/12) #902 tarball policy already uses:node_modules/.modules.yaml, package.jsonpackageManager, or rush.jsonpnpmVersion(pnpm_spec_version/*_pnpm_version; the*_majorhelpers now wrap these).pnpm_records_scoped_tarballsis true for 12.0–12.12. When it is,pnpm_tarball_registrycompares a scoped name with the default registry. BothPnpmTarballPolicy::derives_tarball(the restore) and the lock-evidence check use it.Test evidence
in_process_redirect::pnpm_12_restore_keeps_the_scoped_registry_tarball: covers rollback andremove pkg:npm/%40socktest/scoped-pkg@1.0.0on a lock with a scoped entry plus a bare npmjs entry (the shape of the issue's repro). pnpm 12.12.1, 12.0.0 and 12.11.2 keeptarball:; 12.13.0, 12.14.0 and 11.28.6 stay bare. The pnpm version comes from.modules.yamlor the package.json pin, and the scope registry from.npmrcor pnpm-workspace.yamlregistries.pnpm 12.12.1 rollback (modules true, workspace false)restored{integrity: sha512-UPSTREAMupstream==}where pnpm had written{integrity: …, tarball: http://127.0.0.1:…/scoped/@socktest/scoped-pkg/-/scoped-pkg-1.0.0.tgz}. Green after.upstream::npm::tests):pnpm_12_before_13_records_scoped_tarballs(version bounds),pnpm_12_scoped_tarball_is_no_include_tarball_evidence(evidence check),pnpm_12_policy_compares_scoped_tarballs_with_the_default_registry(policy with a non-npmjs default mirror).cargo fmt --all -- --check: okcargo clippy --workspace --all-features -- -D warnings: okcargo test -p socket-patch-cli --all-features --test e2e_redirect_pnpm_build -- --ignored --skip pnpm_pinned_matrix: 11/11 passedin_process_redirect: passedcargo test --workspace --all-features --no-fail-fast: 234 test binaries ok. The failures are all from this sandbox, not the change: 9 permission-denial tests can't fail when run as root (chmod 0o555doesn't stop root; e.g.repair_cleanup_failure_is_reported…,an_unremovable_hidden_lock…,wire_write_failure_maps_error…), ande2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managedfetches a blob from the livepatches-api.socket.dev, which this sandbox can't reach. None of them touch pnpm code. CI runs as non-root with network access.Per-issue checklist
tarball:URL pnpm recorded, so the restored lock isn't byte-exact #1619 rollback:pnpm_12_restore_keeps_the_scoped_registry_tarball(rollback rows)tarball:URL pnpm recorded, so the restored lock isn't byte-exact #1619 remove of the scoped pin: same test (remove rows)tarball:URL pnpm recorded, so the restored lock isn't byte-exact #1619 hosted → vendored takeover: samerestore_upstream→restore_pnpm_lockspath (commands/vendor.rs); no separate e2eregistries."@scope"workspace spelling: workspace rowsReview notes (/code-review high)
Fixed: merged the two duplicate derivability calls into one, shared the "own registry" rule (
pnpm_registry_is_own), and added a unit test of the policy branch with a non-npmjs default. Not fixed, with reasons:.modules.yamlnames the pnpm that last installed the project, which may not be the one that wrote the lock: this is the same tier-2 assumption the Hosted pnpm rollback/remove adds registrytarball:URLs the lock never had whenlockfileIncludeTarballUrlsits in a settings file the installed pnpm ignores (workspace file on pnpm 9,.npmrcon pnpm 11/12) #902 policy already makes.${VAR}scope registry is read as unset: this predates the change and applies to every registry lookup.https://claude.ai/code/session_01SRLnkLy52eFiW2Un4SHYXU
Generated by Claude Code