Fix berry restore keeping removed descriptors (#1435)

[Mikola Lysenko (mikolalysenko)]

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

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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.}

[cursor]

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.

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.}