GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix berry restore keeping removed descriptors (#1435) by mikolalysenko · Pull Request #1436 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

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

Fixes #1435

Summary

After a hosted yarn berry pin, a yarn remove in one workspace member
(while another member still uses the package) followed by
socket-patch rollback or remove <pkg> exited 0. It left a lock that
every yarn install --immutable rejected with YN0028. Rollback and
remove now restore the registry entry keyed only by the descriptors the
project still declares, so the next immutable install passes.

Root cause

A hosted berry pin routes each descriptor of the patched package through
its own root resolutions selector and keys the lock entry by the
tarball URL. restore_berry
(crates/socket-patch-core/src/patch/redirect/upstream/npm.rs) rebuilt
the restored key from every selector routed to that URL. It never
checked whether any lock entry still declares that descriptor. #1321
(retire_stale_selectors) only covers a URL entry that is gone entirely.

Yarn keeps each dependent's original descriptor in its lock
dependencies: map (checked with yarn 4.9.1: after the yarn remove,
member a's entry loses left-pad: "npm:1.3.0" and b keeps
left-pad: "npm:^1.3.0"), so the lock itself says which descriptors
are still in use.

Fix

  • berry_declared_descriptors reads, once per restore, the exact
    descriptors the lock declares. These come from every dependency map
    and from the descriptor a patch: descriptor wraps, in a key or a
    dependency map. berry_descriptor_declared matches yarn 4's
    npm:range and yarn 2/3's bare range spellings.
  • The restored key keeps only the declared descriptors. All routed
    selectors are still removed from package.json.
  • If the lock declares none of them (a spelling this doesn't read), the
    previous behavior is kept, so nothing that restores today regresses.

Tests

Issue Test Without fix With fix
#1435 in_process_redirect::yarn_berry_unwind_drops_a_descriptor_no_entry_declares (rollback and remove <purl>, stale selector undeclared vs declared by a real workspace member) fails: restored key "in-proc-redirect@npm:1.0.0, in-proc-redirect@npm:^1.0.0" passes
#1435 npm::tests::berry_descriptor_declared_reads_what_the_lock_still_uses (yarn 4/3 spellings, patch: key and dep, alias, scoped namesake, prerelease range) n/a (new helper) passes

Real-yarn check (yarn 4.9.1, the issue's two-workspace repro with a local
tarball server): with the old two-descriptor key yarn install --immutable fails YN0028 (-"left-pad@npm:1.3.0, left-pad@npm:^1.3.0"
/ +"left-pad@npm:^1.3.0"). With the key this PR writes, it passes.

Local: cargo fmt --all -- --check ok; cargo clippy --workspace --all-features -- -D warnings ok. Suites: socket-patch-core --lib
6357 passed, hosted_inventory 39/39, in_process_redirect 140/143,
in_process_rollback_hosted 42/42, hosted_memory_engine and
hosted_memory_parity all pass. The 7 failures (4 core, 3
in_process_redirect) are chmod-based write-failure tests. They fail the
same way on main here because this sandbox runs as root. A full
cargo test --workspace run ran out of the sandbox's disk while linking,
so I ran these suites instead.
I couldn't run the real-yarn e2e matrix (scripts/yarn-berry-vex-matrix.sh)
here because corepack can't fetch yarn in this sandbox. CI runs it.

/code-review high

I fixed these findings in 3977e12:

  • Substring false positives from referenced (a scoped namesake or a
    longer range counted as a declaration). Replaced with exact matching.
  • A lock re-scan for each selector. It is now read once.
  • The duplicated doc comment on referenced. The change was reverted.
  • The alias test case, which tested the wrong form.
  • A fixture with no workspaces field.

Not fixed: the silent fallback when no descriptor matches. That fallback
keeps today's behavior for lock spellings this code doesn't read. A
new warning code there would fire only on shapes nobody has reported.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JbxZ6TGnJBvpiAJkB9N2XZ


Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@claude
Start fix for #1435
f1a95af
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 12, 2026
Hosted yarn berry rollback/remove put back a descriptor that yarn remove deleted from one workspace member, so every yarn install --immutable fails YN0028 #1435
Closed
Claude (claude) added 2 commits October 12, 2026 16:36
@claude
Keep removed descriptors out of berry restore
4893864
A hosted yarn berry pin routes each descriptor of the patched package
through its own root `resolutions` selector. After `yarn remove` in one
workspace member (another member still using the package), rollback
and remove rebuilt the restored lock key from every selector, putting
the removed member's descriptor back. Every `yarn install --immutable`
then failed with YN0028 even though the command reported success.

The restore now keys the entry only by the descriptors some lock entry
still declares (a dependency map, in yarn 4's `npm:` or yarn 2/3's bare
spelling, or a `patch:` wrapper). All routed selectors are still
dropped. If the lock declares none of them, the old behavior is kept.

Fixes #1435

Assisted-by: Claude Code:claude-opus-5-5
@claude
Match declared berry descriptors exactly
3977e12
Read what the lock still declares once per restore, as a set of exact
descriptors from dependency maps and `patch:` wrappers, instead of a
substring scan per selector. A scoped namesake (`@acme/left-pad`) or a
longer range (`1.3.0-rc.1`) no longer counts as declaring
`left-pad@npm:1.3.0`, so its stale selector can't slip back into the
restored key.

Refs #1435

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

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor[bot]
cursor Bot reviewed Oct 12, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/socket-patch-core/src/patch/redirect/upstream/npm.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

cursor[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor[bot]
cursor Bot reviewed Oct 12, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

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 3977e12. Configure here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@cursor

cursor Bot commented Oct 12, 2026

Copy link
Copy Markdown

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Peer ranges dropped from restored key
    • Created dependency_and_peer_descriptors() to read both dependency types and updated berry_declared_descriptors to use it, ensuring peer-only descriptors are retained in restored lock keys.

Create PR

Or push these changes by commenting:

@cursor push 95f8bed94a
Preview (95f8bed94a)
diff --git a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs
--- a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs
+++ b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs
@@ -60,6 +60,44 @@
     out
 }
 
+/// The descriptors (`name@range`) an entry's `dependencies:` and
+/// `peerDependencies:` maps name. Yarn keys a lock entry by the union of
+/// every descriptor that resolved to it, including peer requests.
+pub(crate) fn dependency_and_peer_descriptors(stanza: &str) -> Vec<String> {
+    let mut out = Vec::new();
+    let mut in_deps = false;
+    for line in stanza.lines().skip(1) {
+        if let Some(field) = line.strip_prefix("  ").filter(|r| !r.starts_with(' ')) {
+            in_deps =
+                field.trim_end() == "dependencies:" || field.trim_end() == "peerDependencies:";
+            continue;
+        }
+        if !in_deps {
+            continue;
+        }
+        let Some(rest) = line.strip_prefix("    ").filter(|r| !r.starts_with(' ')) else {
+            continue;
+        };
+        let (name, range) = match rest.strip_prefix('"') {
+            Some(quoted) => match quoted.split_once("\": ") {
+                Some((name, range)) => (name, range),
+                None => continue,
+            },
+            None => match rest.split_once(": ") {
+                Some((name, range)) => (name, range),
+                None => continue,
+            },
+        };
+        let range = range.trim_end();
+        let range = range
+            .strip_prefix('"')
+            .and_then(|r| r.strip_suffix('"'))
+            .unwrap_or(range);
+        out.push(format!("{name}@{range}"));
+    }
+    out
+}
+
 /// The descriptors `before` depends on that `after` (the same entry,
 /// rewritten) no longer does: the edges a rewrite cut.
 pub(crate) fn cut_descriptors(before: &str, after: &str) -> Vec<String> {

diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs
--- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs
+++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs
@@ -1068,13 +1068,13 @@
 }
 
 /// The descriptors a berry lock's `stanzas` still declare: every
-/// entry's `dependencies:` map, plus the descriptor a `patch:` descriptor
-/// wraps (`name@patch:name@npm%3A1.3.0#…` wraps `name@npm:1.3.0`), in
-/// either a dependency map or a key. Yarn 4 spells a registry range
-/// `npm:range`, yarn 2 and 3 a bare `range`; see
+/// entry's `dependencies:` and `peerDependencies:` map, plus the
+/// descriptor a `patch:` descriptor wraps (`name@patch:name@npm%3A1.3.0#…`
+/// wraps `name@npm:1.3.0`), in either a dependency map or a key. Yarn 4
+/// spells a registry range `npm:range`, yarn 2 and 3 a bare `range`; see
 /// [`berry_descriptor_declared`].
 fn berry_declared_descriptors(stanzas: &[String]) -> BTreeSet<String> {
-    use crate::formats::yarn::berry_prune::dependency_descriptors;
+    use crate::formats::yarn::berry_prune::dependency_and_peer_descriptors;
     use crate::formats::yarn::patterns::split_berry_key_patterns;
     use crate::formats::yarn::stanzas::stanza_key;
 
@@ -1083,7 +1083,7 @@
         let keys = stanza_key(stanza)
             .map(split_berry_key_patterns)
             .unwrap_or_default();
-        for descriptor in dependency_descriptors(stanza) {
+        for descriptor in dependency_and_peer_descriptors(stanza) {
             if let Some(inner) = berry_patch_inner(&descriptor) {
                 out.insert(inner);
             }

You can send follow-ups to the cloud agent here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay182003
Tanmay Singla (Tanmay182003) approved these changes Oct 12, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 12, 2026
Merged via the queue into main with commit d198fe6 Oct 12, 2026
87 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-berry-restore-stale-selectors branch October 12, 2026 17:58
This was referenced Oct 12, 2026
CI flake: coverage job fails intermittently in vendor / in_process_edge_cases #1416
Closed
chore(release): bump to 5.0.1 with release notes #1446
Closed
Vendored yarn berry revert restores the pre-vendor lock entry verbatim, so after upgrading from yarn 4.0.x it writes back a bare-hex checksum and every yarn install --immutable fails YN0028 #759
Closed
Fix vendored berry revert restoring stale lock (#759) #1464
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@Tanmay182003 Tanmay Singla (Tanmay182003) Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Hosted yarn berry rollback/remove put back a descriptor that yarn remove deleted from one workspace member, so every yarn install --immutable fails YN0028

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@mikolalysenko @Tanmay182003 @claude
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.