Skip to content

Fix yarn berry pin entry rendering (#697, #718) - #719

Open
Mikola Lysenko (mikolalysenko) wants to merge 7 commits into
mainfrom
agent/fix-yarn-berry-pin-entry-render
Open

Mikola Lysenko (mikolalysenko) wants to merge 7 commits into
mainfrom
agent/fix-yarn-berry-pin-entry-render

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #697
Fixes #718

Summary

Yarn Berry pins written by scan --mode hosted and vendor / scan --mode vendored could produce a yarn.lock that yarn rewrites on the next install. CI then fails with yarn install --immutable (YN0028), and vendored projects hit it on every install. This PR makes both writers produce the entry exactly as yarn writes it.

Shared root cause

Both writers built the pinned entry by copying the registry npm: entry's body and inserting checksum: wherever it fit:

  • hosted (patch/redirect/mod.rs): kept every registry line and, if the entry had no checksum, inserted one directly after resolution:;
  • vendored (vendor/yarn_berry_lock.rs): wrote version, resolution, the carried sections, then checksum, languageName, linkType.

Yarn does something different when it re-resolves the pin:

  1. Field order (Yarn berry vendored and hosted pins put checksum: out of yarn's field order on platform-conditional lock entries (conditions: os=…), so every yarn install --immutable fails YN0028 #697). Yarn always writes the fields in one order: version, resolution, dependencies, peerDependencies, dependenciesMeta, peerDependenciesMeta, then every other field alphabetically (bin, checksum, conditions, languageName, linkType). Platform-conditional packages carry a conditions: line, so a checksum written after it (vendored) or before the dependency maps (hosted) gets moved.
  2. bin: comes from the tarball (Yarn berry vendored and hosted pins copy the registry entry's bin: paths, but yarn re-reads them from the tarball (./dist/bin/uuid), so vendored installs and hardened hosted installs fail YN0028 for packages like uuid and acorn #718). For a tarball-URL or file: locator, yarn builds the entry from the tarball's own package.json. That file keeps the published spelling (./dist/bin/uuid), but the registry metadata has a normalized one (dist/bin/uuid).

Fix

  • New formats::yarn::berry_entry holds one pure renderer, render_pinned_entry, that both writers now use. It orders fields the way yarn's lockfile serializer does and replaces bin: with the tarball manifest's map. That map is read the way yarn's Manifest reads it: scope dropped from keys, a string bin names the package itself, backslashes become /, and values are quoted the way yarn's YAML writer quotes them.
  • Vendored reads bin from the package.json inside the tarball it just packed and verified.
  • Hosted needs the served tarball's package.json. It now downloads the served tarball, the same way it already downloads wheel metadata for uv, and checks it against the grant's sha512. It only does this for berry lock entries that have a bin: map, so berry projects without bins make no extra requests. Both the CLI flow (scan/hosted.rs) and the in-memory engine (hosted::memory, which the Node binding uses) do this. If the tarball can't be fetched or read, the patch is skipped as npm_manifest_unavailable (redacted detail) rather than pinning an entry yarn would rewrite.
  • The URL-keyed metadata map the redirect already took (wheel METADATA) now also carries these manifests. Only the berry rewriter reads npm URLs from it.

Verification with real yarn 4.12.0 (manual, this run)

Tests (red → green)

Run on the pre-fix code, the four regression tests failed (test result: FAILED. 0 passed; 4 failed). With the fix, they pass.

Issue Test
#697 hosted patch::redirect::tests::issue_697_checksum_lands_in_yarn_field_order
#697 vendored vendor::yarn_berry_lock::tests::issue_697_conditional_entry_keeps_yarn_field_order
#718 hosted patch::redirect::tests::issue_718_bin_comes_from_the_served_tarball_manifest, CLI scan::hosted_yarn_berry_manifest::issue_718_hosted_pin_takes_bin_from_the_served_tarball, in-memory hosted_memory_engine::yarn_berry_pin_takes_bin_from_the_served_tarball
#718 vendored vendor::yarn_berry_lock::tests::issue_718_bin_map_comes_from_the_tarball_manifest

The fetch targeting has its own test, patch::redirect::tests::berry_pin_needs_manifest_only_for_entries_the_pin_rekeys, added after Bugbot found the fork-alias case. New unit tests also cover the renderer (formats::yarn::berry_entry::tests), the manifest decoder (hosted::npm_manifest::tests), and the fetch-failure skip (scan::hosted_yarn_berry_manifest::unfetchable_served_manifest_skips_the_patch).

Local checks:

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test -p socket-patch-core --all-features --lib: 4853 passed, 4 failed. The 4 (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…) inject failures with read-only permissions, which root ignores. This sandbox runs as uid 0, and those files are untouched by this PR.
  • cargo test -p socket-patch-cli --all-features --test scan --test hosted_memory_engine ...: all green for scan (106), hosted_memory_engine (29), in_process_vendor (104) and the real-yarn 4.12.0 suites e2e_redirect_yarn_berry_build (14), e2e_vendor_yarn_berry_build (14), e2e_yarn4_pnpm_linker_build (15), e2e_yarn4_workspaces_build (13). The sandbox-only failures: 3 write-failure-injection tests in in_process_redirect (root again), and 2 mode_migration_npm berry tests whose harness fetches left-pad from registry.npmjs.org with reqwest and rejects the sandbox proxy CA (InvalidCertificate(UnknownIssuer)) before socket-patch runs. CI covers both.
  • cargo fmt --all -- --check is not clean on main with the pinned 1.93.1 toolchain (131 files), and CI doesn't run it, so I only formatted the code this PR adds. I didn't reformat whole files.
  • A full cargo test --workspace --all-features build of every test binary didn't fit in this session's disk allowance, so I'm relying on CI for the complete run.
  • Wrappers (npm/, pypi/, gem/) only dispatch to the binary, so they needed no changes. The Node binding gets the fix through run_in_memory.

🤖 Generated with Claude Code

https://claude.ai/code/session_015eyeXdc6C6LN4UcGYpw3hR


Note

Medium Risk
Changes hosted redirect and lock rewrite behavior for Yarn Berry projects with bin: entries; failed manifest fetches now skip patches instead of pinning, which is safer but changes outcomes for offline or broken artifact URLs.

Overview
Fixes yarn Berry hosted and vendored lock pins so yarn install --immutable no longer fails (YN0028).

Shared renderer (#697, #718): Adds formats::yarn::berry_entry with render_pinned_entry, used by both the hosted redirect and vendored yarn_berry_lock writers instead of copying registry lines and regex-splicing resolution/checksum. Entries now match yarn’s field order and YAML scalar quoting; bin: comes from the tarball’s package.json via manifest_bin, not the registry lock entry.

Hosted scan (#718): For berry locks whose pin would re-key an entry that already has bin:, the CLI and in-memory engine download the served npm tarball (sha512-checked), decode package.json through new hosted::npm_manifest, and pass manifests in the same URL-keyed metadata map as wheel METADATA. Unfetchable tarballs skip the patch as npm_manifest_unavailable (redacted URL) instead of writing a pin yarn would rewrite. yarn_berry_manifest_targets / berry_pin_needs_manifest limit fetches to re-keyable bin entries (not fork aliases).

Docs: docs/ecosystems.md documents the manifest fetch and skip reason for yarn berry hosted redirects.

Reviewed by Cursor Bugbot for commit 635ca29. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Hosted and vendored Yarn Berry pins copied the registry entry's body
and spliced checksum: in wherever it fit. Yarn re-renders the entry
from the tarball's own package.json in a fixed field order, so
platform-conditional packages (checksum before conditions:, #697) and
packages whose bin paths the registry normalizes (uuid, acorn, #718)
got a lock that every yarn install --immutable rewrote (YN0028).

Both writers now build the entry through one renderer that orders
fields as yarn does and takes bin: from the tarball manifest: the
vendored tarball's own package.json, or the served tarball's when the
hosted caller supplies it.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the agent/fix-yarn-berry-pin-entry-render branch from 0a06c9d to 4703893 Compare October 3, 2026 19:38
The hosted yarn berry pin needs the served tarball's own package.json
to write the bin: map yarn will expect (#718). Hosted scans now fetch
that tarball, the way they already fetch wheel metadata, but only for
berry lock entries that carry a bin: map, so projects without bins
make no extra requests. Both the CLI scan and the in-memory engine do
this. If the tarball can't be fetched or read, the patch is skipped as
npm_manifest_unavailable instead of pinning an entry yarn would
rewrite on the next install.

Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 3, 2026 20:05
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/hosted/engine.rs Outdated
The served-manifest fetch matched any lock key that started with the
package name, so a fork alias (left-pad@npm:other@...) with a bin: map
could queue a download. If that download failed, the real patch was
dropped. The fetch now targets only entries the berry pin would
re-key: plain npm: descriptors of the package, or an earlier hosted
pin of it.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] PDM patch compatibility / native (ubuntu-latest, 2.22.4) failed one cell on 55d4244 (direct agent: appliedExactlyOne). That's PyPI agent mode, which this PR never touches, and the same workflow passed on 900f5d3; the only change since then is the yarn berry fetch targeting. I re-ran the failed job once and it passed, so every check on the head is green.


Generated by Claude Code

Hosted yarn berry scans got about 40% slower: the served-manifest
targeting split and copied every lock entry once per patched package,
even in projects with no bin: maps. The lock is now split once,
keeping only entries that carry bin:, and each package is checked
against just those (usually none). Locally the yarn-berry hosted and
rescan benchmarks went from +37%/+45% back to within noise.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] scan performance was red on 55d4244 (and 900f5d3): yarn-berry/hosted +46%, yarn-berry/rescan +51%. The cause was this PR. The served-manifest targeting re-split (and, on 55d4244, copied) every lock entry once per patched package. That's 60 × 3000 entries in the bench fixture, even though none of them has a bin: map. 635ca29 splits the lock once into its bin: entries and checks each package against only those. Local socket-patch-bench compare --filter yarn-berry (release builds, interleaved): old head +36.9% / +44.8% (Regression), fixed head +3.4% / +3.5% and +8.1% / +0.9% over two runs (Unchanged).


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 635ca29. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Three CI failures on 635ca29 turned out to be infrastructure, not this PR. Each failed job was re-run once and passed, and all 491 checks on 635ca29 are now green.

  • PDM patch compatibility: one cell failed per job, and a different cell each time (crlf hosted / marker-excluded hosted rescanIdempotent, extras vendored appliedExactlyOne). Those cells passed in other runs. The new code only runs when a yarn berry yarn.lock is present, so the PDM paths are the same as on 900f5d3, where this workflow passed.
  • CI / coverage-docker (deno): the Docker image build failed at deno --version (exit 127), before any socket-patch code ran.

Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 3, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review at head 635ca29.

  • CI: 491/491 check runs on 635ca29 are green. The three infra failures (PDM cells, a deno Docker image build) each passed on a single re-run, as the previous comment explains.
  • Bugbot: reviewed 635ca29 and found no new issues. The one earlier thread (hosted/engine.rs) is resolved and outdated.
  • Merges cleanly: 0 commits behind main.
  • Reviewer focus:
    • The new shared formats::yarn::berry_entry::render_pinned_entry replaces line-copying and regex splicing in both the hosted and vendored berry writers.
    • Hosted scans now download the served tarball to read bin:. An unfetchable tarball skips the patch as npm_manifest_unavailable instead of pinning it, which changes behavior offline.

Slack announcement not sent: this run has no Slack send tool.


Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

2 participants