Skip to content

Commit 59db6fc

Browse files
committed
Fold later yarn berry descriptors into the pin
A hosted yarn berry pin routes each descriptor that existed at scan time to the Socket tarball. A descriptor added later (a workspace member's exact range, an npm: alias) gets its own registry entry, so that copy installs unpatched. A re-scan used to refuse with a "run yarn install to dedupe" remedy that never works, since the pin keeps the entries apart. It now adds a selector for the new descriptor and drops its registry entry, which is the lock yarn writes once that selector exists. rollback and remove restored the hosted entry next to that registry entry with the same resolution. Yarn merges such entries, so every fresh yarn install --immutable failed with YN0028 while the unwind reported success. The restored entry is now merged into it. Fixes #1082 Assisted-by: Claude Code:claude-opus-5-5
1 parent 0a16c2d commit 59db6fc

3 files changed

Lines changed: 253 additions & 0 deletions

File tree

‎crates/socket-patch-cli/tests/in_process_redirect.rs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5325,6 +5325,89 @@ async fn yarn_berry_rollback_keeps_a_bare_locator_for_conventional_urls() {
53255325
assert!(!restored.contains("__archiveUrl"), "{restored}");
53265326
}
53275327

5328+
/// #1082: a descriptor added after pinning (a workspace member's plain
5329+
/// `NAME: VERSION`, or an `npm:` alias) gets its own registry entry beside
5330+
/// the hosted one. `rollback` and `remove` restore the hosted entry into
5331+
/// that entry, the way yarn merges two entries with one resolution, so a
5332+
/// fresh `yarn install --immutable` accepts the lock; two blocks sharing a
5333+
/// `resolution:` fail it with YN0028.
5334+
#[tokio::test]
5335+
#[serial]
5336+
async fn yarn_berry_unwind_merges_into_a_registry_entry_added_after_pinning() {
5337+
let server = MockServer::start().await;
5338+
mock_discovery(&server).await;
5339+
let hosted_url = HOSTED_URL.replace("http://patch.test", &server.uri());
5340+
mock_reference_with_berry_url(&server, &hosted_url).await;
5341+
mock_view(&server).await;
5342+
mock_npm_registry_advertising(
5343+
&server,
5344+
&vlt_hosted_common::sha512_sri(&upstream_tarball()),
5345+
&format!(
5346+
"{}/npm-registry/{NAME}/-/{NAME}-{VERSION}.tgz",
5347+
server.uri()
5348+
),
5349+
)
5350+
.await;
5351+
let checksum = socket_patch_core::vendor::test_support::service_fixture::berry_checksum(
5352+
&upstream_tarball(),
5353+
NAME,
5354+
)
5355+
.unwrap();
5356+
let block = |key: &str| {
5357+
format!(
5358+
"\n{key}:\n version: {VERSION}\n resolution: \"{NAME}@npm:{VERSION}\"\n \
5359+
checksum: {checksum}\n languageName: node\n linkType: hard\n"
5360+
)
5361+
};
5362+
let plain = format!("\"{NAME}@npm:{VERSION}\"");
5363+
let alias = format!("\"lp@npm:{NAME}@{VERSION}\"");
5364+
for (added, merged) in [
5365+
(
5366+
plain.clone(),
5367+
format!("\"{NAME}@npm:{VERSION}, {NAME}@npm:^{VERSION}\":"),
5368+
),
5369+
(
5370+
alias.clone(),
5371+
format!("\"{NAME}@npm:^{VERSION}, lp@npm:{NAME}@{VERSION}\":"),
5372+
),
5373+
] {
5374+
for command in ["rollback", "remove"] {
5375+
let tmp = tempfile::tempdir().unwrap();
5376+
write_berry_project(tmp.path());
5377+
let env = run_redirect_subprocess_with(
5378+
tmp.path(),
5379+
&server.uri(),
5380+
&["--patch-server-url", &server.uri()],
5381+
);
5382+
assert_eq!(vlt_hosted_common::redirected(&env), 1, "{env:#}");
5383+
let lock_path = tmp.path().join("yarn.lock");
5384+
let pinned = std::fs::read_to_string(&lock_path).unwrap();
5385+
std::fs::write(&lock_path, format!("{pinned}{}", block(&added))).unwrap();
5386+
5387+
let args: &[&str] = if command == "rollback" {
5388+
&["rollback"]
5389+
} else {
5390+
&["remove", PURL]
5391+
};
5392+
let (code, env) = hosted_unwind_json(tmp.path(), &server, &server.uri(), args);
5393+
assert_eq!(code, Some(0), "{command} {added}: {env:#}");
5394+
let restored = std::fs::read_to_string(&lock_path).unwrap();
5395+
assert_eq!(
5396+
restored
5397+
.matches(&format!("resolution: \"{NAME}@npm:{VERSION}\""))
5398+
.count(),
5399+
1,
5400+
"{command} {added}: one entry per resolution:\n{restored}"
5401+
);
5402+
assert_eq!(restored.matches(&merged).count(), 1, "{restored}");
5403+
assert!(!restored.contains(&format!("\n{added}:")), "{restored}");
5404+
assert!(!restored.contains(&hosted_url), "{restored}");
5405+
let pkg = std::fs::read_to_string(tmp.path().join("package.json")).unwrap();
5406+
assert!(!pkg.contains("resolutions"), "{pkg}");
5407+
}
5408+
}
5409+
}
5410+
53285411
/// #908: the restore reads `dist.tarball` from the registry the project
53295412
/// resolves against (`.yarnrc.yml` `npmRegistryServer`), not from the
53305413
/// default registry. A mirror whose tarball URLs are off the conventional

‎crates/socket-patch-core/src/patch/redirect/mod.rs‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4707,6 +4707,18 @@ fn rewrite_yarn_berry_with_manifests(
47074707
});
47084708
continue;
47094709
}
4710+
// #1082: a descriptor added after pinning, which no selector routes,
4711+
// gets its own registry entry beside the one an earlier hosted run
4712+
// keyed by its tarball URL; the pin is what keeps yarn from merging
4713+
// them. Folding it in (one more selector, its entry dropped) is what
4714+
// yarn writes once the selector exists.
4715+
let mut folded: Vec<(usize, Vec<String>)> = Vec::new();
4716+
if targets.len() > 1 && targets.iter().filter(|(_, r)| r.is_empty()).count() == 1 {
4717+
let (url_entry, registry): (Vec<_>, Vec<_>) =
4718+
targets.drain(..).partition(|(_, r)| r.is_empty());
4719+
targets = url_entry;
4720+
folded = registry;
4721+
}
47104722
if targets.len() > 1 {
47114723
result.warnings.push(RewriteWarning {
47124724
code: "redirect_yarn_berry_ambiguous_entry".into(),
@@ -4786,6 +4798,10 @@ fn rewrite_yarn_berry_with_manifests(
47864798
&fname,
47874799
&dep.version,
47884800
&key_ranges,
4801+
&folded
4802+
.iter()
4803+
.flat_map(|(_, ranges)| ranges.iter().cloned())
4804+
.collect::<Vec<_>>(),
47894805
current_url.as_deref(),
47904806
&dep.artifact_url,
47914807
) {
@@ -4821,6 +4837,9 @@ fn rewrite_yarn_berry_with_manifests(
48214837
)
48224838
.join("\n");
48234839
cut.extend(cut_descriptors(&block, &rewritten));
4840+
for (idx, _) in &folded {
4841+
cut.extend(cut_descriptors(&blocks[*idx], &rewritten));
4842+
}
48244843
for (selector, original) in pin.apply(manifest_obj, &dep.artifact_url) {
48254844
manifest_changed = true;
48264845
result.edits.push(FileEdit {
@@ -4853,6 +4872,20 @@ fn rewrite_yarn_berry_with_manifests(
48534872
moved_keys.push(new_key);
48544873
changed = true;
48554874
}
4875+
// Highest index first, so the lower ones stay valid.
4876+
folded.sort_by_key(|(idx, _)| std::cmp::Reverse(*idx));
4877+
for (idx, _) in folded {
4878+
let removed = blocks.remove(idx);
4879+
result.edits.push(FileEdit {
4880+
path: "yarn.lock".into(),
4881+
kind: "redirect_yarn_berry_entry_folded".into(),
4882+
action: "removed".into(),
4883+
key: stanza_key(&removed).map(|k| k.trim_matches('"').to_string()),
4884+
original: Some(Value::String(doc.on_disk(&removed).into_owned())),
4885+
new: None,
4886+
});
4887+
changed = true;
4888+
}
48564889
result
48574890
.confirmed_yarn_berry_uuids
48584891
.insert(dep.patch_uuid.clone());
@@ -4915,6 +4948,7 @@ fn rewrite_yarn_berry_with_manifests(
49154948
result.edits.retain(|e| {
49164949
e.kind != "redirect_yarn_berry_entry"
49174950
&& e.kind != "redirect_yarn_berry_entry_pruned"
4951+
&& e.kind != "redirect_yarn_berry_entry_folded"
49184952
&& e.kind != "redirect_yarn_berry_resolution"
49194953
});
49204954
for dep in &npm {
@@ -5081,6 +5115,7 @@ fn berry_resolutions_pin(
50815115
name: &str,
50825116
version: &str,
50835117
key_ranges: &[String],
5118+
folded_ranges: &[String],
50845119
current_url: Option<&str>,
50855120
artifact_url: &str,
50865121
) -> Result<BerryResolutionsPin, RewriteWarning> {
@@ -5137,6 +5172,13 @@ fn berry_resolutions_pin(
51375172
),
51385173
});
51395174
}
5175+
// The registry entries of descriptors added after pinning (#1082).
5176+
for range in folded_ranges {
5177+
let selector = format!("{name}@{range}");
5178+
if !selectors.contains(&selector) {
5179+
selectors.push(selector);
5180+
}
5181+
}
51405182
let ranges: Vec<&str> = selectors
51415183
.iter()
51425184
.filter_map(|selector| crate::formats::yarn::patterns::split_pattern(selector))
@@ -10520,6 +10562,83 @@ mod tests {
1052010562
);
1052110563
}
1052210564

10565+
/// #1082: a descriptor added after pinning (a workspace member's
10566+
/// `left-pad: 1.3.0`) gets its own registry entry beside the hosted
10567+
/// one, because no selector routes it. A re-run folds it into the pin:
10568+
/// one more selector, the registry entry gone, the URL entry kept. A
10569+
/// superseding uuid folds it the same way.
10570+
#[test]
10571+
fn yarn_berry_rerun_folds_a_descriptor_added_after_pinning() {
10572+
let checksum = format!("10c0/{}", "7".repeat(128));
10573+
let url = berry_hosted_url("left-pad", "left-pad", "1.3.0");
10574+
let ovr = berry_override("left-pad", "1.3.0", &url, &checksum);
10575+
let mut first = RewriteResult::default();
10576+
rewrite_yarn_berry(
10577+
&berry_files(berry_lock("10c0"), berry_manifest()),
10578+
std::slice::from_ref(&ovr),
10579+
&mut first,
10580+
);
10581+
let registry_block = format!(
10582+
"\n\"left-pad@npm:1.3.0\":\n version: 1.3.0\n resolution: \"left-pad@npm:1.3.0\"\n \
10583+
checksum: 10c0/{}\n languageName: node\n linkType: hard\n",
10584+
"3".repeat(128)
10585+
);
10586+
let drifted = berry_files(
10587+
format!("{}{registry_block}", first.files["yarn.lock"]),
10588+
first.files["package.json"].clone(),
10589+
);
10590+
let newer_url = url.replace(BERRY_UUID, "22222222-2222-4222-8222-222222222222");
10591+
for (dep_url, gone) in [(&url, None), (&newer_url, Some(BERRY_UUID))] {
10592+
let dep = berry_override("left-pad", "1.3.0", dep_url, &checksum);
10593+
let mut r = RewriteResult::default();
10594+
rewrite_yarn_berry(&drifted, std::slice::from_ref(&dep), &mut r);
10595+
assert!(
10596+
!r.warnings
10597+
.iter()
10598+
.any(|w| w.code == "redirect_yarn_berry_ambiguous_entry"),
10599+
"{:?}",
10600+
r.warnings
10601+
);
10602+
assert!(r
10603+
.confirmed_yarn_berry_uuids
10604+
.contains(dep.patch_uuid.as_str()));
10605+
let out = &r.files["yarn.lock"];
10606+
assert!(out.contains(&format!("\"left-pad@{dep_url}\":")), "{out}");
10607+
assert!(!out.contains("left-pad@npm:1.3.0"), "{out}");
10608+
assert_eq!(out.matches("version: 1.3.0").count(), 1, "{out}");
10609+
if let Some(old) = gone {
10610+
assert!(!out.contains(old), "{out}");
10611+
}
10612+
let manifest: Value = serde_json::from_str(&r.files["package.json"]).unwrap();
10613+
assert_eq!(
10614+
manifest["resolutions"],
10615+
json!({"left-pad@npm:^1.3.0": dep_url, "left-pad@npm:1.3.0": dep_url})
10616+
);
10617+
// The folded pin is complete: another run changes nothing.
10618+
let mut again = RewriteResult::default();
10619+
rewrite_yarn_berry(
10620+
&berry_files(out.clone(), r.files["package.json"].clone()),
10621+
std::slice::from_ref(&dep),
10622+
&mut again,
10623+
);
10624+
assert!(again.warnings.is_empty(), "{:?}", again.warnings);
10625+
assert!(again.files.is_empty(), "{:?}", again.files);
10626+
}
10627+
10628+
// Two registry entries and no pin of ours stay ambiguous.
10629+
let two = berry_files(
10630+
format!("{}{registry_block}", berry_lock("10c0")),
10631+
berry_manifest(),
10632+
);
10633+
let mut r = RewriteResult::default();
10634+
rewrite_yarn_berry(&two, std::slice::from_ref(&ovr), &mut r);
10635+
assert!(r.files.is_empty(), "{:?}", r.files);
10636+
assert!(r
10637+
.warnings
10638+
.iter()
10639+
.any(|w| w.code == "redirect_yarn_berry_ambiguous_entry"));
10640+
}
10641+
1052310642
/// The URL-keyed lock entry alone is half a pin: with its manifest
1052410643
/// `resolutions` routing removed, yarn installs nothing from it, so the
1052510644
/// rewriter owns the dep yet never confirms it — a URL in the lock must

‎crates/socket-patch-core/src/patch/redirect/upstream/npm.rs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,7 @@ async fn restore_berry(
886886
let dists = fetch_dists_on(&wanted, registry, ctx, result).await;
887887
let mut changed = false;
888888
let mut moved: Vec<String> = Vec::new();
889+
let mut restored: Vec<usize> = Vec::new();
889890
for Hit {
890891
idx,
891892
uuid,
@@ -1003,11 +1004,13 @@ async fn restore_berry(
10031004
}
10041005
}
10051006
result.handled.insert(uuid);
1007+
restored.push(idx);
10061008
changed = true;
10071009
}
10081010
if !changed {
10091011
return;
10101012
}
1013+
merge_same_resolution(&mut blocks, &restored, &mut moved);
10111014
if pkg_changed {
10121015
if let (Some(text), Some(value)) = (pkg_text.as_deref(), pkg.as_mut()) {
10131016
if let Some(obj) = value.as_object_mut() {
@@ -1037,6 +1040,54 @@ async fn restore_berry(
10371040
view.write(rel, doc.render(&moved));
10381041
}
10391042

1043+
/// Fold each restored berry entry into another entry of `blocks` with the
1044+
/// same `resolution:` (#1082). A descriptor added after the hosted pin (a
1045+
/// workspace member's range, an `npm:` alias) got its own registry entry,
1046+
/// because no selector routed it to the tarball; once the pin is gone,
1047+
/// yarn writes the two as one entry keyed by every descriptor, sorted,
1048+
/// and `--immutable` rejects a lock that keeps them apart (YN0028). The
1049+
/// entry yarn wrote keeps its body; the restored one only adds its keys.
1050+
fn merge_same_resolution(blocks: &mut Vec<String>, restored: &[usize], moved: &mut Vec<String>) {
1051+
use crate::formats::yarn::blocks::berry_field;
1052+
use crate::formats::yarn::patterns::split_berry_key_patterns;
1053+
use crate::formats::yarn::stanzas::{stanza_key, stanza_lines};
1054+
1055+
let resolution_of =
1056+
|block: &str| berry_field(&stanza_lines(block), "resolution").map(str::to_string);
1057+
let mut dropped: Vec<usize> = Vec::new();
1058+
for &idx in restored {
1059+
let Some(resolution) = resolution_of(&blocks[idx]) else {
1060+
continue;
1061+
};
1062+
let Some(sibling) = (0..blocks.len()).find(|&j| {
1063+
j != idx
1064+
&& !dropped.contains(&j)
1065+
&& stanza_key(&blocks[j]).is_some_and(|k| k != "__metadata")
1066+
&& resolution_of(&blocks[j]).as_deref() == Some(resolution.as_str())
1067+
}) else {
1068+
continue;
1069+
};
1070+
let mut patterns: Vec<String> = [&blocks[sibling], &blocks[idx]]
1071+
.iter()
1072+
.flat_map(|b| split_berry_key_patterns(stanza_key(b).unwrap_or("")))
1073+
.collect();
1074+
patterns.sort();
1075+
patterns.dedup();
1076+
let old_key = stanza_key(&blocks[idx]).unwrap_or("").to_string();
1077+
moved.retain(|k| *k != old_key);
1078+
let key = format!("\"{}\"", patterns.join(", "));
1079+
let mut lines = stanza_lines(&blocks[sibling]);
1080+
lines[0] = format!("{key}:");
1081+
blocks[sibling] = lines.join("\n");
1082+
moved.push(key);
1083+
dropped.push(idx);
1084+
}
1085+
dropped.sort_unstable_by(|a, b| b.cmp(a));
1086+
for idx in dropped {
1087+
blocks.remove(idx);
1088+
}
1089+
}
1090+
10401091
/// Retire the leftover hosted `resolutions` selectors of a yarn berry root
10411092
/// `package.json` (#1203): `yarn remove` / `yarn up` deleted the lock entry
10421093
/// a hosted pin keyed by its tarball URL but left its selector, which now

0 commit comments

Comments
 (0)