diff --git a/crates/socket-patch-cli/tests/e2e_yarn4_workspaces_build.rs b/crates/socket-patch-cli/tests/e2e_yarn4_workspaces_build.rs index 19a0f1f21..d4eb34e0e 100644 --- a/crates/socket-patch-cli/tests/e2e_yarn4_workspaces_build.rs +++ b/crates/socket-patch-cli/tests/e2e_yarn4_workspaces_build.rs @@ -358,6 +358,15 @@ fn fresh_checkout_install(tmp: &Path, proj: &Path, yarnrc: &str) -> (PathBuf, Ou fresh.join("packages/app/package.json"), ) .unwrap(); + // The member a test adds after pinning (#1082). + if proj.join("packages/b").is_dir() { + std::fs::create_dir_all(fresh.join("packages/b")).unwrap(); + std::fs::copy( + proj.join("packages/b/package.json"), + fresh.join("packages/b/package.json"), + ) + .unwrap(); + } std::fs::copy(proj.join("yarn.lock"), fresh.join("yarn.lock")).unwrap(); std::fs::write(fresh.join(".yarnrc.yml"), yarnrc).unwrap(); // v5 hosted mode may leave no `.socket/` at all (no ledger, no manifest). @@ -374,42 +383,15 @@ fn fresh_checkout_install(tmp: &Path, proj: &Path, yarnrc: &str) -> (PathBuf, Ou (fresh, ci) } -// ── hosted capstone ─────────────────────────────────────────────────── - -#[tokio::test(flavor = "multi_thread")] -async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { - if !has_corepack_pm(yarn_berry()) { - skip!( - "SKIP e2e_yarn4_workspaces_build (hosted): `corepack {}` unavailable", - yarn_berry() - ); - return; - } - if !has_command("tar") { - skip!("SKIP e2e_yarn4_workspaces_build (hosted): `tar` not installed"); - return; - } - - let tmp = tempfile::tempdir().unwrap(); - let proj = tmp.path().join("proj"); - std::fs::create_dir_all(&proj).unwrap(); - let Some(orig) = install_workspace_fixture("hosted", tmp.path(), &proj) else { - return; - }; - let patched: Vec = [MARKER.as_bytes(), orig.as_slice()].concat(); - let registry_lock = std::fs::read(proj.join("yarn.lock")).unwrap(); - - // Patched tarball + the exact `10c0` checksum yarn computes for it. - let tgz_path = tmp.path().join(format!("{DEP}-{DEP_VERSION}.tgz")); - build_patched_tgz(&proj.join("node_modules").join(DEP), &patched, &tgz_path); - let tgz = std::fs::read(&tgz_path).unwrap(); - let Some(checksum) = bootstrap_berry_checksum(tmp.path(), &tgz_path) else { - return; - }; - - // API mocks + the hosted tarball route yarn will hit at install time. - let server = MockServer::start().await; - let host = server.uri().replace("http://", "").replace("https://", ""); +/// The patch API mocks and the hosted tarball route yarn hits at install +/// time; returns the hosted tarball URL. +async fn mount_hosted_api( + server: &MockServer, + checksum: &str, + orig: &[u8], + patched: &[u8], + tgz: &[u8], +) -> String { let hosted_url = format!( "{}/patch/npm/{DEP}/{DEP_VERSION}/{TOKEN}/{UUID}/{DEP}-{DEP_VERSION}.tgz", server.uri() @@ -427,7 +409,7 @@ async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { }], "canAccessPaidPatches": false, }))) - .mount(&server) + .mount(server) .await; Mock::given(method("GET")) .and(path_regex(format!( @@ -442,7 +424,7 @@ async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { }], "canAccessPaidPatches": false, }))) - .mount(&server) + .mount(server) .await; Mock::given(method("POST")) .and(path(format!("/v0/orgs/{ORG}/patches/package"))) @@ -462,7 +444,7 @@ async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { } } }))) - .mount(&server) + .mount(server) .await; Mock::given(method("GET")) .and(path(format!("/v0/orgs/{ORG}/patches/view/{UUID}"))) @@ -472,8 +454,8 @@ async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { "publishedAt": "2026-01-01T00:00:00Z", "files": { "package/index.js": { - "beforeHash": compute_git_sha256_from_bytes(&orig), - "afterHash": compute_git_sha256_from_bytes(&patched), + "beforeHash": compute_git_sha256_from_bytes(orig), + "afterHash": compute_git_sha256_from_bytes(patched), } }, "vulnerabilities": { @@ -484,18 +466,59 @@ async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { }, "description": "x", "license": "MIT", "tier": "free" }))) - .mount(&server) + .mount(server) .await; Mock::given(method("GET")) .and(path(format!( "/patch/npm/{DEP}/{DEP_VERSION}/{TOKEN}/{UUID}/{DEP}-{DEP_VERSION}.tgz" ))) .respond_with( - ResponseTemplate::new(200).set_body_raw(tgz.clone(), "application/octet-stream"), + ResponseTemplate::new(200).set_body_raw(tgz.to_vec(), "application/octet-stream"), ) - .mount(&server) + .mount(server) .await; + hosted_url +} + +// ── hosted capstone ─────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread")] +async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { + if !has_corepack_pm(yarn_berry()) { + skip!( + "SKIP e2e_yarn4_workspaces_build (hosted): `corepack {}` unavailable", + yarn_berry() + ); + return; + } + if !has_command("tar") { + skip!("SKIP e2e_yarn4_workspaces_build (hosted): `tar` not installed"); + return; + } + + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let Some(orig) = install_workspace_fixture("hosted", tmp.path(), &proj) else { + return; + }; + let patched: Vec = [MARKER.as_bytes(), orig.as_slice()].concat(); + let registry_lock = std::fs::read(proj.join("yarn.lock")).unwrap(); + + // Patched tarball + the exact `10c0` checksum yarn computes for it. + let tgz_path = tmp.path().join(format!("{DEP}-{DEP_VERSION}.tgz")); + build_patched_tgz(&proj.join("node_modules").join(DEP), &patched, &tgz_path); + let tgz = std::fs::read(&tgz_path).unwrap(); + let Some(checksum) = bootstrap_berry_checksum(tmp.path(), &tgz_path) else { + return; + }; + + // API mocks + the hosted tarball route yarn will hit at install time. + let server = MockServer::start().await; + let host = server.uri().replace("http://", "").replace("https://", ""); + let hosted_url = mount_hosted_api(&server, &checksum, &orig, &patched, &tgz).await; + let root_pkg_before = std::fs::read(proj.join("package.json")).unwrap(); let member_pkg_before = std::fs::read(proj.join("packages/app/package.json")).unwrap(); @@ -650,6 +673,229 @@ async fn yarn4_workspaces_hosted_redirect_rewires_member_dep_from_root_scan() { yarn_berry_common::off_runtime(|| yarn_berry_common::run_manifestless_vex_matrix(&flow)); } +/// #1082 with a real yarn: a member added after the hosted pin depends on +/// `left-pad@^1.3.0`, which no selector routes, so yarn locks a separate +/// registry entry and installs that copy unpatched. A re-scan folds it +/// into the pin (a fresh `--immutable` install then gets the patched bytes +/// only), and `rollback` leaves the one merged registry entry yarn writes, +/// so a fresh `--immutable` install of the restored lock passes (two +/// entries sharing a resolution fail it with YN0028). +#[tokio::test(flavor = "multi_thread")] +async fn yarn4_hosted_pin_folds_a_member_added_later_and_rolls_back() { + if !has_corepack_pm(yarn_berry()) { + skip!( + "SKIP e2e_yarn4_workspaces_build (hosted, #1082): `corepack {}` unavailable", + yarn_berry() + ); + return; + } + if !has_command("tar") { + skip!("SKIP e2e_yarn4_workspaces_build (hosted, #1082): `tar` not installed"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let Some(orig) = install_workspace_fixture("hosted-1082", tmp.path(), &proj) else { + return; + }; + let patched: Vec = [MARKER.as_bytes(), orig.as_slice()].concat(); + let registry_lock = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + let upstream_checksum = yarn_berry_common::yarn_written_checksum(®istry_lock) + .expect("the registry lock carries left-pad's checksum"); + let global = tmp.path().join("yarn-global"); + let global_env = [("YARN_GLOBAL_FOLDER", global.to_str().unwrap())]; + let info = corepack( + &proj, + yarn_berry(), + &[ + "npm", + "info", + &format!("{DEP}@{DEP_VERSION}"), + "--fields", + "dist", + "--json", + ], + &global_env, + ); + if !info.status.success() { + skip!( + "SKIP e2e_yarn4_workspaces_build (hosted, #1082): `yarn npm info` failed:\n{}", + yarn_berry_common::yarn_output(&info) + ); + return; + } + let info: serde_json::Value = serde_json::from_slice(&info.stdout).unwrap(); + let integrity = info["dist"]["integrity"].as_str().unwrap().to_string(); + + let tgz_path = tmp.path().join(format!("{DEP}-{DEP_VERSION}.tgz")); + build_patched_tgz(&proj.join("node_modules").join(DEP), &patched, &tgz_path); + let tgz = std::fs::read(&tgz_path).unwrap(); + let Some(checksum) = bootstrap_berry_checksum(tmp.path(), &tgz_path) else { + return; + }; + let server = MockServer::start().await; + let host = server.uri().replace("http://", "").replace("https://", ""); + let host = host.split(':').next().unwrap_or("127.0.0.1").to_string(); + let hosted_url = mount_hosted_api(&server, &checksum, &orig, &patched, &tgz).await; + // What `rollback` restores the registry entry's checksum from. + Mock::given(method("GET")) + .and(path(format!("/upstream/npm/{UUID}.json"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": DEP, "version": DEP_VERSION, + "integrity": integrity, "yarnBerry10c0": upstream_checksum, + }))) + .mount(&server) + .await; + let scan = |label: &str| { + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "scan", + "--mode", + "hosted", + "--json", + "--yes", + "--cwd", + proj.to_str().unwrap(), + "--api-url", + &server.uri(), + "--org", + ORG, + "--api-token", + "fake", + ], + ); + assert_eq!(code, 0, "{label}: stdout:\n{stdout}\nstderr:\n{stderr}"); + let env: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!(env["summary"]["applied"], 1, "{label}: {env}"); + env + }; + scan("first scan"); + + // A second member, added after pinning, with a range no selector routes. + std::fs::create_dir_all(proj.join("packages/b")).unwrap(); + std::fs::write( + proj.join("packages/b/package.json"), + format!(r#"{{"name":"b","version":"1.0.0","dependencies":{{"{DEP}":"^{DEP_VERSION}"}}}}"#), + ) + .unwrap(); + let mut root: serde_json::Value = + serde_json::from_slice(&std::fs::read(proj.join("package.json")).unwrap()).unwrap(); + root["workspaces"] = serde_json::json!(["packages/app", "packages/b"]); + std::fs::write( + proj.join("package.json"), + serde_json::to_string_pretty(&root).unwrap() + "\n", + ) + .unwrap(); + std::fs::write( + proj.join(".yarnrc.yml"), + format!( + "nodeLinker: node-modules\nenableGlobalCache: false\n\ + unsafeHttpWhitelist:\n - \"{host}\"\n" + ), + ) + .unwrap(); + let install = corepack(&proj, yarn_berry(), &["install"], &global_env); + assert!( + install.status.success(), + "{}", + yarn_berry_common::yarn_output(&install) + ); + let drifted = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + assert!( + drifted.contains(&format!("\"{DEP}@npm:^{DEP_VERSION}\":")), + "yarn locks the new range as its own registry entry:\n{drifted}" + ); + + let rollback = || { + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "rollback", + "--json", + "--yes", + "--cwd", + proj.to_str().unwrap(), + "--patch-server-url", + &server.uri(), + ], + ); + assert_eq!(code, 0, "rollback: stdout:\n{stdout}\nstderr:\n{stderr}"); + let restored = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + assert!( + restored.contains(&format!( + "\"{DEP}@npm:{DEP_VERSION}, {DEP}@npm:^{DEP_VERSION}\":" + )), + "{restored}" + ); + assert!(!restored.contains(&hosted_url), "{restored}"); + let (fresh, ci) = fresh_checkout_install( + tmp.path(), + &proj, + "nodeLinker: node-modules\nenableGlobalCache: false\n", + ); + assert!( + ci.status.success(), + "a fresh `--immutable` install of the rolled-back lock (YN0028 = two \ + entries with one resolution):\n{}", + yarn_berry_common::yarn_output(&ci) + ); + assert_eq!( + std::fs::read(fresh.join("node_modules").join(DEP).join("index.js")).unwrap(), + orig + ); + std::fs::remove_dir_all(&fresh).unwrap(); + }; + + // Rolled back as it stands: the restored entry merges into the new one. + let drifted_pkg = std::fs::read(proj.join("package.json")).unwrap(); + rollback(); + eprintln!("#1082 ROLLBACK OF THE DRIFTED PIN OK"); + std::fs::write(proj.join("package.json"), drifted_pkg).unwrap(); + std::fs::write(proj.join("yarn.lock"), &drifted).unwrap(); + + // The re-scan folds it into the pin. + scan("re-scan"); + let folded = std::fs::read_to_string(proj.join("yarn.lock")).unwrap(); + assert!(!folded.contains(&format!("{DEP}@npm:")), "{folded}"); + assert_eq!( + folded.matches(&format!("resolution: \"{DEP}@")).count(), + 1, + "{folded}" + ); + let root: serde_json::Value = + serde_json::from_slice(&std::fs::read(proj.join("package.json")).unwrap()).unwrap(); + assert_eq!( + root["resolutions"], + serde_json::json!({ + format!("{DEP}@npm:{DEP_VERSION}"): hosted_url, + format!("{DEP}@npm:^{DEP_VERSION}"): hosted_url, + }) + ); + let offline = format!( + "nodeLinker: node-modules\nenableGlobalCache: false\n\ + unsafeHttpWhitelist:\n - \"{host}\"\n\ + npmRegistryServer: \"http://127.0.0.1:1\"\n" + ); + let (fresh, ci) = fresh_checkout_install(tmp.path(), &proj, &offline); + assert!( + ci.status.success(), + "a fresh `--immutable` install of the folded pin:\n{}", + yarn_berry_common::yarn_output(&ci) + ); + assert_eq!( + std::fs::read(fresh.join("node_modules").join(DEP).join("index.js")).unwrap(), + patched + ); + assert!(!fresh.join("packages/b/node_modules").join(DEP).exists()); + std::fs::remove_dir_all(&fresh).unwrap(); + eprintln!("#1082 RE-SCAN FOLD OK"); + + rollback(); + eprintln!("#1082 ROLLBACK OK"); +} + // ── vendored capstone ───────────────────────────────────────────────── #[test] diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index 05a048bd1..3b5b178f8 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -5325,6 +5325,89 @@ async fn yarn_berry_rollback_keeps_a_bare_locator_for_conventional_urls() { assert!(!restored.contains("__archiveUrl"), "{restored}"); } +/// #1082: a descriptor added after pinning (a workspace member's plain +/// `NAME: VERSION`, or an `npm:` alias) gets its own registry entry beside +/// the hosted one. `rollback` and `remove` restore the hosted entry into +/// that entry, the way yarn merges two entries with one resolution, so a +/// fresh `yarn install --immutable` accepts the lock; two blocks sharing a +/// `resolution:` fail it with YN0028. +#[tokio::test] +#[serial] +async fn yarn_berry_unwind_merges_into_a_registry_entry_added_after_pinning() { + 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; + let checksum = socket_patch_core::vendor::test_support::service_fixture::berry_checksum( + &upstream_tarball(), + NAME, + ) + .unwrap(); + let block = |key: &str| { + format!( + "\n{key}:\n version: {VERSION}\n resolution: \"{NAME}@npm:{VERSION}\"\n \ + checksum: {checksum}\n languageName: node\n linkType: hard\n" + ) + }; + let plain = format!("\"{NAME}@npm:{VERSION}\""); + let alias = format!("\"lp@npm:{NAME}@{VERSION}\""); + for (added, merged) in [ + ( + plain.clone(), + format!("\"{NAME}@npm:{VERSION}, {NAME}@npm:^{VERSION}\":"), + ), + ( + alias.clone(), + format!("\"{NAME}@npm:^{VERSION}, lp@npm:{NAME}@{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:#}"); + let lock_path = tmp.path().join("yarn.lock"); + let pinned = std::fs::read_to_string(&lock_path).unwrap(); + std::fs::write(&lock_path, format!("{pinned}{}", block(&added))).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} {added}: {env:#}"); + let restored = std::fs::read_to_string(&lock_path).unwrap(); + assert_eq!( + restored + .matches(&format!("resolution: \"{NAME}@npm:{VERSION}\"")) + .count(), + 1, + "{command} {added}: one entry per resolution:\n{restored}" + ); + assert_eq!(restored.matches(&merged).count(), 1, "{restored}"); + assert!(!restored.contains(&format!("\n{added}:")), "{restored}"); + assert!(!restored.contains(&hosted_url), "{restored}"); + let pkg = std::fs::read_to_string(tmp.path().join("package.json")).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/stanzas.rs b/crates/socket-patch-core/src/formats/yarn/stanzas.rs index 566c6bacb..519361ba6 100644 --- a/crates/socket-patch-core/src/formats/yarn/stanzas.rs +++ b/crates/socket-patch-core/src/formats/yarn/stanzas.rs @@ -82,6 +82,64 @@ pub(crate) fn stanza_lines(stanza: &str) -> Vec { stanza.lines().map(str::to_string).collect() } +/// Fold each `restored` stanza into another stanza with the same +/// `resolution:` (#1082), and say whether any was. A descriptor added +/// while a pin or vendored wiring routed the package elsewhere (a +/// workspace member's range, an `npm:` alias) got its own registry entry; +/// once the original entry is back, yarn writes the two as one entry +/// keyed by every descriptor, sorted, and `--immutable` rejects a lock +/// that keeps them apart (YN0028). The entry yarn wrote keeps its body; +/// the restored one only adds its keys. The merged key goes to `moved`, +/// for [`BerryStanzas::render`] to sort. Returns the indices (before the +/// call) of the restored stanzas merged away. +pub(crate) fn merge_same_resolution( + stanzas: &mut Vec, + restored: &[usize], + moved: &mut Vec, +) -> Vec { + use super::blocks::berry_field; + use super::patterns::split_berry_key_patterns; + + let resolution_of = + |stanza: &str| berry_field(&stanza_lines(stanza), "resolution").map(str::to_string); + let mut dropped: Vec = Vec::new(); + for &idx in restored { + let Some(resolution) = resolution_of(&stanzas[idx]) else { + continue; + }; + let Some(sibling) = (0..stanzas.len()).find(|&j| { + j != idx + && !dropped.contains(&j) + && sort_key(&stanzas[j]).is_some() + && resolution_of(&stanzas[j]).as_deref() == Some(resolution.as_str()) + }) else { + continue; + }; + let mut patterns: Vec = [&stanzas[sibling], &stanzas[idx]] + .iter() + .flat_map(|s| split_berry_key_patterns(stanza_key(s).unwrap_or(""))) + .collect(); + patterns.sort(); + patterns.dedup(); + for old in [idx, sibling] { + let old_key = stanza_key(&stanzas[old]).unwrap_or("").to_string(); + moved.retain(|k| *k != old_key); + } + let key = format!("\"{}\"", patterns.join(", ")); + let mut lines = stanza_lines(&stanzas[sibling]); + lines[0] = format!("{key}:"); + stanzas[sibling] = lines.join("\n"); + moved.push(key); + dropped.push(idx); + } + let mut descending = dropped.clone(); + descending.sort_unstable_by(|a, b| b.cmp(a)); + for idx in descending { + stanzas.remove(idx); + } + dropped +} + /// A stanza's sort key: its unquoted key, or `None` for header comment /// runs and `__metadata`. fn sort_key(stanza: &str) -> Option<&str> { @@ -145,6 +203,28 @@ mod tests { } } + /// #1082: a restored entry joins the entry yarn wrote for the same + /// resolution (sorted union of keys, yarn's body); one with no such + /// sibling, and an entry of another resolution, are left alone. + #[test] + fn restored_entries_merge_into_a_sibling_with_their_resolution() { + let raw = "__metadata:\n version: 8\n\n\ + \"a@npm:^1.0.0\":\n version: 1.0.0\n resolution: \"a@npm:1.0.0\"\n restored: x\n\n\ + \"b@npm:^2\":\n version: 2.0.0\n resolution: \"b@npm:2.0.0\"\n\n\ + \"c@npm:a@1.0.0\":\n version: 1.0.0\n resolution: \"a@npm:1.0.0\"\n yarn: y\n"; + let mut doc = BerryStanzas::parse(raw); + let mut moved = vec!["\"a@npm:^1.0.0\"".to_string()]; + let merged = merge_same_resolution(&mut doc.stanzas, &[1, 2], &mut moved); + assert_eq!(merged, vec![1]); + assert_eq!(moved, vec!["\"a@npm:^1.0.0, c@npm:a@1.0.0\"".to_string()]); + assert_eq!( + doc.render(&moved), + "__metadata:\n version: 8\n\n\ + \"a@npm:^1.0.0, c@npm:a@1.0.0\":\n version: 1.0.0\n resolution: \"a@npm:1.0.0\"\n yarn: y\n\n\ + \"b@npm:^2\":\n version: 2.0.0\n resolution: \"b@npm:2.0.0\"\n" + ); + } + #[test] fn a_moved_entry_goes_to_its_sorted_place_only_in_a_sorted_lock() { let raw = "__metadata:\n version: 8\n\n\"a@npm:^1\":\n v: 1\n\n\"b@npm:^1\":\n v: 1\n"; diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index dcdcbe249..b74b4ad87 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -4707,6 +4707,18 @@ fn rewrite_yarn_berry_with_manifests( }); continue; } + // #1082: a descriptor added after pinning, which no selector routes, + // gets its own registry entry beside the one an earlier hosted run + // keyed by its tarball URL; the pin is what keeps yarn from merging + // them. Folding it in (one more selector, its entry dropped) is what + // yarn writes once the selector exists. + let mut folded: Vec<(usize, Vec)> = Vec::new(); + if targets.len() > 1 && targets.iter().filter(|(_, r)| r.is_empty()).count() == 1 { + let (url_entry, registry): (Vec<_>, Vec<_>) = + targets.drain(..).partition(|(_, r)| r.is_empty()); + targets = url_entry; + folded = registry; + } if targets.len() > 1 { result.warnings.push(RewriteWarning { code: "redirect_yarn_berry_ambiguous_entry".into(), @@ -4789,7 +4801,16 @@ fn rewrite_yarn_berry_with_manifests( current_url.as_deref(), &dep.artifact_url, ) { - Ok(pin) => pin, + Ok(mut pin) => { + let folded_ranges: Vec<&str> = folded + .iter() + .flat_map(|(_, ranges)| ranges.iter().map(String::as_str)) + .collect(); + if !folded_ranges.is_empty() { + pin.add_ranges(yarnrc, &fname, &folded_ranges); + } + pin + } Err(warning) => { result.warnings.push(warning); continue; @@ -4821,6 +4842,9 @@ fn rewrite_yarn_berry_with_manifests( ) .join("\n"); cut.extend(cut_descriptors(&block, &rewritten)); + for (idx, _) in &folded { + cut.extend(cut_descriptors(&blocks[*idx], &rewritten)); + } for (selector, original) in pin.apply(manifest_obj, &dep.artifact_url) { manifest_changed = true; result.edits.push(FileEdit { @@ -4853,6 +4877,20 @@ fn rewrite_yarn_berry_with_manifests( moved_keys.push(new_key); changed = true; } + // Highest index first, so the lower ones stay valid. + folded.sort_by_key(|(idx, _)| std::cmp::Reverse(*idx)); + for (idx, _) in folded { + let removed = blocks.remove(idx); + result.edits.push(FileEdit { + path: "yarn.lock".into(), + kind: "redirect_yarn_berry_entry_folded".into(), + action: "removed".into(), + key: stanza_key(&removed).map(|k| k.trim_matches('"').to_string()), + original: Some(Value::String(doc.on_disk(&removed).into_owned())), + new: None, + }); + changed = true; + } result .confirmed_yarn_berry_uuids .insert(dep.patch_uuid.clone()); @@ -4915,6 +4953,7 @@ fn rewrite_yarn_berry_with_manifests( result.edits.retain(|e| { e.kind != "redirect_yarn_berry_entry" && e.kind != "redirect_yarn_berry_entry_pruned" + && e.kind != "redirect_yarn_berry_entry_folded" && e.kind != "redirect_yarn_berry_resolution" }); for dep in &npm { @@ -5018,6 +5057,20 @@ struct BerryResolutionsPin { } impl BerryResolutionsPin { + /// Route `ranges` too: the registry entries of descriptors added after + /// pinning, which the pin folds in (#1082), with their catalog + /// selectors. + fn add_ranges(&mut self, yarnrc: Option<&str>, name: &str, ranges: &[&str]) { + let mut added: Vec = ranges.iter().map(|r| format!("{name}@{r}")).collect(); + added.extend(berry_catalog_selectors(yarnrc, name, ranges)); + for selector in added { + self.stale.retain(|s| *s != selector); + if !self.selectors.contains(&selector) { + self.selectors.push(selector); + } + } + } + /// Write the pin into `manifest`: every selector routed to `url`, stale /// ones dropped, and an emptied `resolutions` table removed. Returns /// `(selector, previous value)` for each selector it added or changed. @@ -10520,6 +10573,83 @@ mod tests { ); } + /// #1082: a descriptor added after pinning (a workspace member's + /// `left-pad: 1.3.0`) gets its own registry entry beside the hosted + /// one, because no selector routes it. A re-run folds it into the pin: + /// one more selector, the registry entry gone, the URL entry kept. A + /// superseding uuid folds it the same way. + #[test] + fn yarn_berry_rerun_folds_a_descriptor_added_after_pinning() { + let checksum = format!("10c0/{}", "7".repeat(128)); + let url = berry_hosted_url("left-pad", "left-pad", "1.3.0"); + let ovr = berry_override("left-pad", "1.3.0", &url, &checksum); + let mut first = RewriteResult::default(); + rewrite_yarn_berry( + &berry_files(berry_lock("10c0"), berry_manifest()), + std::slice::from_ref(&ovr), + &mut first, + ); + let registry_block = format!( + "\n\"left-pad@npm:1.3.0\":\n version: 1.3.0\n resolution: \"left-pad@npm:1.3.0\"\n \ + checksum: 10c0/{}\n languageName: node\n linkType: hard\n", + "3".repeat(128) + ); + let drifted = berry_files( + format!("{}{registry_block}", first.files["yarn.lock"]), + first.files["package.json"].clone(), + ); + let newer_url = url.replace(BERRY_UUID, "22222222-2222-4222-8222-222222222222"); + for (dep_url, gone) in [(&url, None), (&newer_url, Some(BERRY_UUID))] { + let dep = berry_override("left-pad", "1.3.0", dep_url, &checksum); + let mut r = RewriteResult::default(); + rewrite_yarn_berry(&drifted, std::slice::from_ref(&dep), &mut r); + assert!( + !r.warnings + .iter() + .any(|w| w.code == "redirect_yarn_berry_ambiguous_entry"), + "{:?}", + r.warnings + ); + assert!(r + .confirmed_yarn_berry_uuids + .contains(dep.patch_uuid.as_str())); + let out = &r.files["yarn.lock"]; + assert!(out.contains(&format!("\"left-pad@{dep_url}\":")), "{out}"); + assert!(!out.contains("left-pad@npm:1.3.0"), "{out}"); + assert_eq!(out.matches("version: 1.3.0").count(), 1, "{out}"); + if let Some(old) = gone { + assert!(!out.contains(old), "{out}"); + } + let manifest: Value = serde_json::from_str(&r.files["package.json"]).unwrap(); + assert_eq!( + manifest["resolutions"], + json!({"left-pad@npm:^1.3.0": dep_url, "left-pad@npm:1.3.0": dep_url}) + ); + // The folded pin is complete: another run changes nothing. + let mut again = RewriteResult::default(); + rewrite_yarn_berry( + &berry_files(out.clone(), r.files["package.json"].clone()), + std::slice::from_ref(&dep), + &mut again, + ); + assert!(again.warnings.is_empty(), "{:?}", again.warnings); + assert!(again.files.is_empty(), "{:?}", again.files); + } + + // Two registry entries and no pin of ours stay ambiguous. + let two = berry_files( + format!("{}{registry_block}", berry_lock("10c0")), + berry_manifest(), + ); + let mut r = RewriteResult::default(); + rewrite_yarn_berry(&two, std::slice::from_ref(&ovr), &mut r); + assert!(r.files.is_empty(), "{:?}", r.files); + assert!(r + .warnings + .iter() + .any(|w| w.code == "redirect_yarn_berry_ambiguous_entry")); + } + /// The URL-keyed lock entry alone is half a pin: with its manifest /// `resolutions` routing removed, yarn installs nothing from it, so the /// rewriter owns the dep yet never confirms it — a URL in the lock must 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 d32748ddb..688aa9947 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -886,6 +886,10 @@ async fn restore_berry( let dists = fetch_dists_on(&wanted, registry, ctx, result).await; let mut changed = false; let mut moved: Vec = Vec::new(); + let mut restored: Vec = Vec::new(); + // `(block index, warning)`: kept only for an entry that is not merged + // into a sibling below, whose own body yarn wrote. + let mut node_gyp_unresolved: Vec<(usize, String)> = Vec::new(); for Hit { idx, uuid, @@ -977,8 +981,8 @@ async fn restore_berry( if resolvable { lines = crate::formats::yarn::berry_entry::with_implicit_node_gyp(&lines); } else { - result.warnings.push(( - "yarn_berry_node_gyp_unresolved", + node_gyp_unresolved.push(( + idx, format!( "{name}@{version}: yarn gives the restored registry entry an \ implicit `node-gyp` dependency that {rel} no longer resolves; run \ @@ -1003,11 +1007,21 @@ async fn restore_berry( } } result.handled.insert(uuid); + restored.push(idx); changed = true; } if !changed { return; } + let merged = + crate::formats::yarn::stanzas::merge_same_resolution(&mut blocks, &restored, &mut moved); + for (idx, warning) in node_gyp_unresolved { + if !merged.contains(&idx) { + result + .warnings + .push(("yarn_berry_node_gyp_unresolved", warning)); + } + } if pkg_changed { if let (Some(text), Some(value)) = (pkg_text.as_deref(), pkg.as_mut()) { if let Some(obj) = value.as_object_mut() { diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index c01b074b7..9115351dc 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -762,6 +762,13 @@ pub async fn revert_yarn_berry_opts( match read_regular_to_string(&lock_path).await { Ok(mut text) => { let mut changed = false; + let restored_entries: Vec = lock_recs + .iter() + .filter(|rec| rec.kind == KIND_LOCK_ENTRY || rec.kind == KIND_LOCK_ENTRY_PRUNED) + .filter_map(|rec| rec.original.as_ref()) + .filter_map(super::yarn_classic_lock::json_to_lines) + .map(|lines| lines.join("\n")) + .collect(); for rec in lock_recs { if rec.kind == KIND_LOCK_ENTRY_PRUNED { changed |= restore_pruned_entry(&mut text, rec, &mut outcome.warnings); @@ -778,6 +785,22 @@ pub async fn revert_yarn_berry_opts( ); } if changed { + // The restored entries rejoin any registry entry added + // since with the same resolution (#1082). + let mut doc = BerryStanzas::parse(&text); + let restored: Vec = (0..doc.stanzas.len()) + .filter(|&i| restored_entries.contains(&doc.stanzas[i])) + .collect(); + let mut moved = Vec::new(); + if !crate::formats::yarn::stanzas::merge_same_resolution( + &mut doc.stanzas, + &restored, + &mut moved, + ) + .is_empty() + { + text = doc.render(&moved); + } forget_block_scans(); if let Err(e) = atomic_write_bytes_preserving_mode(&lock_path, text.as_bytes()).await @@ -2525,6 +2548,43 @@ __metadata: .exists()); } + /// #1082: an `npm:` alias of the vendored package added after + /// vendoring keeps its own registry entry. The revert puts the + /// pre-vendor entry back, which resolves to the same locator, so it is + /// merged into the alias entry the way yarn writes them; two entries + /// with one `resolution:` fail every `yarn install --immutable`. + #[tokio::test] + async fn revert_merges_into_an_alias_entry_added_after_vendoring() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let checksum = "3fb59c76e281a2f5c810ad71dbbb8eba8b10c6cf94733dc7f27b8c516a5376cacea53543e76f6ae477d866c8954b27f1e15ca349424c2542474eb5bb1d2b6955"; + let alias = format!( + "\"lp@npm:left-pad@1.3.0\":\n version: 1.3.0\n resolution: \"left-pad@npm:1.3.0\"\n \ + checksum: 10c0/{checksum}\n languageName: node\n linkType: hard\n\n" + ); + let vendored = tokio::fs::read_to_string(fx.lock_path()).await.unwrap(); + let with_alias = vendored.replacen( + "\"vendor-spike@workspace:.\":", + &format!("{alias}\"vendor-spike@workspace:.\":"), + 1, + ); + assert_ne!(with_alias, vendored); + tokio::fs::write(fx.lock_path(), &with_alias).await.unwrap(); + + let outcome = revert_yarn_berry(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert_eq!( + tokio::fs::read_to_string(fx.lock_path()).await.unwrap(), + B3_BEFORE_LOCK.replace( + "\"left-pad@npm:1.3.0\":", + "\"left-pad@npm:1.3.0, lp@npm:left-pad@1.3.0\":" + ), + "the restored entry and the alias entry are one entry" + ); + assert_eq!(tokio::fs::read(fx.pkg_path()).await.unwrap(), fx.pkg_bytes); + } + #[tokio::test] async fn revert_leaves_drifted_fragments_alone_with_warnings() { // Lock drift: the user re-resolved our entry back to the registry. diff --git a/docs/ecosystems.md b/docs/ecosystems.md index d79318605..9a00b75f3 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -248,7 +248,15 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: downloads the served tarball to read it. If that download fails, the patch is skipped as `npm_manifest_unavailable`. A user-authored `resolutions` entry for the package is never overwritten (`redirect_yarn_berry_resolutions_conflict`), and - `.yarnrc.yml`'s `compressionLevel` must stay 0. The node-modules linker + `.yarnrc.yml`'s `compressionLevel` must stay 0. A range of the package + added after the pin (say a workspace member's `"left-pad": "1.3.0"`) is not + routed by any selector, so yarn locks it as a separate registry entry and + installs that copy unpatched; re-running `scan --mode hosted` adds a selector + for it and folds its entry into the pinned one. `rollback` and `remove` merge + the restored registry entry into such an entry (or into an `npm:` alias entry + of the package), as yarn writes two entries with one resolution, so + `--immutable` installs keep passing; `vendor --revert` does the same for a + vendored package. The node-modules linker is e2e-covered; PnP is untested for hosted — the lock rewrite fires, but PnP's `.yarn/cache` resolution isn't exercised. CRLF locks — what yarn writes on Windows, and what a `core.autocrlf` checkout produces anywhere — are rewritten in their own