Skip to content

fix(lockfile): preserve pnpm registry tarball urls#378

Merged
jdx merged 1 commit intomainfrom
codex/pnpm-native-compat-roundtrip
Apr 29, 2026
Merged

fix(lockfile): preserve pnpm registry tarball urls#378
jdx merged 1 commit intomainfrom
codex/pnpm-native-compat-roundtrip

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented Apr 29, 2026 •
edited by cursor Bot
Loading

Summary

  • preserve pnpm registry packages as registry packages when lockfileIncludeTarballUrl records a tarball URL
  • add an adversarial pnpm roundtrip test covering aliases, catalogs, overrides, patches, skipped optionals, peer metadata, platform optionals, and section ordering together

Root Cause

The pnpm parser classified any resolution.tarball HTTP URL as a remote-tarball dependency. That is only correct for URL-keyed package entries; ordinary name@version registry entries can also carry tarball URLs when lockfileIncludeTarballUrl=true.

Validation

  • cargo fmt --check
  • cargo test -p aube-lockfile

Note

Medium Risk
Touches pnpm lockfile parsing/classification used by the installer; misclassification fixes are targeted but can change how dependencies are fetched/resolved for some lockfiles.

Overview
Preserves pnpm registry dependencies as registry-sourced even when lockfileIncludeTarballUrl adds an HTTP resolution.tarball, by only treating RemoteTarball as local_source for URL-keyed entries.

Adds an adversarial pnpm v9 parse→write→reparse test that exercises multiple native lockfile features together (overrides, catalogs, patched deps, aliasing, skipped/ignored optionals, peer metadata, platform constraints) and asserts top-level section ordering and tarball URL retention.

Reviewed by Cursor Bugbot for commit e94a504. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR fixes a mis-classification in the pnpm lockfile parser: when lockfileIncludeTarballUrl: true is set, ordinary name@version entries gain a resolution.tarball HTTP URL that was being interpreted as a remote-tarball dependency, causing installs to fail. The fix checks version_is_http_url (already computed earlier in the same loop) to gate the RemoteTarball classification to URL-keyed entries only, and adds a thorough adversarial roundtrip test covering aliases, catalogs, overrides, patches, peer metadata, platform optionals, skipped optionals, and section ordering in a single pass.

Confidence Score: 5/5

Safe to merge — the fix is minimal and correct, and the new test directly exercises the regression scenario via a full parse/write/reparse cycle.

No P0 or P1 findings. The guard reuses an already-computed boolean, does not alter behavior for URL-keyed entries, and leaves all other LocalSource variants untouched. The adversarial test catches the regression and also validates a broad set of pnpm-specific features.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube-lockfile/src/pnpm.rs Adds a 7-line guard after local_source_from_resolution that discards spurious RemoteTarball classifications on non-URL-keyed entries caused by lockfileIncludeTarballUrl; backs it with a comprehensive adversarial roundtrip test.

Reviews (1): Last reviewed commit: "fix(lockfile): preserve pnpm registry ta..." | Re-trigger Greptile

@jdx jdx merged commit 97cf455 into main Apr 29, 2026
19 checks passed
@jdx jdx deleted the codex/pnpm-native-compat-roundtrip branch April 29, 2026 13:51
@greptile-apps greptile-apps Bot mentioned this pull request Apr 29, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jdx