From f1a95af4e9a3c0aff8f059bfef389911c5f60ba2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:23:27 +0000 Subject: [PATCH 1/3] Start fix for #1435 Assisted-by: Claude Code:claude-opus-5-5 From 4893864705752960e3ff140ee1a06a3d785377c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:36:41 +0000 Subject: [PATCH 2/3] Keep removed descriptors out of berry restore 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 --- .../tests/in_process_redirect.rs | 79 +++++++++++++++++++ .../src/formats/yarn/berry_prune.rs | 5 +- .../src/patch/redirect/upstream/npm.rs | 63 +++++++++++++++ 3 files changed, 146 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index 3b5b178f8..5cf2b5804 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -5408,6 +5408,85 @@ async fn yarn_berry_unwind_merges_into_a_registry_entry_added_after_pinning() { } } +/// #1435: a hosted berry pin routes each descriptor through its own +/// `resolutions` selector. `yarn remove` in one workspace member, while +/// another still uses the package, keeps the URL-keyed entry and both +/// selectors. `rollback` and `remove` must rebuild the restored key from +/// the descriptors the lock still declares, not from every selector, or +/// every `yarn install --immutable` fails YN0028. A descriptor another +/// entry still declares stays in the key. +#[tokio::test] +#[serial] +async fn yarn_berry_unwind_drops_a_descriptor_no_entry_declares() { + let server = MockServer::start().await; + mock_discovery(&server).await; + let hosted_url = HOSTED_URL.replace("http://patch.test", &server.uri()); + mock_reference_with_berry_url(&server, &hosted_url).await; + mock_view(&server).await; + mock_npm_registry_advertising( + &server, + &vlt_hosted_common::sha512_sri(&upstream_tarball()), + &format!( + "{}/npm-registry/{NAME}/-/{NAME}-{VERSION}.tgz", + server.uri() + ), + ) + .await; + // A workspace member that declares the exact version: its entry is + // yarn's record that the second descriptor is still in use. + let member = format!( + "\n\"member@workspace:packages/member\":\n version: 0.0.0-use.local\n \ + resolution: \"member@workspace:packages/member\"\n dependencies:\n \ + {NAME}: \"npm:{VERSION}\"\n languageName: unknown\n linkType: soft\n" + ); + for (declared, key) in [ + (false, format!("\"{NAME}@npm:^{VERSION}\":")), + ( + true, + format!("\"{NAME}@npm:{VERSION}, {NAME}@npm:^{VERSION}\":"), + ), + ] { + for command in ["rollback", "remove"] { + let tmp = tempfile::tempdir().unwrap(); + write_berry_project(tmp.path()); + let env = run_redirect_subprocess_with( + tmp.path(), + &server.uri(), + &["--patch-server-url", &server.uri()], + ); + assert_eq!(vlt_hosted_common::redirected(&env), 1, "{env:#}"); + // The selector a since-removed member's descriptor left behind. + let pkg_path = tmp.path().join("package.json"); + let mut pkg: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&pkg_path).unwrap()).unwrap(); + pkg["resolutions"][format!("{NAME}@npm:{VERSION}")] = + serde_json::Value::String(hosted_url.clone()); + std::fs::write(&pkg_path, serde_json::to_string_pretty(&pkg).unwrap()).unwrap(); + let lock_path = tmp.path().join("yarn.lock"); + if declared { + let pinned = std::fs::read_to_string(&lock_path).unwrap(); + std::fs::write(&lock_path, format!("{pinned}{member}")).unwrap(); + } + + let args: &[&str] = if command == "rollback" { + &["rollback"] + } else { + &["remove", PURL] + }; + let (code, env) = hosted_unwind_json(tmp.path(), &server, &server.uri(), args); + assert_eq!(code, Some(0), "{command} declared={declared}: {env:#}"); + let restored = std::fs::read_to_string(&lock_path).unwrap(); + assert!( + restored.contains(&format!("\n{key}\n")), + "{command} declared={declared}: restored key must be {key}:\n{restored}" + ); + assert!(!restored.contains(&hosted_url), "{restored}"); + let pkg = std::fs::read_to_string(&pkg_path).unwrap(); + assert!(!pkg.contains("resolutions"), "{pkg}"); + } + } +} + /// #908: the restore reads `dist.tarball` from the registry the project /// resolves against (`.yarnrc.yml` `npmRegistryServer`), not from the /// default registry. A mirror whose tarball URLs are off the conventional diff --git a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs index 1a58ff50c..38249d18a 100644 --- a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs +++ b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs @@ -146,7 +146,10 @@ pub(crate) fn prune_unreferenced( /// `dependencies:` map naming it, or any mention of it in another entry's /// text (a `patch:` locator embeds its base descriptor, `:` spelled /// `%3A`). The entry keyed by it does not count. -fn referenced(stanzas: &[String], descriptor: &str) -> bool { +/// Whether some entry of `stanzas` other than one keyed by `descriptor` +/// references it: names it in its `dependencies:` map, or wraps it (a +/// `patch:` descriptor spells it percent-encoded). +pub(crate) fn referenced(stanzas: &[String], descriptor: &str) -> bool { let encoded = descriptor.replace(':', "%3A"); stanzas.iter().any(|s| { let own = stanza_key(s) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 688aa9947..9bc156ed8 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -825,6 +825,19 @@ async fn restore_berry( }) .cloned() .collect(); + // Only descriptors some entry still declares key the restored + // entry: `yarn remove` in one workspace member leaves its + // selector behind while another member keeps the URL entry + // live, and yarn keys the entry by what is declared (#1435). + // The selectors are all dropped below either way. + let declared: Vec = descriptors + .iter() + .filter(|d| berry_descriptor_declared(&blocks, d)) + .cloned() + .collect(); + if !declared.is_empty() { + descriptors = declared; + } if descriptors.is_empty() { result.refuse( &uuid, @@ -1051,6 +1064,23 @@ async fn restore_berry( view.write(rel, doc.render(&moved)); } +/// Whether a berry lock's `stanzas` still declare the `name@npm:range` +/// `descriptor`: an entry's `dependencies:` map names it (yarn 4 spells +/// the range `npm:range`, yarn 2 and 3 a bare `range`) or a `patch:` +/// descriptor wraps it. +fn berry_descriptor_declared(stanzas: &[String], descriptor: &str) -> bool { + use crate::formats::yarn::berry_prune::{dependency_descriptors, referenced}; + use crate::formats::yarn::patterns::split_pattern; + let bare = split_pattern(descriptor) + .and_then(|(name, range)| Some(format!("{name}@{}", range.strip_prefix("npm:")?))); + referenced(stanzas, descriptor) + || bare.is_some_and(|bare| { + stanzas + .iter() + .any(|s| dependency_descriptors(s).contains(&bare)) + }) +} + /// Retire the leftover hosted `resolutions` selectors of a yarn berry root /// `package.json` (#1203): `yarn remove` / `yarn up` deleted the lock entry /// a hosted pin keyed by its tarball URL but left its selector, which now @@ -2723,6 +2753,39 @@ mod tests { }; use crate::patch::redirect::upstream::client::NpmDist; + /// #1435: what still declares a descriptor the hosted restore keys + /// the registry entry by. + #[test] + fn berry_descriptor_declared_reads_what_the_lock_still_uses() { + let declared = |stanzas: &[&str], d: &str| { + let stanzas: Vec = stanzas.iter().map(|s| s.to_string()).collect(); + super::berry_descriptor_declared(&stanzas, d) + }; + let url_entry = "\"left-pad@http://patch.test/left-pad-1.3.0.tgz\":\n version: 1.3.0\n \ + resolution: \"left-pad@http://patch.test/left-pad-1.3.0.tgz\""; + let member = |range: &str| { + format!( + "\"b@workspace:packages/b\":\n version: 0.0.0-use.local\n dependencies:\n \ + left-pad: \"{range}\"\n linkType: soft" + ) + }; + // Yarn 4 spells the range `npm:range`; yarn 2 and 3 a bare one. + let yarn4 = member("npm:^1.3.0"); + let yarn3 = member("^1.3.0"); + assert!(declared(&[url_entry, &yarn4], "left-pad@npm:^1.3.0")); + assert!(declared(&[url_entry, &yarn3], "left-pad@npm:^1.3.0")); + // The member a `yarn remove` emptied declares nothing. + assert!(!declared(&[url_entry, &yarn4], "left-pad@npm:1.3.0")); + assert!(!declared(&[url_entry, &yarn3], "left-pad@npm:1.3.0")); + // A `yarn patch` of the descriptor still uses it. + let patched = "\"left-pad@patch:left-pad@npm%3A1.3.0#~/.yarn/patches/x.patch\":\n \ + version: 1.3.0"; + assert!(declared(&[url_entry, patched], "left-pad@npm:1.3.0")); + // An alias of the same version declares another descriptor. + let alias = "\"lp@npm:left-pad@1.3.0\":\n version: 1.3.0"; + assert!(!declared(&[url_entry, alias], "left-pad@npm:1.3.0")); + } + fn files(npmrc: &[(&str, bool)], bunfig: &[(&str, bool)]) -> super::BunConfigFiles { let layer = |list: &[(&str, bool)]| { list.iter() From 3977e124f113d25a86b36841be3ef6eff13c11f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:44:39 +0000 Subject: [PATCH 3/3] Match declared berry descriptors exactly 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 --- .../tests/in_process_redirect.rs | 12 +- .../src/formats/yarn/berry_prune.rs | 5 +- .../src/patch/redirect/upstream/npm.rs | 114 +++++++++++++----- 3 files changed, 93 insertions(+), 38 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index 5cf2b5804..d4f7dfe3c 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -5461,12 +5461,22 @@ async fn yarn_berry_unwind_drops_a_descriptor_no_entry_declares() { serde_json::from_str(&std::fs::read_to_string(&pkg_path).unwrap()).unwrap(); pkg["resolutions"][format!("{NAME}@npm:{VERSION}")] = serde_json::Value::String(hosted_url.clone()); - std::fs::write(&pkg_path, serde_json::to_string_pretty(&pkg).unwrap()).unwrap(); let lock_path = tmp.path().join("yarn.lock"); if declared { + pkg["workspaces"] = serde_json::json!(["packages/*"]); + let dir = tmp.path().join("packages").join("member"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!( + r#"{{ "name": "member", "version": "0.0.0", "dependencies": {{ "{NAME}": "{VERSION}" }} }}"# + ), + ) + .unwrap(); let pinned = std::fs::read_to_string(&lock_path).unwrap(); std::fs::write(&lock_path, format!("{pinned}{member}")).unwrap(); } + std::fs::write(&pkg_path, serde_json::to_string_pretty(&pkg).unwrap()).unwrap(); let args: &[&str] = if command == "rollback" { &["rollback"] diff --git a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs index 38249d18a..1a58ff50c 100644 --- a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs +++ b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs @@ -146,10 +146,7 @@ pub(crate) fn prune_unreferenced( /// `dependencies:` map naming it, or any mention of it in another entry's /// text (a `patch:` locator embeds its base descriptor, `:` spelled /// `%3A`). The entry keyed by it does not count. -/// Whether some entry of `stanzas` other than one keyed by `descriptor` -/// references it: names it in its `dependencies:` map, or wraps it (a -/// `patch:` descriptor spells it percent-encoded). -pub(crate) fn referenced(stanzas: &[String], descriptor: &str) -> bool { +fn referenced(stanzas: &[String], descriptor: &str) -> bool { let encoded = descriptor.replace(':', "%3A"); stanzas.iter().any(|s| { let own = stanza_key(s) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 9bc156ed8..c4406a58c 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -757,6 +757,8 @@ async fn restore_berry( url_pin: bool, } let mut hits: Vec = Vec::new(); + // What the lock still declares, read once for the URL-keyed hits. + let mut declared: Option> = None; for (i, block) in blocks.iter().enumerate() { let lines = stanza_lines(block); let Some(resolution) = berry_field(&lines, "resolution").map(str::to_string) else { @@ -829,14 +831,15 @@ async fn restore_berry( // entry: `yarn remove` in one workspace member leaves its // selector behind while another member keeps the URL entry // live, and yarn keys the entry by what is declared (#1435). - // The selectors are all dropped below either way. - let declared: Vec = descriptors + // The selectors are all dropped below either way. A lock that + // declares none of them (a spelling not read here) keeps every + // selector's descriptor, as before. + let declared = declared.get_or_insert_with(|| berry_declared_descriptors(&blocks)); + if descriptors .iter() - .filter(|d| berry_descriptor_declared(&blocks, d)) - .cloned() - .collect(); - if !declared.is_empty() { - descriptors = declared; + .any(|d| berry_descriptor_declared(declared, d)) + { + descriptors.retain(|d| berry_descriptor_declared(declared, d)); } if descriptors.is_empty() { result.refuse( @@ -1064,21 +1067,50 @@ async fn restore_berry( view.write(rel, doc.render(&moved)); } -/// Whether a berry lock's `stanzas` still declare the `name@npm:range` -/// `descriptor`: an entry's `dependencies:` map names it (yarn 4 spells -/// the range `npm:range`, yarn 2 and 3 a bare `range`) or a `patch:` -/// descriptor wraps it. -fn berry_descriptor_declared(stanzas: &[String], descriptor: &str) -> bool { - use crate::formats::yarn::berry_prune::{dependency_descriptors, referenced}; +/// 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 +/// [`berry_descriptor_declared`]. +fn berry_declared_descriptors(stanzas: &[String]) -> BTreeSet { + use crate::formats::yarn::berry_prune::dependency_descriptors; + use crate::formats::yarn::patterns::split_berry_key_patterns; + use crate::formats::yarn::stanzas::stanza_key; + + let mut out = BTreeSet::new(); + for stanza in stanzas { + let keys = stanza_key(stanza) + .map(split_berry_key_patterns) + .unwrap_or_default(); + for descriptor in dependency_descriptors(stanza) { + if let Some(inner) = berry_patch_inner(&descriptor) { + out.insert(inner); + } + out.insert(descriptor); + } + out.extend(keys.iter().filter_map(|k| berry_patch_inner(k))); + } + out +} + +/// The descriptor a `name@patch:#` descriptor wraps, with +/// yarn's percent-encoding of the inner `:` undone. +fn berry_patch_inner(descriptor: &str) -> Option { + let (_, rest) = descriptor.split_once("@patch:")?; + let inner = rest.split('#').next().unwrap_or(rest); + Some(inner.replace("%3A", ":")) +} + +/// Whether the `name@npm:range` `descriptor` is in `declared` +/// ([`berry_declared_descriptors`]), in yarn 4's spelling or yarn 2/3's +/// bare `name@range`. +fn berry_descriptor_declared(declared: &BTreeSet, descriptor: &str) -> bool { use crate::formats::yarn::patterns::split_pattern; - let bare = split_pattern(descriptor) - .and_then(|(name, range)| Some(format!("{name}@{}", range.strip_prefix("npm:")?))); - referenced(stanzas, descriptor) - || bare.is_some_and(|bare| { - stanzas - .iter() - .any(|s| dependency_descriptors(s).contains(&bare)) - }) + declared.contains(descriptor) + || split_pattern(descriptor) + .and_then(|(name, range)| Some(format!("{name}@{}", range.strip_prefix("npm:")?))) + .is_some_and(|bare| declared.contains(&bare)) } /// Retire the leftover hosted `resolutions` selectors of a yarn berry root @@ -2759,31 +2791,47 @@ mod tests { fn berry_descriptor_declared_reads_what_the_lock_still_uses() { let declared = |stanzas: &[&str], d: &str| { let stanzas: Vec = stanzas.iter().map(|s| s.to_string()).collect(); - super::berry_descriptor_declared(&stanzas, d) + super::berry_descriptor_declared(&super::berry_declared_descriptors(&stanzas), d) }; let url_entry = "\"left-pad@http://patch.test/left-pad-1.3.0.tgz\":\n version: 1.3.0\n \ resolution: \"left-pad@http://patch.test/left-pad-1.3.0.tgz\""; - let member = |range: &str| { + let member = |dep: &str| { format!( "\"b@workspace:packages/b\":\n version: 0.0.0-use.local\n dependencies:\n \ - left-pad: \"{range}\"\n linkType: soft" + {dep}\n linkType: soft" ) }; // Yarn 4 spells the range `npm:range`; yarn 2 and 3 a bare one. - let yarn4 = member("npm:^1.3.0"); - let yarn3 = member("^1.3.0"); + let yarn4 = member("left-pad: \"npm:^1.3.0\""); + let yarn3 = member("left-pad: ^1.3.0"); assert!(declared(&[url_entry, &yarn4], "left-pad@npm:^1.3.0")); assert!(declared(&[url_entry, &yarn3], "left-pad@npm:^1.3.0")); // The member a `yarn remove` emptied declares nothing. assert!(!declared(&[url_entry, &yarn4], "left-pad@npm:1.3.0")); assert!(!declared(&[url_entry, &yarn3], "left-pad@npm:1.3.0")); - // A `yarn patch` of the descriptor still uses it. - let patched = "\"left-pad@patch:left-pad@npm%3A1.3.0#~/.yarn/patches/x.patch\":\n \ - version: 1.3.0"; - assert!(declared(&[url_entry, patched], "left-pad@npm:1.3.0")); - // An alias of the same version declares another descriptor. - let alias = "\"lp@npm:left-pad@1.3.0\":\n version: 1.3.0"; - assert!(!declared(&[url_entry, alias], "left-pad@npm:1.3.0")); + // A `yarn patch` of the descriptor still uses it, in a dependency + // map or a key. + let patch = "left-pad@patch:left-pad@npm%3A1.3.0#~/.yarn/patches/x.patch"; + let patched = format!("\"{patch}\":\n version: 1.3.0"); + assert!(declared(&[url_entry, &patched], "left-pad@npm:1.3.0")); + let patch_dep = member(&format!("left-pad: \"{}\"", &patch["left-pad@".len()..])); + assert!(declared(&[url_entry, &patch_dep], "left-pad@npm:1.3.0")); + // An alias, a scoped namesake and a longer range declare other + // descriptors. + for other in [ + member("lp: \"npm:left-pad@1.3.0\""), + member("\"@acme/left-pad\": \"npm:1.3.0\""), + member("left-pad: \"npm:1.3.0-rc.1\""), + "\"@acme/left-pad@patch:@acme/left-pad@npm%3A1.3.0#x.patch\":\n version: 1.3.0" + .to_string(), + "\"left-pad@patch:left-pad@npm%3A1.3.0-rc.1#x.patch\":\n version: 1.3.0-rc.1" + .to_string(), + ] { + assert!( + !declared(&[url_entry, &other], "left-pad@npm:1.3.0"), + "{other}" + ); + } } fn files(npmrc: &[(&str, bool)], bunfig: &[(&str, bool)]) -> super::BunConfigFiles {